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

[rom_ext] Implement Integrator Specific FW Binding logic. #25147

Open
wants to merge 1 commit into
base: earlgrey_es_sival
Choose a base branch
from

Conversation

moidx
Copy link
Contributor

@moidx moidx commented Nov 14, 2024

Implements Integrator Specific FW Binding functionality to provide anti-rollback and product binding functionality to application firmware.

The following changes are included:

  1. Add flash_ctrl helper function to generate the info flash page configuration based on the ownership ISFB config.
  2. Implement isfb logic as a separate module to simplify ROM_EXT integration.
  3. Add unittest cases to verify anti-rollback and product association functionality.
  4. Add minimum level of hardening against FI.

The ROM_EXT integration and e2e test cases will be implemented in a follow up commit.

@moidx moidx requested a review from a team as a code owner November 14, 2024 19:09
@moidx moidx removed the request for review from a team November 14, 2024 19:09
Copy link

@vbendeb vbendeb left a comment

Choose a reason for hiding this comment

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

some questions, maybe I'm missing something.

It would be good to have a one pager explaining the design...

sw/device/silicon_creator/lib/ownership/isfb.h Outdated Show resolved Hide resolved
sw/device/silicon_creator/lib/ownership/isfb.c Outdated Show resolved Hide resolved
sw/device/silicon_creator/lib/error.h Show resolved Hide resolved
sw/device/silicon_creator/lib/ownership/isfb.c Outdated Show resolved Hide resolved
sw/device/silicon_creator/lib/ownership/isfb.c Outdated Show resolved Hide resolved
sw/device/silicon_creator/lib/ownership/isfb.c Outdated Show resolved Hide resolved
sw/device/silicon_creator/lib/ownership/isfb.c Outdated Show resolved Hide resolved
sw/device/silicon_creator/lib/ownership/isfb.c Outdated Show resolved Hide resolved
sw/device/silicon_creator/lib/ownership/isfb.c Outdated Show resolved Hide resolved
sw/device/silicon_creator/lib/error.h Outdated Show resolved Hide resolved
@moidx moidx force-pushed the isfb-implementation branch 3 times, most recently from 2decbca to ea6cfa7 Compare November 15, 2024 02:20
sw/device/silicon_creator/lib/manifest.h Outdated Show resolved Hide resolved
uint32_t *checks_performed_count) {
*checks_performed_count = UINT32_MAX;
if (ext->header.name != kManifestExtIdIsfb ||
(hardened_bool_t)owner_config->isfb == kHardenedBoolFalse) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I tried, but I cannot find the definition of owner_isfb_config_t any where. Can you point it out to me please?

Also why do we cast owner_config->isfb to hardened_bool_t then later to owner_isfb_config_t? That doesn't seem valid.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You can find the data structures in the ownership datatypes.h header in the earlgrey_es_sival branch. This will be merged to earlgrey_1.0.0 once the feature is complete.

Also take a look at owner_config_default() inside owner_block.c. Owner config entries are initialized to kHardenedBoolFalse to flag when an owner TLV entry is not present in the ownership blob.

sw/device/silicon_creator/lib/ownership/isfb.c Outdated Show resolved Hide resolved
Implements Integrator Specific FW Binding (ISFB) functionality to provide
anti-rollback and product binding functionality to application firmware.

The following changes are included:

1. Add flash_ctrl helper function to generate the info flash page
   configuration based on the ownership ISFB config.
2. Implement isfb logic as a separate module to simplify ROM_EXT
   integration.
3. Add unittest cases to verify anti-rollback and product association
   functionality.
4. Add minimum level of hardening against FI.

The ROM_EXT integration and e2e test cases will be implemented in a
follow up commit.

Signed-off-by: Miguel Osorio <[email protected]>
Copy link
Contributor

@jettr jettr left a comment

Choose a reason for hiding this comment

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

LGTM, the only case to think about is the ECC failure recovery process

"Strike mask size mismatch");
static_assert(sizeof(data) >= kFlashPageBytes, "Unexpected data size");

HARDENED_RETURN_IF_ERROR(flash_ctrl_info_read(&isfb_info_page, /*offset=*/0,
Copy link
Contributor

Choose a reason for hiding this comment

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

What happens within flash_ctrl_info_read is there is an ECC failure? Does the chip reset or does it return an error?

How would we recovery this chip in a lab if that every occurred? We could make an owner firmware that has no restrictions?

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