powerpc: powernv: Enable stop4 via cpuidle

PATCH Superseded Gautham R. Shenoy 3 participants 7 messages Jul 19, 2017 PowerPC

lore

Share & tools

lore

Gautham R. Shenoy

author

2017-07-19 08:19 UTC #0 lore raw

v2 cover powerpc: powernv: Enable stop4 via cpuidle

From: "Gautham R. Shenoy" <[email protected]>

Hi,

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

The earlier version can be found here :
https://lkml.org/lkml/2017/7/18/691

The change from the first version is to the second patch titled " powernv/powerpc: Clear PECE1 in LPCR via stop-api only on Hotplug" where all the LPCR manipulations for CPU-Hotplug are moved 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/paca.h       |  7 ++++++
 arch/powerpc/kernel/asm-offsets.c     | 12 +++++++++
 arch/powerpc/kernel/idle_book3s.S     | 46 +++++++++++++++++++++++++++++++++--
 arch/powerpc/platforms/powernv/idle.c | 45 +++++++++++++++++++++++++++++++++-
 arch/powerpc/platforms/powernv/smp.c  | 10 --------
 5 files changed, 107 insertions(+), 13 deletions(-)

Signature · 1 line

1.9.4

Gautham R. Shenoy

author

2017-07-19 08:19 UTC #1 lore raw

PATCH v2 1/2 powernv/powerpc:Save/Restore additional SPRs for stop4 cpuidle

From: "Gautham R. Shenoy" <[email protected]>

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.

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 <[email protected]>

---
 arch/powerpc/include/asm/paca.h   |  7 ++++++
 arch/powerpc/kernel/asm-offsets.c | 12 ++++++++++
 arch/powerpc/kernel/idle_book3s.S | 46 +++++++++++++++++++++++++++++++++++++--
 3 files changed, 63 insertions(+), 2 deletions(-)

arch/powerpc/include/asm/paca.h +7 -0

index dc88a31..a6b9ea6 100644
@@ -48,6 +48,7 @@
 #define get_lppaca()	(get_paca()->lppaca_ptr)
 #define get_slb_shadow()	(get_paca()->slb_shadow_ptr)
 
+#define MAX_STOP_SPRS     7
 struct task_struct;
 
 /*

@@ -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.
+	 */
+	u64 stop_spr_save_area[MAX_STOP_SPRS];
 #endif
 
 #ifdef CONFIG_PPC_STD_MMU_64

arch/powerpc/kernel/asm-offsets.c +12 -0

index a7b5af3..0262283 100644
@@ -743,6 +743,18 @@ 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);
+
+	OFFSET(PACA_PID, paca_struct, stop_spr_save_area[0]);
+	OFFSET(PACA_LDBAR, paca_struct, stop_spr_save_area[1]);
+	OFFSET(PACA_FSCR, paca_struct, stop_spr_save_area[2]);
+	OFFSET(PACA_HFSCR, paca_struct, stop_spr_save_area[3]);
+
+	/* On POWER9, we are already saving MMCR0 for ESL=EC=1 */
+	OFFSET(PACA_MMCRA, paca_struct, stop_spr_save_area[4]);
+	OFFSET(PACA_MMCR1, paca_struct, stop_spr_save_area[5]);
+	OFFSET(PACA_MMCR2, paca_struct, stop_spr_save_area[6]);
+
+
 #endif
 
 	DEFINE(PPC_DBELL_SERVER, PPC_DBELL_SERVER);

arch/powerpc/kernel/idle_book3s.S +44 -2

59 lines not on this page · raw message

Signature · 1 line

1.9.4

Nicholas Piggin

reviewer Reviewed-by on v2 1/2

2017-07-19 09:03 UTC #2 lore raw

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

Quoted 61 lines · On Wed, 19 Jul 2017 13:48:49 +0530 "Gautham R. Shenoy"…

From: "Gautham R. Shenoy" <[email protected]>
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 <[email protected]>
---
 arch/powerpc/include/asm/paca.h   |  7 ++++++
 arch/powerpc/kernel/asm-offsets.c | 12 ++++++++++
 arch/powerpc/kernel/idle_book3s.S | 46 +++++++++++++++++++++++++++++++++++++--
 3 files changed, 63 insertions(+), 2 deletions(-)
arch/powerpc/include/asm/paca.h +7 -0
index dc88a31..a6b9ea6 100644
@@ -48,6 +48,7 @@
 #define get_lppaca()	(get_paca()->lppaca_ptr)
 #define get_slb_shadow()	(get_paca()->slb_shadow_ptr)
 
+#define MAX_STOP_SPRS     7
 struct task_struct;
 
 /*
@@ -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.
+	 */
+	u64 stop_spr_save_area[MAX_STOP_SPRS];
 #endif
 
 #ifdef CONFIG_PPC_STD_MMU_64
arch/powerpc/kernel/asm-offsets.c +0 -0
index a7b5af3..0262283 100644
@@ -743,6 +743,18 @@ 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);

arch/powerpc/kernel/asm-offsets.c int main(void) · line 755

Quoted 10 lines

arch/powerpc/kernel/asm-offsets.c +10 -0

+
+	OFFSET(PACA_PID, paca_struct, stop_spr_save_area[0]);
+	OFFSET(PACA_LDBAR, paca_struct, stop_spr_save_area[1]);
+	OFFSET(PACA_FSCR, paca_struct, stop_spr_save_area[2]);
+	OFFSET(PACA_HFSCR, paca_struct, stop_spr_save_area[3]);
+
+	/* On POWER9, we are already saving MMCR0 for ESL=EC=1 */
+	OFFSET(PACA_MMCRA, paca_struct, stop_spr_save_area[4]);
+	OFFSET(PACA_MMCR1, paca_struct, stop_spr_save_area[5]);
+	OFFSET(PACA_MMCR2, paca_struct, stop_spr_save_area[6]);

Don't these offset names go against convention?

Look at e.g., how PACA_EXGEN is used. I would prefer using that convention. You could make the name slightly shorter too, e.g., just stop_sprs or so.

arch/powerpc/kernel/asm-offsets.c no_segments: · line 840

Quoted 10 lines

arch/powerpc/kernel/asm-offsets.c +1 -2

@@ -791,8 +834,7 @@ no_segments:
 	bctrl
 
 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:

Also perhaps just a little comment in the code explaining why power8 does not need this because it only uses it for hotplug as you do in the changelog would help when reading the code.

Sorry I didn't pick this up on your v1 patches. If you change those minor things, please add

Reviewed-by Nicholas Piggin <[email protected]>

Thanks,
Nick

Michael Ellerman

reviewer

2017-07-19 12:07 UTC #3 lore raw

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

Quoted 77 lines · Nicholas Piggin

Quoted 61 lines · On Wed, 19 Jul 2017 13:48:49 +0530 "Gautham R. Shenoy"…
From: "Gautham R. Shenoy" <[email protected]>
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 <[email protected]>
---
 arch/powerpc/include/asm/paca.h   |  7 ++++++
 arch/powerpc/kernel/asm-offsets.c | 12 ++++++++++
 arch/powerpc/kernel/idle_book3s.S | 46 +++++++++++++++++++++++++++++++++++++--
 3 files changed, 63 insertions(+), 2 deletions(-)
arch/powerpc/include/asm/paca.h +7 -0
index dc88a31..a6b9ea6 100644
@@ -48,6 +48,7 @@
 #define get_lppaca()	(get_paca()->lppaca_ptr)
 #define get_slb_shadow()	(get_paca()->slb_shadow_ptr)
 
+#define MAX_STOP_SPRS     7
 struct task_struct;
 
 /*
@@ -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.
+	 */
+	u64 stop_spr_save_area[MAX_STOP_SPRS];
 #endif
 
 #ifdef CONFIG_PPC_STD_MMU_64
arch/powerpc/kernel/asm-offsets.c +0 -0
index a7b5af3..0262283 100644
@@ -743,6 +743,18 @@ 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);
arch/powerpc/kernel/asm-offsets.c int main(void) · line 755
Quoted 10 lines
arch/powerpc/kernel/asm-offsets.c +10 -0
+
+	OFFSET(PACA_PID, paca_struct, stop_spr_save_area[0]);
+	OFFSET(PACA_LDBAR, paca_struct, stop_spr_save_area[1]);
+	OFFSET(PACA_FSCR, paca_struct, stop_spr_save_area[2]);
+	OFFSET(PACA_HFSCR, paca_struct, stop_spr_save_area[3]);
+
+	/* On POWER9, we are already saving MMCR0 for ESL=EC=1 */
+	OFFSET(PACA_MMCRA, paca_struct, stop_spr_save_area[4]);
+	OFFSET(PACA_MMCR1, paca_struct, stop_spr_save_area[5]);
+	OFFSET(PACA_MMCR2, paca_struct, stop_spr_save_area[6]);
Don't these offset names go against convention?

Quoted 3 lines

Look at e.g., how PACA_EXGEN is used. I would prefer using that convention. You could make the name slightly shorter too, e.g., just stop_sprs or so.

Yes please.

If I see PACA_MMCRA I'm expecting that's paca->mmcra.

Also if the same values always go in the same place then please use a proper struct, rather than an array. ie.

struct stop_sprs

{
	u64 pid;
        u64 ldbar;
        ...
}

cheers

Gautham R Shenoy

author

2017-07-19 12:43 UTC #4 lore raw

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

Hi Nicholas, Michael,

Quoted 22 lines · Michael Ellerman

Quoted 21 lines · Nicholas Piggin

Quoted 8 lines

arch/powerpc/kernel/asm-offsets.c +0 -0

index a7b5af3..0262283 100644
@@ -743,6 +743,18 @@ 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);

arch/powerpc/kernel/asm-offsets.c int main(void) · line 755

Quoted 10 lines

arch/powerpc/kernel/asm-offsets.c +10 -0

+
+	OFFSET(PACA_PID, paca_struct, stop_spr_save_area[0]);
+	OFFSET(PACA_LDBAR, paca_struct, stop_spr_save_area[1]);
+	OFFSET(PACA_FSCR, paca_struct, stop_spr_save_area[2]);
+	OFFSET(PACA_HFSCR, paca_struct, stop_spr_save_area[3]);
+
+	/* On POWER9, we are already saving MMCR0 for ESL=EC=1 */
+	OFFSET(PACA_MMCRA, paca_struct, stop_spr_save_area[4]);
+	OFFSET(PACA_MMCR1, paca_struct, stop_spr_save_area[5]);
+	OFFSET(PACA_MMCR2, paca_struct, stop_spr_save_area[6]);

Don't these offset names go against convention?

Quoted 7 lines

Quoted 3 lines
Look at e.g., how PACA_EXGEN is used. I would prefer using that convention. You could make the name slightly shorter too, e.g., just stop_sprs or so.
Yes please.
If I see PACA_MMCRA I'm expecting that's paca->mmcra.

Ah, ok. I will fix this.

Quoted 4 lines

Also if the same values always go in the same place then please use a proper struct, rather than an array. ie.

Ok, I will add the struct instead of a array.

Quoted 9 lines

struct stop_sprs
{
	u64 pid;
        u64 ldbar;
        ...
}
cheers

--
Thanks and Regards
gautham.

Gautham R. Shenoy

author

2017-07-19 08:19 UTC #5 lore raw

PATCH v2 2/2 powernv/powerpc: Clear PECE1 in LPCR via stop-api only on Hotplug

In reply to Gautham R. Shenoy

From: "Gautham R. Shenoy" <[email protected]>

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, for POWER9, we program the stop-api for LPCR with PECE1 bit cleared only when we are offlining the CPU.

Signed-off-by Gautham R. Shenoy <[email protected]>

---
 arch/powerpc/platforms/powernv/idle.c | 45 ++++++++++++++++++++++++++++++++++-
 arch/powerpc/platforms/powernv/smp.c  | 10 --------
 2 files changed, 44 insertions(+), 11 deletions(-)

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

index 2abee07..a6c69b7 100644
@@ -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);

@@ -85,6 +85,16 @@ static int pnv_save_sprs_for_deep_states(void)
 		if (rc != 0)
 			return rc;
 
+		/*
+		 * On POWER8, the only state that uses SLW engine is
+		 * winkle.  This is only used for CPU-Hotplug. So we
+		 * clear the decrementer bit from LPCR since we
+		 * don't want to be woken up on decrementer when in
+		 * winkle.
+		 */
+		if (!cpu_has_feature(CPU_FTR_ARCH_300))
+			lpcr_val = lpcr_val & ~(u64)LPCR_PECE1;
+
 		rc = opal_slw_set_reg(pir, SPRN_LPCR, lpcr_val);
 		if (rc != 0)
 			return rc;

@@ -355,6 +365,15 @@ 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);
+	if (cpu_has_feature(CPU_FTR_ARCH_300))
+		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 +383,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 +427,16 @@ unsigned long pnv_cpu_offline(unsigned int cpu)
 
 	__ppc64_runlatch_on();
 
+	/*
+	 * Re-enable decrementer interrupts in LPCR.
+	 *
+	 * Further on POWER9, 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

arch/powerpc/platforms/powernv/smp.c +0 ~~-10~~

index 40dae96..9ef6b37 100644
@@ -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

@@ -181,7 +175,6 @@ static void pnv_smp_cpu_kill_self(void)
 		kvmppc_set_host_ipi(cpu, 0);
 
 		srr1 = pnv_cpu_offline(cpu);
-
 		WARN_ON(lazy_irq_pending());
 
 		/*

@@ -216,11 +209,8 @@ static void pnv_smp_cpu_kill_self(void)
 		if (srr1 && !generic_check_cpu_restart(cpu))
 			DBG("CPU%d Unexpected exit while offline srr1=%lx!\n",
 					cpu, srr1);
-
 	}
 
-	/* Re-enable decrementer interrupts */
-	mtspr(SPRN_LPCR, mfspr(SPRN_LPCR) | LPCR_PECE1);
 	DBG("CPU%d coming online...\n", cpu);
 }

Signature · 1 line

1.9.4

Nicholas Piggin

reviewer Reviewed-by on v2 2/2

2017-07-19 09:09 UTC #6 lore raw

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

Quoted 53 lines · On Wed, 19 Jul 2017 13:48:50 +0530 "Gautham R. Shenoy"…

From: "Gautham R. Shenoy" <[email protected]>
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, for POWER9, we program the stop-api for LPCR with PECE1 bit cleared only when we are offlining the CPU.
Signed-off-by Gautham R. Shenoy <[email protected]>
---
 arch/powerpc/platforms/powernv/idle.c | 45 ++++++++++++++++++++++++++++++++++-
 arch/powerpc/platforms/powernv/smp.c  | 10 --------
 2 files changed, 44 insertions(+), 11 deletions(-)
arch/powerpc/platforms/powernv/idle.c +11 -1
index 2abee07..a6c69b7 100644
@@ -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);
@@ -85,6 +85,16 @@ static int pnv_save_sprs_for_deep_states(void)
 		if (rc != 0)
 			return rc;
 
+		/*
+		 * On POWER8, the only state that uses SLW engine is
+		 * winkle.  This is only used for CPU-Hotplug. So we
+		 * clear the decrementer bit from LPCR since we
+		 * don't want to be woken up on decrementer when in
+		 * winkle.
+		 */
+		if (!cpu_has_feature(CPU_FTR_ARCH_300))
+			lpcr_val = lpcr_val & ~(u64)LPCR_PECE1;
+
 		rc = opal_slw_set_reg(pir, SPRN_LPCR, lpcr_val);
 		if (rc != 0)
 			return rc;

arch/powerpc/platforms/powernv/idle.c void power9_idle(void) · line 374

Quoted 10 lines

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

@@ -355,6 +365,15 @@ 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);
+	if (cpu_has_feature(CPU_FTR_ARCH_300))
+		opal_slw_set_reg(pir, SPRN_LPCR, lpcr_val);

Is opal_slw_set_reg very heavyweight? If not, I would just remove the POWER9 special casing entirely from both these functions, which follows the principle of least surprise. Otherwise I guess it's reasonable.

Thanks for making those other changes, it looks good.

Reviewed-by Nicholas Piggin <[email protected]>