Messages in this thread | | | From | "Tian, Kevin" <> | Subject | RE: [PATCH v2 1/3] iommu/uapi: Define uapi version and capabilities | Date | Fri, 27 Mar 2020 02:49:55 +0000 |
| |
> From: Jacob Pan <jacob.jun.pan@linux.intel.com> > Sent: Friday, March 27, 2020 12:45 AM > > Hi Christoph, > > Thanks for the review. Please see my comments inline. > > On Thu, 26 Mar 2020 02:23:16 -0700 > Christoph Hellwig <hch@infradead.org> wrote: > > > On Wed, Mar 25, 2020 at 04:17:05PM -0700, Jacob Pan wrote: > > > Having a single UAPI version to govern the user-kernel data > > > structures makes compatibility check straightforward. On the > > > contrary, supporting combinations of multiple versions of the data > > > can be a nightmare to maintain. > > > > > > This patch defines a unified UAPI version to be used for > > > compatibility checks between user and kernel. > > > > This is bullshit. Version numbers don't scale and we've avoided them > > everywhere. You need need flags specifying specific behavior. > > > We have flags or the equivalent in each UAPI structures. The flags > are used for checking validity of extensions as well. And you are right > we can use them for checking specific behavior. > > So we have two options here: > 1. Have a overall version for a quick compatibility check while > starting a VM. If not compatible, we will stop guest SVA entirely. > > 2. Let each API calls check its own capabilities/flags at runtime. It > may fail later on and lead to more complex error handling. > For example, guest starts with SVA support, allocate a PASID, bind > guest PASID works great. Then when IO page fault happens, it try to do > page request service and found out certain flags are not compatible. > This case is more complex to handle.
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?
> > I am guessing your proposal is #2. right? > > Overall, we don;t expect much change to the UAPI other than adding some > vendor specific part of the union. Is the scalability concern based on > frequent changes? > > > > +#define IOMMU_UAPI_VERSION 1 > > > +static inline int iommu_get_uapi_version(void) > > > +{ > > > + return IOMMU_UAPI_VERSION; > > > +} > > > > Also inline functions like this in UAPI headers that actually get > > included by userspace programs simply don't work. > > I will remove that, user can just use IOMMU_UAPI_VERSION directly.
| |