Skip to content

[rtl] Add common security features of regfiles to separate module #2266

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

[rtl] Add common security features of regfiles to separate module #2266

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
36e220a
[rtl] Add common security features of regfiles to separate module
aklsh Apr 19, 2025
c4a9389
rtl: update regfile implementations to use common security module
aklsh Apr 25, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions dv/uvm/core_ibex/ibex_dv.f
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,7 @@
${PRJ_DIR}/rtl/ibex_multdiv_fast.sv
${PRJ_DIR}/rtl/ibex_prefetch_buffer.sv
${PRJ_DIR}/rtl/ibex_fetch_fifo.sv
${PRJ_DIR}/rtl/ibex_register_file_common.sv
${PRJ_DIR}/rtl/ibex_register_file_ff.sv
${PRJ_DIR}/rtl/ibex_register_file_fpga.sv
${PRJ_DIR}/rtl/ibex_register_file_latch.sv
Expand Down
1 change: 1 addition & 0 deletions ibex_top.core
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ filesets:
- lowrisc:prim:onehot_check
- lowrisc:prim:onehot
files:
- rtl/ibex_register_file_common.sv
- rtl/ibex_register_file_ff.sv # generic FF-based
- rtl/ibex_register_file_fpga.sv # FPGA
- rtl/ibex_register_file_latch.sv # ASIC
Expand Down
1 change: 1 addition & 0 deletions rtl/ibex_core.f
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,5 +17,6 @@
ibex_multdiv_fast.sv
ibex_prefetch_buffer.sv
ibex_fetch_fifo.sv
ibex_register_file_common.sv
ibex_register_file_ff.sv
ibex_core.sv
175 changes: 175 additions & 0 deletions rtl/ibex_register_file_common.sv
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,175 @@
// Copyright lowRISC contributors.
// Copyright 2018 ETH Zurich and University of Bologna, see also CREDITS.md.
// Licensed under the Apache License, Version 2.0, see LICENSE for details.
// SPDX-License-Identifier: Apache-2.0

/**
* RISC-V register file
*
* Register file common security functionality across multiple implementations
* - Contains common security hardening logic across implementations
* - Contains checks for onehot encoding of addresses and write enable(s)
*/

module ibex_register_file_common #(
parameter bit FPGA = 0,
parameter int unsigned AddrWidth = 5,
parameter int unsigned NumWords = 2 ** AddrWidth,
parameter bit WrenCheck = 0,
parameter bit RdataMuxCheck = 0
) (
/* verilator lint_off UNUSED */
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of these lint off messages that apply to the whole file, would you mind marking the signals as unused inside the else statements themselves? Similar to how the addresses are marked as unused there.

// Clock and Reset
input logic clk_i,
input logic rst_ni,
/* verilator lint_on UNUSED */

//Read port R1
input logic [4:0] raddr_a_i,
output logic [NumWords-1:0] raddr_onehot_a,
output logic oh_raddr_a_err,

//Read port R2
input logic [4:0] raddr_b_i,
output logic [NumWords-1:0] raddr_onehot_b,
output logic oh_raddr_b_err,

// Write port W1
input logic [4:0] waddr_a_i,
input logic we_a_i,
output logic [NumWords-1:0] we_onehot_a,
output logic oh_we_err,

// This indicates whether spurious WE or non-one-hot encoded raddr are detected.
output logic err_o
);

if (FPGA) begin : gen_fpga_oh_we_a_o_decoder
always_comb begin : oh_we_a_o_decoder
for (int unsigned i = 0; i < NumWords; i++) begin
we_onehot_a[i] = (i == 0) ? we_a_i : 1'b0;
end
end
end else begin : gen_other_oh_we_a_o_decoder
always_comb begin : oh_we_a_o_decoder
for (int unsigned i = 0; i < NumWords; i++) begin
we_onehot_a[i] = (waddr_a_i == 5'(i)) ? we_a_i : 1'b0;
end
end
end

// SEC_CM: DATA_REG_SW.GLITCH_DETECT
// This checks for spurious WE strobes on the regfile.
if (WrenCheck) begin : gen_wren_check
// Buffer the decoded write enable bits so that the checker
// is not optimized into the address decoding logic.
logic [NumWords-1:0] we_onehot_a_buf;
prim_buf #(
.Width(NumWords)
) u_prim_buf (
.in_i (we_onehot_a),
.out_o(we_onehot_a_buf)
);

prim_onehot_check #(
.AddrWidth (AddrWidth),
.OneHotWidth(NumWords),
.AddrCheck (FPGA ? 0 : 1), // disable in case of FPGA impl, as we use [0] only
.EnableCheck(1),
) u_prim_onehot_check (
.clk_i,
.rst_ni,
.oh_i (we_onehot_a_buf),
.addr_i(waddr_a_i),
.en_i (we_a_i),
.err_o (oh_we_err)
);
end else begin : gen_no_wren_check
if (FPGA) begin : gen_unused_wren_check
logic unused_waddr_a;
assign unused_waddr_a = ^waddr_a_i;
end
assign oh_we_err = 1'b0;
end

if (RdataMuxCheck) begin : gen_rdata_mux_check
// Encode raddr_a/b into one-hot encoded signals.
logic [NumWords-1:0] raddr_onehot_a_buf, raddr_onehot_b_buf;
prim_onehot_enc #(
.OneHotWidth(NumWords)
) u_prim_onehot_enc_raddr_a (
.in_i (raddr_a_i),
.en_i (1'b1),
.out_o(raddr_onehot_a)
);

prim_onehot_enc #(
.OneHotWidth(NumWords)
) u_prim_onehot_enc_raddr_b (
.in_i (raddr_b_i),
.en_i (1'b1),
.out_o(raddr_onehot_b)
);

// Buffer the one-hot encoded signals so that the checkers
// are not optimized.
prim_buf #(
.Width(NumWords)
) u_prim_buf_raddr_a (
.in_i (raddr_onehot_a),
.out_o(raddr_onehot_a_buf)
);

prim_buf #(
.Width(NumWords)
) u_prim_buf_raddr_b (
.in_i (raddr_onehot_b),
.out_o(raddr_onehot_b_buf)
);

// SEC_CM: DATA_REG_SW.GLITCH_DETECT
// Check the one-hot encoded signals for glitches.
prim_onehot_check #(
.AddrWidth (AddrWidth),
.OneHotWidth(NumWords),
.AddrCheck (1),
// When AddrCheck=1 also EnableCheck needs to be 1.
.EnableCheck(1)
) u_prim_onehot_check_raddr_a (
.clk_i,
.rst_ni,
.oh_i (raddr_onehot_a_buf),
.addr_i(raddr_a_i),
// Set enable=1 as address is always valid.
.en_i (1'b1),
.err_o (oh_raddr_a_err)
);

prim_onehot_check #(
.AddrWidth (AddrWidth),
.OneHotWidth(NumWords),
.AddrCheck (1),
// When AddrCheck=1 also EnableCheck needs to be 1.
.EnableCheck(1)
) u_prim_onehot_check_raddr_b (
.clk_i,
.rst_ni,
.oh_i (raddr_onehot_b_buf),
.addr_i(raddr_b_i),
// Set enable=1 as address is always valid.
.en_i (1'b1),
.err_o (oh_raddr_b_err)
);
end else begin : gen_no_rdata_mux_check
assign oh_raddr_a_err = 1'b0;
assign oh_raddr_b_err = 1'b0;
assign raddr_onehot_a = '0;
assign raddr_onehot_b = '0;

logic unused_raddr;
assign unused_raddr = ^{raddr_a_i, raddr_b_i};
end

assign err_o = oh_raddr_a_err || oh_raddr_b_err || oh_we_err;

endmodule
148 changes: 37 additions & 111 deletions rtl/ibex_register_file_ff.sv
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,46 +48,34 @@ module ibex_register_file_ff #(
localparam int unsigned NUM_WORDS = 2**ADDR_WIDTH;

logic [DataWidth-1:0] rf_reg [NUM_WORDS];
logic [NUM_WORDS-1:0] we_a_dec;

// Encode we_a/raddr_a/raddr_b into one-hot encoded signals
logic [NUM_WORDS-1:0] raddr_onehot_a, raddr_onehot_b, we_onehot_a;

// One-hot encoding error signals
logic oh_raddr_a_err, oh_raddr_b_err, oh_we_err;

always_comb begin : we_a_decoder
for (int unsigned i = 0; i < NUM_WORDS; i++) begin
we_a_dec[i] = (waddr_a_i == 5'(i)) ? we_a_i : 1'b0;
end
end

// SEC_CM: DATA_REG_SW.GLITCH_DETECT
// This checks for spurious WE strobes on the regfile.
if (WrenCheck) begin : gen_wren_check
// Buffer the decoded write enable bits so that the checker
// is not optimized into the address decoding logic.
logic [NUM_WORDS-1:0] we_a_dec_buf;
prim_buf #(
.Width(NUM_WORDS)
) u_prim_buf (
.in_i(we_a_dec),
.out_o(we_a_dec_buf)
);

prim_onehot_check #(
.AddrWidth(ADDR_WIDTH),
.AddrCheck(1),
.EnableCheck(1)
) u_prim_onehot_check (
.clk_i,
.rst_ni,
.oh_i(we_a_dec_buf),
.addr_i(waddr_a_i),
.en_i(we_a_i),
.err_o(oh_we_err)
);
end else begin : gen_no_wren_check
logic unused_strobe;
assign unused_strobe = we_a_dec[0]; // this is never read from in this case
assign oh_we_err = 1'b0;
end
// Common security functionality
ibex_register_file_common #(
.AddrWidth(ADDR_WIDTH),
.NumWords(NUM_WORDS),
.WrenCheck(WrenCheck),
.RdataMuxCheck(RdataMuxCheck)
) security_module (
.clk_i,
.rst_ni,
.raddr_a_i,
.raddr_onehot_a,
.oh_raddr_a_err,
.raddr_b_i,
.raddr_onehot_b,
.oh_raddr_b_err,
.waddr_a_i,
.we_a_i,
.we_onehot_a,
.oh_we_err,
.err_o
);

// No flops for R0 as it's hard-wired to 0
for (genvar i = 1; i < NUM_WORDS; i++) begin : g_rf_flops
Expand All @@ -96,7 +84,7 @@ module ibex_register_file_ff #(
always_ff @(posedge clk_i or negedge rst_ni) begin
if (!rst_ni) begin
rf_reg_q <= WordZeroVal;
end else if (we_a_dec[i]) begin
end else if (we_onehot_a[i]) begin
rf_reg_q <= wdata_a_i;
end
end
Expand Down Expand Up @@ -134,75 +122,6 @@ module ibex_register_file_ff #(
end

if (RdataMuxCheck) begin : gen_rdata_mux_check
// Encode raddr_a/b into one-hot encoded signals.
logic [NUM_WORDS-1:0] raddr_onehot_a, raddr_onehot_b;
logic [NUM_WORDS-1:0] raddr_onehot_a_buf, raddr_onehot_b_buf;
prim_onehot_enc #(
.OneHotWidth(NUM_WORDS)
) u_prim_onehot_enc_raddr_a (
.in_i (raddr_a_i),
.en_i (1'b1),
.out_o (raddr_onehot_a)
);

prim_onehot_enc #(
.OneHotWidth(NUM_WORDS)
) u_prim_onehot_enc_raddr_b (
.in_i (raddr_b_i),
.en_i (1'b1),
.out_o (raddr_onehot_b)
);

// Buffer the one-hot encoded signals so that the checkers
// are not optimized.
prim_buf #(
.Width(NUM_WORDS)
) u_prim_buf_raddr_a (
.in_i (raddr_onehot_a),
.out_o(raddr_onehot_a_buf)
);

prim_buf #(
.Width(NUM_WORDS)
) u_prim_buf_raddr_b (
.in_i (raddr_onehot_b),
.out_o(raddr_onehot_b_buf)
);

// SEC_CM: DATA_REG_SW.GLITCH_DETECT
// Check the one-hot encoded signals for glitches.
prim_onehot_check #(
.AddrWidth(ADDR_WIDTH),
.OneHotWidth(NUM_WORDS),
.AddrCheck(1),
// When AddrCheck=1 also EnableCheck needs to be 1.
.EnableCheck(1)
) u_prim_onehot_check_raddr_a (
.clk_i,
.rst_ni,
.oh_i (raddr_onehot_a_buf),
.addr_i (raddr_a_i),
// Set enable=1 as address is always valid.
.en_i (1'b1),
.err_o (oh_raddr_a_err)
);

prim_onehot_check #(
.AddrWidth(ADDR_WIDTH),
.OneHotWidth(NUM_WORDS),
.AddrCheck(1),
// When AddrCheck=1 also EnableCheck needs to be 1.
.EnableCheck(1)
) u_prim_onehot_check_raddr_b (
.clk_i,
.rst_ni,
.oh_i (raddr_onehot_b_buf),
.addr_i (raddr_b_i),
// Set enable=1 as address is always valid.
.en_i (1'b1),
.err_o (oh_raddr_b_err)
);

// MUX register to rdata_a/b_o according to raddr_a/b_onehot.
prim_onehot_mux #(
.Width(DataWidth),
Expand All @@ -228,14 +147,21 @@ module ibex_register_file_ff #(
end else begin : gen_no_rdata_mux_check
assign rdata_a_o = rf_reg[raddr_a_i];
assign rdata_b_o = rf_reg[raddr_b_i];
assign oh_raddr_a_err = 1'b0;
assign oh_raddr_b_err = 1'b0;
end

assign err_o = oh_raddr_a_err || oh_raddr_b_err || oh_we_err;
logic unused_raddr_onehot, unused_oh_raddr_err;
assign unused_raddr_onehot = ^{raddr_onehot_a, raddr_onehot_b};
assign unused_oh_raddr_err = ^{oh_raddr_a_err, oh_raddr_b_err};
end

// Signal not used in FF register file
logic unused_test_en;
assign unused_test_en = test_en_i;

if (WrenCheck) begin : gen_wren_check
end else begin : gen_no_wren_check
logic unused_strobe, unused_oh_we_err;
assign unused_strobe = we_onehot_a[0]; // this is never read from in this case
assign unused_oh_we_err = oh_we_err; // this is never read from in this case
end

endmodule
Loading