Re: TPM selftest failure in 4.15

[James Bottomley <James.Bottomley@HansenPartnership.com>]

On Thu, 2018-02-01 at 11:59 -0700, Jason Gunthorpe wrote:
> On Thu, Feb 01, 2018 at 07:46:04PM +0100, James Bottomley wrote:
> 
> > 
> > I honestly don't think we should be waiting for the self test at
> > all.
> > We should kick it off and treat any TPM_RC_TESTING error as
> > -EAGAIN.
> > We're already under fire for slow boot sequences and adding 2s just
> > to
> > wait for the TPM to self test adds to that for no real value.
> 
> Arguably the BIOS should have completed the selftest - this stuff
> generally only exists to support embedded.
> 
> I don't like the idea of EAGAIN, that just expose all our users to
> this mess.
> 
> I would support making transmit_cmd genericly wait and retry if the
> TPM insists we need to wait for selftest to complete the specific
> command though.

OK, how about this then?

James

---

diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
index 1d6729be4cd6..84ed271c060b 100644
--- a/drivers/char/tpm/tpm-interface.c
+++ b/drivers/char/tpm/tpm-interface.c
@@ -521,12 +521,32 @@ ssize_t tpm_transmit_cmd(struct tpm_chip *chip, struct tpm_space *space,
 	const struct tpm_output_header *header = buf;
 	int err;
 	ssize_t len;
+	unsigned int delay_msec = 20;
 
-	len = tpm_transmit(chip, space, (u8 *)buf, bufsiz, flags);
-	if (len <  0)
-		return len;
+	/*
+	 * on first probe we kick off a TPM self test in the
+	 * background This means the TPM may return RC_TESTING to any
+	 * command that tries to use a subsystem under test, so do an
+	 * exponential backoff wait if that happens
+	 */
+	for (;;) {
+		len = tpm_transmit(chip, space, (u8 *)buf, bufsiz, flags);
+		if (len <  0)
+			return len;
+
+		err = be32_to_cpu(header->return_code);
+		if (err != TPM2_RC_TESTING ||
+		    (flags & TPM_TRANSMIT_NOWAIT))
+			break;
+
+		delay_msec *= 2;
+		if (delay_msec > TPM2_DURATION_LONG) {
+			dev_err(&chip->dev,"TPM: still running self tests, giving up waiting\n");
+			break;
+		}
+		tpm_msleep(delay_msec);
+	}
 
-	err = be32_to_cpu(header->return_code);
 	if (err != 0 && desc)
 		dev_err(&chip->dev, "A TPM error (%d) occurred %s\n", err,
 			desc);
diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
index 528cffbd49d3..47c5a5206325 100644
--- a/drivers/char/tpm/tpm.h
+++ b/drivers/char/tpm/tpm.h
@@ -495,6 +495,7 @@ extern struct idr dev_nums_idr;
 enum tpm_transmit_flags {
 	TPM_TRANSMIT_UNLOCKED	= BIT(0),
 	TPM_TRANSMIT_RAW	= BIT(1),
+	TPM_TRANSMIT_NOWAIT	= BIT(2),
 };
 
 ssize_t tpm_transmit(struct tpm_chip *chip, struct tpm_space *space,
diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
index f40d20671a78..106c126b4fe0 100644
--- a/drivers/char/tpm/tpm2-cmd.c
+++ b/drivers/char/tpm/tpm2-cmd.c
@@ -849,28 +849,24 @@ static const struct tpm_input_header tpm2_selftest_header = {
 static int tpm2_do_selftest(struct tpm_chip *chip)
 {
 	int rc;
-	unsigned int delay_msec = 20;
-	long duration;
 	struct tpm2_cmd cmd;
 
-	duration = jiffies_to_msecs(
-		tpm2_calc_ordinal_duration(chip, TPM2_CC_SELF_TEST));
-
-	while (duration > 0) {
-		cmd.header.in = tpm2_selftest_header;
-		cmd.params.selftest_in.full_test = 0;
-
-		rc = tpm_transmit_cmd(chip, NULL, &cmd, TPM2_SELF_TEST_IN_SIZE,
-				      0, 0, "continue selftest");
-
-		if (rc != TPM2_RC_TESTING)
-			break;
-
-		tpm_msleep(delay_msec);
-		duration -= delay_msec;
-
-		/* wait longer the next round */
-		delay_msec *= 2;
+	cmd.header.in = tpm2_selftest_header;
+	cmd.params.selftest_in.full_test = 0;
+
+	rc = tpm_transmit_cmd(chip, NULL, &cmd, TPM2_SELF_TEST_IN_SIZE,
+			      0, TPM_TRANSMIT_NOWAIT, "continue selftest");
+
+	if (rc == TPM2_RC_TESTING) {
+		/*
+		 * A return of RC_TESTING means the TPM is still
+		 * running self tests.  If one fails it will go into
+		 * failure mode and return RC_FAILED to every command,
+		 * so treat a still in testing return as a success
+		 * rather than causing a driver detach.
+		 */
+		dev_info(&chip->dev,"TPM: Running self test in background\n");
+		rc = TPM2_RC_SUCCESS;
 	}
 
 	return rc;