Skip to content
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

boot: zephyr: Check for unexpected flash sector size not compatible w/ series with variable sector sizes #2120

Open
erwango opened this issue Nov 13, 2024 · 6 comments

Comments

@erwango
Copy link

erwango commented Nov 13, 2024

#2059 introduce sanity check based on optional property erase-block-size.

This prevents use of mcuboot on platform not defining this property.
One example are STM32F2, STM32F4 and STM32F7 series which have variable sector size. On these series, erase-block-size depends on sector size and then cannot be defined as done usually in flash controller parent.

Check should have a fallback for this case.

@erwango erwango changed the title boot: zephyr: Check for unexpected flash sector size broke mcuboot support on series with variable sector sizes boot: zephyr: Check for unexpected flash sector size not compatible w/ series with variable sector sizes Nov 13, 2024
@de-nordic
Copy link
Collaborator

de-nordic commented Nov 15, 2024

OK, so there are two issues here:

  1. validation code should not be called each time mcuboot starts, that is pointless usage of flash and should be only done when user still tunes the MCUboot for their purpose
  2. The sector size should not be verified against values set by Kconfig, and that value would be the MCUboot expected sector swap size.

The MCUboot should stop playing with discovering, at run time, what device layout is and instead allow user to basically tune that value via Kconfig to most proper for their configuration - we have to move data by largest minimal erase unit in range of storage device uses in a given range.
Largest minimal means largest of minimal for all devices, so if internally we have 16k but external device has 4k, we are moving data by 16k anyway.

This should be option selected by Kconfig, validate once and then used.

If we would change to using such Kconfig tuned value, there would be some decrease in code needed so serve stuff and reduced requirement for ram, as we no longer have to store layout - layout consists then of: size of erase page, number of pages (two variables).

Setting this via Kconfig also allows to easily tune the setting without touching the dts, which also means that software that runs after MCUboot can use the most efficient value for its purpose, for example the DFU can still erase by 4k sectors while MCUboot can swap by 16k sectors.

This separation also means that user can tune how much log will be needed and evaluate easier whether bigger swap size is desired and smaller log or opposite (of course to the limits of the hardware).

@erwango
Copy link
Author

erwango commented Nov 15, 2024

Looks like a sane proposal from my user point of view, if I understood everything.

@Laczen
Copy link

Laczen commented Nov 21, 2024

This option already exists, is is called flash_map_legacy and has been used to support swapping images of st devices with internal 2kB pages with external 4kB pages. The sysbuild supported image size calculation fails however.

@de-nordic
Copy link
Collaborator

de-nordic commented Nov 21, 2024

This option already exists, is is called flash_map_legacy and has been used to support swapping images of st devices with internal 2kB pages with external 4kB pages. The sysbuild supported image size calculation fails however.

No it is not what that is doing. It literally creating in mem map of sectors with offset and sizes, and the #2120 (comment) clearly states that the layout is not needed, because once you have decided what the software sector size is you only need the number.

@Laczen
Copy link

Laczen commented Nov 21, 2024

This option already exists, is is called flash_map_legacy and has been used to support swapping images of st devices with internal 2kB pages with external 4kB pages. The sysbuild supported image size calculation fails however.

No it is not what that is doing. It literally creating in mem map of sectors with offset and sizes, and the #2120 (comment) clearly states that the layout is not needed, because once you have decided what the software sector size is you only need the number.

The basic idea is the same, overrule the erase-block-size that is retrieved from a device. The rest is implementation detail.

The definition of 1 global sectorsize will improve the behavior, but it will not be sufficient for all use cases. As an example take a st device with a blocksize of 16kB, 64kB and 128kB. One might want to use a little program in the 64kB region and one in the 128kB region updateable from external flash. This will not be supported by 1 global sectorsize.

@de-nordic
Copy link
Collaborator

Such scenario is only supported by swap-scratch, which means that all image swaps require scratch partition of the minimal size of the biggest page that will be moved. In this case you move images by image slot size or scrach size, whichever is smaller.
So again values are determined by compile time already.

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

No branches or pull requests

3 participants