Patrick Rudolph has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/34586 )
Change subject: soc/*: Report mp_init errors ......................................................................
soc/*: Report mp_init errors
Print or return errors if mp_run_on_all_cpus failed.
Change-Id: I740505e3b6a46ebb3311d0e6b9669e7f929f9ab9 Signed-off-by: Patrick Rudolph patrick.rudolph@9elements.com --- M src/soc/amd/common/block/pi/def_callouts.c M src/soc/intel/skylake/cpu.c 2 files changed, 10 insertions(+), 5 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/86/34586/1
diff --git a/src/soc/amd/common/block/pi/def_callouts.c b/src/soc/amd/common/block/pi/def_callouts.c index 6734b55..e45b5fd 100644 --- a/src/soc/amd/common/block/pi/def_callouts.c +++ b/src/soc/amd/common/block/pi/def_callouts.c @@ -220,8 +220,8 @@ agesadata.Func = Func; agesadata.Data = Data; agesadata.ConfigPtr = ConfigPtr; - mp_run_on_aps(callout_ap_entry, NULL, MP_RUN_ON_ALL_CPUS, - 100 * USECS_PER_MSEC); + if (mp_run_on_aps(callout_ap_entry, NULL, MP_RUN_ON_ALL_CPUS, 100 * USECS_PER_MSEC)) + return AGESA_ERROR;
return AGESA_SUCCESS; } diff --git a/src/soc/intel/skylake/cpu.c b/src/soc/intel/skylake/cpu.c index 2fd01b4..a558447 100644 --- a/src/soc/intel/skylake/cpu.c +++ b/src/soc/intel/skylake/cpu.c @@ -476,6 +476,8 @@
static void post_mp_init(void) { + int ret = 0; + /* Set Max Ratio */ cpu_set_max_ratio();
@@ -489,11 +491,14 @@ if (CONFIG(HAVE_SMI_HANDLER)) smm_lock();
- mp_run_on_all_cpus(vmx_configure, NULL, 2 * USECS_PER_MSEC); + ret |= mp_run_on_all_cpus(vmx_configure, NULL, 2 * USECS_PER_MSEC);
- mp_run_on_all_cpus(sgx_configure, NULL, 14 * USECS_PER_MSEC); + ret |= mp_run_on_all_cpus(sgx_configure, NULL, 14 * USECS_PER_MSEC);
- mp_run_on_all_cpus(fc_lock_configure, NULL, 2 * USECS_PER_MSEC); + ret |= mp_run_on_all_cpus(fc_lock_configure, NULL, 2 * USECS_PER_MSEC); + + if (ret) + printk(BIOS_CRIT, "CRITICAL ERROR: MP post init failed\n"); }
static const struct mp_ops mp_ops = {
Christian Walter has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34586 )
Change subject: soc/*: Report mp_init errors ......................................................................
Patch Set 1: Code-Review+1
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34586 )
Change subject: soc/*: Report mp_init errors ......................................................................
Patch Set 1:
Would it be better to add "CRITICAL ERROR:" to this https://review.coreboot.org/cgit/coreboot.git/tree/src/cpu/x86/mp_init.c?id=... so that it can benefit all callers?
Hello Patrick Rudolph, Christian Walter, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/34586
to look at the new patch set (#2).
Change subject: soc/*: Report mp_init errors ......................................................................
soc/*: Report mp_init errors
* Increase log level from ERR to CRITICAL in run_ap_work(). * Print or return errors if mp_run_on_all_cpus() failed.
Tested on Supermicro X11SSH-TF.
Change-Id: I740505e3b6a46ebb3311d0e6b9669e7f929f9ab9 Signed-off-by: Patrick Rudolph patrick.rudolph@9elements.com --- M src/cpu/x86/mp_init.c M src/soc/amd/common/block/pi/def_callouts.c M src/soc/intel/skylake/cpu.c 3 files changed, 11 insertions(+), 6 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/86/34586/2
Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34586 )
Change subject: soc/*: Report mp_init errors ......................................................................
Patch Set 2:
Done. Increased the error verbosity in run_ap_work().
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34586 )
Change subject: soc/*: Report mp_init errors ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/34586/2/src/soc/intel/skylake/cpu.c File src/soc/intel/skylake/cpu.c:
https://review.coreboot.org/c/coreboot/+/34586/2/src/soc/intel/skylake/cpu.c... PS2, Line 479: int ret = 0; Now that the error verbosity is increased in run_ap_work(), is the print in this function still required?
Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34586 )
Change subject: soc/*: Report mp_init errors ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/34586/2/src/soc/intel/skylake/cpu.c File src/soc/intel/skylake/cpu.c:
https://review.coreboot.org/c/coreboot/+/34586/2/src/soc/intel/skylake/cpu.c... PS2, Line 479: int ret = 0;
Now that the error verbosity is increased in run_ap_work(), is the print in this function still requ […]
Yes, all other users of mp_run_on_all_cpus() check for errors and report them or pass them to the caller. It will help to identify where mp_run_on_all_cpus() was called.
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34586 )
Change subject: soc/*: Report mp_init errors ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/34586/2/src/soc/amd/common/block/pi... File src/soc/amd/common/block/pi/def_callouts.c:
https://review.coreboot.org/c/coreboot/+/34586/2/src/soc/amd/common/block/pi... PS2, Line 237: mp_run_on_aps(callout_ap_entry, NULL, MP_RUN_ON_ALL_CPUS, : 100 * USECS_PER_MSEC); What about this?
Hello Patrick Rudolph, Christian Walter, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/34586
to look at the new patch set (#3).
Change subject: soc/*: Report mp_init errors ......................................................................
soc/*: Report mp_init errors
* Increase log level from ERR to CRITICAL in run_ap_work(). * Print or return errors if mp_run_on_all_cpus() failed.
Tested on Supermicro X11SSH-TF.
Change-Id: I740505e3b6a46ebb3311d0e6b9669e7f929f9ab9 Signed-off-by: Patrick Rudolph patrick.rudolph@9elements.com --- M src/cpu/x86/mp_init.c M src/soc/amd/common/block/pi/def_callouts.c M src/soc/intel/skylake/cpu.c 3 files changed, 13 insertions(+), 8 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/86/34586/3
Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34586 )
Change subject: soc/*: Report mp_init errors ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/c/coreboot/+/34586/2/src/soc/amd/common/block/pi... File src/soc/amd/common/block/pi/def_callouts.c:
https://review.coreboot.org/c/coreboot/+/34586/2/src/soc/amd/common/block/pi... PS2, Line 237: mp_run_on_aps(callout_ap_entry, NULL, MP_RUN_ON_ALL_CPUS, : 100 * USECS_PER_MSEC);
What about this?
Done
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34586 )
Change subject: soc/*: Report mp_init errors ......................................................................
Patch Set 3: Code-Review+2
Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34586 )
Change subject: soc/*: Report mp_init errors ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/c/coreboot/+/34586/2/src/soc/intel/skylake/cpu.c File src/soc/intel/skylake/cpu.c:
https://review.coreboot.org/c/coreboot/+/34586/2/src/soc/intel/skylake/cpu.c... PS2, Line 479: int ret = 0;
Yes, all other users of mp_run_on_all_cpus() check for errors and report them or pass them to the ca […]
Done
Patrick Rudolph has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/34586 )
Change subject: soc/*: Report mp_init errors ......................................................................
soc/*: Report mp_init errors
* Increase log level from ERR to CRITICAL in run_ap_work(). * Print or return errors if mp_run_on_all_cpus() failed.
Tested on Supermicro X11SSH-TF.
Change-Id: I740505e3b6a46ebb3311d0e6b9669e7f929f9ab9 Signed-off-by: Patrick Rudolph patrick.rudolph@9elements.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/34586 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Furquan Shaikh furquan@google.com --- M src/cpu/x86/mp_init.c M src/soc/amd/common/block/pi/def_callouts.c M src/soc/intel/skylake/cpu.c 3 files changed, 13 insertions(+), 8 deletions(-)
Approvals: build bot (Jenkins): Verified Furquan Shaikh: Looks good to me, approved
diff --git a/src/cpu/x86/mp_init.c b/src/cpu/x86/mp_init.c index 9528149..dbaf73f 100644 --- a/src/cpu/x86/mp_init.c +++ b/src/cpu/x86/mp_init.c @@ -910,7 +910,7 @@ return 0; } while (expire_us <= 0 || !stopwatch_expired(&sw));
- printk(BIOS_ERR, "AP call expired. %d/%d CPUs accepted.\n", + printk(BIOS_CRIT, "CIRTICAL ERROR: AP call expired. %d/%d CPUs accepted.\n", cpus_accepted, global_num_aps); return -1; } diff --git a/src/soc/amd/common/block/pi/def_callouts.c b/src/soc/amd/common/block/pi/def_callouts.c index 6734b55..299a98a 100644 --- a/src/soc/amd/common/block/pi/def_callouts.c +++ b/src/soc/amd/common/block/pi/def_callouts.c @@ -220,8 +220,8 @@ agesadata.Func = Func; agesadata.Data = Data; agesadata.ConfigPtr = ConfigPtr; - mp_run_on_aps(callout_ap_entry, NULL, MP_RUN_ON_ALL_CPUS, - 100 * USECS_PER_MSEC); + if (mp_run_on_aps(callout_ap_entry, NULL, MP_RUN_ON_ALL_CPUS, 100 * USECS_PER_MSEC)) + return AGESA_ERROR;
return AGESA_SUCCESS; } @@ -234,8 +234,8 @@ agesadata.Func = Func; agesadata.Data = Data; agesadata.ConfigPtr = ConfigPtr; - mp_run_on_aps(callout_ap_entry, NULL, MP_RUN_ON_ALL_CPUS, - 100 * USECS_PER_MSEC); + if (mp_run_on_aps(callout_ap_entry, NULL, MP_RUN_ON_ALL_CPUS, 100 * USECS_PER_MSEC)) + return AGESA_ERROR;
return AGESA_SUCCESS; } diff --git a/src/soc/intel/skylake/cpu.c b/src/soc/intel/skylake/cpu.c index cb0ceaa..eecb004 100644 --- a/src/soc/intel/skylake/cpu.c +++ b/src/soc/intel/skylake/cpu.c @@ -476,6 +476,8 @@
static void post_mp_init(void) { + int ret = 0; + /* Set Max Ratio */ cpu_set_max_ratio();
@@ -489,11 +491,14 @@ if (CONFIG(HAVE_SMI_HANDLER)) smm_lock();
- mp_run_on_all_cpus(vmx_configure, NULL, 2 * USECS_PER_MSEC); + ret |= mp_run_on_all_cpus(vmx_configure, NULL, 2 * USECS_PER_MSEC);
- mp_run_on_all_cpus(sgx_configure, NULL, 14 * USECS_PER_MSEC); + ret |= mp_run_on_all_cpus(sgx_configure, NULL, 14 * USECS_PER_MSEC);
- mp_run_on_all_cpus(fc_lock_configure, NULL, 2 * USECS_PER_MSEC); + ret |= mp_run_on_all_cpus(fc_lock_configure, NULL, 2 * USECS_PER_MSEC); + + if (ret) + printk(BIOS_CRIT, "CRITICAL ERROR: MP post init failed\n"); }
static const struct mp_ops mp_ops = {