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.

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

Add support for RAMLOAD mode with revert #2197

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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 boot/bootutil/include/bootutil/boot_status.h
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,7 @@ extern "C" {
#define BLINFO_MAX_APPLICATION_SIZE_IMAGE_2 0x07
#define BLINFO_MAX_APPLICATION_SIZE_IMAGE_3 0x08
#define BLINFO_MAX_APPLICATION_SIZE_IMAGE_4 0x09
#define BLINFO_FLASH_AREA 0x0A
Copy link
Collaborator

Choose a reason for hiding this comment

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

Isn't this already done vis BLINFI_RUNNING_SLOT?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

RUNNING_SLOT gives the index of the active slot, but for the MCUBoot subsystem code in Zephyr we need access to the flash area ID. For example, Slot 0 on the RT1050 has flash area ID 1 (because the MCUBoot partition has area ID 0)

We could change the value set by RUNNING_SLOT, but I wanted to avoid breaking any APIs so I added this. It was needed in order to get features like confirming a running image working with MCUBoot

Copy link
Collaborator

Choose a reason for hiding this comment

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

You have to convert from running slot to flash area ID in your application itself, as img mgmt does e.g. https://github.com/zephyrproject-rtos/zephyr/blob/main/subsys/mgmt/mcumgr/grp/img_mgmt/src/zephyr_img_mgmt.c#L128 the flash area ID has no meaning between different applications e.g. application could have bootloader slot split into 2 in future, now you open the wrong thing

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ahh, ok. I was testing this with the DFU subsystem- so would it make sense to call img_mgmt_flash_area_id here in boot_fetch_active_slot: https://github.com/zephyrproject-rtos/zephyr/pull/85254/files#diff-696b0049cfb981ee66f0191ccaf755f25a632c35b86da7dc1d739c3de8a60ed3L78? Otherwise, functions like marking the image as pending weren't working, since boot_fetch_active_slot is really fetching the flash area ID of the active slot, not the slot number.

Copy link
Collaborator

Choose a reason for hiding this comment

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

That returns either:

#define BOOT_PRIMARY_SLOT               0
#define BOOT_SECONDARY_SLOT             1

Which should then be used to check if it's slot0 if 0 or slot1 if 1 (it doesn't care about additional images since they are not "running"),


enum mcuboot_mode {
MCUBOOT_MODE_SINGLE_SLOT,
Expand Down
6 changes: 6 additions & 0 deletions boot/bootutil/src/boot_record.c
Original file line number Diff line number Diff line change
Expand Up @@ -330,6 +330,12 @@ int boot_save_shared_data(const struct image_header *hdr, const struct flash_are
BLINFO_RUNNING_SLOT,
sizeof(slot), (void *)&slot);
}

if (!rc) {
rc = boot_add_data_to_shared_area(TLV_MAJOR_BLINFO,
BLINFO_FLASH_AREA,
sizeof(fap->fa_id), (void *)&fap->fa_id);
}
Comment on lines +334 to +338
Copy link
Collaborator

Choose a reason for hiding this comment

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

Three issues:

  1. i am trying to disentangle MCUboot from Flash Map API as this entanglement blocks us from freely modifying both Zephyr and MCUboot, and so the usage of flash area id and flash_area within MCUboot is going to be phased out. There is also problem with flash area functions having different prototypes and flash_area objects having different members, depending on a system.
  2. area id in MCUboot and Zephyr app that got boot does not have to be the same; it is enough if there is storage before images that gets removed with overlay in mcuboot and numbering will differ
  3. mcuboot should not directly access flash_area objects, rather use getter functions like flash_area_get_id.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, makes sense. I have no issue changing this, really all I want is a workable solution for indicating the flash area that Zephyr's MCUBoot subsystem should use when marking an image as confirmed. Is there an API for mapping the boot slot number back to a flash area we can use?

#endif

#if defined(MCUBOOT_VERSION_AVAILABLE)
Expand Down
3 changes: 2 additions & 1 deletion boot/bootutil/src/bootutil_priv.h
Original file line number Diff line number Diff line change
Expand Up @@ -259,7 +259,8 @@ struct boot_loader_state {
/* Image destination and size for the active slot */
uint32_t img_dst;
uint32_t img_sz;
#elif defined(MCUBOOT_DIRECT_XIP_REVERT)
#endif
#if defined(MCUBOOT_DIRECT_XIP_REVERT) || defined(MCUBOOT_RAM_LOAD_REVERT)
/* Swap status for the active slot */
struct boot_swap_state swap_state;
#endif
Expand Down
9 changes: 5 additions & 4 deletions boot/bootutil/src/loader.c
Original file line number Diff line number Diff line change
Expand Up @@ -2634,7 +2634,8 @@ print_loaded_images(struct boot_loader_state *state)
}
#endif

#if defined(MCUBOOT_DIRECT_XIP) && defined(MCUBOOT_DIRECT_XIP_REVERT)
#if (defined(MCUBOOT_DIRECT_XIP) && defined(MCUBOOT_DIRECT_XIP_REVERT)) || \
(defined(MCUBOOT_RAM_LOAD) && defined(MCUBOOT_RAM_LOAD_REVERT))
/**
* Checks whether the active slot of the current image was previously selected
* to run. Erases the image if it was selected but its execution failed,
Expand Down Expand Up @@ -2758,17 +2759,17 @@ boot_load_and_validate_images(struct boot_loader_state *state)
state->slot_usage[BOOT_CURR_IMG(state)].active_slot = NO_ACTIVE_SLOT;
continue;
}
#endif /* MCUBOOT_DIRECT_XIP */

#ifdef MCUBOOT_DIRECT_XIP_REVERT
#if defined(MCUBOOT_DIRECT_XIP_REVERT) || defined(MCUBOOT_RAM_LOAD_REVERT)
rc = boot_select_or_erase(state);
if (rc != 0) {
/* The selected image slot has been erased. */
state->slot_usage[BOOT_CURR_IMG(state)].slot_available[active_slot] = false;
state->slot_usage[BOOT_CURR_IMG(state)].active_slot = NO_ACTIVE_SLOT;
continue;
}
#endif /* MCUBOOT_DIRECT_XIP_REVERT */
#endif /* MCUBOOT_DIRECT_XIP */
#endif /* MCUBOOT_DIRECT_XIP_REVERT || MCUBOOT_RAM_LOAD_REVERT */

#ifdef MCUBOOT_RAM_LOAD
/* Image is first loaded to RAM and authenticated there in order to
Expand Down
11 changes: 11 additions & 0 deletions boot/zephyr/Kconfig
Original file line number Diff line number Diff line change
Expand Up @@ -344,6 +344,17 @@ config BOOT_DIRECT_XIP_REVERT
attempt to boot the previous image. The images can also be made permanent
(marked as confirmed in advance) just like in swap mode.

config BOOT_RAM_LOAD_REVERT
bool "Enable the revert mechanism in ramload mode"
depends on BOOT_RAM_LOAD
help
If y, enables the revert mechanism in ramload similar to the one in
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
If y, enables the revert mechanism in ramload similar to the one in
If y, enables the revert mechanism in ram-load similar to the one in

swap mode. It requires the trailer magic to be added to the signed image.
When a reboot happens without the image being confirmed at runtime, the
bootloader considers the image faulty and erases it. After this it will
attempt to boot the previous image. The images can also be made permanent
(marked as confirmed in advance) just like in swap mode.

config BOOT_BOOTSTRAP
bool "Bootstrap erased the primary slot from the secondary slot"
default n
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
/ {
sram@80007F00 {
compatible = "zephyr,memory-region", "mmio-sram";
reg = <0x80007F00 0x100>;
zephyr,memory-region = "RetainedMem";
status = "okay";

retainedmem {
compatible = "zephyr,retained-ram";
status = "okay";
#address-cells = <1>;
#size-cells = <1>;

boot_info0: boot_info@0 {
compatible = "zephyr,retention";
status = "okay";
reg = <0x0 0x100>;
};
};
};

chosen {
zephyr,bootloader-info = &boot_info0;
zephyr,code-partition = &boot_partition;
};
};
26 changes: 26 additions & 0 deletions boot/zephyr/boards/nrf52840dk_nrf52840_ram_load.overlay
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
/ {
sram@2003FF00 {
compatible = "zephyr,memory-region", "mmio-sram";
reg = <0x2003FF00 0x100>;
zephyr,memory-region = "RetainedMem";
status = "okay";

retainedmem {
compatible = "zephyr,retained-ram";
status = "okay";
#address-cells = <1>;
#size-cells = <1>;

boot_info0: boot_info@0 {
compatible = "zephyr,retention";
status = "okay";
reg = <0x0 0x100>;
};
};
};

chosen {
zephyr,bootloader-info = &boot_info0;
zephyr,code-partition = &boot_partition;
};
};
4 changes: 4 additions & 0 deletions boot/zephyr/include/mcuboot_config/mcuboot_config.h
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,10 @@
#define MCUBOOT_DIRECT_XIP_REVERT
#endif

#ifdef CONFIG_BOOT_RAM_LOAD_REVERT
#define MCUBOOT_RAM_LOAD_REVERT
#endif

#ifdef CONFIG_BOOT_RAM_LOAD
#define MCUBOOT_RAM_LOAD 1
#define IMAGE_EXECUTABLE_RAM_START CONFIG_BOOT_IMAGE_EXECUTABLE_RAM_START
Expand Down
Loading