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

[Alexey Kardashevskiy]

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: address@hidden
  0000000000000000-ffffffffffffffff (prio 0, RW): 
address@hidden
    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