Re: [PATCH v2 03/22] vfio: check VFIO_TYPE1_NESTING_IOMMU support

[Auger Eric]

Yi,

On 3/30/20 6:24 AM, Liu Yi L wrote:
> VFIO needs to check VFIO_TYPE1_NESTING_IOMMU support with Kernel before
> further using it. e.g. requires to check IOMMU UAPI version.
> 
> Cc: Kevin Tian <address@hidden>
> Cc: Jacob Pan <address@hidden>
> Cc: Peter Xu <address@hidden>
> Cc: Eric Auger <address@hidden>
> Cc: Yi Sun <address@hidden>
> Cc: David Gibson <address@hidden>
> Cc: Alex Williamson <address@hidden>
> Signed-off-by: Liu Yi L <address@hidden>
> Signed-off-by: Yi Sun <address@hidden>
> ---
>  hw/vfio/common.c | 14 ++++++++++++--
>  1 file changed, 12 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/vfio/common.c b/hw/vfio/common.c
> index 0b3593b..c276732 100644
> --- a/hw/vfio/common.c
> +++ b/hw/vfio/common.c
> @@ -1157,12 +1157,21 @@ static void vfio_put_address_space(VFIOAddressSpace 
> *space)
>  static int vfio_get_iommu_type(VFIOContainer *container,
>                                 Error **errp)
>  {
> -    int iommu_types[] = { VFIO_TYPE1v2_IOMMU, VFIO_TYPE1_IOMMU,
> +    int iommu_types[] = { VFIO_TYPE1_NESTING_IOMMU,
> +                          VFIO_TYPE1v2_IOMMU, VFIO_TYPE1_IOMMU,
>                            VFIO_SPAPR_TCE_v2_IOMMU, VFIO_SPAPR_TCE_IOMMU };
> -    int i;
> +    int i, version;
>  
>      for (i = 0; i < ARRAY_SIZE(iommu_types); i++) {
>          if (ioctl(container->fd, VFIO_CHECK_EXTENSION, iommu_types[i])) {
> +            if (iommu_types[i] == VFIO_TYPE1_NESTING_IOMMU) {
> +                version = ioctl(container->fd, VFIO_CHECK_EXTENSION,
> +                                VFIO_NESTING_IOMMU_UAPI);
> +                if (version < IOMMU_UAPI_VERSION) {
> +                    info_report("IOMMU UAPI incompatible for nesting");
> +                    continue;
> +                }
> +            }
This means that by default VFIO_TYPE1_NESTING_IOMMU wwould be chosen. I
don't think this what we want. On ARM this would mean that for a
standard VFIO assignment without vIOMMU, SL will be used instead of FL.
This may not be harmless.

For instance, in "[RFC v6 09/24] vfio: Force nested if iommu requires
it", I use nested only if I detect we have a vSMMU. Otherwise I keep the
legacy VFIO_TYPE1v2_IOMMU.

Thanks

Eric
>              return iommu_types[i];
>          }
>      }
> @@ -1278,6 +1287,7 @@ static int vfio_connect_container(VFIOGroup *group, 
> AddressSpace *as,
>      }
>  
>      switch (container->iommu_type) {
> +    case VFIO_TYPE1_NESTING_IOMMU:
>      case VFIO_TYPE1v2_IOMMU:
>      case VFIO_TYPE1_IOMMU:
>      {
>