Messages in this thread | | | Date | Wed, 1 Feb 2023 13:18:03 -0800 | From | Nicolin Chen <> | Subject | Re: [PATCH v2 1/3] iommufd: Add devices_users to track the hw_pagetable usage by device |
| |
On Wed, Feb 01, 2023 at 04:00:40PM -0400, Jason Gunthorpe wrote: > On Wed, Feb 01, 2023 at 11:25:10AM -0800, Nicolin Chen wrote: > > > The "finalise" is one of the very first lines of the attach_dev() > > callback function in SMMU driver, though it might still undesirably > > fail the replace(). > > It won't get that far. > > Remember how this all works - only autodomains have the special path > that allocates a domain, attaches the empty domain, and then populates > it with the ioas. We made this special path specifically to accomodate > the current ARM drivers, otherwise they wouldn't work at all.
Yes.
> replace can't do this - replace must always start out with a > pre-existing hwpt that was allocated with a dedicated hwpt allocation > ioctl. > > Wwhen the hwpt was allocated it must be linked to the IOAS at that > time, because we definately don't do defered IOAS linkage. > > So on ARM when you create an unfinalizes iommu_domain it cannot be > added to the IOAS (because it has an empty aperture) and creation will > fail, or worse, it will get added to an empty IOAS and make the IOAS > permanently unusable.
IIUIC, user space might add some IOVA mappings to the hwpt/iopt, before doing a replace(). If we do a deferred IOAS linkage to this hwpt/iopt, it will cause a problem because we are adding the reserved region for the MSI window later than IOMMU_IOAS_MAP calls. Thus, "we definately don't do defered IOAS linkage".
With this justification, I think I should include my patch of moving iopt_table_add/remove_domain(), in the replace series.
> > Hmm...in that case, we should hold two ioas->mutex locks in > > addition to two device locks? > > No, the device lock is the thing that protects the data you are > touching no reason to make it any different.
Thinking it deeper: we don't really touch the old_hwpt->ioas or its iopt, but only the new_hwpt->ioas/iopt to reserve the MSI window, so there isn't an actual need to hold old_hwpt->ioas's mutex.
I will prepare the replace series and send it by the end of the day, upon certain level of sanity/verification.
Thanks! Nic
| |