[tlul] Add support for data integrity passthrough
- Top level/otbn rams are now fully connected without dropping or pading bits
- The integrity is fully passed through, however, integrity recalculation on byte writes for top level memories will be done in a separate PR.
- ROM handling will also be separately done as it needs to be padded out to byte alignment
Signed-off-by: Timothy Chen <timothytim@google.com>
diff --git a/hw/ip/otbn/rtl/otbn.sv b/hw/ip/otbn/rtl/otbn.sv
index fe7c9a9..f1d7205 100644
--- a/hw/ip/otbn/rtl/otbn.sv
+++ b/hw/ip/otbn/rtl/otbn.sv
@@ -113,8 +113,8 @@
logic imem_req;
logic imem_write;
logic [ImemIndexWidth-1:0] imem_index;
- logic [31:0] imem_wdata;
- logic [31:0] imem_wmask;
+ logic [38:0] imem_wdata;
+ logic [38:0] imem_wmask;
logic [38:0] imem_rdata;
logic imem_rvalid;
logic [1:0] imem_rerror_vec;
@@ -131,9 +131,9 @@
logic imem_req_bus;
logic imem_write_bus;
logic [ImemIndexWidth-1:0] imem_index_bus;
- logic [31:0] imem_wdata_bus;
- logic [31:0] imem_wmask_bus;
- logic [31:0] imem_rdata_bus;
+ logic [38:0] imem_wdata_bus;
+ logic [38:0] imem_wmask_bus;
+ logic [38:0] imem_rdata_bus;
logic imem_rvalid_bus;
logic [1:0] imem_rerror_bus;
@@ -146,15 +146,15 @@
prim_ram_1p_adv #(
.Width (39),
.Depth (ImemSizeWords),
- .DataBitsPerMask (39) // Write masks are not supported.
+ .DataBitsPerMask (39)
) u_imem (
.clk_i,
.rst_ni,
.req_i (imem_req),
.write_i (imem_write),
.addr_i (imem_index),
- .wdata_i ({7'd0, imem_wdata}),
- .wmask_i ({{7{imem_wmask[0]}}, imem_wmask}),
+ .wdata_i (imem_wdata),
+ .wmask_i (imem_wmask),
.rdata_o (imem_rdata),
.rvalid_o (imem_rvalid),
.rerror_o (imem_rerror_vec),
@@ -175,7 +175,8 @@
.SramDw (32),
.Outstanding (1),
.ByteAccess (0),
- .ErrOnRead (0)
+ .ErrOnRead (0),
+ .EnableDataIntgPt (1)
) u_tlul_adapter_sram_imem (
.clk_i,
.rst_ni,
@@ -197,10 +198,10 @@
// Mux core and bus access into IMEM
assign imem_access_core = busy_q | start;
- assign imem_req = imem_access_core ? imem_req_core : imem_req_bus;
- assign imem_write = imem_access_core ? imem_write_core : imem_write_bus;
- assign imem_index = imem_access_core ? imem_index_core : imem_index_bus;
- assign imem_wdata = imem_access_core ? imem_wdata_core : imem_wdata_bus;
+ assign imem_req = imem_access_core ? imem_req_core : imem_req_bus;
+ assign imem_write = imem_access_core ? imem_write_core : imem_write_bus;
+ assign imem_index = imem_access_core ? imem_index_core : imem_index_bus;
+ assign imem_wdata = imem_access_core ? 39'(imem_wdata_core) : imem_wdata_bus;
// The instruction memory only supports 32b word writes, so we hardcode its
// wmask here.
@@ -210,14 +211,14 @@
// the wmask signal from the bus is indeed '1 when it requests a write. We
// don't have the corresponding check for writes from the core because the
// core cannot perform writes (and has no imem_wmask_o port).
- assign imem_wmask = 32'hFFFFFFFF;
+ assign imem_wmask = imem_access_core ? '1 : imem_wmask_bus;
`ASSERT(ImemWmaskBusIsFullWord_A,
- imem_req_bus && imem_write_bus |-> imem_wmask_bus == 32'hFFFFFFFF)
+ imem_req_bus && imem_write_bus |-> imem_wmask_bus == '1)
// Explicitly tie off bus interface during core operation to avoid leaking
// the currently executed instruction from IMEM through the bus
// unintentionally.
- assign imem_rdata_bus = !imem_access_core ? imem_rdata[31:0] : 32'b0;
+ assign imem_rdata_bus = !imem_access_core ? imem_rdata : 39'b0;
assign imem_rdata_core = imem_rdata[31:0];
assign imem_rvalid_bus = !imem_access_core ? imem_rvalid : 1'b0;
@@ -272,9 +273,6 @@
logic dmem_req_bus;
logic dmem_write_bus;
logic [DmemIndexWidth-1:0] dmem_index_bus;
- logic [WLEN-1:0] dmem_wdata_bus_nointeg;
- logic [WLEN-1:0] dmem_wmask_bus_nointeg;
- logic [WLEN-1:0] dmem_rdata_bus_nointeg;
logic [ExtWLEN-1:0] dmem_wdata_bus;
logic [ExtWLEN-1:0] dmem_wmask_bus;
logic [ExtWLEN-1:0] dmem_rdata_bus;
@@ -290,7 +288,7 @@
prim_ram_1p_adv #(
.Width (ExtWLEN),
.Depth (DmemSizeWords),
- .DataBitsPerMask (39) // 39b write masks for 32b word writes from bus plus checksum
+ .DataBitsPerMask (39)
) u_dmem (
.clk_i,
.rst_ni,
@@ -318,37 +316,26 @@
.SramDw (WLEN),
.Outstanding (1),
.ByteAccess (0),
- .ErrOnRead (0)
+ .ErrOnRead (0),
+ .EnableDataIntgPt (1)
) u_tlul_adapter_sram_dmem (
.clk_i,
.rst_ni,
-
- .tl_i (tl_win_h2d[TlWinDmem] ),
- .tl_o (tl_win_d2h[TlWinDmem] ),
- .en_ifetch_i (tlul_pkg::InstrDis ),
- .req_o (dmem_req_bus ),
- .gnt_i (dmem_gnt_bus ),
- .we_o (dmem_write_bus ),
- .addr_o (dmem_index_bus ),
- .wdata_o (dmem_wdata_bus_nointeg),
- .wmask_o (dmem_wmask_bus_nointeg),
- .intg_error_o( ),
- .rdata_i (dmem_rdata_bus_nointeg),
- .rvalid_i (dmem_rvalid_bus ),
- .rerror_i (dmem_rerror_bus )
+ .tl_i (tl_win_h2d[TlWinDmem]),
+ .tl_o (tl_win_d2h[TlWinDmem]),
+ .en_ifetch_i (tlul_pkg::InstrDis ),
+ .req_o (dmem_req_bus ),
+ .gnt_i (dmem_gnt_bus ),
+ .we_o (dmem_write_bus ),
+ .addr_o (dmem_index_bus ),
+ .wdata_o (dmem_wdata_bus ),
+ .wmask_o (dmem_wmask_bus ),
+ .intg_error_o( ),
+ .rdata_i (dmem_rdata_bus ),
+ .rvalid_i (dmem_rvalid_bus ),
+ .rerror_i (dmem_rerror_bus )
);
- // The tlul_adapter doesn't currently handle the integrity bits that we will insert into the
- // memory. Convert between its view and that of the actual SRAM macro here.
- logic [BaseWordsPerWLEN-1:0] unused_rdata_bus_integrity;
- for (genvar i = 0; i < BaseWordsPerWLEN; i++) begin: gen_bus_dmem_adapter
- assign dmem_wdata_bus[i*39 +: 39] = {7'd0, dmem_wdata_bus_nointeg[i*32 +: 32]};
- assign dmem_wmask_bus[i*39 +: 39] = {{7{dmem_wmask_bus_nointeg[i*32]}},
- dmem_wmask_bus_nointeg[i*32 +: 32]};
- assign dmem_rdata_bus_nointeg[i*32 +: 32] = dmem_rdata_bus[i*39 +: 32];
- assign unused_rdata_bus_integrity[i] = &{dmem_rdata_bus[i*39 + 32 +: 7]};
- end
-
// Mux core and bus access into dmem
assign dmem_access_core = busy_q;
diff --git a/hw/ip/tlul/rtl/tlul_adapter_sram.sv b/hw/ip/tlul/rtl/tlul_adapter_sram.sv
index 4d6b150..87c03eb 100644
--- a/hw/ip/tlul/rtl/tlul_adapter_sram.sv
+++ b/hw/ip/tlul/rtl/tlul_adapter_sram.sv
@@ -10,6 +10,12 @@
* - Intentionally omitted BaseAddr in case of multiple memory maps are used in a SoC,
* it means that aliasing can happen if target device size in TL-UL crossbar is bigger
* than SRAM size
+ * - At most one of EnableDataIntgGen / EnableDataIntgPt can be enabled. However it
+ * possible for both to be disabled.
+ * A module can neither generate an integrity response nor pass through any pre-existing
+ * integrity. This might be the case for non-security critical memories where there is
+ * no stored integrity AND another entity upstream is already generating returning integrity.
+ * There is however no case where EnableDataIntgGen and EnableDataIntgPt are both true.
*/
module tlul_adapter_sram import tlul_pkg::*; #(
parameter int SramAw = 12,
@@ -20,7 +26,11 @@
parameter bit ErrOnRead = 0, // 1: Reads not allowed, automatically error
parameter bit CmdIntgCheck = 0, // 1: Enable command integrity check
parameter bit EnableRspIntgGen = 0, // 1: Generate response integrity
- parameter bit EnableDataIntgGen = 0 // 1: Generate data integrity
+ parameter bit EnableDataIntgGen = 0, // 1: Generate response data integrity
+ parameter bit EnableDataIntgPt = 0, // 1: Passthrough command/response data integrity
+ localparam int WidthMult = SramDw / top_pkg::TL_DW,
+ localparam int IntgWidth = tlul_pkg::DataIntgWidth * WidthMult,
+ localparam int DataOutW = EnableDataIntgPt ? SramDw + IntgWidth : SramDw
) (
input clk_i,
input rst_ni,
@@ -33,21 +43,20 @@
input tl_instr_en_e en_ifetch_i,
// SRAM interface
- output logic req_o,
- input gnt_i,
- output logic we_o,
- output logic [SramAw-1:0] addr_o,
- output logic [SramDw-1:0] wdata_o,
- output logic [SramDw-1:0] wmask_o,
- output logic intg_error_o,
- input [SramDw-1:0] rdata_i,
- input rvalid_i,
- input [1:0] rerror_i // 2 bit error [1]: Uncorrectable, [0]: Correctable
+ output logic req_o,
+ input gnt_i,
+ output logic we_o,
+ output logic [SramAw-1:0] addr_o,
+ output logic [DataOutW-1:0] wdata_o,
+ output logic [DataOutW-1:0] wmask_o,
+ output logic intg_error_o,
+ input [DataOutW-1:0] rdata_i,
+ input rvalid_i,
+ input [1:0] rerror_i // 2 bit error [1]: Uncorrectable, [0]: Correctable
);
localparam int SramByte = SramDw/8;
localparam int DataBitWidth = prim_util_pkg::vbits(SramByte);
- localparam int WidthMult = SramDw / top_pkg::TL_DW;
localparam int WoffsetWidth = (SramByte == top_pkg::TL_DBW) ? 1 :
DataBitWidth - prim_util_pkg::vbits(top_pkg::TL_DBW);
@@ -71,6 +80,7 @@
typedef struct packed {
logic [top_pkg::TL_DW-1:0] data ;
+ logic [DataIntgWidth-1:0] data_intg ;
logic error ;
} rsp_t ;
@@ -152,7 +162,7 @@
d_sink : 1'b0,
d_data : (d_valid && rspfifo_rvalid && reqfifo_rdata.op == OpRead)
? rspfifo_rdata.data : '0,
- d_user : TL_D_USER_DEFAULT,
+ d_user : '{default: '1, data_intg: d_valid ? rspfifo_rdata.data_intg : '1},
d_error : d_valid && d_error,
a_ready : (gnt_i | error_internal) & reqfifo_wready & sramreqfifo_wready
@@ -188,10 +198,23 @@
assign woffset = '0;
end
- // Convert byte mask to SRAM bit mask for writes, and only forward valid data
+ // The size of the data/wmask depends on whether passthrough integrity is enabled.
+ // If passthrough integrity is enabled, the data is concatenated with the integrity passed through
+ // the user bits. Otherwise, it is the data only.
+ localparam int DataWidth = EnableDataIntgPt ? top_pkg::TL_DW + DataIntgWidth : top_pkg::TL_DW;
+
+ // Final combined wmask / wdata
+ logic [WidthMult-1:0][DataWidth-1:0] wmask_combined;
+ logic [WidthMult-1:0][DataWidth-1:0] wdata_combined;
+
+ // Original tlul portion
logic [WidthMult-1:0][top_pkg::TL_DW-1:0] wmask_int;
logic [WidthMult-1:0][top_pkg::TL_DW-1:0] wdata_int;
+ // Integrity portion
+ logic [WidthMult-1:0][DataIntgWidth-1:0] wmask_intg;
+ logic [WidthMult-1:0][DataIntgWidth-1:0] wdata_intg;
+
always_comb begin
wmask_int = '0;
wdata_int = '0;
@@ -204,8 +227,36 @@
end
end
- assign wmask_o = wmask_int;
- assign wdata_o = wdata_int;
+ // TODO: The logic below is incomplete. If the adapter detects a write is NOT
+ // the full word, it must read back the other parts of the data from memory and
+ // re-generate the integrity.
+ // Since that will cause back-pressure to the upstream agent and likely substantial
+ // change into this module, it is left to a different PR.
+ always_comb begin
+ wmask_intg = '0;
+ wdata_intg = '0;
+
+ if (tl_i.a_valid) begin
+ wmask_intg[woffset] = '1;
+ wdata_intg[woffset] = tl_i.a_user.data_intg;
+ end
+ end
+
+ for (genvar i = 0; i < WidthMult; i++) begin : gen_write_output
+ if (EnableDataIntgPt) begin : gen_combined_output
+ assign wmask_combined[i] = {wmask_intg[i], wmask_int[i]};
+ assign wdata_combined[i] = {wdata_intg[i], wdata_int[i]};
+ end else begin : gen_ft_output
+ logic unused_w;
+ assign wmask_combined[i] = wmask_int[i];
+ assign wdata_combined[i] = wdata_int[i];
+ assign unused_w = |wmask_intg & |wdata_intg;
+ end
+ end
+
+ assign wmask_o = wmask_combined;
+ assign wdata_o = wdata_combined;
+
// Begin: Request Error Detection
@@ -287,15 +338,20 @@
assign rspfifo_wvalid = rvalid_i & reqfifo_rvalid;
// Make sure only requested bytes are forwarded
- logic [WidthMult-1:0][top_pkg::TL_DW-1:0] rdata;
- logic [WidthMult-1:0][top_pkg::TL_DW-1:0] rmask;
- //logic [SramDw-1:0] rmask;
- logic [top_pkg::TL_DW-1:0] rdata_tlword;
+ logic [WidthMult-1:0][DataWidth-1:0] rdata;
+ logic [WidthMult-1:0][DataWidth-1:0] rmask;
+ logic [DataWidth-1:0] rdata_tlword;
- always_comb begin
- rmask = '0;
- for (int i = 0 ; i < top_pkg::TL_DW/8 ; i++) begin
- rmask[sramreqfifo_rdata.woffset][8*i +: 8] = {8{sramreqfifo_rdata.mask[i]}};
+ // When passing through data integrity, we must feedback the entire
+ // read data, otherwise the stored integrity will not calculate correctly
+ if (EnableDataIntgPt) begin : gen_no_rmask
+ assign rmask = {DataOutW{|sramreqfifo_rdata.mask}};
+ end else begin : gen_rmask
+ always_comb begin
+ rmask = '0;
+ for (int i = 0 ; i < top_pkg::TL_DW/8 ; i++) begin
+ rmask[sramreqfifo_rdata.woffset][8*i +: 8] = {8{sramreqfifo_rdata.mask[i]}};
+ end
end
end
@@ -303,8 +359,9 @@
assign rdata_tlword = rdata[sramreqfifo_rdata.woffset];
assign rspfifo_wdata = '{
- data : rdata_tlword,
- error: rerror_i[1] // Only care for Uncorrectable error
+ data : rdata_tlword[top_pkg::TL_DW-1:0],
+ data_intg : EnableDataIntgPt ? rdata_tlword[DataWidth-1 -: DataIntgWidth] : '1,
+ error : rerror_i[1] // Only care for Uncorrectable error
};
assign rspfifo_rready = (reqfifo_rdata.op == OpRead & ~reqfifo_rdata.error)
? reqfifo_rready : 1'b0 ;
@@ -405,7 +462,10 @@
`ASSERT_INIT(adapterNoReadOrWrite, (ErrOnWrite & ErrOnRead) == 0)
`ASSERT_INIT(SramDwHasByteGranularity_A, SramDw % 8 == 0)
- `ASSERT_INIT(SramDwIsMultipleOfTlUlWidth_A, SramDw % top_pkg::TL_DW == 0)
+ `ASSERT_INIT(SramDwIsMultipleOfTlulWidth_A, SramDw % top_pkg::TL_DW == 0)
+
+ // These parameter options cannot both be true at the same time
+ `ASSERT_INIT(DataIntgOptions_A, ~(EnableDataIntgGen & EnableDataIntgPt))
// make sure outputs are defined
`ASSERT_KNOWN(TlOutKnown_A, tl_o )
diff --git a/hw/top_earlgrey/rtl/autogen/top_earlgrey.sv b/hw/top_earlgrey/rtl/autogen/top_earlgrey.sv
index 8dcd0df..faac82e 100644
--- a/hw/top_earlgrey/rtl/autogen/top_earlgrey.sv
+++ b/hw/top_earlgrey/rtl/autogen/top_earlgrey.sv
@@ -768,8 +768,8 @@
logic ram_main_we;
logic ram_main_intg_err;
logic [14:0] ram_main_addr;
- logic [31:0] ram_main_wdata;
- logic [31:0] ram_main_wmask;
+ logic [38:0] ram_main_wdata;
+ logic [38:0] ram_main_wmask;
logic [38:0] ram_main_rdata;
logic ram_main_rvalid;
logic [1:0] ram_main_rerror;
@@ -780,7 +780,8 @@
.Outstanding(2),
.CmdIntgCheck(1),
.EnableRspIntgGen(1),
- .EnableDataIntgGen(1) // TODO: Needs to be updated for integrity passthrough
+ .EnableDataIntgGen(0),
+ .EnableDataIntgPt(1)
) u_tl_adapter_ram_main (
.clk_i (clkmgr_aon_clocks.clk_main_infra),
.rst_ni (rstmgr_aon_resets.rst_sys_n[rstmgr_pkg::Domain0Sel]),
@@ -794,7 +795,7 @@
.wdata_o (ram_main_wdata),
.wmask_o (ram_main_wmask),
.intg_error_o(ram_main_intg_err),
- .rdata_i (ram_main_rdata[31:0]),
+ .rdata_i (ram_main_rdata),
.rvalid_i (ram_main_rvalid),
.rerror_i (ram_main_rerror)
);
@@ -823,8 +824,8 @@
.gnt_o (ram_main_gnt),
.write_i (ram_main_we),
.addr_i (ram_main_addr),
- .wdata_i (39'(ram_main_wdata)),
- .wmask_i (39'(ram_main_wmask)),
+ .wdata_i (ram_main_wdata),
+ .wmask_i (ram_main_wmask),
.rdata_o (ram_main_rdata),
.rvalid_o (ram_main_rvalid),
.rerror_o (ram_main_rerror),
@@ -841,8 +842,8 @@
logic ram_ret_aon_we;
logic ram_ret_aon_intg_err;
logic [9:0] ram_ret_aon_addr;
- logic [31:0] ram_ret_aon_wdata;
- logic [31:0] ram_ret_aon_wmask;
+ logic [38:0] ram_ret_aon_wdata;
+ logic [38:0] ram_ret_aon_wmask;
logic [38:0] ram_ret_aon_rdata;
logic ram_ret_aon_rvalid;
logic [1:0] ram_ret_aon_rerror;
@@ -853,7 +854,8 @@
.Outstanding(2),
.CmdIntgCheck(1),
.EnableRspIntgGen(1),
- .EnableDataIntgGen(1) // TODO: Needs to be updated for integrity passthrough
+ .EnableDataIntgGen(0),
+ .EnableDataIntgPt(1)
) u_tl_adapter_ram_ret_aon (
.clk_i (clkmgr_aon_clocks.clk_io_div4_infra),
.rst_ni (rstmgr_aon_resets.rst_sys_io_div4_n[rstmgr_pkg::DomainAonSel]),
@@ -867,7 +869,7 @@
.wdata_o (ram_ret_aon_wdata),
.wmask_o (ram_ret_aon_wmask),
.intg_error_o(ram_ret_aon_intg_err),
- .rdata_i (ram_ret_aon_rdata[31:0]),
+ .rdata_i (ram_ret_aon_rdata),
.rvalid_i (ram_ret_aon_rvalid),
.rerror_i (ram_ret_aon_rerror)
);
@@ -896,8 +898,8 @@
.gnt_o (ram_ret_aon_gnt),
.write_i (ram_ret_aon_we),
.addr_i (ram_ret_aon_addr),
- .wdata_i (39'(ram_ret_aon_wdata)),
- .wmask_i (39'(ram_ret_aon_wmask)),
+ .wdata_i (ram_ret_aon_wdata),
+ .wmask_i (ram_ret_aon_wmask),
.rdata_o (ram_ret_aon_rdata),
.rvalid_o (ram_ret_aon_rvalid),
.rerror_o (ram_ret_aon_rerror),
diff --git a/util/topgen/templates/toplevel.sv.tpl b/util/topgen/templates/toplevel.sv.tpl
index 7199a37..ddfecad 100644
--- a/util/topgen/templates/toplevel.sv.tpl
+++ b/util/topgen/templates/toplevel.sv.tpl
@@ -359,8 +359,8 @@
logic ${lib.bitarray(1, max_char)} ${m["name"]}_we;
logic ${lib.bitarray(1, max_char)} ${m["name"]}_intg_err;
logic ${lib.bitarray(addr_width, max_char)} ${m["name"]}_addr;
- logic ${lib.bitarray(data_width, max_char)} ${m["name"]}_wdata;
- logic ${lib.bitarray(data_width, max_char)} ${m["name"]}_wmask;
+ logic ${lib.bitarray(full_data_width, max_char)} ${m["name"]}_wdata;
+ logic ${lib.bitarray(full_data_width, max_char)} ${m["name"]}_wmask;
logic ${lib.bitarray(full_data_width, max_char)} ${m["name"]}_rdata;
logic ${lib.bitarray(1, max_char)} ${m["name"]}_rvalid;
logic ${lib.bitarray(2, max_char)} ${m["name"]}_rerror;
@@ -371,7 +371,8 @@
.Outstanding(2),
.CmdIntgCheck(1),
.EnableRspIntgGen(1),
- .EnableDataIntgGen(1) // TODO: Needs to be updated for integrity passthrough
+ .EnableDataIntgGen(0),
+ .EnableDataIntgPt(1)
) u_tl_adapter_${m["name"]} (
% for key in clocks:
.${key} (${clocks[key]}),
@@ -389,7 +390,7 @@
.wdata_o (${m["name"]}_wdata),
.wmask_o (${m["name"]}_wmask),
.intg_error_o(${m["name"]}_intg_err),
- .rdata_i (${m["name"]}_rdata[${data_width-1}:0]),
+ .rdata_i (${m["name"]}_rdata),
.rvalid_i (${m["name"]}_rvalid),
.rerror_i (${m["name"]}_rerror)
);
@@ -426,8 +427,8 @@
.gnt_o (${m["name"]}_gnt),
.write_i (${m["name"]}_we),
.addr_i (${m["name"]}_addr),
- .wdata_i (${full_data_width}'(${m["name"]}_wdata)),
- .wmask_i (${full_data_width}'(${m["name"]}_wmask)),
+ .wdata_i (${m["name"]}_wdata),
+ .wmask_i (${m["name"]}_wmask),
.rdata_o (${m["name"]}_rdata),
.rvalid_o (${m["name"]}_rvalid),
.rerror_o (${m["name"]}_rerror),
