On Wed, May 27, 2026 at 1:54 AM Philippe Mathieu-Daudé <[email protected]> wrote: > > 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 > > Hi Philippe,
I looked into this, the target_page.decided assertion only shows up with CONFIG_DEBUG_TCG enabled, which is I guess how I hit it. I rebuilt with --disable-debug-tcg and got the same crash as your macOS backtrace, mutex assert in ram_block_add(). > >>>>> > >>>>> 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: Makes sense, will do in v2. > -- >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. > Will update thank you for the review. Thanks --Faiz
