[dv] Remove phase argument from monitor's collect_trans

This argument isn't actually used anywhere it's passed. Let's clean it up. This patch is *almost* trivial, except that it touches some vendored code from Ibex. Apply a patch to the vendored code to make it match the function signature that we expect. Signed-off-by: Rupert Swarbrick <[email protected]>
lowRISC · Mar 25, 2024 · 18b0ced · 18b0ced
1 parent 5ff5034
commit 18b0ced
Show file tree

Hide file tree

Showing 22 changed files with 82 additions and 26 deletions.
diff --git a/hw/dv/sv/dv_lib/dv_base_monitor.sv b/hw/dv/sv/dv_lib/dv_base_monitor.sv
@@ -38,12 +38,12 @@ class dv_base_monitor #(type ITEM_T = uvm_sequence_item,
 
   virtual task run_phase(uvm_phase phase);
     fork
-      collect_trans(phase);
+      collect_trans();
     join
   endtask
 
   // collect transactions forever
-  virtual protected task collect_trans(uvm_phase phase);
+  virtual protected task collect_trans();
     `uvm_fatal(`gfn, "this method is not supposed to be called directly!")
   endtask
 

diff --git a/hw/dv/sv/flash_phy_prim_agent/flash_phy_prim_monitor.sv b/hw/dv/sv/flash_phy_prim_agent/flash_phy_prim_monitor.sv
@@ -76,7 +76,7 @@ class flash_phy_prim_monitor extends dv_base_monitor #(
   endtask
 
   // collect transactions forever - already forked in dv_base_monitor::run_phase
-  virtual protected task collect_trans(uvm_phase phase);
+  virtual protected task collect_trans();
     `DV_SPINWAIT(wait(cfg.vif.rst_n == 1);,
                  "timeout waiting for reset deassert", 100_000)
     `uvm_info(`gfn, $sformatf("flash_phy_prim_monitor %s", (cfg.scb_otf_en)? "enabled" :

diff --git a/hw/dv/sv/jtag_agent/jtag_monitor.sv b/hw/dv/sv/jtag_agent/jtag_monitor.sv
@@ -25,7 +25,7 @@ class jtag_monitor extends dv_base_monitor #(
   endtask
 
   // collect transactions forever - already forked in dv_base_monitor::run_phase
-  virtual protected task collect_trans(uvm_phase phase);
+  virtual protected task collect_trans();
     jtag_fsm_state_e   jtag_state;
     jtag_item          item;
     int                counter;

diff --git a/hw/dv/sv/jtag_dmi_agent/jtag_dmi_monitor.sv b/hw/dv/sv/jtag_dmi_agent/jtag_dmi_monitor.sv
@@ -36,7 +36,7 @@ class jtag_dmi_monitor #(type ITEM_T = jtag_dmi_item) extends dv_base_monitor#(
     join
   endtask
 
-  virtual protected task collect_trans(uvm_phase phase);
+  virtual protected task collect_trans();
     jtag_item jtag_item;
     bit dmi_selected;
 

diff --git a/hw/dv/sv/jtag_dmi_agent/sba_access_monitor.sv b/hw/dv/sv/jtag_dmi_agent/sba_access_monitor.sv
@@ -74,7 +74,7 @@ class sba_access_monitor #(type ITEM_T = sba_access_item) extends dv_base_monito
     `DV_EOT_PRINT_TLM_FIFO_CONTENTS(jtag_dmi_item, jtag_dmi_fifo)
   endfunction
 
-  virtual protected task collect_trans(uvm_phase phase);
+  virtual protected task collect_trans();
     jtag_dmi_item dmi_item;
 
     forever begin

diff --git a/hw/dv/sv/jtag_riscv_agent/jtag_riscv_monitor.sv b/hw/dv/sv/jtag_riscv_agent/jtag_riscv_monitor.sv
@@ -19,7 +19,7 @@ class jtag_riscv_monitor extends dv_base_monitor #(
   endfunction
 
   // collect transactions
-  virtual protected task collect_trans(uvm_phase phase);
+  virtual protected task collect_trans();
     jtag_item item;
     logic [DMI_OPW-1:0] op_raw;
     jtag_op_e op;

diff --git a/hw/dv/sv/key_sideload_agent/key_sideload_monitor.sv b/hw/dv/sv/key_sideload_agent/key_sideload_monitor.sv
@@ -27,7 +27,7 @@ class key_sideload_monitor #(
   endtask
 
   // collect transactions forever - already forked in dv_base_monitor::run_phase
-  virtual protected task collect_trans(uvm_phase phase);
+  virtual protected task collect_trans();
     key_sideload_item#(KEY_T) prev_item;
     key_sideload_item#(KEY_T) curr_item;
     prev_item = key_sideload_item#(KEY_T)::type_id::create("prev_item");

diff --git a/hw/dv/sv/kmac_app_agent/kmac_app_monitor.sv b/hw/dv/sv/kmac_app_agent/kmac_app_monitor.sv
@@ -23,7 +23,7 @@ class kmac_app_monitor extends dv_base_monitor #(
     data_fifo = new("data_fifo", this);
   endfunction
 
-  virtual protected task collect_trans(uvm_phase phase);
+  virtual protected task collect_trans();
     forever fork
       begin : isolation_fork
         fork

diff --git a/hw/dv/sv/pattgen_agent/pattgen_monitor.sv b/hw/dv/sv/pattgen_agent/pattgen_monitor.sv
@@ -24,10 +24,10 @@ class pattgen_monitor extends dv_base_monitor #(
 
   virtual task run_phase(uvm_phase phase);
     wait(cfg.vif.rst_ni);
-    collect_trans(phase);
+    collect_trans();
   endtask : run_phase
 
-  virtual protected task collect_trans(uvm_phase phase);
+  virtual protected task collect_trans();
     for (uint i = 0; i < NUM_PATTGEN_CHANNELS; i++) begin
       fork
         automatic uint channel = i;

diff --git a/hw/dv/sv/push_pull_agent/push_pull_monitor.sv b/hw/dv/sv/push_pull_agent/push_pull_monitor.sv
@@ -24,7 +24,7 @@ class push_pull_monitor #(parameter int HostDataWidth = 32,
     cfg.in_reset = 0;
     fork
       monitor_reset();
-      collect_trans(phase);
+      collect_trans();
       // Collect partial pull reqs for the reactive pull device agent.
       collect_pull_req();
       collect_cov();
@@ -50,7 +50,7 @@ class push_pull_monitor #(parameter int HostDataWidth = 32,
   // Collect fully-completed transactions.
   //
   // TODO : sample covergroups
-  virtual protected task collect_trans(uvm_phase phase);
+  virtual protected task collect_trans();
     if (cfg.agent_type == PushAgent) begin
       forever begin
         @(cfg.vif.mon_cb);

diff --git a/hw/dv/sv/pwm_monitor/pwm_monitor.sv b/hw/dv/sv/pwm_monitor/pwm_monitor.sv
@@ -23,7 +23,7 @@ class pwm_monitor extends dv_base_monitor #(
   endfunction
 
   // collect transactions forever - already forked in dv_base_monitor::run_phase
-  virtual protected task collect_trans(uvm_phase phase);
+  virtual protected task collect_trans();
     uint count_cycles, active_cycles;
     logic pwm_prev = 0;
 

diff --git a/hw/dv/sv/rng_agent/rng_monitor.sv b/hw/dv/sv/rng_agent/rng_monitor.sv
@@ -25,7 +25,7 @@ class rng_monitor extends dv_base_monitor #(
   endtask
 
   // collect transactions forever - already forked in dv_base_monitor::run_phase
-  virtual protected task collect_trans(uvm_phase phase);
+  virtual protected task collect_trans();
   endtask
 
 endclass
diff --git a/hw/dv/sv/spi_agent/spi_monitor.sv b/hw/dv/sv/spi_agent/spi_monitor.sv
@@ -33,11 +33,11 @@ class spi_monitor extends dv_base_monitor#(
   endfunction
 
   virtual task run_phase(uvm_phase phase);
-    forever collect_trans(phase);
+    forever collect_trans();
   endtask
 
   // collect transactions
-  virtual protected task collect_trans(uvm_phase phase);
+  virtual protected task collect_trans();
     bit flash_opcode_received;
 
     wait (cfg.en_monitor);

diff --git a/hw/dv/sv/usb20_agent/usb20_monitor.sv b/hw/dv/sv/usb20_agent/usb20_monitor.sv
@@ -45,12 +45,12 @@ class usb20_monitor extends dv_base_monitor #(
   task run_phase(uvm_phase phase);
     detect_reset();
     forever begin
-      collect_trans(phase);
+      collect_trans();
     end
   endtask
 
 //-----------------------------------------Collect Trans------------------------------------------//
-  virtual protected task collect_trans(uvm_phase phase);
+  virtual protected task collect_trans();
     // Idle state detected here
     while(!(cfg.bif.usb_p & ~cfg.bif.usb_n)) @(posedge cfg.bif.usb_clk);
     @(posedge cfg.bif.usb_clk);

diff --git a/hw/ip/otbn/dv/uvm/env/otbn_trace_monitor.sv b/hw/ip/otbn/dv/uvm/env/otbn_trace_monitor.sv
@@ -11,7 +11,7 @@ class otbn_trace_monitor extends dv_base_monitor #(
   `uvm_component_utils(otbn_trace_monitor)
   `uvm_component_new
 
-  protected task collect_trans(uvm_phase phase);
+  protected task collect_trans();
     otbn_trace_item item;
     bit             item_valid = 1'b0;
 

diff --git a/hw/ip/otbn/dv/uvm/otbn_model_agent/otbn_model_monitor.sv b/hw/ip/otbn/dv/uvm/otbn_model_agent/otbn_model_monitor.sv
@@ -19,7 +19,7 @@ class otbn_model_monitor extends dv_base_monitor #(
 
   `uvm_component_new
 
-  protected task collect_trans(uvm_phase phase);
+  protected task collect_trans();
     fork
       collect_status();
       collect_insns();

diff --git a/hw/vendor/lowrisc_ibex.vendor.hjson b/hw/vendor/lowrisc_ibex.vendor.hjson
@@ -4,6 +4,7 @@
 {
   name: "lowrisc_ibex",
   target_dir: "lowrisc_ibex",
+  patch_dir: "patches/lowrisc_ibex",
 
   upstream: {
     url: "https://github.com/lowRISC/ibex.git",
@@ -12,7 +13,11 @@
 
   mapping: [
     "doc",
-    "dv",
+    {
+      from:      "dv",
+      to:        "dv",
+      patch_dir: "dv"
+    }
     "lint",
     "rtl",
     "syn",

diff --git a/hw/vendor/lowrisc_ibex/dv/uvm/icache/dv/ibex_icache_core_agent/ibex_icache_core_monitor.sv b/hw/vendor/lowrisc_ibex/dv/uvm/icache/dv/ibex_icache_core_agent/ibex_icache_core_monitor.sv
@@ -35,7 +35,7 @@ class ibex_icache_core_monitor extends dv_base_monitor #(
   endtask
 
   // collect transactions forever - already forked in dv_base_moditor::run_phase
-  virtual protected task collect_trans(uvm_phase phase);
+  virtual protected task collect_trans();
     ibex_icache_core_bus_item trans;
     logic                     last_inval = 0;
     logic                     last_enable = 0;

diff --git a/hw/vendor/lowrisc_ibex/dv/uvm/icache/dv/ibex_icache_mem_agent/ibex_icache_mem_monitor.sv b/hw/vendor/lowrisc_ibex/dv/uvm/icache/dv/ibex_icache_mem_agent/ibex_icache_mem_monitor.sv
@@ -30,7 +30,7 @@ class ibex_icache_mem_monitor
   endtask
 
   // Collect transactions forever. Forked in dv_base_moditor::run_phase
-  protected task automatic collect_trans(uvm_phase phase);
+  protected task automatic collect_trans();
     fork
       collect_grants();
       collect_responses();

diff --git a/hw/vendor/patches/lowrisc_ibex/dv/0001-dv-Remove-phase-argument-from-collect_trans.patch b/hw/vendor/patches/lowrisc_ibex/dv/0001-dv-Remove-phase-argument-from-collect_trans.patch
@@ -0,0 +1,51 @@
+From aef478fb6eba46234703f7600d2221f6e50ea025 Mon Sep 17 00:00:00 2001
+From: Rupert Swarbrick <[email protected]>
+Date: Thu, 21 Mar 2024 13:14:32 +0000
+Subject: [PATCH] [dv] Remove phase argument from collect_trans
+
+The prototype of this task has to match the one in dv_base_monitor,
+which we are importing from OpenTitan (called "lowrisc_ip").
+Unfortunately, OpenTitan imports Ibex, causing a circular reference
+which makes it a bit fiddly to change any types.
+
+This commit switches the tasks to match the new prototype we're going
+to use in OpenTitan. We can't just apply it in Ibex
+immediately (because it won't work with our vendored lowrisc_ip code),
+but creating the commit *does* mean we can vendor in the changed Ibex
+code at the OpenTitan end.
+
+Once that's sorted, we can vendor OpenTitan back into Ibex and get
+everything cleaned up properly.
+
+In hindsight, we probably should have made sure our vendoring
+structure was a DAG.
+
+(This patch is manually edited from something that came out of git
+format-patch, but is now applied to just a subdirectory)
+
+diff --git a/uvm/icache/dv/ibex_icache_core_agent/ibex_icache_core_monitor.sv b/dv/uvm/icache/dv/ibex_icache_core_agent/ibex_icache_core_monitor.sv
+index deef3418..8b66823c 100644
+--- a/uvm/icache/dv/ibex_icache_core_agent/ibex_icache_core_monitor.sv
++++ b/uvm/icache/dv/ibex_icache_core_agent/ibex_icache_core_monitor.sv
+@@ -35,7 +35,7 @@ class ibex_icache_core_monitor extends dv_base_monitor #(
+   endtask
+
+   // collect transactions forever - already forked in dv_base_moditor::run_phase
+-  virtual protected task collect_trans(uvm_phase phase);
++  virtual protected task collect_trans();
+     ibex_icache_core_bus_item trans;
+     logic                     last_inval = 0;
+     logic                     last_enable = 0;
+diff --git a/uvm/icache/dv/ibex_icache_mem_agent/ibex_icache_mem_monitor.sv b/dv/uvm/icache/dv/ibex_icache_mem_agent/ibex_icache_mem_monitor.sv
+index b9e5c14d..6778cd5a 100644
+--- a/uvm/icache/dv/ibex_icache_mem_agent/ibex_icache_mem_monitor.sv
++++ b/uvm/icache/dv/ibex_icache_mem_agent/ibex_icache_mem_monitor.sv
+@@ -30,7 +30,7 @@ class ibex_icache_mem_monitor
+   endtask
+
+   // Collect transactions forever. Forked in dv_base_moditor::run_phase
+-  protected task automatic collect_trans(uvm_phase phase);
++  protected task automatic collect_trans();
+     fork
+       collect_grants();
+       collect_responses();
diff --git a/util/uvmdvgen/README.md b/util/uvmdvgen/README.md
@@ -104,7 +104,7 @@ IP. The following describes their contents in each source generated:
     This is the monitor component extended from `dv_base_monitor`. It provides
     the following items:
 
-    * `virtual protected task collect_trans(uvm_phase phase)`
+    * `virtual protected task collect_trans()`
 
         This is a shell task within which user is required to add logic to detect
         an event, sample the interface and create a transaction object and write

diff --git a/util/uvmdvgen/monitor.sv.tpl b/util/uvmdvgen/monitor.sv.tpl
@@ -25,7 +25,7 @@ class ${name}_monitor extends dv_base_monitor #(
   endtask
 
   // collect transactions forever - already forked in dv_base_monitor::run_phase
-  virtual protected task collect_trans(uvm_phase phase);
+  virtual protected task collect_trans();
     forever begin
       // TODO: detect event