From 94beb3c026c7e928cd936e14e62de78acbcee1ee Mon Sep 17 00:00:00 2001 From: Andrew Waterman Date: Mon, 28 Jul 2025 20:08:02 -0700 Subject: [PATCH 1/4] Remove physical address checks in sim_t Processors are responsible for validating physical addresses; the bus is only responsible for making some device lives at a given address. --- riscv/sim.cc | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/riscv/sim.cc b/riscv/sim.cc index 388d7290..2776e33d 100644 --- a/riscv/sim.cc +++ b/riscv/sim.cc @@ -337,22 +337,16 @@ void sim_t::set_procs_debug(bool value) procs[i]->set_debug(value); } -static bool paddr_ok(reg_t addr) -{ - static_assert(MAX_PADDR_BITS == 8 * sizeof(addr)); - return true; -} - bool sim_t::mmio_load(reg_t paddr, size_t len, uint8_t* bytes) { - if (paddr + len < paddr || !paddr_ok(paddr + len - 1)) + if (paddr + len < paddr) return false; return bus.load(paddr, len, bytes); } bool sim_t::mmio_store(reg_t paddr, size_t len, const uint8_t* bytes) { - if (paddr + len < paddr || !paddr_ok(paddr + len - 1)) + if (paddr + len < paddr) return false; return bus.store(paddr, len, bytes); } @@ -403,8 +397,6 @@ void sim_t::set_rom() } char* sim_t::addr_to_mem(reg_t paddr) { - if (!paddr_ok(paddr)) - return NULL; auto desc = bus.find_device(paddr >> PGSHIFT << PGSHIFT, PGSIZE); if (auto mem = dynamic_cast(desc.second)) return mem->contents(paddr - desc.first); From 72e8cad123284447f7e1c663e9213b7d1e9209ff Mon Sep 17 00:00:00 2001 From: Andrew Waterman Date: Mon, 28 Jul 2025 20:08:32 -0700 Subject: [PATCH 2/4] Implement processor_t::paddr_bits correctly It wasn't being called anywhere, so the 50- vs. 56-bit discrepancy never manifested. --- riscv/processor.cc | 7 ------- riscv/processor.h | 2 +- 2 files changed, 1 insertion(+), 8 deletions(-) diff --git a/riscv/processor.cc b/riscv/processor.cc index 6fe64abb..47a96c17 100644 --- a/riscv/processor.cc +++ b/riscv/processor.cc @@ -626,13 +626,6 @@ void processor_t::disasm(insn_t insn) } } -int processor_t::paddr_bits() -{ - unsigned max_xlen = isa.get_max_xlen(); - assert(xlen == max_xlen); - return max_xlen == 64 ? 50 : 34; -} - void processor_t::put_csr(int which, reg_t val) { val = zext_xlen(val); diff --git a/riscv/processor.h b/riscv/processor.h index a6e9eeb1..ff4be9e9 100644 --- a/riscv/processor.h +++ b/riscv/processor.h @@ -273,6 +273,7 @@ public: mmu_t* get_mmu() { return mmu; } state_t* get_state() { return &state; } unsigned get_xlen() const { return xlen; } + unsigned paddr_bits() { return isa.get_max_xlen() == 64 ? 56 : 34; } unsigned get_const_xlen() const { // Any code that assumes a const xlen should use this method to // document that assumption. If Spike ever changes to allow @@ -422,7 +423,6 @@ private: void take_trigger_action(triggers::action_t action, reg_t breakpoint_tval, reg_t epc, bool virt); void disasm(insn_t insn); // disassemble and print an instruction void register_insn(insn_desc_t, bool); - int paddr_bits(); void enter_debug_mode(uint8_t cause, uint8_t ext_cause); From a89875689e98e1451358da7cd4e023a3a33a4236 Mon Sep 17 00:00:00 2001 From: Andrew Waterman Date: Mon, 28 Jul 2025 20:09:33 -0700 Subject: [PATCH 3/4] Only support 56-bit PMP addresses As mandated by the ISA spec. --- riscv/csrs.cc | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/riscv/csrs.cc b/riscv/csrs.cc index 3a327126..76f600dd 100644 --- a/riscv/csrs.cc +++ b/riscv/csrs.cc @@ -121,7 +121,7 @@ bool pmpaddr_csr_t::unlogged_write(const reg_t val) noexcept { const bool locked = !lock_bypass && (cfg & PMP_L); if (pmpidx < proc->n_pmp && !locked && !next_locked_and_tor()) { - this->val = val & ((reg_t(1) << (MAX_PADDR_BITS - PMP_SHIFT)) - 1); + this->val = val & ((reg_t(1) << (proc->paddr_bits() - PMP_SHIFT)) - 1); } else return false; @@ -1098,7 +1098,7 @@ bool base_atp_csr_t::satp_valid(reg_t val) const noexcept { } reg_t base_atp_csr_t::compute_new_satp(reg_t val) const noexcept { - reg_t rv64_ppn_mask = (reg_t(1) << (MAX_PADDR_BITS - PGSHIFT)) - 1; + reg_t rv64_ppn_mask = (reg_t(1) << (proc->paddr_bits() - PGSHIFT)) - 1; reg_t mode_mask = proc->get_xlen() == 32 ? SATP32_MODE : SATP64_MODE; reg_t asid_mask_if_enabled = proc->get_xlen() == 32 ? SATP32_ASID : SATP64_ASID; @@ -1326,7 +1326,7 @@ bool hgatp_csr_t::unlogged_write(const reg_t val) noexcept { HGATP32_MODE | (proc->supports_impl(IMPL_MMU_VMID) ? HGATP32_VMID : 0); } else { - mask = (HGATP64_PPN & ((reg_t(1) << (MAX_PADDR_BITS - PGSHIFT)) - 1)) | + mask = (HGATP64_PPN & ((reg_t(1) << (proc->paddr_bits() - PGSHIFT)) - 1)) | (proc->supports_impl(IMPL_MMU_VMID) ? HGATP64_VMID : 0); if (get_field(val, HGATP64_MODE) == HGATP_MODE_OFF || From 352597c4cf2ad4712dabec0234fb22419ccc6bbc Mon Sep 17 00:00:00 2001 From: Andrew Waterman Date: Mon, 28 Jul 2025 20:10:34 -0700 Subject: [PATCH 4/4] Remove MAX_PADDR_BITS It isn't used anymore. --- riscv/mmu.h | 1 - 1 file changed, 1 deletion(-) diff --git a/riscv/mmu.h b/riscv/mmu.h index 94f3a97f..48340cfa 100644 --- a/riscv/mmu.h +++ b/riscv/mmu.h @@ -18,7 +18,6 @@ // virtual memory configuration #define PGSHIFT 12 const reg_t PGSIZE = 1 << PGSHIFT; -#define MAX_PADDR_BITS 64 // observability hooks for load, store and fetch // intentionally empty not to cause runtime overhead