[keymgr] Drop sw_binding_en during normal transactions
- Address #4564
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 04af7ff..73bedad 100644
--- a/hw/ip/keymgr/data/keymgr.hjson
+++ b/hw/ip/keymgr/data/keymgr.hjson
@@ -358,7 +358,9 @@
{ name: "SW_BINDING_EN",
desc: "Register write enable for SOFTWARE_BINDING",
swaccess: "rw0c",
- hwaccess: "hwo",
+ hwaccess: "hrw",
+ hwext: "true",
+ hwqe: "true",
fields: [
{ bits: "0",
name: "EN",
@@ -376,7 +378,7 @@
{ multireg: {
cname: "KEYMGR",
name: "SW_BINDING",
- regwen: "SW_BINDING_EN",
+ regwen: "CFGEN",
desc: '''
Software binding input to key manager.
This register is lockable and shared between key manager stages.
diff --git a/hw/ip/keymgr/rtl/keymgr.sv b/hw/ip/keymgr/rtl/keymgr.sv
index cecbd43..292538e 100644
--- a/hw/ip/keymgr/rtl/keymgr.sv
+++ b/hw/ip/keymgr/rtl/keymgr.sv
@@ -218,7 +218,9 @@
assign hw2reg.working_state.de = 1'b1;
// key manager registers cannot be changed once an operation starts
- keymgr_cfg_en u_cfgen (
+ keymgr_cfg_en #(
+ .NonInitClr(1'b1) // clear has effect even when non-init
+ ) u_cfgen (
.clk_i,
.rst_ni,
.init_i(init),
@@ -228,9 +230,33 @@
.out_o(hw2reg.cfgen.d)
);
- // we can only unlock on a successful advance call
- assign hw2reg.sw_binding_en.d = reg2hw.control.operation.q == OpAdvance;
- assign hw2reg.sw_binding_en.de = sw_binding_unlock;
+
+ logic sw_binding_set;
+ logic sw_binding_clr;
+ logic sw_binding_en;
+
+ // set on a successful advance
+ assign sw_binding_set = reg2hw.control.operation.q == OpAdvance &
+ sw_binding_unlock;
+
+ // this is w0c
+ assign sw_binding_clr = reg2hw.sw_binding_en.qe & ~reg2hw.sw_binding_en.q;
+
+ // software clears the enable
+ // hardware restores it upon successful advance
+ keymgr_cfg_en #(
+ .NonInitClr(1'b0) // clear has no effect until init
+ ) u_sw_binding_en (
+ .clk_i,
+ .rst_ni,
+ .init_i(init),
+ .en_i(lc_keymgr_en[KeyMgrEnSwBindingEn] == lc_ctrl_pkg::On),
+ .set_i(sw_binding_set),
+ .clr_i(sw_binding_clr),
+ .out_o(sw_binding_en)
+ );
+
+ assign hw2reg.sw_binding_en.d = sw_binding_en & hw2reg.cfgen.d;
/////////////////////////////////////
// Key Manager Input Construction
diff --git a/hw/ip/keymgr/rtl/keymgr_cfg_en.sv b/hw/ip/keymgr/rtl/keymgr_cfg_en.sv
index a819e97..9066f5c 100644
--- a/hw/ip/keymgr/rtl/keymgr_cfg_en.sv
+++ b/hw/ip/keymgr/rtl/keymgr_cfg_en.sv
@@ -7,7 +7,10 @@
`include "prim_assert.sv"
-module keymgr_cfg_en (
+module keymgr_cfg_en #(
+ // controls whether clear has an effect on output value during non-init
+ parameter bit NonInitClr = 1'b1
+) (
input clk_i,
input rst_ni,
input init_i,
@@ -20,9 +23,19 @@
logic out_q;
logic init_q;
+ logic vld_clr;
+ logic vld_set;
+ logic vld_dis;
+
+ assign vld_clr = init_q && clr_i;
+ assign vld_set = init_q && set_i;
+ assign vld_dis = init_q && !en_i;
+
// the same cycle where clear is asserted should already block future
// configuration
- assign out_o = ~clr_i & out_q & en_i;
+ logic out_clr;
+ assign out_clr = NonInitClr ? clr_i : vld_clr;
+ assign out_o = ~out_clr & out_q & en_i;
always_ff @(posedge clk_i or negedge rst_ni) begin
if (!rst_ni) begin
@@ -38,11 +51,11 @@
always_ff @(posedge clk_i or negedge rst_ni) begin
if (!rst_ni) begin
out_q <= 1'b1;
- end else if (init_q && !en_i) begin
+ end else if (vld_dis) begin
out_q <= 1'b0;
- end else if (init_q && set_i) begin
+ end else if (vld_set) begin
out_q <= 1'b1;
- end else if (init_q && clr_i) begin
+ end else if (vld_clr) begin
out_q <= 1'b0;
end
end
diff --git a/hw/ip/keymgr/rtl/keymgr_pkg.sv b/hw/ip/keymgr/rtl/keymgr_pkg.sv
index 8fed228..ead7815 100644
--- a/hw/ip/keymgr/rtl/keymgr_pkg.sv
+++ b/hw/ip/keymgr/rtl/keymgr_pkg.sv
@@ -186,6 +186,7 @@
typedef enum logic [1:0] {
KeyMgrEnCtrl,
KeyMgrEnCfgEn,
+ KeyMgrEnSwBindingEn,
KeyMgrEnLast
} keymgr_lc_en_usage_e;
diff --git a/hw/ip/keymgr/rtl/keymgr_reg_pkg.sv b/hw/ip/keymgr/rtl/keymgr_reg_pkg.sv
index 947ed83..594779d 100644
--- a/hw/ip/keymgr/rtl/keymgr_reg_pkg.sv
+++ b/hw/ip/keymgr/rtl/keymgr_reg_pkg.sv
@@ -60,6 +60,11 @@
} keymgr_reg2hw_reseed_interval_reg_t;
typedef struct packed {
+ logic q;
+ logic qe;
+ } keymgr_reg2hw_sw_binding_en_reg_t;
+
+ typedef struct packed {
logic [31:0] q;
} keymgr_reg2hw_sw_binding_mreg_t;
@@ -117,7 +122,6 @@
typedef struct packed {
logic d;
- logic de;
} keymgr_hw2reg_sw_binding_en_reg_t;
typedef struct packed {
@@ -164,13 +168,14 @@
// Register to internal design logic //
///////////////////////////////////////
typedef struct packed {
- keymgr_reg2hw_intr_state_reg_t intr_state; // [418:418]
- keymgr_reg2hw_intr_enable_reg_t intr_enable; // [417:417]
- keymgr_reg2hw_intr_test_reg_t intr_test; // [416:415]
- keymgr_reg2hw_alert_test_reg_t alert_test; // [414:411]
- keymgr_reg2hw_control_reg_t control; // [410:405]
- keymgr_reg2hw_sideload_clear_reg_t sideload_clear; // [404:404]
- keymgr_reg2hw_reseed_interval_reg_t reseed_interval; // [403:388]
+ keymgr_reg2hw_intr_state_reg_t intr_state; // [420:420]
+ keymgr_reg2hw_intr_enable_reg_t intr_enable; // [419:419]
+ keymgr_reg2hw_intr_test_reg_t intr_test; // [418:417]
+ keymgr_reg2hw_alert_test_reg_t alert_test; // [416:413]
+ keymgr_reg2hw_control_reg_t control; // [412:407]
+ keymgr_reg2hw_sideload_clear_reg_t sideload_clear; // [406:406]
+ keymgr_reg2hw_reseed_interval_reg_t reseed_interval; // [405:390]
+ keymgr_reg2hw_sw_binding_en_reg_t sw_binding_en; // [389:388]
keymgr_reg2hw_sw_binding_mreg_t [3:0] sw_binding; // [387:260]
keymgr_reg2hw_salt_mreg_t [3:0] salt; // [259:132]
keymgr_reg2hw_key_version_mreg_t [0:0] key_version; // [131:100]
@@ -184,10 +189,10 @@
// Internal design logic to register //
///////////////////////////////////////
typedef struct packed {
- keymgr_hw2reg_intr_state_reg_t intr_state; // [549:548]
- keymgr_hw2reg_cfgen_reg_t cfgen; // [547:547]
- keymgr_hw2reg_control_reg_t control; // [546:545]
- keymgr_hw2reg_sw_binding_en_reg_t sw_binding_en; // [544:543]
+ keymgr_hw2reg_intr_state_reg_t intr_state; // [548:547]
+ keymgr_hw2reg_cfgen_reg_t cfgen; // [546:546]
+ keymgr_hw2reg_control_reg_t control; // [545:544]
+ keymgr_hw2reg_sw_binding_en_reg_t sw_binding_en; // [543:543]
keymgr_hw2reg_sw_share0_output_mreg_t [7:0] sw_share0_output; // [542:279]
keymgr_hw2reg_sw_share1_output_mreg_t [7:0] sw_share1_output; // [278:15]
keymgr_hw2reg_working_state_reg_t working_state; // [14:11]
diff --git a/hw/ip/keymgr/rtl/keymgr_reg_top.sv b/hw/ip/keymgr/rtl/keymgr_reg_top.sv
index 28fba8d..c162830 100644
--- a/hw/ip/keymgr/rtl/keymgr_reg_top.sv
+++ b/hw/ip/keymgr/rtl/keymgr_reg_top.sv
@@ -103,6 +103,7 @@
logic sw_binding_en_qs;
logic sw_binding_en_wd;
logic sw_binding_en_we;
+ logic sw_binding_en_re;
logic [31:0] sw_binding_0_qs;
logic [31:0] sw_binding_0_wd;
logic sw_binding_0_we;
@@ -466,29 +467,18 @@
);
- // R[sw_binding_en]: V(False)
+ // R[sw_binding_en]: V(True)
- prim_subreg #(
- .DW (1),
- .SWACCESS("W0C"),
- .RESVAL (1'h1)
+ prim_subreg_ext #(
+ .DW (1)
) u_sw_binding_en (
- .clk_i (clk_i ),
- .rst_ni (rst_ni ),
-
- // from register interface
+ .re (sw_binding_en_re),
.we (sw_binding_en_we),
.wd (sw_binding_en_wd),
-
- // from internal hardware
- .de (hw2reg.sw_binding_en.de),
- .d (hw2reg.sw_binding_en.d ),
-
- // to internal hardware
- .qe (),
- .q (),
-
- // to register interface (read)
+ .d (hw2reg.sw_binding_en.d),
+ .qre (),
+ .qe (reg2hw.sw_binding_en.qe),
+ .q (reg2hw.sw_binding_en.q ),
.qs (sw_binding_en_qs)
);
@@ -506,7 +496,7 @@
.rst_ni (rst_ni ),
// from register interface (qualified with register enable)
- .we (sw_binding_0_we & sw_binding_en_qs),
+ .we (sw_binding_0_we & cfgen_qs),
.wd (sw_binding_0_wd),
// from internal hardware
@@ -533,7 +523,7 @@
.rst_ni (rst_ni ),
// from register interface (qualified with register enable)
- .we (sw_binding_1_we & sw_binding_en_qs),
+ .we (sw_binding_1_we & cfgen_qs),
.wd (sw_binding_1_wd),
// from internal hardware
@@ -560,7 +550,7 @@
.rst_ni (rst_ni ),
// from register interface (qualified with register enable)
- .we (sw_binding_2_we & sw_binding_en_qs),
+ .we (sw_binding_2_we & cfgen_qs),
.wd (sw_binding_2_wd),
// from internal hardware
@@ -587,7 +577,7 @@
.rst_ni (rst_ni ),
// from register interface (qualified with register enable)
- .we (sw_binding_3_we & sw_binding_en_qs),
+ .we (sw_binding_3_we & cfgen_qs),
.wd (sw_binding_3_wd),
// from internal hardware
@@ -1633,6 +1623,7 @@
assign sw_binding_en_we = addr_hit[8] & reg_we & ~wr_err;
assign sw_binding_en_wd = reg_wdata[0];
+ assign sw_binding_en_re = addr_hit[8] && reg_re;
assign sw_binding_0_we = addr_hit[9] & reg_we & ~wr_err;
assign sw_binding_0_wd = reg_wdata[31:0];
