Sridhar Siricilla has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35402 )
Change subject: soc/intel/common/block/cse: Add boot partition related APIs ......................................................................
Patch Set 79:
(5 comments)
https://review.coreboot.org/c/coreboot/+/35402/73/src/soc/intel/common/block... File src/soc/intel/common/block/cse/custom_bp.c:
https://review.coreboot.org/c/coreboot/+/35402/73/src/soc/intel/common/block... PS73, Line 152: if (!cse_is_bp_cmd_info_possible()) { : printk(BIOS_ERR, "cse_bp: CSE does not meet prerequisites\n"); : return false; : }
Ah I see. […]
Done
https://review.coreboot.org/c/coreboot/+/35402/73/src/soc/intel/common/block... PS73, Line 345: if (!cse_boot_to_ro(&get_bp_info.cse_bp_info)) { : printk(BIOS_ERR, "cse_bp: Failed to boot to RO\n"); : goto failed; : }
Why is it required? What is the advantage of "just setting CSE boot partition to RO". […]
Agreed.
https://review.coreboot.org/c/coreboot/+/35402/77/src/soc/intel/common/block... File src/soc/intel/common/block/cse/custom_bp.c:
https://review.coreboot.org/c/coreboot/+/35402/77/src/soc/intel/common/block... PS77, Line 129: RW partition
When CSE is in RO, then COM is Soft Temp Disable by default. This will simplify CSE boot flows. […]
Done
https://review.coreboot.org/c/coreboot/+/35402/77/src/soc/intel/common/block... PS77, Line 316: if (vboot_recovery_mode_enabled()) { : : /*
We don't really need to take any action on the CSE in recovery mode. […]
Agreed.
https://review.coreboot.org/c/coreboot/+/35402/77/src/soc/intel/common/block... PS77, Line 322: cse_set_next_boot_partition(RO);
In recovery, coreboot skips CSE FW update and switching to RW. There is no change in this flow. […]
Done
-
Sridhar Siricilla (Code Review)