[RFA] ppc-linux-nat.c AltiVec regs ptrace

[RFA] ppc-linux-nat.c AltiVec regs ptrace

[Elena Zannoni]

This patch adds support for fetching and storing the AltiVec registers
on PPC.  I previously submitted a different patch based on a different
ptrace interface that was rejected by the ppc linux kernel maintainer.
A new ptrace interface has since been added to the kernel:
http://ppc.bkbits.net:8080/linuxppc_2_4/cset@1.543.1.14

This was the old patch, which is now obsolete:
http://sources.redhat.com/ml/gdb-patches/2001-11/msg00548.html


The Altivec registers are defined like this:
#define ELF_NVRREG      33      /* includes vscr */
/* Altivec registers */
typedef __vector128 elf_vrreg_t;
typedef elf_vrreg_t elf_vrregset_t[ELF_NVRREG];

There are 32 vector registers 16 bytes longs, plus a VSCR register
which is only 4 bytes long, but is fetched as a 16 bytes quantity. Up
to here we have the elf_vrregset_t structure.
Appended to this there is space for the VRSAVE register: 4 bytes.
Even though this vrsave register is not included in the regset
typedef, it is handled by the ptrace requests.

The layout is like this:

 |.|.|.|.|.....|.|.|.|.||.|.|.|x||.|
 <------->     <-------><-------><->
   VR0           VR31     VSCR    VRSAVE
(where x is the actual value of the vscr reg)


I have added a typedef to the ppc-linux-nat.c file to deal with
such a layout. I am not sure whether this is the best place to put
that. Maybe in gregset.h?


Elena


2002-02-19  Elena Zannoni  <ezannoni@redhat.com>

	* ppc-linux-nat.c (PTRACE_GETVRREGS, PTRACE_SETVRREGS): Define.
	(have_ptrace_getvrregs): Define for run time checks.
	(gdb_vrregset_t): New type for Altivec register handling.
	(fetch_register, store_register): Fetch/store altivec register
	when needed.
	(fetch_altivec_register, store_altivec-register): New functions.
	(supply_vrregset, fill_vrregset): New functions.
	(fetch_altivec_registers, store_altivec_registers): New functions.
	(fetch_ppc_registers, store_ppc_registers): Fetch/store altivec
	registers as well.

Index: ppc-linux-nat.c
===================================================================
RCS file: /cvs/uberbaum/gdb/ppc-linux-nat.c,v
retrieving revision 1.15
diff -u -p -r1.15 ppc-linux-nat.c
--- ppc-linux-nat.c	2002/02/18 15:08:40	1.15
+++ ppc-linux-nat.c	2002/02/20 19:27:29
@@ -17,7 +17,7 @@
    You should have received a copy of the GNU General Public License
    along with this program; if not, write to the Free Software
    Foundation, Inc., 59 Temple Place - Suite 330,
-   Boston, MA 02111-1307, USA.  */
+   Boston, MA 02111-1307, USA.*/
 
 #include "defs.h"
 #include "frame.h"
@@ -51,6 +51,39 @@
 #define PTRACE_XFER_TYPE int
 #endif
 
+/* This is necessary because of a glibc vs. kernel headers mess up.
+   Glibc's sys/ptrace.h defines PTRACE_GETFPXREGS and
+   PTRACE_SETFPXREGS to the same numbers that ppc's asm/ptrace.h
+   defines PTRACE_GETVRREGS and PTRACE_SETVRREGS to be. To complicate
+   matters further, the presence of such macros in glibc's ptrace.h
+   file doesn't guarantee that there is kernel support for
+   fetching/storing the AltiVec registers. So we have to fall back to
+   do a runtime check as well.  A similar trick is done for the
+   i386. */
+
+#ifdef HAVE_PTRACE_GETFPXREGS
+/* These definitions don't really have any functional impact, they are here 
+   to ease understanding of the ptrace calls. Glibc doesn't know about the
+   vrregs yet.*/
+#define PTRACE_GETVRREGS 18
+#define PTRACE_SETVRREGS 19
+
+/* This oddity is because the linux kernel defines elf_vrregset_t as
+   an array of 33 16 bytes long elements. I.e. it leaves out vrsave.
+   However the PTRACE_GETVRREGS and PTRACE_SETVRREGS requests return the
+   vrsave as an extra 4 bytes at the end. I opted for creating a flat
+   array of chars, so that it is easier to manipulate for gdb.  */
+#define SIZEOF_VRREGS 33*16+4
+typedef char gdb_vrregset_t[SIZEOF_VRREGS];
+#endif
+
+int have_ptrace_getvrregs
+#ifdef HAVE_PTRACE_GETFPXREGS
+     = 1;
+#else
+     = 0;
+#endif
+
 int
 kernel_u_size (void)
 {
@@ -109,6 +142,53 @@ ppc_ptrace_cannot_fetch_store_register (
   return (ppc_register_u_addr (regno) == -1);
 }
 
+#ifdef HAVE_PTRACE_GETFPXREGS
+/* The linux kernel ptrace interface for AltiVec registers uses the
+   registers set mechanism, as opposed to the interface for all the
+   other registers, that stores/fetches each register individually. */
+static void
+fetch_altivec_register (int tid, int regno)
+{
+  int ret;
+  int offset = 0;
+  gdb_vrregset_t regs;
+  struct gdbarch_tdep *tdep = gdbarch_tdep (current_gdbarch);
+  int vrregsize = REGISTER_RAW_SIZE (tdep->ppc_vr0_regnum);
+
+  ret = ptrace (PTRACE_GETVRREGS, tid, 0, &regs);
+  if (ret < 0)
+    {
+      if (errno == EIO)
+        {
+          have_ptrace_getvrregs = 0;
+          return;
+        }
+      perror_with_name ("Unable to fetch AltiVec register");
+    }
+ 
+  /* VSCR is fetched as a 16 bytes quantity, but it is really
+     4 bytes long on the hardware. We deal only with the lower
+     4 bytes of the vector. 
+     VRSAVE is at the end of the array in a 4 bytes slot, so there is no
+     need to define an offset for it. */
+  if (regno == (tdep->ppc_vrsave_regnum - 1))
+    offset = vrregsize - REGISTER_RAW_SIZE (tdep->ppc_vrsave_regnum);
+  
+  supply_register (regno,
+                   regs + (regno - tdep->ppc_vr0_regnum) * vrregsize + offset);
+}
+
+#else
+static void
+fetch_altivec_register (int tid, int regno)
+{
+  internal_error (__FILE__, __LINE__,
+		  "fetch_altivec_register: HAVE_PTRACE_GETFPXREGS not defined
+\nand have_ptrace_getvrregs set.");
+  return;
+}
+#endif /* HAVE_PTRACE_GETFPXREGS */
+
 static void
 fetch_register (int tid, int regno)
 {
@@ -119,6 +199,21 @@ fetch_register (int tid, int regno)
   char *buf = alloca (MAX_REGISTER_RAW_SIZE);
   CORE_ADDR regaddr = ppc_register_u_addr (regno);
 
+  if (altivec_register_p (regno))
+    {
+      /* If this is the first time through, or if it is not the first time
+         through, and we have comfirmed that there is kernel support for
+         such a ptrace request, then go and fetch the register. */
+      if (have_ptrace_getvrregs)
+       {
+         fetch_altivec_register (tid, regno);
+         return;
+       }
+     /* If we have discovered that there is no ptrace support for
+        AltiVec registers, fall through and return zeroes, because regaddr
+        will be -1 in this case. */
+    }
+
   if (regaddr == -1)
     {
       memset (buf, '\0', REGISTER_RAW_SIZE (regno));   /* Supply zeroes */
@@ -142,14 +237,70 @@ fetch_register (int tid, int regno)
   supply_register (regno, buf);
 }
 
+static void
+supply_vrregset (gdb_vrregset_t *vrregsetp)
+{
+  int i;
+  struct gdbarch_tdep *tdep = gdbarch_tdep (current_gdbarch);
+  int num_of_vrregs = tdep->ppc_vrsave_regnum - tdep->ppc_vr0_regnum;
+  int vrregsize = REGISTER_RAW_SIZE (tdep->ppc_vr0_regnum);
+  int offset = vrregsize - REGISTER_RAW_SIZE (tdep->ppc_vrsave_regnum);
+
+  for (i = 0; i < num_of_vrregs - 1; i++)
+    {
+      /* The last 2 registers of this set are only 32 bit long, not 128.
+         However an offset is necessary only for VSCR because it occupies 
+         a whole vector, while VRSAVE occupies a full 4 bytes slot. */
+      if (i == (tdep->ppc_vrsave_regnum - 1))
+        supply_register (tdep->ppc_vr0_regnum + i,
+                         *vrregsetp + i * vrregsize + offset);
+      else
+        supply_register (tdep->ppc_vr0_regnum + i, *vrregsetp + i * vrregsize);
+    }
+}
+
+#ifdef HAVE_PTRACE_GETFPXREGS
+
+static void
+fetch_altivec_registers (int tid)
+{
+  int ret;
+  gdb_vrregset_t regs;
+  
+  ret = ptrace (PTRACE_GETVRREGS, tid, 0, &regs);
+  if (ret < 0)
+    {
+      if (errno == EIO)
+	{
+          have_ptrace_getvrregs = 0;
+	  return;
+	}
+      perror_with_name ("Unable to fetch AltiVec registers");
+    }
+  supply_vrregset (&regs);
+}
+#else
+static void
+fetch_altivec_registers (int tid)
+{
+  internal_error (__FILE__, __LINE__,
+		  "fetch_altivec_registers: HAVE_PTRACE_GETFPXREGS not defined
+\nand have_ptrace_getvrregs set");
+  return;
+}
+#endif /* HAVE_PTRACE_GETFPXREGS */
+
 static void 
 fetch_ppc_registers (int tid)
 {
   int i;
-  int last_register = gdbarch_tdep (current_gdbarch)->ppc_mq_regnum;
-  
-  for (i = 0; i <= last_register; i++)
+  struct gdbarch_tdep *tdep = gdbarch_tdep (current_gdbarch);
+
+  for (i = 0; i <= tdep->ppc_mq_regnum; i++)
     fetch_register (tid, i);
+  if (have_ptrace_getvrregs)
+    if (tdep->ppc_vr0_regnum != -1 && tdep->ppc_vrsave_regnum != -1)
+      fetch_altivec_registers (tid);
 }
 
 /* Fetch registers from the child process.  Fetch all registers if
@@ -158,21 +309,65 @@ fetch_ppc_registers (int tid)
 void
 fetch_inferior_registers (int regno)
 {
- /* Overload thread id onto process id */
+  /* Overload thread id onto process id */
   int tid = TIDGET (inferior_ptid);
 
   /* No thread id, just use process id */
   if (tid == 0)
     tid = PIDGET (inferior_ptid);
 
-   if (regno == -1)
+  if (regno == -1)
     fetch_ppc_registers (tid);
   else 
     fetch_register (tid, regno);
 }
 
+#ifdef HAVE_PTRACE_GETFPXREGS
 /* Store one register. */
 static void
+store_altivec_register (int tid, int regno)
+{
+  int ret;
+  int offset = 0;
+  gdb_vrregset_t regs;
+  struct gdbarch_tdep *tdep = gdbarch_tdep (current_gdbarch);
+  int vrregsize = REGISTER_RAW_SIZE (tdep->ppc_vr0_regnum);
+
+  ret = ptrace (PTRACE_GETVRREGS, tid, 0, &regs);
+  if (ret < 0)
+    {
+      if (errno == EIO)
+        {
+          have_ptrace_getvrregs = 0;
+          return;
+        }
+      perror_with_name ("Unable to fetch AltiVec register");
+    }
+
+  /* VSCR is fetched as a 16 bytes quantity, but it is really
+     4 bytes long on the hardware. */
+  if (regno == (tdep->ppc_vrsave_regnum - 1))
+    offset = vrregsize - REGISTER_RAW_SIZE (tdep->ppc_vrsave_regnum);
+
+  regcache_collect (regno,
+                    regs + (regno - tdep->ppc_vr0_regnum) * vrregsize + offset);
+
+  ret = ptrace (PTRACE_SETVRREGS, tid, 0, &regs);
+  if (ret < 0)
+    perror_with_name ("Unable to store AltiVec register");
+}
+#else
+static void
+store_altivec_register (int tid)
+{
+  internal_error (__FILE__, __LINE__,
+		  "store_altivec_register: HAVE_PTRACE_GETFPXREGS not defined,
+\nand have_ptrace_getvrregs set");
+  return;
+}
+#endif
+
+static void
 store_register (int tid, int regno)
 {
   /* This isn't really an address.  But ptrace thinks of it as one.  */
@@ -182,11 +377,15 @@ store_register (int tid, int regno)
   unsigned int offset;         /* Offset of registers within the u area.  */
   char *buf = alloca (MAX_REGISTER_RAW_SIZE);
 
-  if (regaddr == -1)
+  if (altivec_register_p (regno))
     {
+      store_altivec_register (tid, regno);
       return;
     }
 
+  if (regaddr == -1)
+    return;
+
   regcache_collect (regno, buf);
   for (i = 0; i < REGISTER_RAW_SIZE (regno); i += sizeof (PTRACE_XFER_TYPE))
     {
@@ -204,13 +403,71 @@ store_register (int tid, int regno)
 }
 
 static void
+fill_vrregset (gdb_vrregset_t *vrregsetp)
+{
+  int i;
+  struct gdbarch_tdep *tdep = gdbarch_tdep (current_gdbarch);
+  int num_of_vrregs = tdep->ppc_vrsave_regnum - tdep->ppc_vr0_regnum;
+  int vrregsize = REGISTER_RAW_SIZE (tdep->ppc_vr0_regnum);
+  int offset = vrregsize - REGISTER_RAW_SIZE (tdep->ppc_vrsave_regnum);
+
+  for (i = 0; i < num_of_vrregs; i++)
+    {
+      /* The last 2 registers of this set are only 32 bit long, not 128.
+         Only VSCR is fetched as a 16 bytes quantity. */
+      if (i == (tdep->ppc_vrsave_regnum - 1))
+        regcache_collect (tdep->ppc_vr0_regnum + i,
+                          *vrregsetp + i * vrregsize + offset);
+      else
+        regcache_collect (tdep->ppc_vr0_regnum + i, *vrregsetp + i * vrregsize);
+    }
+}
+
+#ifdef HAVE_PTRACE_GETFPXREGS
+static void
+store_altivec_registers (int tid)
+{
+  int ret;
+  gdb_vrregset_t regs;
+
+  ret = ptrace (PTRACE_GETVRREGS, tid, 0, (int) &regs);
+  if (ret < 0)
+    {
+      if (errno == EIO)
+        {
+          have_ptrace_getvrregs = 0;
+          return;
+        }
+      perror_with_name ("Couldn't get AltiVec registers");
+    }
+
+  fill_vrregset (&regs);
+  
+  if (ptrace (PTRACE_SETVRREGS, tid, 0, (int) &regs) < 0)
+    perror_with_name ("Couldn't write AltiVec registers");
+}
+#else
+static void
+store_altivec_registers (int tid)
+{
+  internal_error (__FILE__, __LINE__,
+		  "store_altivec_registers: HAVE_PTRACE_GETFPXREGS not defined
+\nand have_ptrace_getvrregs set");
+  return;
+}
+#endif
+
+static void
 store_ppc_registers (int tid)
 {
   int i;
-  int last_register = gdbarch_tdep (current_gdbarch)->ppc_mq_regnum;
+  struct gdbarch_tdep *tdep = gdbarch_tdep (current_gdbarch);
   
-  for (i = 0; i <= last_register; i++)
+  for (i = 0; i <= tdep->ppc_mq_regnum; i++)
     store_register (tid, i);
+  if (have_ptrace_getvrregs)
+    if (tdep->ppc_vr0_regnum != -1 && tdep->ppc_vrsave_regnum != -1)
+      store_altivec_registers (tid);
 }
 
 void
@@ -281,6 +538,7 @@ void
 supply_fpregset (gdb_fpregset_t * fpregsetp)
 {
   int regi;
+
   for (regi = 0; regi < 32; regi++)
     supply_register (FP0_REGNUM + regi, (char *) (*fpregsetp + regi));
 }

Re: [RFA] ppc-linux-nat.c AltiVec regs ptrace

[Daniel Jacobowitz]

On Wed, Feb 20, 2002 at 03:20:44PM -0500, Elena Zannoni wrote:
> This patch adds support for fetching and storing the AltiVec registers
> on PPC.  I previously submitted a different patch based on a different
> ptrace interface that was rejected by the ppc linux kernel maintainer.
> A new ptrace interface has since been added to the kernel:
> http://ppc.bkbits.net:8080/linuxppc_2_4/cset@1.543.1.14
> 
> This was the old patch, which is now obsolete:
> http://sources.redhat.com/ml/gdb-patches/2001-11/msg00548.html

Thanks for keeping at this after we made you run around in circles on
the ptrace interface.

> The Altivec registers are defined like this:
> #define ELF_NVRREG      33      /* includes vscr */
> /* Altivec registers */
> typedef __vector128 elf_vrreg_t;
> typedef elf_vrreg_t elf_vrregset_t[ELF_NVRREG];
> 
> There are 32 vector registers 16 bytes longs, plus a VSCR register
> which is only 4 bytes long, but is fetched as a 16 bytes quantity. Up
> to here we have the elf_vrregset_t structure.
> Appended to this there is space for the VRSAVE register: 4 bytes.
> Even though this vrsave register is not included in the regset
> typedef, it is handled by the ptrace requests.
> 
> The layout is like this:
> 
>  |.|.|.|.|.....|.|.|.|.||.|.|.|x||.|
>  <------->     <-------><-------><->
>    VR0           VR31     VSCR    VRSAVE
> (where x is the actual value of the vscr reg)
> 
> 
> I have added a typedef to the ppc-linux-nat.c file to deal with
> such a layout. I am not sure whether this is the best place to put
> that. Maybe in gregset.h?

Ew.  But I think it should remain local to ppc-linux-nat.c rather than
being visible anywhere outside the nat file.  Nothing else should need
to know.  Well, eventually the -tdep, since we will need this for
core files, but cross corefiles are not your problem at this point.

> Index: ppc-linux-nat.c
> ===================================================================
> RCS file: /cvs/uberbaum/gdb/ppc-linux-nat.c,v
> retrieving revision 1.15
> diff -u -p -r1.15 ppc-linux-nat.c
> --- ppc-linux-nat.c	2002/02/18 15:08:40	1.15
> +++ ppc-linux-nat.c	2002/02/20 19:27:29
> @@ -17,7 +17,7 @@
>     You should have received a copy of the GNU General Public License
>     along with this program; if not, write to the Free Software
>     Foundation, Inc., 59 Temple Place - Suite 330,
> -   Boston, MA 02111-1307, USA.  */
> +   Boston, MA 02111-1307, USA.*/

Nit.  Two spaces after periods.  You did this in a number of other
places throughout the patch, too.

> +int have_ptrace_getvrregs
> +#ifdef HAVE_PTRACE_GETFPXREGS
> +     = 1;
> +#else
> +     = 0;
> +#endif
> +

Huh?  You defined GETVRREGS unconditionally above.  GETFPXREGS has no
place in this file, does it?  Or do the headers define GETFPXREGS?
You also continue this confusion all the way down the patch.

-- 
Daniel Jacobowitz                           Carnegie Mellon University
MontaVista Software                         Debian GNU/Linux Developer

Re: [RFA] ppc-linux-nat.c AltiVec regs ptrace

[Elena Zannoni]

Daniel Jacobowitz writes:
 > On Wed, Feb 20, 2002 at 03:20:44PM -0500, Elena Zannoni wrote:
 > > This patch adds support for fetching and storing the AltiVec registers
 > > on PPC.  I previously submitted a different patch based on a different
 > > ptrace interface that was rejected by the ppc linux kernel maintainer.
 > > A new ptrace interface has since been added to the kernel:
 > > http://ppc.bkbits.net:8080/linuxppc_2_4/cset@1.543.1.14
 > > 
 > > This was the old patch, which is now obsolete:
 > > http://sources.redhat.com/ml/gdb-patches/2001-11/msg00548.html
 > 
 > Thanks for keeping at this after we made you run around in circles on
 > the ptrace interface.
 > 
 > > The Altivec registers are defined like this:
 > > #define ELF_NVRREG      33      /* includes vscr */
 > > /* Altivec registers */
 > > typedef __vector128 elf_vrreg_t;
 > > typedef elf_vrreg_t elf_vrregset_t[ELF_NVRREG];
 > > 
 > > There are 32 vector registers 16 bytes longs, plus a VSCR register
 > > which is only 4 bytes long, but is fetched as a 16 bytes quantity. Up
 > > to here we have the elf_vrregset_t structure.
 > > Appended to this there is space for the VRSAVE register: 4 bytes.
 > > Even though this vrsave register is not included in the regset
 > > typedef, it is handled by the ptrace requests.
 > > 
 > > The layout is like this:
 > > 
 > >  |.|.|.|.|.....|.|.|.|.||.|.|.|x||.|
 > >  <------->     <-------><-------><->
 > >    VR0           VR31     VSCR    VRSAVE
 > > (where x is the actual value of the vscr reg)
 > > 
 > > 
 > > I have added a typedef to the ppc-linux-nat.c file to deal with
 > > such a layout. I am not sure whether this is the best place to put
 > > that. Maybe in gregset.h?
 > 
 > Ew.  But I think it should remain local to ppc-linux-nat.c rather than
 > being visible anywhere outside the nat file.  Nothing else should need
 > to know.  Well, eventually the -tdep, since we will need this for
 > core files, but cross corefiles are not your problem at this point.

Yes, I don't like it much myself, that's why it's buried in here.

 > 
 > > Index: ppc-linux-nat.c
 > > ===================================================================
 > > RCS file: /cvs/uberbaum/gdb/ppc-linux-nat.c,v
 > > retrieving revision 1.15
 > > diff -u -p -r1.15 ppc-linux-nat.c
 > > --- ppc-linux-nat.c	2002/02/18 15:08:40	1.15
 > > +++ ppc-linux-nat.c	2002/02/20 19:27:29
 > > @@ -17,7 +17,7 @@
 > >     You should have received a copy of the GNU General Public License
 > >     along with this program; if not, write to the Free Software
 > >     Foundation, Inc., 59 Temple Place - Suite 330,
 > > -   Boston, MA 02111-1307, USA.  */
 > > +   Boston, MA 02111-1307, USA.*/
 > 
 > Nit.  Two spaces after periods.  You did this in a number of other
 > places throughout the patch, too.
 > 

Right will fix. [I had an ongoing bet :-)]

 > > +int have_ptrace_getvrregs
 > > +#ifdef HAVE_PTRACE_GETFPXREGS
 > > +     = 1;
 > > +#else
 > > +     = 0;
 > > +#endif
 > > +
 > 
 > Huh?  You defined GETVRREGS unconditionally above.  GETFPXREGS has no
 > place in this file, does it?  Or do the headers define GETFPXREGS?
 > You also continue this confusion all the way down the patch.
 > 

The glibc headers define GETFPXREGS, and that's what we test for in
the configury.  But we are not dealing with floating point registers
here, so I used the 'correct' name where I could.  It would be more
confusing to talk about FPX regs while instead there are none.
I explained this in the comments.

I guess I can do the following if it helps.
#ifdef HAVE_PTRACE_GETFPXREGS
#define HAVE_PTRACE_GETVRREGS

Whatever I end up using it's partially going to be a lie.  I would
prefer using the VRREGS nomenclature where relevant, though.



Elena

 > -- 
 > Daniel Jacobowitz                           Carnegie Mellon University
 > MontaVista Software                         Debian GNU/Linux Developer

Re: [RFA] ppc-linux-nat.c AltiVec regs ptrace

[Daniel Jacobowitz]

On Wed, Feb 20, 2002 at 04:06:56PM -0500, Elena Zannoni wrote:
> Right will fix. [I had an ongoing bet :-)]

What, whether Andrew would get to you before I did? :)
<duck and run>

>  > > +int have_ptrace_getvrregs
>  > > +#ifdef HAVE_PTRACE_GETFPXREGS
>  > > +     = 1;
>  > > +#else
>  > > +     = 0;
>  > > +#endif
>  > > +
>  > 
>  > Huh?  You defined GETVRREGS unconditionally above.  GETFPXREGS has no
>  > place in this file, does it?  Or do the headers define GETFPXREGS?
>  > You also continue this confusion all the way down the patch.
>  > 
> 
> The glibc headers define GETFPXREGS, and that's what we test for in
> the configury.  But we are not dealing with floating point registers
> here, so I used the 'correct' name where I could.  It would be more
> confusing to talk about FPX regs while instead there are none.
> I explained this in the comments.
> 
> I guess I can do the following if it helps.
> #ifdef HAVE_PTRACE_GETFPXREGS
> #define HAVE_PTRACE_GETVRREGS
> 
> Whatever I end up using it's partially going to be a lie.  I would
> prefer using the VRREGS nomenclature where relevant, though.

I'm confused.

On i386, glibc defines PTRACE_GETFPXREGS.  On PowerPC, in current FSF
glibc, sys/ptrace.h does not define anything along these lines at all. 
The kernel <asm/ptrace.h> define GETVRREGS (not that we should be
including that header, of course).  [<sys/ptrace.h> is an
architecture-specific header, which may not have been apparent.]

If there are outstanding patches to glibc, which defines
PTRACE_GETFPXREGS on PowerPC, then they are still mutable.  They should
be updated to a reasonable value.

-- 
Daniel Jacobowitz                           Carnegie Mellon University
MontaVista Software                         Debian GNU/Linux Developer

Re: [RFA] ppc-linux-nat.c AltiVec regs ptrace

[Andrew Cagney]

> On Wed, Feb 20, 2002 at 04:06:56PM -0500, Elena Zannoni wrote:
> 
>> Right will fix. [I had an ongoing bet :-)]
> 
> 
> What, whether Andrew would get to you before I did? :)
> <duck and run>

Not I, I'm too busy tut-tut-ing people spelling ``Linux kernel'' 
incorrectly :-)

Andrew

Re: [RFA] ppc-linux-nat.c AltiVec regs ptrace

[Elena Zannoni]

Daniel Jacobowitz writes:
 > On Wed, Feb 20, 2002 at 04:06:56PM -0500, Elena Zannoni wrote:
 > > Right will fix. [I had an ongoing bet :-)]
 > 
 > What, whether Andrew would get to you before I did? :)
 > <duck and run>

No it was actually Kevin, I was thinking of.
<duck and run myself>

 > 
 > >  > > +int have_ptrace_getvrregs
 > >  > > +#ifdef HAVE_PTRACE_GETFPXREGS
 > >  > > +     = 1;
 > >  > > +#else
 > >  > > +     = 0;
 > >  > > +#endif
 > >  > > +
 > >  > 
 > >  > Huh?  You defined GETVRREGS unconditionally above.  GETFPXREGS has no
 > >  > place in this file, does it?  Or do the headers define GETFPXREGS?
 > >  > You also continue this confusion all the way down the patch.
 > >  > 
 > > 
 > > The glibc headers define GETFPXREGS, and that's what we test for in
 > > the configury.  But we are not dealing with floating point registers
 > > here, so I used the 'correct' name where I could.  It would be more
 > > confusing to talk about FPX regs while instead there are none.
 > > I explained this in the comments.
 > > 
 > > I guess I can do the following if it helps.
 > > #ifdef HAVE_PTRACE_GETFPXREGS
 > > #define HAVE_PTRACE_GETVRREGS
 > > 
 > > Whatever I end up using it's partially going to be a lie.  I would
 > > prefer using the VRREGS nomenclature where relevant, though.
 > 
 > I'm confused.

Yeah, you are not the only one.

 > 
 > On i386, glibc defines PTRACE_GETFPXREGS.  On PowerPC, in current FSF
 > glibc, sys/ptrace.h does not define anything along these lines at all. 

OK, I have downloaded glibc 2.2.5, and sysdeps/unix/sysv/linux/sys/ptrace.h
defines PTRACE_GETFPXREGS.

Then on my system, I have /usr/include/sys/ptrace.h which also defines it.
But I think I have an older version of glibc installed.

What I am not understanding is where the installed file comes from, is
it the same as sysdeps/unix/sysv/linux/sys/ptrace.h?

 > The kernel <asm/ptrace.h> define GETVRREGS (not that we should be
 > including that header, of course).  [<sys/ptrace.h> is an
 > architecture-specific header, which may not have been apparent.]
 > 

Right. I didn't rely on it.

 > If there are outstanding patches to glibc, which defines
 > PTRACE_GETFPXREGS on PowerPC, then they are still mutable.  They should
 > be updated to a reasonable value.
 > 

I think that rather than oustanding patches we may have older versions.

I see that in glibc2.2.5 the file
sysdeps/unix/sysv/linux/powerpc/sys/ptrace.h
doesn't use the values 18 and 19.

If I determine that the version of glibc I have used is obsolete, then
I can clean that up. Let me have a look.


Elena


 > -- 
 > Daniel Jacobowitz                           Carnegie Mellon University
 > MontaVista Software                         Debian GNU/Linux Developer

Re: [RFA] ppc-linux-nat.c AltiVec regs ptrace

[Daniel Jacobowitz]

On Wed, Feb 20, 2002 at 06:06:55PM -0500, Elena Zannoni wrote:
> Daniel Jacobowitz writes:
>  > I'm confused.
> 
> Yeah, you are not the only one.
> 
>  > 
>  > On i386, glibc defines PTRACE_GETFPXREGS.  On PowerPC, in current FSF
>  > glibc, sys/ptrace.h does not define anything along these lines at all. 
> 
> OK, I have downloaded glibc 2.2.5, and sysdeps/unix/sysv/linux/sys/ptrace.h
> defines PTRACE_GETFPXREGS.
> 
> Then on my system, I have /usr/include/sys/ptrace.h which also defines it.
> But I think I have an older version of glibc installed.
> 
> What I am not understanding is where the installed file comes from, is
> it the same as sysdeps/unix/sysv/linux/sys/ptrace.h?

The way the glibc build process works is a mess.  Every target has a
list of sysdep directories.  The first matching file is installed.  In
this case, it is sysdeps/unix/sysv/linux/powerpc/sys/ptrace.h.  When
looking for a file in the glibc source, I recommend always getting a
list of all files by that name first.

>  > If there are outstanding patches to glibc, which defines
>  > PTRACE_GETFPXREGS on PowerPC, then they are still mutable.  They should
>  > be updated to a reasonable value.
>  > 
> 
> I think that rather than oustanding patches we may have older versions.
> 
> I see that in glibc2.2.5 the file
> sysdeps/unix/sysv/linux/powerpc/sys/ptrace.h
> doesn't use the values 18 and 19.
> 
> If I determine that the version of glibc I have used is obsolete, then
> I can clean that up. Let me have a look.

Hopefully I've clarified this a little.  And you seem to have found the
file in question.

-- 
Daniel Jacobowitz                           Carnegie Mellon University
MontaVista Software                         Debian GNU/Linux Developer

Re: [RFA] ppc-linux-nat.c AltiVec regs ptrace

[Elena Zannoni]

Daniel Jacobowitz writes:
 > On Wed, Feb 20, 2002 at 06:06:55PM -0500, Elena Zannoni wrote:
 > > Daniel Jacobowitz writes:
 > >  > I'm confused.
 > > 
 > > Yeah, you are not the only one.
 > > 
 > >  > 
 > >  > On i386, glibc defines PTRACE_GETFPXREGS.  On PowerPC, in current FSF
 > >  > glibc, sys/ptrace.h does not define anything along these lines at all. 
 > > 
 > > OK, I have downloaded glibc 2.2.5, and sysdeps/unix/sysv/linux/sys/ptrace.h
 > > defines PTRACE_GETFPXREGS.
 > > 
 > > Then on my system, I have /usr/include/sys/ptrace.h which also defines it.
 > > But I think I have an older version of glibc installed.
 > > 
 > > What I am not understanding is where the installed file comes from, is
 > > it the same as sysdeps/unix/sysv/linux/sys/ptrace.h?
 > 
 > The way the glibc build process works is a mess.  Every target has a
 > list of sysdep directories.  The first matching file is installed.  In
 > this case, it is sysdeps/unix/sysv/linux/powerpc/sys/ptrace.h.  When
 > looking for a file in the glibc source, I recommend always getting a
 > list of all files by that name first.
 > 

Ohhh, thanks!

Ok, now I understand where the file comes from.

I have glibc-2.2.1 installed:
$ find . -name ptrace.h
./sysdeps/generic/sys/ptrace.h
./sysdeps/unix/sysv/linux/alpha/alpha/ptrace.h
./sysdeps/unix/sysv/linux/s390/sys/ptrace.h
./sysdeps/unix/sysv/linux/sparc/sys/ptrace.h
./sysdeps/unix/sysv/linux/sys/ptrace.h


While in the 2.2.5 sources:
$ !find
find . -name ptrace.h
./sysdeps/generic/sys/ptrace.h
./sysdeps/unix/sysv/linux/alpha/alpha/ptrace.h
./sysdeps/unix/sysv/linux/ia64/sys/ptrace.h
./sysdeps/unix/sysv/linux/powerpc/sys/ptrace.h
./sysdeps/unix/sysv/linux/s390/sys/ptrace.h
./sysdeps/unix/sysv/linux/sparc/sys/ptrace.h
./sysdeps/unix/sysv/linux/sys/ptrace.h


In case of 2.2.5 the powerpc version of the file gets installed. While
for 2.2.1 the one with the definitions for PTRACE_GETFPXREGS is installed.

Ok then, should we support the older version or not?
If not we have two options:

1. if glibc gets a patch with the new PTRACE_GETVRREGS requests, then
   we can add another different configuration check.

2. We can just rely on the run time check. Which means I have to redo
   the patch again [where is that bucket].

Actually doing just 2 would work also with the older version, I guess.
Unless I am missing some other subtlety. Ok I'll change it.

Elena

Re: [RFA] ppc-linux-nat.c AltiVec regs ptrace

[Daniel Jacobowitz]

On Wed, Feb 20, 2002 at 07:28:27PM -0500, Elena Zannoni wrote:
> In case of 2.2.5 the powerpc version of the file gets installed. While
> for 2.2.1 the one with the definitions for PTRACE_GETFPXREGS is installed.
> 
> Ok then, should we support the older version or not?
> If not we have two options:
> 
> 1. if glibc gets a patch with the new PTRACE_GETVRREGS requests, then
>    we can add another different configuration check.
> 
> 2. We can just rely on the run time check. Which means I have to redo
>    the patch again [where is that bucket].
> 
> Actually doing just 2 would work also with the older version, I guess.
> Unless I am missing some other subtlety. Ok I'll change it.

Sounds good to me.  Might want to submit a patch to add GETVRREGS to
libc, also, I suppose...

-- 
Daniel Jacobowitz                           Carnegie Mellon University
MontaVista Software                         Debian GNU/Linux Developer

Re: [RFA] ppc-linux-nat.c AltiVec regs ptrace

[Elena Zannoni]

Daniel Jacobowitz writes:
 > On Wed, Feb 20, 2002 at 07:28:27PM -0500, Elena Zannoni wrote:
 > > In case of 2.2.5 the powerpc version of the file gets installed. While
 > > for 2.2.1 the one with the definitions for PTRACE_GETFPXREGS is installed.
 > > 
 > > Ok then, should we support the older version or not?
 > > If not we have two options:
 > > 
 > > 1. if glibc gets a patch with the new PTRACE_GETVRREGS requests, then
 > >    we can add another different configuration check.
 > > 
 > > 2. We can just rely on the run time check. Which means I have to redo
 > >    the patch again [where is that bucket].
 > > 
 > > Actually doing just 2 would work also with the older version, I guess.
 > > Unless I am missing some other subtlety. Ok I'll change it.
 > 
 > Sounds good to me.  Might want to submit a patch to add GETVRREGS to
 > libc, also, I suppose...

Yes, it's probably better.

Here is a new patch.

2002-02-20  Elena Zannoni  <ezannoni@redhat.com>

	* ppc-linux-nat.c (PTRACE_GETVRREGS, PTRACE_SETVRREGS): Define.
	(have_ptrace_getvrregs): Define for run time checks.
	(fetch_register, store_register): Fetch/store altivec register
	when needed.
	(fetch_altivec_register, store_altivec-register): New functions.
	(supply_vrregset, fill_vrregset): New functions.
	(fetch_altivec_registers, store_altivec_registers): New functions.
	(fetch_ppc_registers, store_ppc_registers): Fetch/store altivec
	registers as well.

Index: ppc-linux-nat.c
===================================================================
RCS file: /cvs/uberbaum/gdb/ppc-linux-nat.c,v
retrieving revision 1.15
diff -u -p -r1.15 ppc-linux-nat.c
--- ppc-linux-nat.c	2002/02/18 15:08:40	1.15
+++ ppc-linux-nat.c	2002/02/21 02:06:49
@@ -51,6 +51,31 @@
 #define PTRACE_XFER_TYPE int
 #endif
 
+/* Glibc's headers don't define PTRACE_GETVRREGS so we cannot use a
+   configure time check.  Some older glibc's (for instance 2.2.1)
+   don't have a specific powerpc version of ptrace.h, and fall back on
+   a generic one.  In such cases, sys/ptrace.h defines
+   PTRACE_GETFPXREGS and PTRACE_SETFPXREGS to the same numbers that
+   ppc kernel's asm/ptrace.h defines PTRACE_GETVRREGS and
+   PTRACE_SETVRREGS to be.  This also makes a configury check pretty
+   much useless.  */
+
+/* These definitions should really come from the glibc header files,
+   but Glibc doesn't know about the vrregs yet.  */
+#define PTRACE_GETVRREGS 18
+#define PTRACE_SETVRREGS 19
+
+/* This oddity is because the linux kernel defines elf_vrregset_t as
+   an array of 33 16 bytes long elements.  I.e. it leaves out vrsave.
+   However the PTRACE_GETVRREGS and PTRACE_SETVRREGS requests return
+   the vrsave as an extra 4 bytes at the end.  I opted for creating a
+   flat array of chars, so that it is easier to manipulate for gdb.  */
+#define SIZEOF_VRREGS 33*16+4
+typedef char gdb_vrregset_t[SIZEOF_VRREGS];
+
+/* For runtime check of ptrace support for VRREGS.  */
+int have_ptrace_getvrregs = 1;
+
 int
 kernel_u_size (void)
 {
@@ -109,6 +134,40 @@ ppc_ptrace_cannot_fetch_store_register (
   return (ppc_register_u_addr (regno) == -1);
 }
 
+/* The linux kernel ptrace interface for AltiVec registers uses the
+   registers set mechanism, as opposed to the interface for all the
+   other registers, that stores/fetches each register individually.  */
+static void
+fetch_altivec_register (int tid, int regno)
+{
+  int ret;
+  int offset = 0;
+  gdb_vrregset_t regs;
+  struct gdbarch_tdep *tdep = gdbarch_tdep (current_gdbarch);
+  int vrregsize = REGISTER_RAW_SIZE (tdep->ppc_vr0_regnum);
+
+  ret = ptrace (PTRACE_GETVRREGS, tid, 0, &regs);
+  if (ret < 0)
+    {
+      if (errno == EIO)
+        {
+          have_ptrace_getvrregs = 0;
+          return;
+        }
+      perror_with_name ("Unable to fetch AltiVec register");
+    }
+ 
+  /* VSCR is fetched as a 16 bytes quantity, but it is really 4 bytes
+     long on the hardware.  We deal only with the lower 4 bytes of the
+     vector.  VRSAVE is at the end of the array in a 4 bytes slot, so
+     there is no need to define an offset for it.  */
+  if (regno == (tdep->ppc_vrsave_regnum - 1))
+    offset = vrregsize - REGISTER_RAW_SIZE (tdep->ppc_vrsave_regnum);
+  
+  supply_register (regno,
+                   regs + (regno - tdep->ppc_vr0_regnum) * vrregsize + offset);
+}
+
 static void
 fetch_register (int tid, int regno)
 {
@@ -119,6 +178,22 @@ fetch_register (int tid, int regno)
   char *buf = alloca (MAX_REGISTER_RAW_SIZE);
   CORE_ADDR regaddr = ppc_register_u_addr (regno);
 
+  if (altivec_register_p (regno))
+    {
+      /* If this is the first time through, or if it is not the first
+         time throuhg, and we have comfirmed that there is kernel
+         support for such a ptrace request, then go and fetch the
+         register.  */
+      if (have_ptrace_getvrregs)
+       {
+         fetch_altivec_register (tid, regno);
+         return;
+       }
+     /* If we have discovered that there is no ptrace support for
+        AltiVec registers, fall through and return zeroes, because
+        regaddr will be -1 in this case.  */
+    }
+
   if (regaddr == -1)
     {
       memset (buf, '\0', REGISTER_RAW_SIZE (regno));   /* Supply zeroes */
@@ -142,14 +217,59 @@ fetch_register (int tid, int regno)
   supply_register (regno, buf);
 }
 
+static void
+supply_vrregset (gdb_vrregset_t *vrregsetp)
+{
+  int i;
+  struct gdbarch_tdep *tdep = gdbarch_tdep (current_gdbarch);
+  int num_of_vrregs = tdep->ppc_vrsave_regnum - tdep->ppc_vr0_regnum;
+  int vrregsize = REGISTER_RAW_SIZE (tdep->ppc_vr0_regnum);
+  int offset = vrregsize - REGISTER_RAW_SIZE (tdep->ppc_vrsave_regnum);
+
+  for (i = 0; i < num_of_vrregs - 1; i++)
+    {
+      /* The last 2 registers of this set are only 32 bit long, not
+         128.  However an offset is necessary only for VSCR because it
+         occupies a whole vector, while VRSAVE occupies a full 4 bytes
+         slot.  */
+      if (i == (tdep->ppc_vrsave_regnum - 1))
+        supply_register (tdep->ppc_vr0_regnum + i,
+                         *vrregsetp + i * vrregsize + offset);
+      else
+        supply_register (tdep->ppc_vr0_regnum + i, *vrregsetp + i * vrregsize);
+    }
+}
+
+static void
+fetch_altivec_registers (int tid)
+{
+  int ret;
+  gdb_vrregset_t regs;
+  
+  ret = ptrace (PTRACE_GETVRREGS, tid, 0, &regs);
+  if (ret < 0)
+    {
+      if (errno == EIO)
+	{
+          have_ptrace_getvrregs = 0;
+	  return;
+	}
+      perror_with_name ("Unable to fetch AltiVec registers");
+    }
+  supply_vrregset (&regs);
+}
+
 static void 
 fetch_ppc_registers (int tid)
 {
   int i;
-  int last_register = gdbarch_tdep (current_gdbarch)->ppc_mq_regnum;
-  
-  for (i = 0; i <= last_register; i++)
+  struct gdbarch_tdep *tdep = gdbarch_tdep (current_gdbarch);
+
+  for (i = 0; i <= tdep->ppc_mq_regnum; i++)
     fetch_register (tid, i);
+  if (have_ptrace_getvrregs)
+    if (tdep->ppc_vr0_regnum != -1 && tdep->ppc_vrsave_regnum != -1)
+      fetch_altivec_registers (tid);
 }
 
 /* Fetch registers from the child process.  Fetch all registers if
@@ -158,14 +278,14 @@ fetch_ppc_registers (int tid)
 void
 fetch_inferior_registers (int regno)
 {
- /* Overload thread id onto process id */
+  /* Overload thread id onto process id */
   int tid = TIDGET (inferior_ptid);
 
   /* No thread id, just use process id */
   if (tid == 0)
     tid = PIDGET (inferior_ptid);
 
-   if (regno == -1)
+  if (regno == -1)
     fetch_ppc_registers (tid);
   else 
     fetch_register (tid, regno);
@@ -173,6 +293,39 @@ fetch_inferior_registers (int regno)
 
 /* Store one register. */
 static void
+store_altivec_register (int tid, int regno)
+{
+  int ret;
+  int offset = 0;
+  gdb_vrregset_t regs;
+  struct gdbarch_tdep *tdep = gdbarch_tdep (current_gdbarch);
+  int vrregsize = REGISTER_RAW_SIZE (tdep->ppc_vr0_regnum);
+
+  ret = ptrace (PTRACE_GETVRREGS, tid, 0, &regs);
+  if (ret < 0)
+    {
+      if (errno == EIO)
+        {
+          have_ptrace_getvrregs = 0;
+          return;
+        }
+      perror_with_name ("Unable to fetch AltiVec register");
+    }
+
+  /* VSCR is fetched as a 16 bytes quantity, but it is really 4 bytes
+     long on the hardware.  */
+  if (regno == (tdep->ppc_vrsave_regnum - 1))
+    offset = vrregsize - REGISTER_RAW_SIZE (tdep->ppc_vrsave_regnum);
+
+  regcache_collect (regno,
+                    regs + (regno - tdep->ppc_vr0_regnum) * vrregsize + offset);
+
+  ret = ptrace (PTRACE_SETVRREGS, tid, 0, &regs);
+  if (ret < 0)
+    perror_with_name ("Unable to store AltiVec register");
+}
+
+static void
 store_register (int tid, int regno)
 {
   /* This isn't really an address.  But ptrace thinks of it as one.  */
@@ -182,11 +335,15 @@ store_register (int tid, int regno)
   unsigned int offset;         /* Offset of registers within the u area.  */
   char *buf = alloca (MAX_REGISTER_RAW_SIZE);
 
-  if (regaddr == -1)
+  if (altivec_register_p (regno))
     {
+      store_altivec_register (tid, regno);
       return;
     }
 
+  if (regaddr == -1)
+    return;
+
   regcache_collect (regno, buf);
   for (i = 0; i < REGISTER_RAW_SIZE (regno); i += sizeof (PTRACE_XFER_TYPE))
     {
@@ -204,13 +361,60 @@ store_register (int tid, int regno)
 }
 
 static void
+fill_vrregset (gdb_vrregset_t *vrregsetp)
+{
+  int i;
+  struct gdbarch_tdep *tdep = gdbarch_tdep (current_gdbarch);
+  int num_of_vrregs = tdep->ppc_vrsave_regnum - tdep->ppc_vr0_regnum;
+  int vrregsize = REGISTER_RAW_SIZE (tdep->ppc_vr0_regnum);
+  int offset = vrregsize - REGISTER_RAW_SIZE (tdep->ppc_vrsave_regnum);
+
+  for (i = 0; i < num_of_vrregs; i++)
+    {
+      /* The last 2 registers of this set are only 32 bit long, not
+         128.  Only VSCR is fetched as a 16 bytes quantity.  */
+      if (i == (tdep->ppc_vrsave_regnum - 1))
+        regcache_collect (tdep->ppc_vr0_regnum + i,
+                          *vrregsetp + i * vrregsize + offset);
+      else
+        regcache_collect (tdep->ppc_vr0_regnum + i, *vrregsetp + i * vrregsize);
+    }
+}
+
+static void
+store_altivec_registers (int tid)
+{
+  int ret;
+  gdb_vrregset_t regs;
+
+  ret = ptrace (PTRACE_GETVRREGS, tid, 0, (int) &regs);
+  if (ret < 0)
+    {
+      if (errno == EIO)
+        {
+          have_ptrace_getvrregs = 0;
+          return;
+        }
+      perror_with_name ("Couldn't get AltiVec registers");
+    }
+
+  fill_vrregset (&regs);
+  
+  if (ptrace (PTRACE_SETVRREGS, tid, 0, (int) &regs) < 0)
+    perror_with_name ("Couldn't write AltiVec registers");
+}
+
+static void
 store_ppc_registers (int tid)
 {
   int i;
-  int last_register = gdbarch_tdep (current_gdbarch)->ppc_mq_regnum;
+  struct gdbarch_tdep *tdep = gdbarch_tdep (current_gdbarch);
   
-  for (i = 0; i <= last_register; i++)
+  for (i = 0; i <= tdep->ppc_mq_regnum; i++)
     store_register (tid, i);
+  if (have_ptrace_getvrregs)
+    if (tdep->ppc_vr0_regnum != -1 && tdep->ppc_vrsave_regnum != -1)
+      store_altivec_registers (tid);
 }
 
 void
@@ -281,15 +485,15 @@ void
 supply_fpregset (gdb_fpregset_t * fpregsetp)
 {
   int regi;
+
   for (regi = 0; regi < 32; regi++)
     supply_register (FP0_REGNUM + regi, (char *) (*fpregsetp + regi));
 }
-
-/*  Given a pointer to a floating point register set in /proc format
-   (fpregset_t *), update the register specified by REGNO from gdb's idea
-   of the current floating point register set.  If REGNO is -1, update
-   them all. */
 
+/* Given a pointer to a floating point register set in /proc format
+   (fpregset_t *), update the register specified by REGNO from gdb's
+   idea of the current floating point register set.  If REGNO is -1,
+   update them all.  */
 void
 fill_fpregset (gdb_fpregset_t *fpregsetp, int regno)
 {


	

Re: [RFA] ppc-linux-nat.c AltiVec regs ptrace

[Daniel Jacobowitz]

On Wed, Feb 20, 2002 at 09:09:08PM -0500, Elena Zannoni wrote:
> Daniel Jacobowitz writes:
>  > On Wed, Feb 20, 2002 at 07:28:27PM -0500, Elena Zannoni wrote:
>  > > In case of 2.2.5 the powerpc version of the file gets installed. While
>  > > for 2.2.1 the one with the definitions for PTRACE_GETFPXREGS is installed.
>  > > 
>  > > Ok then, should we support the older version or not?
>  > > If not we have two options:
>  > > 
>  > > 1. if glibc gets a patch with the new PTRACE_GETVRREGS requests, then
>  > >    we can add another different configuration check.
>  > > 
>  > > 2. We can just rely on the run time check. Which means I have to redo
>  > >    the patch again [where is that bucket].
>  > > 
>  > > Actually doing just 2 would work also with the older version, I guess.
>  > > Unless I am missing some other subtlety. Ok I'll change it.
>  > 
>  > Sounds good to me.  Might want to submit a patch to add GETVRREGS to
>  > libc, also, I suppose...
> 
> Yes, it's probably better.
> 
> Here is a new patch.
> 
> 2002-02-20  Elena Zannoni  <ezannoni@redhat.com>
> 
> 	* ppc-linux-nat.c (PTRACE_GETVRREGS, PTRACE_SETVRREGS): Define.

I like this much better, thank you!

My only concern is that you'll have a problem when glibc does define
them; might want to conditionally define these.

-- 
Daniel Jacobowitz                           Carnegie Mellon University
MontaVista Software                         Debian GNU/Linux Developer

Re: [RFA] ppc-linux-nat.c AltiVec regs ptrace

[Elena Zannoni]

Daniel Jacobowitz writes:
 > On Wed, Feb 20, 2002 at 09:09:08PM -0500, Elena Zannoni wrote:
 > > Daniel Jacobowitz writes:
 > >  > On Wed, Feb 20, 2002 at 07:28:27PM -0500, Elena Zannoni wrote:
 > >  > > In case of 2.2.5 the powerpc version of the file gets installed. While
 > >  > > for 2.2.1 the one with the definitions for PTRACE_GETFPXREGS is installed.
 > >  > > 
 > >  > > Ok then, should we support the older version or not?
 > >  > > If not we have two options:
 > >  > > 
 > >  > > 1. if glibc gets a patch with the new PTRACE_GETVRREGS requests, then
 > >  > >    we can add another different configuration check.
 > >  > > 
 > >  > > 2. We can just rely on the run time check. Which means I have to redo
 > >  > >    the patch again [where is that bucket].
 > >  > > 
 > >  > > Actually doing just 2 would work also with the older version, I guess.
 > >  > > Unless I am missing some other subtlety. Ok I'll change it.
 > >  > 
 > >  > Sounds good to me.  Might want to submit a patch to add GETVRREGS to
 > >  > libc, also, I suppose...
 > > 
 > > Yes, it's probably better.
 > > 
 > > Here is a new patch.
 > > 
 > > 2002-02-20  Elena Zannoni  <ezannoni@redhat.com>
 > > 
 > > 	* ppc-linux-nat.c (PTRACE_GETVRREGS, PTRACE_SETVRREGS): Define.
 > 
 > I like this much better, thank you!
 > 
 > My only concern is that you'll have a problem when glibc does define
 > them; might want to conditionally define these.

Oh, yes, true.
Will do when I check it in.

Thanks for your help.

Elena

 > 
 > -- 
 > Daniel Jacobowitz                           Carnegie Mellon University
 > MontaVista Software                         Debian GNU/Linux Developer

Re: [RFA] ppc-linux-nat.c AltiVec regs ptrace

[Kevin Buettner]

Elena,

It looks pretty good.  I noticed a typo in a comment.  After your
earlier remarks, I'm beginning to think that you put these in
deliberately to see if I'd notice.  ;-)

There are a few things that I'm confused about though...

On Feb 20,  9:09pm, Elena Zannoni wrote:

> +         time throuhg, and we have comfirmed that there is kernel
                 ^^^^^^^
Here's the typo.

Here's where I'm confused...

> +static void
> +supply_vrregset (gdb_vrregset_t *vrregsetp)
> +{
> +  int i;
> +  struct gdbarch_tdep *tdep = gdbarch_tdep (current_gdbarch);
> +  int num_of_vrregs = tdep->ppc_vrsave_regnum - tdep->ppc_vr0_regnum;

Are you off by one here?  I.e, shouldn't the above statement be

     int num_of_vrregs = tdep->ppc_vrsave_regnum - tdep->ppc_vr0_regnum + 1;

?

> +  int vrregsize = REGISTER_RAW_SIZE (tdep->ppc_vr0_regnum);
> +  int offset = vrregsize - REGISTER_RAW_SIZE (tdep->ppc_vrsave_regnum);
> +
> +  for (i = 0; i < num_of_vrregs - 1; i++)

Why ``num_of_vrregs - 1'' ?  It seems to me that the combination of the
fencepost error (above) combined with this one means that the last two
Altivec registers won't get processed.

> +    {
> +      /* The last 2 registers of this set are only 32 bit long, not
> +         128.  However an offset is necessary only for VSCR because it
> +         occupies a whole vector, while VRSAVE occupies a full 4 bytes
> +         slot.  */
> +      if (i == (tdep->ppc_vrsave_regnum - 1))
> +        supply_register (tdep->ppc_vr0_regnum + i,
> +                         *vrregsetp + i * vrregsize + offset);
> +      else
> +        supply_register (tdep->ppc_vr0_regnum + i, *vrregsetp + i * vrregsize);
> +    }
> +}

[...]

>  static void
> +fill_vrregset (gdb_vrregset_t *vrregsetp)
> +{
> +  int i;
> +  struct gdbarch_tdep *tdep = gdbarch_tdep (current_gdbarch);
> +  int num_of_vrregs = tdep->ppc_vrsave_regnum - tdep->ppc_vr0_regnum;

I think this is another fencepost error.

> +  int vrregsize = REGISTER_RAW_SIZE (tdep->ppc_vr0_regnum);
> +  int offset = vrregsize - REGISTER_RAW_SIZE (tdep->ppc_vrsave_regnum);
> +
> +  for (i = 0; i < num_of_vrregs; i++)

This one looks right to me though, so long as you fix the computation of
num_of_vrregs.

Kevin

Re: [RFA] ppc-linux-nat.c AltiVec regs ptrace

[Elena Zannoni]

Kevin Buettner writes:
 > Elena,
 > 
 > It looks pretty good.  I noticed a typo in a comment.  After your
 > earlier remarks, I'm beginning to think that you put these in
 > deliberately to see if I'd notice.  ;-)

No, that was not intentional! :-)

 > 
 > There are a few things that I'm confused about though...
 > 
 > On Feb 20,  9:09pm, Elena Zannoni wrote:
 > 
 > > +         time throuhg, and we have comfirmed that there is kernel
 >                  ^^^^^^^
 > Here's the typo.
 > 
 > Here's where I'm confused...
 > 
 > > +static void
 > > +supply_vrregset (gdb_vrregset_t *vrregsetp)
 > > +{
 > > +  int i;
 > > +  struct gdbarch_tdep *tdep = gdbarch_tdep (current_gdbarch);
 > > +  int num_of_vrregs = tdep->ppc_vrsave_regnum - tdep->ppc_vr0_regnum;
 > 
 > Are you off by one here?  I.e, shouldn't the above statement be
 > 
 >      int num_of_vrregs = tdep->ppc_vrsave_regnum - tdep->ppc_vr0_regnum + 1;
 > 
 > ?
 > 

Yes, you are right. Thanks for catching this. I was using a different
algorithm, and I changed things at the last minute. I shuld have gone
to sleep instead.

 > > +  int vrregsize = REGISTER_RAW_SIZE (tdep->ppc_vr0_regnum);
 > > +  int offset = vrregsize - REGISTER_RAW_SIZE (tdep->ppc_vrsave_regnum);
 > > +
 > > +  for (i = 0; i < num_of_vrregs - 1; i++)
 > 
 > Why ``num_of_vrregs - 1'' ?  It seems to me that the combination of the
 > fencepost error (above) combined with this one means that the last two
 > Altivec registers won't get processed.
 > 

Yes, right again. I was processing those two separately in an earlier
version of this patch. That's why the loop is off. Will fix.

 > > +    {
 > > +      /* The last 2 registers of this set are only 32 bit long, not
 > > +         128.  However an offset is necessary only for VSCR because it
 > > +         occupies a whole vector, while VRSAVE occupies a full 4 bytes
 > > +         slot.  */
 > > +      if (i == (tdep->ppc_vrsave_regnum - 1))
 > > +        supply_register (tdep->ppc_vr0_regnum + i,
 > > +                         *vrregsetp + i * vrregsize + offset);
 > > +      else
 > > +        supply_register (tdep->ppc_vr0_regnum + i, *vrregsetp + i * vrregsize);
 > > +    }
 > > +}
 > 
 > [...]
 > 
 > >  static void
 > > +fill_vrregset (gdb_vrregset_t *vrregsetp)
 > > +{
 > > +  int i;
 > > +  struct gdbarch_tdep *tdep = gdbarch_tdep (current_gdbarch);
 > > +  int num_of_vrregs = tdep->ppc_vrsave_regnum - tdep->ppc_vr0_regnum;
 > 
 > I think this is another fencepost error.
 > 

Yes, as above.

 > > +  int vrregsize = REGISTER_RAW_SIZE (tdep->ppc_vr0_regnum);
 > > +  int offset = vrregsize - REGISTER_RAW_SIZE (tdep->ppc_vrsave_regnum);
 > > +
 > > +  for (i = 0; i < num_of_vrregs; i++)
 > 
 > This one looks right to me though, so long as you fix the computation of
 > num_of_vrregs.
 > 

Thanks. I had a last minute change of hart before posting the
patch. That will teach me.

I'll repost with fixes.

Elena


 > Kevin

Re: [RFA] ppc-linux-nat.c AltiVec regs ptrace

[Kevin Buettner]

On Feb 20,  3:20pm, Elena Zannoni wrote:

> There are 32 vector registers 16 bytes longs, plus a VSCR register
> which is only 4 bytes long, but is fetched as a 16 bytes quantity. Up
> to here we have the elf_vrregset_t structure.
> Appended to this there is space for the VRSAVE register: 4 bytes.
> Even though this vrsave register is not included in the regset
> typedef, it is handled by the ptrace requests.
> 
> The layout is like this:
> 
>  |.|.|.|.|.....|.|.|.|.||.|.|.|x||.|
>  <------->     <-------><-------><->
>    VR0           VR31     VSCR    VRSAVE
> (where x is the actual value of the vscr reg)

Could you add these remarks and this picture as a comment to
ppc-linux-nat.c?  I found it really useful when reviewing parts of the
code.  In particular, I found it useful when looking over the
following function...


> +static void
> +supply_vrregset (gdb_vrregset_t *vrregsetp)
> +{
> +  int i;
> +  struct gdbarch_tdep *tdep = gdbarch_tdep (current_gdbarch);
> +  int num_of_vrregs = tdep->ppc_vrsave_regnum - tdep->ppc_vr0_regnum;
> +  int vrregsize = REGISTER_RAW_SIZE (tdep->ppc_vr0_regnum);
> +  int offset = vrregsize - REGISTER_RAW_SIZE (tdep->ppc_vrsave_regnum);
> +
> +  for (i = 0; i < num_of_vrregs - 1; i++)
> +    {
> +      /* The last 2 registers of this set are only 32 bit long, not 128.
> +         However an offset is necessary only for VSCR because it occupies 
> +         a whole vector, while VRSAVE occupies a full 4 bytes slot. */
> +      if (i == (tdep->ppc_vrsave_regnum - 1))
> +        supply_register (tdep->ppc_vr0_regnum + i,
> +                         *vrregsetp + i * vrregsize + offset);
> +      else
> +        supply_register (tdep->ppc_vr0_regnum + i, *vrregsetp + i * vrregsize);
> +    }
> +}

I have a question regarding the treatment of VSCR.  Will the offset
that you computed be correct when running on a little endian machine? 
This may be unanswerable, because it'll likely depend upon what the
ptrace() implementation does.  Therefore, I'm not necessarily
suggesting that you change your patch, but do give it a moment or two of
thought...

Kevin

Re: [RFA] ppc-linux-nat.c AltiVec regs ptrace

[Elena Zannoni]

Kevin Buettner writes:
 > On Feb 20,  3:20pm, Elena Zannoni wrote:
 > 
 > > There are 32 vector registers 16 bytes longs, plus a VSCR register
 > > which is only 4 bytes long, but is fetched as a 16 bytes quantity. Up
 > > to here we have the elf_vrregset_t structure.
 > > Appended to this there is space for the VRSAVE register: 4 bytes.
 > > Even though this vrsave register is not included in the regset
 > > typedef, it is handled by the ptrace requests.
 > > 
 > > The layout is like this:
 > > 
 > >  |.|.|.|.|.....|.|.|.|.||.|.|.|x||.|
 > >  <------->     <-------><-------><->
 > >    VR0           VR31     VSCR    VRSAVE
 > > (where x is the actual value of the vscr reg)
 > 
 > Could you add these remarks and this picture as a comment to
 > ppc-linux-nat.c?  I found it really useful when reviewing parts of the
 > code.  In particular, I found it useful when looking over the
 > following function...
 > 

Yes, good idea.


 > 
 > > +static void
 > > +supply_vrregset (gdb_vrregset_t *vrregsetp)
 > > +{
 > > +  int i;
 > > +  struct gdbarch_tdep *tdep = gdbarch_tdep (current_gdbarch);
 > > +  int num_of_vrregs = tdep->ppc_vrsave_regnum - tdep->ppc_vr0_regnum;
 > > +  int vrregsize = REGISTER_RAW_SIZE (tdep->ppc_vr0_regnum);
 > > +  int offset = vrregsize - REGISTER_RAW_SIZE (tdep->ppc_vrsave_regnum);
 > > +
 > > +  for (i = 0; i < num_of_vrregs - 1; i++)
 > > +    {
 > > +      /* The last 2 registers of this set are only 32 bit long, not 128.
 > > +         However an offset is necessary only for VSCR because it occupies 
 > > +         a whole vector, while VRSAVE occupies a full 4 bytes slot. */
 > > +      if (i == (tdep->ppc_vrsave_regnum - 1))
 > > +        supply_register (tdep->ppc_vr0_regnum + i,
 > > +                         *vrregsetp + i * vrregsize + offset);
 > > +      else
 > > +        supply_register (tdep->ppc_vr0_regnum + i, *vrregsetp + i * vrregsize);
 > > +    }
 > > +}
 > 
 > I have a question regarding the treatment of VSCR.  Will the offset
 > that you computed be correct when running on a little endian machine? 
 > This may be unanswerable, because it'll likely depend upon what the
 > ptrace() implementation does.  Therefore, I'm not necessarily
 > suggesting that you change your patch, but do give it a moment or two of
 > thought...
 > 

Yes, I thought about this, I could compute a different offset. But I'll ask
the ptrace implementor...
Is there such a piece of hardware?
I guess I'll add a comment in there, pointing out the endiannes problem.

Elena


 > Kevin

Re: [RFA] ppc-linux-nat.c AltiVec regs ptrace

[Daniel Jacobowitz]

On Thu, Feb 21, 2002 at 10:39:09AM -0500, Elena Zannoni wrote:
> Yes, I thought about this, I could compute a different offset. But I'll ask
> the ptrace implementor...
> Is there such a piece of hardware?
> I guess I'll add a comment in there, pointing out the endiannes problem.

Adding a comment probably suffices.  Linux has never, and from what
I've heard recently will never, support little-endian PowerPC for
technical reasons.

-- 
Daniel Jacobowitz                           Carnegie Mellon University
MontaVista Software                         Debian GNU/Linux Developer

Re: [RFA] ppc-linux-nat.c AltiVec regs ptrace

[Elena Zannoni]

Ok, here is the patch again [(N+1)th time!:-)]

I actually went back and tried to figure out how I didn't catch the
iteration problem. To my excuse (yeah, lame), I can say that running
the gdb testsuite and printing out the names of fetch_ppc_registers
and store_ppc_registers (which are the only callers of the vrregs
fill/supply functions), I *never* see fetch_ppc_registers being
called, and I see store_ppc_registers called only when you do an
inferior function call.

We know very well that by no means the gdb testsuite is complete
(expecially wrt registers), but I think it is a fair sample of
commonly used commands. Also, I don't have Altivec specific tests, so
the vrregs fill/supply will bever be called during a testsuite run,
but still, there is only a slim chance that those functions get
used. I should do a real code inspection to see where/if gdb calls
store/fetch registers with -1 as parameter, but I am too busy with
other stuff at the moment. (No, 'info reg' and 'info all' don't do it
either).

Note the GNU/Linux change freebie!

Elena

2002-02-21  Elena Zannoni  <ezannoni@redhat.com>

	* ppc-linux-nat.c (PTRACE_GETVRREGS, PTRACE_SETVRREGS): Define.
	(have_ptrace_getvrregs): Define for run time checks.
	(fetch_register, store_register): Fetch/store altivec register
	when needed.
	(fetch_altivec_register, store_altivec-register): New functions.
	(supply_vrregset, fill_vrregset): New functions.
	(fetch_altivec_registers, store_altivec_registers): New functions.
	(fetch_ppc_registers, store_ppc_registers): Fetch/store altivec
	registers as well.
	Use GNU/Linux where applicable.

Index: ppc-linux-nat.c
===================================================================
RCS file: /cvs/uberbaum/gdb/ppc-linux-nat.c,v
retrieving revision 1.15
diff -u -p -r1.15 ppc-linux-nat.c
--- ppc-linux-nat.c	2002/02/18 15:08:40	1.15
+++ ppc-linux-nat.c	2002/02/21 21:20:59
@@ -1,4 +1,4 @@
-/* PPC linux native support.
+/* PPC GNU/Linux native support.
    Copyright 1988, 1989, 1991, 1992, 1994, 1996, 2000, 2001, 2002
    Free Software Foundation, Inc.
 
@@ -51,6 +51,56 @@
 #define PTRACE_XFER_TYPE int
 #endif
 
+/* Glibc's headers don't define PTRACE_GETVRREGS so we cannot use a
+   configure time check.  Some older glibc's (for instance 2.2.1)
+   don't have a specific powerpc version of ptrace.h, and fall back on
+   a generic one.  In such cases, sys/ptrace.h defines
+   PTRACE_GETFPXREGS and PTRACE_SETFPXREGS to the same numbers that
+   ppc kernel's asm/ptrace.h defines PTRACE_GETVRREGS and
+   PTRACE_SETVRREGS to be.  This also makes a configury check pretty
+   much useless.  */
+
+/* These definitions should really come from the glibc header files,
+   but Glibc doesn't know about the vrregs yet.  */
+#ifndef PTRACE_GETVRREGS
+#define PTRACE_GETVRREGS 18
+#define PTRACE_SETVRREGS 19
+#endif
+
+/* This oddity is because the Linux kernel defines elf_vrregset_t as
+   an array of 33 16 bytes long elements.  I.e. it leaves out vrsave.
+   However the PTRACE_GETVRREGS and PTRACE_SETVRREGS requests return
+   the vrsave as an extra 4 bytes at the end.  I opted for creating a
+   flat array of chars, so that it is easier to manipulate for gdb.
+
+   There are 32 vector registers 16 bytes longs, plus a VSCR register
+   which is only 4 bytes long, but is fetched as a 16 bytes
+   quantity. Up to here we have the elf_vrregset_t structure.
+   Appended to this there is space for the VRSAVE register: 4 bytes.
+   Even though this vrsave register is not included in the regset
+   typedef, it is handled by the ptrace requests.
+
+   Note that GNU/Linux doesn't support little endian PPC hardware,
+   therefore the offset at which the real value of the VSCR register
+   is located will be always 12 bytes.
+
+   The layout is like this (where x is the actual value of the vscr reg): */
+
+/* *INDENT-OFF* */
+/*
+   |.|.|.|.|.....|.|.|.|.||.|.|.|x||.|
+   <------->     <-------><-------><->
+     VR0           VR31     VSCR    VRSAVE
+*/
+/* *INDENT-ON* */
+
+#define SIZEOF_VRREGS 33*16+4
+
+typedef char gdb_vrregset_t[SIZEOF_VRREGS];
+
+/* For runtime check of ptrace support for VRREGS.  */
+int have_ptrace_getvrregs = 1;
+
 int
 kernel_u_size (void)
 {
@@ -109,7 +159,41 @@ ppc_ptrace_cannot_fetch_store_register (
   return (ppc_register_u_addr (regno) == -1);
 }
 
+/* The Linux kernel ptrace interface for AltiVec registers uses the
+   registers set mechanism, as opposed to the interface for all the
+   other registers, that stores/fetches each register individually.  */
 static void
+fetch_altivec_register (int tid, int regno)
+{
+  int ret;
+  int offset = 0;
+  gdb_vrregset_t regs;
+  struct gdbarch_tdep *tdep = gdbarch_tdep (current_gdbarch);
+  int vrregsize = REGISTER_RAW_SIZE (tdep->ppc_vr0_regnum);
+
+  ret = ptrace (PTRACE_GETVRREGS, tid, 0, &regs);
+  if (ret < 0)
+    {
+      if (errno == EIO)
+        {
+          have_ptrace_getvrregs = 0;
+          return;
+        }
+      perror_with_name ("Unable to fetch AltiVec register");
+    }
+ 
+  /* VSCR is fetched as a 16 bytes quantity, but it is really 4 bytes
+     long on the hardware.  We deal only with the lower 4 bytes of the
+     vector.  VRSAVE is at the end of the array in a 4 bytes slot, so
+     there is no need to define an offset for it.  */
+  if (regno == (tdep->ppc_vrsave_regnum - 1))
+    offset = vrregsize - REGISTER_RAW_SIZE (tdep->ppc_vrsave_regnum);
+  
+  supply_register (regno,
+                   regs + (regno - tdep->ppc_vr0_regnum) * vrregsize + offset);
+}
+
+static void
 fetch_register (int tid, int regno)
 {
   /* This isn't really an address.  But ptrace thinks of it as one.  */
@@ -119,6 +203,22 @@ fetch_register (int tid, int regno)
   char *buf = alloca (MAX_REGISTER_RAW_SIZE);
   CORE_ADDR regaddr = ppc_register_u_addr (regno);
 
+  if (altivec_register_p (regno))
+    {
+      /* If this is the first time through, or if it is not the first
+         time through, and we have comfirmed that there is kernel
+         support for such a ptrace request, then go and fetch the
+         register.  */
+      if (have_ptrace_getvrregs)
+       {
+         fetch_altivec_register (tid, regno);
+         return;
+       }
+     /* If we have discovered that there is no ptrace support for
+        AltiVec registers, fall through and return zeroes, because
+        regaddr will be -1 in this case.  */
+    }
+
   if (regaddr == -1)
     {
       memset (buf, '\0', REGISTER_RAW_SIZE (regno));   /* Supply zeroes */
@@ -142,14 +242,59 @@ fetch_register (int tid, int regno)
   supply_register (regno, buf);
 }
 
+static void
+supply_vrregset (gdb_vrregset_t *vrregsetp)
+{
+  int i;
+  struct gdbarch_tdep *tdep = gdbarch_tdep (current_gdbarch);
+  int num_of_vrregs = tdep->ppc_vrsave_regnum - tdep->ppc_vr0_regnum + 1;
+  int vrregsize = REGISTER_RAW_SIZE (tdep->ppc_vr0_regnum);
+  int offset = vrregsize - REGISTER_RAW_SIZE (tdep->ppc_vrsave_regnum);
+
+  for (i = 0; i < num_of_vrregs; i++)
+    {
+      /* The last 2 registers of this set are only 32 bit long, not
+         128.  However an offset is necessary only for VSCR because it
+         occupies a whole vector, while VRSAVE occupies a full 4 bytes
+         slot.  */
+      if (i == (num_of_vrregs - 2))
+        supply_register (tdep->ppc_vr0_regnum + i,
+                         *vrregsetp + i * vrregsize + offset);
+      else
+        supply_register (tdep->ppc_vr0_regnum + i, *vrregsetp + i * vrregsize);
+    }
+}
+
+static void
+fetch_altivec_registers (int tid)
+{
+  int ret;
+  gdb_vrregset_t regs;
+  
+  ret = ptrace (PTRACE_GETVRREGS, tid, 0, &regs);
+  if (ret < 0)
+    {
+      if (errno == EIO)
+	{
+          have_ptrace_getvrregs = 0;
+	  return;
+	}
+      perror_with_name ("Unable to fetch AltiVec registers");
+    }
+  supply_vrregset (&regs);
+}
+
 static void 
 fetch_ppc_registers (int tid)
 {
   int i;
-  int last_register = gdbarch_tdep (current_gdbarch)->ppc_mq_regnum;
-  
-  for (i = 0; i <= last_register; i++)
+  struct gdbarch_tdep *tdep = gdbarch_tdep (current_gdbarch);
+
+  for (i = 0; i <= tdep->ppc_mq_regnum; i++)
     fetch_register (tid, i);
+  if (have_ptrace_getvrregs)
+    if (tdep->ppc_vr0_regnum != -1 && tdep->ppc_vrsave_regnum != -1)
+      fetch_altivec_registers (tid);
 }
 
 /* Fetch registers from the child process.  Fetch all registers if
@@ -158,14 +303,14 @@ fetch_ppc_registers (int tid)
 void
 fetch_inferior_registers (int regno)
 {
- /* Overload thread id onto process id */
+  /* Overload thread id onto process id */
   int tid = TIDGET (inferior_ptid);
 
   /* No thread id, just use process id */
   if (tid == 0)
     tid = PIDGET (inferior_ptid);
 
-   if (regno == -1)
+  if (regno == -1)
     fetch_ppc_registers (tid);
   else 
     fetch_register (tid, regno);
@@ -173,6 +318,39 @@ fetch_inferior_registers (int regno)
 
 /* Store one register. */
 static void
+store_altivec_register (int tid, int regno)
+{
+  int ret;
+  int offset = 0;
+  gdb_vrregset_t regs;
+  struct gdbarch_tdep *tdep = gdbarch_tdep (current_gdbarch);
+  int vrregsize = REGISTER_RAW_SIZE (tdep->ppc_vr0_regnum);
+
+  ret = ptrace (PTRACE_GETVRREGS, tid, 0, &regs);
+  if (ret < 0)
+    {
+      if (errno == EIO)
+        {
+          have_ptrace_getvrregs = 0;
+          return;
+        }
+      perror_with_name ("Unable to fetch AltiVec register");
+    }
+
+  /* VSCR is fetched as a 16 bytes quantity, but it is really 4 bytes
+     long on the hardware.  */
+  if (regno == (tdep->ppc_vrsave_regnum - 1))
+    offset = vrregsize - REGISTER_RAW_SIZE (tdep->ppc_vrsave_regnum);
+
+  regcache_collect (regno,
+                    regs + (regno - tdep->ppc_vr0_regnum) * vrregsize + offset);
+
+  ret = ptrace (PTRACE_SETVRREGS, tid, 0, &regs);
+  if (ret < 0)
+    perror_with_name ("Unable to store AltiVec register");
+}
+
+static void
 store_register (int tid, int regno)
 {
   /* This isn't really an address.  But ptrace thinks of it as one.  */
@@ -182,11 +360,15 @@ store_register (int tid, int regno)
   unsigned int offset;         /* Offset of registers within the u area.  */
   char *buf = alloca (MAX_REGISTER_RAW_SIZE);
 
-  if (regaddr == -1)
+  if (altivec_register_p (regno))
     {
+      store_altivec_register (tid, regno);
       return;
     }
 
+  if (regaddr == -1)
+    return;
+
   regcache_collect (regno, buf);
   for (i = 0; i < REGISTER_RAW_SIZE (regno); i += sizeof (PTRACE_XFER_TYPE))
     {
@@ -204,13 +386,60 @@ store_register (int tid, int regno)
 }
 
 static void
+fill_vrregset (gdb_vrregset_t *vrregsetp)
+{
+  int i;
+  struct gdbarch_tdep *tdep = gdbarch_tdep (current_gdbarch);
+  int num_of_vrregs = tdep->ppc_vrsave_regnum - tdep->ppc_vr0_regnum + 1;
+  int vrregsize = REGISTER_RAW_SIZE (tdep->ppc_vr0_regnum);
+  int offset = vrregsize - REGISTER_RAW_SIZE (tdep->ppc_vrsave_regnum);
+
+  for (i = 0; i < num_of_vrregs; i++)
+    {
+      /* The last 2 registers of this set are only 32 bit long, not
+         128, but only VSCR is fetched as a 16 bytes quantity.  */
+      if (i == (num_of_vrregs - 2))
+        regcache_collect (tdep->ppc_vr0_regnum + i,
+                          *vrregsetp + i * vrregsize + offset);
+      else
+        regcache_collect (tdep->ppc_vr0_regnum + i, *vrregsetp + i * vrregsize);
+    }
+}
+
+static void
+store_altivec_registers (int tid)
+{
+  int ret;
+  gdb_vrregset_t regs;
+
+  ret = ptrace (PTRACE_GETVRREGS, tid, 0, (int) &regs);
+  if (ret < 0)
+    {
+      if (errno == EIO)
+        {
+          have_ptrace_getvrregs = 0;
+          return;
+        }
+      perror_with_name ("Couldn't get AltiVec registers");
+    }
+
+  fill_vrregset (&regs);
+  
+  if (ptrace (PTRACE_SETVRREGS, tid, 0, (int) &regs) < 0)
+    perror_with_name ("Couldn't write AltiVec registers");
+}
+
+static void
 store_ppc_registers (int tid)
 {
   int i;
-  int last_register = gdbarch_tdep (current_gdbarch)->ppc_mq_regnum;
+  struct gdbarch_tdep *tdep = gdbarch_tdep (current_gdbarch);
   
-  for (i = 0; i <= last_register; i++)
+  for (i = 0; i <= tdep->ppc_mq_regnum; i++)
     store_register (tid, i);
+  if (have_ptrace_getvrregs)
+    if (tdep->ppc_vr0_regnum != -1 && tdep->ppc_vrsave_regnum != -1)
+      store_altivec_registers (tid);
 }
 
 void
@@ -281,15 +510,15 @@ void
 supply_fpregset (gdb_fpregset_t * fpregsetp)
 {
   int regi;
+
   for (regi = 0; regi < 32; regi++)
     supply_register (FP0_REGNUM + regi, (char *) (*fpregsetp + regi));
 }
-
-/*  Given a pointer to a floating point register set in /proc format
-   (fpregset_t *), update the register specified by REGNO from gdb's idea
-   of the current floating point register set.  If REGNO is -1, update
-   them all. */
 
+/* Given a pointer to a floating point register set in /proc format
+   (fpregset_t *), update the register specified by REGNO from gdb's
+   idea of the current floating point register set.  If REGNO is -1,
+   update them all.  */
 void
 fill_fpregset (gdb_fpregset_t *fpregsetp, int regno)
 {


	

Re: [RFA] ppc-linux-nat.c AltiVec regs ptrace

[Kevin Buettner]

On Feb 21,  4:25pm, Elena Zannoni wrote:

> Ok, here is the patch again [(N+1)th time!:-)]
[...]
> Note the GNU/Linux change freebie!

Cool!

> 2002-02-21  Elena Zannoni  <ezannoni@redhat.com>
> 
> 	* ppc-linux-nat.c (PTRACE_GETVRREGS, PTRACE_SETVRREGS): Define.
> 	(have_ptrace_getvrregs): Define for run time checks.
> 	(fetch_register, store_register): Fetch/store altivec register
> 	when needed.
> 	(fetch_altivec_register, store_altivec-register): New functions.

Nit: Make that store_altivec_register.  (Change hyphen to underscore.)

> 	(supply_vrregset, fill_vrregset): New functions.
> 	(fetch_altivec_registers, store_altivec_registers): New functions.
> 	(fetch_ppc_registers, store_ppc_registers): Fetch/store altivec
> 	registers as well.
> 	Use GNU/Linux where applicable.

Looks good otherwise.  Check it in!

Thanks,

Kevin