diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig index 7f4f57eb283a0..1a74190d04d8e 100644 --- a/arch/riscv/Kconfig +++ b/arch/riscv/Kconfig @@ -301,14 +301,6 @@ config RISCV_HART_MASK i.e. 128, 129, ..(0x80, 8x81, ..), this can be configured to 63 (0x7f) such that we can extract the bits that start from 0. -config EXTRA_EXCEPTION_INFO - bool "Collect extra exception info" - depends on EXCEPTION_DEBUG - help - This option enables the collection of extra information, such as - register state, when a fault occurs. This information can be useful - to collect for post-mortem analysis and debug of issues. - config RISCV_PMP bool "RISC-V PMP Support" select THREAD_STACK_INFO diff --git a/arch/riscv/core/fatal.c b/arch/riscv/core/fatal.c index 3bf4e57d874a1..d8ac980e74d4b 100644 --- a/arch/riscv/core/fatal.c +++ b/arch/riscv/core/fatal.c @@ -80,12 +80,7 @@ const char *z_riscv_mcause_str(unsigned long cause) FUNC_NORETURN void z_riscv_fatal_error(unsigned int reason, const struct arch_esf *esf) { - z_riscv_fatal_error_csf(reason, esf, NULL); -} - -FUNC_NORETURN void z_riscv_fatal_error_csf(unsigned int reason, const struct arch_esf *esf, - const _callee_saved_t *csf) -{ + __maybe_unused _callee_saved_t *csf = NULL; unsigned long mcause; __asm__ volatile("csrr %0, mcause" : "=r" (mcause)); @@ -122,6 +117,8 @@ FUNC_NORETURN void z_riscv_fatal_error_csf(unsigned int reason, const struct arc LOG_ERR(" mepc: " PR_REG, esf->mepc); LOG_ERR("mstatus: " PR_REG, esf->mstatus); LOG_ERR(""); + + csf = esf->csf; } if (csf != NULL) { @@ -248,6 +245,12 @@ void z_impl_user_fault(unsigned int reason) { struct arch_esf *oops_esf = _current->syscall_frame; +#ifdef CONFIG_EXCEPTION_DEBUG + if (oops_esf != NULL) { + oops_esf->csf = NULL; + } +#endif /* CONFIG_EXCEPTION_DEBUG */ + if (((_current->base.user_options & K_USER) != 0) && reason != K_ERR_STACK_CHK_FAIL) { reason = K_ERR_KERNEL_OOPS; diff --git a/arch/riscv/core/isr.S b/arch/riscv/core/isr.S index dad96974dcc48..80e894f02997d 100644 --- a/arch/riscv/core/isr.S +++ b/arch/riscv/core/isr.S @@ -56,6 +56,20 @@ RV_I( sr s9, ___callee_saved_t_s9_OFFSET(sp) );\ RV_I( sr s10, ___callee_saved_t_s10_OFFSET(sp) );\ RV_I( sr s11, ___callee_saved_t_s11_OFFSET(sp) ) + +/** + * Convenience macro to store the caller-saved registers (CSF) on stack, + * and link it to the `struct arch_esf`. + * 1. Restore the `s0` we saved early in ISR entry so it shows up properly in the CSF. + * 2. Allocate space for CSF on current thread stack + * 3. Save CSF to the allocated stack + * 4. Link the saved CSF to the `struct arch_esf` + */ +#define STORE_CSF_IN_ESF(_esf) \ + lr s0, __struct_arch_esf_s0_OFFSET(sp) ; \ + addi sp, sp, -__callee_saved_t_SIZEOF ; \ + STORE_CALLEE_SAVED() ; \ + sr sp __struct_arch_esf_csf_OFFSET(_esf) #endif /* CONFIG_EXCEPTION_DEBUG */ .macro get_current_cpu dst @@ -78,12 +92,7 @@ GTEXT(__soc_save_context) GTEXT(__soc_restore_context) #endif /* CONFIG_RISCV_SOC_CONTEXT_SAVE */ -#ifdef CONFIG_EXCEPTION_DEBUG -GTEXT(z_riscv_fatal_error_csf) -#else GTEXT(z_riscv_fatal_error) -#endif /* CONFIG_EXCEPTION_DEBUG */ - GTEXT(z_get_next_switch_handle) GTEXT(z_riscv_switch) GTEXT(z_riscv_thread_start) @@ -388,8 +397,18 @@ no_fp: /* increment _current->arch.exception_depth */ * no_reschedule to restore stack. */ mv a0, sp +#ifdef CONFIG_EXCEPTION_DEBUG + STORE_CSF_IN_ESF(a0) ; + + call z_riscv_fault + /* Restore SP if z_riscv_fault returns before jumping to no_reschedule */ + addi sp, sp, __callee_saved_t_SIZEOF + + j no_reschedule +#else la ra, no_reschedule tail z_riscv_fault +#endif /* CONFIG_EXCEPTION_DEBUG */ is_kernel_syscall: /* @@ -459,28 +478,9 @@ do_fault: 1: mv a1, sp #ifdef CONFIG_EXCEPTION_DEBUG - /* - * Restore the s0 we saved early in ISR entry - * so it shows up properly in the CSF. - */ - lr s0, __struct_arch_esf_s0_OFFSET(sp) - - /* Allocate space for caller-saved registers on current thread stack */ - addi sp, sp, -__callee_saved_t_SIZEOF - - /* Save callee-saved registers to be passed as 3rd arg */ - STORE_CALLEE_SAVED() ; - mv a2, sp - -#ifdef CONFIG_EXTRA_EXCEPTION_INFO - /* Store csf's addr into esf (a1 still holds the pointer to the esf at this point) */ - sr a2 __struct_arch_esf_csf_OFFSET(a1) -#endif /* CONFIG_EXTRA_EXCEPTION_INFO */ - - tail z_riscv_fatal_error_csf -#else - tail z_riscv_fatal_error + STORE_CSF_IN_ESF(a1) ; #endif /* CONFIG_EXCEPTION_DEBUG */ + tail z_riscv_fatal_error #if defined(CONFIG_IRQ_OFFLOAD) do_irq_offload: diff --git a/arch/riscv/core/offsets/offsets.c b/arch/riscv/core/offsets/offsets.c index c526ffbaed0d3..1a89831479489 100644 --- a/arch/riscv/core/offsets/offsets.c +++ b/arch/riscv/core/offsets/offsets.c @@ -122,10 +122,6 @@ GEN_OFFSET_STRUCT(arch_esf, s0); GEN_OFFSET_STRUCT(arch_esf, sp); #endif -#ifdef CONFIG_EXTRA_EXCEPTION_INFO -GEN_OFFSET_STRUCT(arch_esf, csf); -#endif /* CONFIG_EXTRA_EXCEPTION_INFO */ - #if defined(CONFIG_RISCV_SOC_CONTEXT_SAVE) GEN_OFFSET_STRUCT(arch_esf, soc_context); #endif @@ -136,6 +132,7 @@ GEN_SOC_OFFSET_SYMS(); GEN_ABSOLUTE_SYM(__struct_arch_esf_SIZEOF, sizeof(struct arch_esf)); #ifdef CONFIG_EXCEPTION_DEBUG +GEN_OFFSET_STRUCT(arch_esf, csf); GEN_ABSOLUTE_SYM(__callee_saved_t_SIZEOF, ROUND_UP(sizeof(_callee_saved_t), ARCH_STACK_PTR_ALIGN)); #endif /* CONFIG_EXCEPTION_DEBUG */ diff --git a/arch/riscv/include/kernel_arch_func.h b/arch/riscv/include/kernel_arch_func.h index a8fc863c75d06..b2ce84bf176c4 100644 --- a/arch/riscv/include/kernel_arch_func.h +++ b/arch/riscv/include/kernel_arch_func.h @@ -74,13 +74,9 @@ arch_switch(void *switch_to, void **switched_from) #endif } -/* Thin wrapper around z_riscv_fatal_error_csf */ FUNC_NORETURN void z_riscv_fatal_error(unsigned int reason, const struct arch_esf *esf); -FUNC_NORETURN void z_riscv_fatal_error_csf(unsigned int reason, const struct arch_esf *esf, - const _callee_saved_t *csf); - static inline bool arch_is_in_isr(void) { #ifdef CONFIG_SMP diff --git a/doc/releases/migration-guide-4.2.rst b/doc/releases/migration-guide-4.2.rst index dbbe2d03ec2d5..3f1575b9af9c0 100644 --- a/doc/releases/migration-guide-4.2.rst +++ b/doc/releases/migration-guide-4.2.rst @@ -760,3 +760,9 @@ Architectures of vector table in RAM. * Renamed :kconfig:option:`CONFIG_DEBUG_INFO` to :kconfig:option:`CONFIG_X86_DEBUG_INFO` to better reflect its purpose. This option is now only available for x86 architecture. + +* RISCV + + * :kconfig:option:`CONFIG_EXTRA_EXCEPTION_INFO` has been removed, the ``*csf`` pointer will be + available in the ``struct arch_esf`` when :kconfig:option:`CONFIG_EXCEPTION_DEBUG` is + enabled. diff --git a/include/zephyr/arch/riscv/exception.h b/include/zephyr/arch/riscv/exception.h index eb3cabebd419f..e02037b66c09d 100644 --- a/include/zephyr/arch/riscv/exception.h +++ b/include/zephyr/arch/riscv/exception.h @@ -45,11 +45,11 @@ struct soc_esf { }; #endif -#ifdef CONFIG_EXTRA_EXCEPTION_INFO +#ifdef CONFIG_EXCEPTION_DEBUG /* Forward declaration */ struct _callee_saved; typedef struct _callee_saved _callee_saved_t; -#endif /* CONFIG_EXTRA_EXCEPTION_INFO */ +#endif /* CONFIG_EXCEPTION_DEBUG */ #if defined(CONFIG_RISCV_SOC_HAS_ISR_STACKING) SOC_ISR_STACKING_ESF_DECLARE; @@ -91,9 +91,9 @@ struct arch_esf { unsigned long sp; /* preserved (user or kernel) stack pointer */ #endif -#ifdef CONFIG_EXTRA_EXCEPTION_INFO +#ifdef CONFIG_EXCEPTION_DEBUG _callee_saved_t *csf; /* pointer to callee-saved-registers */ -#endif /* CONFIG_EXTRA_EXCEPTION_INFO */ +#endif /* CONFIG_EXCEPTION_DEBUG */ #ifdef CONFIG_RISCV_SOC_CONTEXT_SAVE struct soc_esf soc_context; diff --git a/soc/nordic/common/vpr/soc_isr_stacking.h b/soc/nordic/common/vpr/soc_isr_stacking.h index c5f0e7b2762d7..db7d4a215e2c5 100644 --- a/soc/nordic/common/vpr/soc_isr_stacking.h +++ b/soc/nordic/common/vpr/soc_isr_stacking.h @@ -14,6 +14,12 @@ #define VPR_CPU DT_INST(0, nordic_vpr) +#ifdef CONFIG_EXCEPTION_DEBUG +#define ESF_CSF _callee_saved_t *csf +#else +#define ESF_CSF +#endif /* CONFIG_EXCEPTION_DEBUG */ + #if DT_PROP(VPR_CPU, nordic_bus_width) == 64 #define SOC_ISR_STACKING_ESF_DECLARE \ @@ -21,6 +27,7 @@ unsigned long s0; \ unsigned long mstatus; \ struct soc_esf soc_context; \ + ESF_CSF; \ \ unsigned long t2; \ unsigned long ra; \ @@ -43,6 +50,7 @@ unsigned long s0; \ unsigned long mstatus; \ struct soc_esf soc_context; \ + ESF_CSF; \ \ unsigned long ra; \ unsigned long t2; \ diff --git a/tests/arch/riscv/fatal/CMakeLists.txt b/tests/arch/riscv/fatal/CMakeLists.txt index 366f13f660da4..271564066a2fe 100644 --- a/tests/arch/riscv/fatal/CMakeLists.txt +++ b/tests/arch/riscv/fatal/CMakeLists.txt @@ -4,5 +4,5 @@ cmake_minimum_required(VERSION 3.20.0) find_package(Zephyr REQUIRED HINTS $ENV{ZEPHYR_BASE}) project(riscv_fatal) -FILE(GLOB app_sources src/*.c) +FILE(GLOB app_sources src/*.S) target_sources(app PRIVATE ${app_sources}) diff --git a/tests/arch/riscv/fatal/Kconfig b/tests/arch/riscv/fatal/Kconfig new file mode 100644 index 0000000000000..49873029b520f --- /dev/null +++ b/tests/arch/riscv/fatal/Kconfig @@ -0,0 +1,20 @@ +# Copyright (c) 2024 Meta Platforms +# SPDX-License-Identifier: Apache-2.0 + +choice + prompt "Fatal type" + default TEST_RISCV_FATAL_PANIC + +config TEST_RISCV_FATAL_PANIC + bool "Panic induced fault" + help + Tests the error handling via `z_riscv_fatal_error()` + +config TEST_RISCV_FATAL_ILLEGAL_INSTRUCTION + bool "Illegal instruction induced fault" + help + Tests the error handling via `_Fault()` + +endchoice + +source "Kconfig.zephyr" diff --git a/tests/arch/riscv/fatal/src/main.S b/tests/arch/riscv/fatal/src/main.S new file mode 100644 index 0000000000000..304c409d69080 --- /dev/null +++ b/tests/arch/riscv/fatal/src/main.S @@ -0,0 +1,83 @@ +/* + * Copyright (c) 2025 Meta Platforms. + * + * SPDX-License-Identifier: Apache-2.0 + */ + +#include +#include + +/** + * Load up a bunch of known values into registers + * and expect them to show up in the core dump. + * Value is register ABI name kinda spelled out, + * followed by zeros to pad to 32 bits, + * followed by FF00, followed by hex number of the register, + * follwed by the "hex-coded-decimal" number of the register. + */ + +GTEXT(main) +SECTION_FUNC(TEXT, main) + + li ra, 0xDADA0000FF000101 + + /* SP is skipped because it can messes stuff up */ + +#ifndef CONFIG_RISCV_GP + li gp, 0xDADA0000FF000101 +#endif + +#ifndef CONFIG_THREAD_LOCAL_STORAGE + li tp, 0xE2E20000FF000404 +#endif /* CONFIG_THREAD_LOCAL_STORAGE */ + +#ifndef CONFIG_TEST_RISCV_FATAL_PANIC + /* We will load `0` (RV_ECALL_RUNTIME_EXCEPT) to `t0` */ + li t0, 0xD0FF0000FF000505 +#endif /* CONFIG_TEST_RISCV_FATAL_PANIC */ + li t1, 0xD1FF0000FF000606 + li t2, 0xD2FF0000FF000707 + +#ifndef CONFIG_FRAME_POINTER + li s0, 0xC0FF0000FF000808 +#endif /* CONFIG_FRAME_POINTER */ + + li s1, 0xC1FF0000FF000909 + + li a0, 0xA0FF0000FF000A10 + li a1, 0xA1FF0000FF000B11 + li a2, 0xA2FF0000FF000C12 + li a3, 0xA3FF0000FF000D13 + li a4, 0xA4FF0000FF000E14 + li a5, 0xA5FF0000FF000F15 + +#ifndef CONFIG_RISCV_ISA_RV32E + li a6, 0xA6FF0000FF001016 + li a7, 0xA7FF0000FF001117 + + li s2, 0xC2FF0000FF001218 + li s3, 0xC3FF0000FF001319 + li s4, 0xC4FF0000FF001420 + li s5, 0xC5FF0000FF001521 + li s6, 0xC6FF0000FF001622 + li s7, 0xC7FF0000FF001723 + li s8, 0xC8FF0000FF001824 + li s9, 0xC9FF0000FF001925 + li s10, 0xC10FF000FF001A26 + li s11, 0xC11FF000FF001B27 + + li t3, 0xD3FF0000FF001C28 + li t4, 0xD4FF0000FF001D29 + li t5, 0xD5FF0000FF001E30 + li t6, 0xD6FF0000FF001F31 +#endif /* CONFIG_RISCV_ISA_RV32E */ + +#ifdef CONFIG_TEST_RISCV_FATAL_PANIC + li a0, 4 /* K_ERR_KERNEL_PANIC */ + li t0, 0 /* RV_ECALL_RUNTIME_EXCEPT */ + ecall + +#else /* CONFIG_TEST_RISCV_FATAL_ILLEGAL_INSTRUCTION */ + .insn 2, 0 + .insn 2, 0 +#endif /* CONFIG_TEST_RISCV_FATAL_PANIC */ diff --git a/tests/arch/riscv/fatal/src/main.c b/tests/arch/riscv/fatal/src/main.c deleted file mode 100644 index fc75192a22d5e..0000000000000 --- a/tests/arch/riscv/fatal/src/main.c +++ /dev/null @@ -1,62 +0,0 @@ -/* - * Copyright (c) 2024 Meta Platforms - * - * SPDX-License-Identifier: Apache-2.0 - */ - -int main(void) -{ - __asm__( - /** - * Load up a bunch of known values into registers - * and expect them to show up in the core dump. - * Value is register ABI name kinda spelled out, - * followed by zeros to pad to 32 bits, - * followed by FF00, followed by hex number of the register, - * follwed by the "hex-coded-decismal" number of the register. - */ - - /* "RA" -> "DA". Kind of a stretch, but okay. */ - "li x1, 0xDADA0000FF000101\n\t" - - /* Skip stack pointer because it can mess stuff up. */ - /* "li x2, 0\n\t" */ - - /* T0 -> D0. Kinda close in pronunciation. */ - "li x5, 0xD0FF0000FF000505\n\t" - "li x6, 0xD1FF0000FF000606\n\t" - "li x7, 0xD2FF0000FF000707\n\t" - /* S0 -> C0. Kinda close in pronunciation. */ - "li x8, 0xC0FF0000FF000808\n\t" - "li x9, 0xC1FF0000FF000909\n\t" - /* A0 -> A0. Actual match! */ - "li x10, 0xA0FF0000FF000A10\n\t" - "li x11, 0xA1FF0000FF000B11\n\t" - "li x12, 0xA2FF0000FF000C12\n\t" - "li x13, 0xA3FF0000FF000D13\n\t" - "li x14, 0xA4FF0000FF000E14\n\t" - "li x15, 0xA5FF0000FF000F15\n\t" - "li x16, 0xA6FF0000FF001016\n\t" - "li x17, 0xA7FF0000FF001117\n\t" - "li x18, 0xC2FF0000FF001218\n\t" - "li x19, 0xC3FF0000FF001319\n\t" - "li x20, 0xC4FF0000FF001420\n\t" - "li x21, 0xC5FF0000FF001521\n\t" - "li x22, 0xC6FF0000FF001622\n\t" - "li x23, 0xC7FF0000FF001723\n\t" - "li x24, 0xC8FF0000FF001824\n\t" - "li x25, 0xC9FF0000FF001925\n\t" - "li x26, 0xC10FF000FF001A26\n\t" - "li x27, 0xC11FF000FF001B27\n\t" - "li x28, 0xD3FF0000FF001C28\n\t" - "li x29, 0xD4FF0000FF001D29\n\t" - "li x30, 0xD5FF0000FF001E30\n\t" - "li x31, 0xD6FF0000FF001F31\n\t" - /* K_ERR_KERNEL_PANIC */ - "li a0, 4\n\t" - /* RV_ECALL_RUNTIME_EXCEPT */ - "li t0, 0\n\t" - "ecall\n"); - - return 0; -} diff --git a/tests/arch/riscv/fatal/testcase.yaml b/tests/arch/riscv/fatal/testcase.yaml index d2c265419e553..f84c614578d96 100644 --- a/tests/arch/riscv/fatal/testcase.yaml +++ b/tests/arch/riscv/fatal/testcase.yaml @@ -8,7 +8,10 @@ common: platform_allow: - qemu_riscv64 tests: - arch.riscv64.fatal: + arch.riscv64.fatal.panic_sp: + extra_configs: + - CONFIG_FRAME_POINTER=n + - CONFIG_TEST_RISCV_FATAL_PANIC=y harness_config: type: multi_line regex: @@ -27,3 +30,69 @@ tests: - "E: s3: c3ff0000ff001319 s9: c9ff0000ff001925" - "E: s4: c4ff0000ff001420 s10: c10ff000ff001a26" - "E: s5: c5ff0000ff001521 s11: c11ff000ff001b27" + arch.riscv64.fatal.fault_sp: + extra_configs: + - CONFIG_FRAME_POINTER=n + - CONFIG_TEST_RISCV_FATAL_ILLEGAL_INSTRUCTION=y + harness_config: + type: multi_line + regex: + - "E: a0: a0ff0000ff000a10 t0: d0ff0000ff000505" + - "E: a1: a1ff0000ff000b11 t1: d1ff0000ff000606" + - "E: a2: a2ff0000ff000c12 t2: d2ff0000ff000707" + - "E: a3: a3ff0000ff000d13 t3: d3ff0000ff001c28" + - "E: a4: a4ff0000ff000e14 t4: d4ff0000ff001d29" + - "E: a5: a5ff0000ff000f15 t5: d5ff0000ff001e30" + - "E: a6: a6ff0000ff001016 t6: d6ff0000ff001f31" + - "E: a7: a7ff0000ff001117" + - "E: ra: dada0000ff000101" + - "E: s0: c0ff0000ff000808 s6: c6ff0000ff001622" + - "E: s1: c1ff0000ff000909 s7: c7ff0000ff001723" + - "E: s2: c2ff0000ff001218 s8: c8ff0000ff001824" + - "E: s3: c3ff0000ff001319 s9: c9ff0000ff001925" + - "E: s4: c4ff0000ff001420 s10: c10ff000ff001a26" + - "E: s5: c5ff0000ff001521 s11: c11ff000ff001b27" + arch.riscv64.fatal.panic_fp: + extra_configs: + - CONFIG_FRAME_POINTER=y + - CONFIG_TEST_RISCV_FATAL_PANIC=y + harness_config: + type: multi_line + regex: + - "E: a0: 0000000000000004 t0: 0000000000000000" + - "E: a1: a1ff0000ff000b11 t1: d1ff0000ff000606" + - "E: a2: a2ff0000ff000c12 t2: d2ff0000ff000707" + - "E: a3: a3ff0000ff000d13 t3: d3ff0000ff001c28" + - "E: a4: a4ff0000ff000e14 t4: d4ff0000ff001d29" + - "E: a5: a5ff0000ff000f15 t5: d5ff0000ff001e30" + - "E: a6: a6ff0000ff001016 t6: d6ff0000ff001f31" + - "E: a7: a7ff0000ff001117" + - "E: ra: dada0000ff000101" + - "E: s0: \\w+ s6: c6ff0000ff001622" + - "E: s1: c1ff0000ff000909 s7: c7ff0000ff001723" + - "E: s2: c2ff0000ff001218 s8: c8ff0000ff001824" + - "E: s3: c3ff0000ff001319 s9: c9ff0000ff001925" + - "E: s4: c4ff0000ff001420 s10: c10ff000ff001a26" + - "E: s5: c5ff0000ff001521 s11: c11ff000ff001b27" + arch.riscv64.fatal.fault_fp: + extra_configs: + - CONFIG_FRAME_POINTER=y + - CONFIG_TEST_RISCV_FATAL_ILLEGAL_INSTRUCTION=y + harness_config: + type: multi_line + regex: + - "E: a0: a0ff0000ff000a10 t0: d0ff0000ff000505" + - "E: a1: a1ff0000ff000b11 t1: d1ff0000ff000606" + - "E: a2: a2ff0000ff000c12 t2: d2ff0000ff000707" + - "E: a3: a3ff0000ff000d13 t3: d3ff0000ff001c28" + - "E: a4: a4ff0000ff000e14 t4: d4ff0000ff001d29" + - "E: a5: a5ff0000ff000f15 t5: d5ff0000ff001e30" + - "E: a6: a6ff0000ff001016 t6: d6ff0000ff001f31" + - "E: a7: a7ff0000ff001117" + - "E: ra: dada0000ff000101" + - "E: s0: \\w+ s6: c6ff0000ff001622" + - "E: s1: c1ff0000ff000909 s7: c7ff0000ff001723" + - "E: s2: c2ff0000ff001218 s8: c8ff0000ff001824" + - "E: s3: c3ff0000ff001319 s9: c9ff0000ff001925" + - "E: s4: c4ff0000ff001420 s10: c10ff000ff001a26" + - "E: s5: c5ff0000ff001521 s11: c11ff000ff001b27"