From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:43872) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UgBZF-0006Nm-3H for qemu-devel@nongnu.org; Sat, 25 May 2013 06:19:27 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UgBZ6-0003Gn-Oz for qemu-devel@nongnu.org; Sat, 25 May 2013 06:19:17 -0400 Received: from mout.web.de ([212.227.17.11]:62604) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UgBZ6-0003GV-GA for qemu-devel@nongnu.org; Sat, 25 May 2013 06:19:08 -0400 Message-ID: <51A09018.7000901@web.de> Date: Sat, 25 May 2013 12:19:04 +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> <51A05CDF.1090106@web.de> <51A06C9B.5060302@redhat.com> In-Reply-To: <51A06C9B.5060302@redhat.com> Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="----enig2BVLEQKOIEHMWEJSDPHET" 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) ------enig2BVLEQKOIEHMWEJSDPHET Content-Type: text/plain; charset=ISO-8859-15 Content-Transfer-Encoding: quoted-printable On 2013-05-25 09:47, Paolo Bonzini wrote: > Il 25/05/2013 08:40, Jan Kiszka ha scritto: >> On 2013-05-21 12:57, Paolo Bonzini wrote: >>> Using phys_page_find to translate an AddressSpace to a >>> MemoryRegionSection is unwieldy. It requires to pass the page >>> index rather than the address, and later >>> memory_region_section_addr has to be called. Replace=20 >>> 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 written with a single lookup rather than a page at >>> a time. >>> >>> address_space_translate will also provide a single point where >>> IOMMU forwarding is implemented. >>> >>> Signed-off-by: Paolo Bonzini --- cputlb.c >>> | 20 ++--- exec.c | 201 >>> +++++++++++++++++++++++++++-----------------------=20 >>> include/exec/cputlb.h | 12 ++- include/exec/memory.h | 31 >>> ++++---- translate-all.c | 6 +- 5 files changed, 143 >>> insertions(+), 127 deletions(-) >>> >>> diff --git a/cputlb.c b/cputlb.c index aba7e44..1f85da0 100644=20 >>> --- 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; >>> >>> 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)=20 >>> 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) ||=20 >>> 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; } >>> >>> code_address =3D address; - iotlb =3D >>> memory_region_section_get_iotlb(env, section, vaddr, paddr, >>> prot, - &address); + >>> iotlb =3D memory_region_section_get_iotlb(env, section, vaddr, >>> paddr, xlat, + prot, >>> &address); >>> >>> index =3D (vaddr >> TARGET_PAGE_BITS) & (CPU_TLB_SIZE - 1);=20 >>> env->iotlb[mmu_idx][index] =3D iotlb - vaddr; @@ -300,9 +304,7 @@ >>> void tlb_set_page(CPUArchState *env, target_ulong vaddr, /* 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_addr + xlat)) >>> { te->addr_write =3D address | TLB_NOTDIRTY; } else {=20 >>> 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,=20 >>> phys_page_set_level(&d->phys_map, &index, &nb, leaf, P_L2_LEVELS >>> - 1); } >>> >>> -MemoryRegionSection *phys_page_find(AddressSpaceDispatch *d, >>> hwaddr index) +static MemoryRegionSection >>> *phys_page_find(AddressSpaceDispatch *d, hwaddr index) {=20 >>> PhysPageEntry lp =3D d->phys_map; PhysPageEntry *p; @@ -198,6 >>> +198,22 @@ MemoryRegionSection >>> *phys_page_find(AddressSpaceDispatch *d, hwaddr index) return >>> &phys_sections[lp.ptr]; } >>> >>> +MemoryRegionSection *address_space_translate(AddressSpace *as, >>> hwaddr addr, + hwaddr >>> *xlat, hwaddr *plen, + >>> 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); >=20 >> 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: >=20 >> B (prio 1): X A (prio 0): XXXX... ^access here with 4 bytes >> length >=20 >> 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? >=20 > I guess it depends on the width of the bus. On a 16-bit computer the > right thing to do would be to split the write in two, one two-byte > access to A and one two-byte access to B. But as a first > approximation the fix you suggest is the right one. Implementing bus > width in TCG is tricky, so we should get by with the simple fix. >=20 > This patch is not in the pull request I sent, so there is time to make > it work. Is it simply >=20 > - *plen =3D MIN(section->size - addr, *plen); > + addr +=3D section->offset_within_memory_region; > + *plen =3D MIN(section->mr->size - addr, *plen); >=20 > or something like that? If you post it as a diff I can squash it in. This works for me now: diff --git a/exec.c b/exec.c index 1610604..3d36bc7 100644 --- a/exec.c +++ b/exec.c @@ -210,13 +210,15 @@ MemoryRegionSection *address_space_translate(Addres= sSpace *as, hwaddr addr, IOMMUTLBEntry iotlb; MemoryRegionSection *section; hwaddr len =3D *plen; + Int128 diff; =20 for (;;) { section =3D address_space_lookup_region(as, addr); =20 /* Compute offset within MemoryRegionSection */ addr -=3D section->offset_within_address_space; - len =3D MIN(section->size - addr, len); + diff =3D int128_sub(section->mr->size, int128_make64(addr)); + len =3D MIN(int128_get64(diff), len); =20 /* Compute offset within MemoryRegion */ addr +=3D section->offset_within_region; Jan ------enig2BVLEQKOIEHMWEJSDPHET 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/ iEYEARECAAYFAlGgkBgACgkQitSsb3rl5xSxkACgy983FrLYt8h0vIaLCtzi5BVP yxkAn2EDXjQAbhC65Wzr8fQ0fceuKDbZ =/s2F -----END PGP SIGNATURE----- ------enig2BVLEQKOIEHMWEJSDPHET--