Re: [PATCH] Enable vector instruction debugging for AIX

[Aditya Kamath1 <Aditya.Kamath1@ibm.com>]

[-- Attachment #1.1: Type: text/plain, Size: 14259 bytes --]

Hi Ulrich,

Thank you for the feedback. Please find attached the new patch. [See: 0001-Enable-Vector-instruction-debugging-support-for-AIX.patch ]

Kindly let me know if any changes required.

I have answered point by point for the feedback below. Please find below.

Have a nice day ahead.

Thanks and regards,
Aditya.

--------------------------------
>I just noticed that there seem to be two methods, handling
>user threads and kernel threads, respectively.  You patch
>only adds support for VMX/VSX registers to kernel threads
>-- should this also be done for user threads?  (Same applies
>to store_regs_... of course.)

Yes, this I have added in the new patch..

>>+  /* vector registers.  */
>>+  if (tdep->ppc_vr0_regnum != -1 && regno >= tdep->ppc_vr0_regnum)

>This check isn't quite correct.  I think it should be:
 > if the target has vector registers
 > and (all registers are requested, i.e. regno == -1
  >     or a vector register is requested, i.e. regno is a VR)
>
>Something along these lines:
 >if (tdep->ppc_vr0_regnum != -1 && tdep->vrsave_regnum != -1
  >   && (regno == -1 || (regno >= tdep->ppc_vr0_regnum
  >                       && regno <= tdep->ppc_vrsave_regnum))

Yes, this I have added in the new patch..

>So if access to VMX registers fails because ptrace fails,
>you now zero out the registers.  But if access fails because
>__power_vmx() fails, the registers are left undefined.

>Is this intentional?  What the meaning of this difference?

So, if VMX register fails I will get an output like this below. Otherwise, I set to 0 like it is in all other places. So I am not doing any special handling..


(gdb) info reg vr0

vr0            {uint128 = <unavailable>, v4_float = {<unavailable>, <unavailable>, <unavailable>, <unavailable>}, v4_int32 = {<unavailable>, <unavailable>, <unavailable>, <unavailable>}, v8_int16 = {<unavailable>, <unavailable>, <unavailable>, <unavailable>, <unavailable>, <unavailable>, <unavailable>, <unavailable>}, v16_int8 = {<unavailable> <repeats 16 times>}}


>Is there a reason for not using &(vmx->__vr[regno]) like above?
>That would seem to be more straightforward.
This I have changed in the patch.. No reasons..

>>+      ret = rs6000_ptrace64 (PTT_READ_VSX, thrd_i, (long long) &vsx, 0, 0);
>>+    else
>>+      ret = rs6000_ptrace32 (PTT_READ_VSX, thrd_i, (int *)&vsx, 0, 0);

>Just noticed this here.  Everywhere else in this file, a *pid* is
>passed to rs6000_ptrace64/32.  Only in this new routine, you're
>apparently passing a thread ID.   Why should this be different just
>for these registers?  Shouldn't per-thread registers be handled in
>aix-thread.c anyway?

Well, this is because in the documentation I read, I did not find something like a PT_READ_VSX that I can use. All I have is PTT_READ_VSX and in AIX PTT _* options are usually thread IDs.. I am restricted.. If it existed, I would have loved to use it. Like for writing general purpose registers we have PTT_WRITE_GPRS and PT_WRITE_GPRS and can use it as per convenience of thread or process..

>>+
>>+  fetch_altivec_registers_aix (regcache);
>>+  fetch_vsx_registers_aix (regcache);
>> }


>I think you should call this only if the requested regno
>actually *is* a VMX or VSX register.  Also, if this is the
>case, you don't need to do anything else in this routine.
>This should be separate checks at the beginning of the
>routine, just like you now do in store_register.

I have taken care of it in the new patch..

>>+               || (op & 0xff0f0000) == 0x38000000)     /* li, r4, SIMM */

>Please fix the tabs vs. spaces, and keep the comments at the end aligned.

>>More importantly, the code doesn't match the comment before the if
>>any more.  This now accepts many other "li" variants, and even a few
>>"addi" that aren't actually "li" (it only verifies four out of the
>>five bits of "RA" - for "li" they must all be zero).

>>+               || (op & 0xfc1fffff) == 0x7c012f99 /* stxvd2x Vs,r1, r5 */
>>+               || (op & 0xfc1fffff) == 0x7c012f98 /* stxvd2x vs0, r1, r5 */

>Since for the stxvd2x instruction, the last bit is part of the
>register number, these two lines should be merged into:
 >             || (op & 0xfc1ffffe) == 0x7c012f98 /* stxvd2x Vs, r1, r5 */

>As above, the comment needs to be updated to match.

>>+               || (op & 0xfc1ff000) == 0x7c012000)

>This looks just wrong: that masks out most of the secondary opcode
>field, so it will match many different instructions.  That cannot
>be intended ...

I realised this wasn't required while I experiment. I got your point..

>This means that on older AIX versions, GDB simply won't work anymore,
>right?  If so, I guess I'll leave if up to you to decide whether this
>is acceptable.  But the minimum supported version should be documented
>somewhere (there's still at least mention of AIX 4.x being supported).
>Also, if we do switch the minimum supported version, I think there's
>quite a bit of existing code intended to handle old versions that could
>then just be removed ...

How would you like me to document this. Let me know.. And we can work out if possible, via another patch to remove handling the old version codes.

>Should we change the default for "Motorola PowerPC 620"
>at this stage?  Can we even test on that machine?
>>ANS = No. No we cannot test on that machine.

>Well, why are you changing it then?  Shouldn't those
>defaults be left as they are, and only be overridden
>on platforms where we know we have support (which
>should be done by rs6000_nat_target::read_description).

I have removed the same in this patch...


________________________________
From: Ulrich Weigand <Ulrich.Weigand@de.ibm.com>
Sent: 28 October 2022 18:09
To: gdb-patches@sourceware.org <gdb-patches@sourceware.org>; Aditya Kamath1 <Aditya.Kamath1@ibm.com>; simark@simark.ca <simark@simark.ca>
Cc: Sangamesh Mallayya <sangamesh.swamy@in.ibm.com>
Subject: Re: [PATCH] Enable vector instruction debugging for AIX

Aditya Kamath1 <Aditya.Kamath1@ibm.com> wrote:

>Thank you for the feedback. Please find attached the new patch.

Thanks for the update!  Please find additional comments below.

>Kindly let me know if any changes required.

>@@ -1352,6 +1352,43 @@ fetch_regs_kernel_thread (struct regcache *regcache, int regno,

I just noticed that there seem to be two methods, handling
user threads and kernel threads, respectively.  You patch
only adds support for VMX/VSX registers to kernel threads
-- should this also be done for user threads?  (Same applies
to store_regs_... of course.)

>+  /* vector registers.  */
>+  if (tdep->ppc_vr0_regnum != -1 && regno >= tdep->ppc_vr0_regnum)

This check isn't quite correct.  I think it should be:
  if the target has vector registers
  and (all registers are requested, i.e. regno == -1
       or a vector register is requested, i.e. regno is a VR)

Something along these lines:
 if (tdep->ppc_vr0_regnum != -1 && tdep->vrsave_regnum != -1
     && (regno == -1 || (regno >= tdep->ppc_vr0_regnum
                         && regno <= tdep->ppc_vrsave_regnum))

(Similar for VSX.  Also applies to the store_regs_... code.)

>+      if (__power_vmx())
>+      {
>+        if (arch64)
>+          ret = ptrace64aix (PTT_READ_VEC, tid, (long long) &vmx, 0, 0);
>+        else
>+          ret = ptrace32 (PTT_READ_VEC, tid, (long long) &vmx, 0, 0);
>+        if (ret < 0)
>+          memset(&vmx, 0, sizeof(__vmx_context_t));
>+        for (i = 0; i < num_of_vrregs; i++)
>+          regcache->raw_supply (tdep->ppc_vr0_regnum + i, &(vmx.__vr[i]));
>+      }

So if access to VMX registers fails because ptrace fails,
you now zero out the registers.  But if access fails because
__power_vmx() fails, the registers are left undefined.

Is this intentional?  What the meaning of this difference?

>+  for (regno = 0; regno < num_of_vrregs; regno++)
>+    if (REG_VALID == regcache->get_register_status (tdep->ppc_vr0_regnum + regno))
>+      regcache->raw_collect (tdep->ppc_vr0_regnum + regno,
>+                                 &(vmx->__vr[0]) + regno);

Is there a reason for not using &(vmx->__vr[regno]) like above?
That would seem to be more straightforward.

>+  if (getthrds64(pid, &thrdentry, sizeof(struct thrdentry64),
>+                                            &thrd_i, 1) == 1)
>+    thrd_i = thrdentry.ti_tid;
>+  memset(&vsx, 0, sizeof(__vsx_context_t));
>+  if (__power_vsx())
>+  {
>+    if (ARCH64 ())
>+      ret = rs6000_ptrace64 (PTT_READ_VSX, thrd_i, (long long) &vsx, 0, 0);
>+    else
>+      ret = rs6000_ptrace32 (PTT_READ_VSX, thrd_i, (int *)&vsx, 0, 0);

Just noticed this here.  Everywhere else in this file, a *pid* is
passed to rs6000_ptrace64/32.  Only in this new routine, you're
apparently passing a thread ID.   Why should this be different just
for these registers?  Shouldn't per-thread registers be handled in
aix-thread.c anyway?

>@@ -233,7 +411,7 @@ fetch_register (struct regcache *regcache, int regno)
>            *addr = buf;
>        }
>     }
>-
>+
>   if (!errno)
>     regcache->raw_supply (regno, (char *) addr);
>   else

Please remove these no-op changes from the patch.

>@@ -244,6 +422,9 @@ fetch_register (struct regcache *regcache, int regno)
> #endif
>       errno = 0;
>     }
>+
>+  fetch_altivec_registers_aix (regcache);
>+  fetch_vsx_registers_aix (regcache);
> }


I think you should call this only if the requested regno
actually *is* a VMX or VSX register.  Also, if this is the
case, you don't need to do anything else in this routine.
This should be separate checks at the beginning of the
routine, just like you now do in store_register.

>@@ -1972,7 +1972,8 @@ skip_prologue (struct gdbarch *gdbarch, CORE_ADDR pc, CORE_ADDR lim_pc,
>       /* 001110 00000 00000 iiii iiii iiii iiii  */
>       /* 001110 01110 00000 iiii iiii iiii iiii  */
>       else if ((op & 0xffff0000) == 0x38000000         /* li r0, SIMM */
>-              || (op & 0xffff0000) == 0x39c00000)     /* li r14, SIMM */
>+              || (op & 0xffff0000) == 0x39c00000     /* li r14, SIMM */
>+               || (op & 0xff0f0000) == 0x38000000)     /* li, r4, SIMM */

Please fix the tabs vs. spaces, and keep the comments at the end aligned.

More importantly, the code doesn't match the comment before the if
any more.  This now accepts many other "li" variants, and even a few
"addi" that aren't actually "li" (it only verifies four out of the
five bits of "RA" - for "li" they must all be zero).

Can you explain what exact set of instructions this is *intended*
to match now, and why?  Then we can figure out what the appropriate
bitmask to implement the check would be.

These checks are difficult enough to understand anyway, so it is
crucially important that the comments explaining them are correct
and match what is being done.  So any change must be accompanied
by a corresponding comment update.


>@@ -1986,7 +1987,10 @@ skip_prologue (struct gdbarch *gdbarch, CORE_ADDR pc, CORE_ADDR lim_pc,
>        }
>       /* Store vector register S at (r31+r0) aligned to 16 bytes.  */
>       /* 011111 sssss 11111 00000 00111001110 */
>-      else if ((op & 0xfc1fffff) == 0x7c1f01ce)   /* stvx Vs, R31, R0 */
>+      else if ((op & 0xfc1fffff) == 0x7c1f01ce   /* stvx Vs, R31, R0 */
>+               || (op & 0xfc1fffff) == 0x7c012f99 /* stxvd2x Vs,r1, r5 */
>+               || (op & 0xfc1fffff) == 0x7c012f98 /* stxvd2x vs0, r1, r5 */

Since for the stxvd2x instruction, the last bit is part of the
register number, these two lines should be merged into:
              || (op & 0xfc1ffffe) == 0x7c012f98 /* stxvd2x Vs, r1, r5 */

As above, the comment needs to be updated to match.

>+               || (op & 0xfc1ff000) == 0x7c012000)

This looks just wrong: that masks out most of the secondary opcode
field, so it will match many different instructions.  That cannot
be intended ...


>>+const struct target_desc *
>>+rs6000_nat_target::read_description ()
>>+{
>>+   if (ARCH64())
>>+      return tdesc_powerpc_vsx64;
>>+   else
>>+      return tdesc_powerpc_vsx32;
>>+}
>
>Well, is this true?  Shouldn't this check whether the machine
>actually *has* VSX, and return a different tdesc otherwise?
>
>ANS = AIX 5.3 onwards we get those _vmx_context_t, __power_vmx variables to support these target descriptions.

This means that on older AIX versions, GDB simply won't work anymore,
right?  If so, I guess I'll leave if up to you to decide whether this
is acceptable.  But the minimum supported version should be documented
somewhere (there's still at least mention of AIX 4.x being supported).
Also, if we do switch the minimum supported version, I think there's
quite a bit of existing code intended to handle old versions that could
then just be removed ...


>>+  if (have_altivec)
>>+   cb (".aix-vmx", 560, 560, &rs6000_aix32_vrregset, "AIX altivec",
>cb_data);
>>+
>>+  if (have_vsx)
>>+   cb (".aix-vsx", 256, 256, &rs6000_aix32_vsxregset, "AIX vsx",
>cb_data);
>As those regsets are the same between 32- and 64-bit, they should
>probably be just called rs6000_aix_vrregset etc.
>
>ANS = No, their offsets are different. Hence, we have different variables for them.

That was maybe a misunderstanding, I agree that vrregset and vsxregset
need to be two different variables.  I was simple suggesting that their
names should not contain "32" unless they are 32-bit specific.

I.e. use rs6000_aix_vrregset and rs6000_aix_vsxregset instead of
rs6000_aix32_vrregset and rs6000_aix32_vsxregset.


>>-   bfd_mach_ppc_620, &tdesc_powerpc_64},
>>+   bfd_mach_ppc_620, &tdesc_powerpc_vsx64},
>
>Should we change the default for "Motorola PowerPC 620"
>at this stage?  Can we even test on that machine?
>ANS = No. No we cannot test on that machine.

Well, why are you changing it then?  Shouldn't those
defaults be left as they are, and only be overridden
on platforms where we know we have support (which
should be done by rs6000_nat_target::read_description).


Bye,
Ulrich


[-- Attachment #2: 0001-Enable-Vector-support-for-AIX.patch --]
[-- Type: application/octet-stream, Size: 25841 bytes --]

From 4572c59a3bce6c66c7cab2d9c66c51dd374632e4 Mon Sep 17 00:00:00 2001
From: Aditya Vidyadhar Kamath <Aditya.Kamath1@ibm.com>
Date: Mon, 14 Nov 2022 10:01:40 -0600
Subject: [PATCH] Enable vector instruction debugging for AIX

---
 gdb/aix-thread.c      | 227 ++++++++++++++++++++++++++++++++++++++++++
 gdb/rs6000-aix-nat.c  | 210 +++++++++++++++++++++++++++++++++++++-
 gdb/rs6000-aix-tdep.c | 179 +++++++++++++++++++++++++++++++++
 gdb/rs6000-tdep.c     |   5 +-
 4 files changed, 618 insertions(+), 3 deletions(-)

diff --git a/gdb/aix-thread.c b/gdb/aix-thread.c
index e556c153576..b09cc0b044b 100644
--- a/gdb/aix-thread.c
+++ b/gdb/aix-thread.c
@@ -1259,6 +1259,43 @@ fetch_regs_user_thread (struct regcache *regcache, pthdb_pthread_t pdtid)
   else
     supply_sprs32 (regcache, ctx.iar, ctx.msr, ctx.cr, ctx.lr, ctx.ctr,
 			     ctx.xer, ctx.fpscr);
+
+  /* vector registers.  */
+  if (tdep->ppc_vr0_regnum != -1)
+    {
+      int ret = 0;
+      int num_of_vrregs = tdep->ppc_vrsave_regnum - tdep->ppc_vr0_regnum + 1;
+      __vmx_context_t vmx;
+      if (__power_vmx())
+      {
+        if (arch64)
+          ret = ptrace64aix (PTT_READ_VEC, pdtid, (long long) &vmx, 0, 0);
+        else
+          ret = ptrace32 (PTT_READ_VEC, pdtid, (long long) &vmx, 0, 0);
+        if (ret < 0)
+          memset(&vmx, 0, sizeof(__vmx_context_t));
+        for (i = 0; i < num_of_vrregs; i++)
+          regcache->raw_supply (tdep->ppc_vr0_regnum + i, &(vmx.__vr[i]));
+      }
+    }
+
+  /* vsx registers.  */
+  if (tdep->ppc_vsr0_upper_regnum != -1)
+    {
+      __vsx_context_t vsx;
+      if (__power_vsx())
+      {
+        int ret = 0;
+        if (arch64)
+          ret = ptrace64aix (PTT_READ_VSX, pdtid, (long long) &vsx, 0, 0);
+        else
+          ret = ptrace32 (PTT_READ_VSX, pdtid, (long long) &vsx, 0, 0);
+        if (ret < 0)
+          memset(&vsx, 0, sizeof(__vsx_context_t));
+        for (i = 0; i < ppc_num_vshrs; i++)
+          regcache->raw_supply (tdep->ppc_vsr0_upper_regnum + i, &(vsx.__vsr_dw1[i]));
+        }
+    }
 }
 
 /* Fetch register REGNO if != -1 or all registers otherwise from
@@ -1352,6 +1389,47 @@ fetch_regs_kernel_thread (struct regcache *regcache, int regno,
 	    regcache->raw_supply (tdep->ppc_mq_regnum, (char *) &sprs32.pt_mq);
 	}
     }
+   
+  /* vector registers.  */
+  if (tdep->ppc_vr0_regnum != -1 && tdep->ppc_vrsave_regnum != -1
+      && (regno == -1 || (regno >= tdep->ppc_vr0_regnum
+      && regno <= tdep->ppc_vrsave_regnum)))  
+    {
+      int ret = 0;
+      int num_of_vrregs = tdep->ppc_vrsave_regnum - tdep->ppc_vr0_regnum + 1;
+      __vmx_context_t vmx;
+      if (__power_vmx())
+      {
+        if (arch64)
+          ret = ptrace64aix (PTT_READ_VEC, tid, (long long) &vmx, 0, 0);
+        else
+          ret = ptrace32 (PTT_READ_VEC, tid, (long long) &vmx, 0, 0);
+        if (ret < 0)
+          memset(&vmx, 0, sizeof(__vmx_context_t));
+        for (i = 0; i < num_of_vrregs; i++)
+          regcache->raw_supply (tdep->ppc_vr0_regnum + i, &(vmx.__vr[i]));
+      }
+    }
+
+  /* vsx registers.  */
+  if (tdep->ppc_vsr0_upper_regnum != -1 && tdep->ppc_vrsave_regnum != -1
+      && (regno == -1 || (regno >= tdep->ppc_vsr0_upper_regnum
+      && regno <= tdep->ppc_vrsave_regnum))) 
+    {
+      __vsx_context_t vsx;
+      if (__power_vsx())
+      {
+        int ret = 0;
+        if (arch64)
+          ret = ptrace64aix (PTT_READ_VSX, tid, (long long) &vsx, 0, 0);
+        else
+          ret = ptrace32 (PTT_READ_VSX, tid, (long long) &vsx, 0, 0);
+        if (ret < 0)
+          memset(&vsx, 0, sizeof(__vsx_context_t));
+        for (i = 0; i < ppc_num_vshrs; i++)
+          regcache->raw_supply (tdep->ppc_vsr0_upper_regnum + i, &(vsx.__vsr_dw1[i]));
+        }
+    }
 }
 
 /* Fetch register REGNO if != -1 or all registers otherwise from the
@@ -1496,6 +1574,37 @@ fill_sprs32 (const struct regcache *regcache,
     regcache->raw_collect (tdep->ppc_fpscr_regnum, fpscr);
 }
 
+/* Fill altivec registers.  */
+
+static void
+fill_altivec (const struct regcache *regcache, __vmx_context_t *vmx)
+{
+  struct gdbarch *gdbarch = regcache->arch ();
+  ppc_gdbarch_tdep *tdep = gdbarch_tdep<ppc_gdbarch_tdep> (gdbarch);
+  int num_of_vrregs = tdep->ppc_vrsave_regnum - tdep->ppc_vr0_regnum + 1;
+  int regno;
+
+  for (regno = 0; regno < num_of_vrregs; regno++)
+    if (REG_VALID == regcache->get_register_status (tdep->ppc_vr0_regnum + regno))
+      regcache->raw_collect (tdep->ppc_vr0_regnum + regno,
+                                 &(vmx->__vr[regno]));
+}
+
+/* Fill vsx registers. */
+
+static void
+fill_vsx (const struct regcache *regcache, __vsx_context_t  *vsx)
+{
+  struct gdbarch *gdbarch = regcache->arch ();
+  ppc_gdbarch_tdep *tdep = gdbarch_tdep<ppc_gdbarch_tdep> (gdbarch);
+  int regno;
+
+  for (regno = 0; regno < ppc_num_vshrs; regno++)
+    if (REG_VALID == regcache->get_register_status ( tdep->ppc_vsr0_upper_regnum + regno))
+      regcache->raw_collect (tdep->ppc_vsr0_upper_regnum + regno,
+                                   &(vsx->__vsr_dw1[0]) + regno);
+}
+
 /* Store all registers into pthread PDTID, which doesn't have a kernel
    thread.
 
@@ -1511,6 +1620,7 @@ store_regs_user_thread (const struct regcache *regcache, pthdb_pthread_t pdtid)
   pthdb_context_t ctx;
   uint32_t int32;
   uint64_t int64;
+  int ret;
 
   if (debug_aix_thread)
     gdb_printf (gdb_stdlog, 
@@ -1581,6 +1691,62 @@ store_regs_user_thread (const struct regcache *regcache, pthdb_pthread_t pdtid)
     error (_("aix-thread: store_registers: "
 	     "pthdb_pthread_setcontext returned %s"),
 	   pd_status2str (status));
+
+  /* vector registers.  */
+  if (tdep->ppc_vr0_regnum != -1)
+    {
+      __vmx_context_t vmx;
+      if (__power_vmx())
+      {
+        if (arch64)
+          ret = ptrace64aix (PTT_READ_VEC, pdtid, (long long) &vmx, 0, 0);
+        else
+          ret = ptrace32 (PTT_READ_VEC, pdtid, (long long) &vmx, 0, 0);
+        if (ret < 0)
+        {
+          if (errno == ENXIO)
+          {
+            warning (_("Unable to fetch AltiVec registers."));
+            memset(&vmx, 0, sizeof(__vmx_context_t));
+          }
+          fill_altivec(regcache, &vmx);
+          if (arch64)
+            ret = ptrace64aix (PTT_WRITE_VEC, pdtid, (long long) &vmx, 0, 0);
+          else
+            ret = ptrace32 (PTT_WRITE_VEC, pdtid, (long long) &vmx, 0, 0);
+          if (ret < 0)
+            perror_with_name (_("Unable to store AltiVec register."));
+        }
+      }
+    }
+  
+  /* vmx registers.  */
+  if (tdep->ppc_vsr0_upper_regnum != -1)
+    {
+      __vsx_context_t vsx;
+      if (__power_vsx())
+      {
+        if (arch64)
+          ret =  ptrace64aix (PTT_READ_VSX, pdtid, (long long) &vsx, 0, 0);
+        else
+          ret =  ptrace32 (PTT_READ_VSX, pdtid, (long long) &vsx, 0, 0);
+        if (ret < 0)
+        {
+          if (errno == ENXIO)
+          {
+            warning (_("Unable to fetch VSX registers."));
+            memset(&vsx, 0, sizeof(__vsx_context_t));
+          }
+        }
+        fill_vsx (regcache, &vsx);
+        if (arch64)
+          ret = ptrace64aix (PTT_WRITE_VSX, pdtid, (long long) &vsx, 0, 0);
+        else
+          ret = ptrace32 (PTT_WRITE_VSX, pdtid, (long long) &vsx, 0, 0);
+        if (ret < 0)
+          perror_with_name (_("Unable to store VSX register."));
+      }
+   }
 }
 
 /* Store register REGNO if != -1 or all registers otherwise into
@@ -1602,6 +1768,7 @@ store_regs_kernel_thread (const struct regcache *regcache, int regno,
   double fprs[ppc_num_fprs];
   struct ptxsprs sprs64;
   struct ptsprs  sprs32;
+  int ret = 0; 
 
   if (debug_aix_thread)
     gdb_printf (gdb_stdlog, 
@@ -1692,6 +1859,66 @@ store_regs_kernel_thread (const struct regcache *regcache, int regno,
 	  ptrace32 (PTT_WRITE_SPRS, tid, (uintptr_t) &sprs32, 0, NULL);
 	}
     }
+
+    /* Vector registers.  */
+    if (tdep->ppc_vr0_regnum != -1 && tdep->ppc_vrsave_regnum != -1
+        && (regno == -1 || (regno >= tdep->ppc_vr0_regnum
+        && regno <= tdep->ppc_vrsave_regnum)))
+    {
+      __vmx_context_t vmx;
+      if (__power_vmx())
+      {
+        if (arch64)
+          ret = ptrace64aix (PTT_READ_VEC, tid, (long long) &vmx, 0, 0);
+        else
+          ret = ptrace32 (PTT_READ_VEC, tid, (long long) &vmx, 0, 0);
+        if (ret < 0)
+        {
+          if (errno == ENXIO)
+          {
+            warning (_("Unable to fetch AltiVec registers."));
+            memset(&vmx, 0, sizeof(__vmx_context_t)); 
+          }
+          fill_altivec(regcache, &vmx);
+          if (arch64)
+            ret = ptrace64aix (PTT_WRITE_VEC, tid, (long long) &vmx, 0, 0);
+          else
+            ret = ptrace32 (PTT_WRITE_VEC, tid, (long long) &vmx, 0, 0);
+          if (ret < 0)
+            perror_with_name (_("Unable to store AltiVec register."));
+        }
+      }
+    }
+ 
+    /* vmx registers.  */
+    if (tdep->ppc_vsr0_upper_regnum != -1  && tdep->ppc_vrsave_regnum != -1
+      && (regno == -1 || (regno >= tdep->ppc_vsr0_upper_regnum
+      && regno <= tdep->ppc_vrsave_regnum))) 
+    {
+      __vsx_context_t vsx;
+      if (__power_vsx())
+      {
+        if (arch64)
+          ret =  ptrace64aix (PTT_READ_VSX, tid, (long long) &vsx, 0, 0);
+        else
+          ret =  ptrace32 (PTT_READ_VSX, tid, (long long) &vsx, 0, 0);
+        if (ret < 0)
+        {
+          if (errno == ENXIO)
+          {
+            warning (_("Unable to fetch VSX registers."));
+            memset(&vsx, 0, sizeof(__vsx_context_t)); 
+          }
+        }
+        fill_vsx (regcache, &vsx);
+        if (arch64)
+          ret = ptrace64aix (PTT_WRITE_VSX, tid, (long long) &vsx, 0, 0);
+        else
+          ret = ptrace32 (PTT_WRITE_VSX, tid, (long long) &vsx, 0, 0);
+        if (ret < 0)
+          perror_with_name (_("Unable to store VSX register."));
+      }
+   } 
 }
 
 /* Store gdb's current view of the register set into the
diff --git a/gdb/rs6000-aix-nat.c b/gdb/rs6000-aix-nat.c
index cb141427696..100636059a4 100644
--- a/gdb/rs6000-aix-nat.c
+++ b/gdb/rs6000-aix-nat.c
@@ -54,6 +54,14 @@
 #include <sys/ldr.h>
 #include <sys/systemcfg.h>
 
+#include <sys/context.h>
+#include <sys/pthdebug.h>
+
+#include "features/rs6000/powerpc-vsx64.c"
+#include "features/rs6000/powerpc-vsx32.c"
+#include "features/rs6000/powerpc-altivec32.c"
+#include "features/rs6000/powerpc-altivec64.c"
+
 /* On AIX4.3+, sys/ldr.h provides different versions of struct ld_info for
    debugging 32-bit and 64-bit processes.  Define a typedef and macros for
    accessing fields in the appropriate structures.  */
@@ -91,6 +99,8 @@ class rs6000_nat_target final : public inf_ptrace_target
 
   ptid_t wait (ptid_t, struct target_waitstatus *, target_wait_flags) override;
 
+  const struct target_desc *read_description ()  override;
+
 protected:
 
   void post_startup_inferior (ptid_t ptid) override
@@ -187,6 +197,174 @@ rs6000_ptrace64 (int req, int id, long long addr, int data, void *buf)
   return ret;
 }
 
+/* Store the vsx registers.  */
+
+static void
+store_vsx_register_aix (struct regcache *regcache, int regno)
+{
+  int ret;
+  struct gdbarch *gdbarch = regcache->arch ();
+  ppc_gdbarch_tdep *tdep = gdbarch_tdep<ppc_gdbarch_tdep> (gdbarch);
+  struct thrdentry64 thrdentry;
+  __vsx_context_t vsx;
+  pid_t pid = inferior_ptid.pid ();
+  tid64_t  thrd_i = 0;
+
+  if (getthrds64(pid, &thrdentry, sizeof(struct thrdentry64),
+                                            &thrd_i, 1) == 1)
+    thrd_i = thrdentry.ti_tid;
+  memset(&vsx, 0, sizeof(__vsx_context_t));
+  if (__power_vsx())
+  {
+    if (ARCH64 ())
+      ret = rs6000_ptrace64 (PTT_READ_VSX, thrd_i, (long long) &vsx, 0, 0);
+    else
+      ret = rs6000_ptrace32 (PTT_READ_VSX, thrd_i, (int *)&vsx, 0, 0);
+    if (ret < 0)
+    {
+       if (errno == ENXIO)
+       {
+         warning (_("Unable to fetch VSX registers."));
+         return;
+       }
+    }
+  }
+  regcache->raw_collect (regno, &(vsx.__vsr_dw1[0])+
+                       regno - tdep->ppc_vsr0_upper_regnum);
+
+  if (ARCH64 ())
+    ret = rs6000_ptrace64 (PTT_WRITE_VSX, thrd_i, (long long) &vsx, 0, 0);
+  else
+    ret = rs6000_ptrace32 (PTT_WRITE_VSX, thrd_i, (int *) &vsx, 0, 0);
+
+  if (ret < 0)
+    perror_with_name (_("Unable to store VSX register."));
+}
+
+/* Store Altivec registers.  */
+
+static void
+store_altivec_register_aix (struct regcache *regcache, int regno)
+{
+  int ret;
+  struct gdbarch *gdbarch = regcache->arch ();
+  ppc_gdbarch_tdep *tdep = gdbarch_tdep<ppc_gdbarch_tdep> (gdbarch);
+  struct thrdentry64 thrdentry;
+  __vmx_context_t vmx;
+  pid_t pid = inferior_ptid.pid ();
+  tid64_t  thrd_i = 0;
+
+  if (getthrds64(pid, &thrdentry, sizeof(struct thrdentry64),
+                                            &thrd_i, 1) == 1)
+    thrd_i = thrdentry.ti_tid;
+  memset(&vmx, 0, sizeof(__vmx_context_t));
+  if (__power_vmx())
+  {
+    if (ARCH64 ())
+      ret = rs6000_ptrace64 (PTT_READ_VEC, thrd_i, (long long) &vmx, 0, 0);
+    else
+      ret = rs6000_ptrace32 (PTT_READ_VEC, thrd_i, (int *) &vmx, 0, 0);
+    if (ret < 0)
+    {
+       if (errno == ENXIO)
+       {
+         warning (_("Unable to fetch AltiVec registers."));
+         return;
+       }
+    }
+  }
+
+  regcache->raw_collect (regno, &(vmx.__vr[0]) + regno
+                                - tdep->ppc_vr0_regnum);
+
+  if (ARCH64 ())
+    ret = rs6000_ptrace64 (PTT_WRITE_VEC, thrd_i, (long long) &vmx, 0, 0);
+  else
+    ret = rs6000_ptrace32 (PTT_WRITE_VEC, thrd_i, (int *) &vmx, 0, 0);
+  if (ret < 0)
+     perror_with_name (_("Unable to store AltiVec register."));
+
+}
+
+/* Supply altivec registers.  */
+
+static void
+supply_vrregset_aix (struct regcache *regcache, __vmx_context_t *vmx)
+{
+  int i;
+  struct gdbarch *gdbarch = regcache->arch ();
+  ppc_gdbarch_tdep *tdep = gdbarch_tdep<ppc_gdbarch_tdep> (gdbarch);
+  int num_of_vrregs = tdep->ppc_vrsave_regnum - tdep->ppc_vr0_regnum + 1;
+
+  for (i = 0; i < num_of_vrregs; i++)
+    regcache->raw_supply (tdep->ppc_vr0_regnum + i,
+                                    &(vmx->__vr[i]));
+}
+
+/* Fetch altivec register.  */
+
+static void
+fetch_altivec_registers_aix (struct regcache *regcache)
+{
+  struct thrdentry64 thrdentry;
+  __vmx_context_t vmx;
+  pid_t pid = current_inferior ()->pid;
+  tid64_t  thrd_i = 0;
+
+  if (getthrds64(pid, &thrdentry, sizeof(struct thrdentry64),
+                               &thrd_i, 1) == 1)
+    thrd_i = thrdentry.ti_tid;
+
+  memset(&vmx, 0, sizeof(__vmx_context_t));
+  if (__power_vmx())
+  {
+    if (ARCH64 ())
+      rs6000_ptrace64 (PTT_READ_VEC, thrd_i, (long long) &vmx, 0, 0);
+    else
+      rs6000_ptrace32 (PTT_READ_VEC, thrd_i, (int *) &vmx, 0, 0);
+    supply_vrregset_aix (regcache, &vmx);      
+  }
+}
+
+/* supply vsx register.  */
+
+static void
+supply_vsxregset_aix (struct regcache *regcache, __vsx_context_t *vsx)
+{
+
+  int i;
+  struct gdbarch *gdbarch = regcache->arch ();
+  ppc_gdbarch_tdep *tdep = gdbarch_tdep<ppc_gdbarch_tdep> (gdbarch);
+
+  for (i = 0; i < ppc_num_vshrs; i++)
+   regcache->raw_supply (tdep->ppc_vsr0_upper_regnum + i,
+                                   &(vsx->__vsr_dw1[i]));
+}
+
+/* Fetch vsx registers.  */
+
+static void
+fetch_vsx_registers_aix (struct regcache *regcache)
+{
+  struct thrdentry64 thrdentry;
+  __vsx_context_t vsx;
+  pid_t pid = current_inferior ()->pid;
+  tid64_t  thrd_i = 0;
+
+  if (getthrds64(pid, &thrdentry, sizeof(struct thrdentry64),
+                                            &thrd_i, 1) == 1)
+    thrd_i = thrdentry.ti_tid;
+  memset(&vsx, 0, sizeof(__vsx_context_t));
+  if (__power_vsx())
+  {
+    if (ARCH64 ())
+      rs6000_ptrace64 (PTT_READ_VSX, thrd_i, (long long) &vsx, 0, 0);
+    else
+      rs6000_ptrace32 (PTT_READ_VSX, thrd_i, (int *) &vsx, 0, 0);
+    supply_vsxregset_aix (regcache, &vsx);
+  }
+}
+
 /* Fetch register REGNO from the inferior.  */
 
 static void
@@ -200,6 +378,11 @@ fetch_register (struct regcache *regcache, int regno)
   /* Retrieved values may be -1, so infer errors from errno.  */
   errno = 0;
 
+  if (altivec_register_p (gdbarch, regno))
+    fetch_altivec_registers_aix (regcache);
+  if (vsx_register_p (gdbarch, regno))
+    fetch_vsx_registers_aix (regcache);
+
   nr = regmap (gdbarch, regno, &isfloat);
 
   /* Floating-point registers.  */
@@ -262,6 +445,17 @@ store_register (struct regcache *regcache, int regno)
   /* -1 can be a successful return value, so infer errors from errno.  */
   errno = 0;
 
+  if (altivec_register_p (gdbarch, regno))
+  {
+    store_altivec_register_aix (regcache, regno);
+    return;
+  }
+  if (vsx_register_p (gdbarch, regno))
+  {
+    store_vsx_register_aix (regcache, regno);
+    return;
+  }
+
   nr = regmap (gdbarch, regno, &isfloat);
 
   /* Floating-point registers.  */
@@ -314,7 +508,6 @@ rs6000_nat_target::fetch_registers (struct regcache *regcache, int regno)
   struct gdbarch *gdbarch = regcache->arch ();
   if (regno != -1)
     fetch_register (regcache, regno);
-
   else
     {
       ppc_gdbarch_tdep *tdep = gdbarch_tdep<ppc_gdbarch_tdep> (gdbarch);
@@ -331,7 +524,13 @@ rs6000_nat_target::fetch_registers (struct regcache *regcache, int regno)
       if (tdep->ppc_fp0_regnum >= 0)
 	for (regno = 0; regno < ppc_num_fprs; regno++)
 	  fetch_register (regcache, tdep->ppc_fp0_regnum + regno);
+      
+      if (tdep->ppc_vr0_regnum != -1 && tdep->ppc_vrsave_regnum != -1) 
+        fetch_altivec_registers_aix (regcache);
 
+      if (tdep->ppc_vsr0_upper_regnum != -1)
+        fetch_vsx_registers_aix (regcache);
+          
       /* Read special registers.  */
       fetch_register (regcache, gdbarch_pc_regnum (gdbarch));
       fetch_register (regcache, tdep->ppc_ps_regnum);
@@ -346,6 +545,15 @@ rs6000_nat_target::fetch_registers (struct regcache *regcache, int regno)
     }
 }
 
+const struct target_desc *
+rs6000_nat_target::read_description ()
+{
+   if (ARCH64())
+      return tdesc_powerpc_vsx64;
+   else
+      return tdesc_powerpc_vsx32;
+}
+
 /* Store our register values back into the inferior.
    If REGNO is -1, do this for all registers.
    Otherwise, REGNO specifies which register (so we can save time).  */
diff --git a/gdb/rs6000-aix-tdep.c b/gdb/rs6000-aix-tdep.c
index d47974b51d1..e282c90e8cb 100644
--- a/gdb/rs6000-aix-tdep.c
+++ b/gdb/rs6000-aix-tdep.c
@@ -68,6 +68,176 @@
 /* Minimum possible text address in AIX.  */
 #define AIX_TEXT_SEGMENT_BASE 0x10000000
 
+struct rs6000_aix_reg_vrreg_offset
+{
+  int vr0_offset;
+  int vscr_offset;
+  int vrsave_offset;
+};
+
+static struct rs6000_aix_reg_vrreg_offset rs6000_aix_vrreg_offset =
+{
+   /* AltiVec registers.  */
+  32, /* vr0_offset */
+  544, /* vscr_offset. */
+  560 /* vrsave_offset */
+};
+
+static int
+rs6000_aix_get_vrreg_offset (ppc_gdbarch_tdep *tdep,
+  const struct rs6000_aix_reg_vrreg_offset *offsets,
+                                         int regnum)
+{
+  if (regnum >= tdep->ppc_vr0_regnum &&
+  regnum < tdep->ppc_vr0_regnum + ppc_num_vrs)
+    return offsets->vr0_offset + (regnum - tdep->ppc_vr0_regnum) * 16;
+
+  if (regnum == tdep->ppc_vrsave_regnum - 1)
+    return offsets->vscr_offset;
+
+  if (regnum == tdep->ppc_vrsave_regnum)
+    return offsets->vrsave_offset;
+
+  return -1;
+}
+
+static void
+rs6000_aix_supply_vrregset (const struct regset *regset, struct regcache *regcache,
+                                        int regnum, const void *vrregs, size_t len)
+{
+  struct gdbarch *gdbarch = regcache->arch ();
+  const struct rs6000_aix_reg_vrreg_offset  *offsets;
+  size_t offset;
+  ppc_gdbarch_tdep *tdep = gdbarch_tdep<ppc_gdbarch_tdep> (gdbarch);
+  if (!(tdep->ppc_vr0_regnum >= 0  && tdep->ppc_vrsave_regnum >= 0))
+    return;
+
+  offsets = (const struct rs6000_aix_reg_vrreg_offset *) regset->regmap;
+  if (regnum == -1)
+  {
+    int i;
+
+    for (i = tdep->ppc_vr0_regnum, offset = offsets->vr0_offset;
+                         i < tdep->ppc_vr0_regnum + ppc_num_vrs;
+                                              i++, offset += 16)
+        ppc_supply_reg (regcache, i, (const gdb_byte *) vrregs, offset, 16);
+
+    ppc_supply_reg (regcache, (tdep->ppc_vrsave_regnum - 1),
+        (const gdb_byte *) vrregs, offsets->vscr_offset, 4);
+
+    ppc_supply_reg (regcache, tdep->ppc_vrsave_regnum,
+    (const gdb_byte *) vrregs, offsets->vrsave_offset, 4);
+
+    return;
+  }
+  offset = rs6000_aix_get_vrreg_offset (tdep, offsets, regnum);
+  if (regnum != tdep->ppc_vrsave_regnum &&
+      regnum != tdep->ppc_vrsave_regnum - 1)
+    ppc_supply_reg (regcache, regnum, (const gdb_byte *) vrregs, offset, 16);
+  else
+    ppc_supply_reg (regcache, regnum,
+     (const gdb_byte *) vrregs, offset, 4);
+
+}
+
+static void
+rs6000_aix_supply_vsxregset (const struct regset *regset, struct regcache *regcache,
+                                        int regnum, const void *vsxregs, size_t len)
+{
+  struct gdbarch *gdbarch = regcache->arch ();
+  ppc_gdbarch_tdep *tdep = gdbarch_tdep<ppc_gdbarch_tdep> (gdbarch);
+  if (!(tdep->ppc_vsr0_regnum >= 0))
+    return;
+
+  if (regnum == -1)
+  {
+    int i, offset = 0;
+
+    for (i = tdep->ppc_vsr0_upper_regnum; i < tdep->ppc_vsr0_upper_regnum 
+                                                   + 32; i++, offset += 8)
+      ppc_supply_reg (regcache, i, (const gdb_byte *) vsxregs, offset, 8);
+
+    return;
+  }
+  else
+    ppc_supply_reg (regcache, regnum, (const gdb_byte *) vsxregs, 0, 8);
+}
+
+static void
+rs6000_aix_collect_vsxregset (const struct regset *regset,
+                          const struct regcache *regcache,
+                    int regnum, void *vsxregs, size_t len)
+{
+  struct gdbarch *gdbarch = regcache->arch ();
+  ppc_gdbarch_tdep *tdep = gdbarch_tdep<ppc_gdbarch_tdep> (gdbarch);
+  if (!(tdep->ppc_vsr0_regnum >= 0))
+    return;
+
+  if (regnum == -1)
+  {
+    int i;
+    int offset = 0;
+    for (i = tdep->ppc_vsr0_upper_regnum; i < tdep->ppc_vsr0_upper_regnum
+                                                  + 32; i++, offset += 8)
+      ppc_collect_reg (regcache, i, (gdb_byte *) vsxregs, offset, 8);
+
+    return;
+  }
+
+ else
+    ppc_collect_reg (regcache, regnum, (gdb_byte *) vsxregs, 0, 8);
+}
+
+static void
+rs6000_aix_collect_vrregset (const struct regset *regset,
+                         const struct regcache *regcache,
+                    int regnum, void *vrregs, size_t len)
+{
+  struct gdbarch *gdbarch = regcache->arch ();
+  const struct rs6000_aix_reg_vrreg_offset *offsets;
+  size_t offset;
+  ppc_gdbarch_tdep *tdep = gdbarch_tdep<ppc_gdbarch_tdep> (gdbarch);
+  if (!(tdep->ppc_vr0_regnum >= 0 && tdep->ppc_vrsave_regnum >= 0))
+    return;
+
+  offsets = (const struct rs6000_aix_reg_vrreg_offset *) regset->regmap;
+  if (regnum == -1)
+  {
+    int i;
+
+    for (i = tdep->ppc_vr0_regnum, offset = offsets->vr0_offset; i <
+              tdep->ppc_vr0_regnum + ppc_num_vrs; i++, offset += 16)
+      ppc_collect_reg (regcache, i, (gdb_byte *) vrregs, offset, 16);
+
+    ppc_collect_reg (regcache, (tdep->ppc_vrsave_regnum - 1),
+               (gdb_byte *) vrregs, offsets->vscr_offset, 4);
+
+    ppc_collect_reg (regcache, tdep->ppc_vrsave_regnum,
+             (gdb_byte *) vrregs, offsets->vrsave_offset, 4);
+
+    return;
+  }
+  offset = rs6000_aix_get_vrreg_offset (tdep, offsets, regnum);
+  if (regnum != tdep->ppc_vrsave_regnum
+      && regnum != tdep->ppc_vrsave_regnum - 1)
+    ppc_collect_reg (regcache, regnum, (gdb_byte *) vrregs, offset, 16);
+  else
+    ppc_collect_reg (regcache, regnum,
+                   (gdb_byte *) vrregs, offset, 4);
+}
+
+static const struct regset rs6000_aix_vrregset = {
+  &rs6000_aix_vrreg_offset,
+  rs6000_aix_supply_vrregset,
+  rs6000_aix_collect_vrregset
+};
+
+static const struct regset rs6000_aix_vsxregset = {
+  &rs6000_aix_vrreg_offset,
+  rs6000_aix_supply_vsxregset,
+  rs6000_aix_collect_vsxregset
+};
+
 static struct trad_frame_cache *
 aix_sighandle_frame_cache (frame_info_ptr this_frame,
 			   void **this_cache)
@@ -262,10 +432,19 @@ rs6000_aix_iterate_over_regset_sections (struct gdbarch *gdbarch,
 					 const struct regcache *regcache)
 {
   ppc_gdbarch_tdep *tdep = gdbarch_tdep<ppc_gdbarch_tdep> (gdbarch);
+  int have_altivec = tdep->ppc_vr0_regnum != -1;
+  int have_vsx = tdep->ppc_vsr0_upper_regnum != -1;
+
   if (tdep->wordsize == 4)
     cb (".reg", 592, 592, &rs6000_aix32_regset, NULL, cb_data);
   else
     cb (".reg", 576, 576, &rs6000_aix64_regset, NULL, cb_data);
+
+  if (have_altivec)
+   cb (".aix-vmx", 560, 560, &rs6000_aix_vrregset, "AIX altivec", cb_data);
+
+  if (have_vsx)
+   cb (".aix-vsx", 256, 256, &rs6000_aix_vsxregset, "AIX vsx", cb_data); 
 }
 
 
diff --git a/gdb/rs6000-tdep.c b/gdb/rs6000-tdep.c
index 8b6d666bbe7..7e1c2e3b8ab 100644
--- a/gdb/rs6000-tdep.c
+++ b/gdb/rs6000-tdep.c
@@ -1986,7 +1986,8 @@ skip_prologue (struct gdbarch *gdbarch, CORE_ADDR pc, CORE_ADDR lim_pc,
 	}
       /* Store vector register S at (r31+r0) aligned to 16 bytes.  */      
       /* 011111 sssss 11111 00000 00111001110 */
-      else if ((op & 0xfc1fffff) == 0x7c1f01ce)   /* stvx Vs, R31, R0 */
+      else if ((op & 0xfc1fffff) == 0x7c1f01ce     /* stvx Vs, R31, R0 */
+               || (op & 0xfc1fffff) == 0x7c012f98) /* stxvd2x vs0, r1, r5 */
 	{
 	  if (pc == (li_found_pc + 4))
 	    {
@@ -3474,7 +3475,7 @@ static struct ppc_variant variants[] =
   {"powerpc", "PowerPC user-level", bfd_arch_powerpc,
    bfd_mach_ppc, &tdesc_powerpc_altivec32},
   {"power", "POWER user-level", bfd_arch_rs6000,
-   bfd_mach_rs6k, &tdesc_rs6000},
+   bfd_mach_rs6k, &tdesc_powerpc_vsx32},
   {"403", "IBM PowerPC 403", bfd_arch_powerpc,
    bfd_mach_ppc_403, &tdesc_powerpc_403},
   {"405", "IBM PowerPC 405", bfd_arch_powerpc,
-- 
2.31.1