Add new pico_flash library with flash_safe_execute(func) (#1412)

* Add new pico_flash library, with flash_safe_execute(func) method to help with preventing IRQs and other core accessing flash with pico_multicore or FreeRTOS SMP
This commit is contained in:
Graham Sanderson 2023-06-06 11:19:27 -05:00 committed by GitHub
parent 8188adf98b
commit f28bbfd4ec
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
12 changed files with 415 additions and 12 deletions

View File

@ -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

View File

@ -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__

View File

@ -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)

View File

@ -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
};

View File

@ -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

View File

@ -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);
}
}

View File

@ -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)

View File

@ -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;
}

View File

@ -0,0 +1,131 @@
/*
* Copyright (c) 2023 Raspberry Pi (Trading) Ltd.
*
* SPDX-License-Identifier: BSD-3-Clause
*/
#ifndef _PICO_FLASH_H
#define _PICO_FLASH_H
#include "pico.h"
#include "hardware/flash.h"
#include "pico/time.h"
/** \file pico/flash.h
* \defgroup pico_flash pico_flash
*
* High level flash API
*
* Flash cannot be erased or written to when in XIP mode. However the system cannot directly access memory in the flash
* address space when not in XIP mode.
*
* It is therefore critical that no code or data is being read from flash while flash is been written or erased.
*
* If only one core is being used, then the problem is simple - just disable interrupts; however if code is running on
* the other core, then it has to be asked, nicely, to avoid flash for a bit. This is hard to do if you don't have
* complete control of the code running on that core at all times.
*
* This library provides a \ref flash_safe_execute method which calls a function back having sucessfully gotten
* into a state where interrupts are disabled, and the other core is not executing or reading from flash.
*
* How it does this is dependent on the supported environment (Free RTOS SMP or pico_multicore). Additionally
* the user can provide their own mechanism by providing a strong definition of \ref get_flash_safety_helper().
*
* Using the default settings, flash_safe_execute will only call the callback function if the state is safe
* otherwise returning an error (or an assert depending on \ref PICO_FLASH_ASSERT_ON_UNSAFE).
*
* There are conditions where safety would not be guaranteed:
*
* 1. FreeRTOS smp with `configNUM_CORES=1` - FreeRTOS still uses pico_multicore in this case, so \ref flash_safe_execute
* cannot know what the other core is doing, and there is no way to force code execution between a FreeRTOS core
* and a non FreeRTOS core.
* 2. FreeRTOS non SMP with pico_multicore - Again, there is no way to force code execution between a FreeRTOS core and
* a non FreeRTOS core.
* 3. pico_multicore without \ref flash_safe_execute_core_init() having been called on the other core - The
* \ref flash_safe_execute method does not know if code is executing on the other core, so it has to assume it is. Either
* way, it is not able to intervene if \ref flash_safe_execute_core_init() has not been called on the other core.
*
* Fortunately, all is not lost in this situation, you may:
*
* * Set \ref PICO_FLASH_ASSUME_CORE0_SAFE=1 to explicitly say that core 0 is never using flash.
* * Set \ref PICO_FLASH_ASSUME_CORE1_SAFE=1 to explicitly say that core 1 is never using flash.
*/
#ifdef __cplusplus
extern "C" {
#endif
/**
* Initialize a core such that the other core can lock it out during \ref flash_safe_execute.
*
* \note This is not necessary for FreeRTOS SMP, but should be used when launching via \ref multicore_launch_core1
* \return true on success; there is no need to call \ref flash_safe_execute_core_deinit() on failure.
*/
bool flash_safe_execute_core_init(void);
/**
* De-initialize work done by \ref flash_safe_execute_core_init
* \return true on success
*/
bool flash_safe_execute_core_deinit(void);
/**
* Execute a function with IRQs disabled and with the other core also not executing/reading flash
*
* \param func the function to call
* \param param the parameter to pass to the function
* \param enter_exit_timeout_ms the timeout for each of the enter/exit phases when coordinating with the other core
*
* \return PICO_OK on success (the function will have been called).
* PICO_TIMEOUT on timeout (the function may have been called).
* PICO_ERROR_NOT_PERMITTED if safe execution is not possible (the function will not have been called).
* PICO_ERROR_INSUFFICIENT_RESOURCES if the method fails due to dynamic resource exhaustion (the function will not have been called)
* \note if \ref PICO_FLASH_ASSERT_ON_UNSAFE is 1, this function will assert in debug mode vs returning
* PICO_ERROR_NOT_PERMITTED
*/
int flash_safe_execute(void (*func)(void *), void *param, uint32_t enter_exit_timeout_ms);
// PICO_CONFIG: PICO_FLASH_ASSERT_ON_UNSAFE, Assert in debug mode rather than returning an error if flash_safe_execute cannot guarantee safety to catch bugs early, type=bool, default=1, group=pico_flash
#ifndef PICO_FLASH_ASSERT_ON_UNSAFE
#define PICO_FLASH_ASSERT_ON_UNSAFE 1
#endif
// PICO_CONFIG: PICO_FLASH_ASSUME_CORE0_SAFE, Assume that core 0 will never be accessing flash and so doesn't need to be considered during flash_safe_execute, type=bool, default=0, group=pico_flash
// PICO_CONFIG: PICO_FLASH_ASSUME_CORE1_SAFE, Assume that core 1 will never be accessing flash and so doesn't need to be considered during flash_safe_execute, type=bool, default=0, group=pico_flash
// PICO_CONFIG: PICO_FLASH_SAFE_EXECUTE_SUPPORT_FREERTOS_SMP, Support using FreeRTOS SMP to make the other core safe during flash_safe_execute, type=bool, default=1 when using FreeRTOS SMP, group=pico_flash
#ifndef PICO_FLASH_SAFE_EXECUTE_SUPPORT_FREERTOS_SMP
#if LIB_FREERTOS_KERNEL && FREE_RTOS_KERNEL_SMP // set by RP2040 SMP port
#define PICO_FLASH_SAFE_EXECUTE_SUPPORT_FREERTOS_SMP 1
#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
#endif
#endif
typedef struct {
bool (*core_init_deinit)(bool init);
int (*enter_safe_zone_timeout_ms)(uint32_t timeout_ms);
int (*exit_safe_zone_timeout_ms)(uint32_t timeout_ms);
} flash_safety_helper_t;
/**
* Internal method to return the flash safety helper implementation.
*
* Advanced users can provide their own implementation of this function to perform
* different inter-core coordination before disabling XIP mode.
*
* @return the \ref flash_safety_helper_t
*/
flash_safety_helper_t *get_flash_safety_helper(void);
#ifdef __cplusplus
}
#endif
#endif

View File

@ -196,7 +196,7 @@ static void check_lockout_mutex_init(void) {
hw_claim_unlock(save);
}
void multicore_lockout_victim_init() {
void multicore_lockout_victim_init(void) {
check_lockout_mutex_init();
uint core_num = get_core_num();
irq_set_exclusive_handler(SIO_IRQ_PROC0 + core_num, multicore_lockout_handler);
@ -246,7 +246,7 @@ bool multicore_lockout_start_timeout_us(uint64_t timeout_us) {
return multicore_lockout_start_block_until(make_timeout_time_us(timeout_us));
}
void multicore_lockout_start_blocking() {
void multicore_lockout_start_blocking(void) {
multicore_lockout_start_block_until(at_the_end_of_time);
}

View File

@ -29,6 +29,7 @@ target_link_libraries(kitchen_sink_libs INTERFACE
pico_divider
pico_double
pico_fix_rp2040_usb_device_enumeration
pico_flash
pico_float
pico_i2c_slave
pico_int64_ops

View File

@ -39,6 +39,7 @@
#include "pico/divider.h"
#include "pico/double.h"
#include "pico/fix/rp2040_usb_device_enumeration.h"
#include "pico/flash.h"
#include "pico/float.h"
#include "pico/int64_ops.h"
#include "pico/i2c_slave.h"