RE: [PATCH v3 14/15] iommufd: vfio container FD ioctl compatibility

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

> From: Jason Gunthorpe <jgg@nvidia.com>
> Sent: Tuesday, November 8, 2022 1:08 AM
> 
> On Sat, Nov 05, 2022 at 09:31:39AM +0000, Tian, Kevin wrote:
> > > From: Jason Gunthorpe <jgg@nvidia.com>
> > > Sent: Wednesday, October 26, 2022 2:12 AM
> > >
> > > +int iommufd_vfio_compat_ioas_id(struct iommufd_ctx *ictx, u32
> > > *out_ioas_id)
> > > +{
> > > +	struct iommufd_ioas *ioas = NULL;
> > > +	struct iommufd_ioas *out_ioas;
> > > +
> > > +	ioas = iommufd_ioas_alloc(ictx);
> > > +	if (IS_ERR(ioas))
> > > +		return PTR_ERR(ioas);
> >
> > I tried to find out where the auto-created compat_ioas is destroyed.
> >
> > Is my understanding correct that nobody holds a long-term users
> > count on it then we expect it to be destroyed in iommufd release?
> 
> This is creating a userspace owned ID, like every other IOAS.
> 
> Userspace can obtain the ID using IOMMU_VFIO_IOAS GET and destroy it,
> if it wants. We keep track in a later hunk:
> 
> +	if (ictx->vfio_ioas && &ictx->vfio_ioas->obj == obj)
> +		ictx->vfio_ioas = NULL;
> 
> As with all userspace owned IDs they are always freed during iommufd
> release.
> 
> So, a comment is:
> 
> 	/*
> 	 * An automatically created compat IOAS is treated as a userspace
> 	 * created object. Userspace can learn the ID via
> IOMMU_VFIO_IOAS_GET,
> 	 * and if not manually destroyed it will be destroyed automatically
> 	 * at iommufd release.
> 	 */

this is clear

> 
> > > +	case IOMMU_VFIO_IOAS_SET:
> > > +		ioas = iommufd_get_ioas(ucmd, cmd->ioas_id);
> > > +		if (IS_ERR(ioas))
> > > +			return PTR_ERR(ioas);
> > > +		xa_lock(&ucmd->ictx->objects);
> > > +		ucmd->ictx->vfio_ioas = ioas;
> > > +		xa_unlock(&ucmd->ictx->objects);
> > > +		iommufd_put_object(&ioas->obj);
> > > +		return 0;
> >
> > disallow changing vfio_ioas when it's already in-use e.g. has
> > a list of hwpt attached?
> 
> I don't see a reason to do so..
> 
> The semantic we have is the IOAS, whatever it is, is fixed once the
> device or access object is created. In VFIO sense that means it
> becomes locked to the IOAS that was set as compat when the vfio device
> is bound.
> 
> Other than that, userspace can change the IOAS it wants freely, there
> is no harm to the kernel and it may even be useful.
> 

it allows devices SET_CONTAINER to an same iommufd attached to different
IOAS's if IOAS_SET comes in the middle. Is it desired?

while it's flexible I don't see a real usage would use it. Instead it causes
conceptual confusion to the original vfio semantics.