-
Notifications
You must be signed in to change notification settings - Fork 924
[top, csrng, entropy_src] Remove CS AES halt request interface #28842
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
Conversation
c3a0ac9 to
8d3e178
Compare
ec5e4ef to
7a20c20
Compare
glaserf
left a comment
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.
Thanks for this PR which really removes some further complexity from the entropy complex. This looks good to me, I only have some very minor comments.
I also checked out your branch to search the repo for any leftovers of the AES halt interface. I only found the following (link to master such that it won't die if you rebase your branch):
| Verify that when AES_HALT is set during a generate command that no request is sent to the AES block. |
7a20c20 to
077d3a9
Compare
|
Thanks for your reviews @glaserf and @phani-lowrisc , I've now addressed your comments. |
This interface adds considerable design and verification complexity without substantial gain: The two crypto primitives serialized by this interface (unmasked AES core inside CSRNG, unmasked SHA3 inside ENTROPY_SRC) amount to less than 40 kGE together. Much more logic than this got removed through the CSRNG restructuring effort. At the same time, the interface is the main source of back pressure inside the ENTROPY_SRC thereby introducing the need for additional logic to aborb this back pressure. For this reason, the interface was anyway disabled for Darjeeling which uses a high-rate noise source. This commit removes the interface completely from the top levels and the two involved hardware block IPs CSRNG and ENTROPY_SRC in order to reduce complexity. This resolves lowRISC#28819. Signed-off-by: Pirmin Vogel <vogelpi@lowrisc.org>
This is a follow-up of 357d41a . Signed-off-by: Pirmin Vogel <vogelpi@lowrisc.org>
077d3a9 to
027ef83
Compare
|
@vogelpi Does this have impact on the size of the distribution FIFO of Darjeeling? Giving that it's entropy source runs at a different rate? |
No it doesn't. Because Darjeeling did disable this interface (with a parameter) for the exact same reason. When disabling the parameter, I lowered the depth of the FIFO from 26 to 11 entries. |
|
CHANGE AUTHORIZED: hw/ip/csrng/data/csrng.hjson This PR reduces overall complexity by removing a feature which has no clear benefit. It has been discussed in the Silicon and Integrated WG meeting on 2025-11-25. This is fine. |
|
CHANGE AUTHORIZED: hw/ip/csrng/data/csrng.hjson This PR reduces overall complexity by removing a feature which has no clear benefit. It has been discussed in the Silicon and Integrated WG meeting on 2025-11-25. This is fine. |
This interface adds considerable design and verification complexity without substantial gain: The two crypto primitives serialized by this interface (unmasked AES core inside CSRNG, unmasked SHA3 inside ENTROPY_SRC) amount to less than 40 kGE together. Much more logic than this got removed through the CSRNG restructuring effort.
At the same time, the interface is the main source of back pressure inside the ENTROPY_SRC thereby introducing the need for additional logic to aborb this back pressure. For this reason, the interface was anyway disabled for Darjeeling which uses a high-rate noise source.
This commit removes the interface completely from the top levels and the two involved hardware block IPs CSRNG and ENTROPY_SRC.
This resolves #28819.
This PR is based on top of #28804 and will be rebased once the PR is merged. Please only review the last commit.