Re: RFC: Support target specific qSupported

[Hui Zhu <teawater@gmail.com>]

I think maybe we can add a new interface to target that each gdbarch
can get arch special message from inferior.

Thanks,
Hui

On Wed, Feb 3, 2010 at 12:03, H.J. Lu <hongjiu.lu@intel.com> wrote:
> Hi,
>
> Intel AVX has 256bit YMM registers. XMM registers from SSE are the
> aliases of the lower 128bit YMM registers. gdbserver on AVX machine
> may use 256bit vector registers, instead of 128bit vector registers,
> in the g/G packet.  When gdb talks to gdbserver, they need to negotiate
> to find out the maxium common register size supported by both gdb and
> gdbserver. I added `x86:xstate=BYTES:xcr0=VALUE' to qSupported:
>
> gdb will send
>
>    `x86:xstate=BYTES:xcr0=VALUE'
>          This feature indicates that GDB supports x86 XSAVE extended
>          state. BYTES specifies the maximum size in bytes of x86 XSAVE
>          extended state GDB supports. VALUE specifies the maximum
>          value of the extended control register 0 (the
>          XFEATURE_ENABLED_MASK register) GDB supports.  GDB does not
>          enable x86 XSAVE extended state support unless the stub also
>          reports that it supports them by including
>          `x86:xstate=BYTES:xcr0=VALUE' in its `qSupported' reply.
>          BYTES and VALUE are encoded as ASCII string in hexadecimal or
>          decimal numbers.
>
> in qSupported query.  The remote sub will send back
>
>    `x86:xstate=BYTES:xcr0=VALUE'
>          The remote stub supports x86 XSAVE extended state with the
>          extended state size in BYTES and the extended control
>          register 0 (the XFEATURE_ENABLED_MASK register) of VALUE.
>          BYTES and VALUE are encoded as ASCII string in hexadecimal or
>          decimal numbers.
>
> in qSupported reply.  gdb will use those values sent back by gdbserver
> to determine the proper vector size.
>
> This patch adds qsupported and qsupported_process_ack to gdbarch. My
> later AVX patch will use them.
>
> However, I couldn't find a way to store and pass those values to
> gdbarch used to commnunicate with remote stub. I wound up to use static
> variable in i386-tdep.c and added
>
>          /* Prepare qSupported ACK processing.  */
>          gdbarch_qsupported_process_ack (rs->gdbarch, NULL, NULL);
>
> in remote_query_supported so that I could clear the static variable
> before setting it with qSupported ACK.  Any comments? Does anyone have
> better ideas to accomplish what I need?
>
> Thanks.
>
>
> H.J.
> ----
> 2010-02-02  H.J. Lu  <hongjiu.lu@intel.com>
>
>        * gdbarch.c (gdbarch): Add qsupported and qsupported_process_ack.
>        (startup_gdbarch): Likewise.
>        (gdbarch_alloc): Likewise.
>        (verify_gdbarch): Likewise.
>        (gdbarch_dump): Likewise.
>        (gdbarch_qsupported): New.
>        (set_gdbarch_qsupported): Likewise.
>        (gdbarch_qsupported_process_ack): Likewise.
>        (set_gdbarch_qsupported_process_ack): Likewise.
>
>        * gdbarch.h (gdbarch_qsupported): New.
>        (set_gdbarch_qsupported): Likewise.
>        (gdbarch_qsupported_process_ack_ftype): Likewise.
>        (gdbarch_qsupported_process_ack): Likewise.
>        (set_gdbarch_qsupported_process_ack): Likewise.
>
>        * remote.c (remote_state): Add gdbarch.
>        (init_remote_state): Set gdbarch.
>        (remote_query_supported): Support gdbarch_qsupported and
>        gdbarch_qsupported_process_ack.
>
> diff --git a/gdb/gdbarch.c b/gdb/gdbarch.c
> index 6448fc3..4fd5d0a 100644
> --- a/gdb/gdbarch.c
> +++ b/gdb/gdbarch.c
> @@ -252,6 +252,8 @@ struct gdbarch
>   int has_global_breakpoints;
>   gdbarch_has_shared_address_space_ftype *has_shared_address_space;
>   gdbarch_fast_tracepoint_valid_at_ftype *fast_tracepoint_valid_at;
> +  const char *qsupported;
> +  gdbarch_qsupported_process_ack_ftype *qsupported_process_ack;
>  };
>
>
> @@ -395,6 +397,8 @@ struct gdbarch startup_gdbarch =
>   0,  /* has_global_breakpoints */
>   default_has_shared_address_space,  /* has_shared_address_space */
>   default_fast_tracepoint_valid_at,  /* fast_tracepoint_valid_at */
> +  0,  /* qsupported */
> +  0,  /* qsupported_process_ack */
>   /* startup_gdbarch() */
>  };
>
> @@ -481,6 +485,8 @@ gdbarch_alloc (const struct gdbarch_info *info,
>   gdbarch->target_signal_to_host = default_target_signal_to_host;
>   gdbarch->has_shared_address_space = default_has_shared_address_space;
>   gdbarch->fast_tracepoint_valid_at = default_fast_tracepoint_valid_at;
> +  gdbarch->qsupported = NULL;
> +  gdbarch->qsupported_process_ack = NULL;
>   /* gdbarch_alloc() */
>
>   return gdbarch;
> @@ -661,6 +667,8 @@ verify_gdbarch (struct gdbarch *gdbarch)
>   /* Skip verify of has_global_breakpoints, invalid_p == 0 */
>   /* Skip verify of has_shared_address_space, invalid_p == 0 */
>   /* Skip verify of fast_tracepoint_valid_at, invalid_p == 0 */
> +  /* Skip verify of qsuppoted, invalid_p == 0 */
> +  /* Skip verify of qsupported_process_ack, invalid_p == 0 */
>   buf = ui_file_xstrdup (log, &length);
>   make_cleanup (xfree, buf);
>   if (length > 0)
> @@ -1184,6 +1192,12 @@ gdbarch_dump (struct gdbarch *gdbarch, struct ui_file *file)
>   fprintf_unfiltered (file,
>                       "gdbarch_dump: write_pc = <%s>\n",
>                       host_address_to_string (gdbarch->write_pc));
> +  fprintf_unfiltered (file,
> +                      "gdbarch_dump: qsupported = <%s>\n",
> +                     gdbarch->qsupported);
> +  fprintf_unfiltered (file,
> +                      "gdbarch_dump: qsupported_process_ack = <%s>\n",
> +                      host_address_to_string (gdbarch->qsupported_process_ack));
>   if (gdbarch->dump_tdep != NULL)
>     gdbarch->dump_tdep (gdbarch, file);
>  }
> @@ -3576,6 +3590,41 @@ set_gdbarch_fast_tracepoint_valid_at (struct gdbarch *gdbarch,
>   gdbarch->fast_tracepoint_valid_at = fast_tracepoint_valid_at;
>  }
>
> +const char *
> +gdbarch_qsupported (struct gdbarch *gdbarch)
> +{
> +  gdb_assert (gdbarch != NULL);
> +  gdb_assert (gdbarch->qsupported != NULL);
> +  if (gdbarch_debug >= 2)
> +    fprintf_unfiltered (gdb_stdlog, "gdbarch_qsupported called\n");
> +  return gdbarch->qsupported;
> +}
> +
> +void
> +set_gdbarch_qsupported (struct gdbarch *gdbarch,
> +                       const char *qsupported)
> +{
> +  gdbarch->qsupported = qsupported;
> +}
> +
> +void
> +gdbarch_qsupported_process_ack (struct gdbarch *gdbarch,
> +                               const char *ack, const char *value)
> +{
> +  gdb_assert (gdbarch != NULL);
> +  if (gdbarch_debug >= 2)
> +    fprintf_unfiltered (gdb_stdlog, "gdbarch_qsupported_process_ack called\n");
> +  if (gdbarch->qsupported_process_ack != NULL)
> +    gdbarch->qsupported_process_ack (gdbarch, ack, value);
> +}
> +
> +void
> +set_gdbarch_qsupported_process_ack
> +  (struct gdbarch *gdbarch,
> +   gdbarch_qsupported_process_ack_ftype *qsupported_process_ack)
> +{
> +  gdbarch->qsupported_process_ack = qsupported_process_ack;
> +}
>
>  /* Keep a registry of per-architecture data-pointers required by GDB
>    modules. */
> diff --git a/gdb/gdbarch.h b/gdb/gdbarch.h
> index 661d34b..44e5244 100644
> --- a/gdb/gdbarch.h
> +++ b/gdb/gdbarch.h
> @@ -923,6 +923,23 @@ typedef int (gdbarch_fast_tracepoint_valid_at_ftype) (struct gdbarch *gdbarch, C
>  extern int gdbarch_fast_tracepoint_valid_at (struct gdbarch *gdbarch, CORE_ADDR addr, int *isize, char **msg);
>  extern void set_gdbarch_fast_tracepoint_valid_at (struct gdbarch *gdbarch, gdbarch_fast_tracepoint_valid_at_ftype *fast_tracepoint_valid_at);
>
> +/* Not NULL if a target has additonal field for qSupported.  */
> +
> +extern const char *gdbarch_qsupported (struct gdbarch *gdbarch);
> +extern void set_gdbarch_qsupported (struct gdbarch *gdbarch,
> +                                   const char *qsupported);
> +
> +/* Not NULL if a target has additonal process for qSupported ACK.  */
> +
> +typedef void (gdbarch_qsupported_process_ack_ftype)
> +  (struct gdbarch *gdbarch, const char *ack, const char *value);
> +extern void gdbarch_qsupported_process_ack (struct gdbarch *gdbarch,
> +                                           const char *ack,
> +                                           const char *value);
> +extern void set_gdbarch_qsupported_process_ack
> +  (struct gdbarch *gdbarch,
> +   gdbarch_qsupported_process_ack_ftype *qsupported_process_ack);
> +
>  /* Definition for an unknown syscall, used basically in error-cases.  */
>  #define UNKNOWN_SYSCALL (-1)
>
> diff --git a/gdb/remote.c b/gdb/remote.c
> index bf7568c..afb8f2c 100644
> --- a/gdb/remote.c
> +++ b/gdb/remote.c
> @@ -327,6 +327,9 @@ struct remote_state
>   /* Nonzero if the user has pressed Ctrl-C, but the target hasn't
>      responded to that.  */
>   int ctrlc_pending_p;
> +
> +  /* GDBARCH associated with this target.  */
> +  struct gdbarch *gdbarch;
>  };
>
>  /* Private data that we'll store in (struct thread_info)->private.  */
> @@ -566,6 +569,9 @@ init_remote_state (struct gdbarch *gdbarch)
>       rs->buf = xrealloc (rs->buf, rs->buf_size);
>     }
>
> +  /* Record our GDBARCH.  */
> +  rs->gdbarch = gdbarch;
> +
>   return rsa;
>  }
>
> @@ -3475,10 +3481,27 @@ remote_query_supported (void)
>   rs->buf[0] = 0;
>   if (remote_protocol_packets[PACKET_qSupported].support != PACKET_DISABLE)
>     {
> -      if (rs->extended)
> -       putpkt ("qSupported:multiprocess+");
> +      const char *qsupported = gdbarch_qsupported (rs->gdbarch);
> +      if (qsupported)
> +       {
> +         char *q;
> +         if (rs->extended)
> +           q = concat ("qSupported:multiprocess+;", qsupported, NULL);
> +         else
> +           q = concat ("qSupported:", qsupported, NULL);
> +         putpkt (q);
> +         free (q);
> +
> +         /* Prepare qSupported ACK processing.  */
> +         gdbarch_qsupported_process_ack (rs->gdbarch, NULL, NULL);
> +       }
>       else
> -       putpkt ("qSupported");
> +       {
> +         if (rs->extended)
> +           putpkt ("qSupported:multiprocess+");
> +         else
> +           putpkt ("qSupported");
> +       }
>
>       getpkt (&rs->buf, &rs->buf_size, 0);
>
> @@ -3564,6 +3587,9 @@ remote_query_supported (void)
>            feature->func (feature, is_supported, value);
>            break;
>          }
> +
> +      if (i >= ARRAY_SIZE (remote_protocol_features))
> +       gdbarch_qsupported_process_ack (rs->gdbarch, p, value);
>     }
>
>   /* If we increased the packet size, make sure to increase the global
>