The following 3 patches add support for the TPM's Physical Presence interface (PPI). The patches require the recently posted QEMU patch for support of PPI (PATCH v2):
http://lists.nongnu.org/archive/html/qemu-devel/2018-01/msg03652.html
We assume a device at 0xfffe f000 - 0xfffe f3ff where the OS can write a code into that SeaBIOS needs to find and act upon after a reboot.
Stefan
Stefan Berger (3): tcgbios: pass returnCode through many functions tcgbios: Add TPM Physical Presence interface support tcgbios: extend Physical Presence interface with more functions
src/hw/tpm_drivers.c | 8 +- src/hw/tpm_drivers.h | 2 +- src/post.c | 4 + src/std/acpi.h | 10 ++ src/std/tcg.h | 38 +++++++ src/tcgbios.c | 305 +++++++++++++++++++++++++++++++++++++++------------ src/tcgbios.h | 3 + 7 files changed, 299 insertions(+), 71 deletions(-)
Pass the returnCode parameter through many of the functions so that we can get the TPM return code from hwtpm_transmit, if needed.
Signed-off-by: Stefan Berger stefanb@linux.vnet.ibm.com --- src/hw/tpm_drivers.c | 8 ++- src/hw/tpm_drivers.h | 2 +- src/tcgbios.c | 147 +++++++++++++++++++++++++++------------------------ 3 files changed, 86 insertions(+), 71 deletions(-)
diff --git a/src/hw/tpm_drivers.c b/src/hw/tpm_drivers.c index a137e62..25f4f36 100644 --- a/src/hw/tpm_drivers.c +++ b/src/hw/tpm_drivers.c @@ -377,7 +377,7 @@ tpmhw_is_present(void) int tpmhw_transmit(u8 locty, struct tpm_req_header *req, void *respbuffer, u32 *respbufferlen, - enum tpmDurationType to_t) + enum tpmDurationType to_t, u32 *returnCode) { if (TPMHW_driver_to_use == TPM_INVALID_DRIVER) return -1; @@ -408,6 +408,12 @@ tpmhw_transmit(u8 locty, struct tpm_req_header *req,
td->ready();
+ if (returnCode && *respbufferlen >= sizeof(struct tpm_rsp_header)) { + struct tpm_rsp_header *r = respbuffer; + + *returnCode = be32_to_cpu(r->errcode); + } + return 0; }
diff --git a/src/hw/tpm_drivers.h b/src/hw/tpm_drivers.h index 56fd9e8..59425b9 100644 --- a/src/hw/tpm_drivers.h +++ b/src/hw/tpm_drivers.h @@ -21,7 +21,7 @@ int tpmhw_is_present(void); struct tpm_req_header; int tpmhw_transmit(u8 locty, struct tpm_req_header *req, void *respbuffer, u32 *respbufferlen, - enum tpmDurationType to_t); + enum tpmDurationType to_t, u32 *returnCode); void tpmhw_set_timeouts(u32 timeouts[4], u32 durations[3]);
/* TIS driver */ diff --git a/src/tcgbios.c b/src/tcgbios.c index 40b3028..730b5e7 100644 --- a/src/tcgbios.c +++ b/src/tcgbios.c @@ -350,7 +350,8 @@ tpm_build_digest(struct tpm_log_entry *le, const u8 *sha1, int bigEndian) // optional parameter (0, 1, or 2 bytes) and have no special response. static int tpm_simple_cmd(u8 locty, u32 ordinal - , int param_size, u16 param, enum tpmDurationType to_t) + , int param_size, u16 param, enum tpmDurationType to_t + , u32 *returnCode) { struct { struct tpm_req_header trqh; @@ -374,7 +375,8 @@ tpm_simple_cmd(u8 locty, u32 ordinal u32 obuffer_len = sizeof(obuffer); memset(obuffer, 0x0, sizeof(obuffer));
- int ret = tpmhw_transmit(locty, &req.trqh, obuffer, &obuffer_len, to_t); + int ret = tpmhw_transmit(locty, &req.trqh, obuffer, &obuffer_len, to_t, + returnCode); ret = ret ? -1 : be32_to_cpu(trsh->errcode); dprintf(DEBUG_tcg, "Return from tpm_simple_cmd(%x, %x) = %x\n", ordinal, param, ret); @@ -396,7 +398,7 @@ tpm20_getcapability(u32 capability, u32 property, u32 count,
u32 resp_size = rsize; int ret = tpmhw_transmit(0, &trg.hdr, rsp, &resp_size, - TPM_DURATION_TYPE_SHORT); + TPM_DURATION_TYPE_SHORT, NULL); ret = (ret || rsize < be32_to_cpu(rsp->totlen)) ? -1 : be32_to_cpu(rsp->errcode);
@@ -433,7 +435,8 @@ tpm20_get_pcrbanks(void) }
static int -tpm12_get_capability(u32 cap, u32 subcap, struct tpm_rsp_header *rsp, u32 rsize) +tpm12_get_capability(u32 cap, u32 subcap, struct tpm_rsp_header *rsp, u32 rsize, + u32 *returnCode) { struct tpm_req_getcap trgc = { .hdr.tag = cpu_to_be16(TPM_TAG_RQU_CMD), @@ -445,7 +448,7 @@ tpm12_get_capability(u32 cap, u32 subcap, struct tpm_rsp_header *rsp, u32 rsize) }; u32 resp_size = rsize; int ret = tpmhw_transmit(0, &trgc.hdr, rsp, &resp_size, - TPM_DURATION_TYPE_SHORT); + TPM_DURATION_TYPE_SHORT, returnCode); ret = (ret || resp_size != rsize) ? -1 : be32_to_cpu(rsp->errcode); dprintf(DEBUG_tcg, "TCGBIOS: Return code from TPM_GetCapability(%d, %d)" " = %x\n", cap, subcap, ret); @@ -453,13 +456,13 @@ tpm12_get_capability(u32 cap, u32 subcap, struct tpm_rsp_header *rsp, u32 rsize) }
static int -tpm12_read_permanent_flags(char *buf, int buf_len) +tpm12_read_permanent_flags(char *buf, int buf_len, u32 *returnCode) { memset(buf, 0, buf_len);
struct tpm_res_getcap_perm_flags pf; int ret = tpm12_get_capability(TPM_CAP_FLAG, TPM_CAP_FLAG_PERMANENT - , &pf.hdr, sizeof(pf)); + , &pf.hdr, sizeof(pf), returnCode); if (ret) return -1;
@@ -473,13 +476,13 @@ tpm12_determine_timeouts(void) { struct tpm_res_getcap_timeouts timeouts; int ret = tpm12_get_capability(TPM_CAP_PROPERTY, TPM_CAP_PROP_TIS_TIMEOUT - , &timeouts.hdr, sizeof(timeouts)); + , &timeouts.hdr, sizeof(timeouts), NULL); if (ret) return ret;
struct tpm_res_getcap_durations durations; ret = tpm12_get_capability(TPM_CAP_PROPERTY, TPM_CAP_PROP_DURATION - , &durations.hdr, sizeof(durations)); + , &durations.hdr, sizeof(durations), NULL); if (ret) return ret;
@@ -538,7 +541,7 @@ tpm12_extend(struct tpm_log_entry *le, int digest_len) struct tpm_rsp_extend rsp; u32 resp_length = sizeof(rsp); int ret = tpmhw_transmit(0, &tre.hdr, &rsp, &resp_length, - TPM_DURATION_TYPE_SHORT); + TPM_DURATION_TYPE_SHORT, NULL); if (ret || resp_length != sizeof(rsp) || rsp.hdr.errcode) return -1;
@@ -571,7 +574,7 @@ static int tpm20_extend(struct tpm_log_entry *le, int digest_len) struct tpm_rsp_header rsp; u32 resp_length = sizeof(rsp); int ret = tpmhw_transmit(0, &tre->hdr, &rsp, &resp_length, - TPM_DURATION_TYPE_SHORT); + TPM_DURATION_TYPE_SHORT, NULL); if (ret || resp_length != sizeof(rsp) || rsp.errcode) return -1;
@@ -606,7 +609,7 @@ tpm20_stirrandom(void) struct tpm_rsp_header rsp; u32 resp_length = sizeof(rsp); int ret = tpmhw_transmit(0, &stir.hdr, &rsp, &resp_length, - TPM_DURATION_TYPE_SHORT); + TPM_DURATION_TYPE_SHORT, NULL); if (ret || resp_length != sizeof(rsp) || rsp.errcode) ret = -1;
@@ -633,7 +636,7 @@ tpm20_getrandom(u8 *buf, u16 buf_len) u32 resp_length = sizeof(rsp);
int ret = tpmhw_transmit(0, &trgr.hdr, &rsp, &resp_length, - TPM_DURATION_TYPE_MEDIUM); + TPM_DURATION_TYPE_MEDIUM, NULL); if (ret || resp_length != sizeof(rsp) || rsp.hdr.errcode) ret = -1; else @@ -667,7 +670,7 @@ tpm20_hierarchycontrol(u32 hierarchy, u8 state) struct tpm_rsp_header rsp; u32 resp_length = sizeof(rsp); int ret = tpmhw_transmit(0, &trh.hdr, &rsp, &resp_length, - TPM_DURATION_TYPE_MEDIUM); + TPM_DURATION_TYPE_MEDIUM, NULL); if (ret || resp_length != sizeof(rsp) || rsp.errcode) ret = -1;
@@ -701,7 +704,7 @@ tpm20_hierarchychangeauth(u8 auth[20]) struct tpm_rsp_header rsp; u32 resp_length = sizeof(rsp); int ret = tpmhw_transmit(0, &trhca.hdr, &rsp, &resp_length, - TPM_DURATION_TYPE_MEDIUM); + TPM_DURATION_TYPE_MEDIUM, NULL); if (ret || resp_length != sizeof(rsp) || rsp.errcode) ret = -1;
@@ -736,7 +739,7 @@ tpm_set_failure(void) */
tpm_simple_cmd(0, TPM_ORD_SetTempDeactivated, - 0, 0, TPM_DURATION_TYPE_SHORT); + 0, 0, TPM_DURATION_TYPE_SHORT, NULL); break; case TPM_VERSION_2: tpm20_hierarchycontrol(TPM2_RH_ENDORSEMENT, TPM2_NO); @@ -836,12 +839,12 @@ static int tpm12_assert_physical_presence(void) { int ret = tpm_simple_cmd(0, TPM_ORD_PhysicalPresence, - 2, TPM_PP_PRESENT, TPM_DURATION_TYPE_SHORT); + 2, TPM_PP_PRESENT, TPM_DURATION_TYPE_SHORT, NULL); if (!ret) return 0;
struct tpm_permanent_flags pf; - ret = tpm12_read_permanent_flags((char *)&pf, sizeof(pf)); + ret = tpm12_read_permanent_flags((char *)&pf, sizeof(pf), NULL); if (ret) return -1;
@@ -854,10 +857,11 @@ tpm12_assert_physical_presence(void) if (!pf.flags[PERM_FLAG_IDX_PHYSICAL_PRESENCE_LIFETIME_LOCK] && !pf.flags[PERM_FLAG_IDX_PHYSICAL_PRESENCE_CMD_ENABLE]) { tpm_simple_cmd(0, TPM_ORD_PhysicalPresence, - 2, TPM_PP_CMD_ENABLE, TPM_DURATION_TYPE_SHORT); + 2, TPM_PP_CMD_ENABLE, TPM_DURATION_TYPE_SHORT, NULL);
return tpm_simple_cmd(0, TPM_ORD_PhysicalPresence, - 2, TPM_PP_PRESENT, TPM_DURATION_TYPE_SHORT); + 2, TPM_PP_PRESENT, TPM_DURATION_TYPE_SHORT, + NULL); } return -1; } @@ -867,7 +871,7 @@ tpm12_startup(void) { dprintf(DEBUG_tcg, "TCGBIOS: Starting with TPM_Startup(ST_CLEAR)\n"); int ret = tpm_simple_cmd(0, TPM_ORD_Startup, - 2, TPM_ST_CLEAR, TPM_DURATION_TYPE_SHORT); + 2, TPM_ST_CLEAR, TPM_DURATION_TYPE_SHORT, NULL); if (CONFIG_COREBOOT && ret == TPM_INVALID_POSTINIT) /* with other firmware on the system the TPM may already have been * initialized @@ -886,12 +890,12 @@ tpm12_startup(void) goto err_exit;
ret = tpm_simple_cmd(0, TPM_ORD_SelfTestFull, - 0, 0, TPM_DURATION_TYPE_LONG); + 0, 0, TPM_DURATION_TYPE_LONG, NULL); if (ret) goto err_exit;
ret = tpm_simple_cmd(3, TSC_ORD_ResetEstablishmentBit, - 0, 0, TPM_DURATION_TYPE_SHORT); + 0, 0, TPM_DURATION_TYPE_SHORT, NULL); if (ret && ret != TPM_BAD_LOCALITY) goto err_exit;
@@ -910,7 +914,7 @@ tpm20_startup(void) tpm20_set_timeouts();
int ret = tpm_simple_cmd(0, TPM2_CC_Startup, - 2, TPM2_SU_CLEAR, TPM_DURATION_TYPE_SHORT); + 2, TPM2_SU_CLEAR, TPM_DURATION_TYPE_SHORT, NULL);
dprintf(DEBUG_tcg, "TCGBIOS: Return value from sending TPM2_CC_Startup(SU_CLEAR) = 0x%08x\n", ret); @@ -925,7 +929,7 @@ tpm20_startup(void) goto err_exit;
ret = tpm_simple_cmd(0, TPM2_CC_SelfTest, - 1, TPM2_YES, TPM_DURATION_TYPE_LONG); + 1, TPM2_YES, TPM_DURATION_TYPE_LONG, NULL);
dprintf(DEBUG_tcg, "TCGBIOS: Return value from sending TPM2_CC_SelfTest = 0x%08x\n", ret); @@ -1030,7 +1034,8 @@ tpm_prepboot(void) case TPM_VERSION_1_2: if (TPM_has_physical_presence) tpm_simple_cmd(0, TPM_ORD_PhysicalPresence, - 2, TPM_PP_NOT_PRESENT_LOCK, TPM_DURATION_TYPE_SHORT); + 2, TPM_PP_NOT_PRESENT_LOCK, TPM_DURATION_TYPE_SHORT, + NULL); break; case TPM_VERSION_2: tpm20_prepboot(); @@ -1132,11 +1137,11 @@ tpm_s3_resume(void) switch (TPM_version) { case TPM_VERSION_1_2: ret = tpm_simple_cmd(0, TPM_ORD_Startup, - 2, TPM_ST_STATE, TPM_DURATION_TYPE_SHORT); + 2, TPM_ST_STATE, TPM_DURATION_TYPE_SHORT, NULL); break; case TPM_VERSION_2: ret = tpm_simple_cmd(0, TPM2_CC_Startup, - 2, TPM2_SU_STATE, TPM_DURATION_TYPE_SHORT); + 2, TPM2_SU_STATE, TPM_DURATION_TYPE_SHORT, NULL);
dprintf(DEBUG_tcg, "TCGBIOS: Return value from sending TPM2_CC_Startup(SU_STATE) = 0x%08x\n", ret); @@ -1146,7 +1151,7 @@ tpm_s3_resume(void)
ret = tpm_simple_cmd(0, TPM2_CC_SelfTest, - 1, TPM2_YES, TPM_DURATION_TYPE_LONG); + 1, TPM2_YES, TPM_DURATION_TYPE_LONG, NULL);
dprintf(DEBUG_tcg, "TCGBIOS: Return value from sending TPM2_CC_SelfTest() = 0x%08x\n", ret); @@ -1302,7 +1307,8 @@ pass_through_to_tpm_int(struct pttti *pttti, struct pttto *pttto)
u32 resbuflen = pttti->opblength - offsetof(struct pttto, tpmopout); int ret = tpmhw_transmit(0, trh, pttto->tpmopout, &resbuflen, - TPM_DURATION_TYPE_LONG /* worst case */); + TPM_DURATION_TYPE_LONG /* worst case */, + NULL); if (ret) { rc = TCG_FATAL_COM_ERROR; goto err_exit; @@ -1495,11 +1501,11 @@ tpm_interrupt_handler32(struct bregs *regs) typedef u8 tpm_ppi_code;
static int -tpm12_read_has_owner(int *has_owner) +tpm12_read_has_owner(int *has_owner, u32 *returnCode) { struct tpm_res_getcap_ownerauth oauth; int ret = tpm12_get_capability(TPM_CAP_PROPERTY, TPM_CAP_PROP_OWNER - , &oauth.hdr, sizeof(oauth)); + , &oauth.hdr, sizeof(oauth), returnCode); if (ret) return -1;
@@ -1509,10 +1515,10 @@ tpm12_read_has_owner(int *has_owner) }
static int -tpm12_enable_tpm(int enable, int verbose) +tpm12_enable_tpm(int enable, int verbose, u32 *returnCode) { struct tpm_permanent_flags pf; - int ret = tpm12_read_permanent_flags((char *)&pf, sizeof(pf)); + int ret = tpm12_read_permanent_flags((char *)&pf, sizeof(pf), returnCode); if (ret) return -1;
@@ -1521,7 +1527,8 @@ tpm12_enable_tpm(int enable, int verbose)
ret = tpm_simple_cmd(0, enable ? TPM_ORD_PhysicalEnable : TPM_ORD_PhysicalDisable, - 0, 0, TPM_DURATION_TYPE_SHORT); + 0, 0, TPM_DURATION_TYPE_SHORT, + returnCode); if (ret) { if (enable) dprintf(DEBUG_tcg, "TCGBIOS: Enabling the TPM failed.\n"); @@ -1532,10 +1539,10 @@ tpm12_enable_tpm(int enable, int verbose) }
static int -tpm12_activate_tpm(int activate, int allow_reset, int verbose) +tpm12_activate_tpm(int activate, int allow_reset, int verbose, u32 *returnCode) { struct tpm_permanent_flags pf; - int ret = tpm12_read_permanent_flags((char *)&pf, sizeof(pf)); + int ret = tpm12_read_permanent_flags((char *)&pf, sizeof(pf), returnCode); if (ret) return -1;
@@ -1546,7 +1553,8 @@ tpm12_activate_tpm(int activate, int allow_reset, int verbose) return 0;
ret = tpm_simple_cmd(0, TPM_ORD_PhysicalSetDeactivated, - 1, activate ? 0x00 : 0x01, TPM_DURATION_TYPE_SHORT); + 1, activate ? 0x00 : 0x01, TPM_DURATION_TYPE_SHORT, + returnCode); if (ret) return ret;
@@ -1563,21 +1571,21 @@ tpm12_activate_tpm(int activate, int allow_reset, int verbose) }
static int -tpm12_enable_activate(int allow_reset, int verbose) +tpm12_enable_activate(int allow_reset, int verbose, u32 *returnCode) { - int ret = tpm12_enable_tpm(1, verbose); + int ret = tpm12_enable_tpm(1, verbose, returnCode); if (ret) return ret;
- return tpm12_activate_tpm(1, allow_reset, verbose); + return tpm12_activate_tpm(1, allow_reset, verbose, returnCode); }
static int tpm12_force_clear(int enable_activate_before, int enable_activate_after, - int verbose) + int verbose, u32 *returnCode) { int has_owner; - int ret = tpm12_read_has_owner(&has_owner); + int ret = tpm12_read_has_owner(&has_owner, returnCode); if (ret) return -1; if (!has_owner) { @@ -1587,7 +1595,7 @@ tpm12_force_clear(int enable_activate_before, int enable_activate_after, }
if (enable_activate_before) { - ret = tpm12_enable_activate(0, verbose); + ret = tpm12_enable_activate(0, verbose, returnCode); if (ret) { dprintf(DEBUG_tcg, "TCGBIOS: Enabling/activating the TPM failed.\n"); @@ -1596,7 +1604,7 @@ tpm12_force_clear(int enable_activate_before, int enable_activate_after, }
ret = tpm_simple_cmd(0, TPM_ORD_ForceClear, - 0, 0, TPM_DURATION_TYPE_SHORT); + 0, 0, TPM_DURATION_TYPE_SHORT, returnCode); if (ret) return ret;
@@ -1607,14 +1615,14 @@ tpm12_force_clear(int enable_activate_before, int enable_activate_after, return 0; }
- return tpm12_enable_activate(1, verbose); + return tpm12_enable_activate(1, verbose, returnCode); }
static int -tpm12_set_owner_install(int allow, int verbose) +tpm12_set_owner_install(int allow, int verbose, u32 *returnCode) { int has_owner; - int ret = tpm12_read_has_owner(&has_owner); + int ret = tpm12_read_has_owner(&has_owner, returnCode); if (ret) return -1; if (has_owner) { @@ -1624,7 +1632,7 @@ tpm12_set_owner_install(int allow, int verbose) }
struct tpm_permanent_flags pf; - ret = tpm12_read_permanent_flags((char *)&pf, sizeof(pf)); + ret = tpm12_read_permanent_flags((char *)&pf, sizeof(pf), returnCode); if (ret) return -1;
@@ -1635,7 +1643,8 @@ tpm12_set_owner_install(int allow, int verbose) }
ret = tpm_simple_cmd(0, TPM_ORD_SetOwnerInstall, - 1, allow ? 0x01 : 0x00, TPM_DURATION_TYPE_SHORT); + 1, allow ? 0x01 : 0x00, TPM_DURATION_TYPE_SHORT, + returnCode); if (ret) return ret;
@@ -1646,7 +1655,7 @@ tpm12_set_owner_install(int allow, int verbose) }
static int -tpm12_process_cfg(tpm_ppi_code msgCode, int verbose) +tpm12_process_cfg(tpm_ppi_code msgCode, int verbose, u32 *returnCode) { int ret = 0;
@@ -1655,31 +1664,31 @@ tpm12_process_cfg(tpm_ppi_code msgCode, int verbose) break;
case TPM_PPI_OP_ENABLE: - ret = tpm12_enable_tpm(1, verbose); + ret = tpm12_enable_tpm(1, verbose, returnCode); break;
case TPM_PPI_OP_DISABLE: - ret = tpm12_enable_tpm(0, verbose); + ret = tpm12_enable_tpm(0, verbose, returnCode); break;
case TPM_PPI_OP_ACTIVATE: - ret = tpm12_activate_tpm(1, 1, verbose); + ret = tpm12_activate_tpm(1, 1, verbose, returnCode); break;
case TPM_PPI_OP_DEACTIVATE: - ret = tpm12_activate_tpm(0, 1, verbose); + ret = tpm12_activate_tpm(0, 1, verbose, returnCode); break;
case TPM_PPI_OP_CLEAR: - ret = tpm12_force_clear(1, 0, verbose); + ret = tpm12_force_clear(1, 0, verbose, returnCode); break;
case TPM_PPI_OP_SET_OWNERINSTALL_TRUE: - ret = tpm12_set_owner_install(1, verbose); + ret = tpm12_set_owner_install(1, verbose, returnCode); break;
case TPM_PPI_OP_SET_OWNERINSTALL_FALSE: - ret = tpm12_set_owner_install(0, verbose); + ret = tpm12_set_owner_install(0, verbose, returnCode); break;
default: @@ -1693,7 +1702,7 @@ tpm12_process_cfg(tpm_ppi_code msgCode, int verbose) }
static int -tpm20_clearcontrol(u8 disable, int verbose) +tpm20_clearcontrol(u8 disable, int verbose, u32 *returnCode) { struct tpm2_req_clearcontrol trc = { .hdr.tag = cpu_to_be16(TPM2_ST_SESSIONS), @@ -1712,7 +1721,7 @@ tpm20_clearcontrol(u8 disable, int verbose) struct tpm_rsp_header rsp; u32 resp_length = sizeof(rsp); int ret = tpmhw_transmit(0, &trc.hdr, &rsp, &resp_length, - TPM_DURATION_TYPE_SHORT); + TPM_DURATION_TYPE_SHORT, returnCode); if (ret || resp_length != sizeof(rsp) || rsp.errcode) ret = -1;
@@ -1723,7 +1732,7 @@ tpm20_clearcontrol(u8 disable, int verbose) }
static int -tpm20_clear(void) +tpm20_clear(u32 *returnCode) { struct tpm2_req_clear trq = { .hdr.tag = cpu_to_be16(TPM2_ST_SESSIONS), @@ -1741,7 +1750,7 @@ tpm20_clear(void) struct tpm_rsp_header rsp; u32 resp_length = sizeof(rsp); int ret = tpmhw_transmit(0, &trq.hdr, &rsp, &resp_length, - TPM_DURATION_TYPE_MEDIUM); + TPM_DURATION_TYPE_MEDIUM, returnCode); if (ret || resp_length != sizeof(rsp) || rsp.errcode) ret = -1;
@@ -1752,7 +1761,7 @@ tpm20_clear(void) }
static int -tpm20_process_cfg(tpm_ppi_code msgCode, int verbose) +tpm20_process_cfg(tpm_ppi_code msgCode, int verbose, u32 *returnCode) { int ret = 0;
@@ -1761,9 +1770,9 @@ tpm20_process_cfg(tpm_ppi_code msgCode, int verbose) break;
case TPM_PPI_OP_CLEAR: - ret = tpm20_clearcontrol(0, verbose); + ret = tpm20_clearcontrol(0, verbose, returnCode); if (!ret) - ret = tpm20_clear(); + ret = tpm20_clear(returnCode); break; }
@@ -1780,8 +1789,8 @@ tpm12_get_tpm_state(void) struct tpm_permanent_flags pf; int has_owner;
- if (tpm12_read_permanent_flags((char *)&pf, sizeof(pf)) || - tpm12_read_has_owner(&has_owner)) + if (tpm12_read_permanent_flags((char *)&pf, sizeof(pf), NULL) || + tpm12_read_has_owner(&has_owner, NULL)) return ~0;
if (!pf.flags[PERM_FLAG_IDX_DISABLE]) @@ -1938,7 +1947,7 @@ tpm12_menu(void) break;
if (next_scancodes[i] == scancode) { - tpm12_process_cfg(msgCode, 1); + tpm12_process_cfg(msgCode, 1, NULL); waitkey = 0; break; } @@ -1977,7 +1986,7 @@ tpm20_menu(void) continue; }
- tpm20_process_cfg(msgCode, 0); + tpm20_process_cfg(msgCode, 0, NULL); } }
On Tue, Jan 16, 2018 at 11:41:01AM -0500, Stefan Berger wrote:
Pass the returnCode parameter through many of the functions so that we can get the TPM return code from hwtpm_transmit, if needed.
Signed-off-by: Stefan Berger stefanb@linux.vnet.ibm.com
src/hw/tpm_drivers.c | 8 ++- src/hw/tpm_drivers.h | 2 +- src/tcgbios.c | 147 +++++++++++++++++++++++++++------------------------ 3 files changed, 86 insertions(+), 71 deletions(-)
diff --git a/src/hw/tpm_drivers.c b/src/hw/tpm_drivers.c index a137e62..25f4f36 100644 --- a/src/hw/tpm_drivers.c +++ b/src/hw/tpm_drivers.c @@ -377,7 +377,7 @@ tpmhw_is_present(void) int tpmhw_transmit(u8 locty, struct tpm_req_header *req, void *respbuffer, u32 *respbufferlen,
enum tpmDurationType to_t)
enum tpmDurationType to_t, u32 *returnCode)
{ if (TPMHW_driver_to_use == TPM_INVALID_DRIVER) return -1; @@ -408,6 +408,12 @@ tpmhw_transmit(u8 locty, struct tpm_req_header *req,
td->ready();
- if (returnCode && *respbufferlen >= sizeof(struct tpm_rsp_header)) {
struct tpm_rsp_header *r = respbuffer;
*returnCode = be32_to_cpu(r->errcode);
- }
- return 0;
}
[...]
diff --git a/src/tcgbios.c b/src/tcgbios.c index 40b3028..730b5e7 100644 --- a/src/tcgbios.c +++ b/src/tcgbios.c @@ -350,7 +350,8 @@ tpm_build_digest(struct tpm_log_entry *le, const u8 *sha1, int bigEndian) // optional parameter (0, 1, or 2 bytes) and have no special response. static int tpm_simple_cmd(u8 locty, u32 ordinal
, int param_size, u16 param, enum tpmDurationType to_t)
, int param_size, u16 param, enum tpmDurationType to_t
, u32 *returnCode)
{ struct { struct tpm_req_header trqh; @@ -374,7 +375,8 @@ tpm_simple_cmd(u8 locty, u32 ordinal u32 obuffer_len = sizeof(obuffer); memset(obuffer, 0x0, sizeof(obuffer));
- int ret = tpmhw_transmit(locty, &req.trqh, obuffer, &obuffer_len, to_t);
- int ret = tpmhw_transmit(locty, &req.trqh, obuffer, &obuffer_len, to_t,
ret = ret ? -1 : be32_to_cpu(trsh->errcode);returnCode);
I'm not sure why this patch is necessary. The callers of tpmhw_transmit() can already extract the errcode. Indeed, tpm_simple_cmd() does just that and returns it to its callers via its return code (see the last line of code above).
-Kevin
On 01/16/2018 01:16 PM, Kevin O'Connor wrote:
On Tue, Jan 16, 2018 at 11:41:01AM -0500, Stefan Berger wrote:
Pass the returnCode parameter through many of the functions so that we can get the TPM return code from hwtpm_transmit, if needed.
Signed-off-by: Stefan Berger stefanb@linux.vnet.ibm.com
src/hw/tpm_drivers.c | 8 ++- src/hw/tpm_drivers.h | 2 +- src/tcgbios.c | 147 +++++++++++++++++++++++++++------------------------ 3 files changed, 86 insertions(+), 71 deletions(-)
diff --git a/src/hw/tpm_drivers.c b/src/hw/tpm_drivers.c index a137e62..25f4f36 100644 --- a/src/hw/tpm_drivers.c +++ b/src/hw/tpm_drivers.c @@ -377,7 +377,7 @@ tpmhw_is_present(void) int tpmhw_transmit(u8 locty, struct tpm_req_header *req, void *respbuffer, u32 *respbufferlen,
enum tpmDurationType to_t)
{ if (TPMHW_driver_to_use == TPM_INVALID_DRIVER) return -1;enum tpmDurationType to_t, u32 *returnCode)
@@ -408,6 +408,12 @@ tpmhw_transmit(u8 locty, struct tpm_req_header *req,
td->ready();
- if (returnCode && *respbufferlen >= sizeof(struct tpm_rsp_header)) {
struct tpm_rsp_header *r = respbuffer;
*returnCode = be32_to_cpu(r->errcode);
- }
}return 0;
[...]
diff --git a/src/tcgbios.c b/src/tcgbios.c index 40b3028..730b5e7 100644 --- a/src/tcgbios.c +++ b/src/tcgbios.c @@ -350,7 +350,8 @@ tpm_build_digest(struct tpm_log_entry *le, const u8 *sha1, int bigEndian) // optional parameter (0, 1, or 2 bytes) and have no special response. static int tpm_simple_cmd(u8 locty, u32 ordinal
, int param_size, u16 param, enum tpmDurationType to_t)
, int param_size, u16 param, enum tpmDurationType to_t
{ struct { struct tpm_req_header trqh;, u32 *returnCode)
@@ -374,7 +375,8 @@ tpm_simple_cmd(u8 locty, u32 ordinal u32 obuffer_len = sizeof(obuffer); memset(obuffer, 0x0, sizeof(obuffer));
- int ret = tpmhw_transmit(locty, &req.trqh, obuffer, &obuffer_len, to_t);
- int ret = tpmhw_transmit(locty, &req.trqh, obuffer, &obuffer_len, to_t,
returnCode); ret = ret ? -1 : be32_to_cpu(trsh->errcode);
I'm not sure why this patch is necessary. The callers of tpmhw_transmit() can already extract the errcode. Indeed, tpm_simple_cmd() does just that and returns it to its callers via its return code (see the last line of code above).
We sometimes have code like this:
ret = tpm12... if (ret) return -1;
This just maps a potential TPM error code to -1.
I guess the right thing to do is to avoid this mapping.
Stefan
-Kevin
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.
The underlying spec can be accessed from this page here:
https://trustedcomputinggroup.org/tcg-physical-presence-interface-specificat...
Version 1.30 is implemented.
Signed-off-by: Stefan Berger stefanb@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(); + // Allow user to modify overall boot order. interactive_bootmenu(); wait_threads(); diff --git a/src/std/acpi.h b/src/std/acpi.h index c01fa7b..5f2e8b7 100644 --- 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; + #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); + #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; + } + 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 */
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-specificat...
Version 1.30 is implemented.
Signed-off-by: Stefan Berger stefanb@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
On 01/16/18 19:36, Kevin O'Connor wrote:
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-specificat...
Version 1.30 is implemented.
Signed-off-by: Stefan Berger stefanb@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(+)
[...]
--- 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).
I agree; if the firmware is supposed to consume information from QEMU for locating the register block of this platform device, please expose the address in a new fw_cfg file.
Thanks! Laszlo
On 01/16/2018 01:36 PM, Kevin O'Connor wrote:
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-specificat...
Version 1.30 is implemented.
Signed-off-by: Stefan Berger stefanb@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.
Done.
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.
Move the definition of tpm_ppi struct to tcgbios.c. It's shared between ACPI and firmware.
- #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?
QEMU merely creates the device and the ACPI code. SeaBIOS implements which codes the user can send. The above array inside the virtual device contains flags that describe the codes that the user can send. If OVMF was to implement less codes, less bytes would be set. If another firmware implemented the possibility to prevent the user from sending certain codes, the TPM_PPI_FUNC_BLOCKED flag could be set for example so that ACPI would refuse to set the code for the next reboot. It's parametrizing the interface between ACPI and firmware.
Stefan
-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
On Tue, Jan 16, 2018 at 05:01:51PM -0500, Stefan Berger wrote:
On 01/16/2018 01:36 PM, Kevin O'Connor wrote:
On Tue, Jan 16, 2018 at 11:41:02AM -0500, Stefan Berger wrote:
- 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?
QEMU merely creates the device and the ACPI code. SeaBIOS implements which codes the user can send. The above array inside the virtual device contains flags that describe the codes that the user can send. If OVMF was to implement less codes, less bytes would be set. If another firmware implemented the possibility to prevent the user from sending certain codes, the TPM_PPI_FUNC_BLOCKED flag could be set for example so that ACPI would refuse to set the code for the next reboot. It's parametrizing the interface between ACPI and firmware.
Is this the right approach? If QEMU has all the code to emulate the TPM hardware and QEMU has all the code to build the ACPI tables describing that hardware - why doesn't it just implement the PPI functions itself on a reboot? I'll defer to Laszlo, Igor, Michael, etc, but this seems like a complex firmware<->qemu interface and I'm not sure I understand the gain.
The above aside, if there's a need to communicate firmware capabilities to QEMU (so that QEMU can build the ACPI tables describing that capability), can we reuse the writable fw_cfg file system to transfer that information (instead of having the firmware fill an area of memory)?
-Kevin
On 01/16/2018 05:35 PM, Kevin O'Connor wrote:
On Tue, Jan 16, 2018 at 05:01:51PM -0500, Stefan Berger wrote:
On 01/16/2018 01:36 PM, Kevin O'Connor wrote:
On Tue, Jan 16, 2018 at 11:41:02AM -0500, Stefan Berger wrote:
- 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?
QEMU merely creates the device and the ACPI code. SeaBIOS implements which codes the user can send. The above array inside the virtual device contains flags that describe the codes that the user can send. If OVMF was to implement less codes, less bytes would be set. If another firmware implemented the possibility to prevent the user from sending certain codes, the TPM_PPI_FUNC_BLOCKED flag could be set for example so that ACPI would refuse to set the code for the next reboot. It's parametrizing the interface between ACPI and firmware.
Is this the right approach? If QEMU has all the code to emulate the TPM hardware and QEMU has all the code to build the ACPI tables describing that hardware - why doesn't it just implement the PPI functions itself on a reboot? I'll defer to Laszlo, Igor, Michael,
QEMU does implement the ACPI code. The firmware may implement a subset of what the specs are describing. The array of flags helps to decouple the ACPI code and the firmware and allows OVMF and SeaBIOS and possibly others to only implement a subset of the functionality.
etc, but this seems like a complex firmware<->qemu interface and I'm not sure I understand the gain. The above aside, if there's a need to communicate firmware capabilities to QEMU (so that QEMU can build the ACPI tables describing that capability), can we reuse the writable fw_cfg file system to transfer that information (instead of having the firmware fill an area of memory)?
Its SeaBIOS vor OVMF reporting to ACPI what it implements. QEMU doesn't know what the firmware implements.
Stefan
-Kevin
Implement more functions of the TPM Physical Presence interface. Some of the added functions will automatically reboot the machine. Thus we need to save the next step after the reboot in an additional variable.
Signed-off-by: Stefan Berger stefanb@linux.vnet.ibm.com --- src/std/tcg.h | 7 ++++++ src/tcgbios.c | 69 ++++++++++++++++++++++++++++++++++++++++++++++++++++++----- 2 files changed, 71 insertions(+), 5 deletions(-)
diff --git a/src/std/tcg.h b/src/std/tcg.h index 22353a9..aeee689 100644 --- a/src/std/tcg.h +++ b/src/std/tcg.h @@ -548,8 +548,15 @@ struct pcctes_romex #define TPM_PPI_OP_ACTIVATE 3 #define TPM_PPI_OP_DEACTIVATE 4 #define TPM_PPI_OP_CLEAR 5 +#define TPM_PPI_OP_ENABLE_ACTIVATE 6 +#define TPM_PPI_OP_DEACTIVATE_DISABLE 7 #define TPM_PPI_OP_SET_OWNERINSTALL_TRUE 8 #define TPM_PPI_OP_SET_OWNERINSTALL_FALSE 9 +#define TPM_PPI_OP_ENABLE_ACTIVATE_SET_OWNERINSTALL_TRUE 10 +#define TPM_PPI_OP_SET_OWNERINSTALL_FALSE_DEACTIVATE_DISABLE 11 +#define TPM_PPI_OP_CLEAR_ENABLE_ACTIVATE 14 +#define TPM_PPI_OP_ENABLE_ACTIVATE_CLEAR 21 +#define TPM_PPI_OP_ENABLE_ACTIVATE_CLEAR_ENABLE_ACTIVATE 22
struct tpm_ppi { u8 ppin; /* 0: 1 = initialized */ diff --git a/src/tcgbios.c b/src/tcgbios.c index c8e6ca2..e074d42 100644 --- a/src/tcgbios.c +++ b/src/tcgbios.c @@ -1655,7 +1655,8 @@ tpm12_set_owner_install(int allow, int verbose, u32 *returnCode) }
static int -tpm12_process_cfg(tpm_ppi_code msgCode, int verbose, u32 *returnCode) +tpm12_process_cfg(tpm_ppi_code msgCode, int verbose, u32 *returnCode, + u8 *nextStep) { int ret = 0;
@@ -1683,6 +1684,18 @@ tpm12_process_cfg(tpm_ppi_code msgCode, int verbose, u32 *returnCode) ret = tpm12_force_clear(1, 0, verbose, returnCode); break;
+ case TPM_PPI_OP_ENABLE_ACTIVATE: + ret = tpm12_enable_tpm(1, verbose, returnCode); + if (!ret) + ret = tpm12_activate_tpm(1, 1, verbose, returnCode); + break; + + case TPM_PPI_OP_DEACTIVATE_DISABLE: + ret = tpm12_activate_tpm(0, 1, verbose, returnCode); + if (!ret) + ret = tpm12_enable_tpm(0, verbose, returnCode); + break; + case TPM_PPI_OP_SET_OWNERINSTALL_TRUE: ret = tpm12_set_owner_install(1, verbose, returnCode); break; @@ -1691,6 +1704,43 @@ tpm12_process_cfg(tpm_ppi_code msgCode, int verbose, u32 *returnCode) ret = tpm12_set_owner_install(0, verbose, returnCode); break;
+ case TPM_PPI_OP_ENABLE_ACTIVATE_SET_OWNERINSTALL_TRUE: + *nextStep = TPM_PPI_OP_SET_OWNERINSTALL_TRUE; + ret = tpm12_enable_activate(1, verbose, returnCode); + if (!ret) + ret = tpm12_set_owner_install(1, verbose, returnCode); + break; + + case TPM_PPI_OP_SET_OWNERINSTALL_FALSE_DEACTIVATE_DISABLE: + ret = tpm12_set_owner_install(0, verbose, returnCode); + if (!ret) + ret = tpm12_activate_tpm(0, 0, verbose, returnCode); + if (!ret) + ret = tpm12_enable_tpm(0, verbose, returnCode); + break; + + case TPM_PPI_OP_CLEAR_ENABLE_ACTIVATE: + ret = tpm12_force_clear(0, 1, verbose, returnCode); + break; + + case TPM_PPI_OP_ENABLE_ACTIVATE_CLEAR: + *nextStep = TPM_PPI_OP_CLEAR; + ret = tpm12_enable_activate(1, verbose, returnCode); + /* no reboot happened */ + if (!ret) + ret = tpm12_force_clear(0, 0, verbose, returnCode); + break; + + case TPM_PPI_OP_ENABLE_ACTIVATE_CLEAR_ENABLE_ACTIVATE: + *nextStep = TPM_PPI_OP_CLEAR_ENABLE_ACTIVATE; + ret = tpm12_enable_activate(1, verbose, returnCode); + /* no reboot happened */ + if (!ret) { + *nextStep = TPM_PPI_OP_NOOP; + ret = tpm12_force_clear(0, 1, verbose, returnCode); + } + break; + default: break; } @@ -1783,11 +1833,12 @@ tpm20_process_cfg(tpm_ppi_code msgCode, int verbose, u32 *returnCode) }
static int -tpm_process_cfg(tpm_ppi_code msgCode, int verbose, u32 *returnCode) +tpm_process_cfg(tpm_ppi_code msgCode, int verbose, u32 *returnCode, + u8 *nextStep) { switch (TPM_version) { case TPM_VERSION_1_2: - return tpm12_process_cfg(msgCode, verbose, returnCode); + return tpm12_process_cfg(msgCode, verbose, returnCode, nextStep); case TPM_VERSION_2: return tpm20_process_cfg(msgCode, verbose, returnCode); } @@ -1959,7 +2010,8 @@ tpm12_menu(void) break;
if (next_scancodes[i] == scancode) { - tpm12_process_cfg(msgCode, 1, NULL); + u8 ignore; + tpm12_process_cfg(msgCode, 1, NULL, &ignore); waitkey = 0; break; } @@ -2049,8 +2101,15 @@ static const u8 tpm12_ppi_funcs[] = { [TPM_PPI_OP_ACTIVATE] = FLAGS, [TPM_PPI_OP_DEACTIVATE] = FLAGS, [TPM_PPI_OP_CLEAR] = FLAGS, + [TPM_PPI_OP_ENABLE_ACTIVATE] = FLAGS, + [TPM_PPI_OP_DEACTIVATE_DISABLE] = FLAGS, [TPM_PPI_OP_SET_OWNERINSTALL_TRUE] = FLAGS, [TPM_PPI_OP_SET_OWNERINSTALL_FALSE] = FLAGS, + [TPM_PPI_OP_ENABLE_ACTIVATE_SET_OWNERINSTALL_TRUE] = FLAGS, + [TPM_PPI_OP_SET_OWNERINSTALL_FALSE_DEACTIVATE_DISABLE] = FLAGS, + [TPM_PPI_OP_CLEAR_ENABLE_ACTIVATE] = FLAGS, + [TPM_PPI_OP_ENABLE_ACTIVATE_CLEAR] = FLAGS, + [TPM_PPI_OP_ENABLE_ACTIVATE_CLEAR_ENABLE_ACTIVATE] = FLAGS, };
static const u8 tpm2_ppi_funcs[] = { @@ -2116,7 +2175,7 @@ tpm_ppi_process(void) tp->pprq = 0;
printf("Processing TPM PPI opcode %d\n", op); - tpm_process_cfg(op, 0, &tp->pprp); + tpm_process_cfg(op, 0, &tp->pprp, &nextStep); } } }
On Tue, Jan 16, 2018 at 11:41:03AM -0500, Stefan Berger wrote:
Implement more functions of the TPM Physical Presence interface. Some of the added functions will automatically reboot the machine. Thus we need to save the next step after the reboot in an additional variable.
Signed-off-by: Stefan Berger stefanb@linux.vnet.ibm.com
src/std/tcg.h | 7 ++++++ src/tcgbios.c | 69 ++++++++++++++++++++++++++++++++++++++++++++++++++++++----- 2 files changed, 71 insertions(+), 5 deletions(-)
diff --git a/src/std/tcg.h b/src/std/tcg.h index 22353a9..aeee689 100644 --- a/src/std/tcg.h +++ b/src/std/tcg.h @@ -548,8 +548,15 @@ struct pcctes_romex #define TPM_PPI_OP_ACTIVATE 3 #define TPM_PPI_OP_DEACTIVATE 4 #define TPM_PPI_OP_CLEAR 5 +#define TPM_PPI_OP_ENABLE_ACTIVATE 6 +#define TPM_PPI_OP_DEACTIVATE_DISABLE 7 #define TPM_PPI_OP_SET_OWNERINSTALL_TRUE 8 #define TPM_PPI_OP_SET_OWNERINSTALL_FALSE 9 +#define TPM_PPI_OP_ENABLE_ACTIVATE_SET_OWNERINSTALL_TRUE 10 +#define TPM_PPI_OP_SET_OWNERINSTALL_FALSE_DEACTIVATE_DISABLE 11 +#define TPM_PPI_OP_CLEAR_ENABLE_ACTIVATE 14 +#define TPM_PPI_OP_ENABLE_ACTIVATE_CLEAR 21 +#define TPM_PPI_OP_ENABLE_ACTIVATE_CLEAR_ENABLE_ACTIVATE 22
struct tpm_ppi { u8 ppin; /* 0: 1 = initialized */ diff --git a/src/tcgbios.c b/src/tcgbios.c index c8e6ca2..e074d42 100644 --- a/src/tcgbios.c +++ b/src/tcgbios.c @@ -1655,7 +1655,8 @@ tpm12_set_owner_install(int allow, int verbose, u32 *returnCode) }
static int -tpm12_process_cfg(tpm_ppi_code msgCode, int verbose, u32 *returnCode) +tpm12_process_cfg(tpm_ppi_code msgCode, int verbose, u32 *returnCode,
u8 *nextStep)
{ int ret = 0;
@@ -1683,6 +1684,18 @@ tpm12_process_cfg(tpm_ppi_code msgCode, int verbose, u32 *returnCode) ret = tpm12_force_clear(1, 0, verbose, returnCode); break;
case TPM_PPI_OP_ENABLE_ACTIVATE:
ret = tpm12_enable_tpm(1, verbose, returnCode);
if (!ret)
ret = tpm12_activate_tpm(1, 1, verbose, returnCode);
break;
case TPM_PPI_OP_DEACTIVATE_DISABLE:
ret = tpm12_activate_tpm(0, 1, verbose, returnCode);
if (!ret)
ret = tpm12_enable_tpm(0, verbose, returnCode);
break;
case TPM_PPI_OP_SET_OWNERINSTALL_TRUE: ret = tpm12_set_owner_install(1, verbose, returnCode); break;
@@ -1691,6 +1704,43 @@ tpm12_process_cfg(tpm_ppi_code msgCode, int verbose, u32 *returnCode) ret = tpm12_set_owner_install(0, verbose, returnCode); break;
case TPM_PPI_OP_ENABLE_ACTIVATE_SET_OWNERINSTALL_TRUE:
*nextStep = TPM_PPI_OP_SET_OWNERINSTALL_TRUE;
ret = tpm12_enable_activate(1, verbose, returnCode);
if (!ret)
ret = tpm12_set_owner_install(1, verbose, returnCode);
break;
case TPM_PPI_OP_SET_OWNERINSTALL_FALSE_DEACTIVATE_DISABLE:
ret = tpm12_set_owner_install(0, verbose, returnCode);
if (!ret)
ret = tpm12_activate_tpm(0, 0, verbose, returnCode);
if (!ret)
ret = tpm12_enable_tpm(0, verbose, returnCode);
break;
case TPM_PPI_OP_CLEAR_ENABLE_ACTIVATE:
ret = tpm12_force_clear(0, 1, verbose, returnCode);
break;
case TPM_PPI_OP_ENABLE_ACTIVATE_CLEAR:
*nextStep = TPM_PPI_OP_CLEAR;
ret = tpm12_enable_activate(1, verbose, returnCode);
/* no reboot happened */
if (!ret)
ret = tpm12_force_clear(0, 0, verbose, returnCode);
break;
case TPM_PPI_OP_ENABLE_ACTIVATE_CLEAR_ENABLE_ACTIVATE:
*nextStep = TPM_PPI_OP_CLEAR_ENABLE_ACTIVATE;
I'm strugging to understand "nextStep". I think it would be clearer if the code could do a switch on the request and then execute all the details of that request without the need to set flags indicating the function should be rerun. tpm12_process_cfg() can call itself recursively if it needs to.
-Kevin
On 01/16/2018 01:58 PM, Kevin O'Connor wrote:
On Tue, Jan 16, 2018 at 11:41:03AM -0500, Stefan Berger wrote:
Implement more functions of the TPM Physical Presence interface. Some of the added functions will automatically reboot the machine. Thus we need to save the next step after the reboot in an additional variable.
Signed-off-by: Stefan Berger stefanb@linux.vnet.ibm.com
src/std/tcg.h | 7 ++++++ src/tcgbios.c | 69 ++++++++++++++++++++++++++++++++++++++++++++++++++++++----- 2 files changed, 71 insertions(+), 5 deletions(-)
diff --git a/src/std/tcg.h b/src/std/tcg.h index 22353a9..aeee689 100644 --- a/src/std/tcg.h +++ b/src/std/tcg.h @@ -548,8 +548,15 @@ struct pcctes_romex #define TPM_PPI_OP_ACTIVATE 3 #define TPM_PPI_OP_DEACTIVATE 4 #define TPM_PPI_OP_CLEAR 5 +#define TPM_PPI_OP_ENABLE_ACTIVATE 6 +#define TPM_PPI_OP_DEACTIVATE_DISABLE 7 #define TPM_PPI_OP_SET_OWNERINSTALL_TRUE 8 #define TPM_PPI_OP_SET_OWNERINSTALL_FALSE 9 +#define TPM_PPI_OP_ENABLE_ACTIVATE_SET_OWNERINSTALL_TRUE 10 +#define TPM_PPI_OP_SET_OWNERINSTALL_FALSE_DEACTIVATE_DISABLE 11 +#define TPM_PPI_OP_CLEAR_ENABLE_ACTIVATE 14 +#define TPM_PPI_OP_ENABLE_ACTIVATE_CLEAR 21 +#define TPM_PPI_OP_ENABLE_ACTIVATE_CLEAR_ENABLE_ACTIVATE 22
struct tpm_ppi { u8 ppin; /* 0: 1 = initialized */ diff --git a/src/tcgbios.c b/src/tcgbios.c index c8e6ca2..e074d42 100644 --- a/src/tcgbios.c +++ b/src/tcgbios.c @@ -1655,7 +1655,8 @@ tpm12_set_owner_install(int allow, int verbose, u32 *returnCode) }
static int -tpm12_process_cfg(tpm_ppi_code msgCode, int verbose, u32 *returnCode) +tpm12_process_cfg(tpm_ppi_code msgCode, int verbose, u32 *returnCode,
{ int ret = 0;u8 *nextStep)
@@ -1683,6 +1684,18 @@ tpm12_process_cfg(tpm_ppi_code msgCode, int verbose, u32 *returnCode) ret = tpm12_force_clear(1, 0, verbose, returnCode); break;
case TPM_PPI_OP_ENABLE_ACTIVATE:
ret = tpm12_enable_tpm(1, verbose, returnCode);
if (!ret)
ret = tpm12_activate_tpm(1, 1, verbose, returnCode);
break;
case TPM_PPI_OP_DEACTIVATE_DISABLE:
ret = tpm12_activate_tpm(0, 1, verbose, returnCode);
if (!ret)
ret = tpm12_enable_tpm(0, verbose, returnCode);
break;
case TPM_PPI_OP_SET_OWNERINSTALL_TRUE: ret = tpm12_set_owner_install(1, verbose, returnCode); break;
@@ -1691,6 +1704,43 @@ tpm12_process_cfg(tpm_ppi_code msgCode, int verbose, u32 *returnCode) ret = tpm12_set_owner_install(0, verbose, returnCode); break;
case TPM_PPI_OP_ENABLE_ACTIVATE_SET_OWNERINSTALL_TRUE:
*nextStep = TPM_PPI_OP_SET_OWNERINSTALL_TRUE;
ret = tpm12_enable_activate(1, verbose, returnCode);
if (!ret)
ret = tpm12_set_owner_install(1, verbose, returnCode);
break;
case TPM_PPI_OP_SET_OWNERINSTALL_FALSE_DEACTIVATE_DISABLE:
ret = tpm12_set_owner_install(0, verbose, returnCode);
if (!ret)
ret = tpm12_activate_tpm(0, 0, verbose, returnCode);
if (!ret)
ret = tpm12_enable_tpm(0, verbose, returnCode);
break;
case TPM_PPI_OP_CLEAR_ENABLE_ACTIVATE:
ret = tpm12_force_clear(0, 1, verbose, returnCode);
break;
case TPM_PPI_OP_ENABLE_ACTIVATE_CLEAR:
*nextStep = TPM_PPI_OP_CLEAR;
ret = tpm12_enable_activate(1, verbose, returnCode);
/* no reboot happened */
if (!ret)
ret = tpm12_force_clear(0, 0, verbose, returnCode);
break;
case TPM_PPI_OP_ENABLE_ACTIVATE_CLEAR_ENABLE_ACTIVATE:
*nextStep = TPM_PPI_OP_CLEAR_ENABLE_ACTIVATE;
I'm strugging to understand "nextStep". I think it would be clearer if the code could do a switch on the request and then execute all the details of that request without the need to set flags indicating the function should be rerun. tpm12_process_cfg() can call itself recursively if it needs to.
Some of the opcodes we are executing here need to do a reboot and then execute more steps after the reboot. The last one of the above sequences does this:
enable + activate -> reboot -> clear + enable + activate -> reboot
The post-reboot sequence clear + enable + activate exists with TPM_PPI_OP_CLEAR_ENABLE_ACTIVATE, so we can execute that as the 2nd part.
Stefan
-Kevin