Re: RFC: Support target specific qSupported

["H.J. Lu" <hjl.tools@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 1569 bytes --]

On Wed, Feb 3, 2010 at 6:46 AM, Daniel Jacobowitz <dan@codesourcery.com> wrote:
> On Wed, Feb 03, 2010 at 06:34:14AM -0800, H.J. Lu wrote:
>> We have our own remote gdb stub, which needs to talk to both old gdb,
>> which only understands SSE g/G packet, and new gdb, which understands
>> AVX g/G packet. Does the target description support negotiation so
>> that old gdb and new remote gdb stub can use SSE g/G packet?
>
> The goal of target descriptions is to not need negotiation.
> Everything is controlled by the target.  But with older GDBs, because
> x86 did not get target-described register support right away, there's
> a problem.

It is not a trivial to implement target-described register support
for x86. But I think it is a good thing to do.

> I suggest adding something to the GDB-side qSupported packet saying
> that AVX is OK.  You don't need anything on the stub side of the
> qSupported reply; you just need to reply to qXfer:features.
>

How about this patch? It allows a target to add a field to qSupported.

Thanks.

-- 
H.J.
---
2010-02-03  H.J. Lu  <hongjiu.lu@intel.com>

	* gdbarch.c (gdbarch): Add qsupported.
	(startup_gdbarch): Likewise.
	(gdbarch_alloc): Likewise.
	(verify_gdbarch): Likewise.
	(gdbarch_dump): Likewise.
	(gdbarch_qsupported): New.
	(set_gdbarch_qsupported): Likewise.

	* gdbarch.h (gdbarch_qsupported): New.
	(set_gdbarch_qsupported): Likewise.

	* remote.c (remote_state): Add gdbarch.
	(init_remote_state): Set gdbarch.
	(remote_query_supported): Support gdbarch_qsupported.

[-- Attachment #2: gdb-qsupported-2.patch --]
[-- Type: text/plain, Size: 5086 bytes --]

2010-02-03  H.J. Lu  <hongjiu.lu@intel.com>

	* gdbarch.c (gdbarch): Add qsupported.
	(startup_gdbarch): Likewise.
	(gdbarch_alloc): Likewise.
	(verify_gdbarch): Likewise.
	(gdbarch_dump): Likewise.
	(gdbarch_qsupported): New.
	(set_gdbarch_qsupported): Likewise.

	* gdbarch.h (gdbarch_qsupported): New.
	(set_gdbarch_qsupported): Likewise.

	* remote.c (remote_state): Add gdbarch.
	(init_remote_state): Set gdbarch.
	(remote_query_supported): Support gdbarch_qsupported.

diff --git a/gdb/gdbarch.c b/gdb/gdbarch.c
index 6448fc3..f54a181 100644
--- a/gdb/gdbarch.c
+++ b/gdb/gdbarch.c
@@ -252,6 +252,7 @@ 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;
 };
 
 
@@ -395,6 +396,7 @@ 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 */
   /* startup_gdbarch() */
 };
 
@@ -481,6 +483,7 @@ 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_alloc() */
 
   return gdbarch;
@@ -661,6 +664,7 @@ 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 */
   buf = ui_file_xstrdup (log, &length);
   make_cleanup (xfree, buf);
   if (length > 0)
@@ -1184,6 +1188,9 @@ 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);
   if (gdbarch->dump_tdep != NULL)
     gdbarch->dump_tdep (gdbarch, file);
 }
@@ -3576,6 +3583,22 @@ 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;
+}
 
 /* Keep a registry of per-architecture data-pointers required by GDB
    modules. */
diff --git a/gdb/gdbarch.h b/gdb/gdbarch.h
index 661d34b..e77f027 100644
--- a/gdb/gdbarch.h
+++ b/gdb/gdbarch.h
@@ -923,6 +923,12 @@ 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);
+
 /* 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..e091d4b 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,24 @@ 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);
+	}
       else
-	putpkt ("qSupported");
+	{
+	  if (rs->extended)
+	    putpkt ("qSupported:multiprocess+");
+	  else
+	    putpkt ("qSupported");
+	}
 
       getpkt (&rs->buf, &rs->buf_size, 0);