Add PICO_MULTICORE_LOCKOUT_BEFORE_CORE1_STARTED (defaults to 1) to allow mutlicore_lockout to work before core 1 is started (#3019)
* Add PICO_MULTICORE_LOCKOUT_BEFORE_CORE1_STARTED (defaults to 1) to allow mutlicore_lockout to work before core 1 is started
* Add multicore_lockout_ready() and add a pico_minimize_runtime to turn off new support to save the extra bit of code needed
* add missing (void)
* comment change
* Fix issues with pico_minimize_runtime
CRT0_FAR_CALLS and MULTICORE_LOCKOUT_BEFORE_CORE1_STARTED hadn't been implemented correctly (so were always excluded), and THREAD_LOCAL was missing from docs
* add another missing (void)
* Apply suggestions from code review
Co-authored-by: Andrew Scheller <lurch@durge.org>
* comment change
* fix typo
Co-authored-by: Andrew Scheller <andrew.scheller@raspberrypi.com>
---------
Co-authored-by: William Vinnicombe <william.vinnicombe@raspberrypi.com>
Co-authored-by: Andrew Scheller <lurch@durge.org>
Co-authored-by: will-v-pi <108662275+will-v-pi@users.noreply.github.com>
Co-authored-by: Andrew Scheller <andrew.scheller@raspberrypi.com>
diff --git a/src/rp2_common/pico_flash/flash.c b/src/rp2_common/pico_flash/flash.c
index 1dc68c7..efbd023 100644
--- a/src/rp2_common/pico_flash/flash.c
+++ b/src/rp2_common/pico_flash/flash.c
@@ -6,7 +6,7 @@
#include "pico/flash.h"
#include "hardware/sync.h"
-#if PICO_FLASH_SAFE_EXECUTE_PICO_SUPPORT_MULTICORE_LOCKOUT
+#if PICO_FLASH_SAFE_EXECUTE_SUPPORT_MULTICORE_LOCKOUT
#include "pico/multicore.h"
#endif
#include "pico/time.h"
@@ -30,7 +30,7 @@
// 2. Regular pico_multicore - we need to use multicore lockout.
// 3. FreeRTOS on core 0, no use of core 1 - we just want to disable IRQs
// 4. FreeRTOS SMP on both cores - we need to schedule a high priority task on the other core to disable IRQs.
-// 5. FreeRTOS on one core, but application is using the other core. ** WE CANNOT SUPPORT THIS TODAY ** without
+// 5. FreeRTOS on one core, but application is using the other core. ** WE CANNOT SUPPORT THIS TODAY on RP2040 ** without
// the equivalent PICO_FLASH_ASSUME_COREx_SAFE (i.e. the user making sure the other core is fine)
static bool default_core_init_deinit(bool init);
@@ -93,7 +93,7 @@
#if PICO_FLASH_SAFE_EXECUTE_USE_FREERTOS_SMP
return true;
#endif
-#if PICO_FLASH_SAFE_EXECUTE_PICO_SUPPORT_MULTICORE_LOCKOUT
+#if PICO_FLASH_SAFE_EXECUTE_SUPPORT_MULTICORE_LOCKOUT
if (!init) {
return false;
}
@@ -171,17 +171,17 @@
}
// todo we may get preempted here, but I think that is OK unless what is pre-empts requires
// the other core to be running.
-#elif PICO_FLASH_SAFE_EXECUTE_PICO_SUPPORT_MULTICORE_LOCKOUT
- // we cannot mix multicore_lockout and FreeRTOS as they both use the multicore FIFO...
- // the user, will have to roll their own mechanism in this case.
-#if LIB_FREERTOS_KERNEL
+#elif PICO_FLASH_SAFE_EXECUTE_SUPPORT_MULTICORE_LOCKOUT
+ // we cannot mix multicore_lockout and FreeRTOS on RP2040 as they both use the multicore FIFO...
+ // the user will have to roll their own mechanism in this case. FreeRTOS on RP2350 uses doorbells
+#if LIB_FREERTOS_KERNEL && PICO_RP2040
#if PICO_FLASH_ASSERT_ON_UNSAFE
assert(false); // we expect the other core to have been initialized via flash_safe_execute_core_init()
// unless PICO_FLASH_ASSUME_COREX_SAFE is set
#endif
rc = PICO_ERROR_NOT_PERMITTED;
#else // !LIB_FREERTOS_KERNEL
- if (multicore_lockout_victim_is_initialized(get_core_num()^1)) {
+ if (multicore_lockout_ready()) {
if (!multicore_lockout_start_timeout_us(timeout_ms * 1000ull)) {
rc = PICO_ERROR_TIMEOUT;
}
@@ -220,7 +220,7 @@
if (lockout_state[core_num] != FREERTOS_LOCKOUT_LOCKEE_DONE) {
return PICO_ERROR_TIMEOUT;
}
-#elif PICO_FLASH_SAFE_EXECUTE_PICO_SUPPORT_MULTICORE_LOCKOUT
+#elif PICO_FLASH_SAFE_EXECUTE_SUPPORT_MULTICORE_LOCKOUT
return multicore_lockout_end_timeout_us(timeout_ms * 1000ull) ? PICO_OK : PICO_ERROR_TIMEOUT;
#endif
}
diff --git a/src/rp2_common/pico_flash/include/pico/flash.h b/src/rp2_common/pico_flash/include/pico/flash.h
index fd43946..1d23ad8 100644
--- a/src/rp2_common/pico_flash/include/pico/flash.h
+++ b/src/rp2_common/pico_flash/include/pico/flash.h
@@ -108,10 +108,13 @@
#endif
#endif
-// PICO_CONFIG: PICO_FLASH_SAFE_EXECUTE_PICO_SUPPORT_MULTICORE_LOCKOUT, Support using multicore_lockout functions to make the other core safe during flash_safe_execute, type=bool, default=1 when using pico_multicore, group=pico_flash
-#ifndef PICO_FLASH_SAFE_EXECUTE_PICO_SUPPORT_MULTICORE_LOCKOUT
-#if LIB_PICO_MULTICORE
-#define PICO_FLASH_SAFE_EXECUTE_PICO_SUPPORT_MULTICORE_LOCKOUT 1
+// PICO_CONFIG: PICO_FLASH_SAFE_EXECUTE_SUPPORT_MULTICORE_LOCKOUT, Support using multicore_lockout functions to make the other core safe during flash_safe_execute, type=bool, default=1 when using pico_multicore, group=pico_flash
+#ifndef PICO_FLASH_SAFE_EXECUTE_SUPPORT_MULTICORE_LOCKOUT
+#ifdef PICO_FLASH_SAFE_EXECUTE_PICO_SUPPORT_MULTICORE_LOCKOUT
+// support old misspelling
+#define PICO_FLASH_SAFE_EXECUTE_SUPPORT_MULTICORE_LOCKOUT PICO_FLASH_SAFE_EXECUTE_PICO_SUPPORT_MULTICORE_LOCKOUT
+#elif LIB_PICO_MULTICORE
+#define PICO_FLASH_SAFE_EXECUTE_SUPPORT_MULTICORE_LOCKOUT 1
#endif
#endif
diff --git a/src/rp2_common/pico_multicore/include/pico/multicore.h b/src/rp2_common/pico_multicore/include/pico/multicore.h
index 6c997cb..9fc7f5d 100644
--- a/src/rp2_common/pico_multicore/include/pico/multicore.h
+++ b/src/rp2_common/pico_multicore/include/pico/multicore.h
@@ -38,6 +38,11 @@
#endif
#endif
+// PICO_CONFIG: PICO_MULTICORE_LOCKOUT_BEFORE_CORE1_STARTED, Allow multicore_lockout functions called from core 0 to succeed if core1 has not been started, type=bool, default=1, group=pico_multicore
+#ifndef PICO_MULTICORE_LOCKOUT_BEFORE_CORE1_STARTED
+#define PICO_MULTICORE_LOCKOUT_BEFORE_CORE1_STARTED 1
+#endif
+
/**
* \def SIO_FIFO_IRQ_NUM(core)
* \ingroup pico_multicore
@@ -424,7 +429,7 @@
* system that is not sufficient, and unless the other core is polling in some way, then it will need to be interrupted
* in order to cooperatively enter a blocked state.
*
- * These "lockout" functions use the inter core FIFOs to cause an interrupt on one core from the other, and manage
+ * These "lockout" functions use the inter-core FIFOs to cause an interrupt on one core from the other, and manage
* waiting for the other core to enter the "locked out" state.
*
* The usage is that the "victim" core ... i.e the core that can be "locked out" by the other core calls
@@ -440,6 +445,12 @@
* \ref multicore_lockout_end_timeout_us to release the lockout.
*
* \note Because multicore lockout uses the intercore FIFOs, the FIFOs <b>cannot</b> be used for any other purpose
+ *
+ * \note By default, for convenience, multicore_lockout_start_ functions will succeed on core 0, if core 1 has either not been started
+ * via multicore_launch_core1 functions, or has subsequently been reset via multicore_reset_core1. Therefore, it is not safe to
+ * (though equally not very likely that you would) call multicore_launch_core1 while core 0 is inside of a multicore_lockout_ function.
+ * This default behavior can be disabled by setting PICO_MULTICORE_LOCKOUT_BEFORE_CORE1_STARTED=0 in which case core 1 must be running
+ * and in the "victim initialized" state before multicore_lockout_start functions can be called on core 0
*/
/*! \brief Initialize the current core such that it can be a "victim" of lockout (i.e. forced to pause in a known state by the other core)
@@ -459,8 +470,8 @@
/*! \brief Determine if \ref multicore_lockout_victim_init() has been called on the specified core.
* \ingroup multicore_lockout
*
- * \note this state persists even if the core is subsequently reset; therefore you are advised to
- * always call \ref multicore_lockout_victim_init() again after resetting a core, which had previously
+ * \note this state persists even if the core is subsequently reset (other than via reset via pico_multicore_reset_core1);
+ * therefore you are advised to always call \ref multicore_lockout_victim_init() again after resetting a core, which had previously
* been initialized.
*
* \param core_num the core number (0 or 1)
@@ -468,6 +479,18 @@
*/
bool multicore_lockout_victim_is_initialized(uint core_num);
+/*! \brief Determine whether it is safe to call multicore_lockout_start functions from this core.
+ * \ingroup multicore_lockout
+ *
+ * \return true if \ref multicore_lockout_start_blocking() and \ref multicore_lockout_start_timeout_us() may safely be called from this core
+ *
+ * \note that when PICO_MULTICORE_LOCKOUT_BEFORE_CORE1_STARTED=1 this returns true when called from core 0 if core 1 has
+ * not been launched via a multicore_launch_core1 function, or has since been reset via \ref multicore_reset_core1. Otherwise, it returns
+ * the same value as `multicore_lockout_victim_is_initialized(other_core)`. This behavior is intended to make it easier
+ * for applications which may want to perform operations on core 0, but may or may not yet have launched core 1.
+ */
+bool multicore_lockout_ready(void);
+
/*! \brief Request the other core to pause in a known state and wait for it to do so
* \ingroup multicore_lockout
*
diff --git a/src/rp2_common/pico_multicore/multicore.c b/src/rp2_common/pico_multicore/multicore.c
index ef9a9b0..759d31e 100644
--- a/src/rp2_common/pico_multicore/multicore.c
+++ b/src/rp2_common/pico_multicore/multicore.c
@@ -28,7 +28,19 @@
// and is a no-op if set. We DO have a new `multicore_lockout_victim_deinit()` method, which can be called in a pinch after
// the reset before calling `multicore_lockout_victim_init()` again, so that is good. We will reset the flag
// for core1 in `multicore_reset_core1()` though as a convenience since most people will use that to reset core 1.
-static bool lockout_victim_initialized[NUM_CORES];
+
+#define CORE_STATUS_NOT_RUNNING ((uint8_t)0)
+#if PICO_MULTICORE_LOCKOUT_BEFORE_CORE1_STARTED
+#define CORE_STATUS_LOCKOUT_DISABLED ((uint8_t)1)
+#define CORE_STATUS_LOCKOUT_ENABLED ((uint8_t)2)
+#else
+// we don't care about the distinction between NOT_RUNNING & LOCKOUT_DISABLED
+// when PICO_MULTICORE_LOCKOUT_BEFORE_CORE1_STARTED == 0
+#define CORE_STATUS_LOCKOUT_DISABLED CORE_STATUS_NOT_RUNNING
+#define CORE_STATUS_LOCKOUT_ENABLED ((uint8_t)1)
+#endif
+
+static uint8_t core_status[NUM_CORES];
void multicore_fifo_push_blocking(uint32_t data) {
multicore_fifo_push_blocking_inline(data);
@@ -123,7 +135,7 @@
irq_set_enabled(irq_num, false);
// Core 1 will be in un-initialized state
- lockout_victim_initialized[1] = false;
+ core_status[1] = CORE_STATUS_NOT_RUNNING;
// Bring core 1 back out of reset. It will drain its own mailbox FIFO, then push
// a 0 to our mailbox to tell us it has done this.
@@ -176,7 +188,7 @@
void multicore_launch_core1_raw(void (*entry)(void), uint32_t *sp, uint32_t vector_table) {
// Allow for the fact that the caller may have already enabled the FIFO IRQ for their
// own purposes (expecting FIFO content after core 1 is launched). We must disable
- // the IRQ during the handshake, then restore afterwards.
+ // the IRQ during the handshake, then restore afterward.
uint irq_num = SIO_FIFO_IRQ_NUM(0);
bool enabled = irq_is_enabled(irq_num);
irq_set_enabled(irq_num, false);
@@ -189,6 +201,9 @@
const uint32_t cmd_sequence[] =
{0, 0, 1, (uintptr_t) vector_table, (uintptr_t) sp, (uintptr_t) entry};
+#if PICO_MULTICORE_LOCKOUT_BEFORE_CORE1_STARTED
+ core_status[1] = CORE_STATUS_LOCKOUT_DISABLED; // we'll assume up front
+#endif
uint seq = 0;
do {
uint cmd = cmd_sequence[seq];
@@ -249,23 +264,31 @@
uint fifo_irq_this_core = SIO_FIFO_IRQ_NUM(core_num);
irq_set_exclusive_handler(fifo_irq_this_core, multicore_lockout_handler);
irq_set_enabled(fifo_irq_this_core, true);
- lockout_victim_initialized[core_num] = true;
+ core_status[core_num] = CORE_STATUS_LOCKOUT_ENABLED;
}
void multicore_lockout_victim_deinit(void) {
uint core_num = get_core_num();
- if (lockout_victim_initialized[core_num]) {
+ if (core_status[core_num] == CORE_STATUS_LOCKOUT_ENABLED) {
// On platforms other than RP2040, these are actually the same IRQ number
// (each core only sees its own IRQ, always at the same IRQ number).
uint fifo_irq_this_core = SIO_FIFO_IRQ_NUM(core_num);
irq_remove_handler(fifo_irq_this_core, multicore_lockout_handler);
irq_set_enabled(fifo_irq_this_core, false);
- lockout_victim_initialized[core_num] = false;
+ core_status[core_num] = CORE_STATUS_LOCKOUT_DISABLED;
}
}
static bool multicore_lockout_handshake(uint32_t request_id, absolute_time_t until) {
- uint irq_num = SIO_FIFO_IRQ_NUM(get_core_num());
+ uint core_num = get_core_num();
+#if PICO_MULTICORE_LOCKOUT_BEFORE_CORE1_STARTED
+ if (!core_num && core_status[1] == CORE_STATUS_NOT_RUNNING) {
+ return true;
+ }
+#else
+ ((void)core_num); // SIO_FIFO_IRQ_NUM doesn't necessarily reference it
+#endif
+ uint irq_num = SIO_FIFO_IRQ_NUM(core_num);
bool enabled = irq_is_enabled(irq_num);
if (enabled) irq_set_enabled(irq_num, false);
bool rc = false;
@@ -345,7 +368,17 @@
}
bool multicore_lockout_victim_is_initialized(uint core_num) {
- return lockout_victim_initialized[core_num];
+ return core_status[core_num] == CORE_STATUS_LOCKOUT_ENABLED;
+}
+
+bool multicore_lockout_ready(void) {
+ uint core_num = get_core_num();
+#if PICO_MULTICORE_LOCKOUT_BEFORE_CORE1_STARTED
+ if (!core_num && core_status[1] == CORE_STATUS_NOT_RUNNING) {
+ return true;
+ }
+#endif
+ return multicore_lockout_victim_is_initialized(core_num ^ 1);
}
#if NUM_DOORBELLS
diff --git a/src/rp2_common/pico_runtime/CMakeLists.txt b/src/rp2_common/pico_runtime/CMakeLists.txt
index 07adbab..cdbe0cd 100644
--- a/src/rp2_common/pico_runtime/CMakeLists.txt
+++ b/src/rp2_common/pico_runtime/CMakeLists.txt
@@ -63,6 +63,8 @@
# PANIC - default panic impl which brings in stdio;
# AUTO_INIT_MUTEX - auto init mutexes, without this you get no printf mutex either;
# CRT0_FAR_CALLS - use blx not bl for calls from crt0 to user overridable functions;
+# THREAD_LOCAL - support for thread locals;
+# MULTICORE_LOCKOUT_BEFORE_CORE1_STARTED - allow multicore_lockout_start to be called on core 0 before core 1 is started
#
# \param\ INCLUDE The items to include
# \param\ EXCLUDE The items to exclude
@@ -75,7 +77,9 @@
FPGA_CHECK
PANIC
AUTO_INIT_MUTEX
- THREAD_LOCAL)
+ CRT0_FAR_CALLS
+ THREAD_LOCAL
+ MULTICORE_LOCKOUT_BEFORE_CORE1_STARTED)
cmake_parse_arguments(RUNTIME "" ""
"INCLUDE;EXCLUDE" ${ARGN} )
foreach (INCL_EXCL IN ITEMS INCLUDE EXCLUDE)
@@ -147,7 +151,10 @@
if (NOT RUNTIME_INCLUDE_FPGA_CHECK)
target_compile_definitions(${TARGET} PRIVATE PICO_NO_FPGA_CHECK=1)
endif()
- if (NOT RUNTIME_CRT0_FAR_CALLS)
+ if (NOT RUNTIME_INCLUDE_CRT0_FAR_CALLS)
target_compile_definitions(${TARGET} PRIVATE PICO_CRT0_NEAR_CALLS=1)
endif()
+ if (NOT RUNTIME_INCLUDE_MULTICORE_LOCKOUT_BEFORE_CORE1_STARTED)
+ target_compile_definitions(${TARGET} PRIVATE PICO_MULTICORE_LOCKOUT_BEFORE_CORE1_STARTED=0)
+ endif()
endfunction()
