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

Cherry-pick to earlgrey_es_sival: [rom_ext, ownership] Fix the flash region configuration #26126

Open
wants to merge 2 commits into
base: earlgrey_es_sival
Choose a base branch
from

Conversation

cfrantz
Copy link
Contributor

@cfrantz cfrantz commented Feb 4, 2025

This is a manual cherry-pick of #26014 and #26077 back to earlgrey_es_sival.

Ownership flash lockdown was protecting and locking all regions in the
same slot that booted the owner code.  However, the ROM_EXT and owner
code don't have to boot from the same side of the flash.

1. Disallow ownership configurations that have flash regions that
   overlap with the ROM_EXT region.  It is an error to upload such a
   configuration, but if one already exists in the chip, the
   owner-specified ROM_EXT regions are ignored in favor of the
   self-protection.
2. Always protect the ROM_EXT by using flash regions 0 and 1.
3. Update ownership tests.
4. Update SiVAL tests that used flash MP regions 0 & 1, as the ROM_EXT
   now uses them.

Fixes lowRISC#25435.

Signed-off-by: Chris Frantz <cfrantz@google.com>
(cherry picked from commit d28f9e4)
@cfrantz cfrantz requested a review from moidx February 4, 2025 17:17
@cfrantz cfrantz requested review from a team as code owners February 4, 2025 17:17
@cfrantz cfrantz requested review from timothytrippel and removed request for a team and timothytrippel February 4, 2025 17:17
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 <cfrantz@google.com>
(cherry picked from commit 9be96dc)
@cfrantz cfrantz force-pushed the rom_ext-flash-lock branch from a532468 to 19e0c14 Compare February 4, 2025 19:29
if ((hardened_bool_t)config->rescue != kHardenedBoolFalse)
return kErrorOwnershipDuplicateItem;
config->rescue = (const owner_rescue_config_t *)item;
if (check_only == kHardenedBoolFalse) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Probably a good idea to merge this PR before I merge the ISFB changes. That way I can add the check_only condition.

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.

2 participants