Rocky Phagura has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43684 )
Change subject: cpu/x86/smm: Introduce SMM module loader version 2 ......................................................................
Patch Set 5:
(13 comments)
Incorporated feedback and uploaded the changes. Also updated Documentation/releases/coreboot-4.13.relnotes.md.
Thank you for testing and providing feedback everybody!
https://review.coreboot.org/c/coreboot/+/43684/2/src/cpu/x86/Kconfig File src/cpu/x86/Kconfig:
https://review.coreboot.org/c/coreboot/+/43684/2/src/cpu/x86/Kconfig@125 PS2, Line 125: bool
No need
So no changes that I need to make here?
https://review.coreboot.org/c/coreboot/+/43684/2/src/cpu/x86/Kconfig@129 PS2, Line 129: server
Agreed, this works even on non-server stuff
Done
https://review.coreboot.org/c/coreboot/+/43684/4/src/cpu/x86/mp_init.c File src/cpu/x86/mp_init.c:
https://review.coreboot.org/c/coreboot/+/43684/4/src/cpu/x86/mp_init.c@734 PS4, Line 734: __func__, cpu);
Fits on the previous line (line length limit is 96 characters)
Done
https://review.coreboot.org/c/coreboot/+/43684/2/src/cpu/x86/smm/smm_module_... File src/cpu/x86/smm/smm_module_loaderv2.c:
https://review.coreboot.org/c/coreboot/+/43684/2/src/cpu/x86/smm/smm_module_... PS2, Line 15: uint8_t
*poke* […]
This is same as previous loader, so I didn't change anything. I left it as is.
https://review.coreboot.org/c/coreboot/+/43684/2/src/cpu/x86/smm/smm_module_... PS2, Line 51: unsigned int
*poke*
done
https://review.coreboot.org/c/coreboot/+/43684/2/src/cpu/x86/smm/smm_module_... PS2, Line 108: printk(BIOS_ERR, "%s: unable to get SMM module size\n", __func__);
*poke*
No, we should not continue. Good catch. I've made the fix by adding a return 0 there.
https://review.coreboot.org/c/coreboot/+/43684/2/src/cpu/x86/smm/smm_module_... PS2, Line 112: 0xFFFF
*poke*
Yes, this can be confusing. It means 64K segment. I've now added a clarification in the above comment line.
https://review.coreboot.org/c/coreboot/+/43684/2/src/cpu/x86/smm/smm_module_... PS2, Line 123: not enough space in SMM to setup all CPUs
*poke*
A developer would have to read the Intel Software Developer's Manuals and the datasheet of the specific SOC to determine how much SMRAM is supported. This is based on various features available in the HW.
https://review.coreboot.org/c/coreboot/+/43684/2/src/cpu/x86/smm/smm_module_... PS2, Line 213: unsigned int
*poke*
Done
https://review.coreboot.org/c/coreboot/+/43684/2/src/cpu/x86/smm/smm_module_... PS2, Line 226: params->smm_main_entry_offset < stack_top) {
*poke*
Done
https://review.coreboot.org/c/coreboot/+/43684/2/src/cpu/x86/smm/smm_module_... PS2, Line 231: __func__, cpus[num_cpus].smbase, stack_top);
*poke*
Done
https://review.coreboot.org/c/coreboot/+/43684/2/src/cpu/x86/smm/smm_module_... PS2, Line 604: "%s: need more SMRAM\n", __func__);
*poke* […]
Done
https://review.coreboot.org/c/coreboot/+/43684/2/src/include/cpu/x86/smm.h File src/include/cpu/x86/smm.h:
https://review.coreboot.org/c/coreboot/+/43684/2/src/include/cpu/x86/smm.h@1... PS2, Line 152: #if CONFIG(X86_SMM_LOADER_VERSION2)
*poke*
Done