From 82faefb339a0853dd49f10bafd2c4f5ca1723fb3 Mon Sep 17 00:00:00 2001 From: Felix Held Date: Wed, 20 Oct 2021 20:50:58 +0200 Subject: [PATCH] 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 Change-Id: I4b3f03415a041d3ec9cd0e102980e53868b004b0 Reviewed-on: https://review.coreboot.org/c/coreboot/+/58494 Tested-by: build bot (Jenkins) Reviewed-by: Raul Rangel --- src/cpu/x86/mp_init.c | 19 +++++++++---------- src/drivers/intel/fsp2_0/ppi/mp_service_ppi.c | 8 +++++--- src/drivers/ocp/dmi/smbios.c | 4 +++- src/include/cpu/x86/mp.h | 11 +++++------ src/soc/amd/common/block/cpu/smm/finalize.c | 3 ++- src/soc/amd/common/pi/def_callouts.c | 7 +++++-- src/soc/intel/apollolake/chip.c | 3 ++- src/soc/intel/common/block/cpu/mp_init.c | 2 +- src/soc/intel/skylake/cpu.c | 6 +++--- 9 files changed, 35 insertions(+), 28 deletions(-) diff --git a/src/cpu/x86/mp_init.c b/src/cpu/x86/mp_init.c index 258b9df9da..0624176b75 100644 --- a/src/cpu/x86/mp_init.c +++ b/src/cpu/x86/mp_init.c @@ -974,17 +974,16 @@ static void ap_wait_for_instruction(void) } } -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 @@ int mp_run_on_all_aps(void (*func)(void *), void *arg, long expire_us, bool run_ /* 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 @@ int mp_run_on_all_cpus(void (*func)(void *), void *arg) 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 @@ int mp_park_aps(void) 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 a795c722a6..3f1e83d541 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 #include #include +#include #define BSP_CPU_SLOT 0 @@ -72,7 +73,8 @@ efi_return_status_t mp_startup_all_aps(efi_ap_procedure procedure, 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 @@ efi_return_status_t mp_startup_all_cpus(efi_ap_procedure procedure, /* 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 @@ efi_return_status_t mp_startup_this_ap(efi_ap_procedure procedure, 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 d0ef11508d..51c008d405 100644 --- a/src/drivers/ocp/dmi/smbios.c +++ b/src/drivers/ocp/dmi/smbios.c @@ -11,6 +11,7 @@ #include #include #include +#include #include "ocp_dmi.h" @@ -244,7 +245,8 @@ void ocp_oem_smbios_strings(struct device *dev, struct smbios_type11 *t) 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 aa6289c18c..7ed82dda42 100644 --- a/src/include/cpu/x86/mp.h +++ b/src/include/cpu/x86/mp.h @@ -117,26 +117,25 @@ enum { * 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 2795100d86..ec975bea27 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 #include #include +#include static void per_core_finalize(void *unused) { @@ -31,7 +32,7 @@ static void finalize_cores(void) { 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 2ee7f46056..0d799b94d2 100644 --- a/src/soc/amd/common/pi/def_callouts.c +++ b/src/soc/amd/common/pi/def_callouts.c @@ -10,6 +10,7 @@ #include #include #include +#include #if ENV_BOOTBLOCK const BIOS_CALLOUT_STRUCT BiosCallouts[] = { @@ -205,7 +206,8 @@ AGESA_STATUS agesa_RunFuncOnAp(uint32_t Func, uintptr_t Data, void *ConfigPtr) 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 @@ AGESA_STATUS agesa_RunFcnOnAllAps(uint32_t Func, uintptr_t Data, 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 4cc29e6e21..057138c803 100644 --- a/src/soc/intel/apollolake/chip.c +++ b/src/soc/intel/apollolake/chip.c @@ -35,6 +35,7 @@ #include #include #include +#include #include "chip.h" @@ -704,7 +705,7 @@ struct chip_operations soc_intel_apollolake_ops = { 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 4a429ab8f8..acd15a8f2c 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 @@ static void wrapper_x86_setup_mtrrs(void *unused) /* 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 d88dd36ad6..86e8e52797 100644 --- a/src/soc/intel/skylake/cpu.c +++ b/src/soc/intel/skylake/cpu.c @@ -178,14 +178,14 @@ static void post_mp_init(void) 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)