| Message ID | 1376328163-7323-1-git-send-email-afaerber@suse.de |
|---|---|
| State | New |
| Headers | show |
Andreas Färber <afaerber@suse.de> writes: > Commit a0e372f0c49ac01faeaeb73a6e8f50e8ac615f34 reorganized the register > counting for GDB. While it seems correct not to let the total number of > registers skyrocket in an SMP scenario through a static variable, the > distinction between total register count and 'g' packet register count > (last_reg vs. num_g_regs) got lost among the way. > > Fix this by introducing CPUState::gdb_num_g_regs and using that in > gdb_handle_packet(). > > Reported-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com> > Cc: qemu-stable@nongnu.org (for 1.6) > Cc: Peter Maydell <peter.maydell@linaro.org> > Cc: Max Filippov <jcmvbkbc@gmail.com> > Signed-off-by: Andreas Färber <afaerber@suse.de> Tested-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com> > --- > gdbstub.c | 6 ++++-- > include/qom/cpu.h | 2 ++ > qom/cpu.c | 2 +- > 3 files changed, 7 insertions(+), 3 deletions(-) > > diff --git a/gdbstub.c b/gdbstub.c > index 1af25a6..9d067d6 100644 > --- a/gdbstub.c > +++ b/gdbstub.c > @@ -621,6 +621,8 @@ void gdb_register_coprocessor(CPUState *cpu, > if (g_pos != s->base_reg) { > fprintf(stderr, "Error: Bad gdb register numbering for '%s'\n" > "Expected %d got %d\n", xml, g_pos, s->base_reg); > + } else { > + cpu->gdb_num_g_regs = cpu->gdb_num_regs; > } > } > } > @@ -902,7 +904,7 @@ static int gdb_handle_packet(GDBState *s, const char *line_buf) > case 'g': > cpu_synchronize_state(s->g_cpu); > len = 0; > - for (addr = 0; addr < s->g_cpu->gdb_num_regs; addr++) { > + for (addr = 0; addr < s->g_cpu->gdb_num_g_regs; addr++) { > reg_size = gdb_read_register(s->g_cpu, mem_buf + len, addr); > len += reg_size; > } > @@ -914,7 +916,7 @@ static int gdb_handle_packet(GDBState *s, const char *line_buf) > registers = mem_buf; > len = strlen(p) / 2; > hextomem((uint8_t *)registers, p, len); > - for (addr = 0; addr < s->g_cpu->gdb_num_regs && len > 0; addr++) { > + for (addr = 0; addr < s->g_cpu->gdb_num_g_regs && len > 0; addr++) { > reg_size = gdb_write_register(s->g_cpu, registers, addr); > len -= reg_size; > registers += reg_size; > diff --git a/include/qom/cpu.h b/include/qom/cpu.h > index 0d6e95c..3e49936 100644 > --- a/include/qom/cpu.h > +++ b/include/qom/cpu.h > @@ -152,6 +152,7 @@ struct kvm_run; > * @current_tb: Currently executing TB. > * @gdb_regs: Additional GDB registers. > * @gdb_num_regs: Number of total registers accessible to GDB. > + * @gdb_num_g_regs: Number of registers in GDB 'g' packets. > * @next_cpu: Next CPU sharing TB cache. > * @kvm_fd: vCPU file descriptor for KVM. > * > @@ -188,6 +189,7 @@ struct CPUState { > struct TranslationBlock *current_tb; > struct GDBRegisterState *gdb_regs; > int gdb_num_regs; > + int gdb_num_g_regs; > CPUState *next_cpu; > > int kvm_fd; > diff --git a/qom/cpu.c b/qom/cpu.c > index aa95108..e71e57b 100644 > --- a/qom/cpu.c > +++ b/qom/cpu.c > @@ -240,7 +240,7 @@ static void cpu_common_initfn(Object *obj) > CPUState *cpu = CPU(obj); > CPUClass *cc = CPU_GET_CLASS(obj); > > - cpu->gdb_num_regs = cc->gdb_num_core_regs; > + cpu->gdb_num_regs = cpu->gdb_num_g_regs = cc->gdb_num_core_regs; > } > > static int64_t cpu_common_get_arch_id(CPUState *cpu) > -- > 1.8.1.4
Am 12.08.2013 19:22, schrieb Andreas Färber: > Commit a0e372f0c49ac01faeaeb73a6e8f50e8ac615f34 reorganized the register > counting for GDB. While it seems correct not to let the total number of > registers skyrocket in an SMP scenario through a static variable, the > distinction between total register count and 'g' packet register count > (last_reg vs. num_g_regs) got lost among the way. > > Fix this by introducing CPUState::gdb_num_g_regs and using that in > gdb_handle_packet(). > > Reported-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com> > Cc: qemu-stable@nongnu.org (for 1.6) > Cc: Peter Maydell <peter.maydell@linaro.org> > Cc: Max Filippov <jcmvbkbc@gmail.com> > Signed-off-by: Andreas Färber <afaerber@suse.de> > --- > gdbstub.c | 6 ++++-- > include/qom/cpu.h | 2 ++ > qom/cpu.c | 2 +- > 3 files changed, 7 insertions(+), 3 deletions(-) Ping! Peter, could you verify the ARM FP registers are now as expected? Max, could you test xtensa as the special case? Thanks, Andreas
On Fri, Aug 16, 2013 at 2:59 PM, Andreas Färber <afaerber@suse.de> wrote: > Am 12.08.2013 19:22, schrieb Andreas Färber: >> Commit a0e372f0c49ac01faeaeb73a6e8f50e8ac615f34 reorganized the register >> counting for GDB. While it seems correct not to let the total number of >> registers skyrocket in an SMP scenario through a static variable, the >> distinction between total register count and 'g' packet register count >> (last_reg vs. num_g_regs) got lost among the way. >> >> Fix this by introducing CPUState::gdb_num_g_regs and using that in >> gdb_handle_packet(). >> >> Reported-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com> >> Cc: qemu-stable@nongnu.org (for 1.6) >> Cc: Peter Maydell <peter.maydell@linaro.org> >> Cc: Max Filippov <jcmvbkbc@gmail.com> >> Signed-off-by: Andreas Färber <afaerber@suse.de> >> --- >> gdbstub.c | 6 ++++-- >> include/qom/cpu.h | 2 ++ >> qom/cpu.c | 2 +- >> 3 files changed, 7 insertions(+), 3 deletions(-) > > Ping! > > Peter, could you verify the ARM FP registers are now as expected? > > Max, could you test xtensa as the special case? Tested, appears to be good.
On 16 August 2013 11:59, Andreas Färber <afaerber@suse.de> wrote:
> Peter, could you verify the ARM FP registers are now as expected?
Yes, we seem to be back to sending them separately rather than
in the 'g' packet with the integer registers.
-- PMM
diff --git a/gdbstub.c b/gdbstub.c index 1af25a6..9d067d6 100644 --- a/gdbstub.c +++ b/gdbstub.c @@ -621,6 +621,8 @@ void gdb_register_coprocessor(CPUState *cpu, if (g_pos != s->base_reg) { fprintf(stderr, "Error: Bad gdb register numbering for '%s'\n" "Expected %d got %d\n", xml, g_pos, s->base_reg); + } else { + cpu->gdb_num_g_regs = cpu->gdb_num_regs; } } } @@ -902,7 +904,7 @@ static int gdb_handle_packet(GDBState *s, const char *line_buf) case 'g': cpu_synchronize_state(s->g_cpu); len = 0; - for (addr = 0; addr < s->g_cpu->gdb_num_regs; addr++) { + for (addr = 0; addr < s->g_cpu->gdb_num_g_regs; addr++) { reg_size = gdb_read_register(s->g_cpu, mem_buf + len, addr); len += reg_size; } @@ -914,7 +916,7 @@ static int gdb_handle_packet(GDBState *s, const char *line_buf) registers = mem_buf; len = strlen(p) / 2; hextomem((uint8_t *)registers, p, len); - for (addr = 0; addr < s->g_cpu->gdb_num_regs && len > 0; addr++) { + for (addr = 0; addr < s->g_cpu->gdb_num_g_regs && len > 0; addr++) { reg_size = gdb_write_register(s->g_cpu, registers, addr); len -= reg_size; registers += reg_size; diff --git a/include/qom/cpu.h b/include/qom/cpu.h index 0d6e95c..3e49936 100644 --- a/include/qom/cpu.h +++ b/include/qom/cpu.h @@ -152,6 +152,7 @@ struct kvm_run; * @current_tb: Currently executing TB. * @gdb_regs: Additional GDB registers. * @gdb_num_regs: Number of total registers accessible to GDB. + * @gdb_num_g_regs: Number of registers in GDB 'g' packets. * @next_cpu: Next CPU sharing TB cache. * @kvm_fd: vCPU file descriptor for KVM. * @@ -188,6 +189,7 @@ struct CPUState { struct TranslationBlock *current_tb; struct GDBRegisterState *gdb_regs; int gdb_num_regs; + int gdb_num_g_regs; CPUState *next_cpu; int kvm_fd; diff --git a/qom/cpu.c b/qom/cpu.c index aa95108..e71e57b 100644 --- a/qom/cpu.c +++ b/qom/cpu.c @@ -240,7 +240,7 @@ static void cpu_common_initfn(Object *obj) CPUState *cpu = CPU(obj); CPUClass *cc = CPU_GET_CLASS(obj); - cpu->gdb_num_regs = cc->gdb_num_core_regs; + cpu->gdb_num_regs = cpu->gdb_num_g_regs = cc->gdb_num_core_regs; } static int64_t cpu_common_get_arch_id(CPUState *cpu)
Commit a0e372f0c49ac01faeaeb73a6e8f50e8ac615f34 reorganized the register counting for GDB. While it seems correct not to let the total number of registers skyrocket in an SMP scenario through a static variable, the distinction between total register count and 'g' packet register count (last_reg vs. num_g_regs) got lost among the way. Fix this by introducing CPUState::gdb_num_g_regs and using that in gdb_handle_packet(). Reported-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com> Cc: qemu-stable@nongnu.org (for 1.6) Cc: Peter Maydell <peter.maydell@linaro.org> Cc: Max Filippov <jcmvbkbc@gmail.com> Signed-off-by: Andreas Färber <afaerber@suse.de> --- gdbstub.c | 6 ++++-- include/qom/cpu.h | 2 ++ qom/cpu.c | 2 +- 3 files changed, 7 insertions(+), 3 deletions(-)