> From: Jason Gunthorpe <jgg@xxxxxxxxxx>
> Sent: Tuesday, March 21, 2023 8:01 PM
>
> On Tue, Mar 21, 2023 at 01:30:34AM +0000, Tian, Kevin wrote:
> > > 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.
>
> This isn't the same thing, this will trigger for mdevs and stuff that
> should not be noiommu as well.
But the group path does disallow noiommu usage if the device has
a real iommu_group (the one created by VFIO code is not real). Would
it be better to keep it consistent from this angle?
> If you want to copy what the group code does then noiommu needs to be
> statically determined at physical vfio device allocation time.
There is another reason which may not that strong. For devices protected
by iommu, user needs to program IOVA mappings in order to do DMA. Such
device has a real iommu_group. So if we allow using noiommu mode for such
devices, DMA would be blocked by iommu. Perhaps users that use noiommu
mode should not do DMA at the first place.
Regards,
Yi Liu
