Skip to content

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

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.

Sign up for GitHub

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

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
b2b3304
[csrng/csrng_core] Rework the state db arbitration
glaserf Oct 15, 2025
c9a5677
[csrng/rtl] Remove the bencack FIFO from ctr_drbg_upd
glaserf Oct 6, 2025
612d248
[csrng/rtl] Remove the rcstage FIFO from ctr_drbg_cmd
glaserf Oct 10, 2025
205519d
[csrng/rtl] Remove the keyvrc FIFO from ctr_drbg_cmd
glaserf Oct 15, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 15 additions & 30 deletions hw/ip/csrng/data/csrng.hjson
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,9 @@
{ name: "MAIN_SM.FSM.SPARSE"
desc: "The CSRNG main state machine uses a sparse state encoding."
}
{ name: "UPDRSP.FSM.SPARSE"
desc: "The CSRNG update response state machine uses a sparse state encoding."
}
{ name: "UPDATE.FSM.SPARSE"
desc: "The CSRNG update state machine uses a sparse state encoding."
}
Expand All @@ -188,13 +191,13 @@
desc: "The CSRNG block output state machine uses a sparse state encoding."
}
{ name: "GEN_CMD.CTR.REDUN"
desc: "The generate command uses a counter that is protected by a second counter that counts in the opposite direction."
desc: "The generate command uses a counter that is protected by a second counter that counts in the opposite direction."
}
{ name: "DRBG_UPD.CTR.REDUN"
desc: "The ctr_drbg update algorithm uses a counter that is protected by a second counter that counts in the opposite direction."
desc: "The ctr_drbg update algorithm uses a counter that is protected by a second counter that counts in the opposite direction."
}
{ name: "DRBG_GEN.CTR.REDUN"
desc: "The ctr_drbg generate algorithm uses a counter that is protected by a second counter that counts in the opposite direction."
desc: "The ctr_drbg generate algorithm uses a counter that is protected by a second counter that counts in the opposite direction."
}
{ name: "CTRL.MUBI"
desc: "Multi-bit field used for selection control."
Expand Down Expand Up @@ -728,33 +731,6 @@
This bit will stay set until the next reset.
'''
}
{ bits: "3",
Copy link
Contributor

@rswarbrick rswarbrick Nov 3, 2025

Choose a reason for hiding this comment

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

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.

name: "SFIFO_RCSTAGE_ERR",
desc: '''
This bit will be set to one when an error has been detected for the
rcstage FIFO. The type of error is reflected in the type status
bits (bits 28 through 30 of this register).
This bit will stay set until the next reset.
'''
}
{ bits: "4",
Copy link
Contributor

@rswarbrick rswarbrick Nov 3, 2025

Choose a reason for hiding this comment

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

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.

name: "SFIFO_KEYVRC_ERR",
desc: '''
This bit will be set to one when an error has been detected for the
keyvrc FIFO. The type of error is reflected in the type status
bits (bits 28 through 30 of this register).
This bit will stay set until the next reset.
'''
}
{ bits: "7",
Copy link
Contributor

@rswarbrick rswarbrick Nov 3, 2025

Choose a reason for hiding this comment

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

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.

Copy link
Contributor Author

@glaserf glaserf Nov 3, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

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:

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

name: "SFIFO_BENCACK_ERR",
desc: '''
This bit will be set to one when an error has been detected for the
bencack FIFO. The type of error is reflected in the type status
bits (bits 28 through 30 of this register).
This bit will stay set until the next reset.
'''
}
{ bits: "9",
name: "SFIFO_FINAL_ERR",
desc: '''
Expand Down Expand Up @@ -882,6 +858,15 @@
This bit will stay set until the next reset.
'''
}
{ bits: "27",
name: "DRBG_CMD_SM_ERR",
desc: '''
This bit will be set when the state machine in the ctr_drbg_cmd unit has entered
an illegal state.
This error will signal a fatal alert, and also an interrupt, if enabled.
This bit will stay set until the next reset.
'''
}
{ bits: "28",
name: "FIFO_WRITE_ERR",
desc: '''
Expand Down
11 changes: 11 additions & 0 deletions hw/ip/csrng/data/csrng_sec_cm_testplan.hjson
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,17 @@
stage: V2S
tests: ["csrng_sec_cm", "csrng_intr", "csrng_err"]
}
{
name: sec_cm_updrsp_fsm_sparse
desc: '''
Verify the countermeasure(s) UPDRSP.FSM.SPARSE.
The csrng_intr and csrng_err tests verify that if the FSM state is forced to an illegal state encoding 1) this is reported with a cs_fatal_err interrupt in the INTR_STATE register and 2) the corresponding bit in the ERR_CODE register is set.
They currently don't check whether the DUT actually triggers a fatal alert.
Alert connection and triggering are verified through automated FPV.
'''
stage: V2S
tests: ["csrng_sec_cm", "csrng_intr", "csrng_err"]
}
{
name: sec_cm_update_fsm_sparse
desc: '''
Expand Down
1 change: 1 addition & 0 deletions hw/ip/csrng/doc/interfaces.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ Referring to the [Comportable guideline for peripheral device functionality](htt
| CSRNG.CONFIG.MUBI | Registers have multi-bit encoded fields. |
| CSRNG.INTERSIG.MUBI | OTP signal used to enable software access to registers. |
| CSRNG.MAIN_SM.FSM.SPARSE | The CSRNG main state machine uses a sparse state encoding. |
| CSRNG.UPDRSP.FSM.SPARSE | The CSRNG update response state machine uses a sparse state encoding. |
| CSRNG.UPDATE.FSM.SPARSE | The CSRNG update state machine uses a sparse state encoding. |
| CSRNG.BLK_ENC.FSM.SPARSE | The CSRNG block encrypt state machine uses a sparse state encoding. |
| CSRNG.OUTBLK.FSM.SPARSE | The CSRNG block output state machine uses a sparse state encoding. |
Expand Down
37 changes: 10 additions & 27 deletions hw/ip/csrng/doc/registers.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -555,12 +555,12 @@ Writing a zero resets this status bit.
Hardware detection of error conditions status register
- Offset: `0x54`
- Reset default: `0x0`
- Reset mask: `0x77f0fe9b`
- Reset mask: `0x7ff0fe03`

### Fields

```wavejson
{"reg": [{"name": "SFIFO_CMD_ERR", "bits": 1, "attr": ["ro"], "rotate": -90}, {"name": "SFIFO_GENBITS_ERR", "bits": 1, "attr": ["ro"], "rotate": -90}, {"bits": 1}, {"name": "SFIFO_RCSTAGE_ERR", "bits": 1, "attr": ["ro"], "rotate": -90}, {"name": "SFIFO_KEYVRC_ERR", "bits": 1, "attr": ["ro"], "rotate": -90}, {"bits": 2}, {"name": "SFIFO_BENCACK_ERR", "bits": 1, "attr": ["ro"], "rotate": -90}, {"bits": 1}, {"name": "SFIFO_FINAL_ERR", "bits": 1, "attr": ["ro"], "rotate": -90}, {"name": "SFIFO_GBENCACK_ERR", "bits": 1, "attr": ["ro"], "rotate": -90}, {"name": "SFIFO_GRCSTAGE_ERR", "bits": 1, "attr": ["ro"], "rotate": -90}, {"name": "SFIFO_GGENREQ_ERR", "bits": 1, "attr": ["ro"], "rotate": -90}, {"name": "SFIFO_GADSTAGE_ERR", "bits": 1, "attr": ["ro"], "rotate": -90}, {"name": "SFIFO_GGENBITS_ERR", "bits": 1, "attr": ["ro"], "rotate": -90}, {"name": "SFIFO_CMDID_ERR", "bits": 1, "attr": ["ro"], "rotate": -90}, {"bits": 4}, {"name": "CMD_STAGE_SM_ERR", "bits": 1, "attr": ["ro"], "rotate": -90}, {"name": "MAIN_SM_ERR", "bits": 1, "attr": ["ro"], "rotate": -90}, {"name": "DRBG_GEN_SM_ERR", "bits": 1, "attr": ["ro"], "rotate": -90}, {"name": "DRBG_UPDBE_SM_ERR", "bits": 1, "attr": ["ro"], "rotate": -90}, {"name": "DRBG_UPDOB_SM_ERR", "bits": 1, "attr": ["ro"], "rotate": -90}, {"name": "AES_CIPHER_SM_ERR", "bits": 1, "attr": ["ro"], "rotate": -90}, {"name": "CMD_GEN_CNT_ERR", "bits": 1, "attr": ["ro"], "rotate": -90}, {"bits": 1}, {"name": "FIFO_WRITE_ERR", "bits": 1, "attr": ["ro"], "rotate": -90}, {"name": "FIFO_READ_ERR", "bits": 1, "attr": ["ro"], "rotate": -90}, {"name": "FIFO_STATE_ERR", "bits": 1, "attr": ["ro"], "rotate": -90}, {"bits": 1}], "config": {"lanes": 1, "fontsize": 10, "vspace": 200}}
{"reg": [{"name": "SFIFO_CMD_ERR", "bits": 1, "attr": ["ro"], "rotate": -90}, {"name": "SFIFO_GENBITS_ERR", "bits": 1, "attr": ["ro"], "rotate": -90}, {"bits": 7}, {"name": "SFIFO_FINAL_ERR", "bits": 1, "attr": ["ro"], "rotate": -90}, {"name": "SFIFO_GBENCACK_ERR", "bits": 1, "attr": ["ro"], "rotate": -90}, {"name": "SFIFO_GRCSTAGE_ERR", "bits": 1, "attr": ["ro"], "rotate": -90}, {"name": "SFIFO_GGENREQ_ERR", "bits": 1, "attr": ["ro"], "rotate": -90}, {"name": "SFIFO_GADSTAGE_ERR", "bits": 1, "attr": ["ro"], "rotate": -90}, {"name": "SFIFO_GGENBITS_ERR", "bits": 1, "attr": ["ro"], "rotate": -90}, {"name": "SFIFO_CMDID_ERR", "bits": 1, "attr": ["ro"], "rotate": -90}, {"bits": 4}, {"name": "CMD_STAGE_SM_ERR", "bits": 1, "attr": ["ro"], "rotate": -90}, {"name": "MAIN_SM_ERR", "bits": 1, "attr": ["ro"], "rotate": -90}, {"name": "DRBG_GEN_SM_ERR", "bits": 1, "attr": ["ro"], "rotate": -90}, {"name": "DRBG_UPDBE_SM_ERR", "bits": 1, "attr": ["ro"], "rotate": -90}, {"name": "DRBG_UPDOB_SM_ERR", "bits": 1, "attr": ["ro"], "rotate": -90}, {"name": "AES_CIPHER_SM_ERR", "bits": 1, "attr": ["ro"], "rotate": -90}, {"name": "CMD_GEN_CNT_ERR", "bits": 1, "attr": ["ro"], "rotate": -90}, {"name": "DRBG_CMD_SM_ERR", "bits": 1, "attr": ["ro"], "rotate": -90}, {"name": "FIFO_WRITE_ERR", "bits": 1, "attr": ["ro"], "rotate": -90}, {"name": "FIFO_READ_ERR", "bits": 1, "attr": ["ro"], "rotate": -90}, {"name": "FIFO_STATE_ERR", "bits": 1, "attr": ["ro"], "rotate": -90}, {"bits": 1}], "config": {"lanes": 1, "fontsize": 10, "vspace": 200}}
```

| Bits | Type | Reset | Name |
Expand All @@ -569,7 +569,7 @@ Hardware detection of error conditions status register
| 30 | ro | 0x0 | [FIFO_STATE_ERR](#err_code--fifo_state_err) |
| 29 | ro | 0x0 | [FIFO_READ_ERR](#err_code--fifo_read_err) |
| 28 | ro | 0x0 | [FIFO_WRITE_ERR](#err_code--fifo_write_err) |
| 27 | | | Reserved |
| 27 | ro | 0x0 | [DRBG_CMD_SM_ERR](#err_code--drbg_cmd_sm_err) |
| 26 | ro | 0x0 | [CMD_GEN_CNT_ERR](#err_code--cmd_gen_cnt_err) |
| 25 | ro | 0x0 | [AES_CIPHER_SM_ERR](#err_code--aes_cipher_sm_err) |
| 24 | ro | 0x0 | [DRBG_UPDOB_SM_ERR](#err_code--drbg_updob_sm_err) |
Expand All @@ -585,12 +585,7 @@ Hardware detection of error conditions status register
| 11 | ro | 0x0 | [SFIFO_GRCSTAGE_ERR](#err_code--sfifo_grcstage_err) |
| 10 | ro | 0x0 | [SFIFO_GBENCACK_ERR](#err_code--sfifo_gbencack_err) |
| 9 | ro | 0x0 | [SFIFO_FINAL_ERR](#err_code--sfifo_final_err) |
| 8 | | | Reserved |
| 7 | ro | 0x0 | [SFIFO_BENCACK_ERR](#err_code--sfifo_bencack_err) |
| 6:5 | | | Reserved |
| 4 | ro | 0x0 | [SFIFO_KEYVRC_ERR](#err_code--sfifo_keyvrc_err) |
| 3 | ro | 0x0 | [SFIFO_RCSTAGE_ERR](#err_code--sfifo_rcstage_err) |
| 2 | | | Reserved |
| 8:2 | | | Reserved |
| 1 | ro | 0x0 | [SFIFO_GENBITS_ERR](#err_code--sfifo_genbits_err) |
| 0 | ro | 0x0 | [SFIFO_CMD_ERR](#err_code--sfifo_cmd_err) |

Expand All @@ -612,6 +607,12 @@ this register) are asserted as a result of an error pulse generated from
any full FIFO that has been received a write pulse.
This bit will stay set until the next reset.

### ERR_CODE . DRBG_CMD_SM_ERR
This bit will be set when the state machine in the ctr_drbg_cmd unit has entered
an illegal state.
This error will signal a fatal alert, and also an interrupt, if enabled.
This bit will stay set until the next reset.

### ERR_CODE . CMD_GEN_CNT_ERR
This bit will be set to one when a mismatch in any of the hardened counters
has been detected.
Expand Down Expand Up @@ -697,24 +698,6 @@ final FIFO. The type of error is reflected in the type status
bits (bits 28 through 30 of this register).
This bit will stay set until the next reset.

### ERR_CODE . SFIFO_BENCACK_ERR
This bit will be set to one when an error has been detected for the
bencack FIFO. The type of error is reflected in the type status
bits (bits 28 through 30 of this register).
This bit will stay set until the next reset.

### ERR_CODE . SFIFO_KEYVRC_ERR
This bit will be set to one when an error has been detected for the
keyvrc FIFO. The type of error is reflected in the type status
bits (bits 28 through 30 of this register).
This bit will stay set until the next reset.

### ERR_CODE . SFIFO_RCSTAGE_ERR
This bit will be set to one when an error has been detected for the
rcstage FIFO. The type of error is reflected in the type status
bits (bits 28 through 30 of this register).
This bit will stay set until the next reset.

### ERR_CODE . SFIFO_GENBITS_ERR
This bit will be set to one when an error has been detected for the
command stage genbits FIFO. The type of error is reflected in the type status
Expand Down
14 changes: 7 additions & 7 deletions hw/ip/csrng/dv/cov/csrng_cov_if.sv
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -243,15 +243,15 @@ interface csrng_cov_if (
// If ERR_CODE register has SFIFO related field set, it also needs to set at least one
// FIFO_*_ERR field.
illegal_bins illegal = !binsof(cp_err_codes) intersect { CMD_STAGE_SM_ERR, MAIN_SM_ERR,
DRBG_GEN_SM_ERR, DRBG_UPDBE_SM_ERR,
DRBG_UPDOB_SM_ERR, AES_CIPHER_SM_ERR,
CMD_GEN_CNT_ERR } &&
binsof(cp_fifo_err_type) intersect { 0 };
DRBG_CMD_SM_ERR, DRBG_GEN_SM_ERR,
DRBG_UPDBE_SM_ERR, DRBG_UPDOB_SM_ERR,
AES_CIPHER_SM_ERR, CMD_GEN_CNT_ERR }
&& binsof(cp_fifo_err_type) intersect { 0 };

ignore_bins ignore = binsof(cp_err_codes) intersect { CMD_STAGE_SM_ERR, MAIN_SM_ERR,
DRBG_GEN_SM_ERR, DRBG_UPDBE_SM_ERR,
DRBG_UPDOB_SM_ERR, AES_CIPHER_SM_ERR,
CMD_GEN_CNT_ERR };
DRBG_CMD_SM_ERR, DRBG_GEN_SM_ERR,
DRBG_UPDBE_SM_ERR, DRBG_UPDOB_SM_ERR,
AES_CIPHER_SM_ERR, CMD_GEN_CNT_ERR };
}

cp_csrng_aes_fsm_err: coverpoint
Expand Down
Loading
Loading