Merge branch 'bugfix/esp32s3_cache_disabled_access_err' into 'master'

esp_system: enable "cache disable but cache accessed" interrupt for ESP32-S3

See merge request espressif/esp-idf!15367

.gitlab/ci/target-test.yml

@@ -478,7 +478,7 @@ UT_001:
 
 UT_002:
   extends: .unit_test_esp32_template
-  parallel: 14
+  parallel: 15
   tags:
     - ESP32_IDF
     - UT_T1_1
@@ -683,7 +683,7 @@ UT_S2_SDSPI:
 
 UT_C3:
   extends: .unit_test_esp32c3_template
-  parallel: 32
+  parallel: 33
   tags:
     - ESP32C3_IDF
     - UT_T1_1
@@ -727,7 +727,7 @@ UT_C3_SDSPI:
 
 UT_S3:
   extends: .unit_test_esp32s3_template
-  parallel: 30
+  parallel: 31
   tags:
     - ESP32S3_IDF
     - UT_T1_1

components/esp_system/port/arch/xtensa/panic_arch.c

@@ -422,11 +422,7 @@ void panic_soc_fill_info(void *f, panic_info_t *info)
         "Coprocessor exception",
         "Interrupt wdt timeout on CPU0",
         "Interrupt wdt timeout on CPU1",
-#if CONFIG_IDF_TARGET_ESP32
         "Cache disabled but cached memory region accessed",
-#elif CONFIG_IDF_TARGET_ESP32S2
-        "Cache error",
-#endif
     };
 
     info->reason = pseudo_reason[0];
@@ -441,7 +437,7 @@ void panic_soc_fill_info(void *f, panic_info_t *info)
         info->exception = PANIC_EXCEPTION_DEBUG;
     }
 
-#if CONFIG_IDF_TARGET_ESP32S2
+#if CONFIG_IDF_TARGET_ESP32S2 || CONFIG_IDF_TARGET_ESP32S3
     if (frame->exccause == PANIC_RSN_CACHEERR) {
 #if CONFIG_ESP_SYSTEM_MEMPROT_FEATURE
         if ( esp_memprot_is_intr_ena_any() ) {

components/esp_system/port/cpu_start.c

@@ -137,6 +137,15 @@ static volatile bool s_resume_cores;
 // If CONFIG_SPIRAM_IGNORE_NOTFOUND is set and external RAM is not found or errors out on testing, this is set to false.
 bool g_spiram_ok = true;
 
+static void core_intr_matrix_clear(void)
+{
+    uint32_t core_id = cpu_hal_get_core_id();
+
+    for (int i = 0; i < ETS_MAX_INTR_SOURCE; i++) {
+        intr_matrix_set(core_id, i, ETS_INVALID_INUM);
+    }
+}
+
 #if !CONFIG_ESP_SYSTEM_SINGLE_CORE_MODE
 void startup_resume_other_cores(void)
 {
@@ -170,6 +179,9 @@ void IRAM_ATTR call_start_cpu1(void)
     s_cpu_up[1] = true;
     ESP_EARLY_LOGI(TAG, "App cpu up.");
 
+    // Clear interrupt matrix for APP CPU core
+    core_intr_matrix_clear();
+
     //Take care putting stuff here: if asked, FreeRTOS will happily tell you the scheduler
     //has started, but it isn't active *on this CPU* yet.
     esp_cache_err_int_init();
@@ -244,16 +256,6 @@ static void start_other_core(void)
 }
 #endif // !CONFIG_ESP_SYSTEM_SINGLE_CORE_MODE
 
-static void intr_matrix_clear(void)
-{
-    for (int i = 0; i < ETS_MAX_INTR_SOURCE; i++) {
-        intr_matrix_set(0, i, ETS_INVALID_INUM);
-#if !CONFIG_ESP_SYSTEM_SINGLE_CORE_MODE
-        intr_matrix_set(1, i, ETS_INVALID_INUM);
-#endif
-    }
-}
-
 /*
  * We arrive here after the bootloader finished loading the program from flash. The hardware is mostly uninitialized,
  * and the app CPU is in reset. We do have a stack, so we can do the initialization in C.
@@ -525,7 +527,8 @@ void IRAM_ATTR call_start_cpu0(void)
     // and default RTC-backed system time provider.
     g_startup_time = esp_rtc_get_time_us();
 
-    intr_matrix_clear();
+    // Clear interrupt matrix for PRO CPU core
+    core_intr_matrix_clear();
 
 #ifndef CONFIG_IDF_ENV_FPGA // TODO: on FPGA it should be possible to configure this, not currently working with APB_CLK_FREQ changed
 #ifdef CONFIG_ESP_CONSOLE_UART

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

@@ -1,16 +1,8 @@
-// Copyright 2015-2020 Espressif Systems (Shanghai) PTE LTD
+/*
-//
+ * SPDX-FileCopyrightText: 2021 Espressif Systems (Shanghai) CO LTD
-// Licensed under the Apache License, Version 2.0 (the "License");
+ *
-// you may not use this file except in compliance with the License.
+ * SPDX-License-Identifier: Apache-2.0
-// 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.
 
 /**
  * @file cache_err_int.c
@@ -65,11 +57,69 @@ void esp_cache_err_int_init(void)
                       EXTMEM_ICACHE_PRELOAD_OP_FAULT_INT_ENA |
                       EXTMEM_ICACHE_SYNC_OP_FAULT_INT_ENA);
 
+    if (core_id == PRO_CPU_NUM) {
+        intr_matrix_set(core_id, ETS_CACHE_CORE0_ACS_INTR_SOURCE, ETS_CACHEERR_INUM);
+        /* On the hardware side, stat by clearing all the bits reponsible for
+         * enabling cache access error interrupts.  */
+        SET_PERI_REG_MASK(EXTMEM_CORE0_ACS_CACHE_INT_CLR_REG,
+                EXTMEM_CORE0_DBUS_REJECT_INT_CLR |
+                EXTMEM_CORE0_DBUS_ACS_MSK_DC_INT_CLR |
+                EXTMEM_CORE0_IBUS_REJECT_INT_CLR |
+                EXTMEM_CORE0_IBUS_WR_IC_INT_CLR |
+                EXTMEM_CORE0_IBUS_ACS_MSK_IC_INT_CLR);
+
+        /* Enable cache access error interrupts */
+        SET_PERI_REG_MASK(EXTMEM_CORE0_ACS_CACHE_INT_ENA_REG,
+                EXTMEM_CORE0_DBUS_REJECT_INT_ENA |
+                EXTMEM_CORE0_DBUS_ACS_MSK_DC_INT_ENA |
+                EXTMEM_CORE0_IBUS_REJECT_INT_ENA |
+                EXTMEM_CORE0_IBUS_WR_IC_INT_ENA |
+                EXTMEM_CORE0_IBUS_ACS_MSK_IC_INT_ENA);
+    } else {
+        intr_matrix_set(core_id, ETS_CACHE_CORE1_ACS_INTR_SOURCE, ETS_CACHEERR_INUM);
+
+        /* On the hardware side, stat by clearing all the bits reponsible for
+         * enabling cache access error interrupts.  */
+        SET_PERI_REG_MASK(EXTMEM_CORE1_ACS_CACHE_INT_CLR_REG,
+                EXTMEM_CORE1_DBUS_REJECT_INT_CLR |
+                EXTMEM_CORE1_DBUS_ACS_MSK_DC_INT_CLR |
+                EXTMEM_CORE1_IBUS_REJECT_INT_CLR |
+                EXTMEM_CORE1_IBUS_WR_IC_INT_CLR |
+                EXTMEM_CORE1_IBUS_ACS_MSK_IC_INT_CLR);
+
+        /* Enable cache access error interrupts */
+        SET_PERI_REG_MASK(EXTMEM_CORE1_ACS_CACHE_INT_ENA_REG,
+                EXTMEM_CORE1_DBUS_REJECT_INT_ENA |
+                EXTMEM_CORE1_DBUS_ACS_MSK_DC_INT_ENA |
+                EXTMEM_CORE1_IBUS_REJECT_INT_ENA |
+                EXTMEM_CORE1_IBUS_WR_IC_INT_ENA |
+                EXTMEM_CORE1_IBUS_ACS_MSK_IC_INT_ENA);
+    }
+
     ESP_INTR_ENABLE(ETS_CACHEERR_INUM);
 }
 
 int IRAM_ATTR esp_cache_err_get_cpuid(void)
 {
-    // FIXME
+    const uint32_t pro_mask = EXTMEM_CORE0_DBUS_REJECT_ST |
+                EXTMEM_CORE0_DBUS_ACS_MSK_DCACHE_ST |
+                EXTMEM_CORE0_IBUS_REJECT_ST |
+                EXTMEM_CORE0_IBUS_WR_ICACHE_ST |
+                EXTMEM_CORE0_IBUS_ACS_MSK_ICACHE_ST;
+
+    if (GET_PERI_REG_MASK(EXTMEM_CORE0_ACS_CACHE_INT_ST_REG, pro_mask)) {
+        return PRO_CPU_NUM;
+    }
+
+    const uint32_t app_mask = EXTMEM_CORE1_DBUS_REJECT_ST |
+                EXTMEM_CORE1_DBUS_ACS_MSK_DCACHE_ST |
+                EXTMEM_CORE1_IBUS_REJECT_ST |
+                EXTMEM_CORE1_IBUS_WR_ICACHE_ST |
+                EXTMEM_CORE1_IBUS_ACS_MSK_ICACHE_ST;
+
+    if (GET_PERI_REG_MASK(EXTMEM_CORE1_ACS_CACHE_INT_ST_REG, app_mask)) {
+        return APP_CPU_NUM;
+    }
+
     return -1;
 }

components/spi_flash/test/test_cache_disabled.c

@@ -1,3 +1,9 @@
+/*
+ * SPDX-FileCopyrightText: 2021 Espressif Systems (Shanghai) CO LTD
+ *
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
@@ -51,10 +57,18 @@ TEST_CASE("spi_flash_cache_enabled() works on both CPUs", "[spi_flash][esp_flash
     vQueueDelete(result_queue);
 }
 
-static const uint32_t s_in_rodata[] = { 0x12345678, 0xfedcba98 };
+#if !TEMPORARY_DISABLED_FOR_TARGETS(ESP32S2)
+
+// This needs to sufficiently large array, otherwise it may end up in
+// DRAM (e.g. size <= 8 bytes && ARCH == RISCV)
+static const uint32_t s_in_rodata[8] = { 0x12345678, 0xfedcba98 };
 
 static void IRAM_ATTR cache_access_test_func(void* arg)
 {
+    /* Assert that the array s_in_rodata is in DROM. If not, this test is
+     * invalid as disabling the cache wouldn't have any effect. */
+    TEST_ASSERT(esp_ptr_in_drom(s_in_rodata));
+
     spi_flash_disable_interrupts_caches_and_other_cpu();
     volatile uint32_t* src = (volatile uint32_t*) s_in_rodata;
     uint32_t v1 = src[0];
@@ -65,9 +79,17 @@ static void IRAM_ATTR cache_access_test_func(void* arg)
     vTaskDelete(NULL);
 }
 
+#ifdef CONFIG_IDF_TARGET_ESP32C3
+#define CACHE_ERROR_REASON "Cache error,RTC_SW_CPU_RST"
+#elif CONFIG_IDF_TARGET_ESP32S3
+#define CACHE_ERROR_REASON "Cache disabled,RTC_SW_CPU_RST"
+#else
+#define CACHE_ERROR_REASON "Cache disabled,SW_RESET"
+#endif
+
 // These tests works properly if they resets the chip with the
 // "Cache disabled but cached memory region accessed" reason and the correct CPU is logged.
-TEST_CASE("invalid access to cache raises panic (PRO CPU)", "[spi_flash][ignore]")
+TEST_CASE("invalid access to cache raises panic (PRO CPU)", "[spi_flash][reset="CACHE_ERROR_REASON"]")
 {
     xTaskCreatePinnedToCore(&cache_access_test_func, "ia", 2048, NULL, 5, NULL, 0);
     vTaskDelay(1000/portTICK_PERIOD_MS);
@@ -75,10 +97,11 @@ TEST_CASE("invalid access to cache raises panic (PRO CPU)", "[spi_flash][ignore]
 
 #ifndef CONFIG_FREERTOS_UNICORE
 
-TEST_CASE("invalid access to cache raises panic (APP CPU)", "[spi_flash][ignore]")
+TEST_CASE("invalid access to cache raises panic (APP CPU)", "[spi_flash][reset="CACHE_ERROR_REASON"]")
 {
     xTaskCreatePinnedToCore(&cache_access_test_func, "ia", 2048, NULL, 5, NULL, 1);
     vTaskDelay(1000/portTICK_PERIOD_MS);
 }
 
-#endif
+#endif // !CONFIG_FREERTOS_UNICORE
+#endif // !TEMPORARY_DISABLED_FOR_TARGETS(ESP32S2)

tools/ci/check_copyright_ignore.txt

@@ -1154,7 +1154,6 @@ components/esp_system/port/soc/esp32s2/clk.c
 components/esp_system/port/soc/esp32s2/reset_reason.c
 components/esp_system/port/soc/esp32s2/system_internal.c
 components/esp_system/port/soc/esp32s2/usb_console.c
-components/esp_system/port/soc/esp32s3/cache_err_int.c
 components/esp_system/port/soc/esp32s3/cache_err_int.h
 components/esp_system/port/soc/esp32s3/clk.c
 components/esp_system/port/soc/esp32s3/reset_reason.c
@@ -2744,7 +2743,6 @@ components/spi_flash/spi_flash_chip_mxic_opi.c
 components/spi_flash/spi_flash_chip_winbond.c
 components/spi_flash/spi_flash_os_func_app.c
 components/spi_flash/spi_flash_os_func_noos.c
-components/spi_flash/test/test_cache_disabled.c
 components/spi_flash/test/test_esp_flash.c
 components/spi_flash/test/test_flash_encryption.c
 components/spi_flash/test/test_large_flash_writes.c