> From: Jason Gunthorpe <jgg@xxxxxxxxxx>
> Sent: Tuesday, March 21, 2023 1:17 AM
>
> On Mon, Mar 20, 2023 at 10:31:53PM +0800, Yi Liu wrote:
> > On 2023/3/20 22:09, Jason Gunthorpe wrote:
> > > On Wed, Mar 15, 2023 at 04:40:19AM +0000, Liu, Yi L wrote:
> > >
> > > > # if IS_ENABLED(CONFIG_VFIO_GROUP)
> > > > static inline bool vfio_device_is_noiommu(struct vfio_device *vdev)
> > > > {
> > > > return IS_ENABLED(CONFIG_VFIO_NOIOMMU) &&
> > > > vdev->group->type == VFIO_NO_IOMMU;
> > > > }
> > > > #else
> > > > static inline bool vfio_device_is_noiommu(struct vfio_device *vdev)
> > > > {
> > > > struct iommu_group *iommu_group;
> > > >
> > > > if (!IS_ENABLED(CONFIG_VFIO_NOIOMMU) || !vfio_noiommu)
> > > > return -EINVAL;
> > > >
> > > > iommu_group = iommu_group_get(vdev->dev);
> > > > if (iommu_group)
> > > > iommu_group_put(iommu_group);
> > > >
> > > > return !iommu_group;
> > >
> > > If we don't have VFIO_GROUP then no-iommu is signaled by a NULL
> > > iommu_ctx pointer in the vdev, don't mess with groups
> >
> > yes, NULL iommufd_ctx pointer would be set in vdev and passed to the
> > vfio_device_open(). But here, we want to use this helper to check if
> > user can use noiommu mode. This is before calling vfio_device_open().
> > e.g. if the device is protected by iommu, then user cannot use noiommu
> > mode on it.
>
> Why not allow it?
>
> If the admin has enabled this mode we may as well let it be used.
>
> You explicitly ask for no-iommu mode by passing -1 for the iommufd
> parameter. If the module parameter says it is allowed then that is all
> you need.
>
IMHO we should disallow noiommu on a device which already has
a iommu group. This is how noiommu works with vfio group. I don't
think it's a good idea to further relax it in cdev.
