[keymgr] Clean-up and bug fix for keymgr
This PR started off as an attmempt to fix #7902,
however in the process of doing so, it was discovered there
are more issues with how keymgr handled dummy transactions
in the disabled state.
This PR attempts to fix those issues.
Signed-off-by: Timothy Chen <timothytim@google.com>
diff --git a/hw/ip/keymgr/data/keymgr.hjson b/hw/ip/keymgr/data/keymgr.hjson
index 7ea1350..a329f6b 100644
--- a/hw/ip/keymgr/data/keymgr.hjson
+++ b/hw/ip/keymgr/data/keymgr.hjson
@@ -880,7 +880,7 @@
Detailed per bit breakdown of the INVALID_STATES field in !!ERR_CODE
''',
swaccess: "ro",
- hwaccess: "hwo",
+ hwaccess: "hrw",
fields: [
{ bits: "0",
name: "CMD",
diff --git a/hw/ip/keymgr/doc/_index.md b/hw/ip/keymgr/doc/_index.md
index deae032..bbc8a70 100644
--- a/hw/ip/keymgr/doc/_index.md
+++ b/hw/ip/keymgr/doc/_index.md
@@ -126,11 +126,34 @@
This allows the software to keep the last valid sideload keys while preventing the system from further advancing the valid key.
When advance and generate calls are invoked from this state, the outputs and keys are indiscriminately updated with randomly computed values.
+Key manager enters disabled state based on direct invocation by software:
+* Advance from `OwnerRootKey`
+* Disable operation
### Invalid
-`Invalid` state is entered whenever key manager is disabled through the [life cycle connection](#life-cycle-connection).
+`Invalid` state is entered whenever key manager is disabled through the [life cycle connection](#life-cycle-connection) or when an operation encounters a [fault](#faults-and-operational-faults) .
Upon `Invalid` entry, both the working state and the sideload keys are wiped with entropy directly.
-Note, this is different from `Disabled` state entry, which updates with KMAC outputs.
+Note, this is different from `Disabled` state entry, which updates internal key with KMAC outputs but leaves sideload and software keys intact.
+
+#### Invalid Entry Wiping
+Since the life cycle controller can disable the key manager at any time, the key manager attempts to gracefully handle the wiping process.
+When the disable is seen, the key manager immediately begins wiping all keys (internal key, hardware sideload key, software key) with entropy.
+However, if an operation was already ongoing, the key manager waits for the transaction to complete gracefully before transitioning to invalid state.
+
+While waiting for the transaction to complete, the key manager continuously wipes all keys with entropy.
+
+### Invalid and Disabled State
+
+Note that `Invalid` and `Disabled` states are functionally equivalent.
+The main difference between the two is "how" the state was reached.
+
+`Disabled` state is reached through intentional software commands.
+While `Invalid` state is reached through life cycle disable or operational faults.
+
+This also means that only `Invalid` is a terminal state.
+If after entering `Disabled` life cycle is disabled or a fault is encountered, the same [invalid entry procedure](#Invalid) is followed to bring the system to a terminal `Invalid` state.
+
+If ever multiple conditions collide (a fault is detected at the same time software issues disable command), the `Invalid` entry path always takes precedence.
## Life Cycle Connection
The function of the key manager is directly tied to the life cycle controller.
@@ -193,11 +216,32 @@
Two separate alerts are generated, one corresponding to each category above.
-In addition to the error code register, there is a separate {{< regref FAULT_STATUS >}} that captures the sources that caused `Invalid states` to assert.
-* Command error - A non-one-hot command was issued from the key manager controller to the KMAC data interface. This is not possible by software and indicates a hardware fault. This error can also happen if the KMCA data fsm gets into an invalid state.
-* Kmac fsm error - The kmac fsm has transitioned into an error state.
-* Kmac operation error - The kmac module has returned an error, this should never happen.
-* Register file integrity error - The register file has encountered an integrity error.
+### Faults and Operational Faults
+
+The {{< regref FAULT_STATUS >}} register captures all faults that can occur within the key manager.
+Some of these faults can occur only when there is a key manager operation ongoing, other faults can happen at any time (for example register integrity faults or shadow storage faults).
+{{< regref FAULT_STATUS >}} captures all faults regardless of when they happen.
+
+The {{< regref ERR_CODE.INVALID_STATES >}} field represents the presence of **any** fault during a key manager operation.
+This means if a fault happens before or during an operation, it will be recognized as an operational fault and result in keymgr's transition to `Invalid` state.
+
+#### Example 1: Fault During Operation
+The key manager is running a generate operation and a non-onehot command was observed by the kmac interface.
+Since the non-onehot condition is a fault, it will be reflected in {{< regref FAULT_STATUS >}}.
+Since an operation was ongoing when this fault was seen, it will also be reflected in {{< regref ERR_CODE.INVALID_STATES >}}.
+This is considered an operational fault and begin transition to the `Invalid` [state](#invalid-entry-wiping).
+
+#### Example 2: Fault During Idle
+The key manager is NOT running an operation and is idle.
+During this time, a fault was observed on the regfile (shadow storage error) and FSM (control FSM integrity error).
+The faults will be reflected in {{< regref FAULT_STATUS >}}.
+However, since there was no ongoing key manager operation, the error is **not** reflected in {{< regref ERR_CODE.INVALID_STATES >}}.
+This is **not** considered an operational fault and the key manager will remain in its current state.
+
+#### Example 3: Operation after Fault Detection
+Continuing from the example above, assume now the key manager begins an operation.
+Since the key manager has previous encountered a fault, any operation now is considered an operational fault and will be reflected in {{< regref ERR_CODE.INVALID_STATES >}}.
+This is considered an operational fault and begin transition to the `Invalid` [state](#invalid-entry-wiping).
### Invalid Output
@@ -254,6 +298,7 @@
* During `Initialized`, `CreatorRootKey`, `OwnerIntermediateKey` and `OwnerRootKey` states, a fault error causes the relevant key / outputs to be updated; however an operational error does not.
* During `Invalid` and `Disabled` states, the relevant key / outputs are updated regardless of the error.
* Only the relevant collateral is updated -> ie, advance / disable command leads to working key update, and generate command leads to software or sideload key update.
+* During `Disabled` state, if life cycle is disabled or an operational fault is encountered, the key manager transitions to `Invalid` state, see [here](#invalid-and-disabled-state)
## DICE Support
diff --git a/hw/ip/keymgr/dv/env/keymgr_scoreboard.sv b/hw/ip/keymgr/dv/env/keymgr_scoreboard.sv
index c5efcda..4301ed1 100644
--- a/hw/ip/keymgr/dv/env/keymgr_scoreboard.sv
+++ b/hw/ip/keymgr/dv/env/keymgr_scoreboard.sv
@@ -202,7 +202,7 @@
string csr_name = $sformatf("sw_share%0d_output_%0d", i, j);
uvm_reg csr = ral.get_reg_by_name(csr_name);
- void'(csr.predict(.value(sw_share_output[i][j]), .kind(UVM_PREDICT_READ)));
+ void'(csr.predict(.value(sw_share_output[i][j]), .kind(UVM_PREDICT_DIRECT)));
`uvm_info(`gfn, $sformatf("Predict %0s = 0x%0h", csr_name, sw_share_output[i][j]),
UVM_MEDIUM)
end
@@ -983,17 +983,20 @@
end
fork
begin
- // it takes 2 cycle to wipe sw_share. add one more negedge to avoid race condition
- cfg.clk_rst_vif.wait_n_clks(3);
if (current_op_status == keymgr_pkg::OpWip) begin
+ // if operation is ongoing, then we must wait for response to be received
+ // before transitioning
+ wait(cfg.keymgr_vif.kmac_data_rsp.done);
current_state = keymgr_pkg::StInvalid;
`uvm_info(`gfn, "operation WIP but Keymgr_en is Off, update err_code and move to Invalid",
UVM_LOW)
process_error_n_alert();
current_op_status = keymgr_pkg::OpDoneFail;
end else begin
+ // it takes 2 cycle to wipe sw_share. add one more negedge to avoid race condition
// corner case, keymgr_en is changed while OP is almost done. OP will finish successfully
// delay update_state in 1 cycle
+ cfg.clk_rst_vif.wait_n_clks(3);
update_state(keymgr_pkg::StInvalid);
`uvm_info(`gfn, "operation WIP but Keymgr_en is Off, move to Invalid", UVM_LOW)
end
diff --git a/hw/ip/keymgr/rtl/keymgr.sv b/hw/ip/keymgr/rtl/keymgr.sv
index d562fce..181d891 100644
--- a/hw/ip/keymgr/rtl/keymgr.sv
+++ b/hw/ip/keymgr/rtl/keymgr.sv
@@ -221,8 +221,6 @@
assign kmac_data_truncated[i] = kmac_data[i][KeyWidth-1:0];
end
- logic ctrl_state_intg_err;
-
keymgr_ctrl #(
.KmacEnMasking(KmacEnMasking)
) u_ctrl (
@@ -232,7 +230,6 @@
.regfile_intg_err_i(regfile_intg_err),
.shadowed_update_err_i(shadowed_update_err),
.shadowed_storage_err_i(shadowed_storage_err),
- .state_intg_err_o(ctrl_state_intg_err),
.prng_reseed_req_o(reseed_req),
.prng_reseed_ack_i(reseed_ack),
.prng_en_o(ctrl_lfsr_en),
@@ -245,6 +242,7 @@
.sw_binding_unlock_o(sw_binding_unlock),
.status_o(hw2reg.op_status.d),
.fault_o(fault_code),
+ .fault_i(|reg2hw.fault_status),
.error_o(err_code),
.data_en_o(data_en),
.data_valid_o(data_valid),
@@ -558,12 +556,9 @@
logic op_errs, op_err_req_q, op_err_req_d, op_err_ack;
// Error code fatal faults occur only when keymgr operation is actually invoked.
- // The regfile, state integrity and shadow errors are more persistent / structural issues,
- // thus they cause both the operational faults to occur and are also sent directly.
- assign fault_errs = err_code[ErrInvalidStates] |
- regfile_intg_err |
- ctrl_state_intg_err |
- shadowed_storage_err;
+ // Fault status can happen independently of any operation
+ assign fault_errs = |reg2hw.fault_status |
+ err_code[ErrInvalidStates];
assign fault_err_req_d = fault_errs ? 1'b1 :
fault_err_ack ? 1'b0 : fault_err_req_q;
diff --git a/hw/ip/keymgr/rtl/keymgr_ctrl.sv b/hw/ip/keymgr/rtl/keymgr_ctrl.sv
index adcc166..4f04ed4 100644
--- a/hw/ip/keymgr/rtl/keymgr_ctrl.sv
+++ b/hw/ip/keymgr/rtl/keymgr_ctrl.sv
@@ -20,7 +20,6 @@
input regfile_intg_err_i,
input shadowed_update_err_i,
input shadowed_storage_err_i,
- output logic state_intg_err_o,
// Software interface
input op_start_i,
@@ -36,6 +35,7 @@
output keymgr_working_state_e working_state_o,
output logic sw_binding_unlock_o,
output logic init_o,
+ input fault_i,
// Data input
input otp_ctrl_pkg::otp_keymgr_key_t root_key_i,
@@ -107,9 +107,8 @@
} keymgr_ctrl_state_e;
// Enumeration for operation handling
- typedef enum logic [2:0] {
+ typedef enum logic [1:0] {
StIdle,
- StRandomize,
StAdv,
StAdvAck,
StWait
@@ -125,87 +124,105 @@
logic [CntWidth-1:0] cnt;
logic [CdiWidth-1:0] cdi_cnt;
- logic key_update;
- logic data_update;
+ // error conditions
logic kmac_out_valid;
logic invalid_op;
- logic disabled;
-
- // disable is treated like an advanced call
- logic advance_sel;
- logic disable_sel;
- logic gen_id_sel;
- logic gen_out_sw_sel;
- logic gen_out_hw_sel;
- logic gen_out_sel;
- logic gen_sel;
-
+ logic cnt_err;
// states fall out of sparsely encoded range
logic state_intg_err_q, state_intg_err_d;
- // error types
- logic op_err;
- logic op_fault_err;
- logic op_fault_err_q;
- logic op_fault_err_d;
+ ///////////////////////////
+ // General operation decode
+ ///////////////////////////
- assign op_fault_err = op_fault_err_q | op_fault_err_d;
+ logic adv_op, dis_op, gen_id_op, gen_sw_op, gen_hw_op, gen_op;
+ assign adv_op = (op_i == OpAdvance);
+ assign dis_op = (op_i == OpDisable);
+ assign gen_id_op = (op_i == OpGenId);
+ assign gen_sw_op = (op_i == OpGenSwOut);
+ assign gen_hw_op = (op_i == OpGenHwOut);
+ assign gen_op = (gen_id_op | gen_sw_op | gen_hw_op);
+
+ ///////////////////////////
+ // interaction between software and main fsm
+ ///////////////////////////
+ // disable is treated like an advanced call
+ logic advance_sel;
+ logic disable_sel;
+ logic gen_out_hw_sel;
+
+ assign advance_sel = op_start_i & adv_op & en_i;
+ assign gen_out_hw_sel = op_start_i & gen_hw_op & en_i;
+
+ // disable is selected whenever a normal operation is not set
+ assign disable_sel = (op_start_i & dis_op) | !en_i;
+
+
+ ///////////////////////////
+ // interaction between main control fsm and operation fsm
+ ///////////////////////////
// req/ack interface with op handling fsm
logic op_req;
logic op_ack;
logic op_update;
- logic random_req;
- logic random_ack;
+ logic op_busy;
+ logic disabled;
- // req from main control fsm to key update controls
- logic wipe_req;
+ logic adv_req, dis_req, id_req, gen_req;
+ assign adv_req = op_req & adv_op;
+ assign dis_req = op_req & dis_op;
+ assign id_req = op_req & gen_id_op;
+ assign gen_req = op_req & (gen_sw_op | gen_hw_op);
- assign advance_sel = op_start_i & op_i == OpAdvance & en_i;
- assign gen_id_sel = op_start_i & op_i == OpGenId & en_i;
- assign gen_out_sw_sel = op_start_i & op_i == OpGenSwOut & en_i;
- assign gen_out_hw_sel = op_start_i & op_i == OpGenHwOut & en_i;
- assign gen_out_sel = gen_out_sw_sel | gen_out_hw_sel;
- assign gen_sel = gen_id_sel | gen_out_sel;
+ ///////////////////////////
+ // interaction between operation fsm and software
+ ///////////////////////////
- // disable is selected whenever a normal operation is not, and when
- // keymgr is disabled
- assign disable_sel = (op_start_i & !(gen_sel | advance_sel)) |
- !en_i;
-
- // requestion from working state to operation handling FSM
- logic adv_req, id_req, gen_req;
- assign adv_req = op_req & (advance_sel | disable_sel);
- assign id_req = op_req & gen_id_sel;
- assign gen_req = op_req & gen_out_sel;
+ logic op_err;
+ logic op_fault_err;
// unlock sw binding configuration whenever an advance call is made without errors
- assign sw_binding_unlock_o = adv_en_o & op_ack & ~|error_o;
+ assign sw_binding_unlock_o = adv_req & op_ack & ~|error_o;
+ // error definition
// check incoming kmac data validity
assign kmac_out_valid = valid_data_chk(kmac_data_i[0]) &
(~KmacEnMasking | valid_data_chk(kmac_data_i[1]));
- // error definition
- assign op_fault_err_d = |fault_o | op_fault_err_q;
- assign op_err = kmac_input_invalid_i | invalid_op;
+ assign op_err = error_o[ErrInvalidOp] |
+ error_o[ErrInvalidIn];
- // key update conditions
- assign key_update = advance_sel | disable_sel;
-
- // external collateral update conditions
- assign data_update = gen_sel;
+ assign op_fault_err = error_o[ErrInvalidStates];
- always_ff @(posedge clk_i or negedge rst_ni) begin
- if (!rst_ni) begin
- state_intg_err_q <= '0;
- op_state_q <= StIdle;
- end else begin
- state_intg_err_q <= state_intg_err_d;
- op_state_q <= op_state_d;
- end
- end
+ ///////////////////////////
+ // key update controls
+ ///////////////////////////
+
+ // update select can come from both main and operation fsm's
+ keymgr_key_update_e update_sel, op_update_sel;
+
+ // req from main control fsm to key update controls
+ logic wipe_req;
+ logic random_req;
+ logic random_ack;
+
+ // wipe and initialize take precedence
+ assign update_sel = wipe_req ? KeyUpdateWipe :
+ random_req ? KeyUpdateRandom :
+ init_o ? KeyUpdateRoot : op_update_sel;
+
+ ///////////////////////////
+ // interaction between main fsm and prng
+ ///////////////////////////
+
+ logic prng_en;
+ assign prng_en_o = prng_en | disabled | wipe_req;
+
+ //////////////////////////
+ // Main Control FSM
+ //////////////////////////
logic [StateWidth-1:0] state_raw_q;
assign state_q = keymgr_ctrl_state_e'(state_raw_q);
@@ -219,6 +236,14 @@
.q_o ( state_raw_q )
);
+ always_ff @(posedge clk_i or negedge rst_ni) begin
+ if (!rst_ni) begin
+ state_intg_err_q <= '0;
+ end else begin
+ state_intg_err_q <= state_intg_err_d;
+ end
+ end
+
// prevents unknowns from reaching the outside world.
// - whatever operation causes the input data select to be disabled should not expose the key
// state.
@@ -250,12 +275,6 @@
.q_o(root_key_valid_q)
);
- keymgr_key_update_e update_sel, op_update_sel;
- logic key_update_vld;
-
- assign update_sel = wipe_req ? KeyUpdateWipe :
- init_o ? KeyUpdateRoot : op_update_sel;
-
// Do not let the count toggle unless an advance operation is
// selected
assign cdi_cnt = op_req ? cnt[CdiWidth-1:0] : '0;
@@ -264,11 +283,9 @@
key_state_d = key_state_q;
data_valid_o = 1'b0;
wipe_key_o = 1'b0;
- key_update_vld = 1'b0;
// if a wipe request arrives, immediately destroy the
// keys regardless of current state
-
unique case (update_sel)
KeyUpdateRandom: begin
for (int i = 0; i < CDIs; i++) begin
@@ -293,15 +310,8 @@
end
KeyUpdateKmac: begin
- data_valid_o = data_update & ~op_err;
- key_update_vld = key_update & ~op_err;
- key_state_d[cdi_sel_o] = key_update_vld ? kmac_data_i : key_state_q[cdi_sel_o];
- end
-
- KeyUpdateInvalid: begin
- data_valid_o = data_update;
- key_update_vld = key_update;
- key_state_d[cdi_sel_o] = key_update_vld ? kmac_data_i : key_state_q[cdi_sel_o];
+ data_valid_o = gen_op;
+ key_state_d[cdi_sel_o] = (adv_op || dis_op) ? kmac_data_i : key_state_q[cdi_sel_o];
end
KeyUpdateWipe: begin
@@ -317,7 +327,6 @@
endcase // unique case (update_sel)
end
- logic cnt_err;
keymgr_cnt #(
.Width(CntWidth),
.CntStyle(DupCnt)
@@ -327,7 +336,7 @@
.clr_i(op_ack | random_ack),
.set_i('0),
.set_cnt_i('0),
- .en_i(op_update),
+ .en_i(op_update | random_req),
.cnt_o(cnt),
.err_o(cnt_err)
);
@@ -342,11 +351,16 @@
(init_o | invalid_op);
- logic next_state;
- logic invalid_state;
- assign next_state = op_ack & advance_sel & key_update_vld;
- assign invalid_state = op_ack & (disable_sel | op_fault_err);
-
+ // There are 3 possibilities
+ // advance to next state (software command)
+ // advance to disabled state (software command)
+ // advance to invalid state (detected fault)
+ logic adv_state;
+ logic dis_state;
+ logic inv_state;
+ assign adv_state = op_ack & adv_req;
+ assign dis_state = op_ack & dis_req;
+ assign inv_state = op_fault_err;
always_comb begin
// persistent data
@@ -355,6 +369,7 @@
// request to op handling
op_req = 1'b0;
random_req = 1'b0;
+ random_ack = 1'b0;
// request to key updates
wipe_req = 1'b0;
@@ -365,16 +380,16 @@
// data update and select signals
stage_sel_o = Disable;
+ // indication that state is disabled
+ disabled = 1'b0;
+
// enable prng toggling
prng_reseed_req_o = 1'b0;
- prng_en_o = 1'b0;
+ prng_en = 1'b0;
// initialization complete
init_o = 1'b0;
- // Indicates the control state machine is disabled
- disabled = 1'b0;
-
// if state is ever faulted, hold on to this indication
// until reset.
state_intg_err_d = state_intg_err_q;
@@ -383,7 +398,7 @@
// Only advance can be called from reset state
StCtrlReset: begin
// in reset state, don't enable entropy yet, since there are no users.
- prng_en_o = 1'b0;
+ prng_en = 1'b0;
// always use random data for advance, since out of reset state
// the key state will be randomized.
@@ -403,24 +418,30 @@
prng_reseed_req_o = 1'b1;
if (prng_reseed_ack_i) begin
- state_d = en_i ? StCtrlRandom : StCtrlWipe;
+ state_d = StCtrlRandom;
end
end
// This state does not accept any command.
StCtrlRandom: begin
- prng_en_o = 1'b1;
- random_req = 1'b1;
+ prng_en = 1'b1;
- if (random_ack) begin
- state_d = en_i ? StCtrlRootKey : StCtrlWipe;
+ if (cnt < EntropyRounds-1) begin
+ random_req = 1'b1;
+ end
+ // when mask population is complete, xor the root_key into the zero share
+ // if in the future the root key is updated to 2 shares, it will direclty overwrite
+ // the values here
+ else begin
+ random_ack = 1'b1;
+ state_d = StCtrlRootKey;
end
end
// load the root key.
StCtrlRootKey: begin
init_o = 1'b1;
- state_d = StCtrlInit;
+ state_d = (!en_i || inv_state) ? StCtrlWipe : StCtrlInit;
end
// Beginning from the Init state, operations are accepted.
@@ -433,11 +454,11 @@
stage_sel_o = advance_sel ? Creator : Disable;
invalid_op = op_start_i & ~(advance_sel | disable_sel);
- if (!en_i) begin
+ if (!en_i || inv_state) begin
state_d = StCtrlWipe;
- end else if (invalid_state) begin
+ end else if (dis_state) begin
state_d = StCtrlDisabled;
- end else if (next_state) begin
+ end else if (adv_state) begin
state_d = StCtrlCreatorRootKey;
end
end
@@ -452,11 +473,11 @@
stage_sel_o = disable_sel ? Disable :
advance_sel ? OwnerInt : Creator;
- if (!en_i) begin
+ if (!en_i || inv_state) begin
state_d = StCtrlWipe;
- end else if (invalid_state) begin
+ end else if (dis_state) begin
state_d = StCtrlDisabled;
- end else if (next_state) begin
+ end else if (adv_state) begin
state_d = StCtrlOwnerIntKey;
end
end
@@ -471,11 +492,11 @@
stage_sel_o = disable_sel ? Disable :
advance_sel ? Owner : OwnerInt;
- if (!en_i) begin
+ if (!en_i || inv_state) begin
state_d = StCtrlWipe;
- end else if (invalid_state) begin
+ end else if (dis_state) begin
state_d = StCtrlDisabled;
- end else if (next_state) begin
+ end else if (adv_state) begin
state_d = StCtrlOwnerKey;
end
end
@@ -490,9 +511,9 @@
// when disabling, select random data input
stage_sel_o = disable_sel | advance_sel ? Disable : Owner;
- if (!en_i) begin
+ if (!en_i || inv_state) begin
state_d = StCtrlWipe;
- end else if (next_state || invalid_state) begin
+ end else if (adv_state || dis_state) begin
state_d = StCtrlDisabled;
end
end
@@ -503,7 +524,8 @@
// key are wiped.
StCtrlWipe: begin
wipe_req = 1'b1;
- stage_sel_o = Disable;
+ // if there was already an operation ongoing, maintain the request until completion
+ op_req = op_busy;
invalid_op = op_start_i;
// If the enable is dropped during the middle of a transaction, we clear and wait for that
@@ -518,13 +540,23 @@
end
end
- // (StCtrlDisabled and StCtrlInvalid included)
- // Continue to kick off random transactions
+ // StCtrlDisabled and StCtrlInvalid are almost functionally equivalent
+ // The only difference is that Disabled is entered through software invocation,
+ // while Invalid is entered through life cycle disable or operational fault.
+ //
+ // Both states continue to kick off random transactions
// All transactions are treated as invalid despite completing
- StCtrlDisabled, StCtrlInvalid: begin
- stage_sel_o = Disable;
+ StCtrlDisabled: begin
op_req = op_start_i;
- invalid_op = op_req;
+ disabled = 1'b1;
+
+ if (!en_i || inv_state) begin
+ state_d = StCtrlWipe;
+ end
+ end
+
+ StCtrlInvalid: begin
+ op_req = op_start_i;
disabled = 1'b1;
end
@@ -539,7 +571,19 @@
// Current working state provided for software read
// Certain states are collapsed for simplicity
+ keymgr_working_state_e last_working_st;
+ logic state_update;
+
+ always_ff @(posedge clk_i or negedge rst_ni) begin
+ if (!rst_ni) begin
+ last_working_st <= StReset;
+ end else if (state_update) begin
+ last_working_st <= working_state_o;
+ end
+ end
+
always_comb begin
+ state_update = 1'b1;
working_state_o = StInvalid;
unique case (state_q)
@@ -561,7 +605,12 @@
StCtrlDisabled:
working_state_o = StDisabled;
- StCtrlWipe, StCtrlInvalid:
+ StCtrlWipe: begin
+ state_update = 1'b0;
+ working_state_o = last_working_st;
+ end
+
+ StCtrlInvalid:
working_state_o = StInvalid;
default:
@@ -569,28 +618,24 @@
endcase // unique case (state_q)
end
- // If working over multiple CDIs, a fault of any
- // is considered an overall fault
- //
- // faults are always permanently retained, and will be used
- // to transition to control FSM back into DISABLED state
- // if it is somehow glitched OUT of it.
+
+ /////////////////////////
+ // Operateion state, handle advance and generate
+ /////////////////////////
+
always_ff @(posedge clk_i or negedge rst_ni) begin
if (!rst_ni) begin
- op_fault_err_q <= '0;
+ op_state_q <= StIdle;
end else begin
- op_fault_err_q <= op_fault_err_d;
+ op_state_q <= op_state_d;
end
end
always_comb begin
op_state_d = op_state_q;
-
op_update = 1'b0;
op_ack = 1'b0;
- op_update_sel = KeyUpdateIdle;
-
- random_ack = 1'b0;
+ op_busy = 1'b1;
// output to kmac interface
adv_en_o = 1'b0;
@@ -599,30 +644,14 @@
unique case (op_state_q)
StIdle: begin
- if (random_req) begin
- op_state_d = StRandomize;
- end else if (adv_req) begin
+ op_busy = '0;
+ if (adv_req || dis_req) begin
op_state_d = StAdv;
end else if (id_req || gen_req) begin
op_state_d = StWait;
end
end
- StRandomize: begin
- op_update_sel = KeyUpdateRandom;
-
- if (cnt < EntropyRounds-1) begin
- op_update = 1'b1;
- end
- // when mask population is complete, xor the root_key into the zero share
- // if in the future the root key is updated to 2 shares, it will direclty overwrite
- // the values here
- else begin
- random_ack = 1'b1;
- op_state_d = StIdle;
- end
- end
-
StAdv: begin
adv_en_o = 1'b1;
@@ -633,12 +662,6 @@
op_update = 1'b1;
op_state_d = StAdvAck;
end
-
- // Invalidate keys under the following conditions
- if (op_ack || op_update) begin
- op_update_sel = disabled ? KeyUpdateInvalid :
- op_fault_err ? KeyUpdateWipe : KeyUpdateKmac;
- end
end
// drop adv_en_o to allow kmac interface handshake
@@ -648,19 +671,13 @@
// Not an advanced operation
StWait: begin
-
- id_en_o = id_req;
- gen_en_o = gen_req;
+ id_en_o = gen_id_op;
+ gen_en_o = gen_sw_op | gen_hw_op;
if (kmac_done_i) begin
op_ack = 1'b1;
op_state_d = StIdle;
end
-
- if (op_ack) begin
- op_update_sel = disabled ? KeyUpdateInvalid :
- op_fault_err ? KeyUpdateWipe : KeyUpdateKmac;
- end
end
// What should go here?
@@ -669,15 +686,17 @@
endcase // unique case (adv_state_q)
end
+ // operations fsm update precedence
+ // when in disabled state, always update.
+ assign op_update_sel = (op_ack | op_update) & disabled ? KeyUpdateKmac :
+ op_fault_err ? KeyUpdateWipe :
+ op_err ? KeyUpdateIdle :
+ (op_ack | op_update) ? KeyUpdateKmac : KeyUpdateIdle;
- // data errors are not relevant when operation was not accepted.
- // invalid operation errors can happen even when operations are not accepted.
- assign state_intg_err_o = state_intg_err_q;
-
- assign error_o[ErrInvalidOp] = op_done_o & invalid_op;
+ assign error_o[ErrInvalidOp] = op_done_o & (invalid_op | disabled);
assign error_o[ErrInvalidIn] = op_ack & kmac_input_invalid_i;
assign error_o[ErrShadowUpdate] = shadowed_update_err_i;
- assign error_o[ErrInvalidStates] = op_ack & op_fault_err;
+ assign error_o[ErrInvalidStates] = (op_done_o | op_update) & fault_i;
assign fault_o[FaultCmd] = kmac_cmd_err_i;
assign fault_o[FaultKmacFsm] = kmac_fsm_err_i;
@@ -687,7 +706,7 @@
assign fault_o[FaultKmacOut] = op_ack & ~kmac_out_valid;
assign fault_o[FaultRegFileIntg] = regfile_intg_err_i;
assign fault_o[FaultShadow] = shadowed_storage_err_i;
- assign fault_o[FaultCtrlFsm] = state_intg_err_o;
+ assign fault_o[FaultCtrlFsm] = state_intg_err_q;
assign fault_o[FaultCtrlCnt] = cnt_err;
always_comb begin
@@ -744,7 +763,9 @@
StCtrlDataEn: begin
data_en_o = 1'b1;
- if (adv_en_o) begin
+ if (op_done_o) begin
+ data_st_d = StCtrlDataWait;
+ end else if (adv_en_o) begin
data_st_d = StCtrlDataDis;
end
end
@@ -766,9 +787,6 @@
endcase // unique case (data_st_q)
end
-
-
-
///////////////////////////////
// Functions
///////////////////////////////
@@ -806,4 +824,11 @@
// The count value should always be 0 when a transaction start
`ASSERT(CntZero_A, $rose(op_start_i) |-> cnt == '0)
+ // Whenever a transaction completes, data_en must return to 0 on the next cycle
+ `ASSERT(DataEnDis_A, op_start_i & op_done_o |=> ~data_en_o)
+
+ // Whenever data enable asserts, it must be the case that there was a generate or
+ // id operation
+ `ASSERT(DataEn_A, data_en_o |-> (id_en_o | gen_en_o) & ~adv_en_o)
+
endmodule
diff --git a/hw/ip/keymgr/rtl/keymgr_kmac_if.sv b/hw/ip/keymgr/rtl/keymgr_kmac_if.sv
index 54ce29f..397c7ca 100644
--- a/hw/ip/keymgr/rtl/keymgr_kmac_if.sv
+++ b/hw/ip/keymgr/rtl/keymgr_kmac_if.sv
@@ -351,4 +351,8 @@
// request entropy to churn whenever a transaction is accepted
assign prng_en_o = kmac_data_o.valid & kmac_data_i.ready;
+ // as long as we are transmitting, the strobe should never be 0.
+ `ASSERT(LastStrb_A, valid |-> strb != '0)
+
+
endmodule // keymgr_kmac_if
diff --git a/hw/ip/keymgr/rtl/keymgr_pkg.sv b/hw/ip/keymgr/rtl/keymgr_pkg.sv
index 522d396..aee9699 100644
--- a/hw/ip/keymgr/rtl/keymgr_pkg.sv
+++ b/hw/ip/keymgr/rtl/keymgr_pkg.sv
@@ -168,7 +168,6 @@
KeyUpdateRandom,
KeyUpdateRoot,
KeyUpdateKmac,
- KeyUpdateInvalid,
KeyUpdateWipe
} keymgr_key_update_e;
diff --git a/hw/ip/keymgr/rtl/keymgr_reg_pkg.sv b/hw/ip/keymgr/rtl/keymgr_reg_pkg.sv
index c256fe1..21ffbb0 100644
--- a/hw/ip/keymgr/rtl/keymgr_reg_pkg.sv
+++ b/hw/ip/keymgr/rtl/keymgr_reg_pkg.sv
@@ -124,6 +124,33 @@
} keymgr_reg2hw_err_code_reg_t;
typedef struct packed {
+ struct packed {
+ logic q;
+ } cmd;
+ struct packed {
+ logic q;
+ } kmac_fsm;
+ struct packed {
+ logic q;
+ } kmac_op;
+ struct packed {
+ logic q;
+ } kmac_out;
+ struct packed {
+ logic q;
+ } regfile_intg;
+ struct packed {
+ logic q;
+ } shadow;
+ struct packed {
+ logic q;
+ } ctrl_fsm_intg;
+ struct packed {
+ logic q;
+ } ctrl_fsm_cnt;
+ } keymgr_reg2hw_fault_status_reg_t;
+
+ typedef struct packed {
logic d;
logic de;
} keymgr_hw2reg_intr_state_reg_t;
@@ -219,22 +246,23 @@
// Register -> HW type
typedef struct packed {
- keymgr_reg2hw_intr_state_reg_t intr_state; // [936:936]
- keymgr_reg2hw_intr_enable_reg_t intr_enable; // [935:935]
- keymgr_reg2hw_intr_test_reg_t intr_test; // [934:933]
- keymgr_reg2hw_alert_test_reg_t alert_test; // [932:929]
- keymgr_reg2hw_control_reg_t control; // [928:921]
- keymgr_reg2hw_sideload_clear_reg_t sideload_clear; // [920:918]
- keymgr_reg2hw_reseed_interval_shadowed_reg_t reseed_interval_shadowed; // [917:902]
- keymgr_reg2hw_sw_binding_regwen_reg_t sw_binding_regwen; // [901:900]
- keymgr_reg2hw_sealing_sw_binding_mreg_t [7:0] sealing_sw_binding; // [899:644]
- keymgr_reg2hw_attest_sw_binding_mreg_t [7:0] attest_sw_binding; // [643:388]
- keymgr_reg2hw_salt_mreg_t [7:0] salt; // [387:132]
- keymgr_reg2hw_key_version_mreg_t [0:0] key_version; // [131:100]
- keymgr_reg2hw_max_creator_key_ver_shadowed_reg_t max_creator_key_ver_shadowed; // [99:68]
- keymgr_reg2hw_max_owner_int_key_ver_shadowed_reg_t max_owner_int_key_ver_shadowed; // [67:36]
- keymgr_reg2hw_max_owner_key_ver_shadowed_reg_t max_owner_key_ver_shadowed; // [35:4]
- keymgr_reg2hw_err_code_reg_t err_code; // [3:0]
+ keymgr_reg2hw_intr_state_reg_t intr_state; // [944:944]
+ keymgr_reg2hw_intr_enable_reg_t intr_enable; // [943:943]
+ keymgr_reg2hw_intr_test_reg_t intr_test; // [942:941]
+ keymgr_reg2hw_alert_test_reg_t alert_test; // [940:937]
+ keymgr_reg2hw_control_reg_t control; // [936:929]
+ keymgr_reg2hw_sideload_clear_reg_t sideload_clear; // [928:926]
+ keymgr_reg2hw_reseed_interval_shadowed_reg_t reseed_interval_shadowed; // [925:910]
+ keymgr_reg2hw_sw_binding_regwen_reg_t sw_binding_regwen; // [909:908]
+ keymgr_reg2hw_sealing_sw_binding_mreg_t [7:0] sealing_sw_binding; // [907:652]
+ keymgr_reg2hw_attest_sw_binding_mreg_t [7:0] attest_sw_binding; // [651:396]
+ keymgr_reg2hw_salt_mreg_t [7:0] salt; // [395:140]
+ keymgr_reg2hw_key_version_mreg_t [0:0] key_version; // [139:108]
+ keymgr_reg2hw_max_creator_key_ver_shadowed_reg_t max_creator_key_ver_shadowed; // [107:76]
+ keymgr_reg2hw_max_owner_int_key_ver_shadowed_reg_t max_owner_int_key_ver_shadowed; // [75:44]
+ keymgr_reg2hw_max_owner_key_ver_shadowed_reg_t max_owner_key_ver_shadowed; // [43:12]
+ keymgr_reg2hw_err_code_reg_t err_code; // [11:8]
+ keymgr_reg2hw_fault_status_reg_t fault_status; // [7:0]
} keymgr_reg2hw_t;
// HW -> register type
diff --git a/hw/ip/keymgr/rtl/keymgr_reg_top.sv b/hw/ip/keymgr/rtl/keymgr_reg_top.sv
index 038ce9e..fd3a6d9 100644
--- a/hw/ip/keymgr/rtl/keymgr_reg_top.sv
+++ b/hw/ip/keymgr/rtl/keymgr_reg_top.sv
@@ -2089,7 +2089,7 @@
// to internal hardware
.qe (),
- .q (),
+ .q (reg2hw.fault_status.cmd.q),
// to register interface (read)
.qs (fault_status_cmd_qs)
@@ -2115,7 +2115,7 @@
// to internal hardware
.qe (),
- .q (),
+ .q (reg2hw.fault_status.kmac_fsm.q),
// to register interface (read)
.qs (fault_status_kmac_fsm_qs)
@@ -2141,7 +2141,7 @@
// to internal hardware
.qe (),
- .q (),
+ .q (reg2hw.fault_status.kmac_op.q),
// to register interface (read)
.qs (fault_status_kmac_op_qs)
@@ -2167,7 +2167,7 @@
// to internal hardware
.qe (),
- .q (),
+ .q (reg2hw.fault_status.kmac_out.q),
// to register interface (read)
.qs (fault_status_kmac_out_qs)
@@ -2193,7 +2193,7 @@
// to internal hardware
.qe (),
- .q (),
+ .q (reg2hw.fault_status.regfile_intg.q),
// to register interface (read)
.qs (fault_status_regfile_intg_qs)
@@ -2219,7 +2219,7 @@
// to internal hardware
.qe (),
- .q (),
+ .q (reg2hw.fault_status.shadow.q),
// to register interface (read)
.qs (fault_status_shadow_qs)
@@ -2245,7 +2245,7 @@
// to internal hardware
.qe (),
- .q (),
+ .q (reg2hw.fault_status.ctrl_fsm_intg.q),
// to register interface (read)
.qs (fault_status_ctrl_fsm_intg_qs)
@@ -2271,7 +2271,7 @@
// to internal hardware
.qe (),
- .q (),
+ .q (reg2hw.fault_status.ctrl_fsm_cnt.q),
// to register interface (read)
.qs (fault_status_ctrl_fsm_cnt_qs)
