Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45826 )
Change subject: soc/intel/icl: enable common CPU code ......................................................................
Patch Set 5:
(1 comment)
https://review.coreboot.org/c/coreboot/+/45826/5//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/45826/5//COMMIT_MSG@13 PS5, Line 13: like done for SKL,ICL,... already.
This isn't quite clear, IMHO. "like done for [...]ICL"? but this is ICL. I see that the config value is already used in code. That could be clearer.
Oops, I meant CNL here ofc.
Also, there is another Kconfig prompt (locking) enabled by this which is not used, AFAICS. If that is the case, its prompt should be disabled. Either via another Kconfig (HAVE...LOCKING_OPTION?) or at least select it (prompt would show but can't be changed, which I assume is what the code does).
Yes, the feature lock. I haven't mentioned it here yet, because it's not clear to me yet how we want to handle ICL due to SkipMpInit missing. Have a look at this, please: https://review.coreboot.org/c/coreboot/+/45826/2/src/soc/intel/icelake/romst...
In case we want to just rely on FSP, I'd just select SET_IA32_FC_LOCK_BIT by ENABLE_VMX on ICL, to make it clear in menuconfig.
ICL Kconfig:
config ENABLE_VMX select SET_IA32_FC_LOCK_BIT