Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37187 )
Change subject: soc/intel/{apl,cnl,dnv,skl}: Skip ucode loading by FSP-T ......................................................................
Patch Set 3:
(9 comments)
https://review.coreboot.org/c/coreboot/+/37187/2//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/37187/2//COMMIT_MSG@18 PS2, Line 18: skipping skip
https://review.coreboot.org/c/coreboot/+/37187/2//COMMIT_MSG@20 PS2, Line 20: removed remove
https://review.coreboot.org/c/coreboot/+/37187/2//COMMIT_MSG@9 PS2, Line 9: It is a requirement for Firmware to have Firmware Interface Table (FIT), : which contains pointers to each microcode update. : The microcode update is loaded for all logical processors before reset vector. : : FSPT_UPD.MicrocodeRegionBase and FSPT_UPD.MicrocodeRegionLength are : input parameters to TempRamInit API. : If these values are 0, FSP will not attempt to update microcode. : : Since Gen-4 all IA-SoC has FIT loading ucode even before cpu reset in place : hence skipping FSP-T loading ucode after CPU reset options. : : Also removed unused kconfig CONFIG_CPU_MICROCODE_CBFS_LOC and : CONFIG_CPU_MICROCODE_CBFS_LEN Please reformat for the 75 characters text width.
https://review.coreboot.org/c/coreboot/+/37187/2//COMMIT_MSG@22 PS2, Line 22: Tested how? Is any time saved?
https://review.coreboot.org/c/coreboot/+/37187/2/src/soc/intel/apollolake/fs... File src/soc/intel/apollolake/fspcar.c:
https://review.coreboot.org/c/coreboot/+/37187/2/src/soc/intel/apollolake/fs... PS2, Line 32: cpu CPU
https://review.coreboot.org/c/coreboot/+/37187/2/src/soc/intel/apollolake/fs... PS2, Line 34: * All SoC since Gen-4 has above mechanism in place to load microcode microcode updates
https://review.coreboot.org/c/coreboot/+/37187/2/src/soc/intel/apollolake/fs... PS2, Line 35: skipping skip
https://review.coreboot.org/c/coreboot/+/37187/2/src/soc/intel/apollolake/fs... PS2, Line 36: * microcode after CPU reset by passing '0' value to microcode updates
https://review.coreboot.org/c/coreboot/+/37187/2/src/soc/intel/apollolake/fs... PS2, Line 40: .MicrocodeRegionLength = 0, So it was already disabled for APL? Mention that in the commit message, and maybe reference the commit doing that?