On Wed, 2013-08-07 at 18:21 +1000, Alexey Kardashevskiy wrote: > 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 | 149 > +++++++++++++++++++++++++++++++++++++++++++++++++-------- > 1 file changed, 129 insertions(+), 20 deletions(-) > > diff --git a/hw/misc/vfio.c b/hw/misc/vfio.c > index e1ee56e..3855efe 100644 > --- a/hw/misc/vfio.c > +++ b/hw/misc/vfio.c > @@ -150,10 +150,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;
Within vfio the convention is that an ENTRY is next or foo_next and a HEAD is foo_list. So perhaps giommu_list and next? > +} VFIOGuestIOMMU; > + > /* Cache of MSI-X setup plus extra mmap and memory region for split BAR map > */ > typedef struct VFIOMSIXInfo { > uint8_t table_bar; > @@ -1917,19 +1925,70 @@ 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_listener_region_add(MemoryListener *listener, > - MemoryRegionSection *section) > +static void vfio_iommu_map_notify(Notifier *n, void *data) > { > - VFIOContainer *container = container_of(listener, VFIOContainer, > - iommu_data.listener); > - hwaddr iova, end; > + VFIOGuestIOMMU *giommu = container_of(n, VFIOGuestIOMMU, n); > + VFIOContainer *container = giommu->container; > + IOMMUTLBEntry *iotlb = data; > + MemoryRegion *mr; > + hwaddr xlat; > + hwaddr len = iotlb->addr_mask + 1; > void *vaddr; > int ret; > > - assert(!memory_region_is_iommu(section->mr)); Yay! > + 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. */ /* * Multi-line comment this way please */ > + mr = address_space_translate(&address_space_memory, > + iotlb->translated_addr, > + &xlat, &len, iotlb->perm & IOMMU_WO); > + if (!memory_region_is_ram(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(mr) + xlat; > + > + if (iotlb->perm != IOMMU_NONE) { > + ret = vfio_dma_map(container, iotlb->iova, > + iotlb->addr_mask + 1, vaddr, > + !(iotlb->perm & IOMMU_WO) || mr->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, > + MemoryRegionSection *section) > +{ > + VFIOContainer *container = container_of(listener, VFIOContainer, > + iommu_data.listener); > + hwaddr iova, end; > + int ret; > > if (vfio_listener_skipped_section(section)) { > DPRINTF("SKIPPING region_add %"HWADDR_PRIx" - %"PRIx64"\n", > @@ -1952,19 +2011,51 @@ static void vfio_listener_region_add(MemoryListener > *listener, > return; > } > > - 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); > + if (memory_region_is_iommu(section->mr)) { > + VFIOGuestIOMMU *giommu; > + > + DPRINTF("region_add [iommu] %"HWADDR_PRIx" - %"HWADDR_PRIx"\n", > + iova, end - 1); > + > + memory_region_ref(section->mr); > + /* > + * 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); > + > + } else if (memory_region_is_ram(section->mr)) { > + void *vaddr; > + > + vaddr = memory_region_get_ram_ptr(section->mr) + > + section->offset_within_region + > + (iova - section->offset_within_address_space); > + > + DPRINTF("region_add [ram] %"HWADDR_PRIx" - %"HWADDR_PRIx" [%p]\n", > + iova, end - 1, vaddr); > + > + memory_region_ref(section->mr); Where are these unref'd? > + 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); > + } > } > } > > @@ -1989,6 +2080,24 @@ 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); > + QLIST_REMOVE(giommu, list); > + g_free(giommu); > + 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 + > int128_get64(section->size)) & > TARGET_PAGE_MASK;