On Tue, 24 Oct 2023 at 11:49, Antonio Caggiano
<quic_acaggian@quicinc.com> wrote:
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.
(I've cc'd Paolo to check my understanding of how KVM works.)
#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. It looks like KVM basically doesn't expect to be working
with a memory region that large -- the KVM kernel APIs for
registering memslots etc take a uint64_t size, so there's no way
to say "all of the 64 bit address space". Probably the best
available improvement would be if we added an assert at the point
when the memory region initially gets set up with KVM to say
"this is too big".
Given that we don't run into this upstream, this leaves me
suspecting that the underlying problem is that a memory
region this big shouldn't be being registered to KVM in the
first place. Certainly the gpex PCI controller works fine
on the arm64 virt board under KVM, so maybe your board code
is doing something odd when it wires it up?