Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[RFC] Hooks to support choosing image to boot in runtime #2161

Merged
merged 7 commits into from
Feb 13, 2025
Merged

[RFC] Hooks to support choosing image to boot in runtime #2161

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
57f882c
boot/zephyr: Fix SINGLE_APPLICATION_SLOT_RAM_LOAD file inclusion
edersondisouza Feb 4, 2025
a6f22e8
boot/bootutil: Get bootloader state
edersondisouza Dec 20, 2024
b04a37c
boot/bootutil: Make RAM loading methods public
edersondisouza Dec 20, 2024
ae5963b
boot: New boot_go hook
edersondisouza Dec 23, 2024
6d59eaa
boot: Add flash area ID/device ID retrieval hooks
edersondisouza Dec 20, 2024
b987727
samples: Add a sample for runtime chosen image
edersondisouza Aug 7, 2024
27c2202
samples/runtime-source: Make it buildable on twister
edersondisouza Jan 6, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .github/workflows/zephyr_build.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,7 @@ jobs:
-T ../bootloader/mcuboot/boot/zephyr
-T ./tests/subsys/dfu
-T ./samples/subsys/mgmt/mcumgr/smp_svr
-T ../bootloader/mcuboot/samples/runtime-source/zephyr/app
run: |
export ZEPHYR_BASE=${PWD}
export ZEPHYR_TOOLCHAIN_VARIANT=zephyr
Expand Down
98 changes: 90 additions & 8 deletions boot/bootutil/include/bootutil/boot_hooks.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,25 +34,65 @@
#ifndef H_BOOTUTIL_HOOKS
#define H_BOOTUTIL_HOOKS

#ifdef MCUBOOT_IMAGE_ACCESS_HOOKS
#include "bootutil/bootutil.h"
#include "bootutil/fault_injection_hardening.h"

#define BOOT_HOOK_CALL(f, ret_default, ...) f(__VA_ARGS__)
#define DO_HOOK_CALL(f, ret_default, ...) \
f(__VA_ARGS__)

#define BOOT_HOOK_CALL_FIH(f, fih_ret_default, fih_rc, ...) \
#define DO_HOOK_CALL_FIH(f, fih_ret_default, fih_rc, ...) \
do { \
FIH_CALL(f, fih_rc, __VA_ARGS__); \
} while(0);

#else

#define BOOT_HOOK_CALL(f, ret_default, ...) ret_default
#define HOOK_CALL_NOP(f, ret_default, ...) ret_default

#define BOOT_HOOK_CALL_FIH(f, fih_ret_default, fih_rc, ...) \
#define HOOK_CALL_FIH_NOP(f, fih_ret_default, fih_rc, ...) \
do { \
fih_rc = fih_ret_default; \
} while(0);

#endif
#ifdef MCUBOOT_IMAGE_ACCESS_HOOKS

#define BOOT_HOOK_CALL(f, ret_default, ...) \
DO_HOOK_CALL(f, ret_default, __VA_ARGS__)

#define BOOT_HOOK_CALL_FIH(f, fih_ret_default, fih_rc, ...) \
DO_HOOK_CALL_FIH(f, fih_ret_default, fih_rc, __VA_ARGS__)

#else

#define BOOT_HOOK_CALL(f, ret_default, ...) \
HOOK_CALL_NOP(f, ret_default, __VA_ARGS__)

#define BOOT_HOOK_CALL_FIH(f, fih_ret_default, fih_rc, ...) \
HOOK_CALL_FIH_NOP(f, fih_ret_default, fih_rc, __VA_ARGS__)

#endif /* MCUBOOT_IMAGE_ACCESS_HOOKS */

#ifdef MCUBOOT_BOOT_GO_HOOKS

#define BOOT_HOOK_GO_CALL_FIH(f, fih_ret_default, fih_rc, ...) \
DO_HOOK_CALL_FIH(f, fih_ret_default, fih_rc, __VA_ARGS__);

#else

#define BOOT_HOOK_GO_CALL_FIH(f, fih_ret_default, fih_rc, ...) \
HOOK_CALL_FIH_NOP(f, fih_ret_default, fih_rc, __VA_ARGS__)

#endif /* MCUBOOT_BOOT_GO_HOOKS */

#ifdef MCUBOOT_FLASH_AREA_HOOKS

#define BOOT_HOOK_FLASH_AREA_CALL(f, ret_default, ...) \
DO_HOOK_CALL(f, ret_default, __VA_ARGS__)

#else

#define BOOT_HOOK_FLASH_AREA_CALL(f, ret_default, ...) \
HOOK_CALL_NOP(f, ret_default, __VA_ARGS__)

#endif /* MCUBOOT_FLASH_AREA_ID_HOOKS */

/** Hook for provide image header data.
*
Expand Down Expand Up @@ -173,6 +213,48 @@ int boot_img_install_stat_hook(int image_index, int slot,
*/
int boot_reset_request_hook(bool force);

/**
* Hook to implement custom action before boot_go() function.
*
* @param rsp boot response structure.
*
* @retval FIH_SUCCESS: boot_go() should be skipped, boot response is already
* filled.
* FIH_FAILURE: boot_go() should be skipped, boot response is already
* filled with error.
* FIH_BOOT_HOOK_REGULAR: follow the normal execution path.
*/
fih_ret boot_go_hook(struct boot_rsp *rsp);

/**
* Hook to implement custom action before retrieving flash area ID.
*
* @param image_index the index of the image pair
* @param slot slot number
* @param area_id the flash area ID to be populated
*
* @retval 0 the flash area ID was fetched successfully;
* BOOT_HOOK_REGULAR follow the normal execution path to get the flash
* area ID;
* otherwise an error-code value.
*/
int flash_area_id_from_multi_image_slot_hook(int image_index, int slot,
int *area_id);

/**
* Hook to implement custom action before retrieving flash area device ID.
*
* @param fa the flash area structure
* @param device_id the device ID to be populated
*
* @retval 0 the device ID was fetched successfully;
* BOOT_HOOK_REGULAR follow the normal execution path to get the device
* ID;
* otherwise an error-code value.
*/
int flash_area_get_device_id_hook(const struct flash_area *fa,
uint8_t *device_id);

#define BOOT_RESET_REQUEST_HOOK_BUSY 1
#define BOOT_RESET_REQUEST_HOOK_TIMEOUT 2
#define BOOT_RESET_REQUEST_HOOK_CHECK_FAILED 3
Expand Down
2 changes: 1 addition & 1 deletion boot/bootutil/include/bootutil/bootutil.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,9 +86,9 @@ struct image_max_size {
fih_ret boot_go(struct boot_rsp *rsp);
fih_ret boot_go_for_image_id(struct boot_rsp *rsp, uint32_t image_id);

struct boot_loader_state;
void boot_state_clear(struct boot_loader_state *state);
fih_ret context_boot_go(struct boot_loader_state *state, struct boot_rsp *rsp);
struct boot_loader_state *boot_get_loader_state(void);
const struct image_max_size *boot_get_max_app_size(void);
uint32_t boot_get_state_secondary_offset(struct boot_loader_state *state,
const struct flash_area *fap);
Expand Down
37 changes: 37 additions & 0 deletions boot/bootutil/include/bootutil/bootutil_public.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,8 @@ _Static_assert(MCUBOOT_BOOT_MAX_ALIGN >= 8 && MCUBOOT_BOOT_MAX_ALIGN <= 32,
#endif
#endif

struct boot_loader_state;

struct boot_swap_state {
uint8_t magic; /* One of the BOOT_MAGIC_[...] values. */
uint8_t swap_type; /* One of the BOOT_SWAP_TYPE_[...] values. */
Expand Down Expand Up @@ -309,6 +311,41 @@ int
boot_image_load_header(const struct flash_area *fa_p,
struct image_header *hdr);

#ifdef MCUBOOT_RAM_LOAD
/**
* Loads image with given header to RAM.
*
* Destination on RAM and size is described on image header.
*
* @param[in] state boot loader state
* @param[in] hdr image header
*
* @return 0 on success, error code otherwise
*/
int boot_load_image_from_flash_to_sram(struct boot_loader_state *state,
struct image_header *hdr);

/**
* Removes an image from SRAM, by overwriting it with zeros.
*
* @param state Boot loader status information.
*
* @return 0 on success; nonzero on failure.
*/
int boot_remove_image_from_sram(struct boot_loader_state *state);

/**
* Removes an image from flash by erasing the corresponding flash area
*
* @param state Boot loader status information.
* @param slot The flash slot of the image to be erased.
*
* @return 0 on success; nonzero on failure.
*/
int boot_remove_image_from_flash(struct boot_loader_state *state,
uint32_t slot);
#endif

#ifdef __cplusplus
}
#endif
Expand Down
3 changes: 0 additions & 3 deletions boot/bootutil/src/bootutil_priv.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -503,9 +503,6 @@ struct bootsim_ram_info *bootsim_get_ram_info(void);
(size)), 0)

int boot_load_image_to_sram(struct boot_loader_state *state);
int boot_remove_image_from_sram(struct boot_loader_state *state);
int boot_remove_image_from_flash(struct boot_loader_state *state,
uint32_t slot);
#else
#define IMAGE_RAM_BASE ((uintptr_t)0)

Expand Down
5 changes: 5 additions & 0 deletions boot/bootutil/src/loader.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,11 @@ static struct sector_buffer_t sector_buffers;
boot_copy_region(state, fap_pri, fap_sec, pri_off, sec_off, sz)
#endif

struct boot_loader_state *boot_get_loader_state(void)
{
return &boot_data;
}
Comment on lines +121 to +124
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

actually after having a talk with David about other issues I don't think this will work, @d3zd3z

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm... why? My goal here is to have some state that can be prepared and passed to boot_load_image_to_sram without needing to expose struct boot_loader_state itself. I went with getting a static one instead of creating a new one to avoid allocation. Note that the one that matters to me is the boot_get_loader_state at boot/zephyr/single_loader.c - which is there just to be shared - instead of the one here at bootutil, which I only added for completeness.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The use of a static variable will prevent this code from being run in the simulation environment, where the code is invoked by multiple threads simultaneously.

Unfortunately I am not that fluent with the simulator, so unable to comment here.
@d3zd3z Do you find this to be a blocker? How should we deal with this, have you any insight?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So the issue with the statics is that we want the state to be static on target, but only gotten at the top level and passed down. This is effectively just moving this definition from the zephyr-specific code, although I question then why the declaration remains within the Zephyr code as well.

I'm not quite sure I understand why this gets added here as well. If it does get moved to loader.c, we should remove the one in the Zephyr-specific code. I'm not quite I follow it yet, but it almost looks at this point as if we now have two copies of the state, one in a zephyr-specific file, and now one here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

About having it on both zephyr/single_loader.c and loader.c, remember that only one of the files is included on build, so there's no moving happening here.

What I really need is a state on the zephyr/single_loader.c case (to be able to use RAM loading code), so there I both declare the static variable as well as the getter. On common loader.c, the static variable already exists, I just added the getter for consistency - if there's such a state, make it available.

One can argue that loader.c getter "consistency" is not needed, as there's no users, and I can just drop this bit.


static int
boot_read_image_headers(struct boot_loader_state *state, bool require_all,
struct boot_status *bs)
Expand Down
14 changes: 14 additions & 0 deletions boot/bootutil/src/ram_load.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -438,3 +438,17 @@ boot_remove_image_from_flash(struct boot_loader_state *state, uint32_t slot)

return rc;
}

int boot_load_image_from_flash_to_sram(struct boot_loader_state *state,
struct image_header *hdr)
{
int active_slot;

/* boot_load_image_to_sram will load the image from state active_slot,
* so force it before loading the image.
*/
active_slot = state->slot_usage[BOOT_CURR_IMG(state)].active_slot;
BOOT_IMG(state, active_slot).hdr = *hdr;

return boot_load_image_to_sram(state);
}
14 changes: 10 additions & 4 deletions boot/zephyr/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,12 @@

cmake_minimum_required(VERSION 3.13.1)

# This sample shows usage of an external module and we need to set the
# set the extra module path before calling find_package(Zephyr).
if(TEST_RUNTIME_SOURCE_HOOKS)
set(EXTRA_ZEPHYR_MODULES "${CMAKE_SOURCE_DIR}/../../samples/runtime-source/zephyr/hooks")
endif()

# find_package(Zephyr) in order to load application boilerplate:
# http://docs.zephyrproject.org/application/application.html
find_package(Zephyr REQUIRED HINTS $ENV{ZEPHYR_BASE})
Expand Down Expand Up @@ -125,15 +131,15 @@ zephyr_library_sources(
)
endif()

if(CONFIG_SINGLE_APPLICATION_SLOT)
if(CONFIG_SINGLE_APPLICATION_SLOT_RAM_LOAD)
zephyr_library_sources(
${BOOT_DIR}/zephyr/single_loader.c
${BOOT_DIR}/bootutil/src/ram_load.c
)
zephyr_library_include_directories(${BOOT_DIR}/bootutil/src)
elseif(CONFIG_SINGLE_APPLICATION_SLOT OR CONFIG_SINGLE_APPLICATION_SLOT_RAM_LOAD)
elseif(CONFIG_SINGLE_APPLICATION_SLOT)
zephyr_library_sources(
${BOOT_DIR}/zephyr/single_loader.c
${BOOT_DIR}/bootutil/src/ram_load.c
)
zephyr_library_include_directories(${BOOT_DIR}/bootutil/src)
elseif(CONFIG_BOOT_FIRMWARE_LOADER)
Expand Down Expand Up @@ -161,7 +167,7 @@ else()
${BOOT_DIR}/bootutil/src/swap_scratch.c
)

if(CONFIG_BOOT_RAM_LOAD OR CONFIG_SINGLE_APPLICATION_SLOT_RAM_LOAD)
if(CONFIG_BOOT_RAM_LOAD)
zephyr_library_sources(
${BOOT_DIR}/bootutil/src/ram_load.c
)
Expand Down
25 changes: 25 additions & 0 deletions boot/zephyr/Kconfig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -472,6 +472,17 @@ config BOOT_IMAGE_EXECUTABLE_RAM_SIZE
default $(dt_chosen_reg_size_int,$(DT_CHOSEN_Z_SRAM),0)
endif

config FLASH_RUNTIME_SOURCES
bool "Images are read from flash partitions defined at runtime"
depends on SINGLE_APPLICATION_SLOT
help
Instead of using information on the flash slots to decide which images
to load/update, the application provides the information from which
flash slot to load in runtime. This is useful when the application
reads the state for hardware straps or other sources to decide which
image to load. Usually, application will provide a boot_go_hook() to
decide which image to load.

config BOOT_ENCRYPTION_SUPPORT
bool
help
Expand Down Expand Up @@ -855,6 +866,20 @@ config BOOT_IMAGE_ACCESS_HOOKS
update. It is up to the project customization to add required source
files to the build.

config BOOT_GO_HOOKS
bool "Enable hooks for overriding MCUBOOT's boot_go routine"
help
Allow to provide procedures for override or extend native
MCUboot's boot_go routine. It is up to the project customization to
add required source files to the build.

config BOOT_FLASH_AREA_HOOKS
bool "Enable hooks for overriding MCUboot's flash area routines"
help
Allow to provide procedures for override or extend native
MCUboot's flash area routines. It is up to the project customization to
add required source files to the build.

config MCUBOOT_ACTION_HOOKS
bool "Enable hooks for responding to MCUboot status changes"
help
Expand Down
19 changes: 19 additions & 0 deletions boot/zephyr/flash_map_extended.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,9 @@
#include <flash_map_backend/flash_map_backend.h>
#include <sysflash/sysflash.h>

#include "bootutil/boot_hooks.h"
#include "bootutil/bootutil_log.h"
#include "bootutil/bootutil_public.h"

BOOT_LOG_MODULE_DECLARE(mcuboot);

Expand Down Expand Up @@ -58,6 +60,14 @@ int flash_device_base(uint8_t fd_id, uintptr_t *ret)
*/
int flash_area_id_from_multi_image_slot(int image_index, int slot)
{
int rc, id;

rc = BOOT_HOOK_FLASH_AREA_CALL(flash_area_id_from_multi_image_slot_hook,
BOOT_HOOK_REGULAR, image_index, slot, &id);
if (rc != BOOT_HOOK_REGULAR) {
return id;
}

switch (slot) {
case 0: return FLASH_AREA_IMAGE_PRIMARY(image_index);
#if !defined(CONFIG_SINGLE_APPLICATION_SLOT)
Expand Down Expand Up @@ -138,6 +148,15 @@ int flash_area_sector_from_off(off_t off, struct flash_sector *sector)

uint8_t flash_area_get_device_id(const struct flash_area *fa)
{
uint8_t device_id;
int rc;

rc = BOOT_HOOK_FLASH_AREA_CALL(flash_area_get_device_id_hook,
BOOT_HOOK_REGULAR, fa, &device_id);
if (rc != BOOT_HOOK_REGULAR) {
return device_id;
}

#if defined(CONFIG_ARM)
return fa->fa_id;
#else
Expand Down
8 changes: 8 additions & 0 deletions boot/zephyr/include/mcuboot_config/mcuboot_config.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -242,6 +242,14 @@
#define MCUBOOT_IMAGE_ACCESS_HOOKS
#endif

#ifdef CONFIG_BOOT_GO_HOOKS
#define MCUBOOT_BOOT_GO_HOOKS
#endif

#ifdef CONFIG_BOOT_FLASH_AREA_HOOKS
#define MCUBOOT_FLASH_AREA_HOOKS
#endif

#ifdef CONFIG_MCUBOOT_VERIFY_IMG_ADDRESS
#define MCUBOOT_VERIFY_IMG_ADDRESS
#endif
Expand Down
6 changes: 5 additions & 1 deletion boot/zephyr/main.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@
#include "bootutil/bootutil_log.h"
#include "bootutil/image.h"
#include "bootutil/bootutil.h"
#include "bootutil/boot_hooks.h"
#include "bootutil/fault_injection_hardening.h"
#include "bootutil/mcuboot_status.h"
#include "flash_map_backend/flash_map_backend.h"
Expand Down Expand Up @@ -525,7 +526,10 @@ int main(void)
#endif
#endif

FIH_CALL(boot_go, fih_rc, &rsp);
BOOT_HOOK_GO_CALL_FIH(boot_go_hook, FIH_BOOT_HOOK_REGULAR, fih_rc, &rsp);
if (FIH_EQ(fih_rc, FIH_BOOT_HOOK_REGULAR)) {
FIH_CALL(boot_go, fih_rc, &rsp);
}

#ifdef CONFIG_BOOT_SERIAL_BOOT_MODE
if (io_detect_boot_mode()) {
Expand Down
6 changes: 6 additions & 0 deletions boot/zephyr/sample.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,3 +96,9 @@ tests:
integration_platforms:
- nrf52840dk/nrf52840
tags: bootloader_mcuboot
sample.bootloader.mcuboot.runtime_source.hooks:
extra_args: EXTRA_CONF_FILE=../../samples/runtime-source/zephyr/sample.conf
TEST_RUNTIME_SOURCE_HOOKS=y
tags: bootloader_mcuboot runtime_source
platform_allow: frdm_k64f
build_only: true
Loading
Loading