On Tue, Feb 23, 2016 at 10:56:57AM +0100, 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).
Uh.. relative to the source AS, or the source MR? At the moment spapr
implements it relative to the MR.. which seems to match your example.
> 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);
Ah.. yeah.. this needs the address relative to the AS, not the MR
> I think that, in vfio_listener_region_add, the iova variable should be
> stored in VFIOGuestIOMMU for use in vfio_iommu_map_notify.
Hmm, seems a bit clunky, though I guess it's safe due to the BQL.
> ret.translated_addr should be relative to the target AS, which VFIO
> assumes to be address_space_memory.
Right, and as far as I can tell that is the case.
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
signature.asc
Description: PGP signature
