Re: [rfc/rfa] [3/4] SPU enhancements: gdbserver support

["Ulrich Weigand" <uweigand at de dot ibm dot com>]

Daniel Jacobowitz wrote:
> On Mon, Jun 04, 2007 at 04:09:56PM -0400, Daniel Jacobowitz wrote:
> > On Mon, Jun 04, 2007 at 10:05:01PM +0200, Ulrich Weigand wrote:
> > > Fine with me.  I'd appreciate if you could post that code ...
> > 
> > I'll try to do this tomorrow morning.
> 
> There's not as much code for it as I remembered.

Thanks.  It looks like the require_data routine you posted is nearly
identical to the already existing remote_unescape_input routine, so
I just used that one.

The patch below should address both issues you raised.  OK?

Eli, as we're now not adding the LENGTH argument after all, there are
changes to the doc patch too.  Still OK?

Bye,
Ulrich


ChangeLog:

	* remote.c (remote_write_qxfer): New function.
	(remote_xfer_partial): Add handling for TARGET_OBJECT_SPU.
	(remote_read_qxfer): Do not cache empty objects.
	(_initialize_remote): Add PACKET_qXfer_spu_read and
	PACKET_qXfer_spu_write.

doc/ChangeLog:

	* gdb.texinfo (General Query Packets): Document qXfer:spu:read
	and qXfer:spu:write packets and mention them under qSupported.

gdbserver/ChangeLog:

	* remote-utils.c (decode_xfer_write): New function.
	* server.h (decode_xfer_write): Add prototype.
	* server.c (handle_query): Add PACKET_LEN argument.  Support
	qXfer:spu:read and qXfer:spu:write packets.
	(main): Pass packet_len to handle_query.
	* spu-low.c (spu_target_ops): Add spu_proc_xfer_spu.
	* target.h (target_ops): Add qxfer_spu.


diff -urNp gdb-orig/gdb/doc/gdb.texinfo gdb-head/gdb/doc/gdb.texinfo
--- gdb-orig/gdb/doc/gdb.texinfo	2007-06-06 00:07:51.480545727 +0200
+++ gdb-head/gdb/doc/gdb.texinfo	2007-06-05 23:55:34.223883541 +0200
@@ -23628,6 +23628,16 @@ These are the currently defined stub fea
 @tab @samp{-}
 @tab Yes
 
+@item @samp{qXfer:spu:read}
+@tab No
+@tab @samp{-}
+@tab Yes
+
+@item @samp{qXfer:spu:write}
+@tab No
+@tab @samp{-}
+@tab Yes
+
 @item @samp{QPassSignals}
 @tab No
 @tab @samp{-}
@@ -23661,6 +23671,14 @@ The remote stub understands the @samp{qX
 The remote stub understands the @samp{qXfer:memory-map:read} packet
 (@pxref{qXfer memory map read}).
 
+@item qXfer:spu:read
+The remote stub understands the @samp{qXfer:spu:read} packet
+(@pxref{qXfer spu read}).
+
+@item qXfer:spu:write
+The remote stub understands the @samp{qXfer:spu:write} packet
+(@pxref{qXfer spu write}).
+
 @item QPassSignals
 The remote stub understands the @samp{QPassSignals} packet
 (@pxref{QPassSignals}).
@@ -23746,7 +23764,7 @@ packets.)
 Read uninterpreted bytes from the target's special data area
 identified by the keyword @var{object}.  Request @var{length} bytes
 starting at @var{offset} bytes into the data.  The content and
-encoding of @var{annex} is specific to the object; it can supply
+encoding of @var{annex} is specific to @var{object}; it can supply
 additional details about what data to access.
 
 Here are the specific requests of this form defined so far.  All
@@ -23779,6 +23797,17 @@ annex part of the generic @samp{qXfer} p
 
 This packet is not probed by default; the remote stub must request it,
 by supplying an appropriate @samp{qSupported} response (@pxref{qSupported}).
+
+@item qXfer:spu:read:@var{annex}:@var{offset},@var{length}
+@anchor{qXfer spu read}
+Read contents of an @code{spufs} file on the target system.  The
+annex specifies which file to read; it must be of the form 
+@file{@var{id}/@var{name}}, where @var{id} specifies an SPU context ID
+in the target process, and @var{name} identifes the @code{spufs} file
+in that context to be accessed.
+
+This packet is not probed by default; the remote stub must request it,
+by supplying an appropriate @samp{qSupported} response (@pxref{qSupported}).
 @end table
 
 Reply:
@@ -23816,14 +23845,27 @@ the stub, or that the object does not su
 @cindex write data into object, remote request
 Write uninterpreted bytes into the target's special data area
 identified by the keyword @var{object}, starting at @var{offset} bytes
-into the data.  @samp{@var{data}@dots{}} is the binary-encoded data
+into the data.  @var{data}@dots{} is the binary-encoded data
 (@pxref{Binary Data}) to be written.  The content and encoding of @var{annex}
-is specific to the object; it can supply additional details about what data
+is specific to @var{object}; it can supply additional details about what data
 to access.
 
-No requests of this form are presently in use.  This specification
-serves as a placeholder to document the common format that new
-specific request specifications ought to use.
+Here are the specific requests of this form defined so far.  All
+@samp{qXfer:@var{object}:write:@dots{}} requests use the same reply
+formats, listed below.
+
+@table @samp
+@item qXfer:@var{spu}:write:@var{annex}:@var{offset}:@var{data}@dots{}
+@anchor{qXfer spu write}
+Write @var{data} to an @code{spufs} file on the target system.  The
+annex specifies which file to write; it must be of the form
+@file{@var{id}/@var{name}}, where @var{id} specifies an SPU context ID
+in the target process, and @var{name} identifes the @code{spufs} file
+in that context to be accessed.
+
+This packet is not probed by default; the remote stub must request it,
+by supplying an appropriate @samp{qSupported} response (@pxref{qSupported}).
+@end table
 
 Reply:
 @table @samp
diff -urNp gdb-orig/gdb/gdbserver/remote-utils.c gdb-head/gdb/gdbserver/remote-utils.c
--- gdb-orig/gdb/gdbserver/remote-utils.c	2007-06-06 00:07:51.525539187 +0200
+++ gdb-head/gdb/gdbserver/remote-utils.c	2007-06-06 00:14:13.513842788 +0200
@@ -1042,6 +1042,36 @@ decode_X_packet (char *from, int packet_
   return 0;
 }
 
+/* Decode a qXfer write request.  */
+int
+decode_xfer_write (char *buf, int packet_len, char **annex, CORE_ADDR *offset,
+		   unsigned int *len, unsigned char *data)
+{
+  char ch;
+
+  /* Extract and NUL-terminate the annex.  */
+  *annex = buf;
+  while (*buf && *buf != ':')
+    buf++;
+  if (*buf == '\0')
+    return -1;
+  *buf++ = 0;
+
+  /* Extract the offset.  */
+  *offset = 0;
+  while ((ch = *buf++) != ':')
+    {
+      *offset = *offset << 4;
+      *offset |= fromhex (ch) & 0x0f;
+    }
+
+  /* Get encoded data.  */
+  packet_len -= buf - *annex;
+  *len = remote_unescape_input ((const gdb_byte *) buf, packet_len,
+				data, packet_len);
+  return 0;
+}
+
 /* Ask GDB for the address of NAME, and return it in ADDRP if found.
    Returns 1 if the symbol is found, 0 if it is not, -1 on error.  */
 
diff -urNp gdb-orig/gdb/gdbserver/server.c gdb-head/gdb/gdbserver/server.c
--- gdb-orig/gdb/gdbserver/server.c	2007-06-06 00:07:51.531538315 +0200
+++ gdb-head/gdb/gdbserver/server.c	2007-06-05 23:59:35.521561947 +0200
@@ -141,7 +141,7 @@ decode_xfer_read (char *buf, char **anne
     return -1;
   *buf++ = 0;
 
-  /* After the read/write marker and annex, qXfer looks like a
+  /* After the read marker and annex, qXfer looks like a
      traditional 'm' packet.  */
   decode_m_packet (buf, ofs, len);
 
@@ -255,7 +255,7 @@ monitor_show_help (void)
 
 /* Handle all of the extended 'q' packets.  */
 void
-handle_query (char *own_buf, int *new_packet_len_p)
+handle_query (char *own_buf, int packet_len, int *new_packet_len_p)
 {
   static struct inferior_list_entry *thread_ptr;
 
@@ -314,6 +314,69 @@ handle_query (char *own_buf, int *new_pa
       return;
     }
 
+  if (the_target->qxfer_spu != NULL
+      && strncmp ("qXfer:spu:read:", own_buf, 15) == 0)
+    {
+      char *annex;
+      int n;
+      unsigned int len;
+      CORE_ADDR ofs;
+      unsigned char *spu_buf;
+
+      strcpy (own_buf, "E00");
+      if (decode_xfer_read (own_buf + 15, &annex, &ofs, &len) < 0)
+	  return;
+      if (len > PBUFSIZ - 2)
+	len = PBUFSIZ - 2;
+      spu_buf = malloc (len + 1);
+      if (!spu_buf)
+        return;
+
+      n = (*the_target->qxfer_spu) (annex, spu_buf, NULL, ofs, len + 1);
+      if (n < 0) 
+	write_enn (own_buf);
+      else if (n > len)
+	*new_packet_len_p = write_qxfer_response
+			      (own_buf, spu_buf, len, 1);
+      else 
+	*new_packet_len_p = write_qxfer_response
+			      (own_buf, spu_buf, n, 0);
+
+      free (spu_buf);
+      return;
+    }
+
+  if (the_target->qxfer_spu != NULL
+      && strncmp ("qXfer:spu:write:", own_buf, 16) == 0)
+    {
+      char *annex;
+      int n;
+      unsigned int len;
+      CORE_ADDR ofs;
+      unsigned char *spu_buf;
+
+      strcpy (own_buf, "E00");
+      spu_buf = malloc (packet_len - 15);
+      if (!spu_buf)
+        return;
+      if (decode_xfer_write (own_buf + 16, packet_len - 16, &annex,
+			     &ofs, &len, spu_buf) < 0)
+	{
+	  free (spu_buf);
+	  return;
+	}
+
+      n = (*the_target->qxfer_spu) 
+	(annex, NULL, (unsigned const char *)spu_buf, ofs, len);
+      if (n < 0)
+	write_enn (own_buf);
+      else
+	sprintf (own_buf, "%x", n);
+
+      free (spu_buf);
+      return;
+    }
+
   if (the_target->read_auxv != NULL
       && strncmp ("qXfer:auxv:read:", own_buf, 16) == 0)
     {
@@ -403,6 +466,9 @@ handle_query (char *own_buf, int *new_pa
 
       if (the_target->read_auxv != NULL)
 	strcat (own_buf, ";qXfer:auxv:read+");
+     
+      if (the_target->qxfer_spu != NULL)
+	strcat (own_buf, ";qXfer:spu:read+;qXfer:spu:write+");
 
       if (get_features_xml ("target.xml") != NULL)
 	strcat (own_buf, ";qXfer:features:read+");
@@ -809,7 +875,7 @@ main (int argc, char *argv[])
 	  switch (ch)
 	    {
 	    case 'q':
-	      handle_query (own_buf, &new_packet_len);
+	      handle_query (own_buf, packet_len, &new_packet_len);
 	      break;
 	    case 'Q':
 	      handle_general_set (own_buf);
diff -urNp gdb-orig/gdb/gdbserver/server.h gdb-head/gdb/gdbserver/server.h
--- gdb-orig/gdb/gdbserver/server.h	2007-06-06 00:07:51.535537734 +0200
+++ gdb-head/gdb/gdbserver/server.h	2007-06-06 00:01:47.679381837 +0200
@@ -176,6 +176,9 @@ void decode_M_packet (char *from, CORE_A
 		      unsigned int *len_ptr, unsigned char *to);
 int decode_X_packet (char *from, int packet_len, CORE_ADDR * mem_addr_ptr,
 		     unsigned int *len_ptr, unsigned char *to);
+int decode_xfer_write (char *buf, int packet_len, char **annex,
+		       CORE_ADDR *offset, unsigned int *len,
+		       unsigned char *data);
 
 int unhexify (char *bin, const char *hex, int count);
 int hexify (char *hex, const char *bin, int count);
diff -urNp gdb-orig/gdb/gdbserver/spu-low.c gdb-head/gdb/gdbserver/spu-low.c
--- gdb-orig/gdb/gdbserver/spu-low.c	2007-06-06 00:07:51.539537153 +0200
+++ gdb-head/gdb/gdbserver/spu-low.c	2007-06-05 23:55:34.249879776 +0200
@@ -574,7 +574,6 @@ spu_arch_string (void)
   return "spu";
 }
 
-
 static struct target_ops spu_target_ops = {
   spu_create_inferior,
   spu_attach,
@@ -598,6 +597,7 @@ static struct target_ops spu_target_ops 
   NULL,
   NULL,
   spu_arch_string,
+  spu_proc_xfer_spu,
 };
 
 void
diff -urNp gdb-orig/gdb/gdbserver/target.h gdb-head/gdb/gdbserver/target.h
--- gdb-orig/gdb/gdbserver/target.h	2007-06-06 00:07:51.543536571 +0200
+++ gdb-head/gdb/gdbserver/target.h	2007-06-05 23:55:34.254879053 +0200
@@ -185,6 +185,10 @@ struct target_ops
   /* Return a string identifying the current architecture, or NULL if
      this operation is not supported.  */
   const char *(*arch_string) (void);
+
+   /* Read/Write from/to spufs using qXfer packets.  */
+  int (*qxfer_spu) (const char *annex, unsigned char *readbuf,
+		    unsigned const char *writebuf, CORE_ADDR offset, int len);
 };
 
 extern struct target_ops *the_target;
diff -urNp gdb-orig/gdb/remote.c gdb-head/gdb/remote.c
--- gdb-orig/gdb/remote.c	2007-06-06 00:07:51.557534537 +0200
+++ gdb-head/gdb/remote.c	2007-06-05 23:55:34.272011276 +0200
@@ -907,6 +907,8 @@ enum {
   PACKET_qXfer_auxv,
   PACKET_qXfer_features,
   PACKET_qXfer_memory_map,
+  PACKET_qXfer_spu_read,
+  PACKET_qXfer_spu_write,
   PACKET_qGetTLSAddr,
   PACKET_qSupported,
   PACKET_QPassSignals,
@@ -5517,6 +5519,45 @@ the loaded file\n"));
     printf_filtered (_("No loaded section named '%s'.\n"), args);
 }
 
+/* Write LEN bytes from WRITEBUF into OBJECT_NAME/ANNEX at OFFSET
+   into remote target.  The number of bytes written to the remote
+   target is returned, or -1 for error.  */
+
+static LONGEST
+remote_write_qxfer (struct target_ops *ops, const char *object_name,
+                    const char *annex, const gdb_byte *writebuf, 
+                    ULONGEST offset, LONGEST len, 
+                    struct packet_config *packet)
+{
+  int i, buf_len;
+  ULONGEST n;
+  gdb_byte *wbuf;
+  struct remote_state *rs = get_remote_state ();
+  int max_size = get_memory_write_packet_size (); 
+
+  if (packet->support == PACKET_DISABLE)
+    return -1;
+
+  /* Insert header.  */
+  i = snprintf (rs->buf, max_size, 
+		"qXfer:%s:write:%s:%s:",
+		object_name, annex ? annex : "",
+		phex_nz (offset, sizeof offset));
+  max_size -= (i + 1);
+
+  /* Escape as much data as fits into rs->buf.  */
+  buf_len = remote_escape_output 
+    (writebuf, len, (rs->buf + i), &max_size, max_size);
+
+  if (putpkt_binary (rs->buf, i + buf_len) < 0
+      || getpkt_sane (&rs->buf, &rs->buf_size, 0) < 0
+      || packet_ok (rs->buf, packet) != PACKET_OK)
+    return -1;
+
+  unpack_varlen_hex (rs->buf, &n);
+  return n;
+}
+
 /* Read OBJECT_NAME/ANNEX from the remote target using a qXfer packet.
    Data at OFFSET, of up to LEN bytes, is read into READBUF; the
    number of bytes read is returned, or 0 for EOF, or -1 for error.
@@ -5589,9 +5630,9 @@ remote_read_qxfer (struct target_ops *op
   i = remote_unescape_input (rs->buf + 1, packet_len - 1, readbuf, n);
 
   /* 'l' is an EOF marker, possibly including a final block of data,
-     or possibly empty.  Record it to bypass the next read, if one is
-     issued.  */
-  if (rs->buf[0] == 'l')
+     or possibly empty.  If we have the final block of a non-empty
+     object, record this fact to bypass a subsequent partial read.  */
+  if (rs->buf[0] == 'l' && offset + i > 0)
     {
       finished_object = xstrdup (object_name);
       finished_annex = xstrdup (annex ? annex : "");
@@ -5630,6 +5671,19 @@ remote_xfer_partial (struct target_ops *
 	return -1;
     }
 
+  /* Handle SPU memory using qxfer packets. */
+  if (object == TARGET_OBJECT_SPU)
+    {
+      if (readbuf)
+	return remote_read_qxfer (ops, "spu", annex, readbuf, offset, len,
+				  &remote_protocol_packets
+				    [PACKET_qXfer_spu_read]);
+      else
+	return remote_write_qxfer (ops, "spu", annex, writebuf, offset, len,
+				   &remote_protocol_packets
+				     [PACKET_qXfer_spu_write]);
+    }
+
   /* Only handle flash writes.  */
   if (writebuf != NULL)
     {
@@ -6540,6 +6594,12 @@ Show the maximum size of the address (in
   add_packet_config_cmd (&remote_protocol_packets[PACKET_qXfer_memory_map],
 			 "qXfer:memory-map:read", "memory-map", 0);
 
+  add_packet_config_cmd (&remote_protocol_packets[PACKET_qXfer_spu_read],
+                         "qXfer:spu:read", "read-spu-object", 0);
+
+  add_packet_config_cmd (&remote_protocol_packets[PACKET_qXfer_spu_write],
+                         "qXfer:spu:write", "write-spu-object", 0);
+
   add_packet_config_cmd (&remote_protocol_packets[PACKET_qGetTLSAddr],
 			 "qGetTLSAddr", "get-thread-local-storage-address",
 			 0);



-- 
  Dr. Ulrich Weigand
  GNU Toolchain for Linux on System z and Cell BE
  Ulrich.Weigand@de.ibm.com