From 77719b81e993e0b7be16c8465b38ff5b03740235 Mon Sep 17 00:00:00 2001 From: Andy Ross Date: Wed, 21 Aug 2019 09:52:49 -0700 Subject: [PATCH] arch/xtensa: Clean up fatal error handling Update the xtensa backend to work better with the new fatal error architecture. Move the stack frame dump (xtensa uses a variable-size frame becuase we don't spill unused register windows, so it doesn't strictly have an ESF struct) into z_xtensa_fatal_error(). Unify the older exception logging with the newer one (they'd been sort of glomed together in the recent rework), mostly using the asm2 code but with the exception cause stringification and the PS register field extraction from the older one. Note that one shortcoming is that the way the dispatch code works, we don't have access to the spilled frame from within the spurious error handler, so this can't log the interrupted CPU state. This isn't fixable easily without adding overhead to every interrupt entry, so it needs to stay the way it is for now. Longer term we could exract the caller frame from the window state and figure it out with some elaborate assembly, I guess. Fixes #18140 Signed-off-by: Andy Ross --- arch/xtensa/core/fatal.c | 48 ++++++---------------------------- arch/xtensa/core/xtensa-asm2.c | 39 ++++++++++++++++++--------- include/arch/xtensa/exc.h | 10 ++++++- 3 files changed, 43 insertions(+), 54 deletions(-) diff --git a/arch/xtensa/core/fatal.c b/arch/xtensa/core/fatal.c index 8bcc394aeb0..545df90c958 100644 --- a/arch/xtensa/core/fatal.c +++ b/arch/xtensa/core/fatal.c @@ -9,6 +9,7 @@ #include #include #include +#include #ifdef XT_SIMULATOR #include @@ -25,9 +26,9 @@ }) -#if defined(CONFIG_PRINTK) || defined(CONFIG_LOG) -static char *cause_str(unsigned int cause_code) +char *z_xtensa_exccause(unsigned int cause_code) { +#if defined(CONFIG_PRINTK) || defined(CONFIG_LOG) switch (cause_code) { case 0: return "illegal instruction"; @@ -78,49 +79,16 @@ static char *cause_str(unsigned int cause_code) default: return "unknown/reserved"; } -} +#else + ARG_UNUSED(cause_code); + return "na"; #endif - -static inline unsigned int get_bits(int offset, int num_bits, unsigned int val) -{ - int mask; - - mask = BIT(num_bits) - 1; - val = val >> offset; - return val & mask; } -static void dump_exc_state(void) +void z_xtensa_fatal_error(unsigned int reason, const z_arch_esf_t *esf) { -#if defined(CONFIG_PRINTK) || defined (CONFIG_LOG) - unsigned int cause, ps; - - cause = get_sreg(EXCCAUSE); - ps = get_sreg(PS); - - z_fatal_print("Exception cause %d (%s):", cause, cause_str(cause)); - z_fatal_print(" EPC1 : 0x%08x EXCSAVE1 : 0x%08x EXCVADDR : 0x%08x", - get_sreg(EPC_1), get_sreg(EXCSAVE_1), get_sreg(EXCVADDR)); - - z_fatal_print("Program state (PS):"); - z_fatal_print(" INTLEVEL : %02d EXCM : %d UM : %d RING : %d WOE : %d", - get_bits(0, 4, ps), get_bits(4, 1, ps), - get_bits(5, 1, ps), get_bits(6, 2, ps), - get_bits(18, 1, ps)); -#ifndef __XTENSA_CALL0_ABI__ - z_fatal_print(" OWB : %02d CALLINC : %d", - get_bits(8, 4, ps), get_bits(16, 2, ps)); -#endif -#endif /* CONFIG_PRINTK || CONFIG_LOG */ -} - -XTENSA_ERR_NORET void z_xtensa_fatal_error(unsigned int reason, - const z_arch_esf_t *esf) -{ - dump_exc_state(); - if (esf) { - z_fatal_print("Faulting instruction address = 0x%x", esf->pc); + z_xtensa_dump_stack(esf); } z_fatal_error(reason, esf); diff --git a/arch/xtensa/core/xtensa-asm2.c b/arch/xtensa/core/xtensa-asm2.c index d9e1f14ad8e..02008bf3588 100644 --- a/arch/xtensa/core/xtensa-asm2.c +++ b/arch/xtensa/core/xtensa-asm2.c @@ -90,7 +90,7 @@ void z_irq_spurious(void *arg) } #endif -static void dump_stack(int *stack) +void z_xtensa_dump_stack(const z_arch_esf_t *stack) { int *bsa = *(int **)stack; @@ -128,6 +128,15 @@ static void dump_stack(int *stack) z_fatal_print(" ** SAR %p", (void *)bsa[BSA_SAR_OFF/4]); } +static inline unsigned int get_bits(int offset, int num_bits, unsigned int val) +{ + int mask; + + mask = BIT(num_bits) - 1; + val = val >> offset; + return val & mask; +} + /* The wrapper code lives here instead of in the python script that * generates _xtensa_handle_one_int*(). Seems cleaner, still kind of * ugly. @@ -175,7 +184,7 @@ void *xtensa_excint1_c(int *interrupted_stack) /* Just report it to the console for now */ z_fatal_print(" ** SYSCALL PS %p PC %p", (void *)bsa[BSA_PS_OFF/4], (void *)bsa[BSA_PC_OFF/4]); - dump_stack(interrupted_stack); + z_xtensa_dump_stack(interrupted_stack); /* Xtensa exceptions don't automatically advance PC, * have to skip the SYSCALL instruction manually or @@ -184,26 +193,30 @@ void *xtensa_excint1_c(int *interrupted_stack) bsa[BSA_PC_OFF/4] += 3; } else { + u32_t ps = bsa[BSA_PS_OFF/4]; + __asm__ volatile("rsr.excvaddr %0" : "=r"(vaddr)); - /* Wouldn't hurt to translate EXCCAUSE to a string for - * the user... - */ z_fatal_print(" ** FATAL EXCEPTION"); - z_fatal_print(" ** CPU %d EXCCAUSE %d PS %p PC %p VADDR %p", - z_arch_curr_cpu()->id, cause, (void *)bsa[BSA_PS_OFF/4], - (void *)bsa[BSA_PC_OFF/4], (void *)vaddr); - - dump_stack(interrupted_stack); + z_fatal_print(" ** CPU %d EXCCAUSE %d (%s)", + z_arch_curr_cpu()->id, cause, + z_xtensa_exccause(cause)); + z_fatal_print(" ** PC %p VADDR %p", + (void *)bsa[BSA_PC_OFF/4], (void *)vaddr); + z_fatal_print(" ** PS %p", (void *)bsa[BSA_PS_OFF/4]); + z_fatal_print(" ** (INTLEVEL:%d EXCM: %d UM:%d RING:%d WOE:%d OWB:%d CALLINC:%d)", + get_bits(0, 4, ps), get_bits(4, 1, ps), + get_bits(5, 1, ps), get_bits(6, 2, ps), + get_bits(18, 1, ps), + get_bits(8, 4, ps), get_bits(16, 2, ps)); /* FIXME: legacy xtensa port reported "HW" exception * for all unhandled exceptions, which seems incorrect * as these are software errors. Should clean this * up. - * - * FIXME: interrupted_stack and z_arch_esf_t ought to be the same */ - z_xtensa_fatal_error(K_ERR_CPU_EXCEPTION, NULL); + z_xtensa_fatal_error(K_ERR_CPU_EXCEPTION, + (void *)interrupted_stack); } return z_get_next_switch_handle(interrupted_stack); diff --git a/include/arch/xtensa/exc.h b/include/arch/xtensa/exc.h index e5456ccc6fe..d8dd7720931 100644 --- a/include/arch/xtensa/exc.h +++ b/include/arch/xtensa/exc.h @@ -32,7 +32,15 @@ struct __esf { u32_t pc; }; -typedef struct __esf z_arch_esf_t; +/* Xtensa uses a variable length stack frame depending on how many + * register windows are in use. This isn't a struct type, it just + * matches the register/stack-unit width. + */ +typedef int z_arch_esf_t; + +void z_xtensa_dump_stack(const z_arch_esf_t *stack); +char *z_xtensa_exccause(unsigned int cause_code); + #endif #ifdef __cplusplus