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.

components/esp_system/port/soc/esp32s2/system_internal.c

@@ -1,5 +1,5 @@
 /*
- * SPDX-FileCopyrightText: 2018-2021 Espressif Systems (Shanghai) CO LTD
+ * SPDX-FileCopyrightText: 2018-2022 Espressif Systems (Shanghai) CO LTD
  *
  * SPDX-License-Identifier: Apache-2.0
  */
@@ -23,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.
@@ -68,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();

components/esp_system/port/soc/esp32s3/system_internal.c

@@ -1,6 +1,6 @@
 
 /*
- * SPDX-FileCopyrightText: 2018-2021 Espressif Systems (Shanghai) CO LTD
+ * SPDX-FileCopyrightText: 2018-2022 Espressif Systems (Shanghai) CO LTD
  *
  * SPDX-License-Identifier: Apache-2.0
  */
@@ -22,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.
@@ -61,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();

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__

tools/ci/check_copyright_ignore.txt

@@ -2036,7 +2036,6 @@ components/xtensa/esp32s3/include/xtensa/config/tie-asm.h
 components/xtensa/esp32s3/include/xtensa/config/tie.h
 components/xtensa/include/eri.h
 components/xtensa/include/esp_private/panic_reason.h
-components/xtensa/include/xt_instr_macros.h
 components/xtensa/include/xt_trax.h
 components/xtensa/include/xtensa-debug-module.h
 components/xtensa/include/xtensa/cacheasm.h