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 20:17:42 -0400	[thread overview]
Message-ID: <Y2mgJqz8fvm54C+f@nvidia.com> (raw)
In-Reply-To: <BN9PR11MB5276411B47AC310E928BCFDB8C3F9@BN9PR11MB5276.namprd11.prod.outlook.com>

On Tue, Nov 08, 2022 at 12:13:55AM +0000, Tian, Kevin wrote:
> > From: Jason Gunthorpe <jgg@nvidia.com>
> > Sent: Tuesday, November 8, 2022 8:10 AM
> > 
> > On Mon, Nov 07, 2022 at 11:53:24PM +0000, Tian, Kevin wrote:
> > > > 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?
> > 
> > Sure, if userspace does crazy things then userspace gets to keep all
> > the pieces - it doesn't harm the kernel.
> > 
> > The IOAS is bound during get_device, and that is one of the key
> > conceptual things we changed with iommufd.
> > 
> 
> what we changed is the timing of claiming DMA ownership (from
> SET_CONTAINER to GET_DEVICE_FD). this is fine.
> 
> But adding an interface to allow the conceptual 'container' tied to
> multiple IOAS's is kind of overengineering IMHO.

It is not engineered to do this, it just simply happens because we
don't have the concept of a 'container' at all - and the emulation of
that same concept is very simple and doesn't try to block cases that
vfio would have not permitted.

> yes no hard to the kernel but also no benefit to user. It's simpler
> to constrain the compat layer based on what vfio legacy has today
> and then position all new/fancy usages with the new cdev.

That isn't simpler at all, it is whole bunch more code to track these
things and decide when we are allowed to execute an ioctl that doesn't
even exist in the original interface in the first place. Right now we
don't even have APIs to trace when set_container/unset_container have
happened in vfio to even constrain this.

I don't see the issue, the ioctl that manipulates the compat ioas is
new, it is optional, and if userspace mis-uses it without understand
how it conceptually works then userspace just won't do what is
expected.

Jason