> From: Jason Gunthorpe <jgg@xxxxxxxxxx>
> 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@xxxxxxxxxx>
> > > 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.
