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

Zk(Zbk, Zkn, Zks)/Zb: Scalar Cryptography/Bitmanip Extension #2950

Merged
merged 104 commits into from
Jan 18, 2023

Conversation

ZenithalHourlyRate
Copy link
Contributor

Related PR: #2906

Type of change: other enhancement

Impact: API addition (no impact on existing code)

Development Phase: implementation (needs cleanup)

Note
This has borrowed a lot of code (mainly decode logic) from #2906. The decode logic has been rebased on the master due to the merging of H extension.

Some of the logics are also inspired by #2906, thanks!

While #2906 has different implementations for rv32 and rv64, this PR merges them together.

As for Zkr, as #2906 is just a stub, I think a Chisel stub is fairly easy to implement. In the meantime, the CSR logics should be PRed by the owner of #2906 instead of me. Maybe users can just cherry-pick that commit and implement their own Zkr?

There are some notes on Zkt (like ROM optimization), but it is not verified. As this is a single-cycle implementation, Zkt ought to be satisfied (not verified though).

A lot of helpers are temply embedded here, and should be cleaned up. barrel.rightRotate is borrowed from here

This has passed riscv-arch-test on rv64i_m/K and rv32i_m/K when verilated to an emulator

Cc @sequencer @phthinh @ben-marshall

Also thanks @liweiwei90 for his help on riscv-arch-test

TODO

  1. Clean up of the code on mergeing common datapaths and bad handling of some logics (like multiple levels of Mux of io.out)
  2. Clean up of some helpers (e.g. move them into zk/utils.scala) / get helpers upstreamed to Chisel
  3. Add CI tests on Zk for rocket-chip (How?)
  4. Test against FPGA

Discussion

  1. Should we merge zbk into ALU as they share some logics (like ~io.rs2 and logical operation gates for andn/orn/xnor)
  2. Should we embed SBox as ROM here or just write real-time computing logics, as Zk: Scalar Cryptography Extension #2906 and Xiangshan have done

Release Notes

This pull request is to add the hardware implementation to support the Zbk, Zkn, Zks of the cryptographic extensions Zk on the Rocket-chip. This supports the Scalar Instructions specified in the v1.0.1 version. The implementation can work with 32-bit and 64-bit Architectures. The plug-in supports can be enabled by using the WithZBK, WithZKN, WithZKS configuration options.

Copy link
Contributor

@jerryz123 jerryz123 left a comment

Choose a reason for hiding this comment

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

I trust the circuit implementations, but there should be some refactors

@jerryz123
Copy link
Contributor

I will clone this branch myself to check some things.

@ZenithalHourlyRate
Copy link
Contributor Author

Should squash&merge for this PR because the history is too long..

@jerryz123
Copy link
Contributor

Yes we should squash+merge. This PR looks fine for me now.

@jerryz123
Copy link
Contributor

Once CI is done I will squash+merge

@ZenithalHourlyRate
Copy link
Contributor Author

Once CI is done I will squash+merge

CI passed.

@jerryz123 jerryz123 enabled auto-merge (squash) January 18, 2023 09:16
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.

6 participants