From 6db55bfa0bbbb69215b89735d86bf61d4530706c Mon Sep 17 00:00:00 2001 From: Greg Chadwick Date: Thu, 27 Jun 2024 14:22:32 +0200 Subject: [PATCH 1/3] Move mmu_t::pmp_ok to public Cosim needs to make direct use of this to check if certain memory accesses would be allowable under PMP. --- riscv/mmu.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/riscv/mmu.h b/riscv/mmu.h index b3c38c230b..aa14e0de27 100644 --- a/riscv/mmu.h +++ b/riscv/mmu.h @@ -10,7 +10,7 @@ #include "simif.h" #include "processor.h" #include "memtracer.h" -#include "byteorder.h" +#include "fesvr/byteorder.h" #include "triggers.h" #include #include @@ -418,6 +418,7 @@ class mmu_t blocksz = size; } + bool pmp_ok(reg_t addr, reg_t len, access_type type, reg_t mode); private: simif_t* sim; processor_t* proc; @@ -493,7 +494,6 @@ class mmu_t } reg_t pmp_homogeneous(reg_t addr, reg_t len); - bool pmp_ok(reg_t addr, reg_t len, access_type type, reg_t mode); #ifdef RISCV_ENABLE_DUAL_ENDIAN bool target_big_endian; From 19727e84f3722417645ac4e52117430cb07e596a Mon Sep 17 00:00:00 2001 From: Greg Chadwick Date: Thu, 27 Jun 2024 14:27:19 +0200 Subject: [PATCH 2/3] Remove debug memory access check This check does not make sense for our cosimulation environment (the spike debug module is not in use) and leads to false failures when randomly generated programs want to access the spike debug memory address range. --- riscv/mmu.cc | 4 ---- 1 file changed, 4 deletions(-) diff --git a/riscv/mmu.cc b/riscv/mmu.cc index 7bd353166a..0c543dedda 100644 --- a/riscv/mmu.cc +++ b/riscv/mmu.cc @@ -116,10 +116,6 @@ reg_t reg_from_bytes(size_t len, const uint8_t* bytes) bool mmu_t::mmio_ok(reg_t addr, access_type type) { - // Disallow access to debug region when not in debug mode - if (addr >= DEBUG_START && addr <= DEBUG_END && proc && !proc->state.debug_mode) - return false; - return true; } From f62d2a82149c23facdfbddea89c40aa05b857aaf Mon Sep 17 00:00:00 2001 From: Greg Chadwick Date: Thu, 27 Jun 2024 14:46:45 +0200 Subject: [PATCH 3/3] Introduce pre_val MIP concept. When Ibex is executing an instruction it is possible for MIP to change whilst that instruction is stalled in the ID/EX stage. This results in the MIP observed by a CSR instruction differing from the MIP that decides whether or not that instruction may be interrupted which leads to model mis-matches. This adds a new MIP 'pre_val' which is the MIP value used to determine whether or not an interrupt is taken. The existing value is the one observed by CSR instructions. Employing this mechanism avoids the mis-matches described above. --- riscv/csrs.cc | 8 ++++++++ riscv/csrs.h | 11 +++++++++++ riscv/processor.h | 6 +++++- 3 files changed, 24 insertions(+), 1 deletion(-) diff --git a/riscv/csrs.cc b/riscv/csrs.cc index c48279141a..f4f0f3e6a2 100644 --- a/riscv/csrs.cc +++ b/riscv/csrs.cc @@ -680,6 +680,14 @@ void mip_csr_t::backdoor_write_with_mask(const reg_t mask, const reg_t val) noex this->val = (this->val & ~mask) | (val & mask); } +reg_t mip_csr_t::read_pre_val() const noexcept { + return pre_val; +} + +void mip_csr_t::write_pre_val(const reg_t val) noexcept { + pre_val = val; +} + reg_t mip_csr_t::write_mask() const noexcept { const reg_t supervisor_ints = proc->extension_enabled('S') ? MIP_SSIP | MIP_STIP | MIP_SEIP : 0; const reg_t vssip_int = proc->extension_enabled('H') ? MIP_VSSIP : 0; diff --git a/riscv/csrs.h b/riscv/csrs.h index ecb8df8162..7a25516fbd 100644 --- a/riscv/csrs.h +++ b/riscv/csrs.h @@ -341,9 +341,20 @@ class mip_or_mie_csr_t: public csr_t { class mip_csr_t: public mip_or_mie_csr_t { public: mip_csr_t(processor_t* const proc, const reg_t addr); + virtual reg_t read_pre_val() const noexcept; + // The pre_val of MIP is used to determine if an interrupt will be taken at + // the beginning of an instruction step. Whilst the actual CSR value is the + // one accessed by any CSR instruction that may be executed. + // + // Nothing updates the value to become the pre_val. This is designed for use + // with co-simulation where the co-simulation environment is expected to do + // suitable value and pre_val updates before every step. + virtual void write_pre_val(const reg_t val) noexcept; // Does not log. Used by external things (clint) that wiggle bits in mip. void backdoor_write_with_mask(const reg_t mask, const reg_t val) noexcept; + protected: + reg_t pre_val; private: virtual reg_t write_mask() const noexcept override; }; diff --git a/riscv/processor.h b/riscv/processor.h index 46215dc0bd..b5fdf8e1b2 100644 --- a/riscv/processor.h +++ b/riscv/processor.h @@ -374,7 +374,11 @@ class processor_t : public abstract_device_t throw trap_t(((reg_t)1 << get_isa().get_max_xlen()) - 1 - NMI_INTERRUPT_NUM); } - take_interrupt(state.mip->read() & state.mie->read()); + // Interrupt is based on the 'pre_val' of MIP which is set by the + // co-simulation environment. This can differ from the actual value which is + // what is observed by CSR instructions. This models the case where the MIP + // changes whilst an instruction is stalled in the ID/EX stage. + take_interrupt(state.mip->read_pre_val() & state.mie->read()); } void take_interrupt(reg_t mask); // take first enabled interrupt in mask void take_trap(trap_t& t, reg_t epc); // take an exception