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

[Antonio Caggiano <quic_acaggian@quicinc.com>]

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