[v3 PATCH 0/2] powerpc: powernv: Enable stop4 via cpuidle

[v3 PATCH 0/2] powerpc: powernv: Enable stop4 via cpuidle

[Gautham R. Shenoy]

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

Hi,

This is the third iteration of the patchset to enable exploitation of
stop4 idle state on POWER9 via cpuidle.

The earlier version can be found here :
[v2]: https://lkml.org/lkml/2017/7/19/152
[v1]: https://lkml.org/lkml/2017/7/18/691

The changes across the versions are as follows:
v2-->v3:
- Use a structure instead of an array for the stop sprs save area.
- Name the offsets into the paca->stop_sprs as STOP_XXX instead of PACA_XXX.
- Add comments in the assembly code explaining why saving/restoring
  is not needed on POWER8.
- Program the LPCR during platform idle entry/exit on both POWER8 and POWER9
  as suggested by Nicholas Piggin.

v1 --> v2:
- Move the LPCR manipulations for CPU-Hotplug into
arch/powerpc/platforms/powernv/idle.c as per Nicholas Piggin's
suggestion.

====================== Description ===========================
The stop4 idle state on POWER9 is a deep idle state which loses
hypervisor resources, but whose latency is low enough that it can be
exposed via cpuidle.

Until now, the deep idle states which lose hypervisor resources (eg:
winkle) were only exposed via CPU-Hotplug.  Hence currently on wakeup
from such states, barring a few SPRs which need to be restored to
their older value, rest of the SPRS are reinitialized to their values
corresponding to that at boot time. When stop4 is used in the context
of cpuidle, we want these additional SPRs to be restored to their
older value, to ensure that the context on the CPU coming back from
idle is same as it was before going idle.

Additionally, the CPU which is in stop4 while idling can be woken up
by the decrementer interrupts. So we need to ensure that the LPCR is
programmed with PECE1 bit cleared via the stop-api only for the
CPU-Hotplug case and not for cpuidle.

The two patches in the series address this problem.

Gautham R. Shenoy (2):
  powernv/powerpc:Save/Restore additional SPRs for stop4 cpuidle
  powernv/powerpc: Clear PECE1 in LPCR via stop-api only on Hotplug

 arch/powerpc/include/asm/cpuidle.h    | 11 ++++++
 arch/powerpc/include/asm/paca.h       |  7 ++++
 arch/powerpc/kernel/asm-offsets.c     |  8 +++++
 arch/powerpc/kernel/idle_book3s.S     | 65 +++++++++++++++++++++++++++++++++--
 arch/powerpc/platforms/powernv/idle.c | 34 +++++++++++++++++-
 arch/powerpc/platforms/powernv/smp.c  | 10 ------
 6 files changed, 122 insertions(+), 13 deletions(-)

-- 
1.9.4

[v3 PATCH 1/2] powernv/powerpc:Save/Restore additional SPRs for stop4 cpuidle

[Gautham R. Shenoy]

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

The stop4 idle state on POWER9 is a deep idle state which loses
hypervisor resources, but whose latency is low enough that it can be
exposed via cpuidle.

Until now, the deep idle states which lose hypervisor resources (eg:
winkle) were only exposed via CPU-Hotplug.  Hence currently on wakeup
from such states, barring a few SPRs which need to be restored to
their older value, rest of the SPRS are reinitialized to their values
corresponding to that at boot time.

When stop4 is used in the context of cpuidle, we want these additional
SPRs to be restored to their older value, to ensure that the context
on the CPU coming back from idle is same as it was before going idle.

In this patch, we define a SPR save area in PACA (since we have used
up the volatile register space in the stack) and on POWER9, we restore
SPRN_PID, SPRN_LDBAR, SPRN_FSCR, SPRN_HFSCR, SPRN_MMCRA, SPRN_MMCR1,
SPRN_MMCR2 to the values they had before entering stop.

Signed-off-by: Gautham R. Shenoy <ego@linux.vnet.ibm.com>
---
v2-->v3:
- Use a structure instead of an array for the stop sprs save area.
- Name the offsets into the paca->stop_sprs as STOP_XXX instead of
  PACA_XXX.
- Add comments in the assembly code explaining why saving/restoring
  is not needed on POWER8.
v1-->v2:
No change

 arch/powerpc/include/asm/cpuidle.h | 11 +++++++
 arch/powerpc/include/asm/paca.h    |  7 ++++
 arch/powerpc/kernel/asm-offsets.c  |  8 +++++
 arch/powerpc/kernel/idle_book3s.S  | 65 ++++++++++++++++++++++++++++++++++++--
 4 files changed, 89 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/include/asm/cpuidle.h b/arch/powerpc/include/asm/cpuidle.h
index 52586f9..8a174cb 100644
--- a/arch/powerpc/include/asm/cpuidle.h
+++ b/arch/powerpc/include/asm/cpuidle.h
@@ -67,6 +67,17 @@
 #define ERR_DEEP_STATE_ESL_MISMATCH	-2
 
 #ifndef __ASSEMBLY__
+/* Additional SPRs that need to be saved/restored during stop */
+struct stop_sprs {
+	u64 pid;
+	u64 ldbar;
+	u64 fscr;
+	u64 hfscr;
+	u64 mmcr1;
+	u64 mmcr2;
+	u64 mmcra;
+};
+
 extern u32 pnv_fastsleep_workaround_at_entry[];
 extern u32 pnv_fastsleep_workaround_at_exit[];
 
diff --git a/arch/powerpc/include/asm/paca.h b/arch/powerpc/include/asm/paca.h
index dc88a31..04b60af 100644
--- a/arch/powerpc/include/asm/paca.h
+++ b/arch/powerpc/include/asm/paca.h
@@ -31,6 +31,7 @@
 #endif
 #include <asm/accounting.h>
 #include <asm/hmi.h>
+#include <asm/cpuidle.h>
 
 register struct paca_struct *local_paca asm("r13");
 
@@ -183,6 +184,12 @@ struct paca_struct {
 	struct paca_struct **thread_sibling_pacas;
 	/* The PSSCR value that the kernel requested before going to stop */
 	u64 requested_psscr;
+
+	/*
+	 * Save area for additional SPRs that need to be
+	 * saved/restored during cpuidle stop.
+	 */
+	struct stop_sprs stop_sprs;
 #endif
 
 #ifdef CONFIG_PPC_STD_MMU_64
diff --git a/arch/powerpc/kernel/asm-offsets.c b/arch/powerpc/kernel/asm-offsets.c
index a7b5af3..e2a48df 100644
--- a/arch/powerpc/kernel/asm-offsets.c
+++ b/arch/powerpc/kernel/asm-offsets.c
@@ -743,6 +743,14 @@ int main(void)
 	OFFSET(PACA_SUBCORE_SIBLING_MASK, paca_struct, subcore_sibling_mask);
 	OFFSET(PACA_SIBLING_PACA_PTRS, paca_struct, thread_sibling_pacas);
 	OFFSET(PACA_REQ_PSSCR, paca_struct, requested_psscr);
+#define STOP_SPR(x, f)	OFFSET(x, paca_struct, stop_sprs.f)
+	STOP_SPR(STOP_PID, pid);
+	STOP_SPR(STOP_LDBAR, ldbar);
+	STOP_SPR(STOP_FSCR, fscr);
+	STOP_SPR(STOP_HFSCR, hfscr);
+	STOP_SPR(STOP_MMCR1, mmcr1);
+	STOP_SPR(STOP_MMCR2, mmcr2);
+	STOP_SPR(STOP_MMCRA, mmcra);
 #endif
 
 	DEFINE(PPC_DBELL_SERVER, PPC_DBELL_SERVER);
diff --git a/arch/powerpc/kernel/idle_book3s.S b/arch/powerpc/kernel/idle_book3s.S
index 5adb390e..5e6af97 100644
--- a/arch/powerpc/kernel/idle_book3s.S
+++ b/arch/powerpc/kernel/idle_book3s.S
@@ -84,7 +84,61 @@ ALT_FTR_SECTION_END_IFSET(CPU_FTR_ARCH_300)
 	std	r3,_WORT(r1)
 	mfspr	r3,SPRN_WORC
 	std	r3,_WORC(r1)
+/*
+ * On POWER9, there are idle states such as stop4, invoked via cpuidle,
+ * that lose hypervisor resources. In such cases, we need to save
+ * additional SPRs before entering those idle states so that they can
+ * be restored to their older values on wakeup from the idle state.
+ *
+ * On POWER8, the only such deep idle state is winkle which is used
+ * only in the context of CPU-Hotplug, where these additional SPRs are
+ * reinitiazed to a sane value. Hence there is no need to save/restore
+ * these SPRs.
+ */
+BEGIN_FTR_SECTION
+	blr
+END_FTR_SECTION_IFCLR(CPU_FTR_ARCH_300)
+
+power9_save_additional_sprs:
+	mfspr	r3, SPRN_PID
+	mfspr	r4, SPRN_LDBAR
+	std	r3, STOP_PID(r13)
+	std	r4, STOP_LDBAR(r13)
 
+	mfspr	r3, SPRN_FSCR
+	mfspr	r4, SPRN_HFSCR
+	std	r3, STOP_FSCR(r13)
+	std	r4, STOP_HFSCR(r13)
+
+	mfspr	r3, SPRN_MMCRA
+	mfspr	r4, SPRN_MMCR1
+	std	r3, STOP_MMCRA(r13)
+	std	r4, STOP_MMCR1(r13)
+
+	mfspr	r3, SPRN_MMCR2
+	std	r3, STOP_MMCR2(r13)
+	blr
+
+power9_restore_additional_sprs:
+	ld	r3,_LPCR(r1)
+	ld	r4, STOP_PID(r13)
+	mtspr	SPRN_LPCR,r3
+	mtspr	SPRN_PID, r4
+
+	ld	r3, STOP_LDBAR(r13)
+	ld	r4, STOP_FSCR(r13)
+	mtspr	SPRN_LDBAR, r3
+	mtspr	SPRN_FSCR, r4
+
+	ld	r3, STOP_HFSCR(r13)
+	ld	r4, STOP_MMCRA(r13)
+	mtspr	SPRN_HFSCR, r3
+	mtspr	SPRN_MMCRA, r4
+	/* We have already restored PACA_MMCR0 */
+	ld	r3, STOP_MMCR1(r13)
+	ld	r4, STOP_MMCR2(r13)
+	mtspr	SPRN_MMCR1, r3
+	mtspr	SPRN_MMCR2, r4
 	blr
 
 /*
@@ -790,9 +844,16 @@ no_segments:
 	mtctr	r12
 	bctrl
 
+/*
+ * On POWER9, we can come here on wakeup from a cpuidle stop state.
+ * Hence restore the additional SPRs to the saved value.
+ *
+ * On POWER8, we come here only on winkle. Since winkle is used
+ * only in the case of CPU-Hotplug, we don't need to restore
+ * the additional SPRs.
+ */
 BEGIN_FTR_SECTION
-	ld	r4,_LPCR(r1)
-	mtspr	SPRN_LPCR,r4
+	bl 	power9_restore_additional_sprs
 END_FTR_SECTION_IFSET(CPU_FTR_ARCH_300)
 hypervisor_state_restored:
 
-- 
1.9.4

[RESEND] [v3 PATCH 2/2] powernv/powerpc: Clear PECE1 in LPCR via stop-api only on Hotplug

[Gautham R. Shenoy]

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

Currently we use the stop-api provided by the firmware to program the
SLW engine to restore the values of hypervisor resources that get lost
on deeper idle states (such as winkle). Since the deep states were
only used for CPU-Hotplug on POWER8 systems, we would program the LPCR
to have the PECE1 bit since Hotplugged CPUs shouldn't be spuriously
woken up by decrementer.

On POWER9, some of the deep platform idle states such as stop4 can be
used in cpuidle as well. In this case, we want the CPU in stop4 to be
woken up by the decrementer when some timer on the CPU expires.

In this patch, we program the stop-api for LPCR with PECE1
bit cleared only when we are offlining the CPU and set it
back once the CPU is online.

Signed-off-by: Gautham R. Shenoy <ego@linux.vnet.ibm.com>
---
v2 --> v3:
- Program the LPCR during platform idle entry/exit on both POWER8 and
  POWER9

v1 --> v2:
- Move the LPCR manipulations for CPU-Hotplug into idle.c

 arch/powerpc/platforms/powernv/idle.c | 34 +++++++++++++++++++++++++++++++++-
 arch/powerpc/platforms/powernv/smp.c  |  8 --------
 2 files changed, 33 insertions(+), 9 deletions(-)

diff --git a/arch/powerpc/platforms/powernv/idle.c b/arch/powerpc/platforms/powernv/idle.c
index 2abee07..a1296e7 100644
--- a/arch/powerpc/platforms/powernv/idle.c
+++ b/arch/powerpc/platforms/powernv/idle.c
@@ -68,7 +68,7 @@ static int pnv_save_sprs_for_deep_states(void)
 	 * all cpus at boot. Get these reg values of current cpu and use the
 	 * same across all cpus.
 	 */
-	uint64_t lpcr_val = mfspr(SPRN_LPCR) & ~(u64)LPCR_PECE1;
+	uint64_t lpcr_val = mfspr(SPRN_LPCR);
 	uint64_t hid0_val = mfspr(SPRN_HID0);
 	uint64_t hid1_val = mfspr(SPRN_HID1);
 	uint64_t hid4_val = mfspr(SPRN_HID4);
@@ -355,6 +355,14 @@ void power9_idle(void)
 }
 
 #ifdef CONFIG_HOTPLUG_CPU
+static void pnv_program_cpu_hotplug_lpcr(unsigned int cpu, u64 lpcr_val)
+{
+	u64 pir = get_hard_smp_processor_id(cpu);
+
+	mtspr(SPRN_LPCR, lpcr_val);
+	opal_slw_set_reg(pir, SPRN_LPCR, lpcr_val);
+}
+
 /*
  * pnv_cpu_offline: A function that puts the CPU into the deepest
  * available platform idle state on a CPU-Offline.
@@ -364,6 +372,20 @@ unsigned long pnv_cpu_offline(unsigned int cpu)
 {
 	unsigned long srr1;
 	u32 idle_states = pnv_get_supported_cpuidle_states();
+	u64 lpcr_val;
+
+	/*
+	 * We don't want to take decrementer interrupts while we are
+	 * offline, so clear LPCR:PECE1. We keep PECE2 (and
+	 * LPCR_PECE_HVEE on P9) enabled as to let IPIs in.
+	 *
+	 * If the CPU gets woken up by a special wakeup, ensure that
+	 * the SLW engine sets LPCR with decrementer bit cleared, else
+	 * the CPU will come back to the kernel due to a spurious
+	 * wakeup.
+	 */
+	lpcr_val = mfspr(SPRN_LPCR) & ~(u64)LPCR_PECE1;
+	pnv_program_cpu_hotplug_lpcr(cpu, lpcr_val);
 
 	__ppc64_runlatch_off();
 
@@ -394,6 +416,16 @@ unsigned long pnv_cpu_offline(unsigned int cpu)
 
 	__ppc64_runlatch_on();
 
+	/*
+	 * Re-enable decrementer interrupts in LPCR.
+	 *
+	 * Further, we want stop states to be woken up by decrementer
+	 * for non-hotplug cases. So program the LPCR via stop api as
+	 * well.
+	 */
+	lpcr_val = mfspr(SPRN_LPCR) | (u64)LPCR_PECE1;
+	pnv_program_cpu_hotplug_lpcr(cpu, lpcr_val);
+
 	return srr1;
 }
 #endif
diff --git a/arch/powerpc/platforms/powernv/smp.c b/arch/powerpc/platforms/powernv/smp.c
index 40dae96..536b07b 100644
--- a/arch/powerpc/platforms/powernv/smp.c
+++ b/arch/powerpc/platforms/powernv/smp.c
@@ -164,12 +164,6 @@ static void pnv_smp_cpu_kill_self(void)
 	if (cpu_has_feature(CPU_FTR_ARCH_207S))
 		wmask = SRR1_WAKEMASK_P8;
 
-	/* We don't want to take decrementer interrupts while we are offline,
-	 * so clear LPCR:PECE1. We keep PECE2 (and LPCR_PECE_HVEE on P9)
-	 * enabled as to let IPIs in.
-	 */
-	mtspr(SPRN_LPCR, mfspr(SPRN_LPCR) & ~(u64)LPCR_PECE1);
-
 	while (!generic_check_cpu_restart(cpu)) {
 		/*
 		 * Clear IPI flag, since we don't handle IPIs while
@@ -219,8 +213,6 @@ static void pnv_smp_cpu_kill_self(void)
 
 	}
 
-	/* Re-enable decrementer interrupts */
-	mtspr(SPRN_LPCR, mfspr(SPRN_LPCR) | LPCR_PECE1);
 	DBG("CPU%d coming online...\n", cpu);
 }
 
-- 
1.9.4

Re: [v3 PATCH 1/2] powernv/powerpc:Save/Restore additional SPRs for stop4 cpuidle

[Nicholas Piggin]

On Fri, 21 Jul 2017 16:11:37 +0530
"Gautham R. Shenoy" <ego@linux.vnet.ibm.com> wrote:

> From: "Gautham R. Shenoy" <ego@linux.vnet.ibm.com>
> 
> The stop4 idle state on POWER9 is a deep idle state which loses
> hypervisor resources, but whose latency is low enough that it can be
> exposed via cpuidle.
> 
> Until now, the deep idle states which lose hypervisor resources (eg:
> winkle) were only exposed via CPU-Hotplug.  Hence currently on wakeup
> from such states, barring a few SPRs which need to be restored to
> their older value, rest of the SPRS are reinitialized to their values
> corresponding to that at boot time.
> 
> When stop4 is used in the context of cpuidle, we want these additional
> SPRs to be restored to their older value, to ensure that the context
> on the CPU coming back from idle is same as it was before going idle.
> 
> In this patch, we define a SPR save area in PACA (since we have used
> up the volatile register space in the stack) and on POWER9, we restore
> SPRN_PID, SPRN_LDBAR, SPRN_FSCR, SPRN_HFSCR, SPRN_MMCRA, SPRN_MMCR1,
> SPRN_MMCR2 to the values they had before entering stop.
> 
> Signed-off-by: Gautham R. Shenoy <ego@linux.vnet.ibm.com>

Looks good to me. Keeping in mind we need to tidy up and unify
all this SPR handling and save/restore etc. in the longer term.

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

> ---
> v2-->v3:
> - Use a structure instead of an array for the stop sprs save area.
> - Name the offsets into the paca->stop_sprs as STOP_XXX instead of
>   PACA_XXX.
> - Add comments in the assembly code explaining why saving/restoring
>   is not needed on POWER8.
> v1-->v2:
> No change
> 
>  arch/powerpc/include/asm/cpuidle.h | 11 +++++++
>  arch/powerpc/include/asm/paca.h    |  7 ++++
>  arch/powerpc/kernel/asm-offsets.c  |  8 +++++
>  arch/powerpc/kernel/idle_book3s.S  | 65 ++++++++++++++++++++++++++++++++++++--
>  4 files changed, 89 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/powerpc/include/asm/cpuidle.h b/arch/powerpc/include/asm/cpuidle.h
> index 52586f9..8a174cb 100644
> --- a/arch/powerpc/include/asm/cpuidle.h
> +++ b/arch/powerpc/include/asm/cpuidle.h
> @@ -67,6 +67,17 @@
>  #define ERR_DEEP_STATE_ESL_MISMATCH	-2
>  
>  #ifndef __ASSEMBLY__
> +/* Additional SPRs that need to be saved/restored during stop */
> +struct stop_sprs {
> +	u64 pid;
> +	u64 ldbar;
> +	u64 fscr;
> +	u64 hfscr;
> +	u64 mmcr1;
> +	u64 mmcr2;
> +	u64 mmcra;
> +};
> +
>  extern u32 pnv_fastsleep_workaround_at_entry[];
>  extern u32 pnv_fastsleep_workaround_at_exit[];
>  
> diff --git a/arch/powerpc/include/asm/paca.h b/arch/powerpc/include/asm/paca.h
> index dc88a31..04b60af 100644
> --- a/arch/powerpc/include/asm/paca.h
> +++ b/arch/powerpc/include/asm/paca.h
> @@ -31,6 +31,7 @@
>  #endif
>  #include <asm/accounting.h>
>  #include <asm/hmi.h>
> +#include <asm/cpuidle.h>
>  
>  register struct paca_struct *local_paca asm("r13");
>  
> @@ -183,6 +184,12 @@ struct paca_struct {
>  	struct paca_struct **thread_sibling_pacas;
>  	/* The PSSCR value that the kernel requested before going to stop */
>  	u64 requested_psscr;
> +
> +	/*
> +	 * Save area for additional SPRs that need to be
> +	 * saved/restored during cpuidle stop.
> +	 */
> +	struct stop_sprs stop_sprs;
>  #endif
>  
>  #ifdef CONFIG_PPC_STD_MMU_64
> diff --git a/arch/powerpc/kernel/asm-offsets.c b/arch/powerpc/kernel/asm-offsets.c
> index a7b5af3..e2a48df 100644
> --- a/arch/powerpc/kernel/asm-offsets.c
> +++ b/arch/powerpc/kernel/asm-offsets.c
> @@ -743,6 +743,14 @@ int main(void)
>  	OFFSET(PACA_SUBCORE_SIBLING_MASK, paca_struct, subcore_sibling_mask);
>  	OFFSET(PACA_SIBLING_PACA_PTRS, paca_struct, thread_sibling_pacas);
>  	OFFSET(PACA_REQ_PSSCR, paca_struct, requested_psscr);
> +#define STOP_SPR(x, f)	OFFSET(x, paca_struct, stop_sprs.f)
> +	STOP_SPR(STOP_PID, pid);
> +	STOP_SPR(STOP_LDBAR, ldbar);
> +	STOP_SPR(STOP_FSCR, fscr);
> +	STOP_SPR(STOP_HFSCR, hfscr);
> +	STOP_SPR(STOP_MMCR1, mmcr1);
> +	STOP_SPR(STOP_MMCR2, mmcr2);
> +	STOP_SPR(STOP_MMCRA, mmcra);
>  #endif
>  
>  	DEFINE(PPC_DBELL_SERVER, PPC_DBELL_SERVER);
> diff --git a/arch/powerpc/kernel/idle_book3s.S b/arch/powerpc/kernel/idle_book3s.S
> index 5adb390e..5e6af97 100644
> --- a/arch/powerpc/kernel/idle_book3s.S
> +++ b/arch/powerpc/kernel/idle_book3s.S
> @@ -84,7 +84,61 @@ ALT_FTR_SECTION_END_IFSET(CPU_FTR_ARCH_300)
>  	std	r3,_WORT(r1)
>  	mfspr	r3,SPRN_WORC
>  	std	r3,_WORC(r1)
> +/*
> + * On POWER9, there are idle states such as stop4, invoked via cpuidle,
> + * that lose hypervisor resources. In such cases, we need to save
> + * additional SPRs before entering those idle states so that they can
> + * be restored to their older values on wakeup from the idle state.
> + *
> + * On POWER8, the only such deep idle state is winkle which is used
> + * only in the context of CPU-Hotplug, where these additional SPRs are
> + * reinitiazed to a sane value. Hence there is no need to save/restore
> + * these SPRs.
> + */
> +BEGIN_FTR_SECTION
> +	blr
> +END_FTR_SECTION_IFCLR(CPU_FTR_ARCH_300)
> +
> +power9_save_additional_sprs:
> +	mfspr	r3, SPRN_PID
> +	mfspr	r4, SPRN_LDBAR
> +	std	r3, STOP_PID(r13)
> +	std	r4, STOP_LDBAR(r13)
>  
> +	mfspr	r3, SPRN_FSCR
> +	mfspr	r4, SPRN_HFSCR
> +	std	r3, STOP_FSCR(r13)
> +	std	r4, STOP_HFSCR(r13)
> +
> +	mfspr	r3, SPRN_MMCRA
> +	mfspr	r4, SPRN_MMCR1
> +	std	r3, STOP_MMCRA(r13)
> +	std	r4, STOP_MMCR1(r13)
> +
> +	mfspr	r3, SPRN_MMCR2
> +	std	r3, STOP_MMCR2(r13)
> +	blr
> +
> +power9_restore_additional_sprs:
> +	ld	r3,_LPCR(r1)
> +	ld	r4, STOP_PID(r13)
> +	mtspr	SPRN_LPCR,r3
> +	mtspr	SPRN_PID, r4
> +
> +	ld	r3, STOP_LDBAR(r13)
> +	ld	r4, STOP_FSCR(r13)
> +	mtspr	SPRN_LDBAR, r3
> +	mtspr	SPRN_FSCR, r4
> +
> +	ld	r3, STOP_HFSCR(r13)
> +	ld	r4, STOP_MMCRA(r13)
> +	mtspr	SPRN_HFSCR, r3
> +	mtspr	SPRN_MMCRA, r4
> +	/* We have already restored PACA_MMCR0 */
> +	ld	r3, STOP_MMCR1(r13)
> +	ld	r4, STOP_MMCR2(r13)
> +	mtspr	SPRN_MMCR1, r3
> +	mtspr	SPRN_MMCR2, r4
>  	blr
>  
>  /*
> @@ -790,9 +844,16 @@ no_segments:
>  	mtctr	r12
>  	bctrl
>  
> +/*
> + * On POWER9, we can come here on wakeup from a cpuidle stop state.
> + * Hence restore the additional SPRs to the saved value.
> + *
> + * On POWER8, we come here only on winkle. Since winkle is used
> + * only in the case of CPU-Hotplug, we don't need to restore
> + * the additional SPRs.
> + */
>  BEGIN_FTR_SECTION
> -	ld	r4,_LPCR(r1)
> -	mtspr	SPRN_LPCR,r4
> +	bl 	power9_restore_additional_sprs
>  END_FTR_SECTION_IFSET(CPU_FTR_ARCH_300)
>  hypervisor_state_restored:
>  

Re: [RESEND] [v3 PATCH 2/2] powernv/powerpc: Clear PECE1 in LPCR via stop-api only on Hotplug

[Nicholas Piggin]

On Fri, 21 Jul 2017 16:31:34 +0530
"Gautham R. Shenoy" <ego@linux.vnet.ibm.com> wrote:

> From: "Gautham R. Shenoy" <ego@linux.vnet.ibm.com>
> 
> Currently we use the stop-api provided by the firmware to program the
> SLW engine to restore the values of hypervisor resources that get lost
> on deeper idle states (such as winkle). Since the deep states were
> only used for CPU-Hotplug on POWER8 systems, we would program the LPCR
> to have the PECE1 bit since Hotplugged CPUs shouldn't be spuriously
> woken up by decrementer.
> 
> On POWER9, some of the deep platform idle states such as stop4 can be
> used in cpuidle as well. In this case, we want the CPU in stop4 to be
> woken up by the decrementer when some timer on the CPU expires.
> 
> In this patch, we program the stop-api for LPCR with PECE1
> bit cleared only when we are offlining the CPU and set it
> back once the CPU is online.

Looks pretty good to me, thanks!

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


> 
> Signed-off-by: Gautham R. Shenoy <ego@linux.vnet.ibm.com>
> ---
> v2 --> v3:
> - Program the LPCR during platform idle entry/exit on both POWER8 and
>   POWER9
> 
> v1 --> v2:
> - Move the LPCR manipulations for CPU-Hotplug into idle.c
> 
>  arch/powerpc/platforms/powernv/idle.c | 34 +++++++++++++++++++++++++++++++++-
>  arch/powerpc/platforms/powernv/smp.c  |  8 --------
>  2 files changed, 33 insertions(+), 9 deletions(-)
> 
> diff --git a/arch/powerpc/platforms/powernv/idle.c b/arch/powerpc/platforms/powernv/idle.c
> index 2abee07..a1296e7 100644
> --- a/arch/powerpc/platforms/powernv/idle.c
> +++ b/arch/powerpc/platforms/powernv/idle.c
> @@ -68,7 +68,7 @@ static int pnv_save_sprs_for_deep_states(void)
>  	 * all cpus at boot. Get these reg values of current cpu and use the
>  	 * same across all cpus.
>  	 */
> -	uint64_t lpcr_val = mfspr(SPRN_LPCR) & ~(u64)LPCR_PECE1;
> +	uint64_t lpcr_val = mfspr(SPRN_LPCR);
>  	uint64_t hid0_val = mfspr(SPRN_HID0);
>  	uint64_t hid1_val = mfspr(SPRN_HID1);
>  	uint64_t hid4_val = mfspr(SPRN_HID4);
> @@ -355,6 +355,14 @@ void power9_idle(void)
>  }
>  
>  #ifdef CONFIG_HOTPLUG_CPU
> +static void pnv_program_cpu_hotplug_lpcr(unsigned int cpu, u64 lpcr_val)
> +{
> +	u64 pir = get_hard_smp_processor_id(cpu);
> +
> +	mtspr(SPRN_LPCR, lpcr_val);
> +	opal_slw_set_reg(pir, SPRN_LPCR, lpcr_val);
> +}
> +
>  /*
>   * pnv_cpu_offline: A function that puts the CPU into the deepest
>   * available platform idle state on a CPU-Offline.
> @@ -364,6 +372,20 @@ unsigned long pnv_cpu_offline(unsigned int cpu)
>  {
>  	unsigned long srr1;
>  	u32 idle_states = pnv_get_supported_cpuidle_states();
> +	u64 lpcr_val;
> +
> +	/*
> +	 * We don't want to take decrementer interrupts while we are
> +	 * offline, so clear LPCR:PECE1. We keep PECE2 (and
> +	 * LPCR_PECE_HVEE on P9) enabled as to let IPIs in.
> +	 *
> +	 * If the CPU gets woken up by a special wakeup, ensure that
> +	 * the SLW engine sets LPCR with decrementer bit cleared, else
> +	 * the CPU will come back to the kernel due to a spurious
> +	 * wakeup.
> +	 */
> +	lpcr_val = mfspr(SPRN_LPCR) & ~(u64)LPCR_PECE1;
> +	pnv_program_cpu_hotplug_lpcr(cpu, lpcr_val);
>  
>  	__ppc64_runlatch_off();
>  
> @@ -394,6 +416,16 @@ unsigned long pnv_cpu_offline(unsigned int cpu)
>  
>  	__ppc64_runlatch_on();
>  
> +	/*
> +	 * Re-enable decrementer interrupts in LPCR.
> +	 *
> +	 * Further, we want stop states to be woken up by decrementer
> +	 * for non-hotplug cases. So program the LPCR via stop api as
> +	 * well.
> +	 */
> +	lpcr_val = mfspr(SPRN_LPCR) | (u64)LPCR_PECE1;
> +	pnv_program_cpu_hotplug_lpcr(cpu, lpcr_val);
> +
>  	return srr1;
>  }
>  #endif
> diff --git a/arch/powerpc/platforms/powernv/smp.c b/arch/powerpc/platforms/powernv/smp.c
> index 40dae96..536b07b 100644
> --- a/arch/powerpc/platforms/powernv/smp.c
> +++ b/arch/powerpc/platforms/powernv/smp.c
> @@ -164,12 +164,6 @@ static void pnv_smp_cpu_kill_self(void)
>  	if (cpu_has_feature(CPU_FTR_ARCH_207S))
>  		wmask = SRR1_WAKEMASK_P8;
>  
> -	/* We don't want to take decrementer interrupts while we are offline,
> -	 * so clear LPCR:PECE1. We keep PECE2 (and LPCR_PECE_HVEE on P9)
> -	 * enabled as to let IPIs in.
> -	 */
> -	mtspr(SPRN_LPCR, mfspr(SPRN_LPCR) & ~(u64)LPCR_PECE1);
> -
>  	while (!generic_check_cpu_restart(cpu)) {
>  		/*
>  		 * Clear IPI flag, since we don't handle IPIs while
> @@ -219,8 +213,6 @@ static void pnv_smp_cpu_kill_self(void)
>  
>  	}
>  
> -	/* Re-enable decrementer interrupts */
> -	mtspr(SPRN_LPCR, mfspr(SPRN_LPCR) | LPCR_PECE1);
>  	DBG("CPU%d coming online...\n", cpu);
>  }
>  

Re: [v3 PATCH 1/2] powernv/powerpc:Save/Restore additional SPRs for stop4 cpuidle

[Michael Ellerman]

"Gautham R. Shenoy" <ego@linux.vnet.ibm.com> writes:
>
> Subject: [v3 PATCH 1/2] powernv/powerpc:Save/Restore additional SPRs for stop4 cpuidle

I know it's not a big deal, but can we agree on the subject format?

  powerpc/powernv: Save/Restore additional SPRs for stop4 cpuidle

cheers

Re: [v3 PATCH 1/2] powernv/powerpc:Save/Restore additional SPRs for stop4 cpuidle

[Gautham R Shenoy]

Hi Michael,


On Tue, Aug 01, 2017 at 08:56:18PM +1000, Michael Ellerman wrote:
> "Gautham R. Shenoy" <ego@linux.vnet.ibm.com> writes:
> >
> > Subject: [v3 PATCH 1/2] powernv/powerpc:Save/Restore additional SPRs for stop4 cpuidle
> 
> I know it's not a big deal, but can we agree on the subject format?
> 
>   powerpc/powernv: Save/Restore additional SPRs for stop4 cpuidle

Sure. I will repost with the updated subject.

> 
> cheers
> 
--
Thanks and Regards
gautham.

Re: [v3 PATCH 1/2] powernv/powerpc:Save/Restore additional SPRs for stop4 cpuidle

[Michael Ellerman]

Gautham R Shenoy <ego@linux.vnet.ibm.com> writes:
> On Tue, Aug 01, 2017 at 08:56:18PM +1000, Michael Ellerman wrote:
>> "Gautham R. Shenoy" <ego@linux.vnet.ibm.com> writes:
>> >
>> > Subject: [v3 PATCH 1/2] powernv/powerpc:Save/Restore additional SPRs for stop4 cpuidle
>> 
>> I know it's not a big deal, but can we agree on the subject format?
>> 
>>   powerpc/powernv: Save/Restore additional SPRs for stop4 cpuidle
>
> Sure. I will repost with the updated subject.

Sorry I had merged them, and fixed the subject up myself. But I didn't
make that very clear in my mail.

So I've merged v3.

cheers

Re: [v3, 1/2] powernv/powerpc:Save/Restore additional SPRs for stop4 cpuidle

[Michael Ellerman]

On Fri, 2017-07-21 at 10:41:37 UTC, "Gautham R. Shenoy" wrote:
> From: "Gautham R. Shenoy" <ego@linux.vnet.ibm.com>
> 
> The stop4 idle state on POWER9 is a deep idle state which loses
> hypervisor resources, but whose latency is low enough that it can be
> exposed via cpuidle.
> 
> Until now, the deep idle states which lose hypervisor resources (eg:
> winkle) were only exposed via CPU-Hotplug.  Hence currently on wakeup
> from such states, barring a few SPRs which need to be restored to
> their older value, rest of the SPRS are reinitialized to their values
> corresponding to that at boot time.
> 
> When stop4 is used in the context of cpuidle, we want these additional
> SPRs to be restored to their older value, to ensure that the context
> on the CPU coming back from idle is same as it was before going idle.
> 
> In this patch, we define a SPR save area in PACA (since we have used
> up the volatile register space in the stack) and on POWER9, we restore
> SPRN_PID, SPRN_LDBAR, SPRN_FSCR, SPRN_HFSCR, SPRN_MMCRA, SPRN_MMCR1,
> SPRN_MMCR2 to the values they had before entering stop.
> 
> Signed-off-by: Gautham R. Shenoy <ego@linux.vnet.ibm.com>
> Reviewed-by: Nicholas Piggin <npiggin@gmail.com>

Series applied to powerpc next, thanks.

https://git.kernel.org/powerpc/c/e1c1cfed54326fd2b17c78f0c85092

cheers

Re: [v3 PATCH 1/2] powernv/powerpc:Save/Restore additional SPRs for stop4 cpuidle

[Gautham R Shenoy]

On Mon, Aug 07, 2017 at 06:26:44PM +1000, Michael Ellerman wrote:
> Gautham R Shenoy <ego@linux.vnet.ibm.com> writes:
> > On Tue, Aug 01, 2017 at 08:56:18PM +1000, Michael Ellerman wrote:
> >> "Gautham R. Shenoy" <ego@linux.vnet.ibm.com> writes:
> >> >
> >> > Subject: [v3 PATCH 1/2] powernv/powerpc:Save/Restore additional SPRs for stop4 cpuidle
> >> 
> >> I know it's not a big deal, but can we agree on the subject format?
> >> 
> >>   powerpc/powernv: Save/Restore additional SPRs for stop4 cpuidle
> >
> > Sure. I will repost with the updated subject.
> 
> Sorry I had merged them, and fixed the subject up myself. But I didn't
> make that very clear in my mail.

Ah, ok! Thanks for fixing the subject line. I will keep the convention in
mind in the future.


> 
> So I've merged v3.
> 
> cheers
> 
--
Thanks and Regards
gautham.