Move seed updates into sequence in ICache memory agent

The previous code kind of worked, but we were making the "should I
make a new seed" decision in the monitor, rather than the sequence.
The problem is that this is difficult to customize with other test
sequences (they sit adjacent to the monitor in the class hierarchy,
not above it).

The new code seems a little cleaner. We generate new seeds in the
sequence (which is in charge of keeping track of the current seed
anyway). These new seeds get passed to the driver, which has an
analysis port by which it can tell the scoreboard about them. Note
that we have to pass them from the driver, rather than the monitor,
because the new seed doesn't directly appear on the interface.

The rest of the changes are simplifying the ibex_icache_mem_bus_item
class, which now only has two modes and removing the seed field from
the ibex_icache_mem_req_item class.

dv/uvm/icache/dv/env/ibex_icache_env.sv

@@ -39,6 +39,7 @@ class ibex_icache_env extends dv_base_env #(
     if (cfg.en_scb) begin
       core_agent.monitor.analysis_port.connect(scoreboard.core_fifo.analysis_export);
       mem_agent.monitor.analysis_port.connect(scoreboard.mem_fifo.analysis_export);
+      mem_agent.driver.analysis_port.connect(scoreboard.seed_fifo.analysis_export);
     end
     if (cfg.is_active && cfg.core_agent_cfg.is_active) begin
       virtual_sequencer.core_sequencer_h = core_agent.sequencer;

dv/uvm/icache/dv/env/ibex_icache_scoreboard.sv

@@ -8,8 +8,14 @@ class ibex_icache_scoreboard
   `uvm_component_utils(ibex_icache_scoreboard)
 
   // TLM agent fifos
+  //
+  // core_fifo comes from the core monitor. mem_fifo comes from the memory monitor. seed_fifo comes
+  // from the memory driver (not monitor) and tells us about new seeds in the backing memory. These
+  // are generated in the sequence, but don't cause any change on the interface, so need reporting
+  // by the driver.
   uvm_tlm_analysis_fifo #(ibex_icache_core_bus_item) core_fifo;
   uvm_tlm_analysis_fifo #(ibex_icache_mem_bus_item)  mem_fifo;
+  uvm_tlm_analysis_fifo #(bit [31:0])                seed_fifo;
 
   localparam BusWidth = 32;
 
@@ -58,6 +64,7 @@ class ibex_icache_scoreboard
     super.build_phase(phase);
     core_fifo = new("core_fifo", this);
     mem_fifo  = new("mem_fifo",  this);
+    seed_fifo = new("seed_fifo", this);
     mem_model = new("mem_model");
   endfunction
 
@@ -66,6 +73,7 @@ class ibex_icache_scoreboard
     fork
       process_core_fifo();
       process_mem_fifo();
+      process_seed_fifo();
     join_none
   endtask
 
@@ -133,21 +141,22 @@ class ibex_icache_scoreboard
       mem_fifo.get(item);
       `uvm_info(`gfn, $sformatf("received mem transaction:\n%0s", item.sprint()), UVM_HIGH)
 
-      case (item.trans_type)
-        ICacheMemNewSeed: begin
-          mem_seeds.push_back(item.data);
-        end
+      if (item.is_grant) begin
+        mem_trans_count += 1;
+      end else begin
+        busy_check();
+        `DV_CHECK_FATAL(mem_trans_count > 0);
+        mem_trans_count -= 1;
+      end
+    end
+  endtask
 
-        ICacheMemGrant: begin
-          mem_trans_count += 1;
-        end
-
-        ICacheMemResponse: begin
-          busy_check();
-          `DV_CHECK_FATAL(mem_trans_count > 0);
-          mem_trans_count -= 1;
-        end
-      endcase
+  task process_seed_fifo();
+    int unsigned seed;
+    forever begin
+      seed_fifo.get(seed);
+      `uvm_info(`gfn, $sformatf("received new seed: %08h", seed), UVM_HIGH)
+      mem_seeds.push_back(seed);
     end
   endtask
 

dv/uvm/icache/dv/ibex_icache_mem_agent/ibex_icache_mem_agent_pkg.sv

@@ -12,12 +12,6 @@ package ibex_icache_mem_agent_pkg;
   `include "uvm_macros.svh"
   `include "dv_macros.svh"
 
-  typedef enum {
-    ICacheMemNewSeed,
-    ICacheMemGrant,
-    ICacheMemResponse
-  } ibex_icache_mem_trans_type_e;
-
   // package sources
   `include "ibex_icache_mem_req_item.sv"
   `include "ibex_icache_mem_resp_item.sv"

dv/uvm/icache/dv/ibex_icache_mem_agent/ibex_icache_mem_bus_item.sv

@@ -8,20 +8,20 @@
 
 class ibex_icache_mem_bus_item extends uvm_sequence_item;
 
-  // What sort of transaction is this? (new seed, grant or response)
-  ibex_icache_mem_trans_type_e trans_type;
+  // What sort of transaction is this? (grant or response)
+  bit          is_grant;
 
-  // This holds the new seed for a 'new seed' transaction, the request address for a grant
-  // transaction and the returned rdata for a response transaction.
+  // This holds the request address for a grant transaction and the returned rdata for a response
+  // transaction.
   logic [31:0] data;
 
   // Response error flag (only valid for response transactions)
   logic        err;
 
   `uvm_object_utils_begin(ibex_icache_mem_bus_item)
-    `uvm_field_enum(ibex_icache_mem_trans_type_e, trans_type, UVM_DEFAULT)
-    `uvm_field_int(data, UVM_DEFAULT | UVM_HEX)
-    `uvm_field_int(err,  UVM_DEFAULT)
+    `uvm_field_int(is_grant, UVM_DEFAULT)
+    `uvm_field_int(data,     UVM_DEFAULT | UVM_HEX)
+    `uvm_field_int(err,      UVM_DEFAULT)
   `uvm_object_utils_end
 
   `uvm_object_new

dv/uvm/icache/dv/ibex_icache_mem_agent/ibex_icache_mem_driver.sv

@@ -20,13 +20,19 @@ class ibex_icache_mem_driver
   mailbox #(ibex_icache_mem_resp_item) rdata_queue;
   mailbox #(bit [31:0])                pmp_queue;
 
+  // An analysis port that gets hooked up to the scoreboard. This isn't used to report on actual bus
+  // traffic (that's done by the monitor, as usual) but is used to report on new memory seeds, which
+  // the scoreboard needs to know about, but don't in themselves cause any bus traffic.
+  uvm_analysis_port #(bit [31:0]) analysis_port;
+
   `uvm_component_utils(ibex_icache_mem_driver)
   `uvm_component_new
 
   function void build_phase(uvm_phase phase);
     super.build_phase(phase);
-    rdata_queue = new("rdata_queue");
-    pmp_queue   = new("pmp_queue");
+    rdata_queue   = new("rdata_queue");
+    pmp_queue     = new("pmp_queue");
+    analysis_port = new("analysis_port", this);
   endfunction
 
   virtual task reset_signals();
@@ -73,8 +79,15 @@ class ibex_icache_mem_driver
 
       // Is this a request or a grant?
       if (!req.is_grant) begin
-        // If a request, it's ignored unless it causes a PMP error. In that case, we push the
-        // address onto pmp_queue.
+        // A request has two effects.
+        //
+        //   1. If this is a new seed then we need to tell the scoreboard about it.
+        //
+        //   2. If it causes a PMP error, we push the address onto pmp_queue (which will be handled
+        //      by drive_pmp).
+        if (req.seed != 32'd0) begin
+          analysis_port.write(req.seed);
+        end
         if (req.err) begin
           pmp_queue.put(req.address);
         end

dv/uvm/icache/dv/ibex_icache_mem_agent/ibex_icache_mem_monitor.sv

@@ -76,9 +76,9 @@ class ibex_icache_mem_monitor
     forever begin
       if (cfg.vif.monitor_cb.rvalid) begin
         bus_trans = ibex_icache_mem_bus_item::type_id::create("bus_trans");
-        bus_trans.trans_type = ICacheMemResponse;
-        bus_trans.data       = cfg.vif.monitor_cb.rdata;
-        bus_trans.err        = cfg.vif.monitor_cb.err;
+        bus_trans.is_grant = 1'b0;
+        bus_trans.data     = cfg.vif.monitor_cb.rdata;
+        bus_trans.err      = cfg.vif.monitor_cb.err;
         analysis_port.write(bus_trans);
       end
 
@@ -87,24 +87,12 @@ class ibex_icache_mem_monitor
   endtask
 
   // This is called immediately when an address is requested and is used to drive the PMP response.
-  // If we decide to choose a new seed with this transaction, the seed also gets passed to the
-  // scoreboard.
   function automatic void new_request(logic [31:0] addr);
-    ibex_icache_mem_bus_item bus_trans;
     ibex_icache_mem_req_item item = new("item");
 
     item.is_grant = 1'b0;
     item.address = addr;
-    `DV_CHECK_RANDOMIZE_FATAL(item)
     request_port.write(item);
-
-    if (item.seed != 0) begin
-      bus_trans = ibex_icache_mem_bus_item::type_id::create("bus_trans");
-      bus_trans.trans_type = ICacheMemNewSeed;
-      bus_trans.data       = item.seed;
-      bus_trans.err        = '0;
-      analysis_port.write(bus_trans);
-    end
   endfunction
 
   // This is called on a clock edge when an request is granted
@@ -115,13 +103,12 @@ class ibex_icache_mem_monitor
     item = ibex_icache_mem_req_item::type_id::create("item");
     item.is_grant = 1'b1;
     item.address  = addr;
-    item.seed     = '0;
     request_port.write(item);
 
     bus_trans = ibex_icache_mem_bus_item::type_id::create("bus_trans");
-    bus_trans.trans_type = ICacheMemGrant;
-    bus_trans.data       = addr;
-    bus_trans.err        = 0;
+    bus_trans.is_grant = 1'b1;
+    bus_trans.data     = addr;
+    bus_trans.err      = 0;
     analysis_port.write(bus_trans);
   endfunction
 

dv/uvm/icache/dv/ibex_icache_mem_agent/ibex_icache_mem_req_item.sv

@@ -7,7 +7,7 @@
 //
 // When a request first comes in (via a posedge on the req line), it immediately generates a
 // req_item with is_grant = 0. This is used by the sequence to tell the driver whether to generate a
-// PMP error. A req_item with is_grant = 0 may also have a seed update.
+// PMP error.
 //
 // Assuming that the request wasn't squashed by a PMP error, it will be granted on some later clock
 // edge. At that point, another req_item is generated with is_grant = 1. This is added to a queue in
@@ -18,20 +18,9 @@ class ibex_icache_mem_req_item extends uvm_sequence_item;
   bit               is_grant;
   logic [31:0]      address;
 
-  rand bit [31:0]   seed;
-
-  // Change the memory seed roughly one time in 500 reads
-  constraint c_seed_dist {
-    seed dist {
-      32'd0            :/ 499,
-      [1:32'hffffffff] :/ 1
-    };
-  }
-
   `uvm_object_utils_begin(ibex_icache_mem_req_item)
     `uvm_field_int (is_grant, UVM_DEFAULT)
     `uvm_field_int (address,  UVM_DEFAULT | UVM_HEX)
-    `uvm_field_int (seed,     UVM_DEFAULT | UVM_HEX)
   `uvm_object_utils_end
 
   `uvm_object_new

dv/uvm/icache/dv/ibex_icache_mem_agent/ibex_icache_mem_resp_item.sv

@@ -4,8 +4,7 @@
 
 // An item that represents a memory response to the icache
 
-class ibex_icache_mem_resp_item
-  extends uvm_sequence_item;
+class ibex_icache_mem_resp_item extends uvm_sequence_item;
 
   int unsigned min_response_delay = 0;
   int unsigned mid_response_delay = 5;
@@ -28,6 +27,9 @@ class ibex_icache_mem_resp_item
   // Only has an effect if is_grant. The memory data to reply with (will be 'X if err is set)
   logic [31:0]      rdata;
 
+  // A new memory seed if non-zero. Only has an effect if !is_grant.
+  rand bit [31:0]   seed;
+
   constraint c_delay_dist {
     delay dist {
       min_response_delay                        :/ 5,
@@ -36,18 +38,31 @@ class ibex_icache_mem_resp_item
     };
   }
 
+  constraint c_seed_dist {
+    seed dist {
+      32'd0            :/ 499,
+      [1:32'hffffffff] :/ 1
+    };
+  }
+
   // The delay field has no effect for requests (i.e. if is_grant is false). Force it to zero rather
   // than leave mysterious numbers in the logs.
   constraint c_no_delay_for_req {
     (!is_grant) -> delay == 0;
   }
 
+  // Similarly, the seed should only be nonzero if is_grant is false.
+  constraint c_no_new_seed_for_gnt {
+    is_grant -> seed == 0;
+  }
+
   `uvm_object_utils_begin(ibex_icache_mem_resp_item)
     `uvm_field_int (is_grant, UVM_DEFAULT)
     `uvm_field_int (err,      UVM_DEFAULT)
     `uvm_field_int (address,  UVM_DEFAULT | UVM_HEX)
     `uvm_field_int (delay,    UVM_DEFAULT)
     `uvm_field_int (rdata,    UVM_DEFAULT | UVM_HEX)
+    `uvm_field_int (seed,     UVM_DEFAULT | UVM_HEX)
   `uvm_object_utils_end
 
   `uvm_object_new

dv/uvm/icache/dv/ibex_icache_mem_agent/seq_lib/ibex_icache_mem_resp_seq.sv

@@ -48,11 +48,6 @@ class ibex_icache_mem_resp_seq extends ibex_icache_mem_base_seq;
         // If this is a request (not a grant), take any new seed and then check the memory model for
         // a PMP error at this address. Add the address and its corresponding seed to the list of
         // pending grants.
-        if (req_item.seed != 32'd0) begin
-          `uvm_info(`gfn, $sformatf("New memory seed: 0x%08h", req_item.seed), UVM_HIGH)
-          cur_seed = req_item.seed;
-        end
-
         resp_item.is_grant = 1'b0;
         resp_item.err      = mem_model.is_pmp_error(cur_seed, req_item.address);
         resp_item.address  = req_item.address;
@@ -83,10 +78,17 @@ class ibex_icache_mem_resp_seq extends ibex_icache_mem_base_seq;
         resp_item.rdata    = resp_item.err ? 'X : mem_model.read_data(gnt_seed, req_item.address);
       end
 
-      // Use the response item as an entry in the sequence, randomising any delay
+      // Use the response item as an entry in the sequence, randomising any delay and seed update
       start_item(resp_item);
       `DV_CHECK_RANDOMIZE_FATAL(resp_item)
       finish_item(resp_item);
+
+      // If this was a request (not a grant), and caused a seed update, update cur_seed. This seed
+      // update will apply immediately after this item.
+      if (!req_item.is_grant && resp_item.seed != 32'd0) begin
+        `uvm_info(`gfn, $sformatf("New memory seed: 0x%08h", resp_item.seed), UVM_HIGH)
+        cur_seed = resp_item.seed;
+      end
     end
   endtask