[dv] Fix model mismatches in cases where an access crosses PMP regions

Where an access is unaligned Ibex splits it into two transactions, each
of which undergoes a PMP check. It is possible for the first half to
fail a PMP check and the second to succeed and hence produce a request
on the memory interface.

In Spike it accesses memory byte by byte and if it encounters a PMP
error for a particular byte it won't try any further bytes.

This results in a mis-match between Ibex and spike when an unaligned
transaction is split across two PMP regions, one of which allows the
access and the other doesn't. Ibex generates a transaction and spike
doesn't producing an error.

This adds a fixup into the co-simulation environment. It detects when we
have an access that fails PMP that is misaligned. Where this has
resulted in Ibex producing a memory request that spike would not we
remove it from the list of memory requests to check after checking that
the request passes PMP within spike.
This commit is contained in:
Greg Chadwick 2024-06-27 15:04:14 +02:00
parent 89f4d86719
commit 3964804815
12 changed files with 125 additions and 20 deletions

View file

@ -35,6 +35,10 @@ struct DSideAccessInfo {
// `misaligned_first` set to true, there is no second half.
bool misaligned_first;
bool misaligned_second;
bool misaligned_first_saw_error;
bool m_mode_access;
};
class Cosim {

View file

@ -66,7 +66,10 @@ void riscv_cosim_notify_dside_access(Cosim *cosim, svBit store,
svBitVecVal *addr, svBitVecVal *data,
svBitVecVal *be, svBit error,
svBit misaligned_first,
svBit misaligned_second) {
svBit misaligned_second,
svBit misaligned_first_saw_error,
svBit m_mode_access) {
assert(cosim);
cosim->notify_dside_access(
@ -76,7 +79,10 @@ void riscv_cosim_notify_dside_access(Cosim *cosim, svBit store,
.be = be[0],
.error = error != 0,
.misaligned_first = misaligned_first != 0,
.misaligned_second = misaligned_second != 0});
.misaligned_second = misaligned_second != 0,
.misaligned_first_saw_error =
misaligned_first_saw_error != 0,
.m_mode_access = m_mode_access != 0});
}
void riscv_cosim_set_iside_error(Cosim *cosim, svBitVecVal *addr) {

View file

@ -27,7 +27,9 @@ void riscv_cosim_notify_dside_access(Cosim *cosim, svBit store,
svBitVecVal *addr, svBitVecVal *data,
svBitVecVal *be, svBit error,
svBit misaligned_first,
svBit misaligned_second);
svBit misaligned_second,
svBit misaligned_first_saw_error,
svBit m_mode_access);
void riscv_cosim_set_iside_error(Cosim *cosim, svBitVecVal *addr);
int riscv_cosim_get_num_errors(Cosim *cosim);
const char *riscv_cosim_get_error(Cosim *cosim, int index);

View file

@ -22,7 +22,7 @@ import "DPI-C" function void riscv_cosim_set_csr(chandle cosim_handle, int csr_i
import "DPI-C" function void riscv_cosim_set_ic_scr_key_valid(chandle cosim_handle, bit valid);
import "DPI-C" function void riscv_cosim_notify_dside_access(chandle cosim_handle, bit store,
bit [31:0] addr, bit [31:0] data, bit [3:0] be, bit error, bit misaligned_first,
bit misaligned_second);
bit misaligned_second, bit misaligned_first_saw_error, bit m_mode_access);
import "DPI-C" function int riscv_cosim_set_iside_error(chandle cosim_handle, bit [31:0] addr);
import "DPI-C" function int riscv_cosim_get_num_errors(chandle cosim_handle);
import "DPI-C" function string riscv_cosim_get_error(chandle cosim_handle, int index);

View file

@ -8,6 +8,7 @@
#include "riscv/devices.h"
#include "riscv/log_file.h"
#include "riscv/processor.h"
#include "riscv/mmu.h"
#include "riscv/simif.h"
#include <cassert>
@ -411,6 +412,12 @@ bool SpikeCosim::check_sync_trap(uint32_t write_reg,
return false;
}
if ((processor->get_state()->mcause->read() == 0x5) ||
(processor->get_state()->mcause->read() == 0x7)) {
// We have a load or store access fault, apply fixup for misaligned accesses
misaligned_pmp_fixup();
}
// 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) {
@ -619,6 +626,62 @@ void SpikeCosim::early_interrupt_handle() {
}
}
// Ibex splits misaligned accesses into two separate requests. They
// independently undergo PMP access checks. It is possible for one to fail (so
// no request produced for that half of the access) whilst the other successed
// (producing a request for that half of the access).
//
// Spike splits misaligned accesses up into bytes and will apply PMP access
// checks byte by byte in a linear order. As soon as a byte sees a PMP
// permission failure the rest of the misaligned access is aborted.
//
// This results in mismatches as in some misaligned access cases Ibex will
// produce a request and spike will not.
//
// This fixup detects this condition and removes the Ibex access from
// pending_dside_accesses to avoid a mismatch. This removed access is checked
// against PMP using the spike MMU to check spike agrees it passes PMP checks.
//
// There may be a better way to handle this (e.g. altering spike behaviour to match
// Ibex) so for now a warning is generated in fixup cases so they can be easily
// identified.
void SpikeCosim::misaligned_pmp_fixup() {
if (pending_dside_accesses.size() != 0) {
auto &top_pending_access = pending_dside_accesses.front();
auto &top_pending_access_info = top_pending_access.dut_access_info;
// If top access is the second half of a misaligned access where the first
// half saw an error we have the PMP fixup case
if (top_pending_access_info.misaligned_second &&
top_pending_access_info.misaligned_first_saw_error) {
mmu_t* mmu = processor->get_mmu();
// Check if the second half of the access (which Ibex produces a request
// for and spike does not) passes PMP
if (!mmu->pmp_ok(top_pending_access_info.addr, 4,
top_pending_access_info.store ? STORE : LOAD,
top_pending_access_info.m_mode_access ? PRV_M : PRV_U)) {
// Raise an error if the second half shouldn't have passed PMP
std::stringstream err_str;
err_str << "Saw second half of a misaligned access which not have "
<< "generated a memory request as it does not pass a PMP check,"
<< " address: " << std::hex << top_pending_access_info.addr;
errors.emplace_back(err_str.str());
} else {
// Output warning on stdout so we're aware which tests this is happening
// in
std::cout << "WARNING: Cosim dropping second half of misaligned access "
<< "as first half saw an error and second half passed PMP "
<< "check, address: "
<< std::hex << top_pending_access_info.addr << std::endl;
std::cout << std::dec;
pending_dside_accesses.erase(pending_dside_accesses.begin());
}
}
}
}
void SpikeCosim::set_nmi(bool nmi) {
if (nmi && !nmi_mode && !processor->get_state()->debug_mode &&
processor->halt_request != processor_t::HR_REGULAR) {

View file

@ -95,6 +95,8 @@ class SpikeCosim : public simif_t, public Cosim {
void early_interrupt_handle();
void misaligned_pmp_fixup();
unsigned int insn_cnt;
public:

View file

@ -178,7 +178,8 @@ class ibex_cosim_scoreboard extends uvm_scoreboard;
dmem_port.get(mem_op);
// Notify the cosim of all dside accesses emitted by the RTL
riscv_cosim_notify_dside_access(cosim_handle, mem_op.read_write == WRITE, mem_op.addr,
mem_op.data, mem_op.be, mem_op.error, mem_op.misaligned_first, mem_op.misaligned_second);
mem_op.data, mem_op.be, mem_op.error, mem_op.misaligned_first, mem_op.misaligned_second,
mem_op.misaligned_first_saw_error, mem_op.m_mode_access);
end
endtask: run_cosim_dmem

View file

@ -24,6 +24,8 @@ interface ibex_mem_intf#(
wire error;
wire misaligned_first;
wire misaligned_second;
wire misaligned_first_saw_error;
wire m_mode_access;
clocking request_driver_cb @(posedge clk);
input reset;
@ -70,6 +72,8 @@ interface ibex_mem_intf#(
input error;
input misaligned_first;
input misaligned_second;
input misaligned_first_saw_error;
input m_mode_access;
endclocking
task automatic wait_clks(input int num);

View file

@ -55,10 +55,12 @@ class ibex_mem_intf_monitor extends uvm_monitor;
forever begin
trans_collected = ibex_mem_intf_seq_item::type_id::create("trans_collected");
while(!(vif.monitor_cb.request && vif.monitor_cb.grant)) @(vif.monitor_cb);
trans_collected.addr = vif.monitor_cb.addr;
trans_collected.be = vif.monitor_cb.be;
trans_collected.misaligned_first = vif.monitor_cb.misaligned_first;
trans_collected.misaligned_second = vif.monitor_cb.misaligned_second;
trans_collected.addr = vif.monitor_cb.addr;
trans_collected.be = vif.monitor_cb.be;
trans_collected.misaligned_first = vif.monitor_cb.misaligned_first;
trans_collected.misaligned_second = vif.monitor_cb.misaligned_second;
trans_collected.misaligned_first_saw_error = vif.monitor_cb.misaligned_first_saw_error;
trans_collected.m_mode_access = vif.monitor_cb.m_mode_access;
`uvm_info(get_full_name(), $sformatf("Detect request with address: %0x",
trans_collected.addr), UVM_HIGH)
if(vif.monitor_cb.we) begin

View file

@ -19,18 +19,22 @@ class ibex_mem_intf_seq_item extends uvm_sequence_item;
rand bit error;
bit misaligned_first;
bit misaligned_second;
bit misaligned_first_saw_error;
bit m_mode_access;
`uvm_object_utils_begin(ibex_mem_intf_seq_item)
`uvm_field_int (addr, UVM_DEFAULT)
`uvm_field_enum (rw_e, read_write, UVM_DEFAULT)
`uvm_field_int (be, UVM_DEFAULT)
`uvm_field_int (data, UVM_DEFAULT)
`uvm_field_int (intg, UVM_DEFAULT)
`uvm_field_int (gnt_delay, UVM_DEFAULT)
`uvm_field_int (rvalid_delay, UVM_DEFAULT)
`uvm_field_int (error, UVM_DEFAULT)
`uvm_field_int (misaligned_first, UVM_DEFAULT)
`uvm_field_int (misaligned_second, UVM_DEFAULT)
`uvm_field_int (addr, UVM_DEFAULT)
`uvm_field_enum (rw_e, read_write, UVM_DEFAULT)
`uvm_field_int (be, UVM_DEFAULT)
`uvm_field_int (data, UVM_DEFAULT)
`uvm_field_int (intg, UVM_DEFAULT)
`uvm_field_int (gnt_delay, UVM_DEFAULT)
`uvm_field_int (rvalid_delay, UVM_DEFAULT)
`uvm_field_int (error, UVM_DEFAULT)
`uvm_field_int (misaligned_first, UVM_DEFAULT)
`uvm_field_int (misaligned_second, UVM_DEFAULT)
`uvm_field_int (misaligned_first_saw_error, UVM_DEFAULT)
`uvm_field_int (m_mode_access, UVM_DEFAULT)
`uvm_object_utils_end
`uvm_object_new

View file

@ -292,6 +292,13 @@ module core_ibex_tb_top;
assign data_mem_vif.misaligned_second =
dut.u_ibex_top.u_ibex_core.load_store_unit_i.addr_incr_req_o;
assign data_mem_vif.misaligned_first_saw_error =
dut.u_ibex_top.u_ibex_core.load_store_unit_i.addr_incr_req_o &
dut.u_ibex_top.u_ibex_core.load_store_unit_i.lsu_err_d;
assign data_mem_vif.m_mode_access =
dut.u_ibex_top.u_ibex_core.priv_mode_lsu == ibex_pkg::PRIV_LVL_M;
initial begin
// Drive the clock and reset lines. Reset everything and start the clock at the beginning of
// time

View file

@ -76,6 +76,8 @@ module ibex_simple_system_cosim_checker #(
logic [31:0] outstanding_store_data;
logic outstanding_misaligned_first;
logic outstanding_misaligned_second;
logic outstanding_misaligned_first_saw_error;
logic outstanding_m_mode_access;
always @(posedge clk_i or negedge rst_ni) begin
if (!rst_ni) begin
@ -93,12 +95,20 @@ module ibex_simple_system_cosim_checker #(
outstanding_misaligned_second <=
u_top.u_ibex_top.u_ibex_core.load_store_unit_i.addr_incr_req_o;
outstanding_misaligned_first_saw_error <=
u_top.u_ibex_top.u_ibex_core.load_store_unit_i.addr_incr_req_o &
u_top.u_ibex_top.u_ibex_core.load_store_unit_i.lsu_err_d;
outstanding_m_mode_access <=
u_top.u_ibex_top.u_ibex_core.priv_mode_lsu == ibex_pkg::PRIV_LVL_M;
end
if (host_dmem_rvalid) begin
riscv_cosim_notify_dside_access(cosim_handle, outstanding_store, outstanding_addr,
outstanding_store ? outstanding_store_data : host_dmem_rdata, outstanding_be,
host_dmem_err, outstanding_misaligned_first, outstanding_misaligned_second);
host_dmem_err, outstanding_misaligned_first, outstanding_misaligned_second,
outstanding_misaligned_first_saw_error, outstanding_m_mode_access);
end
end
end