Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1604719 > unrolled thread

[v2 PATCH 0/4] powernv:idle: Fixes for CPU-Hotplug on POWER DD1.0

Back to article view | Back to linux.kernel

Contents

  [v2 PATCH 0/4] powernv:idle: Fixes for CPU-Hotplug on POWER DD1.0 "Gautham R. Shenoy" <ego@linux.vnet.ibm.com> - 2017-03-20 17:00 +0100
    [v2 PATCH 3/4] powernv:idle: Don't override default/deepest directly in kernel "Gautham R. Shenoy" <ego@linux.vnet.ibm.com> - 2017-03-20 17:00 +0100
      Re: [v2 PATCH 3/4] powernv:idle: Don't override default/deepest  directly in kernel Nicholas Piggin <npiggin@gmail.com> - 2017-03-20 18:00 +0100
        Re: [v2 PATCH 3/4] powernv:idle: Don't override default/deepest  directly in kernel Gautham R Shenoy <ego@linux.vnet.ibm.com> - 2017-03-22 06:50 +0100

#1604719 — [v2 PATCH 0/4] powernv:idle: Fixes for CPU-Hotplug on POWER DD1.0

From: "Gautham R. Shenoy" <ego@linux.vnet.ibm.com>

Hi,

This is the second version of the patchset containing the fixes to
make CPU-Hotplug working on correctly on POWER9 DD1 systems.

The earlier version of this patchset can be found here:
https://lkml.org/lkml/2017/3/13/46

This patch addresses the feedback provided by Nicholas Piggin. The key
changes from the earlier version are:

- Move the piece of code in powernv/smp.c::pnv_smp_cpu_kill_self() which
  transitions the CPU to the deepest available platform idle state to a
  new function named pnv_cpu_offline() in powernv/idle.c. The rationale
  behind this code movement is that the data required to determine the
  deepest available platform state resides in powernv/idle.c.

- Adds a more descriptive warning describing the consequences of no
  suitable default/deepest stop states being available.

- Update the commit log for "powernv:Recover correct PACA on wakeup
  from a stop on P9 DD1" describing the problem that it is trying to
  solve.  Also ensure that on P9 DD1, NVGPRs are restored from the
  stack on the way out by setting the NAPSTATELOST in paca.

I have retained Nicholas Piggin's Reviewed-by tag for the second patch
in this series (was first patch in the previous series) even though
the file containing the code is different.

There are four patches in the series.

- The first patch moves the platform-idle code invoked when a CPU offlined from powernv/smp.c to powernv/idle.c

- The second adds a fallback mechanism for CPU-Hotplug when no
  platform idle state is available.

- The third patch ensures that the kernel doesn't use any stop state
  that is not exposed by the firmware.

- The fourth patch adds a recovery framework for correctly recovering
  paca pointer of the thread waking up from a stop.

These patches are based on v4.11-rc3.

The patches have been tested with stop1 (ESL=EC=1) as the
deepest-state entered into during CPU-Hotplug.

Gautham R. Shenoy (4):
  powernv: Move CPU-Offline idle state invocation from smp.c to idle.c
  powernv:smp: Add busy-wait loop as fall back for CPU-Hotplug
  powernv:idle: Don't override default/deepest directly in kernel
  powernv: Recover correct PACA on wakeup from a stop on P9 DD1

 arch/powerpc/include/asm/cpuidle.h       |  1 +
 arch/powerpc/include/asm/paca.h          |  5 ++
 arch/powerpc/kernel/asm-offsets.c        |  1 +
 arch/powerpc/kernel/idle_book3s.S        | 49 +++++++++++++++++-
 arch/powerpc/platforms/powernv/idle.c    | 88 ++++++++++++++++++++++++++------
 arch/powerpc/platforms/powernv/powernv.h |  2 -
 arch/powerpc/platforms/powernv/smp.c     | 18 +------
 7 files changed, 129 insertions(+), 35 deletions(-)

-- 
1.9.4

[toc] | [next] | [standalone]

#1604720 — [v2 PATCH 3/4] powernv:idle: Don't override default/deepest directly in kernel

From: "Gautham R. Shenoy" <ego@linux.vnet.ibm.com>

Currently during idle-init on power9, if we don't find suitable stop
states in the device tree that can be used as the
default_stop/deepest_stop, we set stop0 (ESL=1,EC=1) as the default
stop state psscr to be used by power9_idle and deepest stop state
which is used by CPU-Hotplug.

However, if the platform firmware has not configured or enabled a stop
state, the kernel should not make any assumptions and fallback to a
default choice.

If the kernel uses a stop state that is not configured by the platform
firmware, it may lead to further failures which should be avoided.

In this patch, we modify the init code to ensure that the kernel uses
only the stop states exposed by the firmware through the device
tree. When a suitable default stop state isn't found, we disable
ppc_md.power_save for power9. Similarly, when a suitable
deepest_stop_state is not found in the device tree exported by the
firmware, fall back to the default busy-wait loop in the CPU-Hotplug
code.

[Changelog written with inputs from svaidy@linux.vnet.ibm.com]

Signed-off-by: Gautham R. Shenoy <ego@linux.vnet.ibm.com>
---
 arch/powerpc/platforms/powernv/idle.c | 35 ++++++++++++++++++-----------------
 1 file changed, 18 insertions(+), 17 deletions(-)

diff --git a/arch/powerpc/platforms/powernv/idle.c b/arch/powerpc/platforms/powernv/idle.c
index f335e0f..63ade78 100644
--- a/arch/powerpc/platforms/powernv/idle.c
+++ b/arch/powerpc/platforms/powernv/idle.c
@@ -147,7 +147,6 @@ u32 pnv_get_supported_cpuidle_states(void)
 }
 EXPORT_SYMBOL_GPL(pnv_get_supported_cpuidle_states);
 
-
 static void pnv_fastsleep_workaround_apply(void *info)
 
 {
@@ -241,8 +240,9 @@ static DEVICE_ATTR(fastsleep_workaround_applyonce, 0600,
  * The default stop state that will be used by ppc_md.power_save
  * function on platforms that support stop instruction.
  */
-u64 pnv_default_stop_val;
-u64 pnv_default_stop_mask;
+static u64 pnv_default_stop_val;
+static u64 pnv_default_stop_mask;
+static bool default_stop_found;
 
 /*
  * Used for ppc_md.power_save which needs a function with no parameters
@@ -262,8 +262,9 @@ static void power9_idle(void)
  * psscr value and mask of the deepest stop idle state.
  * Used when a cpu is offlined.
  */
-u64 pnv_deepest_stop_psscr_val;
-u64 pnv_deepest_stop_psscr_mask;
+static u64 pnv_deepest_stop_psscr_val;
+static u64 pnv_deepest_stop_psscr_mask;
+static bool deepest_stop_found;
 
 /*
  * pnv_cpu_offline: A function that puts the CPU into the deepest
@@ -275,7 +276,7 @@ unsigned long pnv_cpu_offline(unsigned int cpu)
 
 	u32 idle_states = pnv_get_supported_cpuidle_states();
 
-	if (cpu_has_feature(CPU_FTR_ARCH_300)) {
+	if (cpu_has_feature(CPU_FTR_ARCH_300) && deepest_stop_found) {
 		srr1 = power9_idle_stop(pnv_deepest_stop_psscr_val,
 					pnv_deepest_stop_psscr_mask);
 	} else if (idle_states & OPAL_PM_WINKLE_ENABLED) {
@@ -385,7 +386,6 @@ static int __init pnv_power9_idle_init(struct device_node *np, u32 *flags,
 	u32 *residency_ns = NULL;
 	u64 max_residency_ns = 0;
 	int rc = 0, i;
-	bool default_stop_found = false, deepest_stop_found = false;
 
 	psscr_val = kcalloc(dt_idle_states, sizeof(*psscr_val), GFP_KERNEL);
 	psscr_mask = kcalloc(dt_idle_states, sizeof(*psscr_mask), GFP_KERNEL);
@@ -465,21 +465,24 @@ static int __init pnv_power9_idle_init(struct device_node *np, u32 *flags,
 		}
 	}
 
-	if (!default_stop_found) {
-		pnv_default_stop_val = PSSCR_HV_DEFAULT_VAL;
-		pnv_default_stop_mask = PSSCR_HV_DEFAULT_MASK;
-		pr_warn("Setting default stop psscr val=0x%016llx,mask=0x%016llx\n",
+	if (unlikely(!default_stop_found)) {
+		pr_warn("cpuidle-powernv: No suitable default stop state found. Disabling platform idle.\n");
+	} else {
+		ppc_md.power_save = power9_idle;
+		pr_info("cpuidle-powernv: Default stop: psscr = 0x%016llx,mask=0x%016llx\n",
 			pnv_default_stop_val, pnv_default_stop_mask);
 	}
 
-	if (!deepest_stop_found) {
-		pnv_deepest_stop_psscr_val = PSSCR_HV_DEFAULT_VAL;
-		pnv_deepest_stop_psscr_mask = PSSCR_HV_DEFAULT_MASK;
-		pr_warn("Setting default stop psscr val=0x%016llx,mask=0x%016llx\n",
+	if (unlikely(!deepest_stop_found)) {
+		pr_warn("cpuidle-powernv: No suitable stop state for CPU-Hotplug. Offlined CPUs will busy wait");
+	} else {
+		pr_info("cpuidle-powernv: Deepest stop: psscr = 0x%016llx,mask=0x%016llx\n",
 			pnv_deepest_stop_psscr_val,
 			pnv_deepest_stop_psscr_mask);
 	}
 
+	pr_info("cpuidle-powernv: Requested Level (RL) value of first deep stop = 0x%llx\n",
+		pnv_first_deep_stop_state);
 out:
 	kfree(psscr_val);
 	kfree(psscr_mask);
@@ -559,8 +562,6 @@ static int __init pnv_init_idle_states(void)
 
 	if (supported_cpuidle_states & OPAL_PM_NAP_ENABLED)
 		ppc_md.power_save = power7_idle;
-	else if (supported_cpuidle_states & OPAL_PM_STOP_INST_FAST)
-		ppc_md.power_save = power9_idle;
 
 out:
 	return 0;
-- 
1.9.4

[toc] | [prev] | [next] | [standalone]

#1604803 — Re: [v2 PATCH 3/4] powernv:idle: Don't override default/deepest directly in kernel

On Mon, 20 Mar 2017 21:24:17 +0530
"Gautham R. Shenoy" <ego@linux.vnet.ibm.com> wrote:

> From: "Gautham R. Shenoy" <ego@linux.vnet.ibm.com>
> 
> Currently during idle-init on power9, if we don't find suitable stop
> states in the device tree that can be used as the
> default_stop/deepest_stop, we set stop0 (ESL=1,EC=1) as the default
> stop state psscr to be used by power9_idle and deepest stop state
> which is used by CPU-Hotplug.
> 
> However, if the platform firmware has not configured or enabled a stop
> state, the kernel should not make any assumptions and fallback to a
> default choice.
> 
> If the kernel uses a stop state that is not configured by the platform
> firmware, it may lead to further failures which should be avoided.
> 
> In this patch, we modify the init code to ensure that the kernel uses
> only the stop states exposed by the firmware through the device
> tree. When a suitable default stop state isn't found, we disable
> ppc_md.power_save for power9. Similarly, when a suitable
> deepest_stop_state is not found in the device tree exported by the
> firmware, fall back to the default busy-wait loop in the CPU-Hotplug
> code.
> 
> [Changelog written with inputs from svaidy@linux.vnet.ibm.com]
> 
> Signed-off-by: Gautham R. Shenoy <ego@linux.vnet.ibm.com>
> ---
>  arch/powerpc/platforms/powernv/idle.c | 35 ++++++++++++++++++-----------------
>  1 file changed, 18 insertions(+), 17 deletions(-)
> 
> diff --git a/arch/powerpc/platforms/powernv/idle.c b/arch/powerpc/platforms/powernv/idle.c
> index f335e0f..63ade78 100644
> --- a/arch/powerpc/platforms/powernv/idle.c
> +++ b/arch/powerpc/platforms/powernv/idle.c
> @@ -147,7 +147,6 @@ u32 pnv_get_supported_cpuidle_states(void)
>  }
>  EXPORT_SYMBOL_GPL(pnv_get_supported_cpuidle_states);
>  
> -
>  static void pnv_fastsleep_workaround_apply(void *info)
>  
>  {
> @@ -241,8 +240,9 @@ static DEVICE_ATTR(fastsleep_workaround_applyonce, 0600,
>   * The default stop state that will be used by ppc_md.power_save
>   * function on platforms that support stop instruction.
>   */
> -u64 pnv_default_stop_val;
> -u64 pnv_default_stop_mask;
> +static u64 pnv_default_stop_val;
> +static u64 pnv_default_stop_mask;
> +static bool default_stop_found;
>  
>  /*
>   * Used for ppc_md.power_save which needs a function with no parameters
> @@ -262,8 +262,9 @@ static void power9_idle(void)
>   * psscr value and mask of the deepest stop idle state.
>   * Used when a cpu is offlined.
>   */
> -u64 pnv_deepest_stop_psscr_val;
> -u64 pnv_deepest_stop_psscr_mask;
> +static u64 pnv_deepest_stop_psscr_val;
> +static u64 pnv_deepest_stop_psscr_mask;
> +static bool deepest_stop_found;

Aha you have made them static. Nitpick withdrawn :)

The log messages look good now.

Reviewed-by: Nicholas Piggin <npiggin@gmail.com>

[toc] | [prev] | [next] | [standalone]

#1606216 — Re: [v2 PATCH 3/4] powernv:idle: Don't override default/deepest directly in kernel

Hi,

On Tue, Mar 21, 2017 at 02:39:34AM +1000, Nicholas Piggin wrote:
> > @@ -241,8 +240,9 @@ static DEVICE_ATTR(fastsleep_workaround_applyonce, 0600,
> >   * The default stop state that will be used by ppc_md.power_save
> >   * function on platforms that support stop instruction.
> >   */
> > -u64 pnv_default_stop_val;
> > -u64 pnv_default_stop_mask;
> > +static u64 pnv_default_stop_val;
> > +static u64 pnv_default_stop_mask;
> > +static bool default_stop_found;
> >  
> >  /*
> >   * Used for ppc_md.power_save which needs a function with no parameters
> > @@ -262,8 +262,9 @@ static void power9_idle(void)
> >   * psscr value and mask of the deepest stop idle state.
> >   * Used when a cpu is offlined.
> >   */
> > -u64 pnv_deepest_stop_psscr_val;
> > -u64 pnv_deepest_stop_psscr_mask;
> > +static u64 pnv_deepest_stop_psscr_val;
> > +static u64 pnv_deepest_stop_psscr_mask;
> > +static bool deepest_stop_found;
> 
> Aha you have made them static. Nitpick withdrawn :)
> 
> The log messages look good now.
> 
> Reviewed-by: Nicholas Piggin <npiggin@gmail.com>
> 
Thanks!

--
Thanks and Regards
gautham.

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web