-
Notifications
You must be signed in to change notification settings - Fork 8k
stm32: mcuboot: add support for mcuboot tests and samples #93862
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
base: main
Are you sure you want to change the base?
stm32: mcuboot: add support for mcuboot tests and samples #93862
Conversation
959b37f
to
9b3a129
Compare
add boot and slots partitions for flash memory in dedicated mcu boards Signed-off-by: Fabrice DJIATSA <[email protected]>
Add STM32 supported boards for the test. Signed-off-by: Fabrice DJIATSA <[email protected]>
Add STM32 supported boards for the test. Signed-off-by: Fabrice DJIATSA <[email protected]>
Add STM32 supported boards for the test. Signed-off-by: Fabrice DJIATSA <[email protected]>
Since the addition of a new storage partition definition in the nucleo_f746zg dts, this node must be deleted before performing this test. Signed-off-by: Fabrice DJIATSA <[email protected]>
9b3a129
to
920d124
Compare
|
boot_partition: partition@0 { | ||
label = "mcuboot"; | ||
reg = <0x00000000 DT_SIZE_K(34)>; | ||
reg = <0x00000000 DT_SIZE_K(64)>; |
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.
48KB is the minimum MCUboot partition size, Flash sectors are atmost 4KB on this SoC, so why give more to MCUboot?
Same for other Flash-constrainted SoCs with sub-32KB sector size.
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 just based it on how the nucleo_wba55cg was added, and also compared the sizes of the flash sectors/pages in the refman and adapted with the rest so that it worked on the tests.
If you have some references on the rules to follow, that would be welcome.
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.
see comments, there is a bunch of different things in this PR so not sure what the idea is...
/* Set 4KB of storage at the end of 128KB flash */ | ||
storage_partition: partition@1f000 { | ||
label = "storage"; | ||
reg = <0x0001f000 DT_SIZE_K(4)>; | ||
}; |
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.
don't know what the sector size is but for nvs this would need to be at least 3 sectors to be usable
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.
Indeed sectors (named "pages" in th SoC refman) are 2kB large.
slot0_partition: partition@10000 { | ||
label = "image-0"; | ||
reg = <0x00010000 DT_SIZE_K(32)>; | ||
}; | ||
|
||
slot1_partition: partition@18000 { | ||
label = "image-1"; | ||
reg = <0x00018000 DT_SIZE_K(30)>; | ||
}; |
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.
sizes are different?
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.
Aren't the sizes required to be different? There is no scratch partition, so this is using one of the move algorithms.
slot0_partition: partition@40000 { | ||
label = "image-0"; | ||
reg = <0x00040000 DT_SIZE_K(448)>; | ||
}; | ||
|
||
slot1_partition: partition@b0000 { | ||
label = "image-1"; | ||
reg = <0x000b0000 DT_SIZE_K(320)>; | ||
}; |
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.
sizes are very different? If using different sizes then the extra sector should be in slot 1 and swap using offset should be used rather than swap using move
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.
Thank you for your feedback.
Before addressing the comments, if you could please provide some references on algorithm-based partition distribution or other useful resources, they would be very helpful.
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.
Seen F746xx SoCs flash sectors distribution (rm0385 rev9/table 3/page 79), I think the sole supported layout requires: slot0 is 2x256kB, slot1 is 1x256kB, storage is 3x32kB (placed at flash start) and boot occupies a signle 128kB sector.
*/ | ||
|
||
/delete-node/ &storage_partition; | ||
&flash0 { |
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.
newline line 8
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.
It's good to get better test coverage, but I don't think it is reasonable to increase the bootloader partition so much. You may want add additional configuration overlays to trim it down and look at smaller nordic nrf51/nrf52 socs to see what is possible.
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.
Since the dts files in the zephyr tree are where most people will start when trying to support something new, I think it is important that we get this right. For the stm32 this means:
- scratch should only be used on the devices that need it. These generally have very large sectors, and there ends up being three partitions for code, slot0, slot1, and scratch all being 128k.
- For other devices, we should always be configured for swap-offset, and the partition tables should have a slot1 image that is 1 sector larger than slot0.
slot1_partition: partition@27800 { | ||
label = "image-1"; | ||
reg = <0x00027800 DT_SIZE_K(94)>; | ||
}; |
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.
Does this actually work? At least as I remember it, without a scratch partition, there are specific requirements for the sizes, and specifically one slot must be exactly one sector larger than the other (we'd have to look up which is which.)
slot0_partition: partition@10000 { | ||
label = "image-0"; | ||
reg = <0x00010000 DT_SIZE_K(32)>; | ||
}; | ||
|
||
slot1_partition: partition@18000 { | ||
label = "image-1"; | ||
reg = <0x00018000 DT_SIZE_K(30)>; | ||
}; |
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.
Aren't the sizes required to be different? There is no scratch partition, so this is using one of the move algorithms.
* SPDX-License-Identifier: Apache-2.0 | ||
*/ | ||
|
||
/delete-node/ &storage_partition; |
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.
Since the board already provides a storage area, it's seems not needed to override it.
slot0_partition: partition@40000 { | ||
label = "image-0"; | ||
reg = <0x00040000 DT_SIZE_K(448)>; | ||
}; | ||
|
||
slot1_partition: partition@b0000 { | ||
label = "image-1"; | ||
reg = <0x000b0000 DT_SIZE_K(320)>; | ||
}; |
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.
Seen F746xx SoCs flash sectors distribution (rm0385 rev9/table 3/page 79), I think the sole supported layout requires: slot0 is 2x256kB, slot1 is 1x256kB, storage is 3x32kB (placed at flash start) and boot occupies a signle 128kB sector.
|
||
slot1_partition: partition@17800 { | ||
label = "image-1"; | ||
reg = <0x00017800 DT_SIZE_K(30)>; |
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.
Could you update the flash:
tag in board specific YAML files to reflect the Zephyr application max flash size for each of these modified layouts?
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.
Could you update the
flash:
tag in board specific YAML files to reflect the Zephyr application max flash size for each of these modified layouts?
No, it's not a good idea, unless the target is meant to be used exclusively with MCUboot.
Instead, there should be a board mcuboot
variant, with - sysbuild=true
in its YAML file and MIN(slot0,slot1) as flash size.
/* Set 4KB of storage at the end of 128KB flash */ | ||
storage_partition: partition@1f000 { | ||
label = "storage"; | ||
reg = <0x0001f000 DT_SIZE_K(4)>; | ||
}; |
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.
Indeed sectors (named "pages" in th SoC refman) are 2kB large.
This PR adds support for testing MCUboot tests and samples. However, not all series are supported yet due to some limitations and hardware incapacity.
Related tests and samples :