Il 14/05/2013 11:13, David Gibson ha scritto:
> This patch uses the new IOMMU notifiers to allow VFIO pass through devices
> to work with guest side IOMMUs, as long as the host-side VFIO iommu has
> sufficient capability and granularity to match the guest side. This works
> by tracking all map and unmap operations on the guest IOMMU using the
> notifiers, and mirroring them into VFIO.
>
> There are a number of FIXMEs, and the scheme involves rather more notifier
> structures than I'd like, but it shuold make for a reasonable proof of
> concept.
>
> Signed-off-by: David Gibson <da...@gibson.dropbear.id.au>
> ---
> hw/misc/vfio.c | 139
> ++++++++++++++++++++++++++++++++++++++++++++++++++------
> 1 file changed, 126 insertions(+), 13 deletions(-)
>
> diff --git a/hw/misc/vfio.c b/hw/misc/vfio.c
> index f4e3792..62a83ca 100644
> --- a/hw/misc/vfio.c
> +++ b/hw/misc/vfio.c
> @@ -133,10 +133,18 @@ typedef struct VFIOContainer {
> };
> void (*release)(struct VFIOContainer *);
> } iommu_data;
> + QLIST_HEAD(, VFIOGuestIOMMU) guest_iommus;
> QLIST_HEAD(, VFIOGroup) group_list;
> QLIST_ENTRY(VFIOContainer) next;
> } VFIOContainer;
>
> +typedef struct VFIOGuestIOMMU {
> + VFIOContainer *container;
> + MemoryRegion *iommu;
> + Notifier n;
> + QLIST_ENTRY(VFIOGuestIOMMU) list;
> +} VFIOGuestIOMMU;
> +
> /* Cache of MSI-X setup plus extra mmap and memory region for split BAR map
> */
> typedef struct VFIOMSIXInfo {
> uint8_t table_bar;
> @@ -1940,7 +1948,64 @@ static int vfio_dma_map(VFIOContainer *container,
> hwaddr iova,
>
> static bool vfio_listener_skipped_section(MemoryRegionSection *section)
> {
> - return !memory_region_is_ram(section->mr);
> + return !memory_region_is_ram(section->mr) &&
> + !memory_region_is_iommu(section->mr);
> +}
> +
> +static void vfio_iommu_map_notify(Notifier *n, void *data)
> +{
> + VFIOGuestIOMMU *giommu = container_of(n, VFIOGuestIOMMU, n);
> + MemoryRegion *iommu = giommu->iommu;
> + VFIOContainer *container = giommu->container;
> + IOMMUTLBEntry *iotlb = data;
> + MemoryRegionSection *mrs;
> + hwaddr xlat;
> + hwaddr len = iotlb->addr_mask + 1;
> + void *vaddr;
> + int ret;
> +
> + DPRINTF("iommu map @ %"HWADDR_PRIx" - %"HWADDR_PRIx"\n",
> + iotlb->iova, iotlb->iova + iotlb->address_mask);
> +
> + /* The IOMMU TLB entry we have just covers translation through
> + * this IOMMU to its immediate target. We need to translate
> + * it the rest of the way through to memory. */
> + mrs = address_space_translate(iommu->iommu_target_as,
> + iotlb->translated_addr,
> + &xlat, &len, iotlb->perm[1]);
> + if (!memory_region_is_ram(mrs->mr)) {
> + DPRINTF("iommu map to non memory area %"HWADDR_PRIx"\n",
> + xlat);
> + return;
> + }
> + if (len & iotlb->addr_mask) {
> + DPRINTF("iommu has granularity incompatible with target AS\n");
> + return;
> + }
> +
> + vaddr = memory_region_get_ram_ptr(mrs->mr) +
> + mrs->offset_within_region +
> + (xlat - mrs->offset_within_address_space);
Are you sure you need this translation? It's done already in
address_space_translate, so it should be just
vaddr = memory_region_get_ram_ptr(mrs->mr) + xlat;
Paolo
> + if (iotlb->perm[0] || iotlb->perm[1]) {
> + ret = vfio_dma_map(container, iotlb->iova,
> + iotlb->addr_mask + 1, vaddr,
> + !iotlb->perm[1] || mrs->readonly);
> + if (ret) {
> + error_report("vfio_dma_map(%p, 0x%"HWADDR_PRIx", "
> + "0x%"HWADDR_PRIx", %p) = %d (%m)",
> + container, iotlb->iova,
> + iotlb->addr_mask + 1, vaddr, ret);
> + }
> + } else {
> + ret = vfio_dma_unmap(container, iotlb->iova, iotlb->addr_mask + 1);
> + if (ret) {
> + error_report("vfio_dma_unmap(%p, 0x%"HWADDR_PRIx", "
> + "0x%"HWADDR_PRIx") = %d (%m)",
> + container, iotlb->iova,
> + iotlb->addr_mask + 1, ret);
> + }
> + }
> }
>
> static void vfio_listener_region_add(MemoryListener *listener,
> @@ -1949,11 +2014,8 @@ static void vfio_listener_region_add(MemoryListener
> *listener,
> VFIOContainer *container = container_of(listener, VFIOContainer,
> iommu_data.listener);
> hwaddr iova, end;
> - void *vaddr;
> int ret;
>
> - assert(!memory_region_is_iommu(section->mr));
> -
> if (vfio_listener_skipped_section(section)) {
> DPRINTF("SKIPPING region_add %"HWADDR_PRIx" - %"PRIx64"\n",
> section->offset_within_address_space,
> @@ -1975,20 +2037,55 @@ static void vfio_listener_region_add(MemoryListener
> *listener,
> return;
> }
>
> - vaddr = memory_region_get_ram_ptr(section->mr) +
> + memory_region_ref(section->mr);
> +
> + if (memory_region_is_ram(section->mr)) {
> + void *vaddr;
> +
> + DPRINTF("region_add [ram] %"HWADDR_PRIx" - %"HWADDR_PRIx" [%p]\n",
> + iova, end - 1, vaddr);
> +
> + vaddr = memory_region_get_ram_ptr(section->mr) +
> section->offset_within_region +
> (iova - section->offset_within_address_space);
>
> - DPRINTF("region_add %"HWADDR_PRIx" - %"HWADDR_PRIx" [%p]\n",
> - iova, end - 1, vaddr);
>
> - memory_region_ref(section->mr);
> - ret = vfio_dma_map(container, iova, end - iova, vaddr,
> section->readonly);
> - if (ret) {
> - error_report("vfio_dma_map(%p, 0x%"HWADDR_PRIx", "
> - "0x%"HWADDR_PRIx", %p) = %d (%m)",
> - container, iova, end - iova, vaddr, ret);
> + ret = vfio_dma_map(container, iova, end - iova, vaddr,
> + section->readonly);
> + if (ret) {
> + error_report("vfio_dma_map(%p, 0x%"HWADDR_PRIx", "
> + "0x%"HWADDR_PRIx", %p) = %d (%m)",
> + container, iova, end - iova, vaddr, ret);
> + }
> + } else if (memory_region_is_iommu(section->mr)) {
> + VFIOGuestIOMMU *giommu;
> +
> + DPRINTF("region_add [iommu] %"HWADDR_PRIx" - %"HWADDR_PRIx"\n",
> + iova, end - 1);
> +
> + /*
> + * FIXME: We should do some checking to see if the
> + * capabilities of the host VFIO IOMMU are adequate to model
> + * the guest IOMMU
> + *
> + * FIXME: This assumes that the guest IOMMU is empty of
> + * mappings at this point - we should either enforce this, or
> + * loop through existing mappings to map them into VFIO.
> + *
> + * FIXME: For VFIO iommu types which have KVM acceleration to
> + * avoid bouncing all map/unmaps through qemu this way, this
> + * would be the right place to wire that up (tell the KVM
> + * device emulation the VFIO iommu handles to use).
> + */
> + giommu = g_malloc0(sizeof(*giommu));
> + giommu->iommu = section->mr;
> + giommu->container = container;
> + giommu->n.notify = vfio_iommu_map_notify;
> +
> + QLIST_INSERT_HEAD(&container->guest_iommus, giommu, list);
> + memory_region_register_iommu_notifier(giommu->iommu, &giommu->n);
> }
> +
> }
>
> static void vfio_listener_region_del(MemoryListener *listener,
> @@ -2012,6 +2109,22 @@ static void vfio_listener_region_del(MemoryListener
> *listener,
> return;
> }
>
> + if (memory_region_is_iommu(section->mr)) {
> + VFIOGuestIOMMU *giommu;
> +
> + QLIST_FOREACH(giommu, &container->guest_iommus, list) {
> + if (giommu->iommu == section->mr) {
> + memory_region_unregister_iommu_notifier(&giommu->n);
> + break;
> + }
> + }
> +
> + /* FIXME: We assume the one big unmap below is adequate to
> + * remove any individual page mappings in the IOMMU which
> + * might have been copied into VFIO. That may not be true for
> + * all IOMMU types */
> + }
> +
> iova = TARGET_PAGE_ALIGN(section->offset_within_address_space);
> end = (section->offset_within_address_space + section->size) &
> TARGET_PAGE_MASK;
>
