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

[sw,multitop] port kmac_{app_rom,mode_cshake,mode_kmac}_test to devicetables #26243

Merged
merged 3 commits into from
Feb 13, 2025

Conversation

AlexJones0
Copy link
Contributor

@AlexJones0 AlexJones0 commented Feb 11, 2025

Fix #26216
Fix #26218
Fix #26219
Fix #26220

This PR ports 3 KMAC tests to use the devicetables API so that they no longer depend on Earlgrey-specific constants. All three tests remain passing on Earlgrey on FPGA (fpga_cw310_rom_with_fake_keys), and will compile for Darjeeling using e.g.

bazel build //sw/device/tests:kmac_app_rom_test_sim_dv --//hw/top=darjeeling 

It might be worth updating kmac_app_rom_test in the future so that each exec_env provides a way to get the hashfile of its ROM, so the information here is not hard-coded. But that is out of scope for this PR.

Edit: I've also now tested chip_sw_kmac_mode_kmac_jitter_en is passing on Earlgrey via DVsim so I'm marking this as closing that issue as well.

@AlexJones0 AlexJones0 requested a review from a team as a code owner February 11, 2025 13:51
@AlexJones0 AlexJones0 requested review from alees24 and removed request for a team February 11, 2025 13:51
@AlexJones0 AlexJones0 requested review from nbdd0121, jwnrt and pamaury and removed request for alees24 February 11, 2025 14:02
("//sw/device/silicon_creator/rom:mask_rom_fpga_cw310_hashfile", "fpga_cw310_sival"),
("//sw/device/silicon_creator/rom:mask_rom_fpga_cw340_hashfile", "fpga_cw340_sival"),
("//sw/device/lib/testing/test_rom:test_rom_sim_dv_hashfile", "sim_dv"),
(
Copy link
Contributor

Choose a reason for hiding this comment

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

Comment for the future: we should find a way to make the execution environment provide the hashmap to avoid this manual list. This requires some bazel hacking, or it could just be done as part of the build system revamp for multitop that we are going to do.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll add this as a TODO comment to the code I think as well

Copy link
Contributor

@pamaury pamaury left a comment

Choose a reason for hiding this comment

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

Looks great :)

@nbdd0121 nbdd0121 merged commit 458549f into lowRISC:master Feb 13, 2025
42 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
3 participants