-
Notifications
You must be signed in to change notification settings - Fork 7.8k
modules: mcuboot: enable support for RAMLOAD mode with revert #85254
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
modules: mcuboot: enable support for RAMLOAD mode with revert #85254
Conversation
The following west manifest projects have changed revision in this Pull Request:
✅ All manifest checks OK Note: This message is automatically posted and updated by the Manifest GitHub Action. |
Note: this PR requires mcu-tools/mcuboot#2197 from MCUBoot. It also contains commits from #85096, because I validated this support on the RT1050-EVK |
case 1: | ||
fa_id = FIXED_PARTITION_ID(SLOT1_PARTITION); | ||
break; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
needs #if here too
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added- thanks. What will we need to do on the MCUBoot side to synchronize this? I think this change will have to go in when we update MCUBoot to include mcu-tools/mcuboot#2197 since we are moving the overlays as well
Great, is working now! One outstanding issue then looks good |
aa6490d
to
0c5ec1d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The change looks good. Once mcuboot has moved to a supporting version, I can give an approval.
west.yml
Outdated
@@ -23,6 +23,8 @@ manifest: | |||
url-base: https://github.com/zephyrproject-rtos | |||
- name: babblesim | |||
url-base: https://github.com/BabbleSim | |||
- name: mcuboot |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have merged 2197 into mcuboot, so once this makes it into the Zephyr module, we can directly refer to that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great- I think we will need to be careful with synchronization here- the MCUBoot side change moves some overlay files into the MCUBoot repo that were previously defined here (used for the ramload sample). So I think that these changes (or a subset) will be needed with the PR that bumps the mcuboot manifest revision.
modules/Kconfig.mcuboot
Outdated
from there. The image must be linked to execute from RAM, the address that it is copied | ||
to is specified using the load-addr argument when running imgtool. | ||
This option automatically selectes MCUBOOT_BOOTLOADER_NO_DOWNGRADE as it is not possible | ||
to swap back to older version of the application. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a little ambiguous to me, since the revert does kind of go back to an older version of the application. Not sure how to word it more clearly, though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, that is true. Perhaps
This option automatically selectes MCUBOOT_BOOTLOADER_NO_DOWNGRADE as MCUBoot will automatically select the highest revision of the application to boot. Note however that MCUBoot will select an older revision of the application if the booted revision does not mark itself as confirmed.
MCUMgr ram load support was hardcoding the overlay file for the nrf52840dk. To make this support generic, move the _ram_load overlay file to the MCUBoot repository, as part of the mcuboot application. Signed-off-by: Daniel DeGrasse <[email protected]>
MCUBoot RAMLOAD mode relies on CONFIG_XIP=n, but FLASH_MCUX_FLEXSPI_XIP y-selects this symbol. Disable CONFIG_FLASH_MCUX_FLEXSPI_XIP for the case where we are using MCUBoot ramload mode. Signed-off-by: Daniel DeGrasse <[email protected]>
Add support for using the ramload mode of MCUBoot on the mimxrt1050_evk Signed-off-by: Daniel DeGrasse <[email protected]>
Add Kconfigs for RAMLOAD_WITH_REVERT mode in MCUBoot. This mode works in a manner similar to DIRECT_XIP_WITH_REVERT- namely, mcuboot will only boot an image that is either confirmed or marked as pending. If both images are confirmed, mcuboot will still select the one with the higher version, so downgrading is not possible using this mode. Signed-off-by: Daniel DeGrasse <[email protected]>
Add BOOT_RAM_LOAD_REVERT as a defined Kconfig to the whitelist Signed-off-by: Daniel DeGrasse <[email protected]>
0c5ec1d
to
1b596bf
Compare
Updated now that Zephyr's copy of MCUBoot includes the relevant ramload support |
@@ -39,6 +39,11 @@ if(SB_CONFIG_BOOTLOADER_MCUBOOT) | |||
set_config_bool(${ZCMAKE_APPLICATION} CONFIG_MCUBOOT_BOOTLOADER_MODE_RAM_LOAD y) | |||
set_config_bool(${ZCMAKE_APPLICATION} CONFIG_XIP n) | |||
set_config_int(${ZCMAKE_APPLICATION} CONFIG_FLASH_SIZE 0) | |||
elseif(SB_CONFIG_MCUBOOT_MODE_RAM_LOAD_WITH_REVERT) | |||
# RAM load mode requires XIP be disabled and flash size be set to 0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unrelated to the core of the PR, do you know why RAM_LOAD mode requires XIP=n, but SINGLE_APP_RAM_LOAD does not?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't, no- not familiar with SINGLE_APP_RAM_LOAD
. @nordicjm do you know why this is the case?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
An oversight probably
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
XIP is orthogonal to SINGLE_APP_RAM_LOAD
- it simply loads the image to RAM. If the image was linked to run from the area in the RAM it is going to be loaded, it can use XIP.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
XIP is orthogonal to
SINGLE_APP_RAM_LOAD
- it simply loads the image to RAM. If the image was linked to run from the area in the RAM it is going to be loaded, it can use XIP.
Can you perhaps clarify this in the Kconfig help text, I think it's a very important point that will help many users.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
XIP is orthogonal to
SINGLE_APP_RAM_LOAD
- it simply loads the image to RAM. If the image was linked to run from the area in the RAM it is going to be loaded, it can use XIP.
that is not consistent with what the Kconfig does:
help
This option allows the kernel to operate with its text and read-only
sections residing in ROM (or similar read-only memory). Not all boards
support this option so it must be used with care; you must also
supply a linker command file when building your image. Enabling this
option increases both the code and data footprint of the image.
so XIP is execute from flash, non-XIP is execute from RAM
When signing for ramload mode, respect the write alignment size setting. This is required when creating a confirmed image, as the BOOT_MAGIC value changes based on the alignment size in use. Signed-off-by: Daniel DeGrasse <[email protected]>
Prefer flashing the confirmed image when one is created with CONFIG_MCUBOOT_GENERATE_CONFIRMED_IMAGE. This way, the west runner will flash this image over the signed image if it exists. Signed-off-by: Daniel DeGrasse <[email protected]>
…cies CONFIG_FLASH_MCUX_FLEXSPI_XIP should also be disabled when using MCUBoot ramload mode with revert support. Signed-off-by: Daniel DeGrasse <[email protected]>
Much like in RAMLOAD mode, RAMLOAD_WITH_REVERT requires that mcuboot subsystem fetch bootloader information via the retention subsystem. Signed-off-by: Daniel DeGrasse <[email protected]>
boot_fetch_active_slot needs to map the slot number to a flash ID, as this is what the DFU subsystem expects when interacting with the flash partition. Signed-off-by: Daniel DeGrasse <[email protected]>
We should not block erasing pending images when using ramload with revert mode, because uploading multiple confirmed images with the same version would brick the device (preventing future FW updates). Update the dependencies of img_mgmt_slot_in_use to account for this. Signed-off-by: Daniel DeGrasse <[email protected]>
Add testcase to build mcuboot and smp_svr application using ramload with revert mode Signed-off-by: Daniel DeGrasse <[email protected]>
1b596bf
to
bb8c665
Compare
|
This PR adds support for RAMLOAD mode with revert in MCUBoot, and enables a testcase for it with the mcumgr subsystem. RAMLOAD mode with revert functions similar to the direct xip mode with revert- MCUBoot will not ramload an image that is not confirmed or marked pending.
This PR also modifies the method of getting the active slot number when using ramload modes- we need to query the flash area ID, not the active boot slot for use with mcuboot boot utilities