Philipp Deppenwiese submitted this change.

View Change

Approvals: build bot (Jenkins): Verified Philipp Deppenwiese: Looks good to me, approved Julius Werner: Looks good to me, approved
drivers/spi/tpm: Add support for non CR50 SPI TPM2

Add support for a STM SPI TPM2 by adding checks for CR50.
Tested using ST33HTPH2E32.

Change-Id: I015497ca078979a44ba2b84e4995493de1f7247b
Signed-off-by: Patrick Rudolph <patrick.rudolph@9elements.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/39693
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Philipp Deppenwiese <zaolin.daisuki@gmail.com>
Reviewed-by: Julius Werner <jwerner@chromium.org>
---
M src/drivers/spi/tpm/Kconfig
M src/drivers/spi/tpm/tis.c
M src/drivers/spi/tpm/tpm.c
M src/security/tpm/Kconfig
4 files changed, 89 insertions(+), 49 deletions(-)

diff --git a/src/drivers/spi/tpm/Kconfig b/src/drivers/spi/tpm/Kconfig
index be43e23..8c39a4a 100644
--- a/src/drivers/spi/tpm/Kconfig
+++ b/src/drivers/spi/tpm/Kconfig
@@ -16,6 +16,13 @@
config MAINBOARD_HAS_SPI_TPM_CR50
bool
default n
+ select MAINBOARD_HAS_SPI_TPM
+ help
+ Board has a CR50 SPI TPM
+
+config MAINBOARD_HAS_SPI_TPM
+ bool
+ default n
select SPI_TPM
help
Board has SPI TPM support
diff --git a/src/drivers/spi/tpm/tis.c b/src/drivers/spi/tpm/tis.c
index 6230751..60dc705 100644
--- a/src/drivers/spi/tpm/tis.c
+++ b/src/drivers/spi/tpm/tis.c
@@ -18,6 +18,7 @@
} dev_map[] = {
{ 0x15d1, 0x001b, "SLB9670" },
{ 0x1ae0, 0x0028, "CR50" },
+ { 0x104a, 0x0000, "ST33HTPH2E32" },
};

static const char *tis_get_dev_name(struct tpm2_info *info)
diff --git a/src/drivers/spi/tpm/tpm.c b/src/drivers/spi/tpm/tpm.c
index 8f93e2a..c47aed3 100644
--- a/src/drivers/spi/tpm/tpm.c
+++ b/src/drivers/spi/tpm/tpm.c
@@ -104,47 +104,51 @@
*/
static int start_transaction(int read_write, size_t bytes, unsigned int addr)
{
- spi_frame_header header;
+ spi_frame_header header, header_resp;
uint8_t byte;
int i;
+ int ret;
struct stopwatch sw;
static int tpm_sync_needed;
static struct stopwatch wake_up_sw;
- /*
- * First Cr50 access in each coreboot stage where TPM is used will be
- * prepended by a wake up pulse on the CS line.
- */
- int wakeup_needed = 1;

- /* Wait for TPM to finish previous transaction if needed */
- if (tpm_sync_needed) {
- tpm_sync();
+ if (CONFIG(TPM_CR50)) {
/*
- * During the first invocation of this function on each stage
- * this if () clause code does not run (as tpm_sync_needed
- * value is zero), during all following invocations the
- * stopwatch below is guaranteed to be started.
+ * First Cr50 access in each coreboot stage where TPM is used will be
+ * prepended by a wake up pulse on the CS line.
*/
- if (!stopwatch_expired(&wake_up_sw))
- wakeup_needed = 0;
- } else {
- tpm_sync_needed = 1;
- }
+ int wakeup_needed = 1;

- if (wakeup_needed) {
- /* Just in case Cr50 is asleep. */
- spi_claim_bus(&spi_slave);
- udelay(1);
- spi_release_bus(&spi_slave);
- udelay(100);
- }
+ /* Wait for TPM to finish previous transaction if needed */
+ if (tpm_sync_needed) {
+ tpm_sync();
+ /*
+ * During the first invocation of this function on each stage
+ * this if () clause code does not run (as tpm_sync_needed
+ * value is zero), during all following invocations the
+ * stopwatch below is guaranteed to be started.
+ */
+ if (!stopwatch_expired(&wake_up_sw))
+ wakeup_needed = 0;
+ } else {
+ tpm_sync_needed = 1;
+ }

- /*
- * The Cr50 on H1 does not go to sleep for 1 second after any
- * SPI slave activity, let's be conservative and limit the
- * window to 900 ms.
- */
- stopwatch_init_msecs_expire(&wake_up_sw, 900);
+ if (wakeup_needed) {
+ /* Just in case Cr50 is asleep. */
+ spi_claim_bus(&spi_slave);
+ udelay(1);
+ spi_release_bus(&spi_slave);
+ udelay(100);
+ }
+
+ /*
+ * The Cr50 on H1 does not go to sleep for 1 second after any
+ * SPI slave activity, let's be conservative and limit the
+ * window to 900 ms.
+ */
+ stopwatch_init_msecs_expire(&wake_up_sw, 900);
+ }

/*
* The first byte of the frame header encodes the transaction type
@@ -181,16 +185,30 @@
* transmitted by the TPM during the transaction's last byte.
*
* We know that cr50 is guaranteed to set the flow control bit to 0
- * during the header transfer, but real TPM2 might be fast enough not
- * to require to stall the master, this would present an issue.
+ * during the header transfer. Real TPM2 are fast enough to not require
+ * to stall the master. They might still use this feature, so test the
+ * last bit after shifting in the address bytes.
* crosbug.com/p/52132 has been opened to track this.
*/
- spi_xfer(&spi_slave, header.body, sizeof(header.body), NULL, 0);
+
+ header_resp.body[3] = 0;
+ if (CONFIG(TPM_CR50))
+ ret = spi_xfer(&spi_slave, header.body, sizeof(header.body), NULL, 0);
+ else
+ ret = spi_xfer(&spi_slave, header.body, sizeof(header.body),
+ header_resp.body, sizeof(header_resp.body));
+ if (ret) {
+ printk(BIOS_ERR, "SPI-TPM: transfer error\n");
+ spi_release_bus(&spi_slave);
+ return 0;
+ }
+
+ if (header_resp.body[3] & 1)
+ return 1;

/*
* Now poll the bus until TPM removes the stall bit. Give it up to 100
- * ms to sort it out - it could be saving stuff in nvram at some
- * point.
+ * ms to sort it out - it could be saving stuff in nvram at some point.
*/
stopwatch_init_msecs_expire(&sw, 100);
do {
@@ -201,6 +219,7 @@
}
spi_xfer(&spi_slave, NULL, 0, &byte, 1);
} while (!(byte & 1));
+
return 1;
}

@@ -408,7 +427,8 @@

/* Device/vendor ID values of the TPM devices this driver supports. */
static const uint32_t supported_did_vids[] = {
- 0x00281ae0 /* H1 based Cr50 security chip. */
+ 0x00281ae0, /* H1 based Cr50 security chip. */
+ 0x0000104a /* ST33HTPH2E32 */
};

int tpm2_init(struct spi_slave *spi_if)
@@ -454,7 +474,8 @@

printk(BIOS_INFO, " done!\n");

- if (ENV_SEPARATE_VERSTAGE || ENV_BOOTBLOCK)
+ // FIXME: Move this to tpm_setup()
+ if (ENV_SEPARATE_VERSTAGE || ENV_BOOTBLOCK || !CONFIG(VBOOT))
/*
* Claim locality 0, do it only during the first
* initialization after reset.
@@ -462,7 +483,10 @@
if (!tpm2_claim_locality())
return -1;

- read_tpm_sts(&status);
+ if (!read_tpm_sts(&status)) {
+ printk(BIOS_ERR, "Reading status reg failed\n");
+ return -1;
+ }
if ((status & TPM_STS_FAMILY_MASK) != TPM_STS_FAMILY_TPM_2_0) {
printk(BIOS_ERR, "unexpected TPM family value, status: %#x\n",
status);
diff --git a/src/security/tpm/Kconfig b/src/security/tpm/Kconfig
index d8652b2..359b8f1 100644
--- a/src/security/tpm/Kconfig
+++ b/src/security/tpm/Kconfig
@@ -18,15 +18,19 @@
config TPM1
bool
default y if MAINBOARD_HAS_TPM1 || USER_TPM1
- depends on MAINBOARD_HAS_LPC_TPM || MAINBOARD_HAS_I2C_TPM_GENERIC \
- || MAINBOARD_HAS_I2C_TPM_ATMEL
+ depends on MAINBOARD_HAS_LPC_TPM || \
+ MAINBOARD_HAS_I2C_TPM_GENERIC || \
+ MAINBOARD_HAS_I2C_TPM_ATMEL

config TPM2
bool
default y if MAINBOARD_HAS_TPM2 || USER_TPM2
- depends on MAINBOARD_HAS_I2C_TPM_GENERIC || MAINBOARD_HAS_LPC_TPM \
- || MAINBOARD_HAS_I2C_TPM_ATMEL || MAINBOARD_HAS_I2C_TPM_CR50 \
- || MAINBOARD_HAS_SPI_TPM_CR50 || MAINBOARD_HAS_CRB_TPM
+ depends on MAINBOARD_HAS_I2C_TPM_GENERIC || \
+ MAINBOARD_HAS_LPC_TPM || \
+ MAINBOARD_HAS_I2C_TPM_ATMEL || \
+ MAINBOARD_HAS_I2C_TPM_CR50 || \
+ MAINBOARD_HAS_SPI_TPM || \
+ MAINBOARD_HAS_CRB_TPM

config MAINBOARD_HAS_TPM1
bool
@@ -45,8 +49,9 @@

config USER_TPM1
bool "1.2"
- depends on MAINBOARD_HAS_LPC_TPM || MAINBOARD_HAS_I2C_TPM_GENERIC \
- || MAINBOARD_HAS_I2C_TPM_ATMEL
+ depends on MAINBOARD_HAS_LPC_TPM || \
+ MAINBOARD_HAS_I2C_TPM_GENERIC || \
+ MAINBOARD_HAS_I2C_TPM_ATMEL
help
Enable this option to enable TPM 1.0 - 1.2 support in coreboot.

@@ -54,9 +59,12 @@

config USER_TPM2
bool "2.0"
- depends on MAINBOARD_HAS_I2C_TPM_GENERIC || MAINBOARD_HAS_LPC_TPM \
- || MAINBOARD_HAS_I2C_TPM_ATMEL || MAINBOARD_HAS_I2C_TPM_CR50 \
- || MAINBOARD_HAS_SPI_TPM_CR50 || MAINBOARD_HAS_CRB_TPM
+ depends on MAINBOARD_HAS_I2C_TPM_GENERIC || \
+ MAINBOARD_HAS_LPC_TPM || \
+ MAINBOARD_HAS_I2C_TPM_ATMEL || \
+ MAINBOARD_HAS_I2C_TPM_CR50 || \
+ MAINBOARD_HAS_SPI_TPM || \
+ MAINBOARD_HAS_CRB_TPM
help
Enable this option to enable TPM 2.0 support in coreboot.


To view, visit change 39693. To unsubscribe, or for help writing mail filters, visit settings.

Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I015497ca078979a44ba2b84e4995493de1f7247b
Gerrit-Change-Number: 39693
Gerrit-PatchSet: 7
Gerrit-Owner: Patrick Rudolph <patrick.rudolph@9elements.com>
Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com>
Gerrit-Reviewer: Christian Walter <christian.walter@9elements.com>
Gerrit-Reviewer: Julius Werner <jwerner@chromium.org>
Gerrit-Reviewer: Philipp Deppenwiese <zaolin.daisuki@gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org>
Gerrit-CC: Michael Niewöhner
Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net>
Gerrit-MessageType: merged