Change in coreboot[master]: soc/intel/common: Implement recovery mode for CSE run time errors
Sridhar Siricilla has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/40563 )
Change subject: soc/intel/common: Implement recovery mode for CSE run time errors ......................................................................
soc/intel/common: Implement recovery mode for CSE run time errors
Implement triggering recovery mode for runtime errors which may occur during CSE firmware update flow. Also, define recovery sub-errors for various possible runtime errors.
Signed-off-by: Sridhar Siricilla sridhar.siricilla@intel.com Change-Id: Ib7744fc4fd0e41804d9b45079bf706b300220c62 --- M src/soc/intel/common/block/cse/custom_bp.c 1 file changed, 39 insertions(+), 3 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/63/40563/1
diff --git a/src/soc/intel/common/block/cse/custom_bp.c b/src/soc/intel/common/block/cse/custom_bp.c index 379eb03..8d4f436 100644 --- a/src/soc/intel/common/block/cse/custom_bp.c +++ b/src/soc/intel/common/block/cse/custom_bp.c @@ -11,6 +11,7 @@ #include <intelblocks/cse.h> #include <lib.h> #include <security/vboot/vboot_common.h> +#include <security/vboot/misc.h> #include <commonlib/region.h> #include <soc/intel/common/reset.h> #include <soc/me.h> @@ -49,6 +50,24 @@ */ #define CSE_MAX_BOOT_PARTITIONS 3
+/* CSE recovery sub-error codes */ +enum csme_failure_reason { + /* Unspecified error */ + CSME_FAILURE_UNSPECIFIED, + + /* CSE fails to boot from RW */ + CSME_FAILURE_RW_JUMP_ERROR, + + /* CSE firmware update failure */ + CSME_FAILURE_FW_UPDATE_ERROR, + + /* Fails to communicate with CSE */ + CSME_FAILURE_COMMUNICATION_ERROR, + + /* Fails to wipe CSE runtime data */ + CSME_FAILURE_DATA_WIPE_ERROR +}; + /* CSE's Boot Partition Ids */ enum boot_partition_id { /* RO(BP1) contains recovery/minimal boot FW, and it's a valid bootable partition. */ @@ -256,6 +275,15 @@ return true; }
+static void cse_request_recovery(uint8_t rec_sub_code) +{ + struct vb2_context *ctx; + ctx = vboot_get_context(); + vb2api_fail(ctx, VB2_RECOVERY_CSME_FAILURE, rec_sub_code); + vboot_save_data(ctx); + do_global_reset(); +} + static uint8_t cse_get_current_bp(struct cse_bp_info *cse_bp_info) { return cse_bp_info->current_bp; @@ -728,6 +756,7 @@ void cse_fw_sync(void *unused) { static struct get_bp_info_rsp cse_bp_info; + uint8_t rec_sub_code;
/* If CSE SKU type is not Custom, skip enabling CSE Custom SKU */ if (!cse_is_hfs3_fw_sku_custom()) { @@ -737,6 +766,7 @@
if (!cse_get_bp_info(&cse_bp_info)) { printk(BIOS_ERR, "cse_bp: Failed to switch to CSE RO\n"); + rec_sub_code = CSME_FAILURE_COMMUNICATION_ERROR; goto failed; }
@@ -754,28 +784,34 @@ return; }
- if (!cse_fix_data_mismatch_err(&cse_bp_info.bp_info)) + if (!cse_fix_data_mismatch_err(&cse_bp_info.bp_info)) { + rec_sub_code = CSME_FAILURE_DATA_WIPE_ERROR; goto failed; + }
struct region_device target_rdev; struct region_device source_rw_n_ver_rdev;
if (!cse_is_rw_info_valid(&cse_bp_info.bp_info, &target_rdev, &source_rw_n_ver_rdev)) { + rec_sub_code = CSME_FAILURE_RW_JUMP_ERROR; printk(BIOS_ERR, "cse_bp: CSE RW partition is not valid\n"); goto failed; }
uint32_t rst_req = 0; if (!cse_update_rw(&cse_bp_info.bp_info, &target_rdev, &source_rw_n_ver_rdev, - &rst_req)) + &rst_req)) { + rec_sub_code = CSME_FAILURE_FW_UPDATE_ERROR; goto failed; + }
if (!cse_boot_to_rw(&cse_bp_info.bp_info, rst_req)) { printk(BIOS_ERR, "cse_bp: Failed to switch to RW\n"); + rec_sub_code = CSME_FAILURE_UNSPECIFIED; goto failed; } return; failed: - do_global_reset(); + cse_request_recovery(rec_sub_code); }
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40563 )
Change subject: soc/intel/common: Implement recovery mode for CSE run time errors ......................................................................
Patch Set 2:
(3 comments)
https://review.coreboot.org/c/coreboot/+/40563/2/src/soc/intel/common/block/... File src/soc/intel/common/block/cse/custom_bp.c:
https://review.coreboot.org/c/coreboot/+/40563/2/src/soc/intel/common/block/... PS2, Line 280: struct vb2_context *ctx; Shouldn't there be some sort of CONFIG(VBOOT) guard somewhere around here?
https://review.coreboot.org/c/coreboot/+/40563/2/src/soc/intel/common/block/... PS2, Line 283: vboot_save_data(ctx); nit: vboot_save_nvdata_only() should be enough (and might save code size by avoiding to link all the TPM stuff into ramstage)
https://review.coreboot.org/c/coreboot/+/40563/2/src/soc/intel/common/block/... PS2, Line 284: do_global_reset(); nit: should probably be vboot_reboot()?
Hello build bot (Jenkins), Patrick Georgi, Furquan Shaikh, Sridhar Siricilla, Rizwan Qureshi, Angel Pons, Subrata Banik, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/40563
to look at the new patch set (#3).
Change subject: soc/intel/common: Implement recovery mode for CSE run time errors ......................................................................
soc/intel/common: Implement recovery mode for CSE run time errors
Implement triggering recovery mode for runtime errors which may occur during CSE firmware update flow. Also, define recovery sub-errors for various possible runtime errors.
Signed-off-by: Sridhar Siricilla sridhar.siricilla@intel.com Change-Id: Ib7744fc4fd0e41804d9b45079bf706b300220c62 --- M src/soc/intel/common/block/cse/custom_bp.c 1 file changed, 40 insertions(+), 3 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/63/40563/3
Sridhar Siricilla has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40563 )
Change subject: soc/intel/common: Implement recovery mode for CSE run time errors ......................................................................
Patch Set 3:
(3 comments)
https://review.coreboot.org/c/coreboot/+/40563/2/src/soc/intel/common/block/... File src/soc/intel/common/block/cse/custom_bp.c:
https://review.coreboot.org/c/coreboot/+/40563/2/src/soc/intel/common/block/... PS2, Line 280: struct vb2_context *ctx;
Shouldn't there be some sort of CONFIG(VBOOT) guard somewhere around here?
Done
https://review.coreboot.org/c/coreboot/+/40563/2/src/soc/intel/common/block/... PS2, Line 283: vboot_save_data(ctx);
nit: vboot_save_nvdata_only() should be enough (and might save code size by avoiding to link all the […]
Done
https://review.coreboot.org/c/coreboot/+/40563/2/src/soc/intel/common/block/... PS2, Line 284: do_global_reset();
nit: should probably be vboot_reboot()?
Done
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40563 )
Change subject: soc/intel/common: Implement recovery mode for CSE run time errors ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/c/coreboot/+/40563/3/src/soc/intel/common/block/... File src/soc/intel/common/block/cse/custom_bp.c:
https://review.coreboot.org/c/coreboot/+/40563/3/src/soc/intel/common/block/... PS3, Line 279: #if CONFIG(VBOOT_LIB) CONFIG(VBOOT_LIB) means that the vboot library is compiled in (e.g. for using the hash routines from there), that does not yet mean the vboot verification flow is actually enabled. You really want CONFIG(VBOOT) here. Also, please use a C if () instead of a preprocessor #if.
Hello build bot (Jenkins), Patrick Georgi, Furquan Shaikh, Sridhar Siricilla, Rizwan Qureshi, Angel Pons, Subrata Banik, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/40563
to look at the new patch set (#4).
Change subject: soc/intel/common: Implement recovery mode for CSE run time errors ......................................................................
soc/intel/common: Implement recovery mode for CSE run time errors
Implement triggering recovery mode for runtime errors which may occur during CSE firmware update flow. Also, define recovery sub-errors for various possible runtime errors.
Signed-off-by: Sridhar Siricilla sridhar.siricilla@intel.com Change-Id: Ib7744fc4fd0e41804d9b45079bf706b300220c62 --- M src/soc/intel/common/block/cse/custom_bp.c 1 file changed, 41 insertions(+), 3 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/63/40563/4
Hello build bot (Jenkins), Patrick Georgi, Furquan Shaikh, Sridhar Siricilla, Rizwan Qureshi, Angel Pons, Subrata Banik, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/40563
to look at the new patch set (#5).
Change subject: soc/intel/common: Implement recovery mode for CSE run time errors ......................................................................
soc/intel/common: Implement recovery mode for CSE run time errors
Implement triggering recovery mode for runtime errors which may occur during CSE firmware update flow. Also, define recovery sub-errors for various possible runtime errors.
Signed-off-by: Sridhar Siricilla sridhar.siricilla@intel.com Change-Id: Ib7744fc4fd0e41804d9b45079bf706b300220c62 --- M src/soc/intel/common/block/cse/custom_bp.c 1 file changed, 42 insertions(+), 3 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/63/40563/5
Sridhar Siricilla has removed Furquan Shaikh from this change. ( https://review.coreboot.org/c/coreboot/+/40563 )
Change subject: soc/intel/common: Implement recovery mode for CSE run time errors ......................................................................
Removed reviewer Furquan Shaikh.
Sridhar Siricilla has removed Julius Werner from this change. ( https://review.coreboot.org/c/coreboot/+/40563 )
Change subject: soc/intel/common: Implement recovery mode for CSE run time errors ......................................................................
Removed reviewer Julius Werner.
Sridhar Siricilla has removed Rizwan Qureshi from this change. ( https://review.coreboot.org/c/coreboot/+/40563 )
Change subject: soc/intel/common: Implement recovery mode for CSE run time errors ......................................................................
Removed reviewer Rizwan Qureshi.
Sridhar Siricilla has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/40563 )
Change subject: soc/intel/common: Implement recovery mode for CSE run time errors ......................................................................
Abandoned
Not required.
Sridhar Siricilla has restored this change. ( https://review.coreboot.org/c/coreboot/+/40563 )
Change subject: soc/intel/common: Implement recovery mode for CSE run time errors ......................................................................
Restored
Sridhar Siricilla has removed Sridhar Siricilla from this change. ( https://review.coreboot.org/c/coreboot/+/40563 )
Change subject: soc/intel/common: Implement recovery mode for CSE run time errors ......................................................................
Removed reviewer Sridhar Siricilla.
Sridhar Siricilla has removed Subrata Banik from this change. ( https://review.coreboot.org/c/coreboot/+/40563 )
Change subject: soc/intel/common: Implement recovery mode for CSE run time errors ......................................................................
Removed reviewer Subrata Banik.
Sridhar Siricilla has removed Angel Pons from this change. ( https://review.coreboot.org/c/coreboot/+/40563 )
Change subject: soc/intel/common: Implement recovery mode for CSE run time errors ......................................................................
Removed reviewer Angel Pons.
Hello build bot (Jenkins), Patrick Georgi, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/40563
to look at the new patch set (#6).
Change subject: soc/intel/common: Implement recovery mode for CSE run time errors ......................................................................
soc/intel/common: Implement recovery mode for CSE run time errors
Implement triggering recovery mode for CSE runtime errors. Also, define recovery sub-errors for various possible CSE runtime errors.
BUG=b:153520354 TEST=Verified on hatch
Signed-off-by: Sridhar Siricilla sridhar.siricilla@intel.com Change-Id: Ib7744fc4fd0e41804d9b45079bf706b300220c62 --- M src/soc/intel/common/block/cse/custom_bp.c 1 file changed, 40 insertions(+), 7 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/63/40563/6
Sridhar Siricilla has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40563 )
Change subject: soc/intel/common: Implement recovery mode for CSE run time errors ......................................................................
Patch Set 6:
(1 comment)
https://review.coreboot.org/c/coreboot/+/40563/3/src/soc/intel/common/block/... File src/soc/intel/common/block/cse/custom_bp.c:
https://review.coreboot.org/c/coreboot/+/40563/3/src/soc/intel/common/block/... PS3, Line 279: #if CONFIG(VBOOT_LIB)
CONFIG(VBOOT_LIB) means that the vboot library is compiled in (e.g. […]
Done
Sridhar Siricilla has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40563 )
Change subject: soc/intel/common: Implement recovery mode for CSE run time errors ......................................................................
Patch Set 7:
(1 comment)
https://review.coreboot.org/c/coreboot/+/40563/7/src/soc/intel/common/block/... File src/soc/intel/common/block/cse/custom_bp.c:
https://review.coreboot.org/c/coreboot/+/40563/7/src/soc/intel/common/block/... PS7, Line 147: VB2_RECOVERY_CSME_FAILURE Rename recovery reason code to VB2_RECOVERY_INTEL_CSME from VB2_RECOVERY_CSME_FAILURE.
Hello build bot (Jenkins), Furquan Shaikh, Patrick Georgi, Sridhar Siricilla, Tim Wawrzynczak, Rizwan Qureshi, Angel Pons, Julius Werner, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/40563
to look at the new patch set (#8).
Change subject: soc/intel/common: Trigger recovery mode for CSE Lite SKU run time errors ......................................................................
soc/intel/common: Trigger recovery mode for CSE Lite SKU run time errors
Implement triggering recovery mode for CSE Lite SKU runtime errors. Also, define recovery subcodes for various possible Lite SKU runtime errors.
BUG=b:153520354 TEST=Verified on hatch
Signed-off-by: Sridhar Siricilla sridhar.siricilla@intel.com Change-Id: Ib7744fc4fd0e41804d9b45079bf706b300220c62 --- M src/soc/intel/common/block/cse/custom_bp.c 1 file changed, 40 insertions(+), 7 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/63/40563/8
Sridhar Siricilla has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40563 )
Change subject: soc/intel/common: Trigger recovery mode for CSE Lite SKU run time errors ......................................................................
Patch Set 8:
(1 comment)
https://review.coreboot.org/c/coreboot/+/40563/7/src/soc/intel/common/block/... File src/soc/intel/common/block/cse/custom_bp.c:
https://review.coreboot.org/c/coreboot/+/40563/7/src/soc/intel/common/block/... PS7, Line 147: VB2_RECOVERY_CSME_FAILURE
Rename recovery reason code to VB2_RECOVERY_INTEL_CSME from VB2_RECOVERY_CSME_FAILURE.
Done
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40563 )
Change subject: soc/intel/common: Trigger recovery mode for CSE Lite SKU run time errors ......................................................................
Patch Set 8:
I guess I'm just confused whether we're calling this the 'CSE Custom SKU' or 'CSE Lite SKU
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40563 )
Change subject: soc/intel/common: Trigger recovery mode for CSE Lite SKU run time errors ......................................................................
Patch Set 8:
Patch Set 8:
I guess I'm just confused whether we're calling this the 'CSE Custom SKU' or 'CSE Lite SKU
I think we should make it consistent and call it "CSE Lite SKU".
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40563 )
Change subject: soc/intel/common: Trigger recovery mode for CSE Lite SKU run time errors ......................................................................
Patch Set 8:
(1 comment)
https://review.coreboot.org/c/coreboot/+/40563/8/src/soc/intel/common/block/... File src/soc/intel/common/block/cse/custom_bp.c:
https://review.coreboot.org/c/coreboot/+/40563/8/src/soc/intel/common/block/... PS8, Line 151: do_global_reset(); Please add brackets around the else branch too.
https://doc.coreboot.org/coding_style.html
Hello build bot (Jenkins), Furquan Shaikh, Patrick Georgi, Sridhar Siricilla, Tim Wawrzynczak, Rizwan Qureshi, Angel Pons, Julius Werner, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/40563
to look at the new patch set (#9).
Change subject: soc/intel/common: Trigger recovery mode for CSE Lite SKU run time errors ......................................................................
soc/intel/common: Trigger recovery mode for CSE Lite SKU run time errors
Implement triggering recovery mode for CSE Lite SKU runtime errors. Also, define recovery subcodes for various possible Lite SKU runtime errors.
BUG=b:153520354 TEST=Verified on hatch
Signed-off-by: Sridhar Siricilla sridhar.siricilla@intel.com Change-Id: Ib7744fc4fd0e41804d9b45079bf706b300220c62 --- M src/soc/intel/common/block/cse/custom_bp.c 1 file changed, 41 insertions(+), 7 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/63/40563/9
Sridhar Siricilla has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40563 )
Change subject: soc/intel/common: Trigger recovery mode for CSE Lite SKU run time errors ......................................................................
Patch Set 9:
(1 comment)
Patch Set 8:
Patch Set 8:
I guess I'm just confused whether we're calling this the 'CSE Custom SKU' or 'CSE Lite SKU
I think we should make it consistent and call it "CSE Lite SKU".
Done.
https://review.coreboot.org/c/coreboot/+/40563/8/src/soc/intel/common/block/... File src/soc/intel/common/block/cse/custom_bp.c:
https://review.coreboot.org/c/coreboot/+/40563/8/src/soc/intel/common/block/... PS8, Line 151: do_global_reset();
Please add brackets around the else branch too. […]
Done
Hello build bot (Jenkins), Furquan Shaikh, Patrick Georgi, Sridhar Siricilla, Tim Wawrzynczak, Rizwan Qureshi, Angel Pons, Julius Werner, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/40563
to look at the new patch set (#10).
Change subject: soc/intel/common: Trigger recovery mode for CSE Lite SKU run time errors ......................................................................
soc/intel/common: Trigger recovery mode for CSE Lite SKU run time errors
Implement triggering recovery mode for CSE Lite SKU runtime errors. Also, define recovery subcodes for various possible Lite SKU runtime errors.
BUG=b:153520354 TEST=Verified on hatch
Signed-off-by: Sridhar Siricilla sridhar.siricilla@intel.com Change-Id: Ib7744fc4fd0e41804d9b45079bf706b300220c62 --- M src/soc/intel/common/block/cse/cse_lite.c 1 file changed, 41 insertions(+), 7 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/63/40563/10
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40563 )
Change subject: soc/intel/common: Trigger recovery mode for CSE Lite SKU run time errors ......................................................................
Patch Set 11:
(5 comments)
https://review.coreboot.org/c/coreboot/+/40563/11/src/soc/intel/common/block... File src/soc/intel/common/block/cse/cse_lite.c:
https://review.coreboot.org/c/coreboot/+/40563/11/src/soc/intel/common/block... PS11, Line 5: <security/vboot/misc.h> nit: Put this below along with security/vboot/vboot_common.h
https://review.coreboot.org/c/coreboot/+/40563/11/src/soc/intel/common/block... PS11, Line 9: #include <vb2_api.h>
since you are using vb2api_fail()
https://review.coreboot.org/c/coreboot/+/40563/11/src/soc/intel/common/block... PS11, Line 43: * Can you also add: (Failure to set next boot partition as RW).
It is a little confusing how CSE_LITE_SKU_RW_JUMP_ERROR and CSE_LITE_SKU_RW_SWITCH_ERROR are different. Above comment might help provide some more info.
https://review.coreboot.org/c/coreboot/+/40563/11/src/soc/intel/common/block... PS11, Line 146: VB2_RECOVERY_INTEL_CSE_LITE_SKU This is going to require an uprev of vboot in coreboot: https://review.coreboot.org/c/coreboot/+/41536
https://review.coreboot.org/c/coreboot/+/40563/11/src/soc/intel/common/block... PS11, Line 150: do_global_reset(); I think we can just add a die("") here. This condition is not really expected.
Hello build bot (Jenkins), Furquan Shaikh, Patrick Georgi, Sridhar Siricilla, Tim Wawrzynczak, Rizwan Qureshi, Angel Pons, Julius Werner, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/40563
to look at the new patch set (#12).
Change subject: soc/intel/common: Trigger recovery mode for CSE Lite SKU run time errors ......................................................................
soc/intel/common: Trigger recovery mode for CSE Lite SKU run time errors
Implement triggering recovery mode for CSE Lite SKU runtime errors. Also, define recovery subcodes for various possible Lite SKU runtime errors.
BUG=b:153520354 TEST=Verified on hatch
Signed-off-by: Sridhar Siricilla sridhar.siricilla@intel.com Change-Id: Ib7744fc4fd0e41804d9b45079bf706b300220c62 --- M src/soc/intel/common/block/cse/cse_lite.c 1 file changed, 42 insertions(+), 7 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/63/40563/12
Sridhar Siricilla has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40563 )
Change subject: soc/intel/common: Trigger recovery mode for CSE Lite SKU run time errors ......................................................................
Patch Set 12:
(5 comments)
https://review.coreboot.org/c/coreboot/+/40563/11/src/soc/intel/common/block... File src/soc/intel/common/block/cse/cse_lite.c:
https://review.coreboot.org/c/coreboot/+/40563/11/src/soc/intel/common/block... PS11, Line 5: <security/vboot/misc.h>
nit: Put this below along with security/vboot/vboot_common. […]
Done
https://review.coreboot.org/c/coreboot/+/40563/11/src/soc/intel/common/block... PS11, Line 9:
#include <vb2_api.h> […]
Done
https://review.coreboot.org/c/coreboot/+/40563/11/src/soc/intel/common/block... PS11, Line 43: *
Can you also add: […]
Ack
https://review.coreboot.org/c/coreboot/+/40563/11/src/soc/intel/common/block... PS11, Line 146: VB2_RECOVERY_INTEL_CSE_LITE_SKU
This is going to require an uprev of vboot in coreboot: https://review.coreboot. […]
Ack
https://review.coreboot.org/c/coreboot/+/40563/11/src/soc/intel/common/block... PS11, Line 150: do_global_reset();
I think we can just add a die("") here. This condition is not really expected.
Ack
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40563 )
Change subject: soc/intel/common: Trigger recovery mode for CSE Lite SKU run time errors ......................................................................
Patch Set 12: Code-Review+1
V Sowmya has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40563 )
Change subject: soc/intel/common: Trigger recovery mode for CSE Lite SKU run time errors ......................................................................
Patch Set 12: Code-Review+2
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40563 )
Change subject: soc/intel/common: Trigger recovery mode for CSE Lite SKU run time errors ......................................................................
Patch Set 12: Code-Review+2
(1 comment)
https://review.coreboot.org/c/coreboot/+/40563/12/src/soc/intel/common/block... File src/soc/intel/common/block/cse/cse_lite.c:
https://review.coreboot.org/c/coreboot/+/40563/12/src/soc/intel/common/block... PS12, Line 151: bp lite?
Sridhar Siricilla has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40563 )
Change subject: soc/intel/common: Trigger recovery mode for CSE Lite SKU run time errors ......................................................................
Patch Set 12:
(1 comment)
https://review.coreboot.org/c/coreboot/+/40563/12/src/soc/intel/common/block... File src/soc/intel/common/block/cse/cse_lite.c:
https://review.coreboot.org/c/coreboot/+/40563/12/src/soc/intel/common/block... PS12, Line 151: bp
lite?
Yes, I will change it later.
Hello V Sowmya, build bot (Jenkins), Furquan Shaikh, Patrick Georgi, Sridhar Siricilla, Tim Wawrzynczak, Rizwan Qureshi, Angel Pons, Julius Werner, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/40563
to look at the new patch set (#13).
Change subject: soc/intel/common: Trigger recovery mode for CSE Lite SKU run time errors ......................................................................
soc/intel/common: Trigger recovery mode for CSE Lite SKU run time errors
Implement triggering recovery mode for CSE Lite SKU runtime errors. Also, define recovery subcodes for various possible Lite SKU runtime errors.
BUG=b:153520354 TEST=Verified on hatch
Signed-off-by: Sridhar Siricilla sridhar.siricilla@intel.com Change-Id: Ib7744fc4fd0e41804d9b45079bf706b300220c62 --- M src/soc/intel/common/block/cse/cse_lite.c 1 file changed, 42 insertions(+), 7 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/63/40563/13
Sridhar Siricilla has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40563 )
Change subject: soc/intel/common: Trigger recovery mode for CSE Lite SKU run time errors ......................................................................
Patch Set 13:
(1 comment)
https://review.coreboot.org/c/coreboot/+/40563/12/src/soc/intel/common/block... File src/soc/intel/common/block/cse/cse_lite.c:
https://review.coreboot.org/c/coreboot/+/40563/12/src/soc/intel/common/block... PS12, Line 151: bp
Yes, I will change it later.
Done
V Sowmya has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40563 )
Change subject: soc/intel/common: Trigger recovery mode for CSE Lite SKU run time errors ......................................................................
Patch Set 13: Code-Review+2
Rizwan Qureshi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40563 )
Change subject: soc/intel/common: Trigger recovery mode for CSE Lite SKU run time errors ......................................................................
Patch Set 13: Code-Review+2
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40563 )
Change subject: soc/intel/common: Trigger recovery mode for CSE Lite SKU run time errors ......................................................................
Patch Set 13: Code-Review+2
Furquan Shaikh has submitted this change. ( https://review.coreboot.org/c/coreboot/+/40563 )
Change subject: soc/intel/common: Trigger recovery mode for CSE Lite SKU run time errors ......................................................................
soc/intel/common: Trigger recovery mode for CSE Lite SKU run time errors
Implement triggering recovery mode for CSE Lite SKU runtime errors. Also, define recovery subcodes for various possible Lite SKU runtime errors.
BUG=b:153520354 TEST=Verified on hatch
Signed-off-by: Sridhar Siricilla sridhar.siricilla@intel.com Change-Id: Ib7744fc4fd0e41804d9b45079bf706b300220c62 Reviewed-on: https://review.coreboot.org/c/coreboot/+/40563 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: V Sowmya v.sowmya@intel.com Reviewed-by: Rizwan Qureshi rizwan.qureshi@intel.com Reviewed-by: Furquan Shaikh furquan@google.com --- M src/soc/intel/common/block/cse/cse_lite.c 1 file changed, 42 insertions(+), 7 deletions(-)
Approvals: build bot (Jenkins): Verified Furquan Shaikh: Looks good to me, approved Rizwan Qureshi: Looks good to me, approved V Sowmya: Looks good to me, approved
diff --git a/src/soc/intel/common/block/cse/cse_lite.c b/src/soc/intel/common/block/cse/cse_lite.c index b985a94..1f9e2ce 100644 --- a/src/soc/intel/common/block/cse/cse_lite.c +++ b/src/soc/intel/common/block/cse/cse_lite.c @@ -4,6 +4,8 @@ #include <soc/intel/common/reset.h> #include <intelblocks/cse.h> #include <security/vboot/vboot_common.h> +#include <security/vboot/misc.h> +#include <vb2_api.h>
/* Converts bp index to boot partition string */ #define GET_BP_STR(bp_index) (bp_index ? "RW" : "RO") @@ -27,6 +29,30 @@ RW = 1 };
+/* CSE recovery sub-error codes */ +enum csme_failure_reason { + /* Unspecified error */ + CSE_LITE_SKU_UNSPECIFIED = 1, + + /* CSE fails to boot from RW */ + CSE_LITE_SKU_RW_JUMP_ERROR = 2, + + /* CSE RW boot partition access error */ + CSE_LITE_SKU_RW_ACCESS_ERROR = 3, + + /* Fails to set next boot partition as RW */ + CSE_LITE_SKU_RW_SWITCH_ERROR = 4, + + /* CSE firmware update failure */ + CSE_LITE_SKU_FW_UPDATE_ERROR = 5, + + /* Fails to communicate with CSE */ + CSE_LITE_SKU_COMMUNICATION_ERROR = 6, + + /* Fails to wipe CSE runtime data */ + CSE_LITE_SKU_DATA_WIPE_ERROR = 7 +}; + /* * Boot partition status. * The status is returned in response to MKHI_BUP_COMMON_GET_BOOT_PARTITION_INFO cmd. @@ -112,6 +138,19 @@ struct cse_bp_info bp_info; } __packed;
+static void cse_trigger_recovery(uint8_t rec_sub_code) +{ + if (CONFIG(VBOOT)) { + struct vb2_context *ctx; + ctx = vboot_get_context(); + vb2api_fail(ctx, VB2_RECOVERY_INTEL_CSE_LITE_SKU, rec_sub_code); + vboot_save_data(ctx); + vboot_reboot(); + } + + die("cse_lite: Failed to trigger recovery mode(recovery subcode:%d)\n", rec_sub_code); +} + static uint8_t cse_get_current_bp(const struct cse_bp_info *cse_bp_info) { return cse_bp_info->current_bp; @@ -306,22 +345,18 @@
if (!cse_get_bp_info(&cse_bp_info)) { printk(BIOS_ERR, "cse_bp: Failed to get CSE boot partition info\n"); - goto failed; + cse_trigger_recovery(CSE_LITE_SKU_COMMUNICATION_ERROR); }
- if (!cse_is_rw_info_valid(&cse_bp_info.bp_info)) { printk(BIOS_ERR, "cse_bp: CSE RW partition is not valid\n"); - goto failed; + cse_trigger_recovery(CSE_LITE_SKU_RW_JUMP_ERROR); }
if (!cse_boot_to_rw(&cse_bp_info.bp_info)) { printk(BIOS_ERR, "cse_bp: Failed to switch to RW\n"); - goto failed; + cse_trigger_recovery(CSE_LITE_SKU_RW_SWITCH_ERROR); } - return; -failed: - do_global_reset(); }
BOOT_STATE_INIT_ENTRY(BS_PRE_DEVICE, BS_ON_ENTRY, cse_fw_sync, NULL);
9elements QA has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40563 )
Change subject: soc/intel/common: Trigger recovery mode for CSE Lite SKU run time errors ......................................................................
Patch Set 14:
Automatic boot test returned (PASS/FAIL/TOTAL): 4/0/4 Emulation targets: "QEMU x86 q35/ich9" using payload TianoCore : SUCCESS : https://lava.9esec.io/r/4492 "QEMU x86 q35/ich9" using payload SeaBIOS : SUCCESS : https://lava.9esec.io/r/4491 "QEMU x86 i440fx/piix4" using payload SeaBIOS : SUCCESS : https://lava.9esec.io/r/4490 "QEMU AArch64" using payload LinuxBoot_u-root_kexec : SUCCESS : https://lava.9esec.io/r/4489
Please note: This test is under development and might not be accurate at all!
-
9elements QA (Code Review) -
Furquan Shaikh (Code Review)
-
Julius Werner (Code Review)
-
Paul Menzel (Code Review)
-
Rizwan Qureshi (Code Review)
-
Sridhar Siricilla (Code Review)
-
Tim Wawrzynczak (Code Review)
-
V Sowmya (Code Review)