diff mbox series

[v2,10/11] tpm_crb_sysbus: introduce TPM CRB SysBus device

Message ID 20230714070931.23476-11-j@getutm.app
State New
Headers show
Series tpm: introduce TPM CRB SysBus device | expand

Commit Message

Joelle van Dyne July 14, 2023, 7:09 a.m. UTC
This SysBus variant of the CRB interface supports dynamically locating
the MMIO interface so that Virt machines can use it. This interface
is currently the only one supported by QEMU that works on Windows 11
ARM64. We largely follow the TPM TIS SysBus device as a template.

To try out this device with Windows 11 before OVMF is updated, you
will need to modify `sysbud-fdt.c` and change the added line from:

```c
    TYPE_BINDING(TYPE_TPM_CRB_SYSBUS, no_fdt_node),
```

to

```c
    TYPE_BINDING(TYPE_TPM_CRB_SYSBUS, add_tpm_tis_fdt_node),
```

This change was not included because it can confuse Linux (although
from testing, it seems like Linux is able to properly ignore the
device from the TPM TIS driver and recognize it from the ACPI device
in the TPM CRB driver). A proper fix would require OVMF to recognize
the ACPI device and not depend on the FDT node for recognizing TPM.

The command line to try out this device with SWTPM is:

```
$ qemu-system-aarch64 \
    -chardev socket,id=chrtpm0,path=tpm.sock \
    -tpmdev emulator,id=tpm0,chardev=chrtpm0 \
    -device tpm-crb-device,tpmdev=tpm0
```

along with SWTPM:

```
$ swtpm \
    --ctrl type=unixio,path=tpm.sock,terminate \
    --tpmstate backend-uri=file://tpm.data \
    --tpm2
```

Signed-off-by: Joelle van Dyne <j@getutm.app>
---
 docs/specs/tpm.rst          |   1 +
 include/hw/acpi/aml-build.h |   1 +
 include/sysemu/tpm.h        |   3 +
 hw/acpi/aml-build.c         |   7 +-
 hw/arm/virt.c               |   1 +
 hw/core/sysbus-fdt.c        |   1 +
 hw/loongarch/virt.c         |   1 +
 hw/riscv/virt.c             |   1 +
 hw/tpm/tpm_crb_sysbus.c     | 170 ++++++++++++++++++++++++++++++++++++
 hw/arm/Kconfig              |   1 +
 hw/riscv/Kconfig            |   1 +
 hw/tpm/Kconfig              |   5 ++
 hw/tpm/meson.build          |   2 +
 13 files changed, 194 insertions(+), 1 deletion(-)
 create mode 100644 hw/tpm/tpm_crb_sysbus.c

Comments

Stefan Berger July 14, 2023, 2:27 p.m. UTC | #1
On 7/14/23 03:09, Joelle van Dyne wrote:
> This SysBus variant of the CRB interface supports dynamically locating
> the MMIO interface so that Virt machines can use it. This interface
> is currently the only one supported by QEMU that works on Windows 11
> ARM64. We largely follow the TPM TIS SysBus device as a template.
> 
> To try out this device with Windows 11 before OVMF is updated, you
> will need to modify `sysbud-fdt.c` and change the added line from:
> 
> ```c
>      TYPE_BINDING(TYPE_TPM_CRB_SYSBUS, no_fdt_node),
> ```
> 
> to
> 
> ```c
>      TYPE_BINDING(TYPE_TPM_CRB_SYSBUS, add_tpm_tis_fdt_node),
> ```
> 
> This change was not included because it can confuse Linux (although
> from testing, it seems like Linux is able to properly ignore the
> device from the TPM TIS driver and recognize it from the ACPI device
> in the TPM CRB driver). A proper fix would require OVMF to recognize
> the ACPI device and not depend on the FDT node for recognizing TPM.
> 
> The command line to try out this device with SWTPM is:
> 
> ```
> $ qemu-system-aarch64 \
>      -chardev socket,id=chrtpm0,path=tpm.sock \
>      -tpmdev emulator,id=tpm0,chardev=chrtpm0 \
>      -device tpm-crb-device,tpmdev=tpm0
> ```
> 
> along with SWTPM:
> 
> ```
> $ swtpm \
>      --ctrl type=unixio,path=tpm.sock,terminate \
>      --tpmstate backend-uri=file://tpm.data \
>      --tpm2
> ```
> 
> Signed-off-by: Joelle van Dyne <j@getutm.app>
> ---
>   docs/specs/tpm.rst          |   1 +
>   include/hw/acpi/aml-build.h |   1 +
>   include/sysemu/tpm.h        |   3 +
>   hw/acpi/aml-build.c         |   7 +-
>   hw/arm/virt.c               |   1 +
>   hw/core/sysbus-fdt.c        |   1 +
>   hw/loongarch/virt.c         |   1 +
>   hw/riscv/virt.c             |   1 +
>   hw/tpm/tpm_crb_sysbus.c     | 170 ++++++++++++++++++++++++++++++++++++
>   hw/arm/Kconfig              |   1 +
>   hw/riscv/Kconfig            |   1 +
>   hw/tpm/Kconfig              |   5 ++
>   hw/tpm/meson.build          |   2 +
>   13 files changed, 194 insertions(+), 1 deletion(-)
>   create mode 100644 hw/tpm/tpm_crb_sysbus.c
> 
> diff --git a/docs/specs/tpm.rst b/docs/specs/tpm.rst
> index 2bc29c9804..95aeb49220 100644
> --- a/docs/specs/tpm.rst
> +++ b/docs/specs/tpm.rst
> @@ -46,6 +46,7 @@ operating system.
>   QEMU files related to TPM CRB interface:
>    - ``hw/tpm/tpm_crb.c``
>    - ``hw/tpm/tpm_crb_common.c``
> + - ``hw/tpm/tpm_crb_sysbus.c``
> 

If you added the command line to use for Windows guests to this document
I think this would be helpful. And also mention that this must be used for Windows
since the other ones don't work.


>   SPAPR interface
>   ---------------
> diff --git a/include/hw/acpi/aml-build.h b/include/hw/acpi/aml-build.h
> index d1fb08514b..9660e16148 100644
> --- a/include/hw/acpi/aml-build.h
> +++ b/include/hw/acpi/aml-build.h
> @@ -3,6 +3,7 @@
> 
>   #include "hw/acpi/acpi-defs.h"
>   #include "hw/acpi/bios-linker-loader.h"
> +#include "exec/hwaddr.h"
> 
>   #define ACPI_BUILD_APPNAME6 "BOCHS "
>   #define ACPI_BUILD_APPNAME8 "BXPC    "
> diff --git a/include/sysemu/tpm.h b/include/sysemu/tpm.h
> index 66e3b45f30..f79c8f3575 100644
> --- a/include/sysemu/tpm.h
> +++ b/include/sysemu/tpm.h
> @@ -47,6 +47,7 @@ struct TPMIfClass {
>   #define TYPE_TPM_TIS_ISA            "tpm-tis"
>   #define TYPE_TPM_TIS_SYSBUS         "tpm-tis-device"
>   #define TYPE_TPM_CRB                "tpm-crb"
> +#define TYPE_TPM_CRB_SYSBUS         "tpm-crb-device"
>   #define TYPE_TPM_SPAPR              "tpm-spapr"
>   #define TYPE_TPM_TIS_I2C            "tpm-tis-i2c"
> 
> @@ -56,6 +57,8 @@ struct TPMIfClass {
>       object_dynamic_cast(OBJECT(chr), TYPE_TPM_TIS_SYSBUS)
>   #define TPM_IS_CRB(chr)                             \
>       object_dynamic_cast(OBJECT(chr), TYPE_TPM_CRB)
> +#define TPM_IS_CRB_SYSBUS(chr)                      \
> +    object_dynamic_cast(OBJECT(chr), TYPE_TPM_CRB_SYSBUS)
>   #define TPM_IS_SPAPR(chr)                           \
>       object_dynamic_cast(OBJECT(chr), TYPE_TPM_SPAPR)
>   #define TPM_IS_TIS_I2C(chr)                      \
> diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
> index ea331a20d1..f809137fc9 100644
> --- a/hw/acpi/aml-build.c
> +++ b/hw/acpi/aml-build.c
> @@ -31,6 +31,7 @@
>   #include "hw/pci/pci_bus.h"
>   #include "hw/pci/pci_bridge.h"
>   #include "qemu/cutils.h"
> +#include "qom/object.h"
> 
>   static GArray *build_alloc_array(void)
>   {
> @@ -2218,7 +2219,7 @@ void build_tpm2(GArray *table_data, BIOSLinker *linker, GArray *tcpalog,
>   {
>       uint8_t start_method_params[12] = {};
>       unsigned log_addr_offset;
> -    uint64_t control_area_start_address;
> +    uint64_t baseaddr, control_area_start_address;
>       TPMIf *tpmif = tpm_find();
>       uint32_t start_method;
>       AcpiTable table = { .sig = "TPM2", .rev = 4,
> @@ -2236,6 +2237,10 @@ void build_tpm2(GArray *table_data, BIOSLinker *linker, GArray *tcpalog,
>       } else if (TPM_IS_CRB(tpmif)) {
>           control_area_start_address = TPM_CRB_ADDR_CTRL;
>           start_method = TPM2_START_METHOD_CRB;
> +    } else if (TPM_IS_CRB_SYSBUS(tpmif)) {
> +        baseaddr = object_property_get_uint(OBJECT(tpmif), "baseaddr", NULL);
> +        control_area_start_address = baseaddr + A_CRB_CTRL_REQ;
> +        start_method = TPM2_START_METHOD_CRB;
>       } else {
>           g_assert_not_reached();
>       }
> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> index 432148ef47..88e8b16103 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -2977,6 +2977,7 @@ static void virt_machine_class_init(ObjectClass *oc, void *data)
>       machine_class_allow_dynamic_sysbus_dev(mc, TYPE_VFIO_PLATFORM);
>   #ifdef CONFIG_TPM
>       machine_class_allow_dynamic_sysbus_dev(mc, TYPE_TPM_TIS_SYSBUS);
> +    machine_class_allow_dynamic_sysbus_dev(mc, TYPE_TPM_CRB_SYSBUS);
>   #endif
>       mc->block_default_type = IF_VIRTIO;
>       mc->no_cdrom = 1;
> diff --git a/hw/core/sysbus-fdt.c b/hw/core/sysbus-fdt.c
> index eebcd28f9a..9c783f88eb 100644
> --- a/hw/core/sysbus-fdt.c
> +++ b/hw/core/sysbus-fdt.c
> @@ -493,6 +493,7 @@ static const BindingEntry bindings[] = {
>   #endif
>   #ifdef CONFIG_TPM
>       TYPE_BINDING(TYPE_TPM_TIS_SYSBUS, add_tpm_tis_fdt_node),
> +    TYPE_BINDING(TYPE_TPM_CRB_SYSBUS, no_fdt_node),
>   #endif
>       TYPE_BINDING(TYPE_RAMFB_DEVICE, no_fdt_node),
>       TYPE_BINDING("", NULL), /* last element */
> diff --git a/hw/loongarch/virt.c b/hw/loongarch/virt.c
> index 9c536c52bc..eb59fb04ee 100644
> --- a/hw/loongarch/virt.c
> +++ b/hw/loongarch/virt.c
> @@ -1194,6 +1194,7 @@ static void loongarch_class_init(ObjectClass *oc, void *data)
>       machine_class_allow_dynamic_sysbus_dev(mc, TYPE_RAMFB_DEVICE);
>   #ifdef CONFIG_TPM
>       machine_class_allow_dynamic_sysbus_dev(mc, TYPE_TPM_TIS_SYSBUS);
> +    machine_class_allow_dynamic_sysbus_dev(mc, TYPE_TPM_CRB_SYSBUS);
>   #endif
>   }
> 
> diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
> index d90286dc46..5d639a870a 100644
> --- a/hw/riscv/virt.c
> +++ b/hw/riscv/virt.c
> @@ -1681,6 +1681,7 @@ static void virt_machine_class_init(ObjectClass *oc, void *data)
>       machine_class_allow_dynamic_sysbus_dev(mc, TYPE_RAMFB_DEVICE);
>   #ifdef CONFIG_TPM
>       machine_class_allow_dynamic_sysbus_dev(mc, TYPE_TPM_TIS_SYSBUS);
> +    machine_class_allow_dynamic_sysbus_dev(mc, TYPE_TPM_CRB_SYSBUS);
>   #endif
> 
>       if (tcg_enabled()) {
> diff --git a/hw/tpm/tpm_crb_sysbus.c b/hw/tpm/tpm_crb_sysbus.c
> new file mode 100644
> index 0000000000..66be57a532
> --- /dev/null
> +++ b/hw/tpm/tpm_crb_sysbus.c
> @@ -0,0 +1,170 @@
> +/*
> + * tpm_crb_sysbus.c - QEMU's TPM CRB interface emulator
> + *
> + * Copyright (c) 2018 Red Hat, Inc.
> + *
> + * Authors:
> + *   Marc-André Lureau <marcandre.lureau@redhat.com>
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> + * See the COPYING file in the top-level directory.
> + *
> + * tpm_crb is a device for TPM 2.0 Command Response Buffer (CRB) Interface
> + * as defined in TCG PC Client Platform TPM Profile (PTP) Specification
> + * Family “2.0” Level 00 Revision 01.03 v22
> + */
> +
> +#include "qemu/osdep.h"
> +#include "hw/acpi/acpi_aml_interface.h"
> +#include "hw/acpi/tpm.h"
> +#include "hw/qdev-properties.h"
> +#include "migration/vmstate.h"
> +#include "tpm_prop.h"
> +#include "hw/pci/pci_ids.h"
> +#include "hw/sysbus.h"
> +#include "qapi/visitor.h"
> +#include "qom/object.h"
> +#include "sysemu/tpm_util.h"
> +#include "trace.h"
> +#include "tpm_crb.h"
> +
> +struct TPMCRBStateSysBus {
> +    /*< private >*/
> +    SysBusDevice parent_obj;
> +
> +    /*< public >*/
> +    TPMCRBState state;
> +    uint64_t baseaddr;
> +    uint64_t size;
> +};
> +
> +OBJECT_DECLARE_SIMPLE_TYPE(TPMCRBStateSysBus, TPM_CRB_SYSBUS)
> +
> +static void tpm_crb_sysbus_request_completed(TPMIf *ti, int ret)
> +{
> +    TPMCRBStateSysBus *s = TPM_CRB_SYSBUS(ti);
> +
> +    return tpm_crb_request_completed(&s->state, ret);
> +}
> +
> +static enum TPMVersion tpm_crb_sysbus_get_tpm_version(TPMIf *ti)
> +{
> +    TPMCRBStateSysBus *s = TPM_CRB_SYSBUS(ti);
> +
> +    return tpm_crb_get_version(&s->state);
> +}
> +
> +static int tpm_crb_sysbus_pre_save(void *opaque)
> +{
> +    TPMCRBStateSysBus *s = opaque;
> +
> +    return tpm_crb_pre_save(&s->state);
> +}
> +
> +static const VMStateDescription vmstate_tpm_crb_sysbus = {
> +    .name = "tpm-crb-sysbus",
> +    .pre_save = tpm_crb_sysbus_pre_save,
> +    .fields = (VMStateField[]) {
> +        VMSTATE_END_OF_LIST(),
> +    }
> +};
> +
> +static Property tpm_crb_sysbus_properties[] = {
> +    DEFINE_PROP_TPMBE("tpmdev", TPMCRBStateSysBus, state.tpmbe),
> +    DEFINE_PROP_UINT64("baseaddr", TPMCRBStateSysBus,
> +                       baseaddr, TPM_CRB_ADDR_BASE),
> +    DEFINE_PROP_UINT64("size", TPMCRBStateSysBus, size, TPM_CRB_ADDR_SIZE),
> +    DEFINE_PROP_END_OF_LIST(),
> +};
> +
> +static void tpm_crb_sysbus_initfn(Object *obj)
> +{
> +    TPMCRBStateSysBus *s = TPM_CRB_SYSBUS(obj);
> +
> +    tpm_crb_init_memory(obj, &s->state, NULL);
> +
> +    sysbus_init_mmio(SYS_BUS_DEVICE(obj), &s->state.mmio);
> +}
> +
> +static void tpm_crb_sysbus_reset(DeviceState *dev)
> +{
> +    TPMCRBStateSysBus *s = TPM_CRB_SYSBUS(dev);
> +
> +    return tpm_crb_reset(&s->state, s->baseaddr);
> +}
> +
> +static void tpm_crb_sysbus_realizefn(DeviceState *dev, Error **errp)
> +{
> +    TPMCRBStateSysBus *s = TPM_CRB_SYSBUS(dev);
> +
> +    if (!tpm_find()) {
> +        error_setg(errp, "at most one TPM device is permitted");
> +        return;
> +    }
> +
> +    if (!s->state.tpmbe) {
> +        error_setg(errp, "'tpmdev' property is required");
> +        return;
> +    }
> +
> +    if (tpm_crb_sysbus_get_tpm_version(TPM_IF(s)) != TPM_VERSION_2_0) {
> +        error_setg(errp, "TPM CRB only supports TPM 2.0 backends");
> +        return;
> +    }
> +}
> +
> +static void build_tpm_crb_sysbus_aml(AcpiDevAmlIf *adev, Aml *scope)
> +{
> +    Aml *dev, *crs;
> +    TPMCRBStateSysBus *s = TPM_CRB_SYSBUS(adev);
> +    TPMIf *ti = TPM_IF(s);

../hw/tpm/tpm_crb_sysbus.c: In function 'build_tpm_crb_sysbus_aml':
../hw/tpm/tpm_crb_sysbus.c:120:12: error: unused variable 'ti' [-Werror=unused-variable]
   120 |     TPMIf *ti = TPM_IF(s);
       |            ^~


Rest LGTM.

> +
> +    dev = aml_device("TPM");
> +    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(1)));
> +    aml_append(dev, aml_name_decl("_STA", aml_int(0xF)));
> +    crs = aml_resource_template();
> +    aml_append(crs, aml_memory32_fixed(s->baseaddr, s->size,
> +                                      AML_READ_WRITE));
> +    aml_append(dev, aml_name_decl("_CRS", crs));
> +    aml_append(scope, dev);
> +}
> +
> +static void tpm_crb_sysbus_class_init(ObjectClass *klass, void *data)
> +{
> +    DeviceClass *dc = DEVICE_CLASS(klass);
> +    TPMIfClass *tc = TPM_IF_CLASS(klass);
> +    AcpiDevAmlIfClass *adevc = ACPI_DEV_AML_IF_CLASS(klass);
> +
> +    device_class_set_props(dc, tpm_crb_sysbus_properties);
> +    dc->vmsd  = &vmstate_tpm_crb_sysbus;
> +    tc->model = TPM_MODEL_TPM_CRB;
> +    dc->realize = tpm_crb_sysbus_realizefn;
> +    dc->user_creatable = true;
> +    dc->reset = tpm_crb_sysbus_reset;
> +    tc->request_completed = tpm_crb_sysbus_request_completed;
> +    tc->get_version = tpm_crb_sysbus_get_tpm_version;
> +    set_bit(DEVICE_CATEGORY_MISC, dc->categories);
> +    adevc->build_dev_aml = build_tpm_crb_sysbus_aml;
> +}
> +
> +static const TypeInfo tpm_crb_sysbus_info = {
> +    .name = TYPE_TPM_CRB_SYSBUS,
> +    .parent = TYPE_SYS_BUS_DEVICE,
> +    .instance_size = sizeof(TPMCRBStateSysBus),
> +    .instance_init = tpm_crb_sysbus_initfn,
> +    .class_init  = tpm_crb_sysbus_class_init,
> +    .interfaces = (InterfaceInfo[]) {
> +        { TYPE_TPM_IF },
> +        { TYPE_ACPI_DEV_AML_IF },
> +        { }
> +    }
> +};
> +
> +static void tpm_crb_sysbus_register(void)
> +{
> +    type_register_static(&tpm_crb_sysbus_info);
> +}
> +
> +type_init(tpm_crb_sysbus_register)
> diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
> index 7e68348440..efe1beaa7b 100644
> --- a/hw/arm/Kconfig
> +++ b/hw/arm/Kconfig
> @@ -5,6 +5,7 @@ config ARM_VIRT
>       imply VFIO_AMD_XGBE
>       imply VFIO_PLATFORM
>       imply VFIO_XGMAC
> +    imply TPM_CRB_SYSBUS
>       imply TPM_TIS_SYSBUS
>       imply TPM_TIS_I2C
>       imply NVDIMM
> diff --git a/hw/riscv/Kconfig b/hw/riscv/Kconfig
> index b6a5eb4452..d824cb58f9 100644
> --- a/hw/riscv/Kconfig
> +++ b/hw/riscv/Kconfig
> @@ -29,6 +29,7 @@ config RISCV_VIRT
>       imply PCI_DEVICES
>       imply VIRTIO_VGA
>       imply TEST_DEVICES
> +    imply TPM_CRB_SYSBUS
>       imply TPM_TIS_SYSBUS
>       select RISCV_NUMA
>       select GOLDFISH_RTC
> diff --git a/hw/tpm/Kconfig b/hw/tpm/Kconfig
> index 1fd73fe617..3f294a20ba 100644
> --- a/hw/tpm/Kconfig
> +++ b/hw/tpm/Kconfig
> @@ -25,6 +25,11 @@ config TPM_CRB
>       depends on TPM && ISA_BUS
>       select TPM_BACKEND
> 
> +config TPM_CRB_SYSBUS
> +    bool
> +    depends on TPM
> +    select TPM_BACKEND
> +
>   config TPM_SPAPR
>       bool
>       default y
> diff --git a/hw/tpm/meson.build b/hw/tpm/meson.build
> index cb8204d5bc..d96de92c16 100644
> --- a/hw/tpm/meson.build
> +++ b/hw/tpm/meson.build
> @@ -4,6 +4,8 @@ system_ss.add(when: 'CONFIG_TPM_TIS_SYSBUS', if_true: files('tpm_tis_sysbus.c'))
>   system_ss.add(when: 'CONFIG_TPM_TIS_I2C', if_true: files('tpm_tis_i2c.c'))
>   system_ss.add(when: 'CONFIG_TPM_CRB', if_true: files('tpm_crb.c'))
>   system_ss.add(when: 'CONFIG_TPM_CRB', if_true: files('tpm_crb_common.c'))
> +system_ss.add(when: 'CONFIG_TPM_CRB_SYSBUS', if_true: files('tpm_crb_sysbus.c'))
> +system_ss.add(when: 'CONFIG_TPM_CRB_SYSBUS', if_true: files('tpm_crb_common.c'))
>   system_ss.add(when: 'CONFIG_TPM_TIS', if_true: files('tpm_ppi.c'))
>   system_ss.add(when: 'CONFIG_TPM_CRB', if_true: files('tpm_ppi.c'))
>
Joelle van Dyne July 14, 2023, 5:20 p.m. UTC | #2
On Fri, Jul 14, 2023 at 7:27 AM Stefan Berger <stefanb@linux.ibm.com> wrote:
>
>
>
> On 7/14/23 03:09, Joelle van Dyne wrote:
> > This SysBus variant of the CRB interface supports dynamically locating
> > the MMIO interface so that Virt machines can use it. This interface
> > is currently the only one supported by QEMU that works on Windows 11
> > ARM64. We largely follow the TPM TIS SysBus device as a template.
> >
> > To try out this device with Windows 11 before OVMF is updated, you
> > will need to modify `sysbud-fdt.c` and change the added line from:
> >
> > ```c
> >      TYPE_BINDING(TYPE_TPM_CRB_SYSBUS, no_fdt_node),
> > ```
> >
> > to
> >
> > ```c
> >      TYPE_BINDING(TYPE_TPM_CRB_SYSBUS, add_tpm_tis_fdt_node),
> > ```
> >
> > This change was not included because it can confuse Linux (although
> > from testing, it seems like Linux is able to properly ignore the
> > device from the TPM TIS driver and recognize it from the ACPI device
> > in the TPM CRB driver). A proper fix would require OVMF to recognize
> > the ACPI device and not depend on the FDT node for recognizing TPM.
> >
> > The command line to try out this device with SWTPM is:
> >
> > ```
> > $ qemu-system-aarch64 \
> >      -chardev socket,id=chrtpm0,path=tpm.sock \
> >      -tpmdev emulator,id=tpm0,chardev=chrtpm0 \
> >      -device tpm-crb-device,tpmdev=tpm0
> > ```
> >
> > along with SWTPM:
> >
> > ```
> > $ swtpm \
> >      --ctrl type=unixio,path=tpm.sock,terminate \
> >      --tpmstate backend-uri=file://tpm.data \
> >      --tpm2
> > ```
> >
> > Signed-off-by: Joelle van Dyne <j@getutm.app>
> > ---
> >   docs/specs/tpm.rst          |   1 +
> >   include/hw/acpi/aml-build.h |   1 +
> >   include/sysemu/tpm.h        |   3 +
> >   hw/acpi/aml-build.c         |   7 +-
> >   hw/arm/virt.c               |   1 +
> >   hw/core/sysbus-fdt.c        |   1 +
> >   hw/loongarch/virt.c         |   1 +
> >   hw/riscv/virt.c             |   1 +
> >   hw/tpm/tpm_crb_sysbus.c     | 170 ++++++++++++++++++++++++++++++++++++
> >   hw/arm/Kconfig              |   1 +
> >   hw/riscv/Kconfig            |   1 +
> >   hw/tpm/Kconfig              |   5 ++
> >   hw/tpm/meson.build          |   2 +
> >   13 files changed, 194 insertions(+), 1 deletion(-)
> >   create mode 100644 hw/tpm/tpm_crb_sysbus.c
> >
> > diff --git a/docs/specs/tpm.rst b/docs/specs/tpm.rst
> > index 2bc29c9804..95aeb49220 100644
> > --- a/docs/specs/tpm.rst
> > +++ b/docs/specs/tpm.rst
> > @@ -46,6 +46,7 @@ operating system.
> >   QEMU files related to TPM CRB interface:
> >    - ``hw/tpm/tpm_crb.c``
> >    - ``hw/tpm/tpm_crb_common.c``
> > + - ``hw/tpm/tpm_crb_sysbus.c``
> >
>
> If you added the command line to use for Windows guests to this document
> I think this would be helpful. And also mention that this must be used for Windows
> since the other ones don't work.
>
>
> >   SPAPR interface
> >   ---------------
> > diff --git a/include/hw/acpi/aml-build.h b/include/hw/acpi/aml-build.h
> > index d1fb08514b..9660e16148 100644
> > --- a/include/hw/acpi/aml-build.h
> > +++ b/include/hw/acpi/aml-build.h
> > @@ -3,6 +3,7 @@
> >
> >   #include "hw/acpi/acpi-defs.h"
> >   #include "hw/acpi/bios-linker-loader.h"
> > +#include "exec/hwaddr.h"
> >
> >   #define ACPI_BUILD_APPNAME6 "BOCHS "
> >   #define ACPI_BUILD_APPNAME8 "BXPC    "
> > diff --git a/include/sysemu/tpm.h b/include/sysemu/tpm.h
> > index 66e3b45f30..f79c8f3575 100644
> > --- a/include/sysemu/tpm.h
> > +++ b/include/sysemu/tpm.h
> > @@ -47,6 +47,7 @@ struct TPMIfClass {
> >   #define TYPE_TPM_TIS_ISA            "tpm-tis"
> >   #define TYPE_TPM_TIS_SYSBUS         "tpm-tis-device"
> >   #define TYPE_TPM_CRB                "tpm-crb"
> > +#define TYPE_TPM_CRB_SYSBUS         "tpm-crb-device"
> >   #define TYPE_TPM_SPAPR              "tpm-spapr"
> >   #define TYPE_TPM_TIS_I2C            "tpm-tis-i2c"
> >
> > @@ -56,6 +57,8 @@ struct TPMIfClass {
> >       object_dynamic_cast(OBJECT(chr), TYPE_TPM_TIS_SYSBUS)
> >   #define TPM_IS_CRB(chr)                             \
> >       object_dynamic_cast(OBJECT(chr), TYPE_TPM_CRB)
> > +#define TPM_IS_CRB_SYSBUS(chr)                      \
> > +    object_dynamic_cast(OBJECT(chr), TYPE_TPM_CRB_SYSBUS)
> >   #define TPM_IS_SPAPR(chr)                           \
> >       object_dynamic_cast(OBJECT(chr), TYPE_TPM_SPAPR)
> >   #define TPM_IS_TIS_I2C(chr)                      \
> > diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
> > index ea331a20d1..f809137fc9 100644
> > --- a/hw/acpi/aml-build.c
> > +++ b/hw/acpi/aml-build.c
> > @@ -31,6 +31,7 @@
> >   #include "hw/pci/pci_bus.h"
> >   #include "hw/pci/pci_bridge.h"
> >   #include "qemu/cutils.h"
> > +#include "qom/object.h"
> >
> >   static GArray *build_alloc_array(void)
> >   {
> > @@ -2218,7 +2219,7 @@ void build_tpm2(GArray *table_data, BIOSLinker *linker, GArray *tcpalog,
> >   {
> >       uint8_t start_method_params[12] = {};
> >       unsigned log_addr_offset;
> > -    uint64_t control_area_start_address;
> > +    uint64_t baseaddr, control_area_start_address;
> >       TPMIf *tpmif = tpm_find();
> >       uint32_t start_method;
> >       AcpiTable table = { .sig = "TPM2", .rev = 4,
> > @@ -2236,6 +2237,10 @@ void build_tpm2(GArray *table_data, BIOSLinker *linker, GArray *tcpalog,
> >       } else if (TPM_IS_CRB(tpmif)) {
> >           control_area_start_address = TPM_CRB_ADDR_CTRL;
> >           start_method = TPM2_START_METHOD_CRB;
> > +    } else if (TPM_IS_CRB_SYSBUS(tpmif)) {
> > +        baseaddr = object_property_get_uint(OBJECT(tpmif), "baseaddr", NULL);
> > +        control_area_start_address = baseaddr + A_CRB_CTRL_REQ;
> > +        start_method = TPM2_START_METHOD_CRB;
> >       } else {
> >           g_assert_not_reached();
> >       }
> > diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> > index 432148ef47..88e8b16103 100644
> > --- a/hw/arm/virt.c
> > +++ b/hw/arm/virt.c
> > @@ -2977,6 +2977,7 @@ static void virt_machine_class_init(ObjectClass *oc, void *data)
> >       machine_class_allow_dynamic_sysbus_dev(mc, TYPE_VFIO_PLATFORM);
> >   #ifdef CONFIG_TPM
> >       machine_class_allow_dynamic_sysbus_dev(mc, TYPE_TPM_TIS_SYSBUS);
> > +    machine_class_allow_dynamic_sysbus_dev(mc, TYPE_TPM_CRB_SYSBUS);
> >   #endif
> >       mc->block_default_type = IF_VIRTIO;
> >       mc->no_cdrom = 1;
> > diff --git a/hw/core/sysbus-fdt.c b/hw/core/sysbus-fdt.c
> > index eebcd28f9a..9c783f88eb 100644
> > --- a/hw/core/sysbus-fdt.c
> > +++ b/hw/core/sysbus-fdt.c
> > @@ -493,6 +493,7 @@ static const BindingEntry bindings[] = {
> >   #endif
> >   #ifdef CONFIG_TPM
> >       TYPE_BINDING(TYPE_TPM_TIS_SYSBUS, add_tpm_tis_fdt_node),
> > +    TYPE_BINDING(TYPE_TPM_CRB_SYSBUS, no_fdt_node),
> >   #endif
> >       TYPE_BINDING(TYPE_RAMFB_DEVICE, no_fdt_node),
> >       TYPE_BINDING("", NULL), /* last element */
> > diff --git a/hw/loongarch/virt.c b/hw/loongarch/virt.c
> > index 9c536c52bc..eb59fb04ee 100644
> > --- a/hw/loongarch/virt.c
> > +++ b/hw/loongarch/virt.c
> > @@ -1194,6 +1194,7 @@ static void loongarch_class_init(ObjectClass *oc, void *data)
> >       machine_class_allow_dynamic_sysbus_dev(mc, TYPE_RAMFB_DEVICE);
> >   #ifdef CONFIG_TPM
> >       machine_class_allow_dynamic_sysbus_dev(mc, TYPE_TPM_TIS_SYSBUS);
> > +    machine_class_allow_dynamic_sysbus_dev(mc, TYPE_TPM_CRB_SYSBUS);
> >   #endif
> >   }
> >
> > diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
> > index d90286dc46..5d639a870a 100644
> > --- a/hw/riscv/virt.c
> > +++ b/hw/riscv/virt.c
> > @@ -1681,6 +1681,7 @@ static void virt_machine_class_init(ObjectClass *oc, void *data)
> >       machine_class_allow_dynamic_sysbus_dev(mc, TYPE_RAMFB_DEVICE);
> >   #ifdef CONFIG_TPM
> >       machine_class_allow_dynamic_sysbus_dev(mc, TYPE_TPM_TIS_SYSBUS);
> > +    machine_class_allow_dynamic_sysbus_dev(mc, TYPE_TPM_CRB_SYSBUS);
> >   #endif
> >
> >       if (tcg_enabled()) {
> > diff --git a/hw/tpm/tpm_crb_sysbus.c b/hw/tpm/tpm_crb_sysbus.c
> > new file mode 100644
> > index 0000000000..66be57a532
> > --- /dev/null
> > +++ b/hw/tpm/tpm_crb_sysbus.c
> > @@ -0,0 +1,170 @@
> > +/*
> > + * tpm_crb_sysbus.c - QEMU's TPM CRB interface emulator
> > + *
> > + * Copyright (c) 2018 Red Hat, Inc.
> > + *
> > + * Authors:
> > + *   Marc-André Lureau <marcandre.lureau@redhat.com>
> > + *
> > + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> > + * See the COPYING file in the top-level directory.
> > + *
> > + * tpm_crb is a device for TPM 2.0 Command Response Buffer (CRB) Interface
> > + * as defined in TCG PC Client Platform TPM Profile (PTP) Specification
> > + * Family “2.0” Level 00 Revision 01.03 v22
> > + */
> > +
> > +#include "qemu/osdep.h"
> > +#include "hw/acpi/acpi_aml_interface.h"
> > +#include "hw/acpi/tpm.h"
> > +#include "hw/qdev-properties.h"
> > +#include "migration/vmstate.h"
> > +#include "tpm_prop.h"
> > +#include "hw/pci/pci_ids.h"
> > +#include "hw/sysbus.h"
> > +#include "qapi/visitor.h"
> > +#include "qom/object.h"
> > +#include "sysemu/tpm_util.h"
> > +#include "trace.h"
> > +#include "tpm_crb.h"
> > +
> > +struct TPMCRBStateSysBus {
> > +    /*< private >*/
> > +    SysBusDevice parent_obj;
> > +
> > +    /*< public >*/
> > +    TPMCRBState state;
> > +    uint64_t baseaddr;
> > +    uint64_t size;
> > +};
> > +
> > +OBJECT_DECLARE_SIMPLE_TYPE(TPMCRBStateSysBus, TPM_CRB_SYSBUS)
> > +
> > +static void tpm_crb_sysbus_request_completed(TPMIf *ti, int ret)
> > +{
> > +    TPMCRBStateSysBus *s = TPM_CRB_SYSBUS(ti);
> > +
> > +    return tpm_crb_request_completed(&s->state, ret);
> > +}
> > +
> > +static enum TPMVersion tpm_crb_sysbus_get_tpm_version(TPMIf *ti)
> > +{
> > +    TPMCRBStateSysBus *s = TPM_CRB_SYSBUS(ti);
> > +
> > +    return tpm_crb_get_version(&s->state);
> > +}
> > +
> > +static int tpm_crb_sysbus_pre_save(void *opaque)
> > +{
> > +    TPMCRBStateSysBus *s = opaque;
> > +
> > +    return tpm_crb_pre_save(&s->state);
> > +}
> > +
> > +static const VMStateDescription vmstate_tpm_crb_sysbus = {
> > +    .name = "tpm-crb-sysbus",
> > +    .pre_save = tpm_crb_sysbus_pre_save,
> > +    .fields = (VMStateField[]) {
> > +        VMSTATE_END_OF_LIST(),
> > +    }
> > +};
> > +
> > +static Property tpm_crb_sysbus_properties[] = {
> > +    DEFINE_PROP_TPMBE("tpmdev", TPMCRBStateSysBus, state.tpmbe),
> > +    DEFINE_PROP_UINT64("baseaddr", TPMCRBStateSysBus,
> > +                       baseaddr, TPM_CRB_ADDR_BASE),
> > +    DEFINE_PROP_UINT64("size", TPMCRBStateSysBus, size, TPM_CRB_ADDR_SIZE),
> > +    DEFINE_PROP_END_OF_LIST(),
> > +};
> > +
> > +static void tpm_crb_sysbus_initfn(Object *obj)
> > +{
> > +    TPMCRBStateSysBus *s = TPM_CRB_SYSBUS(obj);
> > +
> > +    tpm_crb_init_memory(obj, &s->state, NULL);
> > +
> > +    sysbus_init_mmio(SYS_BUS_DEVICE(obj), &s->state.mmio);
> > +}
> > +
> > +static void tpm_crb_sysbus_reset(DeviceState *dev)
> > +{
> > +    TPMCRBStateSysBus *s = TPM_CRB_SYSBUS(dev);
> > +
> > +    return tpm_crb_reset(&s->state, s->baseaddr);
> > +}
> > +
> > +static void tpm_crb_sysbus_realizefn(DeviceState *dev, Error **errp)
> > +{
> > +    TPMCRBStateSysBus *s = TPM_CRB_SYSBUS(dev);
> > +
> > +    if (!tpm_find()) {
> > +        error_setg(errp, "at most one TPM device is permitted");
> > +        return;
> > +    }
> > +
> > +    if (!s->state.tpmbe) {
> > +        error_setg(errp, "'tpmdev' property is required");
> > +        return;
> > +    }
> > +
> > +    if (tpm_crb_sysbus_get_tpm_version(TPM_IF(s)) != TPM_VERSION_2_0) {
> > +        error_setg(errp, "TPM CRB only supports TPM 2.0 backends");
> > +        return;
> > +    }
> > +}
> > +
> > +static void build_tpm_crb_sysbus_aml(AcpiDevAmlIf *adev, Aml *scope)
> > +{
> > +    Aml *dev, *crs;
> > +    TPMCRBStateSysBus *s = TPM_CRB_SYSBUS(adev);
> > +    TPMIf *ti = TPM_IF(s);
>
> ../hw/tpm/tpm_crb_sysbus.c: In function 'build_tpm_crb_sysbus_aml':
> ../hw/tpm/tpm_crb_sysbus.c:120:12: error: unused variable 'ti' [-Werror=unused-variable]
>    120 |     TPMIf *ti = TPM_IF(s);
>        |            ^~
>
>
> Rest LGTM.
>
> > +
> > +    dev = aml_device("TPM");
> > +    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(1)));
> > +    aml_append(dev, aml_name_decl("_STA", aml_int(0xF)));
> > +    crs = aml_resource_template();
> > +    aml_append(crs, aml_memory32_fixed(s->baseaddr, s->size,
> > +                                      AML_READ_WRITE));
> > +    aml_append(dev, aml_name_decl("_CRS", crs));
> > +    aml_append(scope, dev);
> > +}
> > +
> > +static void tpm_crb_sysbus_class_init(ObjectClass *klass, void *data)
> > +{
> > +    DeviceClass *dc = DEVICE_CLASS(klass);
> > +    TPMIfClass *tc = TPM_IF_CLASS(klass);
> > +    AcpiDevAmlIfClass *adevc = ACPI_DEV_AML_IF_CLASS(klass);
> > +
> > +    device_class_set_props(dc, tpm_crb_sysbus_properties);
> > +    dc->vmsd  = &vmstate_tpm_crb_sysbus;
> > +    tc->model = TPM_MODEL_TPM_CRB;
> > +    dc->realize = tpm_crb_sysbus_realizefn;
> > +    dc->user_creatable = true;
> > +    dc->reset = tpm_crb_sysbus_reset;
> > +    tc->request_completed = tpm_crb_sysbus_request_completed;
> > +    tc->get_version = tpm_crb_sysbus_get_tpm_version;
> > +    set_bit(DEVICE_CATEGORY_MISC, dc->categories);
> > +    adevc->build_dev_aml = build_tpm_crb_sysbus_aml;
> > +}
> > +
> > +static const TypeInfo tpm_crb_sysbus_info = {
> > +    .name = TYPE_TPM_CRB_SYSBUS,
> > +    .parent = TYPE_SYS_BUS_DEVICE,
> > +    .instance_size = sizeof(TPMCRBStateSysBus),
> > +    .instance_init = tpm_crb_sysbus_initfn,
> > +    .class_init  = tpm_crb_sysbus_class_init,
> > +    .interfaces = (InterfaceInfo[]) {
> > +        { TYPE_TPM_IF },
> > +        { TYPE_ACPI_DEV_AML_IF },
> > +        { }
> > +    }
> > +};
> > +
> > +static void tpm_crb_sysbus_register(void)
> > +{
> > +    type_register_static(&tpm_crb_sysbus_info);
> > +}
> > +
> > +type_init(tpm_crb_sysbus_register)
> > diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
> > index 7e68348440..efe1beaa7b 100644
> > --- a/hw/arm/Kconfig
> > +++ b/hw/arm/Kconfig
> > @@ -5,6 +5,7 @@ config ARM_VIRT
> >       imply VFIO_AMD_XGBE
> >       imply VFIO_PLATFORM
> >       imply VFIO_XGMAC
> > +    imply TPM_CRB_SYSBUS
> >       imply TPM_TIS_SYSBUS
> >       imply TPM_TIS_I2C
> >       imply NVDIMM
> > diff --git a/hw/riscv/Kconfig b/hw/riscv/Kconfig
> > index b6a5eb4452..d824cb58f9 100644
> > --- a/hw/riscv/Kconfig
> > +++ b/hw/riscv/Kconfig
> > @@ -29,6 +29,7 @@ config RISCV_VIRT
> >       imply PCI_DEVICES
> >       imply VIRTIO_VGA
> >       imply TEST_DEVICES
> > +    imply TPM_CRB_SYSBUS
> >       imply TPM_TIS_SYSBUS
> >       select RISCV_NUMA
> >       select GOLDFISH_RTC
> > diff --git a/hw/tpm/Kconfig b/hw/tpm/Kconfig
> > index 1fd73fe617..3f294a20ba 100644
> > --- a/hw/tpm/Kconfig
> > +++ b/hw/tpm/Kconfig
> > @@ -25,6 +25,11 @@ config TPM_CRB
> >       depends on TPM && ISA_BUS
> >       select TPM_BACKEND
> >
> > +config TPM_CRB_SYSBUS
> > +    bool
> > +    depends on TPM
> > +    select TPM_BACKEND
> > +
> >   config TPM_SPAPR
> >       bool
> >       default y
> > diff --git a/hw/tpm/meson.build b/hw/tpm/meson.build
> > index cb8204d5bc..d96de92c16 100644
> > --- a/hw/tpm/meson.build
> > +++ b/hw/tpm/meson.build
> > @@ -4,6 +4,8 @@ system_ss.add(when: 'CONFIG_TPM_TIS_SYSBUS', if_true: files('tpm_tis_sysbus.c'))
> >   system_ss.add(when: 'CONFIG_TPM_TIS_I2C', if_true: files('tpm_tis_i2c.c'))
> >   system_ss.add(when: 'CONFIG_TPM_CRB', if_true: files('tpm_crb.c'))
> >   system_ss.add(when: 'CONFIG_TPM_CRB', if_true: files('tpm_crb_common.c'))
> > +system_ss.add(when: 'CONFIG_TPM_CRB_SYSBUS', if_true: files('tpm_crb_sysbus.c'))
> > +system_ss.add(when: 'CONFIG_TPM_CRB_SYSBUS', if_true: files('tpm_crb_common.c'))
> >   system_ss.add(when: 'CONFIG_TPM_TIS', if_true: files('tpm_ppi.c'))
> >   system_ss.add(when: 'CONFIG_TPM_CRB', if_true: files('tpm_ppi.c'))
> >

New commit description looks good?

"This SysBus variant of the CRB interface supports dynamically
locating the MMIO interface so that Virt machines can use it. This
interface is currently the only one supported by QEMU that works on
Windows 11 ARM64 as 'tpm-tis-device' does not work with current
Windows drivers. We largely follow that device as a template."

Also, I'm not sure what else you need for the Windows command. Do you
mean the command to initiate a Windows install? Because I feel like
there would be a lot of text there that would be off-topic from the
commit. Additionally, do we also need to include steps on creating the
QCOW boot device, setting up the UEFI secure boot, and so on? Because
there's quite a few steps involved to get a working Windows boot and
I'm sure it's documented in length in various blog posts.
Stefan Berger July 14, 2023, 5:52 p.m. UTC | #3
On 7/14/23 13:20, Joelle van Dyne wrote:
> On Fri, Jul 14, 2023 at 7:27 AM Stefan Berger <stefanb@linux.ibm.com> wrote:
>>
>>
>>
>> On 7/14/23 03:09, Joelle van Dyne wrote:
>>> This SysBus variant of the CRB interface supports dynamically locating
>>> the MMIO interface so that Virt machines can use it. This interface
>>> is currently the only one supported by QEMU that works on Windows 11
>>> ARM64. We largely follow the TPM TIS SysBus device as a template.
>>>
>>> To try out this device with Windows 11 before OVMF is updated, you
>>> will need to modify `sysbud-fdt.c` and change the added line from:
>>>
>>> ```c
>>>       TYPE_BINDING(TYPE_TPM_CRB_SYSBUS, no_fdt_node),
>>> ```
>>>
>>> to
>>>
>>> ```c
>>>       TYPE_BINDING(TYPE_TPM_CRB_SYSBUS, add_tpm_tis_fdt_node),
>>> ```
>>>
>>> This change was not included because it can confuse Linux (although
>>> from testing, it seems like Linux is able to properly ignore the
>>> device from the TPM TIS driver and recognize it from the ACPI device
>>> in the TPM CRB driver). A proper fix would require OVMF to recognize
>>> the ACPI device and not depend on the FDT node for recognizing TPM.
>>>
>>> The command line to try out this device with SWTPM is:
>>>
>>> ```
>>> $ qemu-system-aarch64 \
>>>       -chardev socket,id=chrtpm0,path=tpm.sock \
>>>       -tpmdev emulator,id=tpm0,chardev=chrtpm0 \
>>>       -device tpm-crb-device,tpmdev=tpm0
>>> ```
>>>
>>> along with SWTPM:
>>>
>>> ```
>>> $ swtpm \
>>>       --ctrl type=unixio,path=tpm.sock,terminate \
>>>       --tpmstate backend-uri=file://tpm.data \
>>>       --tpm2
>>> ```
>>>
>>> Signed-off-by: Joelle van Dyne <j@getutm.app>
>>> ---
>>>    docs/specs/tpm.rst          |   1 +
>>>    include/hw/acpi/aml-build.h |   1 +
>>>    include/sysemu/tpm.h        |   3 +
>>>    hw/acpi/aml-build.c         |   7 +-
>>>    hw/arm/virt.c               |   1 +
>>>    hw/core/sysbus-fdt.c        |   1 +
>>>    hw/loongarch/virt.c         |   1 +
>>>    hw/riscv/virt.c             |   1 +
>>>    hw/tpm/tpm_crb_sysbus.c     | 170 ++++++++++++++++++++++++++++++++++++
>>>    hw/arm/Kconfig              |   1 +
>>>    hw/riscv/Kconfig            |   1 +
>>>    hw/tpm/Kconfig              |   5 ++
>>>    hw/tpm/meson.build          |   2 +
>>>    13 files changed, 194 insertions(+), 1 deletion(-)
>>>    create mode 100644 hw/tpm/tpm_crb_sysbus.c
>>>
>>> diff --git a/docs/specs/tpm.rst b/docs/specs/tpm.rst
>>> index 2bc29c9804..95aeb49220 100644
>>> --- a/docs/specs/tpm.rst
>>> +++ b/docs/specs/tpm.rst
>>> @@ -46,6 +46,7 @@ operating system.
>>>    QEMU files related to TPM CRB interface:
>>>     - ``hw/tpm/tpm_crb.c``
>>>     - ``hw/tpm/tpm_crb_common.c``
>>> + - ``hw/tpm/tpm_crb_sysbus.c``
>>>
>>
>> If you added the command line to use for Windows guests to this document
>> I think this would be helpful. And also mention that this must be used for Windows
>> since the other ones don't work.
>>
>>
>>>    SPAPR interface
>>>    ---------------
>>> diff --git a/include/hw/acpi/aml-build.h b/include/hw/acpi/aml-build.h
>>> index d1fb08514b..9660e16148 100644
>>> --- a/include/hw/acpi/aml-build.h
>>> +++ b/include/hw/acpi/aml-build.h
>>> @@ -3,6 +3,7 @@
>>>
>>>    #include "hw/acpi/acpi-defs.h"
>>>    #include "hw/acpi/bios-linker-loader.h"
>>> +#include "exec/hwaddr.h"
>>>
>>>    #define ACPI_BUILD_APPNAME6 "BOCHS "
>>>    #define ACPI_BUILD_APPNAME8 "BXPC    "
>>> diff --git a/include/sysemu/tpm.h b/include/sysemu/tpm.h
>>> index 66e3b45f30..f79c8f3575 100644
>>> --- a/include/sysemu/tpm.h
>>> +++ b/include/sysemu/tpm.h
>>> @@ -47,6 +47,7 @@ struct TPMIfClass {
>>>    #define TYPE_TPM_TIS_ISA            "tpm-tis"
>>>    #define TYPE_TPM_TIS_SYSBUS         "tpm-tis-device"
>>>    #define TYPE_TPM_CRB                "tpm-crb"
>>> +#define TYPE_TPM_CRB_SYSBUS         "tpm-crb-device"
>>>    #define TYPE_TPM_SPAPR              "tpm-spapr"
>>>    #define TYPE_TPM_TIS_I2C            "tpm-tis-i2c"
>>>
>>> @@ -56,6 +57,8 @@ struct TPMIfClass {
>>>        object_dynamic_cast(OBJECT(chr), TYPE_TPM_TIS_SYSBUS)
>>>    #define TPM_IS_CRB(chr)                             \
>>>        object_dynamic_cast(OBJECT(chr), TYPE_TPM_CRB)
>>> +#define TPM_IS_CRB_SYSBUS(chr)                      \
>>> +    object_dynamic_cast(OBJECT(chr), TYPE_TPM_CRB_SYSBUS)
>>>    #define TPM_IS_SPAPR(chr)                           \
>>>        object_dynamic_cast(OBJECT(chr), TYPE_TPM_SPAPR)
>>>    #define TPM_IS_TIS_I2C(chr)                      \
>>> diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
>>> index ea331a20d1..f809137fc9 100644
>>> --- a/hw/acpi/aml-build.c
>>> +++ b/hw/acpi/aml-build.c
>>> @@ -31,6 +31,7 @@
>>>    #include "hw/pci/pci_bus.h"
>>>    #include "hw/pci/pci_bridge.h"
>>>    #include "qemu/cutils.h"
>>> +#include "qom/object.h"
>>>
>>>    static GArray *build_alloc_array(void)
>>>    {
>>> @@ -2218,7 +2219,7 @@ void build_tpm2(GArray *table_data, BIOSLinker *linker, GArray *tcpalog,
>>>    {
>>>        uint8_t start_method_params[12] = {};
>>>        unsigned log_addr_offset;
>>> -    uint64_t control_area_start_address;
>>> +    uint64_t baseaddr, control_area_start_address;
>>>        TPMIf *tpmif = tpm_find();
>>>        uint32_t start_method;
>>>        AcpiTable table = { .sig = "TPM2", .rev = 4,
>>> @@ -2236,6 +2237,10 @@ void build_tpm2(GArray *table_data, BIOSLinker *linker, GArray *tcpalog,
>>>        } else if (TPM_IS_CRB(tpmif)) {
>>>            control_area_start_address = TPM_CRB_ADDR_CTRL;
>>>            start_method = TPM2_START_METHOD_CRB;
>>> +    } else if (TPM_IS_CRB_SYSBUS(tpmif)) {
>>> +        baseaddr = object_property_get_uint(OBJECT(tpmif), "baseaddr", NULL);
>>> +        control_area_start_address = baseaddr + A_CRB_CTRL_REQ;
>>> +        start_method = TPM2_START_METHOD_CRB;
>>>        } else {
>>>            g_assert_not_reached();
>>>        }
>>> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
>>> index 432148ef47..88e8b16103 100644
>>> --- a/hw/arm/virt.c
>>> +++ b/hw/arm/virt.c
>>> @@ -2977,6 +2977,7 @@ static void virt_machine_class_init(ObjectClass *oc, void *data)
>>>        machine_class_allow_dynamic_sysbus_dev(mc, TYPE_VFIO_PLATFORM);
>>>    #ifdef CONFIG_TPM
>>>        machine_class_allow_dynamic_sysbus_dev(mc, TYPE_TPM_TIS_SYSBUS);
>>> +    machine_class_allow_dynamic_sysbus_dev(mc, TYPE_TPM_CRB_SYSBUS);
>>>    #endif
>>>        mc->block_default_type = IF_VIRTIO;
>>>        mc->no_cdrom = 1;
>>> diff --git a/hw/core/sysbus-fdt.c b/hw/core/sysbus-fdt.c
>>> index eebcd28f9a..9c783f88eb 100644
>>> --- a/hw/core/sysbus-fdt.c
>>> +++ b/hw/core/sysbus-fdt.c
>>> @@ -493,6 +493,7 @@ static const BindingEntry bindings[] = {
>>>    #endif
>>>    #ifdef CONFIG_TPM
>>>        TYPE_BINDING(TYPE_TPM_TIS_SYSBUS, add_tpm_tis_fdt_node),
>>> +    TYPE_BINDING(TYPE_TPM_CRB_SYSBUS, no_fdt_node),
>>>    #endif
>>>        TYPE_BINDING(TYPE_RAMFB_DEVICE, no_fdt_node),
>>>        TYPE_BINDING("", NULL), /* last element */
>>> diff --git a/hw/loongarch/virt.c b/hw/loongarch/virt.c
>>> index 9c536c52bc..eb59fb04ee 100644
>>> --- a/hw/loongarch/virt.c
>>> +++ b/hw/loongarch/virt.c
>>> @@ -1194,6 +1194,7 @@ static void loongarch_class_init(ObjectClass *oc, void *data)
>>>        machine_class_allow_dynamic_sysbus_dev(mc, TYPE_RAMFB_DEVICE);
>>>    #ifdef CONFIG_TPM
>>>        machine_class_allow_dynamic_sysbus_dev(mc, TYPE_TPM_TIS_SYSBUS);
>>> +    machine_class_allow_dynamic_sysbus_dev(mc, TYPE_TPM_CRB_SYSBUS);
>>>    #endif
>>>    }
>>>
>>> diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
>>> index d90286dc46..5d639a870a 100644
>>> --- a/hw/riscv/virt.c
>>> +++ b/hw/riscv/virt.c
>>> @@ -1681,6 +1681,7 @@ static void virt_machine_class_init(ObjectClass *oc, void *data)
>>>        machine_class_allow_dynamic_sysbus_dev(mc, TYPE_RAMFB_DEVICE);
>>>    #ifdef CONFIG_TPM
>>>        machine_class_allow_dynamic_sysbus_dev(mc, TYPE_TPM_TIS_SYSBUS);
>>> +    machine_class_allow_dynamic_sysbus_dev(mc, TYPE_TPM_CRB_SYSBUS);
>>>    #endif
>>>
>>>        if (tcg_enabled()) {
>>> diff --git a/hw/tpm/tpm_crb_sysbus.c b/hw/tpm/tpm_crb_sysbus.c
>>> new file mode 100644
>>> index 0000000000..66be57a532
>>> --- /dev/null
>>> +++ b/hw/tpm/tpm_crb_sysbus.c
>>> @@ -0,0 +1,170 @@
>>> +/*
>>> + * tpm_crb_sysbus.c - QEMU's TPM CRB interface emulator
>>> + *
>>> + * Copyright (c) 2018 Red Hat, Inc.
>>> + *
>>> + * Authors:
>>> + *   Marc-André Lureau <marcandre.lureau@redhat.com>
>>> + *
>>> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
>>> + * See the COPYING file in the top-level directory.
>>> + *
>>> + * tpm_crb is a device for TPM 2.0 Command Response Buffer (CRB) Interface
>>> + * as defined in TCG PC Client Platform TPM Profile (PTP) Specification
>>> + * Family “2.0” Level 00 Revision 01.03 v22
>>> + */
>>> +
>>> +#include "qemu/osdep.h"
>>> +#include "hw/acpi/acpi_aml_interface.h"
>>> +#include "hw/acpi/tpm.h"
>>> +#include "hw/qdev-properties.h"
>>> +#include "migration/vmstate.h"
>>> +#include "tpm_prop.h"
>>> +#include "hw/pci/pci_ids.h"
>>> +#include "hw/sysbus.h"
>>> +#include "qapi/visitor.h"
>>> +#include "qom/object.h"
>>> +#include "sysemu/tpm_util.h"
>>> +#include "trace.h"
>>> +#include "tpm_crb.h"
>>> +
>>> +struct TPMCRBStateSysBus {
>>> +    /*< private >*/
>>> +    SysBusDevice parent_obj;
>>> +
>>> +    /*< public >*/
>>> +    TPMCRBState state;
>>> +    uint64_t baseaddr;
>>> +    uint64_t size;
>>> +};
>>> +
>>> +OBJECT_DECLARE_SIMPLE_TYPE(TPMCRBStateSysBus, TPM_CRB_SYSBUS)
>>> +
>>> +static void tpm_crb_sysbus_request_completed(TPMIf *ti, int ret)
>>> +{
>>> +    TPMCRBStateSysBus *s = TPM_CRB_SYSBUS(ti);
>>> +
>>> +    return tpm_crb_request_completed(&s->state, ret);
>>> +}
>>> +
>>> +static enum TPMVersion tpm_crb_sysbus_get_tpm_version(TPMIf *ti)
>>> +{
>>> +    TPMCRBStateSysBus *s = TPM_CRB_SYSBUS(ti);
>>> +
>>> +    return tpm_crb_get_version(&s->state);
>>> +}
>>> +
>>> +static int tpm_crb_sysbus_pre_save(void *opaque)
>>> +{
>>> +    TPMCRBStateSysBus *s = opaque;
>>> +
>>> +    return tpm_crb_pre_save(&s->state);
>>> +}
>>> +
>>> +static const VMStateDescription vmstate_tpm_crb_sysbus = {
>>> +    .name = "tpm-crb-sysbus",
>>> +    .pre_save = tpm_crb_sysbus_pre_save,
>>> +    .fields = (VMStateField[]) {
>>> +        VMSTATE_END_OF_LIST(),
>>> +    }
>>> +};
>>> +
>>> +static Property tpm_crb_sysbus_properties[] = {
>>> +    DEFINE_PROP_TPMBE("tpmdev", TPMCRBStateSysBus, state.tpmbe),
>>> +    DEFINE_PROP_UINT64("baseaddr", TPMCRBStateSysBus,
>>> +                       baseaddr, TPM_CRB_ADDR_BASE),
>>> +    DEFINE_PROP_UINT64("size", TPMCRBStateSysBus, size, TPM_CRB_ADDR_SIZE),
>>> +    DEFINE_PROP_END_OF_LIST(),
>>> +};
>>> +
>>> +static void tpm_crb_sysbus_initfn(Object *obj)
>>> +{
>>> +    TPMCRBStateSysBus *s = TPM_CRB_SYSBUS(obj);
>>> +
>>> +    tpm_crb_init_memory(obj, &s->state, NULL);
>>> +
>>> +    sysbus_init_mmio(SYS_BUS_DEVICE(obj), &s->state.mmio);
>>> +}
>>> +
>>> +static void tpm_crb_sysbus_reset(DeviceState *dev)
>>> +{
>>> +    TPMCRBStateSysBus *s = TPM_CRB_SYSBUS(dev);
>>> +
>>> +    return tpm_crb_reset(&s->state, s->baseaddr);
>>> +}
>>> +
>>> +static void tpm_crb_sysbus_realizefn(DeviceState *dev, Error **errp)
>>> +{
>>> +    TPMCRBStateSysBus *s = TPM_CRB_SYSBUS(dev);
>>> +
>>> +    if (!tpm_find()) {
>>> +        error_setg(errp, "at most one TPM device is permitted");
>>> +        return;
>>> +    }
>>> +
>>> +    if (!s->state.tpmbe) {
>>> +        error_setg(errp, "'tpmdev' property is required");
>>> +        return;
>>> +    }
>>> +
>>> +    if (tpm_crb_sysbus_get_tpm_version(TPM_IF(s)) != TPM_VERSION_2_0) {
>>> +        error_setg(errp, "TPM CRB only supports TPM 2.0 backends");
>>> +        return;
>>> +    }
>>> +}
>>> +
>>> +static void build_tpm_crb_sysbus_aml(AcpiDevAmlIf *adev, Aml *scope)
>>> +{
>>> +    Aml *dev, *crs;
>>> +    TPMCRBStateSysBus *s = TPM_CRB_SYSBUS(adev);
>>> +    TPMIf *ti = TPM_IF(s);
>>
>> ../hw/tpm/tpm_crb_sysbus.c: In function 'build_tpm_crb_sysbus_aml':
>> ../hw/tpm/tpm_crb_sysbus.c:120:12: error: unused variable 'ti' [-Werror=unused-variable]
>>     120 |     TPMIf *ti = TPM_IF(s);
>>         |            ^~
>>
>>
>> Rest LGTM.
>>
>>> +
>>> +    dev = aml_device("TPM");
>>> +    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(1)));
>>> +    aml_append(dev, aml_name_decl("_STA", aml_int(0xF)));
>>> +    crs = aml_resource_template();
>>> +    aml_append(crs, aml_memory32_fixed(s->baseaddr, s->size,
>>> +                                      AML_READ_WRITE));
>>> +    aml_append(dev, aml_name_decl("_CRS", crs));
>>> +    aml_append(scope, dev);
>>> +}
>>> +
>>> +static void tpm_crb_sysbus_class_init(ObjectClass *klass, void *data)
>>> +{
>>> +    DeviceClass *dc = DEVICE_CLASS(klass);
>>> +    TPMIfClass *tc = TPM_IF_CLASS(klass);
>>> +    AcpiDevAmlIfClass *adevc = ACPI_DEV_AML_IF_CLASS(klass);
>>> +
>>> +    device_class_set_props(dc, tpm_crb_sysbus_properties);
>>> +    dc->vmsd  = &vmstate_tpm_crb_sysbus;
>>> +    tc->model = TPM_MODEL_TPM_CRB;
>>> +    dc->realize = tpm_crb_sysbus_realizefn;
>>> +    dc->user_creatable = true;
>>> +    dc->reset = tpm_crb_sysbus_reset;
>>> +    tc->request_completed = tpm_crb_sysbus_request_completed;
>>> +    tc->get_version = tpm_crb_sysbus_get_tpm_version;
>>> +    set_bit(DEVICE_CATEGORY_MISC, dc->categories);
>>> +    adevc->build_dev_aml = build_tpm_crb_sysbus_aml;
>>> +}
>>> +
>>> +static const TypeInfo tpm_crb_sysbus_info = {
>>> +    .name = TYPE_TPM_CRB_SYSBUS,
>>> +    .parent = TYPE_SYS_BUS_DEVICE,
>>> +    .instance_size = sizeof(TPMCRBStateSysBus),
>>> +    .instance_init = tpm_crb_sysbus_initfn,
>>> +    .class_init  = tpm_crb_sysbus_class_init,
>>> +    .interfaces = (InterfaceInfo[]) {
>>> +        { TYPE_TPM_IF },
>>> +        { TYPE_ACPI_DEV_AML_IF },
>>> +        { }
>>> +    }
>>> +};
>>> +
>>> +static void tpm_crb_sysbus_register(void)
>>> +{
>>> +    type_register_static(&tpm_crb_sysbus_info);
>>> +}
>>> +
>>> +type_init(tpm_crb_sysbus_register)
>>> diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
>>> index 7e68348440..efe1beaa7b 100644
>>> --- a/hw/arm/Kconfig
>>> +++ b/hw/arm/Kconfig
>>> @@ -5,6 +5,7 @@ config ARM_VIRT
>>>        imply VFIO_AMD_XGBE
>>>        imply VFIO_PLATFORM
>>>        imply VFIO_XGMAC
>>> +    imply TPM_CRB_SYSBUS
>>>        imply TPM_TIS_SYSBUS
>>>        imply TPM_TIS_I2C
>>>        imply NVDIMM
>>> diff --git a/hw/riscv/Kconfig b/hw/riscv/Kconfig
>>> index b6a5eb4452..d824cb58f9 100644
>>> --- a/hw/riscv/Kconfig
>>> +++ b/hw/riscv/Kconfig
>>> @@ -29,6 +29,7 @@ config RISCV_VIRT
>>>        imply PCI_DEVICES
>>>        imply VIRTIO_VGA
>>>        imply TEST_DEVICES
>>> +    imply TPM_CRB_SYSBUS
>>>        imply TPM_TIS_SYSBUS
>>>        select RISCV_NUMA
>>>        select GOLDFISH_RTC
>>> diff --git a/hw/tpm/Kconfig b/hw/tpm/Kconfig
>>> index 1fd73fe617..3f294a20ba 100644
>>> --- a/hw/tpm/Kconfig
>>> +++ b/hw/tpm/Kconfig
>>> @@ -25,6 +25,11 @@ config TPM_CRB
>>>        depends on TPM && ISA_BUS
>>>        select TPM_BACKEND
>>>
>>> +config TPM_CRB_SYSBUS
>>> +    bool
>>> +    depends on TPM
>>> +    select TPM_BACKEND
>>> +
>>>    config TPM_SPAPR
>>>        bool
>>>        default y
>>> diff --git a/hw/tpm/meson.build b/hw/tpm/meson.build
>>> index cb8204d5bc..d96de92c16 100644
>>> --- a/hw/tpm/meson.build
>>> +++ b/hw/tpm/meson.build
>>> @@ -4,6 +4,8 @@ system_ss.add(when: 'CONFIG_TPM_TIS_SYSBUS', if_true: files('tpm_tis_sysbus.c'))
>>>    system_ss.add(when: 'CONFIG_TPM_TIS_I2C', if_true: files('tpm_tis_i2c.c'))
>>>    system_ss.add(when: 'CONFIG_TPM_CRB', if_true: files('tpm_crb.c'))
>>>    system_ss.add(when: 'CONFIG_TPM_CRB', if_true: files('tpm_crb_common.c'))
>>> +system_ss.add(when: 'CONFIG_TPM_CRB_SYSBUS', if_true: files('tpm_crb_sysbus.c'))
>>> +system_ss.add(when: 'CONFIG_TPM_CRB_SYSBUS', if_true: files('tpm_crb_common.c'))
>>>    system_ss.add(when: 'CONFIG_TPM_TIS', if_true: files('tpm_ppi.c'))
>>>    system_ss.add(when: 'CONFIG_TPM_CRB', if_true: files('tpm_ppi.c'))
>>>
> 
> New commit description looks good?
> 
> "This SysBus variant of the CRB interface supports dynamically
> locating the MMIO interface so that Virt machines can use it. This
> interface is currently the only one supported by QEMU that works on
> Windows 11 ARM64 as 'tpm-tis-device' does not work with current
> Windows drivers. We largely follow that device as a template."

Sounds good.
> 
> Also, I'm not sure what else you need for the Windows command. Do you
> mean the command to initiate a Windows install? Because I feel like

No. docs/spec/tpm.rst has this here at the moment for aarch64:

In case an Arm virt machine is emulated, use the following command line:

.. code-block:: console

   qemu-system-aarch64 -machine virt,gic-version=3,accel=kvm \
     -cpu host -m 4G \
     -nographic -no-acpi \
     -chardev socket,id=chrtpm,path=/tmp/mytpm1/swtpm-sock \
     -tpmdev emulator,id=tpm0,chardev=chrtpm \
     -device tpm-tis-device,tpmdev=tpm0 \
     -device virtio-blk-pci,drive=drv0 \
     -drive format=qcow2,file=hda.qcow2,if=none,id=drv0 \
     -drive if=pflash,format=raw,file=flash0.img,readonly=on \
     -drive if=pflash,format=raw,file=flash1.img


Above his I would put:

In case an Arm virt machine is emulated to run Windows or Linux, use the following command line:

.. code-block:: console

   qemu-system-aarch64 -machine virt,gic-version=3,accel=kvm \
     -cpu host -m 4G \
     -nographic -no-acpi \
     -chardev socket,id=chrtpm,path=/tmp/mytpm1/swtpm-sock \
     -tpmdev emulator,id=tpm0,chardev=chrtpm \
     -device tpm-crb-device,tpmdev=tpm0 \
     ...

and adjust the text below to:

In case an Arm virt machine is emulated to run Linux, you may also use the following command line:

.. code-block:: console

   qemu-system-aarch64 -machine virt,gic-version=3,accel=kvm \
     -cpu host -m 4G \
     -nographic -no-acpi \
     -chardev socket,id=chrtpm,path=/tmp/mytpm1/swtpm-sock \
     -tpmdev emulator,id=tpm0,chardev=chrtpm \
     -device tpm-tis-device,tpmdev=tpm0 \
     -device virtio-blk-pci,drive=drv0 \
     -drive format=qcow2,file=hda.qcow2,if=none,id=drv0 \
     -drive if=pflash,format=raw,file=flash0.img,readonly=on \
     -drive if=pflash,format=raw,file=flash1.img

> there would be a lot of text there that would be off-topic from the
> commit. Additionally, do we also need to include steps on creating the
> QCOW boot device, setting up the UEFI secure boot, and so on? Because
> there's quite a few steps involved to get a working Windows boot and
> I'm sure it's documented in length in various blog posts.
Igor Mammedov July 17, 2023, 2:23 p.m. UTC | #4
On Fri, 14 Jul 2023 00:09:26 -0700
Joelle van Dyne <j@getutm.app> wrote:

> This SysBus variant of the CRB interface supports dynamically locating
> the MMIO interface so that Virt machines can use it. This interface
> is currently the only one supported by QEMU that works on Windows 11
> ARM64. We largely follow the TPM TIS SysBus device as a template.
> 
> To try out this device with Windows 11 before OVMF is updated, you
> will need to modify `sysbud-fdt.c` and change the added line from:
> 
> ```c
>     TYPE_BINDING(TYPE_TPM_CRB_SYSBUS, no_fdt_node),
> ```
> 
> to
> 
> ```c
>     TYPE_BINDING(TYPE_TPM_CRB_SYSBUS, add_tpm_tis_fdt_node),
> ```
> 
> This change was not included because it can confuse Linux (although
> from testing, it seems like Linux is able to properly ignore the
> device from the TPM TIS driver and recognize it from the ACPI device
> in the TPM CRB driver). A proper fix would require OVMF to recognize
> the ACPI device and not depend on the FDT node for recognizing TPM.
> 
> The command line to try out this device with SWTPM is:
> 
> ```
> $ qemu-system-aarch64 \
>     -chardev socket,id=chrtpm0,path=tpm.sock \
>     -tpmdev emulator,id=tpm0,chardev=chrtpm0 \
>     -device tpm-crb-device,tpmdev=tpm0
> ```
> 
> along with SWTPM:
> 
> ```
> $ swtpm \
>     --ctrl type=unixio,path=tpm.sock,terminate \
>     --tpmstate backend-uri=file://tpm.data \
>     --tpm2
> ```
> 
> Signed-off-by: Joelle van Dyne <j@getutm.app>
> ---
>  docs/specs/tpm.rst          |   1 +
>  include/hw/acpi/aml-build.h |   1 +
>  include/sysemu/tpm.h        |   3 +
>  hw/acpi/aml-build.c         |   7 +-
>  hw/arm/virt.c               |   1 +
>  hw/core/sysbus-fdt.c        |   1 +
>  hw/loongarch/virt.c         |   1 +
>  hw/riscv/virt.c             |   1 +
>  hw/tpm/tpm_crb_sysbus.c     | 170 ++++++++++++++++++++++++++++++++++++
>  hw/arm/Kconfig              |   1 +
>  hw/riscv/Kconfig            |   1 +
>  hw/tpm/Kconfig              |   5 ++
>  hw/tpm/meson.build          |   2 +
>  13 files changed, 194 insertions(+), 1 deletion(-)
>  create mode 100644 hw/tpm/tpm_crb_sysbus.c
> 
> diff --git a/docs/specs/tpm.rst b/docs/specs/tpm.rst
> index 2bc29c9804..95aeb49220 100644
> --- a/docs/specs/tpm.rst
> +++ b/docs/specs/tpm.rst
> @@ -46,6 +46,7 @@ operating system.
>  QEMU files related to TPM CRB interface:
>   - ``hw/tpm/tpm_crb.c``
>   - ``hw/tpm/tpm_crb_common.c``
> + - ``hw/tpm/tpm_crb_sysbus.c``
>  
>  SPAPR interface
>  ---------------
> diff --git a/include/hw/acpi/aml-build.h b/include/hw/acpi/aml-build.h
> index d1fb08514b..9660e16148 100644
> --- a/include/hw/acpi/aml-build.h
> +++ b/include/hw/acpi/aml-build.h
> @@ -3,6 +3,7 @@
>  
>  #include "hw/acpi/acpi-defs.h"
>  #include "hw/acpi/bios-linker-loader.h"
> +#include "exec/hwaddr.h"
>  
>  #define ACPI_BUILD_APPNAME6 "BOCHS "
>  #define ACPI_BUILD_APPNAME8 "BXPC    "
> diff --git a/include/sysemu/tpm.h b/include/sysemu/tpm.h
> index 66e3b45f30..f79c8f3575 100644
> --- a/include/sysemu/tpm.h
> +++ b/include/sysemu/tpm.h
> @@ -47,6 +47,7 @@ struct TPMIfClass {
>  #define TYPE_TPM_TIS_ISA            "tpm-tis"
>  #define TYPE_TPM_TIS_SYSBUS         "tpm-tis-device"
>  #define TYPE_TPM_CRB                "tpm-crb"
> +#define TYPE_TPM_CRB_SYSBUS         "tpm-crb-device"
>  #define TYPE_TPM_SPAPR              "tpm-spapr"
>  #define TYPE_TPM_TIS_I2C            "tpm-tis-i2c"
>  
> @@ -56,6 +57,8 @@ struct TPMIfClass {
>      object_dynamic_cast(OBJECT(chr), TYPE_TPM_TIS_SYSBUS)
>  #define TPM_IS_CRB(chr)                             \
>      object_dynamic_cast(OBJECT(chr), TYPE_TPM_CRB)
> +#define TPM_IS_CRB_SYSBUS(chr)                      \
> +    object_dynamic_cast(OBJECT(chr), TYPE_TPM_CRB_SYSBUS)
>  #define TPM_IS_SPAPR(chr)                           \
>      object_dynamic_cast(OBJECT(chr), TYPE_TPM_SPAPR)
>  #define TPM_IS_TIS_I2C(chr)                      \
> diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
> index ea331a20d1..f809137fc9 100644
> --- a/hw/acpi/aml-build.c
> +++ b/hw/acpi/aml-build.c
> @@ -31,6 +31,7 @@
>  #include "hw/pci/pci_bus.h"
>  #include "hw/pci/pci_bridge.h"
>  #include "qemu/cutils.h"
> +#include "qom/object.h"
>  
>  static GArray *build_alloc_array(void)
>  {
> @@ -2218,7 +2219,7 @@ void build_tpm2(GArray *table_data, BIOSLinker *linker, GArray *tcpalog,
>  {
>      uint8_t start_method_params[12] = {};
>      unsigned log_addr_offset;
> -    uint64_t control_area_start_address;
> +    uint64_t baseaddr, control_area_start_address;
>      TPMIf *tpmif = tpm_find();
>      uint32_t start_method;
>      AcpiTable table = { .sig = "TPM2", .rev = 4,
> @@ -2236,6 +2237,10 @@ void build_tpm2(GArray *table_data, BIOSLinker *linker, GArray *tcpalog,
>      } else if (TPM_IS_CRB(tpmif)) {
>          control_area_start_address = TPM_CRB_ADDR_CTRL;
>          start_method = TPM2_START_METHOD_CRB;
> +    } else if (TPM_IS_CRB_SYSBUS(tpmif)) {
> +        baseaddr = object_property_get_uint(OBJECT(tpmif), "baseaddr", NULL);
> +        control_area_start_address = baseaddr + A_CRB_CTRL_REQ;
> +        start_method = TPM2_START_METHOD_CRB;
>      } else {
>          g_assert_not_reached();
>      }
> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> index 432148ef47..88e8b16103 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -2977,6 +2977,7 @@ static void virt_machine_class_init(ObjectClass *oc, void *data)
>      machine_class_allow_dynamic_sysbus_dev(mc, TYPE_VFIO_PLATFORM);
>  #ifdef CONFIG_TPM
>      machine_class_allow_dynamic_sysbus_dev(mc, TYPE_TPM_TIS_SYSBUS);
> +    machine_class_allow_dynamic_sysbus_dev(mc, TYPE_TPM_CRB_SYSBUS);
>  #endif
>      mc->block_default_type = IF_VIRTIO;
>      mc->no_cdrom = 1;
> diff --git a/hw/core/sysbus-fdt.c b/hw/core/sysbus-fdt.c
> index eebcd28f9a..9c783f88eb 100644
> --- a/hw/core/sysbus-fdt.c
> +++ b/hw/core/sysbus-fdt.c
> @@ -493,6 +493,7 @@ static const BindingEntry bindings[] = {
>  #endif
>  #ifdef CONFIG_TPM
>      TYPE_BINDING(TYPE_TPM_TIS_SYSBUS, add_tpm_tis_fdt_node),
> +    TYPE_BINDING(TYPE_TPM_CRB_SYSBUS, no_fdt_node),
>  #endif
>      TYPE_BINDING(TYPE_RAMFB_DEVICE, no_fdt_node),
>      TYPE_BINDING("", NULL), /* last element */
> diff --git a/hw/loongarch/virt.c b/hw/loongarch/virt.c
> index 9c536c52bc..eb59fb04ee 100644
> --- a/hw/loongarch/virt.c
> +++ b/hw/loongarch/virt.c
> @@ -1194,6 +1194,7 @@ static void loongarch_class_init(ObjectClass *oc, void *data)
>      machine_class_allow_dynamic_sysbus_dev(mc, TYPE_RAMFB_DEVICE);
>  #ifdef CONFIG_TPM
>      machine_class_allow_dynamic_sysbus_dev(mc, TYPE_TPM_TIS_SYSBUS);
> +    machine_class_allow_dynamic_sysbus_dev(mc, TYPE_TPM_CRB_SYSBUS);
>  #endif
>  }
>  
> diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
> index d90286dc46..5d639a870a 100644
> --- a/hw/riscv/virt.c
> +++ b/hw/riscv/virt.c
> @@ -1681,6 +1681,7 @@ static void virt_machine_class_init(ObjectClass *oc, void *data)
>      machine_class_allow_dynamic_sysbus_dev(mc, TYPE_RAMFB_DEVICE);
>  #ifdef CONFIG_TPM
>      machine_class_allow_dynamic_sysbus_dev(mc, TYPE_TPM_TIS_SYSBUS);
> +    machine_class_allow_dynamic_sysbus_dev(mc, TYPE_TPM_CRB_SYSBUS);
>  #endif
>  
>      if (tcg_enabled()) {
> diff --git a/hw/tpm/tpm_crb_sysbus.c b/hw/tpm/tpm_crb_sysbus.c
> new file mode 100644
> index 0000000000..66be57a532
> --- /dev/null
> +++ b/hw/tpm/tpm_crb_sysbus.c
> @@ -0,0 +1,170 @@
> +/*
> + * tpm_crb_sysbus.c - QEMU's TPM CRB interface emulator
> + *
> + * Copyright (c) 2018 Red Hat, Inc.
> + *
> + * Authors:
> + *   Marc-André Lureau <marcandre.lureau@redhat.com>
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> + * See the COPYING file in the top-level directory.
> + *
> + * tpm_crb is a device for TPM 2.0 Command Response Buffer (CRB) Interface
> + * as defined in TCG PC Client Platform TPM Profile (PTP) Specification
> + * Family “2.0” Level 00 Revision 01.03 v22
> + */
> +
> +#include "qemu/osdep.h"
> +#include "hw/acpi/acpi_aml_interface.h"
> +#include "hw/acpi/tpm.h"
> +#include "hw/qdev-properties.h"
> +#include "migration/vmstate.h"
> +#include "tpm_prop.h"
> +#include "hw/pci/pci_ids.h"
> +#include "hw/sysbus.h"
> +#include "qapi/visitor.h"
> +#include "qom/object.h"
> +#include "sysemu/tpm_util.h"
> +#include "trace.h"
> +#include "tpm_crb.h"

is it possible to weed off some unnecessary includes?

> +
> +struct TPMCRBStateSysBus {
> +    /*< private >*/
> +    SysBusDevice parent_obj;
> +
> +    /*< public >*/
> +    TPMCRBState state;
> +    uint64_t baseaddr;
> +    uint64_t size;
> +};
> +
> +OBJECT_DECLARE_SIMPLE_TYPE(TPMCRBStateSysBus, TPM_CRB_SYSBUS)
> +
> +static void tpm_crb_sysbus_request_completed(TPMIf *ti, int ret)
> +{
> +    TPMCRBStateSysBus *s = TPM_CRB_SYSBUS(ti);
> +
> +    return tpm_crb_request_completed(&s->state, ret);
> +}
> +
> +static enum TPMVersion tpm_crb_sysbus_get_tpm_version(TPMIf *ti)
> +{
> +    TPMCRBStateSysBus *s = TPM_CRB_SYSBUS(ti);
> +
> +    return tpm_crb_get_version(&s->state);
> +}
> +
> +static int tpm_crb_sysbus_pre_save(void *opaque)
> +{
> +    TPMCRBStateSysBus *s = opaque;
> +
> +    return tpm_crb_pre_save(&s->state);
> +}
> +
> +static const VMStateDescription vmstate_tpm_crb_sysbus = {
> +    .name = "tpm-crb-sysbus",
> +    .pre_save = tpm_crb_sysbus_pre_save,
> +    .fields = (VMStateField[]) {
> +        VMSTATE_END_OF_LIST(),
> +    }
> +};
> +
> +static Property tpm_crb_sysbus_properties[] = {
> +    DEFINE_PROP_TPMBE("tpmdev", TPMCRBStateSysBus, state.tpmbe),
> +    DEFINE_PROP_UINT64("baseaddr", TPMCRBStateSysBus,
> +                       baseaddr, TPM_CRB_ADDR_BASE),
> +    DEFINE_PROP_UINT64("size", TPMCRBStateSysBus, size, TPM_CRB_ADDR_SIZE),
> +    DEFINE_PROP_END_OF_LIST(),
> +};
> +
> +static void tpm_crb_sysbus_initfn(Object *obj)
> +{
> +    TPMCRBStateSysBus *s = TPM_CRB_SYSBUS(obj);
> +
> +    tpm_crb_init_memory(obj, &s->state, NULL);
> +
> +    sysbus_init_mmio(SYS_BUS_DEVICE(obj), &s->state.mmio);
> +}
> +
> +static void tpm_crb_sysbus_reset(DeviceState *dev)
> +{
> +    TPMCRBStateSysBus *s = TPM_CRB_SYSBUS(dev);
> +
> +    return tpm_crb_reset(&s->state, s->baseaddr);
> +}
> +
> +static void tpm_crb_sysbus_realizefn(DeviceState *dev, Error **errp)
> +{
> +    TPMCRBStateSysBus *s = TPM_CRB_SYSBUS(dev);
> +
> +    if (!tpm_find()) {
> +        error_setg(errp, "at most one TPM device is permitted");
> +        return;
> +    }
> +
> +    if (!s->state.tpmbe) {
> +        error_setg(errp, "'tpmdev' property is required");
> +        return;
> +    }

above parts can be common with ISA vesion

> +    if (tpm_crb_sysbus_get_tpm_version(TPM_IF(s)) != TPM_VERSION_2_0) {
> +        error_setg(errp, "TPM CRB only supports TPM 2.0 backends");
> +        return;
why ISA version doesn't require this?

> +    }
> +}
> +
> +static void build_tpm_crb_sysbus_aml(AcpiDevAmlIf *adev, Aml *scope)
> +{
> +    Aml *dev, *crs;
> +    TPMCRBStateSysBus *s = TPM_CRB_SYSBUS(adev);
> +    TPMIf *ti = TPM_IF(s);
> +
> +    dev = aml_device("TPM");
> +    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(1)));
> +    aml_append(dev, aml_name_decl("_STA", aml_int(0xF)));
> +    crs = aml_resource_template();
> +    aml_append(crs, aml_memory32_fixed(s->baseaddr, s->size,
> +                                      AML_READ_WRITE));
> +    aml_append(dev, aml_name_decl("_CRS", crs));
> +    aml_append(scope, dev);
> +}

this almost matches isa version, modulo hard-codded address/size
(suggest to generalize and reuse it in both places)
and ppi, why sysbus variant doesn't implement PPI parts?

> +
> +static void tpm_crb_sysbus_class_init(ObjectClass *klass, void *data)
> +{
> +    DeviceClass *dc = DEVICE_CLASS(klass);
> +    TPMIfClass *tc = TPM_IF_CLASS(klass);
> +    AcpiDevAmlIfClass *adevc = ACPI_DEV_AML_IF_CLASS(klass);
> +
> +    device_class_set_props(dc, tpm_crb_sysbus_properties);
> +    dc->vmsd  = &vmstate_tpm_crb_sysbus;
> +    tc->model = TPM_MODEL_TPM_CRB;
> +    dc->realize = tpm_crb_sysbus_realizefn;
> +    dc->user_creatable = true;
> +    dc->reset = tpm_crb_sysbus_reset;
> +    tc->request_completed = tpm_crb_sysbus_request_completed;
> +    tc->get_version = tpm_crb_sysbus_get_tpm_version;
> +    set_bit(DEVICE_CATEGORY_MISC, dc->categories);
> +    adevc->build_dev_aml = build_tpm_crb_sysbus_aml;
> +}
> +
> +static const TypeInfo tpm_crb_sysbus_info = {
> +    .name = TYPE_TPM_CRB_SYSBUS,
> +    .parent = TYPE_SYS_BUS_DEVICE,
> +    .instance_size = sizeof(TPMCRBStateSysBus),
> +    .instance_init = tpm_crb_sysbus_initfn,
> +    .class_init  = tpm_crb_sysbus_class_init,
> +    .interfaces = (InterfaceInfo[]) {
> +        { TYPE_TPM_IF },
> +        { TYPE_ACPI_DEV_AML_IF },
> +        { }
> +    }
> +};
> +
> +static void tpm_crb_sysbus_register(void)
> +{
> +    type_register_static(&tpm_crb_sysbus_info);
> +}
> +
> +type_init(tpm_crb_sysbus_register)
> diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
> index 7e68348440..efe1beaa7b 100644
> --- a/hw/arm/Kconfig
> +++ b/hw/arm/Kconfig
> @@ -5,6 +5,7 @@ config ARM_VIRT
>      imply VFIO_AMD_XGBE
>      imply VFIO_PLATFORM
>      imply VFIO_XGMAC
> +    imply TPM_CRB_SYSBUS
>      imply TPM_TIS_SYSBUS
>      imply TPM_TIS_I2C
>      imply NVDIMM
> diff --git a/hw/riscv/Kconfig b/hw/riscv/Kconfig
> index b6a5eb4452..d824cb58f9 100644
> --- a/hw/riscv/Kconfig
> +++ b/hw/riscv/Kconfig
> @@ -29,6 +29,7 @@ config RISCV_VIRT
>      imply PCI_DEVICES
>      imply VIRTIO_VGA
>      imply TEST_DEVICES
> +    imply TPM_CRB_SYSBUS
>      imply TPM_TIS_SYSBUS
>      select RISCV_NUMA
>      select GOLDFISH_RTC
> diff --git a/hw/tpm/Kconfig b/hw/tpm/Kconfig
> index 1fd73fe617..3f294a20ba 100644
> --- a/hw/tpm/Kconfig
> +++ b/hw/tpm/Kconfig
> @@ -25,6 +25,11 @@ config TPM_CRB
>      depends on TPM && ISA_BUS
>      select TPM_BACKEND
>  
> +config TPM_CRB_SYSBUS
> +    bool
> +    depends on TPM
> +    select TPM_BACKEND
> +
>  config TPM_SPAPR
>      bool
>      default y
> diff --git a/hw/tpm/meson.build b/hw/tpm/meson.build
> index cb8204d5bc..d96de92c16 100644
> --- a/hw/tpm/meson.build
> +++ b/hw/tpm/meson.build
> @@ -4,6 +4,8 @@ system_ss.add(when: 'CONFIG_TPM_TIS_SYSBUS', if_true: files('tpm_tis_sysbus.c'))
>  system_ss.add(when: 'CONFIG_TPM_TIS_I2C', if_true: files('tpm_tis_i2c.c'))
>  system_ss.add(when: 'CONFIG_TPM_CRB', if_true: files('tpm_crb.c'))
>  system_ss.add(when: 'CONFIG_TPM_CRB', if_true: files('tpm_crb_common.c'))
> +system_ss.add(when: 'CONFIG_TPM_CRB_SYSBUS', if_true: files('tpm_crb_sysbus.c'))
> +system_ss.add(when: 'CONFIG_TPM_CRB_SYSBUS', if_true: files('tpm_crb_common.c'))
>  system_ss.add(when: 'CONFIG_TPM_TIS', if_true: files('tpm_ppi.c'))
>  system_ss.add(when: 'CONFIG_TPM_CRB', if_true: files('tpm_ppi.c'))
>
Joelle van Dyne Oct. 29, 2023, 2:21 a.m. UTC | #5
On Mon, Jul 17, 2023 at 7:23 AM Igor Mammedov <imammedo@redhat.com> wrote:
>
> On Fri, 14 Jul 2023 00:09:26 -0700
> Joelle van Dyne <j@getutm.app> wrote:
>
> > This SysBus variant of the CRB interface supports dynamically locating
> > the MMIO interface so that Virt machines can use it. This interface
> > is currently the only one supported by QEMU that works on Windows 11
> > ARM64. We largely follow the TPM TIS SysBus device as a template.
> >
> > To try out this device with Windows 11 before OVMF is updated, you
> > will need to modify `sysbud-fdt.c` and change the added line from:
> >
> > ```c
> >     TYPE_BINDING(TYPE_TPM_CRB_SYSBUS, no_fdt_node),
> > ```
> >
> > to
> >
> > ```c
> >     TYPE_BINDING(TYPE_TPM_CRB_SYSBUS, add_tpm_tis_fdt_node),
> > ```
> >
> > This change was not included because it can confuse Linux (although
> > from testing, it seems like Linux is able to properly ignore the
> > device from the TPM TIS driver and recognize it from the ACPI device
> > in the TPM CRB driver). A proper fix would require OVMF to recognize
> > the ACPI device and not depend on the FDT node for recognizing TPM.
> >
> > The command line to try out this device with SWTPM is:
> >
> > ```
> > $ qemu-system-aarch64 \
> >     -chardev socket,id=chrtpm0,path=tpm.sock \
> >     -tpmdev emulator,id=tpm0,chardev=chrtpm0 \
> >     -device tpm-crb-device,tpmdev=tpm0
> > ```
> >
> > along with SWTPM:
> >
> > ```
> > $ swtpm \
> >     --ctrl type=unixio,path=tpm.sock,terminate \
> >     --tpmstate backend-uri=file://tpm.data \
> >     --tpm2
> > ```
> >
> > Signed-off-by: Joelle van Dyne <j@getutm.app>
> > ---
> >  docs/specs/tpm.rst          |   1 +
> >  include/hw/acpi/aml-build.h |   1 +
> >  include/sysemu/tpm.h        |   3 +
> >  hw/acpi/aml-build.c         |   7 +-
> >  hw/arm/virt.c               |   1 +
> >  hw/core/sysbus-fdt.c        |   1 +
> >  hw/loongarch/virt.c         |   1 +
> >  hw/riscv/virt.c             |   1 +
> >  hw/tpm/tpm_crb_sysbus.c     | 170 ++++++++++++++++++++++++++++++++++++
> >  hw/arm/Kconfig              |   1 +
> >  hw/riscv/Kconfig            |   1 +
> >  hw/tpm/Kconfig              |   5 ++
> >  hw/tpm/meson.build          |   2 +
> >  13 files changed, 194 insertions(+), 1 deletion(-)
> >  create mode 100644 hw/tpm/tpm_crb_sysbus.c
> >
> > diff --git a/docs/specs/tpm.rst b/docs/specs/tpm.rst
> > index 2bc29c9804..95aeb49220 100644
> > --- a/docs/specs/tpm.rst
> > +++ b/docs/specs/tpm.rst
> > @@ -46,6 +46,7 @@ operating system.
> >  QEMU files related to TPM CRB interface:
> >   - ``hw/tpm/tpm_crb.c``
> >   - ``hw/tpm/tpm_crb_common.c``
> > + - ``hw/tpm/tpm_crb_sysbus.c``
> >
> >  SPAPR interface
> >  ---------------
> > diff --git a/include/hw/acpi/aml-build.h b/include/hw/acpi/aml-build.h
> > index d1fb08514b..9660e16148 100644
> > --- a/include/hw/acpi/aml-build.h
> > +++ b/include/hw/acpi/aml-build.h
> > @@ -3,6 +3,7 @@
> >
> >  #include "hw/acpi/acpi-defs.h"
> >  #include "hw/acpi/bios-linker-loader.h"
> > +#include "exec/hwaddr.h"
> >
> >  #define ACPI_BUILD_APPNAME6 "BOCHS "
> >  #define ACPI_BUILD_APPNAME8 "BXPC    "
> > diff --git a/include/sysemu/tpm.h b/include/sysemu/tpm.h
> > index 66e3b45f30..f79c8f3575 100644
> > --- a/include/sysemu/tpm.h
> > +++ b/include/sysemu/tpm.h
> > @@ -47,6 +47,7 @@ struct TPMIfClass {
> >  #define TYPE_TPM_TIS_ISA            "tpm-tis"
> >  #define TYPE_TPM_TIS_SYSBUS         "tpm-tis-device"
> >  #define TYPE_TPM_CRB                "tpm-crb"
> > +#define TYPE_TPM_CRB_SYSBUS         "tpm-crb-device"
> >  #define TYPE_TPM_SPAPR              "tpm-spapr"
> >  #define TYPE_TPM_TIS_I2C            "tpm-tis-i2c"
> >
> > @@ -56,6 +57,8 @@ struct TPMIfClass {
> >      object_dynamic_cast(OBJECT(chr), TYPE_TPM_TIS_SYSBUS)
> >  #define TPM_IS_CRB(chr)                             \
> >      object_dynamic_cast(OBJECT(chr), TYPE_TPM_CRB)
> > +#define TPM_IS_CRB_SYSBUS(chr)                      \
> > +    object_dynamic_cast(OBJECT(chr), TYPE_TPM_CRB_SYSBUS)
> >  #define TPM_IS_SPAPR(chr)                           \
> >      object_dynamic_cast(OBJECT(chr), TYPE_TPM_SPAPR)
> >  #define TPM_IS_TIS_I2C(chr)                      \
> > diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
> > index ea331a20d1..f809137fc9 100644
> > --- a/hw/acpi/aml-build.c
> > +++ b/hw/acpi/aml-build.c
> > @@ -31,6 +31,7 @@
> >  #include "hw/pci/pci_bus.h"
> >  #include "hw/pci/pci_bridge.h"
> >  #include "qemu/cutils.h"
> > +#include "qom/object.h"
> >
> >  static GArray *build_alloc_array(void)
> >  {
> > @@ -2218,7 +2219,7 @@ void build_tpm2(GArray *table_data, BIOSLinker *linker, GArray *tcpalog,
> >  {
> >      uint8_t start_method_params[12] = {};
> >      unsigned log_addr_offset;
> > -    uint64_t control_area_start_address;
> > +    uint64_t baseaddr, control_area_start_address;
> >      TPMIf *tpmif = tpm_find();
> >      uint32_t start_method;
> >      AcpiTable table = { .sig = "TPM2", .rev = 4,
> > @@ -2236,6 +2237,10 @@ void build_tpm2(GArray *table_data, BIOSLinker *linker, GArray *tcpalog,
> >      } else if (TPM_IS_CRB(tpmif)) {
> >          control_area_start_address = TPM_CRB_ADDR_CTRL;
> >          start_method = TPM2_START_METHOD_CRB;
> > +    } else if (TPM_IS_CRB_SYSBUS(tpmif)) {
> > +        baseaddr = object_property_get_uint(OBJECT(tpmif), "baseaddr", NULL);
> > +        control_area_start_address = baseaddr + A_CRB_CTRL_REQ;
> > +        start_method = TPM2_START_METHOD_CRB;
> >      } else {
> >          g_assert_not_reached();
> >      }
> > diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> > index 432148ef47..88e8b16103 100644
> > --- a/hw/arm/virt.c
> > +++ b/hw/arm/virt.c
> > @@ -2977,6 +2977,7 @@ static void virt_machine_class_init(ObjectClass *oc, void *data)
> >      machine_class_allow_dynamic_sysbus_dev(mc, TYPE_VFIO_PLATFORM);
> >  #ifdef CONFIG_TPM
> >      machine_class_allow_dynamic_sysbus_dev(mc, TYPE_TPM_TIS_SYSBUS);
> > +    machine_class_allow_dynamic_sysbus_dev(mc, TYPE_TPM_CRB_SYSBUS);
> >  #endif
> >      mc->block_default_type = IF_VIRTIO;
> >      mc->no_cdrom = 1;
> > diff --git a/hw/core/sysbus-fdt.c b/hw/core/sysbus-fdt.c
> > index eebcd28f9a..9c783f88eb 100644
> > --- a/hw/core/sysbus-fdt.c
> > +++ b/hw/core/sysbus-fdt.c
> > @@ -493,6 +493,7 @@ static const BindingEntry bindings[] = {
> >  #endif
> >  #ifdef CONFIG_TPM
> >      TYPE_BINDING(TYPE_TPM_TIS_SYSBUS, add_tpm_tis_fdt_node),
> > +    TYPE_BINDING(TYPE_TPM_CRB_SYSBUS, no_fdt_node),
> >  #endif
> >      TYPE_BINDING(TYPE_RAMFB_DEVICE, no_fdt_node),
> >      TYPE_BINDING("", NULL), /* last element */
> > diff --git a/hw/loongarch/virt.c b/hw/loongarch/virt.c
> > index 9c536c52bc..eb59fb04ee 100644
> > --- a/hw/loongarch/virt.c
> > +++ b/hw/loongarch/virt.c
> > @@ -1194,6 +1194,7 @@ static void loongarch_class_init(ObjectClass *oc, void *data)
> >      machine_class_allow_dynamic_sysbus_dev(mc, TYPE_RAMFB_DEVICE);
> >  #ifdef CONFIG_TPM
> >      machine_class_allow_dynamic_sysbus_dev(mc, TYPE_TPM_TIS_SYSBUS);
> > +    machine_class_allow_dynamic_sysbus_dev(mc, TYPE_TPM_CRB_SYSBUS);
> >  #endif
> >  }
> >
> > diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
> > index d90286dc46..5d639a870a 100644
> > --- a/hw/riscv/virt.c
> > +++ b/hw/riscv/virt.c
> > @@ -1681,6 +1681,7 @@ static void virt_machine_class_init(ObjectClass *oc, void *data)
> >      machine_class_allow_dynamic_sysbus_dev(mc, TYPE_RAMFB_DEVICE);
> >  #ifdef CONFIG_TPM
> >      machine_class_allow_dynamic_sysbus_dev(mc, TYPE_TPM_TIS_SYSBUS);
> > +    machine_class_allow_dynamic_sysbus_dev(mc, TYPE_TPM_CRB_SYSBUS);
> >  #endif
> >
> >      if (tcg_enabled()) {
> > diff --git a/hw/tpm/tpm_crb_sysbus.c b/hw/tpm/tpm_crb_sysbus.c
> > new file mode 100644
> > index 0000000000..66be57a532
> > --- /dev/null
> > +++ b/hw/tpm/tpm_crb_sysbus.c
> > @@ -0,0 +1,170 @@
> > +/*
> > + * tpm_crb_sysbus.c - QEMU's TPM CRB interface emulator
> > + *
> > + * Copyright (c) 2018 Red Hat, Inc.
> > + *
> > + * Authors:
> > + *   Marc-André Lureau <marcandre.lureau@redhat.com>
> > + *
> > + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> > + * See the COPYING file in the top-level directory.
> > + *
> > + * tpm_crb is a device for TPM 2.0 Command Response Buffer (CRB) Interface
> > + * as defined in TCG PC Client Platform TPM Profile (PTP) Specification
> > + * Family “2.0” Level 00 Revision 01.03 v22
> > + */
> > +
> > +#include "qemu/osdep.h"
> > +#include "hw/acpi/acpi_aml_interface.h"
> > +#include "hw/acpi/tpm.h"
> > +#include "hw/qdev-properties.h"
> > +#include "migration/vmstate.h"
> > +#include "tpm_prop.h"
> > +#include "hw/pci/pci_ids.h"
> > +#include "hw/sysbus.h"
> > +#include "qapi/visitor.h"
> > +#include "qom/object.h"
> > +#include "sysemu/tpm_util.h"
> > +#include "trace.h"
> > +#include "tpm_crb.h"
>
> is it possible to weed off some unnecessary includes?
Done.

>
> > +
> > +struct TPMCRBStateSysBus {
> > +    /*< private >*/
> > +    SysBusDevice parent_obj;
> > +
> > +    /*< public >*/
> > +    TPMCRBState state;
> > +    uint64_t baseaddr;
> > +    uint64_t size;
> > +};
> > +
> > +OBJECT_DECLARE_SIMPLE_TYPE(TPMCRBStateSysBus, TPM_CRB_SYSBUS)
> > +
> > +static void tpm_crb_sysbus_request_completed(TPMIf *ti, int ret)
> > +{
> > +    TPMCRBStateSysBus *s = TPM_CRB_SYSBUS(ti);
> > +
> > +    return tpm_crb_request_completed(&s->state, ret);
> > +}
> > +
> > +static enum TPMVersion tpm_crb_sysbus_get_tpm_version(TPMIf *ti)
> > +{
> > +    TPMCRBStateSysBus *s = TPM_CRB_SYSBUS(ti);
> > +
> > +    return tpm_crb_get_version(&s->state);
> > +}
> > +
> > +static int tpm_crb_sysbus_pre_save(void *opaque)
> > +{
> > +    TPMCRBStateSysBus *s = opaque;
> > +
> > +    return tpm_crb_pre_save(&s->state);
> > +}
> > +
> > +static const VMStateDescription vmstate_tpm_crb_sysbus = {
> > +    .name = "tpm-crb-sysbus",
> > +    .pre_save = tpm_crb_sysbus_pre_save,
> > +    .fields = (VMStateField[]) {
> > +        VMSTATE_END_OF_LIST(),
> > +    }
> > +};
> > +
> > +static Property tpm_crb_sysbus_properties[] = {
> > +    DEFINE_PROP_TPMBE("tpmdev", TPMCRBStateSysBus, state.tpmbe),
> > +    DEFINE_PROP_UINT64("baseaddr", TPMCRBStateSysBus,
> > +                       baseaddr, TPM_CRB_ADDR_BASE),
> > +    DEFINE_PROP_UINT64("size", TPMCRBStateSysBus, size, TPM_CRB_ADDR_SIZE),
> > +    DEFINE_PROP_END_OF_LIST(),
> > +};
> > +
> > +static void tpm_crb_sysbus_initfn(Object *obj)
> > +{
> > +    TPMCRBStateSysBus *s = TPM_CRB_SYSBUS(obj);
> > +
> > +    tpm_crb_init_memory(obj, &s->state, NULL);
> > +
> > +    sysbus_init_mmio(SYS_BUS_DEVICE(obj), &s->state.mmio);
> > +}
> > +
> > +static void tpm_crb_sysbus_reset(DeviceState *dev)
> > +{
> > +    TPMCRBStateSysBus *s = TPM_CRB_SYSBUS(dev);
> > +
> > +    return tpm_crb_reset(&s->state, s->baseaddr);
> > +}
> > +
> > +static void tpm_crb_sysbus_realizefn(DeviceState *dev, Error **errp)
> > +{
> > +    TPMCRBStateSysBus *s = TPM_CRB_SYSBUS(dev);
> > +
> > +    if (!tpm_find()) {
> > +        error_setg(errp, "at most one TPM device is permitted");
> > +        return;
> > +    }
> > +
> > +    if (!s->state.tpmbe) {
> > +        error_setg(errp, "'tpmdev' property is required");
> > +        return;
> > +    }
>
> above parts can be common with ISA vesion
This was done in order to abstract away the callback interface. All
the common code uses `TPMCRBState` instead of `TPMIf` as the argument
and it is cleaner this way IMO.

>
> > +    if (tpm_crb_sysbus_get_tpm_version(TPM_IF(s)) != TPM_VERSION_2_0) {
> > +        error_setg(errp, "TPM CRB only supports TPM 2.0 backends");
> > +        return;
> why ISA version doesn't require this?
TPM CRB is only defined for 2.0. We can add this in the ISA version
but it would break any existing VM which depended on this erroneous
behaviour. Better to keep this check only on the new device.

>
> > +    }
> > +}
> > +
> > +static void build_tpm_crb_sysbus_aml(AcpiDevAmlIf *adev, Aml *scope)
> > +{
> > +    Aml *dev, *crs;
> > +    TPMCRBStateSysBus *s = TPM_CRB_SYSBUS(adev);
> > +    TPMIf *ti = TPM_IF(s);
> > +
> > +    dev = aml_device("TPM");
> > +    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(1)));
> > +    aml_append(dev, aml_name_decl("_STA", aml_int(0xF)));
> > +    crs = aml_resource_template();
> > +    aml_append(crs, aml_memory32_fixed(s->baseaddr, s->size,
> > +                                      AML_READ_WRITE));
> > +    aml_append(dev, aml_name_decl("_CRS", crs));
> > +    aml_append(scope, dev);
> > +}
>
> this almost matches isa version, modulo hard-codded address/size
> (suggest to generalize and reuse it in both places)
Done.

> and ppi, why sysbus variant doesn't implement PPI parts?
The support has not been implemented. See
4c46fe2ed492f35f411632c8b5a8442f322bc3f0 for disabling this option on
TIS sysbus devices.

>
> > +
> > +static void tpm_crb_sysbus_class_init(ObjectClass *klass, void *data)
> > +{
> > +    DeviceClass *dc = DEVICE_CLASS(klass);
> > +    TPMIfClass *tc = TPM_IF_CLASS(klass);
> > +    AcpiDevAmlIfClass *adevc = ACPI_DEV_AML_IF_CLASS(klass);
> > +
> > +    device_class_set_props(dc, tpm_crb_sysbus_properties);
> > +    dc->vmsd  = &vmstate_tpm_crb_sysbus;
> > +    tc->model = TPM_MODEL_TPM_CRB;
> > +    dc->realize = tpm_crb_sysbus_realizefn;
> > +    dc->user_creatable = true;
> > +    dc->reset = tpm_crb_sysbus_reset;
> > +    tc->request_completed = tpm_crb_sysbus_request_completed;
> > +    tc->get_version = tpm_crb_sysbus_get_tpm_version;
> > +    set_bit(DEVICE_CATEGORY_MISC, dc->categories);
> > +    adevc->build_dev_aml = build_tpm_crb_sysbus_aml;
> > +}
> > +
> > +static const TypeInfo tpm_crb_sysbus_info = {
> > +    .name = TYPE_TPM_CRB_SYSBUS,
> > +    .parent = TYPE_SYS_BUS_DEVICE,
> > +    .instance_size = sizeof(TPMCRBStateSysBus),
> > +    .instance_init = tpm_crb_sysbus_initfn,
> > +    .class_init  = tpm_crb_sysbus_class_init,
> > +    .interfaces = (InterfaceInfo[]) {
> > +        { TYPE_TPM_IF },
> > +        { TYPE_ACPI_DEV_AML_IF },
> > +        { }
> > +    }
> > +};
> > +
> > +static void tpm_crb_sysbus_register(void)
> > +{
> > +    type_register_static(&tpm_crb_sysbus_info);
> > +}
> > +
> > +type_init(tpm_crb_sysbus_register)
> > diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
> > index 7e68348440..efe1beaa7b 100644
> > --- a/hw/arm/Kconfig
> > +++ b/hw/arm/Kconfig
> > @@ -5,6 +5,7 @@ config ARM_VIRT
> >      imply VFIO_AMD_XGBE
> >      imply VFIO_PLATFORM
> >      imply VFIO_XGMAC
> > +    imply TPM_CRB_SYSBUS
> >      imply TPM_TIS_SYSBUS
> >      imply TPM_TIS_I2C
> >      imply NVDIMM
> > diff --git a/hw/riscv/Kconfig b/hw/riscv/Kconfig
> > index b6a5eb4452..d824cb58f9 100644
> > --- a/hw/riscv/Kconfig
> > +++ b/hw/riscv/Kconfig
> > @@ -29,6 +29,7 @@ config RISCV_VIRT
> >      imply PCI_DEVICES
> >      imply VIRTIO_VGA
> >      imply TEST_DEVICES
> > +    imply TPM_CRB_SYSBUS
> >      imply TPM_TIS_SYSBUS
> >      select RISCV_NUMA
> >      select GOLDFISH_RTC
> > diff --git a/hw/tpm/Kconfig b/hw/tpm/Kconfig
> > index 1fd73fe617..3f294a20ba 100644
> > --- a/hw/tpm/Kconfig
> > +++ b/hw/tpm/Kconfig
> > @@ -25,6 +25,11 @@ config TPM_CRB
> >      depends on TPM && ISA_BUS
> >      select TPM_BACKEND
> >
> > +config TPM_CRB_SYSBUS
> > +    bool
> > +    depends on TPM
> > +    select TPM_BACKEND
> > +
> >  config TPM_SPAPR
> >      bool
> >      default y
> > diff --git a/hw/tpm/meson.build b/hw/tpm/meson.build
> > index cb8204d5bc..d96de92c16 100644
> > --- a/hw/tpm/meson.build
> > +++ b/hw/tpm/meson.build
> > @@ -4,6 +4,8 @@ system_ss.add(when: 'CONFIG_TPM_TIS_SYSBUS', if_true: files('tpm_tis_sysbus.c'))
> >  system_ss.add(when: 'CONFIG_TPM_TIS_I2C', if_true: files('tpm_tis_i2c.c'))
> >  system_ss.add(when: 'CONFIG_TPM_CRB', if_true: files('tpm_crb.c'))
> >  system_ss.add(when: 'CONFIG_TPM_CRB', if_true: files('tpm_crb_common.c'))
> > +system_ss.add(when: 'CONFIG_TPM_CRB_SYSBUS', if_true: files('tpm_crb_sysbus.c'))
> > +system_ss.add(when: 'CONFIG_TPM_CRB_SYSBUS', if_true: files('tpm_crb_common.c'))
> >  system_ss.add(when: 'CONFIG_TPM_TIS', if_true: files('tpm_ppi.c'))
> >  system_ss.add(when: 'CONFIG_TPM_CRB', if_true: files('tpm_ppi.c'))
> >
>
diff mbox series

Patch

diff --git a/docs/specs/tpm.rst b/docs/specs/tpm.rst
index 2bc29c9804..95aeb49220 100644
--- a/docs/specs/tpm.rst
+++ b/docs/specs/tpm.rst
@@ -46,6 +46,7 @@  operating system.
 QEMU files related to TPM CRB interface:
  - ``hw/tpm/tpm_crb.c``
  - ``hw/tpm/tpm_crb_common.c``
+ - ``hw/tpm/tpm_crb_sysbus.c``
 
 SPAPR interface
 ---------------
diff --git a/include/hw/acpi/aml-build.h b/include/hw/acpi/aml-build.h
index d1fb08514b..9660e16148 100644
--- a/include/hw/acpi/aml-build.h
+++ b/include/hw/acpi/aml-build.h
@@ -3,6 +3,7 @@ 
 
 #include "hw/acpi/acpi-defs.h"
 #include "hw/acpi/bios-linker-loader.h"
+#include "exec/hwaddr.h"
 
 #define ACPI_BUILD_APPNAME6 "BOCHS "
 #define ACPI_BUILD_APPNAME8 "BXPC    "
diff --git a/include/sysemu/tpm.h b/include/sysemu/tpm.h
index 66e3b45f30..f79c8f3575 100644
--- a/include/sysemu/tpm.h
+++ b/include/sysemu/tpm.h
@@ -47,6 +47,7 @@  struct TPMIfClass {
 #define TYPE_TPM_TIS_ISA            "tpm-tis"
 #define TYPE_TPM_TIS_SYSBUS         "tpm-tis-device"
 #define TYPE_TPM_CRB                "tpm-crb"
+#define TYPE_TPM_CRB_SYSBUS         "tpm-crb-device"
 #define TYPE_TPM_SPAPR              "tpm-spapr"
 #define TYPE_TPM_TIS_I2C            "tpm-tis-i2c"
 
@@ -56,6 +57,8 @@  struct TPMIfClass {
     object_dynamic_cast(OBJECT(chr), TYPE_TPM_TIS_SYSBUS)
 #define TPM_IS_CRB(chr)                             \
     object_dynamic_cast(OBJECT(chr), TYPE_TPM_CRB)
+#define TPM_IS_CRB_SYSBUS(chr)                      \
+    object_dynamic_cast(OBJECT(chr), TYPE_TPM_CRB_SYSBUS)
 #define TPM_IS_SPAPR(chr)                           \
     object_dynamic_cast(OBJECT(chr), TYPE_TPM_SPAPR)
 #define TPM_IS_TIS_I2C(chr)                      \
diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
index ea331a20d1..f809137fc9 100644
--- a/hw/acpi/aml-build.c
+++ b/hw/acpi/aml-build.c
@@ -31,6 +31,7 @@ 
 #include "hw/pci/pci_bus.h"
 #include "hw/pci/pci_bridge.h"
 #include "qemu/cutils.h"
+#include "qom/object.h"
 
 static GArray *build_alloc_array(void)
 {
@@ -2218,7 +2219,7 @@  void build_tpm2(GArray *table_data, BIOSLinker *linker, GArray *tcpalog,
 {
     uint8_t start_method_params[12] = {};
     unsigned log_addr_offset;
-    uint64_t control_area_start_address;
+    uint64_t baseaddr, control_area_start_address;
     TPMIf *tpmif = tpm_find();
     uint32_t start_method;
     AcpiTable table = { .sig = "TPM2", .rev = 4,
@@ -2236,6 +2237,10 @@  void build_tpm2(GArray *table_data, BIOSLinker *linker, GArray *tcpalog,
     } else if (TPM_IS_CRB(tpmif)) {
         control_area_start_address = TPM_CRB_ADDR_CTRL;
         start_method = TPM2_START_METHOD_CRB;
+    } else if (TPM_IS_CRB_SYSBUS(tpmif)) {
+        baseaddr = object_property_get_uint(OBJECT(tpmif), "baseaddr", NULL);
+        control_area_start_address = baseaddr + A_CRB_CTRL_REQ;
+        start_method = TPM2_START_METHOD_CRB;
     } else {
         g_assert_not_reached();
     }
diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 432148ef47..88e8b16103 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -2977,6 +2977,7 @@  static void virt_machine_class_init(ObjectClass *oc, void *data)
     machine_class_allow_dynamic_sysbus_dev(mc, TYPE_VFIO_PLATFORM);
 #ifdef CONFIG_TPM
     machine_class_allow_dynamic_sysbus_dev(mc, TYPE_TPM_TIS_SYSBUS);
+    machine_class_allow_dynamic_sysbus_dev(mc, TYPE_TPM_CRB_SYSBUS);
 #endif
     mc->block_default_type = IF_VIRTIO;
     mc->no_cdrom = 1;
diff --git a/hw/core/sysbus-fdt.c b/hw/core/sysbus-fdt.c
index eebcd28f9a..9c783f88eb 100644
--- a/hw/core/sysbus-fdt.c
+++ b/hw/core/sysbus-fdt.c
@@ -493,6 +493,7 @@  static const BindingEntry bindings[] = {
 #endif
 #ifdef CONFIG_TPM
     TYPE_BINDING(TYPE_TPM_TIS_SYSBUS, add_tpm_tis_fdt_node),
+    TYPE_BINDING(TYPE_TPM_CRB_SYSBUS, no_fdt_node),
 #endif
     TYPE_BINDING(TYPE_RAMFB_DEVICE, no_fdt_node),
     TYPE_BINDING("", NULL), /* last element */
diff --git a/hw/loongarch/virt.c b/hw/loongarch/virt.c
index 9c536c52bc..eb59fb04ee 100644
--- a/hw/loongarch/virt.c
+++ b/hw/loongarch/virt.c
@@ -1194,6 +1194,7 @@  static void loongarch_class_init(ObjectClass *oc, void *data)
     machine_class_allow_dynamic_sysbus_dev(mc, TYPE_RAMFB_DEVICE);
 #ifdef CONFIG_TPM
     machine_class_allow_dynamic_sysbus_dev(mc, TYPE_TPM_TIS_SYSBUS);
+    machine_class_allow_dynamic_sysbus_dev(mc, TYPE_TPM_CRB_SYSBUS);
 #endif
 }
 
diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
index d90286dc46..5d639a870a 100644
--- a/hw/riscv/virt.c
+++ b/hw/riscv/virt.c
@@ -1681,6 +1681,7 @@  static void virt_machine_class_init(ObjectClass *oc, void *data)
     machine_class_allow_dynamic_sysbus_dev(mc, TYPE_RAMFB_DEVICE);
 #ifdef CONFIG_TPM
     machine_class_allow_dynamic_sysbus_dev(mc, TYPE_TPM_TIS_SYSBUS);
+    machine_class_allow_dynamic_sysbus_dev(mc, TYPE_TPM_CRB_SYSBUS);
 #endif
 
     if (tcg_enabled()) {
diff --git a/hw/tpm/tpm_crb_sysbus.c b/hw/tpm/tpm_crb_sysbus.c
new file mode 100644
index 0000000000..66be57a532
--- /dev/null
+++ b/hw/tpm/tpm_crb_sysbus.c
@@ -0,0 +1,170 @@ 
+/*
+ * tpm_crb_sysbus.c - QEMU's TPM CRB interface emulator
+ *
+ * Copyright (c) 2018 Red Hat, Inc.
+ *
+ * Authors:
+ *   Marc-André Lureau <marcandre.lureau@redhat.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ *
+ * tpm_crb is a device for TPM 2.0 Command Response Buffer (CRB) Interface
+ * as defined in TCG PC Client Platform TPM Profile (PTP) Specification
+ * Family “2.0” Level 00 Revision 01.03 v22
+ */
+
+#include "qemu/osdep.h"
+#include "hw/acpi/acpi_aml_interface.h"
+#include "hw/acpi/tpm.h"
+#include "hw/qdev-properties.h"
+#include "migration/vmstate.h"
+#include "tpm_prop.h"
+#include "hw/pci/pci_ids.h"
+#include "hw/sysbus.h"
+#include "qapi/visitor.h"
+#include "qom/object.h"
+#include "sysemu/tpm_util.h"
+#include "trace.h"
+#include "tpm_crb.h"
+
+struct TPMCRBStateSysBus {
+    /*< private >*/
+    SysBusDevice parent_obj;
+
+    /*< public >*/
+    TPMCRBState state;
+    uint64_t baseaddr;
+    uint64_t size;
+};
+
+OBJECT_DECLARE_SIMPLE_TYPE(TPMCRBStateSysBus, TPM_CRB_SYSBUS)
+
+static void tpm_crb_sysbus_request_completed(TPMIf *ti, int ret)
+{
+    TPMCRBStateSysBus *s = TPM_CRB_SYSBUS(ti);
+
+    return tpm_crb_request_completed(&s->state, ret);
+}
+
+static enum TPMVersion tpm_crb_sysbus_get_tpm_version(TPMIf *ti)
+{
+    TPMCRBStateSysBus *s = TPM_CRB_SYSBUS(ti);
+
+    return tpm_crb_get_version(&s->state);
+}
+
+static int tpm_crb_sysbus_pre_save(void *opaque)
+{
+    TPMCRBStateSysBus *s = opaque;
+
+    return tpm_crb_pre_save(&s->state);
+}
+
+static const VMStateDescription vmstate_tpm_crb_sysbus = {
+    .name = "tpm-crb-sysbus",
+    .pre_save = tpm_crb_sysbus_pre_save,
+    .fields = (VMStateField[]) {
+        VMSTATE_END_OF_LIST(),
+    }
+};
+
+static Property tpm_crb_sysbus_properties[] = {
+    DEFINE_PROP_TPMBE("tpmdev", TPMCRBStateSysBus, state.tpmbe),
+    DEFINE_PROP_UINT64("baseaddr", TPMCRBStateSysBus,
+                       baseaddr, TPM_CRB_ADDR_BASE),
+    DEFINE_PROP_UINT64("size", TPMCRBStateSysBus, size, TPM_CRB_ADDR_SIZE),
+    DEFINE_PROP_END_OF_LIST(),
+};
+
+static void tpm_crb_sysbus_initfn(Object *obj)
+{
+    TPMCRBStateSysBus *s = TPM_CRB_SYSBUS(obj);
+
+    tpm_crb_init_memory(obj, &s->state, NULL);
+
+    sysbus_init_mmio(SYS_BUS_DEVICE(obj), &s->state.mmio);
+}
+
+static void tpm_crb_sysbus_reset(DeviceState *dev)
+{
+    TPMCRBStateSysBus *s = TPM_CRB_SYSBUS(dev);
+
+    return tpm_crb_reset(&s->state, s->baseaddr);
+}
+
+static void tpm_crb_sysbus_realizefn(DeviceState *dev, Error **errp)
+{
+    TPMCRBStateSysBus *s = TPM_CRB_SYSBUS(dev);
+
+    if (!tpm_find()) {
+        error_setg(errp, "at most one TPM device is permitted");
+        return;
+    }
+
+    if (!s->state.tpmbe) {
+        error_setg(errp, "'tpmdev' property is required");
+        return;
+    }
+
+    if (tpm_crb_sysbus_get_tpm_version(TPM_IF(s)) != TPM_VERSION_2_0) {
+        error_setg(errp, "TPM CRB only supports TPM 2.0 backends");
+        return;
+    }
+}
+
+static void build_tpm_crb_sysbus_aml(AcpiDevAmlIf *adev, Aml *scope)
+{
+    Aml *dev, *crs;
+    TPMCRBStateSysBus *s = TPM_CRB_SYSBUS(adev);
+    TPMIf *ti = TPM_IF(s);
+
+    dev = aml_device("TPM");
+    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(1)));
+    aml_append(dev, aml_name_decl("_STA", aml_int(0xF)));
+    crs = aml_resource_template();
+    aml_append(crs, aml_memory32_fixed(s->baseaddr, s->size,
+                                      AML_READ_WRITE));
+    aml_append(dev, aml_name_decl("_CRS", crs));
+    aml_append(scope, dev);
+}
+
+static void tpm_crb_sysbus_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+    TPMIfClass *tc = TPM_IF_CLASS(klass);
+    AcpiDevAmlIfClass *adevc = ACPI_DEV_AML_IF_CLASS(klass);
+
+    device_class_set_props(dc, tpm_crb_sysbus_properties);
+    dc->vmsd  = &vmstate_tpm_crb_sysbus;
+    tc->model = TPM_MODEL_TPM_CRB;
+    dc->realize = tpm_crb_sysbus_realizefn;
+    dc->user_creatable = true;
+    dc->reset = tpm_crb_sysbus_reset;
+    tc->request_completed = tpm_crb_sysbus_request_completed;
+    tc->get_version = tpm_crb_sysbus_get_tpm_version;
+    set_bit(DEVICE_CATEGORY_MISC, dc->categories);
+    adevc->build_dev_aml = build_tpm_crb_sysbus_aml;
+}
+
+static const TypeInfo tpm_crb_sysbus_info = {
+    .name = TYPE_TPM_CRB_SYSBUS,
+    .parent = TYPE_SYS_BUS_DEVICE,
+    .instance_size = sizeof(TPMCRBStateSysBus),
+    .instance_init = tpm_crb_sysbus_initfn,
+    .class_init  = tpm_crb_sysbus_class_init,
+    .interfaces = (InterfaceInfo[]) {
+        { TYPE_TPM_IF },
+        { TYPE_ACPI_DEV_AML_IF },
+        { }
+    }
+};
+
+static void tpm_crb_sysbus_register(void)
+{
+    type_register_static(&tpm_crb_sysbus_info);
+}
+
+type_init(tpm_crb_sysbus_register)
diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
index 7e68348440..efe1beaa7b 100644
--- a/hw/arm/Kconfig
+++ b/hw/arm/Kconfig
@@ -5,6 +5,7 @@  config ARM_VIRT
     imply VFIO_AMD_XGBE
     imply VFIO_PLATFORM
     imply VFIO_XGMAC
+    imply TPM_CRB_SYSBUS
     imply TPM_TIS_SYSBUS
     imply TPM_TIS_I2C
     imply NVDIMM
diff --git a/hw/riscv/Kconfig b/hw/riscv/Kconfig
index b6a5eb4452..d824cb58f9 100644
--- a/hw/riscv/Kconfig
+++ b/hw/riscv/Kconfig
@@ -29,6 +29,7 @@  config RISCV_VIRT
     imply PCI_DEVICES
     imply VIRTIO_VGA
     imply TEST_DEVICES
+    imply TPM_CRB_SYSBUS
     imply TPM_TIS_SYSBUS
     select RISCV_NUMA
     select GOLDFISH_RTC
diff --git a/hw/tpm/Kconfig b/hw/tpm/Kconfig
index 1fd73fe617..3f294a20ba 100644
--- a/hw/tpm/Kconfig
+++ b/hw/tpm/Kconfig
@@ -25,6 +25,11 @@  config TPM_CRB
     depends on TPM && ISA_BUS
     select TPM_BACKEND
 
+config TPM_CRB_SYSBUS
+    bool
+    depends on TPM
+    select TPM_BACKEND
+
 config TPM_SPAPR
     bool
     default y
diff --git a/hw/tpm/meson.build b/hw/tpm/meson.build
index cb8204d5bc..d96de92c16 100644
--- a/hw/tpm/meson.build
+++ b/hw/tpm/meson.build
@@ -4,6 +4,8 @@  system_ss.add(when: 'CONFIG_TPM_TIS_SYSBUS', if_true: files('tpm_tis_sysbus.c'))
 system_ss.add(when: 'CONFIG_TPM_TIS_I2C', if_true: files('tpm_tis_i2c.c'))
 system_ss.add(when: 'CONFIG_TPM_CRB', if_true: files('tpm_crb.c'))
 system_ss.add(when: 'CONFIG_TPM_CRB', if_true: files('tpm_crb_common.c'))
+system_ss.add(when: 'CONFIG_TPM_CRB_SYSBUS', if_true: files('tpm_crb_sysbus.c'))
+system_ss.add(when: 'CONFIG_TPM_CRB_SYSBUS', if_true: files('tpm_crb_common.c'))
 system_ss.add(when: 'CONFIG_TPM_TIS', if_true: files('tpm_ppi.c'))
 system_ss.add(when: 'CONFIG_TPM_CRB', if_true: files('tpm_ppi.c'))