From f9d2be538b134ac42bf33b4b04e97d182ced5e36 Mon Sep 17 00:00:00 2001 From: Tim Newsome Date: Mon, 4 Mar 2019 09:17:00 -0800 Subject: [PATCH] Implement halt groups (#280) * Update debug_defines from latest spec. * Implement halt groups. This lets the debugger halt multiple harts near simultaneously. * Revert encoding, which I updated accidentally. --- riscv/debug_defines.h | 735 +++++++++++++++++++++++++++++++++--------- riscv/debug_module.cc | 76 ++++- riscv/debug_module.h | 14 +- 3 files changed, 649 insertions(+), 176 deletions(-) diff --git a/riscv/debug_defines.h b/riscv/debug_defines.h index d6ddd4ff..e6c2c5d3 100644 --- a/riscv/debug_defines.h +++ b/riscv/debug_defines.h @@ -84,8 +84,7 @@ /* * 0: Version described in spec version 0.11. * -* 1: Version described in spec version 0.13 (and later?), which -* reduces the DMI data width to 32 bits. +* 1: Version described in spec version 0.13. * * 15: Version not described in any available version of this spec. */ @@ -134,7 +133,7 @@ * cleared by writing \Fdmireset in \Rdtmcs. * * This indicates that the DM itself responded with an error. -* Note: there are no specified cases in which the DM would +* There are no specified cases in which the DM would * respond with an error, and DMI is not required to support * returning errors. * @@ -145,11 +144,6 @@ * needs to give the target more TCK edges between Update-DR and * Capture-DR. The simplest way to do that is to add extra transitions * in Run-Test/Idle. -* -* (The DTM, DM, and/or component may be in different clock domains, -* so synchronization may be required. Some relatively fixed number of -* TCK ticks may be needed for the request to reach the DM, complete, -* and for the response to be synchronized back into the TCK domain.) */ #define DTM_DMI_OP_OFFSET 0 #define DTM_DMI_OP_LENGTH 2 @@ -167,20 +161,28 @@ #define CSR_DCSR_XDEBUGVER_LENGTH 4 #define CSR_DCSR_XDEBUGVER (0xfU << CSR_DCSR_XDEBUGVER_OFFSET) /* -* When 1, {\tt ebreak} instructions in Machine Mode enter Debug Mode. +* 0: {\tt ebreak} instructions in M-mode behave as described in the +* Privileged Spec. +* +* 1: {\tt ebreak} instructions in M-mode enter Debug Mode. */ #define CSR_DCSR_EBREAKM_OFFSET 15 #define CSR_DCSR_EBREAKM_LENGTH 1 #define CSR_DCSR_EBREAKM (0x1U << CSR_DCSR_EBREAKM_OFFSET) /* -* When 1, {\tt ebreak} instructions in Supervisor Mode enter Debug Mode. +* 0: {\tt ebreak} instructions in S-mode behave as described in the +* Privileged Spec. +* +* 1: {\tt ebreak} instructions in S-mode enter Debug Mode. */ #define CSR_DCSR_EBREAKS_OFFSET 13 #define CSR_DCSR_EBREAKS_LENGTH 1 #define CSR_DCSR_EBREAKS (0x1U << CSR_DCSR_EBREAKS_OFFSET) /* -* When 1, {\tt ebreak} instructions in User/Application Mode enter -* Debug Mode. +* 0: {\tt ebreak} instructions in U-mode behave as described in the +* Privileged Spec. +* +* 1: {\tt ebreak} instructions in U-mode enter Debug Mode. */ #define CSR_DCSR_EBREAKU_OFFSET 12 #define CSR_DCSR_EBREAKU_LENGTH 1 @@ -191,9 +193,10 @@ * 1: Interrupts are enabled during single stepping. * * Implementations may hard wire this bit to 0. -* The debugger must read back the value it -* writes to check whether the feature is supported. If not -* supported, interrupt behavior can be emulated by the debugger. +* In that case interrupt behavior can be emulated by the debugger. +* +* The debugger must not change the value of this bit while the hart +* is running. */ #define CSR_DCSR_STEPIE_OFFSET 11 #define CSR_DCSR_STEPIE_LENGTH 1 @@ -201,14 +204,13 @@ /* * 0: Increment counters as usual. * -* 1: Don't increment any counters while in Debug Mode or on {\tt -* ebreak} instructions that cause entry into Debug Mode. These -* counters include the {\tt cycle} and {\tt instret} CSRs. This is -* preferred for most debugging scenarios. +* 1: Don't increment any hart-local counters while in Debug Mode or +* on {\tt ebreak} instructions that cause entry into Debug Mode. +* These counters include the {\tt instret} CSR. On single-hart cores +* {\tt cycle} should be stopped, but on multi-hart cores it must keep +* incrementing. * -* An implementation may choose not to support writing to this bit. -* The debugger must read back the value it writes to check whether -* the feature is supported. +* An implementation may hardwire this bit to 0 or 1. */ #define CSR_DCSR_STOPCOUNT_OFFSET 10 #define CSR_DCSR_STOPCOUNT_LENGTH 1 @@ -218,9 +220,7 @@ * * 1: Don't increment any hart-local timers while in Debug Mode. * -* An implementation may choose not to support writing to this bit. -* The debugger must read back the value it writes to check whether -* the feature is supported. +* An implementation may hardwire this bit to 0 or 1. */ #define CSR_DCSR_STOPTIME_OFFSET 9 #define CSR_DCSR_STOPTIME_LENGTH 1 @@ -236,9 +236,16 @@ * * 2: The Trigger Module caused a breakpoint exception. (priority 4) * -* 3: The debugger requested entry to Debug Mode. (priority 2) +* 3: The debugger requested entry to Debug Mode using \Fhaltreq. +* (priority 1) * -* 4: The hart single stepped because \Fstep was set. (priority 1) +* 4: The hart single stepped because \Fstep was set. (priority 0, lowest) +* +* 5: The hart halted directly out of reset due to \Fresethaltreq. It +* is also acceptable to report 3 when this happens. (priority 2) +* +* 6: The hart halted because it's part of a halt group. (priority 5, +* highest) Harts may report 3 for this cause instead. * * Other values are reserved for future use. */ @@ -246,10 +253,11 @@ #define CSR_DCSR_CAUSE_LENGTH 3 #define CSR_DCSR_CAUSE (0x7U << CSR_DCSR_CAUSE_OFFSET) /* -* When 1, \Fmprv in \Rmstatus takes effect during debug mode. -* When 0, it is ignored during debug mode. -* Implementing this bit is optional. -* If not implemented it should be tied to 0. +* 0: \Fmprv in \Rmstatus is ignored in Debug Mode. +* +* 1: \Fmprv in \Rmstatus takes effect in Debug Mode. +* +* Implementing this bit is optional. It may be tied to either 0 or 1. */ #define CSR_DCSR_MPRVEN_OFFSET 4 #define CSR_DCSR_MPRVEN_LENGTH 1 @@ -270,6 +278,9 @@ * If the instruction does not complete due to an exception, * the hart will immediately enter Debug Mode before executing * the trap handler, with appropriate exception registers set. +* +* The debugger must not change the value of this bit while the hart +* is running. */ #define CSR_DCSR_STEP_OFFSET 2 #define CSR_DCSR_STEP_LENGTH 1 @@ -289,14 +300,14 @@ #define CSR_DCSR_PRV (0x3U << CSR_DCSR_PRV_OFFSET) #define CSR_DPC 0x7b1 #define CSR_DPC_DPC_OFFSET 0 -#define CSR_DPC_DPC_LENGTH MXLEN -#define CSR_DPC_DPC (((1L<0, 2->1, 3->2, 4->2 +static unsigned field_width(unsigned n) +{ + unsigned i = 0; + n -= 1; + while (n) { + i++; + n >>= 1; + } + return i; +} + ///////////////////////// debug_module_t debug_module_t::debug_module_t(sim_t *sim, unsigned progbufsize, unsigned max_bus_master_bits, bool require_authentication, unsigned abstract_rti) : + nprocs(sim->nprocs()), progbufsize(progbufsize), program_buffer_bytes(4 + 4*progbufsize), max_bus_master_bits(max_bus_master_bits), @@ -27,18 +42,21 @@ debug_module_t::debug_module_t(sim_t *sim, unsigned progbufsize, unsigned max_bu debug_progbuf_start(debug_data_start - program_buffer_bytes), debug_abstract_start(debug_progbuf_start - debug_abstract_size*4), custom_base(0), - sim(sim) + hartsellen(field_width(sim->nprocs())), + sim(sim), + // The spec lets a debugger select nonexistent harts. Create hart_state for + // them because I'm too lazy to add the code to just ignore accesses. + hart_state(1 << field_width(sim->nprocs())) { D(fprintf(stderr, "debug_data_start=0x%x\n", debug_data_start)); D(fprintf(stderr, "debug_progbuf_start=0x%x\n", debug_progbuf_start)); D(fprintf(stderr, "debug_abstract_start=0x%x\n", debug_abstract_start)); + assert(nprocs <= 1024); + program_buffer = new uint8_t[program_buffer_bytes]; - memset(halted, 0, sizeof(halted)); memset(debug_rom_flags, 0, sizeof(debug_rom_flags)); - memset(resumeack, 0, sizeof(resumeack)); - memset(havereset, 0, sizeof(havereset)); memset(program_buffer, 0, program_buffer_bytes); program_buffer[4*progbufsize] = ebreak(); program_buffer[4*progbufsize+1] = ebreak() >> 8; @@ -180,7 +198,20 @@ bool debug_module_t::store(reg_t addr, size_t len, const uint8_t* bytes) if (addr == DEBUG_ROM_HALTED) { assert (len == 4); - halted[id] = true; + if (!hart_state[id].halted) { + hart_state[id].halted = true; + if (hart_state[id].haltgroup) { + for (unsigned i = 0; i < nprocs; i++) { + if (!hart_state[i].halted && + hart_state[i].haltgroup == hart_state[id].haltgroup) { + processor_t *proc = sim->get_core(i); + proc->halt_request = true; + // TODO: What if the debugger comes and writes dmcontrol before the + // halt occurs? + } + } + } + } if (dmcontrol.hartsel == id) { if (0 == (debug_rom_flags[id] & (1 << DEBUG_ROM_FLAG_GO))){ if (dmcontrol.hartsel == id) { @@ -198,8 +229,8 @@ bool debug_module_t::store(reg_t addr, size_t len, const uint8_t* bytes) if (addr == DEBUG_ROM_RESUMING) { assert (len == 4); - halted[id] = false; - resumeack[id] = true; + hart_state[id].halted = false; + hart_state[id].resumeack = true; debug_rom_flags[id] &= ~(1 << DEBUG_ROM_FLAG_RESUME); return true; } @@ -368,7 +399,7 @@ bool debug_module_t::dmi_read(unsigned address, uint32_t *value) dmstatus.allhalted = false; dmstatus.allresumeack = false; if (proc) { - if (halted[dmcontrol.hartsel]) { + if (hart_state[dmcontrol.hartsel].halted) { dmstatus.allhalted = true; } else { dmstatus.allrunning = true; @@ -381,7 +412,7 @@ bool debug_module_t::dmi_read(unsigned address, uint32_t *value) dmstatus.anyrunning = dmstatus.allrunning; dmstatus.anyhalted = dmstatus.allhalted; if (proc) { - if (resumeack[dmcontrol.hartsel]) { + if (hart_state[dmcontrol.hartsel].resumeack) { dmstatus.allresumeack = true; } else { dmstatus.allresumeack = false; @@ -393,9 +424,9 @@ bool debug_module_t::dmi_read(unsigned address, uint32_t *value) result = set_field(result, DMI_DMSTATUS_IMPEBREAK, dmstatus.impebreak); result = set_field(result, DMI_DMSTATUS_ALLHAVERESET, - havereset[dmcontrol.hartsel]); + hart_state[dmcontrol.hartsel].havereset); result = set_field(result, DMI_DMSTATUS_ANYHAVERESET, - havereset[dmcontrol.hartsel]); + hart_state[dmcontrol.hartsel].havereset); result = set_field(result, DMI_DMSTATUS_ALLNONEXISTENT, dmstatus.allnonexistant); result = set_field(result, DMI_DMSTATUS_ALLUNAVAIL, dmstatus.allunavail); result = set_field(result, DMI_DMSTATUS_ALLRUNNING, dmstatus.allrunning); @@ -480,6 +511,10 @@ bool debug_module_t::dmi_read(unsigned address, uint32_t *value) case DMI_AUTHDATA: result = challenge; break; + case DMI_DMCS2: + result = set_field(result, DMI_DMCS2_HALTGROUP, + hart_state[dmcontrol.hartsel].haltgroup); + break; default: result = 0; D(fprintf(stderr, "Unexpected. Returning Error.")); @@ -512,11 +547,11 @@ bool debug_module_t::perform_abstract_command() if ((command >> 24) == 0) { // register access - unsigned size = get_field(command, AC_ACCESS_REGISTER_SIZE); + unsigned size = get_field(command, AC_ACCESS_REGISTER_AARSIZE); bool write = get_field(command, AC_ACCESS_REGISTER_WRITE); unsigned regno = get_field(command, AC_ACCESS_REGISTER_REGNO); - if (!halted[dmcontrol.hartsel]) { + if (!hart_state[dmcontrol.hartsel].halted) { abstractcs.cmderr = CMDERR_HALTRESUME; return true; } @@ -704,7 +739,7 @@ bool debug_module_t::dmi_write(unsigned address, uint32_t value) dmcontrol.hartsel |= get_field(value, DMI_DMCONTROL_HARTSELLO); dmcontrol.hartsel &= (1L<halt_request = dmcontrol.haltreq; if (dmcontrol.resumereq) { debug_rom_flags[dmcontrol.hartsel] |= (1 << DEBUG_ROM_FLAG_RESUME); - resumeack[dmcontrol.hartsel] = false; + hart_state[dmcontrol.hartsel].resumeack = false; } if (dmcontrol.hartreset) { proc->reset(); @@ -794,6 +829,12 @@ bool debug_module_t::dmi_write(unsigned address, uint32_t value) } } return true; + case DMI_DMCS2: + if (get_field(value, DMI_DMCS2_HGWRITE)) { + hart_state[dmcontrol.hartsel].haltgroup = get_field(value, + DMI_DMCS2_HALTGROUP); + } + return true; } } return false; @@ -801,6 +842,7 @@ bool debug_module_t::dmi_write(unsigned address, uint32_t value) void debug_module_t::proc_reset(unsigned id) { - havereset[id] = true; - halted[id] = false; + hart_state[id].havereset = true; + hart_state[id].halted = false; + hart_state[id].haltgroup = 0; } diff --git a/riscv/debug_module.h b/riscv/debug_module.h index 472ae8ec..5f9ba858 100644 --- a/riscv/debug_module.h +++ b/riscv/debug_module.h @@ -73,6 +73,13 @@ typedef struct { bool access8; } sbcs_t; +typedef struct { + bool halted; + bool resumeack; + bool havereset; + uint8_t haltgroup; +} hart_debug_state_t; + class debug_module_t : public abstract_device_t { public: @@ -109,6 +116,7 @@ class debug_module_t : public abstract_device_t private: static const unsigned datasize = 2; + unsigned nprocs; // Size of program_buffer in 32-bit words, as exposed to the rest of the // world. unsigned progbufsize; @@ -129,7 +137,7 @@ class debug_module_t : public abstract_device_t // We only support 1024 harts currently. More requires at least resizing // the arrays below, and their corresponding special memory regions. - static const unsigned hartsellen = 10; + unsigned hartsellen = 10; sim_t *sim; @@ -138,9 +146,7 @@ class debug_module_t : public abstract_device_t uint8_t *program_buffer; uint8_t dmdata[datasize * 4]; - bool halted[1024]; - bool resumeack[1024]; - bool havereset[1024]; + std::vector hart_state; uint8_t debug_rom_flags[1024]; void write32(uint8_t *rom, unsigned int index, uint32_t value);