From ec0b0341b7b18f8699fc8e27e1ea6571fb25ae3e Mon Sep 17 00:00:00 2001 From: Canberk Topal Date: Tue, 20 Sep 2022 12:23:03 +0100 Subject: [PATCH 01/11] [ibex,dv] Switch to new sequence for memory errors Signed-off-by: Canberk Topal --- .../core_ibex/tests/core_ibex_new_seq_lib.sv | 8 +- dv/uvm/core_ibex/tests/core_ibex_test_lib.sv | 104 ++++-------------- 2 files changed, 23 insertions(+), 89 deletions(-) diff --git a/dv/uvm/core_ibex/tests/core_ibex_new_seq_lib.sv b/dv/uvm/core_ibex/tests/core_ibex_new_seq_lib.sv index b33693788d..18c47e200c 100644 --- a/dv/uvm/core_ibex/tests/core_ibex_new_seq_lib.sv +++ b/dv/uvm/core_ibex/tests/core_ibex_new_seq_lib.sv @@ -72,6 +72,7 @@ class core_base_new_seq #(type REQ = uvm_sequence_item) extends uvm_sequence #(R `DV_CHECK_FATAL(iteration_cnt != 0) `uvm_info(`gfn, $sformatf("Number of stimulus iterations = %0d", iteration_cnt), UVM_LOW) for (int i = 0; i <= iteration_cnt; i++) begin + `uvm_info(`gfn, $sformatf("Running %0d/%0d", i, iteration_cnt), UVM_LOW) drive_stimulus(); end end @@ -198,7 +199,7 @@ class debug_new_seq extends core_base_new_seq#(irq_seq_item); endclass -class memory_error_seq extends core_base_new_seq#(irq_seq_item); +class memory_error_seq extends core_base_new_seq#(ibex_mem_intf_seq_item); core_ibex_vseq vseq; rand bit choose_side; bit start_seq = 0; // Use this bit to start any unique sequence once @@ -210,7 +211,6 @@ class memory_error_seq extends core_base_new_seq#(irq_seq_item); function new (string name = ""); super.new(name); - vseq = core_ibex_vseq::type_id::create("vseq"); endfunction virtual task send_req(); @@ -233,10 +233,6 @@ class memory_error_seq extends core_base_new_seq#(irq_seq_item); // DO nothing end endcase - if(!start_seq) begin - vseq.start(p_sequencer); - start_seq = 1; - end endtask endclass: memory_error_seq diff --git a/dv/uvm/core_ibex/tests/core_ibex_test_lib.sv b/dv/uvm/core_ibex/tests/core_ibex_test_lib.sv index ea67b099ae..b7f03f5cf8 100644 --- a/dv/uvm/core_ibex/tests/core_ibex_test_lib.sv +++ b/dv/uvm/core_ibex/tests/core_ibex_test_lib.sv @@ -1630,93 +1630,31 @@ class core_ibex_mem_error_test extends core_ibex_directed_test; `uvm_component_utils(core_ibex_mem_error_test) `uvm_component_new - int err_delay; - - // check memory error inputs and verify that core jumps to correct exception handler virtual task check_stimulus(); - forever begin - while (!vseq.data_intf_seq.get_error_synch()) begin - clk_vif.wait_clks(1); + memory_error_seq memory_error_seq_h; + memory_error_seq_h = memory_error_seq::type_id::create("memory_error_seq_h", this); + + `uvm_info(`gfn, "Running core_ibex_mem_error_test", UVM_LOW) + memory_error_seq_h.vseq = vseq; + memory_error_seq_h.iteration_modes = InfiniteRuns; + memory_error_seq_h.stimulus_delay_cycles_min = 800; // Interval between injected errors + memory_error_seq_h.stimulus_delay_cycles_max = 5000; + fork + begin + forever begin + memory_error_seq_h.start(env.vseqr); + // Wait until we are out of IRQ handler to the inject errors + wait_ret("mret", 20000); + end end - vseq.data_intf_seq.inject_error(); - `uvm_info(`gfn, "Injected dmem error", UVM_LOW) - // Dmem interface error could be either a load or store operation - check_dmem_fault(); - // Random delay before injecting instruction fetch fault - `DV_CHECK_STD_RANDOMIZE_WITH_FATAL(err_delay, err_delay inside { [50:200] };) - clk_vif.wait_clks(err_delay); - inject_imem_error(); - check_imem_fault(); - // Random delay before injecting this series of errors again - `DV_CHECK_STD_RANDOMIZE_WITH_FATAL(err_delay, err_delay inside { [250:750] };) - clk_vif.wait_clks(err_delay); - end - endtask - - virtual task inject_imem_error(); - while (!vseq.instr_intf_seq.get_error_synch()) begin - clk_vif.wait_clks(1); - end - `uvm_info(`gfn, "Injecting imem fault", UVM_LOW) - vseq.instr_intf_seq.inject_error(); - endtask - - virtual task check_dmem_fault(); - bit[ibex_mem_intf_agent_pkg::DATA_WIDTH-1:0] mcause; - core_status_t mem_status; - ibex_pkg::exc_cause_t exc_type; - // Don't impose a timeout period for dmem check, since dmem errors injected by the sequence are - // not guaranteed to be reflected in RTL state until the next memory instruction is executed, - // and the frequency of which is not controllable by the testbench - check_next_core_status(HANDLING_EXCEPTION, "Core did not jump to exception handler"); - check_priv_mode(PRIV_LVL_M); - // Next write of CORE_STATUS will be the load/store fault type - wait_for_mem_txn(cfg.signature_addr, CORE_STATUS); - mem_status = core_status_t'(signature_data_q.pop_front()); - if (mem_status == LOAD_FAULT_EXCEPTION) begin - exc_type = ExcCauseLoadAccessFault; - end else if (mem_status == STORE_FAULT_EXCEPTION) begin - exc_type = ExcCauseStoreAccessFault; - end - check_mcause(1'b0, exc_type.lower_cause); - wait (dut_vif.dut_cb.mret === 1'b1); - `uvm_info(`gfn, "exiting mem fault checker", UVM_LOW) - endtask - - virtual task check_imem_fault(); - bit latched_imem_err = 1'b0; - core_status_t mem_status; - ibex_pkg::exc_cause_t exc_type; - // Need to account for case where imem_error is asserted during an instruction fetch that gets - // killed - due to jumps and control flow changes - do begin - fork - begin - fork : imem_fork - begin - check_next_core_status(HANDLING_EXCEPTION, "Core did not jump to exception handler"); - check_priv_mode(PRIV_LVL_M); - latched_imem_err = 1'b1; - `uvm_info(`gfn, $sformatf("latched_imem_err: 0x%0x", latched_imem_err), UVM_LOW) - end - begin - clk_vif.wait_clks(5000); - end - join_any - disable fork; + begin + forever begin + wait_for_core_status(HANDLING_IRQ); + // Do not allow error injection while we are handling IRQ + memory_error_seq_h.stop(); end - join - if (latched_imem_err === 1'b0) begin - cur_run_phase.drop_objection(this); - inject_imem_error(); end - end while (latched_imem_err === 1'b0); - check_next_core_status(INSTR_FAULT_EXCEPTION, - "Core did not register correct memory fault type", 5000); - exc_type = ExcCauseInstrAccessFault; - check_mcause(1'b0, exc_type.lower_cause); - wait (dut_vif.dut_cb.mret === 1'b1); - `uvm_info(`gfn, "exiting mem fault checker", UVM_LOW) + join_none endtask endclass From 931b35884b891feef9f3c724ba70ef2808c1d9d8 Mon Sep 17 00:00:00 2001 From: Canberk Topal Date: Sun, 16 Oct 2022 03:30:03 +0100 Subject: [PATCH 02/11] [dv] Don't inject mem errors at test_control_addr This will prevent seeing mismatches right at the end of our test. Before this change, mem_error_test could inject error at the store instruction in which we finish up the test, resulting with mismatches with Spike and Ibex on the instructions after finishing. Also do the same prevention for signature_addr as well, since we also don't want to corrupt that memory transaction too. Signed-off-by: Canberk Topal --- .../ibex_mem_intf_agent/ibex_mem_intf_response_seq_lib.sv | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/dv/uvm/core_ibex/common/ibex_mem_intf_agent/ibex_mem_intf_response_seq_lib.sv b/dv/uvm/core_ibex/common/ibex_mem_intf_agent/ibex_mem_intf_response_seq_lib.sv index 47793a8e51..b26115927f 100644 --- a/dv/uvm/core_ibex/common/ibex_mem_intf_agent/ibex_mem_intf_response_seq_lib.sv +++ b/dv/uvm/core_ibex/common/ibex_mem_intf_agent/ibex_mem_intf_response_seq_lib.sv @@ -61,7 +61,12 @@ class ibex_mem_intf_response_seq extends uvm_sequence #(ibex_mem_intf_seq_item); end error_synch = 1'b1; enable_error = 1'b0; // Disable after single inserted error. - + aligned_addr = {req.addr[DATA_WIDTH-1:2], 2'b0}; + // Do not inject any error to the handshake test_control_addr + // TODO: Parametrize this. Until then, this needs to be changed manually. + if (aligned_addr inside {32'h8ffffff8, 32'h8ffffffc}) begin + req.error = 1'b0; + end if (req.error) begin `DV_CHECK_STD_RANDOMIZE_FATAL(rand_data) req.data = rand_data; From 3c12971d6750dd1c70fdf93fa37dc28dd43764d6 Mon Sep 17 00:00:00 2001 From: Canberk Topal Date: Thu, 20 Oct 2022 17:45:43 +0100 Subject: [PATCH 03/11] Fix-up new_seq_lib to start after stopping Signed-off-by: Canberk Topal --- dv/uvm/core_ibex/tests/core_ibex_new_seq_lib.sv | 2 ++ 1 file changed, 2 insertions(+) diff --git a/dv/uvm/core_ibex/tests/core_ibex_new_seq_lib.sv b/dv/uvm/core_ibex/tests/core_ibex_new_seq_lib.sv index 18c47e200c..c06be8b9fb 100644 --- a/dv/uvm/core_ibex/tests/core_ibex_new_seq_lib.sv +++ b/dv/uvm/core_ibex/tests/core_ibex_new_seq_lib.sv @@ -63,6 +63,8 @@ class core_base_new_seq #(type REQ = uvm_sequence_item) extends uvm_sequence #(R `uvm_info(`gfn, $sformatf("Running the \"%s\" schedule for stimulus generation", iteration_modes.name()), UVM_LOW) + stop_seq = 1'b0; + seq_finished = 1'b0; case (iteration_modes) SingleRun: begin drive_stimulus(); From ec36d5d4d364cf0eea6326ead76c3f4b6847454d Mon Sep 17 00:00:00 2001 From: Greg Chadwick Date: Wed, 2 Nov 2022 11:26:12 +0000 Subject: [PATCH 04/11] [dv] Set rdata on write response in sequence not driver This enables the sequence to corrupt the integrity on write responses should it choose to. Previously the driver would always produce correct integrity. --- .../ibex_mem_intf_agent/ibex_mem_intf_response_driver.sv | 9 ++++----- .../ibex_mem_intf_response_seq_lib.sv | 5 +++++ 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/dv/uvm/core_ibex/common/ibex_mem_intf_agent/ibex_mem_intf_response_driver.sv b/dv/uvm/core_ibex/common/ibex_mem_intf_agent/ibex_mem_intf_response_driver.sv index 0a349a5c8c..b907a74419 100644 --- a/dv/uvm/core_ibex/common/ibex_mem_intf_agent/ibex_mem_intf_response_driver.sv +++ b/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; diff --git a/dv/uvm/core_ibex/common/ibex_mem_intf_agent/ibex_mem_intf_response_seq_lib.sv b/dv/uvm/core_ibex/common/ibex_mem_intf_agent/ibex_mem_intf_response_seq_lib.sv index b26115927f..95422300d4 100644 --- a/dv/uvm/core_ibex/common/ibex_mem_intf_agent/ibex_mem_intf_response_seq_lib.sv +++ b/dv/uvm/core_ibex/common/ibex_mem_intf_agent/ibex_mem_intf_response_seq_lib.sv @@ -76,6 +76,11 @@ class ibex_mem_intf_response_seq extends uvm_sequence #(ibex_mem_intf_seq_item); end else if(item.read_write == WRITE) begin // Update memory_model write(aligned_addr, item.data); + if (p_sequencer.cfg.fixed_data_write_response) begin + // When fixed_data_write_response is set drive data in store response to fixed + // 32'hffffffff value. Integrity is calculated below. + req.data = 32'hffffffff; + end end // Add integrity bits {req.intg, req.data} = prim_secded_pkg::prim_secded_inv_39_32_enc(req.data); From cd641e2e976fdf3b2da689e3fd60eaeb51b8130c Mon Sep 17 00:00:00 2001 From: Greg Chadwick Date: Wed, 2 Nov 2022 12:00:01 +0000 Subject: [PATCH 05/11] [dv] Add custom test done functionality This enables tests to implement custom test completion handling --- dv/uvm/core_ibex/tests/core_ibex_base_test.sv | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/dv/uvm/core_ibex/tests/core_ibex_base_test.sv b/dv/uvm/core_ibex/tests/core_ibex_base_test.sv index 3d24ff466a..d75af5ef0e 100644 --- a/dv/uvm/core_ibex/tests/core_ibex_base_test.sv +++ b/dv/uvm/core_ibex/tests/core_ibex_base_test.sv @@ -323,6 +323,9 @@ class core_ibex_base_test extends uvm_test; `uvm_fatal(`gfn, $sformatf("Test failed due to wall-clock timeout. [%0ds]", timeout_seconds)) end + begin + wait_for_custom_test_done(); + end join_any test_done = 1'b1; @@ -454,4 +457,8 @@ class core_ibex_base_test extends uvm_test; end while (signature_data != core_status); endtask + virtual task wait_for_custom_test_done(); + wait (test_done == 1'b1); + endtask + endclass From 85d3072a0ad5b1b71547fc783690c0e3498e5e1b Mon Sep 17 00:00:00 2001 From: Greg Chadwick Date: Wed, 2 Nov 2022 13:42:58 +0000 Subject: [PATCH 06/11] [dv] Add probe and wait functionality for traps --- dv/uvm/core_ibex/env/core_ibex_dut_probe_if.sv | 18 ++++++++++++++++++ dv/uvm/core_ibex/tb/core_ibex_tb_top.sv | 3 +++ dv/uvm/core_ibex/tests/core_ibex_base_test.sv | 6 ++++++ 3 files changed, 27 insertions(+) diff --git a/dv/uvm/core_ibex/env/core_ibex_dut_probe_if.sv b/dv/uvm/core_ibex/env/core_ibex_dut_probe_if.sv index fdf40cce98..9e088daf11 100644 --- a/dv/uvm/core_ibex/env/core_ibex_dut_probe_if.sv +++ b/dv/uvm/core_ibex/env/core_ibex_dut_probe_if.sv @@ -34,6 +34,22 @@ interface core_ibex_dut_probe_if(input logic clk); logic rf_ren_b; logic rf_rd_a_wb_match; logic rf_rd_b_wb_match; + logic sync_exc_seen; + logic irq_exc_seen; + logic csr_save_cause; + ibex_pkg::exc_cause_t exc_cause; + + always @(posedge clk or posedge reset) begin + if (reset) begin + irq_exc_seen <= 1'b0; + end else begin + if (ctrl_fsm_cs == ibex_pkg::IRQ_TAKEN) begin + irq_exc_seen <= 1'b1; + end else if (mret) begin + irq_exc_seen <= 1'b0; + end + end + end clocking dut_cb @(posedge clk); output fetch_enable; @@ -63,6 +79,8 @@ interface core_ibex_dut_probe_if(input logic clk); input rf_ren_b; input rf_rd_a_wb_match; input rf_rd_b_wb_match; + input sync_exc_seen; + input irq_exc_seen; endclocking initial begin diff --git a/dv/uvm/core_ibex/tb/core_ibex_tb_top.sv b/dv/uvm/core_ibex/tb/core_ibex_tb_top.sv index bf9745bab2..bd85b601b1 100644 --- a/dv/uvm/core_ibex/tb/core_ibex_tb_top.sv +++ b/dv/uvm/core_ibex/tb/core_ibex_tb_top.sv @@ -221,6 +221,9 @@ module core_ibex_tb_top; assign dut_if.rf_ren_b = dut.u_ibex_top.u_ibex_core.rf_ren_b; assign dut_if.rf_rd_a_wb_match = dut.u_ibex_top.u_ibex_core.rf_rd_a_wb_match; assign dut_if.rf_rd_b_wb_match = dut.u_ibex_top.u_ibex_core.rf_rd_b_wb_match; + assign dut_if.sync_exc_seen = dut.u_ibex_top.u_ibex_core.cs_registers_i.cpuctrlsts_part_q.sync_exc_seen; + assign dut_if.csr_save_cause = dut.u_ibex_top.u_ibex_core.csr_save_cause; + assign dut_if.exc_cause = dut.u_ibex_top.u_ibex_core.exc_cause; // Instruction monitor connections assign instr_monitor_if.reset = ~rst_n; assign instr_monitor_if.valid_id = dut.u_ibex_top.u_ibex_core.id_stage_i.instr_valid_i; diff --git a/dv/uvm/core_ibex/tests/core_ibex_base_test.sv b/dv/uvm/core_ibex/tests/core_ibex_base_test.sv index d75af5ef0e..cd58ffb1e3 100644 --- a/dv/uvm/core_ibex/tests/core_ibex_base_test.sv +++ b/dv/uvm/core_ibex/tests/core_ibex_base_test.sv @@ -457,6 +457,12 @@ class core_ibex_base_test extends uvm_test; end while (signature_data != core_status); endtask + virtual task wait_for_core_exception(ibex_pkg::exc_cause_t exc_cause); + wait(dut_vif.ctrl_fsm_cs == ibex_pkg::FLUSH && dut_vif.exc_cause == exc_cause && + dut_vif.csr_save_cause); + wait(dut_vif.ctrl_fsm_cs != ibex_pkg::FLUSH); + endtask + virtual task wait_for_custom_test_done(); wait (test_done == 1'b1); endtask From 69fea4033ee57ae2505cf733ce20c5d79e6dc383 Mon Sep 17 00:00:00 2001 From: Canberk Topal Date: Tue, 20 Sep 2022 12:37:14 +0100 Subject: [PATCH 07/11] [ibex,dv] Add new test for memory integrity errors This test picks between inserting an integrity error or a bus error to the response in the case of a memory request from Ibex. Introduces a control knob `enable_mem_intg_err` which can control the rate of having integrity errors per request. This commit also disables checking for double fault alerts in the scoreboard because they're expected to be seen while simulating and they don't cause infinite loop problems because every time a memory response is requested the error causing part is just randomized. That means Ibex trying to execute same instruction again would have a chance to succeed this time. Signed-off-by: Canberk Topal --- .../ibex_mem_intf_response_seq_lib.sv | 25 +++++++++- dv/uvm/core_ibex/env/core_ibex_env_cfg.sv | 3 ++ .../riscv_dv_extension/testlist.yaml | 21 ++++++++ .../core_ibex/tests/core_ibex_new_seq_lib.sv | 43 +++++++++++++--- dv/uvm/core_ibex/tests/core_ibex_test_lib.sv | 49 +++++++++++++------ 5 files changed, 118 insertions(+), 23 deletions(-) diff --git a/dv/uvm/core_ibex/common/ibex_mem_intf_agent/ibex_mem_intf_response_seq_lib.sv b/dv/uvm/core_ibex/common/ibex_mem_intf_agent/ibex_mem_intf_response_seq_lib.sv index 95422300d4..c73a53c066 100644 --- a/dv/uvm/core_ibex/common/ibex_mem_intf_agent/ibex_mem_intf_response_seq_lib.sv +++ b/dv/uvm/core_ibex/common/ibex_mem_intf_agent/ibex_mem_intf_response_seq_lib.sv @@ -11,17 +11,26 @@ class ibex_mem_intf_response_seq extends uvm_sequence #(ibex_mem_intf_seq_item); ibex_mem_intf_seq_item item; mem_model m_mem; ibex_cosim_agent cosim_agent; + bit enable_intg_error = 1'b0; bit enable_error = 1'b0; // Used to ensure that whenever inject_error() is called, the very next transaction will inject an // error, and that enable_error will not be flipped back to 0 immediately bit error_synch = 1'b1; bit is_dmem_seq = 1'b0; + bit suppress_error_on_exc = 1'b0; `uvm_object_utils(ibex_mem_intf_response_seq) `uvm_declare_p_sequencer(ibex_mem_intf_response_sequencer) `uvm_object_new virtual task body(); + virtual core_ibex_dut_probe_if ibex_dut_vif; + + if (!uvm_config_db#(virtual core_ibex_dut_probe_if)::get(null, "", "dut_if", + ibex_dut_vif)) begin + `uvm_fatal(`gfn, "failed to get ibex dut_if from uvm_config_db") + end + if (m_mem == null) `uvm_fatal(get_full_name(), "Cannot get memory model") `uvm_info(`gfn, $sformatf("is_dmem_seq: 0x%0x", is_dmem_seq), UVM_LOW) forever @@ -37,6 +46,13 @@ class ibex_mem_intf_response_seq extends uvm_sequence #(ibex_mem_intf_seq_item); req = ibex_mem_intf_seq_item::type_id::create("req"); error_synch = 1'b0; + + if (suppress_error_on_exc && + (ibex_dut_vif.dut_cb.sync_exc_seen || ibex_dut_vif.dut_cb.irq_exc_seen)) begin + enable_error = 1'b0; + enable_intg_error = 1'b0; + end + if (!req.randomize() with { addr == item.addr; read_write == item.read_write; @@ -66,6 +82,7 @@ class ibex_mem_intf_response_seq extends uvm_sequence #(ibex_mem_intf_seq_item); // TODO: Parametrize this. Until then, this needs to be changed manually. if (aligned_addr inside {32'h8ffffff8, 32'h8ffffffc}) begin req.error = 1'b0; + enable_intg_error = 1'b0; end if (req.error) begin `DV_CHECK_STD_RANDOMIZE_FATAL(rand_data) @@ -87,9 +104,9 @@ class ibex_mem_intf_response_seq extends uvm_sequence #(ibex_mem_intf_seq_item); // If data_was_uninitialized is true then we want to force bad integrity bits: invert the // correct ones, which we know will break things for the codes we use. - if (p_sequencer.cfg.enable_bad_intg_on_uninit_access && - data_was_uninitialized) begin + if ((p_sequencer.cfg.enable_bad_intg_on_uninit_access && data_was_uninitialized) || enable_intg_error) begin req.intg = ~req.intg; + enable_intg_error = 1'b0; end `uvm_info(get_full_name(), $sformatf("Response transfer:\n%0s", req.sprint()), UVM_HIGH) @@ -103,6 +120,10 @@ class ibex_mem_intf_response_seq extends uvm_sequence #(ibex_mem_intf_seq_item); this.enable_error = 1'b1; endfunction + virtual function void inject_intg_error(); + this.enable_intg_error = 1'b1; + endfunction + virtual function bit get_error_synch(); return this.error_synch; endfunction diff --git a/dv/uvm/core_ibex/env/core_ibex_env_cfg.sv b/dv/uvm/core_ibex/env/core_ibex_env_cfg.sv index a717c8a7c7..fba3b1bb49 100644 --- a/dv/uvm/core_ibex/env/core_ibex_env_cfg.sv +++ b/dv/uvm/core_ibex/env/core_ibex_env_cfg.sv @@ -7,6 +7,7 @@ class core_ibex_env_cfg extends uvm_object; virtual clk_rst_if ibex_clk_vif; virtual core_ibex_dut_probe_if ibex_dut_vif; + bit enable_mem_intg_err; bit enable_irq_single_seq; bit enable_irq_multiple_seq; bit enable_irq_nmi_seq; @@ -31,6 +32,7 @@ class core_ibex_env_cfg extends uvm_object; `uvm_object_utils_begin(core_ibex_env_cfg) `uvm_field_int(enable_double_fault_detector, UVM_DEFAULT) `uvm_field_int(is_double_fault_detected_fatal, UVM_DEFAULT) + `uvm_field_int(enable_mem_intg_err, UVM_DEFAULT) `uvm_field_int(enable_irq_single_seq, UVM_DEFAULT) `uvm_field_int(enable_irq_multiple_seq, UVM_DEFAULT) `uvm_field_int(enable_irq_nmi_seq, UVM_DEFAULT) @@ -48,6 +50,7 @@ class core_ibex_env_cfg extends uvm_object; super.new(name); void'($value$plusargs("enable_double_fault_detector=%0d", enable_double_fault_detector)); void'($value$plusargs("is_double_fault_detected_fatal=%0d", is_double_fault_detected_fatal)); + void'($value$plusargs("enable_mem_intg_err=%0d", enable_mem_intg_err)); void'($value$plusargs("enable_irq_single_seq=%0d", enable_irq_single_seq)); void'($value$plusargs("enable_irq_multiple_seq=%0d", enable_irq_multiple_seq)); void'($value$plusargs("enable_irq_nmi_seq=%0d", enable_irq_nmi_seq)); diff --git a/dv/uvm/core_ibex/riscv_dv_extension/testlist.yaml b/dv/uvm/core_ibex/riscv_dv_extension/testlist.yaml index 0a6dba3069..ea5f899236 100644 --- a/dv/uvm/core_ibex/riscv_dv_extension/testlist.yaml +++ b/dv/uvm/core_ibex/riscv_dv_extension/testlist.yaml @@ -588,6 +588,27 @@ +disable_pmp_exception_handler=1 rtl_test: core_ibex_mem_error_test sim_opts: > + +enable_mem_intg_err=0 + +enable_double_fault_detector=0 + +require_signature_addr=1 + compare_opts: + compare_final_value_only: 1 + +- test: riscv_mem_intg_error_test + description: > + Normal random instruction test, but randomly insert memory load/store integrity errors + iterations: 15 + gen_test: riscv_rand_instr_test + gen_opts: > + +require_signature_addr=1 + +instr_cnt=10000 + +randomize_csr=1 + +enable_unaligned_load_store=1 + +suppress_pmp_setup=1 + rtl_test: core_ibex_mem_error_test + sim_opts: > + +enable_mem_intg_err=1 + +enable_double_fault_detector=0 +require_signature_addr=1 compare_opts: compare_final_value_only: 1 diff --git a/dv/uvm/core_ibex/tests/core_ibex_new_seq_lib.sv b/dv/uvm/core_ibex/tests/core_ibex_new_seq_lib.sv index c06be8b9fb..8511b9f46a 100644 --- a/dv/uvm/core_ibex/tests/core_ibex_new_seq_lib.sv +++ b/dv/uvm/core_ibex/tests/core_ibex_new_seq_lib.sv @@ -204,9 +204,17 @@ endclass class memory_error_seq extends core_base_new_seq#(ibex_mem_intf_seq_item); core_ibex_vseq vseq; rand bit choose_side; - bit start_seq = 0; // Use this bit to start any unique sequence once - - rand error_type_e err_type = PickErr; + // When set skip error injection if Ibex is currently handling an exception (incluing IRQs) + bit skip_on_exc = 1'b0; + + error_type_e err_type = PickErr; + rand bit inject_intg_err; + // CONTROL_KNOB: Configure the rate between seeing an integrity error versus seeing a bus error. + int unsigned intg_err_pct = 50; + constraint inject_intg_err_c { + inject_intg_err dist {1 :/ intg_err_pct, + 0 :/ 100 - intg_err_pct}; + } `uvm_object_utils(memory_error_seq) `uvm_declare_p_sequencer(core_ibex_vseqr) @@ -216,14 +224,21 @@ class memory_error_seq extends core_base_new_seq#(ibex_mem_intf_seq_item); endfunction virtual task send_req(); - case (err_type) - IsideErr: begin + vseq.instr_intf_seq.suppress_error_on_exc = skip_on_exc; + vseq.data_intf_seq.suppress_error_on_exc = skip_on_exc; + + `DV_CHECK_MEMBER_RANDOMIZE_FATAL(inject_intg_err) + // If we expect to see only bus errors, we can enable this assertion. Otherwise + // integrity errors would cause alerts to trigger. + `DV_ASSERT_CTRL_REQ("tb_no_alerts_triggered", intg_err_pct == 0) + case ({err_type, inject_intg_err}) + {IsideErr, 1'b0}: begin vseq.instr_intf_seq.inject_error(); end - DsideErr: begin + {DsideErr, 1'b0}: begin vseq.data_intf_seq.inject_error(); end - PickErr: begin + {PickErr, 1'b0}: begin `DV_CHECK_STD_RANDOMIZE_FATAL(choose_side) if (choose_side) begin vseq.instr_intf_seq.inject_error(); @@ -231,6 +246,20 @@ class memory_error_seq extends core_base_new_seq#(ibex_mem_intf_seq_item); vseq.data_intf_seq.inject_error(); end end + {IsideErr, 1'b1}: begin + vseq.instr_intf_seq.inject_intg_error(); + end + {DsideErr, 1'b1}: begin + vseq.data_intf_seq.inject_intg_error(); + end + {PickErr, 1'b1}: begin + `DV_CHECK_STD_RANDOMIZE_FATAL(choose_side) + if (choose_side) begin + vseq.instr_intf_seq.inject_intg_error(); + end else begin + vseq.data_intf_seq.inject_intg_error(); + end + end default: begin // DO nothing end diff --git a/dv/uvm/core_ibex/tests/core_ibex_test_lib.sv b/dv/uvm/core_ibex/tests/core_ibex_test_lib.sv index b7f03f5cf8..83d18e5d8c 100644 --- a/dv/uvm/core_ibex/tests/core_ibex_test_lib.sv +++ b/dv/uvm/core_ibex/tests/core_ibex_test_lib.sv @@ -1630,6 +1630,9 @@ class core_ibex_mem_error_test extends core_ibex_directed_test; `uvm_component_utils(core_ibex_mem_error_test) `uvm_component_new + int illegal_instruction_threshold = 20; + int illegal_instruction_exceptions_seen = 0; + virtual task check_stimulus(); memory_error_seq memory_error_seq_h; memory_error_seq_h = memory_error_seq::type_id::create("memory_error_seq_h", this); @@ -1639,24 +1642,42 @@ class core_ibex_mem_error_test extends core_ibex_directed_test; memory_error_seq_h.iteration_modes = InfiniteRuns; memory_error_seq_h.stimulus_delay_cycles_min = 800; // Interval between injected errors memory_error_seq_h.stimulus_delay_cycles_max = 5000; + memory_error_seq_h.intg_err_pct = cfg.enable_mem_intg_err ? 75 : 0; + memory_error_seq_h.skip_on_exc = 1'b1; fork - begin - forever begin - memory_error_seq_h.start(env.vseqr); - // Wait until we are out of IRQ handler to the inject errors - wait_ret("mret", 20000); - end - end - begin - forever begin - wait_for_core_status(HANDLING_IRQ); - // Do not allow error injection while we are handling IRQ - memory_error_seq_h.stop(); - end - end + run_illegal_instr_watcher(); + memory_error_seq_h.start(env.vseqr); join_none endtask + task run_illegal_instr_watcher(); + // When integrity errors are present loads that see them won't write to the register file. + // Generated code from RISC-DV may be using the loads to produce known constants in register + // that are then used elsewhere, in particular for jump targets. As the register write doesn't + // occur this results in jumping to places that weren't intended which in turn can result in + // illegal instruction exceptions. + // + // As a simple fix for this we observe illegal instruction exceptions and terminate the test + // with a pass after hitting a certain threshold when the test is generating integrity errors. + // + // We don't terminate immediately as sometimes the test hits an illegal instruction exception + // but finds its way back to generated code and terminates as usual. Sometimes it doesn't. The + // treshold allows for normal test termination in cases where that's possible. + if (!cfg.enable_mem_intg_err) begin + return; + end + + forever begin + wait_for_core_exception(ibex_pkg::ExcCauseIllegalInsn); + ++illegal_instruction_exceptions_seen; + end + endtask + + virtual task wait_for_custom_test_done(); + wait(illegal_instruction_exceptions_seen == illegal_instruction_threshold); + `uvm_info(`gfn, "Terminating test early due to illegal instruction threshold reached", UVM_LOW) + endtask + endclass // U-mode mstatus.tw test class From cf7b1a194ee58e5b9f9c7a7241377d28f22d79e1 Mon Sep 17 00:00:00 2001 From: Canberk Topal Date: Fri, 14 Oct 2022 02:37:43 +0100 Subject: [PATCH 08/11] [cosim] Cosim integration of internal NMI This commit is mainly an extension to cosim environment to drive the newly introduced state variable `nmi_int` in Spike. This commit - Extends RVFI interface by a single bit (ext_nmi_int) - Configures cosim to set nmi_int inside Spike Signed-off-by: Canberk Topal --- dv/cosim/cosim.h | 6 +++++ dv/cosim/cosim_dpi.cc | 5 ++++ dv/cosim/cosim_dpi.h | 1 + dv/cosim/cosim_dpi.svh | 1 + dv/cosim/spike_cosim.cc | 23 ++++++++++++++++++- dv/cosim/spike_cosim.h | 1 + .../ibex_cosim_agent/ibex_cosim_scoreboard.sv | 1 + .../ibex_cosim_agent/ibex_rvfi_monitor.sv | 1 + .../ibex_cosim_agent/ibex_rvfi_seq_item.sv | 2 ++ dv/uvm/core_ibex/env/core_ibex_rvfi_if.sv | 2 ++ dv/uvm/core_ibex/tb/core_ibex_tb_top.sv | 1 + .../ibex_simple_system_cosim_checker.sv | 1 + rtl/ibex_core.sv | 18 ++++++++++++++- rtl/ibex_lockstep.sv | 1 + rtl/ibex_top.sv | 2 ++ rtl/ibex_top_tracing.sv | 4 ++++ 16 files changed, 68 insertions(+), 2 deletions(-) diff --git a/dv/cosim/cosim.h b/dv/cosim/cosim.h index 1245f6c415..cbb0b01ba9 100644 --- a/dv/cosim/cosim.h +++ b/dv/cosim/cosim.h @@ -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 diff --git a/dv/cosim/cosim_dpi.cc b/dv/cosim/cosim_dpi.cc index 2d628f0c83..f2fe48c1cb 100644 --- a/dv/cosim/cosim_dpi.cc +++ b/dv/cosim/cosim_dpi.cc @@ -28,6 +28,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); diff --git a/dv/cosim/cosim_dpi.h b/dv/cosim/cosim_dpi.h index df7600bfd3..29a7e25c3a 100644 --- a/dv/cosim/cosim_dpi.h +++ b/dv/cosim/cosim_dpi.h @@ -17,6 +17,7 @@ int riscv_cosim_step(Cosim *cosim, const svBitVecVal *write_reg, svBit sync_trap); 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, diff --git a/dv/cosim/cosim_dpi.svh b/dv/cosim/cosim_dpi.svh index 6a6f1a5814..44361cb557 100644 --- a/dv/cosim/cosim_dpi.svh +++ b/dv/cosim/cosim_dpi.svh @@ -14,6 +14,7 @@ import "DPI-C" function int riscv_cosim_step(chandle cosim_handle, bit [4:0] wri bit [31:0] write_reg_data, bit [31:0] pc, bit sync_trap); 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, diff --git a/dv/cosim/spike_cosim.cc b/dv/cosim/spike_cosim.cc index dac49657c2..1e36697ae2 100644 --- a/dv/cosim/spike_cosim.cc +++ b/dv/cosim/spike_cosim.cc @@ -211,7 +211,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; @@ -376,6 +377,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) { @@ -528,6 +535,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; diff --git a/dv/cosim/spike_cosim.h b/dv/cosim/spike_cosim.h index c911b2ac68..1b73cc1d99 100644 --- a/dv/cosim/spike_cosim.h +++ b/dv/cosim/spike_cosim.h @@ -109,6 +109,7 @@ class SpikeCosim : public simif_t, public Cosim { 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; diff --git a/dv/uvm/core_ibex/common/ibex_cosim_agent/ibex_cosim_scoreboard.sv b/dv/uvm/core_ibex/common/ibex_cosim_agent/ibex_cosim_scoreboard.sv index 7ff04c835d..480651e7b7 100644 --- a/dv/uvm/core_ibex/common/ibex_cosim_agent/ibex_cosim_scoreboard.sv +++ b/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); diff --git a/dv/uvm/core_ibex/common/ibex_cosim_agent/ibex_rvfi_monitor.sv b/dv/uvm/core_ibex/common/ibex_cosim_agent/ibex_rvfi_monitor.sv index 76fa36f9f4..83c99afeb4 100644 --- a/dv/uvm/core_ibex/common/ibex_cosim_agent/ibex_rvfi_monitor.sv +++ b/dv/uvm/core_ibex/common/ibex_cosim_agent/ibex_rvfi_monitor.sv @@ -38,6 +38,7 @@ 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.mcycle = vif.monitor_cb.ext_mcycle; trans_collected.ic_scr_key_valid = vif.monitor_cb.ext_ic_scr_key_valid; diff --git a/dv/uvm/core_ibex/common/ibex_cosim_agent/ibex_rvfi_seq_item.sv b/dv/uvm/core_ibex/common/ibex_cosim_agent/ibex_rvfi_seq_item.sv index 9532ee346a..5356023248 100644 --- a/dv/uvm/core_ibex/common/ibex_cosim_agent/ibex_rvfi_seq_item.sv +++ b/dv/uvm/core_ibex/common/ibex_cosim_agent/ibex_rvfi_seq_item.sv @@ -10,6 +10,7 @@ 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 [63:0] mcycle; @@ -25,6 +26,7 @@ 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 (mcycle, UVM_DEFAULT) `uvm_field_sarray_int (mhpmcounters, UVM_DEFAULT) diff --git a/dv/uvm/core_ibex/env/core_ibex_rvfi_if.sv b/dv/uvm/core_ibex/env/core_ibex_rvfi_if.sv index 7e58df4379..c6ec6ea621 100644 --- a/dv/uvm/core_ibex/env/core_ibex_rvfi_if.sv +++ b/dv/uvm/core_ibex/env/core_ibex_rvfi_if.sv @@ -28,6 +28,7 @@ interface core_ibex_rvfi_if(input logic clk); logic [31:0] mem_wdata; logic [31:0] ext_mip; logic ext_nmi; + logic ext_nmi_int; logic [31:0] ext_debug_req; logic [63:0] ext_mcycle; @@ -61,6 +62,7 @@ interface core_ibex_rvfi_if(input logic clk); input mem_wdata; input ext_mip; input ext_nmi; + input ext_nmi_int; input ext_debug_req; input ext_mcycle; input ext_mhpmcounters; diff --git a/dv/uvm/core_ibex/tb/core_ibex_tb_top.sv b/dv/uvm/core_ibex/tb/core_ibex_tb_top.sv index bd85b601b1..62b6104573 100644 --- a/dv/uvm/core_ibex/tb/core_ibex_tb_top.sv +++ b/dv/uvm/core_ibex/tb/core_ibex_tb_top.sv @@ -192,6 +192,7 @@ module core_ibex_tb_top; assign rvfi_if.mem_wdata = dut.rvfi_mem_wdata; assign rvfi_if.ext_mip = dut.rvfi_ext_mip; 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_mcycle = dut.rvfi_ext_mcycle; assign rvfi_if.ext_mhpmcounters = dut.rvfi_ext_mhpmcounters; diff --git a/dv/verilator/simple_system_cosim/ibex_simple_system_cosim_checker.sv b/dv/verilator/simple_system_cosim/ibex_simple_system_cosim_checker.sv index b65834b1e9..362f548b06 100644 --- a/dv/verilator/simple_system_cosim/ibex_simple_system_cosim_checker.sv +++ b/dv/verilator/simple_system_cosim/ibex_simple_system_cosim_checker.sv @@ -43,6 +43,7 @@ module ibex_simple_system_cosim_checker #( always @(posedge clk_i) begin if (u_top.rvfi_valid) begin riscv_cosim_set_nmi(cosim_handle, u_top.rvfi_ext_nmi); + riscv_cosim_set_nmi_int(cosim_handle, u_top.rvfi_ext_nmi_int); riscv_cosim_set_mip(cosim_handle, u_top.rvfi_ext_mip); riscv_cosim_set_debug_req(cosim_handle, u_top.rvfi_ext_debug_req); riscv_cosim_set_mcycle(cosim_handle, u_top.rvfi_ext_mcycle); diff --git a/rtl/ibex_core.sv b/rtl/ibex_core.sv index ebe67f98aa..5a50ba4ac0 100644 --- a/rtl/ibex_core.sv +++ b/rtl/ibex_core.sv @@ -139,6 +139,7 @@ module ibex_core import ibex_pkg::*; #( output logic [31:0] rvfi_mem_wdata, output logic [31:0] rvfi_ext_mip, output logic rvfi_ext_nmi, + output logic rvfi_ext_nmi_int, output logic rvfi_ext_debug_req, output logic [63:0] rvfi_ext_mcycle, output logic [31:0] rvfi_ext_mhpmcounters [10], @@ -1235,9 +1236,11 @@ module ibex_core import ibex_pkg::*; #( logic new_debug_req; logic new_nmi; + logic new_nmi_int; logic new_irq; ibex_pkg::irqs_t captured_mip; logic captured_nmi; + logic captured_nmi_int; logic captured_debug_req; logic captured_valid; @@ -1245,6 +1248,7 @@ module ibex_core import ibex_pkg::*; #( // debug_req and MIP captured at IF -> ID transition so one extra stage ibex_pkg::irqs_t rvfi_ext_stage_mip [RVFI_STAGES+1]; logic rvfi_ext_stage_nmi [RVFI_STAGES+1]; + logic rvfi_ext_stage_nmi_int [RVFI_STAGES+1]; logic rvfi_ext_stage_debug_req [RVFI_STAGES+1]; logic [63:0] rvfi_ext_stage_mcycle [RVFI_STAGES]; logic [31:0] rvfi_ext_stage_mhpmcounters [RVFI_STAGES][10]; @@ -1293,6 +1297,7 @@ module ibex_core import ibex_pkg::*; #( end assign rvfi_ext_nmi = rvfi_ext_stage_nmi [RVFI_STAGES]; + assign rvfi_ext_nmi_int = rvfi_ext_stage_nmi_int [RVFI_STAGES]; assign rvfi_ext_debug_req = rvfi_ext_stage_debug_req [RVFI_STAGES]; assign rvfi_ext_mcycle = rvfi_ext_stage_mcycle [RVFI_STAGES-1]; assign rvfi_ext_mhpmcounters = rvfi_ext_stage_mhpmcounters [RVFI_STAGES-1]; @@ -1380,6 +1385,7 @@ module ibex_core import ibex_pkg::*; #( // appropriately. assign new_debug_req = (debug_req_i & ~debug_mode); assign new_nmi = irq_nm_i & ~nmi_mode & ~debug_mode; + assign new_nmi_int = id_stage_i.controller_i.irq_nm_int & ~nmi_mode & ~debug_mode; assign new_irq = irq_pending_o & csr_mstatus_mie & ~nmi_mode & ~debug_mode; always_ff @(posedge clk_i or negedge rst_ni) begin @@ -1387,13 +1393,16 @@ module ibex_core import ibex_pkg::*; #( captured_valid <= 1'b0; captured_mip <= '0; captured_nmi <= 1'b0; + captured_nmi_int <= 1'b0; captured_debug_req <= 1'b0; end else begin // Capture when ID stage has emptied out and something occurs that will cause a trap and we // haven't yet captured - if (~instr_valid_id & (new_debug_req | new_irq | new_nmi) & ~captured_valid) begin + if (~instr_valid_id & (new_debug_req | new_irq | new_nmi | new_nmi_int) & + ~captured_valid) begin captured_valid <= 1'b1; captured_nmi <= irq_nm_i; + captured_nmi_int <= id_stage_i.controller_i.irq_nm_int; captured_mip <= cs_registers_i.mip; captured_debug_req <= debug_req_i; end @@ -1415,12 +1424,16 @@ module ibex_core import ibex_pkg::*; #( if (!rst_ni) begin rvfi_ext_stage_mip[0] <= '0; rvfi_ext_stage_nmi[0] <= '0; + rvfi_ext_stage_nmi_int[0] <= '0; rvfi_ext_stage_debug_req[0] <= '0; end else if (if_stage_i.instr_valid_id_d & if_stage_i.instr_new_id_d) begin rvfi_ext_stage_mip[0] <= instr_valid_id | ~captured_valid ? cs_registers_i.mip : captured_mip; rvfi_ext_stage_nmi[0] <= instr_valid_id | ~captured_valid ? irq_nm_i : captured_nmi; + rvfi_ext_stage_nmi_int[0] <= + instr_valid_id | ~captured_valid ? id_stage_i.controller_i.irq_nm_int : + captured_nmi_int; rvfi_ext_stage_debug_req[0] <= instr_valid_id | ~captured_valid ? debug_req_i : captured_debug_req; end @@ -1454,6 +1467,7 @@ module ibex_core import ibex_pkg::*; #( rvfi_stage_mem_addr[i] <= '0; rvfi_ext_stage_mip[i+1] <= '0; rvfi_ext_stage_nmi[i+1] <= '0; + rvfi_ext_stage_nmi_int[i+1] <= '0; rvfi_ext_stage_debug_req[i+1] <= '0; rvfi_ext_stage_mcycle[i] <= '0; rvfi_ext_stage_mhpmcounters[i] <= '{10{'0}}; @@ -1489,6 +1503,7 @@ module ibex_core import ibex_pkg::*; #( rvfi_stage_mem_addr[i] <= rvfi_mem_addr_d; rvfi_ext_stage_mip[i+1] <= rvfi_ext_stage_mip[i]; rvfi_ext_stage_nmi[i+1] <= rvfi_ext_stage_nmi[i]; + rvfi_ext_stage_nmi_int[i+1] <= rvfi_ext_stage_nmi_int[i]; rvfi_ext_stage_debug_req[i+1] <= rvfi_ext_stage_debug_req[i]; rvfi_ext_stage_mcycle[i] <= cs_registers_i.mcycle_counter_i.counter_val_o; rvfi_ext_stage_ic_scr_key_valid[i] <= cs_registers_i.cpuctrlsts_ic_scr_key_valid_q; @@ -1531,6 +1546,7 @@ module ibex_core import ibex_pkg::*; #( rvfi_ext_stage_mip[i+1] <= rvfi_ext_stage_mip[i]; rvfi_ext_stage_nmi[i+1] <= rvfi_ext_stage_nmi[i]; + rvfi_ext_stage_nmi_int[i+1] <= rvfi_ext_stage_nmi_int[i]; rvfi_ext_stage_debug_req[i+1] <= rvfi_ext_stage_debug_req[i]; rvfi_ext_stage_mcycle[i] <= rvfi_ext_stage_mcycle[i-1]; rvfi_ext_stage_ic_scr_key_valid[i] <= rvfi_ext_stage_ic_scr_key_valid[i-1]; diff --git a/rtl/ibex_lockstep.sv b/rtl/ibex_lockstep.sv index ec157ae46e..896fa34c8f 100644 --- a/rtl/ibex_lockstep.sv +++ b/rtl/ibex_lockstep.sv @@ -429,6 +429,7 @@ module ibex_lockstep import ibex_pkg::*; #( .rvfi_mem_wdata (), .rvfi_ext_mip (), .rvfi_ext_nmi (), + .rvfi_ext_nmi_int (), .rvfi_ext_debug_req (), .rvfi_ext_mcycle (), .rvfi_ext_mhpmcounters (), diff --git a/rtl/ibex_top.sv b/rtl/ibex_top.sv index df447a48f7..c2f1004e7a 100644 --- a/rtl/ibex_top.sv +++ b/rtl/ibex_top.sv @@ -118,6 +118,7 @@ module ibex_top import ibex_pkg::*; #( output logic [31:0] rvfi_mem_wdata, output logic [31:0] rvfi_ext_mip, output logic rvfi_ext_nmi, + output logic rvfi_ext_nmi_int, output logic rvfi_ext_debug_req, output logic [63:0] rvfi_ext_mcycle, output logic [31:0] rvfi_ext_mhpmcounters [10], @@ -387,6 +388,7 @@ module ibex_top import ibex_pkg::*; #( .rvfi_mem_wdata, .rvfi_ext_mip, .rvfi_ext_nmi, + .rvfi_ext_nmi_int, .rvfi_ext_debug_req, .rvfi_ext_mcycle, .rvfi_ext_mhpmcounters, diff --git a/rtl/ibex_top_tracing.sv b/rtl/ibex_top_tracing.sv index abdaba2282..d3f682e398 100644 --- a/rtl/ibex_top_tracing.sv +++ b/rtl/ibex_top_tracing.sv @@ -121,6 +121,7 @@ module ibex_top_tracing import ibex_pkg::*; #( logic [31:0] rvfi_mem_wdata; logic [31:0] rvfi_ext_mip; logic rvfi_ext_nmi; + logic rvfi_ext_nmi_int; logic rvfi_ext_debug_req; logic [63:0] rvfi_ext_mcycle; @@ -134,6 +135,7 @@ module ibex_top_tracing import ibex_pkg::*; #( logic [31:0] unused_rvfi_ext_mip; logic unused_rvfi_ext_nmi; + logic unused_rvfi_ext_nmi_int; logic unused_rvfi_ext_debug_req; logic [63:0] unused_rvfi_ext_mcycle; logic unused_rvfi_ext_ic_scr_key_valid; @@ -142,6 +144,7 @@ module ibex_top_tracing import ibex_pkg::*; #( // them. assign unused_rvfi_ext_mip = rvfi_ext_mip; assign unused_rvfi_ext_nmi = rvfi_ext_nmi; + assign unused_rvfi_ext_nmi_int = rvfi_ext_nmi_int; assign unused_rvfi_ext_debug_req = rvfi_ext_debug_req; assign unused_rvfi_ext_mcycle = rvfi_ext_mcycle; assign unused_perf_regs = rvfi_ext_mhpmcounters; @@ -242,6 +245,7 @@ module ibex_top_tracing import ibex_pkg::*; #( .rvfi_mem_wdata, .rvfi_ext_mip, .rvfi_ext_nmi, + .rvfi_ext_nmi_int, .rvfi_ext_debug_req, .rvfi_ext_mcycle, .rvfi_ext_mhpmcounters, From 64c2bec192601a8579dc5544335f6ac986a3f93f Mon Sep 17 00:00:00 2001 From: Canberk Topal Date: Thu, 20 Oct 2022 17:41:52 +0100 Subject: [PATCH 09/11] Fixup RVFI connection for pc_wdata Signed-off-by: Canberk Topal --- dv/uvm/core_ibex/tb/core_ibex_tb_top.sv | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dv/uvm/core_ibex/tb/core_ibex_tb_top.sv b/dv/uvm/core_ibex/tb/core_ibex_tb_top.sv index 62b6104573..7747182b4d 100644 --- a/dv/uvm/core_ibex/tb/core_ibex_tb_top.sv +++ b/dv/uvm/core_ibex/tb/core_ibex_tb_top.sv @@ -185,7 +185,7 @@ module core_ibex_tb_top; assign rvfi_if.rd_addr = dut.rvfi_rd_addr; assign rvfi_if.rd_wdata = dut.rvfi_rd_wdata; assign rvfi_if.pc_rdata = dut.rvfi_pc_rdata; - assign rvfi_if_pc_wdata = dut.rvfi_pc_wdata; + assign rvfi_if.pc_wdata = dut.rvfi_pc_wdata; assign rvfi_if.mem_addr = dut.rvfi_mem_addr; assign rvfi_if.mem_rmask = dut.rvfi_mem_rmask; assign rvfi_if.mem_rdata = dut.rvfi_mem_rdata; From 70638e7d3a461999b507763401e6a98a11537ee5 Mon Sep 17 00:00:00 2001 From: Greg Chadwick Date: Sat, 29 Oct 2022 20:46:52 +0100 Subject: [PATCH 10/11] [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. --- dv/cosim/cosim.h | 2 +- dv/cosim/cosim_dpi.cc | 7 +- dv/cosim/cosim_dpi.h | 2 +- dv/cosim/cosim_dpi.svh | 2 +- dv/cosim/spike_cosim.cc | 106 ++++++++++++++++-- dv/cosim/spike_cosim.h | 8 +- .../ibex_cosim_agent/ibex_cosim_scoreboard.sv | 2 +- .../ibex_cosim_agent/ibex_rvfi_monitor.sv | 1 + .../ibex_cosim_agent/ibex_rvfi_seq_item.sv | 2 + dv/uvm/core_ibex/env/core_ibex_rvfi_if.sv | 2 + dv/uvm/core_ibex/tb/core_ibex_tb_top.sv | 1 + .../ibex_simple_system_cosim_checker.sv | 3 +- rtl/ibex_core.sv | 22 ++++ rtl/ibex_lockstep.sv | 1 + rtl/ibex_top.sv | 2 + rtl/ibex_top_tracing.sv | 4 + 16 files changed, 151 insertions(+), 16 deletions(-) diff --git a/dv/cosim/cosim.h b/dv/cosim/cosim.h index cbb0b01ba9..28c6805b9a 100644 --- a/dv/cosim/cosim.h +++ b/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 diff --git a/dv/cosim/cosim_dpi.cc b/dv/cosim/cosim_dpi.cc index f2fe48c1cb..37862629cc 100644 --- a/dv/cosim/cosim_dpi.cc +++ b/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) { diff --git a/dv/cosim/cosim_dpi.h b/dv/cosim/cosim_dpi.h index 29a7e25c3a..e57bc94228 100644 --- a/dv/cosim/cosim_dpi.h +++ b/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); diff --git a/dv/cosim/cosim_dpi.svh b/dv/cosim/cosim_dpi.svh index 44361cb557..4e2b98deeb 100644 --- a/dv/cosim/cosim_dpi.svh +++ b/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); diff --git a/dv/cosim/spike_cosim.cc b/dv/cosim/spike_cosim.cc index 1e36697ae2..0c4d754a5e 100644 --- a/dv/cosim/spike_cosim.cc +++ b/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 ®_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 ®_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(&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(&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; } diff --git a/dv/cosim/spike_cosim.h b/dv/cosim/spike_cosim.h index 1b73cc1d99..3c07e8c463 100644 --- a/dv/cosim/spike_cosim.h +++ b/dv/cosim/spike_cosim.h @@ -61,6 +61,7 @@ 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); @@ -68,6 +69,9 @@ class SpikeCosim : public simif_t, public Cosim { bool check_gpr_write(const commit_log_reg_t::value_type ®_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 ®_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; diff --git a/dv/uvm/core_ibex/common/ibex_cosim_agent/ibex_cosim_scoreboard.sv b/dv/uvm/core_ibex/common/ibex_cosim_agent/ibex_cosim_scoreboard.sv index 480651e7b7..a347b5dd3c 100644 --- a/dv/uvm/core_ibex/common/ibex_cosim_agent/ibex_cosim_scoreboard.sv +++ b/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 diff --git a/dv/uvm/core_ibex/common/ibex_cosim_agent/ibex_rvfi_monitor.sv b/dv/uvm/core_ibex/common/ibex_cosim_agent/ibex_rvfi_monitor.sv index 83c99afeb4..df5a79d95a 100644 --- a/dv/uvm/core_ibex/common/ibex_cosim_agent/ibex_rvfi_monitor.sv +++ b/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; diff --git a/dv/uvm/core_ibex/common/ibex_cosim_agent/ibex_rvfi_seq_item.sv b/dv/uvm/core_ibex/common/ibex_cosim_agent/ibex_rvfi_seq_item.sv index 5356023248..97a5cf6d9b 100644 --- a/dv/uvm/core_ibex/common/ibex_cosim_agent/ibex_rvfi_seq_item.sv +++ b/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) diff --git a/dv/uvm/core_ibex/env/core_ibex_rvfi_if.sv b/dv/uvm/core_ibex/env/core_ibex_rvfi_if.sv index c6ec6ea621..83e997fc31 100644 --- a/dv/uvm/core_ibex/env/core_ibex_rvfi_if.sv +++ b/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; diff --git a/dv/uvm/core_ibex/tb/core_ibex_tb_top.sv b/dv/uvm/core_ibex/tb/core_ibex_tb_top.sv index 7747182b4d..6ca10ef130 100644 --- a/dv/uvm/core_ibex/tb/core_ibex_tb_top.sv +++ b/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; diff --git a/dv/verilator/simple_system_cosim/ibex_simple_system_cosim_checker.sv b/dv/verilator/simple_system_cosim/ibex_simple_system_cosim_checker.sv index 362f548b06..6ff2d2c1f2 100644 --- a/dv/verilator/simple_system_cosim/ibex_simple_system_cosim_checker.sv +++ b/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 diff --git a/rtl/ibex_core.sv b/rtl/ibex_core.sv index 5a50ba4ac0..31143a40a0 100644 --- a/rtl/ibex_core.sv +++ b/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], @@ -1676,6 +1677,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. diff --git a/rtl/ibex_lockstep.sv b/rtl/ibex_lockstep.sv index 896fa34c8f..3a331efaf3 100644 --- a/rtl/ibex_lockstep.sv +++ b/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 (), diff --git a/rtl/ibex_top.sv b/rtl/ibex_top.sv index c2f1004e7a..84b0f9cd1f 100644 --- a/rtl/ibex_top.sv +++ b/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, diff --git a/rtl/ibex_top_tracing.sv b/rtl/ibex_top_tracing.sv index d3f682e398..c8f1e9d548 100644 --- a/rtl/ibex_top_tracing.sv +++ b/rtl/ibex_top_tracing.sv @@ -123,6 +123,7 @@ module ibex_top_tracing import ibex_pkg::*; #( logic rvfi_ext_nmi; logic rvfi_ext_nmi_int; logic rvfi_ext_debug_req; + logic rvfi_ext_rf_wr_suppress; logic [63:0] rvfi_ext_mcycle; logic [31:0] rvfi_ext_mhpmcounters [10]; @@ -137,6 +138,7 @@ module ibex_top_tracing import ibex_pkg::*; #( logic unused_rvfi_ext_nmi; logic unused_rvfi_ext_nmi_int; logic unused_rvfi_ext_debug_req; + logic unused_rvfi_ext_rf_wr_suppress; logic [63:0] unused_rvfi_ext_mcycle; logic unused_rvfi_ext_ic_scr_key_valid; @@ -146,6 +148,7 @@ module ibex_top_tracing import ibex_pkg::*; #( assign unused_rvfi_ext_nmi = rvfi_ext_nmi; assign unused_rvfi_ext_nmi_int = rvfi_ext_nmi_int; assign unused_rvfi_ext_debug_req = rvfi_ext_debug_req; + assign unused_rvfi_ext_rf_wr_suppress = rvfi_ext_rf_wr_suppress; assign unused_rvfi_ext_mcycle = rvfi_ext_mcycle; assign unused_perf_regs = rvfi_ext_mhpmcounters; assign unused_perf_regsh = rvfi_ext_mhpmcountersh; @@ -247,6 +250,7 @@ module ibex_top_tracing 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, From 53ed7bddd4c5121f797eeaf2bbde2b5322eceacb Mon Sep 17 00:00:00 2001 From: Greg Chadwick Date: Wed, 2 Nov 2022 16:35:18 +0000 Subject: [PATCH 11/11] [dv] Disable bad integrity on uninit accesses for mem error tests --- dv/uvm/core_ibex/riscv_dv_extension/testlist.yaml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/dv/uvm/core_ibex/riscv_dv_extension/testlist.yaml b/dv/uvm/core_ibex/riscv_dv_extension/testlist.yaml index ea5f899236..31f0b7bb27 100644 --- a/dv/uvm/core_ibex/riscv_dv_extension/testlist.yaml +++ b/dv/uvm/core_ibex/riscv_dv_extension/testlist.yaml @@ -591,6 +591,7 @@ +enable_mem_intg_err=0 +enable_double_fault_detector=0 +require_signature_addr=1 + +enable_bad_intg_on_uninit_access=0 compare_opts: compare_final_value_only: 1 @@ -610,6 +611,7 @@ +enable_mem_intg_err=1 +enable_double_fault_detector=0 +require_signature_addr=1 + +enable_bad_intg_on_uninit_access=0 compare_opts: compare_final_value_only: 1