cpuidle/powernv: Interface to handle idle-stop versioning

RFC In review Pratik Rajesh Sampat 3 participants 8 messages Mar 4, 2020 – Apr 12, 2020

Subsystems (2)

PowerPC CPU idle time management framework

lore

Share & tools

lore

Pratik Rajesh Sampat

author

2020-03-04 16:01 UTC #0 lore raw

cover cpuidle/powernv: Interface to handle idle-stop versioning

A design patch series illuminates the idea of handling different versions of idle-stop, the properties they support and the quirks that need to be handled before entering or after exiting stop.

It also adds a functionality to identify firmware-enabled-stop and set the according bits to encapsulate the support and corresponding handling

Corresponding RFC skiboot patch: https://lists.ozlabs.org/pipermail/skiboot/2020-March/016552.html

Pratik Rajesh Sampat (3):
Interface for an idle-stop dependency structure Demonstration of handling an idle-stop quirk version Introduce capability for firmware-enabled-stop

 arch/powerpc/include/asm/processor.h  | 19 +++++++++++++++++
 arch/powerpc/kernel/dt_cpu_ftrs.c     | 30 +++++++++++++++++++++++++++
 arch/powerpc/platforms/powernv/idle.c | 25 ++++++++++++++++++----
 drivers/cpuidle/cpuidle-powernv.c     |  3 ++-
 4 files changed, 72 insertions(+), 5 deletions(-)

Signature · 1 line

2.24.1

Pratik Rajesh Sampat

author

2020-03-04 16:01 UTC #1 lore raw

RFC 1/3 Interface for an idle-stop dependency structure

Design patch to introduce the idea of having a dependency structure for idle-stop. The structure encapsulates the following:
1. Bitmask for version of idle-stop
2. Bitmask for propterties like ENABLE/DISABLE
3. Function pointer which helps handle how the stop must be invoked

The commit lays a foundation for other idle-stop versions to be added and handled cleanly based on their specified requirments. Currently it handles the existing "idle-stop" version by setting the discovery bits and the function pointer.

Signed-off-by Pratik Rajesh Sampat <[email protected]>

---
 arch/powerpc/include/asm/processor.h  | 17 +++++++++++++++++
 arch/powerpc/kernel/dt_cpu_ftrs.c     |  5 +++++
 arch/powerpc/platforms/powernv/idle.c | 17 +++++++++++++----
 drivers/cpuidle/cpuidle-powernv.c     |  3 ++-
 4 files changed, 37 insertions(+), 5 deletions(-)

arch/powerpc/include/asm/processor.h +17 -0

index eedcbfb9a6ff..da59f01a5c09 100644
@@ -429,6 +429,23 @@ extern void power4_idle_nap(void);
 extern unsigned long cpuidle_disable;
 enum idle_boot_override {IDLE_NO_OVERRIDE = 0, IDLE_POWERSAVE_OFF};
 
+#define STOP_ENABLE		0x00000001
+
+#define STOP_VERSION_P9       0x1
+
+/*
+ * Classify the dependencies of the stop states
+ * @idle_stop: function handler to handle the quirk stop version
+ * @cpuidle_prop: Signify support for stop states through kernel and/or firmware
+ * @stop_version: Classify quirk versions for stop states
+ */
+typedef struct {
+	unsigned long (*idle_stop)(unsigned long, bool);
+	uint8_t cpuidle_prop;
+	uint8_t stop_version;
+}stop_deps_t;
+extern stop_deps_t stop_dep;
+
 extern int powersave_nap;	/* set if nap mode can be used in idle loop */
 
 extern void power7_idle_type(unsigned long type);

arch/powerpc/kernel/dt_cpu_ftrs.c +5 -0

index 182b4047c1ef..db1a525e090d 100644
@@ -292,6 +292,8 @@ static int __init feat_enable_idle_stop(struct dt_cpu_feature *f)
 	lpcr |=  LPCR_PECE1;
 	lpcr |=  LPCR_PECE2;
 	mtspr(SPRN_LPCR, lpcr);
+	stop_dep.cpuidle_prop |= STOP_ENABLE;
+	stop_dep.stop_version = STOP_VERSION_P9;
 
 	return 1;
 }

@@ -657,6 +659,9 @@ static void __init cpufeatures_setup_start(u32 isa)
 	}
 }
 
+stop_deps_t stop_dep = {NULL, 0x0, 0x0};
+EXPORT_SYMBOL(stop_dep);
+
 static bool __init cpufeatures_process_feature(struct dt_cpu_feature *f)
 {
 	const struct dt_cpu_feature_match *m;

arch/powerpc/platforms/powernv/idle.c +13 -4

index 78599bca66c2..c32cdc37acf4 100644
@@ -812,7 +812,7 @@ static unsigned long power9_offline_stop(unsigned long psscr)
 
 #ifndef CONFIG_KVM_BOOK3S_HV_POSSIBLE
 	__ppc64_runlatch_off();
-	srr1 = power9_idle_stop(psscr, true);
+	srr1 = stop_dep.idle_stop(psscr, true);
 	__ppc64_runlatch_on();
 #else
 	/*

@@ -828,7 +828,7 @@ static unsigned long power9_offline_stop(unsigned long psscr)
 	local_paca->kvm_hstate.hwthread_state = KVM_HWTHREAD_IN_IDLE;
 
 	__ppc64_runlatch_off();
-	srr1 = power9_idle_stop(psscr, false);
+	srr1 = stop_dep.idle_stop(psscr, true);
 	__ppc64_runlatch_on();
 
 	local_paca->kvm_hstate.hwthread_state = KVM_HWTHREAD_IN_KERNEL;

@@ -856,7 +856,7 @@ void power9_idle_type(unsigned long stop_psscr_val,
 	psscr = (psscr & ~stop_psscr_mask) | stop_psscr_val;
 
 	__ppc64_runlatch_off();
-	srr1 = power9_idle_stop(psscr, true);
+	srr1 = stop_dep.idle_stop(psscr, true);
 	__ppc64_runlatch_on();
 
 	fini_irq_for_idle_irqsoff();

@@ -1353,8 +1353,17 @@ static int __init pnv_init_idle_states(void)
 	nr_pnv_idle_states = 0;
 	supported_cpuidle_states = 0;
 
-	if (cpuidle_disable != IDLE_NO_OVERRIDE)
+	if (cpuidle_disable != IDLE_NO_OVERRIDE ||
+	    !(stop_dep.cpuidle_prop & STOP_ENABLE))
 		goto out;
+	switch(stop_dep.stop_version) {
+	case STOP_VERSION_P9:
+		stop_dep.idle_stop = power9_idle_stop;
+		break;
+	default:
+		stop_dep.idle_stop = NULL;
+		goto out;
+	}
 	rc = pnv_parse_cpuidle_dt();
 	if (rc)
 		return rc;

drivers/cpuidle/cpuidle-powernv.c +2 -1

index 1b299e801f74..7430a8edf5c9 100644
@@ -371,7 +371,8 @@ static int powernv_add_idle_states(void)
  */
 static int powernv_idle_probe(void)
 {
-	if (cpuidle_disable != IDLE_NO_OVERRIDE)
+	if (cpuidle_disable != IDLE_NO_OVERRIDE ||
+	    !(stop_dep.cpuidle_prop & STOP_ENABLE))
 		return -ENODEV;
 
 	if (firmware_has_feature(FW_FEATURE_OPAL)) {

Signature · 1 line

2.24.1

kbuild test robot

build bot

2020-03-05 00:43 UTC #2 lore raw

Re: [RFC 1/3] Interface for an idle-stop dependency structure

[if your patch is applied to the wrong git tree, please drop us a note to help improve the system. BTW, we also suggest to use '--base' option to specify the base tree in git format-patch, please…

Hi Pratik,

[FYI, it's a private test report for your RFC patch.]
[auto build test ERROR on powerpc/next]
[also build test ERROR on linux/master linus/master v5.6-rc4 next-20200304] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system. BTW, we also suggest to use '--base' option to specify the base tree in git format-patch, please see https://stackoverflow.com/a/37406982]

url:    https://github.com/0day-ci/linux/commits/Pratik-Rajesh-Sampat/cpuidle-powernv-Interface-to-handle-idle-stop-versioning/20200305-050402
base:   https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git next
config: powerpc-rhel-kconfig (attached as .config)
compiler: powerpc64le-linux-gcc (GCC) 7.5.0

reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
GCC_VERSION=7.5.0 make.cross ARCH=powerpc

If you fix the issue, kindly add following tag

Reported-by kbuild test robot <[email protected]>

All errors (new ones prefixed by >>):

powerpc64le-linux-ld: warning: orphan section .gnu.hash' from linker stubs' being placed in section `.gnu.hash'.

>> arch/powerpc/platforms/powernv/idle.o:(.toc+0x0): undefined reference to `stop_dep'
>> drivers/cpuidle/cpuidle-powernv.o:(.toc+0x0): undefined reference to `stop_dep'

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

Gautham R Shenoy

2020-04-08 10:52 UTC #3 lore raw

Re: [RFC 1/3] Interface for an idle-stop dependency structure

In reply to Pratik Rajesh Sampat

Hi Pratik,

Quoted 10 lines · Pratik Rajesh Sampat

Design patch to introduce the idea of having a dependency structure for idle-stop. The structure encapsulates the following:
1. Bitmask for version of idle-stop
2. Bitmask for propterties like ENABLE/DISABLE
3. Function pointer which helps handle how the stop must be invoked
The commit lays a foundation for other idle-stop versions to be added and handled cleanly based on their specified requirments. Currently it handles the existing "idle-stop" version by setting the discovery bits and the function pointer.

So, if this patch is applied, and we are running with an OPAL that doesn't publish the "idle-stop" dt-cpu-feature, then the goal is to not enable any stop states. Is this correct ?

Quoted 21 lines

Signed-off-by Pratik Rajesh Sampat <[email protected]>

---
 arch/powerpc/include/asm/processor.h  | 17 +++++++++++++++++
 arch/powerpc/kernel/dt_cpu_ftrs.c     |  5 +++++
 arch/powerpc/platforms/powernv/idle.c | 17 +++++++++++++----
 drivers/cpuidle/cpuidle-powernv.c     |  3 ++-
 4 files changed, 37 insertions(+), 5 deletions(-)

arch/powerpc/include/asm/processor.h +4 -0

index eedcbfb9a6ff..da59f01a5c09 100644
@@ -429,6 +429,23 @@ extern void power4_idle_nap(void);
 extern unsigned long cpuidle_disable;
 enum idle_boot_override {IDLE_NO_OVERRIDE = 0, IDLE_POWERSAVE_OFF};
 
+#define STOP_ENABLE		0x00000001
+
+#define STOP_VERSION_P9       0x1
+

arch/powerpc/include/asm/processor.h extern void power4_idle_nap(void); · line 445

Quoted 10 lines

arch/powerpc/include/asm/processor.h +10 -0

+/*
+ * Classify the dependencies of the stop states
+ * @idle_stop: function handler to handle the quirk stop version
+ * @cpuidle_prop: Signify support for stop states through kernel and/or firmware
+ * @stop_version: Classify quirk versions for stop states
+ */
+typedef struct {
+	unsigned long (*idle_stop)(unsigned long, bool);
+	uint8_t cpuidle_prop;
+	uint8_t stop_version;

Why do we need both cpuidle_prop and stop_version ?

Quoted 11 lines

arch/powerpc/include/asm/processor.h +3 -0

+}stop_deps_t;
+extern stop_deps_t stop_dep;
+
 extern int powersave_nap;	/* set if nap mode can be used in idle loop */
 
 extern void power7_idle_type(unsigned long type);

arch/powerpc/kernel/dt_cpu_ftrs.c +0 -0

index 182b4047c1ef..db1a525e090d 100644

arch/powerpc/kernel/dt_cpu_ftrs.c static int __init feat_enable_idle_stop(struct dt_cpu_feature *f) · line 296

Quoted 5 lines

arch/powerpc/kernel/dt_cpu_ftrs.c +2 -0

@@ -292,6 +292,8 @@ static int __init feat_enable_idle_stop(struct dt_cpu_feature *f)
 	lpcr |=  LPCR_PECE1;
 	lpcr |=  LPCR_PECE2;
 	mtspr(SPRN_LPCR, lpcr);
+	stop_dep.cpuidle_prop |= STOP_ENABLE;
+	stop_dep.stop_version = STOP_VERSION_P9;

Doesn't stop_version != 0 imply (stop_dep.cpuidle_prop & STOP_ENABLE)?

Quoted 36 lines

arch/powerpc/kernel/dt_cpu_ftrs.c +3 -0

 	return 1;
 }

@@ -657,6 +659,9 @@ static void __init cpufeatures_setup_start(u32 isa)
 	}
 }
 
+stop_deps_t stop_dep = {NULL, 0x0, 0x0};
+EXPORT_SYMBOL(stop_dep);
+
 static bool __init cpufeatures_process_feature(struct dt_cpu_feature *f)
 {
 	const struct dt_cpu_feature_match *m;

arch/powerpc/platforms/powernv/idle.c +2 -2

index 78599bca66c2..c32cdc37acf4 100644
@@ -812,7 +812,7 @@ static unsigned long power9_offline_stop(unsigned long psscr)
 
 #ifndef CONFIG_KVM_BOOK3S_HV_POSSIBLE
 	__ppc64_runlatch_off();
-	srr1 = power9_idle_stop(psscr, true);
+	srr1 = stop_dep.idle_stop(psscr, true);
 	__ppc64_runlatch_on();
 #else
 	/*

@@ -828,7 +828,7 @@ static unsigned long power9_offline_stop(unsigned long psscr)
 	local_paca->kvm_hstate.hwthread_state = KVM_HWTHREAD_IN_IDLE;
 
 	__ppc64_runlatch_off();
-	srr1 = power9_idle_stop(psscr, false);
+	srr1 = stop_dep.idle_stop(psscr, true);
 	__ppc64_runlatch_on();
 
 	local_paca->kvm_hstate.hwthread_state = KVM_HWTHREAD_IN_KERNEL;

arch/powerpc/platforms/powernv/idle.c void power9_idle_type(unsigned long stop_psscr_val, · line 861

Quoted 7 lines

arch/powerpc/platforms/powernv/idle.c +1 -1

@@ -856,7 +856,7 @@ void power9_idle_type(unsigned long stop_psscr_val,
 	psscr = (psscr & ~stop_psscr_mask) | stop_psscr_val;
 
 	__ppc64_runlatch_off();
-	srr1 = power9_idle_stop(psscr, true);
+	srr1 = stop_dep.idle_stop(psscr, true);
 	__ppc64_runlatch_on();

There is one other place in arch/powerpc/kvm/book3s_hv_rmhandlers.S where call isa300_idle_stop_mayloss (this is kvm_nap_sequence).

So, if stop states are not supported, then, KVM subsystem should know about it. Some KVM configurations depend on putting the secondary threads of the core offline into an idle state whose wakeup is from 0x100 vector. Your patch doesn't address that part.

arch/powerpc/platforms/powernv/idle.c static int __init pnv_init_idle_states(void) · line 1357

Quoted 8 lines

arch/powerpc/platforms/powernv/idle.c +2 -1

 	fini_irq_for_idle_irqsoff();

@@ -1353,8 +1353,17 @@ static int __init pnv_init_idle_states(void)
 	nr_pnv_idle_states = 0;
 	supported_cpuidle_states = 0;
 
-	if (cpuidle_disable != IDLE_NO_OVERRIDE)
+	if (cpuidle_disable != IDLE_NO_OVERRIDE ||
+	    !(stop_dep.cpuidle_prop & STOP_ENABLE))

This can be (stop_dep.stop_version == STOP_NOT_SUPPORTED) where #define STOP_NOT_SUPPORTED 0

arch/powerpc/platforms/powernv/idle.c static int __init pnv_init_idle_states(void) · line 1364

Quoted 7 lines

arch/powerpc/platforms/powernv/idle.c +6 -0

 		goto out;
+	switch(stop_dep.stop_version) {
+	case STOP_VERSION_P9:
+		stop_dep.idle_stop = power9_idle_stop;
+		break;
+	default:
+		stop_dep.idle_stop = NULL;

You should add a pr_warn() here that stop state isn't supported because the kernel doesn't know about the version.

Quoted 22 lines

arch/powerpc/platforms/powernv/idle.c +2 -0

+		goto out;
+	}
 	rc = pnv_parse_cpuidle_dt();
 	if (rc)
 		return rc;

drivers/cpuidle/cpuidle-powernv.c +2 -1

index 1b299e801f74..7430a8edf5c9 100644
@@ -371,7 +371,8 @@ static int powernv_add_idle_states(void)
  */
 static int powernv_idle_probe(void)
 {
-	if (cpuidle_disable != IDLE_NO_OVERRIDE)
+	if (cpuidle_disable != IDLE_NO_OVERRIDE ||
+	    !(stop_dep.cpuidle_prop & STOP_ENABLE))
 		return -ENODEV;
 
 	if (firmware_has_feature(FW_FEATURE_OPAL)) {

Signature · 1 line

2.24.1

Pratik Sampat

author

2020-04-12 12:18 UTC #4 lore raw

Re: [RFC 1/3] Interface for an idle-stop dependency structure

Hello Gautham

Quoted 9 lines · Gautham R Shenoy

Hi Pratik,
Quoted 6 lines · Pratik Rajesh Sampat
Design patch to introduce the idea of having a dependency structure for idle-stop. The structure encapsulates the following:
1. Bitmask for version of idle-stop
2. Bitmask for propterties like ENABLE/DISABLE
3. Function pointer which helps handle how the stop must be invoked

Quoted 8 lines

Quoted 4 lines
The commit lays a foundation for other idle-stop versions to be added and handled cleanly based on their specified requirments. Currently it handles the existing "idle-stop" version by setting the discovery bits and the function pointer.
So, if this patch is applied, and we are running with an OPAL that doesn't publish the "idle-stop" dt-cpu-feature, then the goal is to not enable any stop states. Is this correct ?

Yes, all states will be disabled with no power saving.

Quoted 16 lines

Signed-off-by Pratik Rajesh Sampat <[email protected]>

---
  arch/powerpc/include/asm/processor.h  | 17 +++++++++++++++++
  arch/powerpc/kernel/dt_cpu_ftrs.c     |  5 +++++
  arch/powerpc/platforms/powernv/idle.c | 17 +++++++++++++----
  drivers/cpuidle/cpuidle-powernv.c     |  3 ++-
  4 files changed, 37 insertions(+), 5 deletions(-)

arch/powerpc/include/asm/processor.h +0 -0

index eedcbfb9a6ff..da59f01a5c09 100644
@@ -429,6 +429,23 @@ extern void power4_idle_nap(void);
  extern unsigned long cpuidle_disable;
  enum idle_boot_override {IDLE_NO_OVERRIDE = 0, IDLE_POWERSAVE_OFF};

Quoted 15 lines

Quoted 4 lines
arch/powerpc/include/asm/processor.h +4 -0
+#define STOP_ENABLE		0x00000001
+
+#define STOP_VERSION_P9       0x1
+
arch/powerpc/include/asm/processor.h extern void power4_idle_nap(void); · line 445
Quoted 10 lines
arch/powerpc/include/asm/processor.h +10 -0
+/*
+ * Classify the dependencies of the stop states
+ * @idle_stop: function handler to handle the quirk stop version
+ * @cpuidle_prop: Signify support for stop states through kernel and/or firmware
+ * @stop_version: Classify quirk versions for stop states
+ */
+typedef struct {
+	unsigned long (*idle_stop)(unsigned long, bool);
+	uint8_t cpuidle_prop;
+	uint8_t stop_version;
Why do we need both cpuidle_prop and stop_version ?

The idea is that each stop_version has house multitude of overlapping properties. So the idea is to give a clean distinction. However, I can see now that the versioning and properties could be embedded in a single bitmask

Quoted 43 lines

Quoted 33 lines

arch/powerpc/include/asm/processor.h +3 -0

@@ -657,6 +659,9 @@ static void __init cpufeatures_setup_start(u32 isa)
  	}
  }
 
+stop_deps_t stop_dep = {NULL, 0x0, 0x0};
+EXPORT_SYMBOL(stop_dep);
+
  static bool __init cpufeatures_process_feature(struct dt_cpu_feature *f)
  {
  	const struct dt_cpu_feature_match *m;

arch/powerpc/platforms/powernv/idle.c +2 -2

index 78599bca66c2..c32cdc37acf4 100644
@@ -812,7 +812,7 @@ static unsigned long power9_offline_stop(unsigned long psscr)
 
  #ifndef CONFIG_KVM_BOOK3S_HV_POSSIBLE
  	__ppc64_runlatch_off();
-	srr1 = power9_idle_stop(psscr, true);
+	srr1 = stop_dep.idle_stop(psscr, true);
  	__ppc64_runlatch_on();
  #else
  	/*

@@ -828,7 +828,7 @@ static unsigned long power9_offline_stop(unsigned long psscr)
  	local_paca->kvm_hstate.hwthread_state = KVM_HWTHREAD_IN_IDLE;
 
  	__ppc64_runlatch_off();
-	srr1 = power9_idle_stop(psscr, false);
+	srr1 = stop_dep.idle_stop(psscr, true);
  	__ppc64_runlatch_on();
 
  	local_paca->kvm_hstate.hwthread_state = KVM_HWTHREAD_IN_KERNEL;

arch/powerpc/platforms/powernv/idle.c void power9_idle_type(unsigned long stop_psscr_val, · line 861

Quoted 7 lines

arch/powerpc/platforms/powernv/idle.c +1 -1

@@ -856,7 +856,7 @@ void power9_idle_type(unsigned long stop_psscr_val,
  	psscr = (psscr & ~stop_psscr_mask) | stop_psscr_val;
 
  	__ppc64_runlatch_off();
-	srr1 = power9_idle_stop(psscr, true);
+	srr1 = stop_dep.idle_stop(psscr, true);
  	__ppc64_runlatch_on();

There is one other place in arch/powerpc/kvm/book3s_hv_rmhandlers.S where call isa300_idle_stop_mayloss (this is kvm_nap_sequence).

Quoted 5 lines

So, if stop states are not supported, then, KVM subsystem should know about it. Some KVM configurations depend on putting the secondary threads of the core offline into an idle state whose wakeup is from 0x100 vector. Your patch doesn't address that part.

Sure, I'll make sure to address it there too.

Quoted 11 lines

arch/powerpc/platforms/powernv/idle.c static int __init pnv_init_idle_states(void) · line 1364
Quoted 8 lines
arch/powerpc/platforms/powernv/idle.c +6 -0
  		goto out;
+	switch(stop_dep.stop_version) {
+	case STOP_VERSION_P9:
+		stop_dep.idle_stop = power9_idle_stop;
+		break;
+	default:
+		stop_dep.idle_stop = NULL;
You should add a pr_warn() here that stop state isn't supported because the kernel doesn't know about the version.

Sure

Thanks
Pratik

Pratik Rajesh Sampat

author

2020-03-04 16:01 UTC #5 lore raw

RFC 2/3 Demonstration of handling an idle-stop quirk version

In reply to Pratik Rajesh Sampat

Concept patch demonstrating an idle-stop version discovery from the device tree, along with population its support and versioning. It also assigns the function pointer to handle any idle-stop specific quirks.

Signed-off-by Pratik Rajesh Sampat <[email protected]>

---
 arch/powerpc/include/asm/processor.h  |  1 +
 arch/powerpc/kernel/dt_cpu_ftrs.c     | 16 ++++++++++++++++
 arch/powerpc/platforms/powernv/idle.c |  8 ++++++++
 3 files changed, 25 insertions(+)

arch/powerpc/include/asm/processor.h +1 -0

index da59f01a5c09..277dbabafd02 100644
@@ -432,6 +432,7 @@ enum idle_boot_override {IDLE_NO_OVERRIDE = 0, IDLE_POWERSAVE_OFF};
 #define STOP_ENABLE		0x00000001
 
 #define STOP_VERSION_P9       0x1
+#define STOP_VERSION_P9_V1    0x2
 
 /*
  * Classify the dependencies of the stop states

arch/powerpc/kernel/dt_cpu_ftrs.c +16 -0

index db1a525e090d..63e30aa49356 100644
@@ -298,6 +298,21 @@ static int __init feat_enable_idle_stop(struct dt_cpu_feature *f)
 	return 1;
 }
 
+static int __init feat_enable_idle_stop_quirk(struct dt_cpu_feature *f)
+{
+	u64 lpcr;
+
+	/* Set PECE wakeup modes for ISAv3.0B */
+	lpcr = mfspr(SPRN_LPCR);
+	lpcr |=  LPCR_PECE0;
+	lpcr |=  LPCR_PECE1;
+	lpcr |=  LPCR_PECE2;
+	mtspr(SPRN_LPCR, lpcr);
+	stop_dep.cpuidle_prop |= STOP_ENABLE;
+	stop_dep.stop_version = STOP_VERSION_P9_V1;
+
+	return 1;
+}
 static int __init feat_enable_mmu_hash(struct dt_cpu_feature *f)
 {
 	u64 lpcr;

@@ -592,6 +607,7 @@ static struct dt_cpu_feature_match __initdata
 	{"idle-nap", feat_enable_idle_nap, 0},
 	{"alignment-interrupt-dsisr", feat_enable_align_dsisr, 0},
 	{"idle-stop", feat_enable_idle_stop, 0},
+	{"idle-stop-v1", feat_enable_idle_stop_quirk, 0},
 	{"machine-check-power8", feat_enable_mce_power8, 0},
 	{"performance-monitor-power8", feat_enable_pmu_power8, 0},
 	{"data-stream-control-register", feat_enable_dscr, CPU_FTR_DSCR},

arch/powerpc/platforms/powernv/idle.c +8 -0

index c32cdc37acf4..9f5b21da2fc5 100644
@@ -805,6 +805,11 @@ static unsigned long power9_idle_stop(unsigned long psscr, bool mmu_on)
 	return srr1;
 }
 
+static unsigned long power9_idle_quirky_stop(unsigned long psscr, bool mmu_on)
+{
+	return power9_idle_stop(psscr, mmu_on);
+}
+
 #ifdef CONFIG_HOTPLUG_CPU
 static unsigned long power9_offline_stop(unsigned long psscr)
 {

@@ -1360,6 +1365,9 @@ static int __init pnv_init_idle_states(void)
 	case STOP_VERSION_P9:
 		stop_dep.idle_stop = power9_idle_stop;
 		break;
+	case STOP_VERSION_P9_V1:
+		stop_dep.idle_stop = power9_idle_quirky_stop;
+		break;
 	default:
 		stop_dep.idle_stop = NULL;
 		goto out;

Signature · 1 line

2.24.1

Pratik Rajesh Sampat

author

2020-03-04 16:01 UTC #6 lore raw

RFC 3/3 Introduce capability for firmware-enabled-stop

In reply to Pratik Rajesh Sampat

Design patch that introduces the capability for firmware to handle the stop states instead. A bit is set based on the discovery of the feature and correspondingly also the responsibility to handle the stop states.

The commit does not contain calling into the firmware to utilize firmware enabled stop.

Signed-off-by Pratik Rajesh Sampat <psampat at linux.ibm.com>

---
 arch/powerpc/include/asm/processor.h | 1 +
 arch/powerpc/kernel/dt_cpu_ftrs.c    | 9 +++++++++
 2 files changed, 10 insertions(+)

arch/powerpc/include/asm/processor.h +1 -0

index 277dbabafd02..978fab35d133 100644
@@ -430,6 +430,7 @@ extern unsigned long cpuidle_disable;
 enum idle_boot_override {IDLE_NO_OVERRIDE = 0, IDLE_POWERSAVE_OFF};
 
 #define STOP_ENABLE		0x00000001
+#define FIRMWARE_STOP_ENABLE	0x00000010
 
 #define STOP_VERSION_P9       0x1
 #define STOP_VERSION_P9_V1    0x2

arch/powerpc/kernel/dt_cpu_ftrs.c +9 -0

index 63e30aa49356..e00f8afabc46 100644
@@ -313,6 +313,14 @@ static int __init feat_enable_idle_stop_quirk(struct dt_cpu_feature *f)
 
 	return 1;
 }
+
+static int __init feat_enable_firmware_stop(struct dt_cpu_feature *f)
+{
+	stop_dep.cpuidle_prop |= FIRMWARE_STOP_ENABLE;
+
+	return 1;
+}
+
 static int __init feat_enable_mmu_hash(struct dt_cpu_feature *f)
 {
 	u64 lpcr;

@@ -608,6 +616,7 @@ static struct dt_cpu_feature_match __initdata
 	{"alignment-interrupt-dsisr", feat_enable_align_dsisr, 0},
 	{"idle-stop", feat_enable_idle_stop, 0},
 	{"idle-stop-v1", feat_enable_idle_stop_quirk, 0},
+	{"firmware-stop-supported", feat_enable_firmware_stop, 0},
 	{"machine-check-power8", feat_enable_mce_power8, 0},
 	{"performance-monitor-power8", feat_enable_pmu_power8, 0},
 	{"data-stream-control-register", feat_enable_dscr, CPU_FTR_DSCR},

Signature · 1 line

2.24.1

Gautham R Shenoy

2020-04-08 10:55 UTC #7 lore raw

Re: [RFC 3/3] Introduce capability for firmware-enabled-stop

Hi Pratik,

Quoted 18 lines · Pratik Rajesh Sampat

Design patch that introduces the capability for firmware to handle the stop states instead. A bit is set based on the discovery of the feature and correspondingly also the responsibility to handle the stop states.
The commit does not contain calling into the firmware to utilize firmware enabled stop.
Signed-off-by Pratik Rajesh Sampat <psampat at linux.ibm.com>
---
 arch/powerpc/include/asm/processor.h | 1 +
 arch/powerpc/kernel/dt_cpu_ftrs.c    | 9 +++++++++
 2 files changed, 10 insertions(+)
arch/powerpc/include/asm/processor.h +0 -0
index 277dbabafd02..978fab35d133 100644

arch/powerpc/include/asm/processor.h extern unsigned long cpuidle_disable; · line 433

Quoted 4 lines

arch/powerpc/include/asm/processor.h +1 -0

@@ -430,6 +430,7 @@ extern unsigned long cpuidle_disable;
 enum idle_boot_override {IDLE_NO_OVERRIDE = 0, IDLE_POWERSAVE_OFF};
 
 #define STOP_ENABLE		0x00000001
+#define FIRMWARE_STOP_ENABLE	0x00000010

This could be made a bit in the "version" variable.

Quoted 8 lines

arch/powerpc/include/asm/processor.h +0 -0
 #define STOP_VERSION_P9       0x1
 #define STOP_VERSION_P9_V1    0x2
arch/powerpc/kernel/dt_cpu_ftrs.c +0 -0
index 63e30aa49356..e00f8afabc46 100644

arch/powerpc/kernel/dt_cpu_ftrs.c static int __init feat_enable_idle_stop_quirk(struct dt_cpu_feature *f) · line 319

Quoted 7 lines

arch/powerpc/kernel/dt_cpu_ftrs.c +4 -0

@@ -313,6 +313,14 @@ static int __init feat_enable_idle_stop_quirk(struct dt_cpu_feature *f)
 
 	return 1;
 }
+
+static int __init feat_enable_firmware_stop(struct dt_cpu_feature *f)
+{
+	stop_dep.cpuidle_prop |= FIRMWARE_STOP_ENABLE;

stop_dep.cpuidle_version |= FIRMWARE_STOP_V1; or some such variant.

Quoted 18 lines

arch/powerpc/kernel/dt_cpu_ftrs.c +5 -0

+
+	return 1;
+}
+
 static int __init feat_enable_mmu_hash(struct dt_cpu_feature *f)
 {
 	u64 lpcr;

@@ -608,6 +616,7 @@ static struct dt_cpu_feature_match __initdata
 	{"alignment-interrupt-dsisr", feat_enable_align_dsisr, 0},
 	{"idle-stop", feat_enable_idle_stop, 0},
 	{"idle-stop-v1", feat_enable_idle_stop_quirk, 0},
+	{"firmware-stop-supported", feat_enable_firmware_stop, 0},
 	{"machine-check-power8", feat_enable_mce_power8, 0},
 	{"performance-monitor-power8", feat_enable_pmu_power8, 0},
 	{"data-stream-control-register", feat_enable_dscr, CPU_FTR_DSCR},

Signature · 1 line

2.24.1