From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751806AbdGZKi6 (ORCPT ); Wed, 26 Jul 2017 06:38:58 -0400 Received: from mail-pg0-f67.google.com ([74.125.83.67]:38073 "EHLO mail-pg0-f67.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751611AbdGZKi5 (ORCPT ); Wed, 26 Jul 2017 06:38:57 -0400 Date: Wed, 26 Jul 2017 20:38:38 +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: [v3 PATCH 1/2] powernv/powerpc:Save/Restore additional SPRs for stop4 cpuidle Message-ID: <20170726203838.5a2b8dc4@roar.ozlabs.ibm.com> In-Reply-To: <57ed82bcd92cd4d0af7ae5113710d597a331c433.1500631040.git.ego@linux.vnet.ibm.com> References: <57ed82bcd92cd4d0af7ae5113710d597a331c433.1500631040.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 On Fri, 21 Jul 2017 16:11:37 +0530 "Gautham R. Shenoy" wrote: > From: "Gautham R. Shenoy" > > 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 Looks good to me. Keeping in mind we need to tidy up and unify all this SPR handling and save/restore etc. in the longer term. Reviewed-by: Nicholas Piggin > --- > v2-->v3: > - Use a structure instead of an array for the stop sprs save area. > - Name the offsets into the paca->stop_sprs as STOP_XXX instead of > PACA_XXX. > - Add comments in the assembly code explaining why saving/restoring > is not needed on POWER8. > v1-->v2: > No change > > arch/powerpc/include/asm/cpuidle.h | 11 +++++++ > arch/powerpc/include/asm/paca.h | 7 ++++ > arch/powerpc/kernel/asm-offsets.c | 8 +++++ > arch/powerpc/kernel/idle_book3s.S | 65 ++++++++++++++++++++++++++++++++++++-- > 4 files changed, 89 insertions(+), 2 deletions(-) > > diff --git a/arch/powerpc/include/asm/cpuidle.h b/arch/powerpc/include/asm/cpuidle.h > index 52586f9..8a174cb 100644 > --- a/arch/powerpc/include/asm/cpuidle.h > +++ b/arch/powerpc/include/asm/cpuidle.h > @@ -67,6 +67,17 @@ > #define ERR_DEEP_STATE_ESL_MISMATCH -2 > > #ifndef __ASSEMBLY__ > +/* Additional SPRs that need to be saved/restored during stop */ > +struct stop_sprs { > + u64 pid; > + u64 ldbar; > + u64 fscr; > + u64 hfscr; > + u64 mmcr1; > + u64 mmcr2; > + u64 mmcra; > +}; > + > extern u32 pnv_fastsleep_workaround_at_entry[]; > extern u32 pnv_fastsleep_workaround_at_exit[]; > > diff --git a/arch/powerpc/include/asm/paca.h b/arch/powerpc/include/asm/paca.h > index dc88a31..04b60af 100644 > --- a/arch/powerpc/include/asm/paca.h > +++ b/arch/powerpc/include/asm/paca.h > @@ -31,6 +31,7 @@ > #endif > #include > #include > +#include > > register struct paca_struct *local_paca asm("r13"); > > @@ -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. > + */ > + struct stop_sprs 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..e2a48df 100644 > --- a/arch/powerpc/kernel/asm-offsets.c > +++ b/arch/powerpc/kernel/asm-offsets.c > @@ -743,6 +743,14 @@ 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); > +#define STOP_SPR(x, f) OFFSET(x, paca_struct, stop_sprs.f) > + STOP_SPR(STOP_PID, pid); > + STOP_SPR(STOP_LDBAR, ldbar); > + STOP_SPR(STOP_FSCR, fscr); > + STOP_SPR(STOP_HFSCR, hfscr); > + STOP_SPR(STOP_MMCR1, mmcr1); > + STOP_SPR(STOP_MMCR2, mmcr2); > + STOP_SPR(STOP_MMCRA, mmcra); > #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..5e6af97 100644 > --- a/arch/powerpc/kernel/idle_book3s.S > +++ b/arch/powerpc/kernel/idle_book3s.S > @@ -84,7 +84,61 @@ ALT_FTR_SECTION_END_IFSET(CPU_FTR_ARCH_300) > std r3,_WORT(r1) > mfspr r3,SPRN_WORC > std r3,_WORC(r1) > +/* > + * On POWER9, there are idle states such as stop4, invoked via cpuidle, > + * that lose hypervisor resources. In such cases, we need to save > + * additional SPRs before entering those idle states so that they can > + * be restored to their older values on wakeup from the idle state. > + * > + * On POWER8, the only such deep idle state is winkle which is used > + * only in the context of CPU-Hotplug, where these additional SPRs are > + * reinitiazed to a sane value. Hence there is no need to save/restore > + * these SPRs. > + */ > +BEGIN_FTR_SECTION > + blr > +END_FTR_SECTION_IFCLR(CPU_FTR_ARCH_300) > + > +power9_save_additional_sprs: > + mfspr r3, SPRN_PID > + mfspr r4, SPRN_LDBAR > + std r3, STOP_PID(r13) > + std r4, STOP_LDBAR(r13) > > + mfspr r3, SPRN_FSCR > + mfspr r4, SPRN_HFSCR > + std r3, STOP_FSCR(r13) > + std r4, STOP_HFSCR(r13) > + > + mfspr r3, SPRN_MMCRA > + mfspr r4, SPRN_MMCR1 > + std r3, STOP_MMCRA(r13) > + std r4, STOP_MMCR1(r13) > + > + mfspr r3, SPRN_MMCR2 > + std r3, STOP_MMCR2(r13) > + blr > + > +power9_restore_additional_sprs: > + ld r3,_LPCR(r1) > + ld r4, STOP_PID(r13) > + mtspr SPRN_LPCR,r3 > + mtspr SPRN_PID, r4 > + > + ld r3, STOP_LDBAR(r13) > + ld r4, STOP_FSCR(r13) > + mtspr SPRN_LDBAR, r3 > + mtspr SPRN_FSCR, r4 > + > + ld r3, STOP_HFSCR(r13) > + ld r4, STOP_MMCRA(r13) > + mtspr SPRN_HFSCR, r3 > + mtspr SPRN_MMCRA, r4 > + /* We have already restored PACA_MMCR0 */ > + ld r3, STOP_MMCR1(r13) > + ld r4, STOP_MMCR2(r13) > + mtspr SPRN_MMCR1, r3 > + mtspr SPRN_MMCR2, r4 > blr > > /* > @@ -790,9 +844,16 @@ no_segments: > mtctr r12 > bctrl > > +/* > + * On POWER9, we can come here on wakeup from a cpuidle stop state. > + * Hence restore the additional SPRs to the saved value. > + * > + * On POWER8, we come here only on winkle. Since winkle is used > + * only in the case of CPU-Hotplug, we don't need to restore > + * the additional SPRs. > + */ > 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: >