| Message ID | 20240117091559.144730-4-peterx@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | intel_iommu: Reset vIOMMU after all the rest of devices | expand |
Hi Peter, On 1/17/24 10:15, peterx@redhat.com wrote: > From: Peter Xu <peterx@redhat.com> > > No bug report for this, but logically tearing down of existing address > space should happen before reset of IOMMU state / registers, because the > current address spaces may still rely on those information. do you mean that vtd_address_space_refresh_all() implementation my rely on data reset by vtd_init()? By the way the comment before the function is a bit confusing because it says that we should not reset as when reset but isn't it was it done here? Thanks Eric > > Signed-off-by: Peter Xu <peterx@redhat.com> > --- > hw/i386/intel_iommu.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c > index 1a07faddb4..8b467cbbd2 100644 > --- a/hw/i386/intel_iommu.c > +++ b/hw/i386/intel_iommu.c > @@ -4090,8 +4090,8 @@ static void vtd_reset(DeviceState *dev) > { > IntelIOMMUState *s = INTEL_IOMMU_DEVICE(dev); > > - vtd_init(s); > vtd_address_space_refresh_all(s); > + vtd_init(s); > } > > static AddressSpace *vtd_host_dma_iommu(PCIBus *bus, void *opaque, int devfn)
Hi Peter, On 17/1/24 10:15, peterx@redhat.com wrote: > From: Peter Xu <peterx@redhat.com> > > No bug report for this, but logically tearing down of existing address > space should happen before reset of IOMMU state / registers, because the > current address spaces may still rely on those information. > > Signed-off-by: Peter Xu <peterx@redhat.com> > --- > hw/i386/intel_iommu.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c > index 1a07faddb4..8b467cbbd2 100644 > --- a/hw/i386/intel_iommu.c > +++ b/hw/i386/intel_iommu.c > @@ -4090,8 +4090,8 @@ static void vtd_reset(DeviceState *dev) > { > IntelIOMMUState *s = INTEL_IOMMU_DEVICE(dev); > > - vtd_init(s); > vtd_address_space_refresh_all(s); > + vtd_init(s); > } You might want to convert to 3-phases reset API here, calling vtd_address_space_refresh_all() in a ResettableEnterPhase handler and vtd_init() in ResettableHoldPhase (or ResettableExitPhase?).
diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c index 1a07faddb4..8b467cbbd2 100644 --- a/hw/i386/intel_iommu.c +++ b/hw/i386/intel_iommu.c @@ -4090,8 +4090,8 @@ static void vtd_reset(DeviceState *dev) { IntelIOMMUState *s = INTEL_IOMMU_DEVICE(dev); - vtd_init(s); vtd_address_space_refresh_all(s); + vtd_init(s); } static AddressSpace *vtd_host_dma_iommu(PCIBus *bus, void *opaque, int devfn)