From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:54701) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UZMvN-0000Tp-Rp for qemu-devel@nongnu.org; Mon, 06 May 2013 11:01:59 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UZMvJ-0000he-0L for qemu-devel@nongnu.org; Mon, 06 May 2013 11:01:57 -0400 Received: from mx1.redhat.com ([209.132.183.28]:7933) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UZMvI-0000hX-ON for qemu-devel@nongnu.org; Mon, 06 May 2013 11:01:52 -0400 Message-ID: <5187C5C9.7040406@redhat.com> Date: Mon, 06 May 2013 17:01:29 +0200 From: Paolo Bonzini MIME-Version: 1.0 References: <5187C473.5070101@redhat.com> <5187C526.9010606@siemens.com> In-Reply-To: <5187C526.9010606@siemens.com> Content-Type: text/plain; charset=ISO-8859-15 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [RFC][PATCH 11/15] memory: Allow unaligned address_space_rw List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Jan Kiszka Cc: Liu Ping Fan , qemu-devel , =?ISO-8859-15?Q?Andreas_F=E4rbe?= =?ISO-8859-15?Q?r?= Il 06/05/2013 16:58, Jan Kiszka ha scritto: > On 2013-05-06 16:55, Paolo Bonzini wrote: >> Il 06/05/2013 16:26, Jan Kiszka ha scritto: >>> This will be needed for some corner cases with para-virtual the I/O >>> ports. >>> >>> Signed-off-by: Jan Kiszka >>> --- >>> exec.c | 33 ++++++++++++++++++--------------- >>> 1 files changed, 18 insertions(+), 15 deletions(-) >>> >>> diff --git a/exec.c b/exec.c >>> index 3ee1f3f..9c582b1 100644 >>> --- a/exec.c >>> +++ b/exec.c >>> @@ -1833,38 +1833,41 @@ void address_space_rw(AddressSpace *as, hwaddr addr, uint8_t *buf, >>> uint8_t *ptr; >>> uint32_t val; >>> MemoryRegionSection *section; >>> + MemoryRegion *mr; >>> >>> while (len > 0) { >>> l = ((addr & TARGET_PAGE_MASK) + TARGET_PAGE_SIZE) - addr; >>> if (l > len) >>> l = len; >>> section = address_space_lookup_region(as, addr); >>> + mr = section->mr; >>> >>> if (is_write) { >>> - if (!memory_region_is_ram(section->mr)) { >>> + if (!memory_region_is_ram(mr)) { >>> hwaddr addr1; >>> addr1 = memory_region_section_addr(section, addr); >>> /* XXX: could force cpu_single_env to NULL to avoid >>> potential bugs */ >>> - if (l >= 4 && ((addr1 & 3) == 0)) { >>> + if (l >= 4 && ((addr1 & 3) == 0 || mr->ops->impl.unaligned)) { >> >> Does the length matter at all if unaligned accesses are allowed? I >> think it shouldn't... > > What do you mean? The length test here is not about alignment, it's > about proper split-up depending on the input size (we cannot use 32 bit > for all accesses and do not want to make them all byte accesses, do we?). Oh right, I was thinking of my IOMMU tree where I have: l = len; section = address_space_translate(as, addr, &addr1, &l, is_write); if (is_write) { if (!memory_region_is_ram(section->mr)) { /* XXX: could force cpu_single_env to NULL to avoid potential bugs */ if (l >= 4 && ((addr1 & 3) == 0)) { and address_space_translate does: *plen = MIN(section->size - addr, *plen); It should not do this if section->mr->ops.unaligned. Whoever rebases on top of the other should keep this in mind. Paolo