Skip to content

Commit d8531cb

Browse files
ChocolateFrogsNutsdevyte
authored andcommitted
Ets intr lock nest (esp8266#6484)
* Replace the SDK's use of ets_intr_lock/unlock with nestable versions Testing has shown that there are several paths in the SDK that result in nested calls to ets_intr_lock() / ets_intr_unlock() which may be a problem. These functions also do not preserve the enabled interrupt level and may result in code running with interrupts enabled when that is not intended. This issue has recently been fixed in the Arduino code by using xt_rsil() / xt_wsr_ps() but still exists in the Espressif SDK code. This commit is intended to fix that and should be used in addition to the above. The maximum nesting I have seen is 2 and lock/unlock calls appear to be balanced. A max of 7 levels of nesting leaves plenty of room for that to change. * make ets_intr_lock_stack uint16_t and behave like the original on over/underflow The PS register is 15 bits, we should store the whole thing as xt_wsr_ps() writes the whole thing. Also if there is an underflow, we should make sure interrupts are enabled. Same goes for overflow making sure interrupts are disabled, although this is less important. * Rename ets_intr_(un)lock_nest to ets_intr_(un)lock This saves having to modify libmain.a, libpp.a and libnet80211.a to use the nested versions. Adjusts fix_sdk_libs.sh accordingly. * Remove ets_intr_(un)lock from the rom .ld as we no longer use them * ets_post() wrapper to preserve interrupt state Add a wrapper around the ets_post code in rom to preserve the interrupt enable state. Rather than modifying the SDK libs, rename ets_post in the .ld file and call the wrapper "ets_post" to replace it. As far as I can establish, ets_post is the only rom function in use by our code or the SDK libs we use that causes calls to ets_intr_(un)lock. * Add IRAM_ATTR to ets_intr_(un)lock and ets_post wrappers. * Throw in a few comments and make ets_intr_lock_stack* static.
1 parent 1f86311 commit d8531cb

File tree

2 files changed

+41
-0
lines changed

2 files changed

+41
-0
lines changed

cores/esp8266/core_esp8266_main.cpp

+36
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,13 @@ static os_event_t s_loop_queue[LOOP_QUEUE_SIZE];
6060
/* Used to implement optimistic_yield */
6161
static uint32_t s_micros_at_task_start;
6262

63+
/* For ets_intr_lock_nest / ets_intr_unlock_nest
64+
* Max nesting seen by SDK so far is 2.
65+
*/
66+
#define ETS_INTR_LOCK_NEST_MAX 7
67+
static uint16_t ets_intr_lock_stack[ETS_INTR_LOCK_NEST_MAX];
68+
static byte ets_intr_lock_stack_ptr=0;
69+
6370

6471
extern "C" {
6572
extern const uint32_t __attribute__((section(".ver_number"))) core_version = ARDUINO_ESP8266_GIT_VER;
@@ -121,6 +128,35 @@ extern "C" void optimistic_yield(uint32_t interval_us) {
121128
}
122129
}
123130

131+
132+
// Replace ets_intr_(un)lock with nestable versions
133+
extern "C" void IRAM_ATTR ets_intr_lock() {
134+
if (ets_intr_lock_stack_ptr < ETS_INTR_LOCK_NEST_MAX)
135+
ets_intr_lock_stack[ets_intr_lock_stack_ptr++] = xt_rsil(3);
136+
else
137+
xt_rsil(3);
138+
}
139+
140+
extern "C" void IRAM_ATTR ets_intr_unlock() {
141+
if (ets_intr_lock_stack_ptr > 0)
142+
xt_wsr_ps(ets_intr_lock_stack[--ets_intr_lock_stack_ptr]);
143+
else
144+
xt_rsil(0);
145+
}
146+
147+
148+
// Save / Restore the PS state across the rom ets_post call as the rom code
149+
// does not implement this correctly.
150+
extern "C" bool ets_post_rom(uint8 prio, ETSSignal sig, ETSParam par);
151+
152+
extern "C" bool IRAM_ATTR ets_post(uint8 prio, ETSSignal sig, ETSParam par) {
153+
uint32_t saved;
154+
asm volatile ("rsr %0,ps":"=a" (saved));
155+
bool rc=ets_post_rom(prio, sig, par);
156+
xt_wsr_ps(saved);
157+
return rc;
158+
}
159+
124160
extern "C" void __loop_end (void)
125161
{
126162
run_scheduled_functions();

tools/sdk/ld/eagle.rom.addr.v6.ld

+5
Original file line numberDiff line numberDiff line change
@@ -119,16 +119,21 @@ PROVIDE ( ets_install_external_printf = 0x40002450 );
119119
PROVIDE ( ets_install_putc1 = 0x4000242c );
120120
PROVIDE ( ets_install_putc2 = 0x4000248c );
121121
PROVIDE ( ets_install_uart_printf = 0x40002438 );
122+
/* permanently hide reimplemented ets_intr_*lock(), see #6484
122123
PROVIDE ( ets_intr_lock = 0x40000f74 );
123124
PROVIDE ( ets_intr_unlock = 0x40000f80 );
125+
*/
124126
PROVIDE ( ets_isr_attach = 0x40000f88 );
125127
PROVIDE ( ets_isr_mask = 0x40000f98 );
126128
PROVIDE ( ets_isr_unmask = 0x40000fa8 );
127129
PROVIDE ( ets_memcmp = 0x400018d4 );
128130
PROVIDE ( ets_memcpy = 0x400018b4 );
129131
PROVIDE ( ets_memmove = 0x400018c4 );
130132
PROVIDE ( ets_memset = 0x400018a4 );
133+
/* renamed to ets_post_rom(), see #6484
131134
PROVIDE ( ets_post = 0x40000e24 );
135+
*/
136+
PROVIDE ( ets_post_rom = 0x40000e24 );
132137
PROVIDE ( ets_printf = 0x400024cc );
133138
PROVIDE ( ets_putc = 0x40002be8 );
134139
PROVIDE ( ets_rtc_int_register = 0x40002a40 );

0 commit comments

Comments
 (0)