[coreboot-gerrit] Change in coreboot[master]: spi/tpm: Make sure AP properly syncs up with Cr50

[Vadim Bendebury (Code Review)]

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


Change subject: spi/tpm: Make sure AP properly syncs up with Cr50
......................................................................

spi/tpm: Make sure AP properly syncs up with Cr50

When Cr50 TPM is being reset, it continues replying to the SPI bus
requests, sends wrong register values in response to read requests.

This patch makes sure that the TPM driver does not proceed unless
proper value is read from the TPM device identification register.

If the read value is till wrong after 10 retries taken with 10 ms
intervals, the driver gives up and declares TPM broken/unavailable.

BRANCH=cr50
BUG=b:68012381
TEST=ran a script resetting the Fizz device as soon as the "index
     0x1007 return code 0" string shows up in the AP console output.
     The script keeps rebooting the Fizz indefinitely, before this
     script Fizz would fail to read TPM properly and fall into
     recovery after no more than four reboots.

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



  git pull ssh://review.coreboot.org:29418/coreboot refs/changes/31/22231/1

diff --git a/src/drivers/spi/tpm/tpm.c b/src/drivers/spi/tpm/tpm.c
index 0b5a835..145f3a1 100644
--- a/src/drivers/spi/tpm/tpm.c
+++ b/src/drivers/spi/tpm/tpm.c
@@ -368,21 +368,48 @@
 	return 1;
 }
 
+/* Device/vendor ID values of the TPM devices this driver supports. */
+static const uint32_t supported_did_vids[] = {
+	0x281ae0  /* H1 based Cr50 security chip. */
+};
+
 int tpm2_init(struct spi_slave *spi_if)
 {
 	uint32_t did_vid, status;
 	uint8_t cmd;
 	struct tpm2_info *tpm_info = car_get_var_ptr(&g_tpm_info);
 	struct spi_slave *spi_slave = car_get_var_ptr(&g_spi_slave);
+	int retries;
 
 	memcpy(spi_slave, spi_if, sizeof(*spi_if));
 
 	/*
-	 * It is enough to check the first register read error status to bail
-	 * out in case of malfunctioning TPM.
+	 * 100 ms should be enough to synchronize with the TPM even under the
+	 * worst nested reset request conditions. In vast majority of cases
+	 * there would be no wait at all.
 	 */
-	if (!tpm2_read_reg(TPM_DID_VID_REG, &did_vid, sizeof(did_vid)))
+	for (retries = 10; retries > 0; retries--) {
+		int i;
+
+		tpm2_read_reg(TPM_DID_VID_REG, &did_vid, sizeof(did_vid));
+
+		for (i = 0; i < ARRAY_SIZE(supported_did_vids); i++)
+			if (did_vid == supported_did_vids[i])
+				break; /* Tpm is up and ready. */
+
+		if (i < ARRAY_SIZE(supported_did_vids))
+			break;
+
+		/* TPM might be resetting, let's retry in a bit. */
+		udelay(10 * 1000);
+		printk(BIOS_ERR, ".");
+	}
+
+	if (!retries) {
+		printk(BIOS_ERR, "%s: Failed to connect to the TPM\n",
+		       __func__);
 		return -1;
+	}
 
 	/* Claim locality 0. */
 	if (!tpm2_claim_locality())

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

Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I7e67ec62c2bf31077b9ae558e09214d07eccf96b
Gerrit-Change-Number: 22231
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/20171030/71c1117b/attachment.html>