diff --git a/dv/uvm/icache/dv/env/seq_lib/ibex_icache_base_vseq.sv b/dv/uvm/icache/dv/env/seq_lib/ibex_icache_base_vseq.sv index 6398e9e1..a2552a67 100644 --- a/dv/uvm/icache/dv/env/seq_lib/ibex_icache_base_vseq.sv +++ b/dv/uvm/icache/dv/env/seq_lib/ibex_icache_base_vseq.sv @@ -39,8 +39,6 @@ class ibex_icache_base_vseq constraint num_trans_c { num_trans inside {[800:1000]}; } virtual task pre_start(); - super.pre_start(); - `uvm_create_on(core_seq, p_sequencer.core_sequencer_h) `uvm_create_on(mem_seq, p_sequencer.mem_sequencer_h) @@ -74,6 +72,8 @@ class ibex_icache_base_vseq foreach (ecc_data_seqs[i]) begin `uvm_create_on(ecc_data_seqs[i], p_sequencer.ecc_data_sequencers[i]) end + + super.pre_start(); endtask : pre_start virtual task body(); @@ -118,4 +118,9 @@ class ibex_icache_base_vseq end join_none endtask + // Clear any interface signals that must be cleared before resetting the DUT. + virtual task reset_ifs(); + core_seq.reset_ifs(); + endtask + endclass : ibex_icache_base_vseq diff --git a/dv/uvm/icache/dv/env/seq_lib/ibex_icache_combo_vseq.sv b/dv/uvm/icache/dv/env/seq_lib/ibex_icache_combo_vseq.sv index b076f966..0d73e64b 100644 --- a/dv/uvm/icache/dv/env/seq_lib/ibex_icache_combo_vseq.sv +++ b/dv/uvm/icache/dv/env/seq_lib/ibex_icache_combo_vseq.sv @@ -57,14 +57,22 @@ class ibex_icache_combo_vseq // little. trans_now = $urandom_range(50, 100); - // We don't need to reset if seq_idx == 0 (because we did a reset before starting this task). - // Otherwise, we always reset between sequences if random_reset is true. We'd always need to - // if we killed the previous sequence, and it's easier not to bother tracking properly. If - // random_reset is false (the usual back-to-back sequence test), we reset 1 time in 2. - if (seq_idx == 0) should_reset = 1'b0; + // We don't need to reset if trans_so_far == 0 (because we did a reset before starting this + // task). Otherwise, we always reset between sequences if random_reset is true. We'd always + // need to if we killed the previous sequence, and it's easier not to bother tracking + // properly. If random_reset is false (the usual back-to-back sequence test), we reset 1 time + // in 2. + if (trans_so_far == 0) should_reset = 1'b0; else if (random_reset) should_reset = 1'b1; else should_reset = $urandom_range(0, 1); + // If we are resetting and trans_so_far > 0, we still have a (stopped) previous child + // sequence. Tell it to reset its interfaces now, clearing signals, to make sure that no + // sensitive input signal is high when the DUT goes into reset. + if (trans_so_far > 0 && should_reset) begin + child_seq.reset_ifs(); + end + `uvm_info(`gfn, $sformatf("Running sequence '%s' (%0d transactions; reset=%0d).", seq_names[seq_idx], trans_now, should_reset), diff --git a/dv/uvm/icache/dv/ibex_icache_core_agent/ibex_icache_core_driver.sv b/dv/uvm/icache/dv/ibex_icache_core_agent/ibex_icache_core_driver.sv index 8f708973..05235b0b 100644 --- a/dv/uvm/icache/dv/ibex_icache_core_agent/ibex_icache_core_driver.sv +++ b/dv/uvm/icache/dv/ibex_icache_core_agent/ibex_icache_core_driver.sv @@ -178,11 +178,11 @@ class ibex_icache_core_driver cfg.vif.driver_cb.ready <= 1'b0; endtask - // Lower the req line for the given number of cycles. Returns early on reset + // Lower the req line for the given number of cycles. Returns early on reset, leaving req low. virtual task automatic lower_req(int unsigned num_cycles); cfg.vif.driver_cb.req <= 1'b0; cfg.vif.wait_clks(num_cycles); - cfg.vif.driver_cb.req <= 1'b1; + if (cfg.vif.rst_n) cfg.vif.driver_cb.req <= 1'b1; endtask // Raise the invalidate line for a randomly chosen number of cycles > 0. diff --git a/dv/uvm/icache/dv/ibex_icache_core_agent/ibex_icache_core_protocol_checker.sv b/dv/uvm/icache/dv/ibex_icache_core_agent/ibex_icache_core_protocol_checker.sv index e7e0df55..f87a3244 100644 --- a/dv/uvm/icache/dv/ibex_icache_core_agent/ibex_icache_core_protocol_checker.sv +++ b/dv/uvm/icache/dv/ibex_icache_core_agent/ibex_icache_core_protocol_checker.sv @@ -40,6 +40,10 @@ interface ibex_icache_core_protocol_checker ( // don't care about it because the core should never do it. `ASSERT(NoReadyWithoutReq, !req |-> !ready, clk, !rst_n) + // The core may not assert 'req' when the cache is in reset (instr_req_o isn't conditioned on + // rst_n, so doing so would cause the cache to make spurious requests on the instruction bus). + `ASSERT(NoReqInReset, req |-> rst_n, clk, 1'b0) + // The 'branch' and 'branch_addr' ports // // The branch signal tells the cache to redirect. There's no real requirement on when it can be diff --git a/dv/uvm/icache/dv/ibex_icache_core_agent/seq_lib/ibex_icache_core_base_seq.sv b/dv/uvm/icache/dv/ibex_icache_core_agent/seq_lib/ibex_icache_core_base_seq.sv index a0c7ade5..c1a1687b 100644 --- a/dv/uvm/icache/dv/ibex_icache_core_agent/seq_lib/ibex_icache_core_base_seq.sv +++ b/dv/uvm/icache/dv/ibex_icache_core_agent/seq_lib/ibex_icache_core_base_seq.sv @@ -195,4 +195,12 @@ class ibex_icache_core_base_seq extends dv_base_seq #( end endtask + // Called from the environment when resetting the DUT. Should ensure that the req_i signal is + // dropped (to avoid any memory requests making it out of the DUT when it should be in reset) + // + // This must not be called while we're in body (otherwise we'd have multiple drivers for req_i). + task reset_ifs(); + cfg.vif.driver_cb.req <= 1'b0; + endtask + endclass