On 26/5/26 22:16, Philippe Mathieu-Daudé wrote:
+Paolo/Peter
On 26/5/26 21:45, Philippe Mathieu-Daudé wrote:
+Richard/Pierrick
On 26/5/26 20:07, Mohammadfaiz Bawa wrote:
On Tue, May 26, 2026 at 11:13 PM Peter Maydell
<[email protected]> wrote:
On Tue, 26 May 2026 at 17:49, Mohammadfaiz Bawa <[email protected]>
wrote:
memory_region_init_ram_device_ptr() requires the target page
size to be finalized, which has not happened during
instance_init. Calling it from tpm_tis_sysbus_initfn() causes
an assertion failure when the device is introspected without
being realized, for example:
$ qemu-system-aarch64 -device tpm-tis-device,help
qemu-system-aarch64: ../system/physmem.c:2524:
qemu_ram_alloc_internal:
Assertion 'target_page.decided' failed.
Aborted (core dumped)
I can not reproduce this error, however en macOS this command triggers:
(lldb) bt
* thread #1, queue = 'com.apple.main-thread', stop reason = hit program
assert
frame #0: 0x000000019c642388 libsystem_kernel.dylib`__pthread_kill + 8
frame #1: 0x000000019c67b848 libsystem_pthread.dylib`pthread_kill +
296
frame #2: 0x000000019c584b50 libsystem_c.dylib`abort + 124
frame #3: 0x000000019c583d84 libsystem_c.dylib`__assert_rtn + 284
* frame #4: 0x0000000100a3870c qemu-system-
aarch64`qemu_mutex_lock_impl.cold.1 at qemu-thread-posix.c:107:5
frame #5: 0x0000000100981e78 qemu-system-
aarch64`qemu_mutex_lock_impl(mutex=<unavailable>, file=<unavailable>,
line=<unavailable>) at qemu-thread-posix.c:107:5
frame #6: 0x0000000100345d08 qemu-system-
aarch64`qemu_mutex_lock_ramlist at physmem.c:1454:5
frame #7: 0x0000000100345ce4 qemu-system-
aarch64`ram_block_add(new_block=0x0000000133104f10,
errp=0x000000016fdfe650) at physmem.c:2161:5
frame #8: 0x0000000100346c34 qemu-system-
aarch64`qemu_ram_alloc_internal(size=16384, max_size=16384,
resized=0x0000000000000000, host=0x000000013380c000, ram_flags=1,
mr=0x000000013380a910, errp=0x0000000101c37c20) at physmem.c:2540:5
frame #9: 0x00000001003469c8 qemu-system-
aarch64`qemu_ram_alloc_from_ptr(size=<unavailable>, host=<unavailable>,
mr=<unavailable>, errp=<unavailable>) at physmem.c:2552:12 [artificial]
frame #10: 0x000000010033b56c qemu-system-
aarch64`memory_region_set_ram_ptr(mr=0x000000013380a910, size=1024,
ptr=0x000000013380c000) at memory.c:1664:20 [inlined]
frame #11: 0x000000010033b550 qemu-system-
aarch64`memory_region_init_ram_device_ptr(mr=0x000000013380a910,
owner=<unavailable>, name=<unavailable>, size=1024,
ptr=0x000000013380c000) at memory.c:1684:5
frame #12: 0x00000001002ca594 qemu-system-
aarch64`tpm_tis_sysbus_initfn(obj=0x0000000133809400) at
tpm_tis_sysbus.c:114:5
frame #13: 0x00000001007e8e58 qemu-system-
aarch64`object_initialize_with_type(obj=0x0000000133809400, size=5680,
type=0x0000000131f4e250) at object.c:504:5
frame #14: 0x00000001007e964c qemu-system-
aarch64`object_new_with_type(type=0x0000000131f4e250) at object.c:707:5
frame #15: 0x00000001007e95fc qemu-system-
aarch64`object_new_with_class(klass=<unavailable>) at object.c:716:12
[artificial]
frame #16: 0x00000001008ca264 qemu-system-
aarch64`qmp_device_list_properties(typename="tpm-tis-device",
errp=0x000000016fdfe8a0) at qom-qmp-cmds.c:206:11
frame #17: 0x000000010034c7a0 qemu-system-
aarch64`qdev_device_help(opts=<unavailable>) at qdev-monitor.c:315:17
frame #18: 0x000000010033179c qemu-system-
aarch64`device_help_func(opaque=<unavailable>, opts=<unavailable>,
errp=<unavailable>) at vl.c:1209:12 [artificial]
frame #19: 0x000000010098aa10 qemu-system-
aarch64`qemu_opts_foreach(list=<unavailable>, func=(qemu-system-
aarch64`device_help_func at vl.c:1208), opaque=0x0000000000000000,
errp=0x0000000000000000) at qemu-option.c:1148:14
frame #20: 0x000000010032d45c qemu-system-
aarch64`qemu_process_help_options at vl.c:2642:9 [inlined]
frame #21: 0x000000010032d408 qemu-system-
aarch64`qemu_init(argc=<unavailable>, argv=0x000000016fdff148) at
vl.c:3738:5
frame #22: 0x00000001008dcef8 qemu-system-
aarch64`main(argc=<unavailable>, argv=<unavailable>) at main.c:71:5
frame #23: 0x000000019c2dab98 dyld`start + 6076
Property introspection only calls instance_init, never
realizefn, so moving the memory region setup to realizefn
avoids the crash while keeping the device fully functional
when actually used in a VM.
Move the PPI buffer allocation, memory_region_init_ram_device_ptr()
and the corresponding sysbus_init_mmio() from
tpm_tis_sysbus_initfn() to tpm_tis_sysbus_realizefn(), placing
them just before the existing vmstate_register_ram() call.
Signed-off-by: Mohammadfaiz Bawa <[email protected]>
---
hw/tpm/tpm_tis_sysbus.c | 13 ++++++-------
1 file changed, 6 insertions(+), 7 deletions(-)
diff --git a/hw/tpm/tpm_tis_sysbus.c b/hw/tpm/tpm_tis_sysbus.c
index 6bec30c36f..33fe9e332c 100644
--- a/hw/tpm/tpm_tis_sysbus.c
+++ b/hw/tpm/tpm_tis_sysbus.c
@@ -100,7 +100,6 @@ static void tpm_tis_sysbus_initfn(Object *obj)
{
TPMStateSysBus *sbdev = TPM_TIS_SYSBUS(obj);
TPMState *s = &sbdev->state;
- size_t host_page_size = qemu_real_host_page_size();
This is asking about the host page size, so why does it wind up
asserting about the target page size not being fixed ?
thanks
-- PMM
The host_page_size line itself doesn't cause the crash -
qemu_real_host_page_size() is safe to call anywhere. It was
moved simply because it has no other consumers in initfn.
The actual crash comes from memory_region_init_ram_device_ptr()
- which calls qemu_ram_alloc_internal(). In there, line 2522
evaluates TARGET_PAGE_SIZE: align = MAX(align, TARGET_PAGE_SIZE);
With CONFIG_DEBUG_TCG, TARGET_PAGE_SIZE expands through
TARGET_PAGE_MASK which contains: assert(target_page.decided)
That's where it blows up: the target page size hasn't been
finalized yet during instance_init.
When a realize() can fail, deferring resources allocation to it
(after checking any failure) is indeed preferred pattern (but it
should come with these resources being released in a unrealize
handler, which this file lacks -- pre-existing).
With that in mind I'd also move tpm-tis-mmio allocation:
-- >8 --
diff --git a/hw/tpm/tpm_tis_sysbus.c b/hw/tpm/tpm_tis_sysbus.c
index 6bec30c36fc..aae9ffd8392 100644
--- a/hw/tpm/tpm_tis_sysbus.c
+++ b/hw/tpm/tpm_tis_sysbus.c
@@ -102,7 +102,2 @@ static void tpm_tis_sysbus_initfn(Object *obj)
TPMState *s = &sbdev->state;
- size_t host_page_size = qemu_real_host_page_size();
-
- memory_region_init_io(&s->mmio, obj, &tpm_tis_memory_ops,
- s, "tpm-tis-mmio",
- TPM_TIS_NUM_LOCALITIES <<
TPM_TIS_LOCALITY_SHIFT);
@@ -111,6 +106,2 @@ static void tpm_tis_sysbus_initfn(Object *obj)
- s->ppi.buf = qemu_memalign(host_page_size,
- ROUND_UP(TPM_PPI_ADDR_SIZE,
host_page_size));
- memory_region_init_ram_device_ptr(&s->ppi.ram, obj, "tpm-ppi",
- TPM_PPI_ADDR_SIZE, s->ppi.buf);
sysbus_init_mmio(SYS_BUS_DEVICE(obj), &s->ppi.ram);
@@ -122,2 +113,3 @@ static void tpm_tis_sysbus_realizefn(DeviceState
*dev, Error **errp)
TPMState *s = &sbdev->state;
+ const size_t host_page_size = qemu_real_host_page_size();
@@ -133,2 +125,10 @@ static void tpm_tis_sysbus_realizefn(DeviceState
*dev, Error **errp)
+ s->ppi.buf = qemu_memalign(host_page_size,
+ ROUND_UP(TPM_PPI_ADDR_SIZE,
host_page_size));
+ memory_region_init_io(&s->mmio, OBJECT(dev), &tpm_tis_memory_ops,
+ s, "tpm-tis-mmio",
+ TPM_TIS_NUM_LOCALITIES <<
TPM_TIS_LOCALITY_SHIFT);
+
+ memory_region_init_ram_device_ptr(&s->ppi.ram, OBJECT(dev), "tpm-ppi",
+ TPM_PPI_ADDR_SIZE, s->ppi.buf);
vmstate_register_ram(&s->ppi.ram, dev);
---
At any rate the commit description has to be reworded.