Attention is currently required from: Jason Glenesk, Raul Rangel, Matt DeVillier, Julius Werner, Fred Reitberger, Yu-Ping Wu, Felix Held.
Jakub Czapiga has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/69208 )
Change subject: vboot: Introduce handy vboot reboot functions ......................................................................
vboot: Introduce handy vboot reboot functions
This patch groups vboot context, recovery reason and subcode saving, and reboot calls into two handy functions: - vboot_save_and_reboot() - save context and reboot - vboot_fail_and_reboot() - store recovery reason and call function above
Signed-off-by: Jakub Czapiga jacz@semihalf.com Change-Id: Ie29410e8985e7cf19bd8d4cccc393b050ca1f1c5 --- M src/security/vboot/vboot_common.c M src/security/vboot/vboot_common.h M src/security/vboot/vboot_logic.c M src/soc/amd/common/psp_verstage/psp_verstage.c M src/soc/intel/common/block/cse/cse.c 5 files changed, 64 insertions(+), 44 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/08/69208/1
diff --git a/src/security/vboot/vboot_common.c b/src/security/vboot/vboot_common.c index c557f25..c9e61d0 100644 --- a/src/security/vboot/vboot_common.c +++ b/src/security/vboot/vboot_common.c @@ -11,6 +11,11 @@
void vboot_save_data(struct vb2_context *ctx) { + if (!verification_should_run() && !(ENV_ROMSTAGE && CONFIG(VBOOT_EARLY_EC_SYNC)) + && (ctx->flags + & (VB2_CONTEXT_SECDATA_FIRMWARE_CHANGED | VB2_CONTEXT_SECDATA_KERNEL_CHANGED))) + die("TPM writeback in " ENV_STRING "?"); + if (ctx->flags & VB2_CONTEXT_SECDATA_FIRMWARE_CHANGED && (CONFIG(VBOOT_MOCK_SECDATA) || tlcl_lib_init() == VB2_SUCCESS)) { printk(BIOS_INFO, "Saving secdata firmware\n"); @@ -64,3 +69,20 @@ vboot_platform_prepare_reboot(); board_reset(); } + +void vboot_save_and_reboot(struct vb2_context *ctx, uint8_t subcode) +{ + printk(BIOS_INFO, "vboot: reboot requested (%#x)\n", subcode); + vboot_save_data(ctx); + vboot_reboot(); +} + +void vboot_fail_and_reboot(uint8_t reason, uint8_t subcode) +{ + struct vb2_context *ctx = vboot_get_context(); + + if (reason) + vb2api_fail(ctx, reason, subcode); + + vboot_save_and_reboot(ctx, subcode); +} diff --git a/src/security/vboot/vboot_common.h b/src/security/vboot/vboot_common.h index 512da0e..382bcaa 100644 --- a/src/security/vboot/vboot_common.h +++ b/src/security/vboot/vboot_common.h @@ -20,6 +20,16 @@ */ void vboot_reboot(void);
+/* + * Save vboot data and reboot device. Subcode will only be printed. To store + * failure reason and subcode vb2api_fail() should be called before this + * function or vboot_fail_and_reboot() should be used instead. + */ +void vboot_save_and_reboot(struct vb2_context *ctx, uint8_t subcode); + +/* Call vb2api_fail() with reason and subcode, save vboot data and reboot. */ +void vboot_fail_and_reboot(uint8_t reason, uint8_t subcode); + /* Allow the platform to do any clean up work when vboot requests a reboot. */ void vboot_platform_prepare_reboot(void);
diff --git a/src/security/vboot/vboot_logic.c b/src/security/vboot/vboot_logic.c index 2230b5ab..b09e953 100644 --- a/src/security/vboot/vboot_logic.c +++ b/src/security/vboot/vboot_logic.c @@ -327,30 +327,22 @@ goto verstage_main_exit; }
- printk(BIOS_INFO, "Reboot requested (%x)\n", rv); - vboot_save_data(ctx); - vboot_reboot(); + vboot_save_and_reboot(ctx, rv); }
/* Determine which firmware slot to boot (based on NVRAM) */ printk(BIOS_INFO, "Phase 2\n"); rv = vb2api_fw_phase2(ctx); - if (rv) { - printk(BIOS_INFO, "Reboot requested (%x)\n", rv); - vboot_save_data(ctx); - vboot_reboot(); - } + if (rv) + vboot_save_and_reboot(ctx, rv);
/* Try that slot (verify its keyblock and preamble) */ printk(BIOS_INFO, "Phase 3\n"); timestamp_add_now(TS_VERIFY_SLOT_START); rv = vb2api_fw_phase3(ctx); timestamp_add_now(TS_VERIFY_SLOT_END); - if (rv) { - printk(BIOS_INFO, "Reboot requested (%x)\n", rv); - vboot_save_data(ctx); - vboot_reboot(); - } + if (rv) + vboot_save_and_reboot(ctx, rv);
printk(BIOS_INFO, "Phase 4\n"); rv = vboot_locate_firmware(ctx, &fw_body); @@ -360,21 +352,16 @@
rv = hash_body(ctx, &fw_body); vboot_save_data(ctx); - if (rv) { - printk(BIOS_INFO, "Reboot requested (%x)\n", rv); - vboot_reboot(); - } + if (rv) + vboot_save_and_reboot(ctx, rv);
/* Only extend PCRs once on boot. */ if (!(ctx->flags & VB2_CONTEXT_S3_RESUME)) { timestamp_add_now(TS_TPMPCR_START); rv = extend_pcrs(ctx); if (rv) { - printk(BIOS_WARNING, - "Failed to extend TPM PCRs (%#x)\n", rv); - vb2api_fail(ctx, VB2_RECOVERY_RO_TPM_U_ERROR, rv); - vboot_save_data(ctx); - vboot_reboot(); + printk(BIOS_WARNING, "Failed to extend TPM PCRs (%#x)\n", rv); + vboot_fail_and_reboot(VB2_RECOVERY_RO_TPM_U_ERROR, rv); } timestamp_add_now(TS_TPMPCR_END); } @@ -385,9 +372,7 @@ rv = antirollback_lock_space_firmware(); if (rv) { printk(BIOS_INFO, "Failed to lock TPM (%x)\n", rv); - vb2api_fail(ctx, VB2_RECOVERY_RO_TPM_L_ERROR, 0); - vboot_save_data(ctx); - vboot_reboot(); + vboot_fail_and_reboot(VB2_RECOVERY_RO_TPM_L_ERROR, rv); } timestamp_add_now(TS_TPMLOCK_END);
@@ -395,12 +380,8 @@ if (CONFIG(VBOOT_HAS_REC_HASH_SPACE)) { rv = antirollback_lock_space_mrc_hash(MRC_REC_HASH_NV_INDEX); if (rv) { - printk(BIOS_INFO, "Failed to lock rec hash space(%x)\n", - rv); - vb2api_fail(ctx, VB2_RECOVERY_RO_TPM_REC_HASH_L_ERROR, - 0); - vboot_save_data(ctx); - vboot_reboot(); + printk(BIOS_INFO, "Failed to lock rec hash space(%x)\n", rv); + vboot_fail_and_reboot(VB2_RECOVERY_RO_TPM_REC_HASH_L_ERROR, 0); } }
diff --git a/src/soc/amd/common/psp_verstage/psp_verstage.c b/src/soc/amd/common/psp_verstage/psp_verstage.c index 0dc3314..d927079 100644 --- a/src/soc/amd/common/psp_verstage/psp_verstage.c +++ b/src/soc/amd/common/psp_verstage/psp_verstage.c @@ -42,11 +42,8 @@ return; }
- vb2api_fail(ctx, VB2_RECOVERY_RO_UNSPECIFIED, (int)subcode); - vboot_save_data(ctx); - svc_debug_print("Rebooting into recovery\n"); - vboot_reboot(); + vboot_fail_and_reboot(VB2_RECOVERY_RO_UNSPECIFIED, (int)subcode); }
static uint32_t check_cmos_recovery(void) diff --git a/src/soc/intel/common/block/cse/cse.c b/src/soc/intel/common/block/cse/cse.c index c2c94ec..47ba064 100644 --- a/src/soc/intel/common/block/cse/cse.c +++ b/src/soc/intel/common/block/cse/cse.c @@ -931,15 +931,9 @@ "HFSTS3: 0x%x\n", me_read_config32(PCI_ME_HFSTS1), me_read_config32(PCI_ME_HFSTS2), me_read_config32(PCI_ME_HFSTS3));
- if (CONFIG(VBOOT)) { - struct vb2_context *ctx = vboot_get_context(); - if (ctx == NULL) - goto failure; - vb2api_fail(ctx, VB2_RECOVERY_INTEL_CSE_LITE_SKU, reason); - vboot_save_data(ctx); - vboot_reboot(); - } -failure: + if (CONFIG(VBOOT)) + vboot_fail_and_reboot(VB2_RECOVERY_INTEL_CSE_LITE_SKU, reason); + die("cse: Failed to trigger recovery mode(recovery subcode:%d)\n", reason); }
-
Jakub Czapiga (Code Review)