Merge branch 'bugfix/flash_write_single_core' into 'master'
spi_flash: fix protection issues This MR fixes the two spi_flash related issues: - esp_intr_noniram_{disable,enable} not being protected by spi_flash_op_{lock,unlock} in single core mode. This caused a safety assert to be triggered in esp_intr_noniram_disable. - spi_flash_unlock not being protected by spi_flash_guard_{start,end}. This caused a conflict between SPI0 and SPI1 controllers when accessing SPI flash, manifesting in cache data corruption and IllegalInstruction exceptions, for some flash chips. See merge request !522
This commit is contained in:
commit
dca0377e19
3 changed files with 17 additions and 8 deletions
|
@ -205,16 +205,16 @@ void spi_flash_op_unlock()
|
||||||
|
|
||||||
void IRAM_ATTR spi_flash_disable_interrupts_caches_and_other_cpu()
|
void IRAM_ATTR spi_flash_disable_interrupts_caches_and_other_cpu()
|
||||||
{
|
{
|
||||||
esp_intr_noniram_disable();
|
|
||||||
spi_flash_op_lock();
|
spi_flash_op_lock();
|
||||||
|
esp_intr_noniram_disable();
|
||||||
spi_flash_disable_cache(0, &s_flash_op_cache_state[0]);
|
spi_flash_disable_cache(0, &s_flash_op_cache_state[0]);
|
||||||
}
|
}
|
||||||
|
|
||||||
void IRAM_ATTR spi_flash_enable_interrupts_caches_and_other_cpu()
|
void IRAM_ATTR spi_flash_enable_interrupts_caches_and_other_cpu()
|
||||||
{
|
{
|
||||||
spi_flash_restore_cache(0, s_flash_op_cache_state[0]);
|
spi_flash_restore_cache(0, s_flash_op_cache_state[0]);
|
||||||
spi_flash_op_unlock();
|
|
||||||
esp_intr_noniram_enable();
|
esp_intr_noniram_enable();
|
||||||
|
spi_flash_op_unlock();
|
||||||
}
|
}
|
||||||
|
|
||||||
void IRAM_ATTR spi_flash_disable_interrupts_caches_and_other_cpu_no_os()
|
void IRAM_ATTR spi_flash_disable_interrupts_caches_and_other_cpu_no_os()
|
||||||
|
|
|
@ -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 mid_size = (size - left_size) & ~3U;
|
||||||
size_t right_off = left_size + mid_size;
|
size_t right_off = left_size + mid_size;
|
||||||
size_t right_size = size - mid_size - left_size;
|
size_t right_size = size - mid_size - left_size;
|
||||||
|
spi_flash_guard_start();
|
||||||
rc = spi_flash_unlock();
|
rc = spi_flash_unlock();
|
||||||
|
spi_flash_guard_end();
|
||||||
if (rc != SPI_FLASH_RESULT_OK) {
|
if (rc != SPI_FLASH_RESULT_OK) {
|
||||||
goto out;
|
goto out;
|
||||||
}
|
}
|
||||||
|
@ -289,7 +291,9 @@ esp_err_t IRAM_ATTR spi_flash_write_encrypted(size_t dest_addr, const void *src,
|
||||||
COUNTER_START();
|
COUNTER_START();
|
||||||
spi_flash_disable_interrupts_caches_and_other_cpu();
|
spi_flash_disable_interrupts_caches_and_other_cpu();
|
||||||
SpiFlashOpResult rc;
|
SpiFlashOpResult rc;
|
||||||
|
spi_flash_guard_start();
|
||||||
rc = spi_flash_unlock();
|
rc = spi_flash_unlock();
|
||||||
|
spi_flash_guard_end();
|
||||||
spi_flash_enable_interrupts_caches_and_other_cpu();
|
spi_flash_enable_interrupts_caches_and_other_cpu();
|
||||||
|
|
||||||
if (rc == SPI_FLASH_RESULT_OK) {
|
if (rc == SPI_FLASH_RESULT_OK) {
|
||||||
|
|
|
@ -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]")
|
TEST_CASE("flash write and erase work both on PRO CPU and on APP CPU", "[spi_flash][ignore]")
|
||||||
{
|
{
|
||||||
SemaphoreHandle_t done = xSemaphoreCreateCounting(4, 0);
|
SemaphoreHandle_t done = xSemaphoreCreateCounting(4, 0);
|
||||||
struct flash_test_ctx ctx[4] = {
|
struct flash_test_ctx ctx[] = {
|
||||||
{ .offset = 0x100 + 6, .done = done },
|
{ .offset = 0x100 + 6, .done = done },
|
||||||
{ .offset = 0x100 + 7, .done = done },
|
{ .offset = 0x100 + 7, .done = done },
|
||||||
{ .offset = 0x100 + 8, .done = done },
|
{ .offset = 0x100 + 8, .done = done },
|
||||||
|
#ifndef CONFIG_FREERTOS_UNICORE
|
||||||
{ .offset = 0x100 + 9, .done = done }
|
{ .offset = 0x100 + 9, .done = done }
|
||||||
|
#endif
|
||||||
};
|
};
|
||||||
|
|
||||||
xTaskCreatePinnedToCore(flash_test_task, "1", 2048, &ctx[0], 3, NULL, 0);
|
xTaskCreatePinnedToCore(flash_test_task, "t0", 2048, &ctx[0], 3, NULL, 0);
|
||||||
xTaskCreatePinnedToCore(flash_test_task, "2", 2048, &ctx[1], 3, NULL, 1);
|
xTaskCreatePinnedToCore(flash_test_task, "t1", 2048, &ctx[1], 3, NULL, tskNO_AFFINITY);
|
||||||
xTaskCreatePinnedToCore(flash_test_task, "3", 2048, &ctx[2], 3, NULL, tskNO_AFFINITY);
|
xTaskCreatePinnedToCore(flash_test_task, "t2", 2048, &ctx[2], 3, NULL, tskNO_AFFINITY);
|
||||||
xTaskCreatePinnedToCore(flash_test_task, "4", 2048, &ctx[3], 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);
|
xSemaphoreTake(done, portMAX_DELAY);
|
||||||
TEST_ASSERT_FALSE(ctx[i].fail);
|
TEST_ASSERT_FALSE(ctx[i].fail);
|
||||||
}
|
}
|
||||||
|
|
Loading…
Reference in a new issue