Felix Held has submitted this change. ( https://review.coreboot.org/c/coreboot/+/58494 )
Change subject: cpu/x86/mp_init: use cb_err as status return type in remaining functions ......................................................................
cpu/x86/mp_init: use cb_err as status return type in remaining functions
Using cb_err as return type of mp_run_on_aps, mp_run_on_all_aps, mp_run_on_all_cpus and mp_park_aps clarifies the meaning of the different return values. This patch also adds the types.h include that provides the definition of the cb_err enum and checks the return value of all 4 functions listed above against the enum values instead of either checking if it's non-zero or less than zero to handle the error case.
Signed-off-by: Felix Held felix-coreboot@felixheld.de Change-Id: I4b3f03415a041d3ec9cd0e102980e53868b004b0 Reviewed-on: https://review.coreboot.org/c/coreboot/+/58494 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Raul Rangel rrangel@chromium.org --- M src/cpu/x86/mp_init.c M src/drivers/intel/fsp2_0/ppi/mp_service_ppi.c M src/drivers/ocp/dmi/smbios.c M src/include/cpu/x86/mp.h M src/soc/amd/common/block/cpu/smm/finalize.c M src/soc/amd/common/pi/def_callouts.c M src/soc/intel/apollolake/chip.c M src/soc/intel/common/block/cpu/mp_init.c M src/soc/intel/skylake/cpu.c 9 files changed, 35 insertions(+), 28 deletions(-)
Approvals: build bot (Jenkins): Verified Raul Rangel: Looks good to me, approved
diff --git a/src/cpu/x86/mp_init.c b/src/cpu/x86/mp_init.c index 258b9df..0624176 100644 --- a/src/cpu/x86/mp_init.c +++ b/src/cpu/x86/mp_init.c @@ -974,17 +974,16 @@ } }
-int mp_run_on_aps(void (*func)(void *), void *arg, int logical_cpu_num, +enum cb_err mp_run_on_aps(void (*func)(void *), void *arg, int logical_cpu_num, long expire_us) { struct mp_callback lcb = { .func = func, .arg = arg, .logical_cpu_number = logical_cpu_num}; - /* TODO: Remove this return value translation after changing the return type of - mp_run_on_aps to enum cb_err */ - return run_ap_work(&lcb, expire_us) == CB_SUCCESS ? 0 : -1; + return run_ap_work(&lcb, expire_us); }
-int mp_run_on_all_aps(void (*func)(void *), void *arg, long expire_us, bool run_parallel) +enum cb_err mp_run_on_all_aps(void (*func)(void *), void *arg, long expire_us, + bool run_parallel) { int ap_index, bsp_index;
@@ -999,14 +998,14 @@ /* skip if BSP */ if (ap_index == bsp_index) continue; - if (mp_run_on_aps(func, arg, ap_index, expire_us)) + if (mp_run_on_aps(func, arg, ap_index, expire_us) != CB_SUCCESS) return CB_ERR; }
return CB_SUCCESS; }
-int mp_run_on_all_cpus(void (*func)(void *), void *arg) +enum cb_err mp_run_on_all_cpus(void (*func)(void *), void *arg) { /* Run on BSP first. */ func(arg); @@ -1015,10 +1014,10 @@ return mp_run_on_aps(func, arg, MP_RUN_ON_ALL_CPUS, 1000 * USECS_PER_MSEC); }
-int mp_park_aps(void) +enum cb_err mp_park_aps(void) { struct stopwatch sw; - int ret; + enum cb_err ret; long duration_msecs;
stopwatch_init(&sw); @@ -1028,7 +1027,7 @@
duration_msecs = stopwatch_duration_msecs(&sw);
- if (!ret) + if (ret == CB_SUCCESS) printk(BIOS_DEBUG, "%s done after %ld msecs.\n", __func__, duration_msecs); else diff --git a/src/drivers/intel/fsp2_0/ppi/mp_service_ppi.c b/src/drivers/intel/fsp2_0/ppi/mp_service_ppi.c index a795c72..3f1e83d 100644 --- a/src/drivers/intel/fsp2_0/ppi/mp_service_ppi.c +++ b/src/drivers/intel/fsp2_0/ppi/mp_service_ppi.c @@ -9,6 +9,7 @@ #include <fsp/ppi/mp_service_ppi.h> #include <intelblocks/cpulib.h> #include <intelblocks/mp_init.h> +#include <types.h>
#define BSP_CPU_SLOT 0
@@ -72,7 +73,8 @@ if (procedure == NULL) return FSP_INVALID_PARAMETER;
- if (mp_run_on_all_aps((void *)procedure, argument, timeout_usec, !run_serial)) { + if (mp_run_on_all_aps((void *)procedure, argument, timeout_usec, !run_serial) != + CB_SUCCESS) { printk(BIOS_DEBUG, "%s: Exit with Failure\n", __func__); return FSP_NOT_STARTED; } @@ -94,7 +96,7 @@
/* Run on APs */ if (mp_run_on_aps((void *)procedure, argument, - MP_RUN_ON_ALL_CPUS, timeout_usec)) { + MP_RUN_ON_ALL_CPUS, timeout_usec) != CB_SUCCESS) { printk(BIOS_DEBUG, "%s: Exit with Failure\n", __func__); return FSP_NOT_STARTED; } @@ -118,7 +120,7 @@ return FSP_INVALID_PARAMETER;
if (mp_run_on_aps((void *)procedure, argument, - processor_number, timeout_usec)) { + processor_number, timeout_usec) != CB_SUCCESS) { printk(BIOS_DEBUG, "%s: Exit with Failure\n", __func__); return FSP_NOT_STARTED; } diff --git a/src/drivers/ocp/dmi/smbios.c b/src/drivers/ocp/dmi/smbios.c index d0ef115..51c008d 100644 --- a/src/drivers/ocp/dmi/smbios.c +++ b/src/drivers/ocp/dmi/smbios.c @@ -11,6 +11,7 @@ #include <soc/soc_util.h> #include <soc/util.h> #include <smbios.h> +#include <types.h>
#include "ocp_dmi.h"
@@ -244,7 +245,8 @@ if (CONFIG_MAX_SOCKET == 2 && CONFIG(PARALLEL_MP_AP_WORK)) { /* Read the last CPU MSR */ if (mp_run_on_aps(read_remote_ppin, (void *)&xeon_sp_ppin[1], - get_platform_thread_count() - 1, 100 * USECS_PER_MSEC)) { + get_platform_thread_count() - 1, 100 * USECS_PER_MSEC) != + CB_SUCCESS) { printk(BIOS_ERR, "Failed to read remote PPIN.\n"); t->count = smbios_add_oem_string(t->eos, TBF); } else { diff --git a/src/include/cpu/x86/mp.h b/src/include/cpu/x86/mp.h index aa6289c..7ed82dd 100644 --- a/src/include/cpu/x86/mp.h +++ b/src/include/cpu/x86/mp.h @@ -117,26 +117,25 @@ * Input parameter expire_us <= 0 to specify an infinite timeout. * logical_cpu_num = MP_RUN_ON_ALL_CPUS to execute function over all cores (BSP * + APs) else specified AP number using logical_cpu_num. - * - * All functions return < 0 on error, 0 on success. */ -int mp_run_on_aps(void (*func)(void *), void *arg, int logical_cpu_num, +enum cb_err mp_run_on_aps(void (*func)(void *), void *arg, int logical_cpu_num, long expire_us);
/* * Runs func on all APs excluding BSP, with a provision to run calls in parallel * or serially per AP. */ -int mp_run_on_all_aps(void (*func)(void *), void *arg, long expire_us, bool run_parallel); +enum cb_err mp_run_on_all_aps(void (*func)(void *), void *arg, long expire_us, + bool run_parallel);
/* Like mp_run_on_aps() but also runs func on BSP. */ -int mp_run_on_all_cpus(void (*func)(void *), void *arg); +enum cb_err mp_run_on_all_cpus(void (*func)(void *), void *arg);
/* * Park all APs to prepare for OS boot. This is handled automatically * by the coreboot infrastructure. */ -int mp_park_aps(void); +enum cb_err mp_park_aps(void);
/* * SMM helpers to use with initializing CPUs. diff --git a/src/soc/amd/common/block/cpu/smm/finalize.c b/src/soc/amd/common/block/cpu/smm/finalize.c index 2795100..ec975be 100644 --- a/src/soc/amd/common/block/cpu/smm/finalize.c +++ b/src/soc/amd/common/block/cpu/smm/finalize.c @@ -7,6 +7,7 @@ #include <bootstate.h> #include <console/console.h> #include <amdblocks/acpi.h> +#include <types.h>
static void per_core_finalize(void *unused) { @@ -31,7 +32,7 @@ { printk(BIOS_SPEW, "Lock SMM configuration\n");
- if (mp_run_on_all_cpus(per_core_finalize, NULL)) + if (mp_run_on_all_cpus(per_core_finalize, NULL) != CB_SUCCESS) printk(BIOS_WARNING, "Failed to finalize all cores\n"); }
diff --git a/src/soc/amd/common/pi/def_callouts.c b/src/soc/amd/common/pi/def_callouts.c index 2ee7f46..0d799b9 100644 --- a/src/soc/amd/common/pi/def_callouts.c +++ b/src/soc/amd/common/pi/def_callouts.c @@ -10,6 +10,7 @@ #include <amdblocks/agesawrapper_call.h> #include <amdblocks/reset.h> #include <soc/southbridge.h> +#include <types.h>
#if ENV_BOOTBLOCK const BIOS_CALLOUT_STRUCT BiosCallouts[] = { @@ -205,7 +206,8 @@ agesadata.Func = Func; agesadata.Data = Data; agesadata.ConfigPtr = ConfigPtr; - if (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) != + CB_SUCCESS) return AGESA_ERROR;
return AGESA_SUCCESS; @@ -219,7 +221,8 @@ agesadata.Func = Func; agesadata.Data = Data; agesadata.ConfigPtr = ConfigPtr; - if (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) != + CB_SUCCESS) return AGESA_ERROR;
return AGESA_SUCCESS; diff --git a/src/soc/intel/apollolake/chip.c b/src/soc/intel/apollolake/chip.c index 4cc29e6..057138c 100644 --- a/src/soc/intel/apollolake/chip.c +++ b/src/soc/intel/apollolake/chip.c @@ -35,6 +35,7 @@ #include <timer.h> #include <soc/ramstage.h> #include <soc/soc_chip.h> +#include <types.h>
#include "chip.h"
@@ -704,7 +705,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) < 0) + if (mp_run_on_all_cpus(&cpu_enable_untrusted_mode, NULL) != CB_SUCCESS) printk(BIOS_ERR, "failed to enable untrusted mode\n"); }
diff --git a/src/soc/intel/common/block/cpu/mp_init.c b/src/soc/intel/common/block/cpu/mp_init.c index 4a429ab..acd15a8 100644 --- a/src/soc/intel/common/block/cpu/mp_init.c +++ b/src/soc/intel/common/block/cpu/mp_init.c @@ -157,7 +157,7 @@ /* Ensure to re-program all MTRRs based on DRAM resource settings */ static void post_cpus_init(void *unused) { - if (mp_run_on_all_cpus(&wrapper_x86_setup_mtrrs, NULL) < 0) + if (mp_run_on_all_cpus(&wrapper_x86_setup_mtrrs, NULL) != CB_SUCCESS) 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 d88dd36..86e8e52 100644 --- a/src/soc/intel/skylake/cpu.c +++ b/src/soc/intel/skylake/cpu.c @@ -178,14 +178,14 @@ if (CONFIG(HAVE_SMI_HANDLER)) smm_lock();
- if (mp_run_on_all_cpus(vmx_configure, NULL)) + if (mp_run_on_all_cpus(vmx_configure, NULL) != CB_SUCCESS) failure = true;
if (CONFIG(SOC_INTEL_COMMON_BLOCK_SGX_ENABLE)) - if (mp_run_on_all_cpus(sgx_configure, NULL)) + if (mp_run_on_all_cpus(sgx_configure, NULL) != CB_SUCCESS) failure = true;
- if (mp_run_on_all_cpus(fc_lock_configure, NULL)) + if (mp_run_on_all_cpus(fc_lock_configure, NULL) != CB_SUCCESS) failure = true;
if (failure)