[coreboot-gerrit] [S] Change in coreboot[master]: soc/intel/cse: Modify cse_check_update_status() input parameters

[Krishna P Bhat D (Code Review)]

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 
Signed-off-by: Rizwan Qureshi 
---
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);