On Wed, May 13, 2020 at 11:08:47AM -0700, Joe Komlodi wrote: > SRegs used to be reported to GDB by iterating over the SRegs array, > however we do not store them in an order that allows them to be > reported to GDB in that way. > > To fix this, a simple map is used to map the register GDB wants to its > location in the SRegs array.
Reviewed-by: Edgar E. Iglesias <edgar.igles...@xilinx.com> > > Signed-off-by: Joe Komlodi <koml...@xilinx.com> > --- > target/microblaze/gdbstub.c | 59 > +++++++++++++++++++++++++++++++++++++-------- > 1 file changed, 49 insertions(+), 10 deletions(-) > > diff --git a/target/microblaze/gdbstub.c b/target/microblaze/gdbstub.c > index af29f00..485b717 100644 > --- a/target/microblaze/gdbstub.c > +++ b/target/microblaze/gdbstub.c > @@ -25,6 +25,21 @@ int mb_cpu_gdb_read_register(CPUState *cs, GByteArray > *mem_buf, int n) > { > MicroBlazeCPU *cpu = MICROBLAZE_CPU(cs); > CPUMBState *env = &cpu->env; > + /* > + * GDB expects SREGs in the following order: > + * PC, MSR, EAR, ESR, FSR, BTR, EDR, PID, ZPR, TLBX, TLBSX, TLBLO, TLBHI. > + * They aren't stored in this order, so make a map. > + * PID, ZPR, TLBx, TLBsx, TLBLO, and TLBHI aren't modeled, so we don't > + * map them to anything and return a value of 0 instead. > + */ > + static const uint8_t sreg_map[6] = { > + SR_PC, > + SR_MSR, > + SR_EAR, > + SR_ESR, > + SR_FSR, > + SR_BTR > + }; > > /* > * GDB expects registers to be reported in this order: > @@ -40,15 +55,16 @@ int mb_cpu_gdb_read_register(CPUState *cs, GByteArray > *mem_buf, int n) > n -= 32; > switch (n) { > case 0 ... 5: > - return gdb_get_reg32(mem_buf, env->sregs[n]); > + return gdb_get_reg32(mem_buf, env->sregs[sreg_map[n]]); > /* PVR12 is intentionally skipped */ > case 6 ... 17: > n -= 6; > return gdb_get_reg32(mem_buf, env->pvr.regs[n]); > - case 18 ... 24: > - /* Add an offset of 6 to resume where we left off with SRegs */ > - n = n - 18 + 6; > - return gdb_get_reg32(mem_buf, env->sregs[n]); > + case 18: > + return gdb_get_reg32(mem_buf, env->sregs[SR_EDR]); > + /* Other SRegs aren't modeled, so report a value of 0 */ > + case 19 ... 24: > + return gdb_get_reg32(mem_buf, 0); > case 25: > return gdb_get_reg32(mem_buf, env->slr); > case 26: > @@ -66,29 +82,52 @@ int mb_cpu_gdb_write_register(CPUState *cs, uint8_t > *mem_buf, int n) > CPUMBState *env = &cpu->env; > uint32_t tmp; > > + /* > + * GDB expects SREGs in the following order: > + * PC, MSR, EAR, ESR, FSR, BTR, EDR, PID, ZPR, TLBX, TLBSX, TLBLO, TLBHI. > + * They aren't stored in this order, so make a map. > + * PID, ZPR, TLBx, TLBsx, TLBLO, and TLBHI aren't modeled, so we don't > + * map them to anything. > + */ > + static const uint8_t sreg_map[6] = { > + SR_PC, > + SR_MSR, > + SR_EAR, > + SR_ESR, > + SR_FSR, > + SR_BTR > + }; > + > if (n > cc->gdb_num_core_regs) { > return 0; > } > > tmp = ldl_p(mem_buf); > > + /* > + * GDB expects registers to be reported in this order: > + * R0-R31 > + * PC-BTR > + * PVR0-PVR11 > + * EDR-TLBHI > + * SLR-SHR > + */ > if (n < 32) { > env->regs[n] = tmp; > } else { > n -= 32; > switch (n) { > case 0 ... 5: > - env->sregs[n] = tmp; > + env->sregs[sreg_map[n]] = tmp; > break; > /* PVR12 is intentionally skipped */ > case 6 ... 17: > n -= 6; > env->pvr.regs[n] = tmp; > break; > - case 18 ... 24: > - /* Add an offset of 6 to resume where we left off with SRegs */ > - n = n - 18 + 6; > - env->sregs[n] = tmp; > + /* Only EDR is modeled in these indeces, so ignore the rest */ > + case 18: > + env->sregs[SR_EDR] = tmp; > break; > case 25: > env->slr = tmp; > -- > 2.7.4 >