coreboot-gerrit December 2016

coreboot-gerrit@coreboot.org

1 participants
1140 discussions

Patch merged into coreboot/master: tpm2: handle failures more gracefully
by gerrit＠coreboot.org 16 Dec '16

16 Dec '16

the following patch was just integrated into master: commit 731ef9b7ad7087918a37db2d1d79ee53dfa9091b Author: Vadim Bendebury <vbendeb(a)chromium.org> Date: Thu Dec 15 21:49:23 2016 -0800 tpm2: handle failures more gracefully When trying to bring up a device with a malfunctioning TPM2 chip, the driver currently gets stuck waiting for SPI flow control, causing bricked devices. This patch puts a 100 ms cap on the waiting time - this should be enough even for a longest NVRAM save operation which could be under way on the TPM device. BRANCH=gru BUG=chrome-os-partner:59807 TEST=with a matching change in depthcharge, now a gru with corrupted SPI TPM comes up to the recovery screen (it was not showing signs of life before this change). Change-Id: I63ef5dde8dddd9afeae91e396c157a1a37d47c80 Signed-off-by: Vadim Bendebury <vbendeb(a)chromium.org> Reviewed-on: https://review.coreboot.org/17898 Reviewed-by: Aaron Durbin <adurbin(a)chromium.org> Tested-by: build bot (Jenkins) See https://review.coreboot.org/17898 for details. -gerrit

1 0

Patch set updated for coreboot: tpm2: handle failures more gracefully
by Vadim Bendebury 16 Dec '16

16 Dec '16

Vadim Bendebury (vbendeb(a)chromium.org) just uploaded a new patch set to gerrit, which you can find at https://review.coreboot.org/17898 -gerrit commit 2b9081e71046a91210f30e20d5a69cb0317a21a2 Author: Vadim Bendebury <vbendeb(a)chromium.org> Date: Thu Dec 15 21:49:23 2016 -0800 tpm2: handle failures more gracefully When trying to bring up a device with a malfunctioning TPM2 chip, the driver currently gets stuck waiting for SPI flow control, causing bricked devices. This patch puts a 100 ms cap on the waiting time - this should be enough even for a longest NVRAM save operation which could be under way on the TPM device. BRANCH=gru BUG=chrome-os-partner:59807 TEST=with a matching change in depthcharge, now a gru with corrupted SPI TPM comes up to the recovery screen (it was not showing signs of life before this change). Change-Id: I63ef5dde8dddd9afeae91e396c157a1a37d47c80 Signed-off-by: Vadim Bendebury <vbendeb(a)chromium.org> --- src/drivers/spi/tpm/tpm.c | 42 +++++++++++++++++++++++++++++++++--------- 1 file changed, 33 insertions(+), 9 deletions(-) diff --git a/src/drivers/spi/tpm/tpm.c b/src/drivers/spi/tpm/tpm.c index 5abfc45..b2bda8c 100644 --- a/src/drivers/spi/tpm/tpm.c +++ b/src/drivers/spi/tpm/tpm.c @@ -105,12 +105,15 @@ void tpm2_get_info(struct tpm2_info *info) /* * Each TPM2 SPI transaction starts the same: CS is asserted, the 4 byte * header is sent to the TPM, the master waits til TPM is ready to continue. + * + * Returns 1 on success, 0 on failure (TPM SPI flow control timeout.) */ -static void start_transaction(int read_write, size_t bytes, unsigned addr) +static int start_transaction(int read_write, size_t bytes, unsigned addr) { spi_frame_header header; uint8_t byte; int i; + struct stopwatch sw; /* * Give it 10 ms. TODO(vbendeb): remove this once cr50 SPS TPM driver @@ -159,10 +162,20 @@ static void start_transaction(int read_write, size_t bytes, unsigned addr) */ tpm_if.xfer(&tpm_if.slave, header.body, sizeof(header.body), NULL, 0); - /* Now poll the bus until TPM removes the stall bit. */ + /* + * Now poll the bus until TPM removes the stall bit. Give it up to 100 + * ms to sort it out - it could be saving stuff in nvram at some + * point. + */ + stopwatch_init_msecs_expire(&sw, 100); do { + if (stopwatch_expired(&sw)) { + printk(BIOS_ERR, "TPM flow control failure\n"); + return 0; + } tpm_if.xfer(&tpm_if.slave, NULL, 0, &byte, 1); } while (!(byte & 1)); + return 1; } /* @@ -243,13 +256,13 @@ static void read_bytes(void *buffer, size_t bytes) * To write a register, start transaction, transfer data to the TPM, deassert * CS when done. * - * Returns one to indicate success, zero (not yet implemented) to indicate - * failure. + * Returns one to indicate success, zero to indicate failure. */ static int tpm2_write_reg(unsigned reg_number, const void *buffer, size_t bytes) { trace_dump("W", reg_number, bytes, buffer, 0); - start_transaction(false, bytes, reg_number); + if (!start_transaction(false, bytes, reg_number)) + return 0; write_bytes(buffer, bytes); tpm_if.cs_deassert(&tpm_if.slave); return 1; @@ -259,12 +272,14 @@ static int tpm2_write_reg(unsigned reg_number, const void *buffer, size_t bytes) * To read a register, start transaction, transfer data from the TPM, deassert * CS when done. * - * Returns one to indicate success, zero (not yet implemented) to indicate - * failure. + * Returns one to indicate success, zero to indicate failure. */ static int tpm2_read_reg(unsigned reg_number, void *buffer, size_t bytes) { - start_transaction(true, bytes, reg_number); + if (!start_transaction(true, bytes, reg_number)) { + memset(buffer, 0, bytes); + return 0; + } read_bytes(buffer, bytes); tpm_if.cs_deassert(&tpm_if.slave); trace_dump("R", reg_number, bytes, buffer, 0); @@ -305,7 +320,12 @@ int tpm2_init(struct spi_slave *spi_if) memcpy(&tpm_if.slave, spi_if, sizeof(*spi_if)); - tpm2_read_reg(TPM_DID_VID_REG, &did_vid, sizeof(did_vid)); + /* + * It is enough to check the first register read error status to bail + * out in case of malfunctioning TPM. + */ + if (!tpm2_read_reg(TPM_DID_VID_REG, &did_vid, sizeof(did_vid))) + return -1; /* Try claiming locality zero. */ tpm2_read_reg(TPM_ACCESS_REG, &cmd, sizeof(cmd)); @@ -485,6 +505,10 @@ size_t tpm2_process_command(const void *tpm2_command, size_t command_size, union fifo_transfer_buffer fifo_buffer; const int HEADER_SIZE = 6; + /* Do not try using an uninitialized TPM. */ + if (!tpm_info.vendor_id) + return 0; + /* Skip the two byte tag, read the size field. */ payload_size = read_be32(cmd_body + 2);

1 0

Patch set updated for coreboot: tpm2: handle failures more gracefully
by Vadim Bendebury 16 Dec '16

16 Dec '16

Vadim Bendebury (vbendeb(a)chromium.org) just uploaded a new patch set to gerrit, which you can find at https://review.coreboot.org/17898 -gerrit commit 8dcde262fe4203851269b6c3459c66c083ba470f Author: Vadim Bendebury <vbendeb(a)chromium.org> Date: Thu Dec 15 21:49:23 2016 -0800 tpm2: handle failures more gracefully When trying to bring up a device with a malfunctioning TPM2 chip, the driver currently gets stuck waiting for SPI flow control, causing bricked devices. This patch puts a 100 ms cap on the waiting time - this should be enough even for a longest NVRAM save operation which could be under way on the TPM device. BRANCH=gru BUG=chrome-os-partner:59807 TEST=with a matching change in depthcharge, now a gru with corrupted SPI TPM comes up to the recovery screen (it was not showing signs of life before this change). Change-Id: I63ef5dde8dddd9afeae91e396c157a1a37d47c80 Signed-off-by: Vadim Bendebury <vbendeb(a)chromium.org> --- src/drivers/spi/tpm/tpm.c | 42 +++++++++++++++++++++++++++++++++--------- 1 file changed, 33 insertions(+), 9 deletions(-) diff --git a/src/drivers/spi/tpm/tpm.c b/src/drivers/spi/tpm/tpm.c index 5abfc45..7ffbbe9 100644 --- a/src/drivers/spi/tpm/tpm.c +++ b/src/drivers/spi/tpm/tpm.c @@ -105,12 +105,15 @@ void tpm2_get_info(struct tpm2_info *info) /* * Each TPM2 SPI transaction starts the same: CS is asserted, the 4 byte * header is sent to the TPM, the master waits til TPM is ready to continue. + * + * Returns 1 on success, 0 on failure (TPM SPI flow control timeout.) */ -static void start_transaction(int read_write, size_t bytes, unsigned addr) +static int start_transaction(int read_write, size_t bytes, unsigned addr) { spi_frame_header header; uint8_t byte; int i; + struct stopwatch sw; /* * Give it 10 ms. TODO(vbendeb): remove this once cr50 SPS TPM driver @@ -159,10 +162,20 @@ static void start_transaction(int read_write, size_t bytes, unsigned addr) */ tpm_if.xfer(&tpm_if.slave, header.body, sizeof(header.body), NULL, 0); - /* Now poll the bus until TPM removes the stall bit. */ + /* + * Now poll the bus until TPM removes the stall bit. Give it up to 100 + * ms to sort it out - it could be saving stuff in nvram at some + * point. + */ + stopwatch_init_msecs_expire(&sw, 100); do { + if (stopwatch_expired(&sw)) { + printk(BIOS_ERR, "TPM flow control failure\n"); + return 0; + } tpm_if.xfer(&tpm_if.slave, NULL, 0, &byte, 1); } while (!(byte & 1)); + return 1; } /* @@ -243,13 +256,13 @@ static void read_bytes(void *buffer, size_t bytes) * To write a register, start transaction, transfer data to the TPM, deassert * CS when done. * - * Returns one to indicate success, zero (not yet implemented) to indicate - * failure. + * Returns one to indicate success, zero to indicate failure. */ static int tpm2_write_reg(unsigned reg_number, const void *buffer, size_t bytes) { trace_dump("W", reg_number, bytes, buffer, 0); - start_transaction(false, bytes, reg_number); + if (!start_transaction(false, bytes, reg_number)) + return 0; write_bytes(buffer, bytes); tpm_if.cs_deassert(&tpm_if.slave); return 1; @@ -259,12 +272,14 @@ static int tpm2_write_reg(unsigned reg_number, const void *buffer, size_t bytes) * To read a register, start transaction, transfer data from the TPM, deassert * CS when done. * - * Returns one to indicate success, zero (not yet implemented) to indicate - * failure. + * Returns one to indicate success, zero to indicate failure. */ static int tpm2_read_reg(unsigned reg_number, void *buffer, size_t bytes) { - start_transaction(true, bytes, reg_number); + if (!start_transaction(true, bytes, reg_number)) { + memset(buffer, 0, bytes); + return 0; + } read_bytes(buffer, bytes); tpm_if.cs_deassert(&tpm_if.slave); trace_dump("R", reg_number, bytes, buffer, 0); @@ -305,7 +320,12 @@ int tpm2_init(struct spi_slave *spi_if) memcpy(&tpm_if.slave, spi_if, sizeof(*spi_if)); - tpm2_read_reg(TPM_DID_VID_REG, &did_vid, sizeof(did_vid)); + /* + * It is enough to check the first register read error status to bail + * out in case of malfunctioning tpm. + */ + if (!tpm2_read_reg(TPM_DID_VID_REG, &did_vid, sizeof(did_vid))) + return -1; /* Try claiming locality zero. */ tpm2_read_reg(TPM_ACCESS_REG, &cmd, sizeof(cmd)); @@ -485,6 +505,10 @@ size_t tpm2_process_command(const void *tpm2_command, size_t command_size, union fifo_transfer_buffer fifo_buffer; const int HEADER_SIZE = 6; + /* Do not try using an uninitialized TPM. */ + if (!tpm_info.vendor_id) + return 0; + /* Skip the two byte tag, read the size field. */ payload_size = read_be32(cmd_body + 2);

1 0

Patch merged into coreboot/master: samsung/exynos5420: Fix test for src < 0
by gerrit＠coreboot.org 16 Dec '16

16 Dec '16

the following patch was just integrated into master: commit 74add8b70f39801ff522fdc8c34d983e51a5a634 Author: Patrick Georgi <pgeorgi(a)chromium.org> Date: Thu Dec 15 15:51:13 2016 +0100 samsung/exynos5420: Fix test for src < 0 It was unsigned, not a good place to be for testing < 0. Change-Id: I126fe86422900bbae2c3ca16052be27985cfed53 Signed-off-by: Patrick Georgi <pgeorgi(a)chromium.org> Found-by: Coverity Scan #1241911 Reviewed-on: https://review.coreboot.org/17888 Reviewed-by: Nico Huber <nico.h(a)gmx.de> Tested-by: build bot (Jenkins) See https://review.coreboot.org/17888 for details. -gerrit

1 0

Patch merged into coreboot/master: util/romcc: remove self-assignment
by gerrit＠coreboot.org 16 Dec '16

16 Dec '16

the following patch was just integrated into master: commit da8421d1e2ee715b6ae396d1ab064c4c3f8a77fb Author: Patrick Georgi <pgeorgi(a)chromium.org> Date: Thu Dec 15 15:17:45 2016 +0100 util/romcc: remove self-assignment Change-Id: I0f78b55b28011cdefc90665bca2a7ea17647e955 Signed-off-by: Patrick Georgi <pgeorgi(a)chromium.org> Found-by: Coverity Scan #1129127 Reviewed-on: https://review.coreboot.org/17885 Tested-by: build bot (Jenkins) Reviewed-by: Martin Roth <martinroth(a)google.com> Reviewed-by: Paul Menzel <paulepanter(a)users.sourceforge.net> See https://review.coreboot.org/17885 for details. -gerrit

1 0

Patch merged into coreboot/master: util/romcc: Move access after NULL-check
by gerrit＠coreboot.org 16 Dec '16

16 Dec '16

the following patch was just integrated into master: commit f78e658dac989a8c467de4d32ecf3f86dbb8e245 Author: Patrick Georgi <pgeorgi(a)chromium.org> Date: Thu Dec 15 15:14:59 2016 +0100 util/romcc: Move access after NULL-check Change-Id: I7f9c38fd6e75b32fe1ed8a60c7054f4dd1fcd5c0 Signed-off-by: Patrick Georgi <pgeorgi(a)chromium.org> Found-by: Coverity Scan #1129104 Reviewed-on: https://review.coreboot.org/17884 Tested-by: build bot (Jenkins) Reviewed-by: Martin Roth <martinroth(a)google.com> See https://review.coreboot.org/17884 for details. -gerrit

1 0

Patch merged into coreboot/master: util/romcc: Fix resource leak
by gerrit＠coreboot.org 16 Dec '16

16 Dec '16

the following patch was just integrated into master: commit f23cba082c86bd7dd33ea3c26057206d790b3dab Author: Patrick Georgi <pgeorgi(a)chromium.org> Date: Thu Dec 15 15:12:16 2016 +0100 util/romcc: Fix resource leak Change-Id: I0d260254bab714ec939fc199b3a133b0fc05b10d Signed-off-by: Patrick Georgi <pgeorgi(a)chromium.org> Found-by: Coverity Scan #1129112 Reviewed-on: https://review.coreboot.org/17883 Tested-by: build bot (Jenkins) Reviewed-by: Martin Roth <martinroth(a)google.com> See https://review.coreboot.org/17883 for details. -gerrit

1 0

Patch merged into coreboot/master: util/broadcom: Add two more NULL checks
by gerrit＠coreboot.org 16 Dec '16

16 Dec '16

the following patch was just integrated into master: commit 8c47b1f833cf83c635bdb04f3c6c363bdc3e6669 Author: Patrick Georgi <pgeorgi(a)chromium.org> Date: Thu Dec 15 15:03:55 2016 +0100 util/broadcom: Add two more NULL checks Change-Id: I088730fd87dd39fa2c36a06c5770fad05a5808b0 Signed-off-by: Patrick Georgi <pgeorgi(a)chromium.org> Found-by: Coverity Scan #1323511, #1323512 Reviewed-on: https://review.coreboot.org/17882 Tested-by: build bot (Jenkins) Reviewed-by: Martin Roth <martinroth(a)google.com> See https://review.coreboot.org/17882 for details. -gerrit

1 0

Patch merged into coreboot/master: util/broadcom: Check return value of stat()
by gerrit＠coreboot.org 16 Dec '16

16 Dec '16

the following patch was just integrated into master: commit a3e928cdf654b5e4923b0d51ada98969886205b8 Author: Patrick Georgi <pgeorgi(a)chromium.org> Date: Thu Dec 15 15:02:09 2016 +0100 util/broadcom: Check return value of stat() Change-Id: Ib53408e8b186c07aa8e42c67131d39c4add05983 Signed-off-by: Patrick Georgi <pgeorgi(a)chromium.org> Found-by: Coverity Scan #1323515 Reviewed-on: https://review.coreboot.org/17881 Tested-by: build bot (Jenkins) Reviewed-by: Martin Roth <martinroth(a)google.com> See https://review.coreboot.org/17881 for details. -gerrit

1 0

Patch merged into coreboot/master: util/broadcom: Initialize variable
by gerrit＠coreboot.org 16 Dec '16

16 Dec '16

the following patch was just integrated into master: commit 3d51a6ac992c1a1e68b37bdfe5c1fb8dffebf261 Author: Patrick Georgi <pgeorgi(a)chromium.org> Date: Thu Dec 15 14:59:37 2016 +0100 util/broadcom: Initialize variable It's later tested for NULL, but never initialized to make that test work reliably. Change-Id: Iadee1af224507a6dd39956306f3eafa687895176 Signed-off-by: Patrick Georgi <pgeorgi(a)chromium.org> Found-by: Coverity Scan #1323515 Reviewed-on: https://review.coreboot.org/17880 Tested-by: build bot (Jenkins) Reviewed-by: Martin Roth <martinroth(a)google.com> See https://review.coreboot.org/17880 for details. -gerrit

1 0

Jump to page:

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit December 2016