Merge branch 'bugfix/support_coredump_stack_xtensa_v4.4' into 'release/v4.4'

coredump: custom core dump stack is now supported on Xtensa targets (backport v4.4) See merge request espressif/esp-idf!21805
2023-02-03 14:37:16 +08:00 · 2023-02-03 14:37:16 +08:00 · dd2c7f8a59
commit dd2c7f8a59
--- a/components/espcoredump/Kconfig
+++ b/components/espcoredump/Kconfig
@ -82,9 +82,6 @@ menu "Core dump"
    config ESP_COREDUMP_STACK_SIZE
        int "Reserved stack size"
        depends on ESP_COREDUMP_ENABLE
-        # Temporarily disable this feature on Xtensa boards as switching stack
-        # pointer triggers an exception (IDF-2797)
-        depends on IDF_TARGET_ARCH_RISCV
        default 0
        help
            Size of the memory to be reserved for core dump stack. If 0 core dump process will run on
--- a/components/espcoredump/include_core_dump/port/riscv/esp_core_dump_port_impl.h
+++ b/components/espcoredump/include_core_dump/port/riscv/esp_core_dump_port_impl.h
@ -31,23 +31,39 @@
 extern "C" {
 #endif

+/**
+ * @brief Define the type that will be used to describe the current context when
+ * doing a backup of the current stack. This same structure is used to restore the stack.
+ */
+typedef struct {
+    uint32_t sp;
+} core_dump_stack_context_t;
+
 /**
 * @brief Set the stack pointer to the address passed as a parameter.
 * @note This function must be inlined.
 *
 * @param new_sp New stack pointer to set in sp register.
- *
- * @return Former stack pointer address (sp register value).
+ * @param old_ctx CPU context, related to SP, to fill. It will be given back when restoring SP.
 */
-FORCE_INLINE_ATTR void* esp_core_dump_replace_sp(void* new_sp)
+FORCE_INLINE_ATTR void esp_core_dump_replace_sp(void* new_sp, core_dump_stack_context_t* old_ctx)
 {
-    void* current_sp = NULL;
    asm volatile ("mv %0, sp \n\t\
                   mv sp, %1 \n\t\
                  "
-                   : "=&r"(current_sp)
+                   : "=&r"(old_ctx->sp)
                   : "r"(new_sp));
-    return current_sp;
+}
+
+
+/**
+ * @brief Restore the stack pointer that was returned when calling `esp_core_dump_replace_sp()` function.
+ *
+ * @param ctx CPU context, related to SP, to restore.
+ */
+FORCE_INLINE_ATTR void esp_core_dump_restore_sp(core_dump_stack_context_t* old_ctx)
+{
+    asm volatile ("mv sp, %0 \n\t" :: "r"(old_ctx->sp));
 }

 #ifdef __cplusplus
--- a/components/espcoredump/include_core_dump/port/xtensa/esp_core_dump_port_impl.h
+++ b/components/espcoredump/include_core_dump/port/xtensa/esp_core_dump_port_impl.h
@ -41,23 +41,75 @@
 extern "C" {
 #endif

+/**
+ * @brief Define the type that will be used to describe the current context when
+ * doing a backup of the current stack. This same structure is used to restore the stack.
+ */
+typedef struct {
+    uint32_t sp;
+    uint32_t a0;
+    uint32_t ps;
+    uint32_t windowbase;
+    uint32_t windowstart;
+} core_dump_stack_context_t;
+
 /**
 * @brief Set the stack pointer to the address passed as a parameter.
 * @note This function must be inlined.
 *
 * @param new_sp New stack pointer to set in sp register.
- *
- * @return Former stack pointer address (sp register value).
+ * @param old_ctx CPU context, related to SP, to fill. It will be given back when restoring SP.
 */
-FORCE_INLINE_ATTR void* esp_core_dump_replace_sp(void* new_sp)
+FORCE_INLINE_ATTR void esp_core_dump_replace_sp(void* new_sp, core_dump_stack_context_t* old_ctx)
 {
-    void* current_sp = NULL;
-    asm volatile ("mov %0, sp \n\t\
-                  "
-                   : "=&r"(current_sp)
-                   :);
+    /* We have to spill all the windows to the stack first as the new stack pointer
+     * represents a clean new environment. */
+    xthal_window_spill();
+
+    /* Backup the special registers PS, WindowBase and WindowStart. We will need to restore them later */
+    asm volatile ("mov %0, sp \n" \
+                  "mov %1, a0 \n" \
+                  "rsr.ps %2 \n"\
+                  "rsr.windowbase %3 \n"\
+                  "rsr.windowstart %4 \n"\
+                : "=r"(old_ctx->sp),
+                  "=r"(old_ctx->a0),
+                  "=r"(old_ctx->ps),
+                  "=r"(old_ctx->windowbase),
+                  "=r"(old_ctx->windowstart) :);
+
+    /* Set the new stack */
    SET_STACK(new_sp);
-    return current_sp;
+}
+
+
+/**
+ * @brief Restore the stack pointer that was returned when calling `esp_core_dump_replace_sp()` function.
+ *
+ * @param ctx CPU context, related to SP, to restore.
+ */
+FORCE_INLINE_ATTR void esp_core_dump_restore_sp(core_dump_stack_context_t* old_ctx)
+{
+    /* Start by disabling WindowOverflowEnable bit from PS to make sure we won't get a Window Overflow exception
+     * restoring WindowBase and WindowStart registers */
+    const uint32_t ps_woe = old_ctx->ps & ~(PS_WOE_MASK);
+    asm volatile ( \
+        "wsr.ps %0 \n"\
+        "rsync \n"\
+        "wsr.windowbase %1 \n"\
+        "rsync \n"\
+        "wsr.windowstart %2 \n"\
+        "rsync \n"\
+        "mov sp, %3 \n" \
+        "mov a0, %4 \n" \
+        "wsr.ps %5 \n"\
+        "rsync \n"\
+        :: "r"(ps_woe),
+           "r"(old_ctx->windowbase),
+           "r"(old_ctx->windowstart),
+           "r"(old_ctx->sp),
+           "r"(old_ctx->a0),
+           "r"(old_ctx->ps));
 }

 #ifdef __cplusplus
--- a/components/espcoredump/src/core_dump_common.c
+++ b/components/espcoredump/src/core_dump_common.c
@ -63,7 +63,7 @@ extern int _coredump_rtc_fast_end;

 static uint8_t s_coredump_stack[ESP_COREDUMP_STACK_SIZE];
 static uint8_t* s_core_dump_sp = NULL;
-static uint8_t* s_core_dump_backup = NULL;
+static core_dump_stack_context_t s_stack_context;

 /**
 * @brief Function setting up the core dump stack.
@ -83,9 +83,9 @@ FORCE_INLINE_ATTR void esp_core_dump_setup_stack(void)
    /* Replace the stack pointer depending on the architecture, but save the
     * current stack pointer, in order to be able too restore it later.
     * This function must be inlined. */
-    s_core_dump_backup = esp_core_dump_replace_sp(s_core_dump_sp);
+    esp_core_dump_replace_sp(s_core_dump_sp, &s_stack_context);
    ESP_COREDUMP_LOGI("Backing up stack @ %p and use core dump stack @ %p",
-                      s_core_dump_backup, esp_cpu_get_sp());
+                      s_stack_context.sp, esp_cpu_get_sp());
 }

 /**
@ -117,8 +117,8 @@ FORCE_INLINE_ATTR void esp_core_dump_report_stack_usage(void)
        s_core_dump_sp - s_coredump_stack - bytes_free, bytes_free);

    /* Restore the stack pointer. */
-    ESP_COREDUMP_LOGI("Restoring stack @ %p", s_core_dump_backup);
-    esp_core_dump_replace_sp(s_core_dump_backup);
+    ESP_COREDUMP_LOGI("Restoring stack @ %p", s_stack_context.sp);
+    esp_core_dump_restore_sp(&s_stack_context);
 }

 #else
--- a/components/xtensa/include/xt_instr_macros.h
+++ b/components/xtensa/include/xt_instr_macros.h
@ -84,11 +84,11 @@
    do { \
        uint32_t sp = (uint32_t)new_sp - SAVE_AREA_OFFSET; \
        *(uint32_t*)(sp - BASE_AREA_SP_OFFSET) = (uint32_t)new_sp; \
+        const uint32_t mask = ~(PS_WOE_MASK | PS_OWB_MASK | PS_CALLINC_MASK); \
        uint32_t tmp1 = 0, tmp2 = 0; \
        asm volatile ( \
          "rsr.ps %1 \n"\
-          "movi %2, ~" XTSTR( PS_WOE_MASK | PS_OWB_MASK | PS_CALLINC_MASK ) " \n"\
-          "and %1, %1, %2 \n"\
+          "and %1, %1, %3 \n"\
          "wsr.ps %1 \n"\
          "rsync \n"\
          " \n"\
@ -99,6 +99,7 @@
          "wsr.windowstart %1 \n"\
          "rsync \n"\
          " \n"\
+          "movi a0, 0\n" \
          "mov sp, %0 \n"\
          "rsr.ps %1 \n"\
          " \n"\
@ -107,6 +108,6 @@
          "or %1, %1, %2 \n"\
          "wsr.ps %1 \n"\
          "rsync \n"\
-          : "+r"(sp), "+r"(tmp1), "+r"(tmp2)); \
+          : "+r"(sp), "+r"(tmp1), "+r"(tmp2) : "r"(mask)); \
    } while (0);
 #endif // __ASSEMBLER__