On 2016-11-02 17:57:01 Wed, Nicholas Piggin wrote: > On Wed, 2 Nov 2016 11:34:59 +0530 > Mahesh Jagannath Salgaonkar <mah...@linux.vnet.ibm.com> wrote: > > > On 10/13/2016 07:47 AM, Nicholas Piggin wrote: > > > This patch does a couple of things. First of all, powernv immediately > > > explodes when running a relocated kernel, because the system reset > > > exception for handling sleeps does not do correct relocated branches. > > > > > > Secondly, the sleep handling code trashes the condition and cfar > > > registers, which we would like to preserve for debugging purposes (for > > > non-sleep case exception). > > > > > > This patch changes the exception to use the standard format that saves > > > registers before any tests or branches are made. It adds the test for > > > idle-wakeup as an "extra" to break out of the normal exception path. > > > Then it branches to a relocated idle handler that calls the various > > > idle handling functions. > > > > > > After this patch, POWER8 CPU simulator now boots powernv kernel that is > > > running at non-zero. > > > > > > Cc: Balbir Singh <bsinghar...@gmail.com> > > > Cc: Shreyas B. Prabhu <shre...@linux.vnet.ibm.com> > > > Cc: Gautham R. Shenoy <e...@linux.vnet.ibm.com> > > > Signed-off-by: Nicholas Piggin <npig...@gmail.com> > > > --- > > > arch/powerpc/include/asm/exception-64s.h | 16 ++++++++++ > > > arch/powerpc/kernel/exceptions-64s.S | 50 > > > ++++++++++++++++++-------------- > > > 2 files changed, 45 insertions(+), 21 deletions(-) > > > > > > diff --git a/arch/powerpc/include/asm/exception-64s.h > > > b/arch/powerpc/include/asm/exception-64s.h > > > index 2e4e7d8..84d49b1 100644 > > > --- a/arch/powerpc/include/asm/exception-64s.h > > > +++ b/arch/powerpc/include/asm/exception-64s.h > > > @@ -93,6 +93,10 @@ > > > ld reg,PACAKBASE(r13); /* get high part of &label */ \ > > > ori reg,reg,(FIXED_SYMBOL_ABS_ADDR(label))@l; > > > > > > +#define __LOAD_HANDLER(reg, label) > > > \ > > > + ld reg,PACAKBASE(r13); \ > > > + ori reg,reg,(ABS_ADDR(label))@l; > > > + > > > /* Exception register prefixes */ > > > #define EXC_HV H > > > #define EXC_STD > > > @@ -208,6 +212,18 @@ END_FTR_SECTION_NESTED(ftr,ftr,943) > > > #define kvmppc_interrupt kvmppc_interrupt_pr > > > #endif > > > > > > +#ifdef CONFIG_RELOCATABLE > > > +#define BRANCH_TO_COMMON(reg, label) > > > \ > > > + __LOAD_HANDLER(reg, label); \ > > > + mtctr reg; \ > > > + bctr > > > + > > > +#else > > > +#define BRANCH_TO_COMMON(reg, label) > > > \ > > > + b label > > > + > > > +#endif > > > + > > > #define __KVM_HANDLER_PROLOG(area, n) > > > \ > > > BEGIN_FTR_SECTION_NESTED(947) \ > > > ld r10,area+EX_CFAR(r13); \ > > > diff --git a/arch/powerpc/kernel/exceptions-64s.S > > > b/arch/powerpc/kernel/exceptions-64s.S > > > index 08992f8..e680e84 100644 > > > --- a/arch/powerpc/kernel/exceptions-64s.S > > > +++ b/arch/powerpc/kernel/exceptions-64s.S > > > @@ -95,19 +95,35 @@ __start_interrupts: > > > /* No virt vectors corresponding with 0x0..0x100 */ > > > EXC_VIRT_NONE(0x4000, 0x4100) > > > > > > -EXC_REAL_BEGIN(system_reset, 0x100, 0x200) > > > - SET_SCRATCH0(r13) > > > + > > > #ifdef CONFIG_PPC_P7_NAP > > > -BEGIN_FTR_SECTION > > > - /* Running native on arch 2.06 or later, check if we are > > > - * waking up from nap/sleep/winkle. > > > + /* > > > + * If running native on arch 2.06 or later, check if we are waking up > > > + * from nap/sleep/winkle, and branch to idle handler. > > > */ > > > - mfspr r13,SPRN_SRR1 > > > - rlwinm. r13,r13,47-31,30,31 > > > - beq 9f > > > +#define IDLETEST(n) > > > \ > > > + BEGIN_FTR_SECTION ; \ > > > + mfspr r10,SPRN_SRR1 ; \ > > > + rlwinm. r10,r10,47-31,30,31 ; \ > > > + beq- 1f ; \ > > > + cmpwi cr3,r10,2 ; \ > > > + BRANCH_TO_COMMON(r10, system_reset_idle_common) ; \ > > > +1: > > > \ > > > + END_FTR_SECTION_IFSET(CPU_FTR_HVMODE | CPU_FTR_ARCH_206) > > > +#else > > > +#define IDLETEST NOTEST > > > +#endif > > > > > > - cmpwi cr3,r13,2 > > > - GET_PACA(r13) > > > +EXC_REAL_BEGIN(system_reset, 0x100, 0x200) > > > + SET_SCRATCH0(r13) > > > + EXCEPTION_PROLOG_PSERIES(PACA_EXGEN, system_reset_common, EXC_STD, > > > + IDLETEST, 0x100) > > > > Very sorry for late review. On arch 2.07 and less if we wakeup from > > winkle then last bit of HSPGR0 would be set to 1. Hence before we access > > paca we need to fix it by clearing that bit and that is done in > > pnv_restore_hyp_resource(). But with this patch, we would end up there > > after going through EXCEPTION_PROLOG_PSERIES(). This macro gets the paca > > using GET_PACA(r13) and all the EXCEPTION_PROLOG_* starts > > using/accessing r13/paca without fixing it. Wouldn't this break things > > badly on arch 2.07 and less ? Am I missing anything ? > > Arg, that's a stupid bug :( Thanks for catching it. > > Would something like the following do the trick, do you think? I obviously > was not reaching winkle state in my testing.
Yup, that will work. > > Thanks, > Nick > > --- > arch/powerpc/include/asm/exception-64s.h | 13 +++++++++++-- > arch/powerpc/kernel/exceptions-64s.S | 11 ++++++++--- > 2 files changed, 19 insertions(+), 5 deletions(-) > > diff --git a/arch/powerpc/include/asm/exception-64s.h > b/arch/powerpc/include/asm/exception-64s.h > index 84d49b1..3ce4366 100644 > --- a/arch/powerpc/include/asm/exception-64s.h > +++ b/arch/powerpc/include/asm/exception-64s.h > @@ -158,14 +158,17 @@ BEGIN_FTR_SECTION_NESTED(943) > \ > std ra,offset(r13); \ > END_FTR_SECTION_NESTED(ftr,ftr,943) > > -#define EXCEPTION_PROLOG_0(area) \ > - GET_PACA(r13); \ > +#define EXCEPTION_PROLOG_0_PACA(area) > \ > std r9,area+EX_R9(r13); /* save r9 */ \ > OPT_GET_SPR(r9, SPRN_PPR, CPU_FTR_HAS_PPR); \ > HMT_MEDIUM; \ > std r10,area+EX_R10(r13); /* save r10 - r12 */ \ > OPT_GET_SPR(r10, SPRN_CFAR, CPU_FTR_CFAR) > > +#define EXCEPTION_PROLOG_0(area) \ > + GET_PACA(r13); \ > + EXCEPTION_PROLOG_0_PACA(area) > + > #define __EXCEPTION_PROLOG_1(area, extra, vec) > \ > OPT_SAVE_REG_TO_PACA(area+EX_PPR, r9, CPU_FTR_HAS_PPR); \ > OPT_SAVE_REG_TO_PACA(area+EX_CFAR, r10, CPU_FTR_CFAR); \ > @@ -196,6 +199,12 @@ END_FTR_SECTION_NESTED(ftr,ftr,943) > EXCEPTION_PROLOG_1(area, extra, vec); \ > EXCEPTION_PROLOG_PSERIES_1(label, h); > > +/* Have the PACA in r13 already */ > +#define EXCEPTION_PROLOG_PSERIES_PACA(area, label, h, extra, vec) \ > + EXCEPTION_PROLOG_0_PACA(area); \ > + EXCEPTION_PROLOG_1(area, extra, vec); \ > + EXCEPTION_PROLOG_PSERIES_1(label, h); > + > #define __KVMTEST(h, n) > \ > lbz r10,HSTATE_IN_GUEST(r13); \ > cmpwi r10,0; \ > diff --git a/arch/powerpc/kernel/exceptions-64s.S > b/arch/powerpc/kernel/exceptions-64s.S > index 08ba447..1ba82ea 100644 > --- a/arch/powerpc/kernel/exceptions-64s.S > +++ b/arch/powerpc/kernel/exceptions-64s.S > @@ -116,7 +116,9 @@ EXC_VIRT_NONE(0x4000, 0x4100) > > EXC_REAL_BEGIN(system_reset, 0x100, 0x200) > SET_SCRATCH0(r13) > - EXCEPTION_PROLOG_PSERIES(PACA_EXGEN, system_reset_common, EXC_STD, > + GET_PACA(r13) > + clrrdi r13,r13,1 /* Last bit of HSPRG0 is set if waking from winkle */ > + EXCEPTION_PROLOG_PSERIES_PACA(PACA_EXGEN, system_reset_common, EXC_STD, > IDLETEST, 0x100) > > EXC_REAL_END(system_reset, 0x100, 0x200) > @@ -124,6 +126,9 @@ EXC_VIRT_NONE(0x4100, 0x4200) > > #ifdef CONFIG_PPC_P7_NAP > EXC_COMMON_BEGIN(system_reset_idle_common) > +BEGIN_FTR_SECTION > + GET_PACA(r13) /* Restore HSPRG0 to get the winkle bit in r13 */ > +END_FTR_SECTION_IFCLR(CPU_FTR_ARCH_300) > bl pnv_restore_hyp_resource > > li r0,PNV_THREAD_RUNNING > @@ -169,7 +174,7 @@ EXC_REAL_BEGIN(machine_check, 0x200, 0x300) > SET_SCRATCH0(r13) /* save r13 */ > /* > * Running native on arch 2.06 or later, we may wakeup from winkle > - * inside machine check. If yes, then last bit of HSPGR0 would be set > + * inside machine check. If yes, then last bit of HSPRG0 would be set > * to 1. Hence clear it unconditionally. > */ > GET_PACA(r13) > @@ -388,7 +393,7 @@ EXC_COMMON_BEGIN(machine_check_handle_early) > /* > * Go back to winkle. Please note that this thread was woken up in > * machine check from winkle and have not restored the per-subcore > - * state. Hence before going back to winkle, set last bit of HSPGR0 > + * state. Hence before going back to winkle, set last bit of HSPRG0 > * to 1. This will make sure that if this thread gets woken up > * again at reset vector 0x100 then it will get chance to restore > * the subcore state. > -- > 2.9.3 > -- Mahesh J Salgaonkar
