Patrick Rudolph has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/35739 )
Change subject: soc/intel/common/block: Update microcode for each core ......................................................................
soc/intel/common/block: Update microcode for each core
On Hyper-Threading enabled platform update the microcde only once for each core, not for each thread.
Follow Intel Software Developer Guidelines as the added comment also states.
Change-Id: I72804753e567a137a5648ca6950009fed332531b Signed-off-by: Patrick Rudolph patrick.rudolph@9elements.com --- M src/soc/intel/common/block/cpu/mp_init.c 1 file changed, 20 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/39/35739/1
diff --git a/src/soc/intel/common/block/cpu/mp_init.c b/src/soc/intel/common/block/cpu/mp_init.c index 2c5061f..e7689cf 100644 --- a/src/soc/intel/common/block/cpu/mp_init.c +++ b/src/soc/intel/common/block/cpu/mp_init.c @@ -26,6 +26,7 @@ #include <intelblocks/fast_spi.h> #include <intelblocks/mp_init.h> #include <intelblocks/msr.h> +#include <cpu/intel/common/common.h> #include <soc/cpu.h>
static const void *microcode_patch; @@ -44,7 +45,24 @@ static void init_one_cpu(struct device *dev) { soc_core_init(dev); - intel_microcode_load_unlocked(microcode_patch); + + /* + * Update just on the first CPU in the core. Other siblings + * get the update automatically according to Document: 253668-060US + * Intel SDM Chapter 9.11.6.3 + * "Update in a System Supporting Intel Hyper-Threading Technology" + * Intel Hyper-Threading Technology has implications on the loading of the + * microcode update. The update must be loaded for each core in a physical + * processor. Thus, for a processor supporting Intel Hyper-Threading + * Technology, only one logical processor per core is required to load the + * microcode update. Each individual logical processor can independently + * load the update. However, MP initialization must provide some mechanism + * (e.g. a software semaphore) to force serialization of microcode update + * loads and to prevent simultaneous load attempts to the same core. + */ + if (!intel_ht_sibling()) { + intel_microcode_load_unlocked(microcode_patch); + } }
static struct device_operations cpu_dev_ops = { @@ -141,6 +159,7 @@ if (CONFIG(USE_INTEL_FSP_MP_INIT)) return;
+ /* Update microcode on BSP */ microcode_patch = intel_microcode_find(); intel_microcode_load_unlocked(microcode_patch);
Hello Patrick Rudolph, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/35739
to look at the new patch set (#2).
Change subject: soc/intel/common/block: Update microcode for each core ......................................................................
soc/intel/common/block: Update microcode for each core
On Hyper-Threading enabled platform update the microcde only once for each core, not for each thread.
Follow Intel Software Developer Guidelines as the added comment also states.
Change-Id: I72804753e567a137a5648ca6950009fed332531b Signed-off-by: Patrick Rudolph patrick.rudolph@9elements.com --- M src/soc/intel/common/block/cpu/Kconfig M src/soc/intel/common/block/cpu/mp_init.c 2 files changed, 22 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/39/35739/2
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35739 )
Change subject: soc/intel/common/block: Update microcode for each core ......................................................................
Patch Set 2: Code-Review+1
(1 comment)
Does this also reduce the boot time?
Tested how?
https://review.coreboot.org/c/coreboot/+/35739/2/src/soc/intel/common/block/... File src/soc/intel/common/block/cpu/mp_init.c:
https://review.coreboot.org/c/coreboot/+/35739/2/src/soc/intel/common/block/... PS2, Line 50: Update just on the first CPU in the core Remove *on*?
Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35739 )
Change subject: soc/intel/common/block: Update microcode for each core ......................................................................
Patch Set 2:
This is also done in the cpu/x86/sipi_vector.S. Should that code also serialize/skip MCU on logical CPUs?
Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35739 )
Change subject: soc/intel/common/block: Update microcode for each core ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/35739/2//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/35739/2//COMMIT_MSG@10 PS2, Line 10: each core, not for each thread. Nit: Intel Software Developer Guidelines speaks of cores and logical processors.
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35739 )
Change subject: soc/intel/common/block: Update microcode for each core ......................................................................
Patch Set 2: Code-Review-1
(4 comments)
You can ignore the bikeshedding comments, that was before I looked into the documentation. It seems the SDM is still talking about Pentium4 style Hyper-Threading.
Starting with Nehalem, their second HT incarnation, I see the requirement to do it on all logical processors with synchronisation per core.
Starting with Ivy Bridge, the synchronisation requirement vanished.
@Intel, please assist to get the Software Developer's Manual updated (or the BWGs fixed if they are wrong).
https://review.coreboot.org/c/coreboot/+/35739/2/src/soc/intel/common/block/... File src/soc/intel/common/block/cpu/mp_init.c:
https://review.coreboot.org/c/coreboot/+/35739/2/src/soc/intel/common/block/... PS2, Line 50: CPU CPU, at least to me, usually refers to the whole package.
https://review.coreboot.org/c/coreboot/+/35739/2/src/soc/intel/common/block/... PS2, Line 51: 253668-060US : * Intel SDM Chapter 9.11.6.3 A much better reference would be the name of the document and the chapter. Chapter numbers change, and even if one knows the document revision that doesn't mean they have access to that specific revision.
https://review.coreboot.org/c/coreboot/+/35739/2/src/soc/intel/common/block/... PS2, Line 53: * "Update in a System Supporting Intel Hyper-Threading Technology" : * Intel Hyper-Threading Technology has implications on the loading of the : * microcode update. The update must be loaded for each core in a physical : * processor. Thus, for a processor supporting Intel Hyper-Threading : * Technology, only one logical processor per core is required to load the : * microcode update. Each individual logical processor can independently : * load the update. However, MP initialization must provide some mechanism : * (e.g. a software semaphore) to force serialization of microcode update : * loads and to prevent simultaneous load attempts to the same core. No need for the quote, imho.
https://review.coreboot.org/c/coreboot/+/35739/2/src/soc/intel/common/block/... PS2, Line 162: /* Update microcode on BSP */ That's obvious. Much more interesting would be to know why?
Hello Lijian Zhao, build bot (Jenkins), Nico Huber, Rizwan Qureshi, Paul Menzel, Subrata Banik, Aaron Durbin, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/35739
to look at the new patch set (#3).
Change subject: soc/intel/common/block: Update microcode for each core ......................................................................
soc/intel/common/block: Update microcode for each core
On Hyper-Threading enabled platform update the microcde only once for each core, not for each thread.
Follow Intel Software Developer Guidelines as the added comment also states.
Change-Id: I72804753e567a137a5648ca6950009fed332531b Signed-off-by: Patrick Rudolph patrick.rudolph@9elements.com --- M src/cpu/intel/common/common.h M src/cpu/intel/common/hyperthreading.c M src/include/cpu/x86/lapic.h M src/soc/intel/common/block/cpu/Kconfig M src/soc/intel/common/block/cpu/mp_init.c 5 files changed, 79 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/39/35739/3
Hello build bot (Jenkins), Lijian Zhao, Nico Huber, Rizwan Qureshi, Paul Menzel, Subrata Banik, Aaron Durbin, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/35739
to look at the new patch set (#4).
Change subject: soc/intel/common/block: Update microcode serialiced for HT threads ......................................................................
soc/intel/common/block: Update microcode serialiced for HT threads
On Hyper-Threading enabled platforms update the microcde on a core serialized for threads running on that core. This prevents the "microcode: Update failed" error seen in the console log.
There's still some confusion as the SDW and BWG state differently. It's not clear if the HT sibling must update the microcode at all, as a core likely shares the microcode unit between HT siblings.
Change-Id: I72804753e567a137a5648ca6950009fed332531b Signed-off-by: Patrick Rudolph patrick.rudolph@9elements.com --- M src/cpu/intel/common/common.h M src/cpu/intel/common/hyperthreading.c M src/include/cpu/x86/lapic.h M src/soc/intel/common/block/cpu/Kconfig M src/soc/intel/common/block/cpu/mp_init.c 5 files changed, 79 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/39/35739/4
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35739 )
Change subject: soc/intel/common/block: Update microcode serialiced for HT threads ......................................................................
Patch Set 4:
(1 comment)
https://review.coreboot.org/c/coreboot/+/35739/4/src/cpu/intel/common/hypert... File src/cpu/intel/common/hyperthreading.c:
https://review.coreboot.org/c/coreboot/+/35739/4/src/cpu/intel/common/hypert... PS4, Line 57: if (!initialized) This would need to be checked inside the `init` lock.
But why can't we initialize them statically? Like DECLARE_SPIN_LOCK() does?
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35739 )
Change subject: soc/intel/common/block: Update microcode serialiced for HT threads ......................................................................
Patch Set 4:
(4 comments)
https://review.coreboot.org/c/coreboot/+/35739/4//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/35739/4//COMMIT_MSG@7 PS4, Line 7: soc/intel/common/block: Update microcode serialiced for HT threads serialized
https://review.coreboot.org/c/coreboot/+/35739/4//COMMIT_MSG@9 PS4, Line 9: microcde microcode
https://review.coreboot.org/c/coreboot/+/35739/4//COMMIT_MSG@11 PS4, Line 11: "microcode: Update failed" error seen in the console log. On what board? Or on all?
https://review.coreboot.org/c/coreboot/+/35739/4//COMMIT_MSG@13 PS4, Line 13: state differently
… contradict each other.
Maybe also add the sections?
Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35739 )
Change subject: soc/intel/common/block: Update microcode serialiced for HT threads ......................................................................
Patch Set 4:
(5 comments)
https://review.coreboot.org/c/coreboot/+/35739/4/src/cpu/intel/common/hypert... File src/cpu/intel/common/hyperthreading.c:
https://review.coreboot.org/c/coreboot/+/35739/4/src/cpu/intel/common/hypert... PS4, Line 40: = 0; no need to initialize to 0 explicitly.
https://review.coreboot.org/c/coreboot/+/35739/4/src/cpu/intel/common/hypert... PS4, Line 46: 1; this value shouldn't be open coded. SPIN_LOCK_UNLOCKED definition should be used.
https://review.coreboot.org/c/coreboot/+/35739/4/src/cpu/intel/common/hypert... PS4, Line 53: /* Is Hyper-Threading supported */ : if (!(cpuid_edx(1) & CPUID_FEAURE_HTT)) : return; : This is duplicated below. Add a helper function?
https://review.coreboot.org/c/coreboot/+/35739/4/src/cpu/intel/common/hypert... PS4, Line 57: if (!initialized)
This would need to be checked inside the `init` lock. […]
Yes, this initialization should be done on BSP first instead of racing on the APs.
https://review.coreboot.org/c/coreboot/+/35739/4/src/soc/intel/common/block/... File src/soc/intel/common/block/cpu/mp_init.c:
https://review.coreboot.org/c/coreboot/+/35739/4/src/soc/intel/common/block/... PS4, Line 40: Tests For which parts? There are many different implementations.
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35739 )
Change subject: soc/intel/common/block: Update microcode serialiced for HT threads ......................................................................
Patch Set 4:
(1 comment)
https://review.coreboot.org/c/coreboot/+/35739/4/src/soc/intel/common/block/... File src/soc/intel/common/block/cpu/mp_init.c:
https://review.coreboot.org/c/coreboot/+/35739/4/src/soc/intel/common/block/... PS4, Line 42: and updates one Some word seems to be missing here.
Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35739 )
Change subject: soc/intel/common/block: Update microcode serialiced for HT threads ......................................................................
Patch Set 4:
(1 comment)
https://review.coreboot.org/c/coreboot/+/35739/4/src/cpu/intel/common/hypert... File src/cpu/intel/common/hyperthreading.c:
https://review.coreboot.org/c/coreboot/+/35739/4/src/cpu/intel/common/hypert... PS4, Line 57: if (!initialized)
Yes, this initialization should be done on BSP first instead of racing on the APs.
It's not possible in C to initialize a variable array to something else than zero.
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35739 )
Change subject: soc/intel/common/block: Update microcode serialiced for HT threads ......................................................................
Patch Set 4:
(1 comment)
https://review.coreboot.org/c/coreboot/+/35739/4/src/cpu/intel/common/hypert... File src/cpu/intel/common/hyperthreading.c:
https://review.coreboot.org/c/coreboot/+/35739/4/src/cpu/intel/common/hypert... PS4, Line 57: if (!initialized)
It's not possible in C to initialize a variable array to something else than zero.
Are you talking about C the language or C in ramstage with some coreboot limitation? I remember things very differently...
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35739 )
Change subject: soc/intel/common/block: Update microcode serialiced for HT threads ......................................................................
Patch Set 4:
(1 comment)
https://review.coreboot.org/c/coreboot/+/35739/4/src/cpu/intel/common/hypert... File src/cpu/intel/common/hyperthreading.c:
https://review.coreboot.org/c/coreboot/+/35739/4/src/cpu/intel/common/hypert... PS4, Line 57: if (!initialized)
Are you talking about C the language or C in ramstage with some coreboot […]
Sorry, read it a few times more. You mean, we don't know CONFIG_MAX_CPUS, right? I get it now. There is nothing variable in C, though.
Just doing it on the BSP seems like a good idea.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35739 )
Change subject: soc/intel/common/block: Update microcode serialiced for HT threads ......................................................................
Patch Set 4:
(1 comment)
https://review.coreboot.org/c/coreboot/+/35739/4//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/35739/4//COMMIT_MSG@13 PS4, Line 13: SDW Does SDW stand for "Software Development (╯°□°)╯︵ lɐnuɐW"? 😄
Hello build bot (Jenkins), Lijian Zhao, Nico Huber, Paul Menzel, Rizwan Qureshi, Subrata Banik, Aaron Durbin, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/35739
to look at the new patch set (#5).
Change subject: soc/intel/common/block: Update microcode serialized for HT threads ......................................................................
soc/intel/common/block: Update microcode serialized for HT threads
On Hyper-Threading enabled platforms update the microcode on a core serialized for threads running on that core. This prevents the "microcode: Update failed" error seen in the console log on boards with multiple cores and Hyper-threading enabled.
There's still some confusion as the Document: 253668-060US Intel SDM Chapter 9.11.6.3 and BWG state differently. It's not clear if the HT sibling must update the microcode at all, as a core likely shares the microcode unit between HT siblings.
The recommondation of Linux engineers is to idle the sibling thread at least, and if possible idle as many cores as possible when updating.
Change-Id: I72804753e567a137a5648ca6950009fed332531b Signed-off-by: Patrick Rudolph patrick.rudolph@9elements.com --- M src/cpu/intel/common/common.h M src/cpu/intel/common/hyperthreading.c M src/include/cpu/x86/lapic.h M src/soc/intel/common/block/cpu/Kconfig M src/soc/intel/common/block/cpu/mp_init.c 5 files changed, 87 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/39/35739/5
Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35739 )
Change subject: soc/intel/common/block: Update microcode serialized for HT threads ......................................................................
Patch Set 4:
(11 comments)
https://review.coreboot.org/c/coreboot/+/35739/4//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/35739/4//COMMIT_MSG@7 PS4, Line 7: soc/intel/common/block: Update microcode serialiced for HT threads
serialized
Done
https://review.coreboot.org/c/coreboot/+/35739/4//COMMIT_MSG@9 PS4, Line 9: microcde
microcode
Done
https://review.coreboot.org/c/coreboot/+/35739/4//COMMIT_MSG@11 PS4, Line 11: "microcode: Update failed" error seen in the console log.
On what board? Or on all?
Done
https://review.coreboot.org/c/coreboot/+/35739/4//COMMIT_MSG@13 PS4, Line 13: state differently
… contradict each other. […]
Done
https://review.coreboot.org/c/coreboot/+/35739/4//COMMIT_MSG@13 PS4, Line 13: SDW
Does SDW stand for "Software Development (╯°□°)╯︵ lɐnuɐW"? 😄
Done
https://review.coreboot.org/c/coreboot/+/35739/4/src/cpu/intel/common/hypert... File src/cpu/intel/common/hyperthreading.c:
https://review.coreboot.org/c/coreboot/+/35739/4/src/cpu/intel/common/hypert... PS4, Line 40: = 0;
no need to initialize to 0 explicitly.
Done
https://review.coreboot.org/c/coreboot/+/35739/4/src/cpu/intel/common/hypert... PS4, Line 46: 1;
this value shouldn't be open coded. SPIN_LOCK_UNLOCKED definition should be used.
Done
https://review.coreboot.org/c/coreboot/+/35739/4/src/cpu/intel/common/hypert... PS4, Line 53: /* Is Hyper-Threading supported */ : if (!(cpuid_edx(1) & CPUID_FEAURE_HTT)) : return; :
This is duplicated below. […]
Done
https://review.coreboot.org/c/coreboot/+/35739/4/src/cpu/intel/common/hypert... PS4, Line 57: if (!initialized)
Sorry, read it a few times more. You mean, we don't know CONFIG_MAX_CPUS, […]
Done
https://review.coreboot.org/c/coreboot/+/35739/4/src/soc/intel/common/block/... File src/soc/intel/common/block/cpu/mp_init.c:
https://review.coreboot.org/c/coreboot/+/35739/4/src/soc/intel/common/block/... PS4, Line 40: Tests
For which parts? There are many different implementations.
Done
https://review.coreboot.org/c/coreboot/+/35739/4/src/soc/intel/common/block/... PS4, Line 42: and updates one
Some word seems to be missing here.
Done
Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35739 )
Change subject: soc/intel/common/block: Update microcode serialized for HT threads ......................................................................
Patch Set 5:
(2 comments)
https://review.coreboot.org/c/coreboot/+/35739/5/src/soc/intel/common/block/... File src/soc/intel/common/block/cpu/mp_init.c:
https://review.coreboot.org/c/coreboot/+/35739/5/src/soc/intel/common/block/... PS5, Line 46: intel_ht_sibling_spin_lock(); This is assuming that all microcode loading needs to be serialized on hyperthreads. There's a whole sequence in src/cpu/x86/sipi_vector.S that should be coordinated. I don't think we should have 2 different things colliding here regarding policy.
See get_microcode_info() below where it provides the ability to load in parallel when starting up the APs in assembly.
https://review.coreboot.org/c/coreboot/+/35739/5/src/soc/intel/common/block/... PS5, Line 139: *parallel = 1; See here regarding my previous comment. Clearly you have some devices which need the lock while others don't. I think the policy should follow based on opt-in behavior.
Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35739 )
Change subject: soc/intel/common/block: Update microcode serialized for HT threads ......................................................................
Patch Set 5:
(1 comment)
https://review.coreboot.org/c/coreboot/+/35739/5/src/soc/intel/common/block/... File src/soc/intel/common/block/cpu/mp_init.c:
https://review.coreboot.org/c/coreboot/+/35739/5/src/soc/intel/common/block/... PS5, Line 139: *parallel = 1;
See here regarding my previous comment. […]
Should I document the existing policies first and then add a new one? That would be: * Serialize all updates * Serialize on HT cores (if HT enabled) * No serializing of microcode loading
Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35739 )
Change subject: soc/intel/common/block: Update microcode serialized for HT threads ......................................................................
Patch Set 5:
(1 comment)
https://review.coreboot.org/c/coreboot/+/35739/5/src/soc/intel/common/block/... File src/soc/intel/common/block/cpu/mp_init.c:
https://review.coreboot.org/c/coreboot/+/35739/5/src/soc/intel/common/block/... PS5, Line 139: *parallel = 1;
Should I document the existing policies first and then add a new one? […]
Sounds like a good idea. Bigger question is why we are doing microcode updates above. Presumably it's because of needing to reload after SMM has been relocated? We should note why we're doing the same load twice. I know there are some cpus that wanted a 2nd reload (haswell, iirc). SGX, while note used, also needs another reload, if my memory serves.
I think we should document as well as ensure we select the right policy for each cpu because I don't think those options are necessary on all cpus.
Hello build bot (Jenkins), Lijian Zhao, Nico Huber, Paul Menzel, Rizwan Qureshi, Subrata Banik, Aaron Durbin, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/35739
to look at the new patch set (#6).
Change subject: [WIP]soc/intel/common/block: Update microcode serialized for HT threads ......................................................................
[WIP]soc/intel/common/block: Update microcode serialized for HT threads
On Hyper-Threading enabled platforms update the microcode on a core serialized for threads running on that core. This prevents the "microcode: Update failed" error seen in the console log on boards with multiple cores and Hyper-threading enabled.
The SDM and most of the BWG suggest doing this. Documented a few to get a common sense what should be done, when and how microcode should be updated.
TODO: * Fix MPinit's first microcode update * Remove comments that state microcode updates should be done after SMM setup (it should be done at end of MPinit) * Guard microcode updates on HT where required
Change-Id: I72804753e567a137a5648ca6950009fed332531b Signed-off-by: Patrick Rudolph patrick.rudolph@9elements.com --- M Documentation/soc/intel/index.md A Documentation/soc/intel/microcode.md M src/cpu/intel/common/common.h M src/cpu/intel/common/hyperthreading.c M src/cpu/intel/microcode/microcode.c M src/include/cpu/intel/microcode.h M src/include/cpu/x86/lapic.h M src/soc/intel/common/block/cpu/Kconfig M src/soc/intel/common/block/cpu/mp_init.c M src/soc/intel/common/block/sgx/sgx.c 10 files changed, 223 insertions(+), 21 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/39/35739/6
Hello build bot (Jenkins), Lijian Zhao, Nico Huber, Christian Walter, Paul Menzel, Rizwan Qureshi, Subrata Banik, Aaron Durbin, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/35739
to look at the new patch set (#9).
Change subject: [WIP]soc/intel/common/block: Update microcode serialized for HT threads ......................................................................
[WIP]soc/intel/common/block: Update microcode serialized for HT threads
On Hyper-Threading enabled platforms update the microcode on a core serialized for threads running on that core. This prevents the "microcode: Update failed" error seen in the console log on boards with multiple cores and Hyper-threading enabled.
The SDM and most of the BWG suggest doing this. Documented a few to get a common sense what should be done, when and how microcode should be updated.
TODO: * Fix MPinit's first microcode update * Remove comments that state microcode updates should be done after SMM setup (it should be done at end of MPinit) * Guard microcode updates on HT where required
Change-Id: I72804753e567a137a5648ca6950009fed332531b Signed-off-by: Patrick Rudolph patrick.rudolph@9elements.com --- M Documentation/soc/intel/index.md A Documentation/soc/intel/microcode.md M src/cpu/intel/common/common.h M src/cpu/intel/common/hyperthreading.c M src/cpu/intel/microcode/microcode.c M src/include/cpu/intel/microcode.h M src/include/cpu/x86/lapic.h M src/soc/intel/common/block/cpu/Kconfig M src/soc/intel/common/block/cpu/mp_init.c M src/soc/intel/common/block/sgx/sgx.c 10 files changed, 235 insertions(+), 21 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/39/35739/9
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35739 )
Change subject: [WIP]soc/intel/common/block: Update microcode serialized for HT threads ......................................................................
Patch Set 11:
(1 comment)
https://review.coreboot.org/c/coreboot/+/35739/2/src/soc/intel/common/block/... File src/soc/intel/common/block/cpu/mp_init.c:
https://review.coreboot.org/c/coreboot/+/35739/2/src/soc/intel/common/block/... PS2, Line 50: Update just on the first CPU in the core
Remove *on*?
Ack
Angel Pons has uploaded a new patch set (#12) to the change originally created by Patrick Rudolph. ( https://review.coreboot.org/c/coreboot/+/35739 )
Change subject: [WIP]soc/intel/common/block: Update microcode serialized for HT threads ......................................................................
[WIP]soc/intel/common/block: Update microcode serialized for HT threads
On Hyper-Threading enabled platforms update the microcode on a core serialized for threads running on that core. This prevents the "microcode: Update failed" error seen in the console log on boards with multiple cores and Hyper-threading enabled.
The SDM and most of the BWG suggest doing this. Documented a few to get a common sense what should be done, when and how microcode should be updated.
TODO: * Fix MPinit's first microcode update * Remove comments that state microcode updates should be done after SMM setup (it should be done at end of MPinit) * Guard microcode updates on HT where required
Change-Id: I72804753e567a137a5648ca6950009fed332531b Signed-off-by: Patrick Rudolph patrick.rudolph@9elements.com --- M Documentation/soc/intel/index.md A Documentation/soc/intel/microcode.md M src/cpu/intel/common/common.h M src/cpu/intel/common/hyperthreading.c M src/cpu/intel/microcode/microcode.c M src/include/cpu/intel/microcode.h M src/include/cpu/x86/lapic.h M src/soc/intel/common/block/cpu/Kconfig M src/soc/intel/common/block/cpu/mp_init.c M src/soc/intel/common/block/sgx/sgx.c 10 files changed, 244 insertions(+), 19 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/39/35739/12
Angel Pons has uploaded a new patch set (#13) to the change originally created by Patrick Rudolph. ( https://review.coreboot.org/c/coreboot/+/35739 )
Change subject: [WIP]soc/intel/common/block: Update microcode serialized for HT threads ......................................................................
[WIP]soc/intel/common/block: Update microcode serialized for HT threads
On HyperThreading-enabled platforms update the microcode on a core serialized for threads running on that core. This prevents the "microcode: Update failed" error seen in the console log on boards with multiple cores and Hyper-threading enabled.
The SDM and most of the BWGs suggest doing this.
TODO: * Fix MPinit's first microcode update * Remove comments that state microcode updates should be done after SMM setup (it should be done at end of MPinit) * Guard microcode updates on HT where required
TOTEST: Can we update the microcode on all primary threads, then on all secondary threads? This would make locking much less complex.
Change-Id: I72804753e567a137a5648ca6950009fed332531b Signed-off-by: Patrick Rudolph patrick.rudolph@9elements.com --- M src/cpu/intel/common/common.h M src/cpu/intel/common/hyperthreading.c M src/cpu/intel/microcode/microcode.c M src/include/cpu/intel/microcode.h M src/include/cpu/x86/lapic.h M src/soc/intel/common/block/cpu/Kconfig M src/soc/intel/common/block/cpu/mp_init.c M src/soc/intel/common/block/sgx/sgx.c 8 files changed, 115 insertions(+), 19 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/39/35739/13
Angel Pons has uploaded a new patch set (#14) to the change originally created by Patrick Rudolph. ( https://review.coreboot.org/c/coreboot/+/35739 )
Change subject: [WIP] soc/intel/common/block: Serialize microcode updates for HT threads ......................................................................
[WIP] soc/intel/common/block: Serialize microcode updates for HT threads
On HyperThreading-enabled platforms update the microcode on a core serialized for threads running on that core. This prevents the "microcode: Update failed" error seen in the console log on boards with multiple cores and HyperThreading enabled.
The SDM and most of the BWGs suggest doing this.
TODO: * Fix MPinit's first microcode update * Remove comments that state microcode updates should be done after SMM setup (it should be done at end of MPinit) * Guard microcode updates on HT where required
TOTEST: Can we update the microcode on all primary threads, then on all secondary threads? This would make locking much less complex.
Change-Id: I72804753e567a137a5648ca6950009fed332531b Signed-off-by: Patrick Rudolph patrick.rudolph@9elements.com --- M src/cpu/intel/common/common.h M src/cpu/intel/common/hyperthreading.c M src/cpu/intel/microcode/microcode.c M src/include/cpu/intel/microcode.h M src/include/cpu/x86/lapic.h M src/soc/intel/common/block/cpu/Kconfig M src/soc/intel/common/block/cpu/mp_init.c M src/soc/intel/common/block/sgx/sgx.c 8 files changed, 115 insertions(+), 19 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/39/35739/14
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35739 )
Change subject: [WIP] soc/intel/common/block: Serialize microcode updates for HT threads ......................................................................
Patch Set 13:
(1 comment)
https://review.coreboot.org/c/coreboot/+/35739/2//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/35739/2//COMMIT_MSG@10 PS2, Line 10: each core, not for each thread.
Nit: Intel Software Developer Guidelines speaks of cores and logical processors.
Other documents talk about cores and threads. The commit message probably needs an update anyway.
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35739 )
Change subject: [WIP] soc/intel/common/block: Serialize microcode updates for HT threads ......................................................................
Patch Set 14:
(1 comment)
https://review.coreboot.org/c/coreboot/+/35739/3/src/soc/intel/common/block/... File src/soc/intel/common/block/cpu/mp_init.c:
https://review.coreboot.org/c/coreboot/+/35739/3/src/soc/intel/common/block/... PS3, Line 40: Testes Tests
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35739 )
Change subject: [WIP] soc/intel/common/block: Serialize microcode updates for HT threads ......................................................................
Patch Set 14:
(1 comment)
https://review.coreboot.org/c/coreboot/+/35739/3//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/35739/3//COMMIT_MSG@14 PS3, Line 14: Is there a boot time penalty caused by serialization?
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35739 )
Change subject: [WIP] soc/intel/common/block: Serialize microcode updates for HT threads ......................................................................
Patch Set 14:
(1 comment)
https://review.coreboot.org/c/coreboot/+/35739/3//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/35739/3//COMMIT_MSG@14 PS3, Line 14:
Is there a boot time penalty caused by serialization?
I don't think you meant to comment on this old patchset? I haven't tried yet.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35739 )
Change subject: [WIP] soc/intel/common/block: Serialize microcode updates for HT threads ......................................................................
Patch Set 15:
(4 comments)
https://review.coreboot.org/c/coreboot/+/35739/2//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/35739/2//COMMIT_MSG@10 PS2, Line 10: each core, not for each thread.
Other documents talk about cores and threads. The commit message probably needs an update anyway.
Ack. Documentation about MSR scope uses "thread" anyway.
https://review.coreboot.org/c/coreboot/+/35739/3/src/soc/intel/common/block/... File src/soc/intel/common/block/cpu/mp_init.c:
https://review.coreboot.org/c/coreboot/+/35739/3/src/soc/intel/common/block/... PS3, Line 40: Testes
Tests
Gone
https://review.coreboot.org/c/coreboot/+/35739/5/src/soc/intel/common/block/... File src/soc/intel/common/block/cpu/mp_init.c:
https://review.coreboot.org/c/coreboot/+/35739/5/src/soc/intel/common/block/... PS5, Line 46: intel_ht_sibling_spin_lock();
This is assuming that all microcode loading needs to be serialized on hyperthreads. […]
Looks like this was changed. Will mark as resolved since the comment is buried in an old patchset.
https://review.coreboot.org/c/coreboot/+/35739/5/src/soc/intel/common/block/... PS5, Line 139: *parallel = 1;
Sounds like a good idea. Bigger question is why we are doing microcode updates above. […]
CB:44400 added documentation. We need to load microcode twice because these are enhanced microcode updates. AFAIUI, they may do security locks regarding SMM.
Only Haswell and Broadwell seem to allow unrestricted parallel microcode loading. No idea why. That only Broadwell is a SoC is annoying.
I'm going to mark this as resolved since it's buried in an old patchset. Feel free to reopen.
Angel Pons has uploaded a new patch set (#17) to the change originally created by Patrick Rudolph. ( https://review.coreboot.org/c/coreboot/+/35739 )
Change subject: soc/intel/common/block: Serialize microcode updates for HT threads ......................................................................
soc/intel/common/block: Serialize microcode updates for HT threads
On HyperThreading-enabled platforms, update the microcode on a core serialized for threads running on that core. This prevents the "microcode: Update failed" error seen in the console log on boards with multiple cores and HyperThreading enabled.
The SDM and most of the BWGs suggest doing this.
Tested on out-of-tree Skylake-U board, still boots.
TODO: * Fix MPinit's first microcode update * Remove comments that state microcode updates should be done after SMM setup (it should be done at end of MPinit) * Guard microcode updates on HT where required
TOTEST: Can we update the microcode on all primary threads, then on all secondary threads? This would make locking much less complex.
Change-Id: I72804753e567a137a5648ca6950009fed332531b Signed-off-by: Patrick Rudolph patrick.rudolph@9elements.com --- M src/cpu/intel/common/common.h M src/cpu/intel/common/hyperthreading.c M src/cpu/intel/microcode/microcode.c M src/include/cpu/intel/microcode.h M src/include/cpu/x86/lapic.h M src/soc/intel/common/block/cpu/Kconfig M src/soc/intel/common/block/cpu/mp_init.c M src/soc/intel/common/block/sgx/sgx.c 8 files changed, 115 insertions(+), 19 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/39/35739/17
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35739 )
Change subject: soc/intel/common/block: Serialize microcode updates for HT threads ......................................................................
Patch Set 17: Code-Review+1
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35739 )
Change subject: soc/intel/common/block: Serialize microcode updates for HT threads ......................................................................
Patch Set 17:
(1 comment)
https://review.coreboot.org/c/coreboot/+/35739/17/src/cpu/intel/microcode/mi... File src/cpu/intel/microcode/microcode.c:
https://review.coreboot.org/c/coreboot/+/35739/17/src/cpu/intel/microcode/mi... PS17, Line 63: Update just on the first CPU in the core. That's not what's happening here, is it?
(I almost ran into the good old forgot-to-review-comments.)
Wouldn't it suffice to say that * some HT capable CPUs need only one thread to do it, * some need both threads and software-sync, * some need both and do hardware-sync, and * software sync'ed version always works! ?
Attention is currently required from: Nico Huber, Angel Pons. Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35739 )
Change subject: soc/intel/common/block: Serialize microcode updates for HT threads ......................................................................
Patch Set 17:
(1 comment)
File src/cpu/intel/microcode/microcode.c:
https://review.coreboot.org/c/coreboot/+/35739/comment/5f449801_51e375b2 PS17, Line 63: Update just on the first CPU in the core.
That's not what's happening here, is it? […]
Yes, that's the summary. Do you mind having more background information in the code?
Attention is currently required from: Patrick Rudolph, Angel Pons. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35739 )
Change subject: soc/intel/common/block: Serialize microcode updates for HT threads ......................................................................
Patch Set 17:
(1 comment)
File src/cpu/intel/microcode/microcode.c:
https://review.coreboot.org/c/coreboot/+/35739/comment/fa65527a_1652dd97 PS17, Line 63: Update just on the first CPU in the core.
Yes, that's the summary. […]
Ufff, it's been a while, got to interpret my own comment here...
The part I highlighted seems wrong. That's why I commented in the first place. What I probably meant was that we should not talk about anything beyond these four topics, because everything else is a distraction and may or may not be true.
Background information is nice, as long as it doesn't make assumptions.
Attention is currently required from: Patrick Rudolph, Angel Pons. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35739 )
Change subject: soc/intel/common/block: Serialize microcode updates for HT threads ......................................................................
Patch Set 17: Code-Review+1
(1 comment)
Patchset:
PS17: Code looks good so far.
Attention is currently required from: Patrick Rudolph, Angel Pons. Hello build bot (Jenkins), Lijian Zhao, Nico Huber, Tim Wawrzynczak, Paul Menzel, Christian Walter, Rizwan Qureshi, Angel Pons, Subrata Banik, Aaron Durbin, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/35739
to look at the new patch set (#18).
Change subject: soc/intel/*: Serialize microcode updates for HT threads ......................................................................
soc/intel/*: Serialize microcode updates for HT threads
This change only affects MPinit related microcode updates and is a NOP for CPUs without HyperThreading enabled.
On HyperThreading-enabled platforms, update the microcode on a core serialized for threads running on that core. This prevents the "microcode: Update failed" error seen in the console log on boards with multiple cores and HyperThreading enabled.
The SDM and most of the BWGs suggest doing this.
Change-Id: I72804753e567a137a5648ca6950009fed332531b Signed-off-by: Patrick Rudolph patrick.rudolph@9elements.com --- M src/cpu/intel/common/common.h M src/cpu/intel/common/hyperthreading.c M src/cpu/intel/haswell/haswell_init.c M src/cpu/intel/microcode/microcode.c M src/cpu/intel/model_1067x/mp_init.c M src/cpu/intel/model_2065x/model_2065x_init.c M src/cpu/intel/model_206ax/model_206ax_init.c M src/include/cpu/intel/microcode.h M src/include/cpu/x86/lapic.h M src/soc/intel/alderlake/cpu.c M src/soc/intel/apollolake/cpu.c M src/soc/intel/baytrail/cpu.c M src/soc/intel/braswell/cpu.c M src/soc/intel/cannonlake/cpu.c M src/soc/intel/common/block/cpu/Kconfig M src/soc/intel/common/block/cpu/mp_init.c M src/soc/intel/common/block/sgx/sgx.c M src/soc/intel/denverton_ns/cpu.c M src/soc/intel/elkhartlake/cpu.c M src/soc/intel/icelake/cpu.c M src/soc/intel/jasperlake/cpu.c M src/soc/intel/skylake/chip.c M src/soc/intel/skylake/cpu.c M src/soc/intel/skylake/include/soc/ramstage.h M src/soc/intel/tigerlake/cpu.c M src/soc/intel/xeon_sp/cpx/cpu.c M src/soc/intel/xeon_sp/skx/cpu.c 27 files changed, 171 insertions(+), 60 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/39/35739/18
Attention is currently required from: Mariusz Szafrański, Suresh Bellampalli, Vanessa Eusebio, Angel Pons, Michal Motyl, Andrey Petrov. Hello build bot (Jenkins), Lijian Zhao, Mariusz Szafrański, Paul Menzel, Rizwan Qureshi, Angel Pons, Subrata Banik, Andrey Petrov, Aaron Durbin, Patrick Rudolph, Nico Huber, Suresh Bellampalli, Tim Wawrzynczak, Christian Walter, Vanessa Eusebio, Michal Motyl,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/35739
to look at the new patch set (#20).
Change subject: soc/intel/*: Serialize microcode updates for HT threads ......................................................................
soc/intel/*: Serialize microcode updates for HT threads
This change only affects MPinit related microcode updates and is a NOP for CPUs without HyperThreading enabled.
On HyperThreading-enabled platforms, update the microcode on a core serialized for threads running on that core. This prevents the "microcode: Update failed" error seen in the console log on boards with multiple cores and HyperThreading enabled.
The SDM and most of the BWGs suggest doing this.
Change-Id: I72804753e567a137a5648ca6950009fed332531b Signed-off-by: Patrick Rudolph patrick.rudolph@9elements.com --- M src/cpu/intel/common/common.h M src/cpu/intel/common/hyperthreading.c M src/cpu/intel/haswell/haswell_init.c M src/cpu/intel/microcode/microcode.c M src/cpu/intel/model_1067x/mp_init.c M src/cpu/intel/model_2065x/model_2065x_init.c M src/cpu/intel/model_206ax/model_206ax_init.c M src/include/cpu/intel/microcode.h M src/include/cpu/x86/lapic.h M src/soc/intel/alderlake/cpu.c M src/soc/intel/apollolake/cpu.c M src/soc/intel/baytrail/cpu.c M src/soc/intel/braswell/cpu.c M src/soc/intel/cannonlake/cpu.c M src/soc/intel/common/block/cpu/Kconfig M src/soc/intel/common/block/cpu/mp_init.c M src/soc/intel/common/block/sgx/sgx.c M src/soc/intel/denverton_ns/cpu.c M src/soc/intel/elkhartlake/cpu.c M src/soc/intel/icelake/cpu.c M src/soc/intel/jasperlake/cpu.c M src/soc/intel/skylake/chip.c M src/soc/intel/skylake/cpu.c M src/soc/intel/skylake/include/soc/ramstage.h M src/soc/intel/tigerlake/cpu.c M src/soc/intel/xeon_sp/cpx/cpu.c M src/soc/intel/xeon_sp/skx/cpu.c 27 files changed, 171 insertions(+), 60 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/39/35739/20
Attention is currently required from: Mariusz Szafrański, Suresh Bellampalli, Vanessa Eusebio, Angel Pons, Michal Motyl, Andrey Petrov. Hello build bot (Jenkins), Lijian Zhao, Mariusz Szafrański, Paul Menzel, Rizwan Qureshi, Angel Pons, Subrata Banik, Andrey Petrov, Aaron Durbin, Patrick Rudolph, Nico Huber, Suresh Bellampalli, Tim Wawrzynczak, Christian Walter, Vanessa Eusebio, Michal Motyl,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/35739
to look at the new patch set (#22).
Change subject: soc/intel/*: Serialize microcode updates for HT threads ......................................................................
soc/intel/*: Serialize microcode updates for HT threads
This change only affects MPinit related microcode updates and is a NOP for CPUs without HyperThreading enabled.
On HyperThreading-enabled platforms, update the microcode on a core serialized for threads running on that core. This prevents the "microcode: Update failed" error seen in the console log on boards with multiple cores and HyperThreading enabled.
The SDM and most of the BWGs suggest doing this.
Change-Id: I72804753e567a137a5648ca6950009fed332531b Signed-off-by: Patrick Rudolph patrick.rudolph@9elements.com --- M src/cpu/intel/common/common.h M src/cpu/intel/common/hyperthreading.c M src/cpu/intel/haswell/haswell_init.c M src/cpu/intel/microcode/microcode.c M src/cpu/intel/model_1067x/mp_init.c M src/cpu/intel/model_2065x/model_2065x_init.c M src/cpu/intel/model_206ax/model_206ax_init.c M src/include/cpu/intel/microcode.h M src/include/cpu/x86/lapic.h M src/soc/intel/alderlake/cpu.c M src/soc/intel/apollolake/cpu.c M src/soc/intel/baytrail/cpu.c M src/soc/intel/braswell/cpu.c M src/soc/intel/cannonlake/cpu.c M src/soc/intel/common/block/cpu/Kconfig M src/soc/intel/common/block/cpu/mp_init.c M src/soc/intel/common/block/sgx/sgx.c M src/soc/intel/denverton_ns/cpu.c M src/soc/intel/elkhartlake/cpu.c M src/soc/intel/icelake/cpu.c M src/soc/intel/jasperlake/cpu.c M src/soc/intel/skylake/chip.c M src/soc/intel/skylake/cpu.c M src/soc/intel/skylake/include/soc/ramstage.h M src/soc/intel/tigerlake/cpu.c M src/soc/intel/xeon_sp/cpx/cpu.c M src/soc/intel/xeon_sp/skx/cpu.c 27 files changed, 171 insertions(+), 55 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/39/35739/22
Attention is currently required from: Mariusz Szafrański, Suresh Bellampalli, Vanessa Eusebio, Angel Pons, Michal Motyl, Andrey Petrov. Hello build bot (Jenkins), Lijian Zhao, Mariusz Szafrański, Paul Menzel, Rizwan Qureshi, Angel Pons, Subrata Banik, Andrey Petrov, Aaron Durbin, Patrick Rudolph, Nico Huber, Suresh Bellampalli, Tim Wawrzynczak, Christian Walter, Vanessa Eusebio, Michal Motyl,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/35739
to look at the new patch set (#23).
Change subject: soc/intel/*: Serialize microcode updates for HT threads ......................................................................
soc/intel/*: Serialize microcode updates for HT threads
This change only affects MPinit related microcode updates and is a NOP for CPUs without HyperThreading enabled.
On HyperThreading-enabled platforms, update the microcode on a core serialized for threads running on that core. This prevents the "microcode: Update failed" error seen in the console log on boards with multiple cores and HyperThreading enabled.
The SDM and most of the BWGs suggest doing this.
Change-Id: I72804753e567a137a5648ca6950009fed332531b Signed-off-by: Patrick Rudolph patrick.rudolph@9elements.com --- M src/cpu/intel/common/common.h M src/cpu/intel/common/hyperthreading.c M src/cpu/intel/haswell/haswell_init.c M src/cpu/intel/microcode/microcode.c M src/cpu/intel/model_1067x/mp_init.c M src/cpu/intel/model_2065x/model_2065x_init.c M src/cpu/intel/model_206ax/model_206ax_init.c M src/include/cpu/intel/microcode.h M src/include/cpu/x86/lapic.h M src/soc/intel/alderlake/cpu.c M src/soc/intel/apollolake/cpu.c M src/soc/intel/baytrail/cpu.c M src/soc/intel/braswell/cpu.c M src/soc/intel/cannonlake/cpu.c M src/soc/intel/common/block/cpu/Kconfig M src/soc/intel/common/block/cpu/mp_init.c M src/soc/intel/common/block/sgx/sgx.c M src/soc/intel/denverton_ns/cpu.c M src/soc/intel/elkhartlake/cpu.c M src/soc/intel/icelake/cpu.c M src/soc/intel/jasperlake/cpu.c M src/soc/intel/skylake/chip.c M src/soc/intel/skylake/cpu.c M src/soc/intel/skylake/include/soc/ramstage.h M src/soc/intel/tigerlake/cpu.c M src/soc/intel/xeon_sp/cpx/cpu.c M src/soc/intel/xeon_sp/skx/cpu.c 27 files changed, 172 insertions(+), 55 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/39/35739/23
Attention is currently required from: Mariusz Szafrański, Suresh Bellampalli, Vanessa Eusebio, Angel Pons, Michal Motyl, Andrey Petrov. Hello build bot (Jenkins), Lijian Zhao, Mariusz Szafrański, Paul Menzel, Rizwan Qureshi, Angel Pons, Subrata Banik, Andrey Petrov, Aaron Durbin, Patrick Rudolph, Nico Huber, Suresh Bellampalli, Tim Wawrzynczak, Christian Walter, Vanessa Eusebio, Michal Motyl,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/35739
to look at the new patch set (#25).
Change subject: soc/intel/*: Serialize microcode updates for HT threads ......................................................................
soc/intel/*: Serialize microcode updates for HT threads
This change only affects MPinit related microcode updates and is a NOP for CPUs without HyperThreading enabled.
On HyperThreading-enabled platforms, update the microcode on a core serialized for threads running on that core. This prevents the "microcode: Update failed" error seen in the console log on boards with multiple cores and HyperThreading enabled.
The SDM and most of the BWGs suggest doing this.
Change-Id: I72804753e567a137a5648ca6950009fed332531b Signed-off-by: Patrick Rudolph patrick.rudolph@9elements.com --- M src/cpu/intel/common/common.h M src/cpu/intel/common/hyperthreading.c M src/cpu/intel/haswell/haswell_init.c M src/cpu/intel/microcode/microcode.c M src/cpu/intel/model_1067x/mp_init.c M src/cpu/intel/model_2065x/model_2065x_init.c M src/cpu/intel/model_206ax/model_206ax_init.c M src/include/cpu/intel/microcode.h M src/include/cpu/x86/lapic.h M src/soc/intel/alderlake/cpu.c M src/soc/intel/apollolake/cpu.c M src/soc/intel/baytrail/cpu.c M src/soc/intel/braswell/cpu.c M src/soc/intel/cannonlake/cpu.c M src/soc/intel/common/block/cpu/Kconfig M src/soc/intel/common/block/cpu/mp_init.c M src/soc/intel/common/block/sgx/sgx.c M src/soc/intel/denverton_ns/cpu.c M src/soc/intel/elkhartlake/cpu.c M src/soc/intel/icelake/cpu.c M src/soc/intel/jasperlake/cpu.c M src/soc/intel/skylake/chip.c M src/soc/intel/skylake/cpu.c M src/soc/intel/skylake/include/soc/ramstage.h M src/soc/intel/tigerlake/cpu.c M src/soc/intel/xeon_sp/cpx/cpu.c M src/soc/intel/xeon_sp/skx/cpu.c 27 files changed, 172 insertions(+), 55 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/39/35739/25
Attention is currently required from: Mariusz Szafrański, Suresh Bellampalli, Vanessa Eusebio, Angel Pons, Michal Motyl, Andrey Petrov. Hello build bot (Jenkins), Lijian Zhao, Mariusz Szafrański, Paul Menzel, Rizwan Qureshi, Angel Pons, Subrata Banik, Andrey Petrov, Aaron Durbin, Patrick Rudolph, Nico Huber, Suresh Bellampalli, Tim Wawrzynczak, Christian Walter, Vanessa Eusebio, Michal Motyl,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/35739
to look at the new patch set (#26).
Change subject: soc/intel/*: Serialize microcode updates for HT threads ......................................................................
soc/intel/*: Serialize microcode updates for HT threads
This change only affects MPinit related microcode updates and is a NOP for CPUs without HyperThreading enabled.
On HyperThreading-enabled platforms, update the microcode on a core serialized for threads running on that core. This prevents the "microcode: Update failed" error seen in the console log on boards with multiple cores and HyperThreading enabled.
The SDM and most of the BWGs suggest doing this.
Change-Id: I72804753e567a137a5648ca6950009fed332531b Signed-off-by: Patrick Rudolph patrick.rudolph@9elements.com --- M src/cpu/intel/common/common.h M src/cpu/intel/common/hyperthreading.c M src/cpu/intel/haswell/haswell_init.c M src/cpu/intel/microcode/microcode.c M src/cpu/intel/model_1067x/mp_init.c M src/cpu/intel/model_2065x/model_2065x_init.c M src/cpu/intel/model_206ax/model_206ax_init.c M src/include/cpu/intel/microcode.h M src/include/cpu/x86/lapic.h M src/soc/intel/alderlake/cpu.c M src/soc/intel/apollolake/cpu.c M src/soc/intel/baytrail/cpu.c M src/soc/intel/braswell/cpu.c M src/soc/intel/cannonlake/cpu.c M src/soc/intel/common/block/cpu/Kconfig M src/soc/intel/common/block/cpu/mp_init.c M src/soc/intel/common/block/sgx/sgx.c M src/soc/intel/denverton_ns/cpu.c M src/soc/intel/elkhartlake/cpu.c M src/soc/intel/icelake/cpu.c M src/soc/intel/jasperlake/cpu.c M src/soc/intel/skylake/chip.c M src/soc/intel/skylake/cpu.c M src/soc/intel/skylake/include/soc/ramstage.h M src/soc/intel/tigerlake/cpu.c M src/soc/intel/xeon_sp/cpx/cpu.c M src/soc/intel/xeon_sp/skx/cpu.c 27 files changed, 172 insertions(+), 55 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/39/35739/26
Attention is currently required from: Mariusz Szafrański, Suresh Bellampalli, Vanessa Eusebio, Angel Pons, Michal Motyl, Andrey Petrov. Hello build bot (Jenkins), Lijian Zhao, Mariusz Szafrański, Paul Menzel, Rizwan Qureshi, Angel Pons, Subrata Banik, Andrey Petrov, Aaron Durbin, Patrick Rudolph, Nico Huber, Suresh Bellampalli, Tim Wawrzynczak, Christian Walter, Vanessa Eusebio, Michal Motyl,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/35739
to look at the new patch set (#27).
Change subject: soc/intel/*: Serialize microcode updates for HT threads ......................................................................
soc/intel/*: Serialize microcode updates for HT threads
This change only affects MPinit related microcode updates and is a NOP for CPUs without HyperThreading enabled.
On HyperThreading-enabled platforms, update the microcode on a core serialized for threads running on that core. This prevents the "microcode: Update failed" error seen in the console log on boards with multiple cores and HyperThreading enabled.
The SDM and most of the BWGs suggest doing this.
Change-Id: I72804753e567a137a5648ca6950009fed332531b Signed-off-by: Patrick Rudolph patrick.rudolph@9elements.com --- M src/cpu/intel/common/common.h M src/cpu/intel/common/hyperthreading.c M src/cpu/intel/haswell/haswell_init.c M src/cpu/intel/microcode/microcode.c M src/cpu/intel/model_1067x/mp_init.c M src/cpu/intel/model_2065x/model_2065x_init.c M src/cpu/intel/model_206ax/model_206ax_init.c M src/include/cpu/intel/microcode.h M src/include/cpu/x86/lapic.h M src/soc/intel/alderlake/cpu.c M src/soc/intel/apollolake/cpu.c M src/soc/intel/baytrail/cpu.c M src/soc/intel/braswell/cpu.c M src/soc/intel/cannonlake/cpu.c M src/soc/intel/common/block/cpu/Kconfig M src/soc/intel/common/block/cpu/mp_init.c M src/soc/intel/common/block/sgx/sgx.c M src/soc/intel/denverton_ns/cpu.c M src/soc/intel/elkhartlake/cpu.c M src/soc/intel/icelake/cpu.c M src/soc/intel/jasperlake/cpu.c M src/soc/intel/skylake/chip.c M src/soc/intel/skylake/cpu.c M src/soc/intel/skylake/include/soc/ramstage.h M src/soc/intel/tigerlake/cpu.c M src/soc/intel/xeon_sp/cpx/cpu.c M src/soc/intel/xeon_sp/skx/cpu.c 27 files changed, 172 insertions(+), 55 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/39/35739/27
Attention is currently required from: Mariusz Szafrański, Suresh Bellampalli, Vanessa Eusebio, Angel Pons, Michal Motyl, Andrey Petrov. Hello build bot (Jenkins), Lijian Zhao, Mariusz Szafrański, Paul Menzel, Rizwan Qureshi, Angel Pons, Subrata Banik, Andrey Petrov, Aaron Durbin, Patrick Rudolph, Nico Huber, Suresh Bellampalli, Tim Wawrzynczak, Christian Walter, Vanessa Eusebio, Michal Motyl,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/35739
to look at the new patch set (#28).
Change subject: soc/intel/*: Serialize microcode updates for HT threads ......................................................................
soc/intel/*: Serialize microcode updates for HT threads
This change only affects MPinit related microcode updates and is a NOP for CPUs without HyperThreading enabled.
On HyperThreading-enabled platforms, update the microcode on a core serialized for threads running on that core. This prevents the "microcode: Update failed" error seen in the console log on boards with multiple cores and HyperThreading enabled.
The SDM and most of the BWGs suggest doing this.
Change-Id: I72804753e567a137a5648ca6950009fed332531b Signed-off-by: Patrick Rudolph patrick.rudolph@9elements.com --- M src/cpu/intel/common/common.h M src/cpu/intel/common/hyperthreading.c M src/cpu/intel/haswell/haswell_init.c M src/cpu/intel/microcode/microcode.c M src/cpu/intel/model_1067x/mp_init.c M src/cpu/intel/model_2065x/model_2065x_init.c M src/cpu/intel/model_206ax/model_206ax_init.c M src/include/cpu/intel/microcode.h M src/include/cpu/x86/lapic.h M src/soc/intel/alderlake/cpu.c M src/soc/intel/apollolake/cpu.c M src/soc/intel/baytrail/cpu.c M src/soc/intel/braswell/cpu.c M src/soc/intel/cannonlake/cpu.c M src/soc/intel/common/block/cpu/Kconfig M src/soc/intel/common/block/cpu/mp_init.c M src/soc/intel/common/block/sgx/sgx.c M src/soc/intel/denverton_ns/cpu.c M src/soc/intel/elkhartlake/cpu.c M src/soc/intel/icelake/cpu.c M src/soc/intel/jasperlake/cpu.c M src/soc/intel/skylake/chip.c M src/soc/intel/skylake/cpu.c M src/soc/intel/skylake/include/soc/ramstage.h M src/soc/intel/tigerlake/cpu.c M src/soc/intel/xeon_sp/cpx/cpu.c M src/soc/intel/xeon_sp/skx/cpu.c 27 files changed, 172 insertions(+), 55 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/39/35739/28
Attention is currently required from: Patrick Rudolph, Mariusz Szafrański, Suresh Bellampalli, Vanessa Eusebio, Angel Pons, Michal Motyl, Andrey Petrov. Hello build bot (Jenkins), Lijian Zhao, Mariusz Szafrański, Paul Menzel, Rizwan Qureshi, Angel Pons, Subrata Banik, Andrey Petrov, Aaron Durbin, Patrick Rudolph, Nico Huber, Suresh Bellampalli, Tim Wawrzynczak, Christian Walter, Vanessa Eusebio, Michal Motyl,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/35739
to look at the new patch set (#29).
Change subject: soc/intel/*: Serialize microcode updates for HT threads ......................................................................
soc/intel/*: Serialize microcode updates for HT threads
This change only affects MPinit related microcode updates and is a NOP for CPUs without HyperThreading enabled.
On HyperThreading-enabled platforms, update the microcode on a core serialized for threads running on that core. This prevents the "microcode: Update failed" error seen in the console log on boards with multiple cores and HyperThreading enabled.
The SDM and most of the BWGs suggest doing this.
Change-Id: I72804753e567a137a5648ca6950009fed332531b Signed-off-by: Patrick Rudolph patrick.rudolph@9elements.com --- M src/cpu/intel/common/common.h M src/cpu/intel/common/hyperthreading.c M src/cpu/intel/haswell/haswell_init.c M src/cpu/intel/microcode/microcode.c M src/cpu/intel/model_1067x/mp_init.c M src/cpu/intel/model_2065x/model_2065x_init.c M src/cpu/intel/model_206ax/model_206ax_init.c M src/include/cpu/intel/microcode.h M src/include/cpu/x86/lapic.h M src/soc/intel/alderlake/cpu.c M src/soc/intel/apollolake/cpu.c M src/soc/intel/baytrail/cpu.c M src/soc/intel/braswell/cpu.c M src/soc/intel/cannonlake/cpu.c M src/soc/intel/common/block/cpu/Kconfig M src/soc/intel/common/block/cpu/mp_init.c M src/soc/intel/common/block/sgx/sgx.c M src/soc/intel/denverton_ns/cpu.c M src/soc/intel/elkhartlake/cpu.c M src/soc/intel/icelake/cpu.c M src/soc/intel/jasperlake/cpu.c M src/soc/intel/skylake/chip.c M src/soc/intel/skylake/cpu.c M src/soc/intel/skylake/include/soc/ramstage.h M src/soc/intel/tigerlake/cpu.c M src/soc/intel/xeon_sp/cpx/cpu.c 26 files changed, 169 insertions(+), 55 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/39/35739/29
Attention is currently required from: Nico Huber, Mariusz Szafrański, Suresh Bellampalli, Vanessa Eusebio, Angel Pons, Michal Motyl, Andrey Petrov. Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35739 )
Change subject: soc/intel/*: Serialize microcode updates for HT threads ......................................................................
Patch Set 30:
(1 comment)
File src/cpu/intel/microcode/microcode.c:
https://review.coreboot.org/c/coreboot/+/35739/comment/3d235e2d_9a783e3b PS17, Line 63: Update just on the first CPU in the core.
Ufff, it's been a while, got to interpret my own comment here... […]
Rephrased the text
Attention is currently required from: Patrick Rudolph, Mariusz Szafrański, Suresh Bellampalli, Vanessa Eusebio, Angel Pons, Michal Motyl, Andrey Petrov. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35739 )
Change subject: soc/intel/*: Serialize microcode updates for HT threads ......................................................................
Patch Set 31:
(1 comment)
File src/cpu/intel/microcode/microcode.c:
https://review.coreboot.org/c/coreboot/+/35739/comment/7a7bfd91_d3b42420 PS17, Line 63: Update just on the first CPU in the core.
Rephrased the text
Done
Attention is currently required from: Patrick Rudolph, Mariusz Szafrański, Suresh Bellampalli, Vanessa Eusebio, Michal Motyl, Andrey Petrov. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35739 )
Change subject: soc/intel/*: Serialize microcode updates for HT threads ......................................................................
Patch Set 31: Code-Review+1
Attention is currently required from: Patrick Rudolph, Mariusz Szafrański, Suresh Bellampalli, Vanessa Eusebio, Michal Motyl, Andrey Petrov. Rocky Phagura has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35739 )
Change subject: soc/intel/*: Serialize microcode updates for HT threads ......................................................................
Patch Set 31: Code-Review+1
(1 comment)
Patchset:
PS31: Hi, Any updates on this patch?
Attention is currently required from: Patrick Rudolph, Mariusz Szafrański, Suresh Bellampalli, Vanessa Eusebio, Michal Motyl, Andrey Petrov. Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35739 )
Change subject: soc/intel/*: Serialize microcode updates for HT threads ......................................................................
Patch Set 31: Code-Review+2
Martin L Roth has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/35739?usp=email )
Change subject: soc/intel/*: Serialize microcode updates for HT threads ......................................................................
Abandoned
This patch has not been touched in over 12 months. Anyone who wants to take over work on this patch, please feel free to restore it and do any work needed to get it merged. If you create a new patch based on this work, please credit the original author.
-
Aaron Durbin (Code Review) -
Angel Pons (Code Review)
-
Arthur Heymans (Code Review)
-
Martin L Roth (Code Review)
-
Nico Huber (Code Review)
-
Patrick Rudolph (Code Review)
-
Paul Menzel (Code Review)
-
Rocky Phagura (Code Review)
-
Tim Wawrzynczak (Code Review)