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;