Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753391AbdGSJKM (ORCPT ); Wed, 19 Jul 2017 05:10:12 -0400 Received: from mail-pg0-f66.google.com ([74.125.83.66]:36008 "EHLO mail-pg0-f66.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752788AbdGSJJb (ORCPT ); Wed, 19 Jul 2017 05:09:31 -0400 Date: Wed, 19 Jul 2017 19:09:13 +1000 From: Nicholas Piggin To: "Gautham R. Shenoy" Cc: Michael Ellerman , Michael Neuling , Vaidyanathan Srinivasan , Shilpasri G Bhat , Akshay Adiga , linuxppc-dev@lists.ozlabs.org, linux-kernel@vger.kernel.org Subject: Re: [v2 PATCH 2/2] powernv/powerpc: Clear PECE1 in LPCR via stop-api only on Hotplug Message-ID: <20170719190913.18ca21e5@roar.ozlabs.ibm.com> In-Reply-To: <4142c685fd33835db00eaecedf69186458c1c039.1500452107.git.ego@linux.vnet.ibm.com> References: <4142c685fd33835db00eaecedf69186458c1c039.1500452107.git.ego@linux.vnet.ibm.com> Organization: IBM X-Mailer: Claws Mail 3.14.1 (GTK+ 2.24.31; x86_64-pc-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2958 Lines: 74 On Wed, 19 Jul 2017 13:48:50 +0530 "Gautham R. Shenoy" wrote: > From: "Gautham R. Shenoy" > > 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 > --- > 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