[coreboot-gerrit] Change in coreboot[master]: spi/tpm: claim locality just once during boot

Vadim Bendebury (Code Review) gerrit at coreboot.org
Fri Nov 17 06:29:45 CET 2017


Vadim Bendebury has uploaded this change for review. ( https://review.coreboot.org/22489


Change subject: spi/tpm: claim locality just once during boot
......................................................................

spi/tpm: claim locality just once during boot

All coreboot stages using TPM start with the same sequence: check if
locality is claimed, if so, release it by writing 'active locality'
bit, then try claiming it.

This is actually not a proper procedure: section "5.5.2.3.1 Command
Aborts" of "TCG PC Client Platform TPM Profile (PTP) Specification
Level 00 Revision 00.430 Family 2" lists overwriting active locality
status bit as a means of triggering TPM command abort.

On top of that, none of the coreboot stages releases locality, it is
enough to claim it once when device starts booting.

In fact, locality being active when the device is in verstage is most
likely due to delayed TPM reset processing by the Cr50 TPM: reset is
an asynchronous event, and is processed once current command
processing completes.

The proper procedure is to wait if locality is active until it is
released (which will happen when Cr50 processes reset) and then
proceed to claim it. This needs to happen only during verstage, other
stages using TPM are guaranteed has been claimed earlier.

BRANCH=gru
BUG=b:65867313

TEST=the new autotest triggering EC reset during key generation
     process does not cause boot failures on Fizz device any more.
     Below are times verstage had to wait:

  TPM ready after 3132 ms
  TPM ready after 22120 ms
  TPM ready after 4936 ms
  TPM ready after 6445 ms
  TPM ready after 11798 ms
  TPM ready after 27421 ms
  TPM ready after 4582 ms
  TPM ready after 7532 ms
  TPM ready after 27920 ms
  TPM ready after 3539 ms
  TPM ready after 12557 ms
  TPM ready after 6773 ms
  TPM ready after 1631 ms
  TPM ready after 197 ms
  TPM ready after 24330 ms
  TPM ready after 3241 ms

Change-Id: Iaee04f009bcde03712483e5e03de4a3441ea32b1
Signed-off-by: Vadim Bendebury <vbendeb at chromium.org>
---
M src/drivers/spi/tpm/tpm.c
1 file changed, 41 insertions(+), 26 deletions(-)



  git pull ssh://review.coreboot.org:29418/coreboot refs/changes/89/22489/1

diff --git a/src/drivers/spi/tpm/tpm.c b/src/drivers/spi/tpm/tpm.c
index c09462e..c506eb9 100644
--- a/src/drivers/spi/tpm/tpm.c
+++ b/src/drivers/spi/tpm/tpm.c
@@ -325,6 +325,7 @@
 	return (status & TPM_STS_BURST_COUNT_MASK) >> TPM_STS_BURST_COUNT_SHIFT;
 }
 
+#if ENV_VERSTAGE
 static uint8_t tpm2_read_access_reg(void)
 {
 	uint8_t access;
@@ -346,38 +347,50 @@
 	uint8_t access;
 	struct stopwatch sw;
 
-	access = tpm2_read_access_reg();
 	/*
-	 * If active locality is set (maybe reset line is not connected?),
-	 * release the locality and try again.
-	 */
-	if (access & TPM_ACCESS_ACTIVE_LOCALITY) {
-		tpm2_write_access_reg(TPM_ACCESS_ACTIVE_LOCALITY);
-		access = tpm2_read_access_reg();
-	}
-
-	/*
-	 * If cr50 is doing a long crypto operation, it can take up to
-	 * 30 seconds to get a valid status value back
+	 * Locality is released by TPM reset.
+	 *
+	 * If locality is taken at this point, this could be due to the fact
+	 * that the TPM is performing a long operation and has not processed
+	 * reset request yet. We'll wait up to CR50_TIMEOUT_INIT_MS and see if
+	 * it releases locality when reset is processed.
 	 */
 	stopwatch_init_msecs_expire(&sw, CR50_TIMEOUT_INIT_MS);
-	while (!stopwatch_expired(&sw) && access != TPM_ACCESS_VALID)
+	do {
 		access = tpm2_read_access_reg();
-	if (access != TPM_ACCESS_VALID) {
-		printk(BIOS_ERR, "Invalid reset status: %#x\n", access);
-		return 0;
-	}
+		if (access & TPM_ACCESS_ACTIVE_LOCALITY) {
+			/*
+			 * Don't bombard the chip with traffic, let it keep
+			 * processing the command.
+			 */
+			mdelay(2);
+			continue;
+		}
 
-	tpm2_write_access_reg(TPM_ACCESS_REQUEST_USE);
-	access = tpm2_read_access_reg();
-	if (access != (TPM_ACCESS_VALID | TPM_ACCESS_ACTIVE_LOCALITY)) {
-		printk(BIOS_ERR, "Failed to claim locality 0, status: %#x\n",
-		       access);
-		return 0;
-	}
+		/*
+		 * Ok, the locality is free, TPM must be reset, let's claim
+		 * it.
+		 */
 
-	return 1;
+		tpm2_write_access_reg(TPM_ACCESS_REQUEST_USE);
+		access = tpm2_read_access_reg();
+		if (access != (TPM_ACCESS_VALID | TPM_ACCESS_ACTIVE_LOCALITY)) {
+			break;
+		}
+
+		printk(BIOS_INFO, "TPM ready after %ld ms\n",
+		       stopwatch_duration_msecs(&sw));
+
+		return 1;
+	} while (!stopwatch_expired(&sw));
+
+	printk(BIOS_ERR,
+	       "Failed to claim locality 0 after %ld ms, status: %#x\n",
+	       stopwatch_duration_msecs(&sw), access);
+
+	return 0;
 }
+#endif
 
 /* Device/vendor ID values of the TPM devices this driver supports. */
 static const uint32_t supported_did_vids[] = {
@@ -426,9 +439,11 @@
 
 	printk(BIOS_INFO, " done!\n");
 
-	/* Claim locality 0. */
+#if ENV_VERSTAGE
+	/* Claim locality 0 if in verstage. */
 	if (!tpm2_claim_locality())
 		return -1;
+#endif
 
 	read_tpm_sts(&status);
 	if ((status & TPM_STS_FAMILY_MASK) != TPM_STS_FAMILY_TPM_2_0) {

-- 
To view, visit https://review.coreboot.org/22489
To unsubscribe, visit https://review.coreboot.org/settings

Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Iaee04f009bcde03712483e5e03de4a3441ea32b1
Gerrit-Change-Number: 22489
Gerrit-PatchSet: 1
Gerrit-Owner: Vadim Bendebury <vbendeb at chromium.org>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.coreboot.org/pipermail/coreboot-gerrit/attachments/20171117/78f0e8e6/attachment.html>


More information about the coreboot-gerrit mailing list