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

[silicon_creator,hmac] ensure process cmd is only invoked once #24030

Merged

Conversation

timothytrippel
Copy link
Contributor

This updates the HMAC silicon_creator driver to separate out the hmac_sha256_process() call from the hmac_sha256_final...() calls to avoid sending multiple process commands to the hardware, and thus triggering an SVA in sim (see #23954). While the consensus in #24022 was that the hardware can handle multiple process commands, this path is less tested. Hence, we make this update to minimize risk in the ROM code that drives the HMAC SHA2 engine.

This updates the ROM, and fixes #23954. A new rom release candidate will tagged after this merges.

@timothytrippel timothytrippel requested a review from a team as a code owner July 16, 2024 16:49
@timothytrippel timothytrippel removed the request for review from a team July 16, 2024 16:49
This updates the HMAC silicon_creator driver to separate out the
`hmac_sha256_process()` call from the `hmac_sha256_final...()` calls to
avoid sending multiple process commands to the hardware, and thus
triggering an SVA in sim (see lowRISC#23954). While the consensus in lowRISC#24022 was
that the hardware can handle multiple `process` commands, this path is
less tested. Hence, we make this update to minimize risk in the ROM code
that drives the HMAC SHA2 engine.

This updates the ROM, and fixes lowRISC#23954. A new rom release candidate will
tagged after this merges.

Signed-off-by: Tim Trippel <ttrippel@google.com>
@moidx
Copy link
Contributor

moidx commented Jul 16, 2024

CHANGE AUTHORIZED: sw/device/silicon_creator/rom/e2e/release/rom_e2e_self_hash_test.c

Copy link
Contributor

@vogelpi vogelpi left a comment

Choose a reason for hiding this comment

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

Thanks a lot @timothytrippel , LGTM!

@vogelpi
Copy link
Contributor

vogelpi commented Jul 16, 2024

CHANGE AUTHORIZED: sw/device/silicon_creator/rom/e2e/release/rom_e2e_self_hash_test.c

@timothytrippel timothytrippel merged commit e907b95 into lowRISC:master Jul 16, 2024
32 checks passed
@timothytrippel timothytrippel deleted the fix-hmac-double-process branch July 16, 2024 21:39
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.

[test-triage] rom_e2e_self_hash
3 participants