On Tue, 13 Jun 2023 15:01:35 +0000
"Liu, Yi L" <yi.l.liu@xxxxxxxxx> wrote:
> > From: Alex Williamson <alex.williamson@xxxxxxxxxx>
> > Sent: Tuesday, June 13, 2023 10:48 PM
> >
> > On Tue, 13 Jun 2023 14:33:01 +0000
> > "Liu, Yi L" <yi.l.liu@xxxxxxxxx> wrote:
> >
> > > > From: Alex Williamson <alex.williamson@xxxxxxxxxx>
> > > > Sent: Tuesday, June 13, 2023 10:19 PM
> > > >
> > > > On Tue, 13 Jun 2023 05:53:42 +0000
> > > > "Liu, Yi L" <yi.l.liu@xxxxxxxxx> wrote:
> > > >
> > > > > > From: Alex Williamson <alex.williamson@xxxxxxxxxx>
> > > > > > Sent: Tuesday, June 13, 2023 6:42 AM
> > > > > >
> > > > > > On Fri, 2 Jun 2023 05:16:50 -0700
> > > > > > Yi Liu <yi.l.liu@xxxxxxxxx> 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;
> }
No, please do not confuse the issue. As we agreed above "noiommu"
means a specific thing, it's a device without IOMMU backing that vfio
has artificially included in the environment. If we don't have
VFIO_NOIOMMU then there's no such thing as a "noiommu" device.
You can certainly use an iommu_group test to decide if a device should
be represented, but there absolutely should never be a vfio_device
created without IOMMU backing and without VFIO_NOIOMMU. Thanks,
Alex
