[PATCH 10/22] x86: mp: Support APs waiting for instructions

[Wolfgang Wallner]

Hi Simon,


-----"Simon Glass" <sjg at chromium.org> schrieb: -----
> Betreff: [PATCH 10/22] x86: mp: Support APs waiting for instructions
> 
> At present the APs (non-boot CPUs) are inited once and then parked ready
> for the OS to use them. However in some cases we want to send new requests
> through, such as to change MTRRs and keep them consistent across CPUs.
> 
> Change the last state of the flight plan to go into a wait loop, accepting
> instructions from the main CPU.
> 
> Signed-off-by: Simon Glass <sjg at chromium.org>
> ---
> 
>  arch/x86/cpu/mp_init.c    | 99 ++++++++++++++++++++++++++++++++++++---
>  arch/x86/include/asm/mp.h | 11 +++++
>  2 files changed, 104 insertions(+), 6 deletions(-)
> 
> diff --git a/arch/x86/cpu/mp_init.c b/arch/x86/cpu/mp_init.c
> index ccb68b8b89b..c424f283807 100644
> --- a/arch/x86/cpu/mp_init.c
> +++ b/arch/x86/cpu/mp_init.c
> @@ -43,14 +43,36 @@ struct mp_flight_plan {
>  	struct mp_flight_record *records;
>  };
>  
> -static struct mp_flight_plan mp_info;
> -
>  struct cpu_map {
>  	struct udevice *dev;
>  	int apic_id;
>  	int err_code;
>  };
>  
> +struct mp_callback {
> +	/**
> +	 * func() - Function to call on the AP
> +	 *
> +	 * @arg: Argument to pass
> +	 */
> +	void (*func)(void *arg);
> +	void *arg;
> +	int logical_cpu_number;
> +};
> +
> +static struct mp_flight_plan mp_info;
> +
> +/*
> + * ap_callbacks - Callback mailbox array
> + *
> + * Array of callback, one entry for each available CPU, indexed by the CPU
> + * number, which is dev->req_seq. The entry for the main CPU is never used.
> + * When this is NULL, there is no pending work for the CPU to run. When
> + * non-NULL it points to the mp_callback structure. This is shared between all
> + * CPUs, so should only be written by the main CPU.
> + */
> +static struct mp_callback **ap_callbacks;
> +
>  static inline void barrier_wait(atomic_t *b)
>  {
>  	while (atomic_read(b) == 0)
> @@ -147,11 +169,9 @@ static void ap_init(unsigned int cpu_index)
>  	debug("AP: slot %d apic_id %x, dev %s\n", cpu_index, apic_id,
>  	      dev ? dev->name : "(apic_id not found)");
>  
> -	/* Walk the flight plan */
> +	/* Walk the flight plan, which never returns */
>  	ap_do_flight_plan(dev);
> -
> -	/* Park the AP */
> -	debug("parking\n");
> +	debug("Unexpected return\n");
>  done:
>  	stop_this_cpu();
>  }
> @@ -442,6 +462,68 @@ static int get_bsp(struct udevice **devp, int *cpu_countp)
>  	return dev->req_seq;
>  }
>  
> +static struct mp_callback *read_callback(struct mp_callback **slot)
> +{
> +	struct mp_callback *ret;
> +
> +	asm volatile ("mov	%1, %0\n"
> +		: "=r" (ret)
> +		: "m" (*slot)
> +		: "memory"
> +	);
> +	return ret;
> +}
> +
> +static void store_callback(struct mp_callback **slot, struct mp_callback *val)
> +{
> +	asm volatile ("mov	%1, %0\n"
> +		: "=m" (*slot)
> +		: "r" (val)
> +		: "memory"
> +	);
> +}

Descriptions for the two functions above would make them easier to understand.

> +
> +/**
> + * ap_wait_for_instruction() - Wait for and process requests from the main CPU
> + *
> + * This is called by APs (here, everything other than the main boot CPU) to
> + * await instructions. They arrive in the form of a function call and argument,
> + * which is then called. This uses a simple mailbox with atomic read/set
> + *
> + * @cpu: CPU that is waiting
> + * @unused: Optional argument provided by struct mp_flight_record, not used here
> + * @return Does not return
> + */
> +static int ap_wait_for_instruction(struct udevice *cpu, void *unused)
> +{
> +	struct mp_callback lcb;
> +	struct mp_callback **per_cpu_slot;
> +
> +	per_cpu_slot = &ap_callbacks[cpu->req_seq];
> +
> +	while (1) {
> +		struct mp_callback *cb = read_callback(per_cpu_slot);
> +
> +		if (!cb) {
> +			asm ("pause");
> +			continue;
> +		}
> +
> +		/* Copy to local variable before using the value */
> +		memcpy(&lcb, cb, sizeof(lcb));
> +		mfence();
> +		if (lcb.logical_cpu_number == MP_SELECT_ALL ||
> +		    lcb.logical_cpu_number == MP_SELECT_APS ||
> +		    cpu->req_seq == lcb.logical_cpu_number)
> +			lcb.func(lcb.arg);
> +
> +		/* Indicate we are finished */
> +		store_callback(per_cpu_slot, NULL);
> +	}
> +
> +	return 0;
> +}
> +
>  static int mp_init_cpu(struct udevice *cpu, void *unused)
>  {
>  	struct cpu_platdata *plat = dev_get_parent_platdata(cpu);
> @@ -454,6 +536,7 @@ static int mp_init_cpu(struct udevice *cpu, void *unused)
>  
>  static struct mp_flight_record mp_steps[] = {
>  	MP_FR_BLOCK_APS(mp_init_cpu, NULL, mp_init_cpu, NULL),
> +	MP_FR_BLOCK_APS(ap_wait_for_instruction, NULL, NULL, NULL),
>  };
>  
>  int mp_init(void)
> @@ -491,6 +574,10 @@ int mp_init(void)
>  	if (ret)
>  		log_warning("Warning: Device tree does not describe all CPUs. Extra ones will not be started correctly\n");
>  
> +	ap_callbacks = calloc(num_cpus, sizeof(struct mp_callback *));
> +	if (!ap_callbacks)
> +		return -ENOMEM;
> +
>  	/* Copy needed parameters so that APs have a reference to the plan */
>  	mp_info.num_records = ARRAY_SIZE(mp_steps);
>  	mp_info.records = mp_steps;
> diff --git a/arch/x86/include/asm/mp.h b/arch/x86/include/asm/mp.h
> index 94af819ad9a..41b1575f4be 100644
> --- a/arch/x86/include/asm/mp.h
> +++ b/arch/x86/include/asm/mp.h
> @@ -11,6 +11,17 @@
>  #include <asm/atomic.h>
>  #include <asm/cache.h>
>  
> +enum {
> +	/* Indicates that the function should run on all CPUs */
> +	MP_SELECT_ALL	= -1,
> +
> +	/* Run on boot CPUs */
> +	MP_SELECT_BSP	= -2,
> +
> +	/* Run on non-boot CPUs */
> +	MP_SELECT_APS	= -3,
> +};
> +
>  typedef int (*mp_callback_t)(struct udevice *cpu, void *arg);
>  
>  /*
> -- 
> 2.27.0.rc0.183.gde8f92d652-goog

regards, Wolfgang