Il 20/06/2013 15:53, Peter Maydell ha scritto:
> On 30 May 2013 22:03, Paolo Bonzini <pbonz...@redhat.com> wrote:
>> +MemoryRegionSection *address_space_translate(AddressSpace *as, hwaddr addr,
>> + hwaddr *xlat, hwaddr *plen,
>> + bool is_write)
>> +{
>> + MemoryRegionSection *section;
>> + Int128 diff;
>> +
>> + section = phys_page_find(as->dispatch, addr >> TARGET_PAGE_BITS);
>> + /* Compute offset within MemoryRegionSection */
>> + addr -= section->offset_within_address_space;
>> +
>> + /* Compute offset within MemoryRegion */
>> + *xlat = addr + section->offset_within_region;
>> +
>> + diff = int128_sub(section->mr->size, int128_make64(addr));
>> + *plen = MIN(int128_get64(diff), *plen);
>
> I've just run into a situation where the assertion in
> int128_get64() that the value fits into a 64 bit integer
> fires. This happened to me for an access to address zero
> in the 'unassigned' region:
> * io_mem_init() sets the size of these to UINT64_MAX
> * memory_region_init() special-cases that size as meaning
> 2^64, ie {hi=1,lo=0}
> * since the addr is zero diff is also {hi=1,lo=0}, and
> then int128_get64() asserts.
This would be fixed by:
*plen = int128_get64(int128_min(diff, int128_make64(*plen)));
(We can optimize int128 so that it produces more than decent code for this).
> There are other places in memory.c which do an int128_get64()
> on mr->size, which also look suspicious...
They are all on I/O regions so they are safe, except those
in "info mtree". I'm going to squash this:
diff --git a/memory.c b/memory.c
index c500d8d..47b005a 100644
--- a/memory.c
+++ b/memory.c
@@ -1744,7 +1744,7 @@ static void mtree_print_mr(fprintf_function mon_printf,
void *f,
"-" TARGET_FMT_plx "\n",
base + mr->addr,
base + mr->addr
- + (hwaddr)int128_get64(mr->size) - 1,
+ + (hwaddr)int128_get64(int128_sub(mr->size,
int128_make64(1))),
mr->priority,
mr->romd_mode ? 'R' : '-',
!mr->readonly && !(mr->rom_device && mr->romd_mode) ? 'W'
@@ -1759,7 +1759,7 @@ static void mtree_print_mr(fprintf_function mon_printf,
void *f,
TARGET_FMT_plx "-" TARGET_FMT_plx " (prio %d, %c%c): %s\n",
base + mr->addr,
base + mr->addr
- + (hwaddr)int128_get64(mr->size) - 1,
+ + (hwaddr)int128_get64(int128_sub(mr->size,
int128_make64(1))),
mr->priority,
mr->romd_mode ? 'R' : '-',
!mr->readonly && !(mr->rom_device && mr->romd_mode) ? 'W'
Paolo
