On 25.04.2023 19:47, Jennifer Herbert 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_version xenstore key.
>
> Signed-off-by: Jennifer Herbert <jennifer.herb...@citrix.com>
> ---
> docs/misc/xenstore-paths.pandoc | 9 +++++
> tools/firmware/hvmloader/util.c | 19 ++++++---
> tools/libacpi/build.c | 69 +++++++++++++++++++--------------
> tools/libacpi/libacpi.h | 3 +-
> 4 files changed, 64 insertions(+), 36 deletions(-)
Please can you get used to providing a brief rev log somewhere here?
> --- 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 |
> - ACPI_HAS_WAET | ACPI_HAS_PMTIMER |
> - ACPI_HAS_BUTTONS | ACPI_HAS_VGA |
> - ACPI_HAS_8042 | ACPI_HAS_CMOS_RTC);
> + config->table_flags |= (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", "1");
> + config->tpm_version = strtoll(s, NULL, 0);
Due to field width, someone specifying 257 will also get a 1.2 TPM,
if I'm not mistaken.
> + switch( config->tpm_version )
Nit: Style (missing blank).
> --- a/tools/libacpi/build.c
> +++ b/tools/libacpi/build.c
> @@ -409,38 +409,47 @@ 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 its 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 0: /* Assume legacy code wanted tpm 1.2 */
Along the lines of what Jason said: Unless this is known to be needed for
anything, I'd prefer if it was omitted.
Jan
