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

feat(boot): enhance firmware write functionality #2543

Merged
merged 9 commits into from
Feb 14, 2024
Merged

feat(boot): enhance firmware write functionality #2543

merged 9 commits into from
Feb 14, 2024

Conversation

badrbouslikhin
Copy link
Contributor

This PR modifies the firmware update process via USB DFU, enhancing compatibility with high-capacity microcontrollers by optimizing how firmware images are received from a host.
Instead of erasing flash pages for each received chunk, which is not feasible for microcontrollers with large erase sizes like the STM32H747XI, we now erase the entire DFU partition upfront and sequentially write incoming chunks without further erasures.

This approach resolves compatibility issues with microcontrollers where the erase block size exceeds the USB control buffer capacity, which is often limited to 4KB. Such a mismatch makes it impossible to efficiently manage page erasures for each chunk received, as required by the standard USB DFU protocol which does not communicate the total number of chunks or their target addresses in advance (such mechanism is supported in DfuSe).

By introducing a write_firmware_without_erase method, used in conjunction with prepare_update, this PR allows for a more efficient firmware update process.
This method aligns with the third mechanism outlined in the USB DFU v1.1 spec, where a significant portion of memory is pre-erased, and firmware chunks are written into the prepared space without additional erasures.

@@ -60,6 +60,21 @@ impl<'d, DFU: NorFlash, STATE: NorFlash, RST: Reset, const BLOCK_SIZE: usize> Ha
}
Ok(Request::Dnload) if self.attrs.contains(DfuAttributes::CAN_DOWNLOAD) => {
if req.value == 0 {
match self.updater.prepare_update() {
Copy link
Member

@lulf lulf Feb 7, 2024

Choose a reason for hiding this comment

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

I think rather than introducing write_firmware_without_erase API, you should use the flash borrow returned from prepare_update to do the writing. Or is there something preventing that?

Copy link
Contributor Author

@badrbouslikhin badrbouslikhin Feb 7, 2024

Choose a reason for hiding this comment

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

That was the first thing I tried, but I struggled with lifetimes. It was especially tricky when I had both a BootLoader and a BlockingFirmwareUpdater instances in different scopes of the bootloader. For example, if the bootloader starts and it detects that a specific button is pressed, it sets State::DfuDetach and begins a USB DFU update.

I agree that the proposed API here isn't great though.

@Dirbaio
Copy link
Member

Dirbaio commented Feb 7, 2024

I think this can be fixed without extending (and therefore complicating) the FirmwareUpdater public API.

The issue is write_firmware requires the data to be page-aligned, which is prohibitive if pages are too big. We can instead make it accept smaller chunks, write them incrementally to the page, and when crossing into a new page, erase it before writing to it.

For example, when having 4kb pages and writing 1kb chunks:

  • 0th write_firmware call erases page 0, writes 1kb of data.
  • 1st write_firmware writes 1kb of data.
  • 2nd write_firmware writes 1kb of data.
  • 3rd write_firmware writes 1kb of data.
  • 4th write_firmware call erases page 1, writes 1kb of data.
  • 5th write_firmware writes 1kb of data.
  • etc

if written properly, this'd allow chunks to span across pages even. So the only requirement for chunk size is to be a multiple of WRITE_SIZE, but not necessarily a divisor of ERASE_SIZE. For example, with 4kb pages writing 2.5kb chunks:

  • 0th write_firmware call erases page 0, writes 2.5kb of data.
  • 1st write_firmware writes 1.5kb of data, erases page 1, writes 1kb of data.
  • 2nd write_firmware writes 2.5kb of data
  • etc

Advantages:

  • Doesn't complicate public API
  • Keeps updates fast. If your partition is 1MB but the firmware is 200kb, you only end up erasing 200kb (rounded up to erase size), instead of the full 1MB.

@badrbouslikhin
Copy link
Contributor Author

@Dirbaio I like your suggestion! Let me give it a shot.

@badrbouslikhin
Copy link
Contributor Author

badrbouslikhin commented Feb 12, 2024

I've implemented the mechanism suggested by @Dirbaio.
I've added a few tests make sure it works if various configurations and tested the changes on an STM32H7 + USB DFU (only the blocking implementation not async, although tests pass).

Let me know what you think.

@badrbouslikhin badrbouslikhin changed the title feat: change usb dfu chunks write mechanism feat(boot): enhance firmware write functionality Feb 12, 2024
Copy link
Member

@lulf lulf left a comment

Choose a reason for hiding this comment

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

Overall looks good, left one comment. And thanks for adding tests!

Copy link
Member

@lulf lulf left a comment

Choose a reason for hiding this comment

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

Looks good! I will give the async version a spin before merging.

Copy link
Member

@lulf lulf left a comment

Choose a reason for hiding this comment

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

Confirmed it works on nrf with async, thanks for the contribution! 🚀

@lulf lulf added this pull request to the merge queue Feb 14, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Feb 14, 2024
@Dirbaio Dirbaio merged commit 63d592c into embassy-rs:main Feb 14, 2024
6 checks passed
@badrbouslikhin badrbouslikhin deleted the usb-dfu-erase-then-write branch February 14, 2024 15:38
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.

3 participants