Re: [RFC 2/3] powernv/cpuidle : Interface for an idle-stop dependency structure
On Sun, Apr 26, 2020 at 09:10:26PM -0500, Abhishek Goel wrote:
> This patch introduces 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
>
> This patch 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.
>
> Earlier this patch was posted as part of this series :
> https://lkml.org/lkml/2020/3/4/589
Please see the review comments to the earlier version:
https://lkml.org/lkml/2020/4/8/245
I still feel that we don't need cpuidle_prop and stop_version to be
separate fields.
>
> Signed-off-by: Pratik Rajesh Sampat
> Signed-off-by: Abhishek Goel
> ---
>
> v1->v2: This patch is newly added in this series.
>
> arch/powerpc/include/asm/processor.h | 17 +
> arch/powerpc/kernel/dt_cpu_ftrs.c | 5 +
> arch/powerpc/platforms/powernv/idle.c | 18 ++
> drivers/cpuidle/cpuidle-powernv.c | 3 ++-
> 4 files changed, 38 insertions(+), 5 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/processor.h
> b/arch/powerpc/include/asm/processor.h
> index eedcbfb9a6ff..66fa20476d0e 100644
> --- a/arch/powerpc/include/asm/processor.h
> +++ b/arch/powerpc/include/asm/processor.h
> @@ -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 0x0001
> +
> +#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 psscr, bool mmu_on);
> + 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);
> diff --git a/arch/powerpc/kernel/dt_cpu_ftrs.c
> b/arch/powerpc/kernel/dt_cpu_ftrs.c
> index 182b4047c1ef..db1a525e090d 100644
> --- a/arch/powerpc/kernel/dt_cpu_ftrs.c
> +++ b/arch/powerpc/kernel/dt_cpu_ftrs.c
> @@ -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;
> diff --git a/arch/powerpc/platforms/powernv/idle.c
> b/arch/powerpc/platforms/powernv/idle.c
> index 1841027b25c5..538f0842ac3f 100644
> --- a/arch/powerpc/platforms/powernv/idle.c
> +++ b/arch/powerpc/platforms/powernv/idle.c
> @@ -842,7 +842,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
> /*
> @@ -858,7 +858,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;
> @@ -886,7 +886,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();
> @@ -1390,8 +1390,18 @@ 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;
> +
> + /* Check for supported version in kernel */
> + if (stop_dep.stop_version & STOP_VERSION_P9) {
> + stop_dep.idle_stop = power9_idle_stop;
> + } else {
> +
[RFC 2/3] powernv/cpuidle : Interface for an idle-stop dependency structure
This patch introduces 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
This patch 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.
Earlier this patch was posted as part of this series :
https://lkml.org/lkml/2020/3/4/589
Signed-off-by: Pratik Rajesh Sampat
Signed-off-by: Abhishek Goel
---
v1->v2: This patch is newly added in this series.
arch/powerpc/include/asm/processor.h | 17 +
arch/powerpc/kernel/dt_cpu_ftrs.c | 5 +
arch/powerpc/platforms/powernv/idle.c | 18 ++
drivers/cpuidle/cpuidle-powernv.c | 3 ++-
4 files changed, 38 insertions(+), 5 deletions(-)
diff --git a/arch/powerpc/include/asm/processor.h
b/arch/powerpc/include/asm/processor.h
index eedcbfb9a6ff..66fa20476d0e 100644
--- a/arch/powerpc/include/asm/processor.h
+++ b/arch/powerpc/include/asm/processor.h
@@ -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_ENABLE0x0001
+
+#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 psscr, bool mmu_on);
+ 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);
diff --git a/arch/powerpc/kernel/dt_cpu_ftrs.c
b/arch/powerpc/kernel/dt_cpu_ftrs.c
index 182b4047c1ef..db1a525e090d 100644
--- a/arch/powerpc/kernel/dt_cpu_ftrs.c
+++ b/arch/powerpc/kernel/dt_cpu_ftrs.c
@@ -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;
diff --git a/arch/powerpc/platforms/powernv/idle.c
b/arch/powerpc/platforms/powernv/idle.c
index 1841027b25c5..538f0842ac3f 100644
--- a/arch/powerpc/platforms/powernv/idle.c
+++ b/arch/powerpc/platforms/powernv/idle.c
@@ -842,7 +842,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
/*
@@ -858,7 +858,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;
@@ -886,7 +886,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();
@@ -1390,8 +1390,18 @@ 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;
+
+ /* Check for supported version in kernel */
+ if (stop_dep.stop_version & STOP_VERSION_P9) {
+ stop_dep.idle_stop = power9_idle_stop;
+ } else {
+ stop_dep.idle_stop = NULL;
+ goto out;
+ }
+
rc = pnv_parse_cpuidle_dt();
if (rc)
return rc;
diff --git a/drivers/cpuidle/cpuidle-powernv.c
b/drivers/cpuidle/cpuidle-powernv.c
index 1b299e801f74..7430a8edf5c9 100644
--- a/drivers/cpuidle/cpuidle-powernv.c
+++ b/drivers/cpuidle/cpuidle-powernv.c
@@ -371,7 +371,8 @@ static int
