From f9f2937694b80b22b391635ed0a0443753e1ef70 Mon Sep 17 00:00:00 2001
From: Ivan Grokhotkov <ivan@espressif.com>
Date: Mon, 30 Oct 2017 18:53:39 +0800
Subject: [PATCH] spi_flash: raise priority of the task performing spi_flash
 operation

The fix is for the situation when cache disabling mechanism causes
a deadlock with user tasks. Situation is as follows:

1. spi_flash operation is started from low-priority task on CPU0
2. It uses IPC to wake up high-priority IPC1 task on CPU1, preventing
   all other tasks on CPU1 from running. This is needed to safely
   disable the cache.
3. While the task which started spi_flash operation is waiting for IPC1
   task to acknowledge that CPU1 is not using cache anymore, it is
   preempted by a higher priority application task ("app0").
4. Task app0 busy-waits for some operation on CPU1 to complete. But
   since application tasks are blocked out by IPC1 task, this never
   happens. Since app0 is busy-waiting, the task doing spi flash
   operation never runs.

The more or less logical soltion to the problem would be to also do
cache disabling on CPU0 and the SPI flash operation itself from IPC0
task. However IPC0 task stack would need to be increased to allow doing
SPI flash operation (and IPC1 stack as well). This would waste some
memory. An alternative approach adopted in this fix is to call FreeRTOS
functions to temporary increase the priority of SPI flash operation task
to the same level as the IPC task.

Fixes https://github.com/espressif/arduino-esp32/issues/740
Fixes https://github.com/espressif/esp-idf/issues/1157
---
 components/spi_flash/cache_utils.c         |  6 +++
 components/spi_flash/test/test_spi_flash.c | 58 ++++++++++++++++++++++
 2 files changed, 64 insertions(+)

diff --git a/components/spi_flash/cache_utils.c b/components/spi_flash/cache_utils.c
index bc4e8885e..56038de37 100644
--- a/components/spi_flash/cache_utils.c
+++ b/components/spi_flash/cache_utils.c
@@ -110,6 +110,10 @@ void IRAM_ATTR spi_flash_disable_interrupts_caches_and_other_cpu()
         assert(other_cpuid == 1);
         spi_flash_disable_cache(other_cpuid, &s_flash_op_cache_state[other_cpuid]);
     } else {
+        // Temporarily raise current task priority to prevent a deadlock while
+        // waiting for IPC task to start on the other CPU
+        int old_prio = uxTaskPriorityGet(NULL);
+        vTaskPrioritySet(NULL, configMAX_PRIORITIES - 1);
         // Signal to the spi_flash_op_block_task on the other CPU that we need it to
         // disable cache there and block other tasks from executing.
         s_flash_op_can_start = false;
@@ -121,6 +125,8 @@ void IRAM_ATTR spi_flash_disable_interrupts_caches_and_other_cpu()
         }
         // Disable scheduler on the current CPU
         vTaskSuspendAll();
+        // Can now set the priority back to the normal one
+        vTaskPrioritySet(NULL, old_prio);
         // This is guaranteed to run on CPU <cpuid> because the other CPU is now
         // occupied by highest priority task
         assert(xPortGetCoreID() == cpuid);
diff --git a/components/spi_flash/test/test_spi_flash.c b/components/spi_flash/test/test_spi_flash.c
index e47cdf840..b1e738d41 100644
--- a/components/spi_flash/test/test_spi_flash.c
+++ b/components/spi_flash/test/test_spi_flash.c
@@ -167,3 +167,61 @@ TEST_CASE("spi flash functions can run along with IRAM interrupts", "[spi_flash]
     free(read_arg.buf);
 }
 
+
+#if portNUM_PROCESSORS > 1
+TEST_CASE("spi_flash deadlock with high priority busy-waiting task", "[spi_flash]")
+{
+    typedef struct {
+        QueueHandle_t queue;
+        volatile bool done;
+    } deadlock_test_arg_t;
+
+    /* Create two tasks: high-priority consumer on CPU0, low-priority producer on CPU1.
+     * Consumer polls the queue until it gets some data, then yields.
+     * Run flash operation on CPU0. Check that when IPC1 task blocks out the producer,
+     * the task which does flash operation does not get blocked by the consumer.
+     */
+
+    void producer_task(void* varg)
+    {
+        int dummy = 0;
+        deadlock_test_arg_t* arg = (deadlock_test_arg_t*) varg;
+        while (!arg->done) {
+            xQueueSend(arg->queue, &dummy, 0);
+            vTaskDelay(1);
+        }
+        vTaskDelete(NULL);
+    }
+
+    void consumer_task(void* varg)
+    {
+        int dummy;
+        deadlock_test_arg_t* arg = (deadlock_test_arg_t*) varg;
+        while (!arg->done) {
+            if (xQueueReceive(arg->queue, &dummy, 0) == pdTRUE) {
+                vTaskDelay(1);
+            }
+        }
+        vTaskDelete(NULL);
+    }
+    deadlock_test_arg_t arg = {
+        .queue = xQueueCreate(32, sizeof(int)),
+        .done = false
+    };
+
+    TEST_ASSERT(xTaskCreatePinnedToCore(&producer_task, "producer", 4096, &arg, 5, NULL, 1));
+    TEST_ASSERT(xTaskCreatePinnedToCore(&consumer_task, "consumer", 4096, &arg, 10, NULL, 0));
+
+    for (int i = 0; i < 1000; i++) {
+        uint32_t dummy;
+        TEST_ESP_OK(spi_flash_read(0, &dummy, sizeof(dummy)));
+    }
+
+    arg.done = true;
+    vTaskDelay(5);
+    vQueueDelete(arg.queue);
+
+    /* Check that current task priority is still correct */
+    TEST_ASSERT_EQUAL_INT(uxTaskPriorityGet(NULL), UNITY_FREERTOS_PRIORITY);
+}
+#endif // portNUM_PROCESSORS > 1