[Intel-gfx] [PATCH v12 21/24] vfio: Determine noiommu device in __vfio_register_dev()

[Liu, Yi L]

> From: Alex Williamson <alex.williamson at redhat.com>
> Sent: Tuesday, June 13, 2023 10:48 PM
> 
> On Tue, 13 Jun 2023 14:33:01 +0000
> "Liu, Yi L" <yi.l.liu at intel.com> wrote:
> 
> > > From: Alex Williamson <alex.williamson at redhat.com>
> > > Sent: Tuesday, June 13, 2023 10:19 PM
> > >
> > > On Tue, 13 Jun 2023 05:53:42 +0000
> > > "Liu, Yi L" <yi.l.liu at intel.com> wrote:
> > >
> > > > > From: Alex Williamson <alex.williamson at redhat.com>
> > > > > Sent: Tuesday, June 13, 2023 6:42 AM
> > > > >
> > > > > On Fri,  2 Jun 2023 05:16:50 -0700
> > > > > Yi Liu <yi.l.liu at intel.com> wrote:
> > > > >
> > > > > > This moves the noiommu device determination and noiommu taint out of
> > > > > > vfio_group_find_or_alloc(). noiommu device is determined in
> > > > > > __vfio_register_dev() and result is stored in flag vfio_device->noiommu,
> > > > > > the noiommu taint is added in the end of __vfio_register_dev().
> > > > > >
> > > > > > This is also a preparation for compiling out vfio_group infrastructure
> > > > > > as it makes the noiommu detection and taint common between the cdev path
> > > > > > and group path though cdev path does not support noiommu.
> > > > >
> > > > > Does this really still make sense?  The motivation for the change is
> > > > > really not clear without cdev support for noiommu.  Thanks,
> > > >
> > > > I think it still makes sense. When CONFIG_VFIO_GROUP==n, the kernel
> > > > only supports cdev interface. If there is noiommu device, vfio should
> > > > fail the registration. So, the noiommu determination is still needed. But
> > > > I'd admit the taint might still be in the group code.
> > >
> > > How is there going to be a noiommu device when VFIO_GROUP is unset?
> >
> > How about booting a kernel with iommu disabled, then all the devices
> > are not protected by iommu. I suppose they are noiommu devices. If
> > user wants to bound them to vfio, the kernel should have VFIO_GROUP.
> > Otherwise, needs to fail.
> 
> "noiommu" is a vfio designation of a device, it must be created by
> vfio.  

Sure.

> There can certainly be devices which are not IOMMU backed, but
> without vfio designating them as noiommu devices, which is only done
> via the legacy and compat paths, there's no such thing as a noiommu
> device. 

Yes.

> Devices without an IOMMU are simply out of scope for cdev,
> there should never be a vfio cdev entry created for them.  Thanks,

Actually, this is what I want to solve. I need to check if a device is
IOMMU backed or not, and based on this info to prevent creating
cdev entry for them in the coming cdev support or may need to
fail registration if VFIO_GROUP is unset.

If this patch is not good. I can use the vfio_device_is_noiommu()
written like below when VFIO_GROUP is unset. What about your
opinion?

static inline bool vfio_device_is_noiommu(struct vfio_device *vdev)
{
	struct iommu_group *iommu_group;

	iommu_group = iommu_group_get(vdev->dev);
	iommu_group_put(iommu_group); /* Accepts NULL */
	return !iommu_group;
}

Regards,
Yi Liu