Move ContinueSelfTest to a later point to save time.

Change-Id: I96b413438359e11315101d408033066e6f0a0981

BUG=chrome-os-partner: 1826
TEST=none

Review URL: http://codereview.chromium.org/6667051

3 files changed, 78 insertions, 11 deletions

diff --git a/firmware/Makefile b/firmware/Makefile
index b3a198b5..accc835c 100644
--- a/firmware/Makefile
+++ b/firmware/Makefile
@@ -15,6 +15,24 @@ ifeq ($(FIRMWARE_ARCH),)
 CFLAGS += -DDISABLE_ROLLBACK_TPM
 endif
 
+# TPM-specific flags.  These depend on the particular TPM we're targeting for.
+# They are needed here only for compiling parts of the firmware code into
+# user-level tests.
+
+# TPM_BLOCKING_CONTINUESELFTEST is defined if TPM_ContinueSelfTest blocks until
+# the self test has completed.
+
+CLAGS += -DTPM_BLOCKING_CONTINUESELFTEST
+
+# TPM_MANUAL_SELFTEST is defined if the self test must be started manually
+# (with a call to TPM_ContinueSelfTest) instead of starting automatically at
+# power on.
+#
+# We sincerely hope that TPM_BLOCKING_CONTINUESELFTEST and TPM_MANUAL_SELFTEST
+# are not both defined at the same time.  (See comment in code.)
+
+# CLAGS += -DTPM_MANUAL_SELFTEST
+
 INCLUDES = \
 	-I$(FWTOP)/include \
 	-I$(LIBDIR)/include \
diff --git a/firmware/lib/rollback_index.c b/firmware/lib/rollback_index.c
index 954dfeaf..94693856 100644
--- a/firmware/lib/rollback_index.c
+++ b/firmware/lib/rollback_index.c
@@ -182,12 +182,24 @@ uint32_t SetupTPM(int recovery_mode, int developer_mode,
   RETURN_ON_FAILURE(TlclLibInit());
 
   RETURN_ON_FAILURE(TlclStartup());
-  /* Use ContinueSelfTest rather than SelfTestFull().  It enables
-   * access to the subset of TPM commands we need in the firmware, and
-   * allows the full self test to run in paralle with firmware
-   * startup.  By the time we get to the OS, self test will have
-   * completed. */
+  /* Some TPMs start the self test automatically at power on.  In that case we
+   * don't need to call ContinueSelfTest.  On some (other) TPMs,
+   * ContinueSelfTest may block.  In that case, we definitely don't want to
+   * call it here.  For TPMs in the intersection of these two sets, we're
+   * screwed.  (In other words: TPMs that require manually starting the
+   * self-test AND block will have poor performance until we split
+   * TlclSendReceive() into Send() and Receive(), and have a state machine to
+   * control setup.)
+   *
+   * This comment is likely to become obsolete in the near future, so don't
+   * trust it.  It may have not been updated.
+   */
+#ifdef TPM_MANUAL_SELFTEST
+#ifdef TPM_BLOCKING_CONTINUESELFTEST
+#warning "lousy TPM!"
+#endif
   RETURN_ON_FAILURE(TlclContinueSelfTest());
+#endif
   result = TlclAssertPhysicalPresence();
   if (result != 0) {
     /* It is possible that the TPM was delivered with the physical presence
diff --git a/firmware/lib/tpm_lite/tlcl.c b/firmware/lib/tpm_lite/tlcl.c
index 6252700d..f36f4633 100644
--- a/firmware/lib/tpm_lite/tlcl.c
+++ b/firmware/lib/tpm_lite/tlcl.c
@@ -44,11 +44,11 @@ static INLINE int TpmReturnCode(const uint8_t* buffer) {
   return TpmCommandCode(buffer);
 }
 
-/* Sends a TPM command and gets a response.  Returns 0 if success or the TPM
- * error code if error. */
-static uint32_t TlclSendReceive(const uint8_t* request, uint8_t* response,
-                                int max_length) {
-
+/* Like TlclSendReceive below, but do not retry if NEEDS_SELFTEST or
+ * DOING_SELFTEST errors are returned.
+ */
+static uint32_t TlclSendReceiveNoRetry(const uint8_t* request,
+                                       uint8_t* response, int max_length) {
   uint32_t result;
 
 #ifdef EXTRA_LOGGING
@@ -83,6 +83,40 @@ static uint32_t TlclSendReceive(const uint8_t* request, uint8_t* response,
 }
 
 
+/* Sends a TPM command and gets a response.  Returns 0 if success or the TPM
+ * error code if error. In the firmware, waits for the self test to complete
+ * if needed. In the host, reports the first error without retries. */
+static uint32_t TlclSendReceive(const uint8_t* request, uint8_t* response,
+                                int max_length) {
+  uint32_t result = TlclSendReceiveNoRetry(request, response, max_length);
+  /* When compiling for the firmware, hide command failures due to the self
+   * test not having run or completed. */
+#ifndef CHROMEOS_ENVIRONMENT
+  /* If the command fails because the self test has not completed, try it
+   * again after attempting to ensure that the self test has completed. */
+  if (result == TPM_E_NEEDS_SELFTEST || result == TPM_E_DOING_SELFTEST) {
+    result = TlclContinueSelfTest();
+    if (result != TPM_SUCCESS) {
+      return result;
+    }
+#if defined(TPM_BLOCKING_CONTINUESELFTEST) || defined(VB_RECOVERY_MODE)
+    /* Retry only once */
+    result = TlclSendReceiveNoRetry(request, response, max_length);
+#else
+    /* This needs serious testing.  The TPM specification says: "iii. The
+     * caller MUST wait for the actions of TPM_ContinueSelfTest to complete
+     * before reissuing the command C1."  But, if ContinueSelfTest is
+     * non-blocking, how do we know that the actions have completed other than
+     * trying again? */
+    do {
+      result = TlclSendReceiveNoRetry(request, response, max_length);
+    } while (result == TPM_E_DOING_SELFTEST);
+#endif
+  }
+#endif  /* ! defined(CHROMEOS_ENVIRONMENT) */
+  return result;
+}
+
 /* Sends a command and returns the error code. */
 static uint32_t Send(const uint8_t* command) {
   uint8_t response[TPM_LARGE_ENOUGH_COMMAND_SIZE];
@@ -116,8 +150,11 @@ uint32_t TlclSelfTestFull(void) {
 }
 
 uint32_t TlclContinueSelfTest(void) {
+  uint8_t response[TPM_LARGE_ENOUGH_COMMAND_SIZE];
   VBDEBUG(("TPM: Continue self test\n"));
-  return Send(tpm_continueselftest_cmd.buffer);
+  /* Call the No Retry version of SendReceive to avoid recursion. */
+  return TlclSendReceiveNoRetry(tpm_continueselftest_cmd.buffer,
+                                response, sizeof(response));
 }
 
 uint32_t TlclDefineSpace(uint32_t index, uint32_t perm, uint32_t size) {