diff --git a/dv/cosim/cosim.h b/dv/cosim/cosim.h index ca6757c69f..b6b4458b86 100644 --- a/dv/cosim/cosim.h +++ b/dv/cosim/cosim.h @@ -10,7 +10,7 @@ // Information about a dside transaction observed on the DUT memory interface struct DSideAccessInfo { - // set when the access is a store, in which case `data` is the store data from + // Set when the access is a store, in which case `data` is the store data from // the DUT. Otherwise `data` is the load data provided to the DUT. bool store; // `addr` is the address and must be 32-bit aligned. `data` and `be` are @@ -20,7 +20,7 @@ struct DSideAccessInfo { uint32_t addr; uint32_t be; - // set if an error response to the transaction is seen. + // Set to indicate an error response to the attempted DUT memory access. bool error; // `misaligned_first` and `misaligned_second` are set when the transaction is @@ -30,9 +30,9 @@ struct DSideAccessInfo { // // For example an unaligned 32-bit load to 0x3 produces a transaction with // `addr` as 0x0 and `misaligned_first` set to true, then a transaction with - // `addr` as 0x4 and `misaligned_second` set to true. An unaligned 16-bit load - // to 0x01 only produces a transaction with `addr` as 0x0 and - // `misaligned_first` set to true, there is no second half. + // `addr` as 0x4 and `misaligned_second` set to true. + // An unaligned 16-bit load to 0x01 only produces a transaction with `addr` + // as 0x0 and `misaligned_first` set to true, there is no second half. bool misaligned_first; bool misaligned_second; }; diff --git a/dv/cosim/spike_cosim.cc b/dv/cosim/spike_cosim.cc index 18684dd63f..52ffe90648 100644 --- a/dv/cosim/spike_cosim.cc +++ b/dv/cosim/spike_cosim.cc @@ -71,40 +71,56 @@ SpikeCosim::SpikeCosim(const std::string &isa_string, uint32_t start_pc, char *SpikeCosim::addr_to_mem(reg_t addr) { return nullptr; } bool SpikeCosim::mmio_load(reg_t addr, size_t len, uint8_t *bytes) { + // Load from the ISS memory model, while checking if the access is + // consistent with the observed DUT memory acceses. + // Return false if any errors occur, or the checking fails. + // Within Spike (mmu_t::load_slow_path), this causes a load_access_fault. + + // Spike does not differentiate between iside and dside memory accesses + // (as ibex does) so we make an educated guess which is which, and + // then after fetching the data, check against the recorded DUT access + // that should be queued as pending. + // + // Spike may attempt to access up to 8-bytes from the PC when fetching, so + // assume as a dside access when it falls outside that range. + // N.B. Heuristic + uint32_t pc = processor->get_state()->pc; + bool is_probably_dmem_access = (addr < pc || addr >= (pc + 8)); + // Check we don't have a heuristic mismatch (this may be a false-positive but + // worth failing explicity if it is seen) + assert(!(is_probably_dmem_access && pending_iside_error)); + + // Attempt to make an identical load from the bus memory device. + // This can fail e.g. if accessing an unmapped address (see SpikeCosim::add_memory) bool bus_error = !bus.load(addr, len, bytes); - bool dut_error = false; + // If the DUT encountered a bus error for its access, or the checking failed, + // this is a dut_error. + bool dut_error = is_probably_dmem_access ? (check_mem_access(false, addr, len, bytes) != kCheckMemOk) : false; - // Incoming access may be an iside or dside access. Use PC to help determine - // which. - uint32_t pc = processor->get_state()->pc; uint32_t aligned_addr = addr & 0xfffffffc; - - if (pending_iside_error && (aligned_addr == pending_iside_err_addr)) { - // Check if the incoming access is subject to an iside error, in which case - // assume it's an iside access and produce an error. + if ( pending_iside_error && + (pending_iside_err_addr == aligned_addr)) { + // Check if the aligned PC matches with the aligned address or an + // incremented aligned PC (to capture the unaligned 4-byte instruction + // case). + // If the pending_iside_error has been set, produce a dut_error. pending_iside_error = false; dut_error = true; - } else if (addr < pc || addr >= (pc + 8)) { - // Spike may attempt to access up to 8-bytes from the PC when fetching, so - // only check as a dside access when it falls outside that range. - - // Otherwise check if the aligned PC matches with the aligned address or an - // incremented aligned PC (to capture the unaligned 4-byte instruction - // case). Assume a successful iside access if either of these checks are - // true, otherwise assume a dside access and check against DUT dside - // accesses. If the RTL produced a bus error for the access, or the - // checking failed produce a memory fault in spike. - dut_error = (check_mem_access(false, addr, len, bytes) != kCheckMemOk); - } + }; return !(bus_error || dut_error); } bool SpikeCosim::mmio_store(reg_t addr, size_t len, const uint8_t *bytes) { + // Store to the ISS memory model, while checking if the access is + // consistent with the observed DUT memory acceses. + // Return false if any errors occur, or the checking fails. + // Within Spike (mmu_t::store_slow_path), this causes a store_access_fault. + bool bus_error = !bus.store(addr, len, bytes); - // If the RTL produced a bus error for the access, or the checking failed - // produce a memory fault in spike. + // If the DUT encountered a bus error for its access, or the checking failed, + // this is a dut_error. bool dut_error = (check_mem_access(true, addr, len, bytes) != kCheckMemOk); return !(bus_error || dut_error); @@ -519,12 +535,76 @@ void SpikeCosim::fixup_csr(int csr_num, uint32_t csr_val) { SpikeCosim::check_mem_result_e SpikeCosim::check_mem_access( bool store, uint32_t addr, size_t len, const uint8_t *bytes) { + // Because Spike is only fed instructions that have been made available + // on the RVFI by IBEX (retired and synchronously trapping), spike will + // always run slightly-behind the ibex, and hence we need to store memory + // accesses and errors when they are seen by the testbench, and then pop + // them from the queues when spike actually executes them. + // This delay may be a few cycles in general, but may never happen if + // the access was iside-speculative. + // + // Whenever a load/store is attempted by spike, we check the following + // conditions.... + // + // 0) First, if an iside error has been detected and + // pending_iside_error=True, a dut_error is generated within mmio_load(), and + // this function is not called. This function is called if the heuristics + // believe the access is a dmem access. (dmem may be accessed with stores or + // loads, while imem is only loads.) + // + // As well as checking for correspondence between the spike access and the + // queued DUT dmem access, we also want to pass along the error if the dmem + // access response came back with an error from the DUT memory system bus. So, + // if all the below checks pass, the final step is to check for a marked error + // in the queued dmem access, and pass it along to Spike if so. + // + // The correspondence checks are as follows: + // + // 1) Check there is actually a dmem access in the queue at all. + // Pop the top dmem access from the queue, and then... + // 2) Check the addr of spike access matches that of the queued dmem access. + // 3) Check the type of spike access (load/store) matches that of the queued + // dmem access. + // + // If the dut dmem access was misaligned : + // (Because ibex dmem can make misaligned and byte-enabled accesses, and spike + // will not make accesses in this way, we need to check the final result of + // the two systems is the same. Spike will make multiple single-byte accesses + // for the same end-result). Therefore, this function (and mmio_load/store) + // may be called multiple times for a single dut dmem access. + // By checking the byte-enables of the memory accesses, we can check that.. + // 4) The spike-accesses do not repeat accessing any bytes the dut accesses. + // 5) The spike-access does not touch any bytes not accessed by the dut. + // ..and while doing this, record the bytes accessed so next time around we + // can make the checks 4) and 5). + // When all of the bytes have been accessed matching the queued access, + // discard this recorded data in preparation for the next access. + // + // If the dut dmem access is aligned : + // 6) Check that the spike-accessed bytes match the dut dmem byte-enables in + // one go. + // + // Until this point, we have not even checked the data of the accesses. + // For accesses not tagged with a dut error: + // 7) Check the data of the spike access matches that of the ibex dmem access, + // for all stores and for loads without errors. + // + // For dut dmem accesses with errors, we can't check the data, but one more + // possible sanity test is that for misaligned accesses... + // 8) Check the second access exists in the pending queue + // 9) Check the second (next) access has the correct address + // After 9), pop both dmem accesses from pending. Only return a single error if needed. + // + // Finally, pass along the error to spike if the dut access at the top of the + // queue was marked with one (DSideAccessInfo.error) + assert(len >= 1 && len <= 4); // Expect that no spike memory accesses cross a 32-bit boundary assert(((addr + (len - 1)) & 0xfffffffc) == (addr & 0xfffffffc)); std::string iss_action = store ? "store" : "load"; + // 1) // Check if there are any pending DUT accesses to check against if (pending_dside_accesses.size() == 0) { std::stringstream err_str; @@ -540,6 +620,7 @@ SpikeCosim::check_mem_result_e SpikeCosim::check_mem_access( std::string dut_action = top_pending_access_info.store ? "store" : "load"; + // 2) // Check for an address match uint32_t aligned_addr = addr & 0xfffffffc; if (aligned_addr != top_pending_access_info.addr) { @@ -552,6 +633,7 @@ SpikeCosim::check_mem_result_e SpikeCosim::check_mem_access( return kCheckMemCheckFailed; } + // 3) // Check access type match if (store != top_pending_access_info.store) { std::stringstream err_str; @@ -575,6 +657,7 @@ SpikeCosim::check_mem_result_e SpikeCosim::check_mem_access( // accesses where the DUT will generate an access covering all bytes within // an aligned 32-bit word. + // 4) // Check bytes accessed this time haven't already been been seen for the DUT // access we are trying to match against if ((expected_be & top_pending_access.be_spike) != 0) { @@ -590,6 +673,7 @@ SpikeCosim::check_mem_result_e SpikeCosim::check_mem_access( return kCheckMemCheckFailed; } + // 5) // Check expected access isn't trying to access bytes that the DUT access // didn't access. if ((expected_be & ~top_pending_access_info.be) != 0) { @@ -610,6 +694,7 @@ SpikeCosim::check_mem_result_e SpikeCosim::check_mem_access( pending_access_done = true; } } else { + // 6) // For aligned accesses bytes from spike access must precisely match bytes // from DUT access in one go if (expected_be != top_pending_access_info.be) { @@ -626,9 +711,10 @@ SpikeCosim::check_mem_result_e SpikeCosim::check_mem_access( pending_access_done = true; } + // 7) // Check data from expected access matches pending DUT access. - // Data is ignored on error responses to loads so don't check it. Always check - // store data. + // Always check store data. + // (Data is ignored on error responses to loads so don't check it.) if (store || !top_pending_access_info.error) { // Combine bytes into a single word uint32_t expected_data = 0; @@ -647,10 +733,10 @@ SpikeCosim::check_mem_result_e SpikeCosim::check_mem_access( if (expected_data != masked_dut_data) { std::stringstream err_str; - err_str << "DUT generated " << iss_action << " at address " << std::hex - << top_pending_access_info.addr << " with data " - << masked_dut_data << " but data " << expected_data - << " was expected with byte mask " << expected_be; + err_str << "DUT generated " << iss_action << " dmem access at address " + << std::hex << top_pending_access_info.addr << " with data " + << masked_dut_data << " but ISS access(es) got data " << std::hex << expected_data + << " with equivalent byte mask of " << std::hex << expected_be; errors.emplace_back(err_str.str()); @@ -659,15 +745,24 @@ SpikeCosim::check_mem_result_e SpikeCosim::check_mem_access( } bool pending_access_error = top_pending_access_info.error; + // If the pending dside access has an error, then we don't need to check to data + // of the accesses. However, we can still check if the correct number of accesses + // took place, and then immediately pop the erroneous dmem access(es) off the top + // of the queue. if (pending_access_error && misaligned) { - // When misaligned accesses see an error, if they have crossed a 32-bit - // boundary DUT will generate two accesses. If the top pending access from - // the DUT was the first half of a misaligned access which accesses the top - // byte, it must have crossed the 32-bit boundary and generated a second - // access - if (top_pending_access_info.misaligned_first && + // If requested to make a misaligned access that crosses a 32-bit + // boundary (this could be a word or halfword access), the DUT + // LSU generates two word-aligned accesses with appropriate + // byte-enables set. + // + // If the top pending access from the DUT was the first half of a misaligned + // access which accesses the top byte, it must have crossed the 32-bit + // boundary and generated a second access + if ( top_pending_access_info.misaligned_first && ((top_pending_access_info.be & 0x8) != 0)) { + + // 8) // Check the second access DUT exists if ((pending_dside_accesses.size() < 2) || !pending_dside_accesses[1].dut_access_info.misaligned_second) { @@ -681,6 +776,7 @@ SpikeCosim::check_mem_result_e SpikeCosim::check_mem_access( return kCheckMemCheckFailed; } + // 9) // Check the second access had the expected address if (pending_dside_accesses[1].dut_access_info.addr != (top_pending_access_info.addr + 4)) { @@ -712,6 +808,10 @@ SpikeCosim::check_mem_result_e SpikeCosim::check_mem_access( pending_dside_accesses.erase(pending_dside_accesses.begin()); } + // If we got this far, just check if the DUT access was marked as + // an error, and pass this on to Spike if so as a dut_error. + // (If we just erased the top dmem access from the pending queue, this + // returned error refers to that.) return pending_access_error ? kCheckMemBusError : kCheckMemOk; }