On Thu, 18 Jun 2026 at 13:55, Mohammadfaiz Bawa <[email protected]> wrote:
>
> Add a "ppi" boolean property (default: true) to tpm-tis-device.
> When ppi=off the RAMBlock is never registered and the migration
> stream omits "tpm-ppi", restoring backward compatibility.
>
> When disabled, realizefn initializes the region with zero size so
> the platform bus skips it and ACPI PPI tables are omitted.
>
> Fixes: 46cd2c1050f0 ("hw/tpm: add PPI support to tpm-tis-device for ARM64 
> virt")
> Signed-off-by: Mohammadfaiz Bawa <[email protected]>
> ---
>  hw/tpm/tpm_tis.h        |  1 +
>  hw/tpm/tpm_tis_sysbus.c | 16 +++++++++++-----
>  2 files changed, 12 insertions(+), 5 deletions(-)
>
> diff --git a/hw/tpm/tpm_tis.h b/hw/tpm/tpm_tis.h
> index 1531620bf9..6ac9804050 100644
> --- a/hw/tpm/tpm_tis.h
> +++ b/hw/tpm/tpm_tis.h
> @@ -76,6 +76,7 @@ typedef struct TPMState {
>      size_t be_buffer_size;
>
>      TPMPPI ppi;
> +    bool ppi_enabled;
>  } TPMState;
>
>  extern const VMStateDescription vmstate_locty;
> diff --git a/hw/tpm/tpm_tis_sysbus.c b/hw/tpm/tpm_tis_sysbus.c
> index 16bb17874b..54691ac272 100644
> --- a/hw/tpm/tpm_tis_sysbus.c
> +++ b/hw/tpm/tpm_tis_sysbus.c
> @@ -94,6 +94,7 @@ static void tpm_tis_sysbus_reset(DeviceState *dev)
>  static const Property tpm_tis_sysbus_properties[] = {
>      DEFINE_PROP_UINT32("irq", TPMStateSysBus, state.irq_num, TPM_TIS_IRQ),
>      DEFINE_PROP_TPMBE("tpmdev", TPMStateSysBus, state.be_driver),
> +    DEFINE_PROP_BOOL("ppi", TPMStateSysBus, state.ppi_enabled, true),
>  };
>
>  static void tpm_tis_sysbus_initfn(Object *obj)
> @@ -122,14 +123,19 @@ 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);
> +
> +    if (s->ppi_enabled) {
> +        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, OBJECT(dev), 
> "tpm-ppi",
> +                                          TPM_PPI_ADDR_SIZE, s->ppi.buf);
> +        vmstate_register_ram(&s->ppi.ram, dev);
> +    } else {
> +        memory_region_init(&s->ppi.ram, OBJECT(dev), "tpm-ppi", 0);
> +    }

Is it valid to create a 0-size memory region? I think the more
usual approach here would be to not expose the 2nd MMIO region
if the device was configured with PPI disabled.

This device also very weirdly calls sysbus_init_mmio() in
its initfn, before it has actually set up the MR, which it
is doing here in realize. I think that also needs to be fixed
(especially since the property will only be available in
realize). It happens to work since all sysbus_init_mmio()
is doing is putting the pointer into its data structure, but
it doesn't make much logical sense to do it that way around.

thanks
-- PMM

Reply via email to