Tim Wawrzynczak has submitted this change. ( https://review.coreboot.org/c/coreboot/+/61977 )
Change subject: drivers/tpm/spi: Refactor out some cr50-specific logic ......................................................................
drivers/tpm/spi: Refactor out some cr50-specific logic
Mainboards accessing the cr50 over an I2C bus may want to reuse some of the same firmware version and BOARD_CFG logic, therefore refactor this logic out into a bus-agnostic file, drivers/tpm/cr50.c. This file uses the new tis_vendor_read/write() functions in order to access the cr50 regardless of the bus which is physically used. In order to leave SPI devices intact, the tis_vendor_* functions are added to the SPI driver.
BUG=b:202246591 TEST=boot to OS on google/dratini, see the same FW version and board_cfg console prints as before the change.
Signed-off-by: Tim Wawrzynczak twawrzynczak@chromium.org Change-Id: Ie68618cbe026a2b9221f93d0fe41d0b2054e8091 Reviewed-on: https://review.coreboot.org/c/coreboot/+/61977 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Subrata Banik subratabanik@google.com --- M src/drivers/spi/tpm/tpm.c M src/drivers/spi/tpm/tpm.h M src/drivers/tpm/Makefile.inc A src/drivers/tpm/cr50.c A src/drivers/tpm/cr50.h M src/mainboard/google/dedede/mainboard.c M src/mainboard/google/volteer/mainboard.c M src/security/tpm/tis.h 8 files changed, 249 insertions(+), 163 deletions(-)
Approvals: build bot (Jenkins): Verified Subrata Banik: Looks good to me, approved
diff --git a/src/drivers/spi/tpm/tpm.c b/src/drivers/spi/tpm/tpm.c index 971e603..53b3a25 100644 --- a/src/drivers/spi/tpm/tpm.c +++ b/src/drivers/spi/tpm/tpm.c @@ -23,8 +23,6 @@
#include "tpm.h"
-#define TPM_LOCALITY_0_SPI_BASE 0x00d40000 - /* Assorted TPM2 registers for interface type FIFO. */ #define TPM_ACCESS_REG (TPM_LOCALITY_0_SPI_BASE + 0) #define TPM_STS_REG (TPM_LOCALITY_0_SPI_BASE + 0x18) @@ -34,14 +32,6 @@ #define TPM_FW_VER (TPM_LOCALITY_0_SPI_BASE + 0xf90) #define CR50_BOARD_CFG (TPM_LOCALITY_0_SPI_BASE + 0xfe0)
-#define CR50_BOARD_CFG_LOCKBIT_MASK 0x80000000U -#define CR50_BOARD_CFG_FEATUREBITS_MASK 0x3FFFFFFFU - -#define CR50_BOARD_CFG_100US_READY_PULSE 0x00000001U -#define CR50_BOARD_CFG_VALUE \ - (CONFIG(CR50_USE_LONG_INTERRUPT_PULSES) \ - ? CR50_BOARD_CFG_100US_READY_PULSE : 0) - #define CR50_TIMEOUT_INIT_MS 30000 /* Very long timeout for TPM init */
/* SPI slave structure for TPM device. */ @@ -49,7 +39,6 @@
/* Cached TPM device identification. */ static struct tpm2_info tpm_info; -static struct cr50_firmware_version cr50_firmware_version;
/* * TODO(vbendeb): make CONFIG(DEBUG_TPM) an int to allow different level of @@ -427,114 +416,12 @@ return CB_ERR; }
-static enum cb_err cr50_parse_fw_version(const char *version_str, - struct cr50_firmware_version *ver) -{ - int epoch, major, minor; - - char *number = strstr(version_str, " RW_A:"); - if (!number) - number = strstr(version_str, " RW_B:"); - if (!number) - return CB_ERR_ARG; - number += 6; /* Skip past the colon. */ - - epoch = skip_atoi(&number); - if (*number++ != '.') - return CB_ERR_ARG; - major = skip_atoi(&number); - if (*number++ != '.') - return CB_ERR_ARG; - minor = skip_atoi(&number); - - ver->epoch = epoch; - ver->major = major; - ver->minor = minor; - return CB_SUCCESS; -} - -static bool cr50_fw_supports_board_cfg(struct cr50_firmware_version *version) -{ - /* Cr50 supports the CR50_BOARD_CFG register from version 0.5.5 / 0.6.5 - * and onwards. */ - if (version->epoch > 0 || version->major >= 7 - || (version->major >= 5 && version->minor >= 5)) - return true; - printk(BIOS_INFO, "Cr50 firmware does not support CR50_BOARD_CFG, version: %d.%d.%d\n", - version->epoch, version->major, version->minor); - return false; -} - -/** - * Set the BOARD_CFG register on the TPM chip to a particular compile-time constant value. - */ -static void cr50_set_board_cfg(void) -{ - uint32_t board_cfg_value; - if (!cr50_fw_supports_board_cfg(&cr50_firmware_version)) - return; - /* Set the CR50_BOARD_CFG register, for e.g. asking cr50 to use longer ready pulses. */ - if (tpm2_read_reg(CR50_BOARD_CFG, &board_cfg_value, sizeof(board_cfg_value)) - != CB_SUCCESS) { - printk(BIOS_INFO, "Error reading from cr50\n"); - return; - } - if ((board_cfg_value & CR50_BOARD_CFG_FEATUREBITS_MASK) == CR50_BOARD_CFG_VALUE) { - printk(BIOS_INFO, - "Current CR50_BOARD_CFG = 0x%08x, matches desired = 0x%08x\n", - board_cfg_value, CR50_BOARD_CFG_VALUE); - return; - } - if (board_cfg_value & CR50_BOARD_CFG_LOCKBIT_MASK) { - /* The high bit is set, meaning that the Cr50 is already locked on a particular - * value for the register, but not the one we wanted. */ - printk(BIOS_ERR, - "Current CR50_BOARD_CFG = 0x%08x, does not match desired = 0x%08x\n", - board_cfg_value, CR50_BOARD_CFG_VALUE); - return; - } - printk(BIOS_INFO, "Current CR50_BOARD_CFG = 0x%08x, setting to 0x%08x\n", - board_cfg_value, CR50_BOARD_CFG_VALUE); - board_cfg_value = CR50_BOARD_CFG_VALUE; - if (tpm2_write_reg(CR50_BOARD_CFG, &board_cfg_value, sizeof(board_cfg_value)) - != CB_SUCCESS) - printk(BIOS_INFO, "Error writing to cr50\n"); -} - -/* - * Expose method to read the CR50_BOARD_CFG register, will return zero if - * register not supported by Cr50 firmware. - */ -static uint32_t cr50_get_board_cfg(void) -{ - uint32_t board_cfg_value; - if (!cr50_fw_supports_board_cfg(&cr50_firmware_version)) - return 0; - - if (tpm2_read_reg(CR50_BOARD_CFG, &board_cfg_value, sizeof(board_cfg_value)) - != CB_SUCCESS) { - printk(BIOS_INFO, "Error reading from cr50\n"); - return 0; - } - return board_cfg_value & CR50_BOARD_CFG_FEATUREBITS_MASK; -} - -bool cr50_is_long_interrupt_pulse_enabled(void) -{ - return cr50_get_board_cfg() & CR50_BOARD_CFG_100US_READY_PULSE; -} - /* Device/vendor ID values of the TPM devices this driver supports. */ static const uint32_t supported_did_vids[] = { 0x00281ae0, /* H1 based Cr50 security chip. */ 0x0000104a /* ST33HTPH2E32 */ };
-static bool first_access_this_boot(void) -{ - return ENV_SEPARATE_VERSTAGE || ENV_BOOTBLOCK || !CONFIG(VBOOT); -} - int tpm2_init(struct spi_slave *spi_if) { uint32_t did_vid, status; @@ -579,7 +466,7 @@ printk(BIOS_INFO, " done!\n");
// FIXME: Move this to tpm_setup() - if (first_access_this_boot()) + if (tpm_first_access_this_boot()) /* * Claim locality 0, do it only during the first * initialization after reset. @@ -609,40 +496,15 @@ printk(BIOS_INFO, "Connected to device vid:did:rid of %4.4x:%4.4x:%2.2x\n", tpm_info.vendor_id, tpm_info.device_id, tpm_info.revision);
- /* Let's report device FW version if available. */ + /* Do some cr50-specific things here. */ if (CONFIG(TPM_CR50) && tpm_info.vendor_id == 0x1ae0) { - int chunk_count = 0; - size_t chunk_size = 50; - char version_str[301]; + struct cr50_firmware_version ver;
- /* - * Does not really matter what's written, this just makes sure - * the version is reported from the beginning. - */ - tpm2_write_reg(TPM_FW_VER, &chunk_size, 1); - - /* - * Read chunk_size bytes at a time, last chunk will be zero padded. - */ - do { - tpm2_read_reg(TPM_FW_VER, - version_str + chunk_count * chunk_size, - chunk_size); - if (!version_str[++chunk_count * chunk_size - 1]) - /* Zero padding detected: end of string. */ - break; - /* Check if there is enough room for reading one more chunk. */ - } while (chunk_count * chunk_size < sizeof(version_str) - chunk_size); - version_str[chunk_count * chunk_size] = '\0'; - printk(BIOS_INFO, "Firmware version: %s\n", version_str); - - if (cr50_parse_fw_version(version_str, &cr50_firmware_version) != CB_SUCCESS) { - printk(BIOS_ERR, "Did not recognize Cr50 version format\n"); - return -1; - } - if (CR50_BOARD_CFG_VALUE) { - if (first_access_this_boot()) - cr50_set_board_cfg(); + if (tpm_first_access_this_boot()) { + /* This is called for the side-effect of printing the firmware version + string */ + cr50_get_firmware_version(&ver); + cr50_set_board_cfg(); } } return 0; @@ -866,7 +728,12 @@ return payload_size; }
-void cr50_get_firmware_version(struct cr50_firmware_version *version) +cb_err_t tis_vendor_write(unsigned int addr, const void *buffer, size_t bytes) { - memcpy(version, &cr50_firmware_version, sizeof(*version)); + return tpm2_write_reg(addr, buffer, bytes); +} + +cb_err_t tis_vendor_read(unsigned int addr, void *buffer, size_t bytes) +{ + return tpm2_read_reg(addr, buffer, bytes); } diff --git a/src/drivers/spi/tpm/tpm.h b/src/drivers/spi/tpm/tpm.h index f48943e..cb09148 100644 --- a/src/drivers/spi/tpm/tpm.h +++ b/src/drivers/spi/tpm/tpm.h @@ -3,9 +3,12 @@ #ifndef __COREBOOT_SRC_DRIVERS_SPI_TPM_TPM_H #define __COREBOOT_SRC_DRIVERS_SPI_TPM_TPM_H
+#include <drivers/tpm/cr50.h> #include <stddef.h> #include <spi-generic.h>
+#define TPM_LOCALITY_0_SPI_BASE 0x00d40000 + /* * A tpm device descriptor, values read from the appropriate device regisrers * are cached here. @@ -16,13 +19,6 @@ uint16_t revision; };
-/* Structure describing the elements of Cr50 firmware version. */ -struct cr50_firmware_version { - int epoch; - int major; - int minor; -}; - /* * Initialize a TPM2 device: read its id, claim locality of zero, verify that * this indeed is a TPM2 device. Use the passed in handle to access the right @@ -47,10 +43,4 @@ /* Get information about previously initialized TPM device. */ void tpm2_get_info(struct tpm2_info *info);
-/* Indicates whether Cr50 ready pulses are guaranteed to be at least 100us. */ -bool cr50_is_long_interrupt_pulse_enabled(void); - -/* Get the cr50 firmware version information. */ -void cr50_get_firmware_version(struct cr50_firmware_version *version); - #endif /* ! __COREBOOT_SRC_DRIVERS_SPI_TPM_TPM_H */ diff --git a/src/drivers/tpm/Makefile.inc b/src/drivers/tpm/Makefile.inc index 9833eb4..a56c02b 100644 --- a/src/drivers/tpm/Makefile.inc +++ b/src/drivers/tpm/Makefile.inc @@ -5,3 +5,9 @@ else ramstage-$(CONFIG_HAVE_ACPI_TABLES) += ppi_stub.c endif + +bootblock-$(CONFIG_TPM_CR50) += cr50.c +verstage-$(CONFIG_TPM_CR50) += cr50.c +romstage-$(CONFIG_TPM_CR50) += cr50.c +ramstage-$(CONFIG_TPM_CR50) += cr50.c +postcar-$(CONFIG_TPM_CR50) += cr50.c diff --git a/src/drivers/tpm/cr50.c b/src/drivers/tpm/cr50.c new file mode 100644 index 0000000..c8ab5e4 --- /dev/null +++ b/src/drivers/tpm/cr50.c @@ -0,0 +1,195 @@ +/* SPDX-License-Identifier: BSD-3-Clause */ + +#include <drivers/spi/tpm/tpm.h> +#include <security/tpm/tis.h> +#include <string.h> +#include <types.h> + +#define CR50_BOARD_CFG_LOCKBIT_MASK 0x80000000U +#define CR50_BOARD_CFG_FEATUREBITS_MASK 0x3FFFFFFFU + +#define CR50_BOARD_CFG_100US_READY_PULSE 0x00000001U +#define CR50_BOARD_CFG_VALUE \ + (CONFIG(CR50_USE_LONG_INTERRUPT_PULSES) \ + ? CR50_BOARD_CFG_100US_READY_PULSE : 0) + +static struct cr50_firmware_version cr50_firmware_version; + +enum cr50_register { + CR50_FW_VER_REG, + CR50_BOARD_CFG_REG, +}; + +#define CR50_FW_VER_REG_SPI (TPM_LOCALITY_0_SPI_BASE + 0xf90) +#define CR50_BOARD_CFG_REG_SPI (TPM_LOCALITY_0_SPI_BASE + 0xfe0) + +/* Return register address, which depends on the bus type, or -1 for error. */ +static int get_reg_addr(enum cr50_register reg) +{ + if (CONFIG(SPI_TPM)) { + switch (reg) { + case CR50_FW_VER_REG: + return CR50_FW_VER_REG_SPI; + case CR50_BOARD_CFG_REG: + return CR50_BOARD_CFG_REG_SPI; + default: + return -1; + } + } + + + return -1; +} + +static bool cr50_fw_supports_board_cfg(struct cr50_firmware_version *version) +{ + /* Cr50 supports the CR50_BOARD_CFG register from version 0.5.5 / 0.6.5 + * and onwards. */ + if (version->epoch > 0 || version->major >= 7 + || (version->major >= 5 && version->minor >= 5)) + return true; + + printk(BIOS_INFO, "Cr50 firmware does not support CR50_BOARD_CFG, version: %d.%d.%d\n", + version->epoch, version->major, version->minor); + + return false; +} + +/* + * Expose method to read the CR50_BOARD_CFG register, will return zero if + * register not supported by Cr50 firmware. + */ +static uint32_t cr50_get_board_cfg(void) +{ + uint32_t value; + + if (!cr50_fw_supports_board_cfg(&cr50_firmware_version)) + return 0; + + const cb_err_t ret = tis_vendor_read(get_reg_addr(CR50_BOARD_CFG_REG), &value, + sizeof(value)); + if (ret != CB_SUCCESS) { + printk(BIOS_INFO, "Error reading from cr50\n"); + return 0; + } + + return value & CR50_BOARD_CFG_FEATUREBITS_MASK; +} + +/** + * Set the BOARD_CFG register on the TPM chip to a particular compile-time constant value. + */ +cb_err_t cr50_set_board_cfg(void) +{ + uint32_t value; + cb_err_t ret; + + if (!cr50_fw_supports_board_cfg(&cr50_firmware_version)) + return CB_ERR; + + /* Set the CR50_BOARD_CFG register, for e.g. asking cr50 to use longer ready pulses. */ + ret = tis_vendor_read(get_reg_addr(CR50_BOARD_CFG_REG), &value, sizeof(value)); + if (ret != CB_SUCCESS) { + printk(BIOS_INFO, "Error reading from cr50\n"); + return CB_ERR; + } + + if ((value & CR50_BOARD_CFG_FEATUREBITS_MASK) == CR50_BOARD_CFG_VALUE) { + printk(BIOS_INFO, "Current CR50_BOARD_CFG = 0x%08x, matches desired = 0x%08x\n", + value, CR50_BOARD_CFG_VALUE); + return CB_SUCCESS; + } + + if (value & CR50_BOARD_CFG_LOCKBIT_MASK) { + /* The high bit is set, meaning that the Cr50 is already locked on a particular + * value for the register, but not the one we wanted. */ + printk(BIOS_ERR, "Current CR50_BOARD_CFG = 0x%08x, does not match" + "desired = 0x%08x\n", value, CR50_BOARD_CFG_VALUE); + return CB_ERR; + } + + printk(BIOS_INFO, "Current CR50_BOARD_CFG = 0x%08x, setting to 0x%08x\n", + value, CR50_BOARD_CFG_VALUE); + value = CR50_BOARD_CFG_VALUE; + + ret = tis_vendor_write(get_reg_addr(CR50_BOARD_CFG_REG), &value, sizeof(value)); + if (ret != CB_SUCCESS) { + printk(BIOS_ERR, "Error writing to cr50\n"); + return ret; + } + + return CB_SUCCESS; +} + +bool cr50_is_long_interrupt_pulse_enabled(void) +{ + return !!(cr50_get_board_cfg() & CR50_BOARD_CFG_100US_READY_PULSE); +} + +static cb_err_t cr50_parse_fw_version(const char *version_str, + struct cr50_firmware_version *ver) +{ + int epoch, major, minor; + + char *number = strstr(version_str, " RW_A:"); + if (!number) + number = strstr(version_str, " RW_B:"); + if (!number) + return CB_ERR_ARG; + number += 6; /* Skip past the colon. */ + + epoch = skip_atoi(&number); + if (*number++ != '.') + return CB_ERR_ARG; + major = skip_atoi(&number); + if (*number++ != '.') + return CB_ERR_ARG; + minor = skip_atoi(&number); + + ver->epoch = epoch; + ver->major = major; + ver->minor = minor; + return CB_SUCCESS; +} + +cb_err_t cr50_get_firmware_version(struct cr50_firmware_version *version) +{ + if (cr50_firmware_version.epoch || cr50_firmware_version.major || + cr50_firmware_version.minor) + goto success; + + int chunk_count = 0; + size_t chunk_size = 50; + char version_str[301]; + int addr = get_reg_addr(CR50_FW_VER_REG); + + /* + * Does not really matter what's written, this just makes sure + * the version is reported from the beginning. + */ + tis_vendor_write(addr, &chunk_size, 1); + + /* + * Read chunk_size bytes at a time, last chunk will be zero padded. + */ + do { + uint8_t *buf = (uint8_t *)version_str + chunk_count * chunk_size; + tis_vendor_read(addr, buf, chunk_size); + if (!version_str[++chunk_count * chunk_size - 1]) + /* Zero padding detected: end of string. */ + break; + /* Check if there is enough room for reading one more chunk. */ + } while (chunk_count * chunk_size < sizeof(version_str) - chunk_size); + + version_str[chunk_count * chunk_size] = '\0'; + printk(BIOS_INFO, "Firmware version: %s\n", version_str); + + if (cr50_parse_fw_version(version_str, &cr50_firmware_version) != CB_SUCCESS) { + printk(BIOS_ERR, "Did not recognize Cr50 version format\n"); + return CB_ERR; + } + +success: + *version = cr50_firmware_version; + return CB_SUCCESS; +} diff --git a/src/drivers/tpm/cr50.h b/src/drivers/tpm/cr50.h new file mode 100644 index 0000000..4fdd06b --- /dev/null +++ b/src/drivers/tpm/cr50.h @@ -0,0 +1,24 @@ +/* SPDX-License-Identifier: BSD-3-Clause */ + +#ifndef __DRIVERS_TPM_CR50_H__ +#define __DRIVERS_TPM_CR50_H__ + +#include <types.h> + +/* Structure describing the elements of Cr50 firmware version. */ +struct cr50_firmware_version { + int epoch; + int major; + int minor; +}; + +/* Indicates whether Cr50 ready pulses are guaranteed to be at least 100us. */ +bool cr50_is_long_interrupt_pulse_enabled(void); + +/* Get the Cr50 firmware version information. */ +cb_err_t cr50_get_firmware_version(struct cr50_firmware_version *version); + +/* Set the BOARD_CFG register depending on Cr50 Kconfigs */ +cb_err_t cr50_set_board_cfg(void); + +#endif /* __DRIVERS_TPM_CR50_H__ */ diff --git a/src/mainboard/google/dedede/mainboard.c b/src/mainboard/google/dedede/mainboard.c index 700b3e3..ca5a21c 100644 --- a/src/mainboard/google/dedede/mainboard.c +++ b/src/mainboard/google/dedede/mainboard.c @@ -4,7 +4,7 @@ #include <baseboard/variants.h> #include <console/console.h> #include <device/device.h> -#include <drivers/spi/tpm/tpm.h> +#include <drivers/tpm/cr50.h> #include <ec/ec.h> #include <security/tpm/tss.h> #include <soc/soc_chip.h> diff --git a/src/mainboard/google/volteer/mainboard.c b/src/mainboard/google/volteer/mainboard.c index 95b9ce7..7089ddd 100644 --- a/src/mainboard/google/volteer/mainboard.c +++ b/src/mainboard/google/volteer/mainboard.c @@ -3,7 +3,7 @@ #include <console/console.h> #include <baseboard/variants.h> #include <device/device.h> -#include <drivers/spi/tpm/tpm.h> +#include <drivers/tpm/cr50.h> #include <ec/ec.h> #include <fw_config.h> #include <gpio.h> diff --git a/src/security/tpm/tis.h b/src/security/tpm/tis.h index 3b65134..0ef5a8a 100644 --- a/src/security/tpm/tis.h +++ b/src/security/tpm/tis.h @@ -111,5 +111,9 @@ */ cb_err_t tis_vendor_read(unsigned int addr, void *recvbuf, size_t recv_size);
+static inline bool tpm_first_access_this_boot(void) +{ + return ENV_SEPARATE_VERSTAGE || ENV_BOOTBLOCK || !CONFIG(VBOOT); +}
#endif /* TIS_H_ */
-
Tim Wawrzynczak (Code Review)