From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:58974) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Ug89p-0001uF-2n for qemu-devel@nongnu.org; Sat, 25 May 2013 02:40:51 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Ug89i-00045t-Hd for qemu-devel@nongnu.org; Sat, 25 May 2013 02:40:48 -0400 Received: from mout.web.de ([212.227.17.12]:55657) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Ug89i-00045c-8B for qemu-devel@nongnu.org; Sat, 25 May 2013 02:40:42 -0400 Message-ID: <51A05CDF.1090106@web.de> Date: Sat, 25 May 2013 08:40:31 +0200 From: Jan Kiszka MIME-Version: 1.0 References: <1369133851-1894-1-git-send-email-pbonzini@redhat.com> <1369133851-1894-18-git-send-email-pbonzini@redhat.com> In-Reply-To: <1369133851-1894-18-git-send-email-pbonzini@redhat.com> Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="----enig2TAAGLKLVBQLAHEKWSNJI" Subject: Re: [Qemu-devel] [PATCH 17/30] memory: add address_space_translate List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paolo Bonzini Cc: peter.maydell@linaro.org, qemu-devel@nongnu.org, David Gibson This is an OpenPGP/MIME signed message (RFC 4880 and 3156) ------enig2TAAGLKLVBQLAHEKWSNJI Content-Type: text/plain; charset=ISO-8859-15 Content-Transfer-Encoding: quoted-printable On 2013-05-21 12:57, Paolo Bonzini wrote: > Using phys_page_find to translate an AddressSpace to a MemoryRegionSect= ion > is unwieldy. It requires to pass the page index rather than the addres= s, > and later memory_region_section_addr has to be called. Replace > memory_region_section_addr with a function that does all of it: call > phys_page_find, compute the offset within the region, and check how > big the current mapping is. This way, a large flat region can be writt= en > with a single lookup rather than a page at a time. >=20 > address_space_translate will also provide a single point where IOMMU > forwarding is implemented. >=20 > Signed-off-by: Paolo Bonzini > --- > cputlb.c | 20 ++--- > exec.c | 201 +++++++++++++++++++++++++++---------------= -------- > include/exec/cputlb.h | 12 ++- > include/exec/memory.h | 31 ++++---- > translate-all.c | 6 +- > 5 files changed, 143 insertions(+), 127 deletions(-) >=20 > diff --git a/cputlb.c b/cputlb.c > index aba7e44..1f85da0 100644 > --- a/cputlb.c > +++ b/cputlb.c > @@ -248,13 +248,18 @@ void tlb_set_page(CPUArchState *env, target_ulong= vaddr, > target_ulong code_address; > uintptr_t addend; > CPUTLBEntry *te; > - hwaddr iotlb; > + hwaddr iotlb, xlat, sz; > =20 > assert(size >=3D TARGET_PAGE_SIZE); > if (size !=3D TARGET_PAGE_SIZE) { > tlb_add_large_page(env, vaddr, size); > } > - section =3D phys_page_find(address_space_memory.dispatch, paddr >>= TARGET_PAGE_BITS); > + > + sz =3D size; > + section =3D address_space_translate(&address_space_memory, paddr, = &xlat, &sz, > + false); > + assert(sz >=3D TARGET_PAGE_SIZE); > + > #if defined(DEBUG_TLB) > printf("tlb_set_page: vaddr=3D" TARGET_FMT_lx " paddr=3D0x" TARGET= _FMT_plx > " prot=3D%x idx=3D%d pd=3D0x%08lx\n", > @@ -269,15 +274,14 @@ void tlb_set_page(CPUArchState *env, target_ulong= vaddr, > } > if (memory_region_is_ram(section->mr) || > memory_region_is_romd(section->mr)) { > - addend =3D (uintptr_t)memory_region_get_ram_ptr(section->mr) > - + memory_region_section_addr(section, paddr); > + addend =3D (uintptr_t)memory_region_get_ram_ptr(section->mr) += xlat; > } else { > addend =3D 0; > } > =20 > code_address =3D address; > - iotlb =3D memory_region_section_get_iotlb(env, section, vaddr, pad= dr, prot, > - &address); > + iotlb =3D memory_region_section_get_iotlb(env, section, vaddr, pad= dr, xlat, > + prot, &address); > =20 > index =3D (vaddr >> TARGET_PAGE_BITS) & (CPU_TLB_SIZE - 1); > env->iotlb[mmu_idx][index] =3D iotlb - vaddr; > @@ -300,9 +304,7 @@ void tlb_set_page(CPUArchState *env, target_ulong v= addr, > /* Write access calls the I/O callback. */ > te->addr_write =3D address | TLB_MMIO; > } else if (memory_region_is_ram(section->mr) > - && !cpu_physical_memory_is_dirty( > - section->mr->ram_addr > - + memory_region_section_addr(section, paddr= ))) { > + && !cpu_physical_memory_is_dirty(section->mr->ram_a= ddr + xlat)) { > te->addr_write =3D address | TLB_NOTDIRTY; > } else { > te->addr_write =3D address; > diff --git a/exec.c b/exec.c > index 82da067..e5ee8ff 100644 > --- a/exec.c > +++ b/exec.c > @@ -182,7 +182,7 @@ static void phys_page_set(AddressSpaceDispatch *d, > phys_page_set_level(&d->phys_map, &index, &nb, leaf, P_L2_LEVELS -= 1); > } > =20 > -MemoryRegionSection *phys_page_find(AddressSpaceDispatch *d, hwaddr in= dex) > +static MemoryRegionSection *phys_page_find(AddressSpaceDispatch *d, hw= addr index) > { > PhysPageEntry lp =3D d->phys_map; > PhysPageEntry *p; > @@ -198,6 +198,22 @@ MemoryRegionSection *phys_page_find(AddressSpaceDi= spatch *d, hwaddr index) > return &phys_sections[lp.ptr]; > } > =20 > +MemoryRegionSection *address_space_translate(AddressSpace *as, hwaddr = addr, > + hwaddr *xlat, hwaddr *ple= n, > + bool is_write) > +{ > + MemoryRegionSection *section; > + > + section =3D phys_page_find(as->dispatch, addr >> TARGET_PAGE_BITS)= ; > + /* Compute offset within MemoryRegionSection */ > + addr -=3D section->offset_within_address_space; > + *plen =3D MIN(section->size - addr, *plen); This limitation causes problems. Consider two overlapping memory regions A and B. A handles 4-byte accesses and is at least 4 bytes long, B only deals with a single byte. They overlap like this: B (prio 1): X A (prio 0): XXXX... ^access here with 4 bytes length Now if an access happens at the marked position, it is split into one 2-byte access to A, followed by a one-byte access to B and another one-byte access to A. But the right access emulation would be 4-bytes to A, no? I think we need to check against the length of the target mr here instead, but I'm not totally sure yet. A concrete scenario where this breaks popped up while reworking PIO dispatching. PCI accesses to 0xcf8 no longer worked: 0000000000000cf8-0000000000000cfb (prio 0, RW): pci-conf-idx 0000000000000cf9-0000000000000cf9 (prio 1, RW): piix3-reset-control Jan ------enig2TAAGLKLVBQLAHEKWSNJI Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.16 (GNU/Linux) Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iEYEARECAAYFAlGgXOIACgkQitSsb3rl5xRz/ACg2nF4fia8Mdd8/1jt1hO/ez6Z /hIAoIMUYPTgVxMHHQTVT3iDBp8NBZb/ =Rkl/ -----END PGP SIGNATURE----- ------enig2TAAGLKLVBQLAHEKWSNJI--