> From: Tian, Kevin <kevin.tian@xxxxxxxxx> > Sent: Friday, January 20, 2023 4:03 PM > > > From: Liu, Yi L <yi.l.liu@xxxxxxxxx> > > Sent: Tuesday, January 17, 2023 9:50 PM > > > > This adds two vfio device ioctls for userspace using iommufd on vfio > > devices. > > > > VFIO_DEVICE_BIND_IOMMUFD: bind device to an iommufd, hence gain > > DMA > > control provided by the iommufd. VFIO no > > iommu is indicated by passing a minus > > fd value. > > Can't this be a flag bit for better readability than using a special value? > > > VFIO_DEVICE_ATTACH_IOMMUFD_PT: attach device to ioas, page > tables > > managed by iommufd. Attach can be > > undo by passing IOMMUFD_INVALID_ID > > to kernel. > > With Alex' remark we need a separate DETACH cmd now. Yes. > > > > + /* > > + * For group path, iommufd pointer is NULL when comes into this > > + * helper. Its noiommu support is in container.c. > > + * > > + * For iommufd compat mode, iommufd pointer here is a valid value. > > + * Its noiommu support is supposed to be in vfio_iommufd_bind(). > > + * > > + * For device cdev path, iommufd pointer here is a valid value for > > + * normal cases, but it is NULL if it's noiommu. The reason is > > + * that userspace uses iommufd==-1 to indicate noiommu mode in > > this > > + * path. So caller of this helper will pass in a NULL iommufd > > + * pointer. To differentiate it from the group path which also > > + * passes NULL iommufd pointer in, df->noiommu is used. For cdev > > + * noiommu, df->noiommu would be set to mark noiommu case for > > cdev > > + * path. > > + * > > + * So if df->noiommu is set then this helper just goes ahead to > > + * open device. If not, it depends on if iommufd pointer is NULL > > + * to handle the group path, iommufd compat mode, normal cases > in > > + * the cdev path. > > + */ > > if (iommufd) > > ret = vfio_iommufd_bind(device, iommufd, dev_id, pt_id); > > - else > > + else if (!df->noiommu) > > ret = vfio_device_group_use_iommu(device); > > if (ret) > > goto err_module_put; > > Isn't 'ret' uninitialized when df->noiommu is true? Done. > > +static int vfio_ioctl_device_attach(struct vfio_device *device, > > + struct vfio_device_feature __user *arg) > > +{ > > + struct vfio_device_attach_iommufd_pt attach; > > + int ret; > > + bool is_attach; > > + > > + if (copy_from_user(&attach, (void __user *)arg, sizeof(attach))) > > + return -EFAULT; > > + > > + if (attach.flags) > > + return -EINVAL; > > + > > + if (!device->ops->bind_iommufd) > > + return -ENODEV; > > + > > this should fail if noiommu is true. Yes. Regards, Yi Liu