Re: RFC: Support target specific qSupported

[Hui Zhu <teawater at gmail dot 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
>