Attention is currently required from: Michał Żygowski, Christian Walter, Krystian Hebel.
Hello Michał Żygowski, Krystian Hebel,
I'd like you to do a code review. Please visit
https://review.coreboot.org/c/coreboot/+/68991
to review the following change.
Change subject: security/tpm/: turn tis_{init,open} into tis_probe ......................................................................
security/tpm/: turn tis_{init,open} into tis_probe
Init was always followed by open and after successful initialization we need only send-receive function, which is now returned by tis_probe on success further reducing number of functions to export from drivers.
Change-Id: Ib4ce35ada24e3959ea1a518c29d431b4ae123809 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/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/security/tpm/tis.h M src/security/tpm/tss/tcg-1.2/tss.c M src/security/tpm/tss/tcg-2.0/tss.c 8 files changed, 140 insertions(+), 182 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/91/68991/1
diff --git a/src/drivers/crb/tis.c b/src/drivers/crb/tis.c index a7d4fa7..0bb53c7 100644 --- a/src/drivers/crb/tis.c +++ b/src/drivers/crb/tis.c @@ -14,8 +14,6 @@ #include "tpm.h" #include "chip.h"
-static unsigned int tpm_is_open; - static const struct { uint16_t vid; uint16_t did; @@ -35,41 +33,8 @@ return "Unknown"; }
-int tis_open(void) -{ - if (tpm_is_open) { - printk(BIOS_ERR, "%s called twice.\n", __func__); - return -1; - } - - if (CONFIG(HAVE_INTEL_PTT)) { - if (!ptt_active()) { - printk(BIOS_ERR, "%s: Intel PTT is not active.\n", __func__); - return -1; - } - printk(BIOS_DEBUG, "%s: Intel PTT is active.\n", __func__); - } - - return 0; -} - -int tis_init(void) -{ - struct tpm2_info info; - - // Wake TPM up (if necessary) - if (tpm2_init() != 0) - return -1; - - tpm2_get_info(&info); - - printk(BIOS_INFO, "Initialized TPM device %s revision %d\n", tis_get_dev_name(&info), - info.revision); - - return 0; -} - -int tis_sendrecv(const uint8_t *sendbuf, size_t sbuf_size, uint8_t *recvbuf, size_t *rbuf_len) +static int crb_tpm_sendrecv(const uint8_t *sendbuf, size_t sbuf_size, uint8_t *recvbuf, + size_t *rbuf_len) { int len = tpm2_process_command(sendbuf, sbuf_size, recvbuf, *rbuf_len);
@@ -81,6 +46,30 @@ return 0; }
+tis_sendrecv_fn tis_probe(void) +{ + struct tpm2_info info; + + /* Wake TPM up (if necessary) */ + if (tpm2_init() != 0) + return NULL; + + tpm2_get_info(&info); + + printk(BIOS_INFO, "Initialized TPM device %s revision %d\n", tis_get_dev_name(&info), + info.revision); + + if (CONFIG(HAVE_INTEL_PTT)) { + if (!ptt_active()) { + printk(BIOS_ERR, "%s: Intel PTT is not active.\n", __func__); + return NULL; + } + printk(BIOS_DEBUG, "%s: Intel PTT is active.\n", __func__); + } + + return &crb_tpm_sendrecv; +} + static void crb_tpm_fill_ssdt(const struct device *dev) { const char *path = acpi_device_path(dev); diff --git a/src/drivers/i2c/tpm/tis.c b/src/drivers/i2c/tpm/tis.c index f010bf3..cc89924 100644 --- a/src/drivers/i2c/tpm/tis.c +++ b/src/drivers/i2c/tpm/tis.c @@ -19,32 +19,6 @@ #define TPM_CMD_COUNT_BYTE 2 #define TPM_CMD_ORDINAL_BYTE 6
-int tis_open(void) -{ - int rc; - - if (chip.is_open) { - printk(BIOS_DEBUG, "%s() called twice.\n", __func__); - return -1; - } - - rc = tpm_vendor_init(&chip, CONFIG_DRIVER_TPM_I2C_BUS, - CONFIG_DRIVER_TPM_I2C_ADDR); - if (rc < 0) - chip.is_open = 0; - - if (rc) - return -1; - - return 0; -} - -int tis_init(void) -{ - return tpm_vendor_probe(CONFIG_DRIVER_TPM_I2C_BUS, - CONFIG_DRIVER_TPM_I2C_ADDR); -} - static ssize_t tpm_transmit(const uint8_t *sbuf, size_t sbufsiz, void *rbuf, size_t rbufsiz) { @@ -107,8 +81,8 @@ return rc; }
-int tis_sendrecv(const uint8_t *sendbuf, size_t sbuf_size, - uint8_t *recvbuf, size_t *rbuf_len) +static int i2c_tpm_sendrecv(const uint8_t *sendbuf, size_t sbuf_size, + uint8_t *recvbuf, size_t *rbuf_len) { ASSERT(sbuf_size >= 10);
@@ -144,3 +118,21 @@
return 0; } + +tis_sendrecv_fn tis_probe(void) +{ + if (tpm_vendor_probe(CONFIG_DRIVER_TPM_I2C_BUS, CONFIG_DRIVER_TPM_I2C_ADDR)) + return NULL; + + if (chip.is_open) { + printk(BIOS_DEBUG, "%s() called twice.\n", __func__); + return NULL; + } + + if (tpm_vendor_init(&chip, CONFIG_DRIVER_TPM_I2C_BUS, CONFIG_DRIVER_TPM_I2C_ADDR)) { + chip.is_open = 0; + return NULL; + } + + return &i2c_tpm_sendrecv; +} diff --git a/src/drivers/i2c/tpm/tis_atmel.c b/src/drivers/i2c/tpm/tis_atmel.c index 669ac68..376586b 100644 --- a/src/drivers/i2c/tpm/tis_atmel.c +++ b/src/drivers/i2c/tpm/tis_atmel.c @@ -22,18 +22,8 @@ uint32_t return_code; } __packed;
-int tis_open(void) -{ - return 0; -} - -int tis_init(void) -{ - return 0; -} - -int tis_sendrecv(const uint8_t *sendbuf, size_t sbuf_size, - uint8_t *recvbuf, size_t *rbuf_len) +static int i2c_tis_sendrecv(const uint8_t *sendbuf, size_t sbuf_size, + uint8_t *recvbuf, size_t *rbuf_len) { size_t hdr_bytes; struct tpm_output_header *header; @@ -112,3 +102,8 @@ /* Successful transfer */ return 0; } + +tis_sendrecv_fn tis_probe(void) +{ + return &i2c_tis_sendrecv; +} diff --git a/src/drivers/pc80/tpm/tis.c b/src/drivers/pc80/tpm/tis.c index 980c785..9f5f9af 100644 --- a/src/drivers/pc80/tpm/tis.c +++ b/src/drivers/pc80/tpm/tis.c @@ -382,7 +382,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 tis_probe(void) +static u32 pc80_tis_probe(void) { const char *device_name = "unknown"; const char *vendor_name = device_name; @@ -617,26 +617,11 @@ }
/* - * tis_init() - * - * Initialize the TPM device. Returns 0 on success or TPM_DRIVER_ERR on - * failure (in case device probing did not succeed). - */ -int tis_init(void) -{ - if (tis_probe()) - return TPM_DRIVER_ERR; - return 0; -} - -/* - * tis_close() - * * terminate the current session with the TPM by releasing the locked * locality. Returns 0 on success of TPM_DRIVER_ERR on failure (in case lock * removal did not succeed). */ -static int tis_close(void) +static int pc80_tis_close(void) { u8 locality = 0; if (tis_has_access(locality)) { @@ -651,17 +636,15 @@ }
/* - * tis_open() - * * Requests access to locality 0 for the caller. * * Returns 0 on success, TPM_DRIVER_ERR on failure. */ -int tis_open(void) +static int pc80_tis_open(void) { u8 locality = 0; /* we use locality zero for everything */
- if (tis_close()) + if (pc80_tis_close()) return TPM_DRIVER_ERR;
/* now request access to locality */ @@ -684,8 +667,6 @@ }
/* - * tis_sendrecv() - * * Send the requested data to the TPM and then try to get its response * * @sendbuf - buffer of the data to send @@ -696,8 +677,8 @@ * Returns 0 on success (and places the number of response bytes at recv_len) * or TPM_DRIVER_ERR on failure. */ -int tis_sendrecv(const uint8_t *sendbuf, size_t send_size, - uint8_t *recvbuf, size_t *recv_len) +static int pc80_tpm_sendrecv(const uint8_t *sendbuf, size_t send_size, + uint8_t *recvbuf, size_t *recv_len) { if (tis_senddata(sendbuf, send_size)) { printf("%s:%d failed sending data to TPM\n", @@ -709,6 +690,23 @@ }
/* + * 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. + */ +tis_sendrecv_fn tis_probe(void) +{ + if (pc80_tis_probe()) + return NULL; + + if (pc80_tis_open()) + return NULL; + + return &pc80_tpm_sendrecv; +} + +/* * tis_setup_interrupt() * * Set up the interrupt vector and polarity for locality 0 and @@ -729,7 +727,7 @@ int has_access = tis_has_access(locality);
/* Open connection and request access if not already granted */ - if (!has_access && tis_open() < 0) + if (!has_access && pc80_tis_open() < 0) return TPM_DRIVER_ERR;
/* Set TPM interrupt vector */ @@ -739,7 +737,7 @@ tpm_write_int_polarity(polarity, locality);
/* Close connection if it was opened */ - if (!has_access && tis_close() < 0) + if (!has_access && pc80_tis_close() < 0) return TPM_DRIVER_ERR;
return 0; diff --git a/src/drivers/spi/tpm/tis.c b/src/drivers/spi/tpm/tis.c index b9b2a4a..310b1c0 100644 --- a/src/drivers/spi/tpm/tis.c +++ b/src/drivers/spi/tpm/tis.c @@ -5,8 +5,6 @@
#include "tpm.h"
-static unsigned int tpm_is_open; - static const struct { uint16_t vid; uint16_t did; @@ -29,41 +27,8 @@ return "Unknown"; }
-int tis_open(void) -{ - if (tpm_is_open) { - printk(BIOS_ERR, "%s() called twice.\n", __func__); - return -1; - } - return 0; -} - -int tis_init(void) -{ - struct spi_slave spi; - struct tpm2_info info; - - 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"); - return -1; - } - - if (tpm2_init(&spi)) { - printk(BIOS_ERR, "Failed to initialize TPM SPI interface\n"); - return -1; - } - - tpm2_get_info(&info); - - printk(BIOS_INFO, "Initialized TPM device %s revision %d\n", - tis_get_dev_name(&info), info.revision); - - return 0; -} - -int tis_sendrecv(const uint8_t *sendbuf, size_t sbuf_size, - uint8_t *recvbuf, size_t *rbuf_len) +static int tpm_sendrecv(const uint8_t *sendbuf, size_t sbuf_size, + uint8_t *recvbuf, size_t *rbuf_len) { int len = tpm2_process_command(sendbuf, sbuf_size, recvbuf, *rbuf_len);
@@ -74,3 +39,27 @@
return 0; } + +tis_sendrecv_fn tis_probe(void) +{ + struct spi_slave spi; + struct tpm2_info info; + + 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"); + return NULL; + } + + if (tpm2_init(&spi)) { + printk(BIOS_ERR, "Failed to initialize TPM SPI interface\n"); + return NULL; + } + + tpm2_get_info(&info); + + printk(BIOS_INFO, "Initialized TPM device %s revision %d\n", + tis_get_dev_name(&info), info.revision); + + return &tpm_sendrecv; +} diff --git a/src/security/tpm/tis.h b/src/security/tpm/tis.h index 04a137f..513a28c 100644 --- a/src/security/tpm/tis.h +++ b/src/security/tpm/tis.h @@ -34,25 +34,6 @@ };
/* - * tis_init() - * - * Initialize the TPM device. Returns 0 on success or -1 on - * failure (in case device probing did not succeed). - */ -int tis_init(void); - -/* - * tis_open() - * - * Requests access to locality 0 for the caller. - * - * Returns 0 on success, -1 on failure. - */ -int tis_open(void); - -/* - * tis_sendrecv() - * * Send the requested data to the TPM and then try to get its response * * @sendbuf - buffer of the data to send @@ -63,8 +44,16 @@ * Returns 0 on success (and places the number of response bytes at recv_len) * or -1 on failure. */ -int tis_sendrecv(const u8 *sendbuf, size_t send_size, u8 *recvbuf, - size_t *recv_len); +typedef int (*tis_sendrecv_fn)(const u8 *sendbuf, size_t send_size, u8 *recvbuf, + size_t *recv_len); + +/* + * 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. + */ +tis_sendrecv_fn tis_probe(void);
/* 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 52bc272..ccdb6d7 100644 --- a/src/security/tpm/tss/tcg-1.2/tss.c +++ b/src/security/tpm/tss/tcg-1.2/tss.c @@ -24,6 +24,8 @@ #include <console/console.h> #define VBDEBUG(format, args...) printk(BIOS_DEBUG, format, ## args)
+static tis_sendrecv_fn tis_sendrecv; + static int tpm_send_receive(const uint8_t *request, uint32_t request_length, uint8_t *response, @@ -140,19 +142,14 @@
/* Exported functions. */
-static uint8_t tlcl_init_done; - uint32_t tlcl_lib_init(void) { - if (tlcl_init_done) + if (tis_sendrecv != NULL) return VB2_SUCCESS;
- if (tis_init()) + tis_sendrecv = tis_probe(); + if (tis_sendrecv == NULL) return VB2_ERROR_UNKNOWN; - if (tis_open()) - return VB2_ERROR_UNKNOWN; - - tlcl_init_done = 1;
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 5d6cbf8..f1acf9e 100644 --- a/src/security/tpm/tss/tcg-2.0/tss.c +++ b/src/security/tpm/tss/tcg-2.0/tss.c @@ -16,6 +16,8 @@ * TPM2 specification. */
+static tis_sendrecv_fn tis_sendrecv; + void *tpm_process_command(TPM_CC command, void *command_body) { struct obuf ob; @@ -182,26 +184,18 @@ return TPM_SUCCESS; }
-static uint8_t tlcl_init_done; - /* This function is called directly by vboot, uses vboot return types. */ uint32_t tlcl_lib_init(void) { - if (tlcl_init_done) + if (tis_sendrecv != NULL) return VB2_SUCCESS;
- if (tis_init()) { - printk(BIOS_ERR, "%s: tis_init returned error\n", __func__); + tis_sendrecv = tis_probe(); + if (tis_sendrecv == NULL) { + printk(BIOS_ERR, "%s: tis_probe returned error\n", __func__); return VB2_ERROR_UNKNOWN; }
- if (tis_open()) { - printk(BIOS_ERR, "%s: tis_open returned error\n", __func__); - return VB2_ERROR_UNKNOWN; - } - - tlcl_init_done = 1; - return VB2_SUCCESS; }
-
Sergii Dmytruk (Code Review)