tpm_tis_sysbus: defer resource allocation to realizefn

[PATCH v2] tpm_tis_sysbus: defer resource allocation to realizefn

Posted by Mohammadfaiz Bawa 2 days, 7 hours ago

Calling memory_region_init_ram_device_ptr() and
memory_region_init_io() from tpm_tis_sysbus_initfn() crashes
when the device is introspected without being realized, because
the memory subsystem has not been initialized at that point.

So running:
  $ qemu-system-aarch64 -device tpm-tis-device,help

triggers qdev_device_help() which creates the device object
to list its properties, calling instance_init, but never
realizefn. The memory region calls in instance_init then hit
uninitialized subsystems:

With CONFIG_DEBUG_TCG:
  Assertion 'target_page.decided' failed. (physmem.c:2524)

Without CONFIG_DEBUG_TCG:
  Assertion 'mutex->initialized' failed. (qemu-thread-posix.c:107)

Since realizefn is only called when the device is actually
used in a running VM, moving resource allocation there avoids
the crash without breaking introspection.


Signed-off-by: Mohammadfaiz Bawa <mbawa@redhat.com>
---
Changes in v2:
- Also move memory_region_init_io() to realizefn (Philippe)
- Make host_page_size const (Philippe)
- Reword commit message to cover both crash variants

 hw/tpm/tpm_tis_sysbus.c | 18 ++++++++----------
 1 file changed, 8 insertions(+), 10 deletions(-)

diff --git a/hw/tpm/tpm_tis_sysbus.c b/hw/tpm/tpm_tis_sysbus.c
index 6bec30c36f..f9cd1c8b5c 100644
--- a/hw/tpm/tpm_tis_sysbus.c
+++ b/hw/tpm/tpm_tis_sysbus.c
@@ -100,19 +100,9 @@ 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();
-
-    memory_region_init_io(&s->mmio, obj, &tpm_tis_memory_ops,
-                          s, "tpm-tis-mmio",
-                          TPM_TIS_NUM_LOCALITIES << TPM_TIS_LOCALITY_SHIFT);
 
     sysbus_init_mmio(SYS_BUS_DEVICE(obj), &s->mmio);
     sysbus_init_irq(SYS_BUS_DEVICE(obj), &s->irq);
-
-    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);
 }
 
@@ -120,6 +110,7 @@ static void tpm_tis_sysbus_realizefn(DeviceState *dev, Error **errp)
 {
     TPMStateSysBus *sbdev = TPM_TIS_SYSBUS(dev);
     TPMState *s = &sbdev->state;
+    const size_t host_page_size = qemu_real_host_page_size();
 
     if (!tpm_find()) {
         error_setg(errp, "at most one TPM device is permitted");
@@ -131,6 +122,13 @@ static void tpm_tis_sysbus_realizefn(DeviceState *dev, Error **errp)
         return;
     }
 
+    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);
 }
 
-- 
2.53.0

Re: [PATCH v2] tpm_tis_sysbus: defer resource allocation to realizefn

Posted by Peter Maydell 1 day, 7 hours ago

On Thu, 28 May 2026 at 10:31, Mohammadfaiz Bawa <mbawa@redhat.com> wrote:
>
> Calling memory_region_init_ram_device_ptr() and
> memory_region_init_io() from tpm_tis_sysbus_initfn() crashes
> when the device is introspected without being realized, because
> the memory subsystem has not been initialized at that point.
>
> So running:
>   $ qemu-system-aarch64 -device tpm-tis-device,help
>
> triggers qdev_device_help() which creates the device object
> to list its properties, calling instance_init, but never
> realizefn. The memory region calls in instance_init then hit
> uninitialized subsystems:
>
> With CONFIG_DEBUG_TCG:
>   Assertion 'target_page.decided' failed. (physmem.c:2524)
>
> Without CONFIG_DEBUG_TCG:
>   Assertion 'mutex->initialized' failed. (qemu-thread-posix.c:107)
>
> Since realizefn is only called when the device is actually
> used in a running VM, moving resource allocation there avoids
> the crash without breaking introspection.
>
>
> Signed-off-by: Mohammadfaiz Bawa <mbawa@redhat.com>

This also fixes a memory leak that is reported by the address
sanitizer during 'make check', because we currently allocate
ppi.buf during instance_init and never free it. "Allocate in
realize and never free" is less bad, because we don't currently
support "unrealize and destroy a sysbus device".

Fixes: 46cd2c1050f ("hw/tpm: add PPI support to tpm-tis-device for ARM64 virt")

thanks
-- PMM

Re: [PATCH v2] tpm_tis_sysbus: defer resource allocation to realizefn

Posted by Philippe Mathieu-Daudé 1 day, 5 hours ago

On 29/5/26 11:15, Peter Maydell wrote:
> On Thu, 28 May 2026 at 10:31, Mohammadfaiz Bawa <mbawa@redhat.com> wrote:
>>
>> Calling memory_region_init_ram_device_ptr() and
>> memory_region_init_io() from tpm_tis_sysbus_initfn() crashes
>> when the device is introspected without being realized, because
>> the memory subsystem has not been initialized at that point.
>>
>> So running:
>>    $ qemu-system-aarch64 -device tpm-tis-device,help
>>
>> triggers qdev_device_help() which creates the device object
>> to list its properties, calling instance_init, but never
>> realizefn. The memory region calls in instance_init then hit
>> uninitialized subsystems:
>>
>> With CONFIG_DEBUG_TCG:
>>    Assertion 'target_page.decided' failed. (physmem.c:2524)
>>
>> Without CONFIG_DEBUG_TCG:
>>    Assertion 'mutex->initialized' failed. (qemu-thread-posix.c:107)
>>
>> Since realizefn is only called when the device is actually
>> used in a running VM, moving resource allocation there avoids
>> the crash without breaking introspection.
>>
>>
>> Signed-off-by: Mohammadfaiz Bawa <mbawa@redhat.com>
> 
> This also fixes a memory leak that is reported by the address
> sanitizer during 'make check', because we currently allocate
> ppi.buf during instance_init and never free it. "Allocate in
> realize and never free" is less bad, because we don't currently
> support "unrealize and destroy a sysbus device".
> 
> Fixes: 46cd2c1050f ("hw/tpm: add PPI support to tpm-tis-device for ARM64 virt")

Commit updated, thanks!

Re: [PATCH v2] tpm_tis_sysbus: defer resource allocation to realizefn

Posted by Philippe Mathieu-Daudé 2 days, 6 hours ago

On 28/5/26 11:31, Mohammadfaiz Bawa wrote:
> Calling memory_region_init_ram_device_ptr() and
> memory_region_init_io() from tpm_tis_sysbus_initfn() crashes
> when the device is introspected without being realized, because
> the memory subsystem has not been initialized at that point.
> 
> So running:
>    $ qemu-system-aarch64 -device tpm-tis-device,help
> 
> triggers qdev_device_help() which creates the device object
> to list its properties, calling instance_init, but never
> realizefn. The memory region calls in instance_init then hit
> uninitialized subsystems:
> 
> With CONFIG_DEBUG_TCG:
>    Assertion 'target_page.decided' failed. (physmem.c:2524)
> 
> Without CONFIG_DEBUG_TCG:
>    Assertion 'mutex->initialized' failed. (qemu-thread-posix.c:107)
> 
> Since realizefn is only called when the device is actually
> used in a running VM, moving resource allocation there avoids
> the crash without breaking introspection.
> 
> 
> Signed-off-by: Mohammadfaiz Bawa <mbawa@redhat.com>
> ---
> Changes in v2:
> - Also move memory_region_init_io() to realizefn (Philippe)
> - Make host_page_size const (Philippe)
> - Reword commit message to cover both crash variants
> 
>   hw/tpm/tpm_tis_sysbus.c | 18 ++++++++----------
>   1 file changed, 8 insertions(+), 10 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>