Re: [Qemu-devel] [PATCH RFT qom-cpu for-next] gdbstub: Fix gdb_register_coprocessor() register counting

["Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com>]

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