| Message ID | 20210525183302.56293-12-mark.rutland@arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | arm64: entry: migrate more code to C | expand |
On Tue, May 25, 2021 at 07:32:53PM +0100, Mark Rutland wrote: > In subsequent patches we'll rework the way bad_mode is called by > exception entry code. In preparation for this, let's move bad_mode() > itself into entry-common.c. > > Let's also mark it as noinstr (e.g. to prevent it being kprobed), and > let's also make the `handler` array a local variable, as this is only > use by bad_mode(), and will be removed entirely in a subsequent patch. > > There should be no functional change as a result of this patch. > > Signed-off-by: Mark Rutland <mark.rutland@arm.com> > Reviewed-by: Joey Gouly <joey.gouly@arm.com> > Cc: Catalin Marinas <catalin.marinas@arm.com> > Cc: James Morse <james.morse@arm.com> > Cc: Marc Zyngier <maz@kernel.org> > Cc: Will Deacon <will@kernel.org> > --- > arch/arm64/kernel/entry-common.c | 27 +++++++++++++++++++++++++++ > arch/arm64/kernel/traps.c | 25 ------------------------- > 2 files changed, 27 insertions(+), 25 deletions(-) > > diff --git a/arch/arm64/kernel/entry-common.c b/arch/arm64/kernel/entry-common.c > index 9c0ed05b98c4..25531a0b547e 100644 > --- a/arch/arm64/kernel/entry-common.c > +++ b/arch/arm64/kernel/entry-common.c > @@ -22,6 +22,7 @@ > #include <asm/processor.h> > #include <asm/stacktrace.h> > #include <asm/sysreg.h> > +#include <asm/system_misc.h> > > /* > * This is intended to match the logic in irqentry_enter(), handling the kernel > @@ -159,6 +160,32 @@ static void do_interrupt_handler(struct pt_regs *regs, > extern void (*handle_arch_irq)(struct pt_regs *); > extern void (*handle_arch_fiq)(struct pt_regs *); > > +/* > + * bad_mode handles the impossible case in the exception vector. This is always > + * fatal. > + */ > +asmlinkage void noinstr bad_mode(struct pt_regs *regs, int reason, unsigned int esr) > +{ > + const char *handler[] = { > + "Synchronous Abort", > + "IRQ", > + "FIQ", > + "Error" > + }; If you're rejigging this array anyway, maybe initialising it as: [BAD_SYNC] = "Synchronous Abort", [BAD_IRQ] = ... might make it even clearer. Up to you. Will
On Fri, Jun 04, 2021 at 05:57:55PM +0100, Will Deacon wrote: > On Tue, May 25, 2021 at 07:32:53PM +0100, Mark Rutland wrote: > > In subsequent patches we'll rework the way bad_mode is called by > > exception entry code. In preparation for this, let's move bad_mode() > > itself into entry-common.c. > > > > Let's also mark it as noinstr (e.g. to prevent it being kprobed), and > > let's also make the `handler` array a local variable, as this is only > > use by bad_mode(), and will be removed entirely in a subsequent patch. > > > > There should be no functional change as a result of this patch. > > > > Signed-off-by: Mark Rutland <mark.rutland@arm.com> > > Reviewed-by: Joey Gouly <joey.gouly@arm.com> > > Cc: Catalin Marinas <catalin.marinas@arm.com> > > Cc: James Morse <james.morse@arm.com> > > Cc: Marc Zyngier <maz@kernel.org> > > Cc: Will Deacon <will@kernel.org> > > --- > > arch/arm64/kernel/entry-common.c | 27 +++++++++++++++++++++++++++ > > arch/arm64/kernel/traps.c | 25 ------------------------- > > 2 files changed, 27 insertions(+), 25 deletions(-) > > > > diff --git a/arch/arm64/kernel/entry-common.c b/arch/arm64/kernel/entry-common.c > > index 9c0ed05b98c4..25531a0b547e 100644 > > --- a/arch/arm64/kernel/entry-common.c > > +++ b/arch/arm64/kernel/entry-common.c > > @@ -22,6 +22,7 @@ > > #include <asm/processor.h> > > #include <asm/stacktrace.h> > > #include <asm/sysreg.h> > > +#include <asm/system_misc.h> > > > > /* > > * This is intended to match the logic in irqentry_enter(), handling the kernel > > @@ -159,6 +160,32 @@ static void do_interrupt_handler(struct pt_regs *regs, > > extern void (*handle_arch_irq)(struct pt_regs *); > > extern void (*handle_arch_fiq)(struct pt_regs *); > > > > +/* > > + * bad_mode handles the impossible case in the exception vector. This is always > > + * fatal. > > + */ > > +asmlinkage void noinstr bad_mode(struct pt_regs *regs, int reason, unsigned int esr) > > +{ > > + const char *handler[] = { > > + "Synchronous Abort", > > + "IRQ", > > + "FIQ", > > + "Error" > > + }; > > If you're rejigging this array anyway, maybe initialising it as: > > [BAD_SYNC] = "Synchronous Abort", > [BAD_IRQ] = ... > > might make it even clearer. Up to you. Since I'm removing the BAD_* definitions (and the array) in a latter patch, I'd prefer to leave this as-is here so that this clearly has no functional change. Thanks, Mark.
On Fri, Jun 04, 2021 at 06:42:16PM +0100, Mark Rutland wrote: > On Fri, Jun 04, 2021 at 05:57:55PM +0100, Will Deacon wrote: > > On Tue, May 25, 2021 at 07:32:53PM +0100, Mark Rutland wrote: > > > In subsequent patches we'll rework the way bad_mode is called by > > > exception entry code. In preparation for this, let's move bad_mode() > > > itself into entry-common.c. > > > > > > Let's also mark it as noinstr (e.g. to prevent it being kprobed), and > > > let's also make the `handler` array a local variable, as this is only > > > use by bad_mode(), and will be removed entirely in a subsequent patch. > > > > > > There should be no functional change as a result of this patch. > > > > > > Signed-off-by: Mark Rutland <mark.rutland@arm.com> > > > Reviewed-by: Joey Gouly <joey.gouly@arm.com> > > > Cc: Catalin Marinas <catalin.marinas@arm.com> > > > Cc: James Morse <james.morse@arm.com> > > > Cc: Marc Zyngier <maz@kernel.org> > > > Cc: Will Deacon <will@kernel.org> > > > --- > > > arch/arm64/kernel/entry-common.c | 27 +++++++++++++++++++++++++++ > > > arch/arm64/kernel/traps.c | 25 ------------------------- > > > 2 files changed, 27 insertions(+), 25 deletions(-) > > > > > > diff --git a/arch/arm64/kernel/entry-common.c b/arch/arm64/kernel/entry-common.c > > > index 9c0ed05b98c4..25531a0b547e 100644 > > > --- a/arch/arm64/kernel/entry-common.c > > > +++ b/arch/arm64/kernel/entry-common.c > > > @@ -22,6 +22,7 @@ > > > #include <asm/processor.h> > > > #include <asm/stacktrace.h> > > > #include <asm/sysreg.h> > > > +#include <asm/system_misc.h> > > > > > > /* > > > * This is intended to match the logic in irqentry_enter(), handling the kernel > > > @@ -159,6 +160,32 @@ static void do_interrupt_handler(struct pt_regs *regs, > > > extern void (*handle_arch_irq)(struct pt_regs *); > > > extern void (*handle_arch_fiq)(struct pt_regs *); > > > > > > +/* > > > + * bad_mode handles the impossible case in the exception vector. This is always > > > + * fatal. > > > + */ > > > +asmlinkage void noinstr bad_mode(struct pt_regs *regs, int reason, unsigned int esr) > > > +{ > > > + const char *handler[] = { > > > + "Synchronous Abort", > > > + "IRQ", > > > + "FIQ", > > > + "Error" > > > + }; > > > > If you're rejigging this array anyway, maybe initialising it as: > > > > [BAD_SYNC] = "Synchronous Abort", > > [BAD_IRQ] = ... > > > > might make it even clearer. Up to you. > > Since I'm removing the BAD_* definitions (and the array) in a latter > patch, I'd prefer to leave this as-is here so that this clearly has no > functional change. Sure thing, I only spotted that after I'd already replied to this. Will
diff --git a/arch/arm64/kernel/entry-common.c b/arch/arm64/kernel/entry-common.c index 9c0ed05b98c4..25531a0b547e 100644 --- a/arch/arm64/kernel/entry-common.c +++ b/arch/arm64/kernel/entry-common.c @@ -22,6 +22,7 @@ #include <asm/processor.h> #include <asm/stacktrace.h> #include <asm/sysreg.h> +#include <asm/system_misc.h> /* * This is intended to match the logic in irqentry_enter(), handling the kernel @@ -159,6 +160,32 @@ static void do_interrupt_handler(struct pt_regs *regs, extern void (*handle_arch_irq)(struct pt_regs *); extern void (*handle_arch_fiq)(struct pt_regs *); +/* + * bad_mode handles the impossible case in the exception vector. This is always + * fatal. + */ +asmlinkage void noinstr bad_mode(struct pt_regs *regs, int reason, unsigned int esr) +{ + const char *handler[] = { + "Synchronous Abort", + "IRQ", + "FIQ", + "Error" + }; + + arm64_enter_nmi(regs); + + console_verbose(); + + pr_crit("Bad mode in %s handler detected on CPU%d, code 0x%08x -- %s\n", + handler[reason], smp_processor_id(), esr, + esr_get_class_string(esr)); + + __show_regs(regs); + panic("bad mode"); +} + + #ifdef CONFIG_ARM64_ERRATUM_1463225 static DEFINE_PER_CPU(int, __in_cortex_a76_erratum_1463225_wa); diff --git a/arch/arm64/kernel/traps.c b/arch/arm64/kernel/traps.c index 5fd12d19ef4b..7def18ff02e2 100644 --- a/arch/arm64/kernel/traps.c +++ b/arch/arm64/kernel/traps.c @@ -45,13 +45,6 @@ #include <asm/system_misc.h> #include <asm/sysreg.h> -static const char *handler[] = { - "Synchronous Abort", - "IRQ", - "FIQ", - "Error" -}; - int show_unhandled_signals = 0; static void dump_kernel_instr(const char *lvl, struct pt_regs *regs) @@ -751,24 +744,6 @@ const char *esr_get_class_string(u32 esr) } /* - * bad_mode handles the impossible case in the exception vector. This is always - * fatal. - */ -asmlinkage void notrace bad_mode(struct pt_regs *regs, int reason, unsigned int esr) -{ - arm64_enter_nmi(regs); - - console_verbose(); - - pr_crit("Bad mode in %s handler detected on CPU%d, code 0x%08x -- %s\n", - handler[reason], smp_processor_id(), esr, - esr_get_class_string(esr)); - - __show_regs(regs); - panic("bad mode"); -} - -/* * bad_el0_sync handles unexpected, but potentially recoverable synchronous * exceptions taken from EL0. Unlike bad_mode, this returns. */