Skip to content

[hw,entropy_src,rtl] Make noise source data width parametrizable #27429

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

Merged
merged 4 commits into from
Jun 19, 2025

Conversation

Razer6
Copy link
Member

@Razer6 Razer6 commented Jun 12, 2025

This PR makes the RNG data path from the entropy source to the external noise source (AST) parametrizable in the bit width. To do so, the struct defining the interface is removed and explicit signals for each field are added to allow the parametrization of the data width. The same is done for the external health test. Here only data and valid are moved out of the struct.

  • Update bit sel register
  • Instantiate bucket health test per 4-bit
  • Update AST interface
  • Parametrize the DV environment
  • Bump Version number

@Razer6 Razer6 force-pushed the entropy-src-custom-width branch 16 times, most recently from 4631e50 to 0dfbe02 Compare June 13, 2025 10:25
@Razer6 Razer6 marked this pull request as ready for review June 13, 2025 10:51
@Razer6 Razer6 requested a review from a team as a code owner June 13, 2025 10:51
@Razer6 Razer6 requested review from marnovandermaas, andreaskurth and vogelpi and removed request for a team June 13, 2025 10:51
@Razer6 Razer6 force-pushed the entropy-src-custom-width branch 3 times, most recently from 79fcd13 to 0af7442 Compare June 13, 2025 12:53
@Razer6 Razer6 force-pushed the entropy-src-custom-width branch 3 times, most recently from 209e06f to 7dc9e34 Compare June 16, 2025 09:46
@Razer6
Copy link
Member Author

Razer6 commented Jun 17, 2025

@vogelpi @andreaskurth Thanks for the review. I added your comments and also implemented a single counter for the bucket test. CI is also almost happy. Could you take another look? Regarding the version bump, I leave that as it is now. It is similar to other IP changes, so nothing critical I think.

@Razer6 Razer6 force-pushed the entropy-src-custom-width branch 2 times, most recently from bbc1b2b to 530ede4 Compare June 18, 2025 05:50
@Razer6
Copy link
Member Author

Razer6 commented Jun 18, 2025

The full regression suite of entropy_src has been run. Their pass rate is now similar to a full run on the master branh.

@Razer6 Razer6 force-pushed the entropy-src-custom-width branch 4 times, most recently from 1bf8620 to 63b9550 Compare June 18, 2025 19:20
Copy link
Contributor

@andreaskurth andreaskurth left a comment

Choose a reason for hiding this comment

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

A few small comments, but LGTM after those. Thx @Razer6!

@andreaskurth
Copy link
Contributor

andreaskurth commented Jun 19, 2025

Regarding the version bump, I leave that as it is now. It is similar to other IP changes, so nothing critical I think.

The version should be incremented before the changes. Otherwise, the changes would technically still be part of the prior version. Could you please make the version bump commit the first commit in this PR?

@Razer6 Razer6 force-pushed the entropy-src-custom-width branch from 63b9550 to 472ef5b Compare June 19, 2025 10:36
@Razer6 Razer6 requested review from vogelpi and andreaskurth June 19, 2025 10:42
@Razer6
Copy link
Member Author

Razer6 commented Jun 19, 2025

Thanks @andreaskurth I added your requests and moved the version number to the beginning of the series of commits. Let's get that merged :)

Razer6 added 3 commits June 19, 2025 12:53
Signed-off-by: Robert Schilling <rschilling@rivosinc.com>
Signed-off-by: Robert Schilling <rschilling@rivosinc.com>
There are 2**N counters necessary for this test, which does
not scale with the parametrizable RNG bit width.

Signed-off-by: Robert Schilling <rschilling@rivosinc.com>
@Razer6 Razer6 force-pushed the entropy-src-custom-width branch from 472ef5b to cf475f6 Compare June 19, 2025 10:53
Copy link
Contributor

@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.

This looks good to me, except for the version increment which needs to be moved to the correct IP core (ENTROPY_SRC) :-)

Copy link
Contributor

@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 @Razer6 !

Signed-off-by: Robert Schilling <rschilling@rivosinc.com>
@Razer6 Razer6 force-pushed the entropy-src-custom-width branch from cf475f6 to 4eba208 Compare June 19, 2025 11:07
@andreaskurth
Copy link
Contributor

andreaskurth commented Jun 19, 2025

CHANGE AUTHORIZED: hw/ip/entropy_src/data/entropy_src.hjson
CHANGE AUTHORIZED: hw/ip/entropy_src/rtl/entropy_src.sv
CHANGE AUTHORIZED: hw/ip/entropy_src/rtl/entropy_src_cntr_reg.sv
CHANGE AUTHORIZED: hw/ip/entropy_src/rtl/entropy_src_core.sv
CHANGE AUTHORIZED: hw/ip/entropy_src/rtl/entropy_src_pkg.sv
CHANGE AUTHORIZED: hw/ip/entropy_src/rtl/entropy_src_reg_pkg.sv
CHANGE AUTHORIZED: hw/ip/entropy_src/rtl/entropy_src_reg_top.sv

^ Changes to add support for configurable entropy bit widths, which lead to a bump of the major version (now 3.0.0) of the IP.

CHANGE AUTHORIZED: hw/top_earlgrey/data/top_earlgrey.hjson
CHANGE AUTHORIZED: hw/top_earlgrey/rtl/autogen/chip_earlgrey_asic.sv
CHANGE AUTHORIZED: hw/top_earlgrey/rtl/autogen/top_earlgrey.sv

^ No functional changes; structural changes to connect entropy_src ports that have been split.

Copy link
Contributor

@andreaskurth andreaskurth left a comment

Choose a reason for hiding this comment

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

Thanks @Razer6!

@vogelpi
Copy link
Contributor

vogelpi commented Jun 19, 2025

CHANGE AUTHORIZED: hw/ip/entropy_src/data/entropy_src.hjson
CHANGE AUTHORIZED: hw/ip/entropy_src/rtl/entropy_src.sv
CHANGE AUTHORIZED: hw/ip/entropy_src/rtl/entropy_src_cntr_reg.sv
CHANGE AUTHORIZED: hw/ip/entropy_src/rtl/entropy_src_core.sv
CHANGE AUTHORIZED: hw/ip/entropy_src/rtl/entropy_src_pkg.sv
CHANGE AUTHORIZED: hw/ip/entropy_src/rtl/entropy_src_reg_pkg.sv
CHANGE AUTHORIZED: hw/ip/entropy_src/rtl/entropy_src_reg_top.sv

^ Changes to add support for configurable entropy bit widths, which lead to a bump of the major version (now 3.0.0) of the IP.

CHANGE AUTHORIZED: hw/top_earlgrey/data/top_earlgrey.hjson
CHANGE AUTHORIZED: hw/top_earlgrey/rtl/autogen/chip_earlgrey_asic.sv
CHANGE AUTHORIZED: hw/top_earlgrey/rtl/autogen/top_earlgrey.sv

^ No functional changes; structural changes to connect entropy_src ports that have been split.

@vogelpi vogelpi added the CI:Rerun Rerun failed CI jobs label Jun 19, 2025
@github-actions github-actions bot removed the CI:Rerun Rerun failed CI jobs label Jun 19, 2025
@andreaskurth andreaskurth merged commit 83f35b5 into lowRISC:master Jun 19, 2025
47 of 48 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.

4 participants