Merge pull request #2322 from EccoTheDolphin/aap-sc/pmp_debug_relaxation

relax pmp checks for RWX accesses to debug ROM in debug mode

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

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

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)

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

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;