Mem intg testing

dv/cosim/cosim.h

@@ -72,7 +72,7 @@ class Cosim {
   //
   // Returns false if there are any errors; use `get_errors` to obtain details
   virtual bool step(uint32_t write_reg, uint32_t write_reg_data, uint32_t pc,
-                    bool sync_trap) = 0;
+                    bool sync_trap, bool suppress_reg_write) = 0;
 
   // When more than one of `set_mip`, `set_nmi` or `set_debug_req` is called
   // before `step` which one takes effect is chosen by the co-simulator. Which
@@ -95,6 +95,12 @@ class Cosim {
   // When an NMI is due to be taken that will occur at the next call of `step`.
   virtual void set_nmi(bool nmi) = 0;
 
+  // Set the state of the internal NMI (non-maskable interrupt) line.
+  // Behaviour wise this is almost as same as external NMI case explained at
+  // set_nmi method. Difference is that this one is a response from Ibex rather
+  // than an input.
+  virtual void set_nmi_int(bool nmi_int) = 0;
+
   // Set the debug request.
   //
   // When set to true the core will enter debug mode at the next step

dv/cosim/cosim_dpi.cc

@@ -10,10 +10,13 @@
 
 int riscv_cosim_step(Cosim *cosim, const svBitVecVal *write_reg,
                      const svBitVecVal *write_reg_data, const svBitVecVal *pc,
-                     svBit sync_trap) {
+                     svBit sync_trap, svBit suppress_reg_write) {
   assert(cosim);
 
-  return cosim->step(write_reg[0], write_reg_data[0], pc[0], sync_trap) ? 1 : 0;
+  return cosim->step(write_reg[0], write_reg_data[0], pc[0], sync_trap,
+                     suppress_reg_write)
+             ? 1
+             : 0;
 }
 
 void riscv_cosim_set_mip(Cosim *cosim, const svBitVecVal *mip) {
@@ -28,6 +31,11 @@ void riscv_cosim_set_nmi(Cosim *cosim, svBit nmi) {
   cosim->set_nmi(nmi);
 }
 
+void riscv_cosim_set_nmi_int(Cosim *cosim, svBit nmi_int) {
+  assert(cosim);
+
+  cosim->set_nmi_int(nmi_int);
+}
 void riscv_cosim_set_debug_req(Cosim *cosim, svBit debug_req) {
   assert(cosim);
 

dv/cosim/cosim_dpi.h

@@ -14,9 +14,10 @@
 extern "C" {
 int riscv_cosim_step(Cosim *cosim, const svBitVecVal *write_reg,
                      const svBitVecVal *write_reg_data, const svBitVecVal *pc,
-                     svBit sync_trap);
+                     svBit sync_trap, svBit suppress_reg_write);
 void riscv_cosim_set_mip(Cosim *cosim, const svBitVecVal *mip);
 void riscv_cosim_set_nmi(Cosim *cosim, svBit nmi);
+void riscv_cosim_set_nmi_int(Cosim *cosim, svBit nmi_int);
 void riscv_cosim_set_debug_req(Cosim *cosim, svBit debug_req);
 void riscv_cosim_set_mcycle(Cosim *cosim, svBitVecVal *mcycle);
 void riscv_cosim_set_csr(Cosim *cosim, const int csr_id,

dv/cosim/cosim_dpi.svh

@@ -11,9 +11,10 @@
 `define COSIM_DPI_SVH
 
 import "DPI-C" function int riscv_cosim_step(chandle cosim_handle, bit [4:0] write_reg,
-  bit [31:0] write_reg_data, bit [31:0] pc, bit sync_trap);
+  bit [31:0] write_reg_data, bit [31:0] pc, bit sync_trap, bit suppress_reg_write);
 import "DPI-C" function void riscv_cosim_set_mip(chandle cosim_handle, bit [31:0] mip);
 import "DPI-C" function void riscv_cosim_set_nmi(chandle cosim_handle, bit nmi);
+import "DPI-C" function void riscv_cosim_set_nmi_int(chandle cosim_handle, bit nmi_int);
 import "DPI-C" function void riscv_cosim_set_debug_req(chandle cosim_handle, bit debug_req);
 import "DPI-C" function void riscv_cosim_set_mcycle(chandle cosim_handle, bit [63:0] mcycle);
 import "DPI-C" function void riscv_cosim_set_csr(chandle cosim_handle, int csr_id,

dv/cosim/spike_cosim.cc

@@ -160,9 +160,8 @@ bool SpikeCosim::backdoor_read_mem(uint32_t addr, size_t len,
 // - If we catch a trap_t&, then the take_trap() fn updates the state of the
 //   processor, and when we call step() again we start executing in the new
 //   context of the trap (trap andler, new MSTATUS, debug rom, etc. etc.)
-bool SpikeCosim::step(uint32_t write_reg, uint32_t write_reg_data,
-                      uint32_t pc,
-                      bool sync_trap) {
+bool SpikeCosim::step(uint32_t write_reg, uint32_t write_reg_data, uint32_t pc,
+                      bool sync_trap, bool suppress_reg_write) {
   assert(write_reg < 32);
 
   // The DUT has just produced an RVFI item
@@ -180,8 +179,28 @@ bool SpikeCosim::step(uint32_t write_reg, uint32_t write_reg_data,
   }
 
   uint32_t initial_spike_pc;
+  uint32_t suppressed_write_reg;
+  uint32_t suppressed_write_reg_data;
   bool pending_sync_exception = false;
 
+  if (suppress_reg_write) {
+    // If Ibex suppressed a register write (which occurs when a load gets data
+    // with bad integrity) record the state of the destination register before
+    // we do the stop, so we can restore it after the step (as spike won't
+    // suppressed the register write).
+    //
+    // First check retired instruciton to ensure load suppression is correct
+    if (!check_suppress_reg_write(write_reg, pc, suppressed_write_reg)) {
+      return false;
+    }
+
+    // The check gives us the destination register the instruction would have
+    // written to (write_reg will be 0 to indicate to write). Record the
+    // contents of that register.
+    suppressed_write_reg_data =
+        processor->get_state()->XPR[suppressed_write_reg];
+  }
+
   // Before stepping Spike, record the current spike pc.
   // (If the current step causes a synchronous trap, it will be
   //  recorded against the current pc)
@@ -211,7 +230,8 @@ bool SpikeCosim::step(uint32_t write_reg, uint32_t write_reg_data,
 
   if (processor->get_state()->last_inst_pc == PC_INVALID) {
     if (!(processor->get_state()->mcause->read() & 0x80000000) ||
-        processor->get_state()->debug_mode) { // (Async-Traps are disabled in debug mode)
+        processor->get_state()
+            ->debug_mode) {  // (Async-Traps are disabled in debug mode)
       // Spike encountered a synchronous trap
       pending_sync_exception = true;
 
@@ -275,7 +295,13 @@ bool SpikeCosim::step(uint32_t write_reg, uint32_t write_reg_data,
   }
   pending_iside_error = false;
 
-  if (!check_retired_instr(write_reg, write_reg_data, pc)) {
+  if (suppress_reg_write) {
+    // If we suppressed a register write restore the old register state now
+    processor->get_state()->XPR.write(suppressed_write_reg,
+                                      suppressed_write_reg_data);
+  }
+
+  if (!check_retired_instr(write_reg, write_reg_data, pc, suppress_reg_write)) {
     return false;
   }
 
@@ -284,8 +310,9 @@ bool SpikeCosim::step(uint32_t write_reg, uint32_t write_reg_data,
   return true;
 }
 
-bool SpikeCosim::check_retired_instr(uint32_t write_reg, uint32_t write_reg_data,
-                                     uint32_t dut_pc) {
+bool SpikeCosim::check_retired_instr(uint32_t write_reg,
+                                     uint32_t write_reg_data, uint32_t dut_pc,
+                                     bool suppress_reg_write) {
   // Check the retired instruction and all of its side-effects match those from
   // the DUT
 
@@ -319,7 +346,8 @@ bool SpikeCosim::check_retired_instr(uint32_t write_reg, uint32_t write_reg_data
       // should never see more than one GPR write per step
       assert(!gpr_write_seen);
 
-      if (!check_gpr_write(reg_change, write_reg, write_reg_data)) {
+      if (!suppress_reg_write &&
+          !check_gpr_write(reg_change, write_reg, write_reg_data)) {
         return false;
       }
 
@@ -376,6 +404,12 @@ bool SpikeCosim::check_sync_trap(uint32_t write_reg,
     return false;
   }
 
+  // If we see an internal NMI, that means we receive an extra memory intf item.
+  // Deleting that is necessary since next Load/Store would fail otherwise.
+  if (processor->get_state()->mcause->read() == 0xFFFFFFE0) {
+    pending_dside_accesses.erase(pending_dside_accesses.begin());
+  }
+
   // Errors may have been generated outside of step() (e.g. in
   // check_mem_access()), return false if there are any.
   if (errors.size() != 0) {
@@ -425,6 +459,31 @@ bool SpikeCosim::check_gpr_write(const commit_log_reg_t::value_type &reg_change,
   return true;
 }
 
+bool SpikeCosim::check_suppress_reg_write(uint32_t write_reg, uint32_t pc,
+                                          uint32_t &suppressed_write_reg) {
+  if (write_reg != 0) {
+    std::stringstream err_str;
+    err_str << "Instruction at " << std::hex << pc
+            << " indicated a suppressed register write but wrote to x"
+            << std::dec << write_reg;
+    errors.emplace_back(err_str.str());
+
+    return false;
+  }
+
+  if (!pc_is_load(pc, suppressed_write_reg)) {
+    std::stringstream err_str;
+    err_str << "Instruction at " << std::hex << pc
+            << " indicated a suppressed register write is it not a load"
+               " only loads can suppress register writes";
+    errors.emplace_back(err_str.str());
+
+    return false;
+  }
+
+  return true;
+}
+
 void SpikeCosim::on_csr_write(const commit_log_reg_t::value_type &reg_change) {
   int cosim_write_csr = (reg_change.first >> 4) & 0xfff;
 
@@ -528,6 +587,20 @@ void SpikeCosim::set_nmi(bool nmi) {
   }
 }
 
+void SpikeCosim::set_nmi_int(bool nmi_int) {
+  if (nmi_int && !nmi_mode && !processor->get_state()->debug_mode) {
+    processor->get_state()->nmi_int = true;
+    nmi_mode = true;
+
+    // When NMI is set it is guaranteed NMI trap will be taken at the next step
+    // so save CSR state for recoverable NMI to mstack now.
+    mstack.mpp = get_field(processor->get_csr(CSR_MSTATUS), MSTATUS_MPP);
+    mstack.mpie = get_field(processor->get_csr(CSR_MSTATUS), MSTATUS_MPIE);
+    mstack.epc = processor->get_csr(CSR_MEPC);
+    mstack.cause = processor->get_csr(CSR_MCAUSE);
+  }
+}
+
 void SpikeCosim::set_debug_req(bool debug_req) {
   processor->halt_request =
       debug_req ? processor_t::HR_REGULAR : processor_t::HR_NONE;
@@ -892,4 +965,44 @@ bool SpikeCosim::check_debug_ebreak(uint32_t write_reg, uint32_t pc,
   return true;
 }
 
+bool SpikeCosim::pc_is_load(uint32_t pc, uint32_t &rd_out) {
+  uint16_t insn_16;
+
+  if (!backdoor_read_mem(pc, 2, reinterpret_cast<uint8_t *>(&insn_16))) {
+    return false;
+  }
+
+  // C.LW
+  if ((insn_16 & 0xE003) == 0x4000) {
+    rd_out = ((insn_16 >> 2) & 0x7) + 8;
+    return true;
+  }
+
+  // C.LWSP
+  if ((insn_16 & 0xE003) == 0x4002) {
+    rd_out = (insn_16 >> 7) & 0x1F;
+    return rd_out != 0;
+  }
+
+  uint16_t insn_32;
+
+  if (!backdoor_read_mem(pc, 4, reinterpret_cast<uint8_t *>(&insn_32))) {
+    return false;
+  }
+
+  // LB/LH/LW/LBU/LHU
+  if ((insn_32 & 0x7F) == 0x3) {
+    uint32_t func = (insn_32 >> 12) & 0x7;
+    if ((func == 0x3) || (func == 0x6) || (func == 0x7)) {
+      // Not valid load encodings
+      return false;
+    }
+
+    rd_out = (insn_32 >> 7) & 0x1F;
+    return true;
+  }
+
+  return false;
+}
+
 unsigned int SpikeCosim::get_insn_cnt() { return insn_cnt; }

dv/cosim/spike_cosim.h

@@ -61,13 +61,17 @@ class SpikeCosim : public simif_t, public Cosim {
                                       const uint8_t *bytes);
 
   bool pc_is_mret(uint32_t pc);
+  bool pc_is_load(uint32_t pc, uint32_t &rd_out);
 
   bool pc_is_debug_ebreak(uint32_t pc);
   bool check_debug_ebreak(uint32_t write_reg, uint32_t pc, bool sync_trap);
 
   bool check_gpr_write(const commit_log_reg_t::value_type &reg_change,
                        uint32_t write_reg, uint32_t write_reg_data);
 
+  bool check_suppress_reg_write(uint32_t write_reg, uint32_t pc,
+                                uint32_t &suppressed_write_reg);
+
   void on_csr_write(const commit_log_reg_t::value_type &reg_change);
 
   void leave_nmi_mode();
@@ -101,14 +105,15 @@ class SpikeCosim : public simif_t, public Cosim {
                           const uint8_t *data_in) override;
   bool backdoor_read_mem(uint32_t addr, size_t len, uint8_t *data_out) override;
   bool step(uint32_t write_reg, uint32_t write_reg_data, uint32_t pc,
-            bool sync_trap) override;
+            bool sync_trap, bool suppress_reg_write) override;
 
   bool check_retired_instr(uint32_t write_reg, uint32_t write_reg_data,
-                           uint32_t pc);
+                           uint32_t dut_pc, bool suppress_reg_write);
   bool check_sync_trap(uint32_t write_reg, uint32_t pc,
                        uint32_t initial_spike_pc);
   void set_mip(uint32_t mip) override;
   void set_nmi(bool nmi) override;
+  void set_nmi_int(bool nmi_int) override;
   void set_debug_req(bool debug_req) override;
   void set_mcycle(uint64_t mcycle) override;
   void set_csr(const int csr_num, const uint32_t new_val) override;

dv/uvm/core_ibex/common/ibex_cosim_agent/ibex_cosim_scoreboard.sv

@@ -121,6 +121,7 @@ class ibex_cosim_scoreboard extends uvm_scoreboard;
       end
 
       riscv_cosim_set_nmi(cosim_handle, rvfi_instr.nmi);
+      riscv_cosim_set_nmi_int(cosim_handle, rvfi_instr.nmi_int);
       riscv_cosim_set_mip(cosim_handle, rvfi_instr.mip);
       riscv_cosim_set_debug_req(cosim_handle, rvfi_instr.debug_req);
       riscv_cosim_set_mcycle(cosim_handle, rvfi_instr.mcycle);
@@ -134,7 +135,7 @@ class ibex_cosim_scoreboard extends uvm_scoreboard;
       riscv_cosim_set_ic_scr_key_valid(cosim_handle, rvfi_instr.ic_scr_key_valid);
 
       if (!riscv_cosim_step(cosim_handle, rvfi_instr.rd_addr, rvfi_instr.rd_wdata, rvfi_instr.pc,
-                            rvfi_instr.trap)) begin
+                            rvfi_instr.trap, rvfi_instr.rf_wr_suppress)) begin
         // cosim instruction step doesn't match rvfi captured instruction, report a fatal error
         // with the details
         if (cfg.relax_cosim_check) begin

dv/uvm/core_ibex/common/ibex_cosim_agent/ibex_rvfi_monitor.sv

@@ -38,7 +38,9 @@ class ibex_rvfi_monitor extends uvm_monitor;
       trans_collected.order            = vif.monitor_cb.order;
       trans_collected.mip              = vif.monitor_cb.ext_mip;
       trans_collected.nmi              = vif.monitor_cb.ext_nmi;
+      trans_collected.nmi_int          = vif.monitor_cb.ext_nmi_int;
       trans_collected.debug_req        = vif.monitor_cb.ext_debug_req;
+      trans_collected.rf_wr_suppress   = vif.monitor_cb.ext_rf_wr_suppress;
       trans_collected.mcycle           = vif.monitor_cb.ext_mcycle;
       trans_collected.ic_scr_key_valid = vif.monitor_cb.ext_ic_scr_key_valid;
 

dv/uvm/core_ibex/common/ibex_cosim_agent/ibex_rvfi_seq_item.sv

@@ -10,7 +10,9 @@ class ibex_rvfi_seq_item extends uvm_sequence_item;
   bit [63:0] order;
   bit [31:0] mip;
   bit        nmi;
+  bit        nmi_int;
   bit        debug_req;
+  bit        rf_wr_suppress;
   bit [63:0] mcycle;
 
   bit [31:0] mhpmcounters  [10];
@@ -25,7 +27,9 @@ class ibex_rvfi_seq_item extends uvm_sequence_item;
     `uvm_field_int (order, UVM_DEFAULT)
     `uvm_field_int (mip, UVM_DEFAULT)
     `uvm_field_int (nmi, UVM_DEFAULT)
+    `uvm_field_int (nmi_int, UVM_DEFAULT)
     `uvm_field_int (debug_req, UVM_DEFAULT)
+    `uvm_field_int (rf_wr_suppress, UVM_DEFAULT)
     `uvm_field_int (mcycle, UVM_DEFAULT)
     `uvm_field_sarray_int (mhpmcounters, UVM_DEFAULT)
     `uvm_field_sarray_int (mhpmcountersh, UVM_DEFAULT)

dv/uvm/core_ibex/common/ibex_mem_intf_agent/ibex_mem_intf_response_driver.sv

@@ -129,11 +129,10 @@ class ibex_mem_intf_response_driver extends uvm_driver #(ibex_mem_intf_seq_item)
         end else begin
           // rdata and intg fields aren't relevant to write responses
           if (cfg.fixed_data_write_response) begin
-            // when fixed_data_write_response is set, set rdata to fixed value with correct matching
-            // rintg field.
-            cfg.vif.response_driver_cb.rdata <= 32'hffffffff;
-            cfg.vif.response_driver_cb.rintg <=
-              prim_secded_pkg::prim_secded_inv_39_32_enc(32'hffffffff)[38:32];
+            // when fixed_data_write_response is set, sequence item is responsible for producing
+            // fixed values so just copy them across here.
+            cfg.vif.response_driver_cb.rdata <= tr.data;
+            cfg.vif.response_driver_cb.rintg <= tr.intg;
           end else begin
             // when fixed_data_write_response is not set, drive the irrelevant fields to x.
             cfg.vif.response_driver_cb.rdata <= 'x;