From 57af22a1744fcf97fcd4c101ea1af9e9896c475d Mon Sep 17 00:00:00 2001 From: Anatoly Parshintsev Date: Tue, 23 Jun 2026 09:49:28 +0300 Subject: [PATCH] relax pmp checks for RWX accesses to debug ROM in debug mode Prior this patch it was impossible to enter debug mode when mseccfg.MML was set. The reason is that debug ROM requires both RW and X permissions for debug module to function properly (for example for progbuf execution). This patch relaxes pmp checks and allows unconditional access to debug firmware area whenever the processor is in debug state. --- riscv/mmu.cc | 15 ++++++++++----- riscv/mmu.h | 2 +- riscv/sim.cc | 5 +++++ riscv/sim.h | 2 ++ riscv/simif.h | 1 + 5 files changed, 19 insertions(+), 6 deletions(-) diff --git a/riscv/mmu.cc b/riscv/mmu.cc index a9e9570d..0db3f456 100644 --- a/riscv/mmu.cc +++ b/riscv/mmu.cc @@ -158,11 +158,10 @@ static reg_t reg_from_bytes(size_t len, const uint8_t* bytes) return res; } -bool mmu_t::mmio_ok(reg_t paddr, access_type UNUSED type) +bool mmu_t::mmio_ok(reg_t paddr, size_t len, access_type UNUSED type) { // Disallow access to debug region when not in debug mode - reg_t debug_start = DEBUG_START; // suppress -Wtype-limits - if (paddr >= debug_start && paddr - debug_start < DEBUG_SIZE && proc && !proc->state.debug_mode) + if (proc && sim && sim->is_debug_module_access(paddr, len) && !proc->state.debug_mode) return false; return true; @@ -170,7 +169,7 @@ bool mmu_t::mmio_ok(reg_t paddr, access_type UNUSED type) bool mmu_t::mmio_fetch(reg_t paddr, size_t len, uint8_t* bytes) { - if (!mmio_ok(paddr, FETCH)) + if (!mmio_ok(paddr, len, FETCH)) return false; return sim->mmio_fetch(paddr, len, bytes); @@ -192,7 +191,7 @@ bool mmu_t::mmio(reg_t paddr, size_t len, uint8_t* bytes, access_type type) bool naturally_aligned = (paddr & (len - 1)) == 0; if (power_of_2 && naturally_aligned) { - if (!mmio_ok(paddr, type)) + if (!mmio_ok(paddr, len, type)) return false; if (type == STORE) @@ -537,6 +536,12 @@ bool mmu_t::pmp_ok(reg_t addr, reg_t len, access_type type, reg_t mode, bool hlv if (!proc || proc->n_pmp == 0) return true; + // The debug module implementation relies on firmware (ROM) owned by spike. + // When executing code from this firmware we keep target PMP rules from + // blocking the simulator-owned debug firmware. + if (proc->state.debug_mode && sim && sim->is_debug_module_access(addr, len)) + return true; + if (auto pmp = pmp_lookup(addr, len, 0, proc->n_pmp); pmp.has_value()) return (*pmp)->access_ok(type, mode, hlvx); diff --git a/riscv/mmu.h b/riscv/mmu.h index 4da647b8..90a67e6e 100644 --- a/riscv/mmu.h +++ b/riscv/mmu.h @@ -444,7 +444,7 @@ private: bool mmio_load(reg_t paddr, size_t len, uint8_t* bytes); bool mmio_store(reg_t paddr, size_t len, const uint8_t* bytes); bool mmio(reg_t paddr, size_t len, uint8_t* bytes, access_type type); - bool mmio_ok(reg_t paddr, access_type type); + bool mmio_ok(reg_t paddr, size_t len, access_type type); void check_triggers(triggers::operation_t operation, reg_t addr, bool virt, std::size_t data_size, const std::uint8_t* bytes); diff --git a/riscv/sim.cc b/riscv/sim.cc index d88c21c8..2738ac87 100644 --- a/riscv/sim.cc +++ b/riscv/sim.cc @@ -376,6 +376,11 @@ void sim_t::set_procs_debug(bool value) procs[i]->set_debug(value); } +bool sim_t::is_debug_module_access(reg_t paddr, size_t len) +{ + return bus.find_device(paddr, len).second == &debug_module; +} + bool sim_t::mmio_load(reg_t paddr, size_t len, uint8_t* bytes) { if (paddr + len < paddr) diff --git a/riscv/sim.h b/riscv/sim.h index fd2f993f..b0540f7d 100644 --- a/riscv/sim.h +++ b/riscv/sim.h @@ -6,6 +6,7 @@ #include "cfg.h" #include "debug_module.h" #include "devices.h" +#include "common.h" #include "log_file.h" #include "processor.h" #include "simif.h" @@ -60,6 +61,7 @@ public: processor_t* get_core(size_t i) { return procs.at(i); } abstract_interrupt_controller_t* get_intctrl() const { assert(plic.get()); return plic.get(); } virtual const cfg_t &get_cfg() const override { return *cfg; } + virtual bool is_debug_module_access(reg_t paddr, size_t len) override; virtual const std::map& get_harts() const override { return harts; } const bus_t& get_bus() const { return bus;} diff --git a/riscv/simif.h b/riscv/simif.h index aeab5dba..ca545e21 100644 --- a/riscv/simif.h +++ b/riscv/simif.h @@ -21,6 +21,7 @@ public: virtual bool mmio_fetch(reg_t paddr, size_t len, uint8_t* bytes) { return mmio_load(paddr, len, bytes); } virtual bool mmio_load(reg_t paddr, size_t len, uint8_t* bytes) = 0; virtual bool mmio_store(reg_t paddr, size_t len, const uint8_t* bytes) = 0; + virtual bool is_debug_module_access(reg_t, size_t) { return false; } // Callback for processors to let the simulation know they were reset. virtual void proc_reset(unsigned id) = 0;