Attention is currently required from: Rizwan Qureshi.
Hello Rizwan Qureshi,
I'd like you to do a code review. Please visit
https://review.coreboot.org/c/coreboot/+/77071?usp=email
to review the following change.
Change subject: soc/intel/cse: Modify cse_check_update_status() input parameters ......................................................................
soc/intel/cse: Modify cse_check_update_status() input parameters
We are introducing a flow to support CSE switch from RO to RW and CSE firmware upgrade early in the romstage and downgrade post FSP-M in romstage in order to backup PSR data post DRAM initialization. A new function to do this on CBMEM_CREATION event will be added in subsequent patches. This function also makes a call to cse_check_update_status() to get the CSE firmware update status.
cse_check_update_status() accepts target_rdev as one of the input parameters. It uses target_rdev only to verify whether the RW BP sign is valid. With the addition of new function, cse_check_update_status() gets called from multiple functions and target_rdev needs to be initialized and passed. So to avoid this, moving the CSE RW BP sign is valid check to cse_fw_update() where target_rdev is already being initialized once.
BUG=b:273207144 TEST=Verify CSE firmware upgrade/downgrade on rex.
Change-Id: I52d12e83c38b12b257ea22cbf56c40f65734cb75 Signed-off-by: Krishna Prasad Bhat krishna.p.bhat.d@intel.com Signed-off-by: Rizwan Qureshi rizwan.qureshi@intel.com --- M src/soc/intel/common/block/cse/cse_lite.c 1 file changed, 11 insertions(+), 10 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/71/77071/1
diff --git a/src/soc/intel/common/block/cse/cse_lite.c b/src/soc/intel/common/block/cse/cse_lite.c index d3b1222..c3d1675 100644 --- a/src/soc/intel/common/block/cse/cse_lite.c +++ b/src/soc/intel/common/block/cse/cse_lite.c @@ -660,15 +660,11 @@ return CB_SUCCESS; }
-static enum cse_update_status cse_check_update_status(const struct cse_bp_info *cse_bp_info, - struct region_device *target_rdev) +static enum cse_update_status cse_check_update_status(const struct cse_bp_info *cse_bp_info) { int ret; struct fw_version cbfs_rw_version;
- if (!cse_is_rw_bp_sign_valid(target_rdev)) - return CSE_UPDATE_CORRUPTED; - if (get_cse_ver_from_cbfs(&cbfs_rw_version) == CB_ERR) return CSE_UPDATE_METADATA_ERROR;
@@ -822,11 +818,16 @@ return CSE_LITE_SKU_RW_ACCESS_ERROR; }
- status = cse_check_update_status(cse_bp_info, &target_rdev); - if (status == CSE_UPDATE_NOT_REQUIRED) - return CSE_NO_ERROR; - if (status == CSE_UPDATE_METADATA_ERROR) - return CSE_LITE_SKU_RW_METADATA_NOT_FOUND; + if (!cse_is_rw_bp_sign_valid(&target_rdev)) { + printk(BIOS_DEBUG, "cse_lite: CSE RW corrupted, trigger an update\n"); + status = CSE_UPDATE_CORRUPTED; + } else { + status = cse_check_update_status(cse_bp_info); + if (status == CSE_UPDATE_NOT_REQUIRED) + return CSE_NO_ERROR; + if (status == CSE_UPDATE_METADATA_ERROR) + return CSE_LITE_SKU_RW_METADATA_NOT_FOUND; + }
printk(BIOS_DEBUG, "cse_lite: CSE RW update is initiated\n"); return cse_trigger_fw_update(cse_bp_info, status, &target_rdev);