| Message ID | 20250206142307.921070-4-eric.auger@redhat.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Fix vIOMMU reset order | expand |
Hi Eric, >-----Original Message----- >From: Eric Auger <eric.auger@redhat.com> >Subject: [PATCH 3/5] hw/i386/intel_iommu: Tear down address spaces before >IOMMU reset > >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> >Signed-off-by: Eric Auger <eric.auger@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 21a8bf45f8..1bd9ae403b 100644 >--- a/hw/i386/intel_iommu.c >+++ b/hw/i386/intel_iommu.c >@@ -4702,8 +4702,8 @@ static void vtd_reset_exit(Object *obj, ResetType type) > IntelIOMMUState *s = INTEL_IOMMU_DEVICE(obj); > > trace_vtd_reset_exit(); >- vtd_init(s); > vtd_address_space_refresh_all(s); >+ vtd_init(s); I'm not sure if we should have this change. vtd_switch_address_space() checks s->dmar_enabled and vtd_init() updates s->dmar_enabled. With this change, will we leave stale mapping there after reset? Thanks Zhenzhong > } > > static AddressSpace *vtd_host_dma_iommu(PCIBus *bus, void *opaque, int >devfn) >-- >2.47.1
Hi Zhenzhong, On 2/17/25 4:02 AM, Duan, Zhenzhong wrote: > Hi Eric, > >> -----Original Message----- >> From: Eric Auger <eric.auger@redhat.com> >> Subject: [PATCH 3/5] hw/i386/intel_iommu: Tear down address spaces before >> IOMMU reset >> >> 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> >> Signed-off-by: Eric Auger <eric.auger@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 21a8bf45f8..1bd9ae403b 100644 >> --- a/hw/i386/intel_iommu.c >> +++ b/hw/i386/intel_iommu.c >> @@ -4702,8 +4702,8 @@ static void vtd_reset_exit(Object *obj, ResetType type) >> IntelIOMMUState *s = INTEL_IOMMU_DEVICE(obj); >> >> trace_vtd_reset_exit(); >> - vtd_init(s); >> vtd_address_space_refresh_all(s); >> + vtd_init(s); > I'm not sure if we should have this change. vtd_switch_address_space() checks s->dmar_enabled and vtd_init() updates s->dmar_enabled. With this change, will we leave stale mapping there after reset? Yes I do agree. This could break the as switch. I will remove this patch. Eric > > Thanks > Zhenzhong > >> } >> >> static AddressSpace *vtd_host_dma_iommu(PCIBus *bus, void *opaque, int >> devfn) >> -- >> 2.47.1
diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c index 21a8bf45f8..1bd9ae403b 100644 --- a/hw/i386/intel_iommu.c +++ b/hw/i386/intel_iommu.c @@ -4702,8 +4702,8 @@ static void vtd_reset_exit(Object *obj, ResetType type) IntelIOMMUState *s = INTEL_IOMMU_DEVICE(obj); trace_vtd_reset_exit(); - vtd_init(s); vtd_address_space_refresh_all(s); + vtd_init(s); } static AddressSpace *vtd_host_dma_iommu(PCIBus *bus, void *opaque, int devfn)