On 25.04.2023 19:47, Jennifer Herbert wrote:
> --- a/tools/libacpi/acpi2_0.h
> +++ b/tools/libacpi/acpi2_0.h
> @@ -121,6 +121,36 @@ struct acpi_20_tcpa {
> };
> #define ACPI_2_0_TCPA_LAML_SIZE (64*1024)
>
> +/*
> + * TPM2
> + */
Nit: While I'm willing to accept the comment style violation here as
(apparently) intentional, ...
> +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
> +
> +/* TPM register I/O Mapped region, location of which defined in the
> + * TCG PC Client Platform TPM Profile Specification for TPM 2.0.
> + * See table 9 - Only Locality 0 is used here. This is emulated by QEMU.
> + * Definition of Register space is found in table 12.
> + */
... this comment wants adjusting to hypervisor style (/* on its own line),
as that looks to be the aimed-at style in this file.
> @@ -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;
Could I talk you into moving this up by two lines, such that it'll be
adjacent to "tcpa"?
> @@ -450,6 +452,43 @@ static int construct_secondary_tables(struct acpi_ctxt
> *ctxt,
> tcpa->header.length);
> }
> break;
> +
> + case 2:
> + /* Check VID stored in bits 37:32 (3rd 16 bit word) of CRB
> + * identifier register. See table 16 of TCG PC client platform
> + * TPM profile specification for TPM 2.0.
> + */
Nit: This comment again wants a style adjustment.
> --- /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.
> + */
While the full conversion to SPDX was done in the hypervisor only so far,
I think new tool stack source files would better use the much shorter
SPDX equivalent, too.
Then on top of Jason's R-b,
Acked-by: Jan Beulich <jbeul...@suse.com>
Jan
