tools/firmware/hvmloader/config.h | 1 + tools/firmware/hvmloader/util.c | 15 +++- tools/libacpi/Makefile | 2 +- tools/libacpi/acpi2_0.h | 24 +++++++ tools/libacpi/build.c | 111 ++++++++++++++++++++++-------- tools/libacpi/libacpi.h | 4 +- tools/libacpi/ssdt_tpm2.asl | 36 ++++++++++ 7 files changed, 159 insertions(+), 34 deletions(-) create mode 100644 tools/libacpi/ssdt_tpm2.asl
This patch introduces an optional TPM 2 interface definition to the ACPI table,
which is to be used as part of a vTPM 2 implementation.
To enable the new interface - I have made the TPM interface version
configurable in the acpi_config, with the default being the existing 1.2.(TCPA)
I have also added to hvmloader an option to utilise this new config, which can
be triggered by setting the platform/tpm_verion xenstore key.
Signed-off-by: Jennifer Herbert <jennifer.herbert@citrix.com>
---
tools/firmware/hvmloader/config.h | 1 +
tools/firmware/hvmloader/util.c | 15 +++-
tools/libacpi/Makefile | 2 +-
tools/libacpi/acpi2_0.h | 24 +++++++
tools/libacpi/build.c | 111 ++++++++++++++++++++++--------
tools/libacpi/libacpi.h | 4 +-
tools/libacpi/ssdt_tpm2.asl | 36 ++++++++++
7 files changed, 159 insertions(+), 34 deletions(-)
create mode 100644 tools/libacpi/ssdt_tpm2.asl
diff --git a/tools/firmware/hvmloader/config.h b/tools/firmware/hvmloader/config.h
index c82adf6dc5..4dec7195f0 100644
--- a/tools/firmware/hvmloader/config.h
+++ b/tools/firmware/hvmloader/config.h
@@ -56,6 +56,7 @@ extern uint8_t ioapic_version;
#define PCI_ISA_IRQ_MASK 0x0c20U /* ISA IRQs 5,10,11 are PCI connected */
#define ACPI_TIS_HDR_ADDRESS 0xFED40F00UL
+#define ACPI_CRB_HDR_ADDRESS 0xFED40034UL
extern uint32_t pci_mem_start;
extern const uint32_t pci_mem_end;
diff --git a/tools/firmware/hvmloader/util.c b/tools/firmware/hvmloader/util.c
index 581b35e5cf..e3af32581b 100644
--- a/tools/firmware/hvmloader/util.c
+++ b/tools/firmware/hvmloader/util.c
@@ -994,13 +994,24 @@ void hvmloader_acpi_build_tables(struct acpi_config *config,
if ( !strncmp(xenstore_read("platform/acpi_laptop_slate", "0"), "1", 1) )
config->table_flags |= ACPI_HAS_SSDT_LAPTOP_SLATE;
- config->table_flags |= (ACPI_HAS_TCPA | ACPI_HAS_IOAPIC |
+ config->table_flags |= (ACPI_HAS_TPM | ACPI_HAS_IOAPIC |
ACPI_HAS_WAET | ACPI_HAS_PMTIMER |
ACPI_HAS_BUTTONS | ACPI_HAS_VGA |
ACPI_HAS_8042 | ACPI_HAS_CMOS_RTC);
config->acpi_revision = 4;
- config->tis_hdr = (uint16_t *)ACPI_TIS_HDR_ADDRESS;
+ if ( !strncmp(xenstore_read("platform/tpm_version", "0"), "2", 1) ) {
+
+ config->tpm_version = 2;
+ config->crb_hdr = (uint16_t *)ACPI_CRB_HDR_ADDRESS;
+ config->tis_hdr = NULL;
+ }
+ else
+ {
+ config->tpm_version = 1;
+ config->crb_hdr = NULL;
+ config->tis_hdr = (uint16_t *)ACPI_TIS_HDR_ADDRESS;
+ }
config->numa.nr_vmemranges = nr_vmemranges;
config->numa.nr_vnodes = nr_vnodes;
diff --git a/tools/libacpi/Makefile b/tools/libacpi/Makefile
index 60860eaa00..125f29fb54 100644
--- a/tools/libacpi/Makefile
+++ b/tools/libacpi/Makefile
@@ -25,7 +25,7 @@ C_SRC-$(CONFIG_X86) = dsdt_anycpu.c dsdt_15cpu.c dsdt_anycpu_qemu_xen.c dsdt_pvh
C_SRC-$(CONFIG_ARM_64) = dsdt_anycpu_arm.c
DSDT_FILES ?= $(C_SRC-y)
C_SRC = $(addprefix $(ACPI_BUILD_DIR)/, $(DSDT_FILES))
-H_SRC = $(addprefix $(ACPI_BUILD_DIR)/, ssdt_s3.h ssdt_s4.h ssdt_pm.h ssdt_tpm.h ssdt_laptop_slate.h)
+H_SRC = $(addprefix $(ACPI_BUILD_DIR)/, ssdt_s3.h ssdt_s4.h ssdt_pm.h ssdt_tpm.h ssdt_tpm2.h ssdt_laptop_slate.h)
MKDSDT_CFLAGS-$(CONFIG_ARM_64) = -DCONFIG_ARM_64
MKDSDT_CFLAGS-$(CONFIG_X86) = -DCONFIG_X86
diff --git a/tools/libacpi/acpi2_0.h b/tools/libacpi/acpi2_0.h
index 2619ba32db..5754daa985 100644
--- a/tools/libacpi/acpi2_0.h
+++ b/tools/libacpi/acpi2_0.h
@@ -121,6 +121,28 @@ struct acpi_20_tcpa {
};
#define ACPI_2_0_TCPA_LAML_SIZE (64*1024)
+/*
+ * TPM2
+ */
+struct Acpi20TPM2 {
+ struct acpi_header header;
+ uint16_t platform_class;
+ uint16_t reserved;
+ uint64_t control_area_address;
+ uint32_t start_method;
+ uint8_t start_method_params[12];
+ uint32_t log_area_minimum_length;
+ uint64_t log_area_start_address;
+};
+#define TPM2_ACPI_CLASS_CLIENT 0
+#define TPM2_START_METHOD_CRB 7
+
+#define TPM_CRB_ADDR_BASE 0xFED40000
+#define TPM_CRB_ADDR_CTRL (TPM_CRB_ADDR_BASE + 0x40)
+
+#define TPM_LOG_AREA_MINIMUM_SIZE (64 << 10)
+#define TPM_LOG_SIZE (64 << 10)
+
/*
* Fixed ACPI Description Table Structure (FADT) in ACPI 1.0.
*/
@@ -431,6 +453,7 @@ struct acpi_20_slit {
#define ACPI_2_0_RSDT_SIGNATURE ASCII32('R','S','D','T')
#define ACPI_2_0_XSDT_SIGNATURE ASCII32('X','S','D','T')
#define ACPI_2_0_TCPA_SIGNATURE ASCII32('T','C','P','A')
+#define ACPI_2_0_TPM2_SIGNATURE ASCII32('T','P','M','2')
#define ACPI_2_0_HPET_SIGNATURE ASCII32('H','P','E','T')
#define ACPI_2_0_WAET_SIGNATURE ASCII32('W','A','E','T')
#define ACPI_2_0_SRAT_SIGNATURE ASCII32('S','R','A','T')
@@ -444,6 +467,7 @@ struct acpi_20_slit {
#define ACPI_2_0_RSDT_REVISION 0x01
#define ACPI_2_0_XSDT_REVISION 0x01
#define ACPI_2_0_TCPA_REVISION 0x02
+#define ACPI_2_0_TPM2_REVISION 0x04
#define ACPI_2_0_HPET_REVISION 0x01
#define ACPI_2_0_WAET_REVISION 0x01
#define ACPI_1_0_FADT_REVISION 0x01
diff --git a/tools/libacpi/build.c b/tools/libacpi/build.c
index fe2db66a62..478cbec5dd 100644
--- a/tools/libacpi/build.c
+++ b/tools/libacpi/build.c
@@ -19,6 +19,7 @@
#include "ssdt_s3.h"
#include "ssdt_s4.h"
#include "ssdt_tpm.h"
+#include "ssdt_tpm2.h"
#include "ssdt_pm.h"
#include "ssdt_laptop_slate.h"
#include <xen/hvm/hvm_info_table.h>
@@ -352,6 +353,8 @@ static int construct_secondary_tables(struct acpi_ctxt *ctxt,
struct acpi_20_tcpa *tcpa;
unsigned char *ssdt;
void *lasa;
+ struct Acpi20TPM2 *tpm2;
+ void *log;
/* MADT. */
if ( (config->hvminfo->nr_vcpus > 1) || config->hvminfo->apic_mode )
@@ -409,38 +412,86 @@ static int construct_secondary_tables(struct acpi_ctxt *ctxt,
memcpy(ssdt, ssdt_laptop_slate, sizeof(ssdt_laptop_slate));
table_ptrs[nr_tables++] = ctxt->mem_ops.v2p(ctxt, ssdt);
}
-
- /* TPM TCPA and SSDT. */
- if ( (config->table_flags & ACPI_HAS_TCPA) &&
- (config->tis_hdr[0] != 0 && config->tis_hdr[0] != 0xffff) &&
- (config->tis_hdr[1] != 0 && config->tis_hdr[1] != 0xffff) )
+ /* TPM and SSDT. */
+ if (config->table_flags & ACPI_HAS_TPM)
{
- ssdt = ctxt->mem_ops.alloc(ctxt, sizeof(ssdt_tpm), 16);
- if (!ssdt) return -1;
- memcpy(ssdt, ssdt_tpm, sizeof(ssdt_tpm));
- table_ptrs[nr_tables++] = ctxt->mem_ops.v2p(ctxt, ssdt);
-
- tcpa = ctxt->mem_ops.alloc(ctxt, sizeof(struct acpi_20_tcpa), 16);
- if (!tcpa) return -1;
- memset(tcpa, 0, sizeof(*tcpa));
- table_ptrs[nr_tables++] = ctxt->mem_ops.v2p(ctxt, tcpa);
-
- tcpa->header.signature = ACPI_2_0_TCPA_SIGNATURE;
- tcpa->header.length = sizeof(*tcpa);
- tcpa->header.revision = ACPI_2_0_TCPA_REVISION;
- fixed_strcpy(tcpa->header.oem_id, ACPI_OEM_ID);
- fixed_strcpy(tcpa->header.oem_table_id, ACPI_OEM_TABLE_ID);
- tcpa->header.oem_revision = ACPI_OEM_REVISION;
- tcpa->header.creator_id = ACPI_CREATOR_ID;
- tcpa->header.creator_revision = ACPI_CREATOR_REVISION;
- if ( (lasa = ctxt->mem_ops.alloc(ctxt, ACPI_2_0_TCPA_LAML_SIZE, 16)) != NULL )
+ if (config-> tpm_version == 2)
+ {
+ if ( (config->crb_hdr) &&
+ (config->crb_hdr[0] != 0 && config->crb_hdr[0] != 0xffff))
+ {
+ ssdt = ctxt->mem_ops.alloc(ctxt, sizeof(ssdt_tpm2), 16);
+ if (!ssdt) return -1;
+ memcpy(ssdt, ssdt_tpm2, sizeof(ssdt_tpm2));
+ table_ptrs[nr_tables++] = ctxt->mem_ops.v2p(ctxt, ssdt);
+
+ tpm2 = ctxt->mem_ops.alloc(ctxt, sizeof(struct Acpi20TPM2), 16);
+ if (!tpm2) return -1;
+ memset(tpm2, 0, sizeof(*tpm2));
+ table_ptrs[nr_tables++] = ctxt->mem_ops.v2p(ctxt, tpm2);
+
+ tpm2->header.signature = ACPI_2_0_TPM2_SIGNATURE;
+ tpm2->header.length = sizeof(*tpm2);
+ tpm2->header.revision = ACPI_2_0_TPM2_REVISION;
+ fixed_strcpy(tpm2->header.oem_id, ACPI_OEM_ID);
+ fixed_strcpy(tpm2->header.oem_table_id, ACPI_OEM_TABLE_ID);
+ tpm2->header.oem_revision = ACPI_OEM_REVISION;
+ tpm2->header.creator_id = ACPI_CREATOR_ID;
+ tpm2->header.creator_revision = ACPI_CREATOR_REVISION;
+ tpm2->platform_class = TPM2_ACPI_CLASS_CLIENT;
+ tpm2->control_area_address = TPM_CRB_ADDR_CTRL;
+ tpm2->start_method = TPM2_START_METHOD_CRB;
+ tpm2->log_area_minimum_length = TPM_LOG_AREA_MINIMUM_SIZE;
+
+ log = ctxt->mem_ops.alloc(ctxt, TPM_LOG_SIZE, 4096);
+ if (!log) return -1;
+
+ memset(log, 0, TPM_LOG_SIZE);
+ tpm2->log_area_start_address = ctxt->mem_ops.v2p(ctxt, log);
+
+ set_checksum(tpm2,
+ offsetof(struct acpi_header, checksum),
+ tpm2->header.length);
+ }
+ else if (!config->crb_hdr)
+ printf("Error: TPM2 configuration requires CRB header!\n");
+ }
+ else
{
- tcpa->lasa = ctxt->mem_ops.v2p(ctxt, lasa);
- tcpa->laml = ACPI_2_0_TCPA_LAML_SIZE;
- memset(lasa, 0, tcpa->laml);
- set_checksum(tcpa,
- offsetof(struct acpi_header, checksum),
- tcpa->header.length);
+ if ((config->tis_hdr) &&
+ (config->tis_hdr[0] != 0 && config->tis_hdr[0] != 0xffff) &&
+ (config->tis_hdr[1] != 0 && config->tis_hdr[1] != 0xffff))
+ {
+ ssdt = ctxt->mem_ops.alloc(ctxt, sizeof(ssdt_tpm), 16);
+ if (!ssdt) return -1;
+ memcpy(ssdt, ssdt_tpm, sizeof(ssdt_tpm));
+ table_ptrs[nr_tables++] = ctxt->mem_ops.v2p(ctxt, ssdt);
+
+ tcpa = ctxt->mem_ops.alloc(ctxt, sizeof(struct acpi_20_tcpa), 16);
+ if (!tcpa) return -1;
+ memset(tcpa, 0, sizeof(*tcpa));
+ table_ptrs[nr_tables++] = ctxt->mem_ops.v2p(ctxt, tcpa);
+
+ tcpa->header.signature = ACPI_2_0_TCPA_SIGNATURE;
+ tcpa->header.length = sizeof(*tcpa);
+ tcpa->header.revision = ACPI_2_0_TCPA_REVISION;
+ fixed_strcpy(tcpa->header.oem_id, ACPI_OEM_ID);
+ fixed_strcpy(tcpa->header.oem_table_id, ACPI_OEM_TABLE_ID);
+ tcpa->header.oem_revision = ACPI_OEM_REVISION;
+ tcpa->header.creator_id = ACPI_CREATOR_ID;
+ tcpa->header.creator_revision = ACPI_CREATOR_REVISION;
+
+ if ( (lasa = ctxt->mem_ops.alloc(ctxt, ACPI_2_0_TCPA_LAML_SIZE, 16)) != NULL )
+ {
+ tcpa->lasa = ctxt->mem_ops.v2p(ctxt, lasa);
+ tcpa->laml = ACPI_2_0_TCPA_LAML_SIZE;
+ memset(lasa, 0, tcpa->laml);
+ set_checksum(tcpa,
+ offsetof(struct acpi_header, checksum),
+ tcpa->header.length);
+ }
+ } else if (!config->tis_hdr)
+ printf("Error: TPM1.x requires TIS Header!\n");
}
}
diff --git a/tools/libacpi/libacpi.h b/tools/libacpi/libacpi.h
index a2efd23b0b..af8925a9ec 100644
--- a/tools/libacpi/libacpi.h
+++ b/tools/libacpi/libacpi.h
@@ -27,7 +27,7 @@
#define ACPI_HAS_SSDT_PM (1<<4)
#define ACPI_HAS_SSDT_S3 (1<<5)
#define ACPI_HAS_SSDT_S4 (1<<6)
-#define ACPI_HAS_TCPA (1<<7)
+#define ACPI_HAS_TPM (1<<7)
#define ACPI_HAS_IOAPIC (1<<8)
#define ACPI_HAS_WAET (1<<9)
#define ACPI_HAS_PMTIMER (1<<10)
@@ -78,7 +78,9 @@ struct acpi_config {
struct acpi_numa numa;
const struct hvm_info_table *hvminfo;
+ uint8_t tpm_version;
const uint16_t *tis_hdr;
+ const uint16_t *crb_hdr;
/*
* Address where acpi_info should be placed.
diff --git a/tools/libacpi/ssdt_tpm2.asl b/tools/libacpi/ssdt_tpm2.asl
new file mode 100644
index 0000000000..1801c338df
--- /dev/null
+++ b/tools/libacpi/ssdt_tpm2.asl
@@ -0,0 +1,36 @@
+/*
+ * ssdt_tpm2.asl
+ *
+ * Copyright (c) 2018-2022, Citrix Systems, Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU Lesser General Public License as published
+ * by the Free Software Foundation; version 2.1 only. with the special
+ * exception on linking described in file LICENSE.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU Lesser General Public License for more details.
+ */
+
+/* SSDT for TPM CRB Interface for Xen with Qemu device model. */
+
+DefinitionBlock ("SSDT_TPM2.aml", "SSDT", 2, "Xen", "HVM", 0)
+{
+ Device (TPM)
+ {
+ Name (_HID, "MSFT0101" /* TPM 2.0 Security Device */) // _HID: Hardware ID
+ Name (_CRS, ResourceTemplate () // _CRS: Current Resource Settings
+ {
+ Memory32Fixed (ReadWrite,
+ 0xFED40000, // Address Base
+ 0x00001000, // Address Length
+ )
+ })
+ Method (_STA, 0, NotSerialized) // _STA: Status
+ {
+ Return (0x0F)
+ }
+ }
+}
--
2.31.1
On 30/08/2022 21:27, Jennifer Herbert wrote: > This patch introduces an optional TPM 2 interface definition to the ACPI table, > which is to be used as part of a vTPM 2 implementation. > To enable the new interface - I have made the TPM interface version > configurable in the acpi_config, with the default being the existing 1.2.(TCPA) > I have also added to hvmloader an option to utilise this new config, which can > be triggered by setting the platform/tpm_verion xenstore key. > > Signed-off-by: Jennifer Herbert <jennifer.herbert@citrix.com> We're past the 4.17 feature submission deadline. CC'ing Henry. Henry: This is a fairly simple change and a critical building block for getting Windows 11 support on Xen. Given that feature freeze was slipped several weeks for other reasons, this should be considered for inclusion too. Jenny: This needs splitting up into at least 2 patches. Patch 1 should be the rename of ACPI_HAS_{TCPA => TPM} and introduction of tpm_version (inc suitable rearranging). Patch 2 should be the introduction of TPM2. > diff --git a/tools/firmware/hvmloader/util.c b/tools/firmware/hvmloader/util.c > index 581b35e5cf..e3af32581b 100644 > --- a/tools/firmware/hvmloader/util.c > +++ b/tools/firmware/hvmloader/util.c > @@ -994,13 +994,24 @@ void hvmloader_acpi_build_tables(struct acpi_config *config, > if ( !strncmp(xenstore_read("platform/acpi_laptop_slate", "0"), "1", 1) ) > config->table_flags |= ACPI_HAS_SSDT_LAPTOP_SLATE; > > - config->table_flags |= (ACPI_HAS_TCPA | ACPI_HAS_IOAPIC | > + config->table_flags |= (ACPI_HAS_TPM | ACPI_HAS_IOAPIC | > ACPI_HAS_WAET | ACPI_HAS_PMTIMER | > ACPI_HAS_BUTTONS | ACPI_HAS_VGA | > ACPI_HAS_8042 | ACPI_HAS_CMOS_RTC); > config->acpi_revision = 4; > > - config->tis_hdr = (uint16_t *)ACPI_TIS_HDR_ADDRESS; > + if ( !strncmp(xenstore_read("platform/tpm_version", "0"), "2", 1) ) { Brace on new line. Also, this is a new key, so needs an entry in docs/misc/xenstore-paths.pandoc > + > + config->tpm_version = 2; > + config->crb_hdr = (uint16_t *)ACPI_CRB_HDR_ADDRESS; > + config->tis_hdr = NULL; > + } > + else > + { > + config->tpm_version = 1; > + config->crb_hdr = NULL; > + config->tis_hdr = (uint16_t *)ACPI_TIS_HDR_ADDRESS; > + } This logic makes any value, including "0" mean "use TPM 1", which isn't terribly good. Furthermore, ACPI_HAS_TPM doesn't mean "has a TPM", it means "probe for a TPM". So what this actually wants to be is something more like this: s = xenstore_read("platform/tpm-version"); config->tpm_version = stroll(s, NULL, 0); switch ( config->tpm_version ) { case 1: config->table_flags |= ACPI_HAS_TPM; config->tis_hdr = (uint16_t *)ACPI_TIS_HDR_ADDRESS; break; } You don't need to set the NULL values because config is suitably zeroed to begin with, and patch 2 will just add case 2: config->table_flags |= ACPI_HAS_TPM; config->crb_hdr = (uint16_t *)ACPI_CRB_HDR_ADDRESS; break; > diff --git a/tools/libacpi/acpi2_0.h b/tools/libacpi/acpi2_0.h > index 2619ba32db..5754daa985 100644 > --- a/tools/libacpi/acpi2_0.h > +++ b/tools/libacpi/acpi2_0.h > @@ -121,6 +121,28 @@ struct acpi_20_tcpa { > }; > #define ACPI_2_0_TCPA_LAML_SIZE (64*1024) > > +/* > + * TPM2 > + */ > +struct Acpi20TPM2 { acpi_20_tpm2, consistent with everything else here. > diff --git a/tools/libacpi/build.c b/tools/libacpi/build.c > index fe2db66a62..478cbec5dd 100644 > --- a/tools/libacpi/build.c > +++ b/tools/libacpi/build.c > @@ -409,38 +412,86 @@ static int construct_secondary_tables(struct acpi_ctxt *ctxt, > memcpy(ssdt, ssdt_laptop_slate, sizeof(ssdt_laptop_slate)); > table_ptrs[nr_tables++] = ctxt->mem_ops.v2p(ctxt, ssdt); > } > - > - /* TPM TCPA and SSDT. */ > - if ( (config->table_flags & ACPI_HAS_TCPA) && > - (config->tis_hdr[0] != 0 && config->tis_hdr[0] != 0xffff) && > - (config->tis_hdr[1] != 0 && config->tis_hdr[1] != 0xffff) ) > + /* TPM and SSDT. */ > + if (config->table_flags & ACPI_HAS_TPM) > { Style, here and lower down. The end result wants to look something like: if ( config->table_flags & ACPI_HAS_TPM ) { switch ( config->tpm_version ) { case 1: if ( !config->tis_hdr || config->tis_hdr[0] == 0 || config->tis_hdr[0] == 0xffff ) break; ssdt = ... break; } } In particular, I don't think the printf()'s are particularly useful for bad internal input into a probe function. > - ssdt = ctxt->mem_ops.alloc(ctxt, sizeof(ssdt_tpm), 16); > - if (!ssdt) return -1; > - memcpy(ssdt, ssdt_tpm, sizeof(ssdt_tpm)); > - table_ptrs[nr_tables++] = ctxt->mem_ops.v2p(ctxt, ssdt); > - > - tcpa = ctxt->mem_ops.alloc(ctxt, sizeof(struct acpi_20_tcpa), 16); > - if (!tcpa) return -1; > - memset(tcpa, 0, sizeof(*tcpa)); > - table_ptrs[nr_tables++] = ctxt->mem_ops.v2p(ctxt, tcpa); > - > - tcpa->header.signature = ACPI_2_0_TCPA_SIGNATURE; > - tcpa->header.length = sizeof(*tcpa); > - tcpa->header.revision = ACPI_2_0_TCPA_REVISION; > - fixed_strcpy(tcpa->header.oem_id, ACPI_OEM_ID); > - fixed_strcpy(tcpa->header.oem_table_id, ACPI_OEM_TABLE_ID); > - tcpa->header.oem_revision = ACPI_OEM_REVISION; > - tcpa->header.creator_id = ACPI_CREATOR_ID; > - tcpa->header.creator_revision = ACPI_CREATOR_REVISION; > - if ( (lasa = ctxt->mem_ops.alloc(ctxt, ACPI_2_0_TCPA_LAML_SIZE, 16)) != NULL ) > + if (config-> tpm_version == 2) > + { > + if ( (config->crb_hdr) && > + (config->crb_hdr[0] != 0 && config->crb_hdr[0] != 0xffff)) > + { > + ssdt = ctxt->mem_ops.alloc(ctxt, sizeof(ssdt_tpm2), 16); > + if (!ssdt) return -1; > + memcpy(ssdt, ssdt_tpm2, sizeof(ssdt_tpm2)); > + table_ptrs[nr_tables++] = ctxt->mem_ops.v2p(ctxt, ssdt); > + > + tpm2 = ctxt->mem_ops.alloc(ctxt, sizeof(struct Acpi20TPM2), 16); > + if (!tpm2) return -1; > + memset(tpm2, 0, sizeof(*tpm2)); > + table_ptrs[nr_tables++] = ctxt->mem_ops.v2p(ctxt, tpm2); > + > + tpm2->header.signature = ACPI_2_0_TPM2_SIGNATURE; > + tpm2->header.length = sizeof(*tpm2); > + tpm2->header.revision = ACPI_2_0_TPM2_REVISION; > + fixed_strcpy(tpm2->header.oem_id, ACPI_OEM_ID); > + fixed_strcpy(tpm2->header.oem_table_id, ACPI_OEM_TABLE_ID); > + tpm2->header.oem_revision = ACPI_OEM_REVISION; > + tpm2->header.creator_id = ACPI_CREATOR_ID; > + tpm2->header.creator_revision = ACPI_CREATOR_REVISION; > + tpm2->platform_class = TPM2_ACPI_CLASS_CLIENT; > + tpm2->control_area_address = TPM_CRB_ADDR_CTRL; > + tpm2->start_method = TPM2_START_METHOD_CRB; > + tpm2->log_area_minimum_length = TPM_LOG_AREA_MINIMUM_SIZE; > + > + log = ctxt->mem_ops.alloc(ctxt, TPM_LOG_SIZE, 4096); > + if (!log) return -1; This is buggy. APCI table memory is covered by an E820_ACPI range (specifically, is eligible to be used as general RAM after boot), while the TPM log should be in an E820_RESERVED region. To start with, it's probably fine to hardcode something in the 2M window at 0xfed40000 to be fixed location for the log. ~Andrew
This patch makes the TPM version, for which the ACPI library probes, configurable. This version incorpates feedback from v1, including splitting it into two patches, tidying up some formatting, removing debug, and moving the log into a more suitable memory area. Jennifer Herbert (2): acpi: Make TPM version configurable. acpi: Add TPM2 interface definition. docs/misc/xenstore-paths.pandoc | 8 +++ tools/firmware/hvmloader/config.h | 1 + tools/firmware/hvmloader/util.c | 20 +++++- tools/libacpi/Makefile | 2 +- tools/libacpi/acpi2_0.h | 26 ++++++++ tools/libacpi/build.c | 101 +++++++++++++++++++++--------- tools/libacpi/libacpi.h | 5 +- tools/libacpi/ssdt_tpm2.asl | 36 +++++++++++ 8 files changed, 165 insertions(+), 34 deletions(-) create mode 100644 tools/libacpi/ssdt_tpm2.asl -- 2.31.1
This patch makes the TPM version, for which the ACPI library probes, configurable.
If acpi_config.tpm_verison is set to 1, it indicates that 1.2 (TCPA) should be probed.
I have also added to hvmloader an option to allow setting this new config, which can
be triggered by setting the platform/tpm_verion xenstore key.
Signed-off-by: Jennifer Herbert <jennifer.herbert@citrix.com>
---
docs/misc/xenstore-paths.pandoc | 8 ++++
tools/firmware/hvmloader/util.c | 13 ++++++-
tools/libacpi/build.c | 68 ++++++++++++++++++---------------
tools/libacpi/libacpi.h | 4 +-
4 files changed, 59 insertions(+), 34 deletions(-)
diff --git a/docs/misc/xenstore-paths.pandoc b/docs/misc/xenstore-paths.pandoc
index 5cd5c8a3b9..7270b46721 100644
--- a/docs/misc/xenstore-paths.pandoc
+++ b/docs/misc/xenstore-paths.pandoc
@@ -269,6 +269,14 @@ at the guest physical address in HVM_PARAM_VM_GENERATION_ID_ADDR.
See Microsoft's "Virtual Machine Generation ID" specification for the
circumstances where the generation ID needs to be changed.
+
+#### ~/platform/tpm_version = INTEGER [HVM,INTERNAL]
+
+The TPM version to be probed for.
+
+A value of 1 indicates to probe for TPM 1.2. If unset, or an
+invalid version, then no TPM is probed.
+
### Frontend device paths
Paravirtual device frontends are generally specified by their own
diff --git a/tools/firmware/hvmloader/util.c b/tools/firmware/hvmloader/util.c
index 581b35e5cf..87bc2d677f 100644
--- a/tools/firmware/hvmloader/util.c
+++ b/tools/firmware/hvmloader/util.c
@@ -994,13 +994,22 @@ void hvmloader_acpi_build_tables(struct acpi_config *config,
if ( !strncmp(xenstore_read("platform/acpi_laptop_slate", "0"), "1", 1) )
config->table_flags |= ACPI_HAS_SSDT_LAPTOP_SLATE;
- config->table_flags |= (ACPI_HAS_TCPA | ACPI_HAS_IOAPIC |
+ config->table_flags |= (ACPI_HAS_TPM | ACPI_HAS_IOAPIC |
ACPI_HAS_WAET | ACPI_HAS_PMTIMER |
ACPI_HAS_BUTTONS | ACPI_HAS_VGA |
ACPI_HAS_8042 | ACPI_HAS_CMOS_RTC);
config->acpi_revision = 4;
- config->tis_hdr = (uint16_t *)ACPI_TIS_HDR_ADDRESS;
+ s = xenstore_read("platform/tpm_version", "0");
+ config->tpm_version = strtoll(s, NULL, 0);
+
+ switch( config->tpm_version )
+ {
+ case 1:
+ config->table_flags |= ACPI_HAS_TPM;
+ config->tis_hdr = (uint16_t *)ACPI_TIS_HDR_ADDRESS;
+ break;
+ }
config->numa.nr_vmemranges = nr_vmemranges;
config->numa.nr_vnodes = nr_vnodes;
diff --git a/tools/libacpi/build.c b/tools/libacpi/build.c
index fe2db66a62..d313ccd8cf 100644
--- a/tools/libacpi/build.c
+++ b/tools/libacpi/build.c
@@ -409,38 +409,46 @@ static int construct_secondary_tables(struct acpi_ctxt *ctxt,
memcpy(ssdt, ssdt_laptop_slate, sizeof(ssdt_laptop_slate));
table_ptrs[nr_tables++] = ctxt->mem_ops.v2p(ctxt, ssdt);
}
-
- /* TPM TCPA and SSDT. */
- if ( (config->table_flags & ACPI_HAS_TCPA) &&
- (config->tis_hdr[0] != 0 && config->tis_hdr[0] != 0xffff) &&
- (config->tis_hdr[1] != 0 && config->tis_hdr[1] != 0xffff) )
+ /* TPM and SSDT. */
+ if (config->table_flags & ACPI_HAS_TPM)
{
- ssdt = ctxt->mem_ops.alloc(ctxt, sizeof(ssdt_tpm), 16);
- if (!ssdt) return -1;
- memcpy(ssdt, ssdt_tpm, sizeof(ssdt_tpm));
- table_ptrs[nr_tables++] = ctxt->mem_ops.v2p(ctxt, ssdt);
-
- tcpa = ctxt->mem_ops.alloc(ctxt, sizeof(struct acpi_20_tcpa), 16);
- if (!tcpa) return -1;
- memset(tcpa, 0, sizeof(*tcpa));
- table_ptrs[nr_tables++] = ctxt->mem_ops.v2p(ctxt, tcpa);
-
- tcpa->header.signature = ACPI_2_0_TCPA_SIGNATURE;
- tcpa->header.length = sizeof(*tcpa);
- tcpa->header.revision = ACPI_2_0_TCPA_REVISION;
- fixed_strcpy(tcpa->header.oem_id, ACPI_OEM_ID);
- fixed_strcpy(tcpa->header.oem_table_id, ACPI_OEM_TABLE_ID);
- tcpa->header.oem_revision = ACPI_OEM_REVISION;
- tcpa->header.creator_id = ACPI_CREATOR_ID;
- tcpa->header.creator_revision = ACPI_CREATOR_REVISION;
- if ( (lasa = ctxt->mem_ops.alloc(ctxt, ACPI_2_0_TCPA_LAML_SIZE, 16)) != NULL )
+ switch (config->tpm_version)
{
- tcpa->lasa = ctxt->mem_ops.v2p(ctxt, lasa);
- tcpa->laml = ACPI_2_0_TCPA_LAML_SIZE;
- memset(lasa, 0, tcpa->laml);
- set_checksum(tcpa,
- offsetof(struct acpi_header, checksum),
- tcpa->header.length);
+ case 1:
+ if (!config->tis_hdr ||
+ config->tis_hdr[0] == 0 || config->tis_hdr[0] == 0xffff ||
+ config->tis_hdr[1] == 0 || config->tis_hdr[1] == 0xffff)
+ break;
+
+ ssdt = ctxt->mem_ops.alloc(ctxt, sizeof(ssdt_tpm), 16);
+ if (!ssdt) return -1;
+ memcpy(ssdt, ssdt_tpm, sizeof(ssdt_tpm));
+ table_ptrs[nr_tables++] = ctxt->mem_ops.v2p(ctxt, ssdt);
+
+ tcpa = ctxt->mem_ops.alloc(ctxt, sizeof(struct acpi_20_tcpa), 16);
+ if (!tcpa) return -1;
+ memset(tcpa, 0, sizeof(*tcpa));
+ table_ptrs[nr_tables++] = ctxt->mem_ops.v2p(ctxt, tcpa);
+
+ tcpa->header.signature = ACPI_2_0_TCPA_SIGNATURE;
+ tcpa->header.length = sizeof(*tcpa);
+ tcpa->header.revision = ACPI_2_0_TCPA_REVISION;
+ fixed_strcpy(tcpa->header.oem_id, ACPI_OEM_ID);
+ fixed_strcpy(tcpa->header.oem_table_id, ACPI_OEM_TABLE_ID);
+ tcpa->header.oem_revision = ACPI_OEM_REVISION;
+ tcpa->header.creator_id = ACPI_CREATOR_ID;
+ tcpa->header.creator_revision = ACPI_CREATOR_REVISION;
+
+ if ( (lasa = ctxt->mem_ops.alloc(ctxt, ACPI_2_0_TCPA_LAML_SIZE, 16)) != NULL )
+ {
+ tcpa->lasa = ctxt->mem_ops.v2p(ctxt, lasa);
+ tcpa->laml = ACPI_2_0_TCPA_LAML_SIZE;
+ memset(lasa, 0, tcpa->laml);
+ set_checksum(tcpa,
+ offsetof(struct acpi_header, checksum),
+ tcpa->header.length);
+ }
+ break;
}
}
diff --git a/tools/libacpi/libacpi.h b/tools/libacpi/libacpi.h
index a2efd23b0b..9143616130 100644
--- a/tools/libacpi/libacpi.h
+++ b/tools/libacpi/libacpi.h
@@ -27,7 +27,7 @@
#define ACPI_HAS_SSDT_PM (1<<4)
#define ACPI_HAS_SSDT_S3 (1<<5)
#define ACPI_HAS_SSDT_S4 (1<<6)
-#define ACPI_HAS_TCPA (1<<7)
+#define ACPI_HAS_TPM (1<<7)
#define ACPI_HAS_IOAPIC (1<<8)
#define ACPI_HAS_WAET (1<<9)
#define ACPI_HAS_PMTIMER (1<<10)
@@ -78,8 +78,8 @@ struct acpi_config {
struct acpi_numa numa;
const struct hvm_info_table *hvminfo;
+ uint8_t tpm_version;
const uint16_t *tis_hdr;
-
/*
* Address where acpi_info should be placed.
* This must match the OperationRegion(BIOS, SystemMemory, ....)
--
2.31.1
On 15.12.2022 18:09, Jennifer Herbert wrote: > --- a/docs/misc/xenstore-paths.pandoc > +++ b/docs/misc/xenstore-paths.pandoc > @@ -269,6 +269,14 @@ at the guest physical address in HVM_PARAM_VM_GENERATION_ID_ADDR. > See Microsoft's "Virtual Machine Generation ID" specification for the > circumstances where the generation ID needs to be changed. > > + > +#### ~/platform/tpm_version = INTEGER [HVM,INTERNAL] > + > +The TPM version to be probed for. > + > +A value of 1 indicates to probe for TPM 1.2. If unset, or an > +invalid version, then no TPM is probed. And who's writing this new key? Aren't you regressing existing guests by defaulting to "no TPM" in the absence of this key? > --- a/tools/firmware/hvmloader/util.c > +++ b/tools/firmware/hvmloader/util.c > @@ -994,13 +994,22 @@ void hvmloader_acpi_build_tables(struct acpi_config *config, > if ( !strncmp(xenstore_read("platform/acpi_laptop_slate", "0"), "1", 1) ) > config->table_flags |= ACPI_HAS_SSDT_LAPTOP_SLATE; > > - config->table_flags |= (ACPI_HAS_TCPA | ACPI_HAS_IOAPIC | > + config->table_flags |= (ACPI_HAS_TPM | ACPI_HAS_IOAPIC | I'm puzzled by ACPI_HAS_TPM being present both here and ... > ACPI_HAS_WAET | ACPI_HAS_PMTIMER | > ACPI_HAS_BUTTONS | ACPI_HAS_VGA | > ACPI_HAS_8042 | ACPI_HAS_CMOS_RTC); > config->acpi_revision = 4; > > - config->tis_hdr = (uint16_t *)ACPI_TIS_HDR_ADDRESS; > + s = xenstore_read("platform/tpm_version", "0"); > + config->tpm_version = strtoll(s, NULL, 0); > + > + switch( config->tpm_version ) > + { > + case 1: > + config->table_flags |= ACPI_HAS_TPM; .. here. Also (nit): Missing blank after "switch". > --- a/tools/libacpi/build.c > +++ b/tools/libacpi/build.c > @@ -409,38 +409,46 @@ static int construct_secondary_tables(struct acpi_ctxt *ctxt, > memcpy(ssdt, ssdt_laptop_slate, sizeof(ssdt_laptop_slate)); > table_ptrs[nr_tables++] = ctxt->mem_ops.v2p(ctxt, ssdt); > } > - > - /* TPM TCPA and SSDT. */ > - if ( (config->table_flags & ACPI_HAS_TCPA) && > - (config->tis_hdr[0] != 0 && config->tis_hdr[0] != 0xffff) && > - (config->tis_hdr[1] != 0 && config->tis_hdr[1] != 0xffff) ) > + /* TPM and SSDT. */ May I ask that since you're altering the comment, you make it "TPM and its SSDT"? > + if (config->table_flags & ACPI_HAS_TPM) Nit: The file is predominantly using Xen style, so please adhere to that (also again further down). > { > - ssdt = ctxt->mem_ops.alloc(ctxt, sizeof(ssdt_tpm), 16); > - if (!ssdt) return -1; > - memcpy(ssdt, ssdt_tpm, sizeof(ssdt_tpm)); > - table_ptrs[nr_tables++] = ctxt->mem_ops.v2p(ctxt, ssdt); > - > - tcpa = ctxt->mem_ops.alloc(ctxt, sizeof(struct acpi_20_tcpa), 16); > - if (!tcpa) return -1; > - memset(tcpa, 0, sizeof(*tcpa)); > - table_ptrs[nr_tables++] = ctxt->mem_ops.v2p(ctxt, tcpa); > - > - tcpa->header.signature = ACPI_2_0_TCPA_SIGNATURE; > - tcpa->header.length = sizeof(*tcpa); > - tcpa->header.revision = ACPI_2_0_TCPA_REVISION; > - fixed_strcpy(tcpa->header.oem_id, ACPI_OEM_ID); > - fixed_strcpy(tcpa->header.oem_table_id, ACPI_OEM_TABLE_ID); > - tcpa->header.oem_revision = ACPI_OEM_REVISION; > - tcpa->header.creator_id = ACPI_CREATOR_ID; > - tcpa->header.creator_revision = ACPI_CREATOR_REVISION; > - if ( (lasa = ctxt->mem_ops.alloc(ctxt, ACPI_2_0_TCPA_LAML_SIZE, 16)) != NULL ) > + switch (config->tpm_version) > { > - tcpa->lasa = ctxt->mem_ops.v2p(ctxt, lasa); > - tcpa->laml = ACPI_2_0_TCPA_LAML_SIZE; > - memset(lasa, 0, tcpa->laml); > - set_checksum(tcpa, > - offsetof(struct acpi_header, checksum), > - tcpa->header.length); > + case 1: > + if (!config->tis_hdr || There was no such check in the original code, and I think it would better remain the case that the field is set if and only if ACPI_HAS_TPM is set and (now) version is 1. (Aiui this check actually covers for the double setting of ACPI_HAS_TPM commented on above.) > + config->tis_hdr[0] == 0 || config->tis_hdr[0] == 0xffff || > + config->tis_hdr[1] == 0 || config->tis_hdr[1] == 0xffff) > + break; > + > + ssdt = ctxt->mem_ops.alloc(ctxt, sizeof(ssdt_tpm), 16); > + if (!ssdt) return -1; > + memcpy(ssdt, ssdt_tpm, sizeof(ssdt_tpm)); > + table_ptrs[nr_tables++] = ctxt->mem_ops.v2p(ctxt, ssdt); > + > + tcpa = ctxt->mem_ops.alloc(ctxt, sizeof(struct acpi_20_tcpa), 16); > + if (!tcpa) return -1; > + memset(tcpa, 0, sizeof(*tcpa)); > + table_ptrs[nr_tables++] = ctxt->mem_ops.v2p(ctxt, tcpa); > + > + tcpa->header.signature = ACPI_2_0_TCPA_SIGNATURE; > + tcpa->header.length = sizeof(*tcpa); > + tcpa->header.revision = ACPI_2_0_TCPA_REVISION; > + fixed_strcpy(tcpa->header.oem_id, ACPI_OEM_ID); > + fixed_strcpy(tcpa->header.oem_table_id, ACPI_OEM_TABLE_ID); > + tcpa->header.oem_revision = ACPI_OEM_REVISION; > + tcpa->header.creator_id = ACPI_CREATOR_ID; > + tcpa->header.creator_revision = ACPI_CREATOR_REVISION; > + > + if ( (lasa = ctxt->mem_ops.alloc(ctxt, ACPI_2_0_TCPA_LAML_SIZE, 16)) != NULL ) Nit: Please wrap this line (it was too long before, but it's getting worse with the re-indentation). > @@ -78,8 +78,8 @@ struct acpi_config { > struct acpi_numa numa; > const struct hvm_info_table *hvminfo; > > + uint8_t tpm_version; > const uint16_t *tis_hdr; Hmm, sticking a uint8_t between two pointers is kind of inefficient. How about putting it next to acpi_revision and, if so desired to keep related fields together, moving up the tis_hdr field? > - > /* > * Address where acpi_info should be placed. Please don't remove blank lines like the one here, the more when they're unrelated. Jan
This patch introduces an optional TPM 2 interface definition to the ACPI table,
which is to be used as part of a vTPM 2 implementation.
Signed-off-by: Jennifer Herbert <jennifer.herbert@citrix.com>
---
tools/firmware/hvmloader/config.h | 1 +
tools/firmware/hvmloader/util.c | 7 ++++++
tools/libacpi/Makefile | 2 +-
tools/libacpi/acpi2_0.h | 26 ++++++++++++++++++++++
tools/libacpi/build.c | 35 ++++++++++++++++++++++++++++++
tools/libacpi/libacpi.h | 1 +
tools/libacpi/ssdt_tpm2.asl | 36 +++++++++++++++++++++++++++++++
7 files changed, 107 insertions(+), 1 deletion(-)
create mode 100644 tools/libacpi/ssdt_tpm2.asl
diff --git a/tools/firmware/hvmloader/config.h b/tools/firmware/hvmloader/config.h
index c82adf6dc5..4dec7195f0 100644
--- a/tools/firmware/hvmloader/config.h
+++ b/tools/firmware/hvmloader/config.h
@@ -56,6 +56,7 @@ extern uint8_t ioapic_version;
#define PCI_ISA_IRQ_MASK 0x0c20U /* ISA IRQs 5,10,11 are PCI connected */
#define ACPI_TIS_HDR_ADDRESS 0xFED40F00UL
+#define ACPI_CRB_HDR_ADDRESS 0xFED40034UL
extern uint32_t pci_mem_start;
extern const uint32_t pci_mem_end;
diff --git a/tools/firmware/hvmloader/util.c b/tools/firmware/hvmloader/util.c
index 87bc2d677f..6e5d3609b9 100644
--- a/tools/firmware/hvmloader/util.c
+++ b/tools/firmware/hvmloader/util.c
@@ -1009,6 +1009,13 @@ void hvmloader_acpi_build_tables(struct acpi_config *config,
config->table_flags |= ACPI_HAS_TPM;
config->tis_hdr = (uint16_t *)ACPI_TIS_HDR_ADDRESS;
break;
+ case 2:
+ config->table_flags |= ACPI_HAS_TPM;
+ config->crb_hdr = (uint16_t *)ACPI_CRB_HDR_ADDRESS;
+
+ mem_hole_populate_ram(TPM_LOG_AREA_ADDRESS >> PAGE_SHIFT, TPM_LOG_SIZE >> PAGE_SHIFT);
+ memset((void *)(TPM_LOG_AREA_ADDRESS), 0, TPM_LOG_SIZE);
+ break;
}
config->numa.nr_vmemranges = nr_vmemranges;
diff --git a/tools/libacpi/Makefile b/tools/libacpi/Makefile
index 60860eaa00..125f29fb54 100644
--- a/tools/libacpi/Makefile
+++ b/tools/libacpi/Makefile
@@ -25,7 +25,7 @@ C_SRC-$(CONFIG_X86) = dsdt_anycpu.c dsdt_15cpu.c dsdt_anycpu_qemu_xen.c dsdt_pvh
C_SRC-$(CONFIG_ARM_64) = dsdt_anycpu_arm.c
DSDT_FILES ?= $(C_SRC-y)
C_SRC = $(addprefix $(ACPI_BUILD_DIR)/, $(DSDT_FILES))
-H_SRC = $(addprefix $(ACPI_BUILD_DIR)/, ssdt_s3.h ssdt_s4.h ssdt_pm.h ssdt_tpm.h ssdt_laptop_slate.h)
+H_SRC = $(addprefix $(ACPI_BUILD_DIR)/, ssdt_s3.h ssdt_s4.h ssdt_pm.h ssdt_tpm.h ssdt_tpm2.h ssdt_laptop_slate.h)
MKDSDT_CFLAGS-$(CONFIG_ARM_64) = -DCONFIG_ARM_64
MKDSDT_CFLAGS-$(CONFIG_X86) = -DCONFIG_X86
diff --git a/tools/libacpi/acpi2_0.h b/tools/libacpi/acpi2_0.h
index 2619ba32db..f4eb4d715b 100644
--- a/tools/libacpi/acpi2_0.h
+++ b/tools/libacpi/acpi2_0.h
@@ -121,6 +121,30 @@ struct acpi_20_tcpa {
};
#define ACPI_2_0_TCPA_LAML_SIZE (64*1024)
+/*
+ * TPM2
+ */
+struct acpi_20_tpm2 {
+ struct acpi_header header;
+ uint16_t platform_class;
+ uint16_t reserved;
+ uint64_t control_area_address;
+ uint32_t start_method;
+ uint8_t start_method_params[12];
+ uint32_t log_area_minimum_length;
+ uint64_t log_area_start_address;
+};
+#define TPM2_ACPI_CLASS_CLIENT 0
+#define TPM2_START_METHOD_CRB 7
+
+#define TPM_CRB_ADDR_BASE 0xFED40000
+#define TPM_CRB_ADDR_CTRL (TPM_CRB_ADDR_BASE + 0x40)
+
+#define TPM_LOG_AREA_ADDRESS 0xFED50000
+
+#define TPM_LOG_AREA_MINIMUM_SIZE (64 << 10)
+#define TPM_LOG_SIZE (64 << 10)
+
/*
* Fixed ACPI Description Table Structure (FADT) in ACPI 1.0.
*/
@@ -431,6 +455,7 @@ struct acpi_20_slit {
#define ACPI_2_0_RSDT_SIGNATURE ASCII32('R','S','D','T')
#define ACPI_2_0_XSDT_SIGNATURE ASCII32('X','S','D','T')
#define ACPI_2_0_TCPA_SIGNATURE ASCII32('T','C','P','A')
+#define ACPI_2_0_TPM2_SIGNATURE ASCII32('T','P','M','2')
#define ACPI_2_0_HPET_SIGNATURE ASCII32('H','P','E','T')
#define ACPI_2_0_WAET_SIGNATURE ASCII32('W','A','E','T')
#define ACPI_2_0_SRAT_SIGNATURE ASCII32('S','R','A','T')
@@ -444,6 +469,7 @@ struct acpi_20_slit {
#define ACPI_2_0_RSDT_REVISION 0x01
#define ACPI_2_0_XSDT_REVISION 0x01
#define ACPI_2_0_TCPA_REVISION 0x02
+#define ACPI_2_0_TPM2_REVISION 0x04
#define ACPI_2_0_HPET_REVISION 0x01
#define ACPI_2_0_WAET_REVISION 0x01
#define ACPI_1_0_FADT_REVISION 0x01
diff --git a/tools/libacpi/build.c b/tools/libacpi/build.c
index d313ccd8cf..d4f25a68d2 100644
--- a/tools/libacpi/build.c
+++ b/tools/libacpi/build.c
@@ -19,6 +19,7 @@
#include "ssdt_s3.h"
#include "ssdt_s4.h"
#include "ssdt_tpm.h"
+#include "ssdt_tpm2.h"
#include "ssdt_pm.h"
#include "ssdt_laptop_slate.h"
#include <xen/hvm/hvm_info_table.h>
@@ -352,6 +353,7 @@ static int construct_secondary_tables(struct acpi_ctxt *ctxt,
struct acpi_20_tcpa *tcpa;
unsigned char *ssdt;
void *lasa;
+ struct acpi_20_tpm2 *tpm2;
/* MADT. */
if ( (config->hvminfo->nr_vcpus > 1) || config->hvminfo->apic_mode )
@@ -449,6 +451,39 @@ static int construct_secondary_tables(struct acpi_ctxt *ctxt,
tcpa->header.length);
}
break;
+
+ case 2:
+ if (!config->crb_hdr ||
+ config->crb_hdr[0] == 0 || config->crb_hdr[0] == 0xffff)
+ break;
+
+ ssdt = ctxt->mem_ops.alloc(ctxt, sizeof(ssdt_tpm2), 16);
+ if (!ssdt) return -1;
+ memcpy(ssdt, ssdt_tpm2, sizeof(ssdt_tpm2));
+ table_ptrs[nr_tables++] = ctxt->mem_ops.v2p(ctxt, ssdt);
+
+ tpm2 = ctxt->mem_ops.alloc(ctxt, sizeof(struct acpi_20_tpm2), 16);
+ if (!tpm2) return -1;
+ memset(tpm2, 0, sizeof(*tpm2));
+ table_ptrs[nr_tables++] = ctxt->mem_ops.v2p(ctxt, tpm2);
+
+ tpm2->header.signature = ACPI_2_0_TPM2_SIGNATURE;
+ tpm2->header.length = sizeof(*tpm2);
+ tpm2->header.revision = ACPI_2_0_TPM2_REVISION;
+ fixed_strcpy(tpm2->header.oem_id, ACPI_OEM_ID);
+ fixed_strcpy(tpm2->header.oem_table_id, ACPI_OEM_TABLE_ID);
+ tpm2->header.oem_revision = ACPI_OEM_REVISION;
+ tpm2->header.creator_id = ACPI_CREATOR_ID;
+ tpm2->header.creator_revision = ACPI_CREATOR_REVISION;
+ tpm2->platform_class = TPM2_ACPI_CLASS_CLIENT;
+ tpm2->control_area_address = TPM_CRB_ADDR_CTRL;
+ tpm2->start_method = TPM2_START_METHOD_CRB;
+ tpm2->log_area_minimum_length = TPM_LOG_AREA_MINIMUM_SIZE;
+ tpm2->log_area_start_address = TPM_LOG_AREA_ADDRESS;
+
+ set_checksum(tpm2,
+ offsetof(struct acpi_header, checksum),
+ tpm2->header.length);
}
}
diff --git a/tools/libacpi/libacpi.h b/tools/libacpi/libacpi.h
index 9143616130..b5d08ff09b 100644
--- a/tools/libacpi/libacpi.h
+++ b/tools/libacpi/libacpi.h
@@ -80,6 +80,7 @@ struct acpi_config {
uint8_t tpm_version;
const uint16_t *tis_hdr;
+ const uint16_t *crb_hdr;
/*
* Address where acpi_info should be placed.
* This must match the OperationRegion(BIOS, SystemMemory, ....)
diff --git a/tools/libacpi/ssdt_tpm2.asl b/tools/libacpi/ssdt_tpm2.asl
new file mode 100644
index 0000000000..1801c338df
--- /dev/null
+++ b/tools/libacpi/ssdt_tpm2.asl
@@ -0,0 +1,36 @@
+/*
+ * ssdt_tpm2.asl
+ *
+ * Copyright (c) 2018-2022, Citrix Systems, Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU Lesser General Public License as published
+ * by the Free Software Foundation; version 2.1 only. with the special
+ * exception on linking described in file LICENSE.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU Lesser General Public License for more details.
+ */
+
+/* SSDT for TPM CRB Interface for Xen with Qemu device model. */
+
+DefinitionBlock ("SSDT_TPM2.aml", "SSDT", 2, "Xen", "HVM", 0)
+{
+ Device (TPM)
+ {
+ Name (_HID, "MSFT0101" /* TPM 2.0 Security Device */) // _HID: Hardware ID
+ Name (_CRS, ResourceTemplate () // _CRS: Current Resource Settings
+ {
+ Memory32Fixed (ReadWrite,
+ 0xFED40000, // Address Base
+ 0x00001000, // Address Length
+ )
+ })
+ Method (_STA, 0, NotSerialized) // _STA: Status
+ {
+ Return (0x0F)
+ }
+ }
+}
--
2.31.1
On 15.12.2022 18:09, Jennifer Herbert wrote: > --- a/tools/firmware/hvmloader/util.c > +++ b/tools/firmware/hvmloader/util.c > @@ -1009,6 +1009,13 @@ void hvmloader_acpi_build_tables(struct acpi_config *config, > config->table_flags |= ACPI_HAS_TPM; > config->tis_hdr = (uint16_t *)ACPI_TIS_HDR_ADDRESS; > break; > + case 2: Nit: Blank line please between non-fall-through case blocks. > + config->table_flags |= ACPI_HAS_TPM; > + config->crb_hdr = (uint16_t *)ACPI_CRB_HDR_ADDRESS; > + > + mem_hole_populate_ram(TPM_LOG_AREA_ADDRESS >> PAGE_SHIFT, TPM_LOG_SIZE >> PAGE_SHIFT); Nit: Overlong line. > + memset((void *)(TPM_LOG_AREA_ADDRESS), 0, TPM_LOG_SIZE); Nit: Excess pair of parentheses. > --- a/tools/libacpi/Makefile > +++ b/tools/libacpi/Makefile > @@ -25,7 +25,7 @@ C_SRC-$(CONFIG_X86) = dsdt_anycpu.c dsdt_15cpu.c dsdt_anycpu_qemu_xen.c dsdt_pvh > C_SRC-$(CONFIG_ARM_64) = dsdt_anycpu_arm.c > DSDT_FILES ?= $(C_SRC-y) > C_SRC = $(addprefix $(ACPI_BUILD_DIR)/, $(DSDT_FILES)) > -H_SRC = $(addprefix $(ACPI_BUILD_DIR)/, ssdt_s3.h ssdt_s4.h ssdt_pm.h ssdt_tpm.h ssdt_laptop_slate.h) > +H_SRC = $(addprefix $(ACPI_BUILD_DIR)/, ssdt_s3.h ssdt_s4.h ssdt_pm.h ssdt_tpm.h ssdt_tpm2.h ssdt_laptop_slate.h) This line could (the latest) now also do with splitting up. > --- a/tools/libacpi/acpi2_0.h > +++ b/tools/libacpi/acpi2_0.h > @@ -121,6 +121,30 @@ struct acpi_20_tcpa { > }; > #define ACPI_2_0_TCPA_LAML_SIZE (64*1024) > > +/* > + * TPM2 > + */ > +struct acpi_20_tpm2 { > + struct acpi_header header; > + uint16_t platform_class; > + uint16_t reserved; > + uint64_t control_area_address; > + uint32_t start_method; > + uint8_t start_method_params[12]; > + uint32_t log_area_minimum_length; > + uint64_t log_area_start_address; > +}; > +#define TPM2_ACPI_CLASS_CLIENT 0 > +#define TPM2_START_METHOD_CRB 7 > + > +#define TPM_CRB_ADDR_BASE 0xFED40000 > +#define TPM_CRB_ADDR_CTRL (TPM_CRB_ADDR_BASE + 0x40) What is the relation between these two and ACPI_CRB_HDR_ADDRESS (0xFED40034)? Independent of the answer it would be nice to have a BUILD_BUG_ON()-like check somewhere tying the two together (and I have a vague recollection that I might have asked for such in a comment on v1 already). And since afaics the space at that address also isn't filled anywhere in hvmloader, the description could also do with saying what entity is doing that (qemu?) and hence with whom this needs to remain in sync. > @@ -449,6 +451,39 @@ static int construct_secondary_tables(struct acpi_ctxt *ctxt, > tcpa->header.length); > } > break; > + > + case 2: > + if (!config->crb_hdr || See the respective comment on the earlier patch. Jan
Hi Andrew, (+ Anthony as I believe he is the toolstack maintainer) > -----Original Message----- > From: Andrew Cooper <Andrew.Cooper3@citrix.com> > Subject: Re: [for-4.17 PATCH] acpi: Add TPM2 interface definition and make > the TPM version configurable. > > On 30/08/2022 21:27, Jennifer Herbert wrote: > > This patch introduces an optional TPM 2 interface definition to the ACPI > table, > > which is to be used as part of a vTPM 2 implementation. > > To enable the new interface - I have made the TPM interface version > > configurable in the acpi_config, with the default being the existing > 1.2.(TCPA) > > I have also added to hvmloader an option to utilise this new config, which > can > > be triggered by setting the platform/tpm_verion xenstore key. > > > > Signed-off-by: Jennifer Herbert <jennifer.herbert@citrix.com> > > We're past the 4.17 feature submission deadline. CC'ing Henry. > > Henry: This is a fairly simple change and a critical building block for > getting Windows 11 support on Xen. Given that feature freeze was > slipped several weeks for other reasons, this should be considered for > inclusion too. We delayed the feature freeze to this Friday. So it actually depends on if we can have enough bandwidth for maintainers to provide feedback and if Jennifer can fix them in time. Kind regards, Henry
This patch makes the TPM version, for which the ACPI libary probes, configurable.
If acpi_config.tpm_verison is set to 1, it indicates that 1.2 (TCPA) should be probed.
I have also added to hvmloader an option to allow setting this new config, which can
be triggered by setting the platform/tpm_verion xenstore key.
Signed-off-by: Jennifer Herbert <jennifer.herbert@citrix.com>
---
docs/misc/xenstore-paths.pandoc | 8 ++++
tools/firmware/hvmloader/util.c | 13 ++++++-
tools/libacpi/build.c | 68 ++++++++++++++++++---------------
tools/libacpi/libacpi.h | 4 +-
4 files changed, 59 insertions(+), 34 deletions(-)
diff --git a/docs/misc/xenstore-paths.pandoc b/docs/misc/xenstore-paths.pandoc
index 5cd5c8a3b9..7270b46721 100644
--- a/docs/misc/xenstore-paths.pandoc
+++ b/docs/misc/xenstore-paths.pandoc
@@ -269,6 +269,14 @@ at the guest physical address in HVM_PARAM_VM_GENERATION_ID_ADDR.
See Microsoft's "Virtual Machine Generation ID" specification for the
circumstances where the generation ID needs to be changed.
+
+#### ~/platform/tpm_version = INTEGER [HVM,INTERNAL]
+
+The TPM version to be probed for.
+
+A value of 1 indicates to probe for TPM 1.2. If unset, or an
+invalid version, then no TPM is probed.
+
### Frontend device paths
Paravirtual device frontends are generally specified by their own
diff --git a/tools/firmware/hvmloader/util.c b/tools/firmware/hvmloader/util.c
index 581b35e5cf..87bc2d677f 100644
--- a/tools/firmware/hvmloader/util.c
+++ b/tools/firmware/hvmloader/util.c
@@ -994,13 +994,22 @@ void hvmloader_acpi_build_tables(struct acpi_config *config,
if ( !strncmp(xenstore_read("platform/acpi_laptop_slate", "0"), "1", 1) )
config->table_flags |= ACPI_HAS_SSDT_LAPTOP_SLATE;
- config->table_flags |= (ACPI_HAS_TCPA | ACPI_HAS_IOAPIC |
+ config->table_flags |= (ACPI_HAS_TPM | ACPI_HAS_IOAPIC |
ACPI_HAS_WAET | ACPI_HAS_PMTIMER |
ACPI_HAS_BUTTONS | ACPI_HAS_VGA |
ACPI_HAS_8042 | ACPI_HAS_CMOS_RTC);
config->acpi_revision = 4;
- config->tis_hdr = (uint16_t *)ACPI_TIS_HDR_ADDRESS;
+ s = xenstore_read("platform/tpm_version", "0");
+ config->tpm_version = strtoll(s, NULL, 0);
+
+ switch( config->tpm_version )
+ {
+ case 1:
+ config->table_flags |= ACPI_HAS_TPM;
+ config->tis_hdr = (uint16_t *)ACPI_TIS_HDR_ADDRESS;
+ break;
+ }
config->numa.nr_vmemranges = nr_vmemranges;
config->numa.nr_vnodes = nr_vnodes;
diff --git a/tools/libacpi/build.c b/tools/libacpi/build.c
index fe2db66a62..d313ccd8cf 100644
--- a/tools/libacpi/build.c
+++ b/tools/libacpi/build.c
@@ -409,38 +409,46 @@ static int construct_secondary_tables(struct acpi_ctxt *ctxt,
memcpy(ssdt, ssdt_laptop_slate, sizeof(ssdt_laptop_slate));
table_ptrs[nr_tables++] = ctxt->mem_ops.v2p(ctxt, ssdt);
}
-
- /* TPM TCPA and SSDT. */
- if ( (config->table_flags & ACPI_HAS_TCPA) &&
- (config->tis_hdr[0] != 0 && config->tis_hdr[0] != 0xffff) &&
- (config->tis_hdr[1] != 0 && config->tis_hdr[1] != 0xffff) )
+ /* TPM and SSDT. */
+ if (config->table_flags & ACPI_HAS_TPM)
{
- ssdt = ctxt->mem_ops.alloc(ctxt, sizeof(ssdt_tpm), 16);
- if (!ssdt) return -1;
- memcpy(ssdt, ssdt_tpm, sizeof(ssdt_tpm));
- table_ptrs[nr_tables++] = ctxt->mem_ops.v2p(ctxt, ssdt);
-
- tcpa = ctxt->mem_ops.alloc(ctxt, sizeof(struct acpi_20_tcpa), 16);
- if (!tcpa) return -1;
- memset(tcpa, 0, sizeof(*tcpa));
- table_ptrs[nr_tables++] = ctxt->mem_ops.v2p(ctxt, tcpa);
-
- tcpa->header.signature = ACPI_2_0_TCPA_SIGNATURE;
- tcpa->header.length = sizeof(*tcpa);
- tcpa->header.revision = ACPI_2_0_TCPA_REVISION;
- fixed_strcpy(tcpa->header.oem_id, ACPI_OEM_ID);
- fixed_strcpy(tcpa->header.oem_table_id, ACPI_OEM_TABLE_ID);
- tcpa->header.oem_revision = ACPI_OEM_REVISION;
- tcpa->header.creator_id = ACPI_CREATOR_ID;
- tcpa->header.creator_revision = ACPI_CREATOR_REVISION;
- if ( (lasa = ctxt->mem_ops.alloc(ctxt, ACPI_2_0_TCPA_LAML_SIZE, 16)) != NULL )
+ switch (config->tpm_version)
{
- tcpa->lasa = ctxt->mem_ops.v2p(ctxt, lasa);
- tcpa->laml = ACPI_2_0_TCPA_LAML_SIZE;
- memset(lasa, 0, tcpa->laml);
- set_checksum(tcpa,
- offsetof(struct acpi_header, checksum),
- tcpa->header.length);
+ case 1:
+ if (!config->tis_hdr ||
+ config->tis_hdr[0] == 0 || config->tis_hdr[0] == 0xffff ||
+ config->tis_hdr[1] == 0 || config->tis_hdr[1] == 0xffff)
+ break;
+
+ ssdt = ctxt->mem_ops.alloc(ctxt, sizeof(ssdt_tpm), 16);
+ if (!ssdt) return -1;
+ memcpy(ssdt, ssdt_tpm, sizeof(ssdt_tpm));
+ table_ptrs[nr_tables++] = ctxt->mem_ops.v2p(ctxt, ssdt);
+
+ tcpa = ctxt->mem_ops.alloc(ctxt, sizeof(struct acpi_20_tcpa), 16);
+ if (!tcpa) return -1;
+ memset(tcpa, 0, sizeof(*tcpa));
+ table_ptrs[nr_tables++] = ctxt->mem_ops.v2p(ctxt, tcpa);
+
+ tcpa->header.signature = ACPI_2_0_TCPA_SIGNATURE;
+ tcpa->header.length = sizeof(*tcpa);
+ tcpa->header.revision = ACPI_2_0_TCPA_REVISION;
+ fixed_strcpy(tcpa->header.oem_id, ACPI_OEM_ID);
+ fixed_strcpy(tcpa->header.oem_table_id, ACPI_OEM_TABLE_ID);
+ tcpa->header.oem_revision = ACPI_OEM_REVISION;
+ tcpa->header.creator_id = ACPI_CREATOR_ID;
+ tcpa->header.creator_revision = ACPI_CREATOR_REVISION;
+
+ if ( (lasa = ctxt->mem_ops.alloc(ctxt, ACPI_2_0_TCPA_LAML_SIZE, 16)) != NULL )
+ {
+ tcpa->lasa = ctxt->mem_ops.v2p(ctxt, lasa);
+ tcpa->laml = ACPI_2_0_TCPA_LAML_SIZE;
+ memset(lasa, 0, tcpa->laml);
+ set_checksum(tcpa,
+ offsetof(struct acpi_header, checksum),
+ tcpa->header.length);
+ }
+ break;
}
}
diff --git a/tools/libacpi/libacpi.h b/tools/libacpi/libacpi.h
index a2efd23b0b..9143616130 100644
--- a/tools/libacpi/libacpi.h
+++ b/tools/libacpi/libacpi.h
@@ -27,7 +27,7 @@
#define ACPI_HAS_SSDT_PM (1<<4)
#define ACPI_HAS_SSDT_S3 (1<<5)
#define ACPI_HAS_SSDT_S4 (1<<6)
-#define ACPI_HAS_TCPA (1<<7)
+#define ACPI_HAS_TPM (1<<7)
#define ACPI_HAS_IOAPIC (1<<8)
#define ACPI_HAS_WAET (1<<9)
#define ACPI_HAS_PMTIMER (1<<10)
@@ -78,8 +78,8 @@ struct acpi_config {
struct acpi_numa numa;
const struct hvm_info_table *hvminfo;
+ uint8_t tpm_version;
const uint16_t *tis_hdr;
-
/*
* Address where acpi_info should be placed.
* This must match the OperationRegion(BIOS, SystemMemory, ....)
--
2.31.1
On Thu, Sep 15, 2022 at 4:41 PM Jennifer Herbert <jennifer.herbert@citrix.com> wrote: > > This patch makes the TPM version, for which the ACPI libary probes, configurable. > If acpi_config.tpm_verison is set to 1, it indicates that 1.2 (TCPA) should be probed. > I have also added to hvmloader an option to allow setting this new config, which can > be triggered by setting the platform/tpm_verion xenstore key. > > Signed-off-by: Jennifer Herbert <jennifer.herbert@citrix.com> > --- > docs/misc/xenstore-paths.pandoc | 8 ++++ > tools/firmware/hvmloader/util.c | 13 ++++++- > tools/libacpi/build.c | 68 ++++++++++++++++++--------------- > tools/libacpi/libacpi.h | 4 +- > 4 files changed, 59 insertions(+), 34 deletions(-) > > diff --git a/docs/misc/xenstore-paths.pandoc b/docs/misc/xenstore-paths.pandoc > index 5cd5c8a3b9..7270b46721 100644 > --- a/docs/misc/xenstore-paths.pandoc > +++ b/docs/misc/xenstore-paths.pandoc > @@ -269,6 +269,14 @@ at the guest physical address in HVM_PARAM_VM_GENERATION_ID_ADDR. > See Microsoft's "Virtual Machine Generation ID" specification for the > circumstances where the generation ID needs to be changed. > > + > +#### ~/platform/tpm_version = INTEGER [HVM,INTERNAL] > + > +The TPM version to be probed for. > + > +A value of 1 indicates to probe for TPM 1.2. If unset, or an > +invalid version, then no TPM is probed. > + Hi, In patch 2, I think you want to expand this section for TPM 2.0 support. Regards, Jason
On 15.09.2022 22:40, Jennifer Herbert wrote: First of all - please follow patch submission guidelines and send patches To: the list only, with maintainers Cc:-ed. > --- a/tools/firmware/hvmloader/util.c > +++ b/tools/firmware/hvmloader/util.c > @@ -994,13 +994,22 @@ void hvmloader_acpi_build_tables(struct acpi_config *config, > if ( !strncmp(xenstore_read("platform/acpi_laptop_slate", "0"), "1", 1) ) > config->table_flags |= ACPI_HAS_SSDT_LAPTOP_SLATE; > > - config->table_flags |= (ACPI_HAS_TCPA | ACPI_HAS_IOAPIC | > + config->table_flags |= (ACPI_HAS_TPM | ACPI_HAS_IOAPIC | > ACPI_HAS_WAET | ACPI_HAS_PMTIMER | > ACPI_HAS_BUTTONS | ACPI_HAS_VGA | > ACPI_HAS_8042 | ACPI_HAS_CMOS_RTC); Did you not mean to drop ACPI_HAS_TPM here when ... > config->acpi_revision = 4; > > - config->tis_hdr = (uint16_t *)ACPI_TIS_HDR_ADDRESS; > + s = xenstore_read("platform/tpm_version", "0"); > + config->tpm_version = strtoll(s, NULL, 0); > + > + switch( config->tpm_version ) > + { > + case 1: > + config->table_flags |= ACPI_HAS_TPM; ... you now OR it in here? Or else what use is this statement? As to the use of strtoll() - I realize we have nothing else in hvmloader, but I'm still weary of the overflow potential. Just a remark, not really something to act upon. > @@ -78,8 +78,8 @@ struct acpi_config { > struct acpi_numa numa; > const struct hvm_info_table *hvminfo; > > + uint8_t tpm_version; > const uint16_t *tis_hdr; > - > /* > * Address where acpi_info should be placed. > * This must match the OperationRegion(BIOS, SystemMemory, ....) Please don't remove the blank line here. Jan
This patch introduces an optional TPM 2 interface definition to the ACPI table,
which is to be used as part of a vTPM 2 implementation.
Signed-off-by: Jennifer Herbert <jennifer.herbert@citrix.com>
---
tools/firmware/hvmloader/config.h | 1 +
tools/firmware/hvmloader/util.c | 7 ++++++
tools/libacpi/Makefile | 2 +-
tools/libacpi/acpi2_0.h | 26 ++++++++++++++++++++++
tools/libacpi/build.c | 35 ++++++++++++++++++++++++++++++
tools/libacpi/libacpi.h | 1 +
tools/libacpi/ssdt_tpm2.asl | 36 +++++++++++++++++++++++++++++++
7 files changed, 107 insertions(+), 1 deletion(-)
create mode 100644 tools/libacpi/ssdt_tpm2.asl
diff --git a/tools/firmware/hvmloader/config.h b/tools/firmware/hvmloader/config.h
index c82adf6dc5..4dec7195f0 100644
--- a/tools/firmware/hvmloader/config.h
+++ b/tools/firmware/hvmloader/config.h
@@ -56,6 +56,7 @@ extern uint8_t ioapic_version;
#define PCI_ISA_IRQ_MASK 0x0c20U /* ISA IRQs 5,10,11 are PCI connected */
#define ACPI_TIS_HDR_ADDRESS 0xFED40F00UL
+#define ACPI_CRB_HDR_ADDRESS 0xFED40034UL
extern uint32_t pci_mem_start;
extern const uint32_t pci_mem_end;
diff --git a/tools/firmware/hvmloader/util.c b/tools/firmware/hvmloader/util.c
index 87bc2d677f..6e5d3609b9 100644
--- a/tools/firmware/hvmloader/util.c
+++ b/tools/firmware/hvmloader/util.c
@@ -1009,6 +1009,13 @@ void hvmloader_acpi_build_tables(struct acpi_config *config,
config->table_flags |= ACPI_HAS_TPM;
config->tis_hdr = (uint16_t *)ACPI_TIS_HDR_ADDRESS;
break;
+ case 2:
+ config->table_flags |= ACPI_HAS_TPM;
+ config->crb_hdr = (uint16_t *)ACPI_CRB_HDR_ADDRESS;
+
+ mem_hole_populate_ram(TPM_LOG_AREA_ADDRESS >> PAGE_SHIFT, TPM_LOG_SIZE >> PAGE_SHIFT);
+ memset((void *)(TPM_LOG_AREA_ADDRESS), 0, TPM_LOG_SIZE);
+ break;
}
config->numa.nr_vmemranges = nr_vmemranges;
diff --git a/tools/libacpi/Makefile b/tools/libacpi/Makefile
index 60860eaa00..125f29fb54 100644
--- a/tools/libacpi/Makefile
+++ b/tools/libacpi/Makefile
@@ -25,7 +25,7 @@ C_SRC-$(CONFIG_X86) = dsdt_anycpu.c dsdt_15cpu.c dsdt_anycpu_qemu_xen.c dsdt_pvh
C_SRC-$(CONFIG_ARM_64) = dsdt_anycpu_arm.c
DSDT_FILES ?= $(C_SRC-y)
C_SRC = $(addprefix $(ACPI_BUILD_DIR)/, $(DSDT_FILES))
-H_SRC = $(addprefix $(ACPI_BUILD_DIR)/, ssdt_s3.h ssdt_s4.h ssdt_pm.h ssdt_tpm.h ssdt_laptop_slate.h)
+H_SRC = $(addprefix $(ACPI_BUILD_DIR)/, ssdt_s3.h ssdt_s4.h ssdt_pm.h ssdt_tpm.h ssdt_tpm2.h ssdt_laptop_slate.h)
MKDSDT_CFLAGS-$(CONFIG_ARM_64) = -DCONFIG_ARM_64
MKDSDT_CFLAGS-$(CONFIG_X86) = -DCONFIG_X86
diff --git a/tools/libacpi/acpi2_0.h b/tools/libacpi/acpi2_0.h
index 2619ba32db..f4eb4d715b 100644
--- a/tools/libacpi/acpi2_0.h
+++ b/tools/libacpi/acpi2_0.h
@@ -121,6 +121,30 @@ struct acpi_20_tcpa {
};
#define ACPI_2_0_TCPA_LAML_SIZE (64*1024)
+/*
+ * TPM2
+ */
+struct acpi_20_tpm2 {
+ struct acpi_header header;
+ uint16_t platform_class;
+ uint16_t reserved;
+ uint64_t control_area_address;
+ uint32_t start_method;
+ uint8_t start_method_params[12];
+ uint32_t log_area_minimum_length;
+ uint64_t log_area_start_address;
+};
+#define TPM2_ACPI_CLASS_CLIENT 0
+#define TPM2_START_METHOD_CRB 7
+
+#define TPM_CRB_ADDR_BASE 0xFED40000
+#define TPM_CRB_ADDR_CTRL (TPM_CRB_ADDR_BASE + 0x40)
+
+#define TPM_LOG_AREA_ADDRESS 0xFED50000
+
+#define TPM_LOG_AREA_MINIMUM_SIZE (64 << 10)
+#define TPM_LOG_SIZE (64 << 10)
+
/*
* Fixed ACPI Description Table Structure (FADT) in ACPI 1.0.
*/
@@ -431,6 +455,7 @@ struct acpi_20_slit {
#define ACPI_2_0_RSDT_SIGNATURE ASCII32('R','S','D','T')
#define ACPI_2_0_XSDT_SIGNATURE ASCII32('X','S','D','T')
#define ACPI_2_0_TCPA_SIGNATURE ASCII32('T','C','P','A')
+#define ACPI_2_0_TPM2_SIGNATURE ASCII32('T','P','M','2')
#define ACPI_2_0_HPET_SIGNATURE ASCII32('H','P','E','T')
#define ACPI_2_0_WAET_SIGNATURE ASCII32('W','A','E','T')
#define ACPI_2_0_SRAT_SIGNATURE ASCII32('S','R','A','T')
@@ -444,6 +469,7 @@ struct acpi_20_slit {
#define ACPI_2_0_RSDT_REVISION 0x01
#define ACPI_2_0_XSDT_REVISION 0x01
#define ACPI_2_0_TCPA_REVISION 0x02
+#define ACPI_2_0_TPM2_REVISION 0x04
#define ACPI_2_0_HPET_REVISION 0x01
#define ACPI_2_0_WAET_REVISION 0x01
#define ACPI_1_0_FADT_REVISION 0x01
diff --git a/tools/libacpi/build.c b/tools/libacpi/build.c
index d313ccd8cf..d4f25a68d2 100644
--- a/tools/libacpi/build.c
+++ b/tools/libacpi/build.c
@@ -19,6 +19,7 @@
#include "ssdt_s3.h"
#include "ssdt_s4.h"
#include "ssdt_tpm.h"
+#include "ssdt_tpm2.h"
#include "ssdt_pm.h"
#include "ssdt_laptop_slate.h"
#include <xen/hvm/hvm_info_table.h>
@@ -352,6 +353,7 @@ static int construct_secondary_tables(struct acpi_ctxt *ctxt,
struct acpi_20_tcpa *tcpa;
unsigned char *ssdt;
void *lasa;
+ struct acpi_20_tpm2 *tpm2;
/* MADT. */
if ( (config->hvminfo->nr_vcpus > 1) || config->hvminfo->apic_mode )
@@ -449,6 +451,39 @@ static int construct_secondary_tables(struct acpi_ctxt *ctxt,
tcpa->header.length);
}
break;
+
+ case 2:
+ if (!config->crb_hdr ||
+ config->crb_hdr[0] == 0 || config->crb_hdr[0] == 0xffff)
+ break;
+
+ ssdt = ctxt->mem_ops.alloc(ctxt, sizeof(ssdt_tpm2), 16);
+ if (!ssdt) return -1;
+ memcpy(ssdt, ssdt_tpm2, sizeof(ssdt_tpm2));
+ table_ptrs[nr_tables++] = ctxt->mem_ops.v2p(ctxt, ssdt);
+
+ tpm2 = ctxt->mem_ops.alloc(ctxt, sizeof(struct acpi_20_tpm2), 16);
+ if (!tpm2) return -1;
+ memset(tpm2, 0, sizeof(*tpm2));
+ table_ptrs[nr_tables++] = ctxt->mem_ops.v2p(ctxt, tpm2);
+
+ tpm2->header.signature = ACPI_2_0_TPM2_SIGNATURE;
+ tpm2->header.length = sizeof(*tpm2);
+ tpm2->header.revision = ACPI_2_0_TPM2_REVISION;
+ fixed_strcpy(tpm2->header.oem_id, ACPI_OEM_ID);
+ fixed_strcpy(tpm2->header.oem_table_id, ACPI_OEM_TABLE_ID);
+ tpm2->header.oem_revision = ACPI_OEM_REVISION;
+ tpm2->header.creator_id = ACPI_CREATOR_ID;
+ tpm2->header.creator_revision = ACPI_CREATOR_REVISION;
+ tpm2->platform_class = TPM2_ACPI_CLASS_CLIENT;
+ tpm2->control_area_address = TPM_CRB_ADDR_CTRL;
+ tpm2->start_method = TPM2_START_METHOD_CRB;
+ tpm2->log_area_minimum_length = TPM_LOG_AREA_MINIMUM_SIZE;
+ tpm2->log_area_start_address = TPM_LOG_AREA_ADDRESS;
+
+ set_checksum(tpm2,
+ offsetof(struct acpi_header, checksum),
+ tpm2->header.length);
}
}
diff --git a/tools/libacpi/libacpi.h b/tools/libacpi/libacpi.h
index 9143616130..b5d08ff09b 100644
--- a/tools/libacpi/libacpi.h
+++ b/tools/libacpi/libacpi.h
@@ -80,6 +80,7 @@ struct acpi_config {
uint8_t tpm_version;
const uint16_t *tis_hdr;
+ const uint16_t *crb_hdr;
/*
* Address where acpi_info should be placed.
* This must match the OperationRegion(BIOS, SystemMemory, ....)
diff --git a/tools/libacpi/ssdt_tpm2.asl b/tools/libacpi/ssdt_tpm2.asl
new file mode 100644
index 0000000000..1801c338df
--- /dev/null
+++ b/tools/libacpi/ssdt_tpm2.asl
@@ -0,0 +1,36 @@
+/*
+ * ssdt_tpm2.asl
+ *
+ * Copyright (c) 2018-2022, Citrix Systems, Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU Lesser General Public License as published
+ * by the Free Software Foundation; version 2.1 only. with the special
+ * exception on linking described in file LICENSE.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU Lesser General Public License for more details.
+ */
+
+/* SSDT for TPM CRB Interface for Xen with Qemu device model. */
+
+DefinitionBlock ("SSDT_TPM2.aml", "SSDT", 2, "Xen", "HVM", 0)
+{
+ Device (TPM)
+ {
+ Name (_HID, "MSFT0101" /* TPM 2.0 Security Device */) // _HID: Hardware ID
+ Name (_CRS, ResourceTemplate () // _CRS: Current Resource Settings
+ {
+ Memory32Fixed (ReadWrite,
+ 0xFED40000, // Address Base
+ 0x00001000, // Address Length
+ )
+ })
+ Method (_STA, 0, NotSerialized) // _STA: Status
+ {
+ Return (0x0F)
+ }
+ }
+}
--
2.31.1
On 15.09.2022 22:40, Jennifer Herbert wrote: > --- a/tools/firmware/hvmloader/config.h > +++ b/tools/firmware/hvmloader/config.h > @@ -56,6 +56,7 @@ extern uint8_t ioapic_version; > #define PCI_ISA_IRQ_MASK 0x0c20U /* ISA IRQs 5,10,11 are PCI connected */ > > #define ACPI_TIS_HDR_ADDRESS 0xFED40F00UL > +#define ACPI_CRB_HDR_ADDRESS 0xFED40034UL I understand it may not be feasible to express this here as a proper derivation from other constants, but then you want to have a BUILD_BUG_ON() somewhere making (and guaranteeing) the connection. Thi may of course involve moving the #define to a header which both hvmloader and libacpi can (legitimately) include. > --- a/tools/firmware/hvmloader/util.c > +++ b/tools/firmware/hvmloader/util.c > @@ -1009,6 +1009,13 @@ void hvmloader_acpi_build_tables(struct acpi_config *config, > config->table_flags |= ACPI_HAS_TPM; > config->tis_hdr = (uint16_t *)ACPI_TIS_HDR_ADDRESS; > break; > + case 2: > + config->table_flags |= ACPI_HAS_TPM; > + config->crb_hdr = (uint16_t *)ACPI_CRB_HDR_ADDRESS; > + > + mem_hole_populate_ram(TPM_LOG_AREA_ADDRESS >> PAGE_SHIFT, TPM_LOG_SIZE >> PAGE_SHIFT); Nit: Long line. > + memset((void *)(TPM_LOG_AREA_ADDRESS), 0, TPM_LOG_SIZE); No need to parenthesize the operand of the cast? Jan
Hi,
Are any further changes needed to upstream this patch series?
Cheers,
-jenny
-----Original Message-----
From: Jennifer Herbert <jennifer.herbert@citrix.com>
Sent: 15 September 2022 21:40
To: jbeulich@suse.com; Andrew Cooper <Andrew.Cooper3@citrix.com>; wl@xen.org; Roger Pau Monne <roger.pau@citrix.com>
Cc: xen-devel@lists.xenproject.org; Jennifer Herbert <jennifer.herbert@citrix.com>
Subject: [PATCH 2/2] acpi: Add TPM2 interface definition.
This patch introduces an optional TPM 2 interface definition to the ACPI table, which is to be used as part of a vTPM 2 implementation.
Signed-off-by: Jennifer Herbert <jennifer.herbert@citrix.com>
---
tools/firmware/hvmloader/config.h | 1 +
tools/firmware/hvmloader/util.c | 7 ++++++
tools/libacpi/Makefile | 2 +-
tools/libacpi/acpi2_0.h | 26 ++++++++++++++++++++++
tools/libacpi/build.c | 35 ++++++++++++++++++++++++++++++
tools/libacpi/libacpi.h | 1 +
tools/libacpi/ssdt_tpm2.asl | 36 +++++++++++++++++++++++++++++++
7 files changed, 107 insertions(+), 1 deletion(-) create mode 100644 tools/libacpi/ssdt_tpm2.asl
diff --git a/tools/firmware/hvmloader/config.h b/tools/firmware/hvmloader/config.h
index c82adf6dc5..4dec7195f0 100644
--- a/tools/firmware/hvmloader/config.h
+++ b/tools/firmware/hvmloader/config.h
@@ -56,6 +56,7 @@ extern uint8_t ioapic_version;
#define PCI_ISA_IRQ_MASK 0x0c20U /* ISA IRQs 5,10,11 are PCI connected */
#define ACPI_TIS_HDR_ADDRESS 0xFED40F00UL
+#define ACPI_CRB_HDR_ADDRESS 0xFED40034UL
extern uint32_t pci_mem_start;
extern const uint32_t pci_mem_end;
diff --git a/tools/firmware/hvmloader/util.c b/tools/firmware/hvmloader/util.c index 87bc2d677f..6e5d3609b9 100644
--- a/tools/firmware/hvmloader/util.c
+++ b/tools/firmware/hvmloader/util.c
@@ -1009,6 +1009,13 @@ void hvmloader_acpi_build_tables(struct acpi_config *config,
config->table_flags |= ACPI_HAS_TPM;
config->tis_hdr = (uint16_t *)ACPI_TIS_HDR_ADDRESS;
break;
+ case 2:
+ config->table_flags |= ACPI_HAS_TPM;
+ config->crb_hdr = (uint16_t *)ACPI_CRB_HDR_ADDRESS;
+
+ mem_hole_populate_ram(TPM_LOG_AREA_ADDRESS >> PAGE_SHIFT, TPM_LOG_SIZE >> PAGE_SHIFT);
+ memset((void *)(TPM_LOG_AREA_ADDRESS), 0, TPM_LOG_SIZE);
+ break;
}
config->numa.nr_vmemranges = nr_vmemranges; diff --git a/tools/libacpi/Makefile b/tools/libacpi/Makefile index 60860eaa00..125f29fb54 100644
--- a/tools/libacpi/Makefile
+++ b/tools/libacpi/Makefile
@@ -25,7 +25,7 @@ C_SRC-$(CONFIG_X86) = dsdt_anycpu.c dsdt_15cpu.c dsdt_anycpu_qemu_xen.c dsdt_pvh
C_SRC-$(CONFIG_ARM_64) = dsdt_anycpu_arm.c DSDT_FILES ?= $(C_SRC-y) C_SRC = $(addprefix $(ACPI_BUILD_DIR)/, $(DSDT_FILES)) -H_SRC = $(addprefix $(ACPI_BUILD_DIR)/, ssdt_s3.h ssdt_s4.h ssdt_pm.h ssdt_tpm.h ssdt_laptop_slate.h)
+H_SRC = $(addprefix $(ACPI_BUILD_DIR)/, ssdt_s3.h ssdt_s4.h ssdt_pm.h
+ssdt_tpm.h ssdt_tpm2.h ssdt_laptop_slate.h)
MKDSDT_CFLAGS-$(CONFIG_ARM_64) = -DCONFIG_ARM_64
MKDSDT_CFLAGS-$(CONFIG_X86) = -DCONFIG_X86 diff --git a/tools/libacpi/acpi2_0.h b/tools/libacpi/acpi2_0.h index 2619ba32db..f4eb4d715b 100644
--- a/tools/libacpi/acpi2_0.h
+++ b/tools/libacpi/acpi2_0.h
@@ -121,6 +121,30 @@ struct acpi_20_tcpa { }; #define ACPI_2_0_TCPA_LAML_SIZE (64*1024)
+/*
+ * TPM2
+ */
+struct acpi_20_tpm2 {
+ struct acpi_header header;
+ uint16_t platform_class;
+ uint16_t reserved;
+ uint64_t control_area_address;
+ uint32_t start_method;
+ uint8_t start_method_params[12];
+ uint32_t log_area_minimum_length;
+ uint64_t log_area_start_address;
+};
+#define TPM2_ACPI_CLASS_CLIENT 0
+#define TPM2_START_METHOD_CRB 7
+
+#define TPM_CRB_ADDR_BASE 0xFED40000
+#define TPM_CRB_ADDR_CTRL (TPM_CRB_ADDR_BASE + 0x40)
+
+#define TPM_LOG_AREA_ADDRESS 0xFED50000
+
+#define TPM_LOG_AREA_MINIMUM_SIZE (64 << 10)
+#define TPM_LOG_SIZE (64 << 10)
+
/*
* Fixed ACPI Description Table Structure (FADT) in ACPI 1.0.
*/
@@ -431,6 +455,7 @@ struct acpi_20_slit { #define ACPI_2_0_RSDT_SIGNATURE ASCII32('R','S','D','T') #define ACPI_2_0_XSDT_SIGNATURE ASCII32('X','S','D','T') #define ACPI_2_0_TCPA_SIGNATURE ASCII32('T','C','P','A')
+#define ACPI_2_0_TPM2_SIGNATURE ASCII32('T','P','M','2')
#define ACPI_2_0_HPET_SIGNATURE ASCII32('H','P','E','T') #define ACPI_2_0_WAET_SIGNATURE ASCII32('W','A','E','T') #define ACPI_2_0_SRAT_SIGNATURE ASCII32('S','R','A','T') @@ -444,6 +469,7 @@ struct acpi_20_slit { #define ACPI_2_0_RSDT_REVISION 0x01 #define ACPI_2_0_XSDT_REVISION 0x01 #define ACPI_2_0_TCPA_REVISION 0x02
+#define ACPI_2_0_TPM2_REVISION 0x04
#define ACPI_2_0_HPET_REVISION 0x01
#define ACPI_2_0_WAET_REVISION 0x01
#define ACPI_1_0_FADT_REVISION 0x01
diff --git a/tools/libacpi/build.c b/tools/libacpi/build.c index d313ccd8cf..d4f25a68d2 100644
--- a/tools/libacpi/build.c
+++ b/tools/libacpi/build.c
@@ -19,6 +19,7 @@
#include "ssdt_s3.h"
#include "ssdt_s4.h"
#include "ssdt_tpm.h"
+#include "ssdt_tpm2.h"
#include "ssdt_pm.h"
#include "ssdt_laptop_slate.h"
#include <xen/hvm/hvm_info_table.h>
@@ -352,6 +353,7 @@ static int construct_secondary_tables(struct acpi_ctxt *ctxt,
struct acpi_20_tcpa *tcpa;
unsigned char *ssdt;
void *lasa;
+ struct acpi_20_tpm2 *tpm2;
/* MADT. */
if ( (config->hvminfo->nr_vcpus > 1) || config->hvminfo->apic_mode ) @@ -449,6 +451,39 @@ static int construct_secondary_tables(struct acpi_ctxt *ctxt,
tcpa->header.length);
}
break;
+
+ case 2:
+ if (!config->crb_hdr ||
+ config->crb_hdr[0] == 0 || config->crb_hdr[0] == 0xffff)
+ break;
+
+ ssdt = ctxt->mem_ops.alloc(ctxt, sizeof(ssdt_tpm2), 16);
+ if (!ssdt) return -1;
+ memcpy(ssdt, ssdt_tpm2, sizeof(ssdt_tpm2));
+ table_ptrs[nr_tables++] = ctxt->mem_ops.v2p(ctxt, ssdt);
+
+ tpm2 = ctxt->mem_ops.alloc(ctxt, sizeof(struct acpi_20_tpm2), 16);
+ if (!tpm2) return -1;
+ memset(tpm2, 0, sizeof(*tpm2));
+ table_ptrs[nr_tables++] = ctxt->mem_ops.v2p(ctxt, tpm2);
+
+ tpm2->header.signature = ACPI_2_0_TPM2_SIGNATURE;
+ tpm2->header.length = sizeof(*tpm2);
+ tpm2->header.revision = ACPI_2_0_TPM2_REVISION;
+ fixed_strcpy(tpm2->header.oem_id, ACPI_OEM_ID);
+ fixed_strcpy(tpm2->header.oem_table_id, ACPI_OEM_TABLE_ID);
+ tpm2->header.oem_revision = ACPI_OEM_REVISION;
+ tpm2->header.creator_id = ACPI_CREATOR_ID;
+ tpm2->header.creator_revision = ACPI_CREATOR_REVISION;
+ tpm2->platform_class = TPM2_ACPI_CLASS_CLIENT;
+ tpm2->control_area_address = TPM_CRB_ADDR_CTRL;
+ tpm2->start_method = TPM2_START_METHOD_CRB;
+ tpm2->log_area_minimum_length = TPM_LOG_AREA_MINIMUM_SIZE;
+ tpm2->log_area_start_address = TPM_LOG_AREA_ADDRESS;
+
+ set_checksum(tpm2,
+ offsetof(struct acpi_header, checksum),
+ tpm2->header.length);
}
}
diff --git a/tools/libacpi/libacpi.h b/tools/libacpi/libacpi.h index 9143616130..b5d08ff09b 100644
--- a/tools/libacpi/libacpi.h
+++ b/tools/libacpi/libacpi.h
@@ -80,6 +80,7 @@ struct acpi_config {
uint8_t tpm_version;
const uint16_t *tis_hdr;
+ const uint16_t *crb_hdr;
/*
* Address where acpi_info should be placed.
* This must match the OperationRegion(BIOS, SystemMemory, ....) diff --git a/tools/libacpi/ssdt_tpm2.asl b/tools/libacpi/ssdt_tpm2.asl new file mode 100644 index 0000000000..1801c338df
--- /dev/null
+++ b/tools/libacpi/ssdt_tpm2.asl
@@ -0,0 +1,36 @@
+/*
+ * ssdt_tpm2.asl
+ *
+ * Copyright (c) 2018-2022, Citrix Systems, Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU Lesser General Public License as
+published
+ * by the Free Software Foundation; version 2.1 only. with the special
+ * exception on linking described in file LICENSE.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU Lesser General Public License for more details.
+ */
+
+/* SSDT for TPM CRB Interface for Xen with Qemu device model. */
+
+DefinitionBlock ("SSDT_TPM2.aml", "SSDT", 2, "Xen", "HVM", 0) {
+ Device (TPM)
+ {
+ Name (_HID, "MSFT0101" /* TPM 2.0 Security Device */) // _HID: Hardware ID
+ Name (_CRS, ResourceTemplate () // _CRS: Current Resource Settings
+ {
+ Memory32Fixed (ReadWrite,
+ 0xFED40000, // Address Base
+ 0x00001000, // Address Length
+ )
+ })
+ Method (_STA, 0, NotSerialized) // _STA: Status
+ {
+ Return (0x0F)
+ }
+ }
+}
--
2.31.1
On 11.10.2022 17:53, Jennifer Herbert wrote: > Are any further changes needed to upstream this patch series? On Sept 19th Jason and I gave comments on the series, which will want addressing one way or another (presumably in a v2). Jan
On Tue, Aug 30, 2022 at 4:30 PM Jennifer Herbert <jennifer.herbert@citrix.com> wrote: > > This patch introduces an optional TPM 2 interface definition to the ACPI table, > which is to be used as part of a vTPM 2 implementation. > To enable the new interface - I have made the TPM interface version > configurable in the acpi_config, with the default being the existing 1.2.(TCPA) > I have also added to hvmloader an option to utilise this new config, which can > be triggered by setting the platform/tpm_verion xenstore key. > > Signed-off-by: Jennifer Herbert <jennifer.herbert@citrix.com> Reviewed-by: Jason Andryuk <jandryuk@gmail.com> Thanks. Is there a particular reason why CRB (Command Response Buffer) was chosen over TIS (TPM Interface Specification)? I think of CRB as more of an embedded device TPM interface, and TIS is what is usually used with physical TPMs. My experiences have only been with TIS devices, so that is influencing my outlook. Hmm, this patch seems to reference the Intel Platform Trust Technology (PTT) fTPM (firmware-TPM) as using the CRB interface: https://patchwork.kernel.org/project/tpmdd-devel/patch/1417672167-3489-8-git-send-email-jarkko.sakkinen@linux.intel.com/ If PTT fTPMs are using CRB, then it's more than just embedded devices.. Regards, Jason
On 9/1/22 08:55, Jason Andryuk wrote: > On Tue, Aug 30, 2022 at 4:30 PM Jennifer Herbert > <jennifer.herbert@citrix.com> wrote: >> >> This patch introduces an optional TPM 2 interface definition to the ACPI table, >> which is to be used as part of a vTPM 2 implementation. >> To enable the new interface - I have made the TPM interface version >> configurable in the acpi_config, with the default being the existing 1.2.(TCPA) >> I have also added to hvmloader an option to utilise this new config, which can >> be triggered by setting the platform/tpm_verion xenstore key. >> >> Signed-off-by: Jennifer Herbert <jennifer.herbert@citrix.com> > > Reviewed-by: Jason Andryuk <jandryuk@gmail.com> > > Thanks. > > Is there a particular reason why CRB (Command Response Buffer) was > chosen over TIS (TPM Interface Specification)? I think of CRB as more > of an embedded device TPM interface, and TIS is what is usually used > with physical TPMs. My experiences have only been with TIS devices, > so that is influencing my outlook. Hmm, this patch seems to reference > the Intel Platform Trust Technology (PTT) fTPM (firmware-TPM) as using > the CRB interface: > https://patchwork.kernel.org/project/tpmdd-devel/patch/1417672167-3489-8-git-send-email-jarkko.sakkinen@linux.intel.com/ > If PTT fTPMs are using CRB, then it's more than just embedded > devices.. This continues to create much confusion. There are two CRB interfaces, one is the PC Client CRB interface defined in the TCG PTP specification, which is based on an MMIO HW interface. There are claims that Intel's PTT provided one, but I myself have never seen an MMIO CRB in the wild. Then there is the Mobile CRB specification, which defines a mailbox/doorbell HW interface, particularly for Arm devices. The Mobile CRB interface has no notion of locality. As a result, there are ongoing discussions on how the specifications may be normalized and enable locality support for a mailbox/doorbell HW interface to support the recent Arm DRTM specification. v/r, dps
© 2016 - 2024 Red Hat, Inc.