On Fri, Jun 24, 2022 at 06:51:44PM +0100, Robin Murphy wrote: > Since IOMMU groups are mandatory for drivers to support, it stands to > reason that any device which has been successfully added to a group > must be on a bus supported by that IOMMU driver, and therefore a domain > viable for any device in the group must be viable for all devices in > the group. This already has to be the case for the IOMMU API's internal > default domain, for instance. Thus even if the group contains devices on > different buses, that can only mean that the IOMMU driver actually > supports such an odd topology, and so without loss of generality we can > expect the bus type of any device in a group to be suitable for IOMMU > API calls. > > Furthermore, scrutiny reveals a lack of protection for the bus being > removed while vfio_iommu_type1_attach_group() is using it; the reference > that VFIO holds on the iommu_group ensures that data remains valid, but > does not prevent the group's membership changing underfoot. > > We can address both concerns by recycling vfio_bus_type() into some > superficially similar logic to indirect the IOMMU API calls themselves. > Each call is thus protected from races by the IOMMU group's own locking, > and we no longer need to hold group-derived pointers beyond that scope. > It also gives us an easy path for the IOMMU API's migration of bus-based > interfaces to device-based, of which we can already take the first step > with device_iommu_capable(). As with domains, any capability must in > practice be consistent for devices in a given group - and after all it's > still the same capability which was expected to be consistent across an > entire bus! - so there's no need for any complicated validation. > > Signed-off-by: Robin Murphy <robin.murphy@xxxxxxx> > --- > > v3: Complete rewrite yet again, and finally it doesn't feel like we're > stretching any abstraction boundaries the wrong way, and the diffstat > looks right too. I did think about embedding IOMMU_CAP_INTR_REMAP > directly in the callback, but decided I like the consistency of minimal > generic wrappers. And yes, if the capability isn't supported then it > does end up getting tested for the whole group, but meh, it's harmless. I think this is really weird, but it works and isn't worth debating further, so OK: Reviewed-by: Jason Gunthorpe <jgg@xxxxxxxxxx> Jason
