Attention is currently required from: Michał Żygowski, Krystian Hebel.
Hello Michał Żygowski, Krystian Hebel,
I'd like you to do a code review. Please visit
https://review.coreboot.org/c/coreboot/+/68989
to review the following change.
Change subject: security/tpm: remove public tis_close() ......................................................................
security/tpm: remove public tis_close()
This function is never called from outside of drivers and src/drivers/pc80/tpm/tis.c is the only one doing it.
tpm_vendor_cleanup() also isn't needed as one of tis_close() functions was its only caller.
Change-Id: I9df76adfc21fca9fa1d1af7c40635ec0684ceb0f 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 9 files changed, 40 insertions(+), 86 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/89/68989/1
diff --git a/src/drivers/crb/tis.c b/src/drivers/crb/tis.c index bb1cf21..a7d4fa7 100644 --- a/src/drivers/crb/tis.c +++ b/src/drivers/crb/tis.c @@ -53,19 +53,6 @@ return 0; }
-int tis_close(void) -{ - if (tpm_is_open) { - /* - * Do we need to do something here, like waiting for a - * transaction to stop? - */ - tpm_is_open = 0; - } - - return 0; -} - int tis_init(void) { struct tpm2_info info; diff --git a/src/drivers/i2c/tpm/cr50.c b/src/drivers/i2c/tpm/cr50.c index b5cb32c..42eeff4 100644 --- a/src/drivers/i2c/tpm/cr50.c +++ b/src/drivers/i2c/tpm/cr50.c @@ -494,10 +494,6 @@ return 0; }
-void tpm_vendor_cleanup(struct tpm_chip *chip) -{ -} - enum cb_err tis_vendor_write(unsigned int addr, const void *buffer, size_t bytes) { return cr50_i2c_write(addr & 0xff, buffer, bytes) ? CB_ERR : CB_SUCCESS; diff --git a/src/drivers/i2c/tpm/tis.c b/src/drivers/i2c/tpm/tis.c index 80de2df..5b199be 100644 --- a/src/drivers/i2c/tpm/tis.c +++ b/src/drivers/i2c/tpm/tis.c @@ -39,16 +39,6 @@ return 0; }
-int tis_close(void) -{ - if (chip.is_open) { - tpm_vendor_cleanup(&chip); - chip.is_open = 0; - } - - return 0; -} - int tis_init(void) { return tpm_vendor_probe(CONFIG_DRIVER_TPM_I2C_BUS, diff --git a/src/drivers/i2c/tpm/tis_atmel.c b/src/drivers/i2c/tpm/tis_atmel.c index 3a87dec..669ac68 100644 --- a/src/drivers/i2c/tpm/tis_atmel.c +++ b/src/drivers/i2c/tpm/tis_atmel.c @@ -27,11 +27,6 @@ return 0; }
-int tis_close(void) -{ - return 0; -} - int tis_init(void) { return 0; diff --git a/src/drivers/i2c/tpm/tpm.c b/src/drivers/i2c/tpm/tpm.c index 840b947..606f14d 100644 --- a/src/drivers/i2c/tpm/tpm.c +++ b/src/drivers/i2c/tpm/tpm.c @@ -550,8 +550,3 @@ release_locality(chip, 0, 1); return -1; } - -void tpm_vendor_cleanup(struct tpm_chip *chip) -{ - release_locality(chip, chip->vendor.locality, 1); -} diff --git a/src/drivers/i2c/tpm/tpm.h b/src/drivers/i2c/tpm/tpm.h index eb4fef1..868b306 100644 --- a/src/drivers/i2c/tpm/tpm.h +++ b/src/drivers/i2c/tpm/tpm.h @@ -61,6 +61,4 @@
int tpm_vendor_init(struct tpm_chip *chip, unsigned int bus, uint32_t dev_addr);
-void tpm_vendor_cleanup(struct tpm_chip *chip); - #endif /* __DRIVERS_TPM_SLB9635_I2C_TPM_H__ */ diff --git a/src/drivers/pc80/tpm/tis.c b/src/drivers/pc80/tpm/tis.c index 06f5434..980c785 100644 --- a/src/drivers/pc80/tpm/tis.c +++ b/src/drivers/pc80/tpm/tis.c @@ -630,10 +630,30 @@ }
/* + * 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) +{ + u8 locality = 0; + if (tis_has_access(locality)) { + tis_drop_access(locality); + if (tis_wait_dropped_access(locality)) { + printf("%s:%d - failed to release locality %u\n", + __FILE__, __LINE__, locality); + return TPM_DRIVER_ERR; + } + } + return 0; +} + +/* * tis_open() * - * Requests access to locality 0 for the caller. After all commands have been - * completed the caller is supposed to call tis_close(). + * Requests access to locality 0 for the caller. * * Returns 0 on success, TPM_DRIVER_ERR on failure. */ @@ -664,27 +684,6 @@ }
/* - * 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). - */ -int tis_close(void) -{ - u8 locality = 0; - if (tis_has_access(locality)) { - tis_drop_access(locality); - if (tis_wait_dropped_access(locality)) { - printf("%s:%d - failed to release locality %u\n", - __FILE__, __LINE__, locality); - return TPM_DRIVER_ERR; - } - } - return 0; -} - -/* * tis_sendrecv() * * Send the requested data to the TPM and then try to get its response diff --git a/src/drivers/spi/tpm/tis.c b/src/drivers/spi/tpm/tis.c index 5106fc0..b9b2a4a 100644 --- a/src/drivers/spi/tpm/tis.c +++ b/src/drivers/spi/tpm/tis.c @@ -38,19 +38,6 @@ return 0; }
-int tis_close(void) -{ - if (tpm_is_open) { - /* - * Do we need to do something here, like waiting for a - * transaction to stop? - */ - tpm_is_open = 0; - } - - return 0; -} - int tis_init(void) { struct spi_slave spi; diff --git a/src/security/tpm/tis.h b/src/security/tpm/tis.h index 8868e1a..04a137f 100644 --- a/src/security/tpm/tis.h +++ b/src/security/tpm/tis.h @@ -44,23 +44,13 @@ /* * tis_open() * - * Requests access to locality 0 for the caller. After all commands have been - * completed the caller is supposed to call tis_close(). + * Requests access to locality 0 for the caller. * * Returns 0 on success, -1 on failure. */ int tis_open(void);
/* - * tis_close() - * - * terminate the current session with the TPM by releasing the locked - * locality. Returns 0 on success of -1 on failure (in case lock - * removal did not succeed). - */ -int tis_close(void); - -/* * tis_sendrecv() * * Send the requested data to the TPM and then try to get its response
-
Sergii Dmytruk (Code Review)