> From: Liu, Yi L <yi.l.liu@xxxxxxxxx> > Sent: Saturday, March 11, 2023 6:24 PM > > > > > > > > - ret = vdev->ops->bind_iommufd(vdev, ictx, &device_id); > > > > - if (ret) > > > > - return ret; > > > > + /* The legacy path has no way to return the device id */ > > > > + return vdev->ops->bind_iommufd(vdev, ictx, &device_id); > > > > +} > > > > > > > > - ret = iommufd_vfio_compat_ioas_get_id(ictx, &ioas_id); > > > > - if (ret) > > > > - goto err_unbind; > > > > - ret = vdev->ops->attach_ioas(vdev, &ioas_id); > > > > - if (ret) > > > > - goto err_unbind; > > > > > > after noiommu check and attach_ioas are moved out then this > > > entire function can be removed now. Just call the ops in > > > vfio_device_first_open(). > > > > Yes. and also no vfio_iommufd_unbind(). > > Seems still necessary to have this wrapper. .bind_iommufd callback would > be NULL if CONFIG_IOMMUFD==n. If we call ops->bind_iommufd directly > in vfio_device_first_open() of vfio_main.c, it may trigger kernel panic > for NULL pointer dereference if there is wrong code that passes valid > iommufd pointer.. Ideally, if CONFIG_IOMMUFD==n, vfio_device_first_open > should not receive valid iommufd pointer hence won't call ops- > >bind_iommufd > at all. So it deserves a panic. However, if we have a wrapper for it, such code > may just fail with -EOPNOTSUPPT. > ok, let's keep this wrapper then. I didn't realize it's NULL if CONFIG_IOMMUFD==n.
