Skip to content

sim: fix swap-scratch trailer estimate for asymmetric slot geometry - #2808

Merged
nordicjm merged 2 commits into
mcu-tools:mainfrom
d3zd3z:fix-oversized-secondary-slot
Jul 30, 2026
Merged

sim: fix swap-scratch trailer estimate for asymmetric slot geometry#2808
nordicjm merged 2 commits into
mcu-tools:mainfrom
d3zd3z:fix-oversized-secondary-slot

Conversation

@d3zd3z

@d3zd3z d3zd3z commented Jul 29, 2026

Copy link
Copy Markdown
Member

The simulator's model of MCUboot's maximum-image-size calculation diverges from the firmware for swap-using-scratch on devices whose two slots have different sector sizes.

app_max_size_adjust_to_trailer() in boot/bootutil/src/swap_scratch.c evaluates get_first_trailer_sector_end_off() for both the primary and the secondary slot and takes the larger, because the swap logic copies on the common sector boundary of the two slots and so the sector that matters is the larger one. The sim's estimate_swap_scratch_trailer_size() looked at only one slot -- whichever one the caller happened to be installing an image into.

On a device with asymmetric slot geometry the sim could therefore compute the trailer padding from the small sector while the bootloader computed it from the large one, understating the space available to an image. It also meant an image installed into the primary and one installed into the secondary disagreed with each other, even though the firmware's app_max_size() is a per-image property rather than a per-slot one.

This is a test-fixture change only. No bootloader behaviour is affected; nothing invalid is accepted, and signature/TLV validation is untouched.

What changed

  • sim: export boot_scratch_trailer_sz through the C shim -- the sim was re-deriving the scratch trailer size by hand as 3 * align + (boot_trailer_sz - boot_status_sz), which hardcodes BOOT_STATUS_STATE_COUNT and recovers boot_trailer_info_sz() by subtraction. Expose the firmware's own boot_scratch_trailer_sz() the way boot_trailer_sz() and boot_status_sz() already are. bootutil_area.c only compiles it for swap-using-scratch builds, so run.c supplies a stub returning zero for the other configurations.

  • sim: consider both slots in the swap-scratch trailer estimate -- the fix itself. Take the maximum of the two slots' first-trailer-sector remainders, and drop the now-redundant slot index from the caller chain. This also matches the firmware's stopping condition when walking back over the trailer sectors: boot_get_first_trailer_sector() stops once the accumulated sectors reach the trailer size, so an exact fit leaves a whole sector rather than zero.

Effect today

Very nearly a no-op. The only configuration whose numbers move is Nrf52840SpiFlash (4K primary sectors, 8K secondary sectors, both 0x68000) at max-align 32:

slot before after firmware
primary (4K sectors) 0x3100 0x30a0 0x30a0
secondary (8K sectors) 0x30a0 0x30a0 0x30a0

The primary's first trailer sector holds only 5 * BOOT_MAX_ALIGN = 160 bytes of trailer, which is less than scratch_trailer_sz = 256, so the old code added 96 bytes of padding there; the secondary's 8K sector holds 4256 bytes and needs none. The firmware takes the max and adds none. So images installed into the primary slot gain the 96 bytes the bootloader had always been willing to give them. The estimate only ever increases, so this cannot turn a rejection into an acceptance.

Relationship to #2797

This is a standalone sim cleanup, but it unblocks #2797 ("boot: imgtool: Extend align to 128B"). The divergence is latent on main and only becomes load-bearing once MCUBOOT_BOOT_MAX_ALIGN can exceed 32. As the trailer grows by 384 * align, slot_trailer_off moves down until the smaller-sectored slot's first-trailer-sector remainder collapses to 5 * BOOT_MAX_ALIGN, which is smaller than scratch_trailer_sz = 3 * align + 5 * BOOT_MAX_ALIGN. The sim then adds padding the firmware does not (192 bytes at max-align 64, 384 at 128), while ImageSize::Oversized adds back only one alignment unit -- so the image built for oversized_secondary_slot is not actually oversized, the bootloader correctly accepts it, and the test fails.

Commit 473529cc on that branch works around this by disabling oversized_secondary_slot for every device and every swap mode at the new alignments:

#[cfg(all(not(feature = "max-align-64"), not(feature = "max-align-128")))]

With this PR in, that #[cfg] guard can and should be dropped -- leaving it in place would hide any real regression at max-align 64/128.

@URNHere -- flagging you as the author of #2797, since this is the piece that lets that suppression come back out.

Testing

Full cargo test --release suite, all passing, on: default, max-align-16, max-align-32, overwrite-only, swap-move, swap-offset, enc-rsa, logical-sectors-4k, sig-rsa, multiimage, bootstrap, and bootstrap,max-align-32.

To confirm the mechanism rather than just the outcome, the info! line in compute_largest_image_size() was captured across default, max-align-16, max-align-32, enc-rsa and logical-sectors-4k both with and without the patch. The output is byte-identical everywhere except the four Nrf52840SpiFlash lines at max-align 32 shown in the table above.

The max-align 64/128 verification can only be done on the #2797 branch, where those alignments exist.

d3zd3z added 2 commits July 29, 2026 09:30
The simulator's swap-scratch trailer estimate re-derives the scratch
trailer size by hand as 3 * align + (boot_trailer_sz - boot_status_sz),
which hardcodes BOOT_STATUS_STATE_COUNT and recovers
boot_trailer_info_sz() by subtraction.  Expose the firmware's own
boot_scratch_trailer_sz() the way boot_trailer_sz() and boot_status_sz()
are already exposed, so the sim can call it directly instead.

bootutil_area.c only compiles the function for swap-using-scratch
builds, so run.c supplies a stub returning zero for the other
configurations; the Rust side only calls it on the scratch path.

Signed-off-by: David Brown <david.brown@linaro.org>
Assisted-by: Claude:opus-5
estimate_swap_scratch_trailer_size() computed the trailer padding from
the geometry of a single slot -- whichever one the caller happened to be
installing an image into.  The firmware does not: app_max_size_adjust_
to_trailer() in swap_scratch.c evaluates get_first_trailer_sector_end_
off() for both the primary and the secondary slot and takes the larger,
because the swap logic copies on the common sector boundary of the two
slots and so the sector that matters is the larger one.

On a device whose two slots have different sector sizes the sim could
therefore compute padding from the small sector while the bootloader
computed it from the large one, understating the space available to an
image.  Since the maximum image size is a per-image property rather than
a per-slot one, an image installed into the primary and one installed
into the secondary also disagreed with each other.

Mirror the firmware: take the maximum of the two slots' first-trailer-
sector remainders, and drop the now-redundant slot index from the caller
chain.  Also match the firmware's stopping condition when walking back
over the trailer sectors -- boot_get_first_trailer_sector() stops once
the accumulated sectors reach the trailer size, so an exact fit leaves a
whole sector rather than zero -- and use boot_scratch_trailer_sz()
instead of re-deriving it.

This is a test-fixture change; no bootloader behaviour is affected.  It
is very nearly a no-op today: the only configuration whose numbers move
is Nrf52840SpiFlash (4K primary sectors, 8K secondary sectors) at
max-align 32, where images installed into the primary slot gain the 96
bytes the bootloader had always been willing to give them.  It matters
more for the pending work raising MCUBOOT_BOOT_MAX_ALIGN to 64 and 128,
where the divergence grows large enough to make the
oversized_secondary_slot test build an image that is not actually
oversized.

Signed-off-by: David Brown <david.brown@linaro.org>
Assisted-by: Claude:opus-5
Copilot AI review requested due to automatic review settings July 29, 2026 16:10

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@nordicjm
nordicjm merged commit 7ad6710 into mcu-tools:main Jul 30, 2026
87 checks passed
@URNHere

URNHere commented Jul 30, 2026

Copy link
Copy Markdown

Just to keep track of it here as well: Amended in #2797 commit 473529cc with commit 4890af6, which is the same, but without the oversized_secondary_slot suppression.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants