Re: [RFC v7 12/26] vfio: Set up nested stage mappings

[Zenghui Yu]

Hi Eric,

On 2020/11/17 2:13, Eric Auger wrote:
> In nested mode, legacy vfio_iommu_map_notify cannot be used as
> there is no "caching" mode and we do not trap on map.
>
> On Intel, vfio_iommu_map_notify was used to DMA map the RAM
> through the host single stage.
>
> With nested mode, we need to setup the stage 2 and the stage 1
> separately. This patch introduces a prereg_listener to setup
> the stage 2 mapping.
>
> The stage 1 mapping, owned by the guest, is passed to the host
> when the guest invalidates the stage 1 configuration, through
> a dedicated PCIPASIDOps callback. Guest IOTLB invalidations
> are cascaded downto the host through another IOMMU MR UNMAP
> notifier.
>
> Signed-off-by: Eric Auger <eric.auger@redhat.com>
>
> ---
>
> v6 -> v7:
> - remove PASID based invalidation
>
> v5 -> v6:
> - add error_report_err()
> - remove the abort in case of nested stage case
>
> v4 -> v5:
> - use VFIO_IOMMU_SET_PASID_TABLE
> - use PCIPASIDOps for config notification
>
> v3 -> v4:
> - use iommu_inv_pasid_info for ASID invalidation
>
> v2 -> v3:
> - use VFIO_IOMMU_ATTACH_PASID_TABLE
> - new user API
> - handle leaf
>
> v1 -> v2:
> - adapt to uapi changes
> - pass the asid
> - pass IOMMU_NOTIFIER_S1_CFG when initializing the config notifier
> ---
>   hw/vfio/common.c     | 111 +++++++++++++++++++++++++++++++++++++++++--
>   hw/vfio/pci.c        |  21 ++++++++
>   hw/vfio/trace-events |   2 +
>   3 files changed, 129 insertions(+), 5 deletions(-)
>
> diff --git a/hw/vfio/common.c b/hw/vfio/common.c
> index a9e3c77863..6b3bba7ff9 100644
> --- a/hw/vfio/common.c
> +++ b/hw/vfio/common.c
> @@ -597,6 +597,45 @@ static bool vfio_get_xlat_addr(IOMMUTLBEntry *iotlb, void 
> **vaddr,
>       return true;
>   }
>   
> +/* Propagate a guest IOTLB invalidation to the host (nested mode) */
> +static void vfio_iommu_unmap_notify(IOMMUNotifier *n, IOMMUTLBEntry *iotlb)
> +{
> +    VFIOGuestIOMMU *giommu = container_of(n, VFIOGuestIOMMU, n);
> +    hwaddr start = iotlb->iova + giommu->iommu_offset;
> +    VFIOContainer *container = giommu->container;
> +    struct vfio_iommu_type1_cache_invalidate ustruct = {};
> +    struct iommu_inv_addr_info *addr_info = &ustruct.info.granu.addr_info;
> +    size_t size = iotlb->addr_mask + 1;
> +    int ret;
> +
> +    assert(iotlb->perm == IOMMU_NONE);
> +
> +    ustruct.argsz = sizeof(ustruct);
> +    ustruct.flags = 0;
> +    ustruct.info.argsz = sizeof(struct iommu_cache_invalidate_info);
> +    ustruct.info.version = IOMMU_CACHE_INVALIDATE_INFO_VERSION_1;
> +
> +    ustruct.info.cache = IOMMU_CACHE_INV_TYPE_IOTLB;
> +    ustruct.info.granularity = IOMMU_INV_GRANU_ADDR;

It seems that all guest IOTLB invalidation will be propagated to host
using the ADDR granularity. I'm afraid that TLBI_{NS}NH_ALL command
emulating will be a time consuming process...

> +    addr_info->flags = IOMMU_INV_ADDR_FLAGS_ARCHID;
> +    if (iotlb->leaf) {
> +        addr_info->flags |= IOMMU_INV_ADDR_FLAGS_LEAF;
> +    }
> +    addr_info->archid = iotlb->arch_id;

...and looks like both @arch_id and @leaf are not properly set in that
case. The same for TLBI_NH_ASID command.

(Or maybe I've missed something in the vsmmu code?)


Thanks,
Zenghui

> +    addr_info->addr = start;
> +    addr_info->granule_size = size;
> +    addr_info->nb_granules = 1;
> +    trace_vfio_iommu_addr_inv_iotlb(iotlb->arch_id, start, size, 1,
> +                                    iotlb->leaf);
> +
> +    ret = ioctl(container->fd, VFIO_IOMMU_CACHE_INVALIDATE, &ustruct);
> +    if (ret) {
> +        error_report("%p: failed to invalidate CACHE for 0x%"PRIx64
> +                     " mask=0x%"PRIx64" (%d)",
> +                     container, start, iotlb->addr_mask, ret);
> +    }
> +}