Nico Huber has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/31664
Change subject: cpu/intel/model_1067x: Implement microcode loading ......................................................................
cpu/intel/model_1067x: Implement microcode loading
We load it once for the BSP in advance and let the MP init handle it for the APs. The BSP load could also be done earlier, e.g. before CAR setup, to align with other platforms.
TEST=Booted ThinkPad X200s and checked log: Microcode is loaded correctly on the BSP before SMM setup, and reported to be up to date on all cores after.
Change-Id: I85adb22a608ca3e7355bd486ebba52ec8fdd396c Signed-off-by: Nico Huber nico.h@gmx.de --- M src/cpu/intel/model_1067x/mp_init.c 1 file changed, 10 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/64/31664/1
diff --git a/src/cpu/intel/model_1067x/mp_init.c b/src/cpu/intel/model_1067x/mp_init.c index acd56c8..b8b3159 100644 --- a/src/cpu/intel/model_1067x/mp_init.c +++ b/src/cpu/intel/model_1067x/mp_init.c @@ -42,6 +42,12 @@ return cores; }
+static void get_microcode_info(const void **microcode, int *parallel) +{ + *microcode = microcode_patch; + *parallel = 1; +} + /* the SMRR enable and lock bit need to be set in IA32_FEATURE_CONTROL to enable SMRR so configure IA32_FEATURE_CONTROL early on */ static void pre_mp_smm_init(void) @@ -98,6 +104,7 @@ .pre_mp_init = pre_mp_init, .get_cpu_count = get_cpu_count, .get_smm_info = smm_info, + .get_microcode_info = get_microcode_info, .pre_mp_smm_init = pre_mp_smm_init, .per_cpu_smm_trigger = per_cpu_smm_trigger, .relocation_handler = smm_relocation_handler, @@ -106,6 +113,9 @@
void bsp_init_and_start_aps(struct bus *cpu_bus) { + microcode_patch = intel_microcode_find(); + intel_microcode_load_unlocked(microcode_patch); + if (mp_init_with_smm(cpu_bus, &mp_ops)) printk(BIOS_ERR, "MP initialization failure.\n"); }
Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31664 )
Change subject: cpu/intel/model_1067x: Implement microcode loading ......................................................................
Patch Set 1: Code-Review+2
(1 comment)
https://review.coreboot.org/#/c/31664/1/src/cpu/intel/model_1067x/mp_init.c File src/cpu/intel/model_1067x/mp_init.c:
https://review.coreboot.org/#/c/31664/1/src/cpu/intel/model_1067x/mp_init.c@... PS1, Line 116: microcode_patch = intel_microcode_find(); : intel_microcode_load_unlocked(microcode_patch); can also be done in pre_mp_init. I don't feel strongly about either one.
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31664 )
Change subject: cpu/intel/model_1067x: Implement microcode loading ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/#/c/31664/1/src/cpu/intel/model_1067x/mp_init.c File src/cpu/intel/model_1067x/mp_init.c:
https://review.coreboot.org/#/c/31664/1/src/cpu/intel/model_1067x/mp_init.c@... PS1, Line 116: microcode_patch = intel_microcode_find(); : intel_microcode_load_unlocked(microcode_patch);
can also be done in pre_mp_init. I don't feel strongly about either one.
Oh, right. I intuitively put it here, because I knew exactly that `microcode_patch` will be initialized early enough. pre_mp_init() guarantees that too (just read the documentation in the header file now).
Hmmm, or maybe split them up? Keep the initialization here as a prerequisite. Move the loading into pre_mp_init()?
Masanori Ogino has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31664 )
Change subject: cpu/intel/model_1067x: Implement microcode loading ......................................................................
Patch Set 1: Code-Review+1
Tested with SeaBIOS, Linux 4.19.0 booted successfully and microcode was loaded.
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31664 )
Change subject: cpu/intel/model_1067x: Implement microcode loading ......................................................................
Patch Set 1: Code-Review+2
Hello Kyösti Mälkki, Masanori Ogino, Patrick Rudolph, Angel Pons, Arthur Heymans, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/31664
to look at the new patch set (#2).
Change subject: cpu/intel/model_1067x: Implement microcode loading ......................................................................
cpu/intel/model_1067x: Implement microcode loading
We load it once for the BSP in advance and let the MP init handle it for the APs. The BSP load could also be done earlier, e.g. before CAR setup, to align with other platforms.
TEST=Booted ThinkPad X200s and checked log: Microcode is loaded correctly on the BSP before SMM setup, and reported to be up to date on all cores after.
Change-Id: I85adb22a608ca3e7355bd486ebba52ec8fdd396c Signed-off-by: Nico Huber nico.h@gmx.de --- M src/cpu/intel/model_1067x/mp_init.c 1 file changed, 11 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/64/31664/2
Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31664 )
Change subject: cpu/intel/model_1067x: Implement microcode loading ......................................................................
Patch Set 2: Code-Review+2
-
Arthur Heymans (Code Review) -
Kyösti Mälkki (Code Review)
-
Masanori Ogino (Code Review)
-
Nico Huber (Code Review)
-
Patrick Rudolph (Code Review)