Re: [PATCH] hw/riscv/virt-acpi-build.c: Add TPM2 device node and ACPI table support

[Sunil V L <sunilvl@ventanamicro.com>]

Hi Tuan,

On Tue, Dec 02, 2025 at 04:22:59PM -0800, Tuan Phan wrote:
> This patch enables TPM2 support in the RISC-V virt machine ACPI builder.
> 
> Signed-off-by: Tuan Phan <tphan@ventanamicro.com>
> ---
>  hw/riscv/virt-acpi-build.c | 56 ++++++++++++++++++++++++++++++++++++++
>  1 file changed, 56 insertions(+)
> 
> diff --git a/hw/riscv/virt-acpi-build.c b/hw/riscv/virt-acpi-build.c
> index f1406cb68339..b5bf812a9d36 100644
> --- a/hw/riscv/virt-acpi-build.c
> +++ b/hw/riscv/virt-acpi-build.c
> @@ -32,6 +32,7 @@
>  #include "hw/intc/riscv_aclint.h"
>  #include "hw/nvram/fw_cfg_acpi.h"
>  #include "hw/pci-host/gpex.h"
> +#include "hw/platform-bus.h"
>  #include "hw/riscv/virt.h"
>  #include "hw/riscv/numa.h"
>  #include "hw/virtio/virtio-acpi.h"
> @@ -39,6 +40,7 @@
>  #include "qapi/error.h"
>  #include "qemu/error-report.h"
>  #include "system/reset.h"
> +#include "system/tpm.h"
>  
>  #define ACPI_BUILD_TABLE_SIZE             0x20000
>  #define ACPI_BUILD_INTC_ID(socket, index) ((socket << 24) | (index))
> @@ -224,6 +226,41 @@ static void acpi_dsdt_add_iommu_sys(Aml *scope, const MemMapEntry *iommu_memmap,
>      aml_append(scope, dev);
>  }
>  
> +#ifdef CONFIG_TPM
> +static void acpi_dsdt_add_tpm(Aml *scope, RISCVVirtState *s)
> +{
> +    PlatformBusDevice *pbus = PLATFORM_BUS_DEVICE(s->platform_bus_dev);
> +    hwaddr pbus_base = s->memmap[VIRT_PLATFORM_BUS].base;
> +    SysBusDevice *sbdev = SYS_BUS_DEVICE(tpm_find());
> +    MemoryRegion *sbdev_mr;
> +    hwaddr tpm_base;
> +
> +    if (!sbdev) {
> +        return;
> +    }
> +
> +    tpm_base = platform_bus_get_mmio_addr(pbus, sbdev, 0);
> +    assert(tpm_base != -1);
> +
> +    tpm_base += pbus_base;
> +
> +    sbdev_mr = sysbus_mmio_get_region(sbdev, 0);
> +
> +    Aml *dev = aml_device("TPM0");
> +    aml_append(dev, aml_name_decl("_HID", aml_string("MSFT0101")));
> +    aml_append(dev, aml_name_decl("_STR", aml_string("TPM 2.0 Device")));
> +    aml_append(dev, aml_name_decl("_UID", aml_int(0)));
> +
> +    Aml *crs = aml_resource_template();
> +    aml_append(crs,
> +               aml_memory32_fixed(tpm_base,
> +                                  (uint32_t)memory_region_size(sbdev_mr),
> +                                  AML_READ_WRITE));
> +    aml_append(dev, aml_name_decl("_CRS", crs));
> +    aml_append(scope, dev);
> +}
> +#endif
> +
It looks like this logic is duplicated in several places. Would it make
sense to create a shared function for it?

Thanks,
Sunil