[rom_ext, ownership] Fix the flash region configuration #26077
+612
−110
Conversation
This file contains 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
cfrantz
force-pushed
the
a1-rom_ext-protect
branch
from
dfef607 to
d34904e
Compare
jettr
reviewed
Jan 31, 2025
I did not write correct test cases that would check a flash configuration similar to the owner config already deployed in some skus. This oversight could cause ownership initialization to fail on devies with such a configuration. 1. Permit a maximum of 3 regions per side in the flash configuration. The regions must be fully within the bounds of SlotA or SlotB and may not overlap the ROM_EXT region. 2. Apply the region configuration in order. Previously, there was a correspondence between the index of the region in the owner config and the MP_REGION register that it would land in, but this makes ownership transfers prone to configuration clashes. 3. Flash configuration is done in two passes to configure each side independently (the reason for this is to allow next_owner's flash config to apply to the non-primary side during ownership transfer). The flash_apply function now takex an index parameter to manage the desination MP_REGION register between passes. 4. Create a unittest case with a flash configuration similar to the already-deployed configuration. Include the ROM_EXT, application, filesystem and reserved segments. 5. Update the existing test cases to accomodate the new configuration scheme (e.g. applying in order rather than by index). Signed-off-by: Chris Frantz <[email protected]>
cfrantz
force-pushed
the
a1-rom_ext-protect
branch
from
d34904e to
0c2d6ba
Compare
jettr
approved these changes
Jan 31, 2025
| } | ||
| } | ||
| return kErrorOk; | ||
| } | ||
|
|
||
| rom_error_t owner_block_flash_apply(const owner_flash_config_t *flash, | ||
| uint32_t config_side, | ||
| uint32_t owner_lockdown) { | ||
| uint32_t owner_lockdown, | ||
| uint32_t *mp_index) { |
There was a problem hiding this comment.
Can you add a check to ensure that mp_index isn't null?
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.
I did not write correct test cases that would check a flash configuration similar to the owner config already deployed in some skus. This oversight could cause ownership initialization to fail on devies with such a configuration.
first_passparameter to theflash_applyfunction to allow managing the internal state.Fixes #25435.