Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932864AbdC3JTn (ORCPT ); Thu, 30 Mar 2017 05:19:43 -0400 Received: from mx0a-001b2d01.pphosted.com ([148.163.156.1]:56804 "EHLO mx0a-001b2d01.pphosted.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932774AbdC3JTj (ORCPT ); Thu, 30 Mar 2017 05:19:39 -0400 Date: Thu, 30 Mar 2017 14:49:15 +0530 From: Gautham R Shenoy To: Michael Ellerman Cc: "Gautham R. Shenoy" , Michael Neuling , Benjamin Herrenschmidt , "Shreyas B. Prabhu" , Shilpasri G Bhat , Vaidyanathan Srinivasan , Anton Blanchard , Balbir Singh , Akshay Adiga , Nicholas Piggin , Mahesh J Salgaonkar , linuxppc-dev@lists.ozlabs.org, linux-kernel@vger.kernel.org Subject: Re: [v3 PATCH 2/4] powernv:smp: Add busy-wait loop as fall back for CPU-Hotplug Reply-To: ego@linux.vnet.ibm.com References: <261607d15ec471d88a8d31ee50614c64a4018b90.1490194710.git.ego@linux.vnet.ibm.com> <87fuhzdjwv.fsf@concordia.ellerman.id.au> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <87fuhzdjwv.fsf@concordia.ellerman.id.au> User-Agent: Mutt/1.5.23 (2014-03-12) X-TM-AS-GCONF: 00 x-cbid: 17033009-0008-0000-0000-00000785B742 X-IBM-SpamModules-Scores: X-IBM-SpamModules-Versions: BY=3.00006875; HX=3.00000240; KW=3.00000007; PH=3.00000004; SC=3.00000206; SDB=6.00840724; UDB=6.00413823; IPR=6.00618714; BA=6.00005248; NDR=6.00000001; ZLA=6.00000005; ZF=6.00000009; ZB=6.00000000; ZP=6.00000000; ZH=6.00000000; ZU=6.00000002; MB=3.00014860; XFM=3.00000013; UTC=2017-03-30 09:19:30 X-IBM-AV-DETECTION: SAVI=unused REMOTE=unused XFE=unused x-cbparentid: 17033009-0009-0000-0000-00004112F10D Message-Id: <20170330091915.GA12109@in.ibm.com> X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10432:,, definitions=2017-03-30_07:,, signatures=0 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 spamscore=0 suspectscore=0 malwarescore=0 phishscore=0 adultscore=0 bulkscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.0.1-1702020001 definitions=main-1703300083 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 5758 Lines: 166 On Mon, Mar 27, 2017 at 10:43:44PM +1100, Michael Ellerman wrote: > "Gautham R. Shenoy" writes: > > > diff --git a/arch/powerpc/platforms/powernv/idle.c b/arch/powerpc/platforms/powernv/idle.c > > index 419edff..f335e0f 100644 > > --- a/arch/powerpc/platforms/powernv/idle.c > > +++ b/arch/powerpc/platforms/powernv/idle.c > > @@ -283,8 +283,16 @@ unsigned long pnv_cpu_offline(unsigned int cpu) > > } else if ((idle_states & OPAL_PM_SLEEP_ENABLED) || > > (idle_states & OPAL_PM_SLEEP_ENABLED_ER1)) { > > srr1 = power7_sleep(); > > - } else { > > + } else if (idle_states & OPAL_PM_NAP_ENABLED) { > > srr1 = power7_nap(1); > > + } else { > > + /* This is the fallback method. We emulate snooze */ > > + while (!generic_check_cpu_restart(cpu)) { > > Breaks the SMP=n build :/ Hmmm.. Looks like I have missed adding a CONFIG_HOTPLUG_CPU somewhere. > > arch/powerpc/platforms/powernv/idle.c:299:11: error: implicit declaration of function 'generic_check_cpu_restart' [-Werror=implicit-function-declaration] > Could you add the following patch as the PATCH 1 in the series instead of what was present in this series ? It adds a CONFIG_HOTPLUG_CPU around pnv_cpu_offline(). I have compiled it with both all combinations of CONFIG_HOTPLUG_CPU and CONFIG_SMP and it builds fine. ----------------------------x-----x-----x------------------------------- powernv: Move CPU-Offline idle state invocation from smp.c to idle.c Move the piece of code in powernv/smp.c::pnv_smp_cpu_kill_self() which transitions the CPU to the deepest available platform idle state to a new function named pnv_cpu_offline() in powernv/idle.c. The rationale behind this code movement is that the data required to determine the deepest available platform state resides in powernv/idle.c. Reviewed-by: Nicholas Piggin Signed-off-by: Gautham R. Shenoy --- Restricts pnv_cpu_offline() to CONFIG_HOTPLUG_CPU arch/powerpc/include/asm/cpuidle.h | 1 + arch/powerpc/platforms/powernv/idle.c | 27 +++++++++++++++++++++++++++ arch/powerpc/platforms/powernv/powernv.h | 2 -- arch/powerpc/platforms/powernv/smp.c | 18 ++---------------- 4 files changed, 30 insertions(+), 18 deletions(-) diff --git a/arch/powerpc/include/asm/cpuidle.h b/arch/powerpc/include/asm/cpuidle.h index 1557315..4649ca0 100644 --- a/arch/powerpc/include/asm/cpuidle.h +++ b/arch/powerpc/include/asm/cpuidle.h @@ -46,6 +46,7 @@ extern u64 pnv_first_deep_stop_state; +unsigned long pnv_cpu_offline(unsigned int cpu); int validate_psscr_val_mask(u64 *psscr_val, u64 *psscr_mask, u32 flags); static inline void report_invalid_psscr_val(u64 psscr_val, int err) { diff --git a/arch/powerpc/platforms/powernv/idle.c b/arch/powerpc/platforms/powernv/idle.c index 4ee837e..fafafa3 100644 --- a/arch/powerpc/platforms/powernv/idle.c +++ b/arch/powerpc/platforms/powernv/idle.c @@ -265,6 +265,33 @@ static void power9_idle(void) u64 pnv_deepest_stop_psscr_val; u64 pnv_deepest_stop_psscr_mask; +#ifdef CONFIG_HOTPLUG_CPU +/* + * pnv_cpu_offline: A function that puts the CPU into the deepest + * available platform idle state on a CPU-Offline. + */ +unsigned long pnv_cpu_offline(unsigned int cpu) +{ + unsigned long srr1; + + u32 idle_states = pnv_get_supported_cpuidle_states(); + + if (cpu_has_feature(CPU_FTR_ARCH_300)) { + srr1 = power9_idle_stop(pnv_deepest_stop_psscr_val, + pnv_deepest_stop_psscr_mask); + } else if (idle_states & OPAL_PM_WINKLE_ENABLED) { + srr1 = power7_winkle(); + } else if ((idle_states & OPAL_PM_SLEEP_ENABLED) || + (idle_states & OPAL_PM_SLEEP_ENABLED_ER1)) { + srr1 = power7_sleep(); + } else { + srr1 = power7_nap(1); + } + + return srr1; +} +#endif + /* * Power ISA 3.0 idle initialization. * diff --git a/arch/powerpc/platforms/powernv/powernv.h b/arch/powerpc/platforms/powernv/powernv.h index 6130522..6dbc0a1 100644 --- a/arch/powerpc/platforms/powernv/powernv.h +++ b/arch/powerpc/platforms/powernv/powernv.h @@ -18,8 +18,6 @@ static inline void pnv_pci_shutdown(void) { } #endif extern u32 pnv_get_supported_cpuidle_states(void); -extern u64 pnv_deepest_stop_psscr_val; -extern u64 pnv_deepest_stop_psscr_mask; extern void pnv_lpc_init(void); diff --git a/arch/powerpc/platforms/powernv/smp.c b/arch/powerpc/platforms/powernv/smp.c index 8b67e1e..914b456 100644 --- a/arch/powerpc/platforms/powernv/smp.c +++ b/arch/powerpc/platforms/powernv/smp.c @@ -35,6 +35,7 @@ #include #include #include +#include #include "powernv.h" @@ -140,7 +141,6 @@ static void pnv_smp_cpu_kill_self(void) { unsigned int cpu; unsigned long srr1, wmask; - u32 idle_states; /* Standard hot unplug procedure */ local_irq_disable(); @@ -155,8 +155,6 @@ static void pnv_smp_cpu_kill_self(void) if (cpu_has_feature(CPU_FTR_ARCH_207S)) wmask = SRR1_WAKEMASK_P8; - idle_states = pnv_get_supported_cpuidle_states(); - /* 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. @@ -184,19 +182,7 @@ static void pnv_smp_cpu_kill_self(void) kvmppc_set_host_ipi(cpu, 0); ppc64_runlatch_off(); - - if (cpu_has_feature(CPU_FTR_ARCH_300)) { - srr1 = power9_idle_stop(pnv_deepest_stop_psscr_val, - pnv_deepest_stop_psscr_mask); - } else if (idle_states & OPAL_PM_WINKLE_ENABLED) { - srr1 = power7_winkle(); - } else if ((idle_states & OPAL_PM_SLEEP_ENABLED) || - (idle_states & OPAL_PM_SLEEP_ENABLED_ER1)) { - srr1 = power7_sleep(); - } else { - srr1 = power7_nap(1); - } - + srr1 = pnv_cpu_offline(cpu); ppc64_runlatch_on(); /* -- 1.9.4