Thanks Balbir and Gautham for testing and reviewing.
On Thu, 13 Oct 2016 22:54:32 +1100
Balbir Singh <bsinghar...@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 <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) \
> I wonder if it is a good idea to trap
> .if (reg == 13);
> .error "Don't use r13";
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
Now a scripted test to check the objdump might be nice. You could whitelist the
places that set r13, and then give errors for any other code that sets it.