Attention is currently required from: Michał Żygowski, Christian Walter, Julius Werner, Krystian Hebel.
Hello Michał Żygowski, Julius Werner, Krystian Hebel,
I'd like you to do a code review. Please visit
https://review.coreboot.org/c/coreboot/+/69162
to review the following change.
Change subject: security/tpm: support compiling in multiple TPM drivers ......................................................................
security/tpm: support compiling in multiple TPM drivers
Starting from here CONFIG_TPM1 and CONFIG_TPM2 are no longer mutually exclusive.
Change-Id: I44c5a1d825afe414c2f5c2c90f4cfe41ba9bef5f Ticket: https://ticket.coreboot.org/issues/433 Signed-off-by: Sergii Dmytruk sergii.dmytruk@3mdeb.com --- M src/drivers/crb/tis.c M src/drivers/i2c/tpm/Makefile.inc M src/drivers/i2c/tpm/tis.c M src/drivers/i2c/tpm/tis_atmel.c M src/drivers/pc80/tpm/tis.c M src/drivers/spi/tpm/tis.c M src/lib/program.ld M src/security/tpm/Kconfig M src/security/tpm/tis.h M src/security/tpm/tss/tss.c 10 files changed, 61 insertions(+), 31 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/62/69162/1
diff --git a/src/drivers/crb/tis.c b/src/drivers/crb/tis.c index 812947d..d84f22d 100644 --- a/src/drivers/crb/tis.c +++ b/src/drivers/crb/tis.c @@ -46,7 +46,7 @@ return 0; }
-tis_sendrecv_fn tis_probe(int *tpm_family) +static tis_sendrecv_fn crb_tis_probe(int *tpm_family) { struct tpm2_info info;
@@ -72,6 +72,8 @@ return &crb_tpm_sendrecv; }
+static const __tis_driver tis_probe_fn crb_tis_driver = crb_tis_probe; + static void crb_tpm_fill_ssdt(const struct device *dev) { const char *path = acpi_device_path(dev); diff --git a/src/drivers/i2c/tpm/Makefile.inc b/src/drivers/i2c/tpm/Makefile.inc index ae50f2b..8a13672 100644 --- a/src/drivers/i2c/tpm/Makefile.inc +++ b/src/drivers/i2c/tpm/Makefile.inc @@ -1,14 +1,9 @@ ifeq ($(CONFIG_TPM)$(CONFIG_I2C_TPM),yy)
all-$(CONFIG_DRIVER_TIS_DEFAULT) += tis.c - -ifeq ($(CONFIG_TPM_ATMEL),y) -all-y += tis_atmel.c -else ifeq ($(CONFIG_TPM_GOOGLE),y) -all-y += cr50.c -else +all-$(CONFIG_TPM_ATMEL) += tis_atmel.c +all-$(CONFIG_TPM_GOOGLE) += cr50.c all-y += tpm.c -endif
endif
diff --git a/src/drivers/i2c/tpm/tis.c b/src/drivers/i2c/tpm/tis.c index 68e2a67..3d394b9 100644 --- a/src/drivers/i2c/tpm/tis.c +++ b/src/drivers/i2c/tpm/tis.c @@ -119,7 +119,7 @@ return 0; }
-tis_sendrecv_fn tis_probe(int *tpm_family) +static tis_sendrecv_fn i2c_tis_probe(int *tpm_family) { if (tpm_vendor_probe(CONFIG_DRIVER_TPM_I2C_BUS, CONFIG_DRIVER_TPM_I2C_ADDR, tpm_family)) return NULL; @@ -136,3 +136,5 @@
return &i2c_tpm_sendrecv; } + +static const __tis_driver tis_probe_fn i2c_tis_driver = i2c_tis_probe; diff --git a/src/drivers/i2c/tpm/tis_atmel.c b/src/drivers/i2c/tpm/tis_atmel.c index b1a6ccd..58176a3 100644 --- a/src/drivers/i2c/tpm/tis_atmel.c +++ b/src/drivers/i2c/tpm/tis_atmel.c @@ -103,8 +103,10 @@ return 0; }
-tis_sendrecv_fn tis_probe(int *tpm_family) +static tis_sendrecv_fn atmel_i2c_tis_probe(int *tpm_family) { *tpm_family = 1; return &i2c_tis_sendrecv; } + +static const __tis_driver tis_probe_fn atmel_i2c_tis_driver = atmel_i2c_tis_probe; diff --git a/src/drivers/pc80/tpm/tis.c b/src/drivers/pc80/tpm/tis.c index 937de43..b21191c 100644 --- a/src/drivers/pc80/tpm/tis.c +++ b/src/drivers/pc80/tpm/tis.c @@ -387,7 +387,7 @@ * Returns 0 on success (the device is found or was found during an earlier * invocation) or TPM_DRIVER_ERR if the device is not found. */ -static u32 pc80_tis_probe(int *tpm_family) +static u32 pc80_tpm_probe(int *tpm_family) { const char *device_name = "unknown"; const char *vendor_name = device_name; @@ -708,7 +708,7 @@ }
/* - * tis_probe() + * pc80_tis_probe() * * Probe for the TPM device and set it up for use within locality 0. * @@ -716,9 +716,9 @@ * * Returns pointer to send-receive function on success or NULL on failure. */ -tis_sendrecv_fn tis_probe(int *tpm_family) +static tis_sendrecv_fn pc80_tis_probe(int *tpm_family) { - if (pc80_tis_probe(tpm_family)) + if (pc80_tpm_probe(tpm_family)) return NULL;
if (pc80_tis_open()) @@ -727,6 +727,8 @@ return &pc80_tpm_sendrecv; }
+static const __tis_driver tis_probe_fn pc80_tis_driver = pc80_tis_probe; + /* * tis_setup_interrupt() * diff --git a/src/drivers/spi/tpm/tis.c b/src/drivers/spi/tpm/tis.c index 44576a2..95aeb0c 100644 --- a/src/drivers/spi/tpm/tis.c +++ b/src/drivers/spi/tpm/tis.c @@ -40,7 +40,7 @@ return 0; }
-tis_sendrecv_fn tis_probe(int *tpm_family) +static tis_sendrecv_fn spi_tis_probe(int *tpm_family) { struct spi_slave spi; struct tpm2_info info; @@ -65,3 +65,5 @@
return &tpm_sendrecv; } + +static const __tis_driver tis_probe_fn spi_tis_driver = spi_tis_probe; diff --git a/src/lib/program.ld b/src/lib/program.ld index 67f685f..e552d63 100644 --- a/src/lib/program.ld +++ b/src/lib/program.ld @@ -39,6 +39,12 @@ _ersbe_init_begin = .; RECORD_SIZE(rsbe_init_begin)
+ . = ALIGN(ARCH_POINTER_ALIGN_SIZE); + _tis_drivers = .; + KEEP(*(.rodata.tis_driver)); + _etis_drivers = .; + RECORD_SIZE(tis_drivers) + #if ENV_RAMSTAGE . = ALIGN(ARCH_POINTER_ALIGN_SIZE); _pci_drivers = .; diff --git a/src/security/tpm/Kconfig b/src/security/tpm/Kconfig index fc339a2..9c859f9 100644 --- a/src/security/tpm/Kconfig +++ b/src/security/tpm/Kconfig @@ -4,14 +4,9 @@
menu "Trusted Platform Module"
-choice - prompt "Trusted Platform Module" - default TPM2 if MAINBOARD_HAS_TPM2 - default TPM1 if MAINBOARD_HAS_TPM1 - default NO_TPM - config NO_TPM - bool "No TPM" + bool + default y if !TPM1 && !TPM2 help No TPM support. Select this option if your system doesn't have a TPM, or if you don't want coreboot to communicate with your TPM in any way. @@ -21,19 +16,17 @@ config TPM1 bool "TPM 1.2" depends on I2C_TPM || MEMORY_MAPPED_TPM || SPI_TPM || CRB_TPM - depends on !MAINBOARD_HAS_TPM2 + default y if MAINBOARD_HAS_TPM1 help Select this option if your TPM uses the older TPM 1.2 protocol.
config TPM2 bool "TPM 2.0" depends on I2C_TPM || MEMORY_MAPPED_TPM || SPI_TPM || CRB_TPM - depends on !MAINBOARD_HAS_TPM1 + default y if MAINBOARD_HAS_TPM2 help Select this option if your TPM uses the newer TPM 2.0 protocol.
-endchoice - config TPM bool default y @@ -52,7 +45,7 @@ always uses the 2.0 protocol, and that it should be on by default.
config TPM_DEACTIVATE - bool "Deactivate TPM" + bool "Deactivate TPM (for TPM1)" default n depends on !VBOOT depends on TPM1 diff --git a/src/security/tpm/tis.h b/src/security/tpm/tis.h index eab62bd..c4a2df7 100644 --- a/src/security/tpm/tis.h +++ b/src/security/tpm/tis.h @@ -48,15 +48,13 @@ size_t *recv_len);
/* - * tis_probe() - * * Probe for the TPM device and set it up for use within locality 0. * * @tpm_family - pointer to int which is set to TPM family of the device (1 or 2) * * Returns pointer to send-receive function on success or NULL on failure. */ -tis_sendrecv_fn tis_probe(int *tpm_family); +typedef tis_sendrecv_fn (*tis_probe_fn)(int *tpm_family);
/* TODO: This is supposed to be used only for Google TPM. Consider moving this to drivers/tpm/cr50.h. */ @@ -100,4 +98,11 @@ return ENV_SEPARATE_VERSTAGE || ENV_BOOTBLOCK || !CONFIG(VBOOT); }
+#define __tis_driver __attribute__((used, __section__(".rodata.tis_driver"))) + +/** start of compile time generated tis driver array */ +extern tis_probe_fn _tis_drivers[]; +/** end of compile time generated tis driver array */ +extern tis_probe_fn _etis_drivers[]; + #endif /* TIS_H_ */ diff --git a/src/security/tpm/tss/tss.c b/src/security/tpm/tss/tss.c index 381c45b1..571de1a 100644 --- a/src/security/tpm/tss/tss.c +++ b/src/security/tpm/tss/tss.c @@ -18,12 +18,19 @@ /* Probe for TPM device and chose implementation based on the returned TPM family. */ uint32_t tlcl_lib_init(void) { + tis_probe_fn *tis_probe; tis_sendrecv_fn tis_sendrecv;
if (tpm_family != 0) return VB2_SUCCESS;
- tis_sendrecv = tis_probe(&tpm_family); + tis_sendrecv = NULL; + for (tis_probe = _tis_drivers; tis_probe != _etis_drivers; tis_probe++) { + tis_sendrecv = (*tis_probe)(&tpm_family); + if (tis_sendrecv != NULL) + break; + } + if (tis_sendrecv == NULL) { printk(BIOS_ERR, "%s: tis_probe returned error\n", __func__); return VB2_ERROR_UNKNOWN;