[PATCH 0/8] TPM return code simplifications

[Kevin O'Connor]

Hi Stefan,

I cleaned up the additional patches I mentioned yesterday.

Some time back, you posted a series of patches that removed the use of
TCG 16bit BIOS structs from internal functions.  Many of those
functions were still using return codes from the spec though.  I found
using these return codes in internal functions to be annoying because
they conflict with the TIS return codes.  This series mostly just
separates out the return codes so that only the TCG 16bit BIOS
functions deal with the TCG 16bit BIOS return codes.

This series is on top of patches 1-9 of the previous series (it
replaces patch 10 of the previous series).  Both series are also
available at:

https://github.com/KevinOConnor/seabios/tree/testing

I'm not currently planning any other TPM changes after these.

-Kevin

Kevin O'Connor (8):
  tpm: Don't return a status from external bios measurement functions
  tpm: No need to check the return status of measurements
  tpm: Don't call tpm_set_failure() from tpm_log_extend_event()
  tpm: Don't use 16bit BIOS return codes in build_and_send_cmd()
  tpm: Don't use 16bit BIOS return codes in tpm_log_event()
  tpm: Don't use 16bit BIOS return codes in tpmhw_* functions
  tpm: Don't use 16bit BIOS return codes in TPM menu functions
  tpm: Replace build_and_send_cmd with tpm_send_cmd and
    tpm_send_check_cmd

src/hw/tpm_drivers.c |  20 +-
 src/hw/tpm_drivers.h |   2 +-
 src/tcgbios.c        | 718 +++++++++++++++++++--------------------------------
 src/tcgbios.h        |   8 +-
 4 files changed, 287 insertions(+), 461 deletions(-)

[PATCH 1/8] tpm: Don't return a status from external bios measurement functions

[Kevin O'Connor]

The callers of the measurements don't care what happens, so no need to
return a status.

Signed-off-by: Kevin O'Connor 
---
 src/tcgbios.c | 58 ++++++++++++++++++++++++++++------------------------------
 src/tcgbios.h |  8 ++++----
 2 files changed, 32 insertions(+), 34 deletions(-)

diff --git a/src/tcgbios.c b/src/tcgbios.c
index 39b0e6d..05189d4 100644
--- a/src/tcgbios.c
+++ b/src/tcgbios.c
@@ -552,43 +552,41 @@ err_exit:
 /*
  * Add measurement to the log about an option rom
  */
-u32
+void
 tpm_option_rom(const void *addr, u32 len)
 {
     if (!tpm_is_working())
-        return TCG_GENERAL_ERROR;
+        return;

-    u32 rc;
     struct pcctes_romex pcctes = {
         .eventid = 7,
         .eventdatasize = sizeof(u16) + sizeof(u16) + SHA1_BUFSIZE,
     };
-
-    rc = sha1((const u8 *)addr, len, pcctes.digest);
+    u32 rc = sha1((const u8 *)addr, len, pcctes.digest);
     if (rc)
-        return rc;
+        return;

-    return tpm_add_measurement_to_log(2,
-                                      EV_EVENT_TAG,
-                                      (const char *)&pcctes, sizeof(pcctes),
-                                      (u8 *)&pcctes, sizeof(pcctes));
+    tpm_add_measurement_to_log(2,
+                               EV_EVENT_TAG,
+                               (const char *)&pcctes, sizeof(pcctes),
+                               (u8 *)&pcctes, sizeof(pcctes));
 }

-u32
+void
 tpm_add_bcv(u32 bootdrv, const u8 *addr, u32 length)
 {
     if (!tpm_is_working())
-        return TCG_GENERAL_ERROR;
+        return;

if (length < 0x200)
-        return TCG_INVALID_INPUT_PARA;
+        return;

const char *string = "Booting BCV device 00h (Floppy)";
     if (bootdrv == 0x80)
         string = "Booting BCV device 80h (HDD)";
     u32 rc = tpm_add_action(4, string);
     if (rc)
-        return rc;
+        return;

/* specs: see section 'Hard Disk Device or Hard Disk-Like Devices' */
     /* equivalent to: dd if=/dev/hda ibs=1 count=440 | sha1sum */
@@ -597,47 +595,47 @@ tpm_add_bcv(u32 bootdrv, const u8 *addr, u32 length)
                                     string, strlen(string),
                                     addr, 0x1b8);
     if (rc)
-        return rc;
+        return;

/* equivalent to: dd if=/dev/hda ibs=1 count=72 skip=440 | sha1sum */
     string = "MBR PARTITION_TABLE";
-    return tpm_add_measurement_to_log(5, EV_IPL_PARTITION_DATA,
-                                      string, strlen(string),
-                                      addr + 0x1b8, 0x48);
+    tpm_add_measurement_to_log(5, EV_IPL_PARTITION_DATA,
+                               string, strlen(string),
+                               addr + 0x1b8, 0x48);
 }

-u32
+void
 tpm_add_cdrom(u32 bootdrv, const u8 *addr, u32 length)
 {
     if (!tpm_is_working())
-        return TCG_GENERAL_ERROR;
+        return;

u32 rc = tpm_add_action(4, "Booting from CD ROM device");
     if (rc)
-        return rc;
+        return;

/* specs: see section 'El Torito' */
     const char *string = "EL TORITO IPL";
-    return tpm_add_measurement_to_log(4, EV_IPL,
-                                      string, strlen(string),
-                                      addr, length);
+    tpm_add_measurement_to_log(4, EV_IPL,
+                               string, strlen(string),
+                               addr, length);
 }

-u32
+void
 tpm_add_cdrom_catalog(const u8 *addr, u32 length)
 {
     if (!tpm_is_working())
-        return TCG_GENERAL_ERROR;
+        return;

u32 rc = tpm_add_action(4, "Booting from CD ROM device");
     if (rc)
-        return rc;
+        return;

/* specs: see section 'El Torito' */
     const char *string = "BOOT CATALOG";
-    return tpm_add_measurement_to_log(5, EV_IPL_PARTITION_DATA,
-                                      string, strlen(string),
-                                      addr, length);
+    tpm_add_measurement_to_log(5, EV_IPL_PARTITION_DATA,
+                               string, strlen(string),
+                               addr, length);
 }

void
diff --git a/src/tcgbios.h b/src/tcgbios.h
index 7934fc3..6040b0c 100644
--- a/src/tcgbios.h
+++ b/src/tcgbios.h
@@ -9,10 +9,10 @@ void tpm_interrupt_handler32(struct bregs *regs);
 void tpm_setup(void);
 void tpm_prepboot(void);
 void tpm_s3_resume(void);
-u32 tpm_add_bcv(u32 bootdrv, const u8 *addr, u32 length);
-u32 tpm_add_cdrom(u32 bootdrv, const u8 *addr, u32 length);
-u32 tpm_add_cdrom_catalog(const u8 *addr, u32 length);
-u32 tpm_option_rom(const void *addr, u32 len);
+void tpm_add_bcv(u32 bootdrv, const u8 *addr, u32 length);
+void tpm_add_cdrom(u32 bootdrv, const u8 *addr, u32 length);
+void tpm_add_cdrom_catalog(const u8 *addr, u32 length);
+void tpm_option_rom(const void *addr, u32 len);
 int tpm_is_working(void);
 void tpm_menu(void);

[PATCH 2/8] tpm: No need to check the return status of measurements

[Kevin O'Connor]

The low-level measurement functions already handle error conditions,
there is no need to check for the errors in the high level measurement
functions.

Signed-off-by: Kevin O'Connor 
---
 src/tcgbios.c | 119 ++++++++++++++++++++--------------------------------------
 1 file changed, 40 insertions(+), 79 deletions(-)

diff --git a/src/tcgbios.c b/src/tcgbios.c
index 05189d4..ca2ada0 100644
--- a/src/tcgbios.c
+++ b/src/tcgbios.c
@@ -336,7 +336,7 @@ tpm_fill_hash(struct pcpes *pcpes, const void *hashdata, u32 hashdata_length)
  *  hashdata    : pointer to the data to be hashed
  *  hashdata_length: length of the data to be hashed
  */
-static u32
+static void
 tpm_add_measurement_to_log(u32 pcrindex, u32 event_type,
                            const char *event, u32 event_length,
                            const u8 *hashdata, u32 hashdata_length)
@@ -347,7 +347,7 @@ tpm_add_measurement_to_log(u32 pcrindex, u32 event_type,
         .eventdatasize = event_length,
     };
     tpm_fill_hash(&pcpes, hashdata, hashdata_length);
-    return tpm_log_extend_event(&pcpes, event);
+    tpm_log_extend_event(&pcpes, event);
 }

@@ -356,47 +356,34 @@ tpm_add_measurement_to_log(u32 pcrindex, u32 event_type,
  ****************************************************************/

// Add an EV_ACTION measurement to the list of measurements
-static u32
+static void
 tpm_add_action(u32 pcrIndex, const char *string)
 {
     u32 len = strlen(string);
-    return tpm_add_measurement_to_log(pcrIndex, EV_ACTION,
-                                      string, len, (u8 *)string, len);
+    tpm_add_measurement_to_log(pcrIndex, EV_ACTION,
+                               string, len, (u8 *)string, len);
 }

/*
  * Add event separators for PCRs 0 to 7; specs on 'Measuring Boot Events'
  */
-static u32
+static void
 tpm_add_event_separators(void)
 {
-    u32 rc;
     u32 pcrIndex = 0;
-
-    if (!tpm_is_working())
-        return TCG_GENERAL_ERROR;
-
-    static const u8 evt_separator[] = {0xff,0xff,0xff,0xff};
     while (pcrIndex <= 7) {
-        rc = tpm_add_measurement_to_log(pcrIndex, EV_SEPARATOR,
-                                        NULL, 0,
-                                        (u8 *)evt_separator,
-                                        sizeof(evt_separator));
-        if (rc)
-            break;
-        pcrIndex ++;
+        static const u8 evt_separator[] = {0xff,0xff,0xff,0xff};
+        tpm_add_measurement_to_log(pcrIndex, EV_SEPARATOR,
+                                   NULL, 0,
+                                   (u8 *)evt_separator,
+                                   sizeof(evt_separator));
+        pcrIndex++;
     }
-
-    return rc;
 }

-static u32
+static void
 tpm_smbios_measure(void)
 {
-    if (!tpm_is_working())
-        return TCG_GENERAL_ERROR;
-
-    u32 rc;
     struct pcctes pcctes = {
         .eventid = 1,
         .eventdatasize = SHA1_BUFSIZE,
@@ -406,28 +393,22 @@ tpm_smbios_measure(void)
     dprintf(DEBUG_tcg, "TCGBIOS: SMBIOS at %p\n", sep);

if (!sep)
-        return 0;
-
-    rc = sha1((const u8 *)sep->structure_table_address,
-              sep->structure_table_length, pcctes.digest);
-    if (rc)
-        return rc;
+        return;

-    return tpm_add_measurement_to_log(1,
-                                      EV_EVENT_TAG,
-                                      (const char *)&pcctes, sizeof(pcctes),
-                                      (u8 *)&pcctes, sizeof(pcctes));
+    sha1((const u8 *)sep->structure_table_address,
+         sep->structure_table_length, pcctes.digest);
+    tpm_add_measurement_to_log(1,
+                               EV_EVENT_TAG,
+                               (const char *)&pcctes, sizeof(pcctes),
+                               (u8 *)&pcctes, sizeof(pcctes));
 }

-static u32
+static int
 tpm_startup(void)
 {
     u32 rc;
     u32 returnCode;

-    if (!tpm_is_working())
-        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),
@@ -449,7 +430,7 @@ tpm_startup(void)

int ret = determine_timeouts();
     if (ret)
-        return TCG_TCG_COMMAND_ERROR;
+        return -1;

rc = build_and_send_cmd(0, TPM_ORD_SelfTestFull, NULL, 0,
                             &returnCode, TPM_DURATION_TYPE_LONG);
@@ -469,23 +450,13 @@ tpm_startup(void)
     if (rc || (returnCode != 0 && returnCode != TPM_BAD_LOCALITY))
         goto err_exit;

-    rc = tpm_smbios_measure();
-    if (rc)
-        goto err_exit;
-
-    rc = tpm_add_action(2, "Start Option ROM Scan");
-    if (rc)
-        goto err_exit;
-
     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;
+    return -1;
 }

void
@@ -507,7 +478,12 @@ tpm_setup(void)
     if (runningOnXen())
         return;

-    tpm_startup();
+    ret = tpm_startup();
+    if (ret)
+        return;
+
+    tpm_smbios_measure();
+    tpm_add_action(2, "Start Option ROM Scan");
 }

void
@@ -533,13 +509,8 @@ tpm_prepboot(void)
     if (rc || returnCode)
         goto err_exit;

-    rc = tpm_add_action(4, "Calling INT 19h");
-    if (rc)
-        goto err_exit;
-
-    rc = tpm_add_event_separators();
-    if (rc)
-        goto err_exit;
+    tpm_add_action(4, "Calling INT 19h");
+    tpm_add_event_separators();

return;

@@ -562,10 +533,7 @@ tpm_option_rom(const void *addr, u32 len)
         .eventid = 7,
         .eventdatasize = sizeof(u16) + sizeof(u16) + SHA1_BUFSIZE,
     };
-    u32 rc = sha1((const u8 *)addr, len, pcctes.digest);
-    if (rc)
-        return;
-
+    sha1((const u8 *)addr, len, pcctes.digest);
     tpm_add_measurement_to_log(2,
                                EV_EVENT_TAG,
                                (const char *)&pcctes, sizeof(pcctes),
@@ -584,18 +552,14 @@ tpm_add_bcv(u32 bootdrv, const u8 *addr, u32 length)
     const char *string = "Booting BCV device 00h (Floppy)";
     if (bootdrv == 0x80)
         string = "Booting BCV device 80h (HDD)";
-    u32 rc = tpm_add_action(4, string);
-    if (rc)
-        return;
+    tpm_add_action(4, string);

/* specs: see section 'Hard Disk Device or Hard Disk-Like Devices' */
     /* equivalent to: dd if=/dev/hda ibs=1 count=440 | sha1sum */
     string = "MBR";
-    rc = tpm_add_measurement_to_log(4, EV_IPL,
-                                    string, strlen(string),
-                                    addr, 0x1b8);
-    if (rc)
-        return;
+    tpm_add_measurement_to_log(4, EV_IPL,
+                               string, strlen(string),
+                               addr, 0x1b8);

/* equivalent to: dd if=/dev/hda ibs=1 count=72 skip=440 | sha1sum */
     string = "MBR PARTITION_TABLE";
@@ -610,9 +574,7 @@ tpm_add_cdrom(u32 bootdrv, const u8 *addr, u32 length)
     if (!tpm_is_working())
         return;

-    u32 rc = tpm_add_action(4, "Booting from CD ROM device");
-    if (rc)
-        return;
+    tpm_add_action(4, "Booting from CD ROM device");

/* specs: see section 'El Torito' */
     const char *string = "EL TORITO IPL";
@@ -627,9 +589,7 @@ tpm_add_cdrom_catalog(const u8 *addr, u32 length)
     if (!tpm_is_working())
         return;

-    u32 rc = tpm_add_action(4, "Booting from CD ROM device");
-    if (rc)
-        return;
+    tpm_add_action(4, "Booting from CD ROM device");

/* specs: see section 'El Torito' */
     const char *string = "BOOT CATALOG";
@@ -832,7 +792,8 @@ hash_all_int(const struct hai *hai, u8 *hash)
         hai->algorithmid != TPM_ALG_SHA)
         return TCG_INVALID_INPUT_PARA;

-    return sha1((const u8 *)hai->hashdataptr, hai->hashdatalen, hash);
+    sha1((const u8 *)hai->hashdataptr, hai->hashdatalen, hash);
+    return 0;
 }

static u32

[PATCH 2/8] tpm: No need to check the return status of measurements

[Stefan Berger]

On 12/30/2015 02:31 PM, Kevin O'Connor wrote:
> The low-level measurement functions already handle error conditions,
> there is no need to check for the errors in the high level measurement
> functions.
> Signed-off-by: Kevin O'Connor
> @@ -507,7 +478,12 @@ tpm_setup(void)
>       if (runningOnXen())
>           return;

This looks like a for Xen where we will be missing too much. Cc'in Xu 
Quan for this. I think Xen likely only wants to skip the TPM_Startup in 
tpm_startup() but not the retrieval of the durations and timeouts and so 
on. This part may need to move.
> tpm_startup();
> ret = tpm_startup();
> if (ret)
>    return;
> tpm_smbios_measure();
> tpm_add_action(2, "Start Option ROM Scan");
> }

Stefan

[PATCH 2/8] tpm: No need to check the return status of measurements

[Kevin O'Connor]

On Wed, Dec 30, 2015 at 07:09:54PM -0500, Stefan Berger wrote:
> On 12/30/2015 02:31 PM, Kevin O'Connor wrote:
> > The low-level measurement functions already handle error conditions,
> > there is no need to check for the errors in the high level measurement
> > functions.
> > Signed-off-by: Kevin O'Connor
> > @@ -507,7 +478,12 @@ tpm_setup(void)
> >      if (runningOnXen())
> >          return;
>
> This looks like a for Xen where we will be missing too much. Cc'in Xu Quan
> for this. I think Xen likely only wants to skip the TPM_Startup in
> tpm_startup() but not the retrieval of the durations and timeouts and so on.
> This part may need to move.

That makes sense, but note that the position of the check hasn't
changed in this series.

I wonder if the error from TPM_Startup on Xen could just be ignored as
is done for CONFIG_COREBOOT.

-Kevin

[PATCH 2/8] tpm: No need to check the return status of measurements

[Xu, Quan]

On 31.12.2015 at 8:10am,  wrote:
> On 12/30/2015 02:31 PM, Kevin O'Connor wrote:
> > The low-level measurement functions already handle error conditions,
> > there is no need to check for the errors in the high level measurement
> > functions.
> > Signed-off-by: Kevin O'Connor
> > @@ -507,7 +478,12 @@ tpm_setup(void)
> >       if (runningOnXen())
> >           return;
>
> This looks like a for Xen where we will be missing too much. Cc'in Xu Quan for
> this. I think Xen likely only wants to skip the TPM_Startup in
> tpm_startup() but not the retrieval of the durations and timeouts and so on. This
> part may need to move.

IMO, I think it is still need these code for Xen vtpm.
HVM virtual machine's tpm_tis driver and PV virtual machine's xen_tpmfront driver get the timeout/durations based on 'TPM_GetCapability' cmd.
It doesn't base on seabios.

Now the seabios provides 2 functions for xen vtpm:
  1. ACPI
  2.Initialize some registers.

Cced Graaf for double check.
> > tpm_startup();
> > ret = tpm_startup();
> > if (ret)
> >    return;
> > tpm_smbios_measure();
> > tpm_add_action(2, "Start Option ROM Scan");
> > }

So for, it does not support passing on TPM cmd to vTPM in seabios. It can't extend/write these measure to vTPM on Xen. 
So It does not necessary for Xen vtpm now.

-Quan

[PATCH 3/8] tpm: Don't call tpm_set_failure() from tpm_log_extend_event()

[Kevin O'Connor]

The 16bit BIOS interface shouldn't be able to shutdown the TPM.  Move
the check for tpm_is_working() and tpm_set_failure() to the only
caller of tpm_log_extend_event() that may shutdown the TPM.

Signed-off-by: Kevin O'Connor 
---
 src/tcgbios.c | 21 +++++++++------------
 1 file changed, 9 insertions(+), 12 deletions(-)

diff --git a/src/tcgbios.c b/src/tcgbios.c
index ca2ada0..dd30593 100644
--- a/src/tcgbios.c
+++ b/src/tcgbios.c
@@ -288,9 +288,6 @@ determine_timeouts(void)
 static u32
 tpm_log_extend_event(struct pcpes *pcpes, const void *event)
 {
-    if (!tpm_is_working())
-        return TCG_GENERAL_ERROR;
-
     if (pcpes->pcrindex >= 24)
         return TCG_INVALID_INPUT_PARA;

@@ -306,15 +303,10 @@ tpm_log_extend_event(struct pcpes *pcpes, const void *event)
     u32 resp_length = sizeof(rsp);
     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;
-    }
+    if (rc || resp_length != sizeof(rsp) || rsp.hdr.errcode)
+        return rc ?: TCG_TCG_COMMAND_ERROR;

-    rc = tpm_log_event(pcpes, event);
-    if (rc)
-        tpm_set_failure();
-    return rc;
+    return tpm_log_event(pcpes, event);
 }

static void
@@ -341,13 +333,18 @@ tpm_add_measurement_to_log(u32 pcrindex, u32 event_type,
                            const char *event, u32 event_length,
                            const u8 *hashdata, u32 hashdata_length)
 {
+    if (!tpm_is_working())
+        return;
+
     struct pcpes pcpes = {
         .pcrindex = pcrindex,
         .eventtype = event_type,
         .eventdatasize = event_length,
     };
     tpm_fill_hash(&pcpes, hashdata, hashdata_length);
-    tpm_log_extend_event(&pcpes, event);
+    u32 rc = tpm_log_extend_event(&pcpes, event);
+    if (rc)
+        tpm_set_failure();
 }

[PATCH 4/8] tpm: Don't use 16bit BIOS return codes in build_and_send_cmd()

[Kevin O'Connor]

Don't use the return codes from the 16bit BIOS spec in the internal
function build_and_send_cmd().  Instead, return the TIS command status
code of the command or -1 if there was a command transmission failure.
This eliminates the need for a returnCode pointer parameter.

Also, implement debugging dprintf() in build_and_send_cmd() instead of
in every caller.  This replaces the command name with the integer
command id, but it does make the debugging more consistent.

Signed-off-by: Kevin O'Connor 
---
 src/tcgbios.c | 216 ++++++++++++++++++++--------------------------------------
 1 file changed, 73 insertions(+), 143 deletions(-)

diff --git a/src/tcgbios.c b/src/tcgbios.c
index dd30593..e73636e 100644
--- a/src/tcgbios.c
+++ b/src/tcgbios.c
@@ -169,13 +169,13 @@ tpm_is_working(void)
  * 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.
  */
-static u32
+static int
 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;
-        u8 cmd[20];
+        u8 cmd[2];
     } PACKED req = {
         .trqh.tag = cpu_to_be16(TPM_TAG_RQU_CMD),
         .trqh.totlen = cpu_to_be32(sizeof(req.trqh) + append_size),
@@ -188,37 +188,34 @@ build_and_send_cmd(u8 locty, u32 ordinal, const u8 *append, u32 append_size,

if (append_size > sizeof(req.cmd)) {
         warn_internalerror();
-        return TCG_FIRMWARE_ERROR;
+        return -1;
     }
     if (append_size)
         memcpy(req.cmd, append, 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;
+    int ret = rc ? -1 : be32_to_cpu(trsh->errcode);
+    dprintf(DEBUG_tcg, "Return from build_and_send_cmd(%x, %x %x) = %x\n",
+            ordinal, req.cmd[0], req.cmd[1], ret);
+    return ret;
 }

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;
 }
@@ -403,48 +400,30 @@ tpm_smbios_measure(void)
 static int
 tpm_startup(void)
 {
-    u32 rc;
-    u32 returnCode;
-
     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);
-
-    dprintf(DEBUG_tcg, "Return code from TPM_Startup = 0x%08x\n",
-            returnCode);
-
-    if (CONFIG_COREBOOT) {
+    int ret = build_and_send_cmd(0, TPM_ORD_Startup,
+                                 Startup_ST_CLEAR, sizeof(Startup_ST_CLEAR),
+                                 TPM_DURATION_TYPE_SHORT);
+    if (CONFIG_COREBOOT && ret == TPM_INVALID_POSTINIT)
         /* with other firmware on the system the TPM may already have been
          * initialized
          */
-        if (returnCode == TPM_INVALID_POSTINIT)
-            returnCode = 0;
-    }
-
-    if (rc || returnCode)
+        ret = 0;
+    if (ret)
         goto err_exit;

-    int ret = determine_timeouts();
+    ret = determine_timeouts();
     if (ret)
         return -1;

-    rc = build_and_send_cmd(0, TPM_ORD_SelfTestFull, NULL, 0,
-                            &returnCode, TPM_DURATION_TYPE_LONG);
-
-    dprintf(DEBUG_tcg, "Return code from TPM_SelfTestFull = 0x%08x\n",
-            returnCode);
-
-    if (rc || returnCode)
+    ret = build_and_send_cmd(0, TPM_ORD_SelfTestFull, NULL, 0,
+                             TPM_DURATION_TYPE_LONG);
+    if (ret)
         goto err_exit;

-    rc = build_and_send_cmd(3, TSC_ORD_ResetEstablishmentBit, NULL, 0,
-                            &returnCode, TPM_DURATION_TYPE_SHORT);
-
-    dprintf(DEBUG_tcg, "Return code from TSC_ResetEstablishmentBit = 0x%08x\n",
-            returnCode);
-
-    if (rc || (returnCode != 0 && returnCode != TPM_BAD_LOCALITY))
+    ret = build_and_send_cmd(3, TSC_ORD_ResetEstablishmentBit, NULL, 0,
+                             TPM_DURATION_TYPE_SHORT);
+    if (ret && ret != TPM_BAD_LOCALITY)
         goto err_exit;

return 0;
@@ -486,24 +465,21 @@ tpm_setup(void)
 void
 tpm_prepboot(void)
 {
-    u32 rc;
-    u32 returnCode;
-
     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)
+    int ret = build_and_send_cmd(0, TPM_ORD_PhysicalPresence,
+                                 PhysicalPresence_CMD_ENABLE,
+                                 sizeof(PhysicalPresence_CMD_ENABLE),
+                                 TPM_DURATION_TYPE_SHORT);
+    if (ret)
         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)
+    ret = build_and_send_cmd(0, TPM_ORD_PhysicalPresence,
+                             PhysicalPresence_NOT_PRESENT_LOCK,
+                             sizeof(PhysicalPresence_NOT_PRESENT_LOCK),
+                             TPM_DURATION_TYPE_SHORT);
+    if (ret)
         goto err_exit;

tpm_add_action(4, "Calling INT 19h");
@@ -598,22 +574,15 @@ tpm_add_cdrom_catalog(const u8 *addr, u32 length)
 void
 tpm_s3_resume(void)
 {
-    u32 rc;
-    u32 returnCode;
-
     if (!tpm_is_working())
         return;

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);
-
-    dprintf(DEBUG_tcg, "TCGBIOS: ReturnCode from TPM_Startup = 0x%08x\n",
-            returnCode);
-
-    if (rc || returnCode)
+    int ret = build_and_send_cmd(0, TPM_ORD_Startup,
+                                 Startup_ST_STATE, sizeof(Startup_ST_STATE),
+                                 TPM_DURATION_TYPE_SHORT);
+    if (ret)
         goto err_exit;

return;
@@ -925,11 +894,8 @@ read_stclear_flags(char *buf, int buf_len)
 static u32
 assert_physical_presence(int verbose)
 {
-    u32 rc = 0;
-    u32 returnCode;
     struct tpm_stclear_flags stcf;
-
-    rc = read_stclear_flags((char *)&stcf, sizeof(stcf));
+    u32 rc = read_stclear_flags((char *)&stcf, sizeof(stcf));
     if (rc) {
         dprintf(DEBUG_tcg,
                 "Error reading STClear flags: 0x%08x\n", rc);
@@ -940,31 +906,21 @@ 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);
-
-    dprintf(DEBUG_tcg,
-           "Return code from TSC_PhysicalPresence(CMD_ENABLE) = 0x%08x\n",
-           returnCode);
-
-    if (rc || returnCode) {
+    int ret = build_and_send_cmd(0, TPM_ORD_PhysicalPresence,
+                                 PhysicalPresence_CMD_ENABLE,
+                                 sizeof(PhysicalPresence_CMD_ENABLE),
+                                 TPM_DURATION_TYPE_SHORT);
+    if (ret) {
         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);
-
-    dprintf(DEBUG_tcg,
-           "Return code from TSC_PhysicalPresence(PRESENT) = 0x%08x\n",
-           returnCode);
-
-    if (rc || returnCode) {
+    ret = build_and_send_cmd(0, TPM_ORD_PhysicalPresence,
+                             PhysicalPresence_PRESENT,
+                             sizeof(PhysicalPresence_PRESENT),
+                             TPM_DURATION_TYPE_SHORT);
+    if (ret) {
         if (verbose)
             printf("Error: Could not set presence flag.\n\n");
         goto err_exit;
@@ -976,8 +932,6 @@ err_exit:
     dprintf(DEBUG_tcg, "TCGBIOS: TPM malfunctioning (line %d).\n", __LINE__);

tpm_set_failure();
-    if (rc)
-        return rc;
     return TCG_TCG_COMMAND_ERROR;
 }

@@ -1030,17 +984,11 @@ 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);
-    if (enable)
-        dprintf(DEBUG_tcg, "Return code from TPM_PhysicalEnable = 0x%08x\n",
-                *returnCode);
-    else
-        dprintf(DEBUG_tcg, "Return code from TPM_PhysicalDisable = 0x%08x\n",
-                *returnCode);
-
-    if (rc || *returnCode)
+    int ret = build_and_send_cmd(0, enable ? TPM_ORD_PhysicalEnable
+                                           : TPM_ORD_PhysicalDisable,
+                                 NULL, 0, TPM_DURATION_TYPE_SHORT);
+    *returnCode = ret;
+    if (ret)
         goto err_exit;

return 0;
@@ -1054,8 +1002,6 @@ err_exit:
     dprintf(DEBUG_tcg, "TCGBIOS: TPM malfunctioning (line %d).\n", __LINE__);

tpm_set_failure();
-    if (rc)
-        return rc;
     return TCG_TCG_COMMAND_ERROR;
 }

@@ -1081,18 +1027,14 @@ 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);
-
-    dprintf(DEBUG_tcg,
-            "Return code from PhysicalSetDeactivated(%d) = 0x%08x\n",
-            activate ? 0 : 1, *returnCode);
-
-    if (rc || *returnCode)
+    int ret = build_and_send_cmd(0, TPM_ORD_PhysicalSetDeactivated,
+                                 activate ? CommandFlag_FALSE
+                                          : CommandFlag_TRUE,
+                                 activate ? sizeof(CommandFlag_FALSE)
+                                          : sizeof(CommandFlag_TRUE),
+                                 TPM_DURATION_TYPE_SHORT);
+    *returnCode = ret;
+    if (ret)
         goto err_exit;

if (activate && allow_reset) {
@@ -1110,8 +1052,6 @@ err_exit:
     dprintf(DEBUG_tcg, "TCGBIOS: TPM malfunctioning (line %d).\n", __LINE__);

tpm_set_failure();
-    if (rc)
-        return rc;
     return TCG_TCG_COMMAND_ERROR;
 }

@@ -1160,13 +1100,10 @@ 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);
-
-    dprintf(DEBUG_tcg, "Return code from TPM_ForceClear() = 0x%08x\n",
-            *returnCode);
-
-    if (rc || *returnCode)
+    int ret = build_and_send_cmd(0, TPM_ORD_ForceClear,
+                                 NULL, 0, TPM_DURATION_TYPE_SHORT);
+    *returnCode = ret;
+    if (ret)
         goto err_exit;

if (!enable_activate_after) {
@@ -1184,8 +1121,6 @@ err_exit:
     dprintf(DEBUG_tcg, "TCGBIOS: TPM malfunctioning (line %d).\n", __LINE__);

tpm_set_failure();
-    if (rc)
-        return rc;
     return TCG_TCG_COMMAND_ERROR;
 }

@@ -1221,16 +1156,13 @@ 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);
-
-    dprintf(DEBUG_tcg, "Return code from TPM_SetOwnerInstall() = 0x%08x\n",
-            *returnCode);
-
-    if (rc || *returnCode)
+    int ret = build_and_send_cmd(0, TPM_ORD_SetOwnerInstall,
+                                 (allow) ? CommandFlag_TRUE
+                                         : CommandFlag_FALSE,
+                                 sizeof(CommandFlag_TRUE),
+                                 TPM_DURATION_TYPE_SHORT);
+    *returnCode = ret;
+    if (ret)
         goto err_exit;

if (verbose)
@@ -1241,8 +1173,6 @@ set_owner_install(int allow, u32 *returnCode, int verbose)
 err_exit:
     dprintf(DEBUG_tcg, "TCGBIOS: TPM malfunctioning (line %d).\n", __LINE__);
     tpm_set_failure();
-    if (rc)
-        return rc;
     return TCG_TCG_COMMAND_ERROR;
 }

[PATCH 5/8] tpm: Don't use 16bit BIOS return codes in tpm_log_event()

[Kevin O'Connor]

Don't use the return codes from the 16bit BIOS spec in the internal
tpm_log_event() and tpm_log_extend_event() functions.  Only the 16bit
BIOS interface code should need to handle the details of that spec.

Signed-off-by: Kevin O'Connor 
---
 src/tcgbios.c | 38 +++++++++++++++++++++-----------------
 1 file changed, 21 insertions(+), 17 deletions(-)

diff --git a/src/tcgbios.c b/src/tcgbios.c
index e73636e..da104e4 100644
--- a/src/tcgbios.c
+++ b/src/tcgbios.c
@@ -123,21 +123,21 @@ tpm_tcpa_probe(void)
  * Output:
  *  Returns an error code in case of faiure, 0 in case of success
  */
-static u32
+static int
 tpm_log_event(struct pcpes *pcpes, const void *event)
 {
     dprintf(DEBUG_tcg, "TCGBIOS: LASA = %p, next entry = %p\n",
             tpm_state.log_area_start_address, tpm_state.log_area_next_entry);

if (tpm_state.log_area_next_entry == NULL)
-        return TCG_PC_LOGOVERFLOW;
+        return -1;

u32 size = sizeof(*pcpes) + pcpes->eventdatasize;

if ((tpm_state.log_area_next_entry + size - tpm_state.log_area_start_address) >
          tpm_state.log_area_minimum_length) {
         dprintf(DEBUG_tcg, "TCGBIOS: LOG OVERFLOW: size = %d\n", size);
-        return TCG_PC_LOGOVERFLOW;
+        return -1;
     }

memcpy(tpm_state.log_area_next_entry, pcpes, sizeof(*pcpes));
@@ -282,11 +282,11 @@ determine_timeouts(void)
     return 0;
 }

-static u32
+static int
 tpm_log_extend_event(struct pcpes *pcpes, const void *event)
 {
     if (pcpes->pcrindex >= 24)
-        return TCG_INVALID_INPUT_PARA;
+        return -1;

struct tpm_req_extend tre = {
         .hdr.tag     = cpu_to_be16(TPM_TAG_RQU_CMD),
@@ -301,7 +301,7 @@ tpm_log_extend_event(struct pcpes *pcpes, const void *event)
     u32 rc = tpmhw_transmit(0, &tre.hdr, &rsp, &resp_length,
                             TPM_DURATION_TYPE_SHORT);
     if (rc || resp_length != sizeof(rsp) || rsp.hdr.errcode)
-        return rc ?: TCG_TCG_COMMAND_ERROR;
+        return -1;

return tpm_log_event(pcpes, event);
 }
@@ -339,8 +339,8 @@ tpm_add_measurement_to_log(u32 pcrindex, u32 event_type,
         .eventdatasize = event_length,
     };
     tpm_fill_hash(&pcpes, hashdata, hashdata_length);
-    u32 rc = tpm_log_extend_event(&pcpes, event);
-    if (rc)
+    int ret = tpm_log_extend_event(&pcpes, event);
+    if (ret)
         tpm_set_failure();
 }

@@ -653,9 +653,11 @@ hash_log_extend_event_int(const struct hleei_short *hleei_s,
     }

tpm_fill_hash(pcpes, hleei_s->hashdataptr, hleei_s->hashdatalen);
-    rc = tpm_log_extend_event(pcpes, pcpes->event);
-    if (rc)
+    int ret = tpm_log_extend_event(pcpes, pcpes->event);
+    if (ret) {
+        rc = TCG_TCG_COMMAND_ERROR;
         goto err_exit;
+    }

hleeo->opblength = sizeof(struct hleeo);
     hleeo->reserved  = 0;
@@ -731,9 +733,11 @@ hash_log_event_int(const struct hlei *hlei, struct hleo *hleo)
     }

tpm_fill_hash(pcpes, hlei->hashdataptr, hlei->hashdatalen);
-    rc = tpm_log_event(pcpes, pcpes->event);
-    if (rc)
+    int ret = tpm_log_event(pcpes, pcpes->event);
+    if (ret) {
+        rc = TCG_PC_LOGOVERFLOW;
         goto err_exit;
+    }

/* updating the log was fine */
     hleo->opblength = sizeof(struct hleo);
@@ -785,11 +789,11 @@ compact_hash_log_extend_event_int(u8 *buffer,
     };

tpm_fill_hash(&pcpes, buffer, length);
-    u32 rc = tpm_log_extend_event(&pcpes, &info);
-    if (rc == 0)
-        *edx_ptr = tpm_state.entry_count;
-
-    return rc;
+    int ret = tpm_log_extend_event(&pcpes, &info);
+    if (ret)
+        return TCG_TCG_COMMAND_ERROR;
+    *edx_ptr = tpm_state.entry_count;
+    return 0;
 }

void VISIBLE32FLAT

[PATCH 6/8] tpm: Don't use 16bit BIOS return codes in tpmhw_* functions

[Kevin O'Connor]

Don't use the return codes from the 16bit BIOS spec in the internal
tpmhw functions.  Only the 16bit BIOS interface code should need to
handle the details of that spec.

Signed-off-by: Kevin O'Connor 
---
 src/hw/tpm_drivers.c | 20 ++++++++++----------
 src/hw/tpm_drivers.h |  2 +-
 src/tcgbios.c        | 22 ++++++++++++----------
 3 files changed, 23 insertions(+), 21 deletions(-)

diff --git a/src/hw/tpm_drivers.c b/src/hw/tpm_drivers.c
index dc09779..733c161 100644
--- a/src/hw/tpm_drivers.c
+++ b/src/hw/tpm_drivers.c
@@ -10,7 +10,7 @@
 #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
+#include "std/tcg.h" // TCG_RESPONSE_TIMEOUT
 #include "output.h" // warn_timeout
 #include "stacks.h" // yield
 #include "string.h" // memcpy
@@ -280,7 +280,7 @@ static u32 tis_waitdatavalid(void)
     u32 timeout_c = tpm_drivers[TIS_DRIVER_IDX].timeouts[TIS_TIMEOUT_TYPE_C];

if (tis_wait_sts(locty, timeout_c, TIS_STS_VALID, TIS_STS_VALID) != 0)
-        rc = TCG_NO_RESPONSE;
+        rc = 1;

return rc;
 }
@@ -298,7 +298,7 @@ static u32 tis_waitrespready(enum tpmDurationType to_t)

if (tis_wait_sts(locty, timeout,
                      TIS_STS_DATA_AVAILABLE, TIS_STS_DATA_AVAILABLE) != 0)
-        rc = TCG_NO_RESPONSE;
+        rc = 1;

return rc;
 }
@@ -338,37 +338,37 @@ tpmhw_probe(void)
     return -1;
 }

-u32
+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 TCG_FATAL_COM_ERROR;
+        return -1;

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;
+        return -1;
     }

irc = td->senddata((void*)req, be32_to_cpu(req->totlen));
     if (irc != 0)
-        return TCG_FATAL_COM_ERROR;
+        return -1;

irc = td->waitdatavalid();
     if (irc != 0)
-        return TCG_FATAL_COM_ERROR;
+        return -1;

irc = td->waitrespready(to_t);
     if (irc != 0)
-        return TCG_FATAL_COM_ERROR;
+        return -1;

irc = td->readresp(respbuffer, respbufferlen);
     if (irc != 0)
-        return TCG_FATAL_COM_ERROR;
+        return -1;

td->ready();

diff --git a/src/hw/tpm_drivers.h b/src/hw/tpm_drivers.h
index afa3027..47909e2 100644
--- a/src/hw/tpm_drivers.h
+++ b/src/hw/tpm_drivers.h
@@ -12,7 +12,7 @@ enum tpmDurationType {

int tpmhw_probe(void);
 struct tpm_req_header;
-u32 tpmhw_transmit(u8 locty, struct tpm_req_header *req,
+int 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]);
diff --git a/src/tcgbios.c b/src/tcgbios.c
index da104e4..b31de39 100644
--- a/src/tcgbios.c
+++ b/src/tcgbios.c
@@ -193,8 +193,8 @@ 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 = tpmhw_transmit(locty, &req.trqh, obuffer, &obuffer_len, to_t);
-    int ret = rc ? -1 : be32_to_cpu(trsh->errcode);
+    int ret = tpmhw_transmit(locty, &req.trqh, obuffer, &obuffer_len, to_t);
+    ret = ret ? -1 : be32_to_cpu(trsh->errcode);
     dprintf(DEBUG_tcg, "Return from build_and_send_cmd(%x, %x %x) = %x\n",
             ordinal, req.cmd[0], req.cmd[1], ret);
     return ret;
@@ -232,9 +232,9 @@ tpm_get_capability(u32 cap, u32 subcap, struct tpm_rsp_header *rsp, u32 rsize)
         .subCap = cpu_to_be32(subcap)
     };
     u32 resp_size = rsize;
-    u32 rc = tpmhw_transmit(0, &trgc.hdr, rsp, &resp_size,
-                            TPM_DURATION_TYPE_SHORT);
-    int ret = (rc || resp_size != rsize) ? -1 : be32_to_cpu(rsp->errcode);
+    int ret = tpmhw_transmit(0, &trgc.hdr, rsp, &resp_size,
+                             TPM_DURATION_TYPE_SHORT);
+    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);
     if (ret) {
@@ -298,9 +298,9 @@ tpm_log_extend_event(struct pcpes *pcpes, const void *event)

struct tpm_rsp_extend rsp;
     u32 resp_length = sizeof(rsp);
-    u32 rc = tpmhw_transmit(0, &tre.hdr, &rsp, &resp_length,
+    int ret = tpmhw_transmit(0, &tre.hdr, &rsp, &resp_length,
                             TPM_DURATION_TYPE_SHORT);
-    if (rc || resp_length != sizeof(rsp) || rsp.hdr.errcode)
+    if (ret || resp_length != sizeof(rsp) || rsp.hdr.errcode)
         return -1;

return tpm_log_event(pcpes, event);
@@ -686,10 +686,12 @@ pass_through_to_tpm_int(struct pttti *pttti, struct pttto *pttto)
     }

u32 resbuflen = pttti->opblength - offsetof(struct pttto, tpmopout);
-    rc = tpmhw_transmit(0, trh, pttto->tpmopout, &resbuflen,
-                        TPM_DURATION_TYPE_LONG /* worst case */);
-    if (rc)
+    int ret = tpmhw_transmit(0, trh, pttto->tpmopout, &resbuflen,
+                             TPM_DURATION_TYPE_LONG /* worst case */);
+    if (ret) {
+        rc = TCG_FATAL_COM_ERROR;
         goto err_exit;
+    }

pttto->opblength = offsetof(struct pttto, tpmopout) + resbuflen;
     pttto->reserved  = 0;

[PATCH 7/8] tpm: Don't use 16bit BIOS return codes in TPM menu functions

[Kevin O'Connor]

Don't use the return codes from the 16bit BIOS spec in the internal
menu functions.  Only the 16bit BIOS interface code should need to
handle the details of that spec.  For functions that need to return
the TIS command status, return those codes directly instead of via a
pointer parameter.

Signed-off-by: Kevin O'Connor 
---
 src/tcgbios.c | 227 +++++++++++++++++++++++++---------------------------------
 1 file changed, 97 insertions(+), 130 deletions(-)

diff --git a/src/tcgbios.c b/src/tcgbios.c
index b31de39..cd6a433 100644
--- a/src/tcgbios.c
+++ b/src/tcgbios.c
@@ -889,8 +889,10 @@ read_stclear_flags(char *buf, int buf_len)
     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 TCG_TCG_COMMAND_ERROR;
+    if (ret) {
+        dprintf(DEBUG_tcg, "Error reading STClear flags: 0x%08x\n", ret);
+        return -1;
+    }

memcpy(buf, &stcf.stclear_flags, buf_len);

@@ -901,21 +903,18 @@ static u32
 assert_physical_presence(int verbose)
 {
     struct tpm_stclear_flags stcf;
-    u32 rc = read_stclear_flags((char *)&stcf, sizeof(stcf));
-    if (rc) {
-        dprintf(DEBUG_tcg,
-                "Error reading STClear flags: 0x%08x\n", rc);
-        return rc;
-    }
+    int ret = read_stclear_flags((char *)&stcf, sizeof(stcf));
+    if (ret)
+        return -1;

if (stcf.flags[STCLEAR_FLAG_IDX_PHYSICAL_PRESENCE])
         /* physical presence already asserted */
         return 0;

-    int ret = build_and_send_cmd(0, TPM_ORD_PhysicalPresence,
-                                 PhysicalPresence_CMD_ENABLE,
-                                 sizeof(PhysicalPresence_CMD_ENABLE),
-                                 TPM_DURATION_TYPE_SHORT);
+    ret = build_and_send_cmd(0, TPM_ORD_PhysicalPresence,
+                             PhysicalPresence_CMD_ENABLE,
+                             sizeof(PhysicalPresence_CMD_ENABLE),
+                             TPM_DURATION_TYPE_SHORT);
     if (ret) {
         if (verbose)
             printf("Error: Could not enable physical presence.\n\n");
@@ -936,12 +935,12 @@ assert_physical_presence(int verbose)

err_exit:
     dprintf(DEBUG_tcg, "TCGBIOS: TPM malfunctioning (line %d).\n", __LINE__);
-
     tpm_set_failure();
-    return TCG_TCG_COMMAND_ERROR;
+    dprintf(DEBUG_tcg, "TCGBIOS: Asserting physical presence failed: %x\n", ret);
+    return -1;
 }

-static u32
+static int
 read_permanent_flags(char *buf, int buf_len)
 {
     memset(buf, 0, buf_len);
@@ -950,50 +949,45 @@ read_permanent_flags(char *buf, int buf_len)
     int ret = tpm_get_capability(TPM_CAP_FLAG, TPM_CAP_FLAG_PERMANENT
                                  , &pf.hdr, sizeof(pf));
     if (ret)
-        return TCG_TCG_COMMAND_ERROR;
+        return -1;

memcpy(buf, &pf.perm_flags, buf_len);

return 0;
 }

-static u32
+static int
 read_has_owner(int *has_owner)
 {
     struct tpm_res_getcap_ownerauth oauth;
     int ret = tpm_get_capability(TPM_CAP_PROPERTY, TPM_CAP_PROP_OWNER
                                  , &oauth.hdr, sizeof(oauth));
     if (ret)
-        return TCG_TCG_COMMAND_ERROR;
+        return -1;

*has_owner = oauth.flag;

return 0;
 }

-static u32
-enable_tpm(int enable, u32 *returnCode, int verbose)
+static int
+enable_tpm(int enable, int verbose)
 {
-    u32 rc;
     struct tpm_permanent_flags pf;
-
-    rc = read_permanent_flags((char *)&pf, sizeof(pf));
-    if (rc)
-        return rc;
+    int ret = read_permanent_flags((char *)&pf, sizeof(pf));
+    if (ret)
+        return -1;

if (pf.flags[PERM_FLAG_IDX_DISABLE] && !enable)
         return 0;

-    rc = assert_physical_presence(verbose);
-    if (rc) {
-        dprintf(DEBUG_tcg, "TCGBIOS: Asserting physical presence failed.\n");
-        return rc;
-    }
+    ret = assert_physical_presence(verbose);
+    if (ret)
+        return -1;

-    int ret = build_and_send_cmd(0, enable ? TPM_ORD_PhysicalEnable
-                                           : TPM_ORD_PhysicalDisable,
-                                 NULL, 0, TPM_DURATION_TYPE_SHORT);
-    *returnCode = ret;
+    ret = build_and_send_cmd(0, enable ? TPM_ORD_PhysicalEnable
+                                       : TPM_ORD_PhysicalDisable,
+                             NULL, 0, TPM_DURATION_TYPE_SHORT);
     if (ret)
         goto err_exit;

@@ -1008,18 +1002,16 @@ err_exit:
     dprintf(DEBUG_tcg, "TCGBIOS: TPM malfunctioning (line %d).\n", __LINE__);

tpm_set_failure();
-    return TCG_TCG_COMMAND_ERROR;
+    return ret;
 }

-static u32
-activate_tpm(int activate, int allow_reset, u32 *returnCode, int verbose)
+static int
+activate_tpm(int activate, int allow_reset, int verbose)
 {
-    u32 rc;
     struct tpm_permanent_flags pf;
-
-    rc = read_permanent_flags((char *)&pf, sizeof(pf));
-    if (rc)
-        return rc;
+    int ret = read_permanent_flags((char *)&pf, sizeof(pf));
+    if (ret)
+        return -1;

if (pf.flags[PERM_FLAG_IDX_DEACTIVATED] && !activate)
         return 0;
@@ -1027,19 +1019,16 @@ activate_tpm(int activate, int allow_reset, u32 *returnCode, int verbose)
     if (pf.flags[PERM_FLAG_IDX_DISABLE])
         return 0;

-    rc = assert_physical_presence(verbose);
-    if (rc) {
-        dprintf(DEBUG_tcg, "TCGBIOS: Asserting physical presence failed.\n");
-        return rc;
-    }
+    ret = assert_physical_presence(verbose);
+    if (ret)
+        return -1;

-    int ret = build_and_send_cmd(0, TPM_ORD_PhysicalSetDeactivated,
-                                 activate ? CommandFlag_FALSE
-                                          : CommandFlag_TRUE,
-                                 activate ? sizeof(CommandFlag_FALSE)
-                                          : sizeof(CommandFlag_TRUE),
-                                 TPM_DURATION_TYPE_SHORT);
-    *returnCode = ret;
+    ret = build_and_send_cmd(0, TPM_ORD_PhysicalSetDeactivated,
+                             activate ? CommandFlag_FALSE
+                                      : CommandFlag_TRUE,
+                             activate ? sizeof(CommandFlag_FALSE)
+                                      : sizeof(CommandFlag_TRUE),
+                             TPM_DURATION_TYPE_SHORT);
     if (ret)
         goto err_exit;

@@ -1058,33 +1047,26 @@ err_exit:
     dprintf(DEBUG_tcg, "TCGBIOS: TPM malfunctioning (line %d).\n", __LINE__);

tpm_set_failure();
-    return TCG_TCG_COMMAND_ERROR;
+    return ret;
 }

-static u32
-enable_activate(int allow_reset, u32 *returnCode, int verbose)
+static int
+enable_activate(int allow_reset, int verbose)
 {
-    u32 rc;
-
-    rc = enable_tpm(1, returnCode, verbose);
-    if (rc)
-        return rc;
-
-    rc = activate_tpm(1, allow_reset, returnCode, verbose);
+    int ret = enable_tpm(1, verbose);
+    if (ret)
+        return ret;

-    return rc;
+    return activate_tpm(1, allow_reset, verbose);
 }

-static u32
-force_clear(int enable_activate_before, int enable_activate_after,
-            u32 *returnCode, int verbose)
+static int
+force_clear(int enable_activate_before, int enable_activate_after, int verbose)
 {
-    u32 rc;
     int has_owner;
-
-    rc = read_has_owner(&has_owner);
-    if (rc)
-        return rc;
+    int ret = read_has_owner(&has_owner);
+    if (ret)
+        return -1;
     if (!has_owner) {
         if (verbose)
             printf("TPM does not have an owner.\n");
@@ -1092,23 +1074,20 @@ force_clear(int enable_activate_before, int enable_activate_after,
     }

if (enable_activate_before) {
-        rc = enable_activate(0, returnCode, verbose);
-        if (rc) {
+        ret = enable_activate(0, verbose);
+        if (ret) {
             dprintf(DEBUG_tcg,
                     "TCGBIOS: Enabling/activating the TPM failed.\n");
-            return rc;
+            return ret;
         }
     }

-    rc = assert_physical_presence(verbose);
-    if (rc) {
-        dprintf(DEBUG_tcg, "TCGBIOS: Asserting physical presence failed.\n");
-        return rc;
-    }
+    ret = assert_physical_presence(verbose);
+    if (ret)
+        return -1;

-    int ret = build_and_send_cmd(0, TPM_ORD_ForceClear,
-                                 NULL, 0, TPM_DURATION_TYPE_SHORT);
-    *returnCode = ret;
+    ret = build_and_send_cmd(0, TPM_ORD_ForceClear,
+                             NULL, 0, TPM_DURATION_TYPE_SHORT);
     if (ret)
         goto err_exit;

@@ -1119,36 +1098,32 @@ force_clear(int enable_activate_before, int enable_activate_after,
         return 0;
     }

-    enable_activate(1, returnCode, verbose);
-
-    return 0;
+    return enable_activate(1, verbose);

err_exit:
     dprintf(DEBUG_tcg, "TCGBIOS: TPM malfunctioning (line %d).\n", __LINE__);

tpm_set_failure();
-    return TCG_TCG_COMMAND_ERROR;
+    return ret;
 }

-static u32
-set_owner_install(int allow, u32 *returnCode, int verbose)
+static int
+set_owner_install(int allow, int verbose)
 {
-    u32 rc;
     int has_owner;
-    struct tpm_permanent_flags pf;
-
-    rc = read_has_owner(&has_owner);
-    if (rc)
-        return rc;
+    int ret = read_has_owner(&has_owner);
+    if (ret)
+        return -1;
     if (has_owner) {
         if (verbose)
             printf("Must first remove owner.\n");
         return 0;
     }

-    rc = read_permanent_flags((char *)&pf, sizeof(pf));
-    if (rc)
-        return rc;
+    struct tpm_permanent_flags pf;
+    ret = read_permanent_flags((char *)&pf, sizeof(pf));
+    if (ret)
+        return -1;

if (pf.flags[PERM_FLAG_IDX_DISABLE]) {
         if (verbose)
@@ -1156,18 +1131,15 @@ set_owner_install(int allow, u32 *returnCode, int verbose)
         return 0;
     }

-    rc = assert_physical_presence(verbose);
-    if (rc) {
-        dprintf(DEBUG_tcg, "TCGBIOS: Asserting physical presence failed.\n");
-        return rc;
-    }
+    ret = assert_physical_presence(verbose);
+    if (ret)
+        return -1;

-    int ret = build_and_send_cmd(0, TPM_ORD_SetOwnerInstall,
-                                 (allow) ? CommandFlag_TRUE
-                                         : CommandFlag_FALSE,
-                                 sizeof(CommandFlag_TRUE),
-                                 TPM_DURATION_TYPE_SHORT);
-    *returnCode = ret;
+    ret = build_and_send_cmd(0, TPM_ORD_SetOwnerInstall,
+                             (allow) ? CommandFlag_TRUE
+                                     : CommandFlag_FALSE,
+                             sizeof(CommandFlag_TRUE),
+                             TPM_DURATION_TYPE_SHORT);
     if (ret)
         goto err_exit;

@@ -1179,55 +1151,54 @@ set_owner_install(int allow, u32 *returnCode, int verbose)
 err_exit:
     dprintf(DEBUG_tcg, "TCGBIOS: TPM malfunctioning (line %d).\n", __LINE__);
     tpm_set_failure();
-    return TCG_TCG_COMMAND_ERROR;
+    return ret;
 }

-static u32
-tpm_process_cfg(tpm_ppi_code msgCode, int verbose, u32 *returnCode)
+static int
+tpm_process_cfg(tpm_ppi_code msgCode, int verbose)
 {
-    u32 rc = 0;
+    int ret = 0;

switch (msgCode) {
         case TPM_PPI_OP_NOOP: /* no-op */
             break;

case TPM_PPI_OP_ENABLE:
-            rc = enable_tpm(1, returnCode, verbose);
+            ret = enable_tpm(1, verbose);
             break;

case TPM_PPI_OP_DISABLE:
-            rc = enable_tpm(0, returnCode, verbose);
+            ret = enable_tpm(0, verbose);
             break;

case TPM_PPI_OP_ACTIVATE:
-            rc = activate_tpm(1, 1, returnCode, verbose);
+            ret = activate_tpm(1, 1, verbose);
             break;

case TPM_PPI_OP_DEACTIVATE:
-            rc = activate_tpm(0, 1, returnCode, verbose);
+            ret = activate_tpm(0, 1, verbose);
             break;

case TPM_PPI_OP_CLEAR:
-            rc = force_clear(1, 0, returnCode, verbose);
+            ret = force_clear(1, 0, verbose);
             break;

case TPM_PPI_OP_SET_OWNERINSTALL_TRUE:
-            rc = set_owner_install(1, returnCode, verbose);
+            ret = set_owner_install(1, verbose);
             break;

case TPM_PPI_OP_SET_OWNERINSTALL_FALSE:
-            rc = set_owner_install(0, returnCode, verbose);
+            ret = set_owner_install(0, verbose);
             break;

default:
             break;
     }

-    if (rc)
-        printf("Op %d: An error occurred: 0x%x TPM return code: 0x%x\n",
-               msgCode, rc, *returnCode);
+    if (ret)
+        printf("Op %d: An error occurred: 0x%x\n", msgCode, ret);

-    return rc;
+    return ret;
 }

static int
@@ -1331,7 +1302,6 @@ tpm_menu(void)
         return;

int scancode, next_scancodes[7];
-    u32 rc, returnCode;
     tpm_ppi_code msgCode;
     int state = 0, i;
     int waitkey;
@@ -1403,10 +1373,7 @@ tpm_menu(void)
                     break;

if (next_scancodes[i] == scancode) {
-                    rc = tpm_process_cfg(msgCode, 1, &returnCode);
-
-                    if (rc)
-                        printf("An error occurred: 0x%x\n", rc);
+                    tpm_process_cfg(msgCode, 1);
                     waitkey = 0;
                     break;
                 }

[PATCH 8/8] tpm: Replace build_and_send_cmd with tpm_send_cmd and tpm_send_check_cmd

[Kevin O'Connor]

Rename build_and_send_cmd() to tpm_send_cmd().  Introduce
tpm_send_check_cmd() which is a wrapper around tpm_send_cmd() that
calls tpm_set_failure() on failure.

This also moves the debugging dprintf() preceding all callers of
tpm_set_failure() into tpm_set_failure().  This change eliminates the
code line number in the debugging, but all callers of
tpm_set_failure() will log the action that failed immediately prior to
the dprintf().  So, the line number should not be necessary.

Signed-off-by: Kevin O'Connor 
---
 src/tcgbios.c | 151 +++++++++++++++++++++++-----------------------------------
 1 file changed, 59 insertions(+), 92 deletions(-)

diff --git a/src/tcgbios.c b/src/tcgbios.c
index cd6a433..046b3ce 100644
--- a/src/tcgbios.c
+++ b/src/tcgbios.c
@@ -170,8 +170,8 @@ tpm_is_working(void)
  * the custom part per command) and expect a response of the given size.
  */
 static int
-build_and_send_cmd(u8 locty, u32 ordinal, const u8 *append, u32 append_size,
-                   enum tpmDurationType to_t)
+tpm_send_cmd(u8 locty, u32 ordinal, const u8 *append, u32 append_size,
+             enum tpmDurationType to_t)
 {
     struct {
         struct tpm_req_header trqh;
@@ -195,7 +195,7 @@ build_and_send_cmd(u8 locty, u32 ordinal, const u8 *append, u32 append_size,

int ret = tpmhw_transmit(locty, &req.trqh, obuffer, &obuffer_len, to_t);
     ret = ret ? -1 : be32_to_cpu(trsh->errcode);
-    dprintf(DEBUG_tcg, "Return from build_and_send_cmd(%x, %x %x) = %x\n",
+    dprintf(DEBUG_tcg, "Return from tpm_send_cmd(%x, %x %x) = %x\n",
             ordinal, req.cmd[0], req.cmd[1], ret);
     return ret;
 }
@@ -203,23 +203,36 @@ build_and_send_cmd(u8 locty, u32 ordinal, const u8 *append, u32 append_size,
 static void
 tpm_set_failure(void)
 {
+    dprintf(DEBUG_tcg, "TCGBIOS: TPM malfunctioning.\n");
+
     /* 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),
-                       TPM_DURATION_TYPE_SHORT);
+    tpm_send_cmd(0, TPM_ORD_PhysicalPresence,
+                 PhysicalPresence_CMD_ENABLE,
+                 sizeof(PhysicalPresence_CMD_ENABLE),
+                 TPM_DURATION_TYPE_SHORT);

-    build_and_send_cmd(0, TPM_ORD_PhysicalPresence,
-                       PhysicalPresence_PRESENT,
-                       sizeof(PhysicalPresence_PRESENT),
-                       TPM_DURATION_TYPE_SHORT);
+    tpm_send_cmd(0, TPM_ORD_PhysicalPresence,
+                 PhysicalPresence_PRESENT,
+                 sizeof(PhysicalPresence_PRESENT),
+                 TPM_DURATION_TYPE_SHORT);

-    build_and_send_cmd(0, TPM_ORD_SetTempDeactivated,
-                       NULL, 0, TPM_DURATION_TYPE_SHORT);
+    tpm_send_cmd(0, TPM_ORD_SetTempDeactivated,
+                 NULL, 0, TPM_DURATION_TYPE_SHORT);

TPM_working = 0;
 }

+// Wrapper around tpm_send_cmd that will shutdown on failure
+static int
+tpm_send_check_cmd(u8 locty, u32 ordinal, const u8 *append, u32 append_size,
+                   enum tpmDurationType to_t)
+{
+    int ret = tpm_send_cmd(locty, ordinal, append, append_size, to_t);
+    if (ret)
+        tpm_set_failure();
+    return ret;
+}
+
 static int
 tpm_get_capability(u32 cap, u32 subcap, struct tpm_rsp_header *rsp, u32 rsize)
 {
@@ -237,10 +250,8 @@ tpm_get_capability(u32 cap, u32 subcap, struct tpm_rsp_header *rsp, u32 rsize)
     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);
-    if (ret) {
-        dprintf(DEBUG_tcg, "TCGBIOS: TPM malfunctioning (line %d).\n", __LINE__);
+    if (ret)
         tpm_set_failure();
-    }
     return ret;
 }

@@ -340,8 +351,10 @@ tpm_add_measurement_to_log(u32 pcrindex, u32 event_type,
     };
     tpm_fill_hash(&pcpes, hashdata, hashdata_length);
     int ret = tpm_log_extend_event(&pcpes, event);
-    if (ret)
+    if (ret) {
+        dprintf(DEBUG_tcg, "TCGBIOS: Failed to add internal measurement.\n");
         tpm_set_failure();
+    }
 }

@@ -401,9 +414,9 @@ static int
 tpm_startup(void)
 {
     dprintf(DEBUG_tcg, "TCGBIOS: Starting with TPM_Startup(ST_CLEAR)\n");
-    int ret = build_and_send_cmd(0, TPM_ORD_Startup,
-                                 Startup_ST_CLEAR, sizeof(Startup_ST_CLEAR),
-                                 TPM_DURATION_TYPE_SHORT);
+    int ret = tpm_send_cmd(0, TPM_ORD_Startup,
+                           Startup_ST_CLEAR, sizeof(Startup_ST_CLEAR),
+                           TPM_DURATION_TYPE_SHORT);
     if (CONFIG_COREBOOT && ret == TPM_INVALID_POSTINIT)
         /* with other firmware on the system the TPM may already have been
          * initialized
@@ -416,21 +429,19 @@ tpm_startup(void)
     if (ret)
         return -1;

-    ret = build_and_send_cmd(0, TPM_ORD_SelfTestFull, NULL, 0,
-                             TPM_DURATION_TYPE_LONG);
+    ret = tpm_send_cmd(0, TPM_ORD_SelfTestFull, NULL, 0,
+                       TPM_DURATION_TYPE_LONG);
     if (ret)
         goto err_exit;

-    ret = build_and_send_cmd(3, TSC_ORD_ResetEstablishmentBit, NULL, 0,
-                             TPM_DURATION_TYPE_SHORT);
+    ret = tpm_send_cmd(3, TSC_ORD_ResetEstablishmentBit, NULL, 0,
+                       TPM_DURATION_TYPE_SHORT);
     if (ret && ret != TPM_BAD_LOCALITY)
         goto err_exit;

return 0;

err_exit:
-    dprintf(DEBUG_tcg, "TCGBIOS: TPM malfunctioning (line %d).\n", __LINE__);
-
     tpm_set_failure();
     return -1;
 }
@@ -468,29 +479,22 @@ tpm_prepboot(void)
     if (!tpm_is_working())
         return;

-    int ret = build_and_send_cmd(0, TPM_ORD_PhysicalPresence,
+    int ret = tpm_send_check_cmd(0, TPM_ORD_PhysicalPresence,
                                  PhysicalPresence_CMD_ENABLE,
                                  sizeof(PhysicalPresence_CMD_ENABLE),
                                  TPM_DURATION_TYPE_SHORT);
     if (ret)
-        goto err_exit;
+        return;

-    ret = build_and_send_cmd(0, TPM_ORD_PhysicalPresence,
+    ret = tpm_send_check_cmd(0, TPM_ORD_PhysicalPresence,
                              PhysicalPresence_NOT_PRESENT_LOCK,
                              sizeof(PhysicalPresence_NOT_PRESENT_LOCK),
                              TPM_DURATION_TYPE_SHORT);
     if (ret)
-        goto err_exit;
+        return;

tpm_add_action(4, "Calling INT 19h");
     tpm_add_event_separators();
-
-    return;
-
-err_exit:
-    dprintf(DEBUG_tcg, "TCGBIOS: TPM malfunctioning (line %d).\n", __LINE__);
-
-    tpm_set_failure();
 }

/*
@@ -578,19 +582,9 @@ tpm_s3_resume(void)
         return;

dprintf(DEBUG_tcg, "TCGBIOS: Resuming with TPM_Startup(ST_STATE)\n");
-
-    int ret = build_and_send_cmd(0, TPM_ORD_Startup,
-                                 Startup_ST_STATE, sizeof(Startup_ST_STATE),
-                                 TPM_DURATION_TYPE_SHORT);
-    if (ret)
-        goto err_exit;
-
-    return;
-
-err_exit:
-    dprintf(DEBUG_tcg, "TCGBIOS: TPM malfunctioning (line %d).\n", __LINE__);
-
-    tpm_set_failure();
+    tpm_send_check_cmd(0, TPM_ORD_Startup,
+                       Startup_ST_STATE, sizeof(Startup_ST_STATE),
+                       TPM_DURATION_TYPE_SHORT);
 }

@@ -911,7 +905,7 @@ assert_physical_presence(int verbose)
         /* physical presence already asserted */
         return 0;

-    ret = build_and_send_cmd(0, TPM_ORD_PhysicalPresence,
+    ret = tpm_send_check_cmd(0, TPM_ORD_PhysicalPresence,
                              PhysicalPresence_CMD_ENABLE,
                              sizeof(PhysicalPresence_CMD_ENABLE),
                              TPM_DURATION_TYPE_SHORT);
@@ -921,7 +915,7 @@ assert_physical_presence(int verbose)
         goto err_exit;
     }

-    ret = build_and_send_cmd(0, TPM_ORD_PhysicalPresence,
+    ret = tpm_send_check_cmd(0, TPM_ORD_PhysicalPresence,
                              PhysicalPresence_PRESENT,
                              sizeof(PhysicalPresence_PRESENT),
                              TPM_DURATION_TYPE_SHORT);
@@ -934,8 +928,6 @@ assert_physical_presence(int verbose)
     return 0;

err_exit:
-    dprintf(DEBUG_tcg, "TCGBIOS: TPM malfunctioning (line %d).\n", __LINE__);
-    tpm_set_failure();
     dprintf(DEBUG_tcg, "TCGBIOS: Asserting physical presence failed: %x\n", ret);
     return -1;
 }
@@ -985,23 +977,15 @@ enable_tpm(int enable, int verbose)
     if (ret)
         return -1;

-    ret = build_and_send_cmd(0, enable ? TPM_ORD_PhysicalEnable
+    ret = tpm_send_check_cmd(0, enable ? TPM_ORD_PhysicalEnable
                                        : TPM_ORD_PhysicalDisable,
                              NULL, 0, TPM_DURATION_TYPE_SHORT);
-    if (ret)
-        goto err_exit;
-
-    return 0;
-
-err_exit:
-    if (enable)
-        dprintf(DEBUG_tcg, "TCGBIOS: Enabling the TPM failed.\n");
-    else
-        dprintf(DEBUG_tcg, "TCGBIOS: Disabling the TPM failed.\n");
-
-    dprintf(DEBUG_tcg, "TCGBIOS: TPM malfunctioning (line %d).\n", __LINE__);
-
-    tpm_set_failure();
+    if (ret) {
+        if (enable)
+            dprintf(DEBUG_tcg, "TCGBIOS: Enabling the TPM failed.\n");
+        else
+            dprintf(DEBUG_tcg, "TCGBIOS: Disabling the TPM failed.\n");
+    }
     return ret;
 }

@@ -1023,14 +1007,14 @@ activate_tpm(int activate, int allow_reset, int verbose)
     if (ret)
         return -1;

-    ret = build_and_send_cmd(0, TPM_ORD_PhysicalSetDeactivated,
+    ret = tpm_send_check_cmd(0, TPM_ORD_PhysicalSetDeactivated,
                              activate ? CommandFlag_FALSE
                                       : CommandFlag_TRUE,
                              activate ? sizeof(CommandFlag_FALSE)
                                       : sizeof(CommandFlag_TRUE),
                              TPM_DURATION_TYPE_SHORT);
     if (ret)
-        goto err_exit;
+        return ret;

if (activate && allow_reset) {
         if (verbose) {
@@ -1042,12 +1026,6 @@ activate_tpm(int activate, int allow_reset, int verbose)
     }

return 0;
-
-err_exit:
-    dprintf(DEBUG_tcg, "TCGBIOS: TPM malfunctioning (line %d).\n", __LINE__);
-
-    tpm_set_failure();
-    return ret;
 }

static int
@@ -1086,10 +1064,10 @@ force_clear(int enable_activate_before, int enable_activate_after, int verbose)
     if (ret)
         return -1;

-    ret = build_and_send_cmd(0, TPM_ORD_ForceClear,
+    ret = tpm_send_check_cmd(0, TPM_ORD_ForceClear,
                              NULL, 0, TPM_DURATION_TYPE_SHORT);
     if (ret)
-        goto err_exit;
+        return ret;

if (!enable_activate_after) {
         if (verbose)
@@ -1099,12 +1077,6 @@ force_clear(int enable_activate_before, int enable_activate_after, int verbose)
     }

return enable_activate(1, verbose);
-
-err_exit:
-    dprintf(DEBUG_tcg, "TCGBIOS: TPM malfunctioning (line %d).\n", __LINE__);
-
-    tpm_set_failure();
-    return ret;
 }

static int
@@ -1135,23 +1107,18 @@ set_owner_install(int allow, int verbose)
     if (ret)
         return -1;

-    ret = build_and_send_cmd(0, TPM_ORD_SetOwnerInstall,
+    ret = tpm_send_check_cmd(0, TPM_ORD_SetOwnerInstall,
                              (allow) ? CommandFlag_TRUE
                                      : CommandFlag_FALSE,
                              sizeof(CommandFlag_TRUE),
                              TPM_DURATION_TYPE_SHORT);
     if (ret)
-        goto err_exit;
+        return ret;

if (verbose)
         printf("Installation of owner %s.\n", allow ? "enabled" : "disabled");

return 0;
-
-err_exit:
-    dprintf(DEBUG_tcg, "TCGBIOS: TPM malfunctioning (line %d).\n", __LINE__);
-    tpm_set_failure();
-    return ret;
 }

static int

[Kevin O'Connor]

On Wed, Dec 30, 2015 at 02:31:54PM -0500, Kevin O'Connor wrote:
> Hi Stefan,
>
> I cleaned up the additional patches I mentioned yesterday.
>
> Some time back, you posted a series of patches that removed the use of
> TCG 16bit BIOS structs from internal functions.  Many of those
> functions were still using return codes from the spec though.  I found
> using these return codes in internal functions to be annoying because
> they conflict with the TIS return codes.  This series mostly just
> separates out the return codes so that only the TCG 16bit BIOS
> functions deal with the TCG 16bit BIOS return codes.

FYI, I committed patches 1-7 of this series.  (I want to think further
about patch 8.)

-Kevin