Skip to content

Commit e91da36

Browse files
committed
Fix issue 2454 flash safe execute lockout
The lockout state is controlled by a shared variable. The FIFO is used to begin a lockout and acknowledge it (i.e. multicore_lockout_handshake works as before) but the end of the lockout is now signalled by updating the shared variable. This ensures that timeouts are recognised reliably by the victim core. __wfe and __sev are used to signal updates to the shared variable in order to avoid polling.
1 parent 550aa09 commit e91da36

File tree

1 file changed

+33
-20
lines changed

1 file changed

+33
-20
lines changed

src/rp2_common/pico_multicore/multicore.c

Lines changed: 33 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -203,24 +203,26 @@ void multicore_launch_core1_raw(void (*entry)(void), uint32_t *sp, uint32_t vect
203203
}
204204

205205
#define LOCKOUT_MAGIC_START 0x73a8831eu
206-
#define LOCKOUT_MAGIC_END (~LOCKOUT_MAGIC_START)
207206

208207
static mutex_t lockout_mutex;
209-
static bool lockout_in_progress;
208+
static io_rw_32 lockout_request_id = LOCKOUT_MAGIC_START;
210209

211210
// note this method is in RAM because lockout is used when writing to flash
212211
// it only makes inline calls
213212
static void __isr __not_in_flash_func(multicore_lockout_handler)(void) {
214213
multicore_fifo_clear_irq();
215214
while (multicore_fifo_rvalid()) {
216-
if (sio_hw->fifo_rd == LOCKOUT_MAGIC_START) {
215+
uint32_t request_id = sio_hw->fifo_rd;
216+
if (request_id == lockout_request_id) {
217+
// valid lockout request received
217218
uint32_t save = save_and_disable_interrupts();
218-
multicore_fifo_push_blocking_inline(LOCKOUT_MAGIC_START);
219-
while (multicore_fifo_pop_blocking_inline() != LOCKOUT_MAGIC_END) {
220-
tight_loop_contents(); // not tight but endless potentially
219+
multicore_fifo_push_blocking_inline(request_id);
220+
// wait for the lockout to expire
221+
while (request_id == lockout_request_id) {
222+
// when lockout_request_id is updated, the other CPU core calls __sev
223+
__wfe();
221224
}
222225
restore_interrupts_from_disabled(save);
223-
multicore_fifo_push_blocking_inline(LOCKOUT_MAGIC_END);
224226
}
225227
}
226228
}
@@ -257,7 +259,7 @@ void multicore_lockout_victim_deinit(void) {
257259
}
258260
}
259261

260-
static bool multicore_lockout_handshake(uint32_t magic, absolute_time_t until) {
262+
static bool multicore_lockout_handshake(uint32_t request_id, absolute_time_t until) {
261263
uint irq_num = SIO_FIFO_IRQ_NUM(get_core_num());
262264
bool enabled = irq_is_enabled(irq_num);
263265
if (enabled) irq_set_enabled(irq_num, false);
@@ -267,7 +269,7 @@ static bool multicore_lockout_handshake(uint32_t magic, absolute_time_t until) {
267269
if (next_timeout_us < 0) {
268270
break;
269271
}
270-
multicore_fifo_push_timeout_us(magic, (uint64_t)next_timeout_us);
272+
multicore_fifo_push_timeout_us(request_id, (uint64_t)next_timeout_us);
271273
next_timeout_us = absolute_time_diff_us(get_absolute_time(), until);
272274
if (next_timeout_us < 0) {
273275
break;
@@ -276,22 +278,36 @@ static bool multicore_lockout_handshake(uint32_t magic, absolute_time_t until) {
276278
if (!multicore_fifo_pop_timeout_us((uint64_t)next_timeout_us, &word)) {
277279
break;
278280
}
279-
if (word == magic) {
281+
if (word == request_id) {
280282
rc = true;
281283
}
282284
} while (!rc);
283285
if (enabled) irq_set_enabled(irq_num, true);
284286
return rc;
285287
}
286288

289+
static uint32_t update_lockout_request_id() {
290+
// generate new number and then update shared variable
291+
uint32_t new_request_id = lockout_request_id + 1;
292+
lockout_request_id = new_request_id;
293+
// notify other core
294+
__sev();
295+
return new_request_id;
296+
}
297+
287298
static bool multicore_lockout_start_block_until(absolute_time_t until) {
288299
check_lockout_mutex_init();
289300
if (!mutex_enter_block_until(&lockout_mutex, until)) {
290301
return false;
291302
}
292-
hard_assert(!lockout_in_progress);
293-
bool rc = multicore_lockout_handshake(LOCKOUT_MAGIC_START, until);
294-
lockout_in_progress = rc;
303+
// generate a new request_id number
304+
uint32_t request_id = update_lockout_request_id();
305+
// attempt to lock out
306+
bool rc = multicore_lockout_handshake(request_id, until);
307+
if (!rc) {
308+
// lockout failed - cancel it
309+
update_lockout_request_id();
310+
}
295311
mutex_exit(&lockout_mutex);
296312
return rc;
297313
}
@@ -309,13 +325,10 @@ static bool multicore_lockout_end_block_until(absolute_time_t until) {
309325
if (!mutex_enter_block_until(&lockout_mutex, until)) {
310326
return false;
311327
}
312-
assert(lockout_in_progress);
313-
bool rc = multicore_lockout_handshake(LOCKOUT_MAGIC_END, until);
314-
if (rc) {
315-
lockout_in_progress = false;
316-
}
328+
// lockout finished - cancel it
329+
update_lockout_request_id();
317330
mutex_exit(&lockout_mutex);
318-
return rc;
331+
return true;
319332
}
320333

321334
bool multicore_lockout_end_timeout_us(uint64_t timeout_us) {
@@ -402,4 +415,4 @@ void multicore_doorbell_unclaim(uint doorbell_num, uint core_mask) {
402415
}
403416

404417

405-
#endif
418+
#endif

0 commit comments

Comments
 (0)