Subject: Re: [PATCH v5 26/29] vfio: powerpc/spapr: Define v2 IOMMU

From: Alex Williamson <[email-masked]>
Date: Tue, 10 Mar 2015 18:00:17 -0600

On Tue, 2015-03-10 at 01:07 +1100, Alexey Kardashevskiy wrote:
> The existing IOMMU code takes/releases ownership over the existing IOMMU
> tables created by the platform code, i.e. the tables remain in memory
> all the time. Also, the existing IOMMU requires VFIO_IOMMU_ENABLE call to
> start working as that's where we check the rlimit for locked pages.
> 
> New IOMMU will take over the IOMMU group completely which means the IOMMU
> tables created by the platform code are going to be disposed and VFIO
> will create its own tables. Also, with the DMA memory pre-registration
> feature, the userspace will not need to call VFIO_IOMMU_ENABLE as
> the locked pages accounting will be done by
> VFIO_IOMMU_SPAPR_REGISTER_MEMORY.
> 
> In order to inform the userspace that VFIO supports new capabilities,
> this adds a new SPAPR TCE IOMMU v2 type.

Shouldn't we block some of these new features on v1?  Otherwise v1 now
supports them too and by the property of "we don't break userspace" v1
and v2 are now one-in-the-same.  You'd need to support everything
through both versions.  If that's what you intend to do, maybe this
should be a capability rather than an IOMMU type.

> Signed-off-by: Alexey Kardashevskiy <[email-masked]>
> ---
>  drivers/vfio/vfio_iommu_spapr_tce.c | 3 ++-
>  include/uapi/linux/vfio.h           | 2 ++
>  2 files changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/vfio/vfio_iommu_spapr_tce.c b/drivers/vfio/vfio_iommu_spapr_tce.c
> index e572c28..d665ddc 100644
> --- a/drivers/vfio/vfio_iommu_spapr_tce.c
> +++ b/drivers/vfio/vfio_iommu_spapr_tce.c
> @@ -404,7 +404,7 @@ static void *tce_iommu_open(unsigned long arg)
>  {
>  	struct tce_container *container;
>  
> -	if (arg != VFIO_SPAPR_TCE_IOMMU) {
> +	if ((arg != VFIO_SPAPR_TCE_IOMMU) && (arg != VFIO_SPAPR_TCE_v2_IOMMU)) {
>  		pr_err("tce_vfio: Wrong IOMMU type\n");
>  		return ERR_PTR(-EINVAL);
>  	}
> @@ -588,6 +588,7 @@ static long tce_iommu_ioctl(void *iommu_data,
>  	case VFIO_CHECK_EXTENSION:
>  		switch (arg) {
>  		case VFIO_SPAPR_TCE_IOMMU:
> +		case VFIO_SPAPR_TCE_v2_IOMMU:
>  			ret = 1;
>  			break;
>  		default:
> diff --git a/include/uapi/linux/vfio.h b/include/uapi/linux/vfio.h
> index b17e120..fbc5286 100644
> --- a/include/uapi/linux/vfio.h
> +++ b/include/uapi/linux/vfio.h
> @@ -36,6 +36,8 @@
>  /* Two-stage IOMMU */
>  #define VFIO_TYPE1_NESTING_IOMMU	6	/* Implies v2 */
>  
> +#define VFIO_SPAPR_TCE_v2_IOMMU		7
> +
>  /*
>   * The IOCTL interface is designed for extensibility by embedding the
>   * structure length (argsz) and flags into structures passed between