Re: [Qemu-devel] [PATCH v5 2/2] hw/vfio/common: Fail on VFIO/HW nested paging detection

[Alex Williamson]

On Thu, 29 Aug 2019 11:01:41 +0200
Eric Auger <address@hidden> wrote:

> As of today, VFIO only works along with vIOMMU supporting
> caching mode. The SMMUv3 does not support this mode and
> requires HW nested paging to work properly with VFIO.
> 
> So any attempt to run a VFIO device protected by such IOMMU
> would prevent the assigned device from working and at the
> moment the guest does not even boot as the default
> memory_region_iommu_replay() implementation attempts to
> translate the whole address space and completely stalls
> the guest.

Why doesn't this stall an x86 guest?

I'm a bit confused about what this provides versus the flag_changed
notifier looking for IOMMU_NOTIFIER_MAP, which AIUI is the common
deficiency between VT-d w/o caching-mode and SMMUv3 w/o nested mode.
The iommu notifier is registered prior to calling iommu_replay, so it
seems we already have an opportunity to do something there.  Help me
understand why this is needed.  Thanks,

Alex

> 
> So let's fail on that case.
> 
> Signed-off-by: Eric Auger <address@hidden>
> 
> ---
> 
> v3 -> v4:
> - use IOMMU_ATTR_HW_NESTED_PAGING
> - do not abort anymore but jump to fail
> ---
>  hw/vfio/common.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/hw/vfio/common.c b/hw/vfio/common.c
> index 3e03c495d8..e8c009d019 100644
> --- a/hw/vfio/common.c
> +++ b/hw/vfio/common.c
> @@ -606,9 +606,19 @@ static void vfio_listener_region_add(MemoryListener 
> *listener,
>      if (memory_region_is_iommu(section->mr)) {
>          VFIOGuestIOMMU *giommu;
>          IOMMUMemoryRegion *iommu_mr = IOMMU_MEMORY_REGION(section->mr);
> +        bool nested;
>          int iommu_idx;
>  
>          trace_vfio_listener_region_add_iommu(iova, end);
> +
> +        if (!memory_region_iommu_get_attr(iommu_mr,
> +                                          IOMMU_ATTR_NEED_HW_NESTED_PAGING,
> +                                          (void *)&nested) && nested) {
> +            error_report("VFIO/vIOMMU integration based on HW nested paging "
> +                         "is not yet supported");
> +            ret = -EINVAL;
> +            goto fail;
> +        }
>          /*
>           * FIXME: For VFIO iommu types which have KVM acceleration to
>           * avoid bouncing all map/unmaps through qemu this way, this