Change in coreboot[master]: soc/intel/{skylake,cannonlake}: Co-ordinate lockdown configuration
Attention is currently required from: Nico Huber, Angel Pons, Patrick Rudolph. Benjamin Doron has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/50048 )
Change subject: soc/intel/{skylake,cannonlake}: Co-ordinate lockdown configuration ......................................................................
Patch Set 5:
(1 comment)
File src/soc/intel/skylake/chip.c:
https://review.coreboot.org/c/coreboot/+/50048/comment/adce21fe_527d33ef PS5, Line 332: }
The (implicit) else case would change depending on the defaults in the FSP binary. Is this change intended?
Currently, only asrock/h110m and intel/kblrvp (rvp8) don't define chipset_lockdown. This means that they use CHIPSET_LOCKDOWN_FSP ("0" in the enum). Regardless, I do not intend to change functionality.
If yes, how to test / verify it?
If we can't, why do we drag get_lockdown_config() around when only one setting is supported? (IIRC, it was already broken when added.)
How broken is it? Later platforms define the else case.
It appears to mostly only break assumptions. Based on coreboot logs, SMM is locked before FSP-S, so the only thing this patch might break is nvramtool. And perhaps the fallback mechanism, which is a big(ger) issue.
In general, get_lockdown_config() also determines SPI lockdown UPDs. Is this what broke in the past?
-
Benjamin Doron (Code Review)