From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:33396) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UpfuB-00008S-Lc for qemu-devel@nongnu.org; Thu, 20 Jun 2013 10:32:12 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Upfu6-0002qf-Vi for qemu-devel@nongnu.org; Thu, 20 Jun 2013 10:32:07 -0400 Received: from mx1.redhat.com ([209.132.183.28]:35891) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Upfu6-0002qZ-Mr for qemu-devel@nongnu.org; Thu, 20 Jun 2013 10:32:02 -0400 Message-ID: <51C3125A.4090902@redhat.com> Date: Thu, 20 Jun 2013 16:31:54 +0200 From: Paolo Bonzini MIME-Version: 1.0 References: <1371737884-26020-1-git-send-email-peter.maydell@linaro.org> In-Reply-To: <1371737884-26020-1-git-send-email-peter.maydell@linaro.org> Content-Type: text/plain; charset=ISO-8859-15 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH] exec.c: address_space_translate: handle access to addr 0 of 2^64 sized region List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Peter Maydell Cc: qemu-devel@nongnu.org, patches@linaro.org Il 20/06/2013 16:18, Peter Maydell ha scritto: > The memory API allows a MemoryRegion's size to be 2^64, as a special > case (otherwise the size always fits in a 64 bit integer). This meant > that attempts to access address zero in a 2^64 sized region would > assert in address_space_translate(): > > #3 0x00007ffff3e4d192 in __GI___assert_fail#(assertion=0x555555a43f32 > "!a.hi", file=0x555555a43ef0 "include/qemu/int128.h", line=18, > function=0x555555a4439f "int128_get64") at assert.c:103 > #4 0x0000555555877642 in int128_get64 (a=...) > at include/qemu/int128.h:18 > #5 0x00005555558782f2 in address_space_translate (as=0x55555668d140, > /addr=0, xlat=0x7fffafac9918, plen=0x7fffafac9920, is_write=false) > at exec.c:221 > > Fix this by doing the 'min' operation in 128 bit arithmetic > rather than 64 bit arithmetic (we know the result of the 'min' > definitely fits in 64 bits because one of the inputs did). > > Signed-off-by: Peter Maydell > --- > The other possible approach here would be: > if (!diff.hi) { > *plen = MIN(int128_get64(diff), *plen); > } > (since if diff.hi is nonzero we know plen is smaller) but > it seems slightly cleaner not to "look inside" the Int128. > > exec.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/exec.c b/exec.c > index 5b8b40d..eb200d0 100644 > --- a/exec.c > +++ b/exec.c > @@ -218,7 +218,7 @@ MemoryRegionSection *address_space_translate(AddressSpace *as, hwaddr addr, > *xlat = addr + section->offset_within_region; > > diff = int128_sub(section->mr->size, int128_make64(addr)); > - *plen = MIN(int128_get64(diff), *plen); > + *plen = int128_get64(int128_min(diff, int128_make64(*plen))); > return section; > } > #endif > Thanks, applied. Paolo