[commit] Clean up SPU local store limit wrap around logic

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

Hello,

on SPU hardware, all local store memory accesses wrap around at the
end of memory (the so-called local store limit).  The SPU back-end
attempts to emulate this feature by truncating all pointer values
in spu_pointer_to_address and spu_integer_to_address.

This approach has a couple of drawbacks, however:

- Pointer values as displayed to the user are truncated, so they
  do not reflect actual values as seen by the program.

- Different SPUs can in theory have different local store limit
  values.  At the time the address is truncated, it is not always
  clear which SPU's LSLR should be used.

To clean this up, the patch below removes this address truncation,
and instead implements LSLR wrap-around at the actual point of
memory access itself, i.e. in the various target's xfer_partial
routines.

There's one additional twist: we used the fact that addresses are
always truncated to ensure overlay LMAs always differ from any
real address, by giving them values above the LSLR.  This may now
no longer be sufficient.  Instead, we encode the fact that an
address is an overlay LMA into bit 62 of the 64-bit combined
address value.

Tested on spu-elf and using combined Cell/B.E. debugging, both
native and via gdbserver.  Committed to mainline.

Bye,
Ulrich


ChangeLog:

	* spu-multiarch.c (spu_xfer_partial): Wrap around local store
	limit on local store memory accesses.
	* spu-linux-nat.c (spu_xfer_partial): Likewise.
	* spu-tdep.c (spu_lslr): Remove.
	(spu_pointer_to_address): Do not truncate addresses.
	(spu_integer_to_address): Likewise.
	(spu_overlay_new_objfile): Use SPU_OVERLAY_LMA.
	* spu-tdep.h: Add comments.
	(SPUADDR_SPU): Respect SPU_OVERLAY_LMA bit.
	(SPU_OVERLAY_LMA): Define.

gdbserver/ChangeLog:

	* spu-low.c (spu_read_memory): Wrap around local store limit.
	(spu_write_memory): Likewise.

testsuite/ChangeLog:

	* gdb.arch/spu-ls.exp: New file.
	* gdb.arch/spu-ls.c: Likewise.


Index: gdb/spu-linux-nat.c
===================================================================
RCS file: /cvs/src/src/gdb/spu-linux-nat.c,v
retrieving revision 1.24
diff -u -p -r1.24 spu-linux-nat.c
--- gdb/spu-linux-nat.c	1 Jan 2010 07:31:42 -0000	1.24
+++ gdb/spu-linux-nat.c	14 Jun 2010 18:42:17 -0000
@@ -562,7 +562,10 @@ spu_xfer_partial (struct target_ops *ops
     {
       int fd;
       ULONGEST addr;
-      char mem_annex[32];
+      char mem_annex[32], lslr_annex[32];
+      gdb_byte buf[32];
+      ULONGEST lslr;
+      LONGEST ret;
 
       /* We must be stopped on a spu_run system call.  */
       if (!parse_spufs_run (&fd, &addr))
@@ -570,7 +573,22 @@ spu_xfer_partial (struct target_ops *ops
 
       /* Use the "mem" spufs file to access SPU local store.  */
       xsnprintf (mem_annex, sizeof mem_annex, "%d/mem", fd);
-      return spu_proc_xfer_spu (mem_annex, readbuf, writebuf, offset, len);
+      ret = spu_proc_xfer_spu (mem_annex, readbuf, writebuf, offset, len);
+      if (ret > 0)
+	return ret;
+
+      /* SPU local store access wraps the address around at the
+	 local store limit.  We emulate this here.  To avoid needing
+	 an extra access to retrieve the LSLR, we only do that after
+	 trying the original address first, and getting end-of-file.  */
+      xsnprintf (lslr_annex, sizeof lslr_annex, "%d/lslr", fd);
+      memset (buf, 0, sizeof buf);
+      if (spu_proc_xfer_spu (lslr_annex, buf, NULL, 0, sizeof buf) <= 0)
+	return ret;
+
+      lslr = strtoulst (buf, NULL, 16);
+      return spu_proc_xfer_spu (mem_annex, readbuf, writebuf,
+				offset & lslr, len);
     }
 
   return -1;
Index: gdb/spu-multiarch.c
===================================================================
RCS file: /cvs/src/src/gdb/spu-multiarch.c,v
retrieving revision 1.3
diff -u -p -r1.3 spu-multiarch.c
--- gdb/spu-multiarch.c	1 Jan 2010 07:31:42 -0000	1.3
+++ gdb/spu-multiarch.c	14 Jun 2010 18:42:17 -0000
@@ -259,14 +259,35 @@ spu_xfer_partial (struct target_ops *ops
     {
       int fd = SPUADDR_SPU (offset);
       CORE_ADDR addr = SPUADDR_ADDR (offset);
-      char mem_annex[32];
+      char mem_annex[32], lslr_annex[32];
+      gdb_byte buf[32];
+      ULONGEST lslr;
+      LONGEST ret;
 
-      if (fd >= 0 && addr < SPU_LS_SIZE)
+      if (fd >= 0)
 	{
 	  xsnprintf (mem_annex, sizeof mem_annex, "%d/mem", fd);
+	  ret = ops_beneath->to_xfer_partial (ops_beneath, TARGET_OBJECT_SPU,
+					      mem_annex, readbuf, writebuf,
+					      addr, len);
+	  if (ret > 0)
+	    return ret;
+
+	  /* SPU local store access wraps the address around at the
+	     local store limit.  We emulate this here.  To avoid needing
+	     an extra access to retrieve the LSLR, we only do that after
+	     trying the original address first, and getting end-of-file.  */
+	  xsnprintf (lslr_annex, sizeof lslr_annex, "%d/lslr", fd);
+	  memset (buf, 0, sizeof buf);
+	  if (ops_beneath->to_xfer_partial (ops_beneath, TARGET_OBJECT_SPU,
+					    lslr_annex, buf, NULL,
+					    0, sizeof buf) <= 0)
+	    return ret;
+
+	  lslr = strtoulst (buf, NULL, 16);
 	  return ops_beneath->to_xfer_partial (ops_beneath, TARGET_OBJECT_SPU,
 					       mem_annex, readbuf, writebuf,
-					       addr, len);
+					       addr & lslr, len);
 	}
     }
 
Index: gdb/spu-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/spu-tdep.c,v
retrieving revision 1.58
diff -u -p -r1.58 spu-tdep.c
--- gdb/spu-tdep.c	17 Mar 2010 13:38:56 -0000	1.58
+++ gdb/spu-tdep.c	14 Jun 2010 18:42:18 -0000
@@ -364,23 +364,6 @@ spu_gdbarch_id (struct gdbarch *gdbarch)
   return id;
 }
 
-static ULONGEST
-spu_lslr (int id)
-{
-  gdb_byte buf[32];
-  char annex[32];
-
-  if (id == -1)
-    return SPU_LS_SIZE - 1;
-
-  xsnprintf (annex, sizeof annex, "%d/lslr", id);
-  memset (buf, 0, sizeof buf);
-  target_read (&current_target, TARGET_OBJECT_SPU, annex,
-	       buf, 0, sizeof buf);
-
-  return strtoulst (buf, NULL, 16);
-}
-
 static int
 spu_address_class_type_flags (int byte_size, int dwarf2_addr_class)
 {
@@ -426,7 +409,6 @@ spu_pointer_to_address (struct gdbarch *
 			struct type *type, const gdb_byte *buf)
 {
   int id = spu_gdbarch_id (gdbarch);
-  ULONGEST lslr = spu_lslr (id);
   enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
   ULONGEST addr
     = extract_unsigned_integer (buf, TYPE_LENGTH (type), byte_order);
@@ -435,7 +417,7 @@ spu_pointer_to_address (struct gdbarch *
   if (TYPE_ADDRESS_CLASS_1 (type))
     return addr;
 
-  return addr? SPUADDR (id, addr & lslr) : 0;
+  return addr? SPUADDR (id, addr) : 0;
 }
 
 static CORE_ADDR
@@ -443,10 +425,9 @@ spu_integer_to_address (struct gdbarch *
 			struct type *type, const gdb_byte *buf)
 {
   int id = spu_gdbarch_id (gdbarch);
-  ULONGEST lslr = spu_lslr (id);
   ULONGEST addr = unpack_long (type, buf);
 
-  return SPUADDR (id, addr & lslr);
+  return SPUADDR (id, addr);
 }
 
 
@@ -1777,7 +1758,7 @@ spu_overlay_update (struct obj_section *
 /* Whenever a new objfile is loaded, read the target's _ovly_table.
    If there is one, go through all sections and make sure for non-
    overlay sections LMA equals VMA, while for overlay sections LMA
-   is larger than local store size.  */
+   is larger than SPU_OVERLAY_LMA.  */
 static void
 spu_overlay_new_objfile (struct objfile *objfile)
 {
@@ -1807,7 +1788,7 @@ spu_overlay_new_objfile (struct objfile 
       if (ovly_table[ndx].mapped_ptr == 0)
 	bfd_section_lma (obfd, bsect) = bfd_section_vma (obfd, bsect);
       else
-	bfd_section_lma (obfd, bsect) = bsect->filepos + SPU_LS_SIZE;
+	bfd_section_lma (obfd, bsect) = SPU_OVERLAY_LMA + bsect->filepos;
     }
 }
 
Index: gdb/spu-tdep.h
===================================================================
RCS file: /cvs/src/src/gdb/spu-tdep.h,v
retrieving revision 1.9
diff -u -p -r1.9 spu-tdep.h
--- gdb/spu-tdep.h	1 Jan 2010 07:31:42 -0000	1.9
+++ gdb/spu-tdep.h	14 Jun 2010 18:42:18 -0000
@@ -50,14 +50,71 @@ enum spu_regnum
 /* Local store.  */
 #define SPU_LS_SIZE          0x40000
 
-/* Address conversions.  */
+/* Address conversions.
+
+   In a combined PPU/SPU debugging session, we have to consider multiple
+   address spaces: the PPU 32- or 64-bit address space, and the 32-bit
+   local store address space for each SPU context.  As it is currently
+   not yet possible to use the program_space / address_space mechanism
+   to represent this, we encode all those addresses into one single
+   64-bit address for the whole process.  For SPU programs using overlays,
+   this address space must also include separate ranges reserved for the
+   LMA of overlay sections.
+
+
+   The following combinations are supported for combined debugging:
+
+   PPU address (this relies on the fact that PPC 64-bit user space
+   addresses can never have the highest-most bit set):
+
+      +-+---------------------------------+
+      |0|              ADDR [63]          |
+      +-+---------------------------------+
+
+   SPU address for SPU context with id SPU (this assumes that SPU
+   IDs, which are file descriptors, are never larger than 2^30):
+
+      +-+-+--------------+----------------+
+      |1|0|    SPU [30]  |    ADDR [32]   |
+      +-+-+--------------+----------------+
+
+   SPU overlay section LMA for SPU context with id SPU:
+
+      +-+-+--------------+----------------+
+      |1|1|    SPU [30]  |    ADDR [32]   |
+      +-+-+--------------+----------------+
+
+
+   In SPU stand-alone debugging mode (using spu-linux-nat.c),
+   the following combinations are supported:
+
+   SPU address:
+
+      +-+-+--------------+----------------+
+      |0|0|     0        |    ADDR [32]   |
+      +-+-+--------------+----------------+
+
+   SPU overlay section LMA:
+
+      +-+-+--------------+----------------+
+      |0|1|     0        |    ADDR [32]   |
+      +-+-+--------------+----------------+
+
+
+   The following macros allow manipulation of addresses in the
+   above formats.  */
+
 #define SPUADDR(spu, addr) \
   ((spu) != -1? (ULONGEST)1 << 63 | (ULONGEST)(spu) << 32 | (addr) : (addr))
+
 #define SPUADDR_SPU(addr) \
   (((addr) & (ULONGEST)1 << 63) \
-   ? (int) ((ULONGEST)(addr) >> 32 & 0x7fffffff) \
+   ? (int) ((ULONGEST)(addr) >> 32 & 0x3fffffff) \
    : -1)
+
 #define SPUADDR_ADDR(addr) \
   (((addr) & (ULONGEST)1 << 63)? (ULONGEST)(addr) & 0xffffffff : (addr))
 
+#define SPU_OVERLAY_LMA ((ULONGEST)1 << 62)
+
 #endif
Index: gdb/gdbserver/spu-low.c
===================================================================
RCS file: /cvs/src/src/gdb/gdbserver/spu-low.c,v
retrieving revision 1.30
diff -u -p -r1.30 spu-low.c
--- gdb/gdbserver/spu-low.c	12 Apr 2010 17:39:42 -0000	1.30
+++ gdb/gdbserver/spu-low.c	14 Jun 2010 18:42:19 -0000
@@ -561,7 +561,8 @@ spu_read_memory (CORE_ADDR memaddr, unsi
 {
   int fd, ret;
   CORE_ADDR addr;
-  char annex[32];
+  char annex[32], lslr_annex[32], buf[32];
+  CORE_ADDR lslr;
 
   /* We must be stopped on a spu_run system call.  */
   if (!parse_spufs_run (&fd, &addr))
@@ -570,6 +571,22 @@ spu_read_memory (CORE_ADDR memaddr, unsi
   /* Use the "mem" spufs file to access SPU local store.  */
   sprintf (annex, "%d/mem", fd);
   ret = spu_proc_xfer_spu (annex, myaddr, NULL, memaddr, len);
+  if (ret > 0)
+    return ret == len ? 0 : EIO;
+
+  /* SPU local store access wraps the address around at the
+     local store limit.  We emulate this here.  To avoid needing
+     an extra access to retrieve the LSLR, we only do that after
+     trying the original address first, and getting end-of-file.  */
+  sprintf (lslr_annex, "%d/lslr", fd);
+  memset (buf, 0, sizeof buf);
+  if (spu_proc_xfer_spu (lslr_annex, (unsigned char *)buf, NULL,
+			 0, sizeof buf) <= 0)
+    return ret;
+
+  lslr = strtoul (buf, NULL, 16);
+  ret = spu_proc_xfer_spu (annex, myaddr, NULL, memaddr & lslr, len);
+
   return ret == len ? 0 : EIO;
 }
 
@@ -582,7 +599,8 @@ spu_write_memory (CORE_ADDR memaddr, con
 {
   int fd, ret;
   CORE_ADDR addr;
-  char annex[32];
+  char annex[32], lslr_annex[32], buf[32];
+  CORE_ADDR lslr;
 
   /* We must be stopped on a spu_run system call.  */
   if (!parse_spufs_run (&fd, &addr))
@@ -591,6 +609,22 @@ spu_write_memory (CORE_ADDR memaddr, con
   /* Use the "mem" spufs file to access SPU local store.  */
   sprintf (annex, "%d/mem", fd);
   ret = spu_proc_xfer_spu (annex, NULL, myaddr, memaddr, len);
+  if (ret > 0)
+    return ret == len ? 0 : EIO;
+
+  /* SPU local store access wraps the address around at the
+     local store limit.  We emulate this here.  To avoid needing
+     an extra access to retrieve the LSLR, we only do that after
+     trying the original address first, and getting end-of-file.  */
+  sprintf (lslr_annex, "%d/lslr", fd);
+  memset (buf, 0, sizeof buf);
+  if (spu_proc_xfer_spu (lslr_annex, (unsigned char *)buf, NULL,
+			 0, sizeof buf) <= 0)
+    return ret;
+
+  lslr = strtoul (buf, NULL, 16);
+  ret = spu_proc_xfer_spu (annex, NULL, myaddr, memaddr & lslr, len);
+
   return ret == len ? 0 : EIO;
 }
 
--- /dev/null	2010-06-09 19:31:28.423437333 +0200
+++ gdb/testsuite/gdb.arch/spu-ls.c	2010-06-14 19:25:05.000000000 +0200
@@ -0,0 +1,31 @@
+/* Copyright 2010 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+   This file is part of the gdb testsuite.
+
+   Contributed by Ulrich Weigand <uweigand@de.ibm.com>.
+   Tests for SPU local-store access.  */
+
+char *ptr = (char *)0x12345678;
+
+char array[256];
+
+int
+main (unsigned long long speid, unsigned long long argp, 
+      unsigned long long envp)
+{
+  return 0;
+}
+
--- /dev/null	2010-06-09 19:31:28.423437333 +0200
+++ gdb/testsuite/gdb.arch/spu-ls.exp	2010-06-14 20:15:16.000000000 +0200
@@ -0,0 +1,54 @@
+# Copyright 2010 Free Software Foundation, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+#
+# This file is part of the gdb testsuite.
+#
+# Contributed by Ulrich Weigand  <uweigand@de.ibm.com>.
+# Tests for SPU local-store access.
+
+if { ![istarget "spu-*-elf"] } then {
+  verbose "Skipping SPU-only testcase"
+  return
+}
+
+set testfile "spu-ls"
+set srcfile ${testfile}.c
+set binfile ${objdir}/${subdir}/${testfile}
+set sources ${srcdir}/${subdir}/${srcfile}
+
+if { [gdb_compile $sources ${binfile} executable { debug }] != "" } {
+  return -1
+}
+
+gdb_exit
+gdb_start
+gdb_reinitialize_dir $srcdir/$subdir
+gdb_load ${binfile}
+
+if ![runto_main] then {
+  fail "Can't run to main"
+  return 0
+}
+
+gdb_test "print ptr" " = 0x12345678 \".*\"" "print ptr"
+gdb_test_no_output "set ptr = array + \$lslr + 1" "set ptr = array + \$lslr + 1"
+gdb_test_no_output "set array\[0\] = 1" "set array\[0\] = 1"
+gdb_test "print *ptr" " = 1 '\\\\001'" "print *ptr"
+gdb_test_no_output "set *ptr = 2" "set *ptr = 2"
+gdb_test "print array\[0\]" " = 2 '\\\\002'" "print array\[0\]"
+
+gdb_exit
+
+return 0
-- 
  Dr. Ulrich Weigand
  GNU Toolchain for Linux on System z and Cell BE
  Ulrich.Weigand@de.ibm.com