From bdeaeb8d7fc3e38e4e9266d0e792b75416d925cf Mon Sep 17 00:00:00 2001 From: Mahavir Jain Date: Mon, 11 Oct 2021 15:38:06 +0530 Subject: [PATCH 1/6] esp_system: enable "cache disable but cache accessed" interrupt for ESP32-S3 --- .../esp_system/port/arch/xtensa/panic_arch.c | 6 +- .../port/soc/esp32s3/cache_err_int.c | 78 +++++++++++++++---- tools/ci/check_copyright_ignore.txt | 1 - 3 files changed, 65 insertions(+), 20 deletions(-) diff --git a/components/esp_system/port/arch/xtensa/panic_arch.c b/components/esp_system/port/arch/xtensa/panic_arch.c index 66d995c820..23d0b09453 100644 --- a/components/esp_system/port/arch/xtensa/panic_arch.c +++ b/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() ) { diff --git a/components/esp_system/port/soc/esp32s3/cache_err_int.c b/components/esp_system/port/soc/esp32s3/cache_err_int.c index 96c952e731..c7d7d6eceb 100644 --- a/components/esp_system/port/soc/esp32s3/cache_err_int.c +++ b/components/esp_system/port/soc/esp32s3/cache_err_int.c @@ -1,16 +1,8 @@ -// Copyright 2015-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: 2021 Espressif Systems (Shanghai) CO LTD + * + * SPDX-License-Identifier: Apache-2.0 + */ /** * @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; } diff --git a/tools/ci/check_copyright_ignore.txt b/tools/ci/check_copyright_ignore.txt index ea4ccf7ebf..ee1903eba3 100644 --- a/tools/ci/check_copyright_ignore.txt +++ b/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 From 61820f5b307813cad5b1c02c5b3f2e9c442d1ca1 Mon Sep 17 00:00:00 2001 From: Mahavir Jain Date: Mon, 11 Oct 2021 20:47:27 +0530 Subject: [PATCH 2/6] cpu_start: let individual core clear its interrupt matrix There was race condition where interrupt entries set by APP cpu core could have been cleared during PRO cpu startup. This was observed while setting up "cache access error" interrupt in SMP mode for ESP32-S3. This fix allows to NOT modify or clear any entries set by other core (APP or PRO) and thus avoiding any race conditions during startup code. --- components/esp_system/port/cpu_start.c | 23 +++++++++++++---------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/components/esp_system/port/cpu_start.c b/components/esp_system/port/cpu_start.c index e936223c08..bfa8e48ed9 100644 --- a/components/esp_system/port/cpu_start.c +++ b/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 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 + 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,6 +527,7 @@ void IRAM_ATTR call_start_cpu0(void) // and default RTC-backed system time provider. g_startup_time = esp_rtc_get_time_us(); + // Clear interrupt matrix for PRO CPU 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 From 11d9faf38c66ec9d5e3bfc6df92fa008aa66327b Mon Sep 17 00:00:00 2001 From: Mahavir Jain Date: Tue, 12 Oct 2021 09:55:45 +0530 Subject: [PATCH 3/6] spi_flash: enable cache access error test for all targets except ESP32-S2 --- .../spi_flash/test/test_cache_disabled.c | 31 ++++++++++++++++--- tools/ci/check_copyright_ignore.txt | 1 - 2 files changed, 27 insertions(+), 5 deletions(-) diff --git a/components/spi_flash/test/test_cache_disabled.c b/components/spi_flash/test/test_cache_disabled.c index aae44bd431..5be458b47e 100644 --- a/components/spi_flash/test/test_cache_disabled.c +++ b/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 #include #include @@ -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) diff --git a/tools/ci/check_copyright_ignore.txt b/tools/ci/check_copyright_ignore.txt index ee1903eba3..3c9984eea4 100644 --- a/tools/ci/check_copyright_ignore.txt +++ b/tools/ci/check_copyright_ignore.txt @@ -2743,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 From f441ce977ac48165e66e689632b631e3c64137fc Mon Sep 17 00:00:00 2001 From: Mahavir Jain Date: Tue, 12 Oct 2021 16:15:26 +0530 Subject: [PATCH 4/6] ci: increase parallel UT count for few jobs --- .gitlab/ci/target-test.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.gitlab/ci/target-test.yml b/.gitlab/ci/target-test.yml index 73c449e3d2..a00dc19336 100644 --- a/.gitlab/ci/target-test.yml +++ b/.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 From 81e3eb45ca83f9700b5b919a4c5aa02ab44f7e7f Mon Sep 17 00:00:00 2001 From: Mahavir Jain Date: Thu, 14 Oct 2021 15:21:00 +0530 Subject: [PATCH 5/6] cpu_start: rename function to add core prefix for more clarity --- components/esp_system/port/cpu_start.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/components/esp_system/port/cpu_start.c b/components/esp_system/port/cpu_start.c index bfa8e48ed9..f5a64dd887 100644 --- a/components/esp_system/port/cpu_start.c +++ b/components/esp_system/port/cpu_start.c @@ -137,7 +137,7 @@ 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 intr_matrix_clear(void) +static void core_intr_matrix_clear(void) { uint32_t core_id = cpu_hal_get_core_id(); @@ -180,7 +180,7 @@ void IRAM_ATTR call_start_cpu1(void) ESP_EARLY_LOGI(TAG, "App cpu up."); // Clear interrupt matrix for APP CPU core - intr_matrix_clear(); + 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. @@ -528,7 +528,7 @@ void IRAM_ATTR call_start_cpu0(void) g_startup_time = esp_rtc_get_time_us(); // Clear interrupt matrix for PRO CPU core - intr_matrix_clear(); + 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 From 0b67fe1e654507cf2d4ed301ae730c45660fa08f Mon Sep 17 00:00:00 2001 From: Mahavir Jain Date: Sun, 17 Oct 2021 10:37:43 +0530 Subject: [PATCH 6/6] ci: increase ESP32-S3 UT job count --- .gitlab/ci/target-test.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.gitlab/ci/target-test.yml b/.gitlab/ci/target-test.yml index a00dc19336..1e1fa7573a 100644 --- a/.gitlab/ci/target-test.yml +++ b/.gitlab/ci/target-test.yml @@ -727,7 +727,7 @@ UT_C3_SDSPI: UT_S3: extends: .unit_test_esp32s3_template - parallel: 30 + parallel: 31 tags: - ESP32S3_IDF - UT_T1_1