Nathaniel L Desimone has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36356 )
Change subject: soc/intel/cannonlake: set LT_LOCK_MEMORY at end of POST ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/36356/2/src/soc/intel/cannonlake/fi... File src/soc/intel/cannonlake/finalize.c:
https://review.coreboot.org/c/coreboot/+/36356/2/src/soc/intel/cannonlake/fi... PS2, Line 116: mp_run_on_all_cpus
Current testing results: […]
For #1) Your right that LT_LOCK_MEMORY is package scoped. For #2) PAVPC is locked in the middle of FSP-S (not @ end of PEI), and TSEGMB is locked in FSP-M.
With SGX enabled, the flow needs to be this:
1. Pre-ucode SGX init 2. LT_LOCK_MEMORY 3. Reload ucode 4. Post-ucode SGX init
I don't see any good way of doing SGX enabling with SkipMpInit == 1 since #1 and #4 are in the FSP. This is one of the reasons we added CpuMpPpi.
With SGX disabled and SkipMpInit == 1, your change seems reasonable. I recommend you add an if statement here that only sets LT_LOCK_MEMORY iff FspmUpd->FspmConfig.SkipMpInit == 1.
-
Nathaniel L Desimone (Code Review)