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 <andrew.j.ross@intel.com>

arch/xtensa/core/fatal.c

@@ -9,6 +9,7 @@
 #include <inttypes.h>
 #include <kernel_arch_data.h>
 #include <xtensa/specreg.h>
+#include <xtensa-asm2-context.h>
 
 #ifdef XT_SIMULATOR
 #include <xtensa/simcall.h>
@@ -25,9 +26,9 @@
 	})
 
 
-#if defined(CONFIG_PRINTK) || defined(CONFIG_LOG)
+char *z_xtensa_exccause(unsigned int cause_code)
-static char *cause_str(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);

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_fatal_print(" ** CPU %d EXCCAUSE %d (%s)",
-		       z_arch_curr_cpu()->id, cause, (void *)bsa[BSA_PS_OFF/4],
+			      z_arch_curr_cpu()->id, cause,
-		       (void *)bsa[BSA_PC_OFF/4], (void *)vaddr);
+			      z_xtensa_exccause(cause));
-
+		z_fatal_print(" **  PC %p VADDR %p",
-		dump_stack(interrupted_stack);
+			      (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);

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