From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from va3outboundpool.messaging.microsoft.com (va3ehsobe004.messaging.microsoft.com [216.32.180.14]) (using TLSv1 with cipher AES128-SHA (128/128 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 3FB072C00B0 for ; Mon, 17 Mar 2014 21:50:41 +1100 (EST) Date: Mon, 17 Mar 2014 18:50:32 +0800 From: Chenhui Zhao To: Scott Wood Subject: Re: [PATCH 8/9] powerpc/85xx: add save/restore functions for core registers Message-ID: <20140317105032.GC6204@localhost.localdomain> References: <1394168285-32275-1-git-send-email-chenhui.zhao@freescale.com> <1394168285-32275-8-git-send-email-chenhui.zhao@freescale.com> <1394585114.13761.112.camel@snotra.buserror.net> <20140312094237.GG4706@localhost.localdomain> <1394838105.12479.144.camel@snotra.buserror.net> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" In-Reply-To: <1394838105.12479.144.camel@snotra.buserror.net> Cc: linuxppc-dev@lists.ozlabs.org, linux-kernel@vger.kernel.org, Jason.Jin@freescale.com, Wang Dongsheng-B40534 List-Id: Linux on PowerPC Developers Mail List List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , On Fri, Mar 14, 2014 at 06:01:45PM -0500, Scott Wood wrote: > On Wed, 2014-03-12 at 17:42 +0800, Chenhui Zhao wrote: > > On Tue, Mar 11, 2014 at 07:45:14PM -0500, Scott Wood wrote: > > > On Fri, 2014-03-07 at 12:58 +0800, Chenhui Zhao wrote: > > > > From: Wang Dongsheng > > > > > > > > Add booke_cpu_state_save() and booke_cpu_state_restore() functions which can be > > > > used to save/restore CPU's registers in the case of deep sleep and hibernation. > > > > > > > > Supported processors: E6500, E5500, E500MC, E500v2 and E500v1. > > > > > > > > Signed-off-by: Wang Dongsheng > > > > Signed-off-by: Chenhui Zhao > > > > --- > > > > arch/powerpc/include/asm/booke_save_regs.h | 96 ++++++++ > > > > arch/powerpc/kernel/Makefile | 1 + > > > > arch/powerpc/kernel/booke_save_regs.S | 361 ++++++++++++++++++++++++++++ > > > > 3 files changed, 458 insertions(+), 0 deletions(-) > > > > create mode 100644 arch/powerpc/include/asm/booke_save_regs.h > > > > create mode 100644 arch/powerpc/kernel/booke_save_regs.S > > > > > > > > diff --git a/arch/powerpc/include/asm/booke_save_regs.h b/arch/powerpc/include/asm/booke_save_regs.h > > > > new file mode 100644 > > > > index 0000000..87c357a > > > > --- /dev/null > > > > +++ b/arch/powerpc/include/asm/booke_save_regs.h > > > > @@ -0,0 +1,96 @@ > > > > +/* > > > > + * Save/restore e500 series core registers > > > > > > Filename says booke, comment says e500. > > > > > > Filename and comment also fail to point out that this is specifically > > > for standby/suspend, not for hibernate which is implemented in > > > swsusp_booke.S/swsusp_asm64.S. > > > > Sorry for inconsistency. Will changes e500 to booke. > > Hibernation and suspend can share the code. > > Maybe they could, but AFAICT this patchset doesn't make that happen -- > and I'm not convinced that the churn would be worthwhile. Note that > swsusp_asm64.S is not just for booke, so most of that file would not be > going away if you did make such a change. OK. Let's put Hibernation aside, and change the code just for suspend. > > I also don't like the way it looks like booke_save_regs.S is a booke > version of ppc_save_regs.S, even though they serve different purposes > and ppc_save_regs.S is still relevant to booke. > > > > > + * Software-Use Registers > > > > + * SPRG1 0x260 (dw * 76), 64-bit need to save. > > > > + * SPRG3 0x268 (dw * 77), 32-bit need to save. > > > > > > What about "CPU and NUMA node for VDSO getcpu" on 64-bit? Currently > > > SPRG3, but it will need to change for critical interrupt support. > > > > > > > + * MMU Registers > > > > + * PID0 - PID2 0x270 ~ 0x280 (dw * 78 ~ dw * 80) > > > > > > PID1/PID2 are e500v1/v2 only -- and Linux doesn't use them outside of > > > KVM (and you're not in KVM when you're running this code). > > > > > > Are we ever going to have a non-zero PID at this point? > > > > I incline to the view that saving all registers regardless of used or > > unused. The good point is that it can be compliant to the future > > changes of the usage of registers. > > > > What do you think? > > I agree to a certain extent, but balance it with the complexity of > dealing with registers that don't exist on all booke chips. If they > don't really need to be saved, why go through the hassle of conditional > code? I agree. For these registers, I'll check if they are really needed. -Chenhui