From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:57875) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1c2Y4B-0002Ge-Kp for qemu-devel@nongnu.org; Fri, 04 Nov 2016 02:33:32 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1c2Y48-0000vY-Cz for qemu-devel@nongnu.org; Fri, 04 Nov 2016 02:33:31 -0400 Received: from mx1.redhat.com ([209.132.183.28]:34258) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1c2Y48-0000uX-6F for qemu-devel@nongnu.org; Fri, 04 Nov 2016 02:33:28 -0400 References: <1478165243-4767-1-git-send-email-jasowang@redhat.com> <1478165243-4767-6-git-send-email-jasowang@redhat.com> <8718e40b-9b14-c1ce-7e1f-d8ca2d02ffaa@redhat.com> From: Jason Wang Message-ID: <26e58b6d-2975-03c1-e41d-d7c8c509109d@redhat.com> Date: Fri, 4 Nov 2016 14:33:20 +0800 MIME-Version: 1.0 In-Reply-To: <8718e40b-9b14-c1ce-7e1f-d8ca2d02ffaa@redhat.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH V2 05/11] exec: introduce address_space_get_iotlb_entry() List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paolo Bonzini , mst@redhat.com, peterx@redhat.com, wexu@redhat.com, qemu-devel@nongnu.org Cc: vkaplans@redhat.com, cornelia.huck@de.ibm.com, Richard Henderson , Peter Crosthwaite On 2016=E5=B9=B411=E6=9C=8804=E6=97=A5 01:03, Paolo Bonzini wrote: > > On 03/11/2016 10:27, Jason Wang wrote: >> This patch introduces a helper to query the iotlb entry for a >> possible iova. This will be used by later device IOTLB API to enable >> the capability for a dataplane (e.g vhost) to query the IOTLB. >> >> Cc: Paolo Bonzini >> Cc: Peter Crosthwaite >> Cc: Richard Henderson >> Signed-off-by: Jason Wang >> --- >> exec.c | 33 +++++++++++++++++++++++++++++++++ >> include/exec/memory.h | 6 ++++++ >> 2 files changed, 39 insertions(+) >> >> diff --git a/exec.c b/exec.c >> index b1094c0..00c7a2b 100644 >> --- a/exec.c >> +++ b/exec.c >> @@ -449,6 +449,39 @@ address_space_translate_internal(AddressSpaceDisp= atch *d, hwaddr addr, hwaddr *x >> } >> =20 >> /* Called from RCU critical section */ >> +IOMMUTLBEntry address_space_get_iotlb_entry(AddressSpace *as, hwaddr = addr, >> + bool is_write) >> +{ >> + IOMMUTLBEntry iotlb =3D {0}; >> + MemoryRegionSection *section; >> + MemoryRegion *mr; >> + hwaddr plen; > plen must be initialized on entry to address_space_translate_internal, > since it's set to > > MIN(plen, MIN(section->size, > addr - section->offset_within_address_space)) > > after address_space_translate_internal calls > address_space_lookup_region. But you don't really need plen, so you > should do this: > > section =3D address_space_lookup_region(d, addr, false); > addr =3D addr > - section->offset_within_address_space > + section->offset_within_region; > > instead of > > section =3D address_space_translate_internal(d, addr, &addr, > &plen, true); > > and then you don't even need a plen anymore. Cool, will do this in next version. > > Also: > > >> + for (;;) { >> + AddressSpaceDispatch *d =3D atomic_rcu_read(&as->dispatch); >> + section =3D address_space_translate_internal(d, addr, &addr, = &plen, true); >> + mr =3D section->mr; >> + >> + if (!mr->iommu_ops) { >> + break; >> + } >> + >> + iotlb =3D mr->iommu_ops->translate(mr, addr, is_write); >> + addr =3D ((iotlb.translated_addr & ~iotlb.addr_mask) >> + | (addr & iotlb.addr_mask)); > This addr assignment is only needed after the iotlb.perm check, so move > it there. > > Thanks, > > Paolo Right. Thanks > >> + plen =3D MIN(plen, (addr | iotlb.addr_mask) - addr + 1); >> + if (!(iotlb.perm & (1 << is_write))) { >> + iotlb.target_as =3D NULL; >> + break; >> + } >> + >> + as =3D iotlb.target_as; >> + } >> + >> + return iotlb; >> +} >> + >> +/* Called from RCU critical section */ >> MemoryRegion *address_space_translate(AddressSpace *as, hwaddr addr, >> hwaddr *xlat, hwaddr *plen, >> bool is_write) >> diff --git a/include/exec/memory.h b/include/exec/memory.h >> index 9728a2f..e605de3 100644 >> --- a/include/exec/memory.h >> +++ b/include/exec/memory.h >> @@ -1404,6 +1404,12 @@ void address_space_stq_le(AddressSpace *as, hwa= ddr addr, uint64_t val, >> void address_space_stq_be(AddressSpace *as, hwaddr addr, uint64_t va= l, >> MemTxAttrs attrs, MemTxResult *result); >> =20 >> +/* address_space_get_iotlb_entry: translate an address into an IOTLB >> + * entry. Should be called from an RCU critical section. >> + */ >> +IOMMUTLBEntry address_space_get_iotlb_entry(AddressSpace *as, hwaddr = addr, >> + bool is_write); >> + >> /* address_space_translate: translate an address range into an addre= ss space >> * into a MemoryRegion and an address range into that section. Shou= ld be >> * called from an RCU critical section, to avoid that the last refer= ence >>