Skip to content

Commit

Permalink
[csrng/rtl] Move cmd checks to the cmd stage
Browse files Browse the repository at this point in the history 
Having the command checks in the main SM doesn't make a lot of sense.
This causes a lot of issues including a hang condition inside the cmd stage
in case of generate commands with cmd len greater than 1. For this reason
it is best to not even start processing commands that violate our checks.

Signed-off-by: Hakim Filali <[email protected]>
  • Loading branch information
Hakim Filali committed May 7, 2024
1 parent 1fe5037 commit 2b16016
Show file tree
Hide file tree
Showing 11 changed files with 154 additions and 163 deletions.
4 changes: 2 additions & 2 deletions hw/ip/csrng/data/csrng.hjson
View file
Original file line number Diff line number Diff line change
Expand Up @@ -547,7 +547,7 @@
'''
}
{ bits: "13",
name: "CS_MAIN_SM_ALERT",
name: "CMD_STAGE_INVALID_ACMD_ALERT",
desc: '''
This bit is set when an unsupported/illegal CSRNG command is received by the
main state machine.
Expand All @@ -556,7 +556,7 @@
'''
}
{ bits: "14",
name: "CS_MAIN_SM_INVALID_CMD_SEQ",
name: "CMD_STAGE_INVALID_CMD_SEQ_ALERT",
desc: '''
This bit is set when an out of order command is received by the main state machine.
This happens when an instantiate command is sent for a state that was already
Expand Down
30 changes: 15 additions & 15 deletions hw/ip/csrng/doc/registers.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -366,37 +366,37 @@ Recoverable alert status register
### Fields

```wavejson
{"reg": [{"name": "ENABLE_FIELD_ALERT", "bits": 1, "attr": ["rw0c"], "rotate": -90}, {"name": "SW_APP_ENABLE_FIELD_ALERT", "bits": 1, "attr": ["rw0c"], "rotate": -90}, {"name": "READ_INT_STATE_FIELD_ALERT", "bits": 1, "attr": ["rw0c"], "rotate": -90}, {"name": "ACMD_FLAG0_FIELD_ALERT", "bits": 1, "attr": ["rw0c"], "rotate": -90}, {"bits": 8}, {"name": "CS_BUS_CMP_ALERT", "bits": 1, "attr": ["rw0c"], "rotate": -90}, {"name": "CS_MAIN_SM_ALERT", "bits": 1, "attr": ["rw0c"], "rotate": -90}, {"name": "CS_MAIN_SM_INVALID_CMD_SEQ", "bits": 1, "attr": ["rw0c"], "rotate": -90}, {"name": "CMD_STAGE_RESEED_CNT_ALERT", "bits": 1, "attr": ["rw0c"], "rotate": -90}, {"bits": 16}], "config": {"lanes": 1, "fontsize": 10, "vspace": 280}}
{"reg": [{"name": "ENABLE_FIELD_ALERT", "bits": 1, "attr": ["rw0c"], "rotate": -90}, {"name": "SW_APP_ENABLE_FIELD_ALERT", "bits": 1, "attr": ["rw0c"], "rotate": -90}, {"name": "READ_INT_STATE_FIELD_ALERT", "bits": 1, "attr": ["rw0c"], "rotate": -90}, {"name": "ACMD_FLAG0_FIELD_ALERT", "bits": 1, "attr": ["rw0c"], "rotate": -90}, {"bits": 8}, {"name": "CS_BUS_CMP_ALERT", "bits": 1, "attr": ["rw0c"], "rotate": -90}, {"name": "CMD_STAGE_INVALID_ACMD_ALERT", "bits": 1, "attr": ["rw0c"], "rotate": -90}, {"name": "CMD_STAGE_INVALID_CMD_SEQ_ALERT", "bits": 1, "attr": ["rw0c"], "rotate": -90}, {"name": "CMD_STAGE_RESEED_CNT_ALERT", "bits": 1, "attr": ["rw0c"], "rotate": -90}, {"bits": 16}], "config": {"lanes": 1, "fontsize": 10, "vspace": 330}}
```

| Bits | Type | Reset | Name |
|:------:|:------:|:-------:|:---------------------------------------------------------------------------|
| 31:16 | | | Reserved |
| 15 | rw0c | 0x0 | [CMD_STAGE_RESEED_CNT_ALERT](#recov_alert_sts--cmd_stage_reseed_cnt_alert) |
| 14 | rw0c | 0x0 | [CS_MAIN_SM_INVALID_CMD_SEQ](#recov_alert_sts--cs_main_sm_invalid_cmd_seq) |
| 13 | rw0c | 0x0 | [CS_MAIN_SM_ALERT](#recov_alert_sts--cs_main_sm_alert) |
| 12 | rw0c | 0x0 | [CS_BUS_CMP_ALERT](#recov_alert_sts--cs_bus_cmp_alert) |
| 11:4 | | | Reserved |
| 3 | rw0c | 0x0 | [ACMD_FLAG0_FIELD_ALERT](#recov_alert_sts--acmd_flag0_field_alert) |
| 2 | rw0c | 0x0 | [READ_INT_STATE_FIELD_ALERT](#recov_alert_sts--read_int_state_field_alert) |
| 1 | rw0c | 0x0 | [SW_APP_ENABLE_FIELD_ALERT](#recov_alert_sts--sw_app_enable_field_alert) |
| 0 | rw0c | 0x0 | [ENABLE_FIELD_ALERT](#recov_alert_sts--enable_field_alert) |
| Bits | Type | Reset | Name |
|:------:|:------:|:-------:|:-------------------------------------------------------------------------------------|
| 31:16 | | | Reserved |
| 15 | rw0c | 0x0 | [CMD_STAGE_RESEED_CNT_ALERT](#recov_alert_sts--cmd_stage_reseed_cnt_alert) |
| 14 | rw0c | 0x0 | [CMD_STAGE_INVALID_CMD_SEQ_ALERT](#recov_alert_sts--cmd_stage_invalid_cmd_seq_alert) |
| 13 | rw0c | 0x0 | [CMD_STAGE_INVALID_ACMD_ALERT](#recov_alert_sts--cmd_stage_invalid_acmd_alert) |
| 12 | rw0c | 0x0 | [CS_BUS_CMP_ALERT](#recov_alert_sts--cs_bus_cmp_alert) |
| 11:4 | | | Reserved |
| 3 | rw0c | 0x0 | [ACMD_FLAG0_FIELD_ALERT](#recov_alert_sts--acmd_flag0_field_alert) |
| 2 | rw0c | 0x0 | [READ_INT_STATE_FIELD_ALERT](#recov_alert_sts--read_int_state_field_alert) |
| 1 | rw0c | 0x0 | [SW_APP_ENABLE_FIELD_ALERT](#recov_alert_sts--sw_app_enable_field_alert) |
| 0 | rw0c | 0x0 | [ENABLE_FIELD_ALERT](#recov_alert_sts--enable_field_alert) |

### RECOV_ALERT_STS . CMD_STAGE_RESEED_CNT_ALERT
This bit is set when the maximum number of generate requests between reseeds is
exceeded.
The invalid generate command is ignored and CSRNG continues to operate.
Writing a zero resets this status bit.

### RECOV_ALERT_STS . CS_MAIN_SM_INVALID_CMD_SEQ
### RECOV_ALERT_STS . CMD_STAGE_INVALID_CMD_SEQ_ALERT
This bit is set when an out of order command is received by the main state machine.
This happens when an instantiate command is sent for a state that was already
instantiated or when any command other than instantiate is sent for a state that
wasn't instantiated yet.
The invalid command is ignored and CSRNG continues to operate.
Writing a zero resets this status bit.

### RECOV_ALERT_STS . CS_MAIN_SM_ALERT
### RECOV_ALERT_STS . CMD_STAGE_INVALID_ACMD_ALERT
This bit is set when an unsupported/illegal CSRNG command is received by the
main state machine.
The invalid command is ignored and CSRNG continues to operate.
Expand Down
2 changes: 1 addition & 1 deletion hw/ip/csrng/doc/theory_of_operation.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ This process repeats until the `glen` field value has been matched.
Because each request is pipelined, requests from other `cmd_stage` blocks can be processed before the original generate command is completely done.
This provides some interleaving of commands since a generate command can be programmed to take a very long time.

When sending an unsupported or illegal command, `CS_MAIN_SM_ALERT` will be triggered, but there will be no status response or indication of which app the error occurred in.
When sending an unsupported or illegal command, `CMD_STAGE_INVALID_ACMD_ALERT` will be triggered, but there will be no status response or indication of which app the error occurred in.

#### Working State Values
The CSRNG working state data base (`state_db`) contains the current working state for a given DRBG instance.
Expand Down
14 changes: 8 additions & 6 deletions hw/ip/csrng/dv/env/csrng_env_pkg.sv
View file
Original file line number Diff line number Diff line change
Expand Up @@ -168,12 +168,14 @@ package csrng_env_pkg;
} err_code_bit_e;

typedef enum int {
ENABLE_FIELD_ALERT = 0,
SW_APP_ENABLE_FIELD_ALERT = 1,
READ_INT_STATE_FIELD_ALERT = 2,
ACMD_FLAG0_FIELD_ALERT = 3,
CS_BUS_CMP_ALERT = 12,
CS_MAIN_SM_ALERT = 13
ENABLE_FIELD_ALERT = 0,
SW_APP_ENABLE_FIELD_ALERT = 1,
READ_INT_STATE_FIELD_ALERT = 2,
ACMD_FLAG0_FIELD_ALERT = 3,
CS_BUS_CMP_ALERT = 12,
CMD_STAGE_INVALID_ACMD_ALERT = 13,
CMD_STAGE_INVALID_CMD_SEQ_ALERT = 14,
CMD_STAGE_RESEED_CNT_ALERT = 15
} recov_alert_bit_e;

typedef enum int {
Expand Down
11 changes: 6 additions & 5 deletions hw/ip/csrng/dv/env/seq_lib/csrng_alert_vseq.sv
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ class csrng_alert_vseq extends csrng_base_vseq;
uvm_reg csr;
uvm_reg_field fld;

// Values for the cs_main_sm_alert test.
// Values for the CMD_STAGE_INVALID_ACMD_ALERT test.
`DV_CHECK_MEMBER_RANDOMIZE_WITH_FATAL(illegal_command, illegal_command inside {INV, GENB,
GENU};)
// For clen we just care about 0, 1 and the max value (coverage).
Expand Down Expand Up @@ -205,10 +205,11 @@ class csrng_alert_vseq extends csrng_base_vseq;
// Check recov_alert_sts register has cleared.
csr_rd_check(.ptr(ral.recov_alert_sts), .compare_value(0));

`uvm_info(`gfn, $sformatf("Testing cs_main_sm_alert for app %d", cfg.which_app_err_alert), UVM_MEDIUM)
`uvm_info(`gfn, $sformatf("Testing CMD_STAGE_INVALID_ACMD_ALERT for app %d",
cfg.which_app_err_alert), UVM_MEDIUM)

// Here we send an illegal command to CSRNG to check that cs_main_sm_alert is triggered.
// Sending an illegal command does not get a response from CSRNG.
// Here we send an illegal command to CSRNG to check that CMD_STAGE_INVALID_ACMD_ALERT is
// triggered. Sending an illegal command does not get a response from CSRNG.
cs_item.acmd = illegal_command;
cs_item.clen = clen;
cs_item.flags = get_rand_mubi4_val(.t_weight(4), .f_weight(4), .other_weight(0));
Expand All @@ -220,7 +221,7 @@ class csrng_alert_vseq extends csrng_base_vseq;

`uvm_info(`gfn, $sformatf("Checking RECOV_ALERT_STS register"), UVM_MEDIUM)
exp_recov_alert_sts = 32'b0;
exp_recov_alert_sts[ral.recov_alert_sts.cs_main_sm_alert.get_lsb_pos()] = 1;
exp_recov_alert_sts[ral.recov_alert_sts.cmd_stage_invalid_acmd_alert.get_lsb_pos()] = 1;
csr_spinwait(.ptr(ral.recov_alert_sts), .exp_data(exp_recov_alert_sts));
// Since we already did a backdoor check, sampling with this value is sufficient.
cov_vif.cg_recov_alert_sample(.recov_alert(exp_recov_alert_sts));
Expand Down
83 changes: 75 additions & 8 deletions hw/ip/csrng/rtl/csrng_cmd_stage.sv
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@ module csrng_cmd_stage import csrng_pkg::*; #(
// Command checking interface.
input logic reseed_cnt_reached_i,
output logic reseed_cnt_alert_o,
output logic invalid_cmd_seq_alert_o,
output logic invalid_acmd_alert_o,
// Command to arbiter.
output logic cmd_arb_req_o,
output logic cmd_arb_sop_o,
Expand Down Expand Up @@ -88,13 +90,17 @@ module csrng_cmd_stage import csrng_pkg::*; #(
logic [GenBitsCntrWidth-1:0] cmd_gen_cnt;
csrng_cmd_sts_e err_sts;
logic reseed_cnt_exceeded;
logic invalid_cmd_seq;
logic invalid_acmd;
logic [2:0] acmd;

// Flops.
logic cmd_ack_q, cmd_ack_d;
csrng_cmd_sts_e cmd_ack_sts_q, cmd_ack_sts_d;
logic [3:0] cmd_len_q, cmd_len_d;
logic cmd_gen_flag_q, cmd_gen_flag_d;
logic [11:0] cmd_gen_cmd_q, cmd_gen_cmd_d;
logic instantiated_d, instantiated_q;

logic local_escalate;

Expand All @@ -106,12 +112,14 @@ module csrng_cmd_stage import csrng_pkg::*; #(
cmd_len_q <= '0;
cmd_gen_flag_q <= '0;
cmd_gen_cmd_q <= '0;
instantiated_q <= '0;
end else begin
cmd_ack_q <= cmd_ack_d;
cmd_ack_sts_q <= cmd_ack_sts_d;
cmd_len_q <= cmd_len_d;
cmd_gen_flag_q <= cmd_gen_flag_d;
cmd_gen_cmd_q <= cmd_gen_cmd_d;
instantiated_q <= instantiated_d;
end

assign cmd_stage_sfifo_cmd_err_o = sfifo_cmd_err;
Expand Down Expand Up @@ -173,10 +181,13 @@ module csrng_cmd_stage import csrng_pkg::*; #(
cmd_len_dec ? (cmd_len_q-1) :
cmd_len_q;

// Capture the application command type.
assign acmd = sfifo_cmd_rdata[2:0];

// For gen commands, capture information from the orignal command for use later.
assign cmd_gen_flag_d =
(!cs_enable_i) ? '0 :
cmd_gen_1st_req ? (sfifo_cmd_rdata[2:0] == GEN) :
cmd_gen_1st_req ? (acmd == GEN) :
cmd_gen_flag_q;

assign cmd_gen_cmd_d =
Expand Down Expand Up @@ -263,6 +274,9 @@ module csrng_cmd_stage import csrng_pkg::*; #(
cmd_stage_sm_err_o = 1'b0;
cmd_err_ack = 1'b0;
reseed_cnt_exceeded = 1'b0;
invalid_cmd_seq = 1'b0;
invalid_acmd = 1'b0;
instantiated_d = instantiated_q;

if (state_q == Error) begin
// In case we are in the Error state we must ignore the local escalate and enable signals.
Expand All @@ -274,21 +288,70 @@ module csrng_cmd_stage import csrng_pkg::*; #(
GenReq, GenArbGnt, GenSOP}) begin
// In case the module is disabled and we are in a legal state we must go into idle state.
state_d = Idle;
instantiated_d = 1'b0;
end else begin
// Otherwise do the state machine as normal.
unique case (state_q)
Idle: begin
// Because of the if statement above we won't leave idle if enable is low.
if (!cmd_fifo_zero) begin
// If the issued command is GEN and the reseed count has already been reached, send an
// ack with an error status response.
if ((sfifo_cmd_rdata[2:0] == GEN) && reseed_cnt_reached_i) begin
if (acmd == INS) begin
if (!instantiated_q) begin
state_d = ArbGnt;
instantiated_d = 1'b1;
end
if (instantiated_q) begin
cmd_err_ack = 1'b1;
invalid_cmd_seq = 1'b1;
state_d = Idle;
end
end else if (acmd == RES) begin
if (instantiated_q) begin
state_d = ArbGnt;
end
if (!instantiated_q) begin
cmd_err_ack = 1'b1;
invalid_cmd_seq = 1'b1;
state_d = Idle;
end
end else if (acmd == GEN) begin
if (instantiated_q) begin
// If the issued command is GEN and the reseed count has already been reached,
// send an ack with an error status response.
if ((acmd == GEN) && reseed_cnt_reached_i) begin
cmd_err_ack = 1'b1;
reseed_cnt_exceeded = 1'b1;
state_d = Idle;
end else begin
state_d = ArbGnt;
end
end
if (!instantiated_q) begin
cmd_err_ack = 1'b1;
invalid_cmd_seq = 1'b1;
state_d = Idle;
end
end else if (acmd == UPD) begin
if (instantiated_q) begin
state_d = ArbGnt;
end
if (!instantiated_q) begin
cmd_err_ack = 1'b1;
invalid_cmd_seq = 1'b1;
state_d = Idle;
end
end else if (acmd == UNI) begin
// Set the instantiation to zero.
instantiated_d = 1'b0;
state_d = ArbGnt;
end else begin
// Command was not supported.
cmd_err_ack = 1'b1;
reseed_cnt_exceeded = 1'b1;
invalid_acmd = 1'b1;
state_d = Idle;
end else begin
state_d = ArbGnt;
end
// If we received an invalid command, pop it from the FIFO.
cmd_fifo_pop = cmd_err_ack;
end
end
ArbGnt: begin
Expand Down Expand Up @@ -449,7 +512,9 @@ module csrng_cmd_stage import csrng_pkg::*; #(

assign cmd_stage_ack_o = cmd_ack_q;

assign err_sts = reseed_cnt_exceeded ? CMD_STS_RESEED_CNT_EXCEEDED : CMD_STS_SUCCESS;
assign err_sts = reseed_cnt_exceeded ? CMD_STS_RESEED_CNT_EXCEEDED :
invalid_cmd_seq ? CMD_STS_INVALID_CMD_SEQ :
invalid_acmd ? CMD_STS_INVALID_ACMD : CMD_STS_INVALID_ACMD;

assign cmd_ack_sts_d =
(!cs_enable_i) ? CMD_STS_SUCCESS :
Expand All @@ -460,6 +525,8 @@ module csrng_cmd_stage import csrng_pkg::*; #(
assign cmd_stage_ack_sts_o = cmd_ack_sts_q;

assign reseed_cnt_alert_o = reseed_cnt_exceeded;
assign invalid_cmd_seq_alert_o = invalid_cmd_seq;
assign invalid_acmd_alert_o = invalid_acmd;

// Make sure that the state machine has a stable error state. This means that after the error
// state is entered it will not exit it unless a reset signal is received.
Expand Down
Loading

0 comments on commit 2b16016

Please sign in to comment.