* [PATCH] acpi: Add TPM2 interface definition and make the TPM version configurable.
@ 2022-08-30 20:27 Jennifer Herbert
2022-09-01 12:55 ` Jason Andryuk
2022-09-06 14:18 ` [for-4.17 PATCH] " Andrew Cooper
0 siblings, 2 replies; 17+ messages in thread
From: Jennifer Herbert @ 2022-08-30 20:27 UTC (permalink / raw)
To: jbeulich, andrew.cooper3, wl, roger.pau, ian.jackson
Cc: xen-devel, Jennifer Herbert
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
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH] acpi: Add TPM2 interface definition and make the TPM version configurable.
2022-08-30 20:27 [PATCH] acpi: Add TPM2 interface definition and make the TPM version configurable Jennifer Herbert
@ 2022-09-01 12:55 ` Jason Andryuk
2022-09-06 13:03 ` Daniel P. Smith
2022-09-06 14:18 ` [for-4.17 PATCH] " Andrew Cooper
1 sibling, 1 reply; 17+ messages in thread
From: Jason Andryuk @ 2022-09-01 12:55 UTC (permalink / raw)
To: Jennifer Herbert
Cc: Jan Beulich, Andrew Cooper, Wei Liu, Roger Pau Monne,
Ian Jackson, xen-devel
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
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] acpi: Add TPM2 interface definition and make the TPM version configurable.
2022-09-01 12:55 ` Jason Andryuk
@ 2022-09-06 13:03 ` Daniel P. Smith
0 siblings, 0 replies; 17+ messages in thread
From: Daniel P. Smith @ 2022-09-06 13:03 UTC (permalink / raw)
To: Jason Andryuk, Jennifer Herbert
Cc: Jan Beulich, Andrew Cooper, Wei Liu, Roger Pau Monne,
Ian Jackson, xen-devel
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
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [for-4.17 PATCH] acpi: Add TPM2 interface definition and make the TPM version configurable.
2022-08-30 20:27 [PATCH] acpi: Add TPM2 interface definition and make the TPM version configurable Jennifer Herbert
2022-09-01 12:55 ` Jason Andryuk
@ 2022-09-06 14:18 ` Andrew Cooper
2022-09-06 14:30 ` Henry Wang
` (2 more replies)
1 sibling, 3 replies; 17+ messages in thread
From: Andrew Cooper @ 2022-09-06 14:18 UTC (permalink / raw)
To: Jennifer Herbert, jbeulich, wl, Roger Pau Monne, Henry Wang; +Cc: xen-devel
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
^ permalink raw reply [flat|nested] 17+ messages in thread
* RE: [for-4.17 PATCH] acpi: Add TPM2 interface definition and make the TPM version configurable.
2022-09-06 14:18 ` [for-4.17 PATCH] " Andrew Cooper
@ 2022-09-06 14:30 ` Henry Wang
2022-09-15 20:40 ` [PATCH 1/2] acpi: Make " Jennifer Herbert
2022-12-15 17:09 ` [PATCH v2 0/2] " Jennifer Herbert
2 siblings, 0 replies; 17+ messages in thread
From: Henry Wang @ 2022-09-06 14:30 UTC (permalink / raw)
To: Andrew Cooper, Jennifer Herbert, jbeulich, wl, Roger Pau Monne,
Anthony PERARD
Cc: xen-devel
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
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 1/2] acpi: Make TPM version configurable.
2022-09-06 14:18 ` [for-4.17 PATCH] " Andrew Cooper
2022-09-06 14:30 ` Henry Wang
@ 2022-09-15 20:40 ` Jennifer Herbert
2022-09-15 20:40 ` [PATCH 2/2] acpi: Add TPM2 interface definition Jennifer Herbert
` (2 more replies)
2022-12-15 17:09 ` [PATCH v2 0/2] " Jennifer Herbert
2 siblings, 3 replies; 17+ messages in thread
From: Jennifer Herbert @ 2022-09-15 20:40 UTC (permalink / raw)
To: jbeulich, andrew.cooper3, wl, roger.pau; +Cc: xen-devel, Jennifer Herbert
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
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 2/2] acpi: Add TPM2 interface definition.
2022-09-15 20:40 ` [PATCH 1/2] acpi: Make " Jennifer Herbert
@ 2022-09-15 20:40 ` Jennifer Herbert
2022-09-19 10:30 ` Jan Beulich
2022-10-11 15:53 ` Jennifer Herbert
2022-09-19 10:19 ` [PATCH 1/2] acpi: Make TPM version configurable Jan Beulich
2022-09-19 17:18 ` Jason Andryuk
2 siblings, 2 replies; 17+ messages in thread
From: Jennifer Herbert @ 2022-09-15 20:40 UTC (permalink / raw)
To: jbeulich, andrew.cooper3, wl, roger.pau; +Cc: xen-devel, Jennifer Herbert
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
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH 1/2] acpi: Make TPM version configurable.
2022-09-15 20:40 ` [PATCH 1/2] acpi: Make " Jennifer Herbert
2022-09-15 20:40 ` [PATCH 2/2] acpi: Add TPM2 interface definition Jennifer Herbert
@ 2022-09-19 10:19 ` Jan Beulich
2022-09-19 17:18 ` Jason Andryuk
2 siblings, 0 replies; 17+ messages in thread
From: Jan Beulich @ 2022-09-19 10:19 UTC (permalink / raw)
To: Jennifer Herbert; +Cc: xen-devel, andrew.cooper3, wl, roger.pau
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
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 2/2] acpi: Add TPM2 interface definition.
2022-09-15 20:40 ` [PATCH 2/2] acpi: Add TPM2 interface definition Jennifer Herbert
@ 2022-09-19 10:30 ` Jan Beulich
2022-10-11 15:53 ` Jennifer Herbert
1 sibling, 0 replies; 17+ messages in thread
From: Jan Beulich @ 2022-09-19 10:30 UTC (permalink / raw)
To: Jennifer Herbert; +Cc: xen-devel, andrew.cooper3, wl, roger.pau
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
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 1/2] acpi: Make TPM version configurable.
2022-09-15 20:40 ` [PATCH 1/2] acpi: Make " Jennifer Herbert
2022-09-15 20:40 ` [PATCH 2/2] acpi: Add TPM2 interface definition Jennifer Herbert
2022-09-19 10:19 ` [PATCH 1/2] acpi: Make TPM version configurable Jan Beulich
@ 2022-09-19 17:18 ` Jason Andryuk
2 siblings, 0 replies; 17+ messages in thread
From: Jason Andryuk @ 2022-09-19 17:18 UTC (permalink / raw)
To: Jennifer Herbert; +Cc: jbeulich, andrew.cooper3, wl, roger.pau, xen-devel
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
^ permalink raw reply [flat|nested] 17+ messages in thread
* RE: [PATCH 2/2] acpi: Add TPM2 interface definition.
2022-09-15 20:40 ` [PATCH 2/2] acpi: Add TPM2 interface definition Jennifer Herbert
2022-09-19 10:30 ` Jan Beulich
@ 2022-10-11 15:53 ` Jennifer Herbert
2022-10-12 6:59 ` Jan Beulich
1 sibling, 1 reply; 17+ messages in thread
From: Jennifer Herbert @ 2022-10-11 15:53 UTC (permalink / raw)
To: Jennifer Herbert, jbeulich, Andrew Cooper, wl, Roger Pau Monne; +Cc: xen-devel
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
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH 2/2] acpi: Add TPM2 interface definition.
2022-10-11 15:53 ` Jennifer Herbert
@ 2022-10-12 6:59 ` Jan Beulich
0 siblings, 0 replies; 17+ messages in thread
From: Jan Beulich @ 2022-10-12 6:59 UTC (permalink / raw)
To: Jennifer Herbert; +Cc: xen-devel, Andrew Cooper, wl, Roger Pau Monne
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
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH v2 0/2] acpi: Make TPM version configurable.
2022-09-06 14:18 ` [for-4.17 PATCH] " Andrew Cooper
2022-09-06 14:30 ` Henry Wang
2022-09-15 20:40 ` [PATCH 1/2] acpi: Make " Jennifer Herbert
@ 2022-12-15 17:09 ` Jennifer Herbert
2022-12-15 17:09 ` [PATCH v2 1/2] " Jennifer Herbert
2022-12-15 17:09 ` [PATCH v2 2/2] acpi: Add TPM2 interface definition Jennifer Herbert
2 siblings, 2 replies; 17+ messages in thread
From: Jennifer Herbert @ 2022-12-15 17:09 UTC (permalink / raw)
To: jennifer.herbert
Cc: Jan Beulich, Andrew Cooper, Roger Pau Monné, Wei Liu, xen-devel
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
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH v2 1/2] acpi: Make TPM version configurable.
2022-12-15 17:09 ` [PATCH v2 0/2] " Jennifer Herbert
@ 2022-12-15 17:09 ` Jennifer Herbert
2022-12-20 14:27 ` Jan Beulich
2022-12-15 17:09 ` [PATCH v2 2/2] acpi: Add TPM2 interface definition Jennifer Herbert
1 sibling, 1 reply; 17+ messages in thread
From: Jennifer Herbert @ 2022-12-15 17:09 UTC (permalink / raw)
To: jennifer.herbert
Cc: Jan Beulich, Andrew Cooper, Roger Pau Monné, Wei Liu, xen-devel
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
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH v2 2/2] acpi: Add TPM2 interface definition.
2022-12-15 17:09 ` [PATCH v2 0/2] " Jennifer Herbert
2022-12-15 17:09 ` [PATCH v2 1/2] " Jennifer Herbert
@ 2022-12-15 17:09 ` Jennifer Herbert
2022-12-20 14:44 ` Jan Beulich
1 sibling, 1 reply; 17+ messages in thread
From: Jennifer Herbert @ 2022-12-15 17:09 UTC (permalink / raw)
To: jennifer.herbert
Cc: Jan Beulich, Andrew Cooper, Roger Pau Monné, Wei Liu, xen-devel
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
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH v2 1/2] acpi: Make TPM version configurable.
2022-12-15 17:09 ` [PATCH v2 1/2] " Jennifer Herbert
@ 2022-12-20 14:27 ` Jan Beulich
0 siblings, 0 replies; 17+ messages in thread
From: Jan Beulich @ 2022-12-20 14:27 UTC (permalink / raw)
To: Jennifer Herbert; +Cc: Andrew Cooper, Roger Pau Monné, Wei Liu, xen-devel
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
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2 2/2] acpi: Add TPM2 interface definition.
2022-12-15 17:09 ` [PATCH v2 2/2] acpi: Add TPM2 interface definition Jennifer Herbert
@ 2022-12-20 14:44 ` Jan Beulich
0 siblings, 0 replies; 17+ messages in thread
From: Jan Beulich @ 2022-12-20 14:44 UTC (permalink / raw)
To: Jennifer Herbert; +Cc: Andrew Cooper, Roger Pau Monné, Wei Liu, xen-devel
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
^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2022-12-20 14:44 UTC | newest]
Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-08-30 20:27 [PATCH] acpi: Add TPM2 interface definition and make the TPM version configurable Jennifer Herbert
2022-09-01 12:55 ` Jason Andryuk
2022-09-06 13:03 ` Daniel P. Smith
2022-09-06 14:18 ` [for-4.17 PATCH] " Andrew Cooper
2022-09-06 14:30 ` Henry Wang
2022-09-15 20:40 ` [PATCH 1/2] acpi: Make " Jennifer Herbert
2022-09-15 20:40 ` [PATCH 2/2] acpi: Add TPM2 interface definition Jennifer Herbert
2022-09-19 10:30 ` Jan Beulich
2022-10-11 15:53 ` Jennifer Herbert
2022-10-12 6:59 ` Jan Beulich
2022-09-19 10:19 ` [PATCH 1/2] acpi: Make TPM version configurable Jan Beulich
2022-09-19 17:18 ` Jason Andryuk
2022-12-15 17:09 ` [PATCH v2 0/2] " Jennifer Herbert
2022-12-15 17:09 ` [PATCH v2 1/2] " Jennifer Herbert
2022-12-20 14:27 ` Jan Beulich
2022-12-15 17:09 ` [PATCH v2 2/2] acpi: Add TPM2 interface definition Jennifer Herbert
2022-12-20 14:44 ` Jan Beulich
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.