On Fri, Nov 11, 2022 at 12:12:36PM +0800, Yi Liu wrote: > > +int vfio_iommufd_bind(struct vfio_device *vdev, struct iommufd_ctx *ictx) > > +{ > > + u32 ioas_id; > > + u32 device_id; > > + int ret; > > + > > + lockdep_assert_held(&vdev->dev_set->lock); > > + > > + /* > > + * If the driver doesn't provide this op then it means the device does > > + * not do DMA at all. So nothing to do. > > + */ > > + if (!vdev->ops->bind_iommufd) > > + return 0; > > + > > + ret = vdev->ops->bind_iommufd(vdev, ictx, &device_id); > > + if (ret) > > + return ret; > > + > > + ret = iommufd_vfio_compat_ioas_id(ictx, &ioas_id); > > + if (ret) > > + goto err_unbind; > > + ret = vdev->ops->attach_ioas(vdev, &ioas_id); > > + if (ret) > > + goto err_unbind; > > + vdev->iommufd_attached = true; > > it's better to set this bool in vfio_iommufd_physical_attach_ioas() as > the emulated devices uses iommufd_access instead. is it? or you mean this > flag to cover both cases? Yes, that is probably clearer: @@ -50,7 +50,6 @@ int vfio_iommufd_bind(struct vfio_device *vdev, struct iommufd_ctx *ictx) ret = vdev->ops->attach_ioas(vdev, &ioas_id); if (ret) goto err_unbind; - vdev->iommufd_attached = true; /* * The legacy path has no way to return the device id or the selected @@ -110,10 +109,15 @@ EXPORT_SYMBOL_GPL(vfio_iommufd_physical_unbind); int vfio_iommufd_physical_attach_ioas(struct vfio_device *vdev, u32 *pt_id) { unsigned int flags = 0; + int rc; if (vfio_allow_unsafe_interrupts) flags |= IOMMUFD_ATTACH_FLAGS_ALLOW_UNSAFE_INTERRUPT; - return iommufd_device_attach(vdev->iommufd_device, pt_id, flags); + rc = iommufd_device_attach(vdev->iommufd_device, pt_id, flags); + if (rc) + return rc; + vdev->iommufd_attached = true; + return 0; } EXPORT_SYMBOL_GPL(vfio_iommufd_physical_attach_ioas); Thanks, Jason