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.

Sign up for GitHub

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

Jump to bottom

Libcaliptra fixes for checksum on uninitialized buffers and missing DPE structs #1620

Merged
merged 3 commits into from
Aug 26, 2024
Merged

Libcaliptra fixes for checksum on uninitialized buffers and missing DPE structs #1620

merged 3 commits into from
Aug 26, 2024

Conversation

nquarton
Copy link
Contributor

No description provided.

@nquarton nquarton changed the title Adding missing libcalitpra dpe command structs Adding missing libcaliptra dpe command structs Jul 25, 2024
@nquarton nquarton force-pushed the libcaliptra_dpe_cmd_resp_unions_fix branch 3 times, most recently from f110b2a to 80d4eaf Compare July 26, 2024 00:03
ekoohest
ekoohest suggested changes Jul 26, 2024
View reviewed changes
ekoohest
ekoohest suggested changes Jul 26, 2024
View reviewed changes
@nquarton nquarton force-pushed the libcaliptra_dpe_cmd_resp_unions_fix branch from 80d4eaf to 799bdbd Compare July 26, 2024 18:20
@nquarton nquarton marked this pull request as ready for review July 26, 2024 18:44
jhand2
jhand2 previously approved these changes Jul 26, 2024
View reviewed changes
jlmahowa-amd
jlmahowa-amd previously approved these changes Jul 26, 2024
View reviewed changes
@nquarton nquarton marked this pull request as draft July 26, 2024 23:34
@nquarton
Copy link
Contributor Author

Changing back to draft since I have a related issue I am looking into

@nquarton nquarton dismissed stale reviews from jlmahowa-amd and jhand2 via fecc0f3 July 27, 2024 00:02
@nquarton nquarton force-pushed the libcaliptra_dpe_cmd_resp_unions_fix branch 2 times, most recently from fecc0f3 to 848976e Compare July 29, 2024 22:15
@nquarton nquarton changed the title Adding missing libcaliptra dpe command structs Libcaliptra fixes for checksum on uninitialized buffers and missing DPE structs Jul 29, 2024
@nquarton
Copy link
Contributor Author

This updates and adds a few DPE structs as well as fixes 2 bugs exposed by uninitialized command and response buffers:

  1. The checksum field in the command request may be populated before calculating the checksum. This would get overwritten, but the new checksum value would be incorrect. This is resolved by simply setting the checksum field to 0 before calculating.
  2. The response checksum was calculated over the whole response buffer instead of just the received data. Garbage in the untouched part of the buffer would change the checksum. The actual number of bytes read is recorded and propagated to the checksum calculation now.

@nquarton nquarton force-pushed the libcaliptra_dpe_cmd_resp_unions_fix branch from dd9b124 to 852ff19 Compare August 1, 2024 15:09
@nquarton nquarton marked this pull request as ready for review August 1, 2024 15:30
jlmahowa-amd
jlmahowa-amd previously approved these changes Aug 2, 2024
View reviewed changes
@nquarton nquarton force-pushed the libcaliptra_dpe_cmd_resp_unions_fix branch from 852ff19 to a6ca5be Compare August 23, 2024 18:50
jhand2
jhand2 previously approved these changes Aug 23, 2024
View reviewed changes
@nquarton @jhand2
Libcaliptra fixes
31f1209 
- Fixes to checksum calculations for uninitialized cmd and resp buffers
- Adding missing dpe command structs
@nquarton @jhand2
Updating DPE commands enum
7adba65
@nquarton @jhand2
Adding note on uninitialized response structs for libcaliptra test
8bcce59
@jhand2 jhand2 force-pushed the libcaliptra_dpe_cmd_resp_unions_fix branch from b7d65ed to 8bcce59 Compare August 23, 2024 23:12
@jhand2 jhand2 enabled auto-merge (rebase) August 23, 2024 23:12
@jhand2 jhand2 merged commit ef83a0d into chipsalliance:main Aug 26, 2024
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jhand2 jhand2 jhand2 approved these changes

@jlmahowa-amd jlmahowa-amd jlmahowa-amd approved these changes

@ekoohest ekoohest ekoohest approved these changes

@fdamato fdamato Awaiting requested review from fdamato fdamato is a code owner

@wmaroneAMD wmaroneAMD Awaiting requested review from wmaroneAMD wmaroneAMD is a code owner

@JohnTraverAmd JohnTraverAmd Awaiting requested review from JohnTraverAmd JohnTraverAmd is a code owner

@korran korran Awaiting requested review from korran korran is a code owner

@mhatrevi mhatrevi Awaiting requested review from mhatrevi mhatrevi is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@nquarton @jhand2 @jlmahowa-amd @ekoohest