Re: [PATCH v2 RESEND 3/7] gdbstub: add support for the qRegisterInfo query

["Alex Bennée" <alex.bennee@linaro.org>]

Matheus Tavares Bernardino <quic_mathbern@quicinc.com> writes:

> From: Brian Cain <bcain@quicinc.com>
>
> Signed-off-by: Brian Cain <bcain@quicinc.com>
> Signed-off-by: Matheus Tavares Bernardino <quic_mathbern@quicinc.com>
> ---
>  include/hw/core/cpu.h |  4 ++++
>  gdbstub/gdbstub.c     | 27 +++++++++++++++++++++++++++
>  2 files changed, 31 insertions(+)
>
> diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
> index 397fd3ac68..cfdf5514d9 100644
> --- a/include/hw/core/cpu.h
> +++ b/include/hw/core/cpu.h
> @@ -124,6 +124,8 @@ struct SysemuCPUOps;
>   *       its Harvard architecture split code and data.
>   * @gdb_num_core_regs: Number of core registers accessible to GDB.
>   * @gdb_core_xml_file: File name for core registers GDB XML description.
> + * @gdb_qreg_info_lines: Array of lines of registers qRegisterInfo description.
> + * @gdb_qreg_info_line_count: Count of lines for @gdb_qreg_info_lines.
>   * @gdb_stop_before_watchpoint: Indicates whether GDB expects the CPU to stop
>   *           before the insn which triggers a watchpoint rather than after it.
>   * @gdb_arch_name: Optional callback that returns the architecture name known
> @@ -159,6 +161,8 @@ struct CPUClass {
>      vaddr (*gdb_adjust_breakpoint)(CPUState *cpu, vaddr addr);
>  
>      const char *gdb_core_xml_file;
> +    const char **gdb_qreg_info_lines;
> +    int gdb_qreg_info_line_count;
>      gchar * (*gdb_arch_name)(CPUState *cpu);
>      const char * (*gdb_get_dynamic_xml)(CPUState *cpu, const char *xmlname);
>  
> diff --git a/gdbstub/gdbstub.c b/gdbstub/gdbstub.c
> index be18568d0a..f19f8c58c3 100644
> --- a/gdbstub/gdbstub.c
> +++ b/gdbstub/gdbstub.c
> @@ -1409,6 +1409,27 @@ static void handle_query_curr_tid(GArray *params, void *user_ctx)
>      gdb_put_strbuf();
>  }
>  
> +static void handle_query_regs(GArray *params, void *user_ctx)
> +{
> +    if (!params->len) {
> +        return;
> +    }
> +
> +    CPUClass *cc = CPU_GET_CLASS(gdbserver_state.g_cpu);
> +    if (!cc->gdb_qreg_info_lines) {
> +        gdb_put_packet("");
> +        return;
> +    }
> +
> +    int reg_num = get_param(params, 0)->val_ul;
> +    if (reg_num >= cc->gdb_qreg_info_line_count) {
> +        gdb_put_packet("");
> +        return;
> +    }
> +
> +    gdb_put_packet(cc->gdb_qreg_info_lines[reg_num]);
> +}
> +
>  static void handle_query_threads(GArray *params, void *user_ctx)
>  {
>      if (!gdbserver_state.query_cpu) {
> @@ -1578,6 +1599,12 @@ static const GdbCmdParseEntry gdb_gen_query_table[] = {
>          .handler = handle_query_curr_tid,
>          .cmd = "C",
>      },
> +    {
> +        .handler = handle_query_regs,
> +        .cmd = "RegisterInfo",
> +        .cmd_startswith = 1,
> +        .schema = "l0"
> +    },

Where is this defined in the protocol spec, I can't see it in:

  https://sourceware.org/gdb/onlinedocs/gdb/General-Query-Packets.html#General-Query-Packets

and it seems to be information that is handled by the xml register
description. Is there a reason that isn't used for Hexagon?

>      {
>          .handler = handle_query_threads,
>          .cmd = "sThreadInfo",


-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro