Keith Short has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/30929
Change subject: src/security/tpm: query recovery mode from Cr50 ......................................................................
src/security/tpm: query recovery mode from Cr50
On the Sarien/Arcada platforms, the EC is not trusted to provide the state of the ESC+REFRESH+PWR recovery combination. On these platforms the Cr50 latches the state of REFRESH+PWR for use as the recovery mode key combination.
BUG=b:122715254 BRANCH=none TEST=Verify recovery mode screen shown after pressing REFRESH+PWR Signed-off-by: Keith Short keithshort@chromium.org Change-Id: Ie3ce519956f916023c8c52f1d11fa93331f52f3c --- M src/mainboard/google/sarien/chromeos.c M src/security/tpm/tss/tcg-2.0/tss_marshaling.c M src/security/tpm/tss/tcg-2.0/tss_structures.h M src/security/tpm/tss/vendor/cr50/cr50.c M src/security/tpm/tss/vendor/cr50/cr50.h 5 files changed, 43 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/29/30929/1
diff --git a/src/mainboard/google/sarien/chromeos.c b/src/mainboard/google/sarien/chromeos.c index 33647df..f1da3bb 100644 --- a/src/mainboard/google/sarien/chromeos.c +++ b/src/mainboard/google/sarien/chromeos.c @@ -20,6 +20,8 @@ #include <soc/gpio.h> #include <variant/gpio.h> #include <vendorcode/google/chromeos/chromeos.h> +#include <security/tpm/tss.h> +
void fill_lb_gpios(struct lb_gpios *gpios) { @@ -71,7 +73,16 @@
int get_recovery_mode_switch(void) { - return cros_get_gpio_value(CROS_GPIO_REC); + uint8_t recovery_button_state; + int recovery_mode_switch = 0; + + if (cros_get_gpio_value(CROS_GPIO_REC)) + recovery_mode_switch = 1; + else if (tlcl_cr50_get_recovery_button(&recovery_button_state) + == TPM_SUCCESS) + recovery_mode_switch = recovery_button_state; + + return recovery_mode_switch; }
int get_lid_switch(void) diff --git a/src/security/tpm/tss/tcg-2.0/tss_marshaling.c b/src/security/tpm/tss/tcg-2.0/tss_marshaling.c index 49ac5e8..d8b3097 100644 --- a/src/security/tpm/tss/tcg-2.0/tss_marshaling.c +++ b/src/security/tpm/tss/tcg-2.0/tss_marshaling.c @@ -273,6 +273,9 @@ rc |= obuf_write_be16(ob, sub_command[0]); rc |= obuf_write_be16(ob, sub_command[1]); break; + case TPM2_CR50_SUB_CMD_GET_REC_BTN: + rc |= obuf_write_be16(ob, *sub_command); + break; default: /* Unsupported subcommand. */ printk(BIOS_WARNING, "Unsupported cr50 subcommand: 0x%04x\n", @@ -473,6 +476,8 @@ case TPM2_CR50_SUB_CMD_TURN_UPDATE_ON: return ibuf_read_be8(ib, &vcr->num_restored_headers); break; + case TPM2_CR50_SUB_CMD_GET_REC_BTN: + return ibuf_read_be8(ib, &vcr->recovery_button_state); default: printk(BIOS_ERR, "%s:%d - unsupported vendor command %#04x!\n", diff --git a/src/security/tpm/tss/tcg-2.0/tss_structures.h b/src/security/tpm/tss/tcg-2.0/tss_structures.h index 2bac633..6952169 100644 --- a/src/security/tpm/tss/tcg-2.0/tss_structures.h +++ b/src/security/tpm/tss/tcg-2.0/tss_structures.h @@ -297,6 +297,7 @@ uint16_t vc_subcommand; union { uint8_t num_restored_headers; + uint8_t recovery_button_state; }; };
diff --git a/src/security/tpm/tss/vendor/cr50/cr50.c b/src/security/tpm/tss/vendor/cr50/cr50.c index 90f7963..6c923f0 100644 --- a/src/security/tpm/tss/vendor/cr50/cr50.c +++ b/src/security/tpm/tss/vendor/cr50/cr50.c @@ -52,3 +52,19 @@ *num_restored_headers = response->vcr.num_restored_headers; return TPM_SUCCESS; } + +uint32_t tlcl_cr50_get_recovery_button(uint8_t *recovery_button_state) +{ + struct tpm2_response *response; + uint16_t sub_command = TPM2_CR50_SUB_CMD_GET_REC_BTN; + + printk(BIOS_INFO, "Checking cr50 for pending updates\n"); + + response = tpm_process_command(TPM2_CR50_VENDOR_COMMAND, &sub_command); + + if (!response || response->hdr.tpm_code) + return TPM_E_INTERNAL_INCONSISTENCY; + + *recovery_button_state = response->vcr.recovery_button_state; + return TPM_SUCCESS; +} diff --git a/src/security/tpm/tss/vendor/cr50/cr50.h b/src/security/tpm/tss/vendor/cr50/cr50.h index 9bf3bd5..a1ab539 100644 --- a/src/security/tpm/tss/vendor/cr50/cr50.h +++ b/src/security/tpm/tss/vendor/cr50/cr50.h @@ -25,6 +25,7 @@ #define TPM2_CR50_VENDOR_COMMAND ((TPM_CC)(TPM_CC_VENDOR_BIT_MASK | 0)) #define TPM2_CR50_SUB_CMD_NVMEM_ENABLE_COMMITS (21) #define TPM2_CR50_SUB_CMD_TURN_UPDATE_ON (24) +#define TPM2_CR50_SUB_CMD_GET_REC_BTN (29)
/** * CR50 specific tpm command to enable nvmem commits before internal timeout @@ -44,4 +45,12 @@ uint32_t tlcl_cr50_enable_update(uint16_t timeout_ms, uint8_t *num_restored_headers);
+/** + * CR50 specific tpm command to get the latched state of the recovery button. + * + * Return value indicates success or failure of accessing the TPM; in case of + * success the recovery button state is saved in recovery_button_state. + */ +uint32_t tlcl_cr50_get_recovery_button(uint8_t *recovery_button_state); + #endif /* CR50_TSS_STRUCTURES_H_ */
Keith Short has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30929 )
Change subject: src/security/tpm: query recovery mode from Cr50 ......................................................................
Patch Set 1:
This change works in conjunction with Cr50 change found at CL:1389061
Duncan Laurie has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30929 )
Change subject: src/security/tpm: query recovery mode from Cr50 ......................................................................
Patch Set 1:
(1 comment)
Probably best to pull the sarien change into a separate commit so they can be merged independently.
https://review.coreboot.org/#/c/30929/1/src/security/tpm/tss/vendor/cr50/cr5... File src/security/tpm/tss/vendor/cr50/cr50.c:
https://review.coreboot.org/#/c/30929/1/src/security/tpm/tss/vendor/cr50/cr5... PS1, Line 61: pending updates recovery request
Hello Duncan Laurie, build bot (Jenkins), Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/30929
to look at the new patch set (#2).
Change subject: src/security/tpm: query recovery mode from Cr50 ......................................................................
src/security/tpm: query recovery mode from Cr50
On the Sarien/Arcada platforms, the EC is not trusted to provide the state of the ESC+REFRESH+PWR recovery combination. On these platforms the Cr50 latches the state of REFRESH+PWR for use as the recovery mode key combination.
BUG=b:122715254 BRANCH=none TEST=Verify recovery mode screen shown after pressing REFRESH+PWR Signed-off-by: Keith Short keithshort@chromium.org Change-Id: Ie3ce519956f916023c8c52f1d11fa93331f52f3c --- M src/security/tpm/tss/tcg-2.0/tss_marshaling.c M src/security/tpm/tss/tcg-2.0/tss_structures.h M src/security/tpm/tss/vendor/cr50/cr50.c M src/security/tpm/tss/vendor/cr50/cr50.h 4 files changed, 31 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/29/30929/2
Keith Short has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30929 )
Change subject: src/security/tpm: query recovery mode from Cr50 ......................................................................
Patch Set 2:
(1 comment)
Patch Set 1:
(1 comment)
Probably best to pull the sarien change into a separate commit so they can be merged independently.
Done
https://review.coreboot.org/#/c/30929/1/src/security/tpm/tss/vendor/cr50/cr5... File src/security/tpm/tss/vendor/cr50/cr50.c:
https://review.coreboot.org/#/c/30929/1/src/security/tpm/tss/vendor/cr50/cr5... PS1, Line 61: pending updates
recovery request
Done
Duncan Laurie has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30929 )
Change subject: src/security/tpm: query recovery mode from Cr50 ......................................................................
Patch Set 2: Code-Review+2
Stefan Reinauer has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30929 )
Change subject: src/security/tpm: query recovery mode from Cr50 ......................................................................
Patch Set 2: Code-Review+2
Patrick Georgi has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/30929 )
Change subject: src/security/tpm: query recovery mode from Cr50 ......................................................................
src/security/tpm: query recovery mode from Cr50
On the Sarien/Arcada platforms, the EC is not trusted to provide the state of the ESC+REFRESH+PWR recovery combination. On these platforms the Cr50 latches the state of REFRESH+PWR for use as the recovery mode key combination.
BUG=b:122715254 BRANCH=none TEST=Verify recovery mode screen shown after pressing REFRESH+PWR Signed-off-by: Keith Short keithshort@chromium.org Change-Id: Ie3ce519956f916023c8c52f1d11fa93331f52f3c Reviewed-on: https://review.coreboot.org/c/30929 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Duncan Laurie dlaurie@chromium.org Reviewed-by: Stefan Reinauer stefan.reinauer@coreboot.org --- M src/security/tpm/tss/tcg-2.0/tss_marshaling.c M src/security/tpm/tss/tcg-2.0/tss_structures.h M src/security/tpm/tss/vendor/cr50/cr50.c M src/security/tpm/tss/vendor/cr50/cr50.h 4 files changed, 31 insertions(+), 0 deletions(-)
Approvals: build bot (Jenkins): Verified Stefan Reinauer: Looks good to me, approved Duncan Laurie: Looks good to me, approved
diff --git a/src/security/tpm/tss/tcg-2.0/tss_marshaling.c b/src/security/tpm/tss/tcg-2.0/tss_marshaling.c index 49ac5e8..d8b3097 100644 --- a/src/security/tpm/tss/tcg-2.0/tss_marshaling.c +++ b/src/security/tpm/tss/tcg-2.0/tss_marshaling.c @@ -273,6 +273,9 @@ rc |= obuf_write_be16(ob, sub_command[0]); rc |= obuf_write_be16(ob, sub_command[1]); break; + case TPM2_CR50_SUB_CMD_GET_REC_BTN: + rc |= obuf_write_be16(ob, *sub_command); + break; default: /* Unsupported subcommand. */ printk(BIOS_WARNING, "Unsupported cr50 subcommand: 0x%04x\n", @@ -473,6 +476,8 @@ case TPM2_CR50_SUB_CMD_TURN_UPDATE_ON: return ibuf_read_be8(ib, &vcr->num_restored_headers); break; + case TPM2_CR50_SUB_CMD_GET_REC_BTN: + return ibuf_read_be8(ib, &vcr->recovery_button_state); default: printk(BIOS_ERR, "%s:%d - unsupported vendor command %#04x!\n", diff --git a/src/security/tpm/tss/tcg-2.0/tss_structures.h b/src/security/tpm/tss/tcg-2.0/tss_structures.h index 2bac633..6952169 100644 --- a/src/security/tpm/tss/tcg-2.0/tss_structures.h +++ b/src/security/tpm/tss/tcg-2.0/tss_structures.h @@ -297,6 +297,7 @@ uint16_t vc_subcommand; union { uint8_t num_restored_headers; + uint8_t recovery_button_state; }; };
diff --git a/src/security/tpm/tss/vendor/cr50/cr50.c b/src/security/tpm/tss/vendor/cr50/cr50.c index 90f7963..450ad97 100644 --- a/src/security/tpm/tss/vendor/cr50/cr50.c +++ b/src/security/tpm/tss/vendor/cr50/cr50.c @@ -52,3 +52,19 @@ *num_restored_headers = response->vcr.num_restored_headers; return TPM_SUCCESS; } + +uint32_t tlcl_cr50_get_recovery_button(uint8_t *recovery_button_state) +{ + struct tpm2_response *response; + uint16_t sub_command = TPM2_CR50_SUB_CMD_GET_REC_BTN; + + printk(BIOS_INFO, "Checking cr50 for recovery request\n"); + + response = tpm_process_command(TPM2_CR50_VENDOR_COMMAND, &sub_command); + + if (!response || response->hdr.tpm_code) + return TPM_E_INTERNAL_INCONSISTENCY; + + *recovery_button_state = response->vcr.recovery_button_state; + return TPM_SUCCESS; +} diff --git a/src/security/tpm/tss/vendor/cr50/cr50.h b/src/security/tpm/tss/vendor/cr50/cr50.h index 9bf3bd5..a1ab539 100644 --- a/src/security/tpm/tss/vendor/cr50/cr50.h +++ b/src/security/tpm/tss/vendor/cr50/cr50.h @@ -25,6 +25,7 @@ #define TPM2_CR50_VENDOR_COMMAND ((TPM_CC)(TPM_CC_VENDOR_BIT_MASK | 0)) #define TPM2_CR50_SUB_CMD_NVMEM_ENABLE_COMMITS (21) #define TPM2_CR50_SUB_CMD_TURN_UPDATE_ON (24) +#define TPM2_CR50_SUB_CMD_GET_REC_BTN (29)
/** * CR50 specific tpm command to enable nvmem commits before internal timeout @@ -44,4 +45,12 @@ uint32_t tlcl_cr50_enable_update(uint16_t timeout_ms, uint8_t *num_restored_headers);
+/** + * CR50 specific tpm command to get the latched state of the recovery button. + * + * Return value indicates success or failure of accessing the TPM; in case of + * success the recovery button state is saved in recovery_button_state. + */ +uint32_t tlcl_cr50_get_recovery_button(uint8_t *recovery_button_state); + #endif /* CR50_TSS_STRUCTURES_H_ */
-
Duncan Laurie (Code Review) -
Keith Short (Code Review)
-
Patrick Georgi (Code Review)
-
Stefan Reinauer (Code Review)