LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Radu Rendec <radu.rendec@gmail.com>
To: Christophe Leroy <christophe.leroy@csgroup.eu>
Cc: linux-kernel@vger.kernel.org, Paul Mackerras <paulus@samba.org>,
	linuxppc-dev@lists.ozlabs.org,
	Michael Ellerman <mpe@ellerman.id.au>,
	Benjamin Herrenschmidt <benh@kernel.crashing.org>
Subject: Re: [PATCH] powerpc/32: Fix critical and debug interrupts on BOOKE
Date: Wed, 04 Aug 2021 10:01:37 -0400	[thread overview]
Message-ID: <32e1428e6b872fe6694cc1251bf19bc4b6d403a9.camel@gmail.com> (raw)
In-Reply-To: <c9f3a3e2-e5ed-6074-b700-99911d925012@csgroup.eu>

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@gmail.com>
> > Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
> > Fixes: db297c3b07af ("powerpc/32: Don't save thread.regs on
> > interrupt entry")
> > Cc: stable@vger.kernel.org
> > ---
> >   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__ */
> > 



  reply	other threads:[~2021-08-04 14:01 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-07  5:55 Christophe Leroy
2021-08-04  5:52 ` Christophe Leroy
2021-08-04 14:01   ` Radu Rendec [this message]
2021-08-13 11:57 ` Michael Ellerman

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=32e1428e6b872fe6694cc1251bf19bc4b6d403a9.camel@gmail.com \
    --to=radu.rendec@gmail.com \
    --cc=benh@kernel.crashing.org \
    --cc=christophe.leroy@csgroup.eu \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linuxppc-dev@lists.ozlabs.org \
    --cc=mpe@ellerman.id.au \
    --cc=paulus@samba.org \
    --subject='Re: [PATCH] powerpc/32: Fix critical and debug interrupts on BOOKE' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).