Add support for RAMLOAD mode with revert #2197
Conversation
… boards Add ram load overlays for nrf52840dk and mimxrt1050_evk boards. These ram load overlays are moved from the Zephyr in tree smp_svr sample, to enable ram load support for multiple platforms. Signed-off-by: Daniel DeGrasse <[email protected]>
Add support for revert mode when using ramloading. This includes the same restrictions as revert mode with direct-xip, namely that an image must have a valid BOOT_MAGIC value in order for mcuboot to attempt to load it. Signed-off-by: Daniel DeGrasse <[email protected]>
Add BLINFO_FLASH_AREA TLV to boot info passed via retention. This is useful to report the flash area ID of the booted image, which can be used by boot utilities to read image status Signed-off-by: Daniel DeGrasse <[email protected]>
danieldegrasse
requested review from
nordicjm,
de-nordic and
davidvincze
as code owners
| @@ -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 | |||
There was a problem hiding this comment.
Isn't this already done vis BLINFI_RUNNING_SLOT?
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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"),
| if (!rc) { | ||
| rc = boot_add_data_to_shared_area(TLV_MAJOR_BLINFO, | ||
| BLINFO_FLASH_AREA, | ||
| sizeof(fap->fa_id), (void *)&fap->fa_id); | ||
| } |
There was a problem hiding this comment.
Three issues:
- 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.
- 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
- mcuboot should not directly access flash_area objects, rather use getter functions like flash_area_get_id.
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
can use retention for running image slot so that commit should be dropped, can you also add a section to the documentation similar to direct-xip with revert https://github.com/mcu-tools/mcuboot/blob/main/docs/design.md?plain=1#L452 and add a file here https://github.com/mcu-tools/mcuboot/tree/main/docs/release-notes.d with a small note on adding this (see other files in directory for examples and for maximum line length and style, think it's 64 characters or so
| 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 |
There was a problem hiding this comment.
| 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 |
This PR adds support for ram loading mode with revert to MCUBoot. This mode functions similar to direct XIP mode with revert- MCUBoot will now refuse to load an image to RAM unless the image trailer indicates that the image is confirmed or marked pending.
If multiple images are valid, the one with the highest version will be selected for ramloading, so downgrade is still not possible.
This PR also adds the
BLINFO_FLASH_AREATLV tag to the bootloader boot record, which records the flash area ID in the boot record. This can be used by systems like Zephyr to determine which slot was booted, which is useful for marking an image as confirmed or querying statusNote that this PR includes the commit from #2196, as I validated support for this feature using the RT1050-EVK. If desired, I can rebase this to drop that commit