[csrng/rtl] Remove final three prim_fifo_sync from data path #28611

glaserf · 2025-10-30T16:33:27Z

Follow-up of #28633 and #28428, please see these PRs for details. This PR is the last part of this FIFO-removal series, removing three more FIFOs which together measure about 14kGE.

After this PR, there are in total three (was: 13) FIFOs remaining on the ctr_drbg data path. These cannot be removed due to the rather convoluted sharing of ctr_drbg_upd and the block_encrypt module. Removing any of the remaining FIFOs results in circular dependencies between these units. To get rid of these FIFOs, a general reorganization and simplification of the ctr_drbg data path is required, which will be another follow-up PR.

Please only review the last five commits. Commits before are part of #28633.

Part of #28153.

Razer6 · 2025-10-31T11:05:17Z

hw/ip/csrng/rtl/csrng_ctr_drbg_cmd.sv

+  state_e state_d, state_q;
+
+  // SEC_CM: UPDRSP.FSM.SPARSE
+  `PRIM_FLOP_SPARSE_FSM(u_state_regs, state_d, state_q, state_e, ReqIdle)


This is missing an include I think.

Thanks! Should be fixed now.

glaserf · 2025-11-02T21:41:14Z

Regression results:

CSRNG Simulation Results

Sunday November 02 2025 20:38:25 UTC

GitHub Revision: `478cf94670`

Branch: csrng_sfifo_removal

Testplan

Simulator: XCELIUM

Test Results

Stage	Name	Tests	Max Job Runtime	Simulated Time	Passing	Total	Pass Rate
V1	smoke	csrng_smoke	7.000s	26.546us	50	50	100.00 %
V1	csr_hw_reset	csrng_csr_hw_reset	2.000s	93.435us	5	5	100.00 %
V1	csr_rw	csrng_csr_rw	2.000s	100.755us	20	20	100.00 %
V1	csr_bit_bash	csrng_csr_bit_bash	15.000s	2.909ms	5	5	100.00 %
V1	csr_aliasing	csrng_csr_aliasing	3.000s	307.531us	5	5	100.00 %
V1	csr_mem_rw_with_rand_reset	csrng_csr_mem_rw_with_rand_reset	2.000s	149.229us	20	20	100.00 %
V1	regwen_csr_and_corresponding_lockable_csr	csrng_csr_rw	2.000s	100.755us	20	20	100.00 %
		csrng_csr_aliasing	3.000s	307.531us	5	5	100.00 %
V1		TOTAL			105	105	100.00 %
V2	interrupts	csrng_intr	9.000s	1.730ms	200	200	100.00 %
V2	alerts	csrng_alert	26.000s	4.494ms	500	500	100.00 %
V2	err	csrng_err	6.000s	71.456us	500	500	100.00 %
V2	cmds	csrng_cmds	6.567m	66.227ms	50	50	100.00 %
V2	life cycle	csrng_cmds	6.567m	66.227ms	50	50	100.00 %
V2	stress_all	csrng_stress_all	13.633m	160.858ms	46	50	92.00 %
V2	intr_test	csrng_intr_test	2.000s	113.204us	50	50	100.00 %
V2	alert_test	csrng_alert_test	9.000s	55.724us	50	50	100.00 %
V2	tl_d_oob_addr_access	csrng_tl_errors	5.000s	670.667us	20	20	100.00 %
V2	tl_d_illegal_access	csrng_tl_errors	5.000s	670.667us	20	20	100.00 %
V2	tl_d_outstanding_access	csrng_csr_hw_reset	2.000s	93.435us	5	5	100.00 %
		csrng_csr_rw	2.000s	100.755us	20	20	100.00 %
		csrng_csr_aliasing	3.000s	307.531us	5	5	100.00 %
		csrng_same_csr_outstanding	3.000s	318.626us	20	20	100.00 %
V2	tl_d_partial_access	csrng_csr_hw_reset	2.000s	93.435us	5	5	100.00 %
		csrng_csr_rw	2.000s	100.755us	20	20	100.00 %
		csrng_csr_aliasing	3.000s	307.531us	5	5	100.00 %
		csrng_same_csr_outstanding	3.000s	318.626us	20	20	100.00 %
V2		TOTAL			1436	1440	99.72 %
V2S	tl_intg_err	csrng_sec_cm	2.000s	253.298us	5	5	100.00 %
		csrng_tl_intg_err	16.000s	1.443ms	20	20	100.00 %
V2S	sec_cm_config_regwen	csrng_regwen	4.000s	24.546us	50	50	100.00 %
		csrng_csr_rw	2.000s	100.755us	20	20	100.00 %
V2S	sec_cm_config_mubi	csrng_alert	26.000s	4.494ms	500	500	100.00 %
V2S	sec_cm_intersig_mubi	csrng_stress_all	13.633m	160.858ms	46	50	92.00 %
V2S	sec_cm_main_sm_fsm_sparse	csrng_intr	9.000s	1.730ms	200	200	100.00 %
		csrng_err	6.000s	71.456us	500	500	100.00 %
		csrng_sec_cm	2.000s	253.298us	5	5	100.00 %
V2S	sec_cm_updrsp_fsm_sparse	csrng_intr	9.000s	1.730ms	200	200	100.00 %
		csrng_err	6.000s	71.456us	500	500	100.00 %
		csrng_sec_cm	2.000s	253.298us	5	5	100.00 %
V2S	sec_cm_update_fsm_sparse	csrng_intr	9.000s	1.730ms	200	200	100.00 %
		csrng_err	6.000s	71.456us	500	500	100.00 %
		csrng_sec_cm	2.000s	253.298us	5	5	100.00 %
V2S	sec_cm_blk_enc_fsm_sparse	csrng_intr	9.000s	1.730ms	200	200	100.00 %
		csrng_err	6.000s	71.456us	500	500	100.00 %
		csrng_sec_cm	2.000s	253.298us	5	5	100.00 %
V2S	sec_cm_outblk_fsm_sparse	csrng_intr	9.000s	1.730ms	200	200	100.00 %
		csrng_err	6.000s	71.456us	500	500	100.00 %
		csrng_sec_cm	2.000s	253.298us	5	5	100.00 %
V2S	sec_cm_gen_cmd_ctr_redun	csrng_intr	9.000s	1.730ms	200	200	100.00 %
		csrng_err	6.000s	71.456us	500	500	100.00 %
		csrng_sec_cm	2.000s	253.298us	5	5	100.00 %
V2S	sec_cm_drbg_upd_ctr_redun	csrng_intr	9.000s	1.730ms	200	200	100.00 %
		csrng_err	6.000s	71.456us	500	500	100.00 %
		csrng_sec_cm	2.000s	253.298us	5	5	100.00 %
V2S	sec_cm_drbg_gen_ctr_redun	csrng_intr	9.000s	1.730ms	200	200	100.00 %
		csrng_err	6.000s	71.456us	500	500	100.00 %
		csrng_sec_cm	2.000s	253.298us	5	5	100.00 %
V2S	sec_cm_ctrl_mubi	csrng_alert	26.000s	4.494ms	500	500	100.00 %
V2S	sec_cm_main_sm_ctr_local_esc	csrng_intr	9.000s	1.730ms	200	200	100.00 %
		csrng_err	6.000s	71.456us	500	500	100.00 %
V2S	sec_cm_constants_lc_gated	csrng_stress_all	13.633m	160.858ms	46	50	92.00 %
V2S	sec_cm_sw_genbits_bus_consistency	csrng_alert	26.000s	4.494ms	500	500	100.00 %
V2S	sec_cm_tile_link_bus_integrity	csrng_tl_intg_err	16.000s	1.443ms	20	20	100.00 %
V2S	sec_cm_aes_cipher_fsm_sparse	csrng_intr	9.000s	1.730ms	200	200	100.00 %
		csrng_err	6.000s	71.456us	500	500	100.00 %
		csrng_sec_cm	2.000s	253.298us	5	5	100.00 %
V2S	sec_cm_aes_cipher_fsm_redun	csrng_intr	9.000s	1.730ms	200	200	100.00 %
		csrng_err	6.000s	71.456us	500	500	100.00 %
V2S	sec_cm_aes_cipher_ctrl_sparse	csrng_intr	9.000s	1.730ms	200	200	100.00 %
		csrng_err	6.000s	71.456us	500	500	100.00 %
V2S	sec_cm_aes_cipher_fsm_local_esc	csrng_intr	9.000s	1.730ms	200	200	100.00 %
		csrng_err	6.000s	71.456us	500	500	100.00 %
V2S	sec_cm_aes_cipher_ctr_redun	csrng_intr	9.000s	1.730ms	200	200	100.00 %
		csrng_err	6.000s	71.456us	500	500	100.00 %
		csrng_sec_cm	2.000s	253.298us	5	5	100.00 %
V2S	sec_cm_aes_cipher_data_reg_local_esc	csrng_intr	9.000s	1.730ms	200	200	100.00 %
		csrng_err	6.000s	71.456us	500	500	100.00 %
V2S		TOTAL			75	75	100.00 %
V3	stress_all_with_rand_reset	csrng_stress_all_with_rand_reset	2.117m	16.356ms	10	10	100.00 %
V3		TOTAL			10	10	100.00 %
		TOTAL			1626	1630	99.75 %

Coverage Results

Coverage Dashboard

Score	Block	Branch	Statement	Expression	Toggle	Fsm	Assertion	CoverGroup
97.64 %	98.89 %	97.31 %	99.94 %	96.83 %	92.08 %	100.00 %	95.39 %	89.84 %

Failure Buckets

UVM_ERROR (csrng_scoreboard.sv:166) [scoreboard] Check failed intr_pins[i] === (intr_en[i] & item.d_data[i]) (* [*] vs * [*]) Interrupt_pin: EntropyReq has 4 failures:
- Test csrng_stress_all has 4 failures.
  - 19.csrng_stress_all.73674456822232813723218088839237002501054921809006041481054885486479093715971
    Line 177, in log /scratch/glaserf/opentitan/scratch/csrng_sfifo_removal/csrng-sim-xcelium/19.csrng_stress_all/latest/run.log
```
  UVM_ERROR @ 20675345664 ps: (csrng_scoreboard.sv:166) [uvm_test_top.env.scoreboard] Check failed intr_pins[i] === (intr_en[i] & item.d_data[i]) (0x1 [1] vs 0x0 [0]) Interrupt_pin: EntropyReq
  UVM_INFO @ 20675345664 ps: (uvm_report_catcher.svh:705) [UVM/REPORT/CATCHER]
  --- UVM Report catcher Summary ---
```
  - 20.csrng_stress_all.19712572081550793285328829292963901881167056310847698503663751519984229433200
    Line 147, in log /scratch/glaserf/opentitan/scratch/csrng_sfifo_removal/csrng-sim-xcelium/20.csrng_stress_all/latest/run.log
```
  UVM_ERROR @  12338085 ps: (csrng_scoreboard.sv:166) [uvm_test_top.env.scoreboard] Check failed intr_pins[i] === (intr_en[i] & item.d_data[i]) (0x1 [1] vs 0x0 [0]) Interrupt_pin: EntropyReq
  UVM_INFO @  12338085 ps: (uvm_report_catcher.svh:705) [UVM/REPORT/CATCHER]
  --- UVM Report catcher Summary ---
```
  - ... and 2 more failures.

INFO: [FlowCfg] [scratch_path]: [csrng] [/scratch/glaserf/opentitan/scratch/csrng_sfifo_removal/csrng-sim-xcelium]
ERROR: [dvsim] Errors were encountered in this run.

          [   legend    ]: [Q: queued, D: dispatched, P: passed, F: failed, K: killed, T: total]                                                                                                                                                                    
00:00:10  [    build    ]: [Q: 0, D: 0, P: 2, F: 0, K: 0, T: 2] 100%                                                                                                                                                                                                
00:19:11  [     run     ]: [Q: 0, D: 0, P: 1626, F: 4, K: 0, T: 1630] 100%                                                                                                                                                                                          
00:19:17  [  cov_merge  ]: [Q: 0, D: 0, P: 1, F: 0, K: 0, T: 1] 100%                                                                                                                                                                                                
00:19:21  [ cov_report  ]: [Q: 0, D: 0, P: 1, F: 0, K: 0, T: 1] 100%

vogelpi

Thanks, the last 5 commits of this PR look great to me. I just have two suggestions for the new cross coverpoint.

Once the other PR has been merged, I am happy to approve this one and authorize the changes. Thanks for your work on this @glaserf !

vogelpi · 2025-11-03T18:03:28Z

hw/ip/csrng/dv/cov/csrng_cov_if.sv

      ignore_bins invalid = binsof(cp_acmd) intersect { INV, GENB, GENU };
    }

+    clen_glen_cross: cross cp_acmd, cp_clen, cp_glen {


Thanks for doing this!

Two suggestions:

Explain a comment why this cross matters for the microarchitecture.

Add one more bin each for clen (0 or non-zero) and glen (one block ore more than one block). I believe we want to make sure all relevant cases are hit.

Replace the toggle-style state db arbitration by a simple valid-based scheme that leverages the fact that the ctr_drbg_gen and ctr_drbg_cmd units cannot deadlock or starve each other. Signed-off-by: Florian Glaser <[email protected]>

This removal requires slight adaptions to the handshaking done with block_encrypt as well as the removal of a complex SVA from the tb. No functional or timing impact; the response data from block_encrypt even still feed into registers. Signed-off-by: Florian Glaser <[email protected]>

This commit removes another large FIFO from the data path, measuring about 6kGE. Since the data path bifurcates in ctr_drbg_cmd for all commands that require processing by ctr_drbg_upd, a simple FSM is now required to orchestrate handshaking between the upstream requester, ctr_drbg_upd and the keyvrc FIFO. This step hence also requires the testing of the added sparse-encoded FSM in dv, documentation of this SEC.CM, and a corresponding bit in the ERR_CODE register. Signed-off-by: Florian Glaser <[email protected]>

This commit removes the last FIFO from the ctr_drbg_cmd module. Since the request and response ready signals are now in many cases asserted in the same cycle, slight adaptions to the main FSM are also necessary. Signed-off-by: Florian Glaser <[email protected]>

Signed-off-by: Florian Glaser <[email protected]>

This is a pure refactor commmit that uses the acmd_e type for all `cmd` fields in the core data types instead of the equivalent pure-logic representation. This should substanitally aid during wave debugging. Signed-off-by: Florian Glaser <[email protected]>

Removal of this FIFO requires two additional states in the FSM in ctr_drbg_gen to handle the data path bifurcation to the update unit, depending on the `glast` field. Signed-off-by: Florian Glaser <[email protected]>

Add a crosspoint that crosses the generate command with both the command length (clen) and generate length (glen) to ensure that generates with additional data and more than one block of generated bits are exercised. Signed-off-by: Florian Glaser <[email protected]>

Signed-off-by: Florian Glaser <[email protected]>

Razer6 reviewed Oct 31, 2025

View reviewed changes

glaserf force-pushed the csrng_sfifo_removal branch 4 times, most recently from 4f56f05 to 478cf94 Compare November 2, 2025 20:36

glaserf changed the title ~~Csrng sfifo removal~~ [csrng/rtl] Remove final three prim_fifo_sync from data path Nov 2, 2025

glaserf requested review from andreaskurth, meisnere, moidx, rswarbrick and vogelpi November 3, 2025 08:35

glaserf marked this pull request as ready for review November 3, 2025 08:35

glaserf requested review from a team as code owners November 3, 2025 08:35

glaserf removed request for a team November 3, 2025 08:36

vogelpi reviewed Nov 3, 2025

View reviewed changes

glaserf mentioned this pull request Nov 3, 2025

[csrng/rtl] Remove three prim_fifo_sync from the data path #28633

Open

glaserf force-pushed the csrng_sfifo_removal branch from 478cf94 to 510ac2c Compare November 4, 2025 09:52

glaserf added 9 commits November 4, 2025 19:21

[csrng/csrng_core] Rework the state db arbitration

b2b3304

Replace the toggle-style state db arbitration by a simple valid-based scheme that leverages the fact that the ctr_drbg_gen and ctr_drbg_cmd units cannot deadlock or starve each other. Signed-off-by: Florian Glaser <[email protected]>

[csrng/rtl] Remove the genreq FIFO from ctr_drbg_gen

433a5d9

Signed-off-by: Florian Glaser <[email protected]>

[csrng/rtl] Remove the rcstage FIFO from ctr_drbg_gen

8a66608

Removal of this FIFO requires two additional states in the FSM in ctr_drbg_gen to handle the data path bifurcation to the update unit, depending on the `glast` field. Signed-off-by: Florian Glaser <[email protected]>

[csrng/rtl] Remove the genbits FIFO from ctr_drbg_gen

93747ff

Signed-off-by: Florian Glaser <[email protected]>

glaserf force-pushed the csrng_sfifo_removal branch from 510ac2c to 93747ff Compare November 4, 2025 18:22

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[csrng/rtl] Remove final three prim_fifo_sync from data path #28611

[csrng/rtl] Remove final three prim_fifo_sync from data path #28611

glaserf commented Oct 30, 2025 •

edited

Loading

Uh oh!

Razer6 Oct 31, 2025

Uh oh!

glaserf Nov 3, 2025

Uh oh!

glaserf commented Nov 2, 2025

Uh oh!

vogelpi left a comment

Uh oh!

vogelpi Nov 3, 2025

Uh oh!

vogelpi Nov 3, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

[csrng/rtl] Remove final three prim_fifo_sync from data path #28611

Are you sure you want to change the base?

[csrng/rtl] Remove final three prim_fifo_sync from data path #28611

Conversation

glaserf commented Oct 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Razer6 Oct 31, 2025

Choose a reason for hiding this comment

Uh oh!

glaserf Nov 3, 2025

Choose a reason for hiding this comment

Uh oh!

glaserf commented Nov 2, 2025

CSRNG Simulation Results

Sunday November 02 2025 20:38:25 UTC

GitHub Revision: 478cf94670

Branch: csrng_sfifo_removal

Testplan

Simulator: XCELIUM

Test Results

Coverage Results

Coverage Dashboard

Failure Buckets

Uh oh!

vogelpi left a comment

Choose a reason for hiding this comment

Uh oh!

vogelpi Nov 3, 2025

Choose a reason for hiding this comment

Uh oh!

vogelpi Nov 3, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

glaserf commented Oct 30, 2025 •

edited

Loading

GitHub Revision: `478cf94670`