Fix bug causing long swap times in certain cases

[hjuul]

When the secondary slot is stored in a flash with smaller sector size than
the primary slot, MCUboot would swap more sectors than necessary.
This is an attempt to fix that.

This PR is related to #983.

More details in this Slack thread: https://mcuboot.slack.com/archives/C6CB5BXGC/p1627994014013400

[nvlsianpu]

fix looks sane.
Thanks!

[pennam]

@hjuul tests are complainig about your commit email address... maybe you shoud replace [email protected] with [email protected]

[d3zd3z]

This is failing ci with ../../boot/bootutil/src/swap_scratch.c:575: boot_swap_sectors: Assertion rc == 0' failed.`

[hjuul]

Ok, so I was encouraged to create this PR based on a naïve workaround I had for this issue.
Now that it fails, I don't know the code base enough to make it work.
Are there any maintainers that are willing to take this PR further?

[pennam]

since i'm interested in using this i'll will be happy to help to found out why is failing. Any suggestion from the maintainers is really appreciated. Is there a way to reproduce and debug the issues on the simulator?

[utzig]

I believe this is just plain wrong, the purpose of that loop is to match the copy sizes based on both sectors from primary and secondary, so sizes in multiples will work. The update proposed in this PR would try to write a 4K sector in internal flash into a 2K flash on SPI, for example, which obviously does not work. How many sectors are we talking about (in your case)? I believe it should be only enough to match the internal flash, so say you have 4K internal and 2K external, that would be a single sector, is this really that bad?

[hjuul]

@utzig , in my case I have 256KiB sector size on internal flash (primary slot) and only 4KiB sector size on external flash (secondary slot). So yeah, it's really bad.

[utzig]

@utzig , in my case I have 256KiB sector size on internal flash (primary slot) and only 4KiB sector size on external flash (secondary slot). So yeah, it's really bad.

I think you will basically have to add special handling for the last sector, since the swap code would like to erase 256KiB on the last sector fitting up to the image size, but you only want to erase up to the size enough for fitting the image, which could be much smaller than that, and also try to synchronize this between both slots, erasing one size in the primary and another size for the secondary. I don't think it will be remotely close to this simplistic approach, but feel free to prove me wrong! ;-)

[utzig]

@utzig , in my case I have 256KiB sector size on internal flash (primary slot) and only 4KiB sector size on external flash (secondary slot). So yeah, it's really bad.

How long does it take to erase 4KiB on that flash?

[utzig]

@utzig , in my case I have 256KiB sector size on internal flash (primary slot) and only 4KiB sector size on external flash (secondary slot). So yeah, it's really bad.

Just read your Zephyr thread here: zephyrproject-rtos/zephyr#38548. Your QSPI flash, MT25QU256, supports 4KB, 32KB, and 64KB sector erases, and the driver should choose automatically, for example, I implemented this for Mynewt, and I assume Zephyr's driver can also do it, otherwise it could be implemented in a few lines of code like this: https://github.com/apache/mynewt-core/blob/master/hw/mcu/nxp/kinetis/src/hal_qspi.c#L352.

[github-actions]

This pull request has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this pull request will automatically be closed in 14 days. Note, that you can always re-open a closed pull request at any time.