From 8f5bb28f11c87b6a704c70866520baf1a070d0ab Mon Sep 17 00:00:00 2001 From: Michael Rogenmoser Date: Thu, 8 Feb 2024 11:51:27 +0100 Subject: [PATCH] Add initial L1 parity protection --- src/snitch_icache.sv | 3 + src/snitch_icache_handler.sv | 4 +- src/snitch_icache_lookup_serial.sv | 193 +++++++++++++++++++++++------ src/snitch_icache_pkg.sv | 1 + 4 files changed, 164 insertions(+), 37 deletions(-) diff --git a/src/snitch_icache.sv b/src/snitch_icache.sv index 0204af9..9df7de7 100644 --- a/src/snitch_icache.sv +++ b/src/snitch_icache.sv @@ -26,6 +26,8 @@ module snitch_icache #( parameter int unsigned FILL_AW = -1, /// Fill interface data width. Power of two; >= 8. parameter int unsigned FILL_DW = -1, + /// Extra parity bits to add to a line for L1 reliability. + parameter int unsigned L1_DATA_PARITY_BITS = 0, /// Serialize the L1 lookup (parallel tag/data lookup by default) parameter bit SERIAL_LOOKUP = 0, /// Replace the L1 tag banks with latch-based SCM. @@ -90,6 +92,7 @@ module snitch_icache #( FETCH_DW: FETCH_DW, FILL_AW: FILL_AW, FILL_DW: FILL_DW, + L1_DATA_PARITY_BITS: L1_DATA_PARITY_BITS, L1_TAG_SCM: L1_TAG_SCM, EARLY_LATCH: EARLY_LATCH, BUFFER_LOOKUP: 0, diff --git a/src/snitch_icache_handler.sv b/src/snitch_icache_handler.sv index ed21b39..5c8c8e7 100644 --- a/src/snitch_icache_handler.sv +++ b/src/snitch_icache_handler.sv @@ -218,8 +218,8 @@ module snitch_icache_handler #( in_req_ready_o = hit_ready; // The cache lookup was a miss, but there is already a pending - // refill that covers the line. - end else if (pending) begin + // refill that covers the line and the lookup accepted the request. + end else if (pending && !(write_valid_o && !write_ready_i)) begin push_index = pending_id; push_enable = 1; diff --git a/src/snitch_icache_lookup_serial.sv b/src/snitch_icache_lookup_serial.sv index 0f80eb8..4de8bca 100644 --- a/src/snitch_icache_lookup_serial.sv +++ b/src/snitch_icache_lookup_serial.sv @@ -46,6 +46,7 @@ module snitch_icache_lookup_serial #( ); localparam int unsigned DataAddrWidth = $clog2(CFG.SET_COUNT) + CFG.COUNT_ALIGN; + localparam int unsigned TagParity = (CFG.L1_DATA_PARITY_BITS > 0) ? 1 : 0; `ifndef SYNTHESIS initial assert(CFG != '0); @@ -81,13 +82,19 @@ module snitch_icache_lookup_serial #( logic error; } tag_rsp_t; + typedef struct packed { + logic [ CFG.FETCH_AW-1:0] addr; + logic [CFG.SET_ALIGN-1:0] cset; + logic parity_error; + } tag_inv_req_t; + logic req_valid, req_ready; logic req_handshake; - logic [CFG.COUNT_ALIGN-1:0] tag_addr; - logic [CFG.SET_COUNT-1:0] tag_enable; - logic [CFG.TAG_WIDTH+1:0] tag_wdata, tag_rdata [CFG.SET_COUNT]; - logic tag_write; + logic [CFG.COUNT_ALIGN-1:0] tag_addr; + logic [CFG.SET_COUNT-1:0] tag_enable; + logic [CFG.TAG_WIDTH+1+TagParity:0] tag_wdata, tag_rdata [CFG.SET_COUNT]; + logic tag_write; tag_req_t tag_req_d, tag_req_q; tag_rsp_t tag_rsp_s, tag_rsp_d, tag_rsp_q, tag_rsp; @@ -96,36 +103,82 @@ module snitch_icache_lookup_serial #( logic [CFG.TAG_WIDTH-1:0] required_tag; logic [CFG.SET_COUNT-1:0] line_hit; + logic [CFG.SET_COUNT-1:0] tag_parity_error_d, tag_parity_error_q; + logic faulty_hit_valid, faulty_hit_ready, faulty_hit_d, faulty_hit_q; logic [DataAddrWidth-1:0] lookup_addr; logic [DataAddrWidth-1:0] write_addr; + tag_inv_req_t data_parity_inv_d, data_parity_inv_q; + logic data_fault_valid, data_fault_ready; + // Connect input requests to tag stage assign tag_req_d.addr = in_addr_i; assign tag_req_d.id = in_id_i; // Multiplex read and write access to the tag banks onto one port, prioritizing write accesses + + logic tag_parity_bit; + if (TagParity > 0) begin + always_comb begin + tag_parity_bit = ^write_tag_i; + if (init_phase) begin + tag_parity_bit = 1'b0; + end else if (data_fault_valid) begin + tag_parity_bit = 1'b1; + end else if (write_valid_i) begin + tag_parity_bit = ^write_tag_i; + end else if (faulty_hit_valid) begin + tag_parity_bit = 1'b1; + end else if (in_valid_i) begin + // read phase: write tag not used + end + tag_wdata[CFG.TAG_WIDTH+2] = tag_parity_bit; + end + end else begin + assign tag_parity_bit = '0; + end + + assign data_fault_valid = (CFG.L1_DATA_PARITY_BITS > 0) ? data_parity_inv_q : '0; + always_comb begin - tag_addr = in_addr_i[CFG.LINE_ALIGN +: CFG.COUNT_ALIGN]; - tag_enable = '0; - tag_wdata = {1'b1, write_error_i, write_tag_i}; - tag_write = 1'b0; + tag_addr = in_addr_i[CFG.LINE_ALIGN +: CFG.COUNT_ALIGN]; + tag_enable = '0; + tag_wdata[CFG.TAG_WIDTH+1:0] = {1'b1, write_error_i, write_tag_i}; + tag_write = 1'b0; write_ready_o = 1'b0; in_ready_o = 1'b0; req_valid = 1'b0; + data_fault_ready = 1'b0; + faulty_hit_ready = 1'b0; + if (init_phase) begin - tag_addr = init_count_q; - tag_enable = '1; - tag_wdata = '0; - tag_write = 1'b1; + tag_addr = init_count_q; + tag_enable = '1; + tag_wdata[CFG.TAG_WIDTH+1:0] = '0; + tag_write = 1'b1; + end else if (data_fault_valid) begin // Only if data has parity + tag_addr = data_parity_inv_q.addr >> CFG.LINE_ALIGN; + tag_enable = $unsigned(1 << data_parity_inv_q.cset); + tag_wdata[CFG.TAG_WIDTH+1:0] = '0; + tag_write = 1'b1; + data_fault_ready = 1'b1; end else if (write_valid_i) begin // Write a refill request tag_addr = write_addr_i; tag_enable = $unsigned(1 << write_set_i); tag_write = 1'b1; write_ready_o = 1'b1; + end else if (faulty_hit_valid) begin // Only if tag has parity + // we need to set second bit (valid) of write data of the previous adress to 0 + // we do not accept read requests and we do not store data in the pipeline. + tag_addr = tag_req_q >> CFG.LINE_ALIGN; // buffered version of in_addr_i + tag_enable = tag_parity_error_q; // faulty set must be written to + tag_wdata[CFG.TAG_WIDTH+1:0] = '0; + tag_write = 1'b0; + faulty_hit_ready = 1'b1; end else if (in_valid_i) begin // Check cache tag_enable = '1; @@ -139,8 +192,8 @@ module snitch_icache_lookup_serial #( if (CFG.L1_TAG_SCM) begin : gen_scm for (genvar i = 0; i < CFG.SET_COUNT; i++) begin : g_sets register_file_1r_1w #( - .ADDR_WIDTH ($clog2(CFG.LINE_COUNT)), - .DATA_WIDTH (CFG.TAG_WIDTH+2 ) + .ADDR_WIDTH ($clog2(CFG.LINE_COUNT) ), + .DATA_WIDTH (CFG.TAG_WIDTH+2+TagParity) ) i_tag ( .clk ( clk_i ), .ReadEnable ( tag_enable[i] && !tag_write ), @@ -152,16 +205,16 @@ module snitch_icache_lookup_serial #( ); end end else begin : gen_sram - logic [CFG.SET_COUNT*(CFG.TAG_WIDTH+2)-1:0] tag_rdata_flat; + logic [CFG.SET_COUNT*(CFG.TAG_WIDTH+2+TagParity)-1:0] tag_rdata_flat; for (genvar i = 0; i < CFG.SET_COUNT; i++) begin : g_sets_rdata - assign tag_rdata[i] = tag_rdata_flat[i*(CFG.TAG_WIDTH+2)+:CFG.TAG_WIDTH+2]; + assign tag_rdata[i] = tag_rdata_flat[i*(CFG.TAG_WIDTH+2+TagParity)+:CFG.TAG_WIDTH+2+TagParity]; end tc_sram_impl #( - .DataWidth ( (CFG.TAG_WIDTH+2) * CFG.SET_COUNT ), - .ByteWidth ( CFG.TAG_WIDTH+2 ), - .NumWords ( CFG.LINE_COUNT ), - .NumPorts ( 1 ), - .impl_in_t ( sram_cfg_tag_t ) + .DataWidth ( (CFG.TAG_WIDTH+2+TagParity) * CFG.SET_COUNT ), + .ByteWidth ( CFG.TAG_WIDTH+2+TagPariyt ), + .NumWords ( CFG.LINE_COUNT ), + .NumPorts ( 1 ), + .impl_in_t ( sram_cfg_tag_t ) ) i_tag ( .clk_i ( clk_i ), .rst_ni ( rst_ni ), @@ -176,6 +229,17 @@ module snitch_icache_lookup_serial #( ); end + // compute tag parity bit the cycle before reading the tag and buffer it + logic exp_tag_parity_bit_d, exp_tag_parity_bit_q; + + if (TagParity>0) begin + assign exp_tag_parity_bit_d = ^(tag_req_d.addr >> (CFG.LINE_ALIGN + CFG.COUNT_ALIGN)); + `FFL(exp_tag_parity_bit_q, exp_tag_parity_bit_d, req_valid && req_ready, '0, clk_i, rst_ni); + end else begin + assign exp_tag_parity_bit_d = '0; + assign exp_tag_parity_bit_q = '0; + end + // Determine which set hit logic [CFG.SET_COUNT-1:0] errors; assign required_tag = tag_req_q.addr[CFG.FETCH_AW-1:CFG.LINE_ALIGN + CFG.COUNT_ALIGN]; @@ -184,9 +248,20 @@ module snitch_icache_lookup_serial #( tag_rdata[i][CFG.TAG_WIDTH-1:0] == required_tag; // check valid bit and tag assign errors[i] = tag_rdata[i][CFG.TAG_WIDTH] && line_hit[i]; // check error bit end - assign tag_rsp_s.hit = |line_hit; assign tag_rsp_s.error = |errors; + if (TagParity>0) begin + for (genvar i = 0; i < CFG.SET_COUNT; i++) begin + assign tag_parity_error_d[i] = ~((tag_rdata[i][CFG.TAG_WIDTH+2] == exp_tag_parity_bit_q)); + end + assign tag_rsp_s.hit = |(line_hit & ~tag_parity_error_d); + assign faulty_hit_d = |(line_hit & tag_parity_error_d); + end else begin + assign tag_rsp_s.hit = |line_hit; + assign tag_parity_error_d = '0; + assign faulty_hit_d = '0; + end + lzc #(.WIDTH(CFG.SET_COUNT)) i_lzc ( .in_i ( line_hit ), .cnt_o ( tag_rsp_s.cset ), @@ -196,6 +271,17 @@ module snitch_icache_lookup_serial #( // Buffer the metadata on a valid handshake. Stall on write (implicit in req_valid/ready) `FFL(tag_req_q, tag_req_d, req_valid && req_ready, '0, clk_i, rst_ni) `FF(tag_valid, req_valid ? 1'b1 : tag_ready ? 1'b0 : tag_valid, '0, clk_i, rst_ni) + if (TagParity>0) begin + // save faulty sets and clear when upstream invalidated them + `FFL(tag_parity_error_q, tag_parity_error_d, req_valid && req_ready, '0, clk_i, rst_ni) + `FFL(faulty_hit_q, (faulty_hit_ready && !(req_valid && req_ready)) ? 1'b0 : faulty_hit_d, + req_valid && req_ready || faulty_hit_ready, '0, clk_i, rst_ni) + end else begin + assign tag_parity_error_q = '0; + assign faulty_hit_q = '0; + end + assign faulty_hit_valid = faulty_hit_q; + // Ready if buffer is empy or downstream is reading. Stall on write assign req_ready = (!tag_valid || tag_ready) && !tag_write; @@ -213,7 +299,7 @@ module snitch_icache_lookup_serial #( tag_rsp_d = tag_rsp_s; end // Override the hit if the write that stalled us invalidated the data - if (lookup_addr == write_addr && write_valid_i) begin + if ((lookup_addr == write_addr) && write_valid_i && write_ready_o) begin tag_rsp_d.hit = 1'b0; end end @@ -232,10 +318,15 @@ module snitch_icache_lookup_serial #( typedef logic [CFG.LINE_WIDTH-1:0] data_rsp_t; - logic [DataAddrWidth-1:0] data_addr; - logic data_enable; - data_rsp_t data_wdata, data_rdata; - logic data_write; + logic [DataAddrWidth+CFG.L1_DATA_PARITY_BITS-1:0] data_addr; + logic data_enable; + data_rsp_t data_wdata, data_rdata; + logic data_write; + + data_req_t data_req_d, data_req_q; + data_rsp_t data_rsp_q; + logic data_valid, data_ready; + logic hit_invalid, hit_invalid_q; data_req_t data_req_d, data_req_q; data_rsp_t data_rsp_q; @@ -251,15 +342,22 @@ module snitch_icache_lookup_serial #( assign lookup_addr = {tag_rsp.cset, tag_req_q.addr[CFG.LINE_ALIGN +: CFG.COUNT_ALIGN]}; assign write_addr = {write_set_i, write_addr_i}; + localparam LINE_PARITY_SPLIT = CFG.LINE_WIDTH/CFG.L1_DATA_PARITY_BITS; + if (CFG.L1_DATA_PARITY_BITS>0) begin + for (genvar i = 0; i < CFG.L1_DATA_PARITY_BITS; i++) begin + assign data_wdata[CFG.LINE_WIDTH+CFG.L1_DATA_PARITY_BITS-1-i] = ~^write_data_i[CFG.LINE_WIDTH-LINE_PARITY_SPLIT*i-1 -: LINE_PARITY_SPLIT]; + end + end + assign data_wdata[CFG.LINE_WIDTH-1:0] = write_data_i; + // Data bank port mux always_comb begin // Default read request data_addr = lookup_addr; data_enable = tag_valid && tag_rsp.hit; // Only read data on hit - data_wdata = write_data_i; data_write = 1'b0; - // Write takes priority - if (!init_phase && write_valid_i) begin + // Write takes priority (except with invalidation due to parity error) + if (!init_phase && write_valid_i && !data_fault_valid) begin data_addr = write_addr; data_enable = 1'b1; data_write = 1'b1; @@ -267,10 +365,10 @@ module snitch_icache_lookup_serial #( end tc_sram_impl #( - .DataWidth ( CFG.LINE_WIDTH ), - .NumWords ( CFG.LINE_COUNT * CFG.SET_COUNT ), - .NumPorts ( 1 ), - .impl_in_t ( sram_cfg_data_t ) + .DataWidth ( CFG.LINE_WIDTH + CFG.L1_DATA_PARITY_BITS ), + .NumWords ( CFG.LINE_COUNT * CFG.SET_COUNT ), + .NumPorts ( 1 ), + .impl_in_t ( sram_cfg_data_t ) ) i_data ( .clk_i ( clk_i ), .rst_ni ( rst_ni ), @@ -284,6 +382,21 @@ module snitch_icache_lookup_serial #( .rdata_o ( data_rdata ) ); + // Parity check + if (CFG.L1_DATA_PARITY_BITS>0) begin + logic [CFG.L1_DATA_PARITY_BITS-1:0] data_parity_error; + + for (genvar i = 0; i < CFG.L1_DATA_PARITY_BITS; i++) begin + assign data_parity_error[i] = ^data_rdata[CFG.LINE_WIDTH-LINE_PARITY_SPLIT*i-1 -: LINE_PARITY_SPLIT]; + end + + assign data_parity_inv_d.parity_error = |data_parity_error; + assign data_parity_inv_d.addr = data_req_q.addr; + assign data_parity_inv_d.cset = data_req_q.id; + end else begin + assign data_parity_inv_d = '0; + end + // Buffer the metadata on a valid handshake. Stall on write (implicit in tag_ready) `FFL(data_req_q, data_req_d, tag_valid && tag_ready, '0, clk_i, rst_ni) `FF(data_valid, (tag_valid && !data_write) ? 1'b1 : data_ready ? 1'b0 : data_valid, @@ -300,11 +413,21 @@ module snitch_icache_lookup_serial #( `FFL(data_rsp_q, data_rdata, tag_handshake && !data_ready, '0, clk_i, rst_ni) assign out_data_o = tag_handshake ? data_rdata : data_rsp_q; + // Buffer the metadata when there is faulty data for the invalidation procedure + if (CFG.L1_DATA_PARITY_BITS > 0) begin + `FFL(data_parity_inv_q, (data_fault_ready && !tag_handshake) ? '0 : data_parity_inv_d, tag_handshake || data_fault_ready, '0, clk_i, rst_ni) + `FFL(hit_invalid_q, data_parity_inv_d.parity_error, tag_handshake, '0, clk_i, rst_ni) + end else begin + assign data_parity_inv_q = '0; + assign hit_invalid_q = '0; + end + assign hit_invalid = tag_handshake ? data_parity_inv_d.parity_error : hit_invalid_q; + // Generate the remaining output signals. assign out_addr_o = data_req_q.addr; assign out_id_o = data_req_q.id; assign out_set_o = data_req_q.cset; - assign out_hit_o = data_req_q.hit; + assign out_hit_o = data_req_q.hit && !hit_invalid; assign out_error_o = data_req_q.error; assign out_valid_o = data_valid; assign data_ready = out_ready_i; diff --git a/src/snitch_icache_pkg.sv b/src/snitch_icache_pkg.sv index 4014bd1..dc98a0b 100644 --- a/src/snitch_icache_pkg.sv +++ b/src/snitch_icache_pkg.sv @@ -26,6 +26,7 @@ package snitch_icache_pkg; int unsigned FETCH_DW; int unsigned FILL_AW; int unsigned FILL_DW; + int unsigned L1_DATA_PARITY_BITS; bit L1_TAG_SCM; bit EARLY_LATCH; bit BUFFER_LOOKUP;