Change in coreboot[master]: soc/intel/common: Extend criteria to determine CSE fimrware update
Sridhar Siricilla has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/46553 )
Change subject: soc/intel/common: Extend criteria to determine CSE fimrware update ......................................................................
soc/intel/common: Extend criteria to determine CSE fimrware update
The patch extends criteria to determine the CSE Firmware Update. As part of exended criteria, coreboot checks for IFWI Build Version field besides CSE RW partition signature corruption and CSE RW version. Having this additional check to help coreboot to trigger CSE Fimrware update when there is no change in CSE firmware but in other components like TCSS etc.
TEST=Verified on hatch.
Signed-off-by: Sridhar Siricilla sridhar.siricilla@intel.com Change-Id: I129a6461e2f53259560c0e5d7d3b2fb85c8b3238 --- M src/soc/intel/common/block/cse/cse_lite.c 1 file changed, 38 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/53/46553/1
diff --git a/src/soc/intel/common/block/cse/cse_lite.c b/src/soc/intel/common/block/cse/cse_lite.c index a7bb757..77a8d5f 100644 --- a/src/soc/intel/common/block/cse/cse_lite.c +++ b/src/soc/intel/common/block/cse/cse_lite.c @@ -173,7 +173,7 @@
/* CSE CBFS RW version */ struct fw_version fw_ver; - uint8_t reserved1[4]; + uint32_t ifwi_build_ver; /* * CSE RW blob needs to be moved outside of FW_MAIN_A and FW_MAIN_B to reduce the boot * time. When the RW blob is placed outside of FW_MAIN_A and FW_MAIN_B, coreboot needs @@ -186,6 +186,22 @@ uint8_t reserved2[4]; };
+/* CSE Boot Partition Descriptor Table Header */ +struct cse_bpdt { + + /* Signature:0x000055AA is expected for a valid BP */ + uint32_t signature; + + /* reserved field */ + uint8_t reserved[8]; + + /* ifwi build version */ + uint32_t ifwi_build_ver; + + /* FIT tool version */ + struct fw_version fit_tool_ver; +}; + static void cse_log_status_registers(void) { printk(BIOS_DEBUG, "cse_lite: CSE status registers: HFSTS1: 0x%x, HFSTS2: 0x%x " @@ -592,6 +608,9 @@ source_info->fw_ver.hotfix, source_info->fw_ver.build);
+ printk(BIOS_SPEW, "cse_lite: IFWI build version: %u\n", + source_info->ifwi_build_ver); + printk(BIOS_SPEW, "cse_lite: CSE CBFS RW's hash: ");
for (uint8_t i = 0; i < VB2_SHA256_DIGEST_SIZE; i++) @@ -630,6 +649,22 @@ return source_info->fw_ver.build - cse_rw_ver->build; }
+static bool cse_is_ifwi_build_version_latest(const struct cse_cbfs_rw_info *source_info, + const struct region_device *target_dev) +{ + uint32_t ifwi_build_ver; + uint8_t ifwi_build_ver_offset = offsetof(struct cse_bpdt, ifwi_build_ver); + + if (rdev_readat(target_dev, &ifwi_build_ver, ifwi_build_ver_offset, + sizeof(ifwi_build_ver)) != sizeof(ifwi_build_ver)) { + printk(BIOS_ERR, "cse_lite: Failed to read IFWI build version from" + "CSE RW partition\n"); + return false; + } + + return (ifwi_build_ver == source_info->ifwi_build_ver); +} + /* The function calculates SHA-256 of CSE RW blob and compares it with the provided SHA value */ static bool cse_verify_cbfs_rw_sha256(const uint8_t *expected_rw_blob_sha, const void *rw_blob, const size_t rw_blob_sz) @@ -719,7 +754,8 @@ const struct cse_cbfs_rw_info *source_info, struct region_device *target_rdev) { return (!cse_is_rw_bp_sign_valid(target_rdev) || - !cse_is_rw_version_latest(cse_bp_info, source_info)); + !cse_is_rw_version_latest(cse_bp_info, source_info) || + !cse_is_ifwi_build_version_latest(source_info, target_rdev)); }
static bool cse_write_rw_region(const struct region_device *target_rdev,
Hello build bot (Jenkins), Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/46553
to look at the new patch set (#2).
Change subject: soc/intel/common: Extend criteria to determine CSE firmware update ......................................................................
soc/intel/common: Extend criteria to determine CSE firmware update
The patch extends criteria to determine the CSE Firmware Update. As part of exended criteria, coreboot checks for IFWI Build Version field besides CSE RW partition signature corruption and CSE RW version. Having this additional check to help coreboot to trigger CSE Firmware update when there is no change in CSE firmware but in other components like TCSS etc.
TEST=Verified on hatch.
Signed-off-by: Sridhar Siricilla sridhar.siricilla@intel.com Change-Id: I129a6461e2f53259560c0e5d7d3b2fb85c8b3238 --- M src/soc/intel/common/block/cse/cse_lite.c 1 file changed, 38 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/53/46553/2
Sridhar Siricilla has removed Patrick Rudolph from this change. ( https://review.coreboot.org/c/coreboot/+/46553 )
Change subject: soc/intel/common: Extend criteria to determine CSE firmware update ......................................................................
Removed reviewer Patrick Rudolph.
Stefan Reinauer has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/46553?usp=email )
Change subject: soc/intel/common: Extend criteria to determine CSE firmware update ......................................................................
Abandoned
Sridhar Siricilla has restored this change. ( https://review.coreboot.org/c/coreboot/+/46553?usp=email )
Change subject: soc/intel/common: Extend criteria to determine CSE firmware update ......................................................................
Restored
Attention is currently required from: Sridhar Siricilla.
Martin L Roth has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46553?usp=email )
Change subject: soc/intel/common: Extend criteria to determine CSE firmware update ......................................................................
Patch Set 7:
(3 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/46553/comment/affe197d_af867cec : PS7, Line 10: of exended criteria, coreboot checks for IFWI Build Version field besides Please wrap the commit message to 72 characters. This line looks like it's at 74 characters.
Patchset:
PS7: I'm assuming you plan on updating this since you restored it. Tim and Furquan aren't as involved at coreboot as they used to be, so you might want to pick new reviewers for the patch.
File src/soc/intel/common/block/cse/cse_lite.c:
https://review.coreboot.org/c/coreboot/+/46553/comment/51d69e13_39ae46f8 : PS7, Line 798: rv = cse_trigger_fw_update(cse_bp_info, &source_metadata, &target_rdev, cse_cbfs_rw, region_device_sz(&source_rdev));
line over 96 characters
Please fix.
-
Martin L Roth (Code Review) -
Sridhar Siricilla (Code Review)
-
Stefan Reinauer (Code Review)