Merge branch 'bugfix/rom_int_functions' into 'master'

Fix broken WiFi due to intr allocation code.

TL;DR: The dynamic interrupt allocation code fixed something up to the point it broke. This code breaks off the broken code, fixing everything.

The dynamic allocated interrupt patch also fixed a bug in FreeRTOSs interrupt handling for two cores, which caused the two cores to (most of the time) have exactly the same interrupts enabled. This made WiFi fail: it would wait on a semaphore but never figure out the other core had set the semaphore, so it would keep waiting in the idle task for ever. This was caused by both the cross-core interrupt as well as the tick interrupt being disabled.

The culplrit for this appeared to be the WiFi driver, which enabled a FRC2 interrupt. The code enabling this, through a respectable amount of defines, called a rom function which called a HAL function... which was configured in such a way that it worked okay if both CPUs happened to have the same interrupts set, but broke as soon as that was not the case, possibly setting only the interrupt that was enabled and clearing all others.

The fix for this is to not use the ROM functions, but provide our own instead. Fortunately, this can be done without changes in the WiFi libs.

This MR also addresses a potentially dangerous ESP_LOG as well as some, according to the ISA document, needed rsyncs in the interrupt enable/disable code. It also marks int 9 and 10 as reserved, the WiFi code seems to use this.

See merge request !308

components/esp32/cpu_start.c

@@ -228,8 +228,11 @@ void start_cpu1_default(void)
     while (port_xSchedulerRunning[0] == 0) {
         ;
     }
+    //Take care putting stuff here: if asked, FreeRTOS will happily tell you the scheduler
+    //has started, but it isn't active *on this CPU* yet.
     esp_crosscore_int_init();
-    ESP_LOGI(TAG, "Starting scheduler on APP CPU.");
+
+    ESP_EARLY_LOGI(TAG, "Starting scheduler on APP CPU.");
     xPortStartScheduler();
 }
 #endif //!CONFIG_FREERTOS_UNICORE

components/esp32/intr_alloc.c

@@ -106,8 +106,8 @@ const static int_desc_t int_desc[32]={
     { 1, INTTP_NA,    {INT6RES,        INT6RES       } }, //6
     { 1, INTTP_NA,    {INTDESC_SPECIAL,INTDESC_SPECIAL}}, //7
     { 1, INTTP_LEVEL, {INTDESC_RESVD,  INTDESC_RESVD } }, //8
-    { 1, INTTP_LEVEL, {INTDESC_NORMAL, INTDESC_NORMAL} }, //9
-    { 1, INTTP_EDGE , {INTDESC_RESVD,  INTDESC_NORMAL} }, //10
+    { 1, INTTP_LEVEL, {INTDESC_RESVD,  INTDESC_RESVD } }, //9 //FRC1
+    { 1, INTTP_EDGE , {INTDESC_RESVD,  INTDESC_RESVD } }, //10 //FRC2
     { 3, INTTP_NA,    {INTDESC_SPECIAL,INTDESC_SPECIAL}}, //11
     { 1, INTTP_LEVEL, {INTDESC_NORMAL, INTDESC_NORMAL} }, //12
     { 1, INTTP_LEVEL, {INTDESC_NORMAL, INTDESC_NORMAL} }, //13
@@ -721,7 +721,18 @@ void esp_intr_noniram_enable()
         ::"r"(intmask):"a3");
 }
 
-
+//These functions are provided in ROM, but the ROM-based functions use non-multicore-capable
+//virtualized interrupt levels. Thus, we disable them in the ld file and provide working
+//equivalents here.
+
+
+void IRAM_ATTR ets_isr_unmask(unsigned int mask) {
+    xt_ints_on(mask);
+}
+
+void IRAM_ATTR ets_isr_mask(unsigned int mask) {
+    xt_ints_off(mask);
+}
 
 
 

components/esp32/ld/esp32.rom.ld

@@ -169,12 +169,6 @@ PROVIDE ( ets_get_xtal_scale = 0x4000856c );
 PROVIDE ( ets_install_putc1 = 0x40007d18 );
 PROVIDE ( ets_install_putc2 = 0x40007d38 );
 PROVIDE ( ets_install_uart_printf = 0x40007d28 );
-PROVIDE ( ets_intr_count = 0x3ffe03fc );
-PROVIDE ( ets_intr_lock = 0x400067b0 );
-PROVIDE ( ets_intr_unlock = 0x400067c4 );
-PROVIDE ( ets_isr_attach = 0x400067ec );
-PROVIDE ( ets_isr_mask = 0x400067fc );
-PROVIDE ( ets_isr_unmask = 0x40006808 );
 PROVIDE ( ets_post = 0x4000673c );
 PROVIDE ( ets_printf = 0x40007d54 );
 PROVIDE ( ets_readySet_ = 0x3ffe01f0 );
@@ -1730,6 +1724,13 @@ PROVIDE ( Xthal_intlevel = 0x3ff9c2b4 );
 PROVIDE ( xthal_memcpy = 0x4000c0bc );
 PROVIDE ( xthal_set_ccompare = 0x4000c058 );
 PROVIDE ( xthal_set_intclear = 0x4000c1ec );
+PROVIDE ( _xtos_set_intlevel = 0x4000bfdc );
+/* 
+These functions are xtos-related (or call xtos-related functions) and do not play well 
+with multicore FreeRTOS. Where needed, we provide alternatives that are multicore
+compatible.
+*/
+/*
 PROVIDE ( _xtos_alloca_handler = 0x40000010 );
 PROVIDE ( _xtos_cause3_handler = 0x40000dd8 );
 PROVIDE ( _xtos_c_handler_table = 0x3ffe0548 );
@@ -1748,13 +1749,19 @@ PROVIDE ( _xtos_return_from_exc = 0x4000c034 );
 PROVIDE ( _xtos_set_exception_handler = 0x4000074c );
 PROVIDE ( _xtos_set_interrupt_handler = 0x4000bf78 );
 PROVIDE ( _xtos_set_interrupt_handler_arg = 0x4000bf34 );
-PROVIDE ( _xtos_set_intlevel = 0x4000bfdc );
 PROVIDE ( _xtos_set_min_intlevel = 0x4000bff8 );
 PROVIDE ( _xtos_set_vpri = 0x40000934 );
 PROVIDE ( _xtos_syscall_handler = 0x40000790 );
 PROVIDE ( _xtos_unhandled_exception = 0x4000c024 );
 PROVIDE ( _xtos_unhandled_interrupt = 0x4000c01c );
 PROVIDE ( _xtos_vpri_enabled = 0x3ffe0654 );
+PROVIDE ( ets_intr_count = 0x3ffe03fc );
+PROVIDE ( ets_intr_lock = 0x400067b0 );
+PROVIDE ( ets_intr_unlock = 0x400067c4 );
+PROVIDE ( ets_isr_attach = 0x400067ec );
+PROVIDE ( ets_isr_mask = 0x400067fc );
+PROVIDE ( ets_isr_unmask = 0x40006808 );
+*/
 /* Following are static data, but can be used, not generated by script <<<<< btdm data */
 PROVIDE ( ld_acl_env = 0x3ffb8258 );
 PROVIDE ( ld_active_ch_map = 0x3ffb8334 );

components/esp32/lib

@@ -1 +1 @@
-Subproject commit 3a412c08af1ace47a58d1f8722a8fed5b8d3b944
+Subproject commit 5902a2229e5371aeea45c09e63ea5e233b58750f

components/freertos/test/test_tls_deletecb.c

@@ -16,7 +16,7 @@
 
 static void tskdelcb(int no, void *arg)
 {
-    printf("Delete callback: %d = %p!\n", no,  arg);
+    ets_printf("Delete callback: %d = %p!\n", no,  arg);
 }
 
 

components/freertos/xtensa_intr_asm.S

@@ -158,8 +158,10 @@ xt_ints_on:
 #else
     movi    a3, 0
     xsr     a3, INTENABLE        /* Disables all interrupts   */
+    rsync
     or      a2, a3, a2           /* set bits in mask */
     wsr     a2, INTENABLE        /* Re-enable ints */
+    rsync
     mov     a2, a3               /* return prev mask */
 #endif
 #else
@@ -206,9 +208,11 @@ xt_ints_off:
 #else
     movi    a4, 0
     xsr     a4, INTENABLE        /* Disables all interrupts   */
+    rsync
     or      a3, a4, a2           /* set bits in mask */
     xor     a3, a3, a2           /* invert bits in mask set in mask, essentially clearing them */
     wsr     a3, INTENABLE        /* Re-enable ints */
+    rsync
     mov     a2, a4               /* return prev mask */
 #endif
 #else

components/idf_test/unit_test/TestCaseAll.yml

@@ -7,9 +7,9 @@ test cases:
   category: Function
   cmd set:
   - IDFUnitTest/UnitTest
-  - - test_case = "check if ROM is used for functions"
+  - - test_case = "check if ROM or Flash is used for functions"
     - [dummy]
-  comment: check if ROM is used for functions
+  comment: check if ROM or Flash is used for functions
   execution time: 0
   expected result: 1. set succeed
   initial condition: UTINIT1