| Message ID | 20231114020927.62315-6-j@getutm.app |
|---|---|
| State | New |
| Headers | show |
| Series | tpm: introduce TPM CRB SysBus device | expand |
On 11/13/23 21:09, Joelle van Dyne wrote: > This logic is similar to TPM TIS ISA device. Since TPM CRB can only > support TPM 2.0 backends, we check for this in realize. > > Signed-off-by: Joelle van Dyne <j@getutm.app> > --- > hw/tpm/tpm_crb.h | 2 ++ > hw/i386/acpi-build.c | 16 +--------------- > hw/tpm/tpm_crb.c | 16 ++++++++++++++++ > hw/tpm/tpm_crb_common.c | 19 +++++++++++++++++++ > 4 files changed, 38 insertions(+), 15 deletions(-) > > diff --git a/hw/tpm/tpm_crb.h b/hw/tpm/tpm_crb.h > index 36863e1664..e6a86e3fd1 100644 > --- a/hw/tpm/tpm_crb.h > +++ b/hw/tpm/tpm_crb.h > @@ -73,5 +73,7 @@ void tpm_crb_init_memory(Object *obj, TPMCRBState *s, Error **errp); > void tpm_crb_mem_save(TPMCRBState *s, uint32_t *saved_regs, void *saved_cmdmem); > void tpm_crb_mem_load(TPMCRBState *s, const uint32_t *saved_regs, > const void *saved_cmdmem); > +void tpm_crb_build_aml(TPMIf *ti, Aml *scope, uint32_t baseaddr, uint32_t size, > + bool build_ppi); > > #endif /* TPM_TPM_CRB_H */ > diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c > index 80db183b78..7491cee2af 100644 > --- a/hw/i386/acpi-build.c > +++ b/hw/i386/acpi-build.c > @@ -1792,21 +1792,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker, > > #ifdef CONFIG_TPM > if (TPM_IS_CRB(tpm)) { > - 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"))); > - crs = aml_resource_template(); > - aml_append(crs, aml_memory32_fixed(TPM_CRB_ADDR_BASE, > - TPM_CRB_ADDR_SIZE, AML_READ_WRITE)); > - aml_append(dev, aml_name_decl("_CRS", crs)); > - > - aml_append(dev, aml_name_decl("_STA", aml_int(0xf))); > - aml_append(dev, aml_name_decl("_UID", aml_int(1))); > - > - tpm_build_ppi_acpi(tpm, dev); > - > - aml_append(sb_scope, dev); > + call_dev_aml_func(DEVICE(tpm), scope); For an x86_64 VM we have to call it directly otherwise the ACPI table won't be there. tpm_crb_build_aml(tpm, sb_scope, TPM_CRB_ADDR_BASE, TPM_CRB_ADDR_SIZE, true);
On 11/14/23 11:37, Stefan Berger wrote: > > > On 11/13/23 21:09, Joelle van Dyne wrote: >> This logic is similar to TPM TIS ISA device. Since TPM CRB can only >> support TPM 2.0 backends, we check for this in realize. >> >> Signed-off-by: Joelle van Dyne <j@getutm.app> >> --- >> hw/tpm/tpm_crb.h | 2 ++ >> hw/i386/acpi-build.c | 16 +--------------- >> hw/tpm/tpm_crb.c | 16 ++++++++++++++++ >> hw/tpm/tpm_crb_common.c | 19 +++++++++++++++++++ >> 4 files changed, 38 insertions(+), 15 deletions(-) >> >> diff --git a/hw/tpm/tpm_crb.h b/hw/tpm/tpm_crb.h >> index 36863e1664..e6a86e3fd1 100644 >> --- a/hw/tpm/tpm_crb.h >> +++ b/hw/tpm/tpm_crb.h >> @@ -73,5 +73,7 @@ void tpm_crb_init_memory(Object *obj, TPMCRBState >> *s, Error **errp); >> void tpm_crb_mem_save(TPMCRBState *s, uint32_t *saved_regs, void >> *saved_cmdmem); >> void tpm_crb_mem_load(TPMCRBState *s, const uint32_t *saved_regs, >> const void *saved_cmdmem); >> +void tpm_crb_build_aml(TPMIf *ti, Aml *scope, uint32_t baseaddr, >> uint32_t size, >> + bool build_ppi); >> >> #endif /* TPM_TPM_CRB_H */ >> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c >> index 80db183b78..7491cee2af 100644 >> --- a/hw/i386/acpi-build.c >> +++ b/hw/i386/acpi-build.c >> @@ -1792,21 +1792,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker, >> >> #ifdef CONFIG_TPM >> if (TPM_IS_CRB(tpm)) { >> - 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"))); >> - crs = aml_resource_template(); >> - aml_append(crs, aml_memory32_fixed(TPM_CRB_ADDR_BASE, >> - TPM_CRB_ADDR_SIZE, >> AML_READ_WRITE)); >> - aml_append(dev, aml_name_decl("_CRS", crs)); >> - >> - aml_append(dev, aml_name_decl("_STA", aml_int(0xf))); >> - aml_append(dev, aml_name_decl("_UID", aml_int(1))); >> - >> - tpm_build_ppi_acpi(tpm, dev); >> - >> - aml_append(sb_scope, dev); >> + call_dev_aml_func(DEVICE(tpm), scope); > > For an x86_64 VM we have to call it directly otherwise the ACPI table > won't be there. > > tpm_crb_build_aml(tpm, sb_scope, TPM_CRB_ADDR_BASE, > TPM_CRB_ADDR_SIZE, true); > > I looks like a good place for the moved code would be in hw/acpi/tpm.c
On Tue, Nov 14, 2023 at 8:44 AM Stefan Berger <stefanb@linux.ibm.com> wrote: > > > > On 11/14/23 11:37, Stefan Berger wrote: > > > > > > On 11/13/23 21:09, Joelle van Dyne wrote: > >> This logic is similar to TPM TIS ISA device. Since TPM CRB can only > >> support TPM 2.0 backends, we check for this in realize. > >> > >> Signed-off-by: Joelle van Dyne <j@getutm.app> > >> --- > >> hw/tpm/tpm_crb.h | 2 ++ > >> hw/i386/acpi-build.c | 16 +--------------- > >> hw/tpm/tpm_crb.c | 16 ++++++++++++++++ > >> hw/tpm/tpm_crb_common.c | 19 +++++++++++++++++++ > >> 4 files changed, 38 insertions(+), 15 deletions(-) > >> > >> diff --git a/hw/tpm/tpm_crb.h b/hw/tpm/tpm_crb.h > >> index 36863e1664..e6a86e3fd1 100644 > >> --- a/hw/tpm/tpm_crb.h > >> +++ b/hw/tpm/tpm_crb.h > >> @@ -73,5 +73,7 @@ void tpm_crb_init_memory(Object *obj, TPMCRBState > >> *s, Error **errp); > >> void tpm_crb_mem_save(TPMCRBState *s, uint32_t *saved_regs, void > >> *saved_cmdmem); > >> void tpm_crb_mem_load(TPMCRBState *s, const uint32_t *saved_regs, > >> const void *saved_cmdmem); > >> +void tpm_crb_build_aml(TPMIf *ti, Aml *scope, uint32_t baseaddr, > >> uint32_t size, > >> + bool build_ppi); > >> > >> #endif /* TPM_TPM_CRB_H */ > >> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c > >> index 80db183b78..7491cee2af 100644 > >> --- a/hw/i386/acpi-build.c > >> +++ b/hw/i386/acpi-build.c > >> @@ -1792,21 +1792,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker, > >> > >> #ifdef CONFIG_TPM > >> if (TPM_IS_CRB(tpm)) { > >> - 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"))); > >> - crs = aml_resource_template(); > >> - aml_append(crs, aml_memory32_fixed(TPM_CRB_ADDR_BASE, > >> - TPM_CRB_ADDR_SIZE, > >> AML_READ_WRITE)); > >> - aml_append(dev, aml_name_decl("_CRS", crs)); > >> - > >> - aml_append(dev, aml_name_decl("_STA", aml_int(0xf))); > >> - aml_append(dev, aml_name_decl("_UID", aml_int(1))); > >> - > >> - tpm_build_ppi_acpi(tpm, dev); > >> - > >> - aml_append(sb_scope, dev); > >> + call_dev_aml_func(DEVICE(tpm), scope); > > > > For an x86_64 VM we have to call it directly otherwise the ACPI table > > won't be there. > > > > tpm_crb_build_aml(tpm, sb_scope, TPM_CRB_ADDR_BASE, > > TPM_CRB_ADDR_SIZE, true); > > > > > I looks like a good place for the moved code would be in hw/acpi/tpm.c The change in v5 is that we call "call_dev_aml_func" which calls "tpm_crb_build_aml" from tpm_crb.c (later moved to tpm_crb_common.c). I think it's a better place for it because TPM TIS has a different ACPI table and both can be handled correctly by call_dev_aml_func. In the previous versions when we moved TPM CRB to ISA there was no need to manually call "call_dev_aml_func" because the function for building the ISA bus called it. However, I think this is still better in case it is decided in the future to move the CRB device on ISA or another bus and it makes the code more manageable with more TPM types.
diff --git a/hw/tpm/tpm_crb.h b/hw/tpm/tpm_crb.h index 36863e1664..e6a86e3fd1 100644 --- a/hw/tpm/tpm_crb.h +++ b/hw/tpm/tpm_crb.h @@ -73,5 +73,7 @@ void tpm_crb_init_memory(Object *obj, TPMCRBState *s, Error **errp); void tpm_crb_mem_save(TPMCRBState *s, uint32_t *saved_regs, void *saved_cmdmem); void tpm_crb_mem_load(TPMCRBState *s, const uint32_t *saved_regs, const void *saved_cmdmem); +void tpm_crb_build_aml(TPMIf *ti, Aml *scope, uint32_t baseaddr, uint32_t size, + bool build_ppi); #endif /* TPM_TPM_CRB_H */ diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c index 80db183b78..7491cee2af 100644 --- a/hw/i386/acpi-build.c +++ b/hw/i386/acpi-build.c @@ -1792,21 +1792,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker, #ifdef CONFIG_TPM if (TPM_IS_CRB(tpm)) { - 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"))); - crs = aml_resource_template(); - aml_append(crs, aml_memory32_fixed(TPM_CRB_ADDR_BASE, - TPM_CRB_ADDR_SIZE, AML_READ_WRITE)); - aml_append(dev, aml_name_decl("_CRS", crs)); - - aml_append(dev, aml_name_decl("_STA", aml_int(0xf))); - aml_append(dev, aml_name_decl("_UID", aml_int(1))); - - tpm_build_ppi_acpi(tpm, dev); - - aml_append(sb_scope, dev); + call_dev_aml_func(DEVICE(tpm), scope); } #endif diff --git a/hw/tpm/tpm_crb.c b/hw/tpm/tpm_crb.c index 99c64dd72a..8d57295b15 100644 --- a/hw/tpm/tpm_crb.c +++ b/hw/tpm/tpm_crb.c @@ -19,6 +19,8 @@ #include "qemu/module.h" #include "qapi/error.h" #include "exec/address-spaces.h" +#include "hw/acpi/acpi_aml_interface.h" +#include "hw/acpi/tpm.h" #include "hw/qdev-properties.h" #include "hw/pci/pci_ids.h" #include "hw/acpi/tpm.h" @@ -121,6 +123,11 @@ static void tpm_crb_none_realize(DeviceState *dev, Error **errp) return; } + if (tpm_crb_none_get_version(TPM_IF(s)) != TPM_VERSION_2_0) { + error_setg(errp, "TPM CRB only supports TPM 2.0 backends"); + return; + } + tpm_crb_init_memory(OBJECT(s), &s->state, errp); /* only used for migration */ @@ -142,10 +149,17 @@ static void tpm_crb_none_realize(DeviceState *dev, Error **errp) } } +static void build_tpm_crb_none_aml(AcpiDevAmlIf *adev, Aml *scope) +{ + tpm_crb_build_aml(TPM_IF(adev), scope, TPM_CRB_ADDR_BASE, TPM_CRB_ADDR_SIZE, + true); +} + static void tpm_crb_none_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); dc->realize = tpm_crb_none_realize; device_class_set_props(dc, tpm_crb_none_properties); @@ -154,6 +168,7 @@ static void tpm_crb_none_class_init(ObjectClass *klass, void *data) tc->model = TPM_MODEL_TPM_CRB; tc->get_version = tpm_crb_none_get_version; tc->request_completed = tpm_crb_none_request_completed; + adevc->build_dev_aml = build_tpm_crb_none_aml; set_bit(DEVICE_CATEGORY_MISC, dc->categories); } @@ -166,6 +181,7 @@ static const TypeInfo tpm_crb_none_info = { .class_init = tpm_crb_none_class_init, .interfaces = (InterfaceInfo[]) { { TYPE_TPM_IF }, + { TYPE_ACPI_DEV_AML_IF }, { } } }; diff --git a/hw/tpm/tpm_crb_common.c b/hw/tpm/tpm_crb_common.c index f96a8cf299..09ca55eece 100644 --- a/hw/tpm/tpm_crb_common.c +++ b/hw/tpm/tpm_crb_common.c @@ -241,3 +241,22 @@ void tpm_crb_mem_load(TPMCRBState *s, const uint32_t *saved_regs, memcpy(regs, saved_regs, A_CRB_DATA_BUFFER); memcpy(®s[R_CRB_DATA_BUFFER], saved_cmdmem, CRB_CTRL_CMD_SIZE); } + +void tpm_crb_build_aml(TPMIf *ti, Aml *scope, uint32_t baseaddr, uint32_t size, + bool build_ppi) +{ + Aml *dev, *crs; + + 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(baseaddr, size, AML_READ_WRITE)); + aml_append(dev, aml_name_decl("_CRS", crs)); + if (build_ppi) { + tpm_build_ppi_acpi(ti, dev); + } + aml_append(scope, dev); +}
This logic is similar to TPM TIS ISA device. Since TPM CRB can only support TPM 2.0 backends, we check for this in realize. Signed-off-by: Joelle van Dyne <j@getutm.app> --- hw/tpm/tpm_crb.h | 2 ++ hw/i386/acpi-build.c | 16 +--------------- hw/tpm/tpm_crb.c | 16 ++++++++++++++++ hw/tpm/tpm_crb_common.c | 19 +++++++++++++++++++ 4 files changed, 38 insertions(+), 15 deletions(-)