Skip to content

loader: Allow to specify slot number in version comparison #2343

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

tomchy
Copy link
Contributor

@tomchy tomchy commented Jun 16, 2025

Allow to depend on a specific slot while specifying the version number. This functionality is useful when the Direct XIP mode is used and the booting process of other images is done by the next stage, not the MCUboot itself.

@de-nordic de-nordic self-requested a review June 17, 2025 10:42
@de-nordic
Copy link
Collaborator

Sorry, accidental approval.

@tomchy tomchy force-pushed the feature/mcuboot/NCSDK-NONE_Slotted_dependencies branch 2 times, most recently from ab92081 to a79cd55 Compare June 24, 2025 12:00
@tomchy tomchy marked this pull request as ready for review June 24, 2025 12:30
@de-nordic de-nordic requested a review from robertstypa June 26, 2025 11:22
Copy link
Collaborator

@de-nordic de-nordic left a comment

Choose a reason for hiding this comment

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

Some stuff needs moving around and clarification.
Addition to dependency requires update of design documents.

Comment on lines +123 to +125
#ifdef CONFIG_BOOT_VERSION_CMP_USE_SLOT_NUMBER
#define MCUBOOT_VERSION_CMP_USE_SLOT_NUMBER
#endif
Copy link
Collaborator

Choose a reason for hiding this comment

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

Some more verbose description on MCUBOOT_VERSION_CMP_USE_SLOT_NUMBER please. I know that this, in this case, is translation from Kconfig, but it affects generic code and we are, at this point, very bad at documenting all the crap we add to the mcuboot_config.h files.

@@ -411,6 +411,24 @@ boot_verify_slot_dependency(struct boot_loader_state *state,
uint8_t swap_type = state->swap_type[dep->image_id];
dep_slot = BOOT_IS_UPGRADE(swap_type) ? BOOT_SECONDARY_SLOT
: BOOT_PRIMARY_SLOT;
#elif defined(MCUBOOT_VERSION_CMP_USE_SLOT_NUMBER)
switch(dep->slot) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

boot_verify_slot_dependencies does range check for image number, on the dependency TLV it read, it should do the same with slot number.

@@ -404,7 +424,7 @@ def convert(self, value, param, ctx):
'(for mcuboot <1.5)')
@click.option('-d', '--dependencies', callback=get_dependencies,
required=False, help='''Add dependence on another image, format:
"(<image_ID>,<image_version>), ... "''')
"(<image_ID>,[<slot_ID>],<image_version>), ... "''')
Copy link
Collaborator

Choose a reason for hiding this comment

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

I guess that these
image
are valid ID I can use here?
Bug there is no user input filter here. I know that MCUboot will reject the image, at least by the logic of boot_verify_slot_dependency (which should actually be in boot_verify_slot_dependencies), but it is not even clear what values user should pass here and if 0 is special.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There is only a slight validation, based on the regular expression.
Since there is close to none justification for using more than those three predefined values, I'll try to use words instead of mysterious integers.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure about the change of placement - this logic must affect the local dep_slot variable, that's why it is there.
I guess you have some ideas, so we can discuss possible solutions tomorrow.

@de-nordic de-nordic added area: core Affects core functionality area: zephyr Affects the Zephyr port labels Jul 8, 2025
Allow to depend on a specific slot while specifying the version number.
This functionality is useful when the Direct XIP mode is used and the
booting process of other images is done by the next stage, not the
MCUboot itself.

Signed-off-by: Tomasz Chyrowicz <[email protected]>
@tomchy tomchy force-pushed the feature/mcuboot/NCSDK-NONE_Slotted_dependencies branch from a79cd55 to 89d1d85 Compare July 8, 2025 16:30
@tomchy tomchy requested a review from de-nordic July 8, 2025 16:32
@de-nordic
Copy link
Collaborator

@robertstypa Please take a look at the snake language code.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: core Affects core functionality area: zephyr Affects the Zephyr port
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants