Sridhar Siricilla has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37283 )
Change subject: soc/intel/common/block/cse: Modify handling of HMRFPO_ENABLE command ......................................................................
Patch Set 22:
(12 comments)
https://review.coreboot.org/c/coreboot/+/37283/5//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/37283/5//COMMIT_MSG@11 PS5, Line 11: operation mode is Temp Disable Mode
cse_check_hmrfpo_enable_prerequisites() function takes care prerequisites of original flow, and late […]
Done
https://review.coreboot.org/c/coreboot/+/37283/5//COMMIT_MSG@21 PS5, Line 21: opmode Temp Disable Mode.
Instead of MRP, it checks ME SKU type. […]
Done
https://review.coreboot.org/c/coreboot/+/37283/7//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/37283/7//COMMIT_MSG@10 PS7, Line 10: Allow execution of HMRFPO_ENABLE command only when CSE's : operation mode is Temp Disable Mode
Addendum will be available soon.
Done
https://review.coreboot.org/c/coreboot/+/37283/7//COMMIT_MSG@19 PS7, Line 19: MRP
MRP refers Minimal Recovery Partition.
Done
https://review.coreboot.org/c/coreboot/+/37283/7//COMMIT_MSG@20 PS7, Line 20: Ensure CSE boots from BP1. When CSE boots from BP1, it will have : opmode Temp Disable Mode.
Planned Addendum will cover it.
Done
https://review.coreboot.org/c/coreboot/+/37283/20//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/37283/20//COMMIT_MSG@36 PS20, Line 36: Existing CSE Image Layout without MRP feature: : = BP2 + BP3 + DATA PART
No, lines 20-23 are not applicable in this case. […]
Done
https://review.coreboot.org/c/coreboot/+/37283/7/src/soc/intel/common/block/... File src/soc/intel/common/block/cse/cse.c:
https://review.coreboot.org/c/coreboot/+/37283/7/src/soc/intel/common/block/... PS7, Line 667: if (hfs1.fields.operation_mode != ME_HFS_MODE_TEMP_DISABLE) { : printk(BIOS_ERR, "HECI: ME is not in expected mode/state(0x%x)\n", hfs1.data); : goto failed; : }
The change is made considering ME Custom SKU only. […]
Done
https://review.coreboot.org/c/coreboot/+/37283/20/src/soc/intel/common/block... File src/soc/intel/common/block/cse/cse.c:
https://review.coreboot.org/c/coreboot/+/37283/20/src/soc/intel/common/block... PS20, Line 669: !cse_is_hfs1_com_soft_temp_disable()
Yes you are right. […]
Done
https://review.coreboot.org/c/coreboot/+/37283/20/src/soc/intel/common/block... PS20, Line 687:
Slight change in HMRFPO mode implementation as defined in current BWG. […]
Done
https://review.coreboot.org/c/coreboot/+/37283/20/src/soc/intel/common/block... File src/soc/intel/common/block/include/intelblocks/cse.h:
https://review.coreboot.org/c/coreboot/+/37283/20/src/soc/intel/common/block... PS20, Line 131: When MRP feature is enabled,
Yes, Command has support different conditions based on the SKU type(MRP which is a default feature i […]
Done
https://review.coreboot.org/c/coreboot/+/37283/7/src/soc/intel/common/block/... File src/soc/intel/common/block/include/intelblocks/cse.h:
https://review.coreboot.org/c/coreboot/+/37283/7/src/soc/intel/common/block/... PS7, Line 133: MRP
MRP refers Minimal Recovery Partition. This is implemented in Custom SKU. […]
Done
https://review.coreboot.org/c/coreboot/+/37283/7/src/soc/intel/common/block/... PS7, Line 134: 1. Ensure CSE boots from BP1(RO). : * - Send set_next_boot_partition(BP1) : * - Issue CSE Only Reset : * 2. Send HMRFPO_ENABLE command to CSE. Further, no reset is required.
This info is not yet captured in any doc as of now. We are woring on the documentation. […]
Done