Re: [Qemu-devel] [PATCH 5/8] vfio: Add guest side IOMMU support

[Paolo Bonzini]

Il 15/08/2013 08:02, Alexey Kardashevskiy ha scritto:
> On 08/13/2013 08:07 AM, Alex Williamson wrote:
>>> +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)) {

Please change this to an "else", and leave the refs outside the if, just
after checking for vfio_listener_skipped_section.  This way it's clearer
that vfio_listener_region_add matches vfio_listener_region_del.

Paolo