Merge branch 'fix/intr_alloc_level_bug_v5.4' into 'release/v5.4'

fix(esp_hw_support): fix a bug in the interrupt allocator related to shared interrupts (backport v5.4)

See merge request espressif/esp-idf!35477

components/esp_hw_support/intr_alloc.c

@@ -73,6 +73,15 @@ struct shared_vector_desc_t {
 #define VECDESC_FL_SHARED       (1<<2)
 #define VECDESC_FL_NONSHARED    (1<<3)
 
+#if SOC_CPU_HAS_FLEXIBLE_INTC
+/* On targets that have configurable interrupts levels, store the assigned level in the flags */
+#define VECDESC_FL_LEVEL_SHIFT  (8)
+/* Allocate 4 bits in the flag */
+#define VECDESC_FL_LEVEL_MASK   (0xf)
+/* Help to extract the level from flags */
+#define VECDESC_FL_LEVEL(flags) (((flags) >> VECDESC_FL_LEVEL_SHIFT) & VECDESC_FL_LEVEL_MASK)
+#endif
+
 //Pack using bitfields for better memory use
 struct vector_desc_t {
     int flags: 16;                          //OR of VECDESC_FL_* defines
@@ -258,7 +267,17 @@ static bool is_vect_desc_usable(vector_desc_t *vd, int flags, int cpu, int force
         return false;
     }
 
-#ifndef SOC_CPU_HAS_FLEXIBLE_INTC
+#if SOC_CPU_HAS_FLEXIBLE_INTC
+    /* On target that have configurable interrupts levels, check if the interrupt has already
+     * been allocated, and if yes, make sure the levels are compatible. */
+    const int vector_lvl = VECDESC_FL_LEVEL(vd->flags);
+    /* A non-zero value means the level has already been set prior to this allocation, make
+     * sure the current level matches what we need. */
+    if (vector_lvl != 0 && (flags & (1 << vector_lvl)) == 0) {
+        ALCHLOG("....Unusable: incompatible priority");
+        return false;
+    }
+#else
     //Check if the interrupt priority is acceptable
     if (!(flags & (1 << intr_desc.priority))) {
         ALCHLOG("....Unusable: incompatible priority");
@@ -640,6 +659,7 @@ esp_err_t esp_intr_alloc_intrstatus(int source, int flags, uint32_t intrstatusre
 #if SOC_CPU_HAS_FLEXIBLE_INTC
     //Extract the level from the interrupt passed flags
     int level = esp_intr_flags_to_level(flags);
+    vd->flags |= level << VECDESC_FL_LEVEL_SHIFT;
     esp_cpu_intr_set_priority(intr, level);
 
     if (flags & ESP_INTR_FLAG_EDGE) {
@@ -786,6 +806,10 @@ static esp_err_t intr_free_for_current_cpu(intr_handle_t handle)
         //we save.(We can also not use the same exit path for empty shared ints anymore if we delete
         //the desc.) For now, just mark it as free.
         handle->vector_desc->flags &= ~(VECDESC_FL_NONSHARED|VECDESC_FL_RESERVED|VECDESC_FL_SHARED);
+#if SOC_CPU_HAS_FLEXIBLE_INTC
+        //Clear the assigned level
+        handle->vector_desc->flags &= ~(VECDESC_FL_LEVEL_MASK << VECDESC_FL_LEVEL_SHIFT);
+#endif
         handle->vector_desc->source = ETS_INTERNAL_UNUSED_INTR_SOURCE;
 
         //Also kill non_iram mask bit.
@@ -798,11 +822,17 @@ static esp_err_t intr_free_for_current_cpu(intr_handle_t handle)
 
 int esp_intr_get_intno(intr_handle_t handle)
 {
+    if (handle == NULL || handle->vector_desc == NULL) {
+        return -1;
+    }
     return handle->vector_desc->intno;
 }
 
 int esp_intr_get_cpu(intr_handle_t handle)
 {
+    if (handle == NULL || handle->vector_desc == NULL) {
+        return -1;
+    }
     return handle->vector_desc->cpu;
 }
 

components/esp_hw_support/test_apps/esp_hw_support_unity_tests/main/CMakeLists.txt

@@ -22,7 +22,7 @@ if(CONFIG_SOC_ETM_SUPPORTED)
 endif()
 
 if(CONFIG_SOC_GPTIMER_SUPPORTED)
-    list(APPEND SRC "test_intr_alloc.c")
+    list(APPEND srcs "test_intr_alloc.c")
 endif()
 
 # In order for the cases defined by `TEST_CASE` to be linked into the final elf,

components/esp_hw_support/test_apps/esp_hw_support_unity_tests/main/test_intr_alloc.c

@@ -66,7 +66,7 @@ static void timer_test(int flags)
     }
 
     if ((flags & ESP_INTR_FLAG_SHARED)) {
-        /* Check that the allocated interrupts are acutally shared */
+        /* Check that the allocated interrupts are actually shared */
         int intr_num = esp_intr_get_intno(inth[0]);
         for (int i = 0; i < SOC_TIMER_GROUP_TOTAL_TIMERS; i++) {
             TEST_ASSERT_EQUAL(intr_num, esp_intr_get_intno(inth[i]));
@@ -124,6 +124,73 @@ void static test_isr(void*arg)
 }
 
 
+TEST_CASE("Intr_alloc test, shared interrupts don't affect level", "[intr_alloc]")
+{
+    intr_handle_t handle_lvl_1;
+    intr_handle_t handle_lvl_2;
+
+    /* Allocate an interrupt of level 1 that will be shared with another source */
+    esp_err_t err = esp_intr_alloc(ETS_FROM_CPU_INTR2_SOURCE,
+                                   ESP_INTR_FLAG_LEVEL1 | ESP_INTR_FLAG_SHARED,
+                                   test_isr, NULL, &handle_lvl_1);
+    TEST_ESP_OK(err);
+
+    /* Allocate a shared interrupt of a different level */
+    err = esp_intr_alloc(ETS_FROM_CPU_INTR3_SOURCE,
+                         ESP_INTR_FLAG_LEVEL2  | ESP_INTR_FLAG_SHARED,
+                         test_isr, NULL, &handle_lvl_2);
+    TEST_ESP_OK(err);
+
+    /* Make sure the allocated CPU line is NOT the same for both sources */
+    const int intlvl1 = esp_intr_get_intno(handle_lvl_1);
+    const int intlvl2 = esp_intr_get_intno(handle_lvl_2);
+    printf("Level 1 interrupt allocated: %d\n", intlvl1);
+    printf("Level 2 interrupt allocated: %d\n", intlvl2);
+    TEST_ASSERT(intlvl1 != intlvl2);
+
+    TEST_ESP_OK(esp_intr_free(handle_lvl_1));
+    TEST_ESP_OK(esp_intr_free(handle_lvl_2));
+}
+
+
+#if SOC_CPU_HAS_FLEXIBLE_INTC
+
+/**
+ * On targets that have flexible interrupt levels, make sure that a shared interrupt sees its level
+ * being cleared (and reconfigurable) uupon remove and reallocation.
+ */
+TEST_CASE("Intr_alloc test, shared interrupts custom level cleared", "[intr_alloc]")
+{
+    intr_handle_t handle;
+
+    esp_err_t err = esp_intr_alloc(ETS_FROM_CPU_INTR2_SOURCE,
+                                   ESP_INTR_FLAG_LEVEL1 | ESP_INTR_FLAG_SHARED,
+                                   test_isr, NULL, &handle);
+    TEST_ESP_OK(err);
+    const int first_intno = esp_intr_get_intno(handle);
+    /* Make sure the priority is correct */
+    TEST_ASSERT_EQUAL(1, esp_cpu_intr_get_priority(first_intno));
+
+    /* Free the shared interrupt and try to reallocate it with another level */
+    TEST_ESP_OK(esp_intr_free(handle));
+
+    err = esp_intr_alloc(ETS_FROM_CPU_INTR3_SOURCE,
+                         ESP_INTR_FLAG_LEVEL2  | ESP_INTR_FLAG_SHARED,
+                         test_isr, NULL, &handle);
+    TEST_ESP_OK(err);
+
+    /* Make sure they are both the same and the level has been updated */
+    const int second_intno = esp_intr_get_intno(handle);
+    TEST_ASSERT_EQUAL(2, esp_cpu_intr_get_priority(second_intno));
+    TEST_ASSERT(first_intno == second_intno);
+
+    /* Delete the interrupt */
+    TEST_ESP_OK(esp_intr_free(handle));
+}
+
+#endif
+
+
 TEST_CASE("Allocate previously freed interrupt, with different flags", "[intr_alloc]")
 {
     intr_handle_t intr;