[PATCH 0/3] TPM cleanup

[Stefan Berger]

The following series addresses 3 out of the 4 comments from Kevin from his
email today.

Regards,
   Stefan

Stefan Berger (3):
  tpm: Drop code using the TPM for sha1
  tpm: Introduce a cache variable for TCPA table pointer
  tpm: Set timeouts and durations to default values

src/hw/tpm_drivers.c |  1 -
 src/hw/tpm_drivers.h | 26 +++++++-------
 src/tcgbios.c        | 97 ++++++++--------------------------------------------
 3 files changed, 28 insertions(+), 96 deletions(-)

[PATCH 1/3] tpm: Drop code using the TPM for sha1

[Stefan Berger]

From: Stefan Berger

Drop the code that is using the TPM for sha1 calculations.

Signed-off-by: Stefan Berger 
---
 src/hw/tpm_drivers.c |  1 -
 src/hw/tpm_drivers.h |  3 --
 src/tcgbios.c        | 81 ++--------------------------------------------------
 3 files changed, 2 insertions(+), 83 deletions(-)

diff --git a/src/hw/tpm_drivers.c b/src/hw/tpm_drivers.c
index b5cde00..0bf5997 100644
--- a/src/hw/tpm_drivers.c
+++ b/src/hw/tpm_drivers.c
@@ -286,6 +286,5 @@ struct tpm_driver tpm_drivers[TPM_NUM_DRIVERS] = {
             .readresp      = tis_readresp,
             .waitdatavalid = tis_waitdatavalid,
             .waitrespready = tis_waitrespready,
-            .sha1threshold = 100 * 1024,
         },
 };
diff --git a/src/hw/tpm_drivers.h b/src/hw/tpm_drivers.h
index 48c6615..6357d02 100644
--- a/src/hw/tpm_drivers.h
+++ b/src/hw/tpm_drivers.h
@@ -23,9 +23,6 @@ struct tpm_driver {
     u32 (*readresp)(u8 *buffer, u32 *len);
     u32 (*waitdatavalid)(void);
     u32 (*waitrespready)(enum tpmDurationType to_t);
-    /* the TPM will be used for buffers of sizes below the sha1threshold
-       for calculating the hash */
-    u32 sha1threshold;
 };

extern struct tpm_driver tpm_drivers[];
diff --git a/src/tcgbios.c b/src/tcgbios.c
index 9ae075a..cd5e2cf 100644
--- a/src/tcgbios.c
+++ b/src/tcgbios.c
@@ -619,83 +619,6 @@ get_lasa_last_ptr(u16 *entry_count, u8 **log_area_start_address_next)
     return log_area_start_address_last;
 }

-
-static u32
-tpm_sha1_calc(const u8 *data, u32 length, u8 *hash)
-{
-    u32 rc;
-    u32 returnCode;
-    struct tpm_res_sha1start start;
-    struct tpm_res_sha1complete complete;
-    u32 blocks = length / 64;
-    u32 rest = length & 0x3f;
-    u32 numbytes, numbytes_no;
-    u32 offset = 0;
-
-    rc = build_and_send_cmd(0, TPM_ORD_SHA1Start,
-                            NULL, 0,
-                            (u8 *)&start, sizeof(start),
-                            &returnCode, TPM_DURATION_TYPE_SHORT);
-
-    if (rc || returnCode)
-        goto err_exit;
-
-    while (blocks > 0) {
-
-        numbytes = be32_to_cpu(start.max_num_bytes);
-        if (numbytes > blocks * 64)
-             numbytes = blocks * 64;
-
-        numbytes_no = cpu_to_be32(numbytes);
-
-        rc = build_and_send_cmd_od(0, TPM_ORD_SHA1Update,
-                                   (u8 *)&numbytes_no, sizeof(numbytes_no),
-                                   NULL, 0, &returnCode,
-                                   &data[offset], numbytes,
-                                   TPM_DURATION_TYPE_SHORT);
-
-        if (rc || returnCode)
-            goto err_exit;
-
-        offset += numbytes;
-        blocks -= (numbytes / 64);
-    }
-
-    numbytes_no = cpu_to_be32(rest);
-
-    rc = build_and_send_cmd_od(0, TPM_ORD_SHA1Complete,
-                              (u8 *)&numbytes_no, sizeof(numbytes_no),
-                              (u8 *)&complete, sizeof(complete),
-                              &returnCode,
-                              &data[offset], rest, TPM_DURATION_TYPE_SHORT);
-
-    if (rc || returnCode)
-        goto err_exit;
-
-    memcpy(hash, complete.hash, sizeof(complete.hash));
-
-    return 0;
-
-err_exit:
-    dprintf(DEBUG_tcg, "TCGBIOS: TPM SHA1 malfunctioning.\n");
-
-    tpm_set_failure();
-    if (rc)
-        return rc;
-    return TCG_TCG_COMMAND_ERROR;
-}
-
-
-static u32
-sha1_calc(const u8 *data, u32 length, u8 *hash)
-{
-    if (length < tpm_drivers[tpm_state.tpm_driver_to_use].sha1threshold)
-        return tpm_sha1_calc(data, length, hash);
-
-    return sha1(data, length, hash);
-}
-
-
 /*
  * Extend the ACPI log with the given entry by copying the
  * entry data into the log.
@@ -887,7 +810,7 @@ hash_all(const struct hai *hai, u8 *hash)
         hai->algorithmid != TPM_ALG_SHA)
         return TCG_INVALID_INPUT_PARA;

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

static u32
@@ -902,7 +825,7 @@ hash_log_event(const void *hashdata, u32 hashdata_length,
         return TCG_INVALID_INPUT_PARA;

if (hashdata) {
-        rc = sha1_calc(hashdata, hashdata_length, pcpes->digest);
+        rc = sha1(hashdata, hashdata_length, pcpes->digest);
         if (rc)
             return rc;
     }

[PATCH 2/3] tpm: Introduce a cache variable for TCPA table pointer

[Stefan Berger]

From: Stefan Berger

Introduce a cache variable for the TCPA ACPI table pointer
to avoid having to search for it every time it is needed.

Signed-off-by: Stefan Berger 
---
 src/tcgbios.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/src/tcgbios.c b/src/tcgbios.c
index cd5e2cf..cb741f0 100644
--- a/src/tcgbios.c
+++ b/src/tcgbios.c
@@ -90,6 +90,7 @@ typedef struct {
     u8            tpm_working:1;
     u8            if_shutdown:1;
     u8            tpm_driver_to_use:4;
+    struct tcpa_descriptor_rev2 *tcpa;
 } tpm_state_t;

@@ -195,6 +196,10 @@ find_tcpa_by_rsdp(struct rsdp_descriptor *rsdp)
         ctr++;
     }

+    /* cache it */
+    if (tcpa)
+        tpm_state.tcpa = tcpa;
+
     return tcpa;
 }

@@ -205,6 +210,9 @@ 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);
     else

[PATCH 3/3] tpm: Set timeouts and durations to default values

[Stefan Berger]

From: Stefan Berger

Remove the scaler that scaled the timeouts and durations by a factor of
10. Remove that to use the default values that are being used until the
TPM specific ones are read from the TPM itself.

Get these TPM specific values earlier from the device.

Signed-off-by: Stefan Berger 
---
 src/hw/tpm_drivers.h | 23 ++++++++++++++---------
 src/tcgbios.c        |  8 ++++----
 2 files changed, 18 insertions(+), 13 deletions(-)

diff --git a/src/hw/tpm_drivers.h b/src/hw/tpm_drivers.h
index 6357d02..497037e 100644
--- a/src/hw/tpm_drivers.h
+++ b/src/hw/tpm_drivers.h
@@ -66,12 +66,13 @@ extern struct tpm_driver tpm_drivers[];
 #define TIS_ACCESS_REQUEST_USE         (1 << 1) /* 0x02 */
 #define TIS_ACCESS_TPM_ESTABLISHMENT   (1 << 0) /* 0x01 */

-#define SCALER 10
-
-#define TIS_DEFAULT_TIMEOUT_A          (750  * SCALER)
-#define TIS_DEFAULT_TIMEOUT_B          (2000 * SCALER)
-#define TIS_DEFAULT_TIMEOUT_C          (750  * SCALER)
-#define TIS_DEFAULT_TIMEOUT_D          (750  * SCALER)
+/*
+ * Default TIS timeouts used before getting them from the TPM itself
+ */
+#define TIS_DEFAULT_TIMEOUT_A          750  /* ms */
+#define TIS_DEFAULT_TIMEOUT_B          2000 /* ms */
+#define TIS_DEFAULT_TIMEOUT_C          750  /* ms */
+#define TIS_DEFAULT_TIMEOUT_D          750  /* ms */

enum tisTimeoutType {
     TIS_TIMEOUT_TYPE_A = 0,
@@ -80,8 +81,12 @@ enum tisTimeoutType {
     TIS_TIMEOUT_TYPE_D,
 };

-#define TPM_DEFAULT_DURATION_SHORT     (2000  * SCALER)
-#define TPM_DEFAULT_DURATION_MEDIUM    (20000 * SCALER)
-#define TPM_DEFAULT_DURATION_LONG      (60000 * SCALER)
+/*
+ * Default command durations used before getting them from the
+ * TPM itself
+ */
+#define TPM_DEFAULT_DURATION_SHORT     2000  /* ms */
+#define TPM_DEFAULT_DURATION_MEDIUM    20000 /* ms */
+#define TPM_DEFAULT_DURATION_LONG      60000 /* ms */

#endif /* TPM_DRIVERS_H */
diff --git a/src/tcgbios.c b/src/tcgbios.c
index cb741f0..a5b84b9 100644
--- a/src/tcgbios.c
+++ b/src/tcgbios.c
@@ -488,6 +488,10 @@ tpm_startup(void)
     if (rc || returnCode)
         goto err_exit;

+    rc = determine_timeouts();
+    if (rc)
+        goto err_exit;
+
     rc = build_and_send_cmd(0, TPM_ORD_SelfTestFull, NULL, 0,
                             NULL, 0, &returnCode, TPM_DURATION_TYPE_LONG);

@@ -506,10 +510,6 @@ tpm_startup(void)
     if (rc || (returnCode != 0 && returnCode != TPM_BAD_LOCALITY))
         goto err_exit;

-    rc = determine_timeouts();
-    if (rc)
-        goto err_exit;
-
     rc = tpm_smbios_measure();
     if (rc)
         goto err_exit;

[PATCH 3/3] tpm: Set timeouts and durations to default values

[Kevin O'Connor]

On Fri, Nov 20, 2015 at 01:32:32PM -0500, Stefan Berger wrote:
> From: Stefan Berger
>
> Remove the scaler that scaled the timeouts and durations by a factor of
> 10. Remove that to use the default values that are being used until the
> TPM specific ones are read from the TPM itself.
>
> Get these TPM specific values earlier from the device.

Unless I'm missing something, determine_timeouts() still gets the
values in microseconds while tpm_drivers.c expects them in
milliseconds.

-Kevin

[PATCH 3/3] tpm: Set timeouts and durations to default values

[Stefan Berger]

"Kevin O'Connor"  wrote on 11/20/2015 05:37:37 PM:
> From: "Kevin O'Connor" 
> To: Stefan Berger/Watson/IBM@IBMUS
> Cc: [e-mail], Stefan Berger 
> Date: 11/20/2015 05:37 PM
> Subject: Re: [PATCH 3/3] tpm: Set timeouts and durations to default

values
> On Fri, Nov 20, 2015 at 01:32:32PM -0500, Stefan Berger wrote:
> > From: Stefan Berger
> >
> > Remove the scaler that scaled the timeouts and durations by a factor

of
> > Remove that to use the default values that are being used until

the
> > TPM specific ones are read from the TPM itself.
> >
> > Get these TPM specific values earlier from the device.
>
> Unless I'm missing something, determine_timeouts() still gets the
> values in microseconds while tpm_drivers.c expects them in
> milliseconds.

Let me revise the series.

Stefan