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?
We should probably just change them to provide a 'no dma' set of ops.
> > +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.
What should we return here anyhow if an access was created?
Jason
