[rfc][3/3] gdbserver bi-arch for ppc: enable bi-arch support

[rfc][3/3] gdbserver bi-arch for ppc: enable bi-arch support

[Ulrich Weigand]

Hello,

this patch finally enables bi-arch ppc support.  It follows the same
pattern as the s390 variant by providing a ppc_arch_setup routine that
does a run-time check whether the inferior is 64-bit and installs the
appropriate init_registers_... routine.

Like on s390, adaptations to ppc_get_pc/ppc_set_pc and ppc_fill_gregset
were necessary to deal with varying register sizes.

Tested on ppc-linux and on ppc64-linux (both with -m64 and -m32) in
a native gdbserver test setup, no regressions.

Bye,
Ulrich


ChangeLog:

	* configure.srv [powerpc64-*-linux*]: Add all files mentioned for
	powerpc-*-linux* to srv_regobj and reg_xmlfiles.
	* linux-ppc-low.c (ppc_get_pc): Support bi-arch operation.
	(ppc_set_pc): Likewise.
	(ppc_arch_setup): New function.
	(ppc_fill_gregset): Call ppc_collect_ptrace_register instead
	of collect_register.
	(the_low_target): Use ppc_arch_setup as arch_setup initializer.	


diff -urNp gdb-orig/gdb/gdbserver/configure.srv gdb-head/gdb/gdbserver/configure.srv
--- gdb-orig/gdb/gdbserver/configure.srv	2008-02-07 23:55:15.932772349 +0100
+++ gdb-head/gdb/gdbserver/configure.srv	2008-02-07 22:06:35.448385574 +0100
@@ -106,12 +106,17 @@ case "${target}" in
 			srv_linux_usrregs=yes
 			srv_linux_thread_db=yes
 			;;
-  powerpc64-*-linux*)	srv_regobj="reg-ppc64.o powerpc-64.o"
+  powerpc64-*-linux*)	srv_regobj="reg-ppc.o powerpc-32.o powerpc-e500.o"
+			srv_regobj="${srv_regobj} reg-ppc64.o powerpc-64.o"
 			srv_tgtobj="linux-low.o linux-ppc-low.o"
-			srv_xmlfiles="rs6000/powerpc-64.xml"
+			srv_xmlfiles="rs6000/powerpc-32.xml"
 			srv_xmlfiles="${srv_xmlfiles} rs6000/power-altivec.xml"
-			srv_xmlfiles="${srv_xmlfiles} rs6000/power64-core.xml"
+			srv_xmlfiles="${srv_xmlfiles} rs6000/power-core.xml"
 			srv_xmlfiles="${srv_xmlfiles} rs6000/power-fpu.xml"
+			srv_xmlfiles="${srv_xmlfiles} rs6000/powerpc-e500.xml"
+			srv_xmlfiles="${srv_xmlfiles} rs6000/power-spe.xml"
+			srv_xmlfiles="${srv_xmlfiles} rs6000/powerpc-64.xml"
+			srv_xmlfiles="${srv_xmlfiles} rs6000/power64-core.xml"
 			srv_linux_usrregs=yes
 			srv_linux_regsets=yes
 			srv_linux_thread_db=yes
diff -urNp gdb-orig/gdb/gdbserver/linux-ppc-low.c gdb-head/gdb/gdbserver/linux-ppc-low.c
--- gdb-orig/gdb/gdbserver/linux-ppc-low.c	2008-02-07 23:55:15.940771196 +0100
+++ gdb-head/gdb/gdbserver/linux-ppc-low.c	2008-02-07 23:56:26.898314713 +0100
@@ -138,18 +138,69 @@ ppc_supply_ptrace_register (int regno, c
 static CORE_ADDR
 ppc_get_pc (void)
 {
-  unsigned long pc;
-
-  collect_register_by_name ("pc", &pc);
-  return (CORE_ADDR) pc;
+  if (register_size (0) == 4)
+    {
+      unsigned int pc;
+      collect_register_by_name ("pc", &pc);
+      return (CORE_ADDR) pc;
+    }
+  else
+    {
+      unsigned long pc;
+      collect_register_by_name ("pc", &pc);
+      return (CORE_ADDR) pc;
+    }
 }
 
 static void
 ppc_set_pc (CORE_ADDR pc)
 {
-  unsigned long newpc = pc;
+  if (register_size (0) == 4)
+    {
+      unsigned int newpc = pc;
+      supply_register_by_name ("pc", &newpc);
+    }
+  else
+    {
+      unsigned long newpc = pc;
+      supply_register_by_name ("pc", &newpc);
+    }
+}
 
-  supply_register_by_name ("pc", &newpc);
+static void
+ppc_arch_setup (void)
+{
+#ifdef __powerpc64__
+  long msr;
+
+  /* On a 64-bit host, assume 64-bit inferior process.  */
+#ifdef __ALTIVEC__
+  init_registers_powerpc_64 ();
+#else
+  init_registers_ppc64 ();
+#endif
+
+  /* Only if the high bit of the MSR is set, we actually have
+     a 64-bit inferior.  */
+#ifdef __ALTIVEC__
+  collect_register_by_name ("msr", &msr);
+#else
+  collect_register_by_name ("ps", &msr);
+#endif
+  if (msr < 0)
+    return;
+#endif
+
+  /* OK, we have a 32-bit inferior.  */
+#ifdef __ALTIVEC__
+  init_registers_powerpc_32 ();
+#else
+#ifdef __SPE__
+  init_registers_powerpc_e500 ();
+#else
+  init_registers_ppc ();
+#endif
+#endif
 }
 
 /* Correct in either endianness.
@@ -179,10 +230,10 @@ static void ppc_fill_gregset (void *buf)
   int i;
 
   for (i = 0; i < 32; i++)
-    collect_register (i, (char *) buf + ppc_regmap[i]);
+    ppc_collect_ptrace_register (i, (char *) buf + ppc_regmap[i]);
 
   for (i = 64; i < 70; i++)
-    collect_register (i, (char *) buf + ppc_regmap[i]);
+    ppc_collect_ptrace_register (i, (char *) buf + ppc_regmap[i]);
 }
 
 #ifdef __ALTIVEC__
@@ -285,23 +336,7 @@ struct regset_info target_regsets[] = {
 };
 
 struct linux_target_ops the_low_target = {
-#ifdef __powerpc64__
-#ifdef __ALTIVEC__
-  init_registers_powerpc_64,
-#else
-  init_registers_ppc64,
-#endif
-#else
-#ifdef __ALTIVEC__
-  init_registers_powerpc_32,
-#else
-#ifdef __SPE__
-  init_registers_powerpc_e500,
-#else
-  init_registers_ppc,
-#endif
-#endif
-#endif
+  ppc_arch_setup,
   ppc_num_regs,
   ppc_regmap,
   ppc_cannot_fetch_register,
-- 
  Dr. Ulrich Weigand
  GNU Toolchain for Linux on System z and Cell BE
  Ulrich.Weigand@de.ibm.com

Re: [rfc][3/3] gdbserver bi-arch for ppc: enable bi-arch support

[Daniel Jacobowitz]

On Fri, Feb 08, 2008 at 02:11:22AM +0100, Ulrich Weigand wrote:
> +  /* Only if the high bit of the MSR is set, we actually have
> +     a 64-bit inferior.  */
> +#ifdef __ALTIVEC__
> +  collect_register_by_name ("msr", &msr);
> +#else
> +  collect_register_by_name ("ps", &msr);
> +#endif
> +  if (msr < 0)
> +    return;
> +#endif

This #ifdef can go away; in the manually written regformats files,
calls to collect_register_by_name are the only thing that rely on
the register name, so you could just rename it to msr.

Otherwise OK.

> +  /* OK, we have a 32-bit inferior.  */
> +#ifdef __ALTIVEC__
> +  init_registers_powerpc_32 ();
> +#else
> +#ifdef __SPE__
> +  init_registers_powerpc_e500 ();
> +#else
> +  init_registers_ppc ();
> +#endif
> +#endif

I hope we can autodetect AltiVec and SPE too... Maybe we have to
resort to checking the inferior's auxv?

-- 
Daniel Jacobowitz
CodeSourcery

Re: [rfc][3/3] gdbserver bi-arch for ppc: enable bi-arch support

[Ulrich Weigand]

Daniel Jacobowitz wrote:
> On Fri, Feb 08, 2008 at 02:11:22AM +0100, Ulrich Weigand wrote:
> > +  /* Only if the high bit of the MSR is set, we actually have
> > +     a 64-bit inferior.  */
> > +#ifdef __ALTIVEC__
> > +  collect_register_by_name ("msr", &msr);
> > +#else
> > +  collect_register_by_name ("ps", &msr);
> > +#endif
> > +  if (msr < 0)
> > +    return;
> > +#endif
> 
> This #ifdef can go away; in the manually written regformats files,
> calls to collect_register_by_name are the only thing that rely on
> the register name, so you could just rename it to msr.

Ah, right.  I've updated the patch accordingly.

> Otherwise OK.

Thanks; I've checked in the patch below.

> > +  /* OK, we have a 32-bit inferior.  */
> > +#ifdef __ALTIVEC__
> > +  init_registers_powerpc_32 ();
> > +#else
> > +#ifdef __SPE__
> > +  init_registers_powerpc_e500 ();
> > +#else
> > +  init_registers_ppc ();
> > +#endif
> > +#endif
> 
> I hope we can autodetect AltiVec and SPE too... Maybe we have to
> resort to checking the inferior's auxv?

Agreed.  I'll see if I can put a patch along those lines together.
However, I don't have any non-AltiVec or SPE systems available;
would you be able to test on those?

Bye,
Ulrich


ChangeLog:

	* regformats/reg-ppc.dat: Rename "ps" to "msr".
	* regformats/reg-ppc64.dat: Likewise.

gdbserver/ChangeLog:

	* configure.srv [powerpc64-*-linux*]: Add all files mentioned for
	powerpc-*-linux* to srv_regobj and reg_xmlfiles.
	* linux-ppc-low.c (ppc_get_pc): Support bi-arch operation.
	(ppc_set_pc): Likewise.
	(ppc_arch_setup): New function.
	(ppc_fill_gregset): Call ppc_collect_ptrace_register instead
	of collect_register.
	(the_low_target): Use ppc_arch_setup as arch_setup initializer.	


diff -urNp gdb-orig/gdb/gdbserver/configure.srv gdb-head/gdb/gdbserver/configure.srv
--- gdb-orig/gdb/gdbserver/configure.srv	2008-02-28 04:44:34.000000000 +0100
+++ gdb-head/gdb/gdbserver/configure.srv	2008-02-28 06:01:43.048973708 +0100
@@ -106,12 +106,17 @@ case "${target}" in
 			srv_linux_usrregs=yes
 			srv_linux_thread_db=yes
 			;;
-  powerpc64-*-linux*)	srv_regobj="reg-ppc64.o powerpc-64.o"
+  powerpc64-*-linux*)	srv_regobj="reg-ppc.o powerpc-32.o powerpc-e500.o"
+			srv_regobj="${srv_regobj} reg-ppc64.o powerpc-64.o"
 			srv_tgtobj="linux-low.o linux-ppc-low.o"
-			srv_xmlfiles="rs6000/powerpc-64.xml"
+			srv_xmlfiles="rs6000/powerpc-32.xml"
 			srv_xmlfiles="${srv_xmlfiles} rs6000/power-altivec.xml"
-			srv_xmlfiles="${srv_xmlfiles} rs6000/power64-core.xml"
+			srv_xmlfiles="${srv_xmlfiles} rs6000/power-core.xml"
 			srv_xmlfiles="${srv_xmlfiles} rs6000/power-fpu.xml"
+			srv_xmlfiles="${srv_xmlfiles} rs6000/powerpc-e500.xml"
+			srv_xmlfiles="${srv_xmlfiles} rs6000/power-spe.xml"
+			srv_xmlfiles="${srv_xmlfiles} rs6000/powerpc-64.xml"
+			srv_xmlfiles="${srv_xmlfiles} rs6000/power64-core.xml"
 			srv_linux_usrregs=yes
 			srv_linux_regsets=yes
 			srv_linux_thread_db=yes
diff -urNp gdb-orig/gdb/gdbserver/linux-ppc-low.c gdb-head/gdb/gdbserver/linux-ppc-low.c
--- gdb-orig/gdb/gdbserver/linux-ppc-low.c	2008-02-28 04:44:34.000000000 +0100
+++ gdb-head/gdb/gdbserver/linux-ppc-low.c	2008-02-28 06:03:17.532729696 +0100
@@ -138,18 +138,65 @@ ppc_supply_ptrace_register (int regno, c
 static CORE_ADDR
 ppc_get_pc (void)
 {
-  unsigned long pc;
-
-  collect_register_by_name ("pc", &pc);
-  return (CORE_ADDR) pc;
+  if (register_size (0) == 4)
+    {
+      unsigned int pc;
+      collect_register_by_name ("pc", &pc);
+      return (CORE_ADDR) pc;
+    }
+  else
+    {
+      unsigned long pc;
+      collect_register_by_name ("pc", &pc);
+      return (CORE_ADDR) pc;
+    }
 }
 
 static void
 ppc_set_pc (CORE_ADDR pc)
 {
-  unsigned long newpc = pc;
+  if (register_size (0) == 4)
+    {
+      unsigned int newpc = pc;
+      supply_register_by_name ("pc", &newpc);
+    }
+  else
+    {
+      unsigned long newpc = pc;
+      supply_register_by_name ("pc", &newpc);
+    }
+}
+
+static void
+ppc_arch_setup (void)
+{
+#ifdef __powerpc64__
+  long msr;
 
-  supply_register_by_name ("pc", &newpc);
+  /* On a 64-bit host, assume 64-bit inferior process.  */
+#ifdef __ALTIVEC__
+  init_registers_powerpc_64 ();
+#else
+  init_registers_ppc64 ();
+#endif
+
+  /* Only if the high bit of the MSR is set, we actually have
+     a 64-bit inferior.  */
+  collect_register_by_name ("msr", &msr);
+  if (msr < 0)
+    return;
+#endif
+
+  /* OK, we have a 32-bit inferior.  */
+#ifdef __ALTIVEC__
+  init_registers_powerpc_32 ();
+#else
+#ifdef __SPE__
+  init_registers_powerpc_e500 ();
+#else
+  init_registers_ppc ();
+#endif
+#endif
 }
 
 /* Correct in either endianness.
@@ -179,10 +226,10 @@ static void ppc_fill_gregset (void *buf)
   int i;
 
   for (i = 0; i < 32; i++)
-    collect_register (i, (char *) buf + ppc_regmap[i]);
+    ppc_collect_ptrace_register (i, (char *) buf + ppc_regmap[i]);
 
   for (i = 64; i < 70; i++)
-    collect_register (i, (char *) buf + ppc_regmap[i]);
+    ppc_collect_ptrace_register (i, (char *) buf + ppc_regmap[i]);
 }
 
 #ifdef __ALTIVEC__
@@ -285,23 +332,7 @@ struct regset_info target_regsets[] = {
 };
 
 struct linux_target_ops the_low_target = {
-#ifdef __powerpc64__
-#ifdef __ALTIVEC__
-  init_registers_powerpc_64,
-#else
-  init_registers_ppc64,
-#endif
-#else
-#ifdef __ALTIVEC__
-  init_registers_powerpc_32,
-#else
-#ifdef __SPE__
-  init_registers_powerpc_e500,
-#else
-  init_registers_ppc,
-#endif
-#endif
-#endif
+  ppc_arch_setup,
   ppc_num_regs,
   ppc_regmap,
   ppc_cannot_fetch_register,
diff -urNp gdb-orig/gdb/regformats/reg-ppc64.dat gdb-head/gdb/regformats/reg-ppc64.dat
--- gdb-orig/gdb/regformats/reg-ppc64.dat	2008-02-27 23:18:38.000000000 +0100
+++ gdb-head/gdb/regformats/reg-ppc64.dat	2008-02-28 06:02:20.651277210 +0100
@@ -67,7 +67,7 @@ expedite:r1,pc
 64:f31
 
 64:pc
-64:ps
+64:msr
 
 32:cr
 64:lr
diff -urNp gdb-orig/gdb/regformats/reg-ppc.dat gdb-head/gdb/regformats/reg-ppc.dat
--- gdb-orig/gdb/regformats/reg-ppc.dat	2002-04-11 22:30:08.000000000 +0200
+++ gdb-head/gdb/regformats/reg-ppc.dat	2008-02-28 06:02:06.287084193 +0100
@@ -67,7 +67,7 @@ expedite:r1,pc
 64:f31
 
 32:pc
-32:ps
+32:msr
 
 32:cr
 32:lr

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

Re: [rfc][3/3] gdbserver bi-arch for ppc: enable bi-arch support

[Daniel Jacobowitz]

On Thu, Feb 28, 2008 at 07:02:40AM +0100, Ulrich Weigand wrote:
> Agreed.  I'll see if I can put a patch along those lines together.
> However, I don't have any non-AltiVec or SPE systems available;
> would you be able to test on those?

Yes, definitely.  We've got both.

-- 
Daniel Jacobowitz
CodeSourcery

[rfc/rft] ppc gdbserver: autodetect AltiVec and SPE

[Ulrich Weigand]

Daniel Jacobowitz wrote:
> > Agreed.  I'll see if I can put a patch along those lines together.
> > However, I don't have any non-AltiVec or SPE systems available;
> > would you be able to test on those?
> 
> Yes, definitely.  We've got both.

That would be great, thanks!

What you do think about this patch?  It uses the AT_HWCAP auxv entry
to check for AltiVec and SPE.  One minor annoyance is that you need
to know the target wordsize in order to correctly interpret the auxv
array.  Therefore, we have to do the 64/32 check first, and only then
can we read auxv.

AltiVec and SPE regset access is now enabled uncoditionally, but the
fill/store routines make sure the corresponding HWCAP is set to avoid
accessing registers that are not present in the select register map.

With SPE there were a number of additional issues:
- The ppc_regmap used conditional defines to disable access to 
  floating point registers; I'm now using two different regmaps
  and switch them in the arch_setup routine.
- As the regmap was defined only for 32-bit host machines, I've
  disabled SPE support on 64-bit hosts.  I don't think there are
  any 64-bit machines with SPE anyway, right?
- There was a conditional check for fpscr in ppc_cannot_store_register;
  but as this routine is never called for fpscr in SPE mode, the check
  seems superfluous and I just removed it.

Tested on powerpc-linux and powerpc64-linux (-m64 and -m32) with no
regressions; properly detects AltiVec support.

Bye,
Ulrich


ChangeLog:

	* configure.srv [powerpc64-*-linux*]: Remove powerpc-e500.o from
	srv_regobj.  Remove rs6000/powerpc-e500.xml and rs6000/power-spe.xml	
	from reg_xmlfiles.
	* linux-ppc-low.c: Include <elf.h>.
	(PPC_FEATURE_HAS_ALTIVEC, PPC_FEATURE_HAS_SPE): Define.
	(ppc_hwcap): New global variable.
	(ppc_regmap): Remove __SPE__ #ifdef sections.
	(ppc_regmap_e500): New global variable.
	(ppc_cannot_store_register): Remove __SPE__ special case.
	(ppc_get_hwcap): New function.
	(ppc_arch_setup): Use it to determine whether inferior supports
	AltiVec or SPE registers.  Set the_low_target.regmap if appropriate.
	(ppc_fill_vrregset, ppc_store_vrregset): Define unconditionally.
	Do not access registers if target does not support AltiVec.
	(ppc_fill_evrregset, ppc_store_evrregset): Define unconditionally.
	Do not access registers if target does not support SPE.
	(target_regsets): Unconditionally include AltiVec and SPE regsets.

diff -urNp gdb-orig/gdb/gdbserver/configure.srv gdb-head/gdb/gdbserver/configure.srv
--- gdb-orig/gdb/gdbserver/configure.srv	2008-02-28 06:56:04.000000000 +0100
+++ gdb-head/gdb/gdbserver/configure.srv	2008-02-28 17:17:47.488329501 +0100
@@ -106,15 +106,13 @@ case "${target}" in
 			srv_linux_usrregs=yes
 			srv_linux_thread_db=yes
 			;;
-  powerpc64-*-linux*)	srv_regobj="reg-ppc.o powerpc-32.o powerpc-e500.o"
+  powerpc64-*-linux*)	srv_regobj="reg-ppc.o powerpc-32.o"
 			srv_regobj="${srv_regobj} reg-ppc64.o powerpc-64.o"
 			srv_tgtobj="linux-low.o linux-ppc-low.o"
 			srv_xmlfiles="rs6000/powerpc-32.xml"
 			srv_xmlfiles="${srv_xmlfiles} rs6000/power-altivec.xml"
 			srv_xmlfiles="${srv_xmlfiles} rs6000/power-core.xml"
 			srv_xmlfiles="${srv_xmlfiles} rs6000/power-fpu.xml"
-			srv_xmlfiles="${srv_xmlfiles} rs6000/powerpc-e500.xml"
-			srv_xmlfiles="${srv_xmlfiles} rs6000/power-spe.xml"
 			srv_xmlfiles="${srv_xmlfiles} rs6000/powerpc-64.xml"
 			srv_xmlfiles="${srv_xmlfiles} rs6000/power64-core.xml"
 			srv_linux_usrregs=yes
diff -urNp gdb-orig/gdb/gdbserver/linux-ppc-low.c gdb-head/gdb/gdbserver/linux-ppc-low.c
--- gdb-orig/gdb/gdbserver/linux-ppc-low.c	2008-02-28 06:56:04.000000000 +0100
+++ gdb-head/gdb/gdbserver/linux-ppc-low.c	2008-02-28 17:18:50.462849876 +0100
@@ -21,8 +21,16 @@
 #include "server.h"
 #include "linux-low.h"
 
+#include <elf.h>
 #include <asm/ptrace.h>
 
+/* These are in <asm/cputable.h> in current kernels.  */
+#define PPC_FEATURE_HAS_ALTIVEC         0x10000000
+#define PPC_FEATURE_HAS_SPE             0x00800000
+
+static unsigned long ppc_hwcap;
+
+
 /* Defined in auto-generated file reg-ppc.c.  */
 void init_registers_ppc (void);
 /* Defined in auto-generated file powerpc-32.c.  */
@@ -69,16 +77,6 @@ static int ppc_regmap[] =
   PT_R20 * 4,    PT_R21 * 4,    PT_R22 * 4,    PT_R23 * 4,
   PT_R24 * 4,    PT_R25 * 4,    PT_R26 * 4,    PT_R27 * 4,
   PT_R28 * 4,    PT_R29 * 4,    PT_R30 * 4,    PT_R31 * 4,
-#ifdef __SPE__
-  -1,            -1,            -1,            -1,
-  -1,            -1,            -1,            -1,
-  -1,            -1,            -1,            -1,
-  -1,            -1,            -1,            -1,
-  -1,            -1,            -1,            -1,
-  -1,            -1,            -1,            -1,
-  -1,            -1,            -1,            -1,
-  -1,            -1,            -1,            -1,
-#else
   PT_FPR0*4,     PT_FPR0*4 + 8, PT_FPR0*4+16,  PT_FPR0*4+24,
   PT_FPR0*4+32,  PT_FPR0*4+40,  PT_FPR0*4+48,  PT_FPR0*4+56,
   PT_FPR0*4+64,  PT_FPR0*4+72,  PT_FPR0*4+80,  PT_FPR0*4+88,
@@ -87,20 +85,36 @@ static int ppc_regmap[] =
   PT_FPR0*4+160,  PT_FPR0*4+168,  PT_FPR0*4+176,  PT_FPR0*4+184,
   PT_FPR0*4+192,  PT_FPR0*4+200,  PT_FPR0*4+208,  PT_FPR0*4+216,
   PT_FPR0*4+224,  PT_FPR0*4+232,  PT_FPR0*4+240,  PT_FPR0*4+248,
-#endif
   PT_NIP * 4,    PT_MSR * 4,    PT_CCR * 4,    PT_LNK * 4,
-#ifdef __SPE__
-  PT_CTR * 4,    PT_XER * 4,    -1
-#else
   PT_CTR * 4,    PT_XER * 4,    PT_FPSCR * 4
-#endif
+ };
+
+static int ppc_regmap_e500[] =
+ {PT_R0 * 4,     PT_R1 * 4,     PT_R2 * 4,     PT_R3 * 4,
+  PT_R4 * 4,     PT_R5 * 4,     PT_R6 * 4,     PT_R7 * 4,
+  PT_R8 * 4,     PT_R9 * 4,     PT_R10 * 4,    PT_R11 * 4,
+  PT_R12 * 4,    PT_R13 * 4,    PT_R14 * 4,    PT_R15 * 4,
+  PT_R16 * 4,    PT_R17 * 4,    PT_R18 * 4,    PT_R19 * 4,
+  PT_R20 * 4,    PT_R21 * 4,    PT_R22 * 4,    PT_R23 * 4,
+  PT_R24 * 4,    PT_R25 * 4,    PT_R26 * 4,    PT_R27 * 4,
+  PT_R28 * 4,    PT_R29 * 4,    PT_R30 * 4,    PT_R31 * 4,
+  -1,            -1,            -1,            -1,
+  -1,            -1,            -1,            -1,
+  -1,            -1,            -1,            -1,
+  -1,            -1,            -1,            -1,
+  -1,            -1,            -1,            -1,
+  -1,            -1,            -1,            -1,
+  -1,            -1,            -1,            -1,
+  -1,            -1,            -1,            -1,
+  PT_NIP * 4,    PT_MSR * 4,    PT_CCR * 4,    PT_LNK * 4,
+  PT_CTR * 4,    PT_XER * 4,    -1
  };
 #endif
 
 static int
 ppc_cannot_store_register (int regno)
 {
-#if !defined (__powerpc64__) && !defined (__SPE__)
+#ifndef __powerpc64__
   /* Some kernels do not allow us to store fpscr.  */
   if (regno == find_regno ("fpscr"))
     return 2;
@@ -167,6 +181,42 @@ ppc_set_pc (CORE_ADDR pc)
     }
 }
 
+
+static int
+ppc_get_hwcap (unsigned long *valp)
+{
+  int wordsize = register_size (0);
+  unsigned char *data = alloca (2 * wordsize);
+  int offset = 0;
+
+  while ((*the_target->read_auxv) (offset, data, 2 * wordsize) == 2 * wordsize)
+    {
+      if (wordsize == 4)
+	{
+	  unsigned int *data_p = (unsigned int *)data;
+	  if (data_p[0] == AT_HWCAP)
+	    {
+	      *valp = data_p[1];
+	      return 1;
+	    }
+	}
+      else
+	{
+	  unsigned long *data_p = (unsigned long *)data;
+	  if (data_p[0] == AT_HWCAP)
+	    {
+	      *valp = data_p[1];
+	      return 1;
+	    }
+	}
+
+      offset += 2 * wordsize;
+    }
+
+  *valp = 0;
+  return 0;
+}
+
 static void
 ppc_arch_setup (void)
 {
@@ -174,28 +224,37 @@ ppc_arch_setup (void)
   long msr;
 
   /* On a 64-bit host, assume 64-bit inferior process.  */
-#ifdef __ALTIVEC__
-  init_registers_powerpc_64 ();
-#else
   init_registers_ppc64 ();
-#endif
 
   /* Only if the high bit of the MSR is set, we actually have
      a 64-bit inferior.  */
   collect_register_by_name ("msr", &msr);
   if (msr < 0)
-    return;
+    {
+      ppc_get_hwcap (&ppc_hwcap);
+      if (ppc_hwcap & PPC_FEATURE_HAS_ALTIVEC)
+	init_registers_powerpc_64 ();
+
+      return;
+    }
 #endif
 
   /* OK, we have a 32-bit inferior.  */
-#ifdef __ALTIVEC__
-  init_registers_powerpc_32 ();
-#else
-#ifdef __SPE__
-  init_registers_powerpc_e500 ();
-#else
   init_registers_ppc ();
-#endif
+
+  ppc_get_hwcap (&ppc_hwcap);
+  if (ppc_hwcap & PPC_FEATURE_HAS_ALTIVEC)
+    init_registers_powerpc_32 ();
+
+  /* On 32-bit machines, check for SPE registers.
+     Set the low target's regmap field as appropriately.  */
+#ifndef __powerpc64__
+  the_low_target.regmap = ppc_regmap;
+  if (ppc_hwcap & PPC_FEATURE_HAS_SPE)
+    {
+      init_registers_powerpc_e500 ();
+      the_low_target.regmap = ppc_regmap_e500;
+   }
 #endif
 }
 
@@ -232,8 +291,6 @@ static void ppc_fill_gregset (void *buf)
     ppc_collect_ptrace_register (i, (char *) buf + ppc_regmap[i]);
 }
 
-#ifdef __ALTIVEC__
-
 #ifndef PTRACE_GETVRREGS
 #define PTRACE_GETVRREGS 18
 #define PTRACE_SETVRREGS 19
@@ -247,6 +304,9 @@ ppc_fill_vrregset (void *buf)
   int i, base;
   char *regset = buf;
 
+  if (!(ppc_hwcap & PPC_FEATURE_HAS_ALTIVEC))
+    return;
+
   base = find_regno ("vr0");
   for (i = 0; i < 32; i++)
     collect_register (base + i, &regset[i * 16]);
@@ -261,6 +321,9 @@ ppc_store_vrregset (const void *buf)
   int i, base;
   const char *regset = buf;
 
+  if (!(ppc_hwcap & PPC_FEATURE_HAS_ALTIVEC))
+    return;
+
   base = find_regno ("vr0");
   for (i = 0; i < 32; i++)
     supply_register (base + i, &regset[i * 16]);
@@ -269,10 +332,6 @@ ppc_store_vrregset (const void *buf)
   supply_register_by_name ("vrsave", &regset[33 * 16]);
 }
 
-#endif /* __ALTIVEC__ */
-
-#ifdef __SPE__
-
 #ifndef PTRACE_GETEVRREGS
 #define PTRACE_GETEVRREGS	20
 #define PTRACE_SETEVRREGS	21
@@ -291,6 +350,9 @@ ppc_fill_evrregset (void *buf)
   int i, ev0;
   struct gdb_evrregset_t *regset = buf;
 
+  if (!(ppc_hwcap & PPC_FEATURE_HAS_SPE))
+    return;
+
   ev0 = find_regno ("ev0h");
   for (i = 0; i < 32; i++)
     collect_register (ev0 + i, &regset->evr[i]);
@@ -305,6 +367,9 @@ ppc_store_evrregset (const void *buf)
   int i, ev0;
   const struct gdb_evrregset_t *regset = buf;
 
+  if (!(ppc_hwcap & PPC_FEATURE_HAS_SPE))
+    return;
+
   ev0 = find_regno ("ev0h");
   for (i = 0; i < 32; i++)
     supply_register (ev0 + i, &regset->evr[i]);
@@ -312,21 +377,16 @@ ppc_store_evrregset (const void *buf)
   supply_register_by_name ("acc", &regset->acc);
   supply_register_by_name ("spefscr", &regset->spefscr);
 }
-#endif /* __SPE__ */
 
 struct regset_info target_regsets[] = {
   /* List the extra register sets before GENERAL_REGS.  That way we will
      fetch them every time, but still fall back to PTRACE_PEEKUSER for the
      general registers.  Some kernels support these, but not the newer
      PPC_PTRACE_GETREGS.  */
-#ifdef __ALTIVEC__
   { PTRACE_GETVRREGS, PTRACE_SETVRREGS, SIZEOF_VRREGS, EXTENDED_REGS,
     ppc_fill_vrregset, ppc_store_vrregset },
-#endif
-#ifdef __SPE__
   { PTRACE_GETEVRREGS, PTRACE_SETEVRREGS, 32 * 4 + 8 + 4, EXTENDED_REGS,
     ppc_fill_evrregset, ppc_store_evrregset },
-#endif
   { 0, 0, 0, GENERAL_REGS, ppc_fill_gregset, NULL },
   { 0, 0, -1, -1, NULL, NULL }
 };

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

Re: [rfc/rft] ppc gdbserver: autodetect AltiVec and SPE

[Daniel Jacobowitz]

On Thu, Feb 28, 2008 at 05:57:06PM +0100, Ulrich Weigand wrote:
> What you do think about this patch?

It looks great to me.  It may be a little while before I can test
it, though.  Thanks again for working on this!

> - As the regmap was defined only for 32-bit host machines, I've
>   disabled SPE support on 64-bit hosts.  I don't think there are
>   any 64-bit machines with SPE anyway, right?

That's right.

-- 
Daniel Jacobowitz
CodeSourcery

Re: [rfc/rft] ppc gdbserver: autodetect AltiVec and SPE

[Ulrich Weigand]

Daniel Jacobowitz wrote:
> On Thu, Feb 28, 2008 at 05:57:06PM +0100, Ulrich Weigand wrote:
> > What you do think about this patch?
> 
> It looks great to me.  It may be a little while before I can test
> it, though.  Thanks again for working on this!

Did you get around to testing this?

I've found a problem in --multi mode; it did not reset ppc_hwcap
in ppc_arch_setup before accessing the register cache, causing
gdbserver to crash with "unknown register vr0" on a second attach.

Fixed in the version below.

Bye,
Ulrich


ChangeLog:

	* configure.srv [powerpc64-*-linux*]: Remove powerpc-e500.o from
	srv_regobj.  Remove rs6000/powerpc-e500.xml and rs6000/power-spe.xml	
	from reg_xmlfiles.
	* linux-ppc-low.c: Include <elf.h>.
	(PPC_FEATURE_HAS_ALTIVEC, PPC_FEATURE_HAS_SPE): Define.
	(ppc_hwcap): New global variable.
	(ppc_regmap): Remove __SPE__ #ifdef sections.
	(ppc_regmap_e500): New global variable.
	(ppc_cannot_store_register): Remove __SPE__ special case.
	(ppc_get_hwcap): New function.
	(ppc_arch_setup): Use it to determine whether inferior supports
	AltiVec or SPE registers.  Set the_low_target.regmap if appropriate.
	(ppc_fill_vrregset, ppc_store_vrregset): Define unconditionally.
	Do not access registers if target does not support AltiVec.
	(ppc_fill_evrregset, ppc_store_evrregset): Define unconditionally.
	Do not access registers if target does not support SPE.
	(target_regsets): Unconditionally include AltiVec and SPE regsets.

diff -urNp gdb-orig/gdb/gdbserver/configure.srv gdb-head/gdb/gdbserver/configure.srv
--- gdb-orig/gdb/gdbserver/configure.srv	2008-03-26 20:08:19.614861467 +0100
+++ gdb-head/gdb/gdbserver/configure.srv	2008-03-26 16:44:37.000000000 +0100
@@ -106,15 +106,13 @@ case "${target}" in
 			srv_linux_usrregs=yes
 			srv_linux_thread_db=yes
 			;;
-  powerpc64-*-linux*)	srv_regobj="reg-ppc.o powerpc-32.o powerpc-e500.o"
+  powerpc64-*-linux*)	srv_regobj="reg-ppc.o powerpc-32.o"
 			srv_regobj="${srv_regobj} reg-ppc64.o powerpc-64.o"
 			srv_tgtobj="linux-low.o linux-ppc-low.o"
 			srv_xmlfiles="rs6000/powerpc-32.xml"
 			srv_xmlfiles="${srv_xmlfiles} rs6000/power-altivec.xml"
 			srv_xmlfiles="${srv_xmlfiles} rs6000/power-core.xml"
 			srv_xmlfiles="${srv_xmlfiles} rs6000/power-fpu.xml"
-			srv_xmlfiles="${srv_xmlfiles} rs6000/powerpc-e500.xml"
-			srv_xmlfiles="${srv_xmlfiles} rs6000/power-spe.xml"
 			srv_xmlfiles="${srv_xmlfiles} rs6000/powerpc-64.xml"
 			srv_xmlfiles="${srv_xmlfiles} rs6000/power64-core.xml"
 			srv_linux_usrregs=yes
diff -urNp gdb-orig/gdb/gdbserver/linux-ppc-low.c gdb-head/gdb/gdbserver/linux-ppc-low.c
--- gdb-orig/gdb/gdbserver/linux-ppc-low.c	2008-03-26 20:08:19.622860321 +0100
+++ gdb-head/gdb/gdbserver/linux-ppc-low.c	2008-03-26 20:06:45.447120208 +0100
@@ -21,8 +21,16 @@
 #include "server.h"
 #include "linux-low.h"
 
+#include <elf.h>
 #include <asm/ptrace.h>
 
+/* These are in <asm/cputable.h> in current kernels.  */
+#define PPC_FEATURE_HAS_ALTIVEC         0x10000000
+#define PPC_FEATURE_HAS_SPE             0x00800000
+
+static unsigned long ppc_hwcap;
+
+
 /* Defined in auto-generated file reg-ppc.c.  */
 void init_registers_ppc (void);
 /* Defined in auto-generated file powerpc-32.c.  */
@@ -69,16 +77,6 @@ static int ppc_regmap[] =
   PT_R20 * 4,    PT_R21 * 4,    PT_R22 * 4,    PT_R23 * 4,
   PT_R24 * 4,    PT_R25 * 4,    PT_R26 * 4,    PT_R27 * 4,
   PT_R28 * 4,    PT_R29 * 4,    PT_R30 * 4,    PT_R31 * 4,
-#ifdef __SPE__
-  -1,            -1,            -1,            -1,
-  -1,            -1,            -1,            -1,
-  -1,            -1,            -1,            -1,
-  -1,            -1,            -1,            -1,
-  -1,            -1,            -1,            -1,
-  -1,            -1,            -1,            -1,
-  -1,            -1,            -1,            -1,
-  -1,            -1,            -1,            -1,
-#else
   PT_FPR0*4,     PT_FPR0*4 + 8, PT_FPR0*4+16,  PT_FPR0*4+24,
   PT_FPR0*4+32,  PT_FPR0*4+40,  PT_FPR0*4+48,  PT_FPR0*4+56,
   PT_FPR0*4+64,  PT_FPR0*4+72,  PT_FPR0*4+80,  PT_FPR0*4+88,
@@ -87,20 +85,36 @@ static int ppc_regmap[] =
   PT_FPR0*4+160,  PT_FPR0*4+168,  PT_FPR0*4+176,  PT_FPR0*4+184,
   PT_FPR0*4+192,  PT_FPR0*4+200,  PT_FPR0*4+208,  PT_FPR0*4+216,
   PT_FPR0*4+224,  PT_FPR0*4+232,  PT_FPR0*4+240,  PT_FPR0*4+248,
-#endif
   PT_NIP * 4,    PT_MSR * 4,    PT_CCR * 4,    PT_LNK * 4,
-#ifdef __SPE__
-  PT_CTR * 4,    PT_XER * 4,    -1
-#else
   PT_CTR * 4,    PT_XER * 4,    PT_FPSCR * 4
-#endif
+ };
+
+static int ppc_regmap_e500[] =
+ {PT_R0 * 4,     PT_R1 * 4,     PT_R2 * 4,     PT_R3 * 4,
+  PT_R4 * 4,     PT_R5 * 4,     PT_R6 * 4,     PT_R7 * 4,
+  PT_R8 * 4,     PT_R9 * 4,     PT_R10 * 4,    PT_R11 * 4,
+  PT_R12 * 4,    PT_R13 * 4,    PT_R14 * 4,    PT_R15 * 4,
+  PT_R16 * 4,    PT_R17 * 4,    PT_R18 * 4,    PT_R19 * 4,
+  PT_R20 * 4,    PT_R21 * 4,    PT_R22 * 4,    PT_R23 * 4,
+  PT_R24 * 4,    PT_R25 * 4,    PT_R26 * 4,    PT_R27 * 4,
+  PT_R28 * 4,    PT_R29 * 4,    PT_R30 * 4,    PT_R31 * 4,
+  -1,            -1,            -1,            -1,
+  -1,            -1,            -1,            -1,
+  -1,            -1,            -1,            -1,
+  -1,            -1,            -1,            -1,
+  -1,            -1,            -1,            -1,
+  -1,            -1,            -1,            -1,
+  -1,            -1,            -1,            -1,
+  -1,            -1,            -1,            -1,
+  PT_NIP * 4,    PT_MSR * 4,    PT_CCR * 4,    PT_LNK * 4,
+  PT_CTR * 4,    PT_XER * 4,    -1
  };
 #endif
 
 static int
 ppc_cannot_store_register (int regno)
 {
-#if !defined (__powerpc64__) && !defined (__SPE__)
+#ifndef __powerpc64__
   /* Some kernels do not allow us to store fpscr.  */
   if (regno == find_regno ("fpscr"))
     return 2;
@@ -167,35 +181,83 @@ ppc_set_pc (CORE_ADDR pc)
     }
 }
 
+
+static int
+ppc_get_hwcap (unsigned long *valp)
+{
+  int wordsize = register_size (0);
+  unsigned char *data = alloca (2 * wordsize);
+  int offset = 0;
+
+  while ((*the_target->read_auxv) (offset, data, 2 * wordsize) == 2 * wordsize)
+    {
+      if (wordsize == 4)
+	{
+	  unsigned int *data_p = (unsigned int *)data;
+	  if (data_p[0] == AT_HWCAP)
+	    {
+	      *valp = data_p[1];
+	      return 1;
+	    }
+	}
+      else
+	{
+	  unsigned long *data_p = (unsigned long *)data;
+	  if (data_p[0] == AT_HWCAP)
+	    {
+	      *valp = data_p[1];
+	      return 1;
+	    }
+	}
+
+      offset += 2 * wordsize;
+    }
+
+  *valp = 0;
+  return 0;
+}
+
 static void
 ppc_arch_setup (void)
 {
 #ifdef __powerpc64__
   long msr;
 
-  /* On a 64-bit host, assume 64-bit inferior process.  */
-#ifdef __ALTIVEC__
-  init_registers_powerpc_64 ();
-#else
+  /* On a 64-bit host, assume 64-bit inferior process with no
+     AltiVec registers.  This is required to make sure the
+     collect_register call below does not fail.  */
   init_registers_ppc64 ();
-#endif
+  ppc_hwcap = 0;
 
   /* Only if the high bit of the MSR is set, we actually have
      a 64-bit inferior.  */
   collect_register_by_name ("msr", &msr);
   if (msr < 0)
-    return;
+    {
+      ppc_get_hwcap (&ppc_hwcap);
+      if (ppc_hwcap & PPC_FEATURE_HAS_ALTIVEC)
+	init_registers_powerpc_64 ();
+
+      return;
+    }
 #endif
 
   /* OK, we have a 32-bit inferior.  */
-#ifdef __ALTIVEC__
-  init_registers_powerpc_32 ();
-#else
-#ifdef __SPE__
-  init_registers_powerpc_e500 ();
-#else
   init_registers_ppc ();
-#endif
+
+  ppc_get_hwcap (&ppc_hwcap);
+  if (ppc_hwcap & PPC_FEATURE_HAS_ALTIVEC)
+    init_registers_powerpc_32 ();
+
+  /* On 32-bit machines, check for SPE registers.
+     Set the low target's regmap field as appropriately.  */
+#ifndef __powerpc64__
+  the_low_target.regmap = ppc_regmap;
+  if (ppc_hwcap & PPC_FEATURE_HAS_SPE)
+    {
+      init_registers_powerpc_e500 ();
+      the_low_target.regmap = ppc_regmap_e500;
+   }
 #endif
 }
 
@@ -232,8 +294,6 @@ static void ppc_fill_gregset (void *buf)
     ppc_collect_ptrace_register (i, (char *) buf + ppc_regmap[i]);
 }
 
-#ifdef __ALTIVEC__
-
 #ifndef PTRACE_GETVRREGS
 #define PTRACE_GETVRREGS 18
 #define PTRACE_SETVRREGS 19
@@ -247,6 +307,9 @@ ppc_fill_vrregset (void *buf)
   int i, base;
   char *regset = buf;
 
+  if (!(ppc_hwcap & PPC_FEATURE_HAS_ALTIVEC))
+    return;
+
   base = find_regno ("vr0");
   for (i = 0; i < 32; i++)
     collect_register (base + i, &regset[i * 16]);
@@ -261,6 +324,9 @@ ppc_store_vrregset (const void *buf)
   int i, base;
   const char *regset = buf;
 
+  if (!(ppc_hwcap & PPC_FEATURE_HAS_ALTIVEC))
+    return;
+
   base = find_regno ("vr0");
   for (i = 0; i < 32; i++)
     supply_register (base + i, &regset[i * 16]);
@@ -269,10 +335,6 @@ ppc_store_vrregset (const void *buf)
   supply_register_by_name ("vrsave", &regset[33 * 16]);
 }
 
-#endif /* __ALTIVEC__ */
-
-#ifdef __SPE__
-
 #ifndef PTRACE_GETEVRREGS
 #define PTRACE_GETEVRREGS	20
 #define PTRACE_SETEVRREGS	21
@@ -291,6 +353,9 @@ ppc_fill_evrregset (void *buf)
   int i, ev0;
   struct gdb_evrregset_t *regset = buf;
 
+  if (!(ppc_hwcap & PPC_FEATURE_HAS_SPE))
+    return;
+
   ev0 = find_regno ("ev0h");
   for (i = 0; i < 32; i++)
     collect_register (ev0 + i, &regset->evr[i]);
@@ -305,6 +370,9 @@ ppc_store_evrregset (const void *buf)
   int i, ev0;
   const struct gdb_evrregset_t *regset = buf;
 
+  if (!(ppc_hwcap & PPC_FEATURE_HAS_SPE))
+    return;
+
   ev0 = find_regno ("ev0h");
   for (i = 0; i < 32; i++)
     supply_register (ev0 + i, &regset->evr[i]);
@@ -312,21 +380,16 @@ ppc_store_evrregset (const void *buf)
   supply_register_by_name ("acc", &regset->acc);
   supply_register_by_name ("spefscr", &regset->spefscr);
 }
-#endif /* __SPE__ */
 
 struct regset_info target_regsets[] = {
   /* List the extra register sets before GENERAL_REGS.  That way we will
      fetch them every time, but still fall back to PTRACE_PEEKUSER for the
      general registers.  Some kernels support these, but not the newer
      PPC_PTRACE_GETREGS.  */
-#ifdef __ALTIVEC__
   { PTRACE_GETVRREGS, PTRACE_SETVRREGS, SIZEOF_VRREGS, EXTENDED_REGS,
     ppc_fill_vrregset, ppc_store_vrregset },
-#endif
-#ifdef __SPE__
   { PTRACE_GETEVRREGS, PTRACE_SETEVRREGS, 32 * 4 + 8 + 4, EXTENDED_REGS,
     ppc_fill_evrregset, ppc_store_evrregset },
-#endif
   { 0, 0, 0, GENERAL_REGS, ppc_fill_gregset, NULL },
   { 0, 0, -1, -1, NULL, NULL }
 };


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

Re: [rfc/rft] ppc gdbserver: autodetect AltiVec and SPE

[Daniel Jacobowitz]

On Thu, Feb 28, 2008 at 05:57:06PM +0100, Ulrich Weigand wrote:
> - There was a conditional check for fpscr in ppc_cannot_store_register;
>   but as this routine is never called for fpscr in SPE mode, the check
>   seems superfluous and I just removed it.

This doesn't seem to be true.

gdbserver: Unknown register fpscr requested

There's no code to use PPC_PTRACE_GETREGS, so we do in fact reach
here.  I'm retesting with

  /* Some kernels do not allow us to store fpscr.  */
  if (!(ppc_hwcap & PPC_FEATURE_HAS_SPE) && regno == find_regno ("fpscr"))
    return 2;

-- 
Daniel Jacobowitz
CodeSourcery

Re: [rfc/rft] ppc gdbserver: autodetect AltiVec and SPE

[Daniel Jacobowitz]

On Thu, Apr 17, 2008 at 01:28:01PM -0400, Daniel Jacobowitz wrote:
> This doesn't seem to be true.
> 
> gdbserver: Unknown register fpscr requested
> 
> There's no code to use PPC_PTRACE_GETREGS, so we do in fact reach
> here.  I'm retesting with
> 
>   /* Some kernels do not allow us to store fpscr.  */
>   if (!(ppc_hwcap & PPC_FEATURE_HAS_SPE) && regno == find_regno ("fpscr"))
>     return 2;

Unfortunately that didn't fix it.

FAIL -> PASS: default/gdb.sum:gdb.threads/linux-dp.exp: continue to
breakpoint: about to create philosopher: 2
PASS -> FAIL: te500v2/gdb.sum:gdb.base/store.exp: upvar double l;
print old r, expecting -2
PASS -> FAIL: te500v2/gdb.sum:gdb.base/store.exp: var double l; print
old r, expecting -2
PASS -> FAIL: te500v2/gdb.sum:gdb.base/store.exp: var doublest l;
print old r, expecting -2
PASS -> FAIL: te500v2/gdb.sum:gdb.threads/linux-dp.exp: continue to
breakpoint: about to create philosopher: 4
PASS -> FAIL: te600/gdb.sum:gdb.threads/linux-dp.exp: continue to
breakpoint: about to create philosopher: 1
PASS -> FAIL: te600/gdb.sum:gdb.threads/linux-dp.exp: continue to
breakpoint: about to create philosopher: 2

The linux-dp.exp failures are noise; I have a patch.  The store.exp
failures mean that SPE registers aren't working.

It looks like we've fallen afoul of the code which fetches the
first register set, then if it is not available does not try any
further register sets.  So when we try PTRACE_GETVRREGS and fail
we never try PTRACE_GETEVRREGS.

This code's bitten me before.  It's even worse now, because if which
ptrace operations succeed depends on the inferior's mode, the values
may change during multi.  So I've pulled it out.  The attached patch,
combined with yours, produces no regressions on any of (SPE, AltiVec,
classic PowerPC).

Want to check them both in?

-- 
Daniel Jacobowitz
CodeSourcery

2008-04-17  Daniel Jacobowitz  <dan@codesourcery.com>

	* linux-ppc-low.c (ppc_cannot_store_register): Skip for SPE.
	* linux-low.c (disabled_regsets, num_regsets): New.
	(use_regsets_p): Delete.
	(linux_wait_for_process): Clear disabled_regsets.
	(regsets_fetch_inferior_registers): Check and set it.
	(regsets_store_inferior_registers): Likewise.
	(linux_fetch_registers, linux_store_registers): Do not use
	use_regsets_p.
	(initialize_low): Allocate disabled_regsets.

2008-04-17  Daniel Jacobowitz  <dan@codesourcery.com>

	* gdb.threads/linux-dp.exp: Continue after unrecognized lines.

diff -u gdb-head/gdb/gdbserver/linux-ppc-low.c src/gdb/gdbserver/linux-ppc-low.c
--- gdb-head/gdb/gdbserver/linux-ppc-low.c	2008-03-26 20:06:45.447120208 +0100
+++ src/gdb/gdbserver/linux-ppc-low.c	17 Apr 2008 17:27:32 -0000
@@ -116,7 +116,7 @@
 {
 #ifndef __powerpc64__
   /* Some kernels do not allow us to store fpscr.  */
-  if (regno == find_regno ("fpscr"))
+  if (!(ppc_hwcap & PPC_FEATURE_HAS_SPE) && regno == find_regno ("fpscr"))
     return 2;
 #endif
 
only in patch2:
unchanged:
--- src/gdb/gdbserver/linux-low.c	28 Feb 2008 05:54:09 -0000	1.76
+++ src/gdb/gdbserver/linux-low.c	17 Apr 2008 18:16:18 -0000
@@ -131,7 +131,8 @@ struct pending_signals
 #define PTRACE_XFER_TYPE long
 
 #ifdef HAVE_LINUX_REGSETS
-static int use_regsets_p = 1;
+static char *disabled_regsets;
+static int num_regsets;
 #endif
 
 #define pid_of(proc) ((proc)->head.id)
@@ -631,6 +632,9 @@ retry:
   if (new_inferior)
     {
       the_low_target.arch_setup ();
+#ifdef HAVE_LINUX_REGSETS
+      memset (disabled_regsets, 0, num_regsets);
+#endif
       new_inferior = 0;
     }
 
@@ -1496,7 +1500,7 @@ regsets_fetch_inferior_registers ()
       void *buf;
       int res;
 
-      if (regset->size == 0)
+      if (regset->size == 0 || disabled_regsets[regset - target_regsets])
 	{
 	  regset ++;
 	  continue;
@@ -1508,18 +1512,10 @@ regsets_fetch_inferior_registers ()
 	{
 	  if (errno == EIO)
 	    {
-	      /* If we get EIO on the first regset, do not try regsets again.
-		 If we get EIO on a later regset, disable that regset.  */
-	      if (regset == target_regsets)
-		{
-		  use_regsets_p = 0;
-		  return -1;
-		}
-	      else
-		{
-		  regset->size = 0;
-		  continue;
-		}
+	      /* If we get EIO on a regset, do not try it again for
+		 this process.  */
+	      disabled_regsets[regset - target_regsets] = 1;
+	      continue;
 	    }
 	  else
 	    {
@@ -1553,7 +1549,7 @@ regsets_store_inferior_registers ()
       void *buf;
       int res;
 
-      if (regset->size == 0)
+      if (regset->size == 0 || disabled_regsets[regset - target_regsets])
 	{
 	  regset ++;
 	  continue;
@@ -1579,18 +1575,10 @@ regsets_store_inferior_registers ()
 	{
 	  if (errno == EIO)
 	    {
-	      /* If we get EIO on the first regset, do not try regsets again.
-		 If we get EIO on a later regset, disable that regset.  */
-	      if (regset == target_regsets)
-		{
-		  use_regsets_p = 0;
-		  return -1;
-		}
-	      else
-		{
-		  regset->size = 0;
-		  continue;
-		}
+	      /* If we get EIO on a regset, do not try it again for
+		 this process.  */
+	      disabled_regsets[regset - target_regsets] = 1;
+	      continue;
 	    }
 	  else
 	    {
@@ -1616,11 +1604,8 @@ void
 linux_fetch_registers (int regno)
 {
 #ifdef HAVE_LINUX_REGSETS
-  if (use_regsets_p)
-    {
-      if (regsets_fetch_inferior_registers () == 0)
-	return;
-    }
+  if (regsets_fetch_inferior_registers () == 0)
+    return;
 #endif
 #ifdef HAVE_LINUX_USRREGS
   usr_fetch_inferior_registers (regno);
@@ -1631,11 +1616,8 @@ void
 linux_store_registers (int regno)
 {
 #ifdef HAVE_LINUX_REGSETS
-  if (use_regsets_p)
-    {
-      if (regsets_store_inferior_registers () == 0)
-	return;
-    }
+  if (regsets_store_inferior_registers () == 0)
+    return;
 #endif
 #ifdef HAVE_LINUX_USRREGS
   usr_store_inferior_registers (regno);
@@ -2084,4 +2066,9 @@ initialize_low (void)
 		       the_low_target.breakpoint_len);
   linux_init_signals ();
   linux_test_for_tracefork ();
+#ifdef HAVE_LINUX_REGSETS
+  for (num_regsets = 0; target_regsets[num_regsets].size >= 0; num_regsets++)
+    ;
+  disabled_regsets = malloc (num_regsets);
+#endif
 }
only in patch2:
unchanged:
--- src/gdb/testsuite/gdb.threads/linux-dp.exp	1 Jan 2008 22:53:22 -0000	1.19
+++ src/gdb/testsuite/gdb.threads/linux-dp.exp	17 Apr 2008 18:28:42 -0000
@@ -80,6 +80,7 @@ for {set i 0} {$i < 5} {incr i} {
 	}
 	-re "^\[^\n\]*\n" {
 	    verbose -log "skipping line" 2
+	    exp_continue -continue_timer
 	}
 	-re "^$gdb_prompt $" {
 	}
@@ -162,6 +163,7 @@ for {set i 0} {$i < 5} {incr i} {
 	}
 	-re "^\[^\n\]*\n" {
 	    verbose -log "skipping line" 2
+	    exp_continue -continue_timer
 	}
 	-re "^$gdb_prompt $" {
 	    if { [llength $threads_before] != 0 } {

Re: [rfc/rft] ppc gdbserver: autodetect AltiVec and SPE

[Ulrich Weigand]

Daniel Jacobowitz wrote:

> It looks like we've fallen afoul of the code which fetches the
> first register set, then if it is not available does not try any
> further register sets.  So when we try PTRACE_GETVRREGS and fail
> we never try PTRACE_GETEVRREGS.
> 
> This code's bitten me before.  It's even worse now, because if which
> ptrace operations succeed depends on the inferior's mode, the values
> may change during multi.  So I've pulled it out.  The attached patch,
> combined with yours, produces no regressions on any of (SPE, AltiVec,
> classic PowerPC).

Ah, good catch.  Thank you very much for testing my patch (and fixing
the problems!).

> Want to check them both in?

Sure, I've now checked them in.

Thanks,
Ulrich

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

Re: [rfc/rft] ppc gdbserver: autodetect AltiVec and SPE

[Thiago Jung Bauermann]

Hi Uli,

On Wed, 2008-03-26 at 21:21 +0100, Ulrich Weigand wrote:
> 	* configure.srv [powerpc64-*-linux*]: Remove powerpc-e500.o from
> 	srv_regobj.  Remove rs6000/powerpc-e500.xml and rs6000/power-spe.xml	
> 	from reg_xmlfiles.

gdbserver is not compiling anymore on Linux/ppc. I believe this patch is
the cause:

gcc -Wall -g -O2    -I. -I../../../gdb.hg/gdb/gdbserver -I../../../gdb.hg/gdb/gdbserver/../regformats -I../../../gdb.hg/gdb/gdbserver/../../include -I../../bfd -I../../../gdb.hg/gdb/gdbserver/../../bfd  -rdynamic -o gdbserver inferiors.o regcache.o remote-utils.o server.o signals.o target.o utils.o version.o mem-break.o hostio.o xml-builtin.o reg-ppc.o powerpc-32.o reg-ppc64.o powerpc-64.o linux-low.o linux-ppc-low.o hostio-errno.o thread-db.o proc-service.o  \
  -lthread_db
linux-ppc-low.o: In function `ppc_arch_setup':
../../../gdb.hg/gdb/gdbserver/linux-ppc-low.c:255: undefined reference to `init_registers_powerpc_e500'
collect2: ld returned 1 exit status
make[4]: *** [gdbserver] Error 1
make[4]: Leaving directory `/home/bauermann/tmp/remove-dead-fn/build/gdb/gdbserver'

Reverting my repository to a date before this commit makes it build
again.
-- 
[]'s
Thiago Jung Bauermann
Software Engineer
IBM Linux Technology Center

Re: [rfc/rft] ppc gdbserver: autodetect AltiVec and SPE

[Daniel Jacobowitz]

On Fri, Apr 18, 2008 at 09:07:25PM -0300, Thiago Jung Bauermann wrote:
> gdbserver is not compiling anymore on Linux/ppc. I believe this patch is
> the cause:
> 
> gcc -Wall -g -O2    -I. -I../../../gdb.hg/gdb/gdbserver -I../../../gdb.hg/gdb/gdbserver/../regformats -I../../../gdb.hg/gdb/gdbserver/../../include -I../../bfd -I../../../gdb.hg/gdb/gdbserver/../../bfd  -rdynamic -o gdbserver inferiors.o regcache.o remote-utils.o server.o signals.o target.o utils.o version.o mem-break.o hostio.o xml-builtin.o reg-ppc.o powerpc-32.o reg-ppc64.o powerpc-64.o linux-low.o linux-ppc-low.o hostio-errno.o thread-db.o proc-service.o  \
>   -lthread_db
> linux-ppc-low.o: In function `ppc_arch_setup':
> ../../../gdb.hg/gdb/gdbserver/linux-ppc-low.c:255: undefined reference to `init_registers_powerpc_e500'
> collect2: ld returned 1 exit status
> make[4]: *** [gdbserver] Error 1
> make[4]: Leaving directory `/home/bauermann/tmp/remove-dead-fn/build/gdb/gdbserver'

Looks like you have to rerun configure.  Maybe a missing dependency
somewhere (Makefile on configure.srv?).

-- 
Daniel Jacobowitz
CodeSourcery

Re: [rfc/rft] ppc gdbserver: autodetect AltiVec and SPE

[Thiago Jung Bauermann]

On Fri, 2008-04-18 at 23:58 -0400, Daniel Jacobowitz wrote:
> On Fri, Apr 18, 2008 at 09:07:25PM -0300, Thiago Jung Bauermann wrote:
> > gdbserver is not compiling anymore on Linux/ppc. I believe this patch is
> > the cause:
> > 
> > gcc -Wall -g -O2    -I. -I../../../gdb.hg/gdb/gdbserver -I../../../gdb.hg/gdb/gdbserver/../regformats -I../../../gdb.hg/gdb/gdbserver/../../include -I../../bfd -I../../../gdb.hg/gdb/gdbserver/../../bfd  -rdynamic -o gdbserver inferiors.o regcache.o remote-utils.o server.o signals.o target.o utils.o version.o mem-break.o hostio.o xml-builtin.o reg-ppc.o powerpc-32.o reg-ppc64.o powerpc-64.o linux-low.o linux-ppc-low.o hostio-errno.o thread-db.o proc-service.o  \
> >   -lthread_db
> > linux-ppc-low.o: In function `ppc_arch_setup':
> > ../../../gdb.hg/gdb/gdbserver/linux-ppc-low.c:255: undefined reference to `init_registers_powerpc_e500'
> > collect2: ld returned 1 exit status
> > make[4]: *** [gdbserver] Error 1
> > make[4]: Leaving directory `/home/bauermann/tmp/remove-dead-fn/build/gdb/gdbserver'
> 
> Looks like you have to rerun configure.  Maybe a missing dependency
> somewhere (Makefile on configure.srv?).

No, it was a clean build. I got what's happening. The patch removed
powerpc-e500.o from powerpc64-*-linux*, because e500 is 32-bit only. The
call to init_registers_powerpc_e500 is protected by an #ifndef
__powerpc64__, so it shouldn't be a problem. But in practice it is,
because when using plain "../src/configure && make" in a ppc64 machine
with a 32-bit default gcc, the target is autodetected to be
powerpc64-unknown-linux-gnu. Thus, that call to
init_registers_powerpc_e500 will be compiled in (it's a 32 bit
compilation), but powerpc-e500.o won't be included (it's a powerpc64
target).

Is such way of compiling 32-bit GDB in a ppc64 machine with 32-bit
default gcc not supported? Must one always use --target option to
configure in such case?
-- 
[]'s
Thiago Jung Bauermann
Software Engineer
IBM Linux Technology Center

Re: [rfc/rft] ppc gdbserver: autodetect AltiVec and SPE

[Daniel Jacobowitz]

On Sun, Apr 20, 2008 at 01:51:59AM -0300, Thiago Jung Bauermann wrote:
> No, it was a clean build. I got what's happening. The patch removed
> powerpc-e500.o from powerpc64-*-linux*, because e500 is 32-bit only. The
> call to init_registers_powerpc_e500 is protected by an #ifndef
> __powerpc64__, so it shouldn't be a problem. But in practice it is,
> because when using plain "../src/configure && make" in a ppc64 machine
> with a 32-bit default gcc, the target is autodetected to be
> powerpc64-unknown-linux-gnu. Thus, that call to
> init_registers_powerpc_e500 will be compiled in (it's a 32 bit
> compilation), but powerpc-e500.o won't be included (it's a powerpc64
> target).
> 
> Is such way of compiling 32-bit GDB in a ppc64 machine with 32-bit
> default gcc not supported? Must one always use --target option to
> configure in such case?

I see.  Anything is supported that we're willing to fix... The
simplest possible fix would be to add the e500 objects.  Does
gdbserver work OK with this patch?

-- 
Daniel Jacobowitz
CodeSourcery

2008-04-20  Daniel Jacobowitz  <dan@codesourcery.com>

	* configure.srv: Add e500 objects even for powerpc64.

Index: configure.srv
===================================================================
RCS file: /cvs/src/src/gdb/gdbserver/configure.srv,v
retrieving revision 1.33
diff -u -p -r1.33 configure.srv
--- configure.srv	17 Apr 2008 21:22:41 -0000	1.33
+++ configure.srv	20 Apr 2008 14:27:03 -0000
@@ -106,7 +106,7 @@ case "${target}" in
 			srv_linux_usrregs=yes
 			srv_linux_thread_db=yes
 			;;
-  powerpc64-*-linux*)	srv_regobj="reg-ppc.o powerpc-32.o"
+  powerpc64-*-linux*)	srv_regobj="reg-ppc.o powerpc-32.o powerpc-e500.o"
 			srv_regobj="${srv_regobj} reg-ppc64.o powerpc-64.o"
 			srv_tgtobj="linux-low.o linux-ppc-low.o"
 			srv_xmlfiles="rs6000/powerpc-32.xml"
@@ -115,6 +115,8 @@ case "${target}" in
 			srv_xmlfiles="${srv_xmlfiles} rs6000/power-fpu.xml"
 			srv_xmlfiles="${srv_xmlfiles} rs6000/powerpc-64.xml"
 			srv_xmlfiles="${srv_xmlfiles} rs6000/power64-core.xml"
+			srv_xmlfiles="${srv_xmlfiles} rs6000/powerpc-e500.xml"
+			srv_xmlfiles="${srv_xmlfiles} rs6000/power-spe.xml"
 			srv_linux_usrregs=yes
 			srv_linux_regsets=yes
 			srv_linux_thread_db=yes

Re: [rfc/rft] ppc gdbserver: autodetect AltiVec and SPE

[Thiago Jung Bauermann]

On Sun, 2008-04-20 at 10:27 -0400, Daniel Jacobowitz wrote:
> > Is such way of compiling 32-bit GDB in a ppc64 machine with 32-bit
> > default gcc not supported? Must one always use --target option to
> > configure in such case?
> 
> I see.  Anything is supported that we're willing to fix... The
> simplest possible fix would be to add the e500 objects.  Does
> gdbserver work OK with this patch?

Yes, it compiled fine. Thanks!
-- 
[]'s
Thiago Jung Bauermann
Software Engineer
IBM Linux Technology Center

Re: [rfc/rft] ppc gdbserver: autodetect AltiVec and SPE

[Ulrich Weigand]

Thiago Jung Bauermann wrote:
> On Sun, 2008-04-20 at 10:27 -0400, Daniel Jacobowitz wrote:
> > > Is such way of compiling 32-bit GDB in a ppc64 machine with 32-bit
> > > default gcc not supported? Must one always use --target option to
> > > configure in such case?
> > 
> > I see.  Anything is supported that we're willing to fix... The
> > simplest possible fix would be to add the e500 objects.  Does
> > gdbserver work OK with this patch?
> 
> Yes, it compiled fine. Thanks!

Sorry for the breakage.  My understanding is that building a 32-bit
powerpc64-linux gdbserver was never supported, and would fail at 
run time because ptrace does not support accessing 64-bit inferior
registers from a 32-bit debugger (at least not using the standard
ptrace functions).

However, with bi-arch support, I guess it now would actually work
at least for debugging 32-bit inferiors.

I'm wondering whether it wouldn't make sense now to drop the
configure-time distinction between powerpc-linux and powerpc64-linux
for gdbserver (just like there already is no such distinction 
for GDB itself).

This would mean no matter whether you configure for powerpc-linux
or for powerpc64-linux, the resulting gdbserver would always support
32-bit inferiors, and would in addition support 64-bit inferiors iff
it was built as a 64-bit executable.

Dan, what do you think?

Bye,
Ulrich

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

Re: [rfc/rft] ppc gdbserver: autodetect AltiVec and SPE

[Daniel Jacobowitz]

On Mon, Apr 21, 2008 at 02:55:26PM +0200, Ulrich Weigand wrote:
> I'm wondering whether it wouldn't make sense now to drop the
> configure-time distinction between powerpc-linux and powerpc64-linux
> for gdbserver (just like there already is no such distinction 
> for GDB itself).

Agreed.  We already know that the ppc64 files compile OK with -m32,
since Thiago's doing it.

-- 
Daniel Jacobowitz
CodeSourcery

Re: [rfc/rft] ppc gdbserver: autodetect AltiVec and SPE

[Ulrich Weigand]

Daniel Jacobowitz wrote:
> On Mon, Apr 21, 2008 at 02:55:26PM +0200, Ulrich Weigand wrote:
> > I'm wondering whether it wouldn't make sense now to drop the
> > configure-time distinction between powerpc-linux and powerpc64-linux
> > for gdbserver (just like there already is no such distinction 
> > for GDB itself).
> 
> Agreed.  We already know that the ppc64 files compile OK with -m32,
> since Thiago's doing it.

OK, I've committed the following patch to implement this merge
(and the same for s390-linux vs. s390x-linux).  Tested on 
powerpc-linux, powerpc64-linux, s390-linux, and s390x-linux
via local gdbserver testing.

Bye,
Ulrich


ChangeLog:

	* configure.srv (powerpc64-*-linux*, powerpc-*-linux*): Merge into single
	powerpc*-*-linux* case.
	(s390-*-linux*, s390x-*-linux*): Merge into single s390*-*-linux* case.


diff -urNp gdb-orig/gdb/gdbserver/configure.srv gdb-head/gdb/gdbserver/configure.srv
--- gdb-orig/gdb/gdbserver/configure.srv	2008-04-21 15:15:37.000000000 +0200
+++ gdb-head/gdb/gdbserver/configure.srv	2008-04-21 15:16:50.000000000 +0200
@@ -106,38 +106,22 @@ case "${target}" in
 			srv_linux_usrregs=yes
 			srv_linux_thread_db=yes
 			;;
-  powerpc64-*-linux*)	srv_regobj="reg-ppc.o powerpc-32.o"
+  powerpc*-*-linux*)	srv_regobj="reg-ppc.o powerpc-32.o powerpc-e500.o"
 			srv_regobj="${srv_regobj} reg-ppc64.o powerpc-64.o"
 			srv_tgtobj="linux-low.o linux-ppc-low.o"
 			srv_xmlfiles="rs6000/powerpc-32.xml"
 			srv_xmlfiles="${srv_xmlfiles} rs6000/power-altivec.xml"
 			srv_xmlfiles="${srv_xmlfiles} rs6000/power-core.xml"
 			srv_xmlfiles="${srv_xmlfiles} rs6000/power-fpu.xml"
-			srv_xmlfiles="${srv_xmlfiles} rs6000/powerpc-64.xml"
-			srv_xmlfiles="${srv_xmlfiles} rs6000/power64-core.xml"
-			srv_linux_usrregs=yes
-			srv_linux_regsets=yes
-			srv_linux_thread_db=yes
-			;;
-  powerpc-*-linux*)	srv_regobj="reg-ppc.o powerpc-32.o powerpc-e500.o"
-			srv_tgtobj="linux-low.o linux-ppc-low.o"
-			srv_xmlfiles="rs6000/powerpc-32.xml"
-			srv_xmlfiles="${srv_xmlfiles} rs6000/power-altivec.xml"
-			srv_xmlfiles="${srv_xmlfiles} rs6000/power-core.xml"
-			srv_xmlfiles="${srv_xmlfiles} rs6000/power-fpu.xml"
 			srv_xmlfiles="${srv_xmlfiles} rs6000/powerpc-e500.xml"
 			srv_xmlfiles="${srv_xmlfiles} rs6000/power-spe.xml"
+			srv_xmlfiles="${srv_xmlfiles} rs6000/powerpc-64.xml"
+			srv_xmlfiles="${srv_xmlfiles} rs6000/power64-core.xml"
 			srv_linux_usrregs=yes
 			srv_linux_regsets=yes
 			srv_linux_thread_db=yes
 			;;
-  s390-*-linux*)	srv_regobj=reg-s390.o
-			srv_tgtobj="linux-low.o linux-s390-low.o"
-			srv_linux_usrregs=yes
-			srv_linux_regsets=yes
-			srv_linux_thread_db=yes
-			;;
-  s390x-*-linux*)	srv_regobj="reg-s390.o reg-s390x.o"
+  s390*-*-linux*)	srv_regobj="reg-s390.o reg-s390x.o"
 			srv_tgtobj="linux-low.o linux-s390-low.o"
 			srv_linux_usrregs=yes
 			srv_linux_regsets=yes


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

Re: [rfc/rft] ppc gdbserver: autodetect AltiVec and SPE

[Daniel Jacobowitz]

On Mon, Apr 21, 2008 at 04:58:10PM +0200, Ulrich Weigand wrote:
> OK, I've committed the following patch to implement this merge
> (and the same for s390-linux vs. s390x-linux).  Tested on 
> powerpc-linux, powerpc64-linux, s390-linux, and s390x-linux
> via local gdbserver testing.

Thanks!

-- 
Daniel Jacobowitz
CodeSourcery