> From: Jason Gunthorpe <jgg@xxxxxxxxxx>
> Sent: Thursday, March 2, 2023 1:47 AM
>
> On Wed, Mar 01, 2023 at 09:19:07AM +0000, Liu, Yi L wrote:
> > > From: Liu, Yi L <yi.l.liu@xxxxxxxxx>
> > > Sent: Monday, February 27, 2023 7:12 PM
> > [...]
> > > +long vfio_device_ioctl_bind_iommufd(struct vfio_device_file *df,
> > > + unsigned long arg)
> > > +{
> > > + struct vfio_device *device = df->device;
> > > + struct vfio_device_bind_iommufd bind;
> > > + struct iommufd_ctx *iommufd = NULL;
> > > + unsigned long minsz;
> > > + int ret;
> > > +
> > > + minsz = offsetofend(struct vfio_device_bind_iommufd, out_devid);
> > > +
> > > + if (copy_from_user(&bind, (void __user *)arg, minsz))
> > > + return -EFAULT;
> > > +
> > > + if (bind.argsz < minsz || bind.flags)
> > > + return -EINVAL;
> > > +
> > > + if (!device->ops->bind_iommufd)
> > > + return -ENODEV;
> >
> > Hi Jason,
> >
> > Per the comment in vfio_iommufd_bind(), such device driver
> > won't provide .bind_iommufd(). So shall we allow this ioctl
> > to go longer to call .open_device() instead of failing it here?
> > I think we need to allow it to go further. E.g. leave the check
> > to be in vfio_iommufd_bind(). Otherwise, user may not able
> > to use such devices. Is it?
>
> You are thinking about the crazy mdev samples?
Yes. we don't have real devices which don't do DMA. Is it?
> We should probably just change them to provide a 'no dma' set of ops.
Yes. at least generate iommufd_device I suppose.
> > > +struct vfio_device_bind_iommufd {
> > > + __u32 argsz;
> > > + __u32 flags;
> > > + __aligned_u64 dev_cookie;
> > > + __s32 iommufd;
> > > + __u32 out_devid;
> >
> > As above, for the devices that do not do DMA, there is no .bind_iommufd
> > op, hence no iommufd_device generated. This means no good value
> > can be filled in this out_devid field. So this field is optional. Only
> > for the devices which do DMA, should this out_devid field return a
> > valid ID otherwise an invalid ID would be filled (e.g. value #0 is an
> > invalid value in the iommufd object id pool). Userspace needs to
> > check if the out_devid is valid or not before use. This ID can be further
> > used in iommufd uAPIs like IOMMU_HWPT_ALLOC,
> IOMMU_DEVICE_GET_INFO
> > and etc.
>
> I would say create an access and harmonize the no-DMA devices with the
> emulated devices.
In this case, iommufd_access would be created instead of iommufd_device.
> What should we return here anyhow if an access was created?
It depends on what can be done with this id and whether this field is mandatory.
For iommufd_device ID, the user could further use it to query iommu device info and
alloc hwpt. Do we have a similar usage for iommufd_access? And if we define this
field as optional, then we may return iommufd_access object Id in future if it is
needed.
Regards,
Yi Liu
