Swap move should only need one extra sector

[d3zd3z]

When using swap-move with large sectors, where the code is a single sector, it appears that there needs to be two extra sectors, in the first slot, rather than just one. This shouldn't need to be the case, and is especially wasteful when sectors are large.

As an example, the 128k sector ST device in the sim can be re-configured for swap-move, and it will fail.

[nordicjm]

Well 2 extra sectors are always going to be required, not 2 complete sectors, but one complete sector for the swap and the image overhead, and since you should be able to clear the status of an image or set it as confirmed, then that area should be eraseable, thus 2 sectors

[de-nordic]

Well 2 extra sectors are always going to be required, not 2 complete sectors, but one complete sector for the swap and the image overhead, and since you should be able to clear the status of an image or set it as confirmed, then that area should be eraseable, thus 2 sectors

We do not erase status. What really happens is that the status is not copied when image is swapped that is why there is such complicated logic deciding on what should be executed; for example you may have two slots with images that have no flags set, which means you boot slot1, or primary slot has some flags set but secondary not - now you decide by bits in primary. If you have some flags set in primary and secondary then what happens depends on flags.
What we actually need is one sector + all the space needed for trailer part with flags.

[nordicjm]

Well 2 extra sectors are always going to be required, not 2 complete sectors, but one complete sector for the swap and the image overhead, and since you should be able to clear the status of an image or set it as confirmed, then that area should be eraseable, thus 2 sectors

We do not erase status. What really happens is that the status is not copied when image is swapped that is why there is such complicated logic deciding on what should be executed; for example you may have two slots with images that have no flags set, which means you boot slot1, or primary slot has some flags set but secondary not - now you decide by bits in primary. If you have some flags set in primary and secondary then what happens depends on flags. What we actually need is one sector + all the space needed for trailer part with flags.

There is an optional Kconfig for MCUmgr to delete updates marked for test, if you do not erase the status this means that the flags will remain the next time an image is loaded to the secondary slot if you cannot erase the flags part.

[gschwaer]

According to the documentation, the swap status is kept in the image trailer [1]. Also each slot has it's own image trailer [2]. Two slots should always have image trailers of the same size, since the size is determined by MCUboot compile options and flash layout, which is forced to be identical between slots. So the only reason the first slot should be larger than the second slot is that it can do the "move-up by one" of all sectors [3].

For some reason the code considers the size that the app has at disposal [4], effectively ignoring the trailer, but a trailer has to exist in both slots. Unfortunately, PR #1883 that introduced this does not explain the issue for the fix in detail. In my understanding the check for num_usable_sectors_pri != (num_sectors_sec + 1) is not necessary.

[1] "An image trailer contains the following fields: 1. Swap status" -- https://docs.mcuboot.com/design.html#image-trailer
[2] "When using the term “image trailers” what is meant is the aggregate information provided by both image slot’s trailers." -- https://docs.mcuboot.com/design.html#image-trailers
[3] "This algorithm [i.e., Swap-Move] is an alternative to the swap-using-scratch algorithm. It uses an additional sector in the primary slot to make swap possible." -- https://docs.mcuboot.com/design.html#image-swap-no-scratch
[4] Assuming trailer_sz < sector_sz, app_max_sectors(state) will return boot_img_num_sectors(state, BOOT_PRIMARY_SLOT) - 1, which ignores the trailer. Then the following code will additionally ensure that one more sector is free in primary slot for swap-move. The total free sectors is now at 2:

mcuboot/boot/bootutil/src/swap_move.c

Lines 271 to 275 in 3b38056

	num_usable_sectors_pri = app_max_sectors(state);
	if ((num_sectors_pri != num_sectors_sec) &&
	(num_sectors_pri != (num_sectors_sec + 1)) &&
	(num_usable_sectors_pri != (num_sectors_sec + 1))) {

[nordicjm]

According to the documentation, the swap status is kept in the image trailer [1]. Also each slot has it's own image trailer [2]. Two slots should always have image trailers of the same size, since the size is determined by MCUboot compile options and flash layout, which is forced to be identical between slots. So the only reason the first slot should be larger than the second slot is that it can do the "move-up by one" of all sectors [3].

That is not correct, the swap status goes in the primary image, there is no reason to keep a duplicated one in the secondary image too

[gschwaer]

That is not correct, the swap status goes in the primary image, there is no reason to keep a duplicated one in the secondary image too

I think you are misunderstanding my intent. I'm saying the swap status is part of the image trailer, and each image slot has an image trailer. I'm not saying both are used when swapping! But according to the documentation, they are present, so flash area is allocated for them. So why would the primary slot have to be one sector larger than the secondary (excluding the +1 for swap-move), when both have the same size image trailer?