> From: Liu, Yi L <yi.l....@intel.com> > Sent: Friday, March 10, 2023 4:22 PM > > > > > > +int vfio_iommufd_attach_compat_ioas(struct vfio_device *vdev, > > > + struct iommufd_ctx *ictx) > > > +{ > > > + u32 ioas_id; > > > + int ret; > > > + > > > + lockdep_assert_held(&vdev->dev_set->lock); > > > > > > /* > > > - * The legacy path has no way to return the device id or the selected > > > - * pt_id > > > + * If the driver doesn't provide this op then it means the device does > > > + * not do DMA at all. So nothing to do. > > > */ > > > - return 0; > > > + if (WARN_ON(!vdev->ops->bind_iommufd)) > > > + return -ENODEV; > > > > > > -err_unbind: > > > - if (vdev->ops->unbind_iommufd) > > > - vdev->ops->unbind_iommufd(vdev); > > > - return ret; > > > + if (vfio_device_is_noiommu(vdev)) { > > > + if > > > (WARN_ON(vfio_iommufd_device_probe_comapt_noiommu(vdev, ictx))) > > > + return -EINVAL; > > > + return 0; > > > + } > > > > no need. let's directly call following from vfio_device_group_open(). > > In that case no need to do noiommu check twice in one function. > > Ok. maybe still have vfio_iommufd_attach_compat_ioas() but > only call it if it's not noiommu mode. vfio_device_group_open() > can call probe_noiommu() first and has a bool to mark noiommu. > Jason had a remark that it's better to keep the > iommufd_vfio_compat_ioas_get_id() in iommufd.c >
Probably that remark doesn't hold now if we agree to remove vfio_iommufd_bind() and let vfio_device_group_open() directly call .bind_iommufd(). also group.c already calls other compat API: if (IS_ENABLED(CONFIG_VFIO_NOIOMMU) && group->type == VFIO_NO_IOMMU) ret = iommufd_vfio_compat_set_no_iommu(iommufd); else ret = iommufd_vfio_compat_ioas_create(iommufd);