Re: [RFC] mem: Fix mem region size when is UINT64_MAX

[Antonio Caggiano]

Hi Peter,

Thanks for the quick response.

On 24/10/2023 12:28, Peter Maydell wrote:
> On Tue, 24 Oct 2023 at 10:45, Antonio Caggiano
> <quic_acaggian@quicinc.com> wrote:
> > This looks like a bug. When the size is `UINT64_MAX`, it is reset to
> > (Int128)`1 << 64` which actually is `UINT64_MAX + 1`.
> >
> > Then, an assert is triggered when the size is converted back to uin64_t
> > by using the int128_get64() function, as the new value happens to be
> > different than the previous one.
> >
> > Signed-off-by: Antonio Caggiano <quic_acaggian@quicinc.com>
> > ---
> >   system/memory.c | 3 ---
> >   1 file changed, 3 deletions(-)
> >
> > diff --git a/system/memory.c b/system/memory.c
> > index a800fbc9e5..d41fc6af88 100644
> > --- a/system/memory.c
> > +++ b/system/memory.c
> > @@ -1193,9 +1193,6 @@ static void memory_region_do_init(MemoryRegion *mr,
> >                                     uint64_t size)
> >   {
> >       mr->size = int128_make64(size);
> > -    if (size == UINT64_MAX) {
> > -        mr->size = int128_2_64();
> > -    }
>
> No, this is intentional. In these memory region creation APIs
> that take a uint64_t size parameter, size == UINT64_MAX is a
> special case that means "actually the full 64 bit address space"
> (and there is no way to ask for an MR to have a size that is
> truly UINT64_MAX bytes). When we create the MR, the size is
> stored in the MemoryRegion struct as its true size, because
> we have an Int128 field there.
>
> What assertion (with backtrace) is being hit? The issue is
> probably at that point, not here.

Here you can. I'm basically creating a system_memory of size UINT64_MAX, 
and the assert is being hit when the memory is registered to KVM.

#5  0x0000fffff6fc4040 in __GI___assert_fail (assertion=0xffffe111d9c8 
"r == a", file=0xffffe111d960 "qemu/include/qemu/int128.h", line=34, 
function=0xffffe111f348 <__PRETTY_FUNCTION__.46> "int128_get64") at 
./assert/assert.c:101
#6  0x0000ffffe0c8cf6c in int128_get64 (a=18446744073709551616) at 
qemu/include/qemu/int128.h:34
#7  0x0000ffffe0c92cec in kvm_region_commit (listener=0xffffd83e92e0) at 
qemu/accel/kvm/kvm-all.c:1503
#8  0x0000ffffe0bd495c in memory_region_transaction_commit () at 
qemu/softmmu/memory.c:1109
#9  0x0000ffffe0bd8a90 in memory_region_update_container_subregions 
(subregion=0xaaaaabb6abf0) at qemu/softmmu/memory.c:2606
#10 0x0000ffffe0bd8b3c in memory_region_add_subregion_common 
(mr=0xaaaaabb6ae10, offset=0, subregion=0xaaaaabb6abf0) at 
qemu/softmmu/memory.c:2621
#11 0x0000ffffe0bd8b74 in memory_region_add_subregion 
(mr=0xaaaaabb6ae10, offset=0, subregion=0xaaaaabb6abf0) at 
qemu/softmmu/memory.c:2629
#12 0x0000ffffe05d5508 in gpex_host_realize (dev=0xaaaaabb69910, 
errp=0xffffdd4ce1f0) at qemu/hw/pci-host/gpex.c:132

> thanks
> -- PMM


Cheers,
Antonio