[hmac,rtl/dv] DV synchronization and error handling fixes #23383
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
gdessouky
requested review from
rswarbrick,
andreaskurth and
martin-velay
and removed request for
a team and
rswarbrick
gdessouky
force-pushed
the
hmac_dv_sychronization
branch
from
7db9e77
to
ec90656
Compare
martin-velay
approved these changes
May 30, 2024
| cfg.clk_rst_vif.wait_clks((msg.size() % 4 || !legal_seq_c.constraint_mode()) ? | ||
| HMAC_KEY_PROCESS_CYCLES * 2 : | ||
| $urandom_range(0, HMAC_KEY_PROCESS_CYCLES * 2)); | ||
| key_process_cycles * 2 : |
There was a problem hiding this comment.
Could you add a comment to explain why this factor 2?
There was a problem hiding this comment.
I'm still not entirely sure to be honest, I went down memory lane and git blame takes it back to this 1d1febc. It is a delay to avoid checking fifo_empty interrupt since it is hard to synchronize with the scoreboard and it is doubled since the prim_packer may also be holding more data. I haven't figured it out, I will rework the existing comment and add a TODO, so we can revisit it post M4.
andreaskurth
approved these changes
May 30, 2024
|
@gdessouky: Did you run this through a regression (all block-level tests of HMAC, potentially with reduced reseed factor)? If so, could you please post the results? |
gdessouky
force-pushed
the
hmac_dv_sychronization
branch
from
ec90656
to
01c2bc1
Compare
|
I've pushed now with minor fixes (thanks @martin-velay), and have run earlier the block-level regression with the results below, so it looks good to me (the V3 failing test was always failing).
|
This synchronizes the scoreboard checks for digest sizes 384 and 512, and implements a fix with the loading from the message FIFO. This also implements DV fixes for the error handling checks when simultaneous errors are triggered. Signed-off-by: Ghada Dessouky <[email protected]>
gdessouky
force-pushed
the
hmac_dv_sychronization
branch
from
01c2bc1
to
ace5b41
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This synchronizes the scoreboard checks for digest sizes 384 and 512, and once merged would close #22578. This also implements a minor RTL fix with the loading from the message FIFO for SHA-2 384/512; this has no impact on functionality and digests computation, but aligns reading the words from the FIFO (and thus depth and status throughout) with what would be expected. This also implements additional DV fixes for the error handling checks when simultaneous errors are triggered.