From 246146968904bcda0cdfddc53dfbadec1c4477e8 Mon Sep 17 00:00:00 2001 From: Rupert Swarbrick Date: Thu, 28 Nov 2024 14:07:49 +0000 Subject: [PATCH] [prim_fifo_sync,rtl] Specialize for Depth == 1 This case is pretty simple to reason about, and avoids needing proper read and write addresses (because there is only one element). Unfortunately, it's quite a big commit! This is because we end up changing some of the duplicated counters inside the fifo. Items in the commit: - Update the RTL in prim_fifo_sync.sv to specialize as you'd expect. - Add an interface that can be bound in to prim_fifo_sync.sv and will register a way for sec_cm tests to use forced signals to simulate fault injections. - Bind that interface into the prim_fifo_sync instances - Wrap up the ASSERT_PRIM_COUNT_ERROR_TRIGGER_ALERT calls that are used for prim_fifo_sync instances. After the change, the two macro calls that were previously used (for the read and write pointer) are now handled by a single macro. In instances with Depth >= 2, this turns into ASSERT_PRIM_FIFO_SYNC_ERROR_TRIGGERS_ALERT. For singleton instances, it's ASSERT_PRIM_FIFO_SYNC_ERROR_TRIGGERS_ALERT1. I don't *think* you can make the dispatch between the two options automatic (because the pre-processor can't know the values of the Depth parameter). - Use these new macros in the various blocks that were using prim_fifo_sync in secure mode with Depth = 1. This is CSRNG, OTBN and flash_ctrl. Signed-off-by: Rupert Swarbrick --- hw/dv/sv/sec_cm/prim_singleton_fifo_if.sv | 74 ++++++++++++ hw/dv/sv/sec_cm/sec_cm.core | 2 + hw/dv/sv/sec_cm/sec_cm_pkg.sv | 3 +- .../sec_cm/sec_cm_prim_singleton_fifo_bind.sv | 11 ++ hw/ip/csrng/dv/tb.sv | 12 +- .../dv/uvm/env/seq_lib/otbn_common_vseq.sv | 28 ----- hw/ip/otbn/rtl/otbn.sv | 57 ++++----- hw/ip/prim/prim_fifo.core | 1 + hw/ip/prim/rtl/prim_assert_sec_cm.svh | 3 + hw/ip/prim/rtl/prim_fifo_assert.sv | 47 ++++++++ hw/ip/prim/rtl/prim_fifo_sync.sv | 113 ++++++++++++++---- .../dv/env/seq_lib/flash_ctrl_common_vseq.sv | 2 - .../flash_ctrl/rtl/flash_ctrl.sv.tpl | 89 +++++--------- .../dv/env/seq_lib/flash_ctrl_common_vseq.sv | 2 - .../ip_autogen/flash_ctrl/rtl/flash_ctrl.sv | 89 +++++--------- 15 files changed, 315 insertions(+), 218 deletions(-) create mode 100644 hw/dv/sv/sec_cm/prim_singleton_fifo_if.sv create mode 100644 hw/dv/sv/sec_cm/sec_cm_prim_singleton_fifo_bind.sv create mode 100644 hw/ip/prim/rtl/prim_fifo_assert.sv diff --git a/hw/dv/sv/sec_cm/prim_singleton_fifo_if.sv b/hw/dv/sv/sec_cm/prim_singleton_fifo_if.sv new file mode 100644 index 00000000000000..2d452094d92c7e --- /dev/null +++ b/hw/dv/sv/sec_cm/prim_singleton_fifo_if.sv @@ -0,0 +1,74 @@ +// Copyright lowRISC contributors (OpenTitan project). +// Licensed under the Apache License, Version 2.0, see LICENSE for details. +// SPDX-License-Identifier: Apache-2.0 + +// Countermeasure interface for the hardened 1-bit counter in prim_fifo_sync if Depth=1. +// +// This interface can be bound into a prim_fifo_sync instance. Many instances of the interface will +// not do anything, because Depth > 1 or or Secure = 0. However if Depth = 1 and Secure = 1, the +// nested prim_singleton_fifo_if_proxy class will register the instance in sec_cm_pkg and can +// inject/restore faults. +interface prim_singleton_fifo_if #( + parameter int Depth = 1, + parameter bit Secure = 1'b1 +) ( + input clk_i, + input rst_ni +); + + `include "dv_macros.svh" + `include "uvm_macros.svh" + import uvm_pkg::*; + + string msg_id = $sformatf("%m"); + + string path = dv_utils_pkg::get_parent_hier($sformatf("%m")); + string signal_forced; + + class prim_singleton_fifo_if_proxy extends sec_cm_pkg::sec_cm_base_if_proxy; + logic orig_value; + + function new(string name=""); + super.new(name); + endfunction + + virtual task automatic inject_fault(); + logic force_value; + + @(negedge clk_i); + `DV_CHECK(uvm_hdl_read(signal_forced, orig_value)) + `DV_CHECK(uvm_hdl_force(signal_forced, ~orig_value)) + `uvm_info(msg_id, + $sformatf("Forcing %s from %0d to %0d", signal_forced, orig_value, ~orig_value), + UVM_LOW) + + @(negedge clk_i); + `DV_CHECK(uvm_hdl_release(signal_forced)) + endtask + + virtual task automatic restore_fault(); + `DV_CHECK(uvm_hdl_deposit(signal_forced, orig_value)) + `uvm_info(msg_id, $sformatf("Forcing %s to original value %0d", signal_forced, orig_value), + UVM_LOW) + endtask + endclass + + if (Depth == 1 && Secure) begin : gen_secure_singleton + prim_singleton_fifo_if_proxy if_proxy; + initial begin + string local_signal; + local_signal = $urandom_range(0, 1) ? "inv_full" : "full_q"; + signal_forced = $sformatf("%s.%s", path, local_signal); + `DV_CHECK_FATAL(uvm_hdl_check_path(signal_forced),, msg_id) + + // Store the proxy object for TB to use + if_proxy = new("if_proxy"); + if_proxy.sec_cm_type = sec_cm_pkg::SecCmSingletonFifo; + if_proxy.path = path; + sec_cm_pkg::sec_cm_if_proxy_q.push_back(if_proxy); + + `uvm_info(msg_id, $sformatf("Interface proxy class is added for %s", path), UVM_HIGH) + end + end + +endinterface diff --git a/hw/dv/sv/sec_cm/sec_cm.core b/hw/dv/sv/sec_cm/sec_cm.core index da7bb5e76cdcec..09aa45c226dce8 100644 --- a/hw/dv/sv/sec_cm/sec_cm.core +++ b/hw/dv/sv/sec_cm/sec_cm.core @@ -21,10 +21,12 @@ filesets: - prim_count_if.sv - prim_sparse_fsm_flop_if.sv - prim_double_lfsr_if.sv + - prim_singleton_fifo_if.sv - sec_cm_prim_onehot_check_bind.sv - sec_cm_prim_count_bind.sv - sec_cm_prim_sparse_fsm_flop_bind.sv - sec_cm_prim_double_lfsr_bind.sv + - sec_cm_prim_singleton_fifo_bind.sv file_type: systemVerilogSource targets: diff --git a/hw/dv/sv/sec_cm/sec_cm_pkg.sv b/hw/dv/sv/sec_cm/sec_cm_pkg.sv index 18d23d9188c27e..12d6c6ef611c46 100644 --- a/hw/dv/sv/sec_cm/sec_cm_pkg.sv +++ b/hw/dv/sv/sec_cm/sec_cm_pkg.sv @@ -18,7 +18,8 @@ package sec_cm_pkg; SecCmPrimCount, SecCmPrimSparseFsmFlop, SecCmPrimDoubleLfsr, - SecCmPrimOnehot + SecCmPrimOnehot, + SecCmSingletonFifo } sec_cm_type_e; `include "sec_cm_base_if_proxy.sv" diff --git a/hw/dv/sv/sec_cm/sec_cm_prim_singleton_fifo_bind.sv b/hw/dv/sv/sec_cm/sec_cm_prim_singleton_fifo_bind.sv new file mode 100644 index 00000000000000..a9d362f783a5f5 --- /dev/null +++ b/hw/dv/sv/sec_cm/sec_cm_prim_singleton_fifo_bind.sv @@ -0,0 +1,11 @@ +// Copyright lowRISC contributors (OpenTitan project). +// Licensed under the Apache License, Version 2.0, see LICENSE for details. +// SPDX-License-Identifier: Apache-2.0 + +module sec_cm_prim_singleton_fifo_bind (); + // Bind prim_singleton_fifo_if into each instance of prim_fifo_sync. This will have no effect + // unless the fifo has Depth=1 and Secure=1. + bind + prim_fifo_sync + prim_singleton_fifo_if #(.Depth(Depth), .Secure(Secure)) u_prim_singleton_fifo_if (.*); +endmodule diff --git a/hw/ip/csrng/dv/tb.sv b/hw/ip/csrng/dv/tb.sv index 1ed1e8d18ccf4e..0d79155f332b92 100644 --- a/hw/ip/csrng/dv/tb.sv +++ b/hw/ip/csrng/dv/tb.sv @@ -141,22 +141,22 @@ module tb; // v is released into the ctr_drbg_gen/update blocks. `define BLOCK_ENCRYPT_PATH tb.dut.u_csrng_core.u_csrng_block_encrypt `define CTR_DRBG_GEN tb.dut.u_csrng_core.u_csrng_ctr_drbg_gen - `define CTR_DRBG_GEN_FIFO `CTR_DRBG_GEN.u_prim_fifo_sync_bencack.gen_normal_fifo + `define CTR_DRBG_GEN_FIFO `CTR_DRBG_GEN.u_prim_fifo_sync_bencack.gen_singleton_fifo `ASSERT_INIT(CsrngCtrDrbgGenFifoDepth1, `CTR_DRBG_GEN.BlkEncAckFifoDepth == 1) `ASSERT(CsrngSecCmAesCipherDataRegLocalEscGen, - $past(`CTR_DRBG_GEN_FIFO.fifo_incr_wptr) && + $rose(`CTR_DRBG_GEN_FIFO.full_q) && `BLOCK_ENCRYPT_PATH.block_encrypt_aes_cipher_sm_err_o |=> - $past(`CTR_DRBG_GEN_FIFO.storage[0] + $past(`CTR_DRBG_GEN_FIFO.storage [`CTR_DRBG_GEN.BlkEncAckFifoWidth-1 -: `CTR_DRBG_GEN.BlkLen]) != $past(`BLOCK_ENCRYPT_PATH.cipher_data_out, 2), clk, !rst_n) `define CTR_DRBG_UPD tb.dut.u_csrng_core.u_csrng_ctr_drbg_upd - `define CTR_DRBG_UPD_FIFO `CTR_DRBG_UPD.u_prim_fifo_sync_bencack.gen_normal_fifo + `define CTR_DRBG_UPD_FIFO `CTR_DRBG_UPD.u_prim_fifo_sync_bencack.gen_singleton_fifo `ASSERT_INIT(CsrngCtrDrbgUpdFifoDepth1, `CTR_DRBG_UPD.BlkEncAckFifoDepth == 1) `ASSERT(CsrngSecCmAesCipherDataRegLocalEscUpd, - $past(`CTR_DRBG_UPD_FIFO.fifo_incr_wptr) && + $rose(`CTR_DRBG_UPD_FIFO.full_q) && `BLOCK_ENCRYPT_PATH.block_encrypt_aes_cipher_sm_err_o |=> - $past(`CTR_DRBG_UPD_FIFO.storage[0] + $past(`CTR_DRBG_UPD_FIFO.storage [`CTR_DRBG_UPD.BlkEncAckFifoWidth-1 -: `CTR_DRBG_UPD.BlkLen]) != $past(`BLOCK_ENCRYPT_PATH.cipher_data_out, 2), clk, !rst_n) diff --git a/hw/ip/otbn/dv/uvm/env/seq_lib/otbn_common_vseq.sv b/hw/ip/otbn/dv/uvm/env/seq_lib/otbn_common_vseq.sv index ff5b4b060785a0..4edf3de7a76ad5 100644 --- a/hw/ip/otbn/dv/uvm/env/seq_lib/otbn_common_vseq.sv +++ b/hw/ip/otbn/dv/uvm/env/seq_lib/otbn_common_vseq.sv @@ -10,34 +10,6 @@ class otbn_common_vseq extends otbn_base_vseq; } `uvm_object_new - // Write rubbish to the storage backing memory for a prim_fifo_sync - function void splat_fifo_storage(string fifo_path, int unsigned depth); - for (int unsigned i = 0; i < depth; i++) begin - string storage_path = $sformatf("%0s.gen_normal_fifo.storage[%0d]", fifo_path, i); - bit [31:0] value; - randcase - 1: value = 0; - 1: value = '1; - 1: value = $urandom; - endcase - `DV_CHECK_FATAL(uvm_hdl_deposit(storage_path, value)) - end - endfunction - - task dut_init(string reset_kind = "HARD"); - // Zero the contents of the DMEM/IMEM request fifos if we're about to do fault injection on - // their counters. This avoids a problem where we generate a spurious request when the FIFO was - // actually empty and lots of signals in the design become X. This includes OTBN's status signal - // and we would never get to the "LOCKED" status because we're stuck at X. Zeroing the backing - // memory avoids that problem. - splat_fifo_storage("tb.dut.u_tlul_adapter_sram_dmem.u_reqfifo", 1); - splat_fifo_storage("tb.dut.u_tlul_adapter_sram_dmem.u_sramreqfifo", 1); - splat_fifo_storage("tb.dut.u_tlul_adapter_sram_imem.u_reqfifo", 1); - splat_fifo_storage("tb.dut.u_tlul_adapter_sram_imem.u_sramreqfifo", 1); - - super.dut_init(reset_kind); - endtask - virtual task body(); enable_base_alert_checks = 1'b1; diff --git a/hw/ip/otbn/rtl/otbn.sv b/hw/ip/otbn/rtl/otbn.sv index 840ed81fc6c151..9fe5ac9969e34d 100644 --- a/hw/ip/otbn/rtl/otbn.sv +++ b/hw/ip/otbn/rtl/otbn.sv @@ -3,6 +3,7 @@ // SPDX-License-Identifier: Apache-2.0 `include "prim_assert.sv" +`include "prim_fifo_assert.sv" /** * OpenTitan Big Number Accelerator (OTBN) @@ -1427,41 +1428,23 @@ module otbn u_otbn_core.u_otbn_rf_bignum.gen_rf_bignum_ff.u_otbn_rf_bignum_inner.u_prim_onehot_check, alert_tx_o[AlertFatal]) - `ASSERT_PRIM_COUNT_ERROR_TRIGGER_ALERT(DmemRspFifoWptrCheck_A, - u_tlul_adapter_sram_dmem.u_rspfifo.gen_normal_fifo.u_fifo_cnt.gen_secure_ptrs.u_wptr, - alert_tx_o[AlertFatal]) - `ASSERT_PRIM_COUNT_ERROR_TRIGGER_ALERT(DmemRspFifoRptrCheck_A, - u_tlul_adapter_sram_dmem.u_rspfifo.gen_normal_fifo.u_fifo_cnt.gen_secure_ptrs.u_rptr, - alert_tx_o[AlertFatal]) - `ASSERT_PRIM_COUNT_ERROR_TRIGGER_ALERT(DmemSramReqFifoWptrCheck_A, - u_tlul_adapter_sram_dmem.u_sramreqfifo.gen_normal_fifo.u_fifo_cnt.gen_secure_ptrs.u_wptr, - alert_tx_o[AlertFatal]) - `ASSERT_PRIM_COUNT_ERROR_TRIGGER_ALERT(DmemSramReqFifoRptrCheck_A, - u_tlul_adapter_sram_dmem.u_sramreqfifo.gen_normal_fifo.u_fifo_cnt.gen_secure_ptrs.u_rptr, - alert_tx_o[AlertFatal]) - `ASSERT_PRIM_COUNT_ERROR_TRIGGER_ALERT(DmemReqFifoWptrCheck_A, - u_tlul_adapter_sram_dmem.u_reqfifo.gen_normal_fifo.u_fifo_cnt.gen_secure_ptrs.u_wptr, - alert_tx_o[AlertFatal]) - `ASSERT_PRIM_COUNT_ERROR_TRIGGER_ALERT(DmemReqFifoRptrCheck_A, - u_tlul_adapter_sram_dmem.u_reqfifo.gen_normal_fifo.u_fifo_cnt.gen_secure_ptrs.u_rptr, - alert_tx_o[AlertFatal]) - - `ASSERT_PRIM_COUNT_ERROR_TRIGGER_ALERT(ImemRspFifoWptrCheck_A, - u_tlul_adapter_sram_imem.u_rspfifo.gen_normal_fifo.u_fifo_cnt.gen_secure_ptrs.u_wptr, - alert_tx_o[AlertFatal]) - `ASSERT_PRIM_COUNT_ERROR_TRIGGER_ALERT(ImemRspFifoRptrCheck_A, - u_tlul_adapter_sram_imem.u_rspfifo.gen_normal_fifo.u_fifo_cnt.gen_secure_ptrs.u_rptr, - alert_tx_o[AlertFatal]) - `ASSERT_PRIM_COUNT_ERROR_TRIGGER_ALERT(ImemSramReqFifoWptrCheck_A, - u_tlul_adapter_sram_imem.u_sramreqfifo.gen_normal_fifo.u_fifo_cnt.gen_secure_ptrs.u_wptr, - alert_tx_o[AlertFatal]) - `ASSERT_PRIM_COUNT_ERROR_TRIGGER_ALERT(ImemSramReqFifoRptrCheck_A, - u_tlul_adapter_sram_imem.u_sramreqfifo.gen_normal_fifo.u_fifo_cnt.gen_secure_ptrs.u_rptr, - alert_tx_o[AlertFatal]) - `ASSERT_PRIM_COUNT_ERROR_TRIGGER_ALERT(ImemReqFifoWptrCheck_A, - u_tlul_adapter_sram_imem.u_reqfifo.gen_normal_fifo.u_fifo_cnt.gen_secure_ptrs.u_wptr, - alert_tx_o[AlertFatal]) - `ASSERT_PRIM_COUNT_ERROR_TRIGGER_ALERT(ImemReqFifoRptrCheck_A, - u_tlul_adapter_sram_imem.u_reqfifo.gen_normal_fifo.u_fifo_cnt.gen_secure_ptrs.u_rptr, - alert_tx_o[AlertFatal]) + `ASSERT_PRIM_FIFO_SYNC_ERROR_TRIGGERS_ALERT1(DmemRspFifo, + u_tlul_adapter_sram_dmem.u_rspfifo, + alert_tx_o[AlertFatal]) + `ASSERT_PRIM_FIFO_SYNC_ERROR_TRIGGERS_ALERT1(DmemSramReqFifo, + u_tlul_adapter_sram_dmem.u_sramreqfifo, + alert_tx_o[AlertFatal]) + `ASSERT_PRIM_FIFO_SYNC_ERROR_TRIGGERS_ALERT1(DmemReqFifo, + u_tlul_adapter_sram_dmem.u_reqfifo, + alert_tx_o[AlertFatal]) + + `ASSERT_PRIM_FIFO_SYNC_ERROR_TRIGGERS_ALERT1(ImemRspFifo, + u_tlul_adapter_sram_imem.u_rspfifo, + alert_tx_o[AlertFatal]) + `ASSERT_PRIM_FIFO_SYNC_ERROR_TRIGGERS_ALERT1(ImemSramReqFifo, + u_tlul_adapter_sram_imem.u_sramreqfifo, + alert_tx_o[AlertFatal]) + `ASSERT_PRIM_FIFO_SYNC_ERROR_TRIGGERS_ALERT1(ImemReqFifo, + u_tlul_adapter_sram_imem.u_reqfifo, + alert_tx_o[AlertFatal]) endmodule diff --git a/hw/ip/prim/prim_fifo.core b/hw/ip/prim/prim_fifo.core index dd0fc7eb142f1b..3b0a6bb66034fe 100644 --- a/hw/ip/prim/prim_fifo.core +++ b/hw/ip/prim/prim_fifo.core @@ -17,6 +17,7 @@ filesets: - rtl/prim_fifo_async.sv - rtl/prim_fifo_sync.sv - rtl/prim_fifo_sync_cnt.sv + - rtl/prim_fifo_assert.sv : {is_include_file : true} file_type: systemVerilogSource files_verilator_waiver: diff --git a/hw/ip/prim/rtl/prim_assert_sec_cm.svh b/hw/ip/prim/rtl/prim_assert_sec_cm.svh index a2fe824bdf9736..4f22e00167d3f2 100644 --- a/hw/ip/prim/rtl/prim_assert_sec_cm.svh +++ b/hw/ip/prim/rtl/prim_assert_sec_cm.svh @@ -61,6 +61,9 @@ `ASSERT_PRIM_ONEHOT_ERROR_TRIGGER_ALERT(NAME_, \ REG_TOP_HIER_.u_prim_reg_we_check.u_prim_onehot_check, ALERT_, GATE_, MAX_CYCLES_) +`define ASSERT_PRIM_FIFO_SYNC_SINGLETON_ERROR_TRIGGER_ALERT(NAME, HIER_, ALERT_, GATE_ = 0, MAX_CYCLES_ = `_SEC_CM_ALERT_MAX_CYC) \ + `ASSERT_ERROR_TRIGGER_ALERT(NAME_, HIER_, ALERT_, GATE_, MAX_CYCLES_, err_o) + //////////////////////////////////////////////////////////////////////////////// // // Assertions for CMs that trigger some other form of error diff --git a/hw/ip/prim/rtl/prim_fifo_assert.sv b/hw/ip/prim/rtl/prim_fifo_assert.sv new file mode 100644 index 00000000000000..a11f12c9f041e5 --- /dev/null +++ b/hw/ip/prim/rtl/prim_fifo_assert.sv @@ -0,0 +1,47 @@ +// Copyright lowRISC contributors (OpenTitan project). +// Licensed under the Apache License, Version 2.0, see LICENSE for details. +// SPDX-License-Identifier: Apache-2.0 +// +// Macros that define assertions that relate to FIFO implementations + +`ifndef PRIM_FIFO_ASSERT_SV +`define PRIM_FIFO_ASSERT_SV + +// Use PRIM_COUNT_ERROR_TRIGGER_ALERT appropriately to check that the three prim_counts generated by +// a prim_fifo_sync with depth at least two do indeed generate an alert if they detect an error. +// +// - NAME_ is used as the root of the names of the generated assertions. +// - HIER_ is a hierarchical path to the prim_fifo_sync in question. +// - ALERT_ is the name of the alert that should be generated. +// - GATE_ is a signal that, if true, will cause an error to be ignored. +// - MAX_CYCLES_ is the number of cycles allowed until the alert must be generated. +`define ASSERT_PRIM_FIFO_SYNC_ERROR_TRIGGERS_ALERT(NAME_, HIER_, ALERT_, GATE_ = 0, MAX_CYCLES_ = `_SEC_CM_ALERT_MAX_CYC) \ + `ASSERT_PRIM_COUNT_ERROR_TRIGGER_ALERT(``NAME_``WptrCheck_A, \ + HIER_.gen_normal_fifo.u_fifo_cnt.gen_secure_ptrs.u_wptr, \ + ALERT_, \ + GATE_, \ + MAX_CYCLES_) \ + `ASSERT_PRIM_COUNT_ERROR_TRIGGER_ALERT(``NAME_``RptrCheck_A, \ + HIER_.gen_normal_fifo.u_fifo_cnt.gen_secure_ptrs.u_rptr, \ + ALERT_, \ + GATE_, \ + MAX_CYCLES_) + +// An analagous assertion to PRIM_COUNT_ERROR_TRIGGER_ALERT, but specialised for the case where the +// fifo has depth 1, which means there aren't actually three prim_count instances but instead there +// is just a single error signal. +// +// - NAME_ is used as a root for the name of the generated assertion. +// - HIER_ is a hierarchical path to the prim_fifo_sync in question. +// - ALERT_ is the name of the alert that should be generated. +// - GATE_ is a signal that, if true, will cause an error to be ignored. +// - MAX_CYCLES_ is the number of cycles allowed until the alert must be generated. +`define ASSERT_PRIM_FIFO_SYNC_ERROR_TRIGGERS_ALERT1(NAME_, HIER_, ALERT_, GATE_ = 0, MAX_CYCLES_ = `_SEC_CM_ALERT_MAX_CYC) \ + `ASSERT_ERROR_TRIGGER_ALERT(``NAME_``FullCheck_A, \ + HIER_, \ + ALERT_, \ + GATE_, \ + MAX_CYCLES_, \ + err_o) + +`endif // PRIM_FIFO_ASSERT_SV diff --git a/hw/ip/prim/rtl/prim_fifo_sync.sv b/hw/ip/prim/rtl/prim_fifo_sync.sv index c4c1ac429fd46a..fe30552e4e0fcf 100644 --- a/hw/ip/prim/rtl/prim_fifo_sync.sv +++ b/hw/ip/prim/rtl/prim_fifo_sync.sv @@ -55,6 +55,81 @@ module prim_fifo_sync #( // No error assign err_o = 1'b 0; + // FIFO has space for a single element (and doesn't need proper counters) + end else if (Depth == 1) begin : gen_singleton_fifo + + // full_q is true if the (singleton) queue has data + logic full_d, full_q; + + assign full_o = full_q; + assign depth_o = full_q; + assign wready_o = ~full_q; + + // We can always read from the storage if it contains something, so rvalid_o is true if full_q + // is true. Enabling pass-through mode also allows data to flow through if wvalid_i is true. + assign rvalid_o = full_q || (Pass && wvalid_i); + + // For there to be data on the next cycle, there must either be new data coming in (so !rvalid_o + // && wvalid_i) or we must be keeping the current data (so rvalid_o && !rready_i). Using + // rvalid_o instead of full_q ensures we get the right behaviour with pass-through. + // + // In either case, any stored data will be forgotten if clr_i is true. + assign full_d = (rvalid_o ? !rready_i : wvalid_i) && !clr_i; + + always_ff @(posedge clk_i or negedge rst_ni) begin + if (!rst_ni) begin + full_q <= 1'b0; + end else begin + full_q <= full_d; + end + end + + logic [Width-1:0] storage; + always_ff @(posedge clk_i) begin + if (wvalid_i && wready_o) storage <= wdata_i; + end + + logic [Width-1:0] rdata_int; + assign rdata_int = (full_q || Pass == 1'b0) ? storage : wdata_i; + + assign rdata_o = (OutputZeroIfEmpty && !rvalid_o) ? Width'(0) : rdata_int; + + // The larger FIFO implementation uses prim_count for read and write pointers. If Secure is + // true, prim_count duplicates and checks these pointers to guard against fault injection. We do + // something similar here, duplicating and checking a "1 bit counter". + // + // The duplication is inverted, which means we expect full_q ^ inv_full to be true and generate + // an error signal if it is not. This error signal gets registered to avoid potential CDC issues + // downstream. + if (!Secure) begin : gen_not_secure + assign err_o = 1'b0; + end else begin : gen_secure + logic inv_full; + + prim_flop #( + .Width(1), + .ResetValue(1'b1) + ) u_inv_full ( + .clk_i, + .rst_ni, + .d_i (^full_d), + .q_o (inv_full) + ); + + logic err_d, err_q; + assign err_d = ~(full_q ^ inv_full); + + always_ff @(posedge clk_i or negedge rst_ni) begin + if (!rst_ni) begin + err_q <= 1'b0; + end else begin + err_q <= err_d; + end + end + + assign err_o = err_q; + end + // Normal FIFO construction end else begin : gen_normal_fifo @@ -100,30 +175,15 @@ module prim_fifo_sync #( assign fifo_incr_wptr = wvalid_i & wready_o & ~under_rst; assign fifo_incr_rptr = rvalid_o & rready_i & ~under_rst; - // the generate blocks below are needed to avoid lint errors due to array indexing - // in the where the fifo only has one storage element logic [Depth-1:0][Width-1:0] storage; logic [Width-1:0] storage_rdata; - if (Depth == 1) begin : gen_depth_eq1 - assign storage_rdata = storage[0]; - always_ff @(posedge clk_i) - if (fifo_incr_wptr) begin - storage[0] <= wdata_i; - end - - logic unused_ptrs; - assign unused_ptrs = ^{fifo_wptr, fifo_rptr}; - - // fifo with more than one storage element - end else begin : gen_depth_gt1 - assign storage_rdata = storage[fifo_rptr]; + assign storage_rdata = storage[fifo_rptr]; - always_ff @(posedge clk_i) - if (fifo_incr_wptr) begin - storage[fifo_wptr] <= wdata_i; - end - end + always_ff @(posedge clk_i) + if (fifo_incr_wptr) begin + storage[fifo_wptr] <= wdata_i; + end logic [Width-1:0] rdata_int; if (Pass == 1'b1) begin : gen_pass @@ -153,4 +213,17 @@ module prim_fifo_sync #( `ASSERT_KNOWN(RvalidKnown_A, rvalid_o) `ASSERT_KNOWN(WreadyKnown_A, wready_o) +`ifdef INC_ASSERT + // When Depth=1 and Secure=1, there is a specialized countermeasure that works by replicating + // the full_q flag. To set the logic value below to one, the user must use the + // ASSERT_PRIM_FIFO_SYNC_SINGLETON_ERROR_TRIGGER_ALERT macro (which checks that the error signal + // causes an alert). + // + // If the user hasn't done so, unused_assert_connected will be zero and ASSERT_INIT_NET will + // fail. + logic unused_assert_connected; + if (Depth == 1 && Secure) begin + `ASSERT_INIT_NET(AssertConnected_A, unused_assert_connected === 1'b1) + end +`endif endmodule diff --git a/hw/ip_templates/flash_ctrl/dv/env/seq_lib/flash_ctrl_common_vseq.sv b/hw/ip_templates/flash_ctrl/dv/env/seq_lib/flash_ctrl_common_vseq.sv index 20a560a9960a42..562ed195d45133 100644 --- a/hw/ip_templates/flash_ctrl/dv/env/seq_lib/flash_ctrl_common_vseq.sv +++ b/hw/ip_templates/flash_ctrl/dv/env/seq_lib/flash_ctrl_common_vseq.sv @@ -51,8 +51,6 @@ class flash_ctrl_common_vseq extends flash_ctrl_otf_base_vseq; disable_fi_for_prim_count("*u_tl_adapter_eflash*u_rspfifo*u_fifo_cnt"); disable_fi_for_prim_count("*u_tl_adapter_eflash*u_reqfifo*u_fifo_cnt"); disable_fi_for_prim_count("*u_tl_adapter_eflash*u_sramreqfifo*u_fifo_cnt"); - disable_fi_for_prim_count("*u_to_rd_fifo*u_reqfifo*u_fifo_cnt"); - disable_fi_for_prim_count("*u_to_rd_fifo*u_sramreqfifo*u_fifo_cnt"); super.dut_init(reset_kind); endtask // dut_init diff --git a/hw/ip_templates/flash_ctrl/rtl/flash_ctrl.sv.tpl b/hw/ip_templates/flash_ctrl/rtl/flash_ctrl.sv.tpl index f459b46ac6a9c6..b59ebb3b6f7e92 100644 --- a/hw/ip_templates/flash_ctrl/rtl/flash_ctrl.sv.tpl +++ b/hw/ip_templates/flash_ctrl/rtl/flash_ctrl.sv.tpl @@ -7,6 +7,7 @@ // `include "prim_assert.sv" +`include "prim_fifo_assert.sv" module flash_ctrl import flash_ctrl_pkg::*; import flash_ctrl_reg_pkg::*; @@ -1440,27 +1441,17 @@ module flash_ctrl `define PHY u_eflash.gen_flash_cores[i] `define PHY_CORE `PHY.u_core for (genvar i=0; i