| Message ID | 20161013021714.6624-1-npiggin@gmail.com (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
Hi Nick, [Updated Shreyas's current e-mail address: "Shreyas B. Prabhu" <shreyasbp@gmail.com>] On Thu, Oct 13, 2016 at 01:17:14PM +1100, 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. This patch looks good to me. I have tested this on POWER8 and verified that we are correctly waking up from nap, sleep and winkle. Acked-by: Gautham R. Shenoy <ego@linux.vnet.ibm.com> > > Cc: Balbir Singh <bsingharora@gmail.com> > Cc: Shreyas B. Prabhu <shreyas@linux.vnet.ibm.com> > Cc: Gautham R. Shenoy <ego@linux.vnet.ibm.com> > Signed-off-by: Nicholas Piggin <npiggin@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) > + > +EXC_REAL_END(system_reset, 0x100, 0x200) > +EXC_VIRT_NONE(0x4100, 0x4200) > + > +#ifdef CONFIG_PPC_P7_NAP > +EXC_COMMON_BEGIN(system_reset_idle_common) > bl pnv_restore_hyp_resource > > li r0,PNV_THREAD_RUNNING > @@ -130,14 +146,8 @@ BEGIN_FTR_SECTION > blt cr3,2f > b pnv_wakeup_loss > 2: b pnv_wakeup_noloss > +#endif > > -9: > -END_FTR_SECTION_IFSET(CPU_FTR_HVMODE | CPU_FTR_ARCH_206) > -#endif /* CONFIG_PPC_P7_NAP */ > - EXCEPTION_PROLOG_PSERIES(PACA_EXGEN, system_reset_common, EXC_STD, > - NOTEST, 0x100) > -EXC_REAL_END(system_reset, 0x100, 0x200) > -EXC_VIRT_NONE(0x4100, 0x4200) > EXC_COMMON(system_reset_common, 0x100, system_reset_exception) > > #ifdef CONFIG_PPC_PSERIES > @@ -817,10 +827,8 @@ EXC_VIRT(trap_0b, 0x4b00, 0x4c00, 0xb00) > TRAMP_KVM(PACA_EXGEN, 0xb00) > EXC_COMMON(trap_0b_common, 0xb00, unknown_exception) > > - > -#define LOAD_SYSCALL_HANDLER(reg) \ > - ld reg,PACAKBASE(r13); \ > - ori reg,reg,(ABS_ADDR(system_call_common))@l; > +#define LOAD_SYSCALL_HANDLER(reg) \ > + __LOAD_HANDLER(reg, system_call_common) > > /* Syscall routine is used twice, in reloc-off and reloc-on paths */ > #define SYSCALL_PSERIES_1 \ > -- > 2.9.3 >
On 13/10/16 13:17, 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). Agreed, that is a good side, we also save the PPR in r9 and set the PPR to HMT_MEDIUM > > 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. > Yep, a much required fixup. I had done a version before your changes > Cc: Balbir Singh <bsingharora@gmail.com> > Cc: Shreyas B. Prabhu <shreyas@linux.vnet.ibm.com> > Cc: Gautham R. Shenoy <ego@linux.vnet.ibm.com> > Signed-off-by: Nicholas Piggin <npiggin@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) \ I wonder if it is a good idea to trap .if (reg == 13); .error "Don't use r13"; .abort; .endif; > + 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) > + > +EXC_REAL_END(system_reset, 0x100, 0x200) > +EXC_VIRT_NONE(0x4100, 0x4200) > + > +#ifdef CONFIG_PPC_P7_NAP > +EXC_COMMON_BEGIN(system_reset_idle_common) > bl pnv_restore_hyp_resource > > li r0,PNV_THREAD_RUNNING > @@ -130,14 +146,8 @@ BEGIN_FTR_SECTION > blt cr3,2f > b pnv_wakeup_loss > 2: b pnv_wakeup_noloss > +#endif > > -9: > -END_FTR_SECTION_IFSET(CPU_FTR_HVMODE | CPU_FTR_ARCH_206) > -#endif /* CONFIG_PPC_P7_NAP */ > - EXCEPTION_PROLOG_PSERIES(PACA_EXGEN, system_reset_common, EXC_STD, > - NOTEST, 0x100) > -EXC_REAL_END(system_reset, 0x100, 0x200) > -EXC_VIRT_NONE(0x4100, 0x4200) > EXC_COMMON(system_reset_common, 0x100, system_reset_exception) > > #ifdef CONFIG_PPC_PSERIES > @@ -817,10 +827,8 @@ EXC_VIRT(trap_0b, 0x4b00, 0x4c00, 0xb00) > TRAMP_KVM(PACA_EXGEN, 0xb00) > EXC_COMMON(trap_0b_common, 0xb00, unknown_exception) > > - > -#define LOAD_SYSCALL_HANDLER(reg) \ > - ld reg,PACAKBASE(r13); \ > - ori reg,reg,(ABS_ADDR(system_call_common))@l; > +#define LOAD_SYSCALL_HANDLER(reg) \ > + __LOAD_HANDLER(reg, system_call_common) > > /* Syscall routine is used twice, in reloc-off and reloc-on paths */ > #define SYSCALL_PSERIES_1 \ > Acked-by: Balbir Singh <bsingharora@gmail.com>
Thanks Balbir and Gautham for testing and reviewing. On Thu, 13 Oct 2016 22:54:32 +1100 Balbir Singh <bsingharora@gmail.com> wrote: > On 13/10/16 13:17, 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). > > Agreed, that is a good side, we also save the PPR in r9 and set the > PPR to HMT_MEDIUM > > > > > 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. > > > > Yep, a much required fixup. I had done a version before your changes Yeah, I didn't mean to push ahead of your patch -- I was halfway through writing the register save patch when I realized yours did not get merged yet. Credit to you for originally finding and fixing it. > > > Cc: Balbir Singh <bsingharora@gmail.com> > > Cc: Shreyas B. Prabhu <shreyas@linux.vnet.ibm.com> > > Cc: Gautham R. Shenoy <ego@linux.vnet.ibm.com> > > Signed-off-by: Nicholas Piggin <npiggin@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) \ > > I wonder if it is a good idea to trap > > .if (reg == 13); > .error "Don't use r13"; > .abort; > .endif; Well... I guess checking is always nice, but r13 is taboo for anything but paca for virtually all assembly in the kernel. So I don't know if the benefit is very large. Now a scripted test to check the objdump might be nice. You could whitelist the few places that set r13, and then give errors for any other code that sets it. Thanks, Nick
On 14/10/16 14:16, Nicholas Piggin wrote: > Thanks Balbir and Gautham for testing and reviewing. > > On Thu, 13 Oct 2016 22:54:32 +1100 > Balbir Singh <bsingharora@gmail.com> wrote: > >> On 13/10/16 13:17, 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). >> >> Agreed, that is a good side, we also save the PPR in r9 and set the >> PPR to HMT_MEDIUM >> >>> >>> 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. >>> >> >> Yep, a much required fixup. I had done a version before your changes > > Yeah, I didn't mean to push ahead of your patch -- I was halfway through > writing the register save patch when I realized yours did not get merged > yet. Credit to you for originally finding and fixing it. > No problems with that >> >>> Cc: Balbir Singh <bsingharora@gmail.com> >>> Cc: Shreyas B. Prabhu <shreyas@linux.vnet.ibm.com> >>> Cc: Gautham R. Shenoy <ego@linux.vnet.ibm.com> >>> Signed-off-by: Nicholas Piggin <npiggin@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) \ >> >> I wonder if it is a good idea to trap >> >> .if (reg == 13); >> .error "Don't use r13"; >> .abort; >> .endif; > > Well... I guess checking is always nice, but r13 is taboo for anything but paca > for virtually all assembly in the kernel. So I don't know if the benefit is very > large. > The thing with macros that take arguments is that there is no indication whats OK and whats not. You are right r13 is taboo, but we could probably still use r13 and do a GET_PACA() later, like we did earlier, so lets drop this > Now a scripted test to check the objdump might be nice. You could whitelist the few > places that set r13, and then give errors for any other code that sets it. > Good idea Balbir Singh
On Thu, 2016-13-10 at 02:17:14 UTC, 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 <bsingharora@gmail.com> > Cc: Shreyas B. Prabhu <shreyas@linux.vnet.ibm.com> > Cc: Gautham R. Shenoy <ego@linux.vnet.ibm.com> > Signed-off-by: Nicholas Piggin <npiggin@gmail.com> > Acked-by: Gautham R. Shenoy <ego@linux.vnet.ibm.com> > Acked-by: Balbir Singh <bsingharora@gmail.com> Applied to powerpc fixes, thanks. https://git.kernel.org/powerpc/c/fb479e44a9e240a23c2d208c2ace23 cheers
On 27/10/16 22:38, Michael Ellerman wrote: > On Thu, 2016-13-10 at 02:17:14 UTC, 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 <bsingharora@gmail.com> >> Cc: Shreyas B. Prabhu <shreyas@linux.vnet.ibm.com> >> Cc: Gautham R. Shenoy <ego@linux.vnet.ibm.com> >> Signed-off-by: Nicholas Piggin <npiggin@gmail.com> >> Acked-by: Gautham R. Shenoy <ego@linux.vnet.ibm.com> >> Acked-by: Balbir Singh <bsingharora@gmail.com> > > Applied to powerpc fixes, thanks. > > https://git.kernel.org/powerpc/c/fb479e44a9e240a23c2d208c2ace23 > BTW, this cannot be directly marked back to stable (ref: http://git.kernel.org/cgit/linux/kernel/git/next/linux-next.git/commit/?id=fb479e44a9e240a23c2d208c2ace23542a47f41c) For stable, we might need https://marc.info/?l=linuxppc-embedded&m=146967183624811&w=4 Balbir Singh
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 <bsingharora@gmail.com> > Cc: Shreyas B. Prabhu <shreyas@linux.vnet.ibm.com> > Cc: Gautham R. Shenoy <ego@linux.vnet.ibm.com> > Signed-off-by: Nicholas Piggin <npiggin@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 ? Thanks, -Mahesh.
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) + +EXC_REAL_END(system_reset, 0x100, 0x200) +EXC_VIRT_NONE(0x4100, 0x4200) + +#ifdef CONFIG_PPC_P7_NAP +EXC_COMMON_BEGIN(system_reset_idle_common) bl pnv_restore_hyp_resource li r0,PNV_THREAD_RUNNING @@ -130,14 +146,8 @@ BEGIN_FTR_SECTION blt cr3,2f b pnv_wakeup_loss 2: b pnv_wakeup_noloss +#endif -9: -END_FTR_SECTION_IFSET(CPU_FTR_HVMODE | CPU_FTR_ARCH_206) -#endif /* CONFIG_PPC_P7_NAP */ - EXCEPTION_PROLOG_PSERIES(PACA_EXGEN, system_reset_common, EXC_STD, - NOTEST, 0x100) -EXC_REAL_END(system_reset, 0x100, 0x200) -EXC_VIRT_NONE(0x4100, 0x4200) EXC_COMMON(system_reset_common, 0x100, system_reset_exception) #ifdef CONFIG_PPC_PSERIES @@ -817,10 +827,8 @@ EXC_VIRT(trap_0b, 0x4b00, 0x4c00, 0xb00) TRAMP_KVM(PACA_EXGEN, 0xb00) EXC_COMMON(trap_0b_common, 0xb00, unknown_exception) - -#define LOAD_SYSCALL_HANDLER(reg) \ - ld reg,PACAKBASE(r13); \ - ori reg,reg,(ABS_ADDR(system_call_common))@l; +#define LOAD_SYSCALL_HANDLER(reg) \ + __LOAD_HANDLER(reg, system_call_common) /* Syscall routine is used twice, in reloc-off and reloc-on paths */ #define SYSCALL_PSERIES_1 \
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 <bsingharora@gmail.com> Cc: Shreyas B. Prabhu <shreyas@linux.vnet.ibm.com> Cc: Gautham R. Shenoy <ego@linux.vnet.ibm.com> Signed-off-by: Nicholas Piggin <npiggin@gmail.com> --- arch/powerpc/include/asm/exception-64s.h | 16 ++++++++++ arch/powerpc/kernel/exceptions-64s.S | 50 ++++++++++++++++++-------------- 2 files changed, 45 insertions(+), 21 deletions(-)