* [v2 PATCH 0/2] powerpc: powernv: Enable stop4 via cpuidle
@ 2017-07-19 8:18 Gautham R. Shenoy
2017-07-19 8:18 ` [v2 PATCH 1/2] powernv/powerpc:Save/Restore additional SPRs for stop4 cpuidle Gautham R. Shenoy
2017-07-19 8:18 ` [v2 PATCH 2/2] powernv/powerpc: Clear PECE1 in LPCR via stop-api only on Hotplug Gautham R. Shenoy
0 siblings, 2 replies; 7+ messages in thread
From: Gautham R. Shenoy @ 2017-07-19 8:18 UTC (permalink / raw
To: Michael Ellerman, Michael Neuling, Nicholas Piggin,
Vaidyanathan Srinivasan, Shilpasri G Bhat, Akshay Adiga
Cc: linuxppc-dev, linux-kernel, Gautham R. Shenoy
From: "Gautham R. Shenoy" <ego@linux.vnet.ibm.com>
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(-)
--
1.9.4
^ permalink raw reply [flat|nested] 7+ messages in thread
* [v2 PATCH 1/2] powernv/powerpc:Save/Restore additional SPRs for stop4 cpuidle
2017-07-19 8:18 [v2 PATCH 0/2] powerpc: powernv: Enable stop4 via cpuidle Gautham R. Shenoy
@ 2017-07-19 8:18 ` Gautham R. Shenoy
2017-07-19 9:03 ` Nicholas Piggin
2017-07-19 8:18 ` [v2 PATCH 2/2] powernv/powerpc: Clear PECE1 in LPCR via stop-api only on Hotplug Gautham R. Shenoy
1 sibling, 1 reply; 7+ messages in thread
From: Gautham R. Shenoy @ 2017-07-19 8:18 UTC (permalink / raw
To: Michael Ellerman, Michael Neuling, Nicholas Piggin,
Vaidyanathan Srinivasan, Shilpasri G Bhat, Akshay Adiga
Cc: linuxppc-dev, linux-kernel, 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>
---
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(-)
diff --git a/arch/powerpc/include/asm/paca.h b/arch/powerpc/include/asm/paca.h
index dc88a31..a6b9ea6 100644
--- a/arch/powerpc/include/asm/paca.h
+++ b/arch/powerpc/include/asm/paca.h
@@ -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
diff --git a/arch/powerpc/kernel/asm-offsets.c b/arch/powerpc/kernel/asm-offsets.c
index a7b5af3..0262283 100644
--- a/arch/powerpc/kernel/asm-offsets.c
+++ b/arch/powerpc/kernel/asm-offsets.c
@@ -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);
diff --git a/arch/powerpc/kernel/idle_book3s.S b/arch/powerpc/kernel/idle_book3s.S
index 5adb390e..7083f63 100644
--- a/arch/powerpc/kernel/idle_book3s.S
+++ b/arch/powerpc/kernel/idle_book3s.S
@@ -84,7 +84,50 @@ ALT_FTR_SECTION_END_IFSET(CPU_FTR_ARCH_300)
std r3,_WORT(r1)
mfspr r3,SPRN_WORC
std r3,_WORC(r1)
+BEGIN_FTR_SECTION
+ blr
+END_FTR_SECTION_IFCLR(CPU_FTR_ARCH_300)
+ /* Save additional SPRs for POWER9 */
+power9_save_additional_sprs:
+ mfspr r3, SPRN_PID
+ mfspr r4, SPRN_LDBAR
+ std r3, PACA_PID(r13)
+ std r4, PACA_LDBAR(r13)
+
+ mfspr r3, SPRN_FSCR
+ mfspr r4, SPRN_HFSCR
+ std r3, PACA_FSCR(r13)
+ std r4, PACA_HFSCR(r13)
+
+ mfspr r3, SPRN_MMCRA
+ mfspr r4, SPRN_MMCR1
+ std r3, PACA_MMCRA(r13)
+ std r4, PACA_MMCR1(r13)
+
+ mfspr r3, SPRN_MMCR2
+ std r3, PACA_MMCR2(r13)
+ blr
+power9_restore_additional_sprs:
+ ld r3,_LPCR(r1)
+ ld r4, PACA_PID(r13)
+ mtspr SPRN_LPCR,r3
+ mtspr SPRN_PID, r4
+
+ ld r3, PACA_LDBAR(r13)
+ ld r4, PACA_FSCR(r13)
+ mtspr SPRN_LDBAR, r3
+ mtspr SPRN_FSCR, r4
+
+ ld r3, PACA_HFSCR(r13)
+ ld r4, PACA_MMCRA(r13)
+ mtspr SPRN_HFSCR, r3
+ mtspr SPRN_MMCRA, r4
+ /* We have already restored PACA_MMCR0 */
+ ld r3, PACA_MMCR1(r13)
+ ld r4, PACA_MMCR2(r13)
+ mtspr SPRN_MMCR1, r3
+ mtspr SPRN_MMCR2, r4
blr
/*
@@ -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:
--
1.9.4
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [v2 PATCH 2/2] powernv/powerpc: Clear PECE1 in LPCR via stop-api only on Hotplug
2017-07-19 8:18 [v2 PATCH 0/2] powerpc: powernv: Enable stop4 via cpuidle Gautham R. Shenoy
2017-07-19 8:18 ` [v2 PATCH 1/2] powernv/powerpc:Save/Restore additional SPRs for stop4 cpuidle Gautham R. Shenoy
@ 2017-07-19 8:18 ` Gautham R. Shenoy
2017-07-19 9:09 ` Nicholas Piggin
1 sibling, 1 reply; 7+ messages in thread
From: Gautham R. Shenoy @ 2017-07-19 8:18 UTC (permalink / raw
To: Michael Ellerman, Michael Neuling, Nicholas Piggin,
Vaidyanathan Srinivasan, Shilpasri G Bhat, Akshay Adiga
Cc: linuxppc-dev, linux-kernel, 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, 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 <ego@linux.vnet.ibm.com>
---
arch/powerpc/platforms/powernv/idle.c | 45 ++++++++++++++++++++++++++++++++++-
arch/powerpc/platforms/powernv/smp.c | 10 --------
2 files changed, 44 insertions(+), 11 deletions(-)
diff --git a/arch/powerpc/platforms/powernv/idle.c b/arch/powerpc/platforms/powernv/idle.c
index 2abee07..a6c69b7 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);
@@ -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
diff --git a/arch/powerpc/platforms/powernv/smp.c b/arch/powerpc/platforms/powernv/smp.c
index 40dae96..9ef6b37 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
@@ -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);
}
--
1.9.4
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [v2 PATCH 1/2] powernv/powerpc:Save/Restore additional SPRs for stop4 cpuidle
2017-07-19 8:18 ` [v2 PATCH 1/2] powernv/powerpc:Save/Restore additional SPRs for stop4 cpuidle Gautham R. Shenoy
@ 2017-07-19 9:03 ` Nicholas Piggin
2017-07-19 12:07 ` Michael Ellerman
0 siblings, 1 reply; 7+ messages in thread
From: Nicholas Piggin @ 2017-07-19 9:03 UTC (permalink / raw
To: Gautham R. Shenoy
Cc: Michael Ellerman, Michael Neuling, Vaidyanathan Srinivasan,
Shilpasri G Bhat, Akshay Adiga, linuxppc-dev, linux-kernel
On Wed, 19 Jul 2017 13:48:49 +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>
> ---
> 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(-)
>
> diff --git a/arch/powerpc/include/asm/paca.h b/arch/powerpc/include/asm/paca.h
> index dc88a31..a6b9ea6 100644
> --- a/arch/powerpc/include/asm/paca.h
> +++ b/arch/powerpc/include/asm/paca.h
> @@ -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
> diff --git a/arch/powerpc/kernel/asm-offsets.c b/arch/powerpc/kernel/asm-offsets.c
> index a7b5af3..0262283 100644
> --- a/arch/powerpc/kernel/asm-offsets.c
> +++ b/arch/powerpc/kernel/asm-offsets.c
> @@ -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]);
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.
> @@ -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 <npiggin@gmail.com>
Thanks,
Nick
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [v2 PATCH 2/2] powernv/powerpc: Clear PECE1 in LPCR via stop-api only on Hotplug
2017-07-19 8:18 ` [v2 PATCH 2/2] powernv/powerpc: Clear PECE1 in LPCR via stop-api only on Hotplug Gautham R. Shenoy
@ 2017-07-19 9:09 ` Nicholas Piggin
0 siblings, 0 replies; 7+ messages in thread
From: Nicholas Piggin @ 2017-07-19 9:09 UTC (permalink / raw
To: Gautham R. Shenoy
Cc: Michael Ellerman, Michael Neuling, Vaidyanathan Srinivasan,
Shilpasri G Bhat, Akshay Adiga, linuxppc-dev, linux-kernel
On Wed, 19 Jul 2017 13:48:50 +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, 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 <ego@linux.vnet.ibm.com>
> ---
> arch/powerpc/platforms/powernv/idle.c | 45 ++++++++++++++++++++++++++++++++++-
> arch/powerpc/platforms/powernv/smp.c | 10 --------
> 2 files changed, 44 insertions(+), 11 deletions(-)
>
> diff --git a/arch/powerpc/platforms/powernv/idle.c b/arch/powerpc/platforms/powernv/idle.c
> index 2abee07..a6c69b7 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);
> @@ -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);
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 <npiggin@gmail.com>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [v2 PATCH 1/2] powernv/powerpc:Save/Restore additional SPRs for stop4 cpuidle
2017-07-19 9:03 ` Nicholas Piggin
@ 2017-07-19 12:07 ` Michael Ellerman
2017-07-19 12:43 ` Gautham R Shenoy
0 siblings, 1 reply; 7+ messages in thread
From: Michael Ellerman @ 2017-07-19 12:07 UTC (permalink / raw
To: Nicholas Piggin, Gautham R. Shenoy
Cc: Michael Neuling, Vaidyanathan Srinivasan, Shilpasri G Bhat,
Akshay Adiga, linuxppc-dev, linux-kernel
Nicholas Piggin <npiggin@gmail.com> writes:
> On Wed, 19 Jul 2017 13:48:49 +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>
>> ---
>> 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(-)
>>
>> diff --git a/arch/powerpc/include/asm/paca.h b/arch/powerpc/include/asm/paca.h
>> index dc88a31..a6b9ea6 100644
>> --- a/arch/powerpc/include/asm/paca.h
>> +++ b/arch/powerpc/include/asm/paca.h
>> @@ -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
>> diff --git a/arch/powerpc/kernel/asm-offsets.c b/arch/powerpc/kernel/asm-offsets.c
>> index a7b5af3..0262283 100644
>> --- a/arch/powerpc/kernel/asm-offsets.c
>> +++ b/arch/powerpc/kernel/asm-offsets.c
>> @@ -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]);
>
> 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.
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
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [v2 PATCH 1/2] powernv/powerpc:Save/Restore additional SPRs for stop4 cpuidle
2017-07-19 12:07 ` Michael Ellerman
@ 2017-07-19 12:43 ` Gautham R Shenoy
0 siblings, 0 replies; 7+ messages in thread
From: Gautham R Shenoy @ 2017-07-19 12:43 UTC (permalink / raw
To: Michael Ellerman
Cc: Nicholas Piggin, Gautham R. Shenoy, Michael Neuling,
Vaidyanathan Srinivasan, Shilpasri G Bhat, Akshay Adiga,
linuxppc-dev, linux-kernel
Hi Nicholas, Michael,
On Wed, Jul 19, 2017 at 10:07:05PM +1000, Michael Ellerman wrote:
> Nicholas Piggin <npiggin@gmail.com> writes:
> >> diff --git a/arch/powerpc/kernel/asm-offsets.c b/arch/powerpc/kernel/asm-offsets.c
> >> index a7b5af3..0262283 100644
> >> --- a/arch/powerpc/kernel/asm-offsets.c
> >> +++ b/arch/powerpc/kernel/asm-offsets.c
> >> @@ -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]);
> >
> > 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.
>
> Yes please.
>
> If I see PACA_MMCRA I'm expecting that's paca->mmcra.
Ah, ok. I will fix this.
>
> 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.
> struct stop_sprs
> {
> u64 pid;
> u64 ldbar;
> ...
> }
>
> cheers
>
--
Thanks and Regards
gautham.
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2017-07-19 12:43 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-07-19 8:18 [v2 PATCH 0/2] powerpc: powernv: Enable stop4 via cpuidle Gautham R. Shenoy
2017-07-19 8:18 ` [v2 PATCH 1/2] powernv/powerpc:Save/Restore additional SPRs for stop4 cpuidle Gautham R. Shenoy
2017-07-19 9:03 ` Nicholas Piggin
2017-07-19 12:07 ` Michael Ellerman
2017-07-19 12:43 ` Gautham R Shenoy
2017-07-19 8:18 ` [v2 PATCH 2/2] powernv/powerpc: Clear PECE1 in LPCR via stop-api only on Hotplug Gautham R. Shenoy
2017-07-19 9:09 ` Nicholas Piggin
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.