-
Notifications
You must be signed in to change notification settings - Fork 547
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
[Zk ext] V.1.0.0 Scalar cryptography extension implementation for the Ibex core #1480
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remaining comments which cannot be posted as a review comment to avoid GitHub Rate Limit
verible-verilog-lint
rtl/ibex_decoder.sv|1040 col 41| Remove trailing spaces. [Style: trailing-spaces] [no-trailing-spaces]
rtl/ibex_decoder.sv|1131 col 101| Line length exceeds max: 100; is: 107 [Style: line-length] [line-length]
rtl/ibex_decoder.sv|1132 col 101| Line length exceeds max: 100; is: 107 [Style: line-length] [line-length]
rtl/ibex_decoder.sv|1133 col 101| Line length exceeds max: 100; is: 107 [Style: line-length] [line-length]
rtl/ibex_decoder.sv|1134 col 101| Line length exceeds max: 100; is: 107 [Style: line-length] [line-length]
rtl/ibex_decoder.sv|1135 col 101| Line length exceeds max: 100; is: 107 [Style: line-length] [line-length]
rtl/ibex_decoder.sv|1136 col 101| Line length exceeds max: 100; is: 107 [Style: line-length] [line-length]
rtl/ibex_decoder.sv|1139 col 101| Line length exceeds max: 100; is: 103 [Style: line-length] [line-length]
rtl/ibex_decoder.sv|1140 col 101| Line length exceeds max: 100; is: 103 [Style: line-length] [line-length]
rtl/ibex_decoder.sv|1141 col 101| Line length exceeds max: 100; is: 103 [Style: line-length] [line-length]
rtl/ibex_decoder.sv|1142 col 101| Line length exceeds max: 100; is: 103 [Style: line-length] [line-length]
rtl/ibex_decoder.sv|1143 col 101| Line length exceeds max: 100; is: 104 [Style: line-length] [line-length]
rtl/ibex_decoder.sv|1144 col 101| Line length exceeds max: 100; is: 104 [Style: line-length] [line-length]
rtl/ibex_decoder.sv|1145 col 101| Line length exceeds max: 100; is: 104 [Style: line-length] [line-length]
rtl/ibex_decoder.sv|1146 col 101| Line length exceeds max: 100; is: 104 [Style: line-length] [line-length]
rtl/ibex_decoder.sv|1147 col 101| Line length exceeds max: 100; is: 103 [Style: line-length] [line-length]
rtl/ibex_decoder.sv|1148 col 101| Line length exceeds max: 100; is: 103 [Style: line-length] [line-length]
rtl/ibex_decoder.sv|1149 col 101| Line length exceeds max: 100; is: 103 [Style: line-length] [line-length]
rtl/ibex_decoder.sv|1150 col 101| Line length exceeds max: 100; is: 103 [Style: line-length] [line-length]
rtl/ibex_decoder.sv|1151 col 101| Line length exceeds max: 100; is: 104 [Style: line-length] [line-length]
rtl/ibex_decoder.sv|1152 col 101| Line length exceeds max: 100; is: 104 [Style: line-length] [line-length]
rtl/ibex_decoder.sv|1153 col 101| Line length exceeds max: 100; is: 104 [Style: line-length] [line-length]
rtl/ibex_decoder.sv|1154 col 101| Line length exceeds max: 100; is: 104 [Style: line-length] [line-length]
rtl/ibex_zk.sv|153 col 26| Unpacked dimension range must be declared in big-endian ([0:N-1]) order. Declare zero-based big-endian unpacked dimensions sized as [N]. [Style: unpacked-ordering] [unpacked-dimensions-range-ordering]
rtl/ibex_zk.sv|168 col 26| Unpacked dimension range must be declared in big-endian ([0:N-1]) order. Declare zero-based big-endian unpacked dimensions sized as [N]. [Style: unpacked-ordering] [unpacked-dimensions-range-ordering]
rtl/ibex_zk.sv|194 col 32| Use named ports for module instantiation with more than one port [Style: module-instantiation] [module-port]
rtl/ibex_zk.sv|195 col 32| Use named ports for module instantiation with more than one port [Style: module-instantiation] [module-port]
rtl/ibex_zk.sv|196 col 32| Use named ports for module instantiation with more than one port [Style: module-instantiation] [module-port]
rtl/ibex_zk.sv|222 col 20| All generate block labels must start with g_ or gen_ [Style: generate-constructs] [generate-label-prefix]
rtl/ibex_zk.sv|304 col 101| Line length exceeds max: 100; is: 102 [Style: line-length] [line-length]
rtl/ibex_zk.sv|305 col 101| Line length exceeds max: 100; is: 102 [Style: line-length] [line-length]
rtl/ibex_zk.sv|306 col 101| Line length exceeds max: 100; is: 102 [Style: line-length] [line-length]
rtl/ibex_zk.sv|307 col 101| Line length exceeds max: 100; is: 102 [Style: line-length] [line-length]
rtl/ibex_zk.sv|340 col 20| All generate block labels must start with g_ or gen_ [Style: generate-constructs] [generate-label-prefix]
rtl/ibex_zk.sv|404 col 20| All generate block labels must start with g_ or gen_ [Style: generate-constructs] [generate-label-prefix]
Finally, the PR passes the CI checking. Please do the review and let me know if anything else needs to improve. |
Thanks for the contribution @phthinh. I believe we will be keen to merge some form of this but as you can imagine new ISA extensions require careful consideration. There's quite a lot going on at lowRISC right now so we might not manage a full review before Christmas but rest assured this has not been forgotten! In the meantime I would be interested to hear a bit more about the verification. You said you used simple system, do you have a set of test programs? They may also be useful to add. Eventually we'd want to test it using the RISC-DV instruction generator our main DV uses but some directed tests would suffice in the mean time. |
Thanks for your response and interest in the PR @GregAC. I understand that many active things are going on at lowRISC. I will keep following up with the PR unit when it can be good for merging to lowRISC. I look forward to getting your review after Christmas. In the meantime, I will keep my developing branch up-to-date with new merged PRs at LowRISC, which allows merging Zk ext PR easier later on. Regarding the verification, I successfully tested the Zk ext with my own test program and also with the riscv-compliance [1] test suites. I think the test compliance is more formal but the K extension test of the current the riscv-compliance does not be updated for the ratified v.1.0.0 Scalar cryptography extension spec. I did some small changes on this to support the ratified spec. (more specifically, xperm.b -> xperm8; xperm.n -> xperm4) and to run K extension test for the ibex target. For the target for the compliance test, I built the ibex system with Apart from that, I will look at how to use the RISC-DV instruction generator to verify the Zk extension. [1] https://github.com/riscv-non-isa/riscv-arch-test |
Hi @GregAC, for the verification of the scalar crypto extension, we have created a public Repo consisting of our testing works, including instructions test programs and the RISC-V-compliance test suites, for crypto-supporting ibex core. You can find it here https://github.com/scarv/crypto-ibex-test. |
This PR adds an implementation of the ratified v.1.0.0 Scalar cryptography extension spec. Particularly, this PR adds a set of instructions specified for Zkn (supporting NIST algorithm suite) and Zks (supporting ShangMi Algorithm Suite). The implementation can be configured by RV32K parameter.
The implementation was successfully verified with the ibex_simple_system on Verilator with 2 new experimental configurations: experimental-maxperf-pmp-zkn for Zkn and experimental-maxperf-pmp-zks for Zks.
I am happy to have further discussions and/or sanitizing works to merge this PR to the Ibex core.