freertos: Fix SMP RISC-V Port IDF Style critical sections

Previously the RV port was routing IDF style critical section API to call FreeRTOS style critical section API.
For example, a call to "portENTER_CRITICAL(mux)" would eventually call `vTaskEnterCritical()" via the following call flow:
- portENTER_CRITICAL(mux)
- vPortEnterCritical()
- portSET_INTERRUPT_MASK_FROM_ISR()
- vTaskEnterCritical()

This commit fixes the IDF style critical section by making sure that they are completely orthogonal to FreeRTOS critical sections

components/freertos/FreeRTOS-Kernel-SMP/portable/riscv/include/freertos/portmacro.h

@@ -377,17 +377,6 @@ static inline BaseType_t IRAM_ATTR xPortGetCoreID(void)
 #define portENABLE_INTERRUPTS()                     vPortClearInterruptMask(1)
 #define portRESTORE_INTERRUPTS(x)                   vPortClearInterruptMask(x)
 
-#define portSET_INTERRUPT_MASK_FROM_ISR() ({ \
-    unsigned int cur_level; \
-    cur_level = REG_READ(INTERRUPT_CORE0_CPU_INT_THRESH_REG); \
-    vTaskEnterCritical(); \
-    cur_level; \
-})
-#define portCLEAR_INTERRUPT_MASK_FROM_ISR(x) ({ \
-    vTaskExitCritical(); \
-    portRESTORE_INTERRUPTS(x); \
-})
-
 // ------------------ Critical Sections --------------------
 
 #define portGET_TASK_LOCK()                         vPortTakeLock(&port_xTaskLock)
@@ -395,8 +384,6 @@ static inline BaseType_t IRAM_ATTR xPortGetCoreID(void)
 #define portGET_ISR_LOCK()                          vPortTakeLock(&port_xISRLock)
 #define portRELEASE_ISR_LOCK()                      vPortReleaseLock(&port_xISRLock)
 
-#define portENTER_CRITICAL_IDF(mux)                 {(void)mux;  vPortEnterCritical();}
-#define portEXIT_CRITICAL_IDF(mux)                  {(void)mux;  vPortExitCritical();}
 
 //Critical sections used by FreeRTOS SMP
 extern void vTaskEnterCritical( void );
@@ -412,33 +399,16 @@ extern void vTaskExitCritical( void );
 #define portEXIT_CRITICAL(...)                      CHOOSE_MACRO_VA_ARG(portEXIT_CRITICAL_IDF, portEXIT_CRITICAL_SMP, ##__VA_ARGS__)(__VA_ARGS__)
 #endif
 
-#define portTRY_ENTER_CRITICAL(mux, timeout)    ({  \
-    (void)mux; (void)timeout;                       \
-    vPortEnterCritical();                           \
-    BaseType_t ret = pdPASS;                        \
-    ret;                                            \
-})
-//In single-core RISC-V, we can use the same critical section API
-#define portENTER_CRITICAL_ISR(mux)                 portENTER_CRITICAL(mux)
-#define portEXIT_CRITICAL_ISR(mux)                  portEXIT_CRITICAL(mux)
-#define portTRY_ENTER_CRITICAL_ISR(mux, timeout)    portTRY_ENTER_CRITICAL(mux, timeout)
-
-/* [refactor-todo] on RISC-V, both ISR and non-ISR cases result in the same call. We can redefine this macro */
-#define portENTER_CRITICAL_SAFE(mux)    ({  \
-    if (xPortInIsrContext()) {              \
-        portENTER_CRITICAL_ISR(mux);        \
-    } else {                                \
-        portENTER_CRITICAL(mux);            \
-    }                                       \
+#define portSET_INTERRUPT_MASK_FROM_ISR() ({ \
+    unsigned int cur_level; \
+    cur_level = REG_READ(INTERRUPT_CORE0_CPU_INT_THRESH_REG); \
+    vTaskEnterCritical(); \
+    cur_level; \
 })
-#define portEXIT_CRITICAL_SAFE(mux)     ({  \
-    if (xPortInIsrContext()) {              \
-        portEXIT_CRITICAL_ISR(mux);         \
-    } else {                                \
-        portEXIT_CRITICAL(mux);             \
-    }                                       \
+#define portCLEAR_INTERRUPT_MASK_FROM_ISR(x) ({ \
+    vTaskExitCritical(); \
+    portRESTORE_INTERRUPTS(x); \
 })
-#define portTRY_ENTER_CRITICAL_SAFE(mux, timeout)   portENTER_CRITICAL_SAFE(mux, timeout)
 
 // ---------------------- Yielding -------------------------
 
@@ -574,6 +544,25 @@ static inline void uxPortCompareSetExtram(volatile uint32_t *addr, uint32_t comp
 
 // ------------------ Critical Sections --------------------
 
+/*
+IDF style critical sections which are orthogonal to FreeRTOS critical sections. However, on single core, the IDF style
+critical sections simply disable interrupts, thus we discard the lock and timeout arguments.
+*/
+void vPortEnterCriticalIDF(void);
+void vPortExitCriticalIDF(void);
+
+//IDF task critical sections
+#define portTRY_ENTER_CRITICAL(lock, timeout)       {((void) lock; (void) timeout; vPortEnterCriticalIDF(); pdPASS;)}
+#define portENTER_CRITICAL_IDF(lock)                ({(void) lock; vPortEnterCriticalIDF();})
+#define portEXIT_CRITICAL_IDF(lock)                 ({(void) lock; vPortExitCriticalIDF();})
+//IDF ISR critical sections
+#define portTRY_ENTER_CRITICAL_ISR(lock, timeout)   {((void) lock; (void) timeout; vPortEnterCriticalIDF(); pdPASS;)}
+#define portENTER_CRITICAL_ISR(lock)                ({(void) lock; vPortEnterCriticalIDF();})
+#define portEXIT_CRITICAL_ISR(lock)                 ({(void) lock; vPortExitCriticalIDF();})
+//IDF safe critical sections (they're the same)
+#define portENTER_CRITICAL_SAFE(lock)               ({(void) lock; vPortEnterCriticalIDF();})
+#define portEXIT_CRITICAL_SAFE(lock)                ({(void) lock; vPortExitCriticalIDF();})
+
 // ---------------------- Yielding -------------------------
 
 static inline bool IRAM_ATTR xPortCanYield(void)

components/freertos/FreeRTOS-Kernel-SMP/portable/riscv/port.c

@@ -45,21 +45,17 @@
 
 static const char *TAG = "cpu_start"; // [refactor-todo]: might be appropriate to change in the future, but
 
-/**
- * @brief A variable is used to keep track of the critical section nesting.
- * @note This variable has to be stored as part of the task context and must be initialized to a non zero value
- *       to ensure interrupts don't inadvertently become unmasked before the scheduler starts.
- *       As it is stored as part of the task context it will automatically be set to 0 when the first task is started.
- */
-static UBaseType_t uxCriticalNesting = 0;
-static UBaseType_t uxSavedInterruptState = 0;
 BaseType_t uxSchedulerRunning = 0;
 UBaseType_t uxInterruptNesting = 0;
+portMUX_TYPE port_xTaskLock = portMUX_INITIALIZER_UNLOCKED;
+portMUX_TYPE port_xISRLock = portMUX_INITIALIZER_UNLOCKED;
 BaseType_t xPortSwitchFlag = 0;
 __attribute__((aligned(16))) static StackType_t xIsrStack[configISR_STACK_SIZE];
 StackType_t *xIsrStackTop = &xIsrStack[0] + (configISR_STACK_SIZE & (~((portPOINTER_SIZE_TYPE)portBYTE_ALIGNMENT_MASK)));
-portMUX_TYPE port_xTaskLock = portMUX_INITIALIZER_UNLOCKED;
-portMUX_TYPE port_xISRLock = portMUX_INITIALIZER_UNLOCKED;
+
+// Variables used for IDF style critical sections. These are orthogonal to FreeRTOS critical sections
+static UBaseType_t port_uxCriticalNestingIDF = 0;
+static UBaseType_t port_uxCriticalOldInterruptStateIDF = 0;
 
 /* ------------------------------------------------ IDF Compatibility --------------------------------------------------
  * - These need to be defined for IDF to compile
@@ -318,7 +314,7 @@ extern void esprv_intc_int_set_threshold(int); // FIXME, this function is in ROM
 BaseType_t xPortStartScheduler(void)
 {
     uxInterruptNesting = 0;
-    uxCriticalNesting = 0;
+    port_uxCriticalNestingIDF = 0;
     uxSchedulerRunning = 0;
 
     /* Setup the hardware to generate the tick. */
@@ -421,7 +417,6 @@ __attribute__((noreturn)) static void _prvTaskExitError(void)
 
     Artificially force an assert() to be triggered if configASSERT() is
     defined, then stop here so application writers can catch the error. */
-    configASSERT(uxCriticalNesting == ~0UL);
     portDISABLE_INTERRUPTS();
     abort();
 }
@@ -629,22 +624,25 @@ void vPortCleanUpTCB ( void *pxTCB )
 
 // ------------------ Critical Sections --------------------
 
-void vPortEnterCritical(void)
+void vPortEnterCriticalIDF(void)
 {
-    BaseType_t state = portSET_INTERRUPT_MASK_FROM_ISR();
-    uxCriticalNesting++;
-
-    if (uxCriticalNesting == 1) {
-        uxSavedInterruptState = state;
+    // Save current interrupt threshold and disable interrupts
+    int old_thresh = vPortSetInterruptMask();
+    // Update the IDF critical nesting count
+    port_uxCriticalNestingIDF++;
+    if (port_uxCriticalNestingIDF == 1) {
+        // Save a copy of the old interrupt threshold
+        port_uxCriticalOldInterruptStateIDF = (UBaseType_t) old_thresh;
     }
 }
 
-void vPortExitCritical(void)
+void vPortExitCriticalIDF(void)
 {
-    if (uxCriticalNesting > 0) {
-        uxCriticalNesting--;
-        if (uxCriticalNesting == 0) {
-            portCLEAR_INTERRUPT_MASK_FROM_ISR(uxSavedInterruptState);
+    if (port_uxCriticalNestingIDF > 0) {
+        port_uxCriticalNestingIDF--;
+        if (port_uxCriticalNestingIDF == 0) {
+            // Restore the saved interrupt threshold
+            vPortClearInterruptMask((int)port_uxCriticalOldInterruptStateIDF);
         }
     }
 }
@@ -712,7 +710,7 @@ void vPortYield(void)
            for an instant yield, and if that happens then the WFI would be
            waiting for the next interrupt to occur...)
         */
-        while (uxSchedulerRunning && uxCriticalNesting == 0 && REG_READ(SYSTEM_CPU_INTR_FROM_CPU_0_REG) != 0) {}
+        while (uxSchedulerRunning && REG_READ(SYSTEM_CPU_INTR_FROM_CPU_0_REG) != 0) {}
     }
 }