RE: [PATCH v6 13/24] vfio/iommufd: Split the compat_ioas attach out from vfio_iommufd_bind()

Tian, Kevin
2023-03-10T09:15:17Z

> From: Liu, Yi L <yi.l.liu@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);