Flash status synchronisation tb fix

hw/dv/sv/spi_agent/spi_agent_cfg.sv

@@ -19,7 +19,7 @@ class spi_agent_cfg extends dv_base_agent_cfg;
   bit partial_byte;       // Transfering less than byte
   bit [3:0] bits_to_transfer; // Bits to transfer if using less than byte
   bit decode_commands;  // Used in monitor if decoding of commands needed
-  bit [2:0] cmd_addr_size = 4; //Address size for command
+  bit [2:0] cmd_addr_size = 4; // Address size for command
 
   //-------------------------
   // spi_host regs

hw/dv/sv/spi_agent/spi_host_driver.sv

@@ -198,10 +198,14 @@ class spi_host_driver extends spi_driver;
       if (req.write_command) begin
         issue_data(req.payload_q, dummy_return_q);
       end else begin
+        int iter = 0;
         repeat (req.read_size) begin
           logic [7:0] data;
           cfg.read_byte(.num_lanes(req.num_lanes), .is_device_rsp(1),
                         .csb_id(active_csb), .data(data));
+          `uvm_info(`gfn, $sformatf("HOST_DRV: total_size=%0d - iter=%0d - read_byte=0x%0x",
+                                    req.read_size, iter, data), UVM_DEBUG)
+          iter++;
           rsp.payload_q.push_back(data);
         end
         `uvm_info(`gfn, $sformatf("collect read data for flash: 0x%p", rsp.payload_q), UVM_HIGH)

hw/dv/sv/spi_agent/spi_item.sv

@@ -32,6 +32,9 @@ class spi_item extends uvm_sequence_item;
   // of wait until the entire item is collected. This indicates item has collected all data.
   bit mon_item_complete;
 
+  // Triggered for each byte sampled and when CSB becomes inactive
+  event byte_sampled_ev, item_finished_ev;
+
   // constrain size of data sent / received to be at most 64kB
   constraint data_size_c { data.size() inside {[0:65536]}; }
 

hw/dv/sv/spi_agent/spi_monitor.sv

@@ -23,13 +23,16 @@ class spi_monitor extends dv_base_monitor#(
   // Analysis port for the collected transfer.
   uvm_analysis_port #(spi_item) host_analysis_port;
   uvm_analysis_port #(spi_item) device_analysis_port;
+  // Sends txn on CSB deassertion (CSB becoming active)
+  uvm_analysis_port #(spi_item) csb_active_analysis_port;
 
   `uvm_component_new
 
   function void build_phase(uvm_phase phase);
     super.build_phase(phase);
-    host_analysis_port   = new("host_analysis_port", this);
-    device_analysis_port = new("device_analysis_port", this);
+    host_analysis_port       = new("host_analysis_port", this);
+    device_analysis_port     = new("device_analysis_port", this);
+    csb_active_analysis_port = new("csb_active_analysis_port", this);
   endfunction
 
   virtual task run_phase(uvm_phase phase);
@@ -46,9 +49,9 @@ class spi_monitor extends dv_base_monitor#(
 
     cfg.vif.sck_polarity = cfg.sck_polarity[active_csb];
     cfg.vif.sck_phase = cfg.sck_phase[active_csb];
-
     host_item   = spi_item::type_id::create("host_item", this);
     device_item = spi_item::type_id::create("device_item", this);
+    csb_active_analysis_port.write(host_item);
     fork
       begin : isolation_thread
         fork
@@ -75,6 +78,11 @@ class spi_monitor extends dv_base_monitor#(
         disable fork;
       end : isolation_thread
     join
+
+    -> host_item.item_finished_ev;
+    `uvm_info(`gfn, "Triggering 'host_item.item_finished' since CSB just became inactive",
+              UVM_DEBUG)
+
     // write to host_analysis_port
     case (cfg.spi_func_mode)
       SpiModeFlash: begin
@@ -90,7 +98,7 @@ class spi_monitor extends dv_base_monitor#(
         end
       end
       default: ; // do nothing, in SpiModeGeneric, it writes to fifo for each byte
-    endcase
+    endcase // case (cfg.spi_func_mode)
   endtask : collect_trans
 
   virtual protected task collect_curr_trans();
@@ -238,17 +246,26 @@ class spi_monitor extends dv_base_monitor#(
         num_addr_bytes, item.write_command, item.num_lanes, item.dummy_cycles);
     `uvm_info(`gfn, $sformatf("sampled flash opcode: 0x%0h", item.opcode), UVM_HIGH)
 
+    `uvm_info(`gfn, "Triggering 'host_item.byte_sampled' after sampling opcode", UVM_DEBUG)
+    -> host_item.byte_sampled_ev;
     sample_address(num_addr_bytes, item.address_q);
-
+    if(item.address_q.size > 0) begin
+      `uvm_info(`gfn, "Triggering 'host_item.byte_sampled' after sampling address", UVM_DEBUG)
+      -> host_item.byte_sampled_ev;
+    end
     repeat (item.dummy_cycles) begin
       cfg.wait_sck_edge(SamplingEdge, active_csb);
     end
+    `uvm_info(`gfn, $sformatf("Sending {opcode=0x%0x,address=%p} on the 'req_analysis_port'",
+                              item.opcode, item.address_q), UVM_DEBUG)
     req_analysis_port.write(item);
 
     forever begin
       logic[7:0] byte_data;
       sample_and_check_byte(item.num_lanes, !item.write_command, byte_data);
       item.payload_q.push_back(byte_data);
+      `uvm_info(`gfn, "Triggering 'host_item.byte_sampled' after sampling a data byte", UVM_DEBUG)
+      -> host_item.byte_sampled_ev;
     end
   endtask : collect_flash_trans
 
@@ -262,6 +279,9 @@ class spi_monitor extends dv_base_monitor#(
     decode_tpm_cmd(cmd, item.write_command, size);
     if (!item.write_command) item.read_size = size;
     `uvm_info(`gfn, $sformatf("Received TPM command: 0x%0x", cmd), UVM_MEDIUM)
+    `uvm_info(`gfn, "Triggering 'host_item.byte_sampled' after sampling opcode", UVM_DEBUG)
+    -> host_item.byte_sampled_ev;
+
     // read 3 bytes address
     fork
       begin : tpm_sio_0
@@ -279,6 +299,12 @@ class spi_monitor extends dv_base_monitor#(
       end
     join
     `uvm_info(`gfn, $sformatf("Received TPM addr: %p", item.address_q), UVM_MEDIUM)
+
+    if(item.address_q.size > 0) begin
+      `uvm_info(`gfn, "Triggering 'host_item.byte_sampled' after sampling address", UVM_DEBUG)
+      -> host_item.byte_sampled_ev;
+    end
+
     req_analysis_port.write(item);
 
     // poll TPM_START
@@ -297,6 +323,9 @@ class spi_monitor extends dv_base_monitor#(
                             .data(item.data[i]), .check_data_not_z(1));
       `uvm_info(`gfn, $sformatf("collect %s data for TPM, idx %0d: 0x%0x",
                               item.write_command ? "write" : "read", i, item.data[i]), UVM_MEDIUM)
+      `uvm_info(`gfn, "Triggering 'host_item.byte_sampled' after sampling a data byte", UVM_DEBUG)
+      -> host_item.byte_sampled_ev;
+
     end
   endtask
 

hw/ip/spi_device/dv/env/seq_lib/spi_device_intercept_vseq.sv

@@ -34,10 +34,132 @@ class spi_device_intercept_vseq extends spi_device_pass_cmd_filtering_vseq;
     super.pre_start();
   endtask
 
+  virtual task random_cycle_delay();
+    int unsigned cycle_delay;
+    if(!std::randomize(cycle_delay) with { cycle_delay dist {[1:10] := 80,
+                                                             [11:20] := 15,
+                                                             [21:50] := 4,
+                                                             [51:90] := 1
+                                                             };  })
+      `uvm_fatal(`gfn, "Randomization Failure")
+    `uvm_info(`gfn, $sformatf("Inserting %0d TL-UL cycle delays",cycle_delay), UVM_DEBUG)
+    cfg.clk_rst_vif.wait_clks(cycle_delay);
+  endtask
+
   // randomly set flash_status for every spi transaction
   virtual task spi_host_xfer_flash_item(bit [7:0] op, uint payload_size,
                                         bit [31:0] addr, bit wait_on_busy = 1);
-    random_access_flash_status();
-    super.spi_host_xfer_flash_item(op, payload_size, addr, wait_on_busy);
+    typedef enum  {sequential_access, concurrent_access, two_writes, wrong} access_option_t;
+
+    bit                                            delay_free;
+    bit                                            spi_command_finished;
+    access_option_t             access_option;
+    // Note: there shouldn't be two flash_status writes in a row without SPI command in between
+    // The RTL could absord a second write, but it wouldn't be correct operation in terms of SW
+    // behaviour. In theory, one could write the upper-bits of flash_status, followed by another
+    // write to clear the busy bit (done in least probable case within the randcase below).
+    //
+    // In practice, SW writes to flash_status should only occur as a response to a command which
+    // sets the busy bit.
+    // When the RTL processes a read_status command internally, it commits a whole byte of status
+    // bits to return to the host. So if SW were to write flash_status whilst the host was sending
+    // a READ_STATUS command, and the host left the CSB line low "fow a while" expecting for
+    // instance to read the busy bit unset, the host would see complete bytes written to
+    // flash_status and not a byte made of two different writes
+
+    access_option = wrong;
+    randcase
+      25: access_option = sequential_access;
+      70: access_option = concurrent_access;
+      5 : access_option = two_writes;
+    endcase // randcase
+
+    case (access_option)
+      sequential_access: begin // Sequential accesses
+        random_access_flash_status();
+        super.spi_host_xfer_flash_item(op, payload_size, addr, wait_on_busy);
+      end
+      concurrent_access: begin // Concurrent accesses
+        fork
+          begin
+            bit send_write, write_sent;
+
+            while (spi_command_finished==0) begin
+              randcase
+                9: delay_free = 0;
+                1: delay_free = 1;
+              endcase
+
+              if (!delay_free)
+                random_cycle_delay();
+
+              if (!write_sent) begin
+                // Only randomize until we send the first write
+                if(!std::randomize(send_write) with { send_write dist {1 := 15, 0 := 85}; })
+                  `uvm_fatal(`gfn, "Randomization Failure")
+              end
+              // Sending 1-write only
+              if (!write_sent && send_write) begin
+                random_access_flash_status(.write(send_write));
+                write_sent = 1;
+              end
+              else begin
+                random_access_flash_status(.write(0));
+              end
+            end // while (spi_command_finished==0)
+          end
+          begin
+            // Thread gets killed on the SPI command below completing
+            super.spi_host_xfer_flash_item(op, payload_size, addr, wait_on_busy);
+            spi_command_finished = 1;
+          end
+        join
+
+      end // case: concurrent_access
+      two_writes: begin
+        // keep reading flash_status, and if the busy bit was set, then we send two writes
+        // to flash_status, the first setting the upper bits, and the second clearing the
+        // WEL/BUSY bits
+        fork
+          begin
+            bit [TL_DW-1:0] read_status;
+            bit [21:0]      other_status;
+
+            while (spi_command_finished==0) begin
+              randcase
+                9: delay_free = 0;
+                1: delay_free = 1;
+              endcase
+
+              if (!delay_free)
+                random_cycle_delay();
+
+              // Keep reading until the busy bit is set
+              if (read_status[0] == 0)
+                csr_rd(ral.flash_status, read_status);
+              else begin
+                // Busy bit is set set - setting first the upper bits of flash_status:
+                other_status = $urandom();
+                random_access_flash_status(.write(1), .busy(1), .wel(1),
+                                           .other_status(other_status));
+                cfg.clk_rst_vif.wait_clks($urandom_range(1,5));
+
+                // Clearing busy and wel bits
+                random_access_flash_status(.write(1), .busy(0), .wel(0),
+                                           .other_status(other_status));
+                read_status[0] = 0; // Busy bit has been cleared
+              end
+            end // forever begin
+          end
+          begin
+            // Thread gets killed on the SPI command below completing
+            super.spi_host_xfer_flash_item(op, payload_size, addr, wait_on_busy);
+            spi_command_finished = 1;
+          end
+        join
+      end // case: two_writes
+      default: `uvm_fatal(`gfn, "Wrong case statement")
+    endcase
   endtask
+
 endclass : spi_device_intercept_vseq

hw/ip/spi_device/dv/env/seq_lib/spi_device_pass_base_vseq.sv

@@ -793,12 +793,22 @@ class spi_device_pass_base_vseq extends spi_device_base_vseq;
           bit is_watermark;
           // use zero_delay write, otherwise, spi may read faster than SW write
           bit zero_delay_write = 1;
+          int unsigned delay = $urandom_range(10, 200);
 
-          cfg.clk_rst_vif.wait_clks($urandom_range(10, 200));
+          // We read 'intr_state' through the backdoor every cycle to see if a flip/watermark
+          // event occurs. Just because otherwise, if we apply a "random delay" it's possible the
+          // new value will be written at a time the RTL has already "latched" the older value and
+          // there will be a mem comparison mismatch
+          // Once we read a readbufflip/watermark interrupt via backdoor, then do a front-door read
+          //  to keep the predictions intact
+          cfg.clk_rst_vif.wait_clks(1);
+          csr_rd(.ptr(ral.intr_state), .value(intr_state_val), .backdoor(1));
 
-          csr_rd(ral.intr_state, intr_state_val);
           is_flip = get_field_val(ral.intr_state.readbuf_flip, intr_state_val);
           is_watermark = get_field_val(ral.intr_state.readbuf_watermark, intr_state_val);
+          if (is_flip || is_watermark) // Front-door read to keep the SCB predictions intact
+            csr_rd(ral.intr_state, intr_state_val);
+
 
           // clear flip and watermark event before updating mem
           // if clear too late, scb is hard to handle as the interrupt may happen again
@@ -807,10 +817,8 @@ class spi_device_pass_base_vseq extends spi_device_base_vseq;
           if (is_watermark) intr_state_val[ReadbufWatermark] = 1;
 
           if (!is_flip && !is_watermark) continue;
-          // Wait for register access to complete
-          while(ral.intr_state.is_busy()) begin
-            cfg.clk_rst_vif.wait_clks(1);
-          end
+
+          cfg.clk_rst_vif.wait_clks(1);
           csr_wr(ral.intr_state, intr_state_val);
 
           if (is_flip) begin

hw/ip/spi_device/dv/env/spi_device_env.sv

@@ -35,6 +35,9 @@ class spi_device_env extends cip_base_env #(
           scoreboard.upstream_spi_device_fifo.analysis_export);
       spi_host_agent.monitor.req_analysis_port.connect(
           scoreboard.upstream_spi_req_fifo.analysis_export);
+      spi_host_agent.monitor.csb_active_analysis_port.connect(
+          scoreboard.upstream_csb_active_fifo.analysis_export);
+
       spi_device_agent.monitor.host_analysis_port.connect(
           scoreboard.downstream_spi_host_fifo.analysis_export);
     end