moonboot
moonboot copied to clipboard
WIP: Add swap mode using InternalMemory/Flash scratch pages
A proposal for using the internal memory to store the page that is being swapped, spread over multiple pages to maximize flash life expectancy. Also adds the swap exchange step to the current state, making the process robust against brownouts.
Possible improvements
- Unsure if we should keep the older RAM-only swap mode.
- Distinguish various types of Steps in state (as it is not needed for RAM swap)
- Remove reference to concrete banks in state, and instead use an enum or index to reduce state size.
- Use
usizeinstead ofu32as many HAL and memory operations are based onusizeand currently rather many conversions are required.
TODO's
- Example in how to use this mode.
- Perhaps implement some tests on a simulated processor.
I also implemented flash state on multiple pages for STM32 which is based on a linked list in order to maximize flash life expectancy. As it is rather specific to the type of flash and interface I have not (yet) added it to the repo. Perhaps we can discuss how to introduce it.
Thanks a lot for implementing this, very nice!
One thought: With this library, I tried to keep as much as possible of the configuration in the types generics etc, to make it safe during runtime.
The Exchange scratchpad is defined via an array of Page indices (or a memory range?) in the Memory Banks. This can lead to a situation in which the firmware image written into the Update Bank is so large that it is written into the exchange pages. To keep this as generic as possible (like for example, scratchpad is in a different memory than the Update-/FirmwareBank, e.g. external EEPROM), a third Storage passed in there as a Scratchpad memory would make the distinction clearer.
This makes the usage more verbose though, as another Storage-instance with a small region has to be created (and the size should be a multiple of INTERNAL_PAGE_SIZE, even more complicated!). Additionally, the linker scripts which are generated need to know of a possible scratchpad region to reserve that space.
So to summarise: What do you think about treating the Scratchpad as a separate Bank? This way, it can either be a RAM-scratchpad or an Flash (Internal) scratchpad, currently determined via the MemoryUnit type. Unfortunately not decided at compiletime, but IMO this is OK
But this is of course open to discussion.
About the u32 Addresses: Yes, I also would prefer to use usize, this was just inspired by the embedded-storage crate, which if I remember correctly also uses u32 as an address type. But yeah, it should be changed to usize.
About the flash state: That is also a nice addition to it, wear obviously might be a problem, especially when every exchange-step is stored in flash state. This is exactly why I made the State storage generic, so I'd be glad to integrate such an implementation into this project. It of course is required to make moonboot power-outage-safe, in addition to this PR! :)
I have been thinking about the MemoryUnit-situation. We can either:
- Abolish the use of the
embedded_storagetraits, and fork them to also takeMemoryUnitinto consideration. - Remove
MemoryUnitand let the implementer of theembedded_storagetrait do the heavy lifting by mapping certain address ranges to different units of memory, as is the convention with typical MCU memory models.
A third option would be to pass a Storage unit for every MemoryUnit. This is simple as long as we only distinguish between one Internal and one (optional) External MemoryUnit, but gets more complicated as soon as there are more than two different MemoryUnits (e.g. when using two external memories).
Is the use-case with two different external memories realistic? If so, I propose we either
- replace MemoryUnit with a number (u8 should be enough) indicating which memory to use (not very ergonomic)
- make MemoryUnit have an Internal() and External(u8) value, where External(u8) addresses the n-th external memory
Both of these are pretty complicated and still not as ergonomic as I prefer.
About your ideas:
- I wouldn't want to abolish/fork the
embedded_storagetraits, as they will make usage of an external memory with drivers implementing the traits very easy, in addition to the handy NorFlash implementations - Having a manual memory map would make it easier for moonboot, but part of my goal with this project is to make usage of the framework as ergonomic as possible (as in, integrated into rust code, if possible "autogenerated"). See for example the generation of linker scripts. Making the consumer of this lib manage a virtual memory map in addition to the microcontrollers memory-map would make things more complicated. We could of course use such a memory-map internally and put an abstraction layer over the external API, but that complicate things even further I think.
I have moved the MemoryUnit discussion to #3