On Thu, Nov 10, 2022 at 03:11:16AM +0000, Tian, Kevin wrote:
> > From: Jason Gunthorpe <jgg@xxxxxxxxxx>
> > Sent: Tuesday, November 8, 2022 8:53 AM
> >
> > +
> > +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;
>
> with our discussion in v1:
>
> https://lore.kernel.org/all/Y2mgJqz8fvm54C+f@xxxxxxxxxx/
>
> I got the rationale on iommufd part which doesn't have the concept
> of container hence not necessarily to impose restriction on when
> an user can change a compat ioas.
>
> But from vfio side I wonder whether we should cache the compat
> ioas id when it's attached by the first device and then use it all the
> way for other device attachments coming after. implying IOAS_SET
> only affects containers which haven't been attached.
I can't see a reason to do this. IOAS_SET is a new ioctl and it has
new semantics beyond what original vfio container could do. In this
case having an impact on the next vfio_device that is opened.
This seems generally useful enough I wouldn't want to block it.
In any case, we can't *really* change this because the vfio layer is
working on IDs and the IDs can be destroyed/recreated from under
it. So if we try to hold the ID we could still end up getting it
changed anyhow.
Jason
