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

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

[Ulrich Weigand]

Hello,

this patch, implemented by Markus Deuling, extends TARGET_OBJECT_SPU
support to remote gdbserver debugging.

This is done by extended the remote protocol by two new packet types,
qXfer:spu:read and qXfer:spu:write.  In addition, the patch contains
the following changes to remote packet support:

- It adds a "length" argument to qXfer::write type packets.  This is
  to make parsing of the packet easier, and to bring it in line with
  the 'X' packet format.  (It also provides a bit of extra redundancy
  to detect transmission problems.)

- The remote_read_qxfer attempts to cache received end-of-object
  packets.  However, this is a problem for some spufs objects as
  they can start out with no content (length zero), and acquire
  actual content later on.  If remote_read_qxfer has already cache
  an end-of-object at lenght zero, future re-reads of the object
  will always return zero even if the object by now has actual
  content.  To fix this we've added a CACHEABLE flag to the
  function and use it to disable the cache for spufs objects.

Tested on spu-elf.

Eli, are the doc changes OK?

OK to apply?

Bye,
Ulrich


ChangeLog:

	* remote.c (remote_write_qxfer): New function.
	(remote_xfer_partial): Add handling for TARGET_OBJECT_SPU.
	(remote_read_qxfer): Add cacheable flag.
	(_initialize_remote): Add PACKET_qXfer_spu_read and
	PACKET_qXfer_spu_write.

doc/ChangeLog:

	* gdb.texinfo (General Query Packets): Add length to
	description of qXfer::write packet format.  Document
	qXfer:spu:read and qXfer:spu:write packets and mention
	them under qSupported.	

gdbserver/ChangeLog:

	* server.c (decode_xfer_write): New function.
	(handle_query): Add support for qXfer:spu:read/write packets.
	* 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-05-31 18:54:31.000000000 +0200
+++ gdb-head/gdb/doc/gdb.texinfo	2007-06-02 19:32:26.068097686 +0200
@@ -23590,6 +23590,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{-}
@@ -23623,6 +23633,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}).
@@ -23724,6 +23742,19 @@ auxiliary vector}.  Note @var{annex} mus
 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}
+@cindex spufs
+@cindex SPU 
+Read contents of an @code{spufs} file on the target system.  The
+annex specifies which file to read; it must be of the form 
+@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}).
+
 @item qXfer:features:read:@var{annex}:@var{offset},@var{length}
 @anchor{qXfer target description read}
 Access the @dfn{target description}.  @xref{Target Descriptions}.  The
@@ -23774,18 +23805,29 @@ An empty reply indicates the @var{object
 the stub, or that the object does not support reading.
 @end table
 
-@item qXfer:@var{object}:write:@var{annex}:@var{offset}:@var{data}@dots{}
+@item qXfer:@var{object}:write:@var{annex}:@var{offset},@var{length}:@var{data}@dots{}
 @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
-(@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
-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.
+Write @var{length} bytes of uninterpreted data 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 (@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 to access.
+
+@table @samp
+@item qXfer:@var{spu}:write:@var{annex}:@var{offset},@var{length}:@var{data}@dots{}
+@anchor{qXfer spu write}
+@cindex spufs
+@cindex SPU
+Write @var{length} bytes of @var{data} to an @code{spufs} file on 
+the target system.  The annex specifies which file to write; it must 
+be of the form @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/server.c gdb-head/gdb/gdbserver/server.c
--- gdb-orig/gdb/gdbserver/server.c	2007-05-16 17:25:22.000000000 +0200
+++ gdb-head/gdb/gdbserver/server.c	2007-06-02 19:32:10.732047243 +0200
@@ -141,13 +141,32 @@ 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);
 
   return 0;
 }
 
+/* Decode a qXfer write request.  Return 0 if everything looks OK,
+   or -1 otherwise.  */
+static int
+decode_xfer_write (char *buf, int packet_len, char **annex, CORE_ADDR *ofs, 
+                   unsigned int *len, unsigned char *data)
+{
+  /* Extract and NUL-terminate the annex.  */
+  *annex = buf;
+  while (*buf && *buf != ':')
+    buf++;
+  if (*buf == '\0')
+    return -1;
+  *buf++ = 0;
+
+  /* After the write marker and annex, qXfer looks like
+     a traditional 'X' packet.  */
+  return decode_X_packet (buf, packet_len - strlen (*annex) - 1, ofs, len, data);
+}
+
 /* Write the response to a successful qXfer read.  Returns the
    length of the (binary) data stored in BUF, corresponding
    to as much of DATA/LEN as we could fit.  IS_MORE controls
@@ -255,7 +274,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 +333,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 +485,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 +894,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/spu-low.c gdb-head/gdb/gdbserver/spu-low.c
--- gdb-orig/gdb/gdbserver/spu-low.c	2007-06-01 22:31:04.000000000 +0200
+++ gdb-head/gdb/gdbserver/spu-low.c	2007-06-02 19:32:10.738046378 +0200
@@ -574,7 +574,6 @@ spu_arch_string (void)
   return "spu";
 }
 
-\f
 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-05-10 23:34:34.000000000 +0200
+++ gdb-head/gdb/gdbserver/target.h	2007-06-02 19:32:10.751044505 +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-01 18:51:01.000000000 +0200
+++ gdb-head/gdb/remote.c	2007-06-02 19:32:10.810036003 +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,18 +5519,59 @@ 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,%s:",
+		object_name, annex ? annex : "",
+		phex_nz (offset, sizeof offset),
+		phex_nz (len, sizeof len));
+  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.
    The number of bytes read may be less than LEN without indicating an
    EOF.  PACKET is checked and updated to indicate whether the remote
-   target supports this object.  */
+   target supports this object.  If CACHEABLE is set then the packet
+   is cached.  */
 
 static LONGEST
 remote_read_qxfer (struct target_ops *ops, const char *object_name,
 		   const char *annex,
 		   gdb_byte *readbuf, ULONGEST offset, LONGEST len,
-		   struct packet_config *packet)
+		   struct packet_config *packet, int cacheable)
 {
   static char *finished_object;
   static char *finished_annex;
@@ -5591,7 +5634,8 @@ remote_read_qxfer (struct target_ops *op
   /* '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')
+  if (rs->buf[0] == 'l'
+      && cacheable)
     {
       finished_object = xstrdup (object_name);
       finished_annex = xstrdup (annex ? annex : "");
@@ -5630,6 +5674,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], 0);
+      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)
     {
@@ -5663,22 +5720,21 @@ remote_xfer_partial (struct target_ops *
     case TARGET_OBJECT_AUXV:
       gdb_assert (annex == NULL);
       return remote_read_qxfer (ops, "auxv", annex, readbuf, offset, len,
-				&remote_protocol_packets[PACKET_qXfer_auxv]);
+				&remote_protocol_packets[PACKET_qXfer_auxv], 1);
 
     case TARGET_OBJECT_AVAILABLE_FEATURES:
       return remote_read_qxfer
 	(ops, "features", annex, readbuf, offset, len,
-	 &remote_protocol_packets[PACKET_qXfer_features]);
+	 &remote_protocol_packets[PACKET_qXfer_features], 1);
 
     case TARGET_OBJECT_MEMORY_MAP:
       gdb_assert (annex == NULL);
       return remote_read_qxfer (ops, "memory-map", annex, readbuf, offset, len,
-				&remote_protocol_packets[PACKET_qXfer_memory_map]);
+				&remote_protocol_packets[PACKET_qXfer_memory_map], 1);
 
     default:
       return -1;
     }
-
   /* Note: a zero OFFSET and LEN can be used to query the minimum
      buffer size.  */
   if (offset == 0 && len == 0)
@@ -6540,6 +6596,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

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

[Eli Zaretskii]

> Date: Sat, 2 Jun 2007 21:33:29 +0200 (CEST)
> From: "Ulrich Weigand" <uweigand@de.ibm.com>
> Cc: eliz@gnu.org, drow@false.org
> 
> Eli, are the doc changes OK?

Yes, but see some comments below.

> +@item qXfer:spu:read:@var{annex}:@var{offset},@var{length}
> +@anchor{qXfer spu read}
> +@cindex spufs
> +@cindex SPU 

You already added a "@cindex SPU" entry in your previous patch.  This
is the second index entry with exactly the same name, which is not a
very good idea: a reader looking at the index will not know which one
to choose.  It is better to use an entry qualified by its context, for
example:

  @cindex SPU, read @code{spufs} files

> +Read contents of an @code{spufs} file on the target system.  The
> +annex specifies which file to read; it must be of the form 
> +@var{id}/@var{name}, where @var{id} specifies an SPU context ID

Since you are talking about a file, you should use the @file markup:

  +@file{@var{id}/@var{name}}, where @var{id} specifies an SPU context ID

> -@item qXfer:@var{object}:write:@var{annex}:@var{offset}:@var{data}@dots{}
> +@item qXfer:@var{object}:write:@var{annex}:@var{offset},@var{length}:@var{data}@dots{}

Won't this change break backward compatibility?

> +Write @var{length} bytes of uninterpreted data into the target's
> +special data area identified by the keyword @var{object}, starting

What do you mean by ``keyword''?  Isn't @var{object} a _name_ of an
object or its symbol?

> +at @var{offset} bytes into the data.  @samp{@var{data}@dots{}} is
                                         ^^^^^^^^^^^^^^^^^^^^^^
Why use @samp here?  Does it do anything useful?

> +the binary-encoded data (@pxref{Binary Data}) to be written.  The
> +content and encoding of @var{annex} is specific to the object; it can
                                       ^^^^^^^^^^^^^^^^^^^^^^^^^
Better say "... is specific to @var{object}".

> +@table @samp
> +@item qXfer:@var{spu}:write:@var{annex}:@var{offset},@var{length}:@var{data}@dots{}
> +@anchor{qXfer spu write}
> +@cindex spufs
> +@cindex SPU

See above about "@cindex SPU".

> +be of the form @var{id}/@var{name}, where @var{id} specifies an SPU context ID
                  ^^^^^^^^^^^^^^^^^^^
Likewise, please use @file.

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

[Ulrich Weigand]

Eli Zareteskii wrote:

> You already added a "@cindex SPU" entry in your previous patch.  This
> is the second index entry with exactly the same name, which is not a
> very good idea: a reader looking at the index will not know which one
> to choose.  It is better to use an entry qualified by its context, for
> example:
> 
>   @cindex SPU, read @code{spufs} files

Right.  Actually, I think we probably don't even need an index entry
for that at all ...   (The other qXfer packets don't have index entries
either.)

> > +Read contents of an @code{spufs} file on the target system.  The
> > +annex specifies which file to read; it must be of the form 
> > +@var{id}/@var{name}, where @var{id} specifies an SPU context ID
> 
> Since you are talking about a file, you should use the @file markup:
> 
>   +@file{@var{id}/@var{name}}, where @var{id} specifies an SPU context ID

OK.

> > -@item qXfer:@var{object}:write:@var{annex}:@var{offset}:@var{data}@dots{}
> > +@item qXfer:@var{object}:write:@var{annex}:@var{offset},@var{length}:@var{data}@dots{}
> 
> Won't this change break backward compatibility?

Not really, since there currently are no qXfer::write packets.  This was
simply a generic entry that described how potential such packets might 
look like; now that we've actually implemented the first real packet, it 
seemed better to add the LENGTH.

> > +Write @var{length} bytes of uninterpreted data into the target's
> > +special data area identified by the keyword @var{object}, starting
> 
> What do you mean by ``keyword''?  Isn't @var{object} a _name_ of an
> object or its symbol?

This is just copied from the pre-existing text (also present in the
qXfer::read section).  In any case, I think "keyword" is right here,
as you can only use one of the defined strings ("auxv", "memory-map",
"spu", ...) as "object".

> > +at @var{offset} bytes into the data.  @samp{@var{data}@dots{}} is
>                                          ^^^^^^^^^^^^^^^^^^^^^^
> Why use @samp here?  Does it do anything useful?

Not really, this again was copied from pre-existing text.  I've 
removed the @samp now.

> > +the binary-encoded data (@pxref{Binary Data}) to be written.  The
> > +content and encoding of @var{annex} is specific to the object; it can
>                                        ^^^^^^^^^^^^^^^^^^^^^^^^^
> Better say "... is specific to @var{object}".

OK.  I've made the same change at the corresponding line in the qXfer::read
description.


Is the following version OK?

Bye,
Ulrich

diff -urNp gdb-orig/gdb/doc/gdb.texinfo gdb-head/gdb/doc/gdb.texinfo
--- gdb-orig/gdb/doc/gdb.texinfo	2007-05-31 18:54:31.000000000 +0200
+++ gdb-head/gdb/doc/gdb.texinfo	2007-06-03 14:44:31.860732882 +0200
@@ -23590,6 +23590,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{-}
@@ -23623,6 +23633,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}).
@@ -23708,7 +23726,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
@@ -23741,6 +23759,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:
@@ -23774,18 +23803,31 @@ An empty reply indicates the @var{object
 the stub, or that the object does not support reading.
 @end table
 
-@item qXfer:@var{object}:write:@var{annex}:@var{offset}:@var{data}@dots{}
+@item qXfer:@var{object}:write:@var{annex}:@var{offset},@var{length}:@var{data}@dots{}
 @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
-(@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
-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.
+Write @var{length} bytes of uninterpreted data into the target's
+special data area identified by the keyword @var{object}, starting
+at @var{offset} bytes 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 @var{object}; it can
+supply additional details about what data to access.
+
+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{length}:@var{data}@dots{}
+@anchor{qXfer spu write}
+Write @var{length} bytes of @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


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

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

[Eli Zaretskii]

> Date: Sun, 3 Jun 2007 15:28:18 +0200 (CEST)
> From: "Ulrich Weigand" <uweigand@de.ibm.com>
> Cc: gdb-patches@sourceware.org, drow@false.org
> 
> > > +Write @var{length} bytes of uninterpreted data into the target's
> > > +special data area identified by the keyword @var{object}, starting
> > 
> > What do you mean by ``keyword''?  Isn't @var{object} a _name_ of an
> > object or its symbol?
> 
> This is just copied from the pre-existing text (also present in the
> qXfer::read section).  In any case, I think "keyword" is right here,
> as you can only use one of the defined strings ("auxv", "memory-map",
> "spu", ...) as "object".

Then please say something like this:

  (@var{object} can be @samp{auxv}, @samp{memory-map}, @samp{spu}...)

> Is the following version OK?

Yes, but please add the above.

Thanks.

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

[Ulrich Weigand]

Eli Zaretskii wrote:

> > > What do you mean by ``keyword''?  Isn't @var{object} a _name_ of an
> > > object or its symbol?
> > 
> > This is just copied from the pre-existing text (also present in the
> > qXfer::read section).  In any case, I think "keyword" is right here,
> > as you can only use one of the defined strings ("auxv", "memory-map",
> > "spu", ...) as "object".
> 
> Then please say something like this:
> 
>   (@var{object} can be @samp{auxv}, @samp{memory-map}, @samp{spu}...)

I guess I may be misunderstanding something, but already today the
paragraph immediately following the quoted sentence says:

"Here are the specific requests of this form defined so far."

and then goes on to describe in detail the auxv, memory-map, etc.
variants.

What would be the benefit of repeating that list once again?

Bye,
Ulrich

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

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

[Eli Zaretskii]

> Date: Mon, 4 Jun 2007 15:58:07 +0200 (CEST)
> From: "Ulrich Weigand" <uweigand@de.ibm.com>
> Cc: gdb-patches@sourceware.org, drow@false.org
> 
> > Then please say something like this:
> > 
> >   (@var{object} can be @samp{auxv}, @samp{memory-map}, @samp{spu}...)
> 
> I guess I may be misunderstanding something, but already today the
> paragraph immediately following the quoted sentence says:
> 
> "Here are the specific requests of this form defined so far."
> 
> and then goes on to describe in detail the auxv, memory-map, etc.
> variants.
> 
> What would be the benefit of repeating that list once again?

None, I just missed the fact there was already such a description.
Sorry.

You can commit the patch in the last form you posted here.  Thanks.

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

[Daniel Jacobowitz]

On Sat, Jun 02, 2007 at 09:33:29PM +0200, Ulrich Weigand wrote:
> Hello,
> 
> this patch, implemented by Markus Deuling, extends TARGET_OBJECT_SPU
> support to remote gdbserver debugging.
> 
> This is done by extended the remote protocol by two new packet types,
> qXfer:spu:read and qXfer:spu:write.  In addition, the patch contains
> the following changes to remote packet support:

The TARGET_OBJECT_SPU support looks fine.  I don't really like either
of these changes, though.

> - It adds a "length" argument to qXfer::write type packets.  This is
>   to make parsing of the packet easier, and to bring it in line with
>   the 'X' packet format.  (It also provides a bit of extra redundancy
>   to detect transmission problems.)

It adds another case that stubs need to check for (length != length of
supplied data), and it's redundant.  I have some code already written
to generate and parse binary packets without a specified length, if
you'd like me to post it.  It's for the project I'm going to submit
once I finish with shared library lists - file transfer through
gdbserver.

I think more redundancy is not what we need in an already chatty
protocol :-)  And anyone using this to debug an SPU is probably going
to do so over a reliable link (pipe or TCP socket).

> - The remote_read_qxfer attempts to cache received end-of-object
>   packets.  However, this is a problem for some spufs objects as
>   they can start out with no content (length zero), and acquire
>   actual content later on.  If remote_read_qxfer has already cache
>   an end-of-object at lenght zero, future re-reads of the object
>   will always return zero even if the object by now has actual
>   content.  To fix this we've added a CACHEABLE flag to the
>   function and use it to disable the cache for spufs objects.

How about this simpler fix?

-  if (rs->buf[0] == 'l')
+  if (rs->buf[0] == 'l' && offset + i > 0)

It's only supposed to trigger for the next part of the same read
operation.  If we got no bytes, then the operation is already over.

-- 
Daniel Jacobowitz
CodeSourcery

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

[Ulrich Weigand]

Eli Zaretskii wrote:

> None, I just missed the fact there was already such a description.
> Sorry.
> 
> You can commit the patch in the last form you posted here.  Thanks.

OK, thanks!

Bye,
Ulrich

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

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

[Ulrich Weigand]

Daniel Jacobowitz wrote:

> > - It adds a "length" argument to qXfer::write type packets.  This is
> >   to make parsing of the packet easier, and to bring it in line with
> >   the 'X' packet format.  (It also provides a bit of extra redundancy
> >   to detect transmission problems.)
> 
> It adds another case that stubs need to check for (length != length of
> supplied data), and it's redundant.  I have some code already written
> to generate and parse binary packets without a specified length, if
> you'd like me to post it.  It's for the project I'm going to submit
> once I finish with shared library lists - file transfer through
> gdbserver.

Fine with me.  I'd appreciate if you could post that code ...

> > - The remote_read_qxfer attempts to cache received end-of-object
> >   packets.  However, this is a problem for some spufs objects as
> >   they can start out with no content (length zero), and acquire
> >   actual content later on.  If remote_read_qxfer has already cache
> >   an end-of-object at lenght zero, future re-reads of the object
> >   will always return zero even if the object by now has actual
> >   content.  To fix this we've added a CACHEABLE flag to the
> >   function and use it to disable the cache for spufs objects.
> 
> How about this simpler fix?
> 
> -  if (rs->buf[0] == 'l')
> +  if (rs->buf[0] == 'l' && offset + i > 0)
> 
> It's only supposed to trigger for the next part of the same read
> operation.  If we got no bytes, then the operation is already over.

That should fix our problem as well.  I just wasn't sure if the
existing behaviour was deliberate for the other packet types ...

Bye,
Ulrich

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

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

[Daniel Jacobowitz]

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.

> That should fix our problem as well.  I just wasn't sure if the
> existing behaviour was deliberate for the other packet types ...

Nope - purely accidental.  I also tried pushing the flag into an
argument to to_xfer_partial, but it was too messy.

-- 
Daniel Jacobowitz
CodeSourcery

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

[Daniel Jacobowitz]

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.

Sending such packets from GDB is easy.  In fact, it's easier than the
X packet, because you do not have to patch in the exact length.  I
already added the remote_escape_output helper.  If p is a pointer into
rs->buf just after the packet header, this will add as much data as
can fit from write_buf/len:

  p += remote_escape_output (write_buf, len, p, &out_len,
                             get_remote_packet_size () - (p - rs->buf));


To receive them in gdbserver, I used this:

static int
require_data (char *p, int p_len, char **data, int *data_len)
{
  int input_index, output_index, escaped;

  *data = malloc (p_len);

  output_index = 0;
  escaped = 0;
  for (input_index = 0; input_index < p_len; input_index++)
    {
      char b = p[input_index];

      if (escaped)
	{
	  (*data)[output_index++] = b ^ 0x20;
	  escaped = 0;
	}
      else if (b == '}')
	escaped = 1;
      else
	(*data)[output_index++] = b;
    }

  if (escaped)
    return -1;

  *data_len = output_index;
  return 0;
}


  if (require_data (p, packet_len - (p - own_buf), &data, &len))
    {
      write_enn (own_buf);
      return;
    }

p is after the header in own_buf, packet_len is the total length of
the received packet; I already changed getpkt to return packet_len.

-- 
Daniel Jacobowitz
CodeSourcery

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

[Ulrich Weigand]

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";
 }
 
-\f
 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

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

[Eli Zaretskii]

> Date: Wed, 6 Jun 2007 01:02:31 +0200 (CEST)
> From: "Ulrich Weigand" <uweigand@de.ibm.com>
> Cc: gdb-patches@sourceware.org, eliz@gnu.org
> 
> Eli, as we're now not adding the LENGTH argument after all, there are
> changes to the doc patch too.  Still OK?

Yes, thanks.

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

[Daniel Jacobowitz]

On Wed, Jun 06, 2007 at 01:02:31AM +0200, Ulrich Weigand wrote:
> 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?

Yes, this version is fine with me.  Thanks.

-- 
Daniel Jacobowitz
CodeSourcery

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

[Ulrich Weigand]

Daniel Jacobowitz wrote:
> On Wed, Jun 06, 2007 at 01:02:31AM +0200, Ulrich Weigand wrote:
> > 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?
> 
> Yes, this version is fine with me.  Thanks.

Thanks for the review!  I've committed this version now.

Bye,
Ulrich

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