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/+/69159
to review the following change.
Change subject: security/tpm: make tis_probe() return tpm_family ......................................................................
security/tpm: make tis_probe() return tpm_family
Via an out parameter. This is needed to be able to dynamically pick TSS implementation based on the information discovered on probing.
Change-Id: I5006e0cdfef76ff79ce9e1cf280fcd5515ae01b0 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/cr50.c M src/drivers/i2c/tpm/tis.c M src/drivers/i2c/tpm/tis_atmel.c M src/drivers/i2c/tpm/tpm.c M src/drivers/i2c/tpm/tpm.h M src/drivers/pc80/tpm/tis.c M src/drivers/spi/tpm/tis.c M src/security/tpm/tis.h M src/security/tpm/tss/tcg-1.2/tss.c M src/security/tpm/tss/tcg-2.0/tss.c 11 files changed, 66 insertions(+), 22 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/59/69159/1
diff --git a/src/drivers/crb/tis.c b/src/drivers/crb/tis.c index 0bb53c7..1955466 100644 --- a/src/drivers/crb/tis.c +++ b/src/drivers/crb/tis.c @@ -46,10 +46,12 @@ return 0; }
-tis_sendrecv_fn tis_probe(void) +tis_sendrecv_fn tis_probe(int *tpm_family) { struct tpm2_info info;
+ *tpm_family = 2; + /* Wake TPM up (if necessary) */ if (tpm2_init() != 0) return NULL; diff --git a/src/drivers/i2c/tpm/cr50.c b/src/drivers/i2c/tpm/cr50.c index d908823..fe307bb 100644 --- a/src/drivers/i2c/tpm/cr50.c +++ b/src/drivers/i2c/tpm/cr50.c @@ -411,8 +411,9 @@ chip->cancel = &cr50_i2c_tis_ready; }
-int tpm_vendor_probe(unsigned int bus, uint32_t addr) +int tpm_vendor_probe(unsigned int bus, uint32_t addr, int *tpm_family) { + *tpm_family = 2; return 0; }
diff --git a/src/drivers/i2c/tpm/tis.c b/src/drivers/i2c/tpm/tis.c index cc89924..68e2a67 100644 --- a/src/drivers/i2c/tpm/tis.c +++ b/src/drivers/i2c/tpm/tis.c @@ -119,9 +119,9 @@ return 0; }
-tis_sendrecv_fn tis_probe(void) +tis_sendrecv_fn tis_probe(int *tpm_family) { - if (tpm_vendor_probe(CONFIG_DRIVER_TPM_I2C_BUS, CONFIG_DRIVER_TPM_I2C_ADDR)) + if (tpm_vendor_probe(CONFIG_DRIVER_TPM_I2C_BUS, CONFIG_DRIVER_TPM_I2C_ADDR, tpm_family)) return NULL;
if (chip.is_open) { diff --git a/src/drivers/i2c/tpm/tis_atmel.c b/src/drivers/i2c/tpm/tis_atmel.c index 376586b..b1a6ccd 100644 --- a/src/drivers/i2c/tpm/tis_atmel.c +++ b/src/drivers/i2c/tpm/tis_atmel.c @@ -103,7 +103,8 @@ return 0; }
-tis_sendrecv_fn tis_probe(void) +tis_sendrecv_fn tis_probe(int *tpm_family) { + *tpm_family = 1; return &i2c_tis_sendrecv; } diff --git a/src/drivers/i2c/tpm/tpm.c b/src/drivers/i2c/tpm/tpm.c index 68b7042..47d4b42 100644 --- a/src/drivers/i2c/tpm/tpm.c +++ b/src/drivers/i2c/tpm/tpm.c @@ -451,13 +451,15 @@
/* Initialization of I2C TPM */
-int tpm_vendor_probe(unsigned int bus, uint32_t addr) +int tpm_vendor_probe(unsigned int bus, uint32_t addr, int *tpm_family) { struct stopwatch sw; uint8_t buf = 0; int ret; long sw_run_duration = SLEEP_DURATION_PROBE_MS;
+ *tpm_family = 1; + tpm_dev.chip_type = UNKNOWN; tpm_dev.bus = bus; tpm_dev.addr = addr; diff --git a/src/drivers/i2c/tpm/tpm.h b/src/drivers/i2c/tpm/tpm.h index d4176cc..1cd9890 100644 --- a/src/drivers/i2c/tpm/tpm.h +++ b/src/drivers/i2c/tpm/tpm.h @@ -51,7 +51,7 @@
/* ---------- Interface for TPM vendor ------------ */
-int tpm_vendor_probe(unsigned int bus, uint32_t addr); +int tpm_vendor_probe(unsigned int bus, uint32_t addr, int *tpm_family);
int tpm_vendor_init(struct tpm_chip *chip, unsigned int bus, uint32_t dev_addr);
diff --git a/src/drivers/pc80/tpm/tis.c b/src/drivers/pc80/tpm/tis.c index e2b6a02..e4cd05b 100644 --- a/src/drivers/pc80/tpm/tis.c +++ b/src/drivers/pc80/tpm/tis.c @@ -386,14 +386,14 @@ * 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(void) +static u32 pc80_tis_probe(int *tpm_family) { const char *device_name = "unknown"; const char *vendor_name = device_name; const struct device_name *dev; u32 didvid, intf_id; u16 vid, did; - int i, tpm_family; + int i;
if (vendor_dev_id) return 0; /* Already probed. */ @@ -405,7 +405,7 @@ }
intf_id = be32_to_cpu(tpm_read_intf_id(0)); - tpm_family = ((intf_id & 0xf) == 0xf ? 1 : 2); + *tpm_family = ((intf_id & 0xf) == 0xf ? 1 : 2);
vendor_dev_id = didvid;
@@ -420,7 +420,7 @@ } dev = &vendor_names[i].dev_names[j]; while (dev->dev_id != 0xffff) { - if (dev->dev_id == did && dev->tpm_family == tpm_family) { + if (dev->dev_id == did && dev->tpm_family == *tpm_family) { device_name = dev->dev_name; break; } @@ -440,7 +440,7 @@ return TPM_DRIVER_ERR; }
- printk(BIOS_INFO, "Found TPM %d %s (0x%04x) by %s (0x%04x)\n", tpm_family, device_name, + printk(BIOS_INFO, "Found TPM %d %s (0x%04x) by %s (0x%04x)\n", *tpm_family, device_name, did, vendor_name, vid); return 0; } @@ -709,12 +709,15 @@ /* * tis_probe() * - * Probe for the TPM device and set it up for use within locality 0. Returns - * pointer to send-receive function on success or NULL on failure. + * 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(void) +tis_sendrecv_fn tis_probe(int *tpm_family) { - if (pc80_tis_probe()) + if (pc80_tis_probe(tpm_family)) return NULL;
if (pc80_tis_open()) diff --git a/src/drivers/spi/tpm/tis.c b/src/drivers/spi/tpm/tis.c index 310b1c0..44576a2 100644 --- a/src/drivers/spi/tpm/tis.c +++ b/src/drivers/spi/tpm/tis.c @@ -40,11 +40,13 @@ return 0; }
-tis_sendrecv_fn tis_probe(void) +tis_sendrecv_fn tis_probe(int *tpm_family) { struct spi_slave spi; struct tpm2_info info;
+ *tpm_family = 2; + if (spi_setup_slave(CONFIG_DRIVER_TPM_SPI_BUS, CONFIG_DRIVER_TPM_SPI_CHIP, &spi)) { printk(BIOS_ERR, "Failed to setup TPM SPI slave\n"); diff --git a/src/security/tpm/tis.h b/src/security/tpm/tis.h index 513a28c..eab62bd 100644 --- a/src/security/tpm/tis.h +++ b/src/security/tpm/tis.h @@ -50,10 +50,13 @@ /* * tis_probe() * - * Probe for the TPM device and set it up for use within locality 0. Returns - * pointer to send-receive function on success or NULL on failure. + * 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(void); +tis_sendrecv_fn tis_probe(int *tpm_family);
/* TODO: This is supposed to be used only for Google TPM. Consider moving this to drivers/tpm/cr50.h. */ diff --git a/src/security/tpm/tss/tcg-1.2/tss.c b/src/security/tpm/tss/tcg-1.2/tss.c index ccdb6d7..f7c9788 100644 --- a/src/security/tpm/tss/tcg-1.2/tss.c +++ b/src/security/tpm/tss/tcg-1.2/tss.c @@ -144,13 +144,20 @@
uint32_t tlcl_lib_init(void) { + int tpm_family; + if (tis_sendrecv != NULL) return VB2_SUCCESS;
- tis_sendrecv = tis_probe(); + tis_sendrecv = tis_probe(&tpm_family); if (tis_sendrecv == NULL) return VB2_ERROR_UNKNOWN;
+ if (tpm_family != 1) { + tis_sendrecv = NULL; + return VB2_ERROR_UNKNOWN; + } + return VB2_SUCCESS; }
diff --git a/src/security/tpm/tss/tcg-2.0/tss.c b/src/security/tpm/tss/tcg-2.0/tss.c index f1acf9e..d270329 100644 --- a/src/security/tpm/tss/tcg-2.0/tss.c +++ b/src/security/tpm/tss/tcg-2.0/tss.c @@ -187,15 +187,24 @@ /* This function is called directly by vboot, uses vboot return types. */ uint32_t tlcl_lib_init(void) { + int tpm_family; + if (tis_sendrecv != NULL) return VB2_SUCCESS;
- tis_sendrecv = tis_probe(); + tis_sendrecv = tis_probe(&tpm_family); if (tis_sendrecv == NULL) { printk(BIOS_ERR, "%s: tis_probe returned error\n", __func__); return VB2_ERROR_UNKNOWN; }
+ if (tpm_family != 2) { + tis_sendrecv = NULL; + printk(BIOS_ERR, "%s: tis_probe returned unsupported TPM family: %d\n", + __func__, tpm_family); + return VB2_ERROR_UNKNOWN; + } + return VB2_SUCCESS; }
-
Sergii Dmytruk (Code Review)