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

Compute OTBN checksum over LE values. #94

Open
wants to merge 1 commit into
base: dev/ot-earlgrey-1.0.0-updates
Choose a base branch
from

Conversation

AlexJones0
Copy link

@AlexJones0 AlexJones0 commented Dec 18, 2024

This PR fixes the OTBN checksum computation. The OTBN checksum is currently being computed over big-endian order 48-bit values {imem, addr, value}, but the CRC32 checksum of an OTBN application is actually computed over the values in little-endian order. This PR replaces the existing Big Endian logic with Little Endian so that the computed checksum is correct.

This change has been tested to make the following tests (which were previously failing) pass: ecdsa_p256_verify_functest_hardcoded, entropy_src_ast_rng_req_test, rnd_functest, rsa_3072_verify_functest_hardcoded and rv_core_ibex_rnd_test. All tests were built from master on OpenTitan.

The checksum computation is currently being computed over big-endian
order 48-bit values {imem, addr, value}, but the CRC32 checksum of an
OTBN application is actually computed over the values in little-endian
order. Replace the existing BE logic with little endian so that the
computed checksum is correct.
@AlexJones0
Copy link
Author

@rivos-eblot I think this change also needs to ported to ot-darjeeling-9.1.0 as well?

@AlexJones0 AlexJones0 changed the title [ot] hw/opentitan: ot_otbn: compute checksum over LE values. compute OTBN checksum over LE values. Dec 18, 2024
@AlexJones0 AlexJones0 changed the title compute OTBN checksum over LE values. Compute OTBN checksum over LE values. Dec 18, 2024
Copy link

@rivos-eblot rivos-eblot left a comment

Choose a reason for hiding this comment

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

LGTM

@rivos-eblot
Copy link

@rivos-eblot I think this change also needs to ported to ot-darjeeling-9.1.0 as well?

Probably. It's weird we have not experienced some issues with the OTBN...
I'll check this next .. year (OoO next week).

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