[v3,14/25] x86: mp: Park CPUs before running the OS

Message ID 20200706033741.2169374-13-sjg@chromium.org
State Superseded
Headers show
Series
  • x86: Enhance MTRR functionality to support multiple CPUs
Related show

Commit Message

Simon Glass July 6, 2020, 3:37 a.m.
With the new MP features the CPUs are no-longer parked when the OS is run.
Fix this by calling a special function to park them, just before the OS is
started.

Signed-off-by: Simon Glass <sjg at chromium.org>
Reviewed-by: Wolfgang Wallner <wolfgang.wallner at br-automation.com>
---

Changes in v3:
- Update the comment for mp_park_aps()

 arch/x86/cpu/cpu.c        |  5 +++++
 arch/x86/cpu/mp_init.c    | 18 ++++++++++++++++++
 arch/x86/include/asm/mp.h | 17 +++++++++++++++++
 3 files changed, 40 insertions(+)

Comments

Bin Meng July 7, 2020, 8:26 a.m. | #1
Hi Simon,

On Mon, Jul 6, 2020 at 11:37 AM Simon Glass <sjg at chromium.org> wrote:
>
> With the new MP features the CPUs are no-longer parked when the OS is run.
> Fix this by calling a special function to park them, just before the OS is
> started.
>
> Signed-off-by: Simon Glass <sjg at chromium.org>
> Reviewed-by: Wolfgang Wallner <wolfgang.wallner at br-automation.com>
> ---
>
> Changes in v3:
> - Update the comment for mp_park_aps()
>
>  arch/x86/cpu/cpu.c        |  5 +++++
>  arch/x86/cpu/mp_init.c    | 18 ++++++++++++++++++
>  arch/x86/include/asm/mp.h | 17 +++++++++++++++++
>  3 files changed, 40 insertions(+)
>
> diff --git a/arch/x86/cpu/cpu.c b/arch/x86/cpu/cpu.c
> index d0720fb7fb..baa7dae172 100644
> --- a/arch/x86/cpu/cpu.c
> +++ b/arch/x86/cpu/cpu.c
> @@ -66,6 +66,11 @@ static const char *const x86_vendor_name[] = {
>
>  int __weak x86_cleanup_before_linux(void)
>  {
> +       int ret;
> +
> +       ret = mp_park_aps();
> +       if (ret)
> +               return log_msg_ret("park", ret);
>         bootstage_stash((void *)CONFIG_BOOTSTAGE_STASH_ADDR,
>                         CONFIG_BOOTSTAGE_STASH_SIZE);
>
> diff --git a/arch/x86/cpu/mp_init.c b/arch/x86/cpu/mp_init.c
> index bcb7513084..0bf325ae88 100644
> --- a/arch/x86/cpu/mp_init.c
> +++ b/arch/x86/cpu/mp_init.c
> @@ -651,6 +651,24 @@ int mp_run_on_cpus(int cpu_select, mp_run_func func, void *arg)
>         return 0;
>  }
>
> +static void park_this_cpu(void *unused)
> +{
> +       stop_this_cpu();
> +}
> +
> +int mp_park_aps(void)
> +{
> +       unsigned long start;
> +       int ret;
> +
> +       start = get_timer(0);
> +       ret = mp_run_on_cpus(MP_SELECT_APS, park_this_cpu, NULL);
> +       if (ret)
> +               return ret;
> +
> +       return get_timer(start);

I don't think this API should return time used for parking CPU. The
get_timer() logic should be moved to its caller instead.

> +}
> +
>  int mp_init(void)
>  {
>         int num_aps, num_cpus;
> diff --git a/arch/x86/include/asm/mp.h b/arch/x86/include/asm/mp.h
> index 3f1d70663d..4acce55b8c 100644
> --- a/arch/x86/include/asm/mp.h
> +++ b/arch/x86/include/asm/mp.h
> @@ -106,6 +106,15 @@ typedef void (*mp_run_func)(void *arg);
>   * @return 0 on success, -ve on error
>   */
>  int mp_run_on_cpus(int cpu_select, mp_run_func func, void *arg);
> +
> +/**
> + * mp_park_aps() - Park the APs ready for the OS
> + *
> + * This halts all CPUs except the main one, ready for the OS to use them
> + *
> + * @return time taken to park the APs on success (in microseconds), -ve on error
> + */
> +int mp_park_aps(void);
>  #else
>  static inline int mp_run_on_cpus(int cpu_select, mp_run_func func, void *arg)
>  {
> @@ -114,6 +123,14 @@ static inline int mp_run_on_cpus(int cpu_select, mp_run_func func, void *arg)
>
>         return 0;
>  }
> +
> +static inline int mp_park_aps(void)
> +{
> +       /* No APs to park */
> +
> +       return 0;
> +}
> +
>  #endif
>
>  #endif /* _X86_MP_H_ */

Regards,
Bin
Simon Glass July 8, 2020, 1:55 a.m. | #2
Hi Bin,

On Tue, 7 Jul 2020 at 02:26, Bin Meng <bmeng.cn at gmail.com> wrote:
>
> Hi Simon,
>
> On Mon, Jul 6, 2020 at 11:37 AM Simon Glass <sjg at chromium.org> wrote:
> >
> > With the new MP features the CPUs are no-longer parked when the OS is run.
> > Fix this by calling a special function to park them, just before the OS is
> > started.
> >
> > Signed-off-by: Simon Glass <sjg at chromium.org>
> > Reviewed-by: Wolfgang Wallner <wolfgang.wallner at br-automation.com>
> > ---
> >
> > Changes in v3:
> > - Update the comment for mp_park_aps()
> >
> >  arch/x86/cpu/cpu.c        |  5 +++++
> >  arch/x86/cpu/mp_init.c    | 18 ++++++++++++++++++
> >  arch/x86/include/asm/mp.h | 17 +++++++++++++++++
> >  3 files changed, 40 insertions(+)

OK. Sorry but I missed this.

If there are no other comments I can just update this patch.

Regards,
Simon
Simon Glass July 11, 2020, 4:09 p.m. | #3
Hi Bin,

On Tue, 7 Jul 2020 at 19:55, Simon Glass <sjg at chromium.org> wrote:
>
> Hi Bin,
>
> On Tue, 7 Jul 2020 at 02:26, Bin Meng <bmeng.cn at gmail.com> wrote:
> >
> > Hi Simon,
> >
> > On Mon, Jul 6, 2020 at 11:37 AM Simon Glass <sjg at chromium.org> wrote:
> > >
> > > With the new MP features the CPUs are no-longer parked when the OS is run.
> > > Fix this by calling a special function to park them, just before the OS is
> > > started.
> > >
> > > Signed-off-by: Simon Glass <sjg at chromium.org>
> > > Reviewed-by: Wolfgang Wallner <wolfgang.wallner at br-automation.com>
> > > ---
> > >
> > > Changes in v3:
> > > - Update the comment for mp_park_aps()
> > >
> > >  arch/x86/cpu/cpu.c        |  5 +++++
> > >  arch/x86/cpu/mp_init.c    | 18 ++++++++++++++++++
> > >  arch/x86/include/asm/mp.h | 17 +++++++++++++++++
> > >  3 files changed, 40 insertions(+)
>
> OK. Sorry but I missed this.
>
> If there are no other comments I can just update this patch.
>

OK I sent a new version of that patch.

Regards,
Simon

Patch

diff --git a/arch/x86/cpu/cpu.c b/arch/x86/cpu/cpu.c
index d0720fb7fb..baa7dae172 100644
--- a/arch/x86/cpu/cpu.c
+++ b/arch/x86/cpu/cpu.c
@@ -66,6 +66,11 @@  static const char *const x86_vendor_name[] = {
 
 int __weak x86_cleanup_before_linux(void)
 {
+	int ret;
+
+	ret = mp_park_aps();
+	if (ret)
+		return log_msg_ret("park", ret);
 	bootstage_stash((void *)CONFIG_BOOTSTAGE_STASH_ADDR,
 			CONFIG_BOOTSTAGE_STASH_SIZE);
 
diff --git a/arch/x86/cpu/mp_init.c b/arch/x86/cpu/mp_init.c
index bcb7513084..0bf325ae88 100644
--- a/arch/x86/cpu/mp_init.c
+++ b/arch/x86/cpu/mp_init.c
@@ -651,6 +651,24 @@  int mp_run_on_cpus(int cpu_select, mp_run_func func, void *arg)
 	return 0;
 }
 
+static void park_this_cpu(void *unused)
+{
+	stop_this_cpu();
+}
+
+int mp_park_aps(void)
+{
+	unsigned long start;
+	int ret;
+
+	start = get_timer(0);
+	ret = mp_run_on_cpus(MP_SELECT_APS, park_this_cpu, NULL);
+	if (ret)
+		return ret;
+
+	return get_timer(start);
+}
+
 int mp_init(void)
 {
 	int num_aps, num_cpus;
diff --git a/arch/x86/include/asm/mp.h b/arch/x86/include/asm/mp.h
index 3f1d70663d..4acce55b8c 100644
--- a/arch/x86/include/asm/mp.h
+++ b/arch/x86/include/asm/mp.h
@@ -106,6 +106,15 @@  typedef void (*mp_run_func)(void *arg);
  * @return 0 on success, -ve on error
  */
 int mp_run_on_cpus(int cpu_select, mp_run_func func, void *arg);
+
+/**
+ * mp_park_aps() - Park the APs ready for the OS
+ *
+ * This halts all CPUs except the main one, ready for the OS to use them
+ *
+ * @return time taken to park the APs on success (in microseconds), -ve on error
+ */
+int mp_park_aps(void);
 #else
 static inline int mp_run_on_cpus(int cpu_select, mp_run_func func, void *arg)
 {
@@ -114,6 +123,14 @@  static inline int mp_run_on_cpus(int cpu_select, mp_run_func func, void *arg)
 
 	return 0;
 }
+
+static inline int mp_park_aps(void)
+{
+	/* No APs to park */
+
+	return 0;
+}
+
 #endif
 
 #endif /* _X86_MP_H_ */