From e76c187efb20facf8cad410aae5102a58d2e9145 Mon Sep 17 00:00:00 2001 From: Ivan Grokhotkov Date: Tue, 21 Feb 2017 21:57:53 +0800 Subject: [PATCH 1/2] spi_flash: protect esp_intr_noniram_{disable,enable} in 1-core config MR !441 (7c155ab) has fixed issue with esp_intr_noniram_{disable,enable} calls not being properly protected by spi_flash_op_{lock,unlock}. Unit test was added, but the unit test environment tests only dual-core config. Similar issue was present in the code path for the single-core config, where esp_intr_noniram_{disable,enable} calls were unprotected. This change fixes the protection issue and updates the unit test to run properly in single core config as well. The issue with running unit tests for single core config will be addressed in a separate MR. --- components/spi_flash/cache_utils.c | 4 ++-- components/spi_flash/test/test_spi_flash.c | 17 +++++++++++------ 2 files changed, 13 insertions(+), 8 deletions(-) diff --git a/components/spi_flash/cache_utils.c b/components/spi_flash/cache_utils.c index df9d18c44..5e880ab49 100644 --- a/components/spi_flash/cache_utils.c +++ b/components/spi_flash/cache_utils.c @@ -205,16 +205,16 @@ void spi_flash_op_unlock() void IRAM_ATTR spi_flash_disable_interrupts_caches_and_other_cpu() { - esp_intr_noniram_disable(); spi_flash_op_lock(); + esp_intr_noniram_disable(); spi_flash_disable_cache(0, &s_flash_op_cache_state[0]); } void IRAM_ATTR spi_flash_enable_interrupts_caches_and_other_cpu() { spi_flash_restore_cache(0, s_flash_op_cache_state[0]); - spi_flash_op_unlock(); esp_intr_noniram_enable(); + spi_flash_op_unlock(); } void IRAM_ATTR spi_flash_disable_interrupts_caches_and_other_cpu_no_os() diff --git a/components/spi_flash/test/test_spi_flash.c b/components/spi_flash/test/test_spi_flash.c index 90d0cc1fd..597568ec6 100644 --- a/components/spi_flash/test/test_spi_flash.c +++ b/components/spi_flash/test/test_spi_flash.c @@ -65,19 +65,24 @@ static void flash_test_task(void *arg) TEST_CASE("flash write and erase work both on PRO CPU and on APP CPU", "[spi_flash][ignore]") { SemaphoreHandle_t done = xSemaphoreCreateCounting(4, 0); - struct flash_test_ctx ctx[4] = { + struct flash_test_ctx ctx[] = { { .offset = 0x100 + 6, .done = done }, { .offset = 0x100 + 7, .done = done }, { .offset = 0x100 + 8, .done = done }, +#ifndef CONFIG_FREERTOS_UNICORE { .offset = 0x100 + 9, .done = done } +#endif }; - xTaskCreatePinnedToCore(flash_test_task, "1", 2048, &ctx[0], 3, NULL, 0); - xTaskCreatePinnedToCore(flash_test_task, "2", 2048, &ctx[1], 3, NULL, 1); - xTaskCreatePinnedToCore(flash_test_task, "3", 2048, &ctx[2], 3, NULL, tskNO_AFFINITY); - xTaskCreatePinnedToCore(flash_test_task, "4", 2048, &ctx[3], 3, NULL, tskNO_AFFINITY); + xTaskCreatePinnedToCore(flash_test_task, "t0", 2048, &ctx[0], 3, NULL, 0); + xTaskCreatePinnedToCore(flash_test_task, "t1", 2048, &ctx[1], 3, NULL, tskNO_AFFINITY); + xTaskCreatePinnedToCore(flash_test_task, "t2", 2048, &ctx[2], 3, NULL, tskNO_AFFINITY); +#ifndef CONFIG_FREERTOS_UNICORE + xTaskCreatePinnedToCore(flash_test_task, "t3", 2048, &ctx[3], 3, NULL, 1); +#endif - for (int i = 0; i < 4; ++i) { + const size_t task_count = sizeof(ctx)/sizeof(ctx[0]); + for (int i = 0; i < task_count; ++i) { xSemaphoreTake(done, portMAX_DELAY); TEST_ASSERT_FALSE(ctx[i].fail); } From cbb71baca96d5f4c22be017dafef8d745834dac5 Mon Sep 17 00:00:00 2001 From: Ivan Grokhotkov Date: Wed, 22 Feb 2017 12:51:16 +0800 Subject: [PATCH 2/2] spi_flash: protect spi_flash_unlock spi_flash_unlock was missing spi_flash_guard_start, which caused cache to be enabled during unlock operation, causing hard-to-trace crashes and cache data corruption. --- components/spi_flash/flash_ops.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/components/spi_flash/flash_ops.c b/components/spi_flash/flash_ops.c index 3581b3414..324c02a3d 100644 --- a/components/spi_flash/flash_ops.c +++ b/components/spi_flash/flash_ops.c @@ -202,7 +202,9 @@ esp_err_t IRAM_ATTR spi_flash_write(size_t dst, const void *srcv, size_t size) size_t mid_size = (size - left_size) & ~3U; size_t right_off = left_size + mid_size; size_t right_size = size - mid_size - left_size; + spi_flash_guard_start(); rc = spi_flash_unlock(); + spi_flash_guard_end(); if (rc != SPI_FLASH_RESULT_OK) { goto out; } @@ -289,7 +291,9 @@ esp_err_t IRAM_ATTR spi_flash_write_encrypted(size_t dest_addr, const void *src, COUNTER_START(); spi_flash_disable_interrupts_caches_and_other_cpu(); SpiFlashOpResult rc; + spi_flash_guard_start(); rc = spi_flash_unlock(); + spi_flash_guard_end(); spi_flash_enable_interrupts_caches_and_other_cpu(); if (rc == SPI_FLASH_RESULT_OK) {