system: fix SET_STACK macro crashing in windowoverflow8 exception

If a windowoverflow8 happened after changing the SP, the exception handler would look for the extra save area by looking at the previous frame's SP. This SP would be a garbage value and could cause the windowoverflow exception to write to invalid memory ares.
2024-10-05 20:47:46 -04:00 · 2022-01-05 12:35:52 +08:00 · 2022-01-05 12:35:52 +08:00 · 78d7844b01
commit 78d7844b01
parent d446c4a18f
3 changed files with 70 additions and 40 deletions
--- a/components/esp_system/port/soc/esp32s2/system_internal.c
+++ b/components/esp_system/port/soc/esp32s2/system_internal.c
@ -1,16 +1,8 @@
-// Copyright 2018 Espressif Systems (Shanghai) PTE LTD
-//
-// Licensed under the Apache License, Version 2.0 (the "License");
-// you may not use this file except in compliance with the License.
-// You may obtain a copy of the License at
-//
-//     http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
+/*
+ * SPDX-FileCopyrightText: 2018-2022 Espressif Systems (Shanghai) CO LTD
+ *
+ * SPDX-License-Identifier: Apache-2.0
+ */

 #include <string.h>
 #include "sdkconfig.h"
@ -31,10 +23,15 @@
 #include "soc/rtc_periph.h"
 #include "hal/wdt_hal.h"
 #include "freertos/xtensa_api.h"
+#include "soc/soc_memory_layout.h"
 #include "hal/cpu_hal.h"

 #include "esp32s2/rom/rtc.h"

+#define ALIGN_DOWN(val, align)  ((val) & ~((align) - 1))
+
+extern int _bss_end;
+
 /* "inner" restart function for after RTOS, interrupts & anything else on this
 * core are already stopped. Stalls other core, resets hardware,
 * triggers restart.
@ -76,6 +73,17 @@ void IRAM_ATTR esp_restart_noos(void)
    // Flush any data left in UART FIFOs
    esp_rom_uart_tx_wait_idle(0);
    esp_rom_uart_tx_wait_idle(1);
+
+#ifdef CONFIG_SPIRAM_ALLOW_STACK_EXTERNAL_MEMORY
+    if (esp_ptr_external_ram(esp_cpu_get_sp())) {
+        // If stack_addr is from External Memory (CONFIG_SPIRAM_ALLOW_STACK_EXTERNAL_MEMORY is used)
+        // then need to switch SP to Internal Memory otherwise
+        // we will get the "Cache disabled but cached memory region accessed" error after Cache_Read_Disable.
+        uint32_t new_sp = ALIGN_DOWN(_bss_end, 16);
+        SET_STACK(new_sp);
+    }
+#endif
+
    // Disable cache
    Cache_Disable_ICache();
    Cache_Disable_DCache();
--- a/components/esp_system/port/soc/esp32s3/system_internal.c
+++ b/components/esp_system/port/soc/esp32s3/system_internal.c
@ -1,17 +1,9 @@

-// Copyright 2018 Espressif Systems (Shanghai) PTE LTD
-//
-// Licensed under the Apache License, Version 2.0 (the "License");
-// you may not use this file except in compliance with the License.
-// You may obtain a copy of the License at
-//
-//     http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
+/*
+ * SPDX-FileCopyrightText: 2018-2022 Espressif Systems (Shanghai) CO LTD
+ *
+ * SPDX-License-Identifier: Apache-2.0
+ */

 #include <string.h>
 #include "sdkconfig.h"
@ -30,10 +22,15 @@
 #include "soc/rtc_periph.h"
 #include "hal/wdt_hal.h"
 #include "freertos/xtensa_api.h"
+#include "soc/soc_memory_layout.h"

 #include "esp32s3/rom/cache.h"
 #include "esp32s3/rom/rtc.h"

+#define ALIGN_DOWN(val, align)  ((val) & ~((align) - 1))
+
+extern int _bss_end;
+
 /* "inner" restart function for after RTOS, interrupts & anything else on this
 * core are already stopped. Stalls other core, resets hardware,
 * triggers restart.
@ -69,6 +66,17 @@ void IRAM_ATTR esp_restart_noos(void)
    // Flush any data left in UART FIFOs
    esp_rom_uart_tx_wait_idle(0);
    esp_rom_uart_tx_wait_idle(1);
+
+#ifdef CONFIG_SPIRAM_ALLOW_STACK_EXTERNAL_MEMORY
+    if (esp_ptr_external_ram(esp_cpu_get_sp())) {
+        // If stack_addr is from External Memory (CONFIG_SPIRAM_ALLOW_STACK_EXTERNAL_MEMORY is used)
+        // then need to switch SP to Internal Memory otherwise
+        // we will get the "Cache disabled but cached memory region accessed" error after Cache_Read_Disable.
+        uint32_t new_sp = ALIGN_DOWN(_bss_end, 16);
+        SET_STACK(new_sp);
+    }
+#endif
+
    // Disable cache
    Cache_Disable_ICache();
    Cache_Disable_DCache();
--- a/components/xtensa/include/xt_instr_macros.h
+++ b/components/xtensa/include/xt_instr_macros.h
@ -1,19 +1,13 @@
-// Copyright 2020 Espressif Systems (Shanghai) PTE LTD
-//
-// Licensed under the Apache License, Version 2.0 (the "License");
-// you may not use this file except in compliance with the License.
-// You may obtain a copy of the License at
-
-//     http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
+/*
+ * SPDX-FileCopyrightText: 2020-2022 Espressif Systems (Shanghai) CO LTD
+ *
+ * SPDX-License-Identifier: Apache-2.0
+ */

 #pragma once

+#include "xtensa/xtruntime.h"
+
 #define RSR(reg, at)         asm volatile ("rsr %0, %1" : "=r" (at) : "i" (reg))
 #define WSR(reg, at)         asm volatile ("wsr %0, %1" : : "r" (at), "i" (reg))
 #define XSR(reg, at)         asm volatile ("xsr %0, %1" : "+r" (at) : "i" (reg))
@ -23,14 +17,27 @@
 #define WITLB(at, as)        asm volatile ("witlb  %0, %1; \n isync \n " : : "r" (at), "r" (as))
 #define WDTLB(at, as)        asm volatile ("wdtlb  %0, %1; \n dsync \n " : : "r" (at), "r" (as))

+#define EXTRA_SAVE_AREA_SIZE 32
+#define BASE_SAVE_AREA_SIZE  16
+#define SAVE_AREA_OFFSET (EXTRA_SAVE_AREA_SIZE + BASE_SAVE_AREA_SIZE)
+#define BASE_AREA_SP_OFFSET  12
+
 /* The SET_STACK implements a setting a new stack pointer (sp or a1).
 * to do this the need reset PS_WOE, reset WINDOWSTART, update SP, and return PS_WOE.
 *
+ * In addition, if a windowOverflow8/12 happens the exception handler expects to be able to look at
+ * the previous frames stackpointer to find the extra save area. So this function will reserve space
+ * for this area as well as initialise the previous sp that points to it
+ *
 * Note: It has 2 implementations one for using in assembler files (*.S) and one for using in C.
 *
 * C code prototype for SET_STACK:
 *   uint32_t ps_reg;
 *   uint32_t w_base;
+ *
+ *   uint32_t sp = (uint32_t)new_sp - SAVE_AREA_OFFSET; \
+    *(uint32_t*)(sp - 12) = (uint32_t)new_sp; \
+
 *   RSR(PS, ps_reg);
 *   ps_reg &= ~(PS_WOE_MASK | PS_OWB_MASK | PS_CALLINC_MASK);
 *   WSR(PS, ps_reg);
@ -46,6 +53,10 @@
 */
 #ifdef __ASSEMBLER__
    .macro SET_STACK  new_sp tmp1 tmp2
+    addi tmp1, new_sp, -SAVE_AREA_OFFSET
+    addi tmp2, tmp1, -BASE_AREA_SP_OFFSET
+    s32i new_sp, tmp2, 0
+    addi new_sp, tmp1, 0
    rsr.ps \tmp1
    movi \tmp2, ~(PS_WOE_MASK | PS_OWB_MASK | PS_CALLINC_MASK)
    and \tmp1, \tmp1, \tmp2
@ -68,8 +79,11 @@
    rsync
    .endm
 #else
+
 #define SET_STACK(new_sp) \
    do { \
+        uint32_t sp = (uint32_t)new_sp - SAVE_AREA_OFFSET; \
+        *(uint32_t*)(sp - BASE_AREA_SP_OFFSET) = (uint32_t)new_sp; \
        uint32_t tmp1 = 0, tmp2 = 0; \
        asm volatile ( \
          "rsr.ps %1 \n"\
@ -93,6 +107,6 @@
          "or %1, %1, %2 \n"\
          "wsr.ps %1 \n"\
          "rsync \n"\
-          : "+r"(new_sp), "+r"(tmp1), "+r"(tmp2)); \
+          : "+r"(sp), "+r"(tmp1), "+r"(tmp2)); \
    } while (0);
 #endif // __ASSEMBLER__