bootutil: sector size == padding size?

[jorgenmk]

https://github.com/mcu-tools/mcuboot/blob/main/boot/bootutil/src/bootutil_misc.c#L524

Here the padding size is assumed to always be the same as sector size, is this correct?

We are running into issues when application starts filling up the partition.

For example, try to build and run sdk-nrf/applications/connectivity_bridge for board thingy91x/nrf5340/cpuapp. This goes very wrong runtime.

Forcing padding_sz to 512 (default CONFIG_PM_PARTITION_SIZE_MCUBOOT_PAD) makes it work again.

@taltenbach

[nordicjm]

Padding there is the sector used for swapping images so no you cannot set it to 512, it must be the size of a sector

[taltenbach]

As @nordicjm said, padding_sz in the code you mentioned has nothing to do with CONFIG_PM_PARTITION_SIZE_MCUBOOT_PAD.

CONFIG_PM_PARTITION_SIZE_MCUBOOT_PAD defines the space reserved at the beginning of the image slots for storing the MCUboot header. The MCUboot header only needs 32 bytes but you also have to ensure the vector table (at the very beginning of your firmware image, placed just after the padded MCUboot header) is properly aligned. If the vector table is not properly aligned, the CPU will fault at runtime. The required alignment depends on your CPU and vector table size, e.g. for an ARMv8-M CPU, the Architecture Reference Manual says:

In a PE with a configurable vector table base, the vector table is naturally-aligned to a power of two, with an alignment value that is:
• A minimum of 128 bytes.
• Greater than or equal to (Number of Exceptions supported x4).

[maxd-nordic]

Before @taltenbach 's change, the max_image_size for the application image in our case was reported as 274'432. Now, it is 270'336, 4096 bytes less. The swapping has been working before, so is the new algorithm too strict? Also @nordicjm , shouldn't the build system mark the build failed if it is too large for the bootloader's requirements?

[nordicjm]

Before @taltenbach 's change, the max_image_size for the application image in our case was reported as 274'432. Now, it is 270'336, 4096 bytes less. The swapping has been working before, so is the new algorithm too strict? Also @nordicjm , shouldn't the build system mark the build failed if it is too large for the bootloader's requirements?

For what swap mode with what padding size and what size of each slot? It does, if you're not using NCS

[maxd-nordic]

Those are the partition sizes: partitions.txt
CONFIG_BOOT_SWAP_USING_MOVE=y is set, so I guess swap-move?

[nordicjm]

268KiB was never right then. You have 272KiB slots, you need one extra sector in the primary slot for swapping so you are left with 268KiB, on top of this you have the flags and swap status, the size picked needs 64 sectors but let's assume 128 since NCS doesn't do auto configuration (check your MCUboot Kconfig for the real value) so 128 * 4 * 3 = 1536 bytes for swap status which rounds up to the sector size of 4KiB, 268 - 4 = 264KiB, which is 270336, so the new value is right and old value was wrong

[maxd-nordic]

Alright, do we need to change what we pass to imgtool to make the build check effective or is this somewhere else?

[nordicjm]

Alright, do we need to change what we pass to imgtool to make the build check effective or is this somewhere else?

It's handled in zephyr, it's not and won't be handled in NCS whilst partition manager is in use

[maxd-nordic]

So this will just fail without indication now?
Also, how can it be that DFU succeeds on the previous bootloader versions if we should have needed this extra page?

[nordicjm]

So this will just fail without indication now? Also, how can it be that DFU succeeds on the previous bootloader versions if we should have needed this extra page?

NCS has never had image size checks because of partition manager. No it would never have worked, it would upload and accept the image then MCUboot would reject swapping it (this is assuming you upload the largest size application, most are going to be far smaller than that)

[maxd-nordic]

MCUBoot does swap this successfully with the old check. Why should that work if the threshold was correct?

[nordicjm]

It can't, the swap sector is first used in the swap process then the other sector would be erased and written to with the swap status, if your had image data there then the image would be destroyed

[maxd-nordic]

So the check must be wrong then. I can confirm that it did. The new version of the app is starting up, checksum verified and with correct git hash identification.

[nordicjm]

The check is not wrong, if it worked then you had different slot sizes or whatnot without realising it (or you're in upgrade only mode where there is no swap status), this is how it knows where it is when e.g. an unexpected reboot happens during update seems that there is a discrepancy in the check actually - notice here https://github.com/mcu-tools/mcuboot/blob/main/boot/bootutil/src/swap_move.c#L230 it does a - 2, for a swap sector and trailer sector but in https://github.com/mcu-tools/mcuboot/blob/main/boot/bootutil/src/swap_move.c#L538 it does a - 1 and only subtracts from first_trailer_idx after the size check, it should either subtract 2 or subtract before checking the size