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

From: Jason Gunthorpe <jgg@nvidia.com>
To: "Tian, Kevin" <kevin.tian@intel.com>
Cc: Lu Baolu <baolu.lu@linux.intel.com>,
	"bpf@vger.kernel.org" <bpf@vger.kernel.org>,
	Jonathan Corbet <corbet@lwn.net>,
	David Woodhouse <dwmw2@infradead.org>,
	"iommu@lists.linux.dev" <iommu@lists.linux.dev>,
	Joerg Roedel <joro@8bytes.org>,
	"linux-doc@vger.kernel.org" <linux-doc@vger.kernel.org>,
	"linux-kselftest@vger.kernel.org"
	<linux-kselftest@vger.kernel.org>,
	"llvm@lists.linux.dev" <llvm@lists.linux.dev>,
	Nathan Chancellor <nathan@kernel.org>,
	Nick Desaulniers <ndesaulniers@google.com>,
	Miguel Ojeda <ojeda@kernel.org>,
	Robin Murphy <robin.murphy@arm.com>,
	Shuah Khan <shuah@kernel.org>,
	Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>,
	Tom Rix <trix@redhat.com>, Will Deacon <will@kernel.org>,
	Alex Williamson <alex.williamson@redhat.com>,
	Chaitanya Kulkarni <chaitanyak@nvidia.com>,
	Cornelia Huck <cohuck@redhat.com>,
	Daniel Jordan <daniel.m.jordan@oracle.com>,
	David Gibson <david@gibson.dropbear.id.au>,
	Eric Auger <eric.auger@redhat.com>,
	Eric Farman <farman@linux.ibm.com>,
	Jason Wang <jasowang@redhat.com>,
	Jean-Philippe Brucker <jean-philippe@linaro.org>,
	"Martins, Joao" <joao.m.martins@oracle.com>,
	"kvm@vger.kernel.org" <kvm@vger.kernel.org>,
	Matthew Rosato <mjrosato@linux.ibm.com>,
	"Michael S. Tsirkin" <mst@redhat.com>,
	Nicolin Chen <nicolinc@nvidia.com>,
	Niklas Schnelle <schnelle@linux.ibm.com>,
	Shameerali Kolothum Thodi  <shameerali.kolothum.thodi@huawei.com>,
	"Liu, Yi L" <yi.l.liu@intel.com>,
	Keqian Zhu <zhukeqian1@huawei.com>
Subject: Re: [PATCH v3 14/15] iommufd: vfio container FD ioctl compatibility
Date: Mon, 7 Nov 2022 13:08:14 -0400	[thread overview]
Message-ID: <Y2k7fkxSzxTPTXkN@nvidia.com> (raw)
In-Reply-To: <BN9PR11MB52763ADFBACFE7AFE11F5F078C3A9@BN9PR11MB5276.namprd11.prod.outlook.com>

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.
	 */

> > +	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.

Jason