Add new pico_flash library with flash_safe_execute(func)

docs/index.h

@@ -42,8 +42,9 @@
  * set of functionality above the basic hardware interfaces
  * @{
  * \defgroup pico_async_context pico_async_context
- * \defgroup pico_multicore pico_multicore
+ * \defgroup pico_flash pico_flash
  * \defgroup pico_i2c_slave pico_i2c_slave
+ * \defgroup pico_multicore pico_multicore
  * \defgroup pico_rand pico_rand
  * \defgroup pico_stdlib pico_stdlib
  * \defgroup pico_sync pico_sync

src/common/pico_base/include/pico/error.h

@@ -24,6 +24,7 @@ enum pico_error_codes {
     PICO_ERROR_IO = -6,
     PICO_ERROR_BADAUTH = -7,
     PICO_ERROR_CONNECT_FAILED = -8,
+    PICO_ERROR_INSUFFICIENT_RESOURCES = -9,
 };
 
 #endif // !__ASSEMBLER__

src/rp2_common/CMakeLists.txt

@@ -45,6 +45,7 @@ if (NOT PICO_BARE_METAL)
     pico_add_subdirectory(pico_divider)
     pico_add_subdirectory(pico_double)
     pico_add_subdirectory(pico_int64_ops)
+    pico_add_subdirectory(pico_flash)
     pico_add_subdirectory(pico_float)
     pico_add_subdirectory(pico_mem_ops)
     pico_add_subdirectory(pico_malloc)

src/rp2_common/pico_async_context/include/pico/async_context_freertos.h

@@ -90,7 +90,7 @@ bool async_context_freertos_init(async_context_freertos_t *self, async_context_f
     async_context_freertos_config_t config = {
             .task_priority = ASYNC_CONTEXT_DEFAULT_FREERTOS_TASK_PRIORITY,
             .task_stack_size = ASYNC_CONTEXT_DEFAULT_FREERTOS_TASK_STACK_SIZE,
-#if configUSE_CORE_AFFINITY
+#if configUSE_CORE_AFFINITY && configNUM_CORES > 1
             .task_core_id = (UBaseType_t)-1, // none
 #endif
     };

src/rp2_common/pico_btstack/CMakeLists.txt

@@ -156,7 +156,7 @@ if (EXISTS ${PICO_BTSTACK_PATH}/${BTSTACK_TEST_PATH})
             ${CMAKE_CURRENT_LIST_DIR}/btstack_flash_bank.c
     )
     target_include_directories(pico_btstack_flash_bank_headers INTERFACE ${CMAKE_CURRENT_LIST_DIR}/include)
-    pico_mirrored_target_link_libraries(pico_btstack_flash_bank INTERFACE pico_btstack_base)
+    pico_mirrored_target_link_libraries(pico_btstack_flash_bank INTERFACE pico_btstack_base pico_flash)
 
     pico_add_library(pico_btstack_run_loop_async_context NOFLAG)
     target_sources(pico_btstack_run_loop_async_context INTERFACE

src/rp2_common/pico_btstack/btstack_flash_bank.c

@@ -5,7 +5,7 @@
  */
 
 #include "pico/btstack_flash_bank.h"
-#include "hardware/flash.h"
+#include "pico/flash.h"
 #include "hardware/sync.h"
 #include <string.h>
 
@@ -33,12 +33,32 @@ static uint32_t pico_flash_bank_get_alignment(void * context) {
     return 1;
 }
 
+typedef struct {
+    bool op_is_erase;
+    uintptr_t p0;
+    uintptr_t p1;
+} mutation_operation_t;
+
+static void pico_flash_bank_perform_flash_mutation_operation(void *param) {
+    const mutation_operation_t *mop = (const mutation_operation_t *)param;
+    if (mop->op_is_erase) {
+        flash_range_erase(mop->p0, PICO_FLASH_BANK_SIZE);
+    } else {
+        flash_range_program(mop->p0, (const uint8_t *)mop->p1, FLASH_PAGE_SIZE);
+    }
+}
+
 static void pico_flash_bank_erase(void * context, int bank) {
     (void)(context);
     DEBUG_PRINT("erase: bank %d\n", bank);
-    uint32_t status = save_and_disable_interrupts();
-    flash_range_erase(PICO_FLASH_BANK_STORAGE_OFFSET + (PICO_FLASH_BANK_SIZE * bank), PICO_FLASH_BANK_SIZE);
-    restore_interrupts(status);
+    mutation_operation_t mop = {
+            .op_is_erase = true,
+            .p0 = PICO_FLASH_BANK_STORAGE_OFFSET + (PICO_FLASH_BANK_SIZE * bank),
+    };
+    // todo choice of timeout and check return code... currently we have no way to return an error
+    //      to the caller anyway. flash_safe_execute asserts by default on problem other than timeout,
+    //      so that's fine for now, and UINT32_MAX is a timeout of 49 days which seems long enough
+    flash_safe_execute(pico_flash_bank_perform_flash_mutation_operation, &mop, UINT32_MAX);
 }
 
 static void pico_flash_bank_read(void *context, int bank, uint32_t offset, uint8_t *buffer, uint32_t size) {
@@ -118,9 +138,15 @@ static void pico_flash_bank_write(void * context, int bank, uint32_t offset, con
         offset = 0;
 
         // Now program the entire page
-        uint32_t status = save_and_disable_interrupts();
-        flash_range_program(bank_start_pos + (page * FLASH_PAGE_SIZE), page_data, FLASH_PAGE_SIZE);
-        restore_interrupts(status);
+        mutation_operation_t mop = {
+                .op_is_erase = false,
+                .p0 = bank_start_pos + (page * FLASH_PAGE_SIZE),
+                .p1 = (uintptr_t)page_data
+        };
+        // todo choice of timeout and check return code... currently we have no way to return an error
+        //      to the caller anyway. flash_safe_execute asserts by default on problem other than timeout,
+        //      so that's fine for now, and UINT32_MAX is a timeout of 49 days which seems long enough
+        flash_safe_execute(pico_flash_bank_perform_flash_mutation_operation, &mop, UINT32_MAX);
     }
 }
 

src/rp2_common/pico_flash/CMakeLists.txt

@@ -0,0 +1,12 @@
+pico_add_library(pico_flash)
+
+target_sources(pico_flash INTERFACE
+        ${CMAKE_CURRENT_LIST_DIR}/flash.c
+)
+
+target_include_directories(pico_flash_headers INTERFACE ${CMAKE_CURRENT_LIST_DIR}/include)
+
+# just include multicore headers, as we don't want to pull in the lib if it isn't pulled in already
+target_link_libraries(pico_flash INTERFACE pico_multicore_headers)
+
+pico_mirrored_target_link_libraries(pico_flash INTERFACE pico_time hardware_sync)

src/rp2_common/pico_flash/flash.c

@@ -0,0 +1,229 @@
+/*
+ * Copyright (c) 2023 Raspberry Pi (Trading) Ltd.
+ *
+ * SPDX-License-Identifier: BSD-3-Clause
+ */
+
+#include "pico/flash.h"
+#include "hardware/exception.h"
+#include "hardware/sync.h"
+#if PICO_FLASH_SAFE_EXECUTE_PICO_SUPPORT_MULTICORE_LOCKOUT
+#include "pico/multicore.h"
+#endif
+#if PICO_FLASH_SAFE_EXECUTE_SUPPORT_FREERTOS_SMP
+#include "FreeRTOS.h"
+#include "task.h"
+// now we have FreeRTOS header we can check core count... we can only use FreeRTOS SMP mechanism
+// with two cores
+#if configNUM_CORES == 2
+#if configUSE_CORE_AFFINITY
+#define PICO_FLASH_SAFE_EXECUTE_USE_FREERTOS_SMP 1
+#else
+#error configUSE_CORE_AFFINITY is required for PICO_FLASH_SAFE_EXECUTE_SUPPORT_FREERTOS_SMP
+#endif
+#endif
+#endif
+
+// There are multiple scenarios:
+//
+// 1. No use of core 1 - we just want to disable IRQs and not wait on core 1 to acquiesce
+// 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
+//    the equivalent PICO_FLASH_ASSUME_COREx_SAFE (i.e. the user mkaing sure the other core is fine)
+
+static bool default_core_init_deinit(bool init);
+static int default_enter_safe_zone_timeout_ms(uint32_t timeout_ms);
+static int default_exit_safe_zone_timeout_ms(uint32_t timeout_ms);
+
+// note the default methods are combined, rather than having a separate helper for
+// FreeRTOS, as we may support mixed multicore and non SMP FreeRTOS in the future
+
+static flash_safety_helper_t default_flash_safety_helper = {
+        .core_init_deinit = default_core_init_deinit,
+        .enter_safe_zone_timeout_ms = default_enter_safe_zone_timeout_ms,
+        .exit_safe_zone_timeout_ms = default_exit_safe_zone_timeout_ms
+};
+
+#if PICO_FLASH_SAFE_EXECUTE_PICO_SUPPORT_MULTICORE_LOCKOUT
+// note that these are not reset by core reset, however for now, I think people resetting cores
+// and then doing this again without re-initializing pico_flash for that core, is probably
+// something we can live with breaking.
+static bool core_initialized[NUM_CORES];
+#endif
+
+#if PICO_FLASH_SAFE_EXECUTE_USE_FREERTOS_SMP
+enum {
+    FREERTOS_LOCKOUT_NONE = 0,
+    FREERTOS_LOCKOUT_LOCKER_WAITING,
+    FREERTOS_LOCKOUT_LOCKEE_READY,
+    FREERTOS_LOCKOUT_LOCKER_DONE,
+    FREERTOS_LOCKOUT_LOCKEE_DONE,
+};
+// state for the lockout operation launched from the corresponding core
+static volatile uint8_t lockout_state[NUM_CORES];
+#endif
+
+__attribute__((weak)) flash_safety_helper_t *get_flash_safety_helper(void) {
+    return &default_flash_safety_helper;
+}
+
+bool flash_safe_execute_core_init(void) {
+    flash_safety_helper_t *helper = get_flash_safety_helper();
+    return helper ? helper->core_init_deinit(true) : false;
+}
+
+bool flash_safe_execute_core_deinit(void) {
+    flash_safety_helper_t *helper = get_flash_safety_helper();
+    return helper ? helper->core_init_deinit(false) : false;
+}
+
+int flash_safe_execute(void (*func)(void *), void *param, uint32_t enter_exit_timeout_ms) {
+    flash_safety_helper_t *helper = get_flash_safety_helper();
+    if (!helper) return PICO_ERROR_NOT_PERMITTED;
+    int rc = helper->enter_safe_zone_timeout_ms(enter_exit_timeout_ms);
+    if (!rc) {
+        func(param);
+        rc = helper->exit_safe_zone_timeout_ms(enter_exit_timeout_ms);
+    }
+    return rc;
+}
+
+static bool default_core_init_deinit(__unused bool init) {
+#if PICO_FLASH_ASSUME_CORE0_SAFE
+    if (!get_core_num()) return true;
+#endif
+#if PICO_FLASH_ASSUME_CORE1_SAFE
+    if (get_core_num()) return true;
+#endif
+#if PICO_FLASH_SAFE_EXECUTE_USE_FREERTOS_SMP
+    return true;
+#endif
+#if PICO_FLASH_SAFE_EXECUTE_PICO_SUPPORT_MULTICORE_LOCKOUT
+    if (!init) {
+        return false;
+    }
+    multicore_lockout_victim_init();
+    core_initialized[get_core_num()] = init;
+#endif
+    return true;
+}
+
+// irq_state for the lockout operation launched from the corresponding core
+static uint32_t irq_state[NUM_CORES];
+
+static bool use_irq_only(void) {
+#if PICO_FLASH_ASSUME_CORE0_SAFE
+    if (get_core_num()) return true;
+#endif
+#if PICO_FLASH_ASSUME_CORE1_SAFE
+    if (!get_core_num()) return true;
+#endif
+    return false;
+}
+
+#if PICO_FLASH_SAFE_EXECUTE_USE_FREERTOS_SMP
+static void __not_in_flash_func(flash_lockout_task)(__unused void *vother_core_num) {
+    uint other_core_num = (uintptr_t)vother_core_num;
+    while (lockout_state[other_core_num] != FREERTOS_LOCKOUT_LOCKER_WAITING) {
+        __wfe(); // we don't bother to try to let lower priority tasks run
+    }
+    uint32_t save = save_and_disable_interrupts();
+    lockout_state[other_core_num] = FREERTOS_LOCKOUT_LOCKEE_READY;
+    __sev();
+    while (lockout_state[other_core_num] == FREERTOS_LOCKOUT_LOCKEE_READY) {
+        __wfe(); // we don't bother to try to let lower priority tasks run
+    }
+    restore_interrupts(save);
+    lockout_state[other_core_num] = FREERTOS_LOCKOUT_LOCKEE_DONE;
+    __sev();
+    // bye bye
+    vTaskDelete(NULL);
+}
+#endif
+
+static int default_enter_safe_zone_timeout_ms(__unused uint32_t timeout_ms) {
+    int rc = PICO_OK;
+    if (!use_irq_only()) {
+#if PICO_FLASH_SAFE_EXECUTE_USE_FREERTOS_SMP
+        // Note that whilst taskENTER_CRITICAL sounds promising (and on non SMP it disabled IRQs), on SMP
+        // it only prevents the other core from also entering a critical section.
+        // Therefore, we must do our own handshake which starts a task on the other core and have it disable interrupts
+        uint core_num = get_core_num();
+        // create at low priority
+        TaskHandle_t task_handle;
+        if (pdPASS != xTaskCreate(flash_lockout_task, "flash lockout", configMINIMAL_STACK_SIZE, (void *)core_num, 0, &task_handle)) {
+            return PICO_ERROR_INSUFFICIENT_RESOURCES;
+        }
+        lockout_state[core_num] = FREERTOS_LOCKOUT_LOCKER_WAITING;
+        __sev();
+        // bind to other core
+        vTaskCoreAffinitySet(task_handle, 1u << (core_num ^ 1));
+        // and make it super high priority
+        vTaskPrioritySet(task_handle, configMAX_PRIORITIES -1);
+        absolute_time_t until = make_timeout_time_ms(timeout_ms);
+        while (lockout_state[core_num] != FREERTOS_LOCKOUT_LOCKEE_READY && !time_reached(until)) {
+            __wfe(); // we don't bother to try to let lower priority tasks run
+        }
+        if (lockout_state[core_num] != FREERTOS_LOCKOUT_LOCKEE_READY) {
+            lockout_state[core_num] = FREERTOS_LOCKOUT_LOCKER_DONE;
+            rc = PICO_ERROR_TIMEOUT;
+        }
+        // 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
+#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 (core_initialized[get_core_num()^1]) {
+            if (!multicore_lockout_start_timeout_us(timeout_ms * 1000ull)) {
+                rc = PICO_ERROR_TIMEOUT;
+            }
+        } else {
+#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;
+        }
+#endif // !LIB_FREERTOS_KERNEL
+#else
+        // no support for making other core safe provided, so fall through to irq
+        // note this is the case for a regular single core program
+#endif
+    }
+    if (rc == PICO_OK) {
+        // we always want to disable IRQs on our core
+        irq_state[get_core_num()] = save_and_disable_interrupts();
+    }
+    return rc;
+}
+
+static int default_exit_safe_zone_timeout_ms(__unused uint32_t timeout_ms) {
+    // assume if we're exiting we're called then entry happened successfully
+    restore_interrupts(irq_state[get_core_num()]);
+    if (!use_irq_only()) {
+#if PICO_FLASH_SAFE_EXECUTE_USE_FREERTOS_SMP
+        uint core_num = get_core_num();
+        lockout_state[core_num] = FREERTOS_LOCKOUT_LOCKER_DONE;
+        __sev();
+        absolute_time_t until = make_timeout_time_ms(timeout_ms);
+        while (lockout_state[core_num] != FREERTOS_LOCKOUT_LOCKEE_DONE && !time_reached(until)) {
+            __wfe(); // we don't bother to try to let lower priority tasks run
+        }
+        if (lockout_state[core_num] != FREERTOS_LOCKOUT_LOCKEE_DONE) {
+            return PICO_ERROR_TIMEOUT;
+        }
+#elif PICO_FLASH_SAFE_EXECUTE_PICO_SUPPORT_MULTICORE_LOCKOUT
+        return multicore_lockout_end_timeout_us(timeout_ms * 1000ull) ? PICO_OK : PICO_ERROR_TIMEOUT;
+#endif
+    }
+    return PICO_OK;
+}