Patrick Rudolph has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/34587 )
Change subject: soc/*: mp_run_on_all_cpus: Remove configurable timeout ......................................................................
soc/*: mp_run_on_all_cpus: Remove configurable timeout
Some timeouts given were too small when serial console is enabled due to its spinlock making code runtime worse with every AP present.
In addition we usually don't know how long specific code runs and how long ago it was sent to the APs.
Remove the timeout argument from mp_run_on_all_cpus and instead wait up to 1 second, to prevent possible crashing of secondary APs still processing the old job.
Tested on Supermicro X11SSH-TF.
Change-Id: I456be647b159f7a2ea7d94986a24424e56dcc8c4 Signed-off-by: Patrick Rudolph patrick.rudolph@9elements.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/34587 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Paul Menzel paulepanter@users.sourceforge.net Reviewed-by: Philipp Deppenwiese zaolin.daisuki@gmail.com --- M src/cpu/x86/mp_init.c M src/include/cpu/x86/mp.h M src/soc/amd/picasso/finalize.c M src/soc/amd/stoneyridge/finalize.c M src/soc/intel/apollolake/chip.c M src/soc/intel/apollolake/cpu.c M src/soc/intel/common/block/cpu/mp_init.c M src/soc/intel/skylake/cpu.c 8 files changed, 13 insertions(+), 12 deletions(-)
Approvals: build bot (Jenkins): Verified Paul Menzel: Looks good to me, but someone else must approve Philipp Deppenwiese: Looks good to me, approved
diff --git a/src/cpu/x86/mp_init.c b/src/cpu/x86/mp_init.c index 3658a5b..1755a9d 100644 --- a/src/cpu/x86/mp_init.c +++ b/src/cpu/x86/mp_init.c @@ -964,12 +964,13 @@ return run_ap_work(&lcb, expire_us); }
-int mp_run_on_all_cpus(void (*func)(void *), void *arg, long expire_us) +int mp_run_on_all_cpus(void (*func)(void *), void *arg) { /* Run on BSP first. */ func(arg);
- return mp_run_on_aps(func, arg, MP_RUN_ON_ALL_CPUS, expire_us); + /* For up to 1 second for AP to finish previous work. */ + return mp_run_on_aps(func, arg, MP_RUN_ON_ALL_CPUS, 1000 * USECS_PER_MSEC); }
int mp_park_aps(void) @@ -981,7 +982,7 @@ stopwatch_init(&sw);
ret = mp_run_on_aps(park_this_cpu, NULL, MP_RUN_ON_ALL_CPUS, - 250 * USECS_PER_MSEC); + 1000 * USECS_PER_MSEC);
duration_msecs = stopwatch_duration_msecs(&sw);
diff --git a/src/include/cpu/x86/mp.h b/src/include/cpu/x86/mp.h index c04252e..3ab45cd 100644 --- a/src/include/cpu/x86/mp.h +++ b/src/include/cpu/x86/mp.h @@ -137,7 +137,7 @@ long expire_us);
/* Like mp_run_on_aps() but also runs func on BSP. */ -int mp_run_on_all_cpus(void (*func)(void *), void *arg, long expire_us); +int mp_run_on_all_cpus(void (*func)(void *), void *arg);
/* * Park all APs to prepare for OS boot. This is handled automatically diff --git a/src/soc/amd/picasso/finalize.c b/src/soc/amd/picasso/finalize.c index 5a4627a..0ec7bd9 100644 --- a/src/soc/amd/picasso/finalize.c +++ b/src/soc/amd/picasso/finalize.c @@ -44,7 +44,7 @@ int r; printk(BIOS_SPEW, "Lock SMM configuration\n");
- r = mp_run_on_all_cpus(per_core_finalize, NULL, 10 * USECS_PER_MSEC); + r = mp_run_on_all_cpus(per_core_finalize, NULL); if (r) printk(BIOS_WARNING, "Failed to finalize all cores\n"); } diff --git a/src/soc/amd/stoneyridge/finalize.c b/src/soc/amd/stoneyridge/finalize.c index 5a4627a..0ec7bd9 100644 --- a/src/soc/amd/stoneyridge/finalize.c +++ b/src/soc/amd/stoneyridge/finalize.c @@ -44,7 +44,7 @@ int r; printk(BIOS_SPEW, "Lock SMM configuration\n");
- r = mp_run_on_all_cpus(per_core_finalize, NULL, 10 * USECS_PER_MSEC); + r = mp_run_on_all_cpus(per_core_finalize, NULL); if (r) printk(BIOS_WARNING, "Failed to finalize all cores\n"); } diff --git a/src/soc/intel/apollolake/chip.c b/src/soc/intel/apollolake/chip.c index 361e6a4..b69f9ee 100644 --- a/src/soc/intel/apollolake/chip.c +++ b/src/soc/intel/apollolake/chip.c @@ -764,7 +764,7 @@ static void drop_privilege_all(void) { /* Drop privilege level on all the CPUs */ - if (mp_run_on_all_cpus(&cpu_enable_untrusted_mode, NULL, 1000) < 0) + if (mp_run_on_all_cpus(&cpu_enable_untrusted_mode, NULL) < 0) printk(BIOS_ERR, "failed to enable untrusted mode\n"); }
diff --git a/src/soc/intel/apollolake/cpu.c b/src/soc/intel/apollolake/cpu.c index 9fbcc07..8aa0804 100644 --- a/src/soc/intel/apollolake/cpu.c +++ b/src/soc/intel/apollolake/cpu.c @@ -256,7 +256,7 @@ smm_southbridge_enable(PWRBTN_EN | GBL_EN);
if (CONFIG(SOC_INTEL_COMMON_BLOCK_SGX)) - mp_run_on_all_cpus(sgx_configure, NULL, 2000); + mp_run_on_all_cpus(sgx_configure, NULL); }
static const struct mp_ops mp_ops = { diff --git a/src/soc/intel/common/block/cpu/mp_init.c b/src/soc/intel/common/block/cpu/mp_init.c index d941ab2..e0cee17 100644 --- a/src/soc/intel/common/block/cpu/mp_init.c +++ b/src/soc/intel/common/block/cpu/mp_init.c @@ -159,7 +159,7 @@ if (CONFIG(USE_INTEL_FSP_MP_INIT)) return;
- if (mp_run_on_all_cpus(&wrapper_x86_setup_mtrrs, NULL, 1000) < 0) + if (mp_run_on_all_cpus(&wrapper_x86_setup_mtrrs, NULL) < 0) printk(BIOS_ERR, "MTRR programming failure\n");
x86_mtrr_check(); diff --git a/src/soc/intel/skylake/cpu.c b/src/soc/intel/skylake/cpu.c index eecb004..383a3bd 100644 --- a/src/soc/intel/skylake/cpu.c +++ b/src/soc/intel/skylake/cpu.c @@ -491,11 +491,11 @@ if (CONFIG(HAVE_SMI_HANDLER)) smm_lock();
- ret |= mp_run_on_all_cpus(vmx_configure, NULL, 2 * USECS_PER_MSEC); + ret |= mp_run_on_all_cpus(vmx_configure, NULL);
- ret |= mp_run_on_all_cpus(sgx_configure, NULL, 14 * USECS_PER_MSEC); + ret |= mp_run_on_all_cpus(sgx_configure, NULL);
- ret |= mp_run_on_all_cpus(fc_lock_configure, NULL, 2 * USECS_PER_MSEC); + ret |= mp_run_on_all_cpus(fc_lock_configure, NULL);
if (ret) printk(BIOS_CRIT, "CRITICAL ERROR: MP post init failed\n");