Skip to content

[secded] Switch to inverted ECC codes #1482

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

Merged
merged 2 commits into from
Dec 2, 2021
Merged

[secded] Switch to inverted ECC codes #1482

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
ea38934
Update lowrisc_ip to lowRISC/opentitan@34ba5e45f
msfschaffner Dec 2, 2021
6782176
[secded] Switch to inverted ECC codes
msfschaffner Dec 2, 2021
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
2 changes: 1 addition & 1 deletion doc/03_reference/load_store_unit.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ Bus Integrity Checking
----------------------

The core can optionally generate and verify check bits sent alongside the data for memory accesses.
Checkbits are generated and checked using a 39/32 Hsaio code (see :file:`vendor/lowrisc_ip/ip/prim/rtl/prim_secded_39_32_enc.sv`).
Checkbits are generated and checked using an inverted 39/32 Hsaio code (see :file:`vendor/lowrisc_ip/ip/prim/rtl/prim_secded_inv_39_32_enc.sv`).
When this feature is used, any mismatch in checkbits will generate a major alert.

This feature is only used if the core is configured with the SecureIbex parameter set.
Expand Down
2 changes: 1 addition & 1 deletion dv/uvm/core_ibex/common/ibex_mem_intf_agent/ibex_mem_intf_response_seq_lib.sv
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ class ibex_mem_intf_response_seq extends uvm_sequence #(ibex_mem_intf_seq_item);
end
end
// Add correct integrity bits
{req.intg, req.data} = prim_secded_pkg::prim_secded_39_32_enc(req.data);
{req.intg, req.data} = prim_secded_pkg::prim_secded_inv_39_32_enc(req.data);
`uvm_info(get_full_name(), $sformatf("Response transfer:\n%0s", req.sprint()), UVM_HIGH)
start_item(req);
finish_item(req);
Expand Down
12 changes: 6 additions & 6 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 @@ -18,12 +18,12 @@
${PRJ_DIR}/vendor/lowrisc_ip/ip/prim/rtl/prim_assert.sv
${PRJ_DIR}/vendor/lowrisc_ip/ip/prim/rtl/prim_cipher_pkg.sv
${PRJ_DIR}/vendor/lowrisc_ip/ip/prim/rtl/prim_lfsr.sv
${PRJ_DIR}/vendor/lowrisc_ip/ip/prim/rtl/prim_secded_28_22_enc.sv
${PRJ_DIR}/vendor/lowrisc_ip/ip/prim/rtl/prim_secded_28_22_dec.sv
${PRJ_DIR}/vendor/lowrisc_ip/ip/prim/rtl/prim_secded_39_32_enc.sv
${PRJ_DIR}/vendor/lowrisc_ip/ip/prim/rtl/prim_secded_39_32_dec.sv
${PRJ_DIR}/vendor/lowrisc_ip/ip/prim/rtl/prim_secded_72_64_enc.sv
${PRJ_DIR}/vendor/lowrisc_ip/ip/prim/rtl/prim_secded_72_64_dec.sv
${PRJ_DIR}/vendor/lowrisc_ip/ip/prim/rtl/prim_secded_inv_28_22_enc.sv
${PRJ_DIR}/vendor/lowrisc_ip/ip/prim/rtl/prim_secded_inv_28_22_dec.sv
${PRJ_DIR}/vendor/lowrisc_ip/ip/prim/rtl/prim_secded_inv_39_32_enc.sv
${PRJ_DIR}/vendor/lowrisc_ip/ip/prim/rtl/prim_secded_inv_39_32_dec.sv
${PRJ_DIR}/vendor/lowrisc_ip/ip/prim/rtl/prim_secded_inv_72_64_enc.sv
${PRJ_DIR}/vendor/lowrisc_ip/ip/prim/rtl/prim_secded_inv_72_64_dec.sv

// Until this list is generated by FuseSoC, we have to use manually generated
// wrappers around the prim_* modules to instantiate the prim_generic_* ones,
Expand Down
6 changes: 3 additions & 3 deletions rtl/ibex_core.sv
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -779,19 +779,19 @@ module ibex_core import ibex_pkg::*; #(
logic rf_ecc_err_a_id, rf_ecc_err_b_id;

// ECC checkbit generation for regiter file wdata
prim_secded_39_32_enc regfile_ecc_enc (
prim_secded_inv_39_32_enc regfile_ecc_enc (
.data_i(rf_wdata_wb),
.data_o(rf_wdata_wb_ecc_o)
);

// ECC checking on register file rdata
prim_secded_39_32_dec regfile_ecc_dec_a (
prim_secded_inv_39_32_dec regfile_ecc_dec_a (
.data_i (rf_rdata_a_ecc_i),
.data_o (),
.syndrome_o(),
.err_o (rf_ecc_err_a)
);
prim_secded_39_32_dec regfile_ecc_dec_b (
prim_secded_inv_39_32_dec regfile_ecc_dec_b (
.data_i (rf_rdata_b_ecc_i),
.data_o (),
.syndrome_o(),
Expand Down
8 changes: 4 additions & 4 deletions rtl/ibex_icache.sv
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -290,7 +290,7 @@ module ibex_icache import ibex_pkg::*; #(
assign tag_ecc_input_padded = {{22-IC_TAG_SIZE{1'b0}},fill_tag_ic0};
assign tag_ecc_output_unused = tag_ecc_output_padded[21:IC_TAG_SIZE-1];

prim_secded_28_22_enc tag_ecc_enc (
prim_secded_inv_28_22_enc tag_ecc_enc (
.data_i (tag_ecc_input_padded),
.data_o (tag_ecc_output_padded)
);
Expand All @@ -299,7 +299,7 @@ module ibex_icache import ibex_pkg::*; #(

// Dataram ECC
for (genvar bank = 0; bank < IC_LINE_BEATS; bank++) begin : gen_ecc_banks
prim_secded_39_32_enc data_ecc_enc (
prim_secded_inv_39_32_enc data_ecc_enc (
.data_i (fill_wdata_ic0[bank*BUS_SIZE+:BUS_SIZE]),
.data_o (data_wdata_ic0[bank*BusSizeECC+:BusSizeECC])
);
Expand Down Expand Up @@ -421,7 +421,7 @@ module ibex_icache import ibex_pkg::*; #(
{22-IC_TAG_SIZE{1'b0}},
tag_rdata_ic1[way][IC_TAG_SIZE-1:0]};

prim_secded_28_22_dec data_ecc_dec (
prim_secded_inv_28_22_dec data_ecc_dec (
.data_i (tag_rdata_padded_ic1),
.data_o (),
.syndrome_o (),
Expand All @@ -433,7 +433,7 @@ module ibex_icache import ibex_pkg::*; #(
// Data ECC checking
// Note - could generate for all ways and mux after
for (genvar bank = 0; bank < IC_LINE_BEATS; bank++) begin : gen_ecc_banks
prim_secded_39_32_dec data_ecc_dec (
prim_secded_inv_39_32_dec data_ecc_dec (
.data_i (hit_data_ecc_ic1[bank*BusSizeECC+:BusSizeECC]),
.data_o (),
.syndrome_o (),
Expand Down
6 changes: 3 additions & 3 deletions rtl/ibex_lockstep.sv
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -216,14 +216,14 @@ module ibex_lockstep import ibex_pkg::*; #(
logic [31:0] unused_wdata;

// Checks on incoming data
prim_secded_39_32_dec u_instr_intg_dec (
prim_secded_inv_39_32_dec u_instr_intg_dec (
.data_i ({instr_rdata_intg_q, shadow_inputs_q[LockstepOffset-1].instr_rdata}),
.data_o (),
.syndrome_o (),
.err_o (instr_intg_err)
);

prim_secded_39_32_dec u_data_intg_dec (
prim_secded_inv_39_32_dec u_data_intg_dec (
.data_i ({data_rdata_intg_q, shadow_inputs_q[LockstepOffset-1].data_rdata}),
.data_o (),
.syndrome_o (),
Expand All @@ -234,7 +234,7 @@ module ibex_lockstep import ibex_pkg::*; #(
(shadow_inputs_q[LockstepOffset-1].data_rvalid & |data_intg_err);

// Generate integrity bits
prim_secded_39_32_enc u_data_gen (
prim_secded_inv_39_32_enc u_data_gen (
.data_i (data_wdata_i),
.data_o ({data_wdata_intg_o, unused_wdata})
);
Expand Down
15 changes: 8 additions & 7 deletions rtl/ibex_register_file_ff.sv
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,10 @@
* targeting FPGA synthesis or Verilator simulation.
*/
module ibex_register_file_ff #(
parameter bit RV32E = 0,
parameter int unsigned DataWidth = 32,
parameter bit DummyInstructions = 0
parameter bit RV32E = 0,
parameter int unsigned DataWidth = 32,
parameter bit DummyInstructions = 0,
parameter logic [DataWidth-1:0] WordZeroVal = '0
) (
// Clock and Reset
input logic clk_i,
Expand Down Expand Up @@ -55,7 +56,7 @@ module ibex_register_file_ff #(
for (genvar i = 1; i < NUM_WORDS; i++) begin : g_rf_flops
always_ff @(posedge clk_i or negedge rst_ni) begin
if (!rst_ni) begin
rf_reg_q[i] <= '0;
rf_reg_q[i] <= WordZeroVal;
end else if (we_a_dec[i]) begin
rf_reg_q[i] <= wdata_a_i;
end
Expand All @@ -73,21 +74,21 @@ module ibex_register_file_ff #(

always_ff @(posedge clk_i or negedge rst_ni) begin
if (!rst_ni) begin
rf_r0_q <= '0;
rf_r0_q <= WordZeroVal;
end else if (we_r0_dummy) begin
rf_r0_q <= wdata_a_i;
end
end

// Output the dummy data for dummy instructions, otherwise R0 reads as zero
assign rf_reg[0] = dummy_instr_id_i ? rf_r0_q : '0;
assign rf_reg[0] = dummy_instr_id_i ? rf_r0_q : WordZeroVal;

end else begin : g_normal_r0
logic unused_dummy_instr_id;
assign unused_dummy_instr_id = dummy_instr_id_i;

// R0 is nil
assign rf_reg[0] = '0;
assign rf_reg[0] = WordZeroVal;
end

assign rf_reg[NUM_WORDS-1:1] = rf_reg_q[NUM_WORDS-1:1];
Expand Down
14 changes: 11 additions & 3 deletions rtl/ibex_register_file_fpga.sv
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,10 @@
* FPGA architectures, it will produce RAM32M primitives. Other vendors have not yet been tested.
*/
module ibex_register_file_fpga #(
parameter bit RV32E = 0,
parameter int unsigned DataWidth = 32,
parameter bit DummyInstructions = 0
parameter bit RV32E = 0,
parameter int unsigned DataWidth = 32,
parameter bit DummyInstructions = 0,
parameter logic [DataWidth-1:0] WordZeroVal = '0
) (
// Clock and Reset
input logic clk_i,
Expand Down Expand Up @@ -56,6 +57,13 @@ module ibex_register_file_fpga #(
end
end : sync_write

// Make sure we initialize the BRAM with the correct register reset value.
initial begin
for (int k = 0; k < NUM_WORDS; k++) begin
mem[k] = WordZeroVal;
end
end

// Reset not used in this register file version
logic unused_rst_ni;
assign unused_rst_ni = rst_ni;
Expand Down
13 changes: 7 additions & 6 deletions rtl/ibex_register_file_latch.sv
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,10 @@
* register file when targeting ASIC synthesis or event-based simulators.
*/
module ibex_register_file_latch #(
parameter bit RV32E = 0,
parameter int unsigned DataWidth = 32,
parameter bit DummyInstructions = 0
parameter bit RV32E = 0,
parameter int unsigned DataWidth = 32,
parameter bit DummyInstructions = 0,
parameter logic [DataWidth-1:0] WordZeroVal = '0
) (
// Clock and Reset
input logic clk_i,
Expand Down Expand Up @@ -78,7 +79,7 @@ module ibex_register_file_latch #(
// Use clk_int here, since otherwise we don't want to write anything anyway.
always_ff @(posedge clk_int or negedge rst_ni) begin : sample_wdata
if (!rst_ni) begin
wdata_a_q <= '0;
wdata_a_q <= WordZeroVal;
end else begin
if (we_a_i) begin
wdata_a_q <= wdata_a_i;
Expand Down Expand Up @@ -143,13 +144,13 @@ module ibex_register_file_latch #(
end

// Output the dummy data for dummy instructions, otherwise R0 reads as zero
assign mem[0] = dummy_instr_id_i ? mem_r0 : '0;
assign mem[0] = dummy_instr_id_i ? mem_r0 : WordZeroVal;

end else begin : g_normal_r0
logic unused_dummy_instr_id;
assign unused_dummy_instr_id = dummy_instr_id_i;

assign mem[0] = '0;
assign mem[0] = WordZeroVal;
end

`ifdef VERILATOR
Expand Down
13 changes: 10 additions & 3 deletions rtl/ibex_top.sv
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -308,11 +308,16 @@ module ibex_top import ibex_pkg::*; #(
// Register file Instantiation //
/////////////////////////////////

// We're using an inverted Hsiao code, hence we need to reset
// the regfile ECC bits to 1.
localparam logic [RegFileDataWidth-1:0] WordZeroVal = RegFileDataWidth'({7'h7f, 32'h0});

if (RegFile == RegFileFF) begin : gen_regfile_ff
ibex_register_file_ff #(
.RV32E (RV32E),
.DataWidth (RegFileDataWidth),
.DummyInstructions(DummyInstructions)
.DummyInstructions(DummyInstructions),
.WordZeroVal (WordZeroVal)
) register_file_i (
.clk_i (clk),
.rst_ni(rst_ni),
Expand All @@ -332,7 +337,8 @@ module ibex_top import ibex_pkg::*; #(
ibex_register_file_fpga #(
.RV32E (RV32E),
.DataWidth (RegFileDataWidth),
.DummyInstructions(DummyInstructions)
.DummyInstructions(DummyInstructions),
.WordZeroVal (WordZeroVal)
) register_file_i (
.clk_i (clk),
.rst_ni(rst_ni),
Expand All @@ -352,7 +358,8 @@ module ibex_top import ibex_pkg::*; #(
ibex_register_file_latch #(
.RV32E (RV32E),
.DataWidth (RegFileDataWidth),
.DummyInstructions(DummyInstructions)
.DummyInstructions(DummyInstructions),
.WordZeroVal (WordZeroVal)
) register_file_i (
.clk_i (clk),
.rst_ni(rst_ni),
Expand Down
2 changes: 1 addition & 1 deletion vendor/lowrisc_ip.lock.hjson
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,6 @@
upstream:
{
url: https://github.com/lowRISC/opentitan
rev: 3a672eb36aee5942d0912a15d15055b1d21c33d6
rev: 34ba5e45f9af7d8ca6c9bdae8bd11eeeeb669d6c
}
}
37 changes: 25 additions & 12 deletions vendor/lowrisc_ip/dv/sv/dv_lib/dv_base_vseq.sv
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -187,14 +187,27 @@ class dv_base_vseq #(type RAL_T = dv_base_reg_block,
// arg csr_test_type: what csr test to run {hw_reset, rw, bit_bash, aliasing}
// arg num_test_csrs:instead of testing the entire ral model or passing test chunk info via
// plusarg, provide ability to set a random number of csrs to test from higher level sequence
virtual task run_csr_vseq(string csr_test_type = "",
int num_test_csrs = 0,
bit do_rand_wr_and_reset = 1);
csr_base_seq m_csr_seq;

// env needs to have a ral instance
virtual task run_csr_vseq(string csr_test_type = "",
Copy link

Choose a reason for hiding this comment

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

are these changes part of this pr..?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No they are not really. I.e., the first commit just vendors the newest prims over into Ibex - and that includes some dv functions.

int num_test_csrs = 0,
bit do_rand_wr_and_reset = 1,
string ral_name = "");
csr_base_seq m_csr_seq;
dv_base_reg_block models[string];

// env needs to have at least one ral instance
`DV_CHECK_GE_FATAL(cfg.ral_models.size(), 1)

// If ral_name is specified, only use registers on that named interface. Otherwise, use all
// registers.
if (ral_name != "") begin
`DV_CHECK_FATAL(cfg.ral_models.exists(ral_name))
models[ral_name] = cfg.ral_models[ral_name];
end else begin
foreach (cfg.ral_models[i]) begin
models[i] = cfg.ral_models[i];
end
end

// check which csr test type
case (csr_test_type)
"hw_reset": csr_base_seq::type_id::set_type_override(csr_hw_reset_seq::get_type());
Expand All @@ -206,8 +219,8 @@ class dv_base_vseq #(type RAL_T = dv_base_reg_block,
endcase

// Print the list of available exclusions that are in effect for debug.
foreach (cfg.ral_models[i]) begin
csr_excl_item csr_excl = csr_utils_pkg::get_excl_item(cfg.ral_models[i]);
foreach (models[i]) begin
csr_excl_item csr_excl = csr_utils_pkg::get_excl_item(models[i]);
if (csr_excl != null) csr_excl.print_exclusions();
end

Expand All @@ -227,8 +240,8 @@ class dv_base_vseq #(type RAL_T = dv_base_reg_block,
m_csr_write_seq = csr_write_seq::type_id::create("m_csr_write_seq");
// We have to assign this array in a loop because the element types aren't equivalent, so
// the array types aren't assignment compatible.
foreach (cfg.ral_models[i]) begin
m_csr_write_seq.models[i] = cfg.ral_models[i];
foreach (models[i]) begin
m_csr_write_seq.models[i] = models[i];
end
m_csr_write_seq.external_checker = cfg.en_scb;
m_csr_write_seq.en_rand_backdoor_write = 1'b1;
Expand All @@ -248,8 +261,8 @@ class dv_base_vseq #(type RAL_T = dv_base_reg_block,
m_csr_seq = csr_base_seq::type_id::create("m_csr_seq");
// We have to assign this array in a loop because the element types aren't equivalent, so
// the array types aren't assignment compatible.
foreach (cfg.ral_models[i]) begin
m_csr_seq.models[i] = cfg.ral_models[i];
foreach (models[i]) begin
m_csr_seq.models[i] = models[i];
end
m_csr_seq.external_checker = cfg.en_scb;
m_csr_seq.num_test_csrs = num_test_csrs;
Expand Down
23 changes: 14 additions & 9 deletions vendor/lowrisc_ip/dv/sv/dv_utils/dv_macros.svh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -413,20 +413,25 @@
end
`endif

// This macro converts a string input from plusarg to an enum variable
// ENUM_: the name of enum type
// PLUSARG_: the name of the plusargs, which is also the name of the enum variable
// CHECK_EXIST_: set to 1, `$value$plusargs()` should return true
// Retrieves a plusarg value representing an enum literal.
//
// The plusarg is parsed as a string, which needs to be converted into the enum literal whose name
// matches the string. This functionality is provided by the UVM helper function below.
//
// ENUM_: The enum type.
// VAR_: The enum variable to which the plusarg value will be set (must be declared already).
// PLUSARG_: the name of the plusarg (as raw text). This is typically the same as the enum variable.
// CHECK_EXISTS_: Throws a fatal error if the plusarg is not set.
`ifndef DV_GET_ENUM_PLUSARG
`define DV_GET_ENUM_PLUSARG(ENUM_, PLUSARG_, CHECK_EXIST_ = 0, ID_ = `gfn) \
`define DV_GET_ENUM_PLUSARG(ENUM_, VAR_, PLUSARG_ = VAR_, CHECK_EXISTS_ = 0, ID_ = `gfn) \
begin \
string str; \
if ($value$plusargs("``PLUSARG_``=%0s", str)) begin \
if (!uvm_enum_wrapper#(ENUM_)::from_name(str, PLUSARG_)) begin \
`uvm_fatal(ID_, $sformatf("Cannot find %s from enum ``ENUM_``", PLUSARG_.name)) \
if (!uvm_enum_wrapper#(ENUM_)::from_name(str, VAR_)) begin \
`uvm_fatal(ID_, $sformatf("Cannot find %s from enum ``ENUM_``", VAR_.name())) \
end \
end else if (CHECK_EXIST_) begin \
`uvm_fatal(ID_, "Can't find plusargs ``PLUSARG_``") \
end else if (CHECK_EXISTS_) begin \
`uvm_fatal(ID_, "Please pass the plusarg +``PLUSARG_``=<``ENUM_``-literal>") \
end \
end
`endif
Expand Down
Loading