CHROMIUM: tpm: reconcile mainline changes with resume strategy
authorLuigi Semenzato <semenzato@chromium.org>
Tue, 22 May 2012 23:55:38 +0000 (16:55 -0700)
committerOlof Johansson <olof@lixom.net>
Fri, 1 Jun 2012 06:56:51 +0000 (23:56 -0700)
Our local changes were no longer compatible with the mainline changes
to tpm_continue_selftest.  This introduces the helper function
tpm_continue_selftest_nocheck, which avoids recursion in transmit_cmd.
A large comment also attempts to explain more clearly what is going
on.

The new mainline function tpm_do_selftest hides differences between
TPMs by calling tpm_continue_selftest, then checking if the self test
has completed or not.  (TPM_ContinueSelfTest unfortunately allows
both behaviors.)  This probably works fine, but it would add hundreds
of milliseconds to our resume path.

This fix is necessary because our version of tpm_continue_selftest
was not returning an error code, but the length of the response packet,
which was interpreted as a weird error code.

To test, I ran "powerd_suspend; tpmc getvf", looked at the log, and
noted that waiting for the self test to complete would have cost us
over 300ms.

BUG=chromium-os:31065
TEST=manual test

Change-Id: Iff09a9759c31ea93cd58c7ba32114b04e67e2359
Signed-off-by: Luigi Semenzato <semenzato@chromium.org>
Reviewed-on: https://gerrit.chromium.org/gerrit/23299
Reviewed-by: Grant Grundler <grundler@chromium.org>
drivers/char/tpm/tpm.c

index 7f77ca3..60add08 100644 (file)
@@ -349,7 +349,7 @@ static void timeout_work(struct work_struct *work)
        mutex_unlock(&chip->buffer_mutex);
 }
 
-static void needs_resume(struct tpm_chip *chip)
+static void set_needs_resume(struct tpm_chip *chip)
 {
        mutex_lock(&chip->resume_mutex);
        chip->resume_time = jiffies;
@@ -442,23 +442,12 @@ out:
        return rc;
 }
 
-/**
- * tpm_continue_selftest -- run TPM's selftest
- * @chip: TPM chip to use
- *
- * Returns 0 on success, < 0 in case of fatal error or a value > 0 representing
- * a TPM error code.
- */
-int tpm_continue_selftest(struct tpm_chip *chip)
+void tpm_continue_selftest_nocheck(struct tpm_chip *chip)
 {
-       int rc;
        struct tpm_cmd_t cmd;
-
        cmd.header.in = continue_selftest_header;
-       rc = tpm_transmit(chip, &cmd, CONTINUE_SELFTEST_RESULT_SIZE);
-       return rc;
+       tpm_transmit(chip, (u8 *) &cmd, CONTINUE_SELFTEST_RESULT_SIZE);
 }
-EXPORT_SYMBOL_GPL(tpm_continue_selftest);
 
 /* The maximum time in milliseconds that the TPM self test will take to
  * complete.  TODO(semenzato): 1s should be plenty for all TPMs, but how can we
@@ -466,6 +455,16 @@ EXPORT_SYMBOL_GPL(tpm_continue_selftest);
  */
 #define TPM_SELF_TEST_DURATION_MSEC 1000
 
+/* We don't want to wait for the self test to complete at resume, because it
+ * impacts the resume speed.  TPM commands are infrequent so the wait is
+ * usually not needed and is wasteful.  Instead, before we send any command, we
+ * check that enough time has elapsed from the resume so that we are
+ * comfortable that the self test has completed.  If not, we wait.  Unlike at
+ * boot, here we don't check the return code of continue_self_test, so we can
+ * use a code path which avoids recursion.  Furthermore, this only works when
+ * ContinueSelfTest is blocking, that is it returns only after the self test
+ * has completed, which is the case for the Infineon TPM.
+ */
 static void resume_if_needed(struct tpm_chip *chip)
 {
        mutex_lock(&chip->resume_mutex);
@@ -476,7 +475,7 @@ static void resume_if_needed(struct tpm_chip *chip)
                if (jiffies - chip->resume_time <
                    msecs_to_jiffies(TPM_SELF_TEST_DURATION_MSEC)) {
                        dev_info(chip->dev, "waiting for TPM self test");
-                       tpm_continue_selftest(chip);
+                       tpm_continue_selftest_nocheck(chip);
                }
                chip->needs_resume = 0;
                dev_info(chip->dev, "TPM delayed resume completed");
@@ -504,6 +503,25 @@ static ssize_t transmit_cmd(struct tpm_chip *chip, struct tpm_cmd_t *cmd,
        return err;
 }
 
+/**
+ * tpm_continue_selftest -- run TPM's selftest
+ * @chip: TPM chip to use
+ *
+ * Returns 0 on success, < 0 in case of fatal error or a value > 0 representing
+ * a TPM error code.
+ */
+int tpm_continue_selftest(struct tpm_chip *chip)
+{
+       int rc;
+       struct tpm_cmd_t cmd;
+
+       cmd.header.in = continue_selftest_header;
+       rc = transmit_cmd(chip, (const u8 *) &cmd,
+                         CONTINUE_SELFTEST_RESULT_SIZE, "continue selftest");
+       return rc;
+}
+EXPORT_SYMBOL_GPL(tpm_continue_selftest);
+
 /*
  * Returns max number of jiffies to wait
  */
@@ -1352,7 +1370,7 @@ int tpm_pm_resume(struct device *dev)
        if (chip == NULL)
                return -ENODEV;
 
-       needs_resume(chip);
+       set_needs_resume(chip);
        return 0;
 }
 EXPORT_SYMBOL_GPL(tpm_pm_resume);