Messages in this thread | | | Subject | Re: [PATCH v3] powerpc/traps: Enhance readability for trap types | From | Xiongwei Song <> | Date | Sat, 10 Apr 2021 16:33:49 +0800 |
| |
> On Apr 10, 2021, at 8:04 AM, Michael Ellerman <mpe@ellerman.id.au> wrote: > > Christophe Leroy <christophe.leroy@csgroup.eu> writes: >> Le 08/04/2021 à 16:07, Xiongwei Song a écrit : >>> From: Xiongwei Song <sxwjean@gmail.com> >>> >>> Create a new header named traps.h, define macros to list ppc interrupt >>> types in traps.h, replace the reference of the trap hex values with these >>> macros. > ... >>> diff --git a/arch/powerpc/include/asm/interrupt.h b/arch/powerpc/include/asm/interrupt.h >>> index 7c633896d758..5ce9898bc9a6 100644 >>> --- a/arch/powerpc/include/asm/interrupt.h >>> +++ b/arch/powerpc/include/asm/interrupt.h >>> @@ -8,6 +8,7 @@ >>> #include <asm/ftrace.h> >>> #include <asm/kprobes.h> >>> #include <asm/runlatch.h> >>> +#include <asm/traps.h> >>> >>> struct interrupt_state { >>> #ifdef CONFIG_PPC_BOOK3E_64 >>> @@ -59,7 +60,7 @@ static inline void interrupt_enter_prepare(struct pt_regs *regs, struct interrup >>> * CT_WARN_ON comes here via program_check_exception, >>> * so avoid recursion. >>> */ >>> - if (TRAP(regs) != 0x700) >>> + if (TRAP(regs) != INTERRUPT_PROGRAM) >>> CT_WARN_ON(ct_state() != CONTEXT_KERNEL); >>> } >>> #endif >>> @@ -156,7 +157,8 @@ static inline void interrupt_nmi_enter_prepare(struct pt_regs *regs, struct inte >>> /* Don't do any per-CPU operations until interrupt state is fixed */ >>> #endif >>> /* Allow DEC and PMI to be traced when they are soft-NMI */ >>> - if (TRAP(regs) != 0x900 && TRAP(regs) != 0xf00 && TRAP(regs) != 0x260) { >>> + if (TRAP(regs) != INTERRUPT_DECREMENTER && >>> + TRAP(regs) != INTERRUPT_PERFMON) { >> >> I think too long names hinder readability, see later for suggestions. > > I asked for the longer names :) > > I think they make it easier for people who are less familiar with the > architecture than us to make sense of the names. > > And there's only a couple of cases where it requires splitting a line, > and they could be converted to use switch if we think it's a problem. > >>> diff --git a/arch/powerpc/mm/fault.c b/arch/powerpc/mm/fault.c >>> index 0c0b1c2cfb49..641b3feef7ee 100644 >>> --- a/arch/powerpc/mm/fault.c >>> +++ b/arch/powerpc/mm/fault.c >>> @@ -588,20 +589,24 @@ void __bad_page_fault(struct pt_regs *regs, int sig) >>> /* kernel has accessed a bad area */ >>> >>> switch (TRAP(regs)) { >>> - case 0x300: >>> - case 0x380: >>> - case 0xe00: >>> + case INTERRUPT_DATA_STORAGE: >>> +#ifdef CONFIG_PPC_BOOK3S >>> + case INTERRUPT_DATA_SEGMENT: >>> + case INTERRUPT_H_DATA_STORAGE: >>> +#endif >> >> It would be better to avoid #ifdefs when none where necessary before. > > Yes I agree. > > I think these can all be avoided by defining most of the values > regardless of what platform we're building for. Only the values that > overlap need to be kept behind an ifdef.
Ok.
Regards, Xiongwei
> > cheers
| |