[3/3] AArch64 SVE: Support changing vector lengths for ptrace

Message ID 20190322162709.55222-4-alan.hayward@arm.com
State New, archived
Headers

Commit Message

Alan Hayward March 22, 2019, 4:27 p.m. UTC
  When writing registers to the kernel, check if regcache VG has been changed. If
so then update the thread's vector length, then write back the registers.

When reading registers from the kernel, ensure regcache VG register is updated.
The regcache registers should already be of the correct length.

Remove all the checks that error if the vector length has changed.

2019-03-22  Alan Hayward  <alan.hayward@arm.com>

gdb/
	* aarch64-linux-nat.c (store_sveregs_to_thread): Set vector length.
	* nat/aarch64-sve-linux-ptrace.c (aarch64_sve_set_vq): New function.
	(aarch64_sve_regs_copy_to_reg_buf): Remove VG checks.
	(aarch64_sve_regs_copy_from_reg_buf): Likewise.
	* nat/aarch64-sve-linux-ptrace.h (aarch64_sve_set_vq): New declaration.
---
 gdb/aarch64-linux-nat.c            |  5 ++
 gdb/nat/aarch64-sve-linux-ptrace.c | 92 ++++++++++++++++--------------
 gdb/nat/aarch64-sve-linux-ptrace.h | 12 +++-
 3 files changed, 63 insertions(+), 46 deletions(-)
  

Patch

diff --git a/gdb/aarch64-linux-nat.c b/gdb/aarch64-linux-nat.c
index 1fe3b83aac..79dcf0d696 100644
--- a/gdb/aarch64-linux-nat.c
+++ b/gdb/aarch64-linux-nat.c
@@ -411,6 +411,11 @@  store_sveregs_to_thread (struct regcache *regcache)
   struct iovec iovec;
   int tid = regcache->ptid ().lwp ();
 
+  /* First store vector length to the thread.  This is done first to ensure the
+     ptrace buffers read from the kernel are the correct size.  */
+  if (!aarch64_sve_set_vq (tid, regcache))
+    perror_with_name (_("Unable to set VG register."));
+
   /* Obtain a dump of SVE registers from ptrace.  */
   std::unique_ptr<gdb_byte[]> base = aarch64_sve_get_sveregs (tid);
 
diff --git a/gdb/nat/aarch64-sve-linux-ptrace.c b/gdb/nat/aarch64-sve-linux-ptrace.c
index 30faab22bb..635b4c9d68 100644
--- a/gdb/nat/aarch64-sve-linux-ptrace.c
+++ b/gdb/nat/aarch64-sve-linux-ptrace.c
@@ -27,8 +27,6 @@ 
 #include "common/common-regcache.h"
 #include "common/byte-vector.h"
 
-static bool vq_change_warned = false;
-
 /* See nat/aarch64-sve-linux-ptrace.h.  */
 
 uint64_t
@@ -63,6 +61,48 @@  aarch64_sve_get_vq (int tid)
 
 /* See nat/aarch64-sve-linux-ptrace.h.  */
 
+bool
+aarch64_sve_set_vq (int tid, uint64_t vq)
+{
+  struct iovec iovec;
+  struct user_sve_header header;
+
+  iovec.iov_len = sizeof (header);
+  iovec.iov_base = &header;
+
+  if (ptrace (PTRACE_GETREGSET, tid, NT_ARM_SVE, &iovec) < 0)
+    {
+      /* SVE is not supported.  */
+      return false;
+    }
+
+  header.vl = sve_vl_from_vq (vq);
+
+  if (ptrace (PTRACE_SETREGSET, tid, NT_ARM_SVE, &iovec) < 0)
+    {
+      /* Vector length change failed.  */
+      return false;
+    }
+
+  return true;
+}
+
+/* See nat/aarch64-sve-linux-ptrace.h.  */
+
+bool
+aarch64_sve_set_vq (int tid, struct reg_buffer_common *reg_buf)
+{
+  if (reg_buf->get_register_status (AARCH64_SVE_VG_REGNUM) != REG_VALID)
+    return false;
+
+  uint64_t reg_vg = 0;
+  reg_buf->raw_collect (AARCH64_SVE_VG_REGNUM, &reg_vg);
+
+  return aarch64_sve_set_vq (tid, sve_vq_from_vg (reg_vg));
+}
+
+/* See nat/aarch64-sve-linux-ptrace.h.  */
+
 std::unique_ptr<gdb_byte[]>
 aarch64_sve_get_sveregs (int tid)
 {
@@ -95,37 +135,18 @@  aarch64_sve_regs_copy_to_reg_buf (struct reg_buffer_common *reg_buf,
 {
   char *base = (char *) buf;
   struct user_sve_header *header = (struct user_sve_header *) buf;
-  uint64_t vq, vg_reg_buf = 0;
 
-  vq = sve_vq_from_vl (header->vl);
+  uint64_t vq = sve_vq_from_vl (header->vl);
+  uint64_t vg = sve_vg_from_vl (header->vl);
 
   /* Sanity check the data in the header.  */
   if (!sve_vl_valid (header->vl)
       || SVE_PT_SIZE (vq, header->flags) != header->size)
     error (_("Invalid SVE header from kernel."));
 
-  if (REG_VALID == reg_buf->get_register_status (AARCH64_SVE_VG_REGNUM))
-    reg_buf->raw_collect (AARCH64_SVE_VG_REGNUM, &vg_reg_buf);
-
-  if (vg_reg_buf == 0)
-    {
-      /* VG has not been set.  */
-      vg_reg_buf = sve_vg_from_vl (header->vl);
-      reg_buf->raw_supply (AARCH64_SVE_VG_REGNUM, &vg_reg_buf);
-    }
-  else if (vg_reg_buf != sve_vg_from_vl (header->vl) && !vq_change_warned)
-    {
-      /* Vector length on the running process has changed.  GDB currently does
-	 not support this and will result in GDB showing incorrect partially
-	 incorrect data for the vector registers.  Warn once and continue.  We
-	 do not expect many programs to exhibit this behaviour.  To fix this
-	 we need to spot the change earlier and generate a new target
-	 descriptor.  */
-      warning (_("SVE Vector length has changed (%ld to %d). "
-		 "Vector registers may show incorrect data."),
-	       vg_reg_buf, sve_vg_from_vl (header->vl));
-      vq_change_warned = true;
-    }
+  /* Update VG.  Note, the registers in the regcache will already be of the
+     correct length.  */
+  reg_buf->raw_supply (AARCH64_SVE_VG_REGNUM, &vg);
 
   if (HAS_SVE_STATE (*header))
     {
@@ -187,30 +208,13 @@  aarch64_sve_regs_copy_from_reg_buf (const struct reg_buffer_common *reg_buf,
 {
   struct user_sve_header *header = (struct user_sve_header *) buf;
   char *base = (char *) buf;
-  uint64_t vq, vg_reg_buf = 0;
-
-  vq = sve_vq_from_vl (header->vl);
+  uint64_t vq = sve_vq_from_vl (header->vl);
 
   /* Sanity check the data in the header.  */
   if (!sve_vl_valid (header->vl)
       || SVE_PT_SIZE (vq, header->flags) != header->size)
     error (_("Invalid SVE header from kernel."));
 
-  if (REG_VALID == reg_buf->get_register_status (AARCH64_SVE_VG_REGNUM))
-    reg_buf->raw_collect (AARCH64_SVE_VG_REGNUM, &vg_reg_buf);
-
-  if (vg_reg_buf != 0 && vg_reg_buf != sve_vg_from_vl (header->vl))
-    {
-      /* Vector length on the running process has changed.  GDB currently does
-	 not support this and will result in GDB writing invalid data back to
-	 the vector registers.  Error and exit.  We do not expect many programs
-	 to exhibit this behaviour.  To fix this we need to spot the change
-	 earlier and generate a new target descriptor.  */
-      error (_("SVE Vector length has changed (%ld to %d). "
-	       "Cannot write back registers."),
-	     vg_reg_buf, sve_vg_from_vl (header->vl));
-    }
-
   if (!HAS_SVE_STATE (*header))
     {
       /* There is no SVE state yet - the register dump contains a fpsimd
diff --git a/gdb/nat/aarch64-sve-linux-ptrace.h b/gdb/nat/aarch64-sve-linux-ptrace.h
index 167fc8ef3c..ee994f26c6 100644
--- a/gdb/nat/aarch64-sve-linux-ptrace.h
+++ b/gdb/nat/aarch64-sve-linux-ptrace.h
@@ -39,17 +39,25 @@ 
 
 uint64_t aarch64_sve_get_vq (int tid);
 
+/* Set VQ in the kernel for the given tid, using either the value VQ or
+   reading from the register VG in the register buffer.  */
+
+bool aarch64_sve_set_vq (int tid, uint64_t vq);
+bool aarch64_sve_set_vq (int tid, struct reg_buffer_common *reg_buf);
+
 /* Read the current SVE register set using ptrace, allocating space as
    required.  */
 
 extern std::unique_ptr<gdb_byte[]> aarch64_sve_get_sveregs (int tid);
 
-/* Put the registers from linux structure buf into register buffer.  */
+/* Put the registers from linux structure buf into register buffer.  Assumes the
+   vector lengths in the register buffer match the size in the kernel.  */
 
 extern void aarch64_sve_regs_copy_to_reg_buf (struct reg_buffer_common *reg_buf,
 					      const void *buf);
 
-/* Put the registers from register buffer into linux structure buf.  */
+/* Put the registers from register buffer into linux structure buf.  Assumes the
+   vector lengths in the register buffer match the size in the kernel.  */
 
 extern void
 aarch64_sve_regs_copy_from_reg_buf (const struct reg_buffer_common *reg_buf,