The following series involves some code reorganization in the TPM code that I found useful in understanding the code.
Patches 3-5 simplify the hardware interface by only exporting three commands to the underlying TIS hardware (tpmhw_probe, tpmhw_transmit, tpmhw_set_timeouts).
Patches 8-10 simplify the parameters to the build_and_send_cmd() function.
The remaining patches are mostly just code reorg.
I have only compile tested these changes.
-Kevin
Kevin O'Connor (10): tpm: Add banner separating the TCG bios interface code from TCG menu code tpm: Avoid macro expansion of tpm request / response structs tpm: Simplify hardware probe and detection checks tpm: Add wrapper function tpm_set_timeouts() tpm: Move TPM hardware functions from tcgbios.c to hw/tpm_drivers.c tpm: Rework TPM interface shutdown support tpm: Simplify tcpa probe tpm: Introduce tpm_get_capability() helper function tpm: Eliminate response buffer parameter from build_and_send_cmd() tpm: Return returnCode from build_and_send_cmd() instead of via pointer param
src/hw/tpm_drivers.c | 84 ++++++ src/hw/tpm_drivers.h | 28 +- src/std/tcg.h | 55 ++-- src/tcgbios.c | 704 +++++++++++++++------------------------------------ 4 files changed, 317 insertions(+), 554 deletions(-)
Signed-off-by: Kevin O'Connor kevin@koconnor.net --- src/tcgbios.c | 5 +++++ 1 file changed, 5 insertions(+)
diff --git a/src/tcgbios.c b/src/tcgbios.c index 7b93d87..4f78c42 100644 --- a/src/tcgbios.c +++ b/src/tcgbios.c @@ -1187,6 +1187,11 @@ tpm_interrupt_handler32(struct bregs *regs) return; }
+ +/**************************************************************** + * TPM Configuration Menu + ****************************************************************/ + static u32 read_stclear_flags(char *buf, int buf_len) {
Avoid macros and use regular struct definitions for the request and response headers. This simplifies the header and reduces the need for casts in the code.
Signed-off-by: Kevin O'Connor kevin@koconnor.net --- src/std/tcg.h | 40 +++++++++++++++++----------------------- src/tcgbios.c | 11 +++++------ 2 files changed, 22 insertions(+), 29 deletions(-)
diff --git a/src/std/tcg.h b/src/std/tcg.h index d7b3fc4..70daa41 100644 --- a/src/std/tcg.h +++ b/src/std/tcg.h @@ -227,41 +227,35 @@ struct pcctes_romex } PACKED;
-#define TPM_REQ_HEADER \ - u16 tag; \ - u32 totlen; \ - u32 ordinal; - -#define TPM_RSP_HEADER \ - u16 tag; \ - u32 totlen; \ - u32 errcode; - struct tpm_req_header { - TPM_REQ_HEADER; + u16 tag; + u32 totlen; + u32 ordinal; } PACKED;
struct tpm_rsp_header { - TPM_RSP_HEADER; + u16 tag; + u32 totlen; + u32 errcode; } PACKED;
struct tpm_req_extend { - TPM_REQ_HEADER + struct tpm_req_header hdr; u32 pcrindex; u8 digest[SHA1_BUFSIZE]; } PACKED;
struct tpm_rsp_extend { - TPM_RSP_HEADER + struct tpm_rsp_header hdr; u8 digest[SHA1_BUFSIZE]; } PACKED;
struct tpm_req_getcap_perm_flags { - TPM_REQ_HEADER + struct tpm_req_header hdr; u32 capArea; u32 subCapSize; u32 subCap; @@ -287,13 +281,13 @@ enum permFlagsIndex {
struct tpm_res_getcap_perm_flags { - TPM_RSP_HEADER + struct tpm_rsp_header hdr; u32 size; struct tpm_permanent_flags perm_flags; } PACKED;
struct tpm_req_getcap_stclear_flags { - TPM_REQ_HEADER + struct tpm_req_header hdr; u32 capArea; u32 subCapSize; u32 subCap; @@ -311,40 +305,40 @@ struct tpm_stclear_flags { #define STCLEAR_FLAG_IDX_GLOBAL_LOCK 4
struct tpm_res_getcap_stclear_flags { - TPM_RSP_HEADER + struct tpm_rsp_header hdr; u32 size; struct tpm_stclear_flags stclear_flags; } PACKED;
struct tpm_res_getcap_ownerauth { - TPM_RSP_HEADER + struct tpm_rsp_header hdr; u32 size; u8 flag; } PACKED;
struct tpm_res_getcap_timeouts { - TPM_RSP_HEADER + struct tpm_rsp_header hdr; u32 size; u32 timeouts[4]; } PACKED;
struct tpm_res_getcap_durations { - TPM_RSP_HEADER + struct tpm_rsp_header hdr; u32 size; u32 durations[3]; } PACKED;
struct tpm_res_sha1start { - TPM_RSP_HEADER + struct tpm_rsp_header hdr; u32 max_num_bytes; } PACKED;
struct tpm_res_sha1complete { - TPM_RSP_HEADER + struct tpm_rsp_header hdr; u8 hash[20]; } PACKED;
diff --git a/src/tcgbios.c b/src/tcgbios.c index 4f78c42..55e38a9 100644 --- a/src/tcgbios.c +++ b/src/tcgbios.c @@ -472,17 +472,16 @@ tpm_log_extend_event(struct pcpes *pcpes, const void *event) return TCG_INVALID_INPUT_PARA;
struct tpm_req_extend tre = { - .tag = cpu_to_be16(TPM_TAG_RQU_CMD), - .totlen = cpu_to_be32(sizeof(tre)), - .ordinal = cpu_to_be32(TPM_ORD_Extend), - .pcrindex = cpu_to_be32(pcpes->pcrindex), + .hdr.tag = cpu_to_be16(TPM_TAG_RQU_CMD), + .hdr.totlen = cpu_to_be32(sizeof(tre)), + .hdr.ordinal = cpu_to_be32(TPM_ORD_Extend), + .pcrindex = cpu_to_be32(pcpes->pcrindex), }; memcpy(tre.digest, pcpes->digest, sizeof(tre.digest));
struct tpm_rsp_extend rsp; u32 resp_length = sizeof(rsp); - u32 rc = transmit(0, (void*)&tre, &rsp, &resp_length, - TPM_DURATION_TYPE_SHORT); + u32 rc = transmit(0, &tre.hdr, &rsp, &resp_length, TPM_DURATION_TYPE_SHORT); if (rc || resp_length != sizeof(rsp)) { tpm_set_failure(); return rc;
Perform the hardware probe once during setup instead of checking if the probe has been completed on each measurement event.
Don't probe for hardware during BIOS interface detection. Just check if the hardware is in a working state.
Unify has_working_tpm() with similar tpm_is_working().
Signed-off-by: Kevin O'Connor kevin@koconnor.net --- src/tcgbios.c | 122 ++++++++++++++++------------------------------------------ 1 file changed, 34 insertions(+), 88 deletions(-)
diff --git a/src/tcgbios.c b/src/tcgbios.c index 55e38a9..9448984 100644 --- a/src/tcgbios.c +++ b/src/tcgbios.c @@ -67,11 +67,7 @@ static const u8 GetCapability_Durations[] = { ****************************************************************/
typedef struct { - u8 tpm_probed:1; - u8 tpm_found:1; - u8 tpm_working:1; u8 if_shutdown:1; - u8 tpm_driver_to_use:4; struct tcpa_descriptor_rev2 *tcpa;
/* length of the TCPA log buffer */ @@ -90,9 +86,7 @@ typedef struct { u8 * log_area_last_entry; } tpm_state_t;
-tpm_state_t tpm_state VARLOW = { - .tpm_driver_to_use = TPM_INVALID_DRIVER, -}; +tpm_state_t tpm_state VARLOW;
typedef u8 tpm_ppi_code;
@@ -107,50 +101,21 @@ is_preboot_if_shutdown(void) * TPM hardware interface ****************************************************************/
-static u32 -is_tpm_present(void) +static u8 TPMHW_driver_to_use = TPM_INVALID_DRIVER; + +static int +tpmhw_probe(void) { - u32 rc = 0; unsigned int i; - for (i = 0; i < TPM_NUM_DRIVERS; i++) { struct tpm_driver *td = &tpm_drivers[i]; if (td->probe() != 0) { td->init(); - tpm_state.tpm_driver_to_use = i; - rc = 1; - break; + TPMHW_driver_to_use = i; + return 0; } } - - return rc; -} - -static void -probe_tpm(void) -{ - if (!tpm_state.tpm_probed) { - tpm_state.tpm_probed = 1; - tpm_state.tpm_found = (is_tpm_present() != 0); - tpm_state.tpm_working = tpm_state.tpm_found; - } -} - -static int -has_working_tpm(void) -{ - probe_tpm(); - - return tpm_state.tpm_working; -} - -int -tpm_is_working(void) -{ - if (!CONFIG_TCGBIOS) - return 0; - - return tpm_state.tpm_working; + return -1; }
static u32 @@ -158,10 +123,10 @@ transmit(u8 locty, struct tpm_req_header *req, void *respbuffer, u32 *respbufferlen, enum tpmDurationType to_t) { - if (tpm_state.tpm_driver_to_use == TPM_INVALID_DRIVER) + if (TPMHW_driver_to_use == TPM_INVALID_DRIVER) return TCG_FATAL_COM_ERROR;
- struct tpm_driver *td = &tpm_drivers[tpm_state.tpm_driver_to_use]; + struct tpm_driver *td = &tpm_drivers[TPMHW_driver_to_use];
u32 irc = td->activate(locty); if (irc != 0) { @@ -329,6 +294,14 @@ tpm_log_event(struct pcpes *pcpes, const void *event) * Helper functions ****************************************************************/
+static u8 TPM_working; + +int +tpm_is_working(void) +{ + return CONFIG_TCGBIOS && TPM_working; +} + /* * Send a TPM command with the given ordinal. Append the given buffer * containing all data in network byte order to the command (this is @@ -394,7 +367,7 @@ tpm_set_failure(void) NULL, 0, NULL, 0, &returnCode, TPM_DURATION_TYPE_SHORT);
- tpm_state.tpm_working = 0; + TPM_working = 0; }
static u32 @@ -404,7 +377,7 @@ determine_timeouts(void) u32 returnCode; struct tpm_res_getcap_timeouts timeouts; struct tpm_res_getcap_durations durations; - struct tpm_driver *td = &tpm_drivers[tpm_state.tpm_driver_to_use]; + struct tpm_driver *td = &tpm_drivers[TPMHW_driver_to_use]; u32 i;
rc = build_and_send_cmd(0, TPM_ORD_GetCapability, @@ -465,7 +438,7 @@ err_exit: static u32 tpm_log_extend_event(struct pcpes *pcpes, const void *event) { - if (!has_working_tpm()) + if (!tpm_is_working()) return TCG_GENERAL_ERROR;
if (pcpes->pcrindex >= 24) @@ -549,10 +522,7 @@ tpm_add_event_separators(void) u32 rc; u32 pcrIndex = 0;
- if (!CONFIG_TCGBIOS) - return 0; - - if (!has_working_tpm()) + if (!tpm_is_working()) return TCG_GENERAL_ERROR;
static const u8 evt_separator[] = {0xff,0xff,0xff,0xff}; @@ -572,10 +542,7 @@ tpm_add_event_separators(void) static u32 tpm_smbios_measure(void) { - if (!CONFIG_TCGBIOS) - return 0; - - if (!has_working_tpm()) + if (!tpm_is_working()) return TCG_GENERAL_ERROR;
u32 rc; @@ -607,7 +574,7 @@ tpm_startup(void) u32 rc; u32 returnCode;
- if (!has_working_tpm()) + if (!tpm_is_working()) return TCG_GENERAL_ERROR;
dprintf(DEBUG_tcg, "TCGBIOS: Starting with TPM_Startup(ST_CLEAR)\n"); @@ -677,15 +644,12 @@ err_exit: static void tpm_acpi_init(void) { - tpm_state.if_shutdown = 0; - tpm_state.tpm_probed = 0; - tpm_state.tpm_found = 0; - tpm_state.tpm_working = 0; - - if (!has_working_tpm()) { + int ret = tpmhw_probe(); + if (ret) { tpm_state.if_shutdown = 1; return; } + TPM_working = 1;
reset_acpi_log(); } @@ -709,10 +673,7 @@ tpm_prepboot(void) u32 rc; u32 returnCode;
- if (!CONFIG_TCGBIOS) - return; - - if (!has_working_tpm()) + if (!tpm_is_working()) return;
rc = build_and_send_cmd(0, TPM_ORD_PhysicalPresence, @@ -751,10 +712,7 @@ err_exit: u32 tpm_option_rom(const void *addr, u32 len) { - if (!CONFIG_TCGBIOS) - return 0; - - if (!has_working_tpm()) + if (!tpm_is_working()) return TCG_GENERAL_ERROR;
u32 rc; @@ -776,10 +734,7 @@ tpm_option_rom(const void *addr, u32 len) u32 tpm_add_bcv(u32 bootdrv, const u8 *addr, u32 length) { - if (!CONFIG_TCGBIOS) - return 0; - - if (!has_working_tpm()) + if (!tpm_is_working()) return TCG_GENERAL_ERROR;
if (length < 0x200) @@ -811,10 +766,7 @@ tpm_add_bcv(u32 bootdrv, const u8 *addr, u32 length) u32 tpm_add_cdrom(u32 bootdrv, const u8 *addr, u32 length) { - if (!CONFIG_TCGBIOS) - return 0; - - if (!has_working_tpm()) + if (!tpm_is_working()) return TCG_GENERAL_ERROR;
u32 rc = tpm_add_action(4, "Booting from CD ROM device"); @@ -831,10 +783,7 @@ tpm_add_cdrom(u32 bootdrv, const u8 *addr, u32 length) u32 tpm_add_cdrom_catalog(const u8 *addr, u32 length) { - if (!CONFIG_TCGBIOS) - return 0; - - if (!has_working_tpm()) + if (!tpm_is_working()) return TCG_GENERAL_ERROR;
u32 rc = tpm_add_action(4, "Booting from CD ROM device"); @@ -854,10 +803,7 @@ tpm_s3_resume(void) u32 rc; u32 returnCode;
- if (!CONFIG_TCGBIOS) - return; - - if (!has_working_tpm()) + if (!tpm_is_working()) return;
dprintf(DEBUG_tcg, "TCGBIOS: Resuming with TPM_Startup(ST_STATE)\n"); @@ -1123,7 +1069,7 @@ tpm_interrupt_handler32(struct bregs *regs)
switch ((enum irq_ids)regs->al) { case TCG_StatusCheck: - if (is_tpm_present() == 0) { + if (tpm_is_working() == 0) { /* no TPM available */ regs->eax = TCG_PC_TPM_NOT_PRESENT; } else {
On 12/29/2015 07:17 PM, Kevin O'Connor wrote:
Perform the hardware probe once during setup instead of checking if the probe has been completed on each measurement event.
Don't probe for hardware during BIOS interface detection. Just check if the hardware is in a working state.
Unify has_working_tpm() with similar tpm_is_working().
Signed-off-by: Kevin O'Connor kevin@koconnor.net
[...]
@@ -158,10 +123,10 @@ transmit(u8 locty, struct tpm_req_header *req, void *respbuffer, u32 *respbufferlen, enum tpmDurationType to_t) {
- if (tpm_state.tpm_driver_to_use == TPM_INVALID_DRIVER)
- if (TPMHW_driver_to_use == TPM_INVALID_DRIVER) return TCG_FATAL_COM_ERROR;
- struct tpm_driver *td = &tpm_drivers[tpm_state.tpm_driver_to_use];
struct tpm_driver *td = &tpm_drivers[TPMHW_driver_to_use];
u32 irc = td->activate(locty); if (irc != 0) {
@@ -329,6 +294,14 @@ tpm_log_event(struct pcpes *pcpes, const void *event)
- Helper functions
****************************************************************/
+static u8 TPM_working;
Should this not also have VARLOW to not be ROM'ed?
Stefan
On Wed, Dec 30, 2015 at 06:57:23PM -0500, Stefan Berger wrote:
On 12/29/2015 07:17 PM, Kevin O'Connor wrote:
Perform the hardware probe once during setup instead of checking if the probe has been completed on each measurement event.
Don't probe for hardware during BIOS interface detection. Just check if the hardware is in a working state.
Unify has_working_tpm() with similar tpm_is_working().
Signed-off-by: Kevin O'Connor kevin@koconnor.net
[...]
@@ -158,10 +123,10 @@ transmit(u8 locty, struct tpm_req_header *req, void *respbuffer, u32 *respbufferlen, enum tpmDurationType to_t) {
- if (tpm_state.tpm_driver_to_use == TPM_INVALID_DRIVER)
- if (TPMHW_driver_to_use == TPM_INVALID_DRIVER) return TCG_FATAL_COM_ERROR;
- struct tpm_driver *td = &tpm_drivers[tpm_state.tpm_driver_to_use];
struct tpm_driver *td = &tpm_drivers[TPMHW_driver_to_use];
u32 irc = td->activate(locty); if (irc != 0) {
@@ -329,6 +294,14 @@ tpm_log_event(struct pcpes *pcpes, const void *event)
- Helper functions
****************************************************************/
+static u8 TPM_working;
Should this not also have VARLOW to not be ROM'ed?
The only code that runs after normal variables are made read-only is the 16bit BIOS interface. After a later patch ("Don't call tpm_set_failure() from tpm_log_extend_event()") none of the 16bit BIOS interface functions call tpm_set_failure and thus none attempt to modify TPM_working. Unless I've missed something.
That later patch probably should be ahead of this patch.
-Kevin
On Wed, Dec 30, 2015 at 07:06:58PM -0500, Kevin O'Connor wrote:
On Wed, Dec 30, 2015 at 06:57:23PM -0500, Stefan Berger wrote:
On 12/29/2015 07:17 PM, Kevin O'Connor wrote:
+static u8 TPM_working;
Should this not also have VARLOW to not be ROM'ed?
The only code that runs after normal variables are made read-only is the 16bit BIOS interface. After a later patch ("Don't call tpm_set_failure() from tpm_log_extend_event()") none of the 16bit BIOS interface functions call tpm_set_failure and thus none attempt to modify TPM_working. Unless I've missed something.
That later patch probably should be ahead of this patch.
Actually, tpm_add_cdrom* and tpm_add_bcv are called with the variables read-only, so I think you are correct and the above code is wrong.
-Kevin
Signed-off-by: Kevin O'Connor kevin@koconnor.net --- src/tcgbios.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/src/tcgbios.c b/src/tcgbios.c index 9448984..b680c1d 100644 --- a/src/tcgbios.c +++ b/src/tcgbios.c @@ -155,6 +155,13 @@ transmit(u8 locty, struct tpm_req_header *req, return 0; }
+static void +tpmhw_set_timeouts(u32 timeouts[4], u32 durations[3]) +{ + struct tpm_driver *td = &tpm_drivers[TPMHW_driver_to_use]; + td->set_timeouts(timeouts, durations); +} +
/**************************************************************** * ACPI TCPA table interface @@ -377,7 +384,6 @@ determine_timeouts(void) u32 returnCode; struct tpm_res_getcap_timeouts timeouts; struct tpm_res_getcap_durations durations; - struct tpm_driver *td = &tpm_drivers[TPMHW_driver_to_use]; u32 i;
rc = build_and_send_cmd(0, TPM_ORD_GetCapability, @@ -421,8 +427,7 @@ determine_timeouts(void) durations.durations[1], durations.durations[2]);
- - td->set_timeouts(timeouts.timeouts, durations.durations); + tpmhw_set_timeouts(timeouts.timeouts, durations.durations);
return 0;
Move the hardware interface functions (tpmhw_probe, tpmhw_transmit, and tpmhw_set_timeouts) to tpm_drivers.c code, and only export those functions. This simplifies the hardware interface.
Signed-off-by: Kevin O'Connor kevin@koconnor.net --- src/hw/tpm_drivers.c | 84 ++++++++++++++++++++++++++++++++++++++++++++++++++++ src/hw/tpm_drivers.h | 28 ++++-------------- src/tcgbios.c | 75 ++++------------------------------------------ 3 files changed, 95 insertions(+), 92 deletions(-)
diff --git a/src/hw/tpm_drivers.c b/src/hw/tpm_drivers.c index 0932797..dc09779 100644 --- a/src/hw/tpm_drivers.c +++ b/src/hw/tpm_drivers.c @@ -7,6 +7,7 @@ // // This file may be distributed under the terms of the GNU LGPLv3 license.
+#include "byteorder.h" // be32_to_cpu #include "config.h" // CONFIG_TPM_TIS_SHA1THRESHOLD #include "hw/tpm_drivers.h" // struct tpm_driver #include "std/tcg.h" // TCG_NO_RESPONSE @@ -16,6 +17,28 @@ #include "util.h" // timer_calc_usec #include "x86.h" // readl
+/* low level driver implementation */ +struct tpm_driver { + u32 *timeouts; + u32 *durations; + void (*set_timeouts)(u32 timeouts[4], u32 durations[3]); + u32 (*probe)(void); + u32 (*init)(void); + u32 (*activate)(u8 locty); + u32 (*ready)(void); + u32 (*senddata)(const u8 *const data, u32 len); + u32 (*readresp)(u8 *buffer, u32 *len); + u32 (*waitdatavalid)(void); + u32 (*waitrespready)(enum tpmDurationType to_t); +}; + +extern struct tpm_driver tpm_drivers[]; + +#define TIS_DRIVER_IDX 0 +#define TPM_NUM_DRIVERS 1 + +#define TPM_INVALID_DRIVER 0xf + static const u32 tis_default_timeouts[4] = { TIS_DEFAULT_TIMEOUT_A, TIS_DEFAULT_TIMEOUT_B, @@ -297,3 +320,64 @@ struct tpm_driver tpm_drivers[TPM_NUM_DRIVERS] = { .waitrespready = tis_waitrespready, }, }; + +static u8 TPMHW_driver_to_use = TPM_INVALID_DRIVER; + +int +tpmhw_probe(void) +{ + unsigned int i; + for (i = 0; i < TPM_NUM_DRIVERS; i++) { + struct tpm_driver *td = &tpm_drivers[i]; + if (td->probe() != 0) { + td->init(); + TPMHW_driver_to_use = i; + return 0; + } + } + return -1; +} + +u32 +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 TCG_FATAL_COM_ERROR; + + struct tpm_driver *td = &tpm_drivers[TPMHW_driver_to_use]; + + u32 irc = td->activate(locty); + if (irc != 0) { + /* tpm could not be activated */ + return TCG_FATAL_COM_ERROR; + } + + irc = td->senddata((void*)req, be32_to_cpu(req->totlen)); + if (irc != 0) + return TCG_FATAL_COM_ERROR; + + irc = td->waitdatavalid(); + if (irc != 0) + return TCG_FATAL_COM_ERROR; + + irc = td->waitrespready(to_t); + if (irc != 0) + return TCG_FATAL_COM_ERROR; + + irc = td->readresp(respbuffer, respbufferlen); + if (irc != 0) + return TCG_FATAL_COM_ERROR; + + td->ready(); + + return 0; +} + +void +tpmhw_set_timeouts(u32 timeouts[4], u32 durations[3]) +{ + struct tpm_driver *td = &tpm_drivers[TPMHW_driver_to_use]; + td->set_timeouts(timeouts, durations); +} diff --git a/src/hw/tpm_drivers.h b/src/hw/tpm_drivers.h index ec50cca..afa3027 100644 --- a/src/hw/tpm_drivers.h +++ b/src/hw/tpm_drivers.h @@ -10,28 +10,12 @@ enum tpmDurationType { TPM_DURATION_TYPE_LONG, };
-/* low level driver implementation */ -struct tpm_driver { - u32 *timeouts; - u32 *durations; - void (*set_timeouts)(u32 timeouts[4], u32 durations[3]); - u32 (*probe)(void); - u32 (*init)(void); - u32 (*activate)(u8 locty); - u32 (*ready)(void); - u32 (*senddata)(const u8 *const data, u32 len); - u32 (*readresp)(u8 *buffer, u32 *len); - u32 (*waitdatavalid)(void); - u32 (*waitrespready)(enum tpmDurationType to_t); -}; - -extern struct tpm_driver tpm_drivers[]; - - -#define TIS_DRIVER_IDX 0 -#define TPM_NUM_DRIVERS 1 - -#define TPM_INVALID_DRIVER 0xf +int tpmhw_probe(void); +struct tpm_req_header; +u32 tpmhw_transmit(u8 locty, struct tpm_req_header *req, + void *respbuffer, u32 *respbufferlen, + enum tpmDurationType to_t); +void tpmhw_set_timeouts(u32 timeouts[4], u32 durations[3]);
/* TIS driver */ /* address of locality 0 (TIS) */ diff --git a/src/tcgbios.c b/src/tcgbios.c index b680c1d..e1399ee 100644 --- a/src/tcgbios.c +++ b/src/tcgbios.c @@ -98,72 +98,6 @@ is_preboot_if_shutdown(void)
/**************************************************************** - * TPM hardware interface - ****************************************************************/ - -static u8 TPMHW_driver_to_use = TPM_INVALID_DRIVER; - -static int -tpmhw_probe(void) -{ - unsigned int i; - for (i = 0; i < TPM_NUM_DRIVERS; i++) { - struct tpm_driver *td = &tpm_drivers[i]; - if (td->probe() != 0) { - td->init(); - TPMHW_driver_to_use = i; - return 0; - } - } - return -1; -} - -static u32 -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 TCG_FATAL_COM_ERROR; - - struct tpm_driver *td = &tpm_drivers[TPMHW_driver_to_use]; - - u32 irc = td->activate(locty); - if (irc != 0) { - /* tpm could not be activated */ - return TCG_FATAL_COM_ERROR; - } - - irc = td->senddata((void*)req, be32_to_cpu(req->totlen)); - if (irc != 0) - return TCG_FATAL_COM_ERROR; - - irc = td->waitdatavalid(); - if (irc != 0) - return TCG_FATAL_COM_ERROR; - - irc = td->waitrespready(to_t); - if (irc != 0) - return TCG_FATAL_COM_ERROR; - - irc = td->readresp(respbuffer, respbufferlen); - if (irc != 0) - return TCG_FATAL_COM_ERROR; - - td->ready(); - - return 0; -} - -static void -tpmhw_set_timeouts(u32 timeouts[4], u32 durations[3]) -{ - struct tpm_driver *td = &tpm_drivers[TPMHW_driver_to_use]; - td->set_timeouts(timeouts, durations); -} - - -/**************************************************************** * ACPI TCPA table interface ****************************************************************/
@@ -340,7 +274,7 @@ build_and_send_cmd(u8 locty, u32 ordinal, const u8 *append, u32 append_size, if (append_size) memcpy(req.cmd, append, append_size);
- u32 rc = transmit(locty, &req.trqh, obuffer, &obuffer_len, to_t); + u32 rc = tpmhw_transmit(locty, &req.trqh, obuffer, &obuffer_len, to_t); if (rc) return rc;
@@ -459,7 +393,8 @@ tpm_log_extend_event(struct pcpes *pcpes, const void *event)
struct tpm_rsp_extend rsp; u32 resp_length = sizeof(rsp); - u32 rc = transmit(0, &tre.hdr, &rsp, &resp_length, TPM_DURATION_TYPE_SHORT); + u32 rc = tpmhw_transmit(0, &tre.hdr, &rsp, &resp_length, + TPM_DURATION_TYPE_SHORT); if (rc || resp_length != sizeof(rsp)) { tpm_set_failure(); return rc; @@ -931,8 +866,8 @@ pass_through_to_tpm_int(struct pttti *pttti, struct pttto *pttto) }
u32 resbuflen = pttti->opblength - offsetof(struct pttto, tpmopout); - rc = transmit(0, trh, pttto->tpmopout, &resbuflen, - TPM_DURATION_TYPE_LONG /* worst case */); + rc = tpmhw_transmit(0, trh, pttto->tpmopout, &resbuflen, + TPM_DURATION_TYPE_LONG /* worst case */); if (rc) goto err_exit;
The 16bit BIOS interface should only shutdown on request from that interface - errors from the tcp or acpi log setup should not shutdown the interface. (Errors from those functions will cause the TPM to be in a "not working" state which will cause all the 16bit interface functions to fail.)
Centralize the checking for the interface shutdown condition in tpm_interrupt_handler32().
Signed-off-by: Kevin O'Connor kevin@koconnor.net --- src/tcgbios.c | 63 +++++++++++------------------------------------------------ 1 file changed, 11 insertions(+), 52 deletions(-)
diff --git a/src/tcgbios.c b/src/tcgbios.c index e1399ee..3207478 100644 --- a/src/tcgbios.c +++ b/src/tcgbios.c @@ -67,7 +67,6 @@ static const u8 GetCapability_Durations[] = { ****************************************************************/
typedef struct { - u8 if_shutdown:1; struct tcpa_descriptor_rev2 *tcpa;
/* length of the TCPA log buffer */ @@ -90,12 +89,6 @@ tpm_state_t tpm_state VARLOW;
typedef u8 tpm_ppi_code;
-static u32 -is_preboot_if_shutdown(void) -{ - return tpm_state.if_shutdown; -} -
/**************************************************************** * ACPI TCPA table interface @@ -149,8 +142,6 @@ find_tcpa_table(void)
if (rsdp) tcpa = find_tcpa_by_rsdp(rsdp); - else - tpm_state.if_shutdown = 1;
if (!rsdp) dprintf(DEBUG_tcg, @@ -585,10 +576,8 @@ static void tpm_acpi_init(void) { int ret = tpmhw_probe(); - if (ret) { - tpm_state.if_shutdown = 1; + if (ret) return; - } TPM_working = 1;
reset_acpi_log(); @@ -771,6 +760,8 @@ err_exit: * BIOS interface ****************************************************************/
+u8 TPM_interface_shutdown VARLOW; + static inline void *input_buf32(struct bregs *regs) { return MAKE_FLATPTR(regs->es, regs->di); @@ -793,11 +784,6 @@ hash_log_extend_event_int(const struct hleei_short *hleei_s, struct pcpes *pcpes; u32 pcrindex;
- if (is_preboot_if_shutdown() != 0) { - rc = TCG_INTERFACE_SHUTDOWN; - goto err_exit; - } - /* short or long version? */ switch (hleei_s->ipblength) { case sizeof(struct hleei_short): @@ -850,12 +836,6 @@ static u32 pass_through_to_tpm_int(struct pttti *pttti, struct pttto *pttto) { u32 rc = 0; - - if (is_preboot_if_shutdown()) { - rc = TCG_INTERFACE_SHUTDOWN; - goto err_exit; - } - struct tpm_req_header *trh = (void*)pttti->tpmopin;
if (pttti->ipblength < sizeof(struct pttti) + sizeof(trh) @@ -886,15 +866,8 @@ err_exit: static u32 shutdown_preboot_interface(void) { - u32 rc = 0; - - if (!is_preboot_if_shutdown()) { - tpm_state.if_shutdown = 1; - } else { - rc = TCG_INTERFACE_SHUTDOWN; - } - - return rc; + TPM_interface_shutdown = 1; + return 0; }
static u32 @@ -904,11 +877,6 @@ hash_log_event_int(const struct hlei *hlei, struct hleo *hleo) u16 size; struct pcpes *pcpes;
- if (is_preboot_if_shutdown() != 0) { - rc = TCG_INTERFACE_SHUTDOWN; - goto err_exit; - } - size = hlei->ipblength; if (size != sizeof(*hlei)) { rc = TCG_INVALID_INPUT_PARA; @@ -946,9 +914,6 @@ err_exit: static u32 hash_all_int(const struct hai *hai, u8 *hash) { - if (is_preboot_if_shutdown() != 0) - return TCG_INTERFACE_SHUTDOWN; - if (hai->ipblength != sizeof(struct hai) || hai->hashdataptr == 0 || hai->hashdatalen == 0 || @@ -961,18 +926,10 @@ hash_all_int(const struct hai *hai, u8 *hash) static u32 tss_int(struct ti *ti, struct to *to) { - u32 rc = 0; - - if (is_preboot_if_shutdown() == 0) { - rc = TCG_PC_UNSUPPORTED; - } else { - rc = TCG_INTERFACE_SHUTDOWN; - } - to->opblength = sizeof(struct to); to->reserved = 0;
- return rc; + return TCG_PC_UNSUPPORTED; }
static u32 @@ -988,9 +945,6 @@ compact_hash_log_extend_event_int(u8 *buffer, .eventdatasize = sizeof(info), };
- if (is_preboot_if_shutdown() != 0) - return TCG_INTERFACE_SHUTDOWN; - tpm_fill_hash(&pcpes, buffer, length); u32 rc = tpm_log_extend_event(&pcpes, &info); if (rc == 0) @@ -1007,6 +961,11 @@ tpm_interrupt_handler32(struct bregs *regs)
set_cf(regs, 0);
+ if (TPM_interface_shutdown && regs->al) { + regs->eax = TCG_INTERFACE_SHUTDOWN; + return; + } + switch ((enum irq_ids)regs->al) { case TCG_StatusCheck: if (tpm_is_working() == 0) {
The TPM ACPI tables are only scanned once at startup and the code can rely on that. Merge find_tcpa_table() into find_tcpa_by_rsdp(), merge get_lasa_base_ptr() into reset_acpi_log(), and merge tpm_acpi_init() into tpm_setup().
The tpm_state structure is now only used for TCPA tracking.
Signed-off-by: Kevin O'Connor kevin@koconnor.net --- src/tcgbios.c | 141 +++++++++++++++++++--------------------------------------- 1 file changed, 45 insertions(+), 96 deletions(-)
diff --git a/src/tcgbios.c b/src/tcgbios.c index 3207478..8f9f321 100644 --- a/src/tcgbios.c +++ b/src/tcgbios.c @@ -61,14 +61,14 @@ static const u8 GetCapability_Durations[] = { 0x00, 0x00, 0x01, 0x20 };
+typedef u8 tpm_ppi_code; +
/**************************************************************** - * TPM state tracking + * ACPI TCPA table interface ****************************************************************/
-typedef struct { - struct tcpa_descriptor_rev2 *tcpa; - +struct { /* length of the TCPA log buffer */ u32 log_area_minimum_length;
@@ -83,104 +83,59 @@ typedef struct {
/* address of last entry written (need for TCG_StatusCheck) */ u8 * log_area_last_entry; -} tpm_state_t; - -tpm_state_t tpm_state VARLOW; - -typedef u8 tpm_ppi_code; - - -/**************************************************************** - * ACPI TCPA table interface - ****************************************************************/ +} tpm_state VARLOW;
static struct tcpa_descriptor_rev2 * find_tcpa_by_rsdp(struct rsdp_descriptor *rsdp) { - u32 ctr = 0; - struct tcpa_descriptor_rev2 *tcpa = NULL; - struct rsdt_descriptor *rsdt; - u32 length; - u16 off; - - rsdt = (struct rsdt_descriptor *)rsdp->rsdt_physical_address; + if (!rsdp) { + dprintf(DEBUG_tcg, + "TCGBIOS: RSDP was NOT found! -- Disabling interface.\n"); + return NULL; + } + struct rsdt_descriptor *rsdt = (void*)rsdp->rsdt_physical_address; if (!rsdt) return NULL;
- length = rsdt->length; - off = offsetof(struct rsdt_descriptor, entry); - + u32 length = rsdt->length; + u16 off = offsetof(struct rsdt_descriptor, entry); + u32 ctr = 0; while ((off + sizeof(rsdt->entry[0])) <= length) { /* try all pointers to structures */ - tcpa = (struct tcpa_descriptor_rev2 *)(int)rsdt->entry[ctr]; + struct tcpa_descriptor_rev2 *tcpa = (void*)(int)rsdt->entry[ctr];
/* valid TCPA ACPI table ? */ - if (tcpa->signature == TCPA_SIGNATURE && - checksum((u8 *)tcpa, tcpa->length) == 0) - break; + if (tcpa->signature == TCPA_SIGNATURE + && checksum(tcpa, tcpa->length) == 0) + return tcpa;
- tcpa = NULL; off += sizeof(rsdt->entry[0]); ctr++; }
- /* cache it */ - if (tcpa) - tpm_state.tcpa = tcpa; - - return tcpa; + dprintf(DEBUG_tcg, "TCGBIOS: TCPA ACPI was NOT found!\n"); + return NULL; }
-static struct tcpa_descriptor_rev2 * -find_tcpa_table(void) -{ - struct tcpa_descriptor_rev2 *tcpa = NULL; - struct rsdp_descriptor *rsdp = RsdpAddr; - - if (tpm_state.tcpa) - return tpm_state.tcpa; - - if (rsdp) - tcpa = find_tcpa_by_rsdp(rsdp); - - if (!rsdp) - dprintf(DEBUG_tcg, - "TCGBIOS: RSDP was NOT found! -- Disabling interface.\n"); - else if (!tcpa) - dprintf(DEBUG_tcg, "TCGBIOS: TCPA ACPI was NOT found!\n"); - - return tcpa; -} - -static u8 * -get_lasa_base_ptr(u32 *log_area_minimum_length) -{ - u8 *log_area_start_address = 0; - struct tcpa_descriptor_rev2 *tcpa = find_tcpa_table(); - - if (tcpa) { - log_area_start_address = (u8 *)(long)tcpa->log_area_start_address; - if (log_area_minimum_length) - *log_area_minimum_length = tcpa->log_area_minimum_length; - } - - return log_area_start_address; -} - -/* clear the ACPI log */ -static void -reset_acpi_log(void) +static int +tpm_tcpa_probe(void) { - tpm_state.log_area_start_address = - get_lasa_base_ptr(&tpm_state.log_area_minimum_length); - - if (tpm_state.log_area_start_address) - memset(tpm_state.log_area_start_address, 0, - tpm_state.log_area_minimum_length); - - tpm_state.log_area_next_entry = tpm_state.log_area_start_address; + struct tcpa_descriptor_rev2 *tcpa = find_tcpa_by_rsdp(RsdpAddr); + if (!tcpa) + return -1; + + u8 *log_area_start_address = (u8*)(long)tcpa->log_area_start_address; + u32 log_area_minimum_length = tcpa->log_area_minimum_length; + if (!log_area_start_address || !log_area_minimum_length) + return -1; + + memset(log_area_start_address, 0, log_area_minimum_length); + tpm_state.log_area_start_address = log_area_start_address; + tpm_state.log_area_minimum_length = log_area_minimum_length; + tpm_state.log_area_next_entry = log_area_start_address; tpm_state.log_area_last_entry = NULL; tpm_state.entry_count = 0; + return 0; }
/* @@ -568,28 +523,22 @@ err_exit: return TCG_TCG_COMMAND_ERROR; }
-/* - initialize the TCPA ACPI subsystem; find the ACPI tables and determine - where the TCPA table is. - */ -static void -tpm_acpi_init(void) +void +tpm_setup(void) { + if (!CONFIG_TCGBIOS) + return; + int ret = tpmhw_probe(); if (ret) return; - TPM_working = 1; - - reset_acpi_log(); -}
-void -tpm_setup(void) -{ - if (!CONFIG_TCGBIOS) + ret = tpm_tcpa_probe(); + if (ret) return;
- tpm_acpi_init(); + TPM_working = 1; + if (runningOnXen()) return;
Introduce helper function to call the TPM_ORD_GetCapability command. Update all get capability callers to use this helper.
Signed-off-by: Kevin O'Connor kevin@koconnor.net --- src/std/tcg.h | 17 +++--- src/tcgbios.c | 176 ++++++++++++++++------------------------------------------ 2 files changed, 57 insertions(+), 136 deletions(-)
diff --git a/src/std/tcg.h b/src/std/tcg.h index 70daa41..9f7f021 100644 --- a/src/std/tcg.h +++ b/src/std/tcg.h @@ -254,13 +254,21 @@ struct tpm_rsp_extend { } PACKED;
-struct tpm_req_getcap_perm_flags { +struct tpm_req_getcap { struct tpm_req_header hdr; u32 capArea; u32 subCapSize; u32 subCap; } PACKED;
+#define TPM_CAP_FLAG 0x04 +#define TPM_CAP_PROPERTY 0x05 +#define TPM_CAP_FLAG_PERMANENT 0x108 +#define TPM_CAP_FLAG_VOLATILE 0x109 +#define TPM_CAP_PROP_OWNER 0x111 +#define TPM_CAP_PROP_TIS_TIMEOUT 0x115 +#define TPM_CAP_PROP_DURATION 0x120 +
struct tpm_permanent_flags { u16 tag; @@ -286,13 +294,6 @@ struct tpm_res_getcap_perm_flags { struct tpm_permanent_flags perm_flags; } PACKED;
-struct tpm_req_getcap_stclear_flags { - struct tpm_req_header hdr; - u32 capArea; - u32 subCapSize; - u32 subCap; -} PACKED; - struct tpm_stclear_flags { u16 tag; u8 flags[5]; diff --git a/src/tcgbios.c b/src/tcgbios.c index 8f9f321..510d4b3 100644 --- a/src/tcgbios.c +++ b/src/tcgbios.c @@ -36,31 +36,6 @@ static const u8 PhysicalPresence_NOT_PRESENT_LOCK[] = { 0x00, 0x14 }; static const u8 CommandFlag_FALSE[1] = { 0x00 }; static const u8 CommandFlag_TRUE[1] = { 0x01 };
-static const u8 GetCapability_Permanent_Flags[] = { - 0x00, 0x00, 0x00, 0x04, 0x00, 0x00, 0x00, 0x04, - 0x00, 0x00, 0x01, 0x08 -}; - -static const u8 GetCapability_STClear_Flags[] = { - 0x00, 0x00, 0x00, 0x04, 0x00, 0x00, 0x00, 0x04, - 0x00, 0x00, 0x01, 0x09 -}; - -static const u8 GetCapability_OwnerAuth[] = { - 0x00, 0x00, 0x00, 0x05, 0x00, 0x00, 0x00, 0x04, - 0x00, 0x00, 0x01, 0x11 -}; - -static const u8 GetCapability_Timeouts[] = { - 0x00, 0x00, 0x00, 0x05, 0x00, 0x00, 0x00, 0x04, - 0x00, 0x00, 0x01, 0x15 -}; - -static const u8 GetCapability_Durations[] = { - 0x00, 0x00, 0x00, 0x05, 0x00, 0x00, 0x00, 0x04, - 0x00, 0x00, 0x01, 0x20 -}; - typedef u8 tpm_ppi_code;
@@ -257,39 +232,46 @@ tpm_set_failure(void) TPM_working = 0; }
+static int +tpm_get_capability(u32 cap, u32 subcap, struct tpm_rsp_header *rsp, u32 rsize) +{ + struct tpm_req_getcap trgc = { + .hdr.tag = cpu_to_be16(TPM_TAG_RQU_CMD), + .hdr.totlen = cpu_to_be32(sizeof(trgc)), + .hdr.ordinal = cpu_to_be32(TPM_ORD_GetCapability), + .capArea = cpu_to_be32(cap), + .subCapSize = cpu_to_be32(sizeof(trgc.subCap)), + .subCap = cpu_to_be32(subcap) + }; + u32 resp_size = rsize; + u32 rc = tpmhw_transmit(0, &trgc.hdr, &rsp, &resp_size, + TPM_DURATION_TYPE_SHORT); + dprintf(DEBUG_tcg, "TCGBIOS: Return code from TPM_GetCapability(%d, %d)" + " = %x %x\n", cap, subcap, rc, rsp->errcode); + if (rc || resp_size != rsize || rsp->errcode) { + dprintf(DEBUG_tcg, "TCGBIOS: TPM malfunctioning (line %d).\n", __LINE__); + tpm_set_failure(); + return rc ?: TCG_TCG_COMMAND_ERROR; + } + return 0; +} + static u32 determine_timeouts(void) { - u32 rc; - u32 returnCode; struct tpm_res_getcap_timeouts timeouts; - struct tpm_res_getcap_durations durations; - u32 i; - - rc = build_and_send_cmd(0, TPM_ORD_GetCapability, - GetCapability_Timeouts, - sizeof(GetCapability_Timeouts), - (u8 *)&timeouts, sizeof(timeouts), - &returnCode, TPM_DURATION_TYPE_SHORT); - - dprintf(DEBUG_tcg, "TCGBIOS: Return code from TPM_GetCapability(Timeouts)" - " = 0x%08x\n", returnCode); - - if (rc || returnCode) - goto err_exit; - - rc = build_and_send_cmd(0, TPM_ORD_GetCapability, - GetCapability_Durations, - sizeof(GetCapability_Durations), - (u8 *)&durations, sizeof(durations), - &returnCode, TPM_DURATION_TYPE_SHORT); - - dprintf(DEBUG_tcg, "TCGBIOS: Return code from TPM_GetCapability(Durations)" - " = 0x%08x\n", returnCode); + int ret = tpm_get_capability(TPM_CAP_PROPERTY, TPM_CAP_PROP_TIS_TIMEOUT + , &timeouts.hdr, sizeof(timeouts)); + if (ret) + return ret;
- if (rc || returnCode) - goto err_exit; + struct tpm_res_getcap_durations durations; + ret = tpm_get_capability(TPM_CAP_PROPERTY, TPM_CAP_PROP_DURATION + , &durations.hdr, sizeof(durations)); + if (ret) + return ret;
+ int i; for (i = 0; i < 3; i++) durations.durations[i] = be32_to_cpu(durations.durations[i]);
@@ -310,14 +292,6 @@ determine_timeouts(void) tpmhw_set_timeouts(timeouts.timeouts, durations.durations);
return 0; - -err_exit: - dprintf(DEBUG_tcg, "TCGBIOS: TPM malfunctioning (line %d).\n", __LINE__); - - tpm_set_failure(); - if (rc) - return rc; - return TCG_TCG_COMMAND_ERROR; }
static u32 @@ -988,35 +962,17 @@ tpm_interrupt_handler32(struct bregs *regs) static u32 read_stclear_flags(char *buf, int buf_len) { - u32 rc; - u32 returnCode; - struct tpm_res_getcap_stclear_flags stcf; - memset(buf, 0, buf_len);
- rc = build_and_send_cmd(0, TPM_ORD_GetCapability, - GetCapability_STClear_Flags, - sizeof(GetCapability_STClear_Flags), - (u8 *)&stcf, sizeof(stcf), - &returnCode, TPM_DURATION_TYPE_SHORT); - - dprintf(DEBUG_tcg, "TCGBIOS: Return code from TPM_GetCapability() " - "= 0x%08x\n", returnCode); - - if (rc || returnCode) - goto err_exit; + struct tpm_res_getcap_stclear_flags stcf; + int ret = tpm_get_capability(TPM_CAP_FLAG, TPM_CAP_FLAG_VOLATILE + , &stcf.hdr, sizeof(stcf)); + if (ret) + return ret;
memcpy(buf, &stcf.stclear_flags, buf_len);
return 0; - -err_exit: - dprintf(DEBUG_tcg, "TCGBIOS: TPM malfunctioning (line %d).\n", __LINE__); - - tpm_set_failure(); - if (rc) - return rc; - return TCG_TCG_COMMAND_ERROR; }
static u32 @@ -1081,67 +1037,31 @@ err_exit: static u32 read_permanent_flags(char *buf, int buf_len) { - u32 rc; - u32 returnCode; - struct tpm_res_getcap_perm_flags pf; - memset(buf, 0, buf_len);
- rc = build_and_send_cmd(0, TPM_ORD_GetCapability, - GetCapability_Permanent_Flags, - sizeof(GetCapability_Permanent_Flags), - (u8 *)&pf, sizeof(pf), - &returnCode, TPM_DURATION_TYPE_SHORT); - - dprintf(DEBUG_tcg, "TCGBIOS: Return code from TPM_GetCapability() " - "= 0x%08x\n", returnCode); - - if (rc || returnCode) - goto err_exit; + struct tpm_res_getcap_perm_flags pf; + int ret = tpm_get_capability(TPM_CAP_FLAG, TPM_CAP_FLAG_PERMANENT + , &pf.hdr, sizeof(pf)); + if (ret) + return ret;
memcpy(buf, &pf.perm_flags, buf_len);
return 0; - -err_exit: - dprintf(DEBUG_tcg, "TCGBIOS: TPM malfunctioning (line %d).\n", __LINE__); - - tpm_set_failure(); - if (rc) - return rc; - return TCG_TCG_COMMAND_ERROR; }
static u32 read_has_owner(int *has_owner) { - u32 rc; - u32 returnCode; struct tpm_res_getcap_ownerauth oauth; - - rc = build_and_send_cmd(0, TPM_ORD_GetCapability, - GetCapability_OwnerAuth, - sizeof(GetCapability_OwnerAuth), - (u8 *)&oauth, sizeof(oauth), - &returnCode, TPM_DURATION_TYPE_SHORT); - - dprintf(DEBUG_tcg, "TCGBIOS: Return code from TPM_GetCapability() " - "= 0x%08x\n", returnCode); - - if (rc || returnCode) - goto err_exit; + int ret = tpm_get_capability(TPM_CAP_PROPERTY, TPM_CAP_PROP_OWNER + , &oauth.hdr, sizeof(oauth)); + if (ret) + return ret;
*has_owner = oauth.flag;
return 0; - -err_exit: - dprintf(DEBUG_tcg,"TCGBIOS: TPM malfunctioning (line %d).\n", __LINE__); - - tpm_set_failure(); - if (rc) - return rc; - return TCG_TCG_COMMAND_ERROR; }
static u32
On 12/29/2015 07:17 PM, Kevin O'Connor wrote:
Introduce helper function to call the TPM_ORD_GetCapability command. Update all get capability callers to use this helper.
Signed-off-by: Kevin O'Connor kevin@koconnor.net
src/std/tcg.h | 17 +++--- src/tcgbios.c | 176 ++++++++++++++++------------------------------------------ 2 files changed, 57 insertions(+), 136 deletions(-)
diff --git a/src/std/tcg.h b/src/std/tcg.h index 70daa41..9f7f021 100644 --- a/src/std/tcg.h +++ b/src/std/tcg.h @@ -254,13 +254,21 @@ struct tpm_rsp_extend { } PACKED;
-struct tpm_req_getcap_perm_flags { +struct tpm_req_getcap { struct tpm_req_header hdr; u32 capArea; u32 subCapSize; u32 subCap; } PACKED;
+#define TPM_CAP_FLAG 0x04 +#define TPM_CAP_PROPERTY 0x05 +#define TPM_CAP_FLAG_PERMANENT 0x108 +#define TPM_CAP_FLAG_VOLATILE 0x109 +#define TPM_CAP_PROP_OWNER 0x111 +#define TPM_CAP_PROP_TIS_TIMEOUT 0x115 +#define TPM_CAP_PROP_DURATION 0x120
struct tpm_permanent_flags { u16 tag;
@@ -286,13 +294,6 @@ struct tpm_res_getcap_perm_flags { struct tpm_permanent_flags perm_flags; } PACKED;
-struct tpm_req_getcap_stclear_flags {
- struct tpm_req_header hdr;
- u32 capArea;
- u32 subCapSize;
- u32 subCap;
-} PACKED;
- struct tpm_stclear_flags { u16 tag; u8 flags[5];
diff --git a/src/tcgbios.c b/src/tcgbios.c index 8f9f321..510d4b3 100644 --- a/src/tcgbios.c +++ b/src/tcgbios.c @@ -36,31 +36,6 @@ static const u8 PhysicalPresence_NOT_PRESENT_LOCK[] = { 0x00, 0x14 }; static const u8 CommandFlag_FALSE[1] = { 0x00 }; static const u8 CommandFlag_TRUE[1] = { 0x01 };
-static const u8 GetCapability_Permanent_Flags[] = {
- 0x00, 0x00, 0x00, 0x04, 0x00, 0x00, 0x00, 0x04,
- 0x00, 0x00, 0x01, 0x08
-};
-static const u8 GetCapability_STClear_Flags[] = {
- 0x00, 0x00, 0x00, 0x04, 0x00, 0x00, 0x00, 0x04,
- 0x00, 0x00, 0x01, 0x09
-};
-static const u8 GetCapability_OwnerAuth[] = {
- 0x00, 0x00, 0x00, 0x05, 0x00, 0x00, 0x00, 0x04,
- 0x00, 0x00, 0x01, 0x11
-};
-static const u8 GetCapability_Timeouts[] = {
- 0x00, 0x00, 0x00, 0x05, 0x00, 0x00, 0x00, 0x04,
- 0x00, 0x00, 0x01, 0x15
-};
-static const u8 GetCapability_Durations[] = {
- 0x00, 0x00, 0x00, 0x05, 0x00, 0x00, 0x00, 0x04,
- 0x00, 0x00, 0x01, 0x20
-};
- typedef u8 tpm_ppi_code;
@@ -257,39 +232,46 @@ tpm_set_failure(void) TPM_working = 0; }
+static int +tpm_get_capability(u32 cap, u32 subcap, struct tpm_rsp_header *rsp, u32 rsize) +{
- struct tpm_req_getcap trgc = {
.hdr.tag = cpu_to_be16(TPM_TAG_RQU_CMD),.hdr.totlen = cpu_to_be32(sizeof(trgc)),.hdr.ordinal = cpu_to_be32(TPM_ORD_GetCapability),.capArea = cpu_to_be32(cap),.subCapSize = cpu_to_be32(sizeof(trgc.subCap)),.subCap = cpu_to_be32(subcap)- };
- u32 resp_size = rsize;
- u32 rc = tpmhw_transmit(0, &trgc.hdr, &rsp, &resp_size,
TPM_DURATION_TYPE_SHORT);
Bug: &rsp -> rsp !
Otherwise it looks good.
Stefan
On Tue, Dec 29, 2015 at 09:09:09PM -0500, Stefan Berger wrote:
On 12/29/2015 07:17 PM, Kevin O'Connor wrote:
+static int +tpm_get_capability(u32 cap, u32 subcap, struct tpm_rsp_header *rsp, u32 rsize) +{
- struct tpm_req_getcap trgc = {
.hdr.tag = cpu_to_be16(TPM_TAG_RQU_CMD),.hdr.totlen = cpu_to_be32(sizeof(trgc)),.hdr.ordinal = cpu_to_be32(TPM_ORD_GetCapability),.capArea = cpu_to_be32(cap),.subCapSize = cpu_to_be32(sizeof(trgc.subCap)),.subCap = cpu_to_be32(subcap)- };
- u32 resp_size = rsize;
- u32 rc = tpmhw_transmit(0, &trgc.hdr, &rsp, &resp_size,
TPM_DURATION_TYPE_SHORT);Bug: &rsp -> rsp !
Otherwise it looks good.
Oops - thanks. I fixed locally.
-Kevin
There are no longer any callers that use the response buffer.
Signed-off-by: Kevin O'Connor kevin@koconnor.net --- src/tcgbios.c | 45 +++++++++++++++++---------------------------- 1 file changed, 17 insertions(+), 28 deletions(-)
diff --git a/src/tcgbios.c b/src/tcgbios.c index 510d4b3..c578fd2 100644 --- a/src/tcgbios.c +++ b/src/tcgbios.c @@ -168,12 +168,10 @@ tpm_is_working(void) * Send a TPM command with the given ordinal. Append the given buffer * containing all data in network byte order to the command (this is * the custom part per command) and expect a response of the given size. - * If a buffer is provided, the response will be copied into it. */ static u32 build_and_send_cmd(u8 locty, u32 ordinal, const u8 *append, u32 append_size, - u8 *resbuffer, u32 return_size, u32 *returnCode, - enum tpmDurationType to_t) + u32 *returnCode, enum tpmDurationType to_t) { struct { struct tpm_req_header trqh; @@ -188,7 +186,7 @@ build_and_send_cmd(u8 locty, u32 ordinal, const u8 *append, u32 append_size, u32 obuffer_len = sizeof(obuffer); memset(obuffer, 0x0, sizeof(obuffer));
- if (return_size > sizeof(obuffer) || append_size > sizeof(req.cmd)) { + if (append_size > sizeof(req.cmd)) { warn_internalerror(); return TCG_FIRMWARE_ERROR; } @@ -200,10 +198,6 @@ build_and_send_cmd(u8 locty, u32 ordinal, const u8 *append, u32 append_size, return rc;
*returnCode = be32_to_cpu(trsh->errcode); - - if (resbuffer) - memcpy(resbuffer, trsh, return_size); - return 0; }
@@ -216,18 +210,15 @@ tpm_set_failure(void) build_and_send_cmd(0, TPM_ORD_PhysicalPresence, PhysicalPresence_CMD_ENABLE, sizeof(PhysicalPresence_CMD_ENABLE), - NULL, 0, &returnCode, - TPM_DURATION_TYPE_SHORT); + &returnCode, TPM_DURATION_TYPE_SHORT);
build_and_send_cmd(0, TPM_ORD_PhysicalPresence, PhysicalPresence_PRESENT, sizeof(PhysicalPresence_PRESENT), - NULL, 0, &returnCode, - TPM_DURATION_TYPE_SHORT); + &returnCode, TPM_DURATION_TYPE_SHORT);
build_and_send_cmd(0, TPM_ORD_SetTempDeactivated, - NULL, 0, NULL, 0, &returnCode, - TPM_DURATION_TYPE_SHORT); + NULL, 0, &returnCode, TPM_DURATION_TYPE_SHORT);
TPM_working = 0; } @@ -440,7 +431,7 @@ tpm_startup(void) dprintf(DEBUG_tcg, "TCGBIOS: Starting with TPM_Startup(ST_CLEAR)\n"); rc = build_and_send_cmd(0, TPM_ORD_Startup, Startup_ST_CLEAR, sizeof(Startup_ST_CLEAR), - NULL, 0, &returnCode, TPM_DURATION_TYPE_SHORT); + &returnCode, TPM_DURATION_TYPE_SHORT);
dprintf(DEBUG_tcg, "Return code from TPM_Startup = 0x%08x\n", returnCode); @@ -461,7 +452,7 @@ tpm_startup(void) goto err_exit;
rc = build_and_send_cmd(0, TPM_ORD_SelfTestFull, NULL, 0, - NULL, 0, &returnCode, TPM_DURATION_TYPE_LONG); + &returnCode, TPM_DURATION_TYPE_LONG);
dprintf(DEBUG_tcg, "Return code from TPM_SelfTestFull = 0x%08x\n", returnCode); @@ -470,7 +461,7 @@ tpm_startup(void) goto err_exit;
rc = build_and_send_cmd(3, TSC_ORD_ResetEstablishmentBit, NULL, 0, - NULL, 0, &returnCode, TPM_DURATION_TYPE_SHORT); + &returnCode, TPM_DURATION_TYPE_SHORT);
dprintf(DEBUG_tcg, "Return code from TSC_ResetEstablishmentBit = 0x%08x\n", returnCode); @@ -531,14 +522,14 @@ tpm_prepboot(void) rc = build_and_send_cmd(0, TPM_ORD_PhysicalPresence, PhysicalPresence_CMD_ENABLE, sizeof(PhysicalPresence_CMD_ENABLE), - NULL, 0, &returnCode, TPM_DURATION_TYPE_SHORT); + &returnCode, TPM_DURATION_TYPE_SHORT); if (rc || returnCode) goto err_exit;
rc = build_and_send_cmd(0, TPM_ORD_PhysicalPresence, PhysicalPresence_NOT_PRESENT_LOCK, sizeof(PhysicalPresence_NOT_PRESENT_LOCK), - NULL, 0, &returnCode, TPM_DURATION_TYPE_SHORT); + &returnCode, TPM_DURATION_TYPE_SHORT); if (rc || returnCode) goto err_exit;
@@ -662,7 +653,7 @@ tpm_s3_resume(void)
rc = build_and_send_cmd(0, TPM_ORD_Startup, Startup_ST_STATE, sizeof(Startup_ST_STATE), - NULL, 0, &returnCode, TPM_DURATION_TYPE_SHORT); + &returnCode, TPM_DURATION_TYPE_SHORT);
dprintf(DEBUG_tcg, "TCGBIOS: ReturnCode from TPM_Startup = 0x%08x\n", returnCode); @@ -996,7 +987,7 @@ assert_physical_presence(int verbose) rc = build_and_send_cmd(0, TPM_ORD_PhysicalPresence, PhysicalPresence_CMD_ENABLE, sizeof(PhysicalPresence_CMD_ENABLE), - NULL, 0, &returnCode, TPM_DURATION_TYPE_SHORT); + &returnCode, TPM_DURATION_TYPE_SHORT);
dprintf(DEBUG_tcg, "Return code from TSC_PhysicalPresence(CMD_ENABLE) = 0x%08x\n", @@ -1011,7 +1002,7 @@ assert_physical_presence(int verbose) rc = build_and_send_cmd(0, TPM_ORD_PhysicalPresence, PhysicalPresence_PRESENT, sizeof(PhysicalPresence_PRESENT), - NULL, 0, &returnCode, TPM_DURATION_TYPE_SHORT); + &returnCode, TPM_DURATION_TYPE_SHORT);
dprintf(DEBUG_tcg, "Return code from TSC_PhysicalPresence(PRESENT) = 0x%08x\n", @@ -1085,8 +1076,7 @@ enable_tpm(int enable, u32 *returnCode, int verbose)
rc = build_and_send_cmd(0, enable ? TPM_ORD_PhysicalEnable : TPM_ORD_PhysicalDisable, - NULL, 0, NULL, 0, returnCode, - TPM_DURATION_TYPE_SHORT); + NULL, 0, returnCode, TPM_DURATION_TYPE_SHORT); if (enable) dprintf(DEBUG_tcg, "Return code from TPM_PhysicalEnable = 0x%08x\n", *returnCode); @@ -1140,7 +1130,7 @@ activate_tpm(int activate, int allow_reset, u32 *returnCode, int verbose) : CommandFlag_TRUE, activate ? sizeof(CommandFlag_FALSE) : sizeof(CommandFlag_TRUE), - NULL, 0, returnCode, TPM_DURATION_TYPE_SHORT); + returnCode, TPM_DURATION_TYPE_SHORT);
dprintf(DEBUG_tcg, "Return code from PhysicalSetDeactivated(%d) = 0x%08x\n", @@ -1215,8 +1205,7 @@ force_clear(int enable_activate_before, int enable_activate_after, }
rc = build_and_send_cmd(0, TPM_ORD_ForceClear, - NULL, 0, NULL, 0, returnCode, - TPM_DURATION_TYPE_SHORT); + NULL, 0, returnCode, TPM_DURATION_TYPE_SHORT);
dprintf(DEBUG_tcg, "Return code from TPM_ForceClear() = 0x%08x\n", *returnCode); @@ -1280,7 +1269,7 @@ set_owner_install(int allow, u32 *returnCode, int verbose) (allow) ? CommandFlag_TRUE : CommandFlag_FALSE, sizeof(CommandFlag_TRUE), - NULL, 0, returnCode, TPM_DURATION_TYPE_SHORT); + returnCode, TPM_DURATION_TYPE_SHORT);
dprintf(DEBUG_tcg, "Return code from TPM_SetOwnerInstall() = 0x%08x\n", *returnCode);
The callers interested in the return status of build_and_send_cmd() are only interested in the command return code status (returnCode) and not the status of the message transmission (rc). Simplify the callers by returning returnCode directly instead of via a pointer parameter. For the unlikely case that the command transmission fails, return -1 to signal the failure to the callers.
Signed-off-by: Kevin O'Connor kevin@koconnor.net --- src/tcgbios.c | 139 ++++++++++++++++++++++++++++------------------------------ 1 file changed, 66 insertions(+), 73 deletions(-)
diff --git a/src/tcgbios.c b/src/tcgbios.c index c578fd2..ee5e9f7 100644 --- a/src/tcgbios.c +++ b/src/tcgbios.c @@ -171,7 +171,7 @@ tpm_is_working(void) */ static u32 build_and_send_cmd(u8 locty, u32 ordinal, const u8 *append, u32 append_size, - u32 *returnCode, enum tpmDurationType to_t) + enum tpmDurationType to_t) { struct { struct tpm_req_header trqh; @@ -195,30 +195,26 @@ build_and_send_cmd(u8 locty, u32 ordinal, const u8 *append, u32 append_size,
u32 rc = tpmhw_transmit(locty, &req.trqh, obuffer, &obuffer_len, to_t); if (rc) - return rc; - - *returnCode = be32_to_cpu(trsh->errcode); - return 0; + return -1; + return be32_to_cpu(trsh->errcode); }
static void tpm_set_failure(void) { - u32 returnCode; - /* we will try to deactivate the TPM now - ignoring all errors */ build_and_send_cmd(0, TPM_ORD_PhysicalPresence, PhysicalPresence_CMD_ENABLE, sizeof(PhysicalPresence_CMD_ENABLE), - &returnCode, TPM_DURATION_TYPE_SHORT); + TPM_DURATION_TYPE_SHORT);
build_and_send_cmd(0, TPM_ORD_PhysicalPresence, PhysicalPresence_PRESENT, sizeof(PhysicalPresence_PRESENT), - &returnCode, TPM_DURATION_TYPE_SHORT); + TPM_DURATION_TYPE_SHORT);
build_and_send_cmd(0, TPM_ORD_SetTempDeactivated, - NULL, 0, &returnCode, TPM_DURATION_TYPE_SHORT); + NULL, 0, TPM_DURATION_TYPE_SHORT);
TPM_working = 0; } @@ -429,44 +425,42 @@ tpm_startup(void) return TCG_GENERAL_ERROR;
dprintf(DEBUG_tcg, "TCGBIOS: Starting with TPM_Startup(ST_CLEAR)\n"); - rc = build_and_send_cmd(0, TPM_ORD_Startup, - Startup_ST_CLEAR, sizeof(Startup_ST_CLEAR), - &returnCode, TPM_DURATION_TYPE_SHORT); + returnCode = build_and_send_cmd(0, TPM_ORD_Startup, + Startup_ST_CLEAR, sizeof(Startup_ST_CLEAR), + TPM_DURATION_TYPE_SHORT);
dprintf(DEBUG_tcg, "Return code from TPM_Startup = 0x%08x\n", returnCode);
- if (CONFIG_COREBOOT) { + if (CONFIG_COREBOOT && returnCode == TPM_INVALID_POSTINIT) /* with other firmware on the system the TPM may already have been * initialized */ - if (returnCode == TPM_INVALID_POSTINIT) - returnCode = 0; - } + returnCode = 0;
- if (rc || returnCode) + if (returnCode) goto err_exit;
rc = determine_timeouts(); if (rc) goto err_exit;
- rc = build_and_send_cmd(0, TPM_ORD_SelfTestFull, NULL, 0, - &returnCode, TPM_DURATION_TYPE_LONG); + returnCode = build_and_send_cmd(0, TPM_ORD_SelfTestFull, NULL, 0, + TPM_DURATION_TYPE_LONG);
dprintf(DEBUG_tcg, "Return code from TPM_SelfTestFull = 0x%08x\n", returnCode);
- if (rc || returnCode) + if (returnCode) goto err_exit;
- rc = build_and_send_cmd(3, TSC_ORD_ResetEstablishmentBit, NULL, 0, - &returnCode, TPM_DURATION_TYPE_SHORT); + returnCode = build_and_send_cmd(3, TSC_ORD_ResetEstablishmentBit, NULL, 0, + TPM_DURATION_TYPE_SHORT);
dprintf(DEBUG_tcg, "Return code from TSC_ResetEstablishmentBit = 0x%08x\n", returnCode);
- if (rc || (returnCode != 0 && returnCode != TPM_BAD_LOCALITY)) + if (returnCode != 0 && returnCode != TPM_BAD_LOCALITY) goto err_exit;
rc = tpm_smbios_measure(); @@ -519,18 +513,18 @@ tpm_prepboot(void) if (!tpm_is_working()) return;
- rc = build_and_send_cmd(0, TPM_ORD_PhysicalPresence, - PhysicalPresence_CMD_ENABLE, - sizeof(PhysicalPresence_CMD_ENABLE), - &returnCode, TPM_DURATION_TYPE_SHORT); - if (rc || returnCode) + returnCode = build_and_send_cmd(0, TPM_ORD_PhysicalPresence, + PhysicalPresence_CMD_ENABLE, + sizeof(PhysicalPresence_CMD_ENABLE), + TPM_DURATION_TYPE_SHORT); + if (returnCode) goto err_exit;
- rc = build_and_send_cmd(0, TPM_ORD_PhysicalPresence, - PhysicalPresence_NOT_PRESENT_LOCK, - sizeof(PhysicalPresence_NOT_PRESENT_LOCK), - &returnCode, TPM_DURATION_TYPE_SHORT); - if (rc || returnCode) + returnCode = build_and_send_cmd(0, TPM_ORD_PhysicalPresence, + PhysicalPresence_NOT_PRESENT_LOCK, + sizeof(PhysicalPresence_NOT_PRESENT_LOCK), + TPM_DURATION_TYPE_SHORT); + if (returnCode) goto err_exit;
rc = tpm_add_action(4, "Calling INT 19h"); @@ -643,7 +637,6 @@ tpm_add_cdrom_catalog(const u8 *addr, u32 length) void tpm_s3_resume(void) { - u32 rc; u32 returnCode;
if (!tpm_is_working()) @@ -651,14 +644,14 @@ tpm_s3_resume(void)
dprintf(DEBUG_tcg, "TCGBIOS: Resuming with TPM_Startup(ST_STATE)\n");
- rc = build_and_send_cmd(0, TPM_ORD_Startup, - Startup_ST_STATE, sizeof(Startup_ST_STATE), - &returnCode, TPM_DURATION_TYPE_SHORT); + returnCode = build_and_send_cmd(0, TPM_ORD_Startup, + Startup_ST_STATE, sizeof(Startup_ST_STATE), + TPM_DURATION_TYPE_SHORT);
dprintf(DEBUG_tcg, "TCGBIOS: ReturnCode from TPM_Startup = 0x%08x\n", returnCode);
- if (rc || returnCode) + if (returnCode) goto err_exit;
return; @@ -984,31 +977,31 @@ assert_physical_presence(int verbose) /* physical presence already asserted */ return 0;
- rc = build_and_send_cmd(0, TPM_ORD_PhysicalPresence, - PhysicalPresence_CMD_ENABLE, - sizeof(PhysicalPresence_CMD_ENABLE), - &returnCode, TPM_DURATION_TYPE_SHORT); + returnCode = build_and_send_cmd(0, TPM_ORD_PhysicalPresence, + PhysicalPresence_CMD_ENABLE, + sizeof(PhysicalPresence_CMD_ENABLE), + TPM_DURATION_TYPE_SHORT);
dprintf(DEBUG_tcg, - "Return code from TSC_PhysicalPresence(CMD_ENABLE) = 0x%08x\n", - returnCode); + "Return code from TSC_PhysicalPresence(CMD_ENABLE) = 0x%08x\n", + returnCode);
- if (rc || returnCode) { + if (returnCode) { if (verbose) printf("Error: Could not enable physical presence.\n\n"); goto err_exit; }
- rc = build_and_send_cmd(0, TPM_ORD_PhysicalPresence, - PhysicalPresence_PRESENT, - sizeof(PhysicalPresence_PRESENT), - &returnCode, TPM_DURATION_TYPE_SHORT); + returnCode = build_and_send_cmd(0, TPM_ORD_PhysicalPresence, + PhysicalPresence_PRESENT, + sizeof(PhysicalPresence_PRESENT), + TPM_DURATION_TYPE_SHORT);
dprintf(DEBUG_tcg, - "Return code from TSC_PhysicalPresence(PRESENT) = 0x%08x\n", - returnCode); + "Return code from TSC_PhysicalPresence(PRESENT) = 0x%08x\n", + returnCode);
- if (rc || returnCode) { + if (returnCode) { if (verbose) printf("Error: Could not set presence flag.\n\n"); goto err_exit; @@ -1074,9 +1067,9 @@ enable_tpm(int enable, u32 *returnCode, int verbose) return rc; }
- rc = build_and_send_cmd(0, enable ? TPM_ORD_PhysicalEnable - : TPM_ORD_PhysicalDisable, - NULL, 0, returnCode, TPM_DURATION_TYPE_SHORT); + *returnCode = build_and_send_cmd(0, enable ? TPM_ORD_PhysicalEnable + : TPM_ORD_PhysicalDisable, + NULL, 0, TPM_DURATION_TYPE_SHORT); if (enable) dprintf(DEBUG_tcg, "Return code from TPM_PhysicalEnable = 0x%08x\n", *returnCode); @@ -1084,7 +1077,7 @@ enable_tpm(int enable, u32 *returnCode, int verbose) dprintf(DEBUG_tcg, "Return code from TPM_PhysicalDisable = 0x%08x\n", *returnCode);
- if (rc || *returnCode) + if (*returnCode) goto err_exit;
return 0; @@ -1125,18 +1118,18 @@ activate_tpm(int activate, int allow_reset, u32 *returnCode, int verbose) return rc; }
- rc = build_and_send_cmd(0, TPM_ORD_PhysicalSetDeactivated, - activate ? CommandFlag_FALSE - : CommandFlag_TRUE, - activate ? sizeof(CommandFlag_FALSE) - : sizeof(CommandFlag_TRUE), - returnCode, TPM_DURATION_TYPE_SHORT); + *returnCode = build_and_send_cmd(0, TPM_ORD_PhysicalSetDeactivated, + activate ? CommandFlag_FALSE + : CommandFlag_TRUE, + activate ? sizeof(CommandFlag_FALSE) + : sizeof(CommandFlag_TRUE), + TPM_DURATION_TYPE_SHORT);
dprintf(DEBUG_tcg, "Return code from PhysicalSetDeactivated(%d) = 0x%08x\n", activate ? 0 : 1, *returnCode);
- if (rc || *returnCode) + if (*returnCode) goto err_exit;
if (activate && allow_reset) { @@ -1204,13 +1197,13 @@ force_clear(int enable_activate_before, int enable_activate_after, return rc; }
- rc = build_and_send_cmd(0, TPM_ORD_ForceClear, - NULL, 0, returnCode, TPM_DURATION_TYPE_SHORT); + *returnCode = build_and_send_cmd(0, TPM_ORD_ForceClear, + NULL, 0, TPM_DURATION_TYPE_SHORT);
dprintf(DEBUG_tcg, "Return code from TPM_ForceClear() = 0x%08x\n", *returnCode);
- if (rc || *returnCode) + if (*returnCode) goto err_exit;
if (!enable_activate_after) { @@ -1265,16 +1258,16 @@ set_owner_install(int allow, u32 *returnCode, int verbose) return rc; }
- rc = build_and_send_cmd(0, TPM_ORD_SetOwnerInstall, - (allow) ? CommandFlag_TRUE : - CommandFlag_FALSE, - sizeof(CommandFlag_TRUE), - returnCode, TPM_DURATION_TYPE_SHORT); + *returnCode = build_and_send_cmd(0, TPM_ORD_SetOwnerInstall, + (allow) ? CommandFlag_TRUE + : CommandFlag_FALSE, + sizeof(CommandFlag_TRUE), + TPM_DURATION_TYPE_SHORT);
dprintf(DEBUG_tcg, "Return code from TPM_SetOwnerInstall() = 0x%08x\n", *returnCode);
- if (rc || *returnCode) + if (*returnCode) goto err_exit;
if (verbose)
On 12/29/2015 07:17 PM, Kevin O'Connor wrote:
The callers interested in the return status of build_and_send_cmd() are only interested in the command return code status (returnCode) and not the status of the message transmission (rc). Simplify the callers by returning returnCode directly instead of via a pointer parameter. For the unlikely case that the command transmission fails, return -1 to signal the failure to the callers.
Signed-off-by: Kevin O'Connor kevin@koconnor.net
I looked through all of the patches. I tested them (with the bug fixed) and from what I can see it still behaves the same way as before. So please, apply them. Any more changes coming from you ?
Stefan
On Tue, Dec 29, 2015 at 10:11:14PM -0500, Stefan Berger wrote:
On 12/29/2015 07:17 PM, Kevin O'Connor wrote:
The callers interested in the return status of build_and_send_cmd() are only interested in the command return code status (returnCode) and not the status of the message transmission (rc). Simplify the callers by returning returnCode directly instead of via a pointer parameter. For the unlikely case that the command transmission fails, return -1 to signal the failure to the callers.
Signed-off-by: Kevin O'Connor kevin@koconnor.net
I looked through all of the patches. I tested them (with the bug fixed) and from what I can see it still behaves the same way as before. So please, apply them. Any more changes coming from you ?
Thanks. I was looking at a couple of minor changes (mostly reducing redundant dprintf). Nothing important. I pushed my work tree to:
https://github.com/KevinOConnor/seabios/tree/testing
Did you have patches pending?
-Kevin
On Tue, Dec 29, 2015 at 07:17:40PM -0500, Kevin O'Connor wrote:
The following series involves some code reorganization in the TPM code that I found useful in understanding the code.
FYI, I committed patches 1-9 (after some bug fixes).
-Kevin
"Kevin O'Connor" kevin@koconnor.net wrote on 01/05/2016 12:16:18 PM:
On Tue, Dec 29, 2015 at 07:17:40PM -0500, Kevin O'Connor wrote:
The following series involves some code reorganization in the TPM code that I found useful in understanding the code.
FYI, I committed patches 1-9 (after some bug fixes).
The result of the 2nd patch set also looks good.
Regards, Stefan
-
Kevin O'Connor -
Stefan Berger -
Stefan Berger