Skip to content

Commit

Permalink
[keymgr,keymgr_dpe] Change reseed counter to count PRNG updates inste…
Browse files Browse the repository at this point in the history
…ad of cycles

Previously, the reseed counter was counting clock cycles which was
problematic for the following reasons:
- Once started, the counter couldn't be stopped anymore and keymgr would
  repeatedly request entropy even if being completely idle. This is bad
  for power.
- The number of clock cycles since the last reseed operation is no good
  indication for the seed lifetime.

Therefore, this commit changes the design to only advance the counter
when the PRNG is updated. Such updates only happen while keymgr is busy
and the number of PRNG updates is a more accurate measure for the seed
lifetime.

Co-authored-by: Andreas Kurth <[email protected]>
Signed-off-by: Pirmin Vogel <[email protected]>
  • Loading branch information
vogelpi and andreaskurth committed May 7, 2024
1 parent 2266a06 commit ed7f61a
Show file tree
Hide file tree
Showing 11 changed files with 78 additions and 29 deletions.
2 changes: 1 addition & 1 deletion hw/ip/keymgr/data/keymgr.hjson
Original file line number Diff line number Diff line change
Expand Up @@ -669,7 +669,7 @@
name: "VAL",
resval: "0x100"
desc: '''
Number of key manager cycles before the entropy is reseeded
Number of internal PRNG updates before a reseed is requested.
'''
},
]
Expand Down
42 changes: 31 additions & 11 deletions hw/ip/keymgr/dv/env/keymgr_if.sv
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,9 @@ interface keymgr_if(input clk, input rst_n);
// connect EDN for assertion check
wire edn_clk, edn_rst_n, edn_req, edn_ack;

// connect to PRNG to track PRNG updates for EDN assertion
wire lfsr_en;

// keymgr_en is async, create a sync one for use in scb
lc_ctrl_pkg::lc_tx_t keymgr_en_sync1, keymgr_en_sync2;

Expand Down Expand Up @@ -72,12 +75,15 @@ interface keymgr_if(input clk, input rst_n);
// keymgr will request edn twice for 64 bit data each time, use this to indicate if it's first or
// second req. 0: wait for 1st req, 1: for 2nd
bit edn_req_cnt;
bit edn_wait_cnt_incr;
bit edn_wait_cnt_clr;
int edn_wait_cnt;
int edn_interval;
// synchronize req/ack from async domain
bit edn_req_sync;
bit edn_req_ack_sync;
bit edn_req_ack_sync_done;
bit edn_req_ack_sync_done_qq;

// If we need to wait for internal signal to be certain value, we may not be able to get that
// when the sim is close to end. Define a cnt and MaxWaitCycle to avoid sim hang
Expand All @@ -90,7 +96,7 @@ interface keymgr_if(input clk, input rst_n);

string msg_id = "keymgr_if";

int edn_tolerance_cycs = 20;
int edn_tolerance_upd = 20;

task automatic init(bit rand_otp_key, bit invalid_otp_key);
// Keymgr only latches OTP key once, so this scb does not support change OTP key on the
Expand Down Expand Up @@ -355,8 +361,8 @@ interface keymgr_if(input clk, input rst_n);
otbn_sideload_status <= SideLoadClear;
endfunction

function automatic void update_edn_toleranc_cycs(int edn_clk, int main_clk);
if ((main_clk/edn_clk) * 10 > edn_tolerance_cycs) edn_tolerance_cycs = (main_clk/edn_clk) * 10;
function automatic void update_edn_tolerance_upd(int edn_clk, int main_clk);
if ((main_clk/edn_clk) * 10 > edn_tolerance_upd) edn_tolerance_upd = (main_clk/edn_clk) * 10;
endfunction

logic valid_done_window;
Expand Down Expand Up @@ -649,13 +655,27 @@ interface keymgr_if(input clk, input rst_n);
end
end

always_ff @(posedge clk or negedge rst_n) begin
if (!rst_n) begin
edn_req_ack_sync_done_qq <= 1'b0;
end else begin
edn_req_ack_sync_done_qq <= edn_req_ack_sync_done;
end
end

// Increment the counter for PRNG updates happening unless we're waiting for EDN.
assign edn_wait_cnt_incr = lfsr_en && (!edn_req_sync || (edn_req_sync && edn_req_ack_sync));

// Clear the counter upon rising edges of the synchronized req_ack_done signal.
assign edn_wait_cnt_clr = edn_req_ack_sync_done & !edn_req_ack_sync_done_qq;

always @(posedge clk or negedge rst_n) begin
if (!rst_n) begin
edn_wait_cnt <= 0;
end else if (!edn_req_sync) begin
edn_wait_cnt <= edn_wait_cnt + 1;
end else begin
end else if (edn_wait_cnt_clr) begin
edn_wait_cnt <= 0;
end else if (edn_wait_cnt_incr) begin
edn_wait_cnt <= edn_wait_cnt + 1;
end
end

Expand All @@ -671,16 +691,16 @@ interface keymgr_if(input clk, input rst_n);
end
end

// consider async handshaking and a few cycles to start the req. allow no more than
// `edn_tolerance_cycs` tolerance error on the cnt.
// `edn_tolerance_cycs` default value is 20, but if the frequency difference between edn and main
// consider async handshaking and a few PRNG update requests to start the req. allow no more than
// `edn_tolerance_upd` tolerance error on the cnt.
// `edn_tolerance_upd` default value is 20, but if the frequency difference between edn and main
// clock is too big, the testbench will scale it up to a larger value.
`ASSERT(CheckEdn1stReq, $rose(edn_req_sync) && edn_req_cnt == 0 && start_edn_req |->
(edn_wait_cnt > edn_interval) && (edn_wait_cnt - edn_interval < edn_tolerance_cycs),
(edn_wait_cnt >= edn_interval) && (edn_wait_cnt - edn_interval < edn_tolerance_upd),
clk, !rst_n || !en_chk)

`ASSERT(CheckEdn2ndReq, $rose(edn_req_sync) && edn_req_cnt == 1 |->
edn_wait_cnt < edn_tolerance_cycs,
edn_wait_cnt < edn_tolerance_upd,
clk, !rst_n || !en_chk)

`undef ASSERT_IFF_KEYMGR_LEGAL
Expand Down
2 changes: 1 addition & 1 deletion hw/ip/keymgr/dv/env/seq_lib/keymgr_base_vseq.sv
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ class keymgr_base_vseq extends cip_base_vseq #(
virtual task dut_init(string reset_kind = "HARD");
super.dut_init();

cfg.keymgr_vif.update_edn_toleranc_cycs(cfg.edn_clk_freq_mhz, cfg.clk_freq_mhz);
cfg.keymgr_vif.update_edn_tolerance_upd(cfg.edn_clk_freq_mhz, cfg.clk_freq_mhz);
op_before_enable_keymgr();

cfg.keymgr_vif.init(do_rand_otp_key, do_invalid_otp_key);
Expand Down
1 change: 1 addition & 0 deletions hw/ip/keymgr/dv/tb.sv
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ module tb;
assign keymgr_if.edn_rst_n = edn_if[0].rst_n;
assign keymgr_if.edn_req = edn_if[0].req;
assign keymgr_if.edn_ack = edn_if[0].ack;
assign keymgr_if.lfsr_en = dut.lfsr_en;

// dut
keymgr #(
Expand Down
5 changes: 4 additions & 1 deletion hw/ip/keymgr/rtl/keymgr.sv
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,7 @@ module keymgr
// The second case is less sensitive and is applied directly. If the inputs
// have more bits than the lfsr output, the lfsr value is simply replicated

logic lfsr_en;
logic seed_en;
logic [LfsrWidth-1:0] seed;
logic reseed_req;
Expand All @@ -199,13 +200,15 @@ module keymgr
.reseed_interval_i(reg2hw.reseed_interval_shadowed.q),
.edn_o,
.edn_i,
.lfsr_en_i(lfsr_en),
.seed_en_o(seed_en),
.seed_o(seed),
.cnt_err_o(reseed_cnt_err)
);

logic [63:0] lfsr;
logic ctrl_lfsr_en, data_lfsr_en, sideload_lfsr_en;
assign lfsr_en = ctrl_lfsr_en | data_lfsr_en | sideload_lfsr_en;

prim_lfsr #(
.LfsrDw(LfsrWidth),
Expand All @@ -217,7 +220,7 @@ module keymgr
) u_lfsr (
.clk_i,
.rst_ni,
.lfsr_en_i(ctrl_lfsr_en | data_lfsr_en | sideload_lfsr_en),
.lfsr_en_i(lfsr_en),
.seed_en_i(seed_en),
.seed_i(seed),
.entropy_i('0),
Expand Down
3 changes: 2 additions & 1 deletion hw/ip/keymgr/rtl/keymgr_reseed_ctrl.sv
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ module keymgr_reseed_ctrl import keymgr_pkg::*; (
input edn_pkg::edn_rsp_t edn_i,

// interface to lfsr
input logic lfsr_en_i,
output logic seed_en_o,
output logic [LfsrWidth-1:0] seed_o,

Expand Down Expand Up @@ -97,7 +98,7 @@ module keymgr_reseed_ctrl import keymgr_pkg::*; (
.clr_i(edn_done),
.set_i('0),
.set_cnt_i('0),
.incr_en_i(cnt_en),
.incr_en_i(cnt_en & lfsr_en_i),
.decr_en_i(1'b0),
.step_i(16'h1),
.commit_i(1'b1),
Expand Down
2 changes: 1 addition & 1 deletion hw/ip/keymgr_dpe/data/keymgr_dpe.hjson
Original file line number Diff line number Diff line change
Expand Up @@ -621,7 +621,7 @@ countermeasures: [
name: "VAL",
resval: "0x100"
desc: '''
Number of key manager cycles before the entropy is reseeded
Number of internal PRNG updates before a reseed is requested.
'''
},
]
Expand Down
42 changes: 31 additions & 11 deletions hw/ip/keymgr_dpe/dv/env/keymgr_dpe_if.sv
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,9 @@ interface keymgr_dpe_if(input clk, input rst_n);
// connect EDN for assertion check
wire edn_clk, edn_rst_n, edn_req, edn_ack;

// connect to PRNG to track PRNG updates for EDN assertion
wire lfsr_en;

// keymgr_dpe_en is async, create a sync one for use in scb
lc_ctrl_pkg::lc_tx_t keymgr_dpe_en_sync1, keymgr_dpe_en_sync2;

Expand Down Expand Up @@ -74,12 +77,15 @@ interface keymgr_dpe_if(input clk, input rst_n);
// use this to indicate if it's first or
// second req. 0: wait for 1st req, 1: for 2nd
bit edn_req_cnt;
bit edn_wait_cnt_incr;
bit edn_wait_cnt_clr;
int edn_wait_cnt;
int edn_interval;
// synchronize req/ack from async domain
bit edn_req_sync;
bit edn_req_ack_sync;
bit edn_req_ack_sync_done;
bit edn_req_ack_sync_done_qq;

// If we need to wait for internal signal to be certain value, we may not be able to get that
// when the sim is close to end. Define a cnt and MaxWaitCycle to avoid sim hang
Expand All @@ -92,7 +98,7 @@ interface keymgr_dpe_if(input clk, input rst_n);

string msg_id = "keymgr_dpe_if";

int edn_tolerance_cycs = 20;
int edn_tolerance_upd = 20;

// assigned from the keymgr_dpe.keymgr_dpe_ctrl.key_slots_q signal, which should hold the
// current value of the keyslots in the dut.
Expand Down Expand Up @@ -385,8 +391,8 @@ interface keymgr_dpe_if(input clk, input rst_n);
otbn_sideload_status <= SideLoadClear;
endfunction

function automatic void update_edn_toleranc_cycs(int edn_clk, int main_clk);
if ((main_clk/edn_clk) * 10 > edn_tolerance_cycs) edn_tolerance_cycs = (main_clk/edn_clk) * 10;
function automatic void update_edn_tolerance_upd(int edn_clk, int main_clk);
if ((main_clk/edn_clk) * 10 > edn_tolerance_upd) edn_tolerance_upd = (main_clk/edn_clk) * 10;
endfunction

logic valid_done_window;
Expand Down Expand Up @@ -696,13 +702,27 @@ interface keymgr_dpe_if(input clk, input rst_n);
end
end

always_ff @(posedge clk or negedge rst_n) begin
if (!rst_n) begin
edn_req_ack_sync_done_qq <= 1'b0;
end else begin
edn_req_ack_sync_done_qq <= edn_req_ack_sync_done;
end
end

// Increment the counter for PRNG updates happening unless we're waiting for EDN.
assign edn_wait_cnt_incr = lfsr_en && (!edn_req_sync || (edn_req_sync && edn_req_ack_sync));

// Clear the counter upon rising edges of the synchronized req_ack_done signal.
assign edn_wait_cnt_clr = edn_req_ack_sync_done & !edn_req_ack_sync_done_qq;

always @(posedge clk or negedge rst_n) begin
if (!rst_n) begin
edn_wait_cnt <= 0;
end else if (!edn_req_sync) begin
edn_wait_cnt <= edn_wait_cnt + 1;
end else begin
end else if (edn_wait_cnt_clr) begin
edn_wait_cnt <= 0;
end else if (edn_wait_cnt_incr) begin
edn_wait_cnt <= edn_wait_cnt + 1;
end
end

Expand All @@ -718,16 +738,16 @@ interface keymgr_dpe_if(input clk, input rst_n);
end
end

// consider async handshaking and a few cycles to start the req. allow no more than
// `edn_tolerance_cycs` tolerance error on the cnt.
// `edn_tolerance_cycs` default value is 20, but if the frequency difference between edn and main
// consider async handshaking and a few PRNG update requests to start the req. allow no more than
// `edn_tolerance_upd` tolerance error on the cnt.
// `edn_tolerance_upd` default value is 20, but if the frequency difference between edn and main
// clock is too big, the testbench will scale it up to a larger value.
`ASSERT(CheckEdn1stReq, $rose(edn_req_sync) && edn_req_cnt == 0 && start_edn_req |->
(edn_wait_cnt > edn_interval) && (edn_wait_cnt - edn_interval < edn_tolerance_cycs),
(edn_wait_cnt >= edn_interval) && (edn_wait_cnt - edn_interval < edn_tolerance_upd),
clk, !rst_n || !en_chk)

`ASSERT(CheckEdn2ndReq, $rose(edn_req_sync) && edn_req_cnt == 1 |->
edn_wait_cnt < edn_tolerance_cycs,
edn_wait_cnt < edn_tolerance_upd,
clk, !rst_n || !en_chk)

`undef ASSERT_IFF_KEYMGR_DPE_LEGAL
Expand Down
2 changes: 1 addition & 1 deletion hw/ip/keymgr_dpe/dv/env/seq_lib/keymgr_dpe_base_vseq.sv
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ class keymgr_dpe_base_vseq extends cip_base_vseq #(
virtual task dut_init(string reset_kind = "HARD");
super.dut_init();

cfg.keymgr_dpe_vif.update_edn_toleranc_cycs(cfg.edn_clk_freq_mhz, cfg.clk_freq_mhz);
cfg.keymgr_dpe_vif.update_edn_tolerance_upd(cfg.edn_clk_freq_mhz, cfg.clk_freq_mhz);
op_before_enable_keymgr();

cfg.keymgr_dpe_vif.init(do_rand_otp_key, do_invalid_otp_key);
Expand Down
1 change: 1 addition & 0 deletions hw/ip/keymgr_dpe/dv/tb.sv
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ module tb;
assign keymgr_dpe_if.edn_rst_n = edn_if[0].rst_n;
assign keymgr_dpe_if.edn_req = edn_if[0].req;
assign keymgr_dpe_if.edn_ack = edn_if[0].ack;
assign keymgr_dpe_if.lfsr_en = dut.lfsr_en;

// dut
keymgr_dpe #(
Expand Down
5 changes: 4 additions & 1 deletion hw/ip/keymgr_dpe/rtl/keymgr_dpe.sv
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,7 @@ module keymgr_dpe
// The second case is less sensitive and is applied directly. If the inputs
// have more bits than the lfsr output, the lfsr value is simply replicated

logic lfsr_en;
logic seed_en;
logic [LfsrWidth-1:0] seed;
logic reseed_req;
Expand All @@ -190,13 +191,15 @@ module keymgr_dpe
.reseed_interval_i(reg2hw.reseed_interval_shadowed.q),
.edn_o,
.edn_i,
.lfsr_en_i(lfsr_en),
.seed_en_o(seed_en),
.seed_o(seed),
.cnt_err_o(reseed_cnt_err)
);

logic [63:0] lfsr;
logic ctrl_lfsr_en, data_lfsr_en, sideload_lfsr_en;
assign lfsr_en = ctrl_lfsr_en | data_lfsr_en | sideload_lfsr_en;

prim_lfsr #(
.LfsrDw(LfsrWidth),
Expand All @@ -208,7 +211,7 @@ module keymgr_dpe
) u_lfsr (
.clk_i,
.rst_ni,
.lfsr_en_i(ctrl_lfsr_en | data_lfsr_en | sideload_lfsr_en),
.lfsr_en_i(lfsr_en),
.seed_en_i(seed_en),
.seed_i(seed),
.entropy_i('0),
Expand Down

0 comments on commit ed7f61a

Please sign in to comment.