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

glaserf · 2025-11-02T15:36:54Z

Follow-up of #28428; removes three more FIFOs which together measure about 13kGE. Apart from everything mentioned in #28428, removal of the affected FIFOs required the following changes:

Rework the state db cycle-alternating "arbitration". As there never is a need where both the ctr_drbg_cmd and _gen units want to write to the state db at the same time, the actual arbitration logic can be kept very simple.
Introduce a small FSM to ctr_drbg_cmd that handles the bifurcation of the data stream and waits on commands that require the ctr_drbg_upd unit for both it and the cmd_rsp channel being ready before signaling the cmd_req as ready.

Part of #28153.

glaserf · 2025-11-02T15:39:11Z

Regression results:

CSRNG Simulation Results

Tuesday November 04 2025 09:55:03 UTC

GitHub Revision: `d16ce56753`

Branch: csrng_sfifo_pr

Testplan

Simulator: XCELIUM

Test Results

Stage	Name	Tests	Max Job Runtime	Simulated Time	Passing	Total	Pass Rate
V1	smoke	csrng_smoke	9.000s	246.853us	50	50	100.00 %
V1	csr_hw_reset	csrng_csr_hw_reset	2.000s	233.282us	5	5	100.00 %
V1	csr_rw	csrng_csr_rw	1.000s	13.902us	20	20	100.00 %
V1	csr_bit_bash	csrng_csr_bit_bash	11.000s	1.385ms	5	5	100.00 %
V1	csr_aliasing	csrng_csr_aliasing	3.000s	273.440us	5	5	100.00 %
V1	csr_mem_rw_with_rand_reset	csrng_csr_mem_rw_with_rand_reset	2.000s	294.328us	20	20	100.00 %
V1	regwen_csr_and_corresponding_lockable_csr	csrng_csr_rw	1.000s	13.902us	20	20	100.00 %
		csrng_csr_aliasing	3.000s	273.440us	5	5	100.00 %
V1		TOTAL			105	105	100.00 %
V2	interrupts	csrng_intr	10.000s	313.847us	200	200	100.00 %
V2	alerts	csrng_alert	18.000s	2.210ms	500	500	100.00 %
V2	err	csrng_err	8.000s	24.949us	500	500	100.00 %
V2	cmds	csrng_cmds	3.167m	37.836ms	50	50	100.00 %
V2	life cycle	csrng_cmds	3.167m	37.836ms	50	50	100.00 %
V2	stress_all	csrng_stress_all	8.217m	84.965ms	49	50	98.00 %
V2	intr_test	csrng_intr_test	1.000s	14.493us	50	50	100.00 %
V2	alert_test	csrng_alert_test	8.000s	15.735us	50	50	100.00 %
V2	tl_d_oob_addr_access	csrng_tl_errors	4.000s	175.366us	20	20	100.00 %
V2	tl_d_illegal_access	csrng_tl_errors	4.000s	175.366us	20	20	100.00 %
V2	tl_d_outstanding_access	csrng_csr_hw_reset	2.000s	233.282us	5	5	100.00 %
		csrng_csr_rw	1.000s	13.902us	20	20	100.00 %
		csrng_csr_aliasing	3.000s	273.440us	5	5	100.00 %
		csrng_same_csr_outstanding	2.000s	104.462us	20	20	100.00 %
V2	tl_d_partial_access	csrng_csr_hw_reset	2.000s	233.282us	5	5	100.00 %
		csrng_csr_rw	1.000s	13.902us	20	20	100.00 %
		csrng_csr_aliasing	3.000s	273.440us	5	5	100.00 %
		csrng_same_csr_outstanding	2.000s	104.462us	20	20	100.00 %
V2		TOTAL			1439	1440	99.93 %
V2S	tl_intg_err	csrng_sec_cm	9.000s	179.763us	5	5	100.00 %
		csrng_tl_intg_err	5.000s	815.392us	20	20	100.00 %
V2S	sec_cm_config_regwen	csrng_regwen	8.000s	60.852us	50	50	100.00 %
		csrng_csr_rw	1.000s	13.902us	20	20	100.00 %
V2S	sec_cm_config_mubi	csrng_alert	18.000s	2.210ms	500	500	100.00 %
V2S	sec_cm_intersig_mubi	csrng_stress_all	8.217m	84.965ms	49	50	98.00 %
V2S	sec_cm_main_sm_fsm_sparse	csrng_intr	10.000s	313.847us	200	200	100.00 %
		csrng_err	8.000s	24.949us	500	500	100.00 %
		csrng_sec_cm	9.000s	179.763us	5	5	100.00 %
V2S	sec_cm_updrsp_fsm_sparse	csrng_intr	10.000s	313.847us	200	200	100.00 %
		csrng_err	8.000s	24.949us	500	500	100.00 %
		csrng_sec_cm	9.000s	179.763us	5	5	100.00 %
V2S	sec_cm_update_fsm_sparse	csrng_intr	10.000s	313.847us	200	200	100.00 %
		csrng_err	8.000s	24.949us	500	500	100.00 %
		csrng_sec_cm	9.000s	179.763us	5	5	100.00 %
V2S	sec_cm_blk_enc_fsm_sparse	csrng_intr	10.000s	313.847us	200	200	100.00 %
		csrng_err	8.000s	24.949us	500	500	100.00 %
		csrng_sec_cm	9.000s	179.763us	5	5	100.00 %
V2S	sec_cm_outblk_fsm_sparse	csrng_intr	10.000s	313.847us	200	200	100.00 %
		csrng_err	8.000s	24.949us	500	500	100.00 %
		csrng_sec_cm	9.000s	179.763us	5	5	100.00 %
V2S	sec_cm_gen_cmd_ctr_redun	csrng_intr	10.000s	313.847us	200	200	100.00 %
		csrng_err	8.000s	24.949us	500	500	100.00 %
		csrng_sec_cm	9.000s	179.763us	5	5	100.00 %
V2S	sec_cm_drbg_upd_ctr_redun	csrng_intr	10.000s	313.847us	200	200	100.00 %
		csrng_err	8.000s	24.949us	500	500	100.00 %
		csrng_sec_cm	9.000s	179.763us	5	5	100.00 %
V2S	sec_cm_drbg_gen_ctr_redun	csrng_intr	10.000s	313.847us	200	200	100.00 %
		csrng_err	8.000s	24.949us	500	500	100.00 %
		csrng_sec_cm	9.000s	179.763us	5	5	100.00 %
V2S	sec_cm_ctrl_mubi	csrng_alert	18.000s	2.210ms	500	500	100.00 %
V2S	sec_cm_main_sm_ctr_local_esc	csrng_intr	10.000s	313.847us	200	200	100.00 %
		csrng_err	8.000s	24.949us	500	500	100.00 %
V2S	sec_cm_constants_lc_gated	csrng_stress_all	8.217m	84.965ms	49	50	98.00 %
V2S	sec_cm_sw_genbits_bus_consistency	csrng_alert	18.000s	2.210ms	500	500	100.00 %
V2S	sec_cm_tile_link_bus_integrity	csrng_tl_intg_err	5.000s	815.392us	20	20	100.00 %
V2S	sec_cm_aes_cipher_fsm_sparse	csrng_intr	10.000s	313.847us	200	200	100.00 %
		csrng_err	8.000s	24.949us	500	500	100.00 %
		csrng_sec_cm	9.000s	179.763us	5	5	100.00 %
V2S	sec_cm_aes_cipher_fsm_redun	csrng_intr	10.000s	313.847us	200	200	100.00 %
		csrng_err	8.000s	24.949us	500	500	100.00 %
V2S	sec_cm_aes_cipher_ctrl_sparse	csrng_intr	10.000s	313.847us	200	200	100.00 %
		csrng_err	8.000s	24.949us	500	500	100.00 %
V2S	sec_cm_aes_cipher_fsm_local_esc	csrng_intr	10.000s	313.847us	200	200	100.00 %
		csrng_err	8.000s	24.949us	500	500	100.00 %
V2S	sec_cm_aes_cipher_ctr_redun	csrng_intr	10.000s	313.847us	200	200	100.00 %
		csrng_err	8.000s	24.949us	500	500	100.00 %
		csrng_sec_cm	9.000s	179.763us	5	5	100.00 %
V2S	sec_cm_aes_cipher_data_reg_local_esc	csrng_intr	10.000s	313.847us	200	200	100.00 %
		csrng_err	8.000s	24.949us	500	500	100.00 %
V2S		TOTAL			75	75	100.00 %
V3	stress_all_with_rand_reset	csrng_stress_all_with_rand_reset	2.583m	12.978ms	10	10	100.00 %
V3		TOTAL			10	10	100.00 %
		TOTAL			1629	1630	99.94 %

Coverage Results

Coverage Dashboard

Score	Block	Branch	Statement	Expression	Toggle	Fsm	Assertion	CoverGroup
97.59 %	98.76 %	96.96 %	99.85 %	96.77 %	92.08 %	100.00 %	95.53 %	90.12 %

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 1 failures:
- Test csrng_stress_all has 1 failures.
  - 42.csrng_stress_all.7201815913989753525406440744181533782652175777971277456600672816489512363970
    Line 146, in log /scratch/glaserf/opentitan/scratch/csrng_sfifo_pr/csrng-sim-xcelium/42.csrng_stress_all/latest/run.log
```
  UVM_ERROR @ 5722333422 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 @ 5722333422 ps: (uvm_report_catcher.svh:705) [UVM/REPORT/CATCHER]
  --- UVM Report catcher Summary ---
```

INFO: [FlowCfg] [scratch_path]: [csrng] [/scratch/glaserf/opentitan/scratch/csrng_sfifo_pr/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:01:04  [    build    ]: [Q: 0, D: 0, P: 2, F: 0, K: 0, T: 2] 100%                                                                                                                                                                                         
00:17:11  [     run     ]: [Q: 0, D: 0, P: 1629, F: 1, K: 0, T: 1630] 100%                                                                                                                                                                                   
00:17:41  [  cov_merge  ]: [Q: 0, D: 0, P: 1, F: 0, K: 0, T: 1] 100%                                                                                                                                                                                         
00:17:45  [ cov_report  ]: [Q: 0, D: 0, P: 1, F: 0, K: 0, T: 1] 100%

rswarbrick

I think there's a nontrivial comment on the first commit, but the later commits basically have comments asking whether you can make it easier for me to review :-) I claim that it's not entirely selfish, because it will also make it easier for future readers to understand the history...

I'm excited by how much can be ripped out though! Very nice!

rswarbrick · 2025-11-03T10:07:18Z

hw/ip/csrng/rtl/csrng_core.sv

-            reseed_cnt_reached_q[ai];
+    assign reseed_cnt_reached_d[ai] = (state_db_wr_vld && (state_db_wr_data.inst_id == ai)) ?
+                                      (state_db_wr_data.rs_ctr >= reg2hw.reseed_interval.q) :
+                                       reseed_cnt_reached_q[ai];


Nit: I think this is a character too far in?

I think the alignment is still not right for that last line.

Gnah, ofc you are right. Third time's the charm, I guess :(

hw/ip/csrng/rtl/csrng_core.sv

rswarbrick · 2025-11-03T10:56:17Z

hw/ip/csrng/data/csrng.hjson

                This bit will stay set until the next reset.
                '''
        }
-        { bits: "7",


If it's easy, I'd consider splitting this commit in two, where the first part removes the FIFO (and wires zero to this error signal) and the second part removes the register field. I think the result would probably be a lot easier to follow.

Unfortunately, splitting this up would be quite a bit of work, as this affects in total six commits in this and the follow-up PR (which would then become twelve or 13 commits, given your suggestion of splitting one of them into three). Apart from this, I would like to keep the regfile changes together with the rtl and dv changes in a single commit for each FIFO, respectively, for the following reasons:

Doing otherwise would break consistency with the already merged [csrng/rtl] Remove four prim_fifo_sync from the data path #28428

Splitting the changes up would make the commits not self-contained anymore, which they currently are. We'd introduce commits to master where the documentation claims a register field and associated functionality that is not there. IIRC that goes against the contribution guidelines.

Of course, please let me know if you feel strongly that the split up would be necessary.

rswarbrick · 2025-11-03T11:00:09Z

hw/ip/csrng/data/csrng.hjson

                This bit will stay set until the next reset.
                '''
        }
-        { bits: "3",


A bit like with the previous commit, can this one be split as well? Here, it would be in three parts. I think the SFIFO_RCSTATE_ERR field could be removed as a later commit (and driven with zero beforehand). Similarly, I think DRBG_CMD_SM_ERR be added as a first commit (driven with zero). That way, all the "register shuffling cruft" gets separated from the real work.

Please see my reply to your question in the previous commit which should (hopefully, at least 😊) address all comments that request the split-out of the regfile changes.

rswarbrick · 2025-11-03T11:02:49Z

hw/ip/csrng/data/csrng.hjson

                This bit will stay set until the next reset.
                '''
        }
-        { bits: "4",


I realise that I'm repeating myself (sorry!), but I think it would make sense to split this commit into two pieces: the first one would remove the fifo and wire zero into the error field; the second would remove the field.

vogelpi

I've reviewed the first commits and will continue later. Thanks for the PR @glaserf !

hw/ip/csrng/rtl/csrng_core.sv

vogelpi

I've now also reviewed the final two commits and I have some more questions. But this looks good to me in general.

hw/ip/csrng/rtl/csrng_ctr_drbg_cmd.sv

hw/ip/csrng/rtl/csrng_main_sm.sv

glaserf

Thanks @rswarbrick and @vogelpi for the swift and thorough reviews! I have addressed your comments in individual replies and pushed the appropriate changes.

@rswarbrick I addressed your comments about splitting up the commits in a reply to one of them, I hope that is okay.

glaserf · 2025-11-03T22:07:04Z

hw/ip/csrng/rtl/csrng_core.sv

-            reseed_cnt_reached_q[ai];
+    assign reseed_cnt_reached_d[ai] = (state_db_wr_vld && (state_db_wr_data.inst_id == ai)) ?
+                                      (state_db_wr_data.rs_ctr >= reg2hw.reseed_interval.q) :
+                                       reseed_cnt_reached_q[ai];


hw/ip/csrng/rtl/csrng_core.sv

glaserf · 2025-11-03T22:20:09Z

hw/ip/csrng/data/csrng.hjson

                This bit will stay set until the next reset.
                '''
        }
-        { bits: "7",


Unfortunately, splitting this up would be quite a bit of work, as this affects in total six commits in this and the follow-up PR (which would then become twelve or 13 commits, given your suggestion of splitting one of them into three). Apart from this, I would like to keep the regfile changes together with the rtl and dv changes in a single commit for each FIFO, respectively, for the following reasons:

Doing otherwise would break consistency with the already merged [csrng/rtl] Remove four prim_fifo_sync from the data path #28428

Splitting the changes up would make the commits not self-contained anymore, which they currently are. We'd introduce commits to master where the documentation claims a register field and associated functionality that is not there. IIRC that goes against the contribution guidelines.

Of course, please let me know if you feel strongly that the split up would be necessary.

hw/ip/csrng/rtl/csrng_ctr_drbg_cmd.sv

hw/ip/csrng/rtl/csrng_main_sm.sv

vogelpi

Thanks for addressing my comments, this looks good to me!

vogelpi · 2025-11-05T20:17:42Z

CHANGE AUTHORIZED: hw/ip/csrng/data/csrng.hjson
CHANGE AUTHORIZED: hw/ip/csrng/rtl/csrng.sv
CHANGE AUTHORIZED: hw/ip/csrng/rtl/csrng_block_encrypt.sv
CHANGE AUTHORIZED: hw/ip/csrng/rtl/csrng_core.sv
CHANGE AUTHORIZED: hw/ip/csrng/rtl/csrng_ctr_drbg_cmd.sv
CHANGE AUTHORIZED: hw/ip/csrng/rtl/csrng_ctr_drbg_upd.sv
CHANGE AUTHORIZED: hw/ip/csrng/rtl/csrng_main_sm.sv
CHANGE AUTHORIZED: hw/ip/csrng/rtl/csrng_reg_pkg.sv
CHANGE AUTHORIZED: hw/ip/csrng/rtl/csrng_reg_top.sv
CHANGE AUTHORIZED: hw/ip/csrng/rtl/csrng_state_db.sv

This PR modifies the RTL of CSRNG to remove more area and simplify the design. The changes are part of a bigger restructuring effort which has been discussed in multiple WG meetings. The pass rate and coverage remain above the V2(S) thresholds. This is fine.

marnovandermaas

Some small comments from my side.

marnovandermaas · 2025-11-07T10:00:49Z

hw/ip/csrng/rtl/csrng_core.sv

-            reseed_cnt_reached_q[ai];
+    assign reseed_cnt_reached_d[ai] = (state_db_wr_vld && (state_db_wr_data.inst_id == ai)) ?
+                                      (state_db_wr_data.rs_ctr >= reg2hw.reseed_interval.q) :
+                                       reseed_cnt_reached_q[ai];


I think the alignment is still not right for that last line.

marnovandermaas · 2025-11-07T10:04:04Z

hw/ip/csrng/rtl/csrng_core.sv

  logic               unused_state_db_inst_state;

-  assign unused_err_code_test_bit = (|err_code_test_bit[19:16]) || (|err_code_test_bit[27:26]) ||
+  assign unused_err_code_test_bit = err_code_test_bit[27] || (|err_code_test_bit[19:16]) ||


What bout 26?

Thanks Marno, I will carefully check again if I made a mistake here. It's quite a pain to keep track of these unused bits and in addition there was one bit wrong in there right from the start (before I started the refactoring efforts at all), but I forgot which one it exactly was.

So it turns out, it was indeed err_code_test_bit[26] that is (and always was) indeed used and therefore does not belong here in the unused bits list.

This is what I've noticed as well when reviewing this.

marnovandermaas · 2025-11-07T10:04:17Z

hw/ip/csrng/rtl/csrng_core.sv

                                    err_code_test_bit[8] || (|err_code_test_bit[6:5]) ||
                                    err_code_test_bit[2];
-  assign unused_enable_fo = cs_enable_fo[10] || (|cs_enable_fo[8:7]) || cs_enable_fo[4];
+  assign unused_enable_fo = cs_enable_fo[42] || cs_enable_fo[10] || (|cs_enable_fo[8:7]) ||


Why is 42 added here?

Because it is not used anymore, see this line (I hope the link works after rebase -i, it was on line 1126 of the old state.

marnovandermaas · 2025-11-07T10:07:16Z

hw/ip/csrng/rtl/csrng_core.sv

-                                    err_code_test_bit[8] || (|err_code_test_bit[6:5]) ||
-                                    err_code_test_bit[2];
-  assign unused_enable_fo = cs_enable_fo[42] || cs_enable_fo[10] || (|cs_enable_fo[8:7]) ||
-                            cs_enable_fo[4];
+                                    (|err_code_test_bit[8:5]) || err_code_test_bit[2];
+  assign unused_enable_fo = cs_enable_fo[42] || (|cs_enable_fo[10:7]) || cs_enable_fo[4];


I think some of these changes crept into the previous commit unintentionally.

Hm no, this is fine I think: This commit makes

err_code_test_bit[7]

cs_enable_fo[9]

unused, which is what the diff should add to the respective unused-signals.

phani-lowrisc · 2025-11-07T11:02:45Z

+1 fr DV changes.

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]>

glaserf

Thanks @marnovandermaas and @rswarbrick for your feedback. I hopefully have now addressed all your points. Please have another look :)

glaserf · 2025-11-07T11:34:19Z

hw/ip/csrng/rtl/csrng_core.sv

  logic               unused_state_db_inst_state;

-  assign unused_err_code_test_bit = (|err_code_test_bit[19:16]) || (|err_code_test_bit[27:26]) ||
+  assign unused_err_code_test_bit = err_code_test_bit[27] || (|err_code_test_bit[19:16]) ||


So it turns out, it was indeed err_code_test_bit[26] that is (and always was) indeed used and therefore does not belong here in the unused bits list.

glaserf · 2025-11-07T11:35:54Z

hw/ip/csrng/rtl/csrng_core.sv

                                    err_code_test_bit[8] || (|err_code_test_bit[6:5]) ||
                                    err_code_test_bit[2];
-  assign unused_enable_fo = cs_enable_fo[10] || (|cs_enable_fo[8:7]) || cs_enable_fo[4];
+  assign unused_enable_fo = cs_enable_fo[42] || cs_enable_fo[10] || (|cs_enable_fo[8:7]) ||


Because it is not used anymore, see this line (I hope the link works after rebase -i, it was on line 1126 of the old state.

glaserf · 2025-11-07T11:41:03Z

hw/ip/csrng/rtl/csrng_core.sv

-                                    err_code_test_bit[8] || (|err_code_test_bit[6:5]) ||
-                                    err_code_test_bit[2];
-  assign unused_enable_fo = cs_enable_fo[42] || cs_enable_fo[10] || (|cs_enable_fo[8:7]) ||
-                            cs_enable_fo[4];
+                                    (|err_code_test_bit[8:5]) || err_code_test_bit[2];
+  assign unused_enable_fo = cs_enable_fo[42] || (|cs_enable_fo[10:7]) || cs_enable_fo[4];


Hm no, this is fine I think: This commit makes

err_code_test_bit[7]

cs_enable_fo[9]

unused, which is what the diff should add to the respective unused-signals.

glaserf · 2025-11-07T11:44:19Z

hw/ip/csrng/data/csrng.hjson

                This bit will stay set until the next reset.
                '''
        }
-        { bits: "3",


Please see my reply to your question in the previous commit which should (hopefully, at least 😊) address all comments that request the split-out of the regfile changes.

glaserf · 2025-11-07T11:46:42Z

hw/ip/csrng/rtl/csrng_core.sv

-            reseed_cnt_reached_q[ai];
+    assign reseed_cnt_reached_d[ai] = (state_db_wr_vld && (state_db_wr_data.inst_id == ai)) ?
+                                      (state_db_wr_data.rs_ctr >= reg2hw.reseed_interval.q) :
+                                       reseed_cnt_reached_q[ai];


Gnah, ofc you are right. Third time's the charm, I guess :(

andreaskurth · 2025-11-10T18:15:16Z

CHANGE AUTHORIZED: hw/ip/csrng/data/csrng.hjson
CHANGE AUTHORIZED: hw/ip/csrng/rtl/csrng.sv
CHANGE AUTHORIZED: hw/ip/csrng/rtl/csrng_block_encrypt.sv
CHANGE AUTHORIZED: hw/ip/csrng/rtl/csrng_core.sv
CHANGE AUTHORIZED: hw/ip/csrng/rtl/csrng_ctr_drbg_cmd.sv
CHANGE AUTHORIZED: hw/ip/csrng/rtl/csrng_ctr_drbg_upd.sv
CHANGE AUTHORIZED: hw/ip/csrng/rtl/csrng_main_sm.sv
CHANGE AUTHORIZED: hw/ip/csrng/rtl/csrng_reg_pkg.sv
CHANGE AUTHORIZED: hw/ip/csrng/rtl/csrng_reg_top.sv
CHANGE AUTHORIZED: hw/ip/csrng/rtl/csrng_state_db.sv

This PR modifies the RTL of CSRNG to remove more area and simplify the design. The changes are part of a bigger restructuring effort which has been discussed in multiple WG meetings. The pass rate and coverage remain above the V2(S) thresholds. This is fine.

glaserf force-pushed the csrng_sfifo_pr branch from 38adbd4 to d60af54 Compare November 2, 2025 19:42

glaserf mentioned this pull request Nov 2, 2025

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

Merged

glaserf marked this pull request as ready for review November 3, 2025 07:20

glaserf requested review from a team as code owners November 3, 2025 07:20

glaserf requested review from Razer6, andreaskurth, marnovandermaas, meisnere, moidx, pamaury, rswarbrick and vogelpi and removed request for a team, marnovandermaas and pamaury November 3, 2025 07:20

rswarbrick reviewed Nov 3, 2025

View reviewed changes

vogelpi reviewed Nov 3, 2025

View reviewed changes

hw/ip/csrng/rtl/csrng_core.sv Outdated Show resolved Hide resolved

hw/ip/csrng/rtl/csrng_core.sv Outdated Show resolved Hide resolved

hw/ip/csrng/rtl/csrng_core.sv Outdated Show resolved Hide resolved

hw/ip/csrng/rtl/csrng_core.sv Outdated Show resolved Hide resolved

vogelpi reviewed Nov 3, 2025

View reviewed changes

hw/ip/csrng/rtl/csrng_ctr_drbg_cmd.sv Show resolved Hide resolved

hw/ip/csrng/rtl/csrng_ctr_drbg_cmd.sv Show resolved Hide resolved

hw/ip/csrng/rtl/csrng_ctr_drbg_cmd.sv Outdated Show resolved Hide resolved

hw/ip/csrng/rtl/csrng_main_sm.sv Show resolved Hide resolved

glaserf force-pushed the csrng_sfifo_pr branch from d60af54 to eb2a9f2 Compare November 3, 2025 22:56

glaserf commented Nov 3, 2025

View reviewed changes

glaserf force-pushed the csrng_sfifo_pr branch 2 times, most recently from d16ce56 to 205519d Compare November 4, 2025 18:21

vogelpi approved these changes Nov 5, 2025

View reviewed changes

marnovandermaas reviewed Nov 7, 2025

View reviewed changes

[csrng/csrng_core] Rework the state db arbitration

f74c643

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]>

glaserf added 3 commits November 7, 2025 12:53

glaserf force-pushed the csrng_sfifo_pr branch from 205519d to 936df49 Compare November 7, 2025 11:53

glaserf commented Nov 7, 2025

View reviewed changes

glaserf requested review from marnovandermaas and rswarbrick November 7, 2025 12:05

vogelpi added the CI:Rerun Rerun failed CI jobs label Nov 10, 2025

github-actions bot removed the CI:Rerun Rerun failed CI jobs label Nov 10, 2025

vogelpi added this pull request to the merge queue Nov 10, 2025

Merged via the queue into lowRISC:master with commit 81ccf27 Nov 10, 2025
49 of 50 checks passed

glaserf deleted the csrng_sfifo_pr branch November 13, 2025 11:59

glaserf mentioned this pull request Nov 21, 2025

[csrng/rtl] Simplify the CTR_DRBG data path #28804

Draft

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

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

Uh oh!

Conversation

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

Uh oh!

Uh oh!

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

Uh oh!

CSRNG Simulation Results

Tuesday November 04 2025 09:55:03 UTC

GitHub Revision: d16ce56753

Branch: csrng_sfifo_pr

Simulator: XCELIUM

Test Results

Coverage Results

Failure Buckets

Uh oh!

rswarbrick left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

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

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vogelpi left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

vogelpi left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

glaserf left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

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

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

vogelpi left a comment

Choose a reason for hiding this comment

Uh oh!

vogelpi commented Nov 5, 2025

Uh oh!

marnovandermaas left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

glaserf commented Nov 2, 2025 •

edited

Loading

glaserf commented Nov 2, 2025 •

edited

Loading

GitHub Revision: `d16ce56753`

glaserf Nov 3, 2025 •

edited

Loading

glaserf Nov 3, 2025 •

edited

Loading