Sridhar Siricilla has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35403 )
Change subject: soc/intel/common/basecode: Implement CSE update flow ......................................................................
Patch Set 77:
(5 comments)
https://review.coreboot.org/c/coreboot/+/35403/76/src/soc/intel/common/block... File src/soc/intel/common/block/cse/Kconfig:
https://review.coreboot.org/c/coreboot/+/35403/76/src/soc/intel/common/block... PS76, Line 32: config SOC_INTEL_CSE_RW_FILE : string "Intel CSE CBFS RW path and filename" : default "" : help : Intel CSE CBFS RW blob path and file name
I don't see this being used in this change. […]
Done
https://review.coreboot.org/c/coreboot/+/35403/76/src/soc/intel/common/block... File src/soc/intel/common/block/cse/Makefile.inc:
https://review.coreboot.org/c/coreboot/+/35403/76/src/soc/intel/common/block... PS76, Line 5: ramstage-$(CONFIG_SOC_INTEL_CSE_CUSTOM_SKU) += custom_bp.c
Why was this change required?
Ack
https://review.coreboot.org/c/coreboot/+/35403/76/src/soc/intel/common/block... File src/soc/intel/common/block/cse/custom_bp.c:
https://review.coreboot.org/c/coreboot/+/35403/76/src/soc/intel/common/block... PS76, Line 408: if (!cse_get_rw_partition(cse_bp_info, target_rdev)) { : printk(BIOS_ERR, "cse_bp: Failed to get ME RW partition!\n"); : return false; : }
I don't think it is correct to just go and read the RW partition here. […]
The code assumes Host/BIOS has read access to CSME region. This has to be configured FIT.XML. This is added as requirement in the architecture guide.
If host doesn't have read access, then coreboot has to send HMRFPO_ENABLE command to CSE. coreboot also requires to send GLOBAL_RESET(GRST rst type) cmd to CSE. To avoid this flow, we must configure in FIT.XML to have Host has read access to CSME region.
https://review.coreboot.org/c/coreboot/+/35403/76/src/soc/intel/common/block... PS76, Line 644: *rst_req = 1;
Why is this required? cse_boot_to_rw() can check if current bp is RO/RW and based on that decide to […]
Done
https://review.coreboot.org/c/coreboot/+/35403/76/src/soc/intel/common/block... PS76, Line 680: !cse_is_rw_info_valid(&cse_bp_info.bp_info, &target_rdev, : &source_rw_n_ver_rdev))
The function is doing much more than what the name suggests. […]
Made change to update flow to address the issue mentioned in the b/152826452