Marshall Dawson has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/34759 )
Change subject: cpu/x86 mp_init: Add option for AMD INIT SIPI sequence ......................................................................
cpu/x86 mp_init: Add option for AMD INIT SIPI sequence
The common code adheres to the Intel requirement of bringing up the cores with INIT SIPI SIPI. This sequence is tolerated on some AMD AMD CPUs but fails on others. Add a way to skip the second SIPI.
TEST=Mock up on grunt and verify no errors BUG=b:138919564
Change-Id: I201869003ddc7d04d332cd5734ac6d63979d89e0 Signed-off-by: Marshall Dawson marshalldawson3rd@gmail.com --- M src/cpu/x86/Kconfig M src/cpu/x86/mp_init.c 2 files changed, 11 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/59/34759/1
diff --git a/src/cpu/x86/Kconfig b/src/cpu/x86/Kconfig index caee5db..a8cf54d 100644 --- a/src/cpu/x86/Kconfig +++ b/src/cpu/x86/Kconfig @@ -155,6 +155,14 @@ This option informs the MTRR code to use the RdMem and WrMem fields in the fixed MTRR MSRs.
+config X86_AMD_INIT_SIPI + bool + default n + help + This option limits the number of SIPI signals sent during during the + common AP setup. Intel documentation specifies an INIT SIPI SIPI + sequence, however this doesn't work on some AMD platforms. + config MIRROR_PAYLOAD_TO_RAM_BEFORE_LOADING def_bool n help diff --git a/src/cpu/x86/mp_init.c b/src/cpu/x86/mp_init.c index 9528149..a1d368d 100644 --- a/src/cpu/x86/mp_init.c +++ b/src/cpu/x86/mp_init.c @@ -482,6 +482,9 @@ /* Wait for CPUs to check in up to 200 us. */ wait_for_aps(num_aps, ap_count, 200 /* us */, 15 /* us */);
+ if (CONFIG(X86_AMD_INIT_SIPI)) + return 0; + /* Send 2nd SIPI */ if ((lapic_read(LAPIC_ICR) & LAPIC_ICR_BUSY)) { printk(BIOS_DEBUG, "Waiting for ICR not to be busy...");
Martin Roth has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34759 )
Change subject: cpu/x86 mp_init: Add option for AMD INIT SIPI sequence ......................................................................
Patch Set 2: Code-Review+2
Martin Roth has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/34759 )
Change subject: cpu/x86 mp_init: Add option for AMD INIT SIPI sequence ......................................................................
cpu/x86 mp_init: Add option for AMD INIT SIPI sequence
The common code adheres to the Intel requirement of bringing up the cores with INIT SIPI SIPI. This sequence is tolerated on some AMD AMD CPUs but fails on others. Add a way to skip the second SIPI.
TEST=Mock up on grunt and verify no errors BUG=b:138919564
Change-Id: I201869003ddc7d04d332cd5734ac6d63979d89e0 Signed-off-by: Marshall Dawson marshalldawson3rd@gmail.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/34759 Reviewed-by: Martin Roth martinroth@google.com Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M src/cpu/x86/Kconfig M src/cpu/x86/mp_init.c 2 files changed, 11 insertions(+), 0 deletions(-)
Approvals: build bot (Jenkins): Verified Martin Roth: Looks good to me, approved
diff --git a/src/cpu/x86/Kconfig b/src/cpu/x86/Kconfig index caee5db..a8cf54d 100644 --- a/src/cpu/x86/Kconfig +++ b/src/cpu/x86/Kconfig @@ -155,6 +155,14 @@ This option informs the MTRR code to use the RdMem and WrMem fields in the fixed MTRR MSRs.
+config X86_AMD_INIT_SIPI + bool + default n + help + This option limits the number of SIPI signals sent during during the + common AP setup. Intel documentation specifies an INIT SIPI SIPI + sequence, however this doesn't work on some AMD platforms. + config MIRROR_PAYLOAD_TO_RAM_BEFORE_LOADING def_bool n help diff --git a/src/cpu/x86/mp_init.c b/src/cpu/x86/mp_init.c index dbaf73f..3658a5b 100644 --- a/src/cpu/x86/mp_init.c +++ b/src/cpu/x86/mp_init.c @@ -482,6 +482,9 @@ /* Wait for CPUs to check in up to 200 us. */ wait_for_aps(num_aps, ap_count, 200 /* us */, 15 /* us */);
+ if (CONFIG(X86_AMD_INIT_SIPI)) + return 0; + /* Send 2nd SIPI */ if ((lapic_read(LAPIC_ICR) & LAPIC_ICR_BUSY)) { printk(BIOS_DEBUG, "Waiting for ICR not to be busy...");
-
Marshall Dawson (Code Review) -
Martin Roth (Code Review)