[dv] Fix shape calculations for replicated ECC
data_width is supposed to be the width of a word in bits, not counting
any ECC bits. The computation that was there before wasn't quite
right because it didn't allow for the possibility of multiple
subwords. We use this structure for OTBN, where we have 256-bit words,
split into eight 32-bit subwords where each is protected with the
Ecc_39_32 scheme.
Also bump up the maximum bytes per word (we need more!) and add
read256() and write256() functions.
Finally, fix the instantiations of imem_util and dmem_util in the OTBN
testbench (I'd misunderstood what the n_bits argument did).
Signed-off-by: Rupert Swarbrick <rswarbrick@lowrisc.org>
diff --git a/hw/dv/sv/mem_bkdr_util/mem_bkdr_util.sv b/hw/dv/sv/mem_bkdr_util/mem_bkdr_util.sv
index de6fab9..4400cb7 100644
--- a/hw/dv/sv/mem_bkdr_util/mem_bkdr_util.sv
+++ b/hw/dv/sv/mem_bkdr_util/mem_bkdr_util.sv
@@ -71,11 +71,38 @@
this.width = n_bits / depth;
this.err_detection_scheme = err_detection_scheme;
- data_width = `HAS_ECC ? prim_secded_pkg::get_ecc_data_width(
- prim_secded_pkg::prim_secded_e'(err_detection_scheme)) : width;
+ if (`HAS_ECC) begin
+ import prim_secded_pkg::prim_secded_e;
+ import prim_secded_pkg::get_ecc_data_width;
+ import prim_secded_pkg::get_ecc_parity_width;
+
+ prim_secded_e secded_eds = prim_secded_e'(err_detection_scheme);
+ int non_ecc_bits_per_subword = get_ecc_data_width(secded_eds);
+ int ecc_bits_per_subword = get_ecc_parity_width(secded_eds);
+ int bits_per_subword = non_ecc_bits_per_subword + ecc_bits_per_subword;
+ int subwords_per_word;
+
+ // We shouldn't truncate the actual data word. This check ensures that err_detection_scheme
+ // and width are related sensibly. This only checks we've got enough space for one data word
+ // and at least one check bit. The next check will make sure that we don't truncate if there
+ // are multiple subwords.
+ `DV_CHECK_FATAL(non_ecc_bits_per_subword < this.width)
+
+ // Normally, we'd want width to be divisible by bits_per_subword, which means that we get a
+ // whole number of subwords in a word. As a special case, we also allow a having exactly one
+ // subword and only keeping some of the bits. This is used by the flash controller.
+ `DV_CHECK_FATAL((this.width < bits_per_subword) || (this.width % bits_per_subword == 0),
+ "With multiple subwords, mem width must be a multiple of the ECC width")
+
+ subwords_per_word = (width + bits_per_subword - 1) / bits_per_subword;
+ this.data_width = subwords_per_word * non_ecc_bits_per_subword;
+ end else begin
+ this.data_width = width;
+ end
+
byte_width = `HAS_PARITY ? 9 : 8;
bytes_per_word = data_width / byte_width;
- `DV_CHECK_LE_FATAL(bytes_per_word, 16, "data width > 16 bytes is not supported")
+ `DV_CHECK_LE_FATAL(bytes_per_word, 32, "data width > 32 bytes is not supported")
size_bytes = depth * bytes_per_word;
addr_lsb = $clog2(bytes_per_word);
addr_width = $clog2(depth);
@@ -220,6 +247,11 @@
return {read64(addr + 8), read64(addr)};
endfunction
+ virtual function logic [255:0] read256(bit [bus_params_pkg::BUS_AW-1:0] addr);
+ `_ACCESS_CHECKS(addr, 256)
+ return {read128(addr + 16), read128(addr)};
+ endfunction
+
// Write the entire word at the given address with the specified data.
//
// addr is the byte address starting at offset 0. Mask the upper address bits as needed before
@@ -320,7 +352,14 @@
`_ACCESS_CHECKS(addr, 128)
if (!check_addr_valid(addr)) return;
write64(addr, data[63:0]);
- write64(addr + 4, data[127:63]);
+ write64(addr + 8, data[127:63]);
+ endfunction
+
+ virtual function void write256(bit [bus_params_pkg::BUS_AW-1:0] addr, logic [255:0] data);
+ `_ACCESS_CHECKS(addr, 256)
+ if (!check_addr_valid(addr)) return;
+ write128(addr, data[127:0]);
+ write128(addr + 16, data[255:128]);
endfunction
`undef _ACCESS_CHECKS
diff --git a/hw/ip/otbn/dv/uvm/tb.sv b/hw/ip/otbn/dv/uvm/tb.sv
index 718bdd3..004f42d 100644
--- a/hw/ip/otbn/dv/uvm/tb.sv
+++ b/hw/ip/otbn/dv/uvm/tb.sv
@@ -280,17 +280,19 @@
dut.u_otbn_core.u_otbn_rf_base.i_otbn_rf_base_if);
// Instantiate mem_bkdr_util objects to allow access to IMEM and DMEM
+ //
+ // Note that n_bits is the number of bits in the memory, including ECC check bits.
imem_util = new(.name ("imem_util"),
.path ({"tb.dut.u_imem.u_prim_ram_1p_adv.",
"u_mem.gen_generic.u_impl_generic.mem"}),
.depth (otbn_reg_pkg::OTBN_IMEM_SIZE / 4),
- .n_bits (8 * otbn_reg_pkg::OTBN_IMEM_SIZE),
+ .n_bits (otbn_reg_pkg::OTBN_IMEM_SIZE / 4 * 39),
.err_detection_scheme (mem_bkdr_util_pkg::Ecc_39_32));
dmem_util = new(.name ("dmem_util"),
.path ({"tb.dut.u_dmem.u_prim_ram_1p_adv.",
"u_mem.gen_generic.u_impl_generic.mem"}),
.depth (otbn_reg_pkg::OTBN_DMEM_SIZE / 32),
- .n_bits (8 * otbn_reg_pkg::OTBN_DMEM_SIZE),
+ .n_bits (otbn_reg_pkg::OTBN_DMEM_SIZE / 32 * 312),
.err_detection_scheme (mem_bkdr_util_pkg::Ecc_39_32));
uvm_config_db#(mem_bkdr_util)::set(null, "*.env", imem_util.get_name(), imem_util);
