[cosim] Add write suppress support

When Ibex does a load that receives data with bad integrity it
suppresses the write to the destination register. The implements
matching functionality for cosim.

Author: GregAC

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

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) {

dv/cosim/cosim_dpi.h

@@ -14,7 +14,7 @@
 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);

dv/cosim/cosim_dpi.svh

@@ -11,7 +11,7 @@
 `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);

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)
@@ -276,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;
   }
 
@@ -285,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
 
@@ -320,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;
       }
 
@@ -432,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;
 
@@ -913,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,10 +105,10 @@ 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;

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

@@ -135,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

@@ -40,6 +40,7 @@ class ibex_rvfi_monitor extends uvm_monitor;
       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

@@ -12,6 +12,7 @@ class ibex_rvfi_seq_item extends uvm_sequence_item;
   bit        nmi;
   bit        nmi_int;
   bit        debug_req;
+  bit        rf_wr_suppress;
   bit [63:0] mcycle;
 
   bit [31:0] mhpmcounters  [10];
@@ -28,6 +29,7 @@ class ibex_rvfi_seq_item extends uvm_sequence_item;
     `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/env/core_ibex_rvfi_if.sv

@@ -30,6 +30,7 @@ interface core_ibex_rvfi_if(input logic clk);
   logic        ext_nmi;
   logic        ext_nmi_int;
   logic [31:0] ext_debug_req;
+  logic [31:0] ext_rf_wr_suppress;
   logic [63:0] ext_mcycle;
 
   logic [31:0] ext_mhpmcounters [10];
@@ -64,6 +65,7 @@ interface core_ibex_rvfi_if(input logic clk);
     input ext_nmi;
     input ext_nmi_int;
     input ext_debug_req;
+    input ext_rf_wr_suppress;
     input ext_mcycle;
     input ext_mhpmcounters;
     input ext_mhpmcountersh;

dv/uvm/core_ibex/tb/core_ibex_tb_top.sv

@@ -194,6 +194,7 @@ module core_ibex_tb_top;
   assign rvfi_if.ext_nmi              = dut.rvfi_ext_nmi;
   assign rvfi_if.ext_nmi_int          = dut.rvfi_ext_nmi_int;
   assign rvfi_if.ext_debug_req        = dut.rvfi_ext_debug_req;
+  assign rvfi_if.ext_rf_wr_suppress   = dut.rvfi_ext_rf_wr_suppress;
   assign rvfi_if.ext_mcycle           = dut.rvfi_ext_mcycle;
   assign rvfi_if.ext_mhpmcounters     = dut.rvfi_ext_mhpmcounters;
   assign rvfi_if.ext_mhpmcountersh    = dut.rvfi_ext_mhpmcountersh;

dv/verilator/simple_system_cosim/ibex_simple_system_cosim_checker.sv

@@ -56,7 +56,8 @@ module ibex_simple_system_cosim_checker #(
       riscv_cosim_set_ic_scr_key_valid(cosim_handle, u_top.rvfi_ext_ic_scr_key_valid);
 
       if (riscv_cosim_step(cosim_handle, u_top.rvfi_rd_addr, u_top.rvfi_rd_wdata,
-                           u_top.rvfi_pc_rdata, u_top.rvfi_trap) == 0)
+                           u_top.rvfi_pc_rdata, u_top.rvfi_trap,
+                           u_top.rvfi_ext_rf_wr_suppress) == 0)
       begin
         $display("FAILURE: Co-simulation mismatch at time %t", $time());
         for (int i = 0;i < riscv_cosim_get_num_errors(cosim_handle); ++i) begin

rtl/ibex_core.sv

@@ -141,6 +141,7 @@ module ibex_core import ibex_pkg::*; #(
   output logic                         rvfi_ext_nmi,
   output logic                         rvfi_ext_nmi_int,
   output logic                         rvfi_ext_debug_req,
+  output logic                         rvfi_ext_rf_wr_suppress,
   output logic [63:0]                  rvfi_ext_mcycle,
   output logic [31:0]                  rvfi_ext_mhpmcounters [10],
   output logic [31:0]                  rvfi_ext_mhpmcountersh [10],
@@ -1348,6 +1349,12 @@ module ibex_core import ibex_pkg::*; #(
     assign rvfi_trap_wb = id_stage_i.controller_i.exc_req_lsu;
     // WB is instantly done in the tracking pipeline when a trap is progress through the pipeline
     assign rvfi_wb_done = rvfi_stage_valid[0] & (instr_done_wb | rvfi_stage_trap[0]);
+
+
+    logic rvfi_stage_rf_wr_suppress_wb_q;
+    always_ff @(posedge clk_i or negedge rst_ni) begin
+
+    end
   end else begin : gen_rvfi_no_wb_stage
     // Without writeback stage first RVFI stage is output stage so simply valid the cycle after
     // instruction leaves ID/EX (and so has retired)
@@ -1676,6 +1683,27 @@ module ibex_core import ibex_pkg::*; #(
     end
   end
 
+  if (WritebackStage) begin : g_rvfi_rf_wr_suppress_wb
+    logic rvfi_stage_rf_wr_suppress_wb;
+    logic rvfi_rf_wr_suppress_wb;
+
+    // Set when RF write from load data is suppressed due to an integrity error
+    assign rvfi_rf_wr_suppress_wb =
+      instr_done_wb & ~rf_we_wb_o & outstanding_load_wb & lsu_load_resp_intg_err;
+
+    always@(posedge clk_i or negedge rst_ni) begin
+      if (!rst_ni) begin
+        rvfi_stage_rf_wr_suppress_wb <= 1'b0;
+      end else if (rvfi_wb_done) begin
+        rvfi_stage_rf_wr_suppress_wb <= rvfi_rf_wr_suppress_wb;
+      end
+    end
+
+    assign rvfi_ext_rf_wr_suppress = rvfi_stage_rf_wr_suppress_wb;
+  end else begin : g_rvfi_no_rf_wr_suppress_wb
+    assign rvfi_ext_rf_wr_suppress = 1'b0;
+  end
+
   // rvfi_intr must be set for first instruction that is part of a trap handler.
   // On the first cycle of a new instruction see if a trap PC was set by the previous instruction,
   // otherwise maintain value.

rtl/ibex_lockstep.sv

@@ -431,6 +431,7 @@ module ibex_lockstep import ibex_pkg::*; #(
     .rvfi_ext_nmi              (),
     .rvfi_ext_nmi_int          (),
     .rvfi_ext_debug_req        (),
+    .rvfi_ext_rf_wr_suppress   (),
     .rvfi_ext_mcycle           (),
     .rvfi_ext_mhpmcounters     (),
     .rvfi_ext_mhpmcountersh    (),

rtl/ibex_top.sv

@@ -120,6 +120,7 @@ module ibex_top import ibex_pkg::*; #(
   output logic                         rvfi_ext_nmi,
   output logic                         rvfi_ext_nmi_int,
   output logic                         rvfi_ext_debug_req,
+  output logic                         rvfi_ext_rf_wr_suppress,
   output logic [63:0]                  rvfi_ext_mcycle,
   output logic [31:0]                  rvfi_ext_mhpmcounters [10],
   output logic [31:0]                  rvfi_ext_mhpmcountersh [10],
@@ -390,6 +391,7 @@ module ibex_top import ibex_pkg::*; #(
     .rvfi_ext_nmi,
     .rvfi_ext_nmi_int,
     .rvfi_ext_debug_req,
+    .rvfi_ext_rf_wr_suppress,
     .rvfi_ext_mcycle,
     .rvfi_ext_mhpmcounters,
     .rvfi_ext_mhpmcountersh,