Attention is currently required from: Tim Wawrzynczak, Sridhar Siricilla, Werner Zeh, Patrick Rudolph, EricR Lai. Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/61520 )
Change subject: soc/intel/common/cse: Add function to perform global reset lock ......................................................................
Patch Set 9:
(1 comment)
File src/soc/intel/common/block/cse/cse.c:
https://review.coreboot.org/c/coreboot/+/61520/comment/9b68d97d_9a9ce51e PS8, Line 1022: if (cse_is_hfs1_spi_protected())
Any pointer to about who shares this recommendation and in what form ? I mean any document as such.
You may refer the CSE Lite User Guide (or cse/cse_lite.c) to understand the CSE Lite update flow. The flow gives details what is the method coreboot use to update the CSE Lite firmware. From this, we can infer locking global reset always doesn't make any issue.
Note, `if clause` is about locking the CF9Lock and `else` is about keep the CF9 unlock to allow global reset. What you have asked originally at starting of this thread as below is against what you just said here.
You have asked to skip locking the CF9Lock in case SoC user selects CSE LITE SKU.
if (cse_cse_is_hfs1_spi_protected() && !CONFIG(CONFIG_SOC_INTEL_CSE_LITE_SKU)
And, in above comment you said, the recommendation is global reset remain locked for CSE lite? then why to leave the CF9Lock unlock for CSE Lite SKU?
OMG, my code interpretation is wrong here.
nit: if (cse_cse_is_hfs1_spi_protected() || CONFIG(CONFIG_SOC_INTEL_CSE_LITE_SKU))
@Sridhar, your original comment (!CONFIG(CONFIG_SOC_INTEL_CSE_LITE_SKU)) and intension (lock global reset for CSE lite forcefully without bothering about HFWSTS[4]) is exactly opposite. You just missed `!` and created whole discussion roaming around that.
If you would have been thoughtful while commenting then we could avoid such extra comment exchange cycle.
`if (cse_cse_is_hfs1_spi_protected() || CONFIG(CONFIG_SOC_INTEL_CSE_LITE_SKU))` is still okay to me with note that BIOS should lock down CF9GR bit even during manufacturing and re-manufacturing environment if HFSTS1 [4] is set with CSE Lite enabled.