Fix Vorago VA416x0 IRAM shadow update (use word-aligned stores)#778
Open
dgarske wants to merge 2 commits into
Open
Fix Vorago VA416x0 IRAM shadow update (use word-aligned stores)#778dgarske wants to merge 2 commits into
dgarske wants to merge 2 commits into
Conversation
The VA416xx code RAM (IRAM at 0x00000000-0x0003FFFF) silently drops 8-bit and 16-bit stores when ROM_PROT.WREN=1 - only word-aligned 32-bit stores are honored, because the ECC machinery computes parity per 32-bit word and rejects sub-word writes (no fault is raised). wolfBoot's local memcpy()/memset() are byte-wise (ldrb/strb), so the IRAM shadow update in ext_flash_write/read/erase appeared to succeed but actually left the destination unchanged. The FRAM content was updated correctly, but after the swap wolfBoot branched into the partition at 0xB800 in IRAM where the stale image still lived, so the running app was the OLD version even though the partition header (read from FRAM) reported the new version. Add iram_write/iram_fill helpers that use 32-bit stores for the bulk of an aligned region and read-modify-write the containing word for any unaligned head/tail, and use them in ext_flash_write/read/erase in place of memcpy/memset for the shadow update. Partition addresses are sector-aligned so in practice the path is always pure 32-bit stores. Verified end-to-end on VA41630-EVK with build_test.sh update: app v1 boots, trigger sets, swap runs sector-by-sector, post-swap boot prints "Booting version: 0x2" and the running app prints v2-specific output in TESTING state. After wolfBoot_success() and a manual reset the second boot reports state CONFIRMED.
Contributor
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Fixes VA416xx IRAM shadow updates by avoiding sub-word stores that are silently dropped when ROM_PROT.WREN is enabled, ensuring the in-RAM image actually reflects FRAM writes during swap/boot flows.
Changes:
- Added
iram_write()andiram_fill()helpers that perform bulk 32-bit stores and use read-modify-write for unaligned head/tail. - Replaced
memcpy()/memset()with the new helpers inext_flash_write/read/eraseshadow update paths. - Minor whitespace cleanup.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Two follow-ups to the IRAM shadow update fix (da96f5a): 1. Mark the bulk-copy destination as volatile uint32_t * so the compiler cannot lower a 32-bit assignment into byte/halfword stores. Sub-word stores are silently dropped by the IRAM ECC, so a future codegen change could have broken the workaround. 2. Use (uintptr_t)3u / ~(uintptr_t)3u for alignment masking. On targets where uintptr_t is wider than unsigned int the bare ~3u would zero the high half of the address; harmless on Cortex-M4 today, but the helper pattern is easy to copy elsewhere. The int len type is kept to match the wolfBoot ext_flash_* HAL signature convention across all ports. Verified with arm-none-eabi-objdump: aligned bulk path emits only 32-bit str instructions (no strb/strh).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The VA416xx code RAM (IRAM at 0x00000000-0x0003FFFF) silently drops 8-bit and 16-bit stores when ROM_PROT.WREN=1 - only word-aligned 32-bit stores are honored, because the ECC machinery computes parity per 32-bit word and rejects sub-word writes (no fault is raised). wolfBoot's local memcpy()/memset() are byte-wise (ldrb/strb), so the IRAM shadow update in ext_flash_write/read/erase appeared to succeed but actually left the destination unchanged. The FRAM content was updated correctly, but after the swap wolfBoot branched into the partition at 0xB800 in IRAM where the stale image still lived, so the running app was the OLD version even though the partition header (read from FRAM) reported the new version.
Add iram_write/iram_fill helpers that use 32-bit stores for the bulk of an aligned region and read-modify-write the containing word for any unaligned head/tail, and use them in ext_flash_write/read/erase in place of memcpy/memset for the shadow update. Partition addresses are sector-aligned so in practice the path is always pure 32-bit stores.
Verified end-to-end on VA41630-EVK with build_test.sh update: app v1 boots, trigger sets, swap runs sector-by-sector, post-swap boot prints "Booting version: 0x2" and the running app prints v2-specific output in TESTING state. After wolfBoot_success() and a manual reset the second boot reports state CONFIRMED.