Merge branch 'feature/ipc_noblocking_call_v5.2' into 'release/v5.2'

feat(ipc): Adds a new no blocking IPC call (v5.2)

See merge request espressif/esp-idf!28442

components/app_trace/gcov/gcov_rtio.c

@@ -15,7 +15,7 @@
 #include "esp_app_trace.h"
 #include "esp_freertos_hooks.h"
 #include "esp_private/dbg_stubs.h"
-#include "esp_ipc.h"
+#include "esp_private/esp_ipc.h"
 #include "esp_attr.h"
 #include "hal/wdt_hal.h"
 #if CONFIG_IDF_TARGET_ESP32
@@ -84,9 +84,8 @@ void gcov_create_task(void *arg)
 static IRAM_ATTR
 void gcov_create_task_tick_hook(void)
 {
-    extern esp_err_t esp_ipc_start_gcov_from_isr(uint32_t cpu_id, esp_ipc_func_t func, void* arg);
     if (s_create_gcov_task) {
-        if (esp_ipc_start_gcov_from_isr(xPortGetCoreID(), &gcov_create_task, NULL) == ESP_OK) {
+        if (esp_ipc_call_nonblocking(xPortGetCoreID(), &gcov_create_task, NULL) == ESP_OK) {
             s_create_gcov_task = false;
         }
     }

components/esp_system/esp_ipc.c

@@ -1,5 +1,5 @@
 /*
- * SPDX-FileCopyrightText: 2015-2022 Espressif Systems (Shanghai) CO LTD
+ * SPDX-FileCopyrightText: 2015-2024 Espressif Systems (Shanghai) CO LTD
  *
  * SPDX-License-Identifier: Apache-2.0
  */
@@ -12,11 +12,14 @@
 #include "esp_ipc.h"
 #include "esp_private/esp_ipc_isr.h"
 #include "esp_attr.h"
+#include "esp_cpu.h"
 
 #include "freertos/FreeRTOS.h"
 #include "freertos/task.h"
 #include "freertos/semphr.h"
 
+#define IPC_MAX_PRIORITY (configMAX_PRIORITIES - 1)
+
 #if !defined(CONFIG_FREERTOS_UNICORE) || defined(CONFIG_APPTRACE_GCOV_ENABLE)
 
 #if CONFIG_COMPILER_OPTIMIZATION_NONE
@@ -39,10 +42,11 @@ typedef enum {
     IPC_WAIT_FOR_END,
 } esp_ipc_wait_t;
 
-#if CONFIG_APPTRACE_GCOV_ENABLE
-static volatile esp_ipc_func_t s_gcov_func = NULL;           // Gcov dump starter function which should be called by high priority task
-static void * volatile s_gcov_func_arg;                      // Argument to pass into s_gcov_func
-#endif
+static esp_ipc_wait_t volatile s_wait_for[portNUM_PROCESSORS];
+
+static volatile esp_ipc_func_t s_no_block_func[portNUM_PROCESSORS] = { 0 };
+static volatile bool s_no_block_func_and_arg_are_ready[portNUM_PROCESSORS] = { 0 };
+static void * volatile s_no_block_func_arg[portNUM_PROCESSORS];
 
 static void IRAM_ATTR ipc_task(void* arg)
 {
@@ -54,29 +58,23 @@ static void IRAM_ATTR ipc_task(void* arg)
 #endif
 
     while (true) {
-        uint32_t ipc_wait;
-        xTaskNotifyWait(0, ULONG_MAX, &ipc_wait, portMAX_DELAY);
-
-#if CONFIG_APPTRACE_GCOV_ENABLE
-        if (s_gcov_func) {
-            (*s_gcov_func)(s_gcov_func_arg);
-            s_gcov_func = NULL;
-            /* we can not interfer with IPC calls so no need for further processing */
-            // esp_ipc API and gcov_from_isr APIs can be processed together if they came at the same time
-            if (ipc_wait == IPC_WAIT_NO) {
-                continue;
-            }
+        ulTaskNotifyTake(pdTRUE, portMAX_DELAY);
+
+        if (s_no_block_func_and_arg_are_ready[cpuid] && s_no_block_func[cpuid]) {
+            (*s_no_block_func[cpuid])(s_no_block_func_arg[cpuid]);
+            s_no_block_func_and_arg_are_ready[cpuid] = false;
+            s_no_block_func[cpuid] = NULL;
         }
-#endif // CONFIG_APPTRACE_GCOV_ENABLE
 
 #ifndef CONFIG_FREERTOS_UNICORE
         if (s_func[cpuid]) {
             // we need to cache s_func, s_func_arg and ipc_ack variables locally
             // because they can be changed by a subsequent IPC call (after xTaskNotify(caller_task_handle)).
             esp_ipc_func_t func = s_func[cpuid];
-            s_func[cpuid] = NULL;
             void* func_arg = s_func_arg[cpuid];
+            esp_ipc_wait_t ipc_wait = s_wait_for[cpuid];
             SemaphoreHandle_t ipc_ack = s_ipc_ack[cpuid];
+            s_func[cpuid] = NULL;
 
             if (ipc_wait == IPC_WAIT_FOR_START) {
                 xSemaphoreGive(ipc_ack);
@@ -119,7 +117,7 @@ static void esp_ipc_init(void)
         s_ipc_mutex[i] = xSemaphoreCreateMutexStatic(&s_ipc_mutex_buffer[i]);
         s_ipc_ack[i] = xSemaphoreCreateBinaryStatic(&s_ipc_ack_buffer[i]);
         BaseType_t res = xTaskCreatePinnedToCore(ipc_task, task_name, IPC_STACK_SIZE, (void*) i,
-                                                    configMAX_PRIORITIES - 1, &s_ipc_task_handle[i], i);
+                                                    IPC_MAX_PRIORITY, &s_ipc_task_handle[i], i);
         assert(res == pdTRUE);
         (void)res;
     }
@@ -151,9 +149,11 @@ static esp_err_t esp_ipc_call_and_wait(uint32_t cpu_id, esp_ipc_func_t func, voi
     xSemaphoreTake(s_ipc_mutex[0], portMAX_DELAY);
 #endif
 
-    s_func[cpu_id] = func;
     s_func_arg[cpu_id] = arg;
-    xTaskNotify(s_ipc_task_handle[cpu_id], wait_for, eSetValueWithOverwrite);
+    s_wait_for[cpu_id] = wait_for;
+    // s_func must be set after all other parameters. The ipc_task use this as indicator of the IPC is prepared.
+    s_func[cpu_id] = func;
+    xTaskNotifyGive(s_ipc_task_handle[cpu_id]);
     xSemaphoreTake(s_ipc_ack[cpu_id], portMAX_DELAY);
 
 #ifdef CONFIG_ESP_IPC_USES_CALLERS_PRIORITY
@@ -174,28 +174,33 @@ esp_err_t esp_ipc_call_blocking(uint32_t cpu_id, esp_ipc_func_t func, void* arg)
     return esp_ipc_call_and_wait(cpu_id, func, arg, IPC_WAIT_FOR_END);
 }
 
-// currently this is only called from gcov component
-// the top level guarantees that the next call will be only after the previous one has completed
-#if CONFIG_APPTRACE_GCOV_ENABLE
-esp_err_t esp_ipc_start_gcov_from_isr(uint32_t cpu_id, esp_ipc_func_t func, void* arg)
+esp_err_t esp_ipc_call_nonblocking(uint32_t cpu_id, esp_ipc_func_t func, void* arg)
 {
-    if (xTaskGetSchedulerState() != taskSCHEDULER_RUNNING) {
+    if (cpu_id >= portNUM_PROCESSORS || s_ipc_task_handle[cpu_id] == NULL) {
+        return ESP_ERR_INVALID_ARG;
+    }
+    if (cpu_id == xPortGetCoreID() && xTaskGetSchedulerState() != taskSCHEDULER_RUNNING) {
         return ESP_ERR_INVALID_STATE;
     }
 
-    // Since it is called from an interrupt, it can not wait for a mutex to be released.
-    if (s_gcov_func == NULL) {
-        s_gcov_func_arg = arg;
-        s_gcov_func = func;
+    // Since it can be called from an interrupt or Scheduler is Suspened, it can not wait for a mutex to be released.
+    if (esp_cpu_compare_and_set((volatile uint32_t *)&s_no_block_func[cpu_id], 0, (uint32_t)func)) {
+        s_no_block_func_arg[cpu_id] = arg;
+        s_no_block_func_and_arg_are_ready[cpu_id] = true;
 
-        // If the target task already has a notification pending then its notification value is not updated (WithoutOverwrite).
-        xTaskNotifyFromISR(s_ipc_task_handle[cpu_id], IPC_WAIT_NO, eSetValueWithoutOverwrite, NULL);
+        if (xPortInIsrContext()) {
+            vTaskNotifyGiveFromISR(s_ipc_task_handle[cpu_id], NULL);
+        } else {
+#ifdef CONFIG_ESP_IPC_USES_CALLERS_PRIORITY
+            vTaskPrioritySet(s_ipc_task_handle[cpu_id], IPC_MAX_PRIORITY);
+#endif
+            xTaskNotifyGive(s_ipc_task_handle[cpu_id]);
+        }
         return ESP_OK;
     }
 
     // the previous call was not completed
     return ESP_FAIL;
 }
-#endif // CONFIG_APPTRACE_GCOV_ENABLE
 
 #endif // !defined(CONFIG_FREERTOS_UNICORE) || defined(CONFIG_APPTRACE_GCOV_ENABLE)

components/esp_system/include/esp_ipc.h

@@ -1,5 +1,5 @@
 /*
- * SPDX-FileCopyrightText: 2015-2021 Espressif Systems (Shanghai) CO LTD
+ * SPDX-FileCopyrightText: 2015-2024 Espressif Systems (Shanghai) CO LTD
  *
  * SPDX-License-Identifier: Apache-2.0
  */

components/esp_system/include/esp_private/esp_ipc.h

@@ -0,0 +1,47 @@
+/*
+ * SPDX-FileCopyrightText: 2024 Espressif Systems (Shanghai) CO LTD
+ *
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+#pragma once
+
+#include "../esp_ipc.h"
+#include "sdkconfig.h"
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+#if !defined(CONFIG_FREERTOS_UNICORE) || defined(CONFIG_APPTRACE_GCOV_ENABLE)
+
+/**
+ * @brief Execute a callback on a given CPU without any blocking operations for the caller
+ *
+ * Since it does not have any blocking operations it is suitable to be run from interrupts
+ * or even when the Scheduler on the current core is suspended.
+ *
+ * The function:
+ * - does not wait for the callback to begin or complete execution,
+ * - does not change the IPC priority.
+ * The function returns after sending a notification to the IPC task to execute the callback.
+ *
+ * @param[in]   cpu_id  CPU where the given function should be executed (0 or 1)
+ * @param[in]   func    Pointer to a function of type void func(void* arg) to be executed
+ * @param[in]   arg     Arbitrary argument of type void* to be passed into the function
+ *
+ * @return
+ *      - ESP_ERR_INVALID_ARG if cpu_id is invalid
+ *      - ESP_ERR_INVALID_STATE 1. IPC tasks have not been initialized yet,
+ *                              2. cpu_id requests IPC on the current core, but the FreeRTOS scheduler is not running on it
+ *                              (the IPC task cannot be executed).
+ *      - ESP_FAIL IPC is busy due to a previous call was not completed.
+ *      - ESP_OK otherwise
+ */
+esp_err_t esp_ipc_call_nonblocking(uint32_t cpu_id, esp_ipc_func_t func, void* arg);
+
+#endif // !defined(CONFIG_FREERTOS_UNICORE) || defined(CONFIG_APPTRACE_GCOV_ENABLE)
+
+#ifdef __cplusplus
+}
+#endif

components/esp_system/test_apps/esp_system_unity_tests/main/test_ipc.c

@@ -1,5 +1,5 @@
 /*
- * SPDX-FileCopyrightText: 2015-2022 Espressif Systems (Shanghai) CO LTD
+ * SPDX-FileCopyrightText: 2015-2024 Espressif Systems (Shanghai) CO LTD
  *
  * SPDX-License-Identifier: Apache-2.0
  */
@@ -12,6 +12,7 @@
 #include "unity.h"
 #if !CONFIG_FREERTOS_UNICORE
 #include "esp_ipc.h"
+#include "esp_private/esp_ipc.h"
 #endif
 #include "esp_log.h"
 #include "esp_rom_sys.h"
@@ -162,4 +163,50 @@ TEST_CASE("Test ipc_task can not wake up blocking task early", "[ipc]")
     TEST_ASSERT_EQUAL(31, val2);
 }
 
+TEST_CASE("Test ipc call nonblocking", "[ipc]")
+{
+    int val_for_1_call = 20;
+    TEST_ESP_OK(esp_ipc_call_nonblocking(1, test_func_ipc_cb2, (void*)&val_for_1_call));
+    TEST_ASSERT_EQUAL(20, val_for_1_call);
+
+    int val_for_2_call = 30;
+    TEST_ESP_ERR(ESP_FAIL, esp_ipc_call_nonblocking(1, test_func_ipc_cb3, (void*)&val_for_2_call));
+
+    vTaskDelay(150 / portTICK_PERIOD_MS);
+    TEST_ASSERT_EQUAL(21, val_for_1_call);
+
+    TEST_ESP_OK(esp_ipc_call_nonblocking(1, test_func_ipc_cb3, (void*)&val_for_2_call));
+
+    vTaskDelay(550 / portTICK_PERIOD_MS);
+    TEST_ASSERT_EQUAL(31, val_for_2_call);
+}
+
+static void test_func_ipc_cb4(void *arg)
+{
+    int *val = (int *)arg;
+    *val = *val + 1;
+}
+
+TEST_CASE("Test ipc call nonblocking when FreeRTOS Scheduler is suspended", "[ipc]")
+{
+    #ifdef CONFIG_FREERTOS_SMP
+        //Note: Scheduler suspension behavior changed in FreeRTOS SMP
+        vTaskPreemptionDisable(NULL);
+    #else
+        // Disable scheduler on the current CPU
+        vTaskSuspendAll();
+    #endif // CONFIG_FREERTOS_SMP
+
+    volatile int value = 20;
+    TEST_ESP_OK(esp_ipc_call_nonblocking(1, test_func_ipc_cb4, (void*)&value));
+    while (value == 20) { };
+    TEST_ASSERT_EQUAL(21, value);
+
+    #ifdef CONFIG_FREERTOS_SMP
+        //Note: Scheduler suspension behavior changed in FreeRTOS SMP
+        vTaskPreemptionEnable(NULL);
+    #else
+        xTaskResumeAll();
+    #endif
+}
 #endif /* !CONFIG_FREERTOS_UNICORE */

components/spi_flash/cache_utils.c

@@ -48,7 +48,7 @@
 #include <soc/soc.h>
 #include "sdkconfig.h"
 #ifndef CONFIG_FREERTOS_UNICORE
-#include "esp_ipc.h"
+#include "esp_private/esp_ipc.h"
 #endif
 #include "esp_attr.h"
 #include "esp_memory_utils.h"
@@ -157,8 +157,8 @@ void IRAM_ATTR spi_flash_disable_interrupts_caches_and_other_cpu(void)
 
     spi_flash_op_lock();
 
-    const int cpuid = xPortGetCoreID();
-    const uint32_t other_cpuid = (cpuid == 0) ? 1 : 0;
+    int cpuid = xPortGetCoreID();
+    uint32_t other_cpuid = (cpuid == 0) ? 1 : 0;
 #ifndef NDEBUG
     // For sanity check later: record the CPU which has started doing flash operation
     assert(s_flash_op_cpu == -1);
@@ -173,33 +173,45 @@ void IRAM_ATTR spi_flash_disable_interrupts_caches_and_other_cpu(void)
         // esp_intr_noniram_disable.
         assert(other_cpuid == 1);
     } else {
-        // Temporarily raise current task priority to prevent a deadlock while
-        // waiting for IPC task to start on the other CPU
-        prvTaskSavedPriority_t SavedPriority;
-        prvTaskPriorityRaise(&SavedPriority, 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;
-        ESP_ERROR_CHECK(esp_ipc_call(other_cpuid, &spi_flash_op_block_func, (void *) other_cpuid));
+        bool ipc_call_was_send_to_other_cpu;
+        do {
+            #ifdef CONFIG_FREERTOS_SMP
+                //Note: Scheduler suspension behavior changed in FreeRTOS SMP
+                vTaskPreemptionDisable(NULL);
+            #else
+                // Disable scheduler on the current CPU
+                vTaskSuspendAll();
+            #endif // CONFIG_FREERTOS_SMP
+
+            cpuid = xPortGetCoreID();
+            other_cpuid = (cpuid == 0) ? 1 : 0;
+            #ifndef NDEBUG
+                s_flash_op_cpu = cpuid;
+            #endif
+
+            s_flash_op_can_start = false;
+
+            ipc_call_was_send_to_other_cpu = esp_ipc_call_nonblocking(other_cpuid, &spi_flash_op_block_func, (void *) other_cpuid) == ESP_OK;
+
+            if (!ipc_call_was_send_to_other_cpu) {
+                // IPC call was not send to other cpu because another nonblocking API is running now.
+                // Enable the Scheduler again will not help the IPC to speed it up
+                // but there is a benefit to schedule to a higher priority task before the nonblocking running IPC call is done.
+                #ifdef CONFIG_FREERTOS_SMP
+                    //Note: Scheduler suspension behavior changed in FreeRTOS SMP
+                    vTaskPreemptionEnable(NULL);
+                #else
+                    xTaskResumeAll();
+                #endif // CONFIG_FREERTOS_SMP
+            }
+        } while (!ipc_call_was_send_to_other_cpu);
 
         while (!s_flash_op_can_start) {
             // Busy loop and wait for spi_flash_op_block_func to disable cache
             // on the other CPU
         }
-#ifdef CONFIG_FREERTOS_SMP
-        //Note: Scheduler suspension behavior changed in FreeRTOS SMP
-        vTaskPreemptionDisable(NULL);
-#else
-        // Disable scheduler on the current CPU
-        vTaskSuspendAll();
-#endif // CONFIG_FREERTOS_SMP
-        // Can now set the priority back to the normal one
-        prvTaskPriorityRestore(&SavedPriority);
-        // This is guaranteed to run on CPU <cpuid> because the other CPU is now
-        // occupied by highest priority task
-        assert(xPortGetCoreID() == cpuid);
     }
+
     // Kill interrupts that aren't located in IRAM
     esp_intr_noniram_disable();
     // This CPU executes this routine, with non-IRAM interrupts and the scheduler