Re: [PATCH v1 38/43] x86: mp: Allow use of mp_run_on_cpus() without MP

2020-07-07 Thread Simon Glass
Hi Bin,

On Tue, 30 Jun 2020 at 02:40, Bin Meng  wrote:
>
> Hi Simon,
>
> On Mon, Jun 15, 2020 at 11:58 AM Simon Glass  wrote:
> >
> > At present if MP is not enabled (e.g. booting from coreboot) the 'mtrr'
> > command does not work correctly. It is not easy to make it work for all
> > CPUs, since coreboot has halted them and we would need to start them up
> > again, but it is easy enough to make them work on the boot CPU.
> >
> > Update the code to avoid assuming that the MP init routine has completed,
> > so that this can work.
> >
> > Signed-off-by: Simon Glass 
> > ---
> >
> >  arch/x86/cpu/mp_init.c | 12 
> >  1 file changed, 8 insertions(+), 4 deletions(-)
> >
> > diff --git a/arch/x86/cpu/mp_init.c b/arch/x86/cpu/mp_init.c
> > index ef33a38017..c0ae24686e 100644
> > --- a/arch/x86/cpu/mp_init.c
> > +++ b/arch/x86/cpu/mp_init.c
> > @@ -454,7 +454,7 @@ static int get_bsp(struct udevice **devp, int 
> > *cpu_countp)
> > if (cpu_countp)
> > *cpu_countp = ret;
> >
> > -   return dev->req_seq;
> > +   return dev->req_seq >= 0 ? dev->req_seq : 0;
> >  }
> >
> >  static struct mp_callback *read_callback(struct mp_callback **slot)
> > @@ -589,9 +589,6 @@ int mp_run_on_cpus(int cpu_select, mp_run_func func, 
> > void *arg)
> > int num_cpus;
> > int ret;
> >
> > -   if (!(gd->flags & GD_FLG_SMP_INIT))
> > -   return -ENXIO;
> > -
> > ret = get_bsp(, _cpus);
> > if (ret < 0)
> > return log_msg_ret("bsp", ret);
> > @@ -601,6 +598,13 @@ int mp_run_on_cpus(int cpu_select, mp_run_func func, 
> > void *arg)
> > func(arg);
> > }
> >
> > +   if (!(gd->flags & GD_FLG_SMP_INIT)) {
> > +   /* Allow use of this function on the BSP only */
> > +   if (cpu_select == MP_SELECT_BSP || !cpu_select)
>
> This assumes 0 is the BSP cpu number?

We until i SMP is inited, then it is assumed to be CPU 0, yes.

I ended up pulling this fix into the MTRR series as I had to fix the
minnowmax bug.

Regards,
Simon


Re: [PATCH v1 38/43] x86: mp: Allow use of mp_run_on_cpus() without MP

2020-06-30 Thread Bin Meng
Hi Simon,

On Mon, Jun 15, 2020 at 11:58 AM Simon Glass  wrote:
>
> At present if MP is not enabled (e.g. booting from coreboot) the 'mtrr'
> command does not work correctly. It is not easy to make it work for all
> CPUs, since coreboot has halted them and we would need to start them up
> again, but it is easy enough to make them work on the boot CPU.
>
> Update the code to avoid assuming that the MP init routine has completed,
> so that this can work.
>
> Signed-off-by: Simon Glass 
> ---
>
>  arch/x86/cpu/mp_init.c | 12 
>  1 file changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/arch/x86/cpu/mp_init.c b/arch/x86/cpu/mp_init.c
> index ef33a38017..c0ae24686e 100644
> --- a/arch/x86/cpu/mp_init.c
> +++ b/arch/x86/cpu/mp_init.c
> @@ -454,7 +454,7 @@ static int get_bsp(struct udevice **devp, int *cpu_countp)
> if (cpu_countp)
> *cpu_countp = ret;
>
> -   return dev->req_seq;
> +   return dev->req_seq >= 0 ? dev->req_seq : 0;
>  }
>
>  static struct mp_callback *read_callback(struct mp_callback **slot)
> @@ -589,9 +589,6 @@ int mp_run_on_cpus(int cpu_select, mp_run_func func, void 
> *arg)
> int num_cpus;
> int ret;
>
> -   if (!(gd->flags & GD_FLG_SMP_INIT))
> -   return -ENXIO;
> -
> ret = get_bsp(, _cpus);
> if (ret < 0)
> return log_msg_ret("bsp", ret);
> @@ -601,6 +598,13 @@ int mp_run_on_cpus(int cpu_select, mp_run_func func, 
> void *arg)
> func(arg);
> }
>
> +   if (!(gd->flags & GD_FLG_SMP_INIT)) {
> +   /* Allow use of this function on the BSP only */
> +   if (cpu_select == MP_SELECT_BSP || !cpu_select)

This assumes 0 is the BSP cpu number?

> +   return 0;
> +   return -ENXIO;
> +   }
> +
> /* Allow up to 1 second for all APs to finish */
> ret = run_ap_work(, dev, num_cpus, 1000 /* ms */);
> if (ret)
> --

Regards,
Bin