Skip to content

Commit 2893549

Browse files
vogelpivogelpi
committed
[rtl] Protect core_busy_o with a multi-bit encoding
This commit protects the core_busy_o signal using a multi-bit encoding to reduce the chances of an adversary for glitching this signal to low, thereby putting the core to sleep and e.g. not handling an alert. Without this commit, the glitch would only be detected once both the main core and the shadow core wake up again and the comparison of the core_busy_o signals continues. This resolves #1827. Signed-off-by: Pirmin Vogel <[email protected]>
1 parent f385d4d commit 2893549

13 files changed

+148
-104
lines changed

‎doc/02_user/integration.rst

+1-1
Original file line numberDiff line numberDiff line change
@@ -215,7 +215,7 @@ Interfaces
215215
| | | | instructions in the ID/EX and WB |
216216
| | | | stages have finished. A multi-bit |
217217
| | | | encoding scheme is used. See |
218-
| | | | `FetchEnableOn` / `FetchEnableOff` in |
218+
| | | | `IbexMuBiOn` / `IbexMuBiOff` in |
219219
| | | | :file:`rtl/ibex_pkg.sv` |
220220
+----------------------------+-------------------------+-----+----------------------------------------+
221221
| ``core_sleep_o`` | 1 | out | Core in WFI with no outstanding data |

‎dv/riscv_compliance/rtl/ibex_riscv_compliance.sv

+48-48
Original file line numberDiff line numberDiff line change
@@ -158,57 +158,57 @@ module ibex_riscv_compliance (
158158
.DmHaltAddr (32'h00000000 ),
159159
.DmExceptionAddr (32'h00000000 )
160160
) u_top (
161-
.clk_i (clk_sys ),
162-
.rst_ni (rst_sys_n ),
161+
.clk_i (clk_sys ),
162+
.rst_ni (rst_sys_n ),
163163

164-
.test_en_i ('b0 ),
165-
.scan_rst_ni (1'b1 ),
166-
.ram_cfg_i ('b0 ),
164+
.test_en_i ('b0 ),
165+
.scan_rst_ni (1'b1 ),
166+
.ram_cfg_i ('b0 ),
167167

168-
.hart_id_i (32'b0 ),
168+
.hart_id_i (32'b0 ),
169169
// First instruction executed is at 0x0 + 0x80
170-
.boot_addr_i (32'h00000000 ),
171-
172-
.instr_req_o (host_req[CoreI] ),
173-
.instr_gnt_i (host_gnt[CoreI] ),
174-
.instr_rvalid_i (host_rvalid[CoreI] ),
175-
.instr_addr_o (host_addr[CoreI] ),
176-
.instr_rdata_i (host_rdata[CoreI] ),
177-
.instr_rdata_intg_i (ibex_instr_rdata_intg ),
178-
.instr_err_i (host_err[CoreI] ),
179-
180-
.data_req_o (host_req[CoreD] ),
181-
.data_gnt_i (host_gnt[CoreD] ),
182-
.data_rvalid_i (host_rvalid[CoreD] ),
183-
.data_we_o (host_we[CoreD] ),
184-
.data_be_o (host_be[CoreD] ),
185-
.data_addr_o (host_addr[CoreD] ),
186-
.data_wdata_o (host_wdata[CoreD] ),
187-
.data_wdata_intg_o ( ),
188-
.data_rdata_i (host_rdata[CoreD] ),
189-
.data_rdata_intg_i (ibex_data_rdata_intg ),
190-
.data_err_i (host_err[CoreD] ),
191-
192-
.irq_software_i (1'b0 ),
193-
.irq_timer_i (1'b0 ),
194-
.irq_external_i (1'b0 ),
195-
.irq_fast_i (15'b0 ),
196-
.irq_nm_i (1'b0 ),
197-
198-
.scramble_key_valid_i ('0 ),
199-
.scramble_key_i ('0 ),
200-
.scramble_nonce_i ('0 ),
201-
.scramble_req_o ( ),
202-
203-
.debug_req_i ('b0 ),
204-
.crash_dump_o ( ),
205-
.double_fault_seen_o ( ),
206-
207-
.fetch_enable_i (ibex_pkg::FetchEnableOn),
208-
.alert_minor_o ( ),
209-
.alert_major_internal_o ( ),
210-
.alert_major_bus_o ( ),
211-
.core_sleep_o ( )
170+
.boot_addr_i (32'h00000000 ),
171+
172+
.instr_req_o (host_req[CoreI] ),
173+
.instr_gnt_i (host_gnt[CoreI] ),
174+
.instr_rvalid_i (host_rvalid[CoreI] ),
175+
.instr_addr_o (host_addr[CoreI] ),
176+
.instr_rdata_i (host_rdata[CoreI] ),
177+
.instr_rdata_intg_i (ibex_instr_rdata_intg),
178+
.instr_err_i (host_err[CoreI] ),
179+
180+
.data_req_o (host_req[CoreD] ),
181+
.data_gnt_i (host_gnt[CoreD] ),
182+
.data_rvalid_i (host_rvalid[CoreD] ),
183+
.data_we_o (host_we[CoreD] ),
184+
.data_be_o (host_be[CoreD] ),
185+
.data_addr_o (host_addr[CoreD] ),
186+
.data_wdata_o (host_wdata[CoreD] ),
187+
.data_wdata_intg_o ( ),
188+
.data_rdata_i (host_rdata[CoreD] ),
189+
.data_rdata_intg_i (ibex_data_rdata_intg ),
190+
.data_err_i (host_err[CoreD] ),
191+
192+
.irq_software_i (1'b0 ),
193+
.irq_timer_i (1'b0 ),
194+
.irq_external_i (1'b0 ),
195+
.irq_fast_i (15'b0 ),
196+
.irq_nm_i (1'b0 ),
197+
198+
.scramble_key_valid_i ('0 ),
199+
.scramble_key_i ('0 ),
200+
.scramble_nonce_i ('0 ),
201+
.scramble_req_o ( ),
202+
203+
.debug_req_i ('b0 ),
204+
.crash_dump_o ( ),
205+
.double_fault_seen_o ( ),
206+
207+
.fetch_enable_i (ibex_pkg::IbexMuBiOn ),
208+
.alert_minor_o ( ),
209+
.alert_major_internal_o ( ),
210+
.alert_major_bus_o ( ),
211+
.core_sleep_o ( )
212212
);
213213

214214
// SRAM block for instruction and data storage

‎dv/uvm/core_ibex/env/core_ibex_dut_probe_if.sv

+1-1
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ interface core_ibex_dut_probe_if(input logic clk);
1111
logic ebreak;
1212
logic dret;
1313
logic mret;
14-
ibex_pkg::fetch_enable_t fetch_enable;
14+
ibex_pkg::ibex_mubi_t fetch_enable;
1515
logic core_sleep;
1616
logic alert_minor;
1717
logic alert_major_internal;

‎dv/uvm/core_ibex/tests/core_ibex_base_test.sv

+3-3
Original file line numberDiff line numberDiff line change
@@ -171,10 +171,10 @@ class core_ibex_base_test extends uvm_test;
171171
enable_irq_seq = cfg.enable_irq_single_seq || cfg.enable_irq_multiple_seq;
172172
phase.raise_objection(this);
173173
cur_run_phase = phase;
174-
dut_vif.dut_cb.fetch_enable <= ibex_pkg::FetchEnableOff;
174+
dut_vif.dut_cb.fetch_enable <= ibex_pkg::IbexMuBiOff;
175175
clk_vif.wait_clks(100);
176176
load_binary_to_mem();
177-
dut_vif.dut_cb.fetch_enable <= ibex_pkg::FetchEnableOn;
177+
dut_vif.dut_cb.fetch_enable <= ibex_pkg::IbexMuBiOn;
178178
send_stimulus();
179179
wait_for_test_done();
180180
cur_run_phase = null;
@@ -282,7 +282,7 @@ class core_ibex_base_test extends uvm_test;
282282
check_perf_stats();
283283
// De-assert fetch enable to finish the test
284284
clk_vif.wait_clks(10);
285-
dut_vif.dut_cb.fetch_enable <= ibex_pkg::FetchEnableOff;
285+
dut_vif.dut_cb.fetch_enable <= ibex_pkg::IbexMuBiOff;
286286
// Wait some time for the remaining instruction to finish
287287
clk_vif.wait_clks(3000);
288288
endtask

‎dv/uvm/core_ibex/tests/core_ibex_new_seq_lib.sv

+6-6
Original file line numberDiff line numberDiff line change
@@ -258,29 +258,29 @@ class fetch_enable_seq extends core_base_new_seq#(irq_seq_item);
258258
all_off_values = 0;
259259
end
260260

261-
dut_vif.dut_cb.fetch_enable <= ibex_pkg::FetchEnableOn;
261+
dut_vif.dut_cb.fetch_enable <= ibex_pkg::IbexMuBiOn;
262262
super.body();
263263
endtask: body
264264

265265
virtual task send_req();
266-
ibex_pkg::fetch_enable_t fetch_enable_off;
267-
int unsigned off_delay;
266+
ibex_pkg::ibex_mubi_t fetch_enable_off;
267+
int unsigned off_delay;
268268

269269
if (all_off_values) begin
270270
// Randomise the MUBI fetch_enable value to be one of the many possible off values
271271
`DV_CHECK_STD_RANDOMIZE_WITH_FATAL(fetch_enable_off,
272-
fetch_enable_off != ibex_pkg::FetchEnableOn;)
272+
fetch_enable_off != ibex_pkg::IbexMuBiOn;)
273273
end else begin
274274
// Otherwise use single fixed off value
275-
fetch_enable_off = ibex_pkg::FetchEnableOff;
275+
fetch_enable_off = ibex_pkg::IbexMuBiOff;
276276
end
277277

278278
`DV_CHECK_STD_RANDOMIZE_WITH_FATAL(off_delay,
279279
off_delay inside {[min_delay : max_delay]};)
280280

281281
dut_vif.dut_cb.fetch_enable <= fetch_enable_off;
282282
clk_vif.wait_clks(off_delay);
283-
dut_vif.dut_cb.fetch_enable <= ibex_pkg::FetchEnableOn;
283+
dut_vif.dut_cb.fetch_enable <= ibex_pkg::IbexMuBiOn;
284284

285285
endtask
286286

‎dv/uvm/core_ibex/tests/core_ibex_test_lib.sv

+2-2
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ class core_ibex_reset_test extends core_ibex_base_test;
2626
clk_vif.wait_clks($urandom_range(0, 50000));
2727
fork
2828
begin
29-
dut_vif.dut_cb.fetch_enable <= ibex_pkg::FetchEnableOff;
29+
dut_vif.dut_cb.fetch_enable <= ibex_pkg::IbexMuBiOff;
3030
clk_vif.apply_reset(.reset_width_clks (100));
3131
end
3232
begin
@@ -40,7 +40,7 @@ class core_ibex_reset_test extends core_ibex_base_test;
4040
end
4141
join
4242
// Assert fetch_enable to have the core start executing from boot address
43-
dut_vif.dut_cb.fetch_enable <= ibex_pkg::FetchEnableOn;
43+
dut_vif.dut_cb.fetch_enable <= ibex_pkg::IbexMuBiOn;
4444
end
4545
endtask
4646

‎examples/simple_system/rtl/ibex_simple_system.sv

+1-1
Original file line numberDiff line numberDiff line change
@@ -253,7 +253,7 @@ module ibex_simple_system (
253253
.crash_dump_o (),
254254
.double_fault_seen_o (),
255255

256-
.fetch_enable_i (ibex_pkg::FetchEnableOn),
256+
.fetch_enable_i (ibex_pkg::IbexMuBiOn),
257257
.alert_minor_o (),
258258
.alert_major_internal_o (),
259259
.alert_major_bus_o (),

‎rtl/ibex_core.sv

+36-13
Original file line numberDiff line numberDiff line change
@@ -147,11 +147,11 @@ module ibex_core import ibex_pkg::*; #(
147147

148148
// CPU Control Signals
149149
// SEC_CM: FETCH.CTRL.LC_GATED
150-
input fetch_enable_t fetch_enable_i,
150+
input ibex_mubi_t fetch_enable_i,
151151
output logic alert_minor_o,
152152
output logic alert_major_internal_o,
153153
output logic alert_major_bus_o,
154-
output logic core_busy_o
154+
output ibex_mubi_t core_busy_o
155155
);
156156

157157
localparam int unsigned PMPNumChan = 3;
@@ -368,7 +368,31 @@ module ibex_core import ibex_pkg::*; #(
368368

369369
// Before going to sleep, wait for I- and D-side
370370
// interfaces to finish ongoing operations.
371-
assign core_busy_o = ctrl_busy | if_busy | lsu_busy;
371+
if (SecureIbex) begin : g_core_busy_secure
372+
// For secure Ibex, the individual bits of core_busy_o are generated from different copies of
373+
// the various busy signal.
374+
localparam int unsigned NumBusySignals = 3;
375+
localparam int unsigned NumBusyBits = $bits(ibex_mubi_t) * NumBusySignals;
376+
logic [NumBusyBits-1:0] busy_bits_buf;
377+
prim_buf #(
378+
.Width(NumBusyBits)
379+
) u_fetch_enable_buf (
380+
.in_i ({$bits(ibex_mubi_t){ctrl_busy, if_busy, lsu_busy}}),
381+
.out_o(busy_bits_buf)
382+
);
383+
384+
// Set core_busy_o to IbexMuBiOn if even a single input is high.
385+
for (genvar i = 0; i < $bits(ibex_mubi_t); i++) begin : g_core_busy_bits
386+
if (IbexMuBiOn[i] == 1'b1) begin : g_pos
387+
assign core_busy_o[i] = |busy_bits_buf[i*NumBusySignals +: NumBusySignals];
388+
end else begin : g_neg
389+
assign core_busy_o[i] = ~|busy_bits_buf[i*NumBusySignals +: NumBusySignals];
390+
end
391+
end
392+
end else begin : g_core_busy_non_secure
393+
// For non secure Ibex, synthesis is allowed to optimize core_busy_o.
394+
assign core_busy_o = (ctrl_busy || if_busy || lsu_busy) ? IbexMuBiOn : IbexMuBiOff;
395+
end
372396

373397
//////////////
374398
// IF stage //
@@ -474,22 +498,21 @@ module ibex_core import ibex_pkg::*; #(
474498

475499
// Multi-bit fetch enable used when SecureIbex == 1. When SecureIbex == 0 only use the bottom-bit
476500
// of fetch_enable_i. Ensure the multi-bit encoding has the bottom bit set for on and unset for
477-
// off so FetchEnableOn/FetchEnableOff can be used without needing to know the value of
478-
// SecureIbex.
479-
`ASSERT_INIT(FetchEnableSecureOnBottomBitSet, FetchEnableOn[0] == 1'b1)
480-
`ASSERT_INIT(FetchEnableSecureOffBottomBitClear, FetchEnableOff[0] == 1'b0)
501+
// off so IbexMuBiOn/IbexMuBiOff can be used without needing to know the value of SecureIbex.
502+
`ASSERT_INIT(IbexMuBiSecureOnBottomBitSet, IbexMuBiOn[0] == 1'b1)
503+
`ASSERT_INIT(IbexMuBiSecureOffBottomBitClear, IbexMuBiOff[0] == 1'b0)
481504

482505
// fetch_enable_i can be used to stop the core fetching new instructions
483506
if (SecureIbex) begin : g_instr_req_gated_secure
484507
// For secure Ibex fetch_enable_i must be a specific multi-bit pattern to enable instruction
485508
// fetch
486509
// SEC_CM: FETCH.CTRL.LC_GATED
487-
assign instr_req_gated = instr_req_int & (fetch_enable_i == FetchEnableOn);
488-
assign instr_exec = fetch_enable_i == FetchEnableOn;
510+
assign instr_req_gated = instr_req_int & (fetch_enable_i == IbexMuBiOn);
511+
assign instr_exec = fetch_enable_i == IbexMuBiOn;
489512
end else begin : g_instr_req_gated_non_secure
490513
// For non secure Ibex only the bottom bit of fetch enable is considered
491514
logic unused_fetch_enable;
492-
assign unused_fetch_enable = ^fetch_enable_i[$bits(fetch_enable_t)-1:1];
515+
assign unused_fetch_enable = ^fetch_enable_i[$bits(ibex_mubi_t)-1:1];
493516

494517
assign instr_req_gated = instr_req_int & fetch_enable_i[0];
495518
assign instr_exec = fetch_enable_i[0];
@@ -931,7 +954,7 @@ module ibex_core import ibex_pkg::*; #(
931954

932955
// Keep track of the PC last seen in the ID stage when fetch is disabled
933956
logic [31:0] pc_at_fetch_disable;
934-
fetch_enable_t last_fetch_enable;
957+
ibex_mubi_t last_fetch_enable;
935958

936959
always_ff @(posedge clk_i or negedge rst_ni) begin
937960
if (!rst_ni) begin
@@ -940,7 +963,7 @@ module ibex_core import ibex_pkg::*; #(
940963
end else begin
941964
last_fetch_enable <= fetch_enable_i;
942965

943-
if ((fetch_enable_i != FetchEnableOn) && (last_fetch_enable == FetchEnableOn)) begin
966+
if ((fetch_enable_i != IbexMuBiOn) && (last_fetch_enable == IbexMuBiOn)) begin
944967
pc_at_fetch_disable <= pc_id;
945968
end
946969
end
@@ -949,7 +972,7 @@ module ibex_core import ibex_pkg::*; #(
949972
// When fetch is disabled no instructions should be executed. Once fetch is disabled either the
950973
// ID/EX stage is not valid or the PC of the ID/EX stage must remain as it was at disable. The
951974
// ID/EX valid should not ressert once it has been cleared.
952-
`ASSERT(NoExecWhenFetchEnableNotOn, fetch_enable_i != FetchEnableOn |=>
975+
`ASSERT(NoExecWhenFetchEnableNotOn, fetch_enable_i != IbexMuBiOn |=>
953976
(~instr_valid_id || (pc_id == pc_at_fetch_disable)) && ~$rose(instr_valid_id))
954977

955978
`endif

‎rtl/ibex_lockstep.sv

+4-4
Original file line numberDiff line numberDiff line change
@@ -96,11 +96,11 @@ module ibex_lockstep import ibex_pkg::*; #(
9696
input crash_dump_t crash_dump_i,
9797
input logic double_fault_seen_i,
9898

99-
input fetch_enable_t fetch_enable_i,
99+
input ibex_mubi_t fetch_enable_i,
100100
output logic alert_minor_o,
101101
output logic alert_major_internal_o,
102102
output logic alert_major_bus_o,
103-
input logic core_busy_i,
103+
input ibex_mubi_t core_busy_i,
104104
input logic test_en_i,
105105
input logic scan_rst_ni
106106
);
@@ -183,7 +183,7 @@ module ibex_lockstep import ibex_pkg::*; #(
183183
logic [14:0] irq_fast;
184184
logic irq_nm;
185185
logic debug_req;
186-
fetch_enable_t fetch_enable;
186+
ibex_mubi_t fetch_enable;
187187
logic ic_scr_key_valid;
188188
} delayed_inputs_t;
189189

@@ -263,7 +263,7 @@ module ibex_lockstep import ibex_pkg::*; #(
263263
logic irq_pending;
264264
crash_dump_t crash_dump;
265265
logic double_fault_seen;
266-
logic core_busy;
266+
ibex_mubi_t core_busy;
267267
} delayed_outputs_t;
268268

269269
delayed_outputs_t [OutputsOffset-1:0] core_outputs_q;

‎rtl/ibex_pkg.sv

+8-8
Original file line numberDiff line numberDiff line change
@@ -653,14 +653,14 @@ package ibex_pkg;
653653
parameter logic [SCRAMBLE_NONCE_W-1:0] RndCnstIbexNonceDefault =
654654
64'hf79780bc735f3843;
655655

656-
// Fetch enable. Mult-bit signal used for security hardening. For non-secure implementation all
657-
// bits other than the bottom bit are ignored.
658-
typedef logic [3:0] fetch_enable_t;
656+
// Mult-bit signal used for security hardening. For non-secure implementation all bits other than
657+
// the bottom bit are ignored.
658+
typedef logic [3:0] ibex_mubi_t;
659659

660660
// Note that if adjusting these parameters it is assumed the bottom bit is set for On and unset
661-
// for Off. This allows the use of FetchEnableOn/FetchEnableOff to work for both secure and
662-
// non-secure Ibex. If this assumption is broken the RTL that uses the fetch_enable signal within
663-
// `ibex_core` may need adjusting.
664-
parameter fetch_enable_t FetchEnableOn = 4'b0101;
665-
parameter fetch_enable_t FetchEnableOff = 4'b1010;
661+
// for Off. This allows the use of IbexMuBiOn/IbexMuBiOff to work for both secure and non-secure
662+
// Ibex. If this assumption is broken the RTL that uses ibex_mubi_t types such as the fetch_enable
663+
// and core_busy signals within `ibex_core` may need adjusting.
664+
parameter ibex_mubi_t IbexMuBiOn = 4'b0101;
665+
parameter ibex_mubi_t IbexMuBiOff = 4'b1010;
666666
endpackage

0 commit comments

Comments
 (0)