All of lore.kernel.org
 help / color / mirror / Atom feed
From: David Gibson <david@gibson.dropbear.id.au>
To: Paolo Bonzini <pbonzini@redhat.com>
Cc: Alexey Kardashevskiy <aik@ozlabs.ru>, qemu-devel@nongnu.org
Subject: Re: [Qemu-devel] [PATCH qemu] memory: Fix IOMMU replay base address
Date: Tue, 23 Feb 2016 22:25:58 +1100	[thread overview]
Message-ID: <20160223112558.GX2808@voom.fritz.box> (raw)
In-Reply-To: <56CC2CE9.6030605@redhat.com>

[-- Attachment #1: Type: text/plain, Size: 1899 bytes --]

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

  reply	other threads:[~2016-02-23 11:29 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-02-22  6:09 [Qemu-devel] [PATCH qemu] memory: Fix IOMMU replay base address Alexey Kardashevskiy
2016-02-22  6:26 ` David Gibson
2016-02-22 11:36   ` Alexey Kardashevskiy
2016-02-22 12:12     ` David Gibson
2016-02-23  2:11       ` Alexey Kardashevskiy
2016-02-23  6:20         ` David Gibson
2016-02-23  9:00           ` Alexey Kardashevskiy
2016-02-23  9:56             ` Paolo Bonzini
2016-02-23 11:25               ` David Gibson [this message]
2016-02-24  0:19               ` Alexey Kardashevskiy

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20160223112558.GX2808@voom.fritz.box \
    --to=david@gibson.dropbear.id.au \
    --cc=aik@ozlabs.ru \
    --cc=pbonzini@redhat.com \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.