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 81:
(10 comments)
https://review.coreboot.org/c/coreboot/+/35402/80//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/35402/80//COMMIT_MSG@32 PS80, Line 32: coreboot ensure CSE to boot from RO(BP1) if system is in Recovery mode.
This is no longer true.
Ack
https://review.coreboot.org/c/coreboot/+/35402/80/src/soc/intel/common/block... File src/soc/intel/common/block/cse/Kconfig:
https://review.coreboot.org/c/coreboot/+/35402/80/src/soc/intel/common/block... PS80, Line 21: SKU Custom
Still pending.
Done
https://review.coreboot.org/c/coreboot/+/35402/80/src/soc/intel/common/block... File src/soc/intel/common/block/cse/custom_bp.c:
https://review.coreboot.org/c/coreboot/+/35402/80/src/soc/intel/common/block... PS80, Line 23:
nit: No space before /*
Ack
https://review.coreboot.org/c/coreboot/+/35402/80/src/soc/intel/common/block... PS80, Line 29: It's a valid bootable partition.
nit: Does this fit on the previous line?
Done
https://review.coreboot.org/c/coreboot/+/35402/80/src/soc/intel/common/block... PS80, Line 44: present based on layout, but it is not valid.
nit: I think part of this comment can go to previous line.
Done
https://review.coreboot.org/c/coreboot/+/35402/80/src/soc/intel/common/block... PS80, Line 50: present per initial image layout.
nit: This can go on previous line.
Done
https://review.coreboot.org/c/coreboot/+/35402/80/src/soc/intel/common/block... PS80, Line 58: MKHI_BUP_COMMON_GET_BOOT_PARTITION_INFO_CMD_REQ
nit: MKHI_BUP_COMMON_GET_BOOT_PARTITION_INFO
Ack
https://review.coreboot.org/c/coreboot/+/35402/80/src/soc/intel/common/block... PS80, Line 331: do_global_reset
Just making a note: This is not the right way to handle the failure case. […]
Ack
https://review.coreboot.org/c/coreboot/+/35402/80/src/soc/intel/common/block... File src/soc/intel/common/block/include/intelblocks/cse.h:
https://review.coreboot.org/c/coreboot/+/35402/80/src/soc/intel/common/block... PS80, Line 231: triggers recovery mode if CSE fails to jump to RW
Just a note: This is not yet implemented in this CL. It will be done in follow-up CLs.
Ack
https://review.coreboot.org/c/coreboot/+/35402/80/src/soc/intel/common/block... PS80, Line 232: RW partition
nit: whatever is the currently selected partition.
Done