RE: [PATCH v2 07/11] vfio-iommufd: Support iommufd for physical VFIO devices

["Tian, Kevin" <kevin.tian@intel.com>]

> From: Jason Gunthorpe <jgg@nvidia.com>
> Sent: Friday, November 11, 2022 1:21 AM
> 
> On Thu, Nov 10, 2022 at 03:11:16AM +0000, Tian, Kevin wrote:
> > > From: Jason Gunthorpe <jgg@nvidia.com>
> > > 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@nvidia.com/
> >
> > 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.
> 

OK, this is a valid point.

So a legacy vfio application doesn't use IOAS_SET so the backward
compatibility is guaranteed.

a iommufd native application will use cdev where IOAS_SET and
compat ioas are irrelevant.

here just we allow an interesting usage where an user is allowed
to do more funny things with IOAS_SET on vfio-compat. Not sure
how useful it is but not something we want to prohibit.