diff mbox series

[RFC,v3,4/8] vfio/type1: Add VFIO_NESTING_GET_IOMMU_UAPI_VERSION

Message ID 1580299912-86084-5-git-send-email-yi.l.liu@intel.com (mailing list archive)
State New, archived
Headers show
Series vfio: expose virtual Shared Virtual Addressing to VMs | expand

Commit Message

Yi Liu Jan. 29, 2020, 12:11 p.m. UTC 
From: Liu Yi L <yi.l.liu@intel.com>

In Linux Kernel, the IOMMU nesting translation (a.k.a. IOMMU dual stage
translation capability) is abstracted in uapi/iommu.h, in which the uAPIs
like bind_gpasid/iommu_cache_invalidate/fault_report/pgreq_resp are defined.

VFIO_TYPE1_NESTING_IOMMU stands for the vfio iommu type which is backed by
IOMMU nesting translation capability. VFIO exposes the nesting capability
to userspace and also exposes uAPIs (will be added in later patches) to user
space for setting up nesting translation from userspace. Thus applications
like QEMU could support vIOMMU for pass-through devices with IOMMU nesting
translation capability.

As VFIO expose the nesting IOMMU programming to userspace, it also needs to
provide an API for the uapi/iommu.h version check to ensure compatibility.
This patch reports the iommu uapi version to userspace. Applications could
use this API to do version check before further using the nesting uAPIs.

Cc: Kevin Tian <kevin.tian@intel.com>
CC: Jacob Pan <jacob.jun.pan@linux.intel.com>
Cc: Alex Williamson <alex.williamson@redhat.com>
Cc: Eric Auger <eric.auger@redhat.com>
Cc: Jean-Philippe Brucker <jean-philippe.brucker@arm.com>
Signed-off-by: Liu Yi L <yi.l.liu@intel.com>
---
 drivers/vfio/vfio.c       |  3 +++
 include/uapi/linux/vfio.h | 10 ++++++++++
 2 files changed, 13 insertions(+)

Comments

Alex Williamson Jan. 29, 2020, 11:56 p.m. UTC | #1 
On Wed, 29 Jan 2020 04:11:48 -0800
"Liu, Yi L" <yi.l.liu@intel.com> wrote:

> From: Liu Yi L <yi.l.liu@intel.com>
> 
> In Linux Kernel, the IOMMU nesting translation (a.k.a. IOMMU dual stage
> translation capability) is abstracted in uapi/iommu.h, in which the uAPIs
> like bind_gpasid/iommu_cache_invalidate/fault_report/pgreq_resp are defined.
> 
> VFIO_TYPE1_NESTING_IOMMU stands for the vfio iommu type which is backed by
> IOMMU nesting translation capability. VFIO exposes the nesting capability
> to userspace and also exposes uAPIs (will be added in later patches) to user
> space for setting up nesting translation from userspace. Thus applications
> like QEMU could support vIOMMU for pass-through devices with IOMMU nesting
> translation capability.
> 
> As VFIO expose the nesting IOMMU programming to userspace, it also needs to
> provide an API for the uapi/iommu.h version check to ensure compatibility.
> This patch reports the iommu uapi version to userspace. Applications could
> use this API to do version check before further using the nesting uAPIs.
> 
> Cc: Kevin Tian <kevin.tian@intel.com>
> CC: Jacob Pan <jacob.jun.pan@linux.intel.com>
> Cc: Alex Williamson <alex.williamson@redhat.com>
> Cc: Eric Auger <eric.auger@redhat.com>
> Cc: Jean-Philippe Brucker <jean-philippe.brucker@arm.com>
> Signed-off-by: Liu Yi L <yi.l.liu@intel.com>
> ---
>  drivers/vfio/vfio.c       |  3 +++
>  include/uapi/linux/vfio.h | 10 ++++++++++
>  2 files changed, 13 insertions(+)
> 
> diff --git a/drivers/vfio/vfio.c b/drivers/vfio/vfio.c
> index 425d60a..9087ad4 100644
> --- a/drivers/vfio/vfio.c
> +++ b/drivers/vfio/vfio.c
> @@ -1170,6 +1170,9 @@ static long vfio_fops_unl_ioctl(struct file *filep,
>  	case VFIO_GET_API_VERSION:
>  		ret = VFIO_API_VERSION;
>  		break;
> +	case VFIO_NESTING_GET_IOMMU_UAPI_VERSION:
> +		ret = iommu_get_uapi_version();
> +		break;

Shouldn't the type1 backend report this?  It doesn't make much sense
that the spapr backend reports a version for something it doesn't
support.  Better yet, provide this info gratuitously in the
VFIO_IOMMU_GET_INFO ioctl return like you do with nesting in the next
patch, then it can help the user figure out if this support is present.
Thanks,

Alex

>  	case VFIO_CHECK_EXTENSION:
>  		ret = vfio_ioctl_check_extension(container, arg);
>  		break;
> diff --git a/include/uapi/linux/vfio.h b/include/uapi/linux/vfio.h
> index d4bf415..62113be 100644
> --- a/include/uapi/linux/vfio.h
> +++ b/include/uapi/linux/vfio.h
> @@ -857,6 +857,16 @@ struct vfio_iommu_type1_pasid_quota {
>   */
>  #define VFIO_IOMMU_SET_PASID_QUOTA	_IO(VFIO_TYPE, VFIO_BASE + 23)
>  
> +/**
> + * VFIO_NESTING_GET_IOMMU_UAPI_VERSION - _IO(VFIO_TYPE, VFIO_BASE + 24)
> + *
> + * Report the version of the IOMMU UAPI when dual stage IOMMU is supported.
> + * In VFIO, it is needed for VFIO_TYPE1_NESTING_IOMMU.
> + * Availability: Always.
> + * Return: IOMMU UAPI version
> + */
> +#define VFIO_NESTING_GET_IOMMU_UAPI_VERSION	_IO(VFIO_TYPE, VFIO_BASE + 24)
> +
>  /* -------- Additional API for SPAPR TCE (Server POWERPC) IOMMU -------- */
>  
>  /*
Yi Liu Jan. 31, 2020, 1:04 p.m. UTC | #2 
Hi Alex,

> From: Alex Williamson [mailto:alex.williamson@redhat.com]
> Sent: Thursday, January 30, 2020 7:57 AM
> To: Liu, Yi L <yi.l.liu@intel.com>
> Subject: Re: [RFC v3 4/8] vfio/type1: Add
> VFIO_NESTING_GET_IOMMU_UAPI_VERSION
> 
> On Wed, 29 Jan 2020 04:11:48 -0800
> "Liu, Yi L" <yi.l.liu@intel.com> wrote:
> 
> > From: Liu Yi L <yi.l.liu@intel.com>
> >
> > In Linux Kernel, the IOMMU nesting translation (a.k.a. IOMMU dual stage
> > translation capability) is abstracted in uapi/iommu.h, in which the uAPIs
> > like bind_gpasid/iommu_cache_invalidate/fault_report/pgreq_resp are defined.
> >
> > VFIO_TYPE1_NESTING_IOMMU stands for the vfio iommu type which is backed by
> > IOMMU nesting translation capability. VFIO exposes the nesting capability
> > to userspace and also exposes uAPIs (will be added in later patches) to user
> > space for setting up nesting translation from userspace. Thus applications
> > like QEMU could support vIOMMU for pass-through devices with IOMMU nesting
> > translation capability.
> >
> > As VFIO expose the nesting IOMMU programming to userspace, it also needs to
> > provide an API for the uapi/iommu.h version check to ensure compatibility.
> > This patch reports the iommu uapi version to userspace. Applications could
> > use this API to do version check before further using the nesting uAPIs.
> >
> > Cc: Kevin Tian <kevin.tian@intel.com>
> > CC: Jacob Pan <jacob.jun.pan@linux.intel.com>
> > Cc: Alex Williamson <alex.williamson@redhat.com>
> > Cc: Eric Auger <eric.auger@redhat.com>
> > Cc: Jean-Philippe Brucker <jean-philippe.brucker@arm.com>
> > Signed-off-by: Liu Yi L <yi.l.liu@intel.com>
> > ---
> >  drivers/vfio/vfio.c       |  3 +++
> >  include/uapi/linux/vfio.h | 10 ++++++++++
> >  2 files changed, 13 insertions(+)
> >
> > diff --git a/drivers/vfio/vfio.c b/drivers/vfio/vfio.c
> > index 425d60a..9087ad4 100644
> > --- a/drivers/vfio/vfio.c
> > +++ b/drivers/vfio/vfio.c
> > @@ -1170,6 +1170,9 @@ static long vfio_fops_unl_ioctl(struct file *filep,
> >  	case VFIO_GET_API_VERSION:
> >  		ret = VFIO_API_VERSION;
> >  		break;
> > +	case VFIO_NESTING_GET_IOMMU_UAPI_VERSION:
> > +		ret = iommu_get_uapi_version();
> > +		break;
> 
> Shouldn't the type1 backend report this?  It doesn't make much sense
> that the spapr backend reports a version for something it doesn't
> support.  Better yet, provide this info gratuitously in the
> VFIO_IOMMU_GET_INFO ioctl return like you do with nesting in the next
> patch, then it can help the user figure out if this support is present.

yeah, it would be better to report it by type1 backed. However,
it is kind of issue when QEMU using it.

My series "hooks" vSVA supports on VFIO_TYPE1_NESTING_IOMMU type.
[RFC v3 09/25] vfio: check VFIO_TYPE1_NESTING_IOMMU support
https://www.spinics.net/lists/kvm/msg205197.html

In QEMU, it will determine the iommu type firstly and then invoke
VFIO_SET_IOMMU. I think before selecting VFIO_TYPE1_NESTING_IOMMU,
QEMU needs to check the IOMMU uAPI version. If IOMMU uAPI is incompatible,
QEMU should not use VFIO_TYPE1_NESTING_IOMMU type. If
VFIO_NESTING_GET_IOMMU_UAPI_VERSION is available after set iommu, then it
may be an issue. That's why this series reports the version in vfio layer
instead of type1 backend.

Regards,
Yi Liu
Alex Williamson Feb. 3, 2020, 6 p.m. UTC | #3 
On Fri, 31 Jan 2020 13:04:11 +0000
"Liu, Yi L" <yi.l.liu@intel.com> wrote:

> Hi Alex,
> 
> > From: Alex Williamson [mailto:alex.williamson@redhat.com]
> > Sent: Thursday, January 30, 2020 7:57 AM
> > To: Liu, Yi L <yi.l.liu@intel.com>
> > Subject: Re: [RFC v3 4/8] vfio/type1: Add
> > VFIO_NESTING_GET_IOMMU_UAPI_VERSION
> > 
> > On Wed, 29 Jan 2020 04:11:48 -0800
> > "Liu, Yi L" <yi.l.liu@intel.com> wrote:
> >   
> > > From: Liu Yi L <yi.l.liu@intel.com>
> > >
> > > In Linux Kernel, the IOMMU nesting translation (a.k.a. IOMMU dual stage
> > > translation capability) is abstracted in uapi/iommu.h, in which the uAPIs
> > > like bind_gpasid/iommu_cache_invalidate/fault_report/pgreq_resp are defined.
> > >
> > > VFIO_TYPE1_NESTING_IOMMU stands for the vfio iommu type which is backed by
> > > IOMMU nesting translation capability. VFIO exposes the nesting capability
> > > to userspace and also exposes uAPIs (will be added in later patches) to user
> > > space for setting up nesting translation from userspace. Thus applications
> > > like QEMU could support vIOMMU for pass-through devices with IOMMU nesting
> > > translation capability.
> > >
> > > As VFIO expose the nesting IOMMU programming to userspace, it also needs to
> > > provide an API for the uapi/iommu.h version check to ensure compatibility.
> > > This patch reports the iommu uapi version to userspace. Applications could
> > > use this API to do version check before further using the nesting uAPIs.
> > >
> > > Cc: Kevin Tian <kevin.tian@intel.com>
> > > CC: Jacob Pan <jacob.jun.pan@linux.intel.com>
> > > Cc: Alex Williamson <alex.williamson@redhat.com>
> > > Cc: Eric Auger <eric.auger@redhat.com>
> > > Cc: Jean-Philippe Brucker <jean-philippe.brucker@arm.com>
> > > Signed-off-by: Liu Yi L <yi.l.liu@intel.com>
> > > ---
> > >  drivers/vfio/vfio.c       |  3 +++
> > >  include/uapi/linux/vfio.h | 10 ++++++++++
> > >  2 files changed, 13 insertions(+)
> > >
> > > diff --git a/drivers/vfio/vfio.c b/drivers/vfio/vfio.c
> > > index 425d60a..9087ad4 100644
> > > --- a/drivers/vfio/vfio.c
> > > +++ b/drivers/vfio/vfio.c
> > > @@ -1170,6 +1170,9 @@ static long vfio_fops_unl_ioctl(struct file *filep,
> > >  	case VFIO_GET_API_VERSION:
> > >  		ret = VFIO_API_VERSION;
> > >  		break;
> > > +	case VFIO_NESTING_GET_IOMMU_UAPI_VERSION:
> > > +		ret = iommu_get_uapi_version();
> > > +		break;  
> > 
> > Shouldn't the type1 backend report this?  It doesn't make much sense
> > that the spapr backend reports a version for something it doesn't
> > support.  Better yet, provide this info gratuitously in the
> > VFIO_IOMMU_GET_INFO ioctl return like you do with nesting in the next
> > patch, then it can help the user figure out if this support is present.  
> 
> yeah, it would be better to report it by type1 backed. However,
> it is kind of issue when QEMU using it.
> 
> My series "hooks" vSVA supports on VFIO_TYPE1_NESTING_IOMMU type.
> [RFC v3 09/25] vfio: check VFIO_TYPE1_NESTING_IOMMU support
> https://www.spinics.net/lists/kvm/msg205197.html
> 
> In QEMU, it will determine the iommu type firstly and then invoke
> VFIO_SET_IOMMU. I think before selecting VFIO_TYPE1_NESTING_IOMMU,
> QEMU needs to check the IOMMU uAPI version. If IOMMU uAPI is incompatible,
> QEMU should not use VFIO_TYPE1_NESTING_IOMMU type. If
> VFIO_NESTING_GET_IOMMU_UAPI_VERSION is available after set iommu, then it
> may be an issue. That's why this series reports the version in vfio layer
> instead of type1 backend.

Why wouldn't you use CHECK_EXTENSION?  You could probe specifically for
a VFIO_TYP1_NESTING_IOMMU_UAPI_VERSION extension that returns the
version number.  Thanks,

Alex
Yi Liu Feb. 5, 2020, 6:19 a.m. UTC | #4 
> From: Alex Williamson [mailto:alex.williamson@redhat.com]
> Sent: Tuesday, February 4, 2020 2:01 AM
> To: Liu, Yi L <yi.l.liu@intel.com>
> Subject: Re: [RFC v3 4/8] vfio/type1: Add
> VFIO_NESTING_GET_IOMMU_UAPI_VERSION
> 
> On Fri, 31 Jan 2020 13:04:11 +0000
> "Liu, Yi L" <yi.l.liu@intel.com> wrote:
> 
> > Hi Alex,
> >
> > > From: Alex Williamson [mailto:alex.williamson@redhat.com]
> > > Sent: Thursday, January 30, 2020 7:57 AM
> > > To: Liu, Yi L <yi.l.liu@intel.com>
> > > Subject: Re: [RFC v3 4/8] vfio/type1: Add
> > > VFIO_NESTING_GET_IOMMU_UAPI_VERSION
> > >
> > > On Wed, 29 Jan 2020 04:11:48 -0800
> > > "Liu, Yi L" <yi.l.liu@intel.com> wrote:
> > >
> > > > From: Liu Yi L <yi.l.liu@intel.com>
> > > >
> > > > In Linux Kernel, the IOMMU nesting translation (a.k.a. IOMMU dual stage
> > > > translation capability) is abstracted in uapi/iommu.h, in which the uAPIs
> > > > like bind_gpasid/iommu_cache_invalidate/fault_report/pgreq_resp are defined.
> > > >
> > > > VFIO_TYPE1_NESTING_IOMMU stands for the vfio iommu type which is backed
> by
> > > > IOMMU nesting translation capability. VFIO exposes the nesting capability
> > > > to userspace and also exposes uAPIs (will be added in later patches) to user
> > > > space for setting up nesting translation from userspace. Thus applications
> > > > like QEMU could support vIOMMU for pass-through devices with IOMMU
> nesting
> > > > translation capability.
> > > >
> > > > As VFIO expose the nesting IOMMU programming to userspace, it also needs to
> > > > provide an API for the uapi/iommu.h version check to ensure compatibility.
> > > > This patch reports the iommu uapi version to userspace. Applications could
> > > > use this API to do version check before further using the nesting uAPIs.
> > > >
> > > > Cc: Kevin Tian <kevin.tian@intel.com>
> > > > CC: Jacob Pan <jacob.jun.pan@linux.intel.com>
> > > > Cc: Alex Williamson <alex.williamson@redhat.com>
> > > > Cc: Eric Auger <eric.auger@redhat.com>
> > > > Cc: Jean-Philippe Brucker <jean-philippe.brucker@arm.com>
> > > > Signed-off-by: Liu Yi L <yi.l.liu@intel.com>
> > > > ---
> > > >  drivers/vfio/vfio.c       |  3 +++
> > > >  include/uapi/linux/vfio.h | 10 ++++++++++
> > > >  2 files changed, 13 insertions(+)
> > > >
> > > > diff --git a/drivers/vfio/vfio.c b/drivers/vfio/vfio.c
> > > > index 425d60a..9087ad4 100644
> > > > --- a/drivers/vfio/vfio.c
> > > > +++ b/drivers/vfio/vfio.c
> > > > @@ -1170,6 +1170,9 @@ static long vfio_fops_unl_ioctl(struct file *filep,
> > > >  	case VFIO_GET_API_VERSION:
> > > >  		ret = VFIO_API_VERSION;
> > > >  		break;
> > > > +	case VFIO_NESTING_GET_IOMMU_UAPI_VERSION:
> > > > +		ret = iommu_get_uapi_version();
> > > > +		break;
> > >
> > > Shouldn't the type1 backend report this?  It doesn't make much sense
> > > that the spapr backend reports a version for something it doesn't
> > > support.  Better yet, provide this info gratuitously in the
> > > VFIO_IOMMU_GET_INFO ioctl return like you do with nesting in the next
> > > patch, then it can help the user figure out if this support is present.
> >
> > yeah, it would be better to report it by type1 backed. However,
> > it is kind of issue when QEMU using it.
> >
> > My series "hooks" vSVA supports on VFIO_TYPE1_NESTING_IOMMU type.
> > [RFC v3 09/25] vfio: check VFIO_TYPE1_NESTING_IOMMU support
> > https://www.spinics.net/lists/kvm/msg205197.html
> >
> > In QEMU, it will determine the iommu type firstly and then invoke
> > VFIO_SET_IOMMU. I think before selecting VFIO_TYPE1_NESTING_IOMMU,
> > QEMU needs to check the IOMMU uAPI version. If IOMMU uAPI is incompatible,
> > QEMU should not use VFIO_TYPE1_NESTING_IOMMU type. If
> > VFIO_NESTING_GET_IOMMU_UAPI_VERSION is available after set iommu, then it
> > may be an issue. That's why this series reports the version in vfio layer
> > instead of type1 backend.
> 
> Why wouldn't you use CHECK_EXTENSION?  You could probe specifically for
> a VFIO_TYP1_NESTING_IOMMU_UAPI_VERSION extension that returns the
> version number.  Thanks,

oh, yes. Thanks for this guiding. :-)

Regards,
Yi Liu
diff mbox series

Patch

diff --git a/drivers/vfio/vfio.c b/drivers/vfio/vfio.c
index 425d60a..9087ad4 100644
--- a/drivers/vfio/vfio.c
+++ b/drivers/vfio/vfio.c
@@ -1170,6 +1170,9 @@  static long vfio_fops_unl_ioctl(struct file *filep,
 	case VFIO_GET_API_VERSION:
 		ret = VFIO_API_VERSION;
 		break;
+	case VFIO_NESTING_GET_IOMMU_UAPI_VERSION:
+		ret = iommu_get_uapi_version();
+		break;
 	case VFIO_CHECK_EXTENSION:
 		ret = vfio_ioctl_check_extension(container, arg);
 		break;
diff --git a/include/uapi/linux/vfio.h b/include/uapi/linux/vfio.h
index d4bf415..62113be 100644
--- a/include/uapi/linux/vfio.h
+++ b/include/uapi/linux/vfio.h
@@ -857,6 +857,16 @@  struct vfio_iommu_type1_pasid_quota {
  */
 #define VFIO_IOMMU_SET_PASID_QUOTA	_IO(VFIO_TYPE, VFIO_BASE + 23)
 
+/**
+ * VFIO_NESTING_GET_IOMMU_UAPI_VERSION - _IO(VFIO_TYPE, VFIO_BASE + 24)
+ *
+ * Report the version of the IOMMU UAPI when dual stage IOMMU is supported.
+ * In VFIO, it is needed for VFIO_TYPE1_NESTING_IOMMU.
+ * Availability: Always.
+ * Return: IOMMU UAPI version
+ */
+#define VFIO_NESTING_GET_IOMMU_UAPI_VERSION	_IO(VFIO_TYPE, VFIO_BASE + 24)
+
 /* -------- Additional API for SPAPR TCE (Server POWERPC) IOMMU -------- */
 
 /*