-
Notifications
You must be signed in to change notification settings - Fork 697
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
Make RAM loading available for single loaders #2062
Conversation
boot/zephyr/Kconfig
Outdated
config BOOT_NO_UPGRADE | ||
bool "No upgrade" | ||
help | ||
No upgrade is performed - usually, this option will be used in | ||
conjunction with BOOT_RAM_LOAD. |
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 know what you're doing here but no, this Kconfig is not needed and put the ram load Kconfig back where it was
Because single application slot has nothing to do with RAM load mode, they are two completely different modes, RAM load is it's own dual-slot system, single application image is a single flash image. If you want to add a single RAM slot mode then that's something that can be added as a distinct mode, not as modifying single application slot |
63b0dfe
to
e79c78e
Compare
v2:
|
Ok - using a separate Kconfig instead of reusing the choice under !SINGLE_APPLICATION_SLOT. |
boot/zephyr/Kconfig
Outdated
if SINGLE_APPLICATION_SLOT | ||
|
||
config SINGLE_APPLICATION_SLOT_RAM_LOAD | ||
bool "RAM load for single application slot" | ||
help | ||
If y, the image is loaded to RAM and executed from there. For this reason, | ||
the image has to be linked to be executed from RAM. The address that the | ||
image is copied to is specified using the load-addr argument to the | ||
imgtool.py script which writes it to the image header. | ||
|
||
endif # SINGLE_APPLICATION_SLOT |
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 needs to be a distinct mode, not one that uses SINGLE_APPLICATION_SLOT
, that Kconfig is as I said in the previous review for single flash application only, there needs to be a new Kconfig, one that does not use an existing Kconfig
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.
Ok - moved the Kconfig outside if SINGLE_APPLICATION_SLOT
, so one doesn't need something like:
CONFIG_SINGLE_APPLICATION_SLOT=y
CONFIG_SINGLE_APPLICATION_SLOT_RAM_LOAD=y
Only CONFIG_SINGLE_APPLICATION_SLOT_RAM_LOAD=y
is needed. Implementation-wise though, it will select SINGLE_APPLICATION_SLOT
as the code path is basically the same.
e79c78e
to
a49b6dc
Compare
v3:
|
boot/zephyr/Kconfig
Outdated
@@ -320,6 +307,28 @@ config BOOT_SWAP_SAVE_ENCTLV | |||
|
|||
endif # !SINGLE_APPLICATION_SLOT | |||
|
|||
config SINGLE_APPLICATION_SLOT_RAM_LOAD | |||
bool "RAM load for single application slot" | |||
select SINGLE_APPLICATION_SLOT |
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.
select SINGLE_APPLICATION_SLOT |
This needs to be a distinct mode, it can use the same .c file and check the ifdefs but this Kconfig needs to be its own Kconfig and not do anything with other MCUboot mode Kconfigs
@@ -108,6 +108,12 @@ | |||
|
|||
#endif /* CONFIG_SINGLE_APPLICATION_SLOT */ | |||
|
|||
#ifdef CONFIG_SINGLE_APPLICATION_SLOT_RAM_LOAD | |||
#define MCUBOOT_RAM_LOAD 1 |
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.
And here, add a new define for this mode and add it to the base .c files that needs it to enable the required functions
a49b6dc
to
e85fb1c
Compare
v4:
|
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.
Minor nit., changes are OK
@teburd Needs a signed off line from you |
Done |
@teburd CI still failing on sign off |
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.
CI issues need to be fixed
@nordicjm @nvlsianpu all commits with signed-off now (as the rebase I guess has changed the committer, which makes sense), CI passing, please re-review thanks! |
@nvlsianpu any updates? Seems I'll need a rebase now but would like feedback that this is good to go |
36f710e
to
025235f
Compare
RAM loading code is currently under bootutil/loader.c, and it's not accessible for different loaders, such as the single loaders. Future patches will make use of the RAM loading code outside the bootutil/loader.c context, and this patch prepares for that by making it standalone on boot/bootutil/src/ram_load.c Signed-off-by: Ederson de Souza <[email protected]> Signed-off-by: Tom Burdick <[email protected]>
Following the split of RAM code, these definitions will help use it with single slot applications. Signed-off-by: Ederson de Souza <[email protected]> Signed-off-by: Tom Burdick <[email protected]>
This option basically enables MCUBOOT_RAM_LOAD in a single slot configuration, meaning the image on slot0 will be loaded into RAM. Signed-off-by: Ederson de Souza <[email protected]> Signed-off-by: Tom Burdick <[email protected]>
Now that's possible to load image to RAM on single loaders, add support on Zephyr port for that. Signed-off-by: Ederson de Souza <[email protected]> Signed-off-by: Tom Burdick <[email protected]>
Latest rebase should fix the conflict that was introduced, the boot_enc_decrypt rename cause the rebase to show a conflict and that was fixed. |
They’ve been fixed again, please do review again when you can |
Code for loading the image to RAM is currently under
bootutil/loader.c
. However, single loaders (used when one setsMCUBOOT_SINGLE_APPLICATION_SLOT
) cannot use this file - a single loader in fact re-implements some of the methods ofbootutil/loader.c
.This PR splits the bits of code related to RAM loading to another file inside bootutil (
ram_load.c
) and make them available onbootutil_priv.h
, so that single loaders can simply reuse this code.Note: these are basically the first four patches from #2044 - it seems that the multiple source bits of that PR are a bit controversial, so this PR focus on the RAM loading availability, which is hopefully less controversial.