Re: [v2 PATCH 2/2] powernv/powerpc: Clear PECE1 in LPCR via stop-api only on Hotplug
From: Nicholas Piggin <npiggin@gmail.com>
Date: 2017-07-19 09:10:12
Also in:
lkml
On Wed, 19 Jul 2017 13:48:50 +0530 "Gautham R. Shenoy" [off-list ref] wrote:
quoted hunk ↗ jump to hunk
From: "Gautham R. Shenoy" <redacted> 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 <redacted> --- 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>