2020-04-27 02:12:33

by Abhishek Goel

[permalink] [raw]
Subject: [RFC 1/3] powernv/cpuidle : Support for pre-entry and post exit of stop state in firmware

This patch provides kernel framework fro opal support of save restore
of sprs in idle stop loop. Opal support for stop states is needed to
selectively enable stop states or to introduce a quirk quickly in case
a buggy stop state is present.

We make a opal call from kernel if firmware-stop-support for stop
states is present and enabled. All the quirks for pre-entry of stop
state is handled inside opal. A call from opal is made into kernel
where we execute stop afer saving of NVGPRs.
After waking up from 0x100 vector in kernel, we enter back into opal.
All the quirks in post exit path, if any, are then handled in opal,
from where we return successfully back to kernel.
For deep stop states in which additional SPRs are lost, saving and
restoration will be done in OPAL.

This idea was first proposed by Nick here:
https://patchwork.ozlabs.org/patch/1208159/

The corresponding skiboot patch for this kernel patch is here:
https://patchwork.ozlabs.org/project/skiboot/list/?series=172831

When we callback from OPAL into kernel, r13 is clobbered. So, to
access PACA we need to restore it from HSPRGO. In future we can
handle this into OPAL as in here:
https://patchwork.ozlabs.org/patch/1245275/

Signed-off-by: Abhishek Goel <[email protected]>
Signed-off-by: Nicholas Piggin <[email protected]>
---

v1->v2 : No change in this patch.

arch/powerpc/include/asm/opal-api.h | 8 ++++-
arch/powerpc/include/asm/opal.h | 3 ++
arch/powerpc/kernel/idle_book3s.S | 5 +++
arch/powerpc/platforms/powernv/idle.c | 37 ++++++++++++++++++++++
arch/powerpc/platforms/powernv/opal-call.c | 2 ++
5 files changed, 54 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/include/asm/opal-api.h b/arch/powerpc/include/asm/opal-api.h
index c1f25a760eb1..a2c782c99c9e 100644
--- a/arch/powerpc/include/asm/opal-api.h
+++ b/arch/powerpc/include/asm/opal-api.h
@@ -214,7 +214,9 @@
#define OPAL_SECVAR_GET 176
#define OPAL_SECVAR_GET_NEXT 177
#define OPAL_SECVAR_ENQUEUE_UPDATE 178
-#define OPAL_LAST 178
+#define OPAL_REGISTER_OS_OPS 181
+#define OPAL_CPU_IDLE 182
+#define OPAL_LAST 182

#define QUIESCE_HOLD 1 /* Spin all calls at entry */
#define QUIESCE_REJECT 2 /* Fail all calls with OPAL_BUSY */
@@ -1181,6 +1183,10 @@ struct opal_mpipl_fadump {
struct opal_mpipl_region region[];
} __packed;

+struct opal_os_ops {
+ __be64 os_idle_stop;
+};
+
#endif /* __ASSEMBLY__ */

#endif /* __OPAL_API_H */
diff --git a/arch/powerpc/include/asm/opal.h b/arch/powerpc/include/asm/opal.h
index 9986ac34b8e2..3c340bc4df8e 100644
--- a/arch/powerpc/include/asm/opal.h
+++ b/arch/powerpc/include/asm/opal.h
@@ -400,6 +400,9 @@ void opal_powercap_init(void);
void opal_psr_init(void);
void opal_sensor_groups_init(void);

+extern int64_t opal_register_os_ops(struct opal_os_ops *os_ops);
+extern int64_t opal_cpu_idle(__be64 srr1_addr, uint64_t psscr);
+
#endif /* __ASSEMBLY__ */

#endif /* _ASM_POWERPC_OPAL_H */
diff --git a/arch/powerpc/kernel/idle_book3s.S b/arch/powerpc/kernel/idle_book3s.S
index 22f249b6f58d..8d287d1d06c0 100644
--- a/arch/powerpc/kernel/idle_book3s.S
+++ b/arch/powerpc/kernel/idle_book3s.S
@@ -49,6 +49,8 @@ _GLOBAL(isa300_idle_stop_noloss)
*/
_GLOBAL(isa300_idle_stop_mayloss)
mtspr SPRN_PSSCR,r3
+ mr r6, r13
+ mfspr r13, SPRN_HSPRG0
std r1,PACAR1(r13)
mflr r4
mfcr r5
@@ -74,6 +76,7 @@ _GLOBAL(isa300_idle_stop_mayloss)
std r31,-8*18(r1)
std r4,-8*19(r1)
std r5,-8*20(r1)
+ std r6,-8*21(r1)
/* 168 bytes */
PPC_STOP
b . /* catch bugs */
@@ -91,8 +94,10 @@ _GLOBAL(idle_return_gpr_loss)
ld r1,PACAR1(r13)
ld r4,-8*19(r1)
ld r5,-8*20(r1)
+ ld r6,-8*21(r1)
mtlr r4
mtcr r5
+ mr r13,r6
/*
* KVM nap requires r2 to be saved, rather than just restoring it
* from PACATOC. This could be avoided for that less common case
diff --git a/arch/powerpc/platforms/powernv/idle.c b/arch/powerpc/platforms/powernv/idle.c
index 78599bca66c2..1841027b25c5 100644
--- a/arch/powerpc/platforms/powernv/idle.c
+++ b/arch/powerpc/platforms/powernv/idle.c
@@ -35,6 +35,7 @@
static u32 supported_cpuidle_states;
struct pnv_idle_states_t *pnv_idle_states;
int nr_pnv_idle_states;
+static bool firmware_stop_supported;

/*
* The default stop state that will be used by ppc_md.power_save
@@ -602,6 +603,25 @@ struct p9_sprs {
u64 uamor;
};

+/*
+ * This function is called from OPAL if firmware support for stop
+ * states is present and enabled. It provides a fallback for idle
+ * stop states via OPAL.
+ */
+static uint64_t os_idle_stop(uint64_t psscr, bool save_gprs)
+{
+ /*
+ * For lite state which does not lose even GPRS we call
+ * idle_stop_noloss while for all other states we call
+ * idle_stop_mayloss. Saving and restoration of other additional
+ * SPRs if required is handled in OPAL. All the quirks are also
+ * handled in OPAL.
+ */
+ if (!save_gprs)
+ return isa300_idle_stop_noloss(psscr);
+ return isa300_idle_stop_mayloss(psscr);
+}
+
static unsigned long power9_idle_stop(unsigned long psscr, bool mmu_on)
{
int cpu = raw_smp_processor_id();
@@ -613,6 +633,16 @@ static unsigned long power9_idle_stop(unsigned long psscr, bool mmu_on)
unsigned long mmcr0 = 0;
struct p9_sprs sprs = {}; /* avoid false used-uninitialised */
bool sprs_saved = false;
+ int rc = 0;
+
+ /*
+ * Kernel takes decision whether to make OPAL call or not. This logic
+ * will be combined with the logic for BE opal to take decision.
+ */
+ if (firmware_stop_supported) {
+ rc = opal_cpu_idle(cpu_to_be64(__pa(&srr1)), (uint64_t) psscr);
+ goto out;
+ }

if (!(psscr & (PSSCR_EC|PSSCR_ESL))) {
/* EC=ESL=0 case */
@@ -1232,6 +1262,10 @@ static int pnv_parse_cpuidle_dt(void)
pr_warn("opal: PowerMgmt Node not found\n");
return -ENODEV;
}
+
+ if (of_device_is_compatible(np, "firmware-stop-supported"))
+ firmware_stop_supported = true;
+
nr_idle_states = of_property_count_u32_elems(np,
"ibm,cpu-idle-state-flags");

@@ -1326,6 +1360,7 @@ static int pnv_parse_cpuidle_dt(void)

static int __init pnv_init_idle_states(void)
{
+ struct opal_os_ops os_ops;
int cpu;
int rc = 0;

@@ -1349,6 +1384,8 @@ static int __init pnv_init_idle_states(void)
}
}

+ os_ops.os_idle_stop = be64_to_cpu(os_idle_stop);
+ rc = opal_register_os_ops((struct opal_os_ops *)(&os_ops));
/* In case we error out nr_pnv_idle_states will be zero */
nr_pnv_idle_states = 0;
supported_cpuidle_states = 0;
diff --git a/arch/powerpc/platforms/powernv/opal-call.c b/arch/powerpc/platforms/powernv/opal-call.c
index 5cd0f52d258f..c885e607ba62 100644
--- a/arch/powerpc/platforms/powernv/opal-call.c
+++ b/arch/powerpc/platforms/powernv/opal-call.c
@@ -293,3 +293,5 @@ OPAL_CALL(opal_mpipl_query_tag, OPAL_MPIPL_QUERY_TAG);
OPAL_CALL(opal_secvar_get, OPAL_SECVAR_GET);
OPAL_CALL(opal_secvar_get_next, OPAL_SECVAR_GET_NEXT);
OPAL_CALL(opal_secvar_enqueue_update, OPAL_SECVAR_ENQUEUE_UPDATE);
+OPAL_CALL(opal_register_os_ops, OPAL_REGISTER_OS_OPS);
+OPAL_CALL(opal_cpu_idle, OPAL_CPU_IDLE);
--
2.17.1


2020-04-27 02:12:35

by Abhishek Goel

[permalink] [raw]
Subject: [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 <[email protected]>
Signed-off-by: Abhishek Goel <[email protected]>
---

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 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 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 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)) {
--
2.17.1

2020-04-27 02:15:35

by Abhishek Goel

[permalink] [raw]
Subject: [RFC 3/3] powernv/cpuidle : Introduce capability for firmware-enabled-stop

This patch 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.

If Kernel does not know about stop version, it can fallback to opal for
idle stop support if firmware-stop-supported property is present.

Earlier this patch was posted as part of this series :
https://lkml.org/lkml/2020/3/4/589

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

v1->v2: This patch is newly added in this series.

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

diff --git a/arch/powerpc/include/asm/processor.h b/arch/powerpc/include/asm/processor.h
index 66fa20476d0e..d5c6532b11ef 100644
--- a/arch/powerpc/include/asm/processor.h
+++ b/arch/powerpc/include/asm/processor.h
@@ -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

diff --git a/arch/powerpc/kernel/dt_cpu_ftrs.c b/arch/powerpc/kernel/dt_cpu_ftrs.c
index db1a525e090d..ff4a87b79702 100644
--- a/arch/powerpc/kernel/dt_cpu_ftrs.c
+++ b/arch/powerpc/kernel/dt_cpu_ftrs.c
@@ -298,6 +298,13 @@ static int __init feat_enable_idle_stop(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;
@@ -592,6 +599,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},
+ {"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},
diff --git a/arch/powerpc/platforms/powernv/idle.c b/arch/powerpc/platforms/powernv/idle.c
index 538f0842ac3f..0de5de81902e 100644
--- a/arch/powerpc/platforms/powernv/idle.c
+++ b/arch/powerpc/platforms/powernv/idle.c
@@ -633,16 +633,6 @@ static unsigned long power9_idle_stop(unsigned long psscr, bool mmu_on)
unsigned long mmcr0 = 0;
struct p9_sprs sprs = {}; /* avoid false used-uninitialised */
bool sprs_saved = false;
- int rc = 0;
-
- /*
- * Kernel takes decision whether to make OPAL call or not. This logic
- * will be combined with the logic for BE opal to take decision.
- */
- if (firmware_stop_supported) {
- rc = opal_cpu_idle(cpu_to_be64(__pa(&srr1)), (uint64_t) psscr);
- goto out;
- }

if (!(psscr & (PSSCR_EC|PSSCR_ESL))) {
/* EC=ESL=0 case */
@@ -835,6 +825,19 @@ static unsigned long power9_idle_stop(unsigned long psscr, bool mmu_on)
return srr1;
}

+static unsigned long power9_firmware_idle_stop(unsigned long psscr, bool mmu_on)
+{
+ unsigned long srr1;
+ int rc;
+
+ rc = opal_cpu_idle(cpu_to_be64(__pa(&srr1)), (uint64_t) psscr);
+
+ if (mmu_on)
+ mtmsr(MSR_KERNEL);
+ return srr1;
+
+}
+
#ifdef CONFIG_HOTPLUG_CPU
static unsigned long power9_offline_stop(unsigned long psscr)
{
@@ -1394,9 +1397,11 @@ static int __init pnv_init_idle_states(void)
!(stop_dep.cpuidle_prop & STOP_ENABLE))
goto out;

- /* Check for supported version in kernel */
+ /* Check for supported version in kernel or fallback to kernel*/
if (stop_dep.stop_version & STOP_VERSION_P9) {
stop_dep.idle_stop = power9_idle_stop;
+ } else if (stop_dep.cpuidle_prop & FIRMWARE_STOP_ENABLE) {
+ stop_dep.idle_stop = power9_firmware_idle_stop;
} else {
stop_dep.idle_stop = NULL;
goto out;
--
2.17.1

2020-04-27 10:30:27

by Gautham R Shenoy

[permalink] [raw]
Subject: Re: [RFC 1/3] powernv/cpuidle : Support for pre-entry and post exit of stop state in firmware

Hi Abhishek,

On Sun, Apr 26, 2020 at 09:10:25PM -0500, Abhishek Goel wrote:
> This patch provides kernel framework fro opal support of save restore
> of sprs in idle stop loop. Opal support for stop states is needed to
> selectively enable stop states or to introduce a quirk quickly in case
> a buggy stop state is present.
>
> We make a opal call from kernel if firmware-stop-support for stop
> states is present and enabled. All the quirks for pre-entry of stop
> state is handled inside opal. A call from opal is made into kernel
> where we execute stop afer saving of NVGPRs.
> After waking up from 0x100 vector in kernel, we enter back into opal.
> All the quirks in post exit path, if any, are then handled in opal,
> from where we return successfully back to kernel.
> For deep stop states in which additional SPRs are lost, saving and
> restoration will be done in OPAL.
>
> This idea was first proposed by Nick here:
> https://patchwork.ozlabs.org/patch/1208159/
>
> The corresponding skiboot patch for this kernel patch is here:
> https://patchwork.ozlabs.org/project/skiboot/list/?series=172831
>
> When we callback from OPAL into kernel, r13 is clobbered. So, to
> access PACA we need to restore it from HSPRGO. In future we can
> handle this into OPAL as in here:
> https://patchwork.ozlabs.org/patch/1245275/
>
> Signed-off-by: Abhishek Goel <[email protected]>
> Signed-off-by: Nicholas Piggin <[email protected]>
> ---
>
> v1->v2 : No change in this patch.
>
> arch/powerpc/include/asm/opal-api.h | 8 ++++-
> arch/powerpc/include/asm/opal.h | 3 ++
> arch/powerpc/kernel/idle_book3s.S | 5 +++
> arch/powerpc/platforms/powernv/idle.c | 37 ++++++++++++++++++++++
> arch/powerpc/platforms/powernv/opal-call.c | 2 ++
> 5 files changed, 54 insertions(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/include/asm/opal-api.h b/arch/powerpc/include/asm/opal-api.h
> index c1f25a760eb1..a2c782c99c9e 100644
> --- a/arch/powerpc/include/asm/opal-api.h
> +++ b/arch/powerpc/include/asm/opal-api.h
> @@ -214,7 +214,9 @@
> #define OPAL_SECVAR_GET 176
> #define OPAL_SECVAR_GET_NEXT 177
> #define OPAL_SECVAR_ENQUEUE_UPDATE 178
> -#define OPAL_LAST 178
> +#define OPAL_REGISTER_OS_OPS 181
> +#define OPAL_CPU_IDLE 182
> +#define OPAL_LAST 182
>
> #define QUIESCE_HOLD 1 /* Spin all calls at entry */
> #define QUIESCE_REJECT 2 /* Fail all calls with OPAL_BUSY */
> @@ -1181,6 +1183,10 @@ struct opal_mpipl_fadump {
> struct opal_mpipl_region region[];
> } __packed;
>
> +struct opal_os_ops {
> + __be64 os_idle_stop;
> +};
> +
> #endif /* __ASSEMBLY__ */
>
> #endif /* __OPAL_API_H */
> diff --git a/arch/powerpc/include/asm/opal.h b/arch/powerpc/include/asm/opal.h
> index 9986ac34b8e2..3c340bc4df8e 100644
> --- a/arch/powerpc/include/asm/opal.h
> +++ b/arch/powerpc/include/asm/opal.h
> @@ -400,6 +400,9 @@ void opal_powercap_init(void);
> void opal_psr_init(void);
> void opal_sensor_groups_init(void);
>
> +extern int64_t opal_register_os_ops(struct opal_os_ops *os_ops);
> +extern int64_t opal_cpu_idle(__be64 srr1_addr, uint64_t psscr);
> +
> #endif /* __ASSEMBLY__ */
>
> #endif /* _ASM_POWERPC_OPAL_H */
> diff --git a/arch/powerpc/kernel/idle_book3s.S b/arch/powerpc/kernel/idle_book3s.S
> index 22f249b6f58d..8d287d1d06c0 100644
> --- a/arch/powerpc/kernel/idle_book3s.S
> +++ b/arch/powerpc/kernel/idle_book3s.S
> @@ -49,6 +49,8 @@ _GLOBAL(isa300_idle_stop_noloss)
> */
> _GLOBAL(isa300_idle_stop_mayloss)
> mtspr SPRN_PSSCR,r3
> + mr r6, r13
> + mfspr r13, SPRN_HSPRG0
> std r1,PACAR1(r13)
> mflr r4
> mfcr r5
> @@ -74,6 +76,7 @@ _GLOBAL(isa300_idle_stop_mayloss)
> std r31,-8*18(r1)
> std r4,-8*19(r1)
> std r5,-8*20(r1)
> + std r6,-8*21(r1)
> /* 168 bytes */
> PPC_STOP
> b . /* catch bugs */
> @@ -91,8 +94,10 @@ _GLOBAL(idle_return_gpr_loss)
> ld r1,PACAR1(r13)
> ld r4,-8*19(r1)
> ld r5,-8*20(r1)
> + ld r6,-8*21(r1)
> mtlr r4
> mtcr r5
> + mr r13,r6
> /*
> * KVM nap requires r2 to be saved, rather than just restoring it
> * from PACATOC. This could be avoided for that less common case
> diff --git a/arch/powerpc/platforms/powernv/idle.c b/arch/powerpc/platforms/powernv/idle.c
> index 78599bca66c2..1841027b25c5 100644
> --- a/arch/powerpc/platforms/powernv/idle.c
> +++ b/arch/powerpc/platforms/powernv/idle.c
> @@ -35,6 +35,7 @@
> static u32 supported_cpuidle_states;
> struct pnv_idle_states_t *pnv_idle_states;
> int nr_pnv_idle_states;
> +static bool firmware_stop_supported;
>
> /*
> * The default stop state that will be used by ppc_md.power_save
> @@ -602,6 +603,25 @@ struct p9_sprs {
> u64 uamor;
> };
>
> +/*
> + * This function is called from OPAL if firmware support for stop
> + * states is present and enabled. It provides a fallback for idle
> + * stop states via OPAL.
> + */
> +static uint64_t os_idle_stop(uint64_t psscr, bool save_gprs)
> +{
> + /*
> + * For lite state which does not lose even GPRS we call
> + * idle_stop_noloss while for all other states we call
> + * idle_stop_mayloss. Saving and restoration of other additional
> + * SPRs if required is handled in OPAL. All the quirks are also
> + * handled in OPAL.
> + */
> + if (!save_gprs)
> + return isa300_idle_stop_noloss(psscr);

I think PSSCR[ESL|EC] = 0 case is an overkill to go into OPAL and come
back via a callback. That can be handled in the kernel itself.



> + return isa300_idle_stop_mayloss(psscr);
> +}
> +
> static unsigned long power9_idle_stop(unsigned long psscr, bool mmu_on)
> {
> int cpu = raw_smp_processor_id();
> @@ -613,6 +633,16 @@ static unsigned long power9_idle_stop(unsigned long psscr, bool mmu_on)
> unsigned long mmcr0 = 0;
> struct p9_sprs sprs = {}; /* avoid false used-uninitialised */
> bool sprs_saved = false;
> + int rc = 0;
> +
> + /*
> + * Kernel takes decision whether to make OPAL call or not. This logic
> + * will be combined with the logic for BE opal to take decision.
> + */
> + if (firmware_stop_supported) {
> + rc = opal_cpu_idle(cpu_to_be64(__pa(&srr1)), (uint64_t) psscr);

Couple of comments here.

1) If PSSCR[ESL|EC] = 0, the current code expects mmu_on=true. When
we make an OPAL call and come back into the kernel via the
callback today, we will be in real-mode, with mmu turned off.

2) You seem to be choosing the opal cpuidle support as the default
case, and not as a fallback. Thus, with this patch you will miss
out on the deep stop-state support.


> + goto out;
> + }
>
> if (!(psscr & (PSSCR_EC|PSSCR_ESL))) {
> /* EC=ESL=0 case */
> @@ -1232,6 +1262,10 @@ static int pnv_parse_cpuidle_dt(void)
> pr_warn("opal: PowerMgmt Node not found\n");
> return -ENODEV;
> }
> +
> + if (of_device_is_compatible(np, "firmware-stop-supported"))
> + firmware_stop_supported = true;

IMO, at least for POWER9 generation, you need to do this only when
"idle-stop" device-tree cpu-feature is unavailable.


> +
> nr_idle_states = of_property_count_u32_elems(np,
> "ibm,cpu-idle-state-flags");
>
> @@ -1326,6 +1360,7 @@ static int pnv_parse_cpuidle_dt(void)
>
> static int __init pnv_init_idle_states(void)
> {
> + struct opal_os_ops os_ops;
> int cpu;
> int rc = 0;
>
> @@ -1349,6 +1384,8 @@ static int __init pnv_init_idle_states(void)
> }
> }
>
> + os_ops.os_idle_stop = be64_to_cpu(os_idle_stop);
> + rc = opal_register_os_ops((struct opal_os_ops *)(&os_ops));
> /* In case we error out nr_pnv_idle_states will be zero */
> nr_pnv_idle_states = 0;
> supported_cpuidle_states = 0;
> diff --git a/arch/powerpc/platforms/powernv/opal-call.c b/arch/powerpc/platforms/powernv/opal-call.c
> index 5cd0f52d258f..c885e607ba62 100644
> --- a/arch/powerpc/platforms/powernv/opal-call.c
> +++ b/arch/powerpc/platforms/powernv/opal-call.c
> @@ -293,3 +293,5 @@ OPAL_CALL(opal_mpipl_query_tag, OPAL_MPIPL_QUERY_TAG);
> OPAL_CALL(opal_secvar_get, OPAL_SECVAR_GET);
> OPAL_CALL(opal_secvar_get_next, OPAL_SECVAR_GET_NEXT);
> OPAL_CALL(opal_secvar_enqueue_update, OPAL_SECVAR_ENQUEUE_UPDATE);
> +OPAL_CALL(opal_register_os_ops, OPAL_REGISTER_OS_OPS);
> +OPAL_CALL(opal_cpu_idle, OPAL_CPU_IDLE);
> --
> 2.17.1
>

2020-04-27 13:49:16

by Gautham R Shenoy

[permalink] [raw]
Subject: 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 <[email protected]>
> Signed-off-by: Abhishek Goel <[email protected]>
> ---
>
> 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 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 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 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)) {
> --
> 2.17.1
>

2020-04-27 13:52:46

by Gautham R Shenoy

[permalink] [raw]
Subject: Re: [RFC 3/3] powernv/cpuidle : Introduce capability for firmware-enabled-stop

On Sun, Apr 26, 2020 at 09:10:27PM -0500, Abhishek Goel wrote:
> This patch 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.
>
> If Kernel does not know about stop version, it can fallback to opal for
> idle stop support if firmware-stop-supported property is present.
>
> Earlier this patch was posted as part of this series :
> https://lkml.org/lkml/2020/3/4/589
>
> Signed-off-by: Pratik Rajesh Sampat <[email protected]>
> Signed-off-by: Abhishek Goel <[email protected]>
> ---
>
> v1->v2: This patch is newly added in this series.
>
> arch/powerpc/include/asm/processor.h | 1 +
> arch/powerpc/kernel/dt_cpu_ftrs.c | 8 ++++++++
> arch/powerpc/platforms/powernv/idle.c | 27 ++++++++++++++++-----------
> 3 files changed, 25 insertions(+), 11 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/processor.h b/arch/powerpc/include/asm/processor.h
> index 66fa20476d0e..d5c6532b11ef 100644
> --- a/arch/powerpc/include/asm/processor.h
> +++ b/arch/powerpc/include/asm/processor.h
> @@ -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
>
> diff --git a/arch/powerpc/kernel/dt_cpu_ftrs.c b/arch/powerpc/kernel/dt_cpu_ftrs.c
> index db1a525e090d..ff4a87b79702 100644
> --- a/arch/powerpc/kernel/dt_cpu_ftrs.c
> +++ b/arch/powerpc/kernel/dt_cpu_ftrs.c
> @@ -298,6 +298,13 @@ static int __init feat_enable_idle_stop(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;
> @@ -592,6 +599,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},
> + {"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},
> diff --git a/arch/powerpc/platforms/powernv/idle.c b/arch/powerpc/platforms/powernv/idle.c
> index 538f0842ac3f..0de5de81902e 100644
> --- a/arch/powerpc/platforms/powernv/idle.c
> +++ b/arch/powerpc/platforms/powernv/idle.c
> @@ -633,16 +633,6 @@ static unsigned long power9_idle_stop(unsigned long psscr, bool mmu_on)
> unsigned long mmcr0 = 0;
> struct p9_sprs sprs = {}; /* avoid false used-uninitialised */
> bool sprs_saved = false;
> - int rc = 0;
> -
> - /*
> - * Kernel takes decision whether to make OPAL call or not. This logic
> - * will be combined with the logic for BE opal to take decision.
> - */
> - if (firmware_stop_supported) {
> - rc = opal_cpu_idle(cpu_to_be64(__pa(&srr1)), (uint64_t) psscr);
> - goto out;
> - }
>
> if (!(psscr & (PSSCR_EC|PSSCR_ESL))) {
> /* EC=ESL=0 case */
> @@ -835,6 +825,19 @@ static unsigned long power9_idle_stop(unsigned long psscr, bool mmu_on)
> return srr1;
> }
>
> +static unsigned long power9_firmware_idle_stop(unsigned long psscr, bool mmu_on)
> +{
> + unsigned long srr1;
> + int rc;
> +
> + rc = opal_cpu_idle(cpu_to_be64(__pa(&srr1)), (uint64_t) psscr);
> +
> + if (mmu_on)
> + mtmsr(MSR_KERNEL);
> + return srr1;
> +
> +}
> +
> #ifdef CONFIG_HOTPLUG_CPU
> static unsigned long power9_offline_stop(unsigned long psscr)
> {
> @@ -1394,9 +1397,11 @@ static int __init pnv_init_idle_states(void)
> !(stop_dep.cpuidle_prop & STOP_ENABLE))
> goto out;
>
> - /* Check for supported version in kernel */
> + /* Check for supported version in kernel or fallback to kernel*/
> if (stop_dep.stop_version & STOP_VERSION_P9) {
> stop_dep.idle_stop = power9_idle_stop;
> + } else if (stop_dep.cpuidle_prop & FIRMWARE_STOP_ENABLE) {
> + stop_dep.idle_stop = power9_firmware_idle_stop;

Ok, so in this patch you first check if the "idle-stop" feature is
available. Only otherwise you fallback to the OPAL based cpuidle
driver.

This looks ok to me.


> } else {
> stop_dep.idle_stop = NULL;
> goto out;
> --
> 2.17.1
>

--
Thanks and Regards
gautham.

2020-04-28 01:17:18

by Nicholas Piggin

[permalink] [raw]
Subject: Re: [RFC 1/3] powernv/cpuidle : Support for pre-entry and post exit of stop state in firmware

Thanks for picking this up and pushing it along. I do plan to come back
and take another look at it all, but what we do need to do first is get
a coherent approach to this proposed new calling convention and OS ops.

It's fine to work on this in the meantime, but to start merging things
my idea is:

- OPAL must leave r13-r15 untouched for the OS.
- OS ops are made available only for a "v4" OS that uses the new
calling convention, including kernel stack.
- OS ops baseline (all OSes must provide) will be console / printk
facility, trap handling and crash/symbol decoding on behalf of OPAL,
and runtime virtual memory.

Other OS ops features can be added in the versioned structure, including
this.

I'm trying to get back to cleaning these things up and start getting
them merged now. Any comments or review on those would be helpful.

Thanks,
Nick

2020-04-30 05:55:25

by Abhishek Goel

[permalink] [raw]
Subject: Re: [RFC 1/3] powernv/cpuidle : Support for pre-entry and post exit of stop state in firmware

Hi Nick,

Have you posted out the kernel side of "opal v4" patchset?
I could only find the opal patchset.

Thanks,
Abhishek

On 04/28/2020 06:38 AM, Nicholas Piggin wrote:
> Thanks for picking this up and pushing it along. I do plan to come back
> and take another look at it all, but what we do need to do first is get
> a coherent approach to this proposed new calling convention and OS ops.
>
> It's fine to work on this in the meantime, but to start merging things
> my idea is:
>
> - OPAL must leave r13-r15 untouched for the OS.
> - OS ops are made available only for a "v4" OS that uses the new
> calling convention, including kernel stack.
> - OS ops baseline (all OSes must provide) will be console / printk
> facility, trap handling and crash/symbol decoding on behalf of OPAL,
> and runtime virtual memory.
>
> Other OS ops features can be added in the versioned structure, including
> this.
>
> I'm trying to get back to cleaning these things up and start getting
> them merged now. Any comments or review on those would be helpful.
>
> Thanks,
> Nick
>

2020-05-02 11:45:40

by Nicholas Piggin

[permalink] [raw]
Subject: Re: [RFC 1/3] powernv/cpuidle : Support for pre-entry and post exit of stop state in firmware

Excerpts from Abhishek's message of April 30, 2020 3:52 pm:
> Hi Nick,
>
> Have you posted out the kernel side of "opal v4" patchset?
> I could only find the opal patchset.

I just posted some new ones. I have some change sfor the cpuidle side
but I haven't really looked to see what needs reconciling with your
version, but I'll try to do that when I get time.

Thanks,
Nick