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>

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
+	return run_ap_work(&lcb, expire_us);
-	   mp_run_on_aps to enum cb_err */
-	return run_ap_work(&lcb, expire_us) == CB_SUCCESS ? 0 : -1;
 }
 
-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

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 @@ 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;
 	}

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 @@ 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 {

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.

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 @@ 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");
 }
 

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 @@ 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;

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 @@ 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");
 }
 

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();

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)