On Tue, 15 May 2018 14:14:32 +0200 Marc-André Lureau <marcandre.lur...@redhat.com> wrote:
> From: Stefan Berger <stef...@linux.vnet.ibm.com> > > The TPM Physical Presence interface consists of an ACPI part, a shared > memory part, and code in the firmware. Users can send messages to the > firmware by writing a code into the shared memory through invoking the > ACPI code. When a reboot happens, the firmware looks for the code and > acts on it by sending sequences of commands to the TPM. > > This patch adds the ACPI code. It is similar to the one in EDK2 but doesn't > assume that SMIs are necessary to use. It uses a similar datastructure for > the shared memory as EDK2 does so that EDK2 and SeaBIOS could both make use > of it. I extended the shared memory data structure with an array of 256 > bytes, one for each code that could be implemented. The array contains > flags describing the individual codes. This decouples the ACPI implementation > from the firmware implementation. > > The underlying TCG specification is accessible from the following page. > > https://trustedcomputinggroup.org/tcg-physical-presence-interface-specification/ > > This patch implements version 1.30. > > Signed-off-by: Stefan Berger <stef...@linux.vnet.ibm.com> > > --- > > v4 (Marc-André): > - replace 'DerefOf (FUNC [N])' with a function, to fix Windows ACPI > handling. > - replace 'return Package (..) {} ' with scoped variables, to fix > Windows ACPI handling. > > v3: > - add support for PPI to CRB > - split up OperationRegion TPPI into two parts, one containing > the registers (TPP1) and the other one the flags (TPP2); switched > the order of the flags versus registers in the code > - adapted ACPI code to small changes to the array of flags where > previous flag 0 was removed and now shifting right wasn't always > necessary anymore > > v2: > - get rid of FAIL variable; function 5 was using it and always > returns 0; the value is related to the ACPI function call not > a possible failure of the TPM function call. > - extend shared memory data structure with per-opcode entries > holding flags and use those flags to determine what to return > to caller > - implement interface version 1.3 > --- > include/hw/acpi/tpm.h | 21 +++ > hw/i386/acpi-build.c | 294 +++++++++++++++++++++++++++++++++++++++++- > 2 files changed, 314 insertions(+), 1 deletion(-) > > diff --git a/include/hw/acpi/tpm.h b/include/hw/acpi/tpm.h > index f79d68a77a..fc53f08827 100644 > --- a/include/hw/acpi/tpm.h > +++ b/include/hw/acpi/tpm.h > @@ -196,4 +196,25 @@ REG32(CRB_DATA_BUFFER, 0x80) > #define TPM_PPI_VERSION_NONE 0 > #define TPM_PPI_VERSION_1_30 1 > > +struct tpm_ppi { > + uint8_t func[256]; /* 0x000: per TPM function implementation flags; > + set by BIOS */ > +/* whether function is blocked by BIOS settings; bits 0, 1, 2 */ > +#define TPM_PPI_FUNC_NOT_IMPLEMENTED (0 << 0) > +#define TPM_PPI_FUNC_BIOS_ONLY (1 << 0) > +#define TPM_PPI_FUNC_BLOCKED (2 << 0) > +#define TPM_PPI_FUNC_ALLOWED_USR_REQ (3 << 0) > +#define TPM_PPI_FUNC_ALLOWED_USR_NOT_REQ (4 << 0) > +#define TPM_PPI_FUNC_MASK (7 << 0) > + uint8_t ppin; /* 0x100 : set by BIOS */ > + uint32_t ppip; /* 0x101 : set by ACPI; not used */ > + uint32_t pprp; /* 0x105 : response from TPM; set by BIOS */ > + uint32_t pprq; /* 0x109 : opcode; set by ACPI */ > + uint32_t pprm; /* 0x10d : parameter for opcode; set by ACPI */ > + uint32_t lppr; /* 0x111 : last opcode; set by BIOS */ > + uint32_t fret; /* 0x115 : set by ACPI; not used */ > + uint8_t res1[0x40]; /* 0x119 : reserved for future use */ > + uint8_t next_step; /* 0x159 : next step after reboot; set by BIOS > */ > +} QEMU_PACKED; is this structure plant to be used for accessing data from within QEMU or it is just used for convenience of calculating offsets/sizes for AML? > + > #endif /* HW_ACPI_TPM_H */ > diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c > index f6d447f03a..95be4f0710 100644 > --- a/hw/i386/acpi-build.c > +++ b/hw/i386/acpi-build.c > @@ -43,6 +43,7 @@ > #include "hw/acpi/memory_hotplug.h" > #include "sysemu/tpm.h" > #include "hw/acpi/tpm.h" > +#include "hw/tpm/tpm_ppi.h" > #include "hw/acpi/vmgenid.h" > #include "sysemu/tpm_backend.h" > #include "hw/timer/mc146818rtc_regs.h" > @@ -1789,6 +1790,292 @@ static Aml *build_q35_osc_method(void) > return method; > } > > +static void > +build_tpm_ppi(Aml *dev) > +{ > + Aml *method, *name, *field, *ifctx, *ifctx2, *ifctx3, *pak; > + struct tpm_ppi *tpm_ppi = NULL; > + int i; plain AML variables throughout the function make code rather unreadable (well, like any other AML code would be). Pls take a look at how build_legacy_cpu_hotplug_aml() uses intermediate more or less sanely named variables to make it more understandable. that also should allow to reduce LOC this function takes. > + /* > + * TPP1 is for the flags that indicate which PPI operations > + * are supported by the firmware. The firmware is expected to > + * write these flags. > + */ > + aml_append(dev, > + aml_operation_region("TPP1", AML_SYSTEM_MEMORY, > + aml_int(TPM_PPI_ADDR_BASE), > + sizeof(tpm_ppi->func))); > + field = aml_field("TPP1", AML_ANY_ACC, AML_NOLOCK, AML_PRESERVE); > + for (i = 0; i < sizeof(tpm_ppi->func); i++) { > + char *tmp = g_strdup_printf("FN%02X", i); > + aml_append(field, aml_named_field(tmp, BITS_PER_BYTE)); > + g_free(tmp); > + } > + aml_append(dev, field); > + > + /* > + * TPP2 is for the registers that ACPI code used to pass > + * the PPI code and parameter (PPRQ, PPRM) to the firmware. > + */ > + aml_append(dev, > + aml_operation_region("TPP2", AML_SYSTEM_MEMORY, > + aml_int(TPM_PPI_ADDR_BASE + > + offsetof(struct tpm_ppi, ppin)), > + sizeof(struct tpm_ppi) - > + sizeof(tpm_ppi->func))); > + field = aml_field("TPP2", AML_ANY_ACC, AML_NOLOCK, AML_PRESERVE); > + aml_append(field, aml_named_field("PPIN", > + sizeof(uint8_t) * BITS_PER_BYTE)); here you already don't use tpm_ppi for sizing, so it creates a confusing mix of both approaches. From ACPI pov I'd prefer PPI table documented somewhere in spec docs (similar docs/specs/acpi_mem_hotplug.txt) and would look like in other use-cases: aml_append(field, aml_named_field("PPIN", 8) and drop "struct tpm_ppi" altogether replacing places it was used by explicit constants. > + aml_append(field, aml_named_field("PPIP", > + sizeof(uint32_t) * BITS_PER_BYTE)); > + aml_append(field, aml_named_field("PPRP", > + sizeof(uint32_t) * BITS_PER_BYTE)); > + aml_append(field, aml_named_field("PPRQ", > + sizeof(uint32_t) * BITS_PER_BYTE)); > + aml_append(field, aml_named_field("PPRM", > + sizeof(uint32_t) * BITS_PER_BYTE)); > + aml_append(field, aml_named_field("LPPR", > + sizeof(uint32_t) * BITS_PER_BYTE)); > + aml_append(dev, field); > + > + method = aml_method("TPFN", 1, AML_SERIALIZED); not quite sure how this method (supposed to ) work(s), it could use nice comment explaining mechanics. > + { > + for (i = 0; i < sizeof(tpm_ppi->func); i++) { > + ifctx = aml_if(aml_equal(aml_int(i), aml_arg(0))); > + { > + aml_append(ifctx, aml_return(aml_name("FN%02X", i))); > + } > + aml_append(method, ifctx); > + } > + aml_append(method, aml_return(aml_int(0))); > + } > + aml_append(dev, method); > + > + pak = aml_package(2); > + aml_append(pak, aml_int(0)); > + aml_append(pak, aml_int(0)); > + name = aml_name_decl("TPM2", pak); > + aml_append(dev, name); > + > + pak = aml_package(3); > + aml_append(pak, aml_int(0)); > + aml_append(pak, aml_int(0)); > + aml_append(pak, aml_int(0)); > + name = aml_name_decl("TPM3", pak); > + aml_append(dev, name); > + > + method = aml_method("_DSM", 4, AML_SERIALIZED); > + { > + uint8_t zerobyte[1] = { 0 }; > + > + ifctx = aml_if( > + aml_equal(aml_arg(0), > + aml_touuid("3DDDFAA6-361B-4EB4-A424-8D10089D1653"))); a comment pointing to a specific part/version of spec that documents this _DSM for every uuid/function used here would help to review it. > + { > + aml_append(ifctx, > + aml_store(aml_to_integer(aml_arg(2)), aml_local(0))); > + > + /* standard DSM query function */ > + ifctx2 = aml_if(aml_equal(aml_local(0), aml_int(0))); > + { > + uint8_t byte_list[2] = { 0xff, 0x01 }; > + aml_append(ifctx2, aml_return(aml_buffer(2, byte_list))); > + } > + aml_append(ifctx, ifctx2); > + > + /* interface version: 1.3 */ > + ifctx2 = aml_if(aml_equal(aml_local(0), aml_int(1))); > + { > + aml_append(ifctx2, aml_return(aml_string("1.3"))); > + } > + aml_append(ifctx, ifctx2); > + > + /* submit TPM operation */ > + ifctx2 = aml_if(aml_equal(aml_local(0), aml_int(2))); > + { > + /* get opcode */ > + aml_append(ifctx2, > + aml_store(aml_derefof(aml_index(aml_arg(3), > + aml_int(0))), > + aml_local(0))); > + /* get opcode flags */ > + aml_append(ifctx2, > + aml_store(aml_call1("TPFN", aml_local(0)), > + aml_local(1))); > + ifctx3 = aml_if( > + aml_equal( > + aml_and(aml_local(1), aml_int(TPM_PPI_FUNC_MASK), > NULL), > + aml_int(TPM_PPI_FUNC_NOT_IMPLEMENTED))); > + { > + /* 1: not implemented */ > + aml_append(ifctx3, aml_return(aml_int(1))); > + } > + aml_append(ifctx2, ifctx3); > + aml_append(ifctx2, aml_store(aml_local(0), > aml_name("PPRQ"))); > + aml_append(ifctx2, aml_store(aml_int(0), aml_name("PPRM"))); > + /* 0: success */ > + aml_append(ifctx2, aml_return(aml_int(0))); > + } > + aml_append(ifctx, ifctx2); > + > + /* get pending TPM operation */ > + ifctx2 = aml_if(aml_equal(aml_local(0), aml_int(3))); > + { > + /* revision to integer */ > + aml_append(ifctx2, > + aml_store( > + aml_to_integer(aml_arg(1)), > + aml_local(1))); > + ifctx3 = aml_if(aml_equal(aml_local(1), aml_int(1))); > + { > + aml_append(ifctx3, > + aml_store( > + aml_name("PPRQ"), > + aml_index(aml_name("TPM2"), aml_int(1)))); > + aml_append(ifctx3, aml_return(aml_name("TPM2"))); > + } > + aml_append(ifctx2, ifctx3); > + > + ifctx3 = aml_if(aml_equal(aml_local(1), aml_int(2))); > + { > + aml_append(ifctx3, > + aml_store( > + aml_name("PPRQ"), > + aml_index(aml_name("TPM3"), aml_int(1)))); > + aml_append(ifctx3, > + aml_store( > + aml_name("PPRM"), > + aml_index(aml_name("TPM3"), aml_int(2)))); > + aml_append(ifctx3, aml_return(aml_name("TPM3"))); > + } > + aml_append(ifctx2, ifctx3); > + } > + aml_append(ifctx, ifctx2); > + > + /* get platform-specific action to transition to pre-OS env. */ > + ifctx2 = aml_if(aml_equal(aml_local(0), aml_int(4))); > + { > + /* reboot */ > + aml_append(ifctx2, aml_return(aml_int(2))); > + } > + aml_append(ifctx, ifctx2); > + > + /* get TPM operation response */ > + ifctx2 = aml_if(aml_equal(aml_local(0), aml_int(5))); > + { > + aml_append(ifctx2, > + aml_store( > + aml_name("LPPR"), > + aml_index(aml_name("TPM3"), aml_int(1)))); > + aml_append(ifctx2, > + aml_store( > + aml_name("PPRP"), > + aml_index(aml_name("TPM3"), aml_int(2)))); > + aml_append(ifctx2, aml_return(aml_name("TPM3"))); > + } > + aml_append(ifctx, ifctx2); > + > + /* submit preferred user language */ > + ifctx2 = aml_if(aml_equal(aml_local(0), aml_int(6))); > + { > + /* 3 = not implemented */ > + aml_append(ifctx2, aml_return(aml_int(3))); > + } > + aml_append(ifctx, ifctx2); > + > + /* submit TPM operation v2 */ > + ifctx2 = aml_if(aml_equal(aml_local(0), aml_int(7))); > + { > + /* get opcode */ > + aml_append(ifctx2, > + aml_store(aml_derefof(aml_index(aml_arg(3), > + aml_int(0))), > + aml_local(0))); > + /* get opcode flags */ > + aml_append(ifctx2, > + aml_store(aml_call1("TPFN", aml_local(0)), > + aml_local(1))); > + ifctx3 = aml_if( > + aml_equal( > + aml_and(aml_local(1), aml_int(TPM_PPI_FUNC_MASK), > NULL), > + aml_int(TPM_PPI_FUNC_NOT_IMPLEMENTED))); > + { > + /* 1: not implemented */ > + aml_append(ifctx3, aml_return(aml_int(1))); > + } > + aml_append(ifctx2, ifctx3); > + > + ifctx3 = aml_if( > + aml_equal( > + aml_and(aml_local(1), aml_int(TPM_PPI_FUNC_MASK), > NULL), > + aml_int(TPM_PPI_FUNC_BLOCKED))); > + { > + /* 3: blocked by firmware */ > + aml_append(ifctx3, aml_return(aml_int(3))); > + } > + aml_append(ifctx2, ifctx3); > + > + /* revision to integer */ > + aml_append(ifctx2, > + aml_store( > + aml_to_integer(aml_arg(1)), > + aml_local(1))); > + > + ifctx3 = aml_if(aml_equal(aml_local(1), aml_int(1))); > + { > + /* revision 1 */ > + aml_append(ifctx3, aml_store(aml_local(0), > + aml_name("PPRQ"))); > + aml_append(ifctx3, aml_store(aml_int(0), > + aml_name("PPRM"))); > + } > + aml_append(ifctx2, ifctx3); > + > + ifctx3 = aml_if(aml_equal(aml_local(1), aml_int(2))); > + { > + /* revision 2 */ > + aml_append(ifctx3, > + aml_store(aml_local(0), aml_name("PPRQ"))); > + aml_append(ifctx3, > + aml_store( > + aml_derefof(aml_index(aml_arg(3), > + aml_int(1))), > + aml_name("PPRM"))); > + } > + aml_append(ifctx2, ifctx3); > + /* 0: success */ > + aml_append(ifctx2, aml_return(aml_int(0))); > + } > + aml_append(ifctx, ifctx2); > + > + /* get user confirmation status for operation */ > + ifctx2 = aml_if(aml_equal(aml_local(0), aml_int(8))); > + { > + /* get opcode */ > + aml_append(ifctx2, > + aml_store(aml_derefof(aml_index(aml_arg(3), > + aml_int(0))), > + aml_local(0))); > + /* get opcode flags */ > + aml_append(ifctx2, > + aml_store(aml_call1("TPFN", aml_local(0)), > + aml_local(1))); > + /* return confirmation status code */ > + aml_append(ifctx2, > + aml_return( > + aml_and(aml_local(1), > + aml_int(TPM_PPI_FUNC_MASK), NULL))); > + } > + aml_append(ifctx, ifctx2); > + > + aml_append(ifctx, aml_return(aml_buffer(1, zerobyte))); > + } > + aml_append(method, ifctx); > + } > + aml_append(dev, method); > +} > + > static void > build_dsdt(GArray *table_data, BIOSLinker *linker, > AcpiPmInfo *pm, AcpiMiscInfo *misc, > @@ -2153,6 +2440,9 @@ build_dsdt(GArray *table_data, BIOSLinker *linker, > */ > /* aml_append(crs, aml_irq_no_flags(TPM_TIS_IRQ)); */ > aml_append(dev, aml_name_decl("_CRS", crs)); > + > + build_tpm_ppi(dev); > + > aml_append(scope, dev); > } > > @@ -2172,6 +2462,8 @@ build_dsdt(GArray *table_data, BIOSLinker *linker, > aml_append(method, aml_return(aml_int(0x0f))); > aml_append(dev, method); > > + build_tpm_ppi(dev); > + > aml_append(sb_scope, dev); > } > > @@ -2918,7 +3210,7 @@ void acpi_setup(void) > tpm_config = (FWCfgTPMConfig) { > .tpmppi_address = cpu_to_le32(TPM_PPI_ADDR_BASE), > .tpm_version = cpu_to_le32(tpm_get_version(tpm_find())), > - .tpmppi_version = cpu_to_le32(TPM_PPI_VERSION_NONE) what was the point of introducing TPM_PPI_VERSION_NONE if it would not be used? maybe patch ordering is problem? what if we put fwcfg patch after this one? > + .tpmppi_version = cpu_to_le32(TPM_PPI_VERSION_1_30) > }; > fw_cfg_add_file(pcms->fw_cfg, "etc/tpm/config", > &tpm_config, sizeof tpm_config); hopefully patch would be more review-able with comments and nicely named variables, so I could actually review it functionality wise without reading whole TPM spec(s)
