From 672736304c0d45df5cd80c8317fd7f13f4fadbc0 Mon Sep 17 00:00:00 2001 From: Konstantin Kondrashov Date: Fri, 29 May 2026 08:26:06 +0300 Subject: [PATCH] feat(ipc_isr): adds IPC ISR safe API for other CPU stall API --- components/esp_system/include/esp_ipc_isr.h | 64 ++++++-- .../include/esp_private/esp_ipc_isr.h | 45 +++++- .../port/arch/riscv/esp_ipc_isr_handler.S | 4 +- .../port/arch/riscv/esp_ipc_isr_routines.c | 49 +++++- .../port/arch/xtensa/esp_ipc_isr_handler.S | 19 ++- .../port/arch/xtensa/esp_ipc_isr_routines.S | 65 +++++++- components/esp_system/port/esp_ipc_isr.c | 139 +++++++++++++----- .../main/test_ipc_isr.c | 132 ++++++++++++++++- docs/en/api-reference/system/ipc.rst | 23 ++- docs/zh_CN/api-reference/system/ipc.rst | 23 ++- 10 files changed, 499 insertions(+), 64 deletions(-) diff --git a/components/esp_system/include/esp_ipc_isr.h b/components/esp_system/include/esp_ipc_isr.h index eb6919da9c3..ca382adb3dc 100644 --- a/components/esp_system/include/esp_ipc_isr.h +++ b/components/esp_system/include/esp_ipc_isr.h @@ -1,11 +1,13 @@ /* - * SPDX-FileCopyrightText: 2015-2023 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2015-2026 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ #pragma once +#include +#include "esp_err.h" #include "sdkconfig.h" #ifdef __cplusplus @@ -39,11 +41,14 @@ typedef void (*esp_ipc_isr_func_t)(void* arg); * - in C or assembly for RISCV chips (such as ESP32P4). * * @note This function is not available in single-core mode. + * @note This function can be called even if the other core is stalled + * (either via esp_ipc_isr_stall_other_cpu() or esp_ipc_isr_stall_other_cpu_safe()). + * This allows safely stalling the other CPU and executing one or more callbacks before releasing it. * * @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 */ -void esp_ipc_isr_call(esp_ipc_isr_func_t func, void* arg) ; +void esp_ipc_isr_call(esp_ipc_isr_func_t func, void* arg); /** * @brief Execute an ISR callback on the other CPU @@ -58,6 +63,9 @@ void esp_ipc_isr_call(esp_ipc_isr_func_t func, void* arg) ; * the callback completes. * * @note This function is not available in single-core mode. + * @note This function can be called even if the other core is stalled + * (either via esp_ipc_isr_stall_other_cpu() or esp_ipc_isr_stall_other_cpu_safe()). + * This allows safely stalling the other CPU and executing one or more callbacks before releasing it. * * @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 @@ -71,20 +79,52 @@ void esp_ipc_isr_call_blocking(esp_ipc_isr_func_t func, void* arg); #define esp_ipc_isr_asm_call_blocking(func, arg) esp_ipc_isr_call_blocking(func, arg) /** - * @brief Stall the other CPU + * @brief Stall the other CPU unconditionally * - * This function will stall the other CPU. The other CPU is stalled by busy-waiting in the context of a High Priority - * Interrupt. The other CPU will not be resumed until esp_ipc_isr_release_other_cpu() is called. + * This function forces the other CPU to enter a busy-wait loop within a High Priority Interrupt context, + * effectively stalling its execution until esp_ipc_isr_release_other_cpu() is called. * - * - This function is internally implemented using IPC ISR - * - This function is used for DPORT workaround. - * - If the stall feature is paused using esp_ipc_isr_stall_pause(), this function will have no effect + * Typical use cases include: + * - DPORT workaround (e.g., DPORT or APB registers) on dual-core ESP32 chips prior to v2.0. + * - Scenarios where the state of the other CPU does not need to be checked, stalling unconditionally. * - * @note This function is not available in single-core mode. - * @note It is the caller's responsibility to avoid deadlocking on spinlocks + * Implementation details: + * - The function is initialized after FreeRTOS startup. + * - When invoked, it signals the other CPU to enter a high-priority interrupt and remain stalled. + * - If the other CPU is already in a high-priority interrupt, it is considered stalled. + * - The other CPU will remain stalled until esp_ipc_isr_release_other_cpu() is called. + * - This function does not check the current state (e.g., critical section or ISR) of the other CPU. + * To avoid potential deadlocks on spinlocks, use esp_ipc_isr_stall_other_cpu_safe() instead. + * - If the stall feature is paused via esp_ipc_isr_stall_pause(), this function has no effect. + * + * @note Not available in single-core mode. + * @note The caller is responsible for ensuring no deadlocks on spinlocks occur. Use esp_ipc_isr_stall_other_cpu_safe() for safer operation. */ void esp_ipc_isr_stall_other_cpu(void); +/** + * @brief Safely stall the other CPU + * + * Attempts to stall the other CPU only if it is not currently in a critical section or ISR context. + * If the other CPU is in a critical section or ISR, the function will return an error. + * + * This helps to prevent potential deadlocks when both CPUs may access shared resources/spinlocks. + * + * @note This function is intended for scenarios where safe stalling is required and the state of the other CPU must be checked. + * + * @return + * - ESP_OK: The other CPU was successfully stalled. + * - ESP_ERR_NOT_ALLOWED: The other CPU is in a critical section or ISR context and cannot be stalled. + */ +esp_err_t esp_ipc_isr_stall_other_cpu_safe(void); + +/** + * @brief Check whether the other CPU is currently stalled + * + * @return true if the other CPU has entered the IPC ISR stall loop, false otherwise. + */ +bool esp_ipc_isr_is_other_cpu_stalled(void); + /** * @brief Release the other CPU * @@ -109,7 +149,7 @@ void esp_ipc_isr_stall_pause(void); /** * @brief Abort a CPU stall * - * This function will abort any stalling routine of the other CPU due to a pervious call to + * This function will abort any stalling routine of the other CPU due to a previous call to * esp_ipc_isr_stall_other_cpu(). This function aborts the stall in a non-recoverable manner, thus should only be called * in case of a panic(). * @@ -128,6 +168,8 @@ void esp_ipc_isr_stall_resume(void); #else // CONFIG_ESP_IPC_ISR_ENABLE #define esp_ipc_isr_stall_other_cpu() +#define esp_ipc_isr_stall_other_cpu_safe() (ESP_OK) +#define esp_ipc_isr_is_other_cpu_stalled() (false) #define esp_ipc_isr_release_other_cpu() #define esp_ipc_isr_stall_pause() #define esp_ipc_isr_stall_abort() diff --git a/components/esp_system/include/esp_private/esp_ipc_isr.h b/components/esp_system/include/esp_private/esp_ipc_isr.h index 6a6d9f8cd82..7e330918ec8 100644 --- a/components/esp_system/include/esp_private/esp_ipc_isr.h +++ b/components/esp_system/include/esp_private/esp_ipc_isr.h @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2015-2022 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2015-2026 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ @@ -8,12 +8,53 @@ #include "sdkconfig.h" +#define ESP_IPC_ISR_ARGS_CMD_OFFSET 0 +#define ESP_IPC_ISR_ARGS_FUNC_OFFSET 4 +#define ESP_IPC_ISR_ARGS_ARG_OFFSET 8 +#define ESP_IPC_ISR_ARGS_RET_ADDR_OFFSET 12 +#define ESP_IPC_ISR_ARGS_INTERRUPTED_CONTEXT_UNSAFE_OFFSET 16 + +#ifndef __ASSEMBLER__ +#include +#include +#include "esp_assert.h" + #ifdef __cplusplus extern "C" { #endif #ifdef CONFIG_ESP_IPC_ISR_ENABLE +/** + * @brief IPC ISR command types. + * + * Enumerates the possible commands that caller CPU can issue to the stalled CPU. + */ +typedef enum { + ESP_IPC_ISR_CMD_RESET_STATE = 0, /**< Reset the command state. */ + ESP_IPC_ISR_CMD_FINISH = 1, /**< Signal that the ISR operation is finished. */ +} esp_ipc_isr_cmd_t; + +/** + * @brief Arguments structure for IPC ISR functions. + * + * This structure holds the function pointer and its argument to be used in callbacks. + */ +typedef struct { + esp_ipc_isr_cmd_t cmd; /**< Command to control the IPC ISR operation. */ + void (*func)(void*); /**< Pointer to the function to be called in the ISR. */ + void* arg; /**< Argument to be passed to the function. */ + void* save_ret_addr; /**< Address to save the return address for the ISR (used by xtensa chips). */ + uint32_t interrupted_context_unsafe; /**< Non-zero if the interrupted CPU context is unsafe to stall. */ +} esp_ipc_isr_args_t; + +ESP_STATIC_ASSERT(offsetof(esp_ipc_isr_args_t, cmd) == ESP_IPC_ISR_ARGS_CMD_OFFSET, "Unexpected IPC ISR cmd offset"); +ESP_STATIC_ASSERT(offsetof(esp_ipc_isr_args_t, func) == ESP_IPC_ISR_ARGS_FUNC_OFFSET, "Unexpected IPC ISR func offset"); +ESP_STATIC_ASSERT(offsetof(esp_ipc_isr_args_t, arg) == ESP_IPC_ISR_ARGS_ARG_OFFSET, "Unexpected IPC ISR arg offset"); +ESP_STATIC_ASSERT(offsetof(esp_ipc_isr_args_t, save_ret_addr) == ESP_IPC_ISR_ARGS_RET_ADDR_OFFSET, "Unexpected IPC ISR return address offset"); +ESP_STATIC_ASSERT(offsetof(esp_ipc_isr_args_t, interrupted_context_unsafe) == ESP_IPC_ISR_ARGS_INTERRUPTED_CONTEXT_UNSAFE_OFFSET, + "Unexpected IPC ISR interrupted context unsafe offset"); + /** * @brief Initialize the IPC ISR feature, must be called for each CPU * @@ -35,3 +76,5 @@ void esp_ipc_isr_init(void); #ifdef __cplusplus } #endif + +#endif // __ASSEMBLER__ diff --git a/components/esp_system/port/arch/riscv/esp_ipc_isr_handler.S b/components/esp_system/port/arch/riscv/esp_ipc_isr_handler.S index 789cdd38c1d..25cb3ead31e 100644 --- a/components/esp_system/port/arch/riscv/esp_ipc_isr_handler.S +++ b/components/esp_system/port/arch/riscv/esp_ipc_isr_handler.S @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2023-2025 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2023-2026 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ @@ -35,6 +35,8 @@ esp_ipc_isr_handler: /* MIE is cleared, so nested interrupts are disabled */ + call esp_ipc_isr_record_interrupted_context + /* Reset isr interrupt flags */ li a1, SYSTEM_CPU_INTR_FROM_CPU_2_REG csrr a0, mhartid # Get CORE_ID diff --git a/components/esp_system/port/arch/riscv/esp_ipc_isr_routines.c b/components/esp_system/port/arch/riscv/esp_ipc_isr_routines.c index afd75591a7b..bbb100eef29 100644 --- a/components/esp_system/port/arch/riscv/esp_ipc_isr_routines.c +++ b/components/esp_system/port/arch/riscv/esp_ipc_isr_routines.c @@ -1,13 +1,56 @@ /* - * SPDX-FileCopyrightText: 2023 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2023-2026 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ #include "stdint.h" +#include "soc/interrupt_reg.h" +#include "soc/soc_caps.h" +#include "esp_ipc_isr.h" +#include "esp_private/esp_ipc_isr.h" #include "esp_private/esp_system_attr.h" +#include "soc/soc.h" +#include "riscv/csr.h" -void ESP_SYSTEM_IRAM_ATTR esp_ipc_isr_waiting_for_finish_cmd(void* ipc_isr_finish_cmd) +#if SOC_INT_CLIC_SUPPORTED +#include "esp_private/interrupt_clic.h" +#endif + +#define ESP_IPC_ISR_RISCV_INTR_ENABLE_THRESH 1 + +extern volatile esp_ipc_isr_args_t esp_ipc_isr_stall_args; +extern volatile uint32_t esp_ipc_isr_stall_fl; + +void ESP_SYSTEM_IRAM_ATTR esp_ipc_isr_record_interrupted_context(void) { - while (*(volatile uint32_t *)ipc_isr_finish_cmd == 0) { }; + uint32_t unsafe = 0; + +#if SOC_INT_CLIC_SUPPORTED + if (rv_utils_get_interrupt_threshold() != 0) { + unsafe = 1; + } + + uint32_t mpil = (RV_READ_CSR(mcause) >> 16) & 0xff; + if (BYTE_TO_NLBITS(mpil) != 0) { + unsafe = 1; + } +#else + if (REG_READ(INTERRUPT_CURRENT_CORE_INT_THRESH_REG) > ESP_IPC_ISR_RISCV_INTR_ENABLE_THRESH) { + unsafe = 1; + } +#endif + + esp_ipc_isr_stall_args.interrupted_context_unsafe = unsafe; +} + +void ESP_SYSTEM_IRAM_ATTR esp_ipc_isr_waiting_for_finish_cmd(void* arg) +{ + esp_ipc_isr_stall_fl = 1; + while (esp_ipc_isr_stall_args.cmd == ESP_IPC_ISR_CMD_RESET_STATE) { + if (esp_ipc_isr_stall_args.func != NULL) { + esp_ipc_isr_stall_args.func(esp_ipc_isr_stall_args.arg); + esp_ipc_isr_stall_args.func = NULL; + } + } } diff --git a/components/esp_system/port/arch/xtensa/esp_ipc_isr_handler.S b/components/esp_system/port/arch/xtensa/esp_ipc_isr_handler.S index 2779a813bc0..1376fdb46ac 100644 --- a/components/esp_system/port/arch/xtensa/esp_ipc_isr_handler.S +++ b/components/esp_system/port/arch/xtensa/esp_ipc_isr_handler.S @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2017-2021 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2017-2026 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ @@ -12,6 +12,7 @@ #include "sdkconfig.h" #include "soc/soc.h" #include "soc/dport_reg.h" +#include "esp_private/esp_ipc_isr.h" /* High-priority interrupt - IPC_ISR handler */ @@ -22,6 +23,7 @@ #define LX_INTR_A3_OFFSET 8 #define LX_INTR_A4_OFFSET 12 #define XT_REG_EXCSAVE_X XT_REG_EXCSAVE_5 +#define XT_REG_EPS_X XT_REG_EPS_5 #define RFI_X 5 #elif CONFIG_ESP_SYSTEM_CHECK_INT_LEVEL_4 @@ -32,6 +34,7 @@ #define LX_INTR_A3_OFFSET 8 #define LX_INTR_A4_OFFSET 12 #define XT_REG_EXCSAVE_X XT_REG_EXCSAVE_4 +#define XT_REG_EPS_X XT_REG_EPS_4 #define RFI_X 4 #endif /* CONFIG_ESP_SYSTEM_CHECK_INT_LEVEL_5 */ @@ -58,6 +61,20 @@ esp_ipc_isr_handler: rsr a2, XT_REG_EXCSAVE_X s32i a2, a0, LX_INTR_A0_OFFSET + /* + * Snapshot the interrupted context before this handler changes PS. + * A non-zero INTLEVEL means the other CPU was in an ISR or an + * interrupt-masked section, so safe stall must retry later. + */ + movi a0, esp_ipc_isr_stall_args /* a0 = base address of stall args struct */ + rsr a3, XT_REG_EPS_X /* a3 = interrupted PS (processor status) */ + extui a3, a3, PS_INTLEVEL_SHIFT, 4 /* a3 = extracted PS.INTLEVEL field */ + beqz a3, .Lipc_isr_xtensa_context_unsafe_done /* if INTLEVEL == 0, leave unsafe value as 0 */ + movi a3, 1 /* else interrupted context is unsafe to stall */ +.Lipc_isr_xtensa_context_unsafe_done: + s32i a3, a0, ESP_IPC_ISR_ARGS_INTERRUPTED_CONTEXT_UNSAFE_OFFSET /* store interrupted-context unsafe value */ + memw /* enforce memory ordering/visibility before continuing */ + /* disable nested interrupts */ /* PS.EXCM is changed from 1 to 0 . It allows using usually exception handler instead of the Double exception handler. */ /* PS_UM = 1 */ diff --git a/components/esp_system/port/arch/xtensa/esp_ipc_isr_routines.S b/components/esp_system/port/arch/xtensa/esp_ipc_isr_routines.S index 33d69269999..d57b312ffa5 100644 --- a/components/esp_system/port/arch/xtensa/esp_ipc_isr_routines.S +++ b/components/esp_system/port/arch/xtensa/esp_ipc_isr_routines.S @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2017-2021 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2017-2026 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ @@ -9,11 +9,20 @@ #include #include #include "sdkconfig.h" +#include "esp_private/esp_ipc_isr.h" -/* esp_ipc_isr_waiting_for_finish_cmd(void* finish_cmd) +/* void esp_ipc_isr_waiting_for_finish_cmd(void* arg) * * It should be called by the CALLX0 command from the handler of High-priority interrupt. - * Only these registers [a2, a3, a4] can be used here. + * Only a2, a3, a4 are used/clobbered here. + * + * typedef struct { + * uint32_t cmd; // +0 + * void* func; // +4 + * void* arg; // +8 + * void* save_ret_addr; // +12 + * uint32_t interrupted_context_unsafe; // +16 + * } esp_ipc_isr_args_t; */ #if CONFIG_ESP_SYSTEM_IN_IRAM .section .iram1, "ax" @@ -22,11 +31,51 @@ .global esp_ipc_isr_waiting_for_finish_cmd .type esp_ipc_isr_waiting_for_finish_cmd, @function // Args: -// a2 - finish_cmd (pointer on esp_ipc_isr_finish_cmd) +// a2 - arg + + /* Must match C defines */ + .equ ESP_IPC_ISR_CMD_RESET_STATE, 0 + + /* Field offsets */ + .equ OFFS_CMD, ESP_IPC_ISR_ARGS_CMD_OFFSET + .equ OFFS_FUNC, ESP_IPC_ISR_ARGS_FUNC_OFFSET + .equ OFFS_ARG, ESP_IPC_ISR_ARGS_ARG_OFFSET + .equ OFFS_RET_ADDR, ESP_IPC_ISR_ARGS_RET_ADDR_OFFSET + + .extern esp_ipc_isr_stall_args /* esp_ipc_isr_args_t */ + .extern esp_ipc_isr_stall_fl + esp_ipc_isr_waiting_for_finish_cmd: - /* waiting for the finish command */ -.check_finish_cmd: + movi a3, esp_ipc_isr_stall_fl + movi a4, 1 + s32i a4, a3, 0 memw - l32i a3, a2, 0 - beqz a3, .check_finish_cmd +.loop_top: + /* while (esp_ipc_isr_stall_args.cmd == RESET_STATE) */ + memw + movi a3, esp_ipc_isr_stall_args + l32i a4, a3, OFFS_CMD + bnez a4, .ret_label /* cmd != 0 -> exit */ + + /* if (esp_ipc_isr_stall_args.func != NULL) */ + l32i a4, a3, OFFS_FUNC /* a4 = func */ + beqz a4, .loop_top + + s32i a0, a3, OFFS_RET_ADDR + + /* Call func(arg) with call0 ABI */ + l32i a2, a3, OFFS_ARG /* a2 = arg (callx0 argument) */ + callx0 a4 /* func(a2) */ + + /* esp_ipc_isr_stall_args.func = NULL; */ + movi a3, esp_ipc_isr_stall_args + movi a4, 0 + s32i a4, a3, OFFS_FUNC + memw + + l32i a0, a3, OFFS_RET_ADDR + + j .loop_top + +.ret_label: ret diff --git a/components/esp_system/port/esp_ipc_isr.c b/components/esp_system/port/esp_ipc_isr.c index dc698509139..d316fe5fc10 100644 --- a/components/esp_system/port/esp_ipc_isr.c +++ b/components/esp_system/port/esp_ipc_isr.c @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2017-2025 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2017-2026 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ @@ -21,8 +21,14 @@ static portMUX_TYPE s_ipc_isr_mux = portMUX_INITIALIZER_UNLOCKED; uint32_t volatile esp_ipc_isr_start_fl; // the flag shows that it is about to run esp_ipc_func() uint32_t volatile esp_ipc_isr_end_fl = 1; // the flag shows that esp_ipc_func() is done +uint32_t volatile esp_ipc_isr_stall_fl; // the flag shows that the other CPU entered the stall loop esp_ipc_isr_func_t volatile esp_ipc_func; // the function which will be run in the ipc_isr context void * volatile esp_ipc_func_arg; // the argument of esp_ipc_func() +esp_ipc_isr_args_t volatile esp_ipc_isr_stall_args; // the context for the stalled CPU + +// This asm function is from esp_ipc_isr_routines.S. +// It is waiting for the finish_cmd command in a loop. +void esp_ipc_isr_waiting_for_finish_cmd(void* arg); typedef enum { STALL_STATE_IDLE = 0, @@ -32,20 +38,22 @@ typedef enum { static stall_state_t volatile s_stall_state = STALL_STATE_IDLE; static int32_t volatile s_count_of_nested_calls[CONFIG_FREERTOS_NUMBER_OF_CORES] = { 0 }; static BaseType_t s_stored_interrupt_level; -static uint32_t volatile esp_ipc_isr_finish_cmd; +static bool volatile s_other_cpu_stalled = false; /** * @brief Type of calling */ typedef enum { - IPC_ISR_WAIT_FOR_START = 0, /*!< The caller is waiting for the start */ - IPC_ISR_WAIT_FOR_END = 1, /*!< The caller is waiting for the end */ + IPC_ISR_WAIT_FOR_START = (1 << 0), /*!< The caller is waiting for the start */ + IPC_ISR_WAIT_FOR_END = (1 << 1), /*!< The caller is waiting for the end */ + IPC_ISR_SAFE_STALL = (1 << 2), /*!< Safe stall mode: run the stall command loop and verify the stalled CPU context */ } esp_ipc_isr_wait_t; #define IPC_ISR_ENTER_CRITICAL() portENTER_CRITICAL_SAFE(&s_ipc_isr_mux) #define IPC_ISR_EXIT_CRITICAL() portEXIT_CRITICAL_SAFE(&s_ipc_isr_mux) -static void esp_ipc_isr_call_and_wait(esp_ipc_isr_func_t func, void* arg, esp_ipc_isr_wait_t wait_for); +static esp_err_t ipc_isr_call_and_wait(esp_ipc_isr_func_t func, void* arg, esp_ipc_isr_wait_t wait_for); +static esp_err_t ipc_isr_stall_other_cpu(esp_ipc_isr_wait_t wait_for); /* Initializing IPC_ISR */ @@ -66,21 +74,39 @@ void esp_ipc_isr_init(void) void ESP_SYSTEM_IRAM_ATTR esp_ipc_isr_call(esp_ipc_isr_func_t func, void* arg) { IPC_ISR_ENTER_CRITICAL(); - esp_ipc_isr_call_and_wait(func, arg, IPC_ISR_WAIT_FOR_START); + if (s_other_cpu_stalled) { + /* If the other CPU is already stalled, wait for a previous call + to finish and initiate a new call */ + while (esp_ipc_isr_stall_args.func != NULL) {} + esp_ipc_isr_stall_args.arg = arg; + esp_ipc_isr_stall_args.func = (void*)func; + // do not wait for the user's callback function to finish + } else { + ipc_isr_call_and_wait(func, arg, IPC_ISR_WAIT_FOR_START); + } IPC_ISR_EXIT_CRITICAL(); } void ESP_SYSTEM_IRAM_ATTR esp_ipc_isr_call_blocking(esp_ipc_isr_func_t func, void* arg) { IPC_ISR_ENTER_CRITICAL(); - esp_ipc_isr_call_and_wait(func, arg, IPC_ISR_WAIT_FOR_END); + if (s_other_cpu_stalled) { + // If the other CPU is already stalled, wait for a previous call to finish and initiate a new call + while (esp_ipc_isr_stall_args.func != NULL) {} + esp_ipc_isr_stall_args.arg = arg; + esp_ipc_isr_stall_args.func = (void*)func; + /* Wait for the user's callback function to complete. + The esp_ipc_isr_waiting_for_finish_cmd function will reset + the func field in esp_ipc_isr_stall_args once the callback + has finished, indicating that the other CPU is ready to accept + new callbacks. */ + while (esp_ipc_isr_stall_args.func != NULL) {} + } else { + ipc_isr_call_and_wait(func, arg, IPC_ISR_WAIT_FOR_END); + } IPC_ISR_EXIT_CRITICAL(); } -// This asm function is from esp_ipc_isr_routines.S. -// It is waiting for the finish_cmd command in a loop. -void esp_ipc_isr_waiting_for_finish_cmd(void* finish_cmd); - /* * esp_ipc_isr_stall_other_cpu is used for: * - stall other CPU, @@ -92,13 +118,26 @@ void esp_ipc_isr_waiting_for_finish_cmd(void* finish_cmd); */ void ESP_SYSTEM_IRAM_ATTR esp_ipc_isr_stall_other_cpu(void) { -#if CONFIG_FREERTOS_SMP - /* - Temporary workaround to prevent deadlocking on the SMP FreeRTOS kernel lock after stalling the other CPU. - See IDF-5257 - */ - taskENTER_CRITICAL(); -#endif + ipc_isr_stall_other_cpu(IPC_ISR_WAIT_FOR_START); +} + +/* + * Stall the other CPU core only if it is in a "safe" state (not in a critical section or ISR). + * Returns ESP_OK if the stall was successful, or ESP_ERR_NOT_ALLOWED if the other core + * is currently in a critical section or ISR context. + */ +esp_err_t ESP_SYSTEM_IRAM_ATTR esp_ipc_isr_stall_other_cpu_safe(void) +{ + return ipc_isr_stall_other_cpu(IPC_ISR_WAIT_FOR_START | IPC_ISR_SAFE_STALL); +} + +bool ESP_SYSTEM_IRAM_ATTR esp_ipc_isr_is_other_cpu_stalled(void) +{ + return s_other_cpu_stalled; +} + +static esp_err_t ESP_SYSTEM_IRAM_ATTR ipc_isr_stall_other_cpu(esp_ipc_isr_wait_t wait_for) +{ if (s_stall_state == STALL_STATE_RUNNING) { #if CONFIG_FREERTOS_SMP BaseType_t intLvl = portDISABLE_INTERRUPTS(); @@ -109,9 +148,21 @@ void ESP_SYSTEM_IRAM_ATTR esp_ipc_isr_stall_other_cpu(void) if (s_count_of_nested_calls[cpu_id]++ == 0) { IPC_ISR_ENTER_CRITICAL(); s_stored_interrupt_level = intLvl; - esp_ipc_isr_finish_cmd = 0; - esp_ipc_isr_call_and_wait(&esp_ipc_isr_waiting_for_finish_cmd, (void*)&esp_ipc_isr_finish_cmd, IPC_ISR_WAIT_FOR_START); - return; + esp_ipc_isr_stall_args.cmd = ESP_IPC_ISR_CMD_RESET_STATE; + esp_ipc_isr_stall_args.func = NULL; + esp_ipc_isr_stall_args.arg = NULL; + esp_ipc_isr_stall_args.interrupted_context_unsafe = 1; + s_other_cpu_stalled = false; + esp_ipc_isr_stall_fl = 0; + esp_err_t error = ipc_isr_call_and_wait(esp_ipc_isr_waiting_for_finish_cmd, NULL, wait_for); + if (error != ESP_OK) { + esp_ipc_isr_release_other_cpu(); + } else { + // Publish the stalled state only after the other CPU enters the stall loop. + while (!esp_ipc_isr_stall_fl) {}; + s_other_cpu_stalled = true; + } + return error; } /* Interrupts are already disabled by the parent, we're nested here. */ @@ -120,7 +171,9 @@ void ESP_SYSTEM_IRAM_ATTR esp_ipc_isr_stall_other_cpu(void) #else portCLEAR_INTERRUPT_MASK_FROM_ISR(intLvl); #endif + return ESP_OK; } + return ESP_FAIL; } void ESP_SYSTEM_IRAM_ATTR esp_ipc_isr_release_other_cpu(void) @@ -128,9 +181,11 @@ void ESP_SYSTEM_IRAM_ATTR esp_ipc_isr_release_other_cpu(void) if (s_stall_state == STALL_STATE_RUNNING) { const uint32_t cpu_id = xPortGetCoreID(); if (--s_count_of_nested_calls[cpu_id] == 0) { - esp_ipc_isr_finish_cmd = 1; - // Make sure end flag is cleared and esp_ipc_isr_waiting_for_finish_cmd is done. + s_other_cpu_stalled = false; + esp_ipc_isr_stall_args.cmd = ESP_IPC_ISR_CMD_FINISH; + // Make sure end flag is set and esp_ipc_isr_waiting_for_finish_cmd is done. while (!esp_ipc_isr_end_fl) {}; + esp_ipc_isr_stall_fl = 0; IPC_ISR_EXIT_CRITICAL(); #if CONFIG_FREERTOS_SMP portRESTORE_INTERRUPTS(s_stored_interrupt_level); @@ -141,13 +196,6 @@ void ESP_SYSTEM_IRAM_ATTR esp_ipc_isr_release_other_cpu(void) assert(0); } } -#if CONFIG_FREERTOS_SMP - /* - Temporary workaround to prevent deadlocking on the SMP FreeRTOS kernel lock after stalling the other CPU. - See IDF-5257 - */ - taskEXIT_CRITICAL(); -#endif } void ESP_SYSTEM_IRAM_ATTR esp_ipc_isr_stall_pause(void) @@ -174,7 +222,7 @@ void ESP_SYSTEM_IRAM_ATTR esp_ipc_isr_stall_resume(void) /* Private functions*/ -static void ESP_SYSTEM_IRAM_ATTR esp_ipc_isr_call_and_wait(esp_ipc_isr_func_t func, void* arg, esp_ipc_isr_wait_t wait_for) +static esp_err_t ESP_SYSTEM_IRAM_ATTR ipc_isr_call_and_wait(esp_ipc_isr_func_t func, void* arg, esp_ipc_isr_wait_t wait_for) { const uint32_t cpu_id = xPortGetCoreID(); @@ -191,12 +239,35 @@ static void ESP_SYSTEM_IRAM_ATTR esp_ipc_isr_call_and_wait(esp_ipc_isr_func_t fu esp_ipc_isr_port_int_trigger(!cpu_id); // IPC_ISR handler will be called and `...isr_start` and `...isr_end` will be updated there - if (wait_for == IPC_ISR_WAIT_FOR_START) { + if (wait_for & IPC_ISR_WAIT_FOR_START) { while (!esp_ipc_isr_start_fl) {}; - } else { - // IPC_ISR_WAIT_FOR_END + } + + if (wait_for & IPC_ISR_SAFE_STALL) { + /* The handler snapshots the interrupted context before publishing + * esp_ipc_isr_start_fl. A non-zero value means the other CPU was in an + * ISR or interrupt-masked section, including the small window where + * interrupts are already masked but FreeRTOS nesting counters are not + * updated yet. + */ + bool safe_to_continue = (esp_ipc_isr_stall_args.interrupted_context_unsafe == 0); + if (!safe_to_continue) { + return ESP_ERR_NOT_ALLOWED; + } + /* IPC_ISR_SAFE_STALL is used only with the stall command-loop + * callback. At this point, esp_ipc_isr_waiting_for_finish_cmd() + * is already running on the other CPU and keeps it blocked while + * this CPU checks whether the interrupted context was safe. If it + * was safe, return without waiting for callback completion: later + * IPC ISR stall APIs will either execute callbacks on the stalled + * CPU or release it from the loop. + */ + return ESP_OK; + } else if (wait_for & IPC_ISR_WAIT_FOR_END) { + // Blocking callers wait until the IPC ISR handler finishes the callback. while (!esp_ipc_isr_end_fl) {}; } + return ESP_OK; } /* End private functions*/ diff --git a/components/esp_system/test_apps/esp_system_unity_tests/main/test_ipc_isr.c b/components/esp_system/test_apps/esp_system_unity_tests/main/test_ipc_isr.c index 3996445c0e4..6e1d4ef9a03 100644 --- a/components/esp_system/test_apps/esp_system_unity_tests/main/test_ipc_isr.c +++ b/components/esp_system/test_apps/esp_system_unity_tests/main/test_ipc_isr.c @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2015-2024 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2015-2026 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ @@ -66,6 +66,7 @@ TEST_CASE("Test ipc_isr blocking IPC function calls get_cycle_count_other_cpu", } static bool volatile s_stop; +static bool volatile s_other_cpu_in_critical_section; static void task_asm(void *arg) { @@ -103,4 +104,133 @@ TEST_CASE("Test ipc_isr two tasks use IPC function calls", "[ipc]") vSemaphoreDelete(exit_sema[0]); vSemaphoreDelete(exit_sema[1]); } + +static bool volatile s_cmd_other_cpu_exit_critical_section; + +static void other_cpu_task_spinlock(void *arg) +{ + printf("other_cpu_task_spinlock start, taking critical section...\n"); + + portMUX_TYPE spinlock = portMUX_INITIALIZER_UNLOCKED; + portENTER_CRITICAL(&spinlock); + s_other_cpu_in_critical_section = true; + while (s_cmd_other_cpu_exit_critical_section == false) { } + s_other_cpu_in_critical_section = false; + portEXIT_CRITICAL(&spinlock); + + printf("other_cpu_task_spinlock end, exited critical section.\n"); + xSemaphoreGive(*(SemaphoreHandle_t *) arg); + vTaskDelete(NULL); +} + +static void other_cpu_task_task_critical(void *arg) +{ + printf("other_cpu_task_task_critical start, taking critical section...\n"); + +#if CONFIG_FREERTOS_SMP + taskENTER_CRITICAL(); +#else + portMUX_TYPE spinlock = portMUX_INITIALIZER_UNLOCKED; + taskENTER_CRITICAL(&spinlock); +#endif + s_other_cpu_in_critical_section = true; + while (s_cmd_other_cpu_exit_critical_section == false) { } + s_other_cpu_in_critical_section = false; +#if CONFIG_FREERTOS_SMP + taskEXIT_CRITICAL(); +#else + taskEXIT_CRITICAL(&spinlock); +#endif + + printf("other_cpu_task_task_critical end, exited critical section.\n"); + xSemaphoreGive(*(SemaphoreHandle_t *) arg); + vTaskDelete(NULL); +} + +TEST_CASE("Test stall other CPU safe", "[ipc]") +{ + SemaphoreHandle_t other_cpu_task_finished = xSemaphoreCreateBinary(); + printf("Test start\n"); + + s_cmd_other_cpu_exit_critical_section = false; + s_other_cpu_in_critical_section = false; + xTaskCreatePinnedToCore(other_cpu_task_spinlock, "other_cpu_task", 4096, &other_cpu_task_finished, UNITY_FREERTOS_PRIORITY - 1, NULL, 1); + while (!s_other_cpu_in_critical_section) { + vTaskDelay(1); + } + + TEST_ASSERT_EQUAL(ESP_ERR_NOT_ALLOWED, esp_ipc_isr_stall_other_cpu_safe()); + s_cmd_other_cpu_exit_critical_section = true; + while (esp_ipc_isr_stall_other_cpu_safe() != ESP_OK) { + vTaskDelay(1); + } + TEST_ASSERT_TRUE(esp_ipc_isr_is_other_cpu_stalled()); + int stalled_core_id = -1; + esp_ipc_isr_call_blocking(esp_test_ipc_isr_get_other_core_id, &stalled_core_id); + TEST_ASSERT_EQUAL(1, stalled_core_id); + esp_ipc_isr_release_other_cpu(); + + xSemaphoreTake(other_cpu_task_finished, portMAX_DELAY); + printf("Test end\n"); + vSemaphoreDelete(other_cpu_task_finished); +} + +TEST_CASE("Test nested stall other CPU release", "[ipc]") +{ + printf("Test start\n"); + + esp_ipc_isr_stall_other_cpu(); + TEST_ASSERT_TRUE(esp_ipc_isr_is_other_cpu_stalled()); + + esp_ipc_isr_stall_other_cpu(); + TEST_ASSERT_TRUE(esp_ipc_isr_is_other_cpu_stalled()); + + esp_ipc_isr_release_other_cpu(); + TEST_ASSERT_TRUE(esp_ipc_isr_is_other_cpu_stalled()); + + int stalled_core_id = -1; + esp_ipc_isr_call_blocking(esp_test_ipc_isr_get_other_core_id, &stalled_core_id); + TEST_ASSERT_EQUAL(1, stalled_core_id); + + esp_ipc_isr_release_other_cpu(); + TEST_ASSERT_FALSE(esp_ipc_isr_is_other_cpu_stalled()); + + printf("Test end\n"); +} + +TEST_CASE("Test stall other CPU safe with callbacks", "[ipc]") +{ + SemaphoreHandle_t other_cpu_task_finished = xSemaphoreCreateBinary(); + printf("Test start\n"); + + s_cmd_other_cpu_exit_critical_section = false; + s_other_cpu_in_critical_section = false; + xTaskCreatePinnedToCore(other_cpu_task_task_critical, "other_cpu_task", 4096, &other_cpu_task_finished, UNITY_FREERTOS_PRIORITY - 1, NULL, 1); + while (!s_other_cpu_in_critical_section) { + vTaskDelay(1); + } + + TEST_ASSERT_EQUAL(ESP_ERR_NOT_ALLOWED, esp_ipc_isr_stall_other_cpu_safe()); + s_cmd_other_cpu_exit_critical_section = true; + while (esp_ipc_isr_stall_other_cpu_safe() != ESP_OK) { + vTaskDelay(1); + } + TEST_ASSERT_TRUE(esp_ipc_isr_is_other_cpu_stalled()); + + int original_val = 0x12345678; + int expected_val = 0xa5a5; + int val = original_val; + esp_ipc_isr_call(esp_test_ipc_isr_callback, &val); + while (val != expected_val) {} + + val = original_val; + esp_ipc_isr_call_blocking(esp_test_ipc_isr_callback, &val); + TEST_ASSERT_EQUAL_HEX(expected_val, val); + + esp_ipc_isr_release_other_cpu(); + + xSemaphoreTake(other_cpu_task_finished, portMAX_DELAY); + printf("Test end\n"); + vSemaphoreDelete(other_cpu_task_finished); +} #endif /* CONFIG_ESP_IPC_ISR_ENABLE */ diff --git a/docs/en/api-reference/system/ipc.rst b/docs/en/api-reference/system/ipc.rst index 396c6a96eda..62e7822b4f7 100644 --- a/docs/en/api-reference/system/ipc.rst +++ b/docs/en/api-reference/system/ipc.rst @@ -97,6 +97,11 @@ The IPC feature offers the API listed below to execute a callback in a High Prio - :cpp:func:`esp_ipc_isr_call` triggers an IPC call on the target core. This function will busy-wait until the target core **begins** execution of the callback. - :cpp:func:`esp_ipc_isr_call_blocking` triggers an IPC call on the target core. This function will busy-wait until the target core **completes** execution of the callback. +These functions interrupt the other CPU and execute the callback in the context of a High Priority Interrupt. There are two common usage patterns: + +- For simple callbacks that do not enter critical sections shared with the other CPU, call :cpp:func:`esp_ipc_isr_call` or :cpp:func:`esp_ipc_isr_call_blocking` directly. +- If the calling CPU may enter critical sections used by the other CPU, or if several callbacks must run while the other CPU remains stopped, first stall the other CPU using :cpp:func:`esp_ipc_isr_stall_other_cpu` or :cpp:func:`esp_ipc_isr_stall_other_cpu_safe`. Then use :cpp:func:`esp_ipc_isr_call` or :cpp:func:`esp_ipc_isr_call_blocking` to execute callbacks. After the operation is complete, release the other CPU with :cpp:func:`esp_ipc_isr_release_other_cpu`. + .. only:: CONFIG_IDF_TARGET_ARCH_XTENSA The following code-blocks demonstrates a High Priority Interrupt IPC callback written in assembly that simply reads the target core's cycle count: @@ -117,10 +122,23 @@ The IPC feature offers the API listed below to execute a callback in a High Prio s32i a3, a2, 0 ret + The callback can be called directly when no shared critical section can deadlock: + .. code-block:: c - unit32_t cycle_count; - esp_ipc_isr_call_blocking(esp_test_ipc_isr_get_cycle_count_other_cpu, (void *)cycle_count); + uint32_t cycle_count; + esp_ipc_isr_call_blocking(esp_test_ipc_isr_get_cycle_count_other_cpu, (void *)&cycle_count); + + Alternatively, safely stall the other CPU before making one or more IPC calls: + + .. code-block:: c + + while (esp_ipc_isr_stall_other_cpu_safe() != ESP_OK) { + // Optionally, add a timeout or yield to avoid infinite loop + } + uint32_t cycle_count; + esp_ipc_isr_call_blocking(esp_test_ipc_isr_get_cycle_count_other_cpu, (void *)&cycle_count); + esp_ipc_isr_release_other_cpu(); .. note:: @@ -144,6 +162,7 @@ The High Priority Interrupt IPC API also provides the following convenience func :CONFIG_IDF_TARGET_ARCH_RISCV: - :cpp:func:`esp_ipc_isr_stall_other_cpu` stalls the target core. The calling core disables interrupts of level 3 and lower, while the target core will busy-wait with all interrupts disabled. The target core will busy-wait until :cpp:func:`esp_ipc_isr_release_other_cpu` is called. :CONFIG_IDF_TARGET_ARCH_XTENSA: - :cpp:func:`esp_ipc_isr_stall_other_cpu` stalls the target core. The calling core disables interrupts of level 3 and lower while the target core will busy-wait with interrupts of level 5 and lower disabled. The target core will busy-wait until :cpp:func:`esp_ipc_isr_release_other_cpu` is called. + - :cpp:func:`esp_ipc_isr_stall_other_cpu_safe` attempts to stall the other core only if it is not in a critical section or ISR context. If the other core is in such a state, the function considers it unsafe to stall, releases the core, and returns an error. - :cpp:func:`esp_ipc_isr_release_other_cpu` resumes the target core. Application Examples diff --git a/docs/zh_CN/api-reference/system/ipc.rst b/docs/zh_CN/api-reference/system/ipc.rst index 24cbd6f09ce..ba650e0b2f1 100644 --- a/docs/zh_CN/api-reference/system/ipc.rst +++ b/docs/zh_CN/api-reference/system/ipc.rst @@ -97,6 +97,11 @@ IPC 功能提供了下列 API，以在高优先级中断的上下文中执行回 - :cpp:func:`esp_ipc_isr_call` 能够在目标内核上触发一个 IPC 调用。在目标内核 **开始** 执行回调前，此函数将一直处于忙等待。 - :cpp:func:`esp_ipc_isr_call_blocking` 能够在目标内核上触发一个 IPC 调用。在目标内核 **完成** 回调执行前，此函数将一直处于忙等待。 +这些函数会中断另一 CPU，并在高优先级中断的上下文中执行回调。常见用法有两种： + +- 对于不会进入与另一 CPU 共享的临界区的简单回调，可以直接调用 :cpp:func:`esp_ipc_isr_call` 或 :cpp:func:`esp_ipc_isr_call_blocking`。 +- 如果调用 CPU 可能进入另一 CPU 使用的临界区，或者需要在另一 CPU 保持停止时执行多个回调，则应先使用 :cpp:func:`esp_ipc_isr_stall_other_cpu` 或 :cpp:func:`esp_ipc_isr_stall_other_cpu_safe` 暂停另一 CPU。然后使用 :cpp:func:`esp_ipc_isr_call` 或 :cpp:func:`esp_ipc_isr_call_blocking` 执行回调。操作完成后，使用 :cpp:func:`esp_ipc_isr_release_other_cpu` 释放另一 CPU。 + .. only:: CONFIG_IDF_TARGET_ARCH_XTENSA 以下示例代码用汇编语言编写了一个高优先级中断 IPC 回调，该回调的作用为读取目标内核的周期计数： @@ -117,10 +122,23 @@ IPC 功能提供了下列 API，以在高优先级中断的上下文中执行回 s32i a3, a2, 0 ret + 如果不会因为共享临界区而发生死锁，可以直接调用该回调： + .. code-block:: c - unit32_t cycle_count; - esp_ipc_isr_call_blocking(esp_test_ipc_isr_get_cycle_count_other_cpu, (void *)cycle_count); + uint32_t cycle_count; + esp_ipc_isr_call_blocking(esp_test_ipc_isr_get_cycle_count_other_cpu, (void *)&cycle_count); + + 或者，也可以在进行一次或多次 IPC 调用之前，安全地暂停另一 CPU： + + .. code-block:: c + + while (esp_ipc_isr_stall_other_cpu_safe() != ESP_OK) { + // 在生产代码中，可按需添加超时或 yield，以避免无限循环。 + } + uint32_t cycle_count; + esp_ipc_isr_call_blocking(esp_test_ipc_isr_get_cycle_count_other_cpu, (void *)&cycle_count); + esp_ipc_isr_release_other_cpu(); .. note:: @@ -144,6 +162,7 @@ IPC 功能提供了下列 API，以在高优先级中断的上下文中执行回 :CONFIG_IDF_TARGET_ARCH_RISCV: - :cpp:func:`esp_ipc_isr_stall_other_cpu`：暂停目标内核。调用内核禁用 3 级及以下级别的中断，而目标内核将在所有中断被禁用的情况下进入忙等待。在调用 :cpp:func:`esp_ipc_isr_release_other_cpu` 前，目标内核会保持忙等待。 :CONFIG_IDF_TARGET_ARCH_XTENSA: - :cpp:func:`esp_ipc_isr_stall_other_cpu`：暂停目标内核。调用内核禁用 3 级及以下级别的中断，而目标内核将在 5 级及以下的中断被禁用的情况下进入忙等待。在调用 :cpp:func:`esp_ipc_isr_release_other_cpu` 前，目标内核会保持忙等待。 + - :cpp:func:`esp_ipc_isr_stall_other_cpu_safe`：仅当另一内核不在临界区或 ISR 上下文中时，才尝试暂停该内核。如果另一内核处于此类状态，则认为暂停不安全，会释放该内核并返回错误。 - :cpp:func:`esp_ipc_isr_release_other_cpu`：恢复目标内核。 应用示例