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

[tlul,shim] Expand pre_dv to handle arbitrary test cases #9

Merged
merged 1 commit into from
Nov 21, 2024

Conversation

andrea-caforio
Copy link
Collaborator

This commit generalizes the pre_dv testbench to arbitrary test cases and adds an array of tests covering both AES-GCM and its modes of operation. The process of adding/modify tests is now streamlined reducing the feedback loop.

@andrea-caforio andrea-caforio marked this pull request as ready for review November 18, 2024 15:06
Copy link
Owner

@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 @andrea-caforio for your work on this, it's a big step and nice work!

I did a first review pass and have some comments. Some more background on the interaction of the c_dpi module and the TB would be appreciated. Thanks!

hw/ip/aes/pre_dv/aes_tlul_shim_tb/data/gcm_k128_a0_m0.svh Outdated Show resolved Hide resolved
Comment on lines 687 to 690
write_request(AES_DATA_IN_0_OFFSET, 32'hf8aa56ed), \
write_request(AES_DATA_IN_1_OFFSET, 32'h04672da7), \
write_request(AES_DATA_IN_2_OFFSET, 32'h2892db9f), \
write_request(AES_DATA_IN_3_OFFSET, 32'h2213baed), \
Copy link
Owner

Choose a reason for hiding this comment

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

Please note: There is a commit in my PR which will require updating this value.

For more background: the initial unmasked implementation would output the plain GHASH state. In contrast, the masked implementation always outputs the state XOR S. Upon restoring, the value provided will be again "remasked" internally using the new sharing of S. To align the masked and unmasked implementation (to simplify software and DV), I made the unmasked implementation also outputting state XOR S. S is then automatically subtracted again after restoring the state.

Copy link
Owner

Choose a reason for hiding this comment

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

This has now happened, thank you!

hw/ip/aes/pre_dv/aes_tlul_shim_tb/rtl/aes_tlul_shim_tb.sv Outdated Show resolved Hide resolved
key_length: AES_128, \
key: 256'h000000000000000000000000000000003c4fcf098815f7aba6d2ae2816157e2b, \
iv: '0, \
msg: 512'hd45d7204712023823fade8275e780c7b880603ede3001b8823ce8e597fcdb143afbafd965a8985e79d69b90385d5d3f597ef6624f3ca9ea860367a0db47bd73a,\
Copy link
Owner

Choose a reason for hiding this comment

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

In an ideal world, this data could be extracted from the existing .c/.h files in the repo but I think it's okay to leave this as is.

@andrea-caforio andrea-caforio force-pushed the aes-tlul-shim-nist branch 2 times, most recently from 1fda49d to e683124 Compare November 19, 2024 15:59
This commit generalizes the `pre_dv` testbench to arbitrary test cases
and adds an array of tests covering both AES-GCM and its modes of
operation. The process of adding/modify tests is now streamlined
reducing the feedback loop.

Signed-off-by: Andrea Caforio <[email protected]>
Co-authored-by: Pirmin Vogel <[email protected]>
Comment on lines +5 to +8
// This module serves as the interface to the `c_dpi` API by instantiating the function call as a
// combinatorial block. The `c_dpi` output (data and tag) is flopped and the 32 least significant
// bits of data and tag become visible at the module's output ports in the next clock cycle. The
// flopped data and tag can be rotated by 32 positions to the right to make other bits visible.
Copy link
Owner

Choose a reason for hiding this comment

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

Thanks @andrea-caforio for providing these comments, it makes the design a lot easier to understand :-)

Copy link
Owner

@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 for addressing all comments @andrea-caforio , this is great!

@vogelpi vogelpi merged commit 8a27d11 into vogelpi:aes-gcm-review Nov 21, 2024
7 of 9 checks passed
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