RE: [PATCH v2 1/3] iommu/uapi: Define uapi version and capabilities

From: "Tian, Kevin" <kevin.tian@intel.com>
To: Jacob Pan <jacob.jun.pan@linux.intel.com>,
	Christoph Hellwig <hch@infradead.org>
Cc: Joerg Roedel <joro@8bytes.org>,
	Alex Williamson <alex.williamson@redhat.com>,
	Lu Baolu <baolu.lu@linux.intel.com>,
	"iommu@lists.linux-foundation.org"
	<iommu@lists.linux-foundation.org>,
	LKML <linux-kernel@vger.kernel.org>,
	David Woodhouse <dwmw2@infradead.org>,
	Jean-Philippe Brucker <jean-philippe@linaro.com>,
	"Raj, Ashok" <ashok.raj@intel.com>
Subject: RE: [PATCH v2 1/3] iommu/uapi: Define uapi version and capabilities
Date: Mon, 30 Mar 2020 05:40:40 +0000	[thread overview]
Message-ID: <AADFC41AFE54684AB9EE6CBC0274A5D19D7FE150@SHSMSX104.ccr.corp.intel.com> (raw)
In-Reply-To: <20200327165335.397f24a3@jacob-builder>

> From: Jacob Pan <jacob.jun.pan@linux.intel.com>
> Sent: Saturday, March 28, 2020 7:54 AM
> 
> On Fri, 27 Mar 2020 00:47:02 -0700
> Christoph Hellwig <hch@infradead.org> wrote:
> 
> > On Fri, Mar 27, 2020 at 02:49:55AM +0000, Tian, Kevin wrote:
> > > If those API calls are inter-dependent for composing a feature
> > > (e.g. SVA), shouldn't we need a way to check them together before
> > > exposing the feature to the guest, e.g. through a
> > > iommu_get_uapi_capabilities interface?
> >
> > Yes, that makes sense.  The important bit is to have a capability
> > flags and not version numbers.
> 
> The challenge is that there are two consumers in the kernel for this.
> 1. VFIO only look for compatibility, and size of each data struct such
> that it can copy_from_user.
> 
> 2. IOMMU driver, the "real consumer" of the content.
> 
> For 2, I agree and we do plan to use the capability flags to check
> content and maintain backward compatibility etc.
> 
> For VFIO, it is difficult to do size look up based on capability flags.
> 

Can you elaborate the difficulty in VFIO? if, as Christoph Hellwig
pointed out, version number is already avoided everywhere, it is 
interesting to know whether this work becomes a real exception
or just requires a different mindset.

btw the most relevant discussion which I can find out now is here:
	https://lkml.org/lkml/2020/2/3/1126

It mentioned 3 options for handling extension:
--
1. Disallow adding new members to each structure other than reuse
padding bits or adding union members at the end.
2. Allow extension of the structures beyond union, but union size has
to be fixed with reserved spaces
3. Adopt VFIO argsz scheme, I don't think we need version for each
struct anymore. argsz implies the version that user is using assuming
UAPI data is extension only.
--

the first two are both version-based. Looks most guys agreed with 
option-1 (in this v2), but Alex didn't give his opinion at the moment. 
The last response from him was the raise of option-3 using argsz to 
avoid version. So, we also need hear from him. Alex?

Thanks
Kevin