From 9ab3da0da84d24403ccbb9a9c8ace97c5ae5a3d5 Mon Sep 17 00:00:00 2001 From: Sudeep Mohanty Date: Tue, 4 Aug 2026 16:18:03 +0200 Subject: [PATCH] feat(freertos): speed up SMP critical section enter and exit The critical section port already disables interrupts and reads the core id for the whole section, but spinlock_acquire()/spinlock_release() then disabled interrupts again to the same level and re-read the core id register. Add spinlock_acquire_impl()/spinlock_release_impl(), which take a caller-supplied owner id and skip interrupt management, and reuse them from spinlock_acquire()/spinlock_release() to avoid duplicated code. The Xtensa and RISC-V ports now read the core id once and call the impl variants, removing one core id read and one interrupt mask/restore per critical section enter and exit. Closes https://github.com/espressif/esp-idf/issues/18908 --- components/esp_hw_support/include/spinlock.h | 230 +++++++++++++----- .../portable/xtensa/port.c | 16 +- .../FreeRTOS-Kernel/portable/riscv/port.c | 13 +- .../FreeRTOS-Kernel/portable/xtensa/port.c | 16 +- components/xtensa/include/xt_utils.h | 49 ++-- 5 files changed, 232 insertions(+), 92 deletions(-) diff --git a/components/esp_hw_support/include/spinlock.h b/components/esp_hw_support/include/spinlock.h index e34f7564879..d978c3dd619 100644 --- a/components/esp_hw_support/include/spinlock.h +++ b/components/esp_hw_support/include/spinlock.h @@ -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 */ @@ -9,6 +9,7 @@ #include #include #include "esp_cpu.h" +#include "soc/soc_caps.h" #if __XTENSA__ #include "xtensa/xtruntime.h" @@ -58,40 +59,118 @@ static inline void __attribute__((always_inline)) spinlock_initialize(spinlock_t } /** - * @brief Top level spinlock acquire function, spins until get the lock + * @brief Get the spinlock owner id of the executing core * - * This function will: - * - Save current interrupt state, then disable interrupts - * - Spin until lock is acquired or until timeout occurs - * - Restore interrupt state - * - * @note Spinlocks alone do no constitute true critical sections (as this - * function reenables interrupts once the spinlock is acquired). For critical - * sections, use the interface provided by the operating system. - * @param lock - target spinlock object - * @param timeout - cycles to wait, passing SPINLOCK_WAIT_FOREVER blocks indefinitely + * @return owner id of the current core */ -static inline bool __attribute__((always_inline)) spinlock_acquire(spinlock_t *lock, int32_t timeout) +static inline uint32_t __attribute__((always_inline)) spinlock_owner_id(void) +{ +#if SOC_CPU_CORES_NUM > 1 +#if __XTENSA__ + // On Xtensa the raw PRID register value is used directly as the owner id + // (the full 32 bit CORE_ID_REGVAL_PRO/CORE_ID_REGVAL_APP values). + return xt_utils_get_raw_core_id(); +#else //__riscv + return rv_utils_get_core_id() == 0 ? SPINLOCK_OWNER_ID_0 : SPINLOCK_OWNER_ID_1; +#endif +#else + return 0; +#endif +} + +/** + * @brief Get the spinlock owner id for a given core index + * + * Lets a caller that already knows the executing core's index derive the owner id without reading the core id register. + * + * @param core_id - core index (0 or 1) + * @return owner id for that core + */ +static inline uint32_t __attribute__((always_inline)) spinlock_owner_id_for_core(uint32_t core_id) +{ + return core_id ? SPINLOCK_OWNER_ID_1 : SPINLOCK_OWNER_ID_0; +} + +/** + * @brief Get the core index of a given spinlock owner id + * + * Inverse of spinlock_owner_id_for_core(). Lets a caller that already has the owner id (e.g. from spinlock_owner_id()) + * derive the core index without reading the core id register again. + * + * @param owner_id - a spinlock owner id + * @return core index (0 or 1) that owner id belongs to + */ +static inline uint32_t __attribute__((always_inline)) spinlock_core_id_from_owner_id(uint32_t owner_id) +{ +#if __XTENSA__ + // On Xtensa the owner id is the raw PRID register value. + return xt_utils_get_core_id_from_raw(owner_id); +#else //__riscv + return owner_id == SPINLOCK_OWNER_ID_0 ? 0 : 1; +#endif +} + +/** + * @brief Disable interrupts on the current core and return the previous interrupt state + * + * Disables up to the level used to protect spinlocks. Spinlocks are compiled out to no-ops on single-core and + * bootloader builds. + */ +static inline uint32_t __attribute__((always_inline)) spinlock_int_disable(void) +{ +#if !CONFIG_ESP_SYSTEM_SINGLE_CORE_MODE && !BOOTLOADER_BUILD +#if __XTENSA__ + return XTOS_SET_INTLEVEL(XCHAL_EXCM_LEVEL); +#elif SOC_INT_CLIC_SUPPORTED + return rv_utils_set_intlevel_regval(RVHAL_EXCM_LEVEL_CLIC); +#else + return rv_utils_set_intlevel_regval(RVHAL_EXCM_LEVEL); +#endif +#else + return 0; +#endif +} + +/** + * @brief Restore interrupts to a state previously returned by spinlock_int_disable() + */ +static inline void __attribute__((always_inline)) spinlock_int_restore(uint32_t int_state) +{ +#if !CONFIG_ESP_SYSTEM_SINGLE_CORE_MODE && !BOOTLOADER_BUILD +#if __XTENSA__ + XTOS_RESTORE_INTLEVEL(int_state); +#else //__riscv + rv_utils_restore_intlevel_regval(int_state); +#endif +#else + (void)int_state; +#endif +} + +/** + * @brief Acquire a spinlock without managing interrupts + * + * Performs the lock acquisition (owner tracking + atomic compare-and-set spin) but, unlike spinlock_acquire(), does + * NOT save/disable/restore interrupts and takes a caller-supplied owner id instead of reading the core id register. + * + * @note The caller MUST disable interrupts before calling and keep them disabled until the matching + * spinlock_release_impl(). The owner id must be that of the executing core + * (obtain it via spinlock_owner_id() or spinlock_owner_id_for_core()). + * + * @param lock - target spinlock object + * @param timeout - cycles to wait, passing SPINLOCK_WAIT_FOREVER blocks indefinitely + * @param core_owner_id - owner id of the executing core + * @return true if the lock was acquired, false on timeout + */ +static inline bool __attribute__((always_inline)) spinlock_acquire_impl(spinlock_t *lock, int32_t timeout, uint32_t core_owner_id) { #if !CONFIG_ESP_SYSTEM_SINGLE_CORE_MODE && !BOOTLOADER_BUILD - uint32_t irq_status; - uint32_t core_owner_id; // Unused if asserts are disabled uint32_t __attribute__((unused)) other_core_owner_id; bool lock_set; esp_cpu_cycle_count_t start_count; assert(lock); -#if __XTENSA__ - irq_status = XTOS_SET_INTLEVEL(XCHAL_EXCM_LEVEL); - - // Note: The core IDs are the full 32 bit (CORE_ID_REGVAL_PRO/CORE_ID_REGVAL_APP) values - core_owner_id = xt_utils_get_raw_core_id(); -#else //__riscv - - irq_status = rv_utils_set_intlevel_regval(RVHAL_EXCM_LEVEL_CLIC); - core_owner_id = rv_utils_get_core_id() == 0 ? SPINLOCK_OWNER_ID_0 : SPINLOCK_OWNER_ID_1; -#endif other_core_owner_id = CORE_ID_REGVAL_XOR_SWAP ^ core_owner_id; /* lock->owner should be one of SPINLOCK_FREE, CORE_ID_REGVAL_PRO, @@ -105,11 +184,6 @@ static inline bool __attribute__((always_inline)) spinlock_acquire(spinlock_t *l if (lock->owner == core_owner_id) { assert(lock->count > 0 && lock->count < 0xFF); // Bad count value implies memory corruption lock->count++; -#if __XTENSA__ - XTOS_RESTORE_INTLEVEL(irq_status); -#else - rv_utils_restore_intlevel_regval(irq_status); -#endif return true; } @@ -146,18 +220,74 @@ exit: assert(lock->count < 0xFF); // Bad count value implies memory corruption } -#if __XTENSA__ - XTOS_RESTORE_INTLEVEL(irq_status); -#else - rv_utils_restore_intlevel_regval(irq_status); -#endif return lock_set; #else // !CONFIG_ESP_SYSTEM_SINGLE_CORE_MODE + (void)lock; + (void)timeout; + (void)core_owner_id; return true; #endif } +/** + * @brief Top level spinlock acquire function, spins until get the lock + * + * This function will: + * - Save current interrupt state, then disable interrupts + * - Spin until lock is acquired or until timeout occurs + * - Restore interrupt state + * + * @note Spinlocks alone do no constitute true critical sections (as this + * function reenables interrupts once the spinlock is acquired). For critical + * sections, use the interface provided by the operating system. + * @param lock - target spinlock object + * @param timeout - cycles to wait, passing SPINLOCK_WAIT_FOREVER blocks indefinitely + */ +static inline bool __attribute__((always_inline)) spinlock_acquire(spinlock_t *lock, int32_t timeout) +{ +#if !CONFIG_ESP_SYSTEM_SINGLE_CORE_MODE && !BOOTLOADER_BUILD + uint32_t irq_status = spinlock_int_disable(); + bool lock_set = spinlock_acquire_impl(lock, timeout, spinlock_owner_id()); + spinlock_int_restore(irq_status); + return lock_set; +#else + (void)lock; + (void)timeout; + return true; +#endif +} + +/** + * @brief Release a spinlock without managing interrupts + * + * Unlike spinlock_release(), does NOT save/disable/restore interrupts and takes a caller-supplied owner id (only used + * by the debug assert that validates lock ownership). + * + * @note The caller MUST have interrupts disabled (matching the preceding spinlock_acquire_impl()). + * + * @param lock - target, locked before, spinlock object + * @param core_owner_id - owner id of the executing core (only used for the debug assert) + */ +static inline void __attribute__((always_inline)) spinlock_release_impl(spinlock_t *lock, uint32_t core_owner_id) +{ +#if !CONFIG_ESP_SYSTEM_SINGLE_CORE_MODE && !BOOTLOADER_BUILD + assert(lock); + assert(core_owner_id == lock->owner); // This is a lock that we didn't acquire, or the lock is corrupt + (void)core_owner_id; + lock->count--; + + if (!lock->count) { // If this is the last recursive release of the lock, mark the lock as free + lock->owner = SPINLOCK_FREE; + } else { + assert(lock->count < 0x100); // Indicates memory corruption + } +#else + (void)lock; + (void)core_owner_id; +#endif +} + /** * @brief Top level spinlock unlock function, unlocks a previously locked spinlock * @@ -174,34 +304,12 @@ exit: static inline void __attribute__((always_inline)) spinlock_release(spinlock_t *lock) { #if !CONFIG_ESP_SYSTEM_SINGLE_CORE_MODE && !BOOTLOADER_BUILD - uint32_t irq_status; - // Return value unused if asserts are disabled - uint32_t __attribute__((unused)) core_owner_id; - - assert(lock); -#if __XTENSA__ - irq_status = XTOS_SET_INTLEVEL(XCHAL_EXCM_LEVEL); - - core_owner_id = xt_utils_get_raw_core_id(); + uint32_t irq_status = spinlock_int_disable(); + spinlock_release_impl(lock, spinlock_owner_id()); + spinlock_int_restore(irq_status); #else - irq_status = rv_utils_set_intlevel_regval(RVHAL_EXCM_LEVEL_CLIC); - core_owner_id = rv_utils_get_core_id() == 0 ? SPINLOCK_OWNER_ID_0 : SPINLOCK_OWNER_ID_1; + (void)lock; #endif - assert(core_owner_id == lock->owner); // This is a lock that we didn't acquire, or the lock is corrupt - lock->count--; - - if (!lock->count) { // If this is the last recursive release of the lock, mark the lock as free - lock->owner = SPINLOCK_FREE; - } else { - assert(lock->count < 0x100); // Indicates memory corruption - } - -#if __XTENSA__ - XTOS_RESTORE_INTLEVEL(irq_status); -#else - rv_utils_restore_intlevel_regval(irq_status); -#endif //#if __XTENSA__ -#endif //#if !CONFIG_ESP_SYSTEM_SINGLE_CORE_MODE && !BOOTLOADER_BUILD } #ifdef __cplusplus diff --git a/components/freertos/FreeRTOS-Kernel-SMP/portable/xtensa/port.c b/components/freertos/FreeRTOS-Kernel-SMP/portable/xtensa/port.c index eb594a2ed15..eefbf7881b3 100644 --- a/components/freertos/FreeRTOS-Kernel-SMP/portable/xtensa/port.c +++ b/components/freertos/FreeRTOS-Kernel-SMP/portable/xtensa/port.c @@ -143,13 +143,17 @@ BaseType_t xPortEnterCriticalTimeout(portMUX_TYPE *lock, BaseType_t timeout) * saved level can be restored on the last call to exit the critical. */ BaseType_t xOldInterruptLevel = XTOS_SET_INTLEVEL(XCHAL_EXCM_LEVEL); - if (!spinlock_acquire(lock, timeout)) { + /* Interrupts are masked (to XCHAL_EXCM_LEVEL, the same level the spinlock uses), + * so the core id is stable and the spinlock does not need to mask again. Read the + * core id register once and reuse it for the owner id and the nesting index. */ + uint32_t coreOwnerId = spinlock_owner_id(); + BaseType_t coreID = spinlock_core_id_from_owner_id(coreOwnerId); + if (!spinlock_acquire_impl(lock, timeout, coreOwnerId)) { //Timed out attempting to get spinlock. Restore previous interrupt level and return XTOS_RESTORE_JUST_INTLEVEL((int) xOldInterruptLevel); return pdFAIL; } //Spinlock acquired. Increment the IDF critical nesting count. - BaseType_t coreID = xPortGetCoreID(); BaseType_t newNesting = port_uxCriticalNestingIDF[coreID] + 1; port_uxCriticalNestingIDF[coreID] = newNesting; //If this is the first entry to a critical section. Save the old interrupt level. @@ -171,8 +175,12 @@ void vPortExitCriticalIDF(portMUX_TYPE *lock) * to re-enable interrupts if this is the last call to exit the critical. We * can use the nesting count to determine whether this is the last exit call. */ - spinlock_release(lock); - BaseType_t coreID = xPortGetCoreID(); + /* Interrupts remain disabled for the whole critical section, so the core id is + * stable and the spinlock does not need to mask again. Read the core id register + * once and reuse it for the owner id and the nesting index. */ + uint32_t coreOwnerId = spinlock_owner_id(); + BaseType_t coreID = spinlock_core_id_from_owner_id(coreOwnerId); + spinlock_release_impl(lock, coreOwnerId); BaseType_t nesting = port_uxCriticalNestingIDF[coreID]; /* Critical section nesting count must never be negative */ diff --git a/components/freertos/FreeRTOS-Kernel/portable/riscv/port.c b/components/freertos/FreeRTOS-Kernel/portable/riscv/port.c index 151caa2b9e8..e1b9f3bd894 100644 --- a/components/freertos/FreeRTOS-Kernel/portable/riscv/port.c +++ b/components/freertos/FreeRTOS-Kernel/portable/riscv/port.c @@ -542,13 +542,17 @@ BaseType_t __attribute__((optimize("-O3"))) xPortEnterCriticalTimeout(portMUX_TY * saved level can be restored on the last call to exit the critical. */ BaseType_t xOldInterruptLevel = portSET_INTERRUPT_MASK_FROM_ISR(); - if (!spinlock_acquire(mux, timeout)) { + /* Interrupts are masked, so the core id is stable. Read it once and reuse it for + * both the spinlock owner id and the per-core critical nesting state. Interrupts + * stay masked for the whole critical section (to the same level the spinlock would + * use), so the spinlock does not need to disable them again. */ + BaseType_t coreID = xPortGetCoreID(); + if (!spinlock_acquire_impl(mux, timeout, spinlock_owner_id_for_core(coreID))) { //Timed out attempting to get spinlock. Restore previous interrupt level and return portCLEAR_INTERRUPT_MASK_FROM_ISR(xOldInterruptLevel); return pdFAIL; } //Spinlock acquired. Increment the critical nesting count. - BaseType_t coreID = xPortGetCoreID(); BaseType_t newNesting = port_uxCriticalNesting[coreID] + 1; port_uxCriticalNesting[coreID] = newNesting; //If this is the first entry to a critical section. Save the old interrupt level. @@ -569,8 +573,11 @@ void __attribute__((optimize("-O3"))) vPortExitCriticalMultiCore(portMUX_TYPE *m * to re-enable interrupts if this is the last call to exit the critical. We * can use the nesting count to determine whether this is the last exit call. */ - spinlock_release(mux); + /* Interrupts remain disabled for the whole critical section, so the core id is + * stable and the spinlock does not need to mask them again. Read the core id + * once and reuse it for both the release owner id and the per-core nesting state. */ BaseType_t coreID = xPortGetCoreID(); + spinlock_release_impl(mux, spinlock_owner_id_for_core(coreID)); BaseType_t nesting = port_uxCriticalNesting[coreID]; /* Critical section nesting count must never be negative */ diff --git a/components/freertos/FreeRTOS-Kernel/portable/xtensa/port.c b/components/freertos/FreeRTOS-Kernel/portable/xtensa/port.c index ec2be1c6185..84fa14a63be 100644 --- a/components/freertos/FreeRTOS-Kernel/portable/xtensa/port.c +++ b/components/freertos/FreeRTOS-Kernel/portable/xtensa/port.c @@ -511,13 +511,17 @@ BaseType_t __attribute__((optimize("-O3"))) xPortEnterCriticalTimeout(portMUX_TY * saved level can be restored on the last call to exit the critical. */ BaseType_t xOldInterruptLevel = portSET_INTERRUPT_MASK_FROM_ISR(); - if (!spinlock_acquire(mux, timeout)) { + /* Interrupts are masked, so the core id is stable. Read the core id register once + * (its value is already the spinlock owner id on Xtensa) and reuse it, so + * spinlock_acquire does not read the core id register a second time. */ + uint32_t coreOwnerId = spinlock_owner_id(); + BaseType_t coreID = spinlock_core_id_from_owner_id(coreOwnerId); + if (!spinlock_acquire_impl(mux, timeout, coreOwnerId)) { //Timed out attempting to get spinlock. Restore previous interrupt level and return portCLEAR_INTERRUPT_MASK_FROM_ISR(xOldInterruptLevel); return pdFAIL; } //Spinlock acquired. Increment the critical nesting count. - BaseType_t coreID = xPortGetCoreID(); BaseType_t newNesting = port_uxCriticalNesting[coreID] + 1; port_uxCriticalNesting[coreID] = newNesting; //If this is the first entry to a critical section. Save the old interrupt level. @@ -538,8 +542,12 @@ void __attribute__((optimize("-O3"))) vPortExitCritical(portMUX_TYPE *mux) * to re-enable interrupts if this is the last call to exit the critical. We * can use the nesting count to determine whether this is the last exit call. */ - spinlock_release(mux); - BaseType_t coreID = xPortGetCoreID(); + /* Interrupts remain disabled for the whole critical section, so the core id is + * stable and the spinlock does not need to mask them again. Read the core id + * register once (its value is the spinlock owner id on Xtensa) and reuse it. */ + uint32_t coreOwnerId = spinlock_owner_id(); + BaseType_t coreID = spinlock_core_id_from_owner_id(coreOwnerId); + spinlock_release_impl(mux, coreOwnerId); BaseType_t nesting = port_uxCriticalNesting[coreID]; /* Critical section nesting count must never be negative */ diff --git a/components/xtensa/include/xt_utils.h b/components/xtensa/include/xt_utils.h index 2499d54b29f..70e962fc41c 100644 --- a/components/xtensa/include/xt_utils.h +++ b/components/xtensa/include/xt_utils.h @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2020-2025 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2020-2026 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ @@ -25,25 +25,6 @@ extern "C" { * * ------------------------------------------------------------------------------------------------------------------ */ -FORCE_INLINE_ATTR __attribute__((pure)) uint32_t xt_utils_get_core_id(void) -{ - /* - Note: We depend on SOC_CPU_CORES_NUM instead of XCHAL_HAVE_PRID as some single Xtensa targets (such as ESP32-S2) have - the PRID register even though they are single core. - */ -#if SOC_CPU_CORES_NUM > 1 - // Read and extract bit 13 of special register PRID - uint32_t id; - asm volatile ( - "rsr.prid %0\n" - "extui %0,%0,13,1" - :"=r"(id)); - return id; -#else - return 0; -#endif // SOC_CPU_CORES_NUM > 1 -} - FORCE_INLINE_ATTR __attribute__((pure)) uint32_t xt_utils_get_raw_core_id(void) { #if XCHAL_HAVE_PRID @@ -58,6 +39,34 @@ FORCE_INLINE_ATTR __attribute__((pure)) uint32_t xt_utils_get_raw_core_id(void) #endif // XCHAL_HAVE_PRID } +FORCE_INLINE_ATTR __attribute__((pure)) uint32_t xt_utils_get_core_id_from_raw(uint32_t raw_core_id) +{ + /* + Note: We depend on SOC_CPU_CORES_NUM instead of XCHAL_HAVE_PRID as some single Xtensa targets (such as ESP32-S2) have + the PRID register even though they are single core. + */ +#if SOC_CPU_CORES_NUM > 1 + // Extract bit 13 of the PRID register value + uint32_t id; + asm volatile ( + "extui %0,%1,13,1" + :"=r"(id):"r"(raw_core_id)); + return id; +#else + (void)raw_core_id; + return 0; +#endif // SOC_CPU_CORES_NUM > 1 +} + +FORCE_INLINE_ATTR __attribute__((pure)) uint32_t xt_utils_get_core_id(void) +{ +#if SOC_CPU_CORES_NUM > 1 + return xt_utils_get_core_id_from_raw(xt_utils_get_raw_core_id()); +#else + return 0; +#endif // SOC_CPU_CORES_NUM > 1 +} + FORCE_INLINE_ATTR void *xt_utils_get_sp(void) { void *sp;