On Tue, Jan 16, 2018 at 11:41:02AM -0500, Stefan Berger wrote: > Add support for TPM 1.2 and TPM 2 Physical Presence interface (PPI). > A shared memory structure is located at 0xfffe f000 - 0xfffe f3ff > that SeaBIOS initializes (unless it has already been intialized) and > then searches for a code it is supposed to act upon. A code typically > requires that one or more TPM commands are being sent.
If I'm understanding the code correctly, it no longer hardcodes 0xfffef000 (great!). The commit comment should also be updated. > > The underlying spec can be accessed from this page here: > > https://trustedcomputinggroup.org/tcg-physical-presence-interface-specification/ > > Version 1.30 is implemented. > > Signed-off-by: Stefan Berger <stef...@linux.vnet.ibm.com> > --- > src/post.c | 4 +++ > src/std/acpi.h | 10 ++++++ > src/std/tcg.h | 31 ++++++++++++++++++ > src/tcgbios.c | 99 > ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ > src/tcgbios.h | 3 ++ > 5 files changed, 147 insertions(+) > > diff --git a/src/post.c b/src/post.c > index f93106a..f451013 100644 > --- a/src/post.c > +++ b/src/post.c > @@ -201,6 +201,7 @@ maininit(void) > > // Setup platform devices. > platform_hardware_setup(); > + tpm_ppi_init(); > > // Start hardware initialization (if threads allowed during optionroms) > if (threads_during_optionroms()) > @@ -220,6 +221,9 @@ maininit(void) > // Run option roms > optionrom_setup(); > > + // Process user-requested TPM state change > + tpm_ppi_process(); I think it would be better if these two calls were added to the existing tpm_setup() call. Also, function suffixes of "_init" and "_setup" have specific meaning in seabios - see docs/Execution_and_code_flow.md - so you should not export a function with those sufixes unless they follow the convention. [...] > --- a/src/std/acpi.h > +++ b/src/std/acpi.h > @@ -320,4 +320,14 @@ struct tpm2_descriptor_rev2 > u64 log_area_start_address; > } PACKED; > > +#define QEMU_SIGNATURE 0x554d4551 > +struct qemu_descriptor > +{ > + ACPI_TABLE_HEADER_DEF > + u32 tpmppi_address; > + u8 tpm_version; /* 1 = 1.2, 2 = 2 */ > + u8 tpmppi_version; > +#define TPM_PPI_VERSION_1_30 1 > +} PACKED; I'm confused at the purpose of this acpi table. If I'm understanding it correctly, it is purely to pass information from QEMU to SeaBIOS (and perhaps OVMF?). If so, I don't think this is a good way to do it - a regular fw_cfg setting seems simpler (and less likely to cause problems with OSes). > + > #endif // acpi.h > diff --git a/src/std/tcg.h b/src/std/tcg.h > index 09a92d8..22353a9 100644 > --- a/src/std/tcg.h > +++ b/src/std/tcg.h > @@ -551,4 +551,35 @@ struct pcctes_romex > #define TPM_PPI_OP_SET_OWNERINSTALL_TRUE 8 > #define TPM_PPI_OP_SET_OWNERINSTALL_FALSE 9 > > +struct tpm_ppi { > + u8 ppin; /* 0: 1 = initialized */ > + u32 ppip; /* 1: not used */ > + u32 pprp; /* 5: response from TPM; set by BIOS */ > + u32 pprq; /* 9: opcode; set by ACPI */ > + u32 pprm; /* 13: parameter for opcode; set by ACPI */ > + u32 lppr; /* 17: last opcode; set by BIOS */ > + u32 fret; /* 21: not used */ > + u8 res1; /* 25: reserved */ > + u32 res2[4]; /* 26: reserved */ > + u8 res3[214]; /* 42: reserved */ > + u8 func[256]; /* 256: per function implementation flags; set by > BIOS */ > +/* indication whether function is implemented; bit 0 */ > +#define TPM_PPI_FUNC_IMPLEMENTED (1 << 0) > +/* actions OS should take to transition to the pre-OS env.; bits 1, 2 */ > +#define TPM_PPI_FUNC_ACTION_SHUTDOWN (1 << 1) > +#define TPM_PPI_FUNC_ACTION_REBOOT (2 << 1) > +#define TPM_PPI_FUNC_ACTION_VENDOR (3 << 1) > +#define TPM_PPI_FUNC_ACTION_MASK (3 << 1) > +/* whether function is blocked by BIOS settings; bits 3,4,5 */ > +#define TPM_PPI_FUNC_NOT_IMPLEMENTED (0 << 3) > +#define TPM_PPI_FUNC_BIOS_ONLY (1 << 3) > +#define TPM_PPI_FUNC_BLOCKED (2 << 3) > +#define TPM_PPI_FUNC_ALLOWED_USR_REQ (3 << 3) > +#define TPM_PPI_FUNC_ALLOWED_USR_NOT_REQ (4 << 3) > +#define TPM_PPI_FUNC_MASK (7 << 3) > +} PACKED; > + > +void tpm_ppi_init(void); > +void tpm_ppi_process(void); The files in the std/ directory are for well defined specifications. It should not be used to export seabios functions. Also, it's not clear to me if 'struct tpm_ppi' and the TPM_PPI_FUNC_* defines are a qemu/acpi/seabios interface or a TPM standard. > + > #endif // tcg.h > diff --git a/src/tcgbios.c b/src/tcgbios.c > index 730b5e7..c8e6ca2 100644 > --- a/src/tcgbios.c > +++ b/src/tcgbios.c > @@ -1783,6 +1783,18 @@ tpm20_process_cfg(tpm_ppi_code msgCode, int verbose, > u32 *returnCode) > } > > static int > +tpm_process_cfg(tpm_ppi_code msgCode, int verbose, u32 *returnCode) > +{ > + switch (TPM_version) { > + case TPM_VERSION_1_2: > + return tpm12_process_cfg(msgCode, verbose, returnCode); > + case TPM_VERSION_2: > + return tpm20_process_cfg(msgCode, verbose, returnCode); > + } > + return -1; > +} > + > +static int > tpm12_get_tpm_state(void) > { > int state = 0; > @@ -2021,3 +2033,90 @@ tpm_can_show_menu(void) > } > return 0; > } > + > +static struct tpm_ppi *tp; > +static u8 nextStep = TPM_PPI_OP_NOOP; /* opcode to execute after reboot */ > + > +#define FLAGS (TPM_PPI_FUNC_IMPLEMENTED | \ > + TPM_PPI_FUNC_ACTION_REBOOT | \ > + TPM_PPI_FUNC_ALLOWED_USR_NOT_REQ) > + > +static const u8 tpm12_ppi_funcs[] = { > + [TPM_PPI_OP_NOOP] = TPM_PPI_FUNC_IMPLEMENTED | > + TPM_PPI_FUNC_ALLOWED_USR_NOT_REQ, > + [TPM_PPI_OP_ENABLE] = FLAGS, > + [TPM_PPI_OP_DISABLE] = FLAGS, > + [TPM_PPI_OP_ACTIVATE] = FLAGS, > + [TPM_PPI_OP_DEACTIVATE] = FLAGS, > + [TPM_PPI_OP_CLEAR] = FLAGS, > + [TPM_PPI_OP_SET_OWNERINSTALL_TRUE] = FLAGS, > + [TPM_PPI_OP_SET_OWNERINSTALL_FALSE] = FLAGS, > +}; > + > +static const u8 tpm2_ppi_funcs[] = { > + [TPM_PPI_OP_CLEAR] = FLAGS, > +}; > + > +void > +tpm_ppi_init(void) > +{ > + struct qemu_descriptor *qemu = NULL; > + > + while (1) { > + qemu = find_acpi_table_iter(QEMU_SIGNATURE, qemu); > + if (!qemu) > + return; > + if (!memcmp("QEMU", qemu->oem_id, 5) && !memcmp("CONF", > qemu->oem_table_id, 5)) > + break; > + } > + > + tp = (struct tpm_ppi *)(u32)qemu->tpmppi_address; > + dprintf(DEBUG_tcg, "TCGBIOS: TPM PPI struct at %p\n", tp); > + > + memset(&tp->func, 0, sizeof(tp->func)); > + switch (qemu->tpmppi_version) { > + case TPM_PPI_VERSION_1_30: > + switch (qemu->tpm_version) { > + case TPM_VERSION_1_2: > + memcpy(&tp->func, tpm12_ppi_funcs, sizeof(tpm12_ppi_funcs)); > + break; > + case TPM_VERSION_2: > + memcpy(&tp->func, tpm2_ppi_funcs, sizeof(tpm2_ppi_funcs)); > + break; > + } Can you elaborate on what this does? Why is SeaBIOS filling a memory addreses created by QEMU? Why wouldn't QEMU just fill it with what it wants directly? -Kevin > + break; > + } > + > + if (!tp->ppin) { > + tp->ppin = 1; > + tp->pprq = 0; > + tp->lppr = 0; > + } > +} > + > +void > +tpm_ppi_process(void) > +{ > + tpm_ppi_code op; > + > + if (tp) { > + op = tp->pprq; > + if (!op) { > + /* intermediate step after a reboot? */ > + op = nextStep; > + } else { > + /* last full opcode */ > + tp->lppr = op; > + } > + if (op) { > + /* > + * Reset the opcode so we don't permanently reboot upon > + * code 3 (Activate). > + */ > + tp->pprq = 0; > + > + printf("Processing TPM PPI opcode %d\n", op); > + tpm_process_cfg(op, 0, &tp->pprp); > + } > + } > +} > diff --git a/src/tcgbios.h b/src/tcgbios.h > index 32fb941..52b86f2 100644 > --- a/src/tcgbios.h > +++ b/src/tcgbios.h > @@ -16,4 +16,7 @@ void tpm_option_rom(const void *addr, u32 len); > int tpm_can_show_menu(void); > void tpm_menu(void); > > +void tpm_ppi_init(void); > +void tpm_ppi_process(void); > + > #endif /* TCGBIOS_H */ > -- > 2.5.5 > _______________________________________________ SeaBIOS mailing list SeaBIOS@seabios.org https://mail.coreboot.org/mailman/listinfo/seabios