Re: [PATCH] powerpc/32: Fix critical and debug interrupts on BOOKE

From: Radu Rendec
Date: Wed Aug 04 2021 - 10:01:46 EST


Hi Christophe,

On Wed, 2021-08-04 at 07:52 +0200, Christophe Leroy wrote:
> Le 07/07/2021 à 07:55, Christophe Leroy a écrit :
> > 32 bits BOOKE have special interrupts for debug and other
> > critical events.
>
> Were you able to test this patch ?

I tested it three weeks ago and it works like a charm!

I'm so sorry I forgot to let you know. I got distracted testing the
old ptrace() problem. In fact, I wouldn't have been able to test that
if your interrupts patch hadn't been working.

Thanks,
Radu

> > When handling those interrupts, dedicated registers are saved
> > in the stack frame in addition to the standard registers, leading
> > to a shift of the pt_regs struct.
> >
> > Since commit db297c3b07af ("powerpc/32: Don't save thread.regs on
> > interrupt entry"), the pt_regs struct is expected to be at the
> > same place all the time.
> >
> > Instead of handling a special struct in addition to pt_regs, just
> > add those special registers to struct pt_regs.
> >
> > Reported-by: Radu Rendec <radu.rendec@xxxxxxxxx>
> > Signed-off-by: Christophe Leroy <christophe.leroy@xxxxxxxxxx>
> > Fixes: db297c3b07af ("powerpc/32: Don't save thread.regs on
> > interrupt entry")
> > Cc: stable@xxxxxxxxxxxxxxx
> > ---
> > arch/powerpc/include/asm/ptrace.h | 16 ++++++++++++++++
> > arch/powerpc/kernel/asm-offsets.c | 31
> > ++++++++++++++-----------------
> > arch/powerpc/kernel/head_booke.h | 27
> > +++------------------------
> > 3 files changed, 33 insertions(+), 41 deletions(-)
> >
> > diff --git a/arch/powerpc/include/asm/ptrace.h
> > b/arch/powerpc/include/asm/ptrace.h
> > index 3e5d470a6155..14422e851494 100644
> > --- a/arch/powerpc/include/asm/ptrace.h
> > +++ b/arch/powerpc/include/asm/ptrace.h
> > @@ -70,6 +70,22 @@ struct pt_regs
> > unsigned long __pad[4]; /* Maintain 16 byte
> > interrupt stack alignment */
> > };
> > #endif
> > +#if defined(CONFIG_PPC32) && defined(CONFIG_BOOKE)
> > + struct { /* Must be a multiple of 16 bytes */
> > + unsigned long mas0;
> > + unsigned long mas1;
> > + unsigned long mas2;
> > + unsigned long mas3;
> > + unsigned long mas6;
> > + unsigned long mas7;
> > + unsigned long srr0;
> > + unsigned long srr1;
> > + unsigned long csrr0;
> > + unsigned long csrr1;
> > + unsigned long dsrr0;
> > + unsigned long dsrr1;
> > + };
> > +#endif
> > };
> > #endif
> >
> > diff --git a/arch/powerpc/kernel/asm-offsets.c
> > b/arch/powerpc/kernel/asm-offsets.c
> > index a47eefa09bcb..5bee245d832b 100644
> > --- a/arch/powerpc/kernel/asm-offsets.c
> > +++ b/arch/powerpc/kernel/asm-offsets.c
> > @@ -309,24 +309,21 @@ int main(void)
> > STACK_PT_REGS_OFFSET(STACK_REGS_IAMR, iamr);
> > #endif
> >
> > -#if defined(CONFIG_PPC32)
> > -#if defined(CONFIG_BOOKE) || defined(CONFIG_40x)
> > - DEFINE(EXC_LVL_SIZE, STACK_EXC_LVL_FRAME_SIZE);
> > - DEFINE(MAS0, STACK_INT_FRAME_SIZE+offsetof(struct
> > exception_regs, mas0));
> > +#if defined(CONFIG_PPC32) && defined(CONFIG_BOOKE)
> > + STACK_PT_REGS_OFFSET(MAS0, mas0);
> > /* we overload MMUCR for 44x on MAS0 since they are
> > mutually exclusive */
> > - DEFINE(MMUCR, STACK_INT_FRAME_SIZE+offsetof(struct
> > exception_regs, mas0));
> > - DEFINE(MAS1, STACK_INT_FRAME_SIZE+offsetof(struct
> > exception_regs, mas1));
> > - DEFINE(MAS2, STACK_INT_FRAME_SIZE+offsetof(struct
> > exception_regs, mas2));
> > - DEFINE(MAS3, STACK_INT_FRAME_SIZE+offsetof(struct
> > exception_regs, mas3));
> > - DEFINE(MAS6, STACK_INT_FRAME_SIZE+offsetof(struct
> > exception_regs, mas6));
> > - DEFINE(MAS7, STACK_INT_FRAME_SIZE+offsetof(struct
> > exception_regs, mas7));
> > - DEFINE(_SRR0, STACK_INT_FRAME_SIZE+offsetof(struct
> > exception_regs, srr0));
> > - DEFINE(_SRR1, STACK_INT_FRAME_SIZE+offsetof(struct
> > exception_regs, srr1));
> > - DEFINE(_CSRR0, STACK_INT_FRAME_SIZE+offsetof(struct
> > exception_regs, csrr0));
> > - DEFINE(_CSRR1, STACK_INT_FRAME_SIZE+offsetof(struct
> > exception_regs, csrr1));
> > - DEFINE(_DSRR0, STACK_INT_FRAME_SIZE+offsetof(struct
> > exception_regs, dsrr0));
> > - DEFINE(_DSRR1, STACK_INT_FRAME_SIZE+offsetof(struct
> > exception_regs, dsrr1));
> > -#endif
> > + STACK_PT_REGS_OFFSET(MMUCR, mas0);
> > + STACK_PT_REGS_OFFSET(MAS1, mas1);
> > + STACK_PT_REGS_OFFSET(MAS2, mas2);
> > + STACK_PT_REGS_OFFSET(MAS3, mas3);
> > + STACK_PT_REGS_OFFSET(MAS6, mas6);
> > + STACK_PT_REGS_OFFSET(MAS7, mas7);
> > + STACK_PT_REGS_OFFSET(_SRR0, srr0);
> > + STACK_PT_REGS_OFFSET(_SRR1, srr1);
> > + STACK_PT_REGS_OFFSET(_CSRR0, csrr0);
> > + STACK_PT_REGS_OFFSET(_CSRR1, csrr1);
> > + STACK_PT_REGS_OFFSET(_DSRR0, dsrr0);
> > + STACK_PT_REGS_OFFSET(_DSRR1, dsrr1);
> > #endif
> >
> > /* About the CPU features table */
> > diff --git a/arch/powerpc/kernel/head_booke.h
> > b/arch/powerpc/kernel/head_booke.h
> > index 87b806e8eded..e5503420b6c6 100644
> > --- a/arch/powerpc/kernel/head_booke.h
> > +++ b/arch/powerpc/kernel/head_booke.h
> > @@ -168,20 +168,18 @@ ALT_FTR_SECTION_END_IFSET(CPU_FTR_EMB_HV)
> > /* only on e500mc */
> > #define DBG_STACK_BASE dbgirq_ctx
> >
> > -#define EXC_LVL_FRAME_OVERHEAD (THREAD_SIZE - INT_FRAME_SIZE -
> > EXC_LVL_SIZE)
> > -
> > #ifdef CONFIG_SMP
> > #define BOOKE_LOAD_EXC_LEVEL_STACK(level) \
> > mfspr r8,SPRN_PIR; \
> > slwi r8,r8,2; \
> > addis r8,r8,level##_STACK_BASE@ha; \
> > lwz r8,level##_STACK_BASE@l(r8); \
> > - addi r8,r8,EXC_LVL_FRAME_OVERHEAD;
> > + addi r8,r8,THREAD_SIZE - INT_FRAME_SIZE;
> > #else
> > #define BOOKE_LOAD_EXC_LEVEL_STACK(level) \
> > lis r8,level##_STACK_BASE@ha; \
> > lwz r8,level##_STACK_BASE@l(r8); \
> > - addi r8,r8,EXC_LVL_FRAME_OVERHEAD;
> > + addi r8,r8,THREAD_SIZE - INT_FRAME_SIZE;
> > #endif
> >
> > /*
> > @@ -208,7 +206,7 @@ ALT_FTR_SECTION_END_IFSET(CPU_FTR_EMB_HV)
> > mtmsr r11; \
> > mfspr r11,SPRN_SPRG_THREAD; /* if from user, start at
> > top of */\
> > lwz r11, TASK_STACK - THREAD(r11); /* this thread's
> > kernel stack */\
> > - addi r11,r11,EXC_LVL_FRAME_OVERHEAD; /* allocate stack
> > frame */\
> > + addi r11,r11,THREAD_SIZE - INT_FRAME_SIZE; /* allocate
> > stack frame */\
> > beq 1f;
> > \
> > /* COMING FROM USER MODE
> > */ \
> > stw r9,_CCR(r11); /* save
> > CR */\
> > @@ -516,24 +514,5 @@ ALT_FTR_SECTION_END_IFSET(CPU_FTR_EMB_HV)
> > bl kernel_fp_unavailable_exception;
> > \
> > b interrupt_return
> >
> > -#else /* __ASSEMBLY__ */
> > -struct exception_regs {
> > - unsigned long mas0;
> > - unsigned long mas1;
> > - unsigned long mas2;
> > - unsigned long mas3;
> > - unsigned long mas6;
> > - unsigned long mas7;
> > - unsigned long srr0;
> > - unsigned long srr1;
> > - unsigned long csrr0;
> > - unsigned long csrr1;
> > - unsigned long dsrr0;
> > - unsigned long dsrr1;
> > -};
> > -
> > -/* ensure this structure is always sized to a multiple of the
> > stack alignment */
> > -#define STACK_EXC_LVL_FRAME_SIZE ALIGN(sizeof (struct
> > exception_regs), 16)
> > -
> > #endif /* __ASSEMBLY__ */
> > #endif /* __HEAD_BOOKE_H__ */
> >