From 5397899bb9eddba8c2f87d7d62e6303629ebed90 Mon Sep 17 00:00:00 2001 From: Farid Khaydari Date: Tue, 7 Oct 2025 19:37:33 +0300 Subject: [PATCH] Add configurable datacount for debug module This commit adds support for configuring the number of data registers available in the debug module. Previously, the debug module had a fixed datasize of 2, but now users can specify the number of data registers using the --dm-datacount option when running spike. The changes include: - Adding a datacount parameter to debug_module_config_t - Making dmdata a std::vector instead of a fixed array - Validating that datacount is between 1 and 12 - Updating the debug module to use the configured datacount - Adding command-line option to set datacount - Updating documentation in help output Signed-off-by: Farid Khaydari --- riscv/debug_module.cc | 138 +++++++++++++++++++++++++++++------------- riscv/debug_module.h | 18 +++++- spike_main/spike.cc | 3 + 3 files changed, 115 insertions(+), 44 deletions(-) diff --git a/riscv/debug_module.cc b/riscv/debug_module.cc index a89a4ff9..c878f322 100644 --- a/riscv/debug_module.cc +++ b/riscv/debug_module.cc @@ -1,4 +1,8 @@ +#include +#include #include +#include +#include #include "simif.h" #include "devices.h" @@ -32,6 +36,25 @@ static unsigned field_width(unsigned n) ///////////////////////// debug_module_t +static bool region_descriptor_comparator(const region_descriptor &lhs, + const region_descriptor &rhs) { + return lhs.addr < rhs.addr; +} + +template +static bool has_intersection(It begin, It end) { + assert(std::is_sorted(begin, end, region_descriptor_comparator)); + + // If current interval's end > next interval's start, they intersect + auto intersecion = + std::adjacent_find(begin, end, [](const auto &lhs, const auto &rhs) { + assert(std::numeric_limits::max() - lhs.addr >= lhs.len); + return lhs.addr + lhs.len > rhs.addr; + }); + + return intersecion != end; +} + debug_module_t::debug_module_t(simif_t *sim, const debug_module_config_t &config) : config(config), program_buffer_bytes((config.support_impebreak ? 4 : 0) + 4*config.progbufsize), @@ -57,11 +80,18 @@ debug_module_t::debug_module_t(simif_t *sim, const debug_module_config_t &config exit(1); } + constexpr unsigned max_data_reg = 12; + constexpr unsigned min_data_reg = 1; + if (config.datacount < min_data_reg || config.datacount > max_data_reg) { + fprintf(stderr, "dm-datacount must be between 1 and 12 (got %u)\n", config.datacount); + exit(1); + } + + dmdata.resize(config.datacount * dmdata_reg_size); program_buffer = new uint8_t[program_buffer_bytes]; memset(debug_rom_flags, 0, sizeof(debug_rom_flags)); memset(program_buffer, 0, program_buffer_bytes); - memset(dmdata, 0, sizeof(dmdata)); if (config.support_impebreak) { program_buffer[4*config.progbufsize] = ebreak(); @@ -78,6 +108,20 @@ debug_module_t::debug_module_t(simif_t *sim, const debug_module_config_t &config hart_available_state[i] = true; } + debug_memory_regions = { + region_descriptor{DEBUG_ROM_ENTRY, debug_rom_raw_len, debug_rom_raw}, + region_descriptor{DEBUG_ROM_WHERETO, sizeof(debug_rom_whereto), debug_rom_whereto}, + region_descriptor{DEBUG_ROM_FLAGS, sizeof(debug_rom_flags), debug_rom_flags}, + region_descriptor{debug_data_start, dmdata.size(), dmdata.data()}, + region_descriptor{debug_abstract_start, sizeof(debug_abstract), debug_abstract}, + region_descriptor{debug_progbuf_start, program_buffer_bytes, program_buffer}, + }; + + std::sort(debug_memory_regions.begin(), debug_memory_regions.end(), + region_descriptor_comparator); + assert(!has_intersection(debug_memory_regions.begin(), + debug_memory_regions.end())); + reset(); } @@ -100,7 +144,7 @@ void debug_module_t::reset() dmstatus.version = 2; memset(&abstractcs, 0, sizeof(abstractcs)); - abstractcs.datacount = sizeof(dmdata) / 4; + abstractcs.datacount = config.datacount; abstractcs.progbufsize = config.progbufsize; memset(&abstractauto, 0, sizeof(abstractauto)); @@ -122,38 +166,27 @@ void debug_module_t::reset() challenge = random(); } +static bool belongs_to_range(reg_t access_addr, size_t access_len, + reg_t range_addr, size_t range_len) +{ + assert(std::numeric_limits::max() - access_addr >= access_len); + assert(std::numeric_limits::max() - range_addr >= range_len); + return access_addr >= range_addr && (access_addr < range_addr + range_len) && + ((access_addr + access_len) <= (range_addr + range_len)); +} + bool debug_module_t::load(reg_t addr, size_t len, uint8_t* bytes) { addr = DEBUG_START + addr; - if (addr >= DEBUG_ROM_ENTRY && - (addr + len) <= (DEBUG_ROM_ENTRY + debug_rom_raw_len)) { - memcpy(bytes, debug_rom_raw + addr - DEBUG_ROM_ENTRY, len); - return true; - } - - if (addr >= DEBUG_ROM_WHERETO && (addr + len) <= (DEBUG_ROM_WHERETO + 4)) { - memcpy(bytes, debug_rom_whereto + addr - DEBUG_ROM_WHERETO, len); - return true; - } - - if (addr >= DEBUG_ROM_FLAGS && ((addr + len) <= DEBUG_ROM_FLAGS + 1024)) { - memcpy(bytes, debug_rom_flags + addr - DEBUG_ROM_FLAGS, len); - return true; - } - - if (addr >= debug_abstract_start && ((addr + len) <= (debug_abstract_start + sizeof(debug_abstract)))) { - memcpy(bytes, debug_abstract + addr - debug_abstract_start, len); - return true; - } - - if (addr >= debug_data_start && (addr + len) <= (debug_data_start + sizeof(dmdata))) { - memcpy(bytes, dmdata + addr - debug_data_start, len); - return true; - } + const auto interval_ptr = + std::find_if(debug_memory_regions.begin(), debug_memory_regions.end(), + [addr, len](const auto &range) { + return belongs_to_range(addr, len, range.addr, range.len); + }); - if (addr >= debug_progbuf_start && ((addr + len) <= (debug_progbuf_start + program_buffer_bytes))) { - memcpy(bytes, program_buffer + addr - debug_progbuf_start, len); + if (interval_ptr != debug_memory_regions.end()) { + std::copy_n(std::next(interval_ptr->bytes, addr - interval_ptr->addr), len, bytes); return true; } @@ -163,6 +196,15 @@ bool debug_module_t::load(reg_t addr, size_t len, uint8_t* bytes) return false; } +static bool handle_range_store(reg_t input_addr, size_t input_len, const uint8_t *bytes, + reg_t range_addr, size_t range_len, uint8_t *data) +{ + if (!belongs_to_range(input_addr, input_len, range_addr, range_len)) + return false; + std::copy_n(bytes, input_len, std::next(data, input_addr - range_addr)); + return true; +} + bool debug_module_t::store(reg_t addr, size_t len, const uint8_t* bytes) { D( @@ -188,16 +230,11 @@ bool debug_module_t::store(reg_t addr, size_t len, const uint8_t* bytes) addr = DEBUG_START + addr; - if (addr >= debug_data_start && (addr + len) <= (debug_data_start + sizeof(dmdata))) { - memcpy(dmdata + addr - debug_data_start, bytes, len); + if (handle_range_store(addr, len, bytes, debug_data_start, dmdata.size(), dmdata.data())) return true; - } - - if (addr >= debug_progbuf_start && ((addr + len) <= (debug_progbuf_start + program_buffer_bytes))) { - memcpy(program_buffer + addr - debug_progbuf_start, bytes, len); + if (handle_range_store(addr, len, bytes, debug_progbuf_start, program_buffer_bytes, program_buffer)) return true; - } if (addr == DEBUG_ROM_HALTED) { assert (len == 4); @@ -283,6 +320,16 @@ unsigned debug_module_t::sb_access_bits() return 8 << sbcs.sbaccess; } +uint8_t *debug_module_t::get_dmdata_checked(size_t required_size) +{ + if(dmdata.size() < required_size) { + fprintf(stderr, "dmdata size (%ld) less then required (%ld)\n", + dmdata.size(), required_size); + exit(1); + } + return dmdata.data(); +} + void debug_module_t::sb_autoincrement() { if (!sbcs.autoincrement || !config.max_sba_data_width) @@ -392,7 +439,8 @@ bool debug_module_t::dmi_read(unsigned address, uint32_t *value) D(fprintf(stderr, "dmi_read(0x%x) -> ", address)); if (address >= DM_DATA0 && address < DM_DATA0 + abstractcs.datacount) { unsigned i = address - DM_DATA0; - result = read32(dmdata, i); + assert(dmdata.size() >= 4); + result = read32(get_dmdata_checked(i + 1), i); if (abstractcs.busy) { result = -1; D(fprintf(stderr, "\ndmi_read(0x%02x (data[%d]) -> -1 because abstractcs.busy==true\n", address, i)); @@ -660,6 +708,13 @@ bool debug_module_t::perform_abstract_command() return true; } + assert(size < 8); + // Check if register fit in dmdata + if ((1U << size) > dmdata.size()) { + abstractcs.cmderr = CMDERR_NOTSUP; + return true; + } + unsigned i = 0; if (get_field(command, AC_ACCESS_REGISTER_TRANSFER)) { @@ -781,10 +836,11 @@ bool debug_module_t::perform_abstract_command() if (write) { // Writing V to custom register N will cause future reads of N to // return V, reads of N-1 will return V-1, etc. - custom_base = read32(dmdata, 0) - custom_number; + assert(dmdata.size() >= 4); + custom_base = read32(get_dmdata_checked(1), 0) - custom_number; } else { - write32(dmdata, 0, custom_number + custom_base); - write32(dmdata, 1, 0); + write32(get_dmdata_checked(1), 0, custom_number + custom_base); + write32(get_dmdata_checked(2), 1, 0); } return true; @@ -832,7 +888,7 @@ bool debug_module_t::dmi_write(unsigned address, uint32_t value) if (address >= DM_DATA0 && address < DM_DATA0 + abstractcs.datacount) { unsigned i = address - DM_DATA0; if (!abstractcs.busy) - write32(dmdata, address - DM_DATA0, value); + write32(get_dmdata_checked(address - DM_DATA0), address - DM_DATA0, value); if (abstractcs.busy && abstractcs.cmderr == CMDERR_NONE) { abstractcs.cmderr = CMDERR_BUSY; diff --git a/riscv/debug_module.h b/riscv/debug_module.h index 904f03e2..ddf9aa9f 100644 --- a/riscv/debug_module.h +++ b/riscv/debug_module.h @@ -2,7 +2,7 @@ #ifndef _RISCV_DEBUG_MODULE_H #define _RISCV_DEBUG_MODULE_H -#include +#include #include #include "abstract_device.h" @@ -15,6 +15,7 @@ struct debug_module_config_t { // Size of program_buffer in 32-bit words, as exposed to the rest of the // world. unsigned progbufsize = 2; + unsigned datacount = 2; unsigned max_sba_data_width = 0; bool require_authentication = false; unsigned abstract_rti = 0; @@ -99,6 +100,13 @@ struct hart_debug_state_t { uint8_t haltgroup; }; +// structure to describe mmio region +struct region_descriptor { + reg_t addr; // 1st addr in a range + size_t len; // range size + const uint8_t *bytes; // data +}; + class debug_module_t : public abstract_device_t { public: @@ -131,7 +139,6 @@ class debug_module_t : public abstract_device_t void proc_reset(unsigned id); private: - static const unsigned datasize = 2; debug_module_config_t config; // Actual size of the program buffer, which is 1 word bigger than we let on // to implement the implicit ebreak at the end. @@ -150,7 +157,8 @@ class debug_module_t : public abstract_device_t uint8_t debug_rom_whereto[4]; uint8_t debug_abstract[debug_abstract_size * 4]; uint8_t *program_buffer; - uint8_t dmdata[datasize * 4]; + static constexpr unsigned dmdata_reg_size = 4; + std::vector dmdata; std::vector hart_state; uint8_t debug_rom_flags[1024]; @@ -174,6 +182,8 @@ class debug_module_t : public abstract_device_t unsigned sb_access_bits(); + uint8_t *get_dmdata_checked(size_t required_size); + dmcontrol_t dmcontrol; dmstatus_t dmstatus; abstractcs_t abstractcs; @@ -206,6 +216,8 @@ class debug_module_t : public abstract_device_t bool hart_available(unsigned hart_id) const; unsigned sb_read_wait, sb_write_wait; + + std::array debug_memory_regions; }; #endif diff --git a/spike_main/spike.cc b/spike_main/spike.cc index 3b0e0048..fd180de1 100644 --- a/spike_main/spike.cc +++ b/spike_main/spike.cc @@ -71,6 +71,7 @@ static void help(int exit_code = 1) fprintf(stderr, " --real-time-clint Increment clint time at real-time rate\n"); fprintf(stderr, " --triggers= Number of supported triggers [default 4]\n"); fprintf(stderr, " --dm-progsize= Progsize for the debug module [default 2]\n"); + fprintf(stderr, " --dm-datacount= Number of data registers available for the debug module [default 2]\n"); fprintf(stderr, " --dm-sba= Debug system bus access supports up to " " wide accesses [default 0]\n"); fprintf(stderr, " --dm-auth Debug module requires debugger to authenticate\n"); @@ -413,6 +414,8 @@ int main(int argc, char** argv) }); parser.option(0, "dm-progsize", 1, [&](const char* s){dm_config.progbufsize = atoul_safe(s);}); + parser.option(0, "dm-datacount", 1, + [&](const char* s){dm_config.datacount = atoul_safe(s);}); parser.option(0, "dm-no-impebreak", 0, [&](const char UNUSED *s){dm_config.support_impebreak = false;}); parser.option(0, "dm-sba", 1,