Re: [Qemu-devel] [PATCH qemu] memory: Fix IOMMU replay base address

[Alexey Kardashevskiy <aik@ozlabs.ru>]

On 02/23/2016 08:56 PM, Paolo Bonzini wrote:
>
>
> On 23/02/2016 10:00, Alexey Kardashevskiy wrote:
>>>>
>>>>            tce = tcet->table[addr >> tcet->page_shift];
>>>> -        ret.iova = addr & page_mask;
>>>> +        ret.iova = (addr + iommu->addr) & page_mask;
>>>>            ret.translated_addr = tce & page_mask;
>>>
>>> I wondered about that change, but I'd have to look closer to see if
>>> the iova field here is expected to be relative to the MR as well.  It
>>> would be oddly inconsistent if it wasn't.
>>
>> It is relative and it does not make sense as there is no source MR/AS in
>> iotlb (only target AS) so there is no use in such iova.
>
> ret.iova should be relative to the source AS (i.e. even if a 32-bit
> IOMMU region translates between 4GB and 8GB, ret.iova should have bits
> 32-63 set to 0).


In my test branch with 2 DMA windows I have such PHB AS:

address-space: pci@800000020000000
   0000000000000000-ffffffffffffffff (prio 0, RW): 
pci@800000020000000.iommu-root
     0000000000000000-ffffffffffffffff (prio 0, RW): tce-root-80000001
       0800000000000000-08000000ffffffff (prio 0, RW): tce-iommu-80000001
     0000000000000000-ffffffffffffffff (prio 0, RW): tce-root-80000000
       0000000000000000-000000003fffffff (prio 0, RW): tce-iommu-80000000
     0000040000000000-000004000000ffff (prio 0, RW): msi


The source AS is 0..(u64)-1. iotlb.iova from 
spapr_tce_translate_iommu(tce-root-80000001) will be relative to 
0800000000000000 which is not source AS.

What do I miss here?

>
> So there is a problem in vfio_iommu_map_notify:
>
>          ret = vfio_dma_map(container, iotlb->iova,
>                             iotlb->addr_mask + 1, vaddr,
>                             !(iotlb->perm & IOMMU_WO) || mr->readonly);
>
> I think that, in vfio_listener_region_add, the iova variable should be
> stored in VFIOGuestIOMMU for use in vfio_iommu_map_notify.
>
> ret.translated_addr should be relative to the target AS, which VFIO
> assumes to be address_space_memory.

That is perfectly fine - there is iotlb.target_as.



-- 
Alexey