Browse Source

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.
pull/2322/head
Anatoly Parshintsev 2 months ago
parent
commit
57af22a174
  1. 15
      riscv/mmu.cc
  2. 2
      riscv/mmu.h
  3. 5
      riscv/sim.cc
  4. 2
      riscv/sim.h
  5. 1
      riscv/simif.h

15
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);

2
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);

5
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)

2
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<size_t, processor_t*>& get_harts() const override { return harts; }
const bus_t& get_bus() const { return bus;}

1
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;

Loading…
Cancel
Save