Re: [PATCH 1/8] iommu: Decouple iommu_present() from bus ops

From: Robin Murphy
Date: Fri Jan 27 2023 - 10:20:12 EST


On 2023-01-27 13:50, Baolu Lu wrote:
On 2023/1/26 22:41, Jason Gunthorpe wrote:
On Thu, Jan 26, 2023 at 02:21:29PM +0000, Robin Murphy wrote:

The "check" is inherent in the fact that it's been called at all. VFIO
noiommu*is*  an IOMMU driver in the sense that it provides a bare minimum of
IOMMU API functionality (i.e. creating groups), sufficient to support
(careful) usage by VFIO drivers. There would not seem to be a legitimate
reason for some*other*  driver to be specifically querying a device while it
is already bound to a VFIO driver (and thus may have a noiommu group).
Yes, the devices that VFIO assigns to its internal groups never leak
outside VFIO's control during their assignment - ie they are
continuously bound to VFIO never another driver.

So no other driver can ever see the internal groups unless it is
messing around with devices it is not bound to 😄

Fair enough. I was thinking that probably we could make it like below:

/**
 * device_iommu_mapped - Returns true when the device DMA is translated
 *                       by an IOMMU
 * @dev: Device to perform the check on
 */
static inline bool device_iommu_mapped(struct device *dev)
{
        return (dev->iommu && dev->iommu->iommu_dev);
}

The iommu probe device code guarantees that dev->iommu->iommu_dev is
valid only after the IOMMU driver's .probe_device returned successfully.

Any thoughts?

Heh, I actually wrote that helper yesterday for v2, but as an internal check for valid ops :)

The current implementation of device_iommu_mapped() just dates back to when dev->iommu_group was the only per-device thing we had, so in principle I don't have any conceptual objection to redefining it in terms of "device has ops" rather than "device has a group", but as things stand you'd still have to do something about PPC first (I know Jason had been pushing on that, but I've not kept track of where it got to).

Thanks,
Robin.