From ac5c57d24a30befe5eb772e09b22eac31ba177a1 Mon Sep 17 00:00:00 2001 From: Sean Rhodes Date: Mon, 30 Sep 2024 21:22:13 +0100 Subject: [PATCH] drivers/tpm/ppi: Fix generated ACPI The SSDT contains: CreateByteField (PPOP, Local0, TPPF) However, CreateByteField requires the source argument to be (Buffer | String | Integer). PPOP is an OperationRegion, so iasl correctly reports: Error 6058 - Invalid type ([Region] found) Per ACPI spec, OperationRegions must use CreateField rather than CreateByteField. Replace the AML emission with: CreateField (PPOP, Local0 * 8, 8, TPPF) This reads one byte at an arbitrary offset inside the PPI OpRegion and is fully standards-compliant. This isn't a functional change, just "correct". Test=boot starbook_mtl, verify iasl can decompile and recompile SSDT and TPM is still operational. Change-Id: If80bb5bf69562f8b904c1b315e95a0b5627efbc4 Signed-off-by: Sean Rhodes Reviewed-on: https://review.coreboot.org/c/coreboot/+/84606 Tested-by: build bot (Jenkins) Reviewed-by: Matt DeVillier --- src/drivers/tpm/ppi.c | 21 +++++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-) diff --git a/src/drivers/tpm/ppi.c b/src/drivers/tpm/ppi.c index 68923eaeda..0ddbfe5534 100644 --- a/src/drivers/tpm/ppi.c +++ b/src/drivers/tpm/ppi.c @@ -633,7 +633,10 @@ void tpm_ppi_acpi_fill_ssdt(const struct device *dev) */ acpigen_write_method_serialized("FUNC", 1); + /* Local0 = ToInteger(Arg0) */ acpigen_write_to_integer(ARG0_OP, LOCAL0_OP); + + /* If (Local0 > 0x80) */ acpigen_write_if(); acpigen_emit_byte(LGREATER_OP); acpigen_emit_byte(LOCAL0_OP); @@ -641,17 +644,27 @@ void tpm_ppi_acpi_fill_ssdt(const struct device *dev) acpigen_write_return_integer(0); acpigen_pop_len(); /* If */ - /* TPPF = CreateField("PPOP", Local0) */ - acpigen_emit_byte(CREATE_BYTE_OP); - acpigen_emit_namestring("PPOP"); + /* Local1 = (Local0 * 0x08) */ + acpigen_emit_byte(MULTIPLY_OP); acpigen_emit_byte(LOCAL0_OP); + acpigen_write_integer(8); + acpigen_emit_byte(LOCAL1_OP); + + /* CreateField (PPOP, Local1, 0x08, TPPF) */ + acpigen_emit_ext_op(CREATEFIELD_OP); + acpigen_emit_namestring("PPOP"); + acpigen_emit_byte(LOCAL1_OP); + acpigen_write_integer(8); + + /* Name of the dynamically-created field */ acpigen_emit_namestring("TPPF"); - /* Local0 = ToInteger("TPPF") */ + /* Local0 = ToInteger(TPPF) */ acpigen_emit_byte(TO_INTEGER_OP); acpigen_emit_namestring("TPPF"); acpigen_emit_byte(LOCAL0_OP); + /* Return (Local0) */ acpigen_write_return_op(LOCAL0_OP); acpigen_pop_len(); /* Method */