[dv/clkmgr] Fix clkmgr resets There have been changes in how the clkmgr handles reset in the RTL, and the dv components need to be adjusted to match. Move the call to initialize_on_start to post_apply_reset since it consumes time, and causes randomized sequences with random reset to fail. Block some CSR checks when the dut is under reset. Undo some side effects of frequency tests in apply_resets_concurrently. Use csr_rd_check in more places, since it deals with reset better. Signed-off-by: Guillermo Maturana <maturana@google.com>
diff --git a/hw/dv/sv/cip_lib/seq_lib/cip_base_vseq.sv b/hw/dv/sv/cip_lib/seq_lib/cip_base_vseq.sv index 106a8c7..1da4ab3 100644 --- a/hw/dv/sv/cip_lib/seq_lib/cip_base_vseq.sv +++ b/hw/dv/sv/cip_lib/seq_lib/cip_base_vseq.sv
@@ -17,11 +17,12 @@ end : isolation_fork \ join -class cip_base_vseq #(type RAL_T = dv_base_reg_block, - type CFG_T = cip_base_env_cfg, - type COV_T = cip_base_env_cov, - type VIRTUAL_SEQUENCER_T = cip_base_virtual_sequencer) - extends dv_base_vseq #(RAL_T, CFG_T, COV_T, VIRTUAL_SEQUENCER_T); +class cip_base_vseq #( + type RAL_T = dv_base_reg_block, + type CFG_T = cip_base_env_cfg, + type COV_T = cip_base_env_cov, + type VIRTUAL_SEQUENCER_T = cip_base_virtual_sequencer +) extends dv_base_vseq #(RAL_T, CFG_T, COV_T, VIRTUAL_SEQUENCER_T); `uvm_object_new // knobs to disable post_start clear interrupt routine bit do_clear_all_interrupts = 1'b1; @@ -44,7 +45,7 @@ // address mask struct typedef struct packed { - bit [BUS_AW-1:0] addr; + bit [BUS_AW-1:0] addr; bit [BUS_DBW-1:0] mask; } addr_mask_t; @@ -64,10 +65,10 @@ rand uint rand_reset_delay; constraint rand_reset_delay_c { rand_reset_delay dist { - [1 :1000] :/ 1, - [1001 :100_000] :/ 2, - [100_001 :1_000_000] :/ 6, - [1_000_001 :5_000_000] :/ 1 + [1 : 1000] :/ 1, + [1001 : 100_000] :/ 2, + [100_001 : 1_000_000] :/ 6, + [1_000_001 : 5_000_000] :/ 1 }; } @@ -76,13 +77,13 @@ rand uint csr_access_abort_pct; constraint csr_access_abort_pct_c { csr_access_abort_pct dist { - 0 :/ 50, - [1:99] :/ 40, - 100 :/ 10 + 0 :/ 50, + [1 : 99] :/ 40, + 100 :/ 10 }; } - `uvm_object_param_utils_begin(cip_base_vseq #(RAL_T, CFG_T, COV_T, VIRTUAL_SEQUENCER_T)) + `uvm_object_param_utils_begin(cip_base_vseq#(RAL_T, CFG_T, COV_T, VIRTUAL_SEQUENCER_T)) `uvm_field_string(common_seq_type, UVM_DEFAULT) `uvm_object_utils_end @@ -96,7 +97,7 @@ // Wait for alert init done, then start the sequence. foreach (cfg.list_of_alerts[i]) begin if (cfg.m_alert_agent_cfg[cfg.list_of_alerts[i]].is_active) begin - wait (cfg.m_alert_agent_cfg[cfg.list_of_alerts[i]].alert_init_done == 1); + wait(cfg.m_alert_agent_cfg[cfg.list_of_alerts[i]].alert_init_done == 1); end end @@ -316,7 +317,7 @@ csr_name = {csr_name, suffix}; end // check within scope first, if supplied - if (scope != null) begin + if (scope != null) begin get_interrupt_csr = scope.get_dv_base_reg_by_name(csr_name); end else begin get_interrupt_csr = ral.get_dv_base_reg_by_name(csr_name); @@ -665,6 +666,7 @@ // override this task from {block}_common_vseq if needed virtual task rand_reset_eor_clean_up(); endtask + // Run the given sequence and possibly a TL errors vseq (if do_tl_err is set). Suddenly inject a // reset after at most reset_delay_bound cycles. When we come out of reset, check all CSR values // to ensure they are the documented reset values.
diff --git a/hw/ip/clkmgr/dv/env/clkmgr_env.sv b/hw/ip/clkmgr/dv/env/clkmgr_env.sv index 2bb75fd..8f4aa66 100644 --- a/hw/ip/clkmgr/dv/env/clkmgr_env.sv +++ b/hw/ip/clkmgr/dv/env/clkmgr_env.sv
@@ -35,6 +35,21 @@ )) begin `uvm_fatal(`gfn, "failed to get aon_clk_rst_vif from uvm_config_db") end + if (!uvm_config_db#(virtual clk_rst_if)::get( + this, "", "root_io_clk_rst_vif", cfg.root_io_clk_rst_vif + )) begin + `uvm_fatal(`gfn, "failed to get root_io_clk_rst_vif from uvm_config_db") + end + if (!uvm_config_db#(virtual clk_rst_if)::get( + this, "", "root_main_clk_rst_vif", cfg.root_main_clk_rst_vif + )) begin + `uvm_fatal(`gfn, "failed to get root_main_clk_rst_vif from uvm_config_db") + end + if (!uvm_config_db#(virtual clk_rst_if)::get( + this, "", "root_usb_clk_rst_vif", cfg.root_usb_clk_rst_vif + )) begin + `uvm_fatal(`gfn, "failed to get root_usb_clk_rst_vif from uvm_config_db") + end if (!uvm_config_db#(virtual clkmgr_if)::get(this, "", "clkmgr_vif", cfg.clkmgr_vif)) begin `uvm_fatal(`gfn, "failed to get clkmgr_vif from uvm_config_db") end
diff --git a/hw/ip/clkmgr/dv/env/clkmgr_env_cfg.sv b/hw/ip/clkmgr/dv/env/clkmgr_env_cfg.sv index 5102d00..f302c1a 100644 --- a/hw/ip/clkmgr/dv/env/clkmgr_env_cfg.sv +++ b/hw/ip/clkmgr/dv/env/clkmgr_env_cfg.sv
@@ -7,17 +7,21 @@ ); // This scoreboard handle is used to flag expected errors. - clkmgr_scoreboard scoreboard; + clkmgr_scoreboard scoreboard; // ext component cfgs // ext interfaces - clkmgr_vif clkmgr_vif; + virtual clkmgr_if clkmgr_vif; virtual clk_rst_if main_clk_rst_vif; virtual clk_rst_if io_clk_rst_vif; virtual clk_rst_if usb_clk_rst_vif; virtual clk_rst_if aon_clk_rst_vif; + virtual clk_rst_if root_io_clk_rst_vif; + virtual clk_rst_if root_main_clk_rst_vif; + virtual clk_rst_if root_usb_clk_rst_vif; + `uvm_object_utils_begin(clkmgr_env_cfg) `uvm_object_utils_end
diff --git a/hw/ip/clkmgr/dv/env/seq_lib/clkmgr_base_vseq.sv b/hw/ip/clkmgr/dv/env/seq_lib/clkmgr_base_vseq.sv index 113806f..51b1a46 100644 --- a/hw/ip/clkmgr/dv/env/seq_lib/clkmgr_base_vseq.sv +++ b/hw/ip/clkmgr/dv/env/seq_lib/clkmgr_base_vseq.sv
@@ -392,62 +392,63 @@ }; return max(clk_periods_q); endfunction - + // This is tricky, and we choose to handle it all here, not in "super": // - there are no multiple clk_rst_vifs, // - it would be too complicated to coordinate reset durations with super. - // For hard resets we also reset cfg.por_io_clk_rst_vif, and its reset is shorter than + // For hard resets we also reset the cfg.root*_clk_rst_vif, and its reset is shorter than // that of all others. - virtual task apply_reset(string kind = "HARD"); - if (kind == "HARD") apply_reset_concurrently(); - else fork -// int max_period_ps = maximum_clock_period(); -// fork -// super.apply_reset(kind); -// if (kind == "HARD") cfg.por_io_clk_rst_vif.drive_rst_pin(0); - -// fork - cfg.aon_clk_rst_vif.apply_reset(); - cfg.main_clk_rst_vif.apply_reset(); - cfg.io_clk_rst_vif.apply_reset(); - cfg.usb_clk_rst_vif.apply_reset(); - join -// join - initialize_on_start(); - endtask - virtual task apply_resets_concurrently(int reset_duration_ps = 0); int clk_periods_q[$] = { reset_duration_ps, cfg.aon_clk_rst_vif.clk_period_ps, - cfg.io_clk_rst_vif.clk_period_ps, + cfg.io_clk_rst_vif.clk_period_ps * 4, cfg.main_clk_rst_vif.clk_period_ps, cfg.usb_clk_rst_vif.clk_period_ps }; - // Extra factor of 8 since we are using rst_io_n for io_div2 and io_div4 resets. - reset_duration_ps = max(clk_periods_q) * 2; + reset_duration_ps = max(clk_periods_q); - cfg.por_io_clk_rst_vif.drive_rst_pin(0); + `uvm_info(`gfn, "In apply_resets_concurrently", UVM_MEDIUM) + cfg.root_io_clk_rst_vif.drive_rst_pin(0); + cfg.root_main_clk_rst_vif.drive_rst_pin(0); + cfg.root_usb_clk_rst_vif.drive_rst_pin(0); cfg.aon_clk_rst_vif.drive_rst_pin(0); + cfg.clk_rst_vif.drive_rst_pin(0); cfg.io_clk_rst_vif.drive_rst_pin(0); cfg.main_clk_rst_vif.drive_rst_pin(0); cfg.usb_clk_rst_vif.drive_rst_pin(0); #(reset_duration_ps * $urandom_range(2, 10) * 1ps); - cfg.por_io_clk_rst_vif.drive_rst_pin(1); - -// super.apply_resets_concurrently(reset_duration_ps); + cfg.root_io_clk_rst_vif.drive_rst_pin(1); + cfg.root_main_clk_rst_vif.drive_rst_pin(1); + cfg.root_usb_clk_rst_vif.drive_rst_pin(1); + `uvm_info(`gfn, "apply_resets_concurrently releases POR", UVM_MEDIUM) #(reset_duration_ps * $urandom_range(2, 10) * 1ps); cfg.aon_clk_rst_vif.drive_rst_pin(1); + cfg.clk_rst_vif.drive_rst_pin(1); cfg.io_clk_rst_vif.drive_rst_pin(1); cfg.main_clk_rst_vif.drive_rst_pin(1); cfg.usb_clk_rst_vif.drive_rst_pin(1); - initialize_on_start(); + `uvm_info(`gfn, "apply_resets_concurrently releases other resets", UVM_MEDIUM) + endtask + + virtual task apply_reset(string kind = "HARD"); + if (kind == "HARD") apply_resets_concurrently(); + else begin + fork + cfg.clk_rst_vif.apply_reset(); + cfg.aon_clk_rst_vif.apply_reset(); + cfg.main_clk_rst_vif.apply_reset(); + cfg.io_clk_rst_vif.apply_reset(); + cfg.usb_clk_rst_vif.apply_reset(); + join + end endtask task post_apply_reset(string reset_kind = "HARD"); super.post_apply_reset(reset_kind); + initialize_on_start(); cfg.io_clk_rst_vif.wait_clks(POST_APPLY_RESET_CYCLES); endtask
diff --git a/hw/ip/clkmgr/dv/env/seq_lib/clkmgr_common_vseq.sv b/hw/ip/clkmgr/dv/env/seq_lib/clkmgr_common_vseq.sv index c6fe4c6..d5b45f8 100644 --- a/hw/ip/clkmgr/dv/env/seq_lib/clkmgr_common_vseq.sv +++ b/hw/ip/clkmgr/dv/env/seq_lib/clkmgr_common_vseq.sv
@@ -80,8 +80,7 @@ // Override shadow_reg_errors task // to cover shadow regs under clock div2, div4 - task shadow_reg_errors_check_fatal_alert_nonblocking(dv_base_reg shadowed_csr, - string alert_name); + task shadow_reg_errors_check_fatal_alert_nonblocking(dv_base_reg shadowed_csr, string alert_name); skid_check_fatal_alert_nonblocking(alert_name); endtask endclass
diff --git a/hw/ip/clkmgr/dv/env/seq_lib/clkmgr_frequency_timeout_vseq.sv b/hw/ip/clkmgr/dv/env/seq_lib/clkmgr_frequency_timeout_vseq.sv index e4c04f7..5db9baf 100644 --- a/hw/ip/clkmgr/dv/env/seq_lib/clkmgr_frequency_timeout_vseq.sv +++ b/hw/ip/clkmgr/dv/env/seq_lib/clkmgr_frequency_timeout_vseq.sv
@@ -31,11 +31,20 @@ usb_ip_clk_en == 1; } + // The clock that will be disabled. + clk_mesr_e clk_mesr_timeout; + // This waits a number of AON cycles so that the timeout can get detected. task wait_before_read_recov_err_code(); cfg.aon_clk_rst_vif.wait_clks(CyclesToGetOneMeasurement); endtask + // Get things back in normal order. + virtual task apply_resets_concurrently(int reset_duration_ps = 0); + super.apply_resets_concurrently(reset_duration_ps); + if (cause_timeout) disturb_measured_clock(.clk(clk_mesr_timeout), .enable(1'b1)); + endtask + task body(); logic [TL_DW-1:0] value; int prior_alert_count; @@ -57,7 +66,6 @@ clkmgr_recov_err_t actual_recov_err = '{default: '0}; logic [ClkMesrUsb:0] expected_recov_timeout_err = '0; bit expect_alert = 0; - clk_mesr_e clk_mesr_timeout; `DV_CHECK_RANDOMIZE_FATAL(this) `uvm_info(`gfn, "New round", UVM_MEDIUM) @@ -100,7 +108,7 @@ if (actual_recov_err.measures) begin report_recov_error_mismatch("measurement", recov_bits_t'(0), actual_recov_err.measures); end - if (actual_recov_err.timeouts != expected_recov_timeout_err) begin + if (!cfg.under_reset && actual_recov_err.timeouts != expected_recov_timeout_err) begin report_recov_error_mismatch("timeout", expected_recov_timeout_err, actual_recov_err.timeouts); end @@ -110,7 +118,9 @@ // And check that the alert count increased if there was a timeout. current_alert_count = cfg.scoreboard.get_alert_count("recov_fault"); if (cause_timeout) begin - `DV_CHECK_NE(current_alert_count, prior_alert_count, "expected some alerts to fire") + if (!cfg.under_reset) begin + `DV_CHECK_NE(current_alert_count, prior_alert_count, "expected some alerts to fire") + end end else begin `DV_CHECK_EQ(current_alert_count, prior_alert_count, "expected no alerts to fire") end
diff --git a/hw/ip/clkmgr/dv/env/seq_lib/clkmgr_frequency_vseq.sv b/hw/ip/clkmgr/dv/env/seq_lib/clkmgr_frequency_vseq.sv index a391011..5777d0e 100644 --- a/hw/ip/clkmgr/dv/env/seq_lib/clkmgr_frequency_vseq.sv +++ b/hw/ip/clkmgr/dv/env/seq_lib/clkmgr_frequency_vseq.sv
@@ -166,7 +166,7 @@ csr_rd(.ptr(ral.recov_err_code), .value(actual_recov_err)); `uvm_info(`gfn, $sformatf("Expected recov err register=0x%x", expected_recov_meas_err), UVM_MEDIUM) - if (actual_recov_err.measures != expected_recov_meas_err) begin + if (!cfg.under_reset && actual_recov_err.measures != expected_recov_meas_err) begin report_recov_error_mismatch("measurement", expected_recov_meas_err, actual_recov_err.measures); end @@ -180,7 +180,9 @@ // Check alerts. current_alert_count = cfg.scoreboard.get_alert_count("recov_fault"); if (expect_alert) begin - `DV_CHECK_NE(current_alert_count, prior_alert_count, "expected some alerts to fire") + if (!cfg.under_reset) begin + `DV_CHECK_NE(current_alert_count, prior_alert_count, "expected some alerts to fire") + end end else begin `DV_CHECK_EQ(current_alert_count, prior_alert_count, "expected no alerts to fire") end
diff --git a/hw/ip/clkmgr/dv/env/seq_lib/clkmgr_trans_vseq.sv b/hw/ip/clkmgr/dv/env/seq_lib/clkmgr_trans_vseq.sv index 1be536d..7a31d09 100644 --- a/hw/ip/clkmgr/dv/env/seq_lib/clkmgr_trans_vseq.sv +++ b/hw/ip/clkmgr/dv/env/seq_lib/clkmgr_trans_vseq.sv
@@ -52,26 +52,28 @@ // Extra wait because of synchronizers plus counters. cfg.clk_rst_vif.wait_clks(IDLE_SYNC_CYCLES); // We expect the status to be determined by hints and idle, ignoring scanmode. - csr_rd(.ptr(ral.clk_hints_status), .value(value)); - bool_idle = mubi_hintables_to_hintables(idle); - `DV_CHECK_EQ(value, initial_hints | ~bool_idle, $sformatf( - "Busy units have status high: hints=0x%x, idle=0x%x", initial_hints, bool_idle)) + value = initial_hints | ~bool_idle; + csr_rd_check(.ptr(ral.clk_hints_status), .compare_value(value), + .err_msg($sformatf("Busy units have status high: hints=0x%x, idle=0x%x", + initial_hints, bool_idle))); // Setting all idle should make hint_status match hints. `uvm_info(`gfn, "Setting all units idle", UVM_MEDIUM) cfg.clkmgr_vif.update_idle({NUM_TRANS{MuBi4True}}); cfg.clk_rst_vif.wait_clks(IDLE_SYNC_CYCLES); - csr_rd(.ptr(ral.clk_hints_status), .value(value)); - `DV_CHECK_EQ(value, initial_hints, "All idle: expect status matches hints") + csr_rd_check(.ptr(ral.clk_hints_status), .compare_value(initial_hints), + .err_msg("All idle: expect status matches hints")); // Now set all hints, and the status should also be all ones. - csr_wr(.ptr(ral.clk_hints), .value('1)); + value = '1; + csr_wr(.ptr(ral.clk_hints), .value(value)); cfg.clk_rst_vif.wait_clks(IDLE_SYNC_CYCLES); - csr_rd(.ptr(ral.clk_hints_status), .value(value)); // We expect all units to be on. - `DV_CHECK_EQ(value, '1, "All idle and all hints high: units status should be high") + csr_rd_check(.ptr(ral.clk_hints_status), .compare_value(value), + .err_msg("All idle and all hints high: units status should be high")); + // Set hints to the reset value for stress tests. csr_wr(.ptr(ral.clk_hints), .value(ral.clk_hints.get_reset())); end
diff --git a/hw/ip/clkmgr/dv/tb.sv b/hw/ip/clkmgr/dv/tb.sv index 096503e..28509b3 100644 --- a/hw/ip/clkmgr/dv/tb.sv +++ b/hw/ip/clkmgr/dv/tb.sv
@@ -40,9 +40,17 @@ .clk (clk_aon), .rst_n(rst_aon_n) ); - clk_rst_if por_io_clk_rst_if ( + clk_rst_if root_io_clk_rst_if ( .clk (), - .rst_n(rst_por_io_n) + .rst_n(rst_root_io_n) + ); + clk_rst_if root_main_clk_rst_if ( + .clk (), + .rst_n(rst_root_main_n) + ); + clk_rst_if root_usb_clk_rst_if ( + .clk (), + .rst_n(rst_root_usb_n) ); @@ -61,11 +69,14 @@ .clk_main(clk_main), .rst_io_n(rst_io_n), .rst_main_n(rst_main_n), - .rst_por_io_n(rst_por_io_n), .rst_usb_n(rst_usb_n) ); - rst_shadowed_if rst_shadowed_if(.rst_n(rst_n), .rst_shadowed_n(rst_shadowed_n)); + rst_shadowed_if rst_shadowed_if ( + .rst_n(rst_n), + .rst_shadowed_n(rst_shadowed_n) + ); + initial begin // Clocks must be set to active at time 0. The rest of the clock configuration happens // in clkmgr_base_vseq.sv. @@ -74,6 +85,10 @@ io_clk_rst_if.set_active(); usb_clk_rst_if.set_active(); aon_clk_rst_if.set_active(); + + root_io_clk_rst_if.set_active(); + root_main_clk_rst_if.set_active(); + root_usb_clk_rst_if.set_active(); end `DV_ALERT_IF_CONNECT @@ -95,21 +110,18 @@ .rst_usb_ni (rst_usb_n), .clk_aon_i (clk_aon), .rst_aon_ni (rst_aon_n), - .rst_io_div2_ni(rst_io_n), - .rst_io_div4_ni(rst_io_n), // TODO: This is not the right reset to use here. // the "por" reset should de-assert earlier than the // the other resets. There should also be scenarios // where the other resets assert, but "por" does not, // since por resets are upstream of lc resets. - .rst_root_ni(rst_io_n), - .rst_root_io_ni(rst_io_n), - .rst_root_io_div2_ni(rst_io_n), - .rst_root_io_div4_ni(rst_io_n), - .rst_root_main_ni(rst_main_n), - .rst_root_usb_ni(rst_usb_n), - .rst_por_io_ni(rst_por_io_n), + .rst_root_ni(rst_root_io_n), + .rst_root_io_ni(rst_root_io_n), + .rst_root_io_div2_ni(rst_root_io_n), + .rst_root_io_div4_ni(rst_root_io_n), + .rst_root_main_ni(rst_root_main_n), + .rst_root_usb_ni(rst_root_usb_n), .tl_i(tl_if.h2d), .tl_o(tl_if.d2h), @@ -152,6 +164,13 @@ uvm_config_db#(virtual clk_rst_if)::set(null, "*.env", "usb_clk_rst_vif", usb_clk_rst_if); uvm_config_db#(virtual clk_rst_if)::set(null, "*.env", "aon_clk_rst_vif", aon_clk_rst_if); + uvm_config_db#(virtual clk_rst_if)::set(null, "*.env", "root_io_clk_rst_vif", + root_io_clk_rst_if); + uvm_config_db#(virtual clk_rst_if)::set(null, "*.env", "root_main_clk_rst_vif", + root_main_clk_rst_if); + uvm_config_db#(virtual clk_rst_if)::set(null, "*.env", "root_usb_clk_rst_vif", + root_usb_clk_rst_if); + uvm_config_db#(virtual clkmgr_if)::set(null, "*.env", "clkmgr_vif", clkmgr_if); // FIXME Un-comment this once interrupts are created for this ip.