On 24.02.2021 10:43, Julien Grall wrote: > From: Julien Grall <jgr...@amazon.com> > > The new x86 IOMMU page-tables allocator will release the pages when > relinquishing the domain resources. However, this is not sufficient > when the domain is dying because nothing prevents page-table to be > allocated. > > As the domain is dying, it is not necessary to continue to modify the > IOMMU page-tables as they are going to be destroyed soon. > > At the moment, page-table allocates will only happen when iommu_map(). > So after this change there will be no more page-table allocation > happening.
While I'm still not happy about this asymmetry, I'm willing to accept it in the interest of getting the underlying issue addressed. May I ask though that you add something like "... because we don't use superpage mappings yet when not sharing page tables"? But there are two more minor things: > --- a/xen/drivers/passthrough/x86/iommu.c > +++ b/xen/drivers/passthrough/x86/iommu.c > @@ -267,6 +267,12 @@ int iommu_free_pgtables(struct domain *d) > struct page_info *pg; > unsigned int done = 0; > > + if ( !is_iommu_enabled(d) ) > + return 0; Why is this addition needed? Hitting a not yet initialize spin lock is - afaict - no worse than a not yet initialized list, so it would seem to me that this can't be the reason. No other reason looks to be called out by the description. > + /* After this barrier, no more IOMMU mapping can happen */ > + spin_barrier(&hd->arch.mapping_lock); On the v3 discussion I thought you did agree to change the wording of the comment to something like "no new IOMMU mappings can be inserted"? Jan
