[PATCH 0/8] Add SVE support for Aarch64 GDB

classic Classic list List threaded Threaded
47 messages Options
123
Reply | Threaded
Open this post in threaded view
|

[PATCH 0/8] Add SVE support for Aarch64 GDB

Alan Hayward
This set of patches adds gdb support for SVE on AArch64.

SVE is the new vector extension for Aarch64. SVE is different in that the
length of a vector register is not fixed - it is can vary for different
hardware implementors. All code compiled for SVE is vector length agnostic.
The Linux kernel can then further restrict the vector length up to the
supported maximum for the hardware. Potentially different process and
threads can have different vector lengths, and they could change at any
time. However, in practice, we expect the vector length to remain constant
across the lifetime of a process.
This set of patches assumes the vector length will not change and either
warns once (when reading registers) or errors (when writing registers) if the
vector length has changed. A future set of patches will offer full support.

This series does not support gdbserver. However, where it makes sense I have
commonised as much code as possible and have added the groundwork in
gdbserver. Enabling gdbserver should be a small set of additional patches.

Core files and watchpoints are not yet supported.

The vector length is read from ptrace and is required to create the target
description for the running process. There is no hardcoded SVE XML.

The patches require recent linux header files for all the SVE ptrace macros.
To allow builds for older headers, I've added all required macros and
structures within ifdef checks. I'm not sure if this is the ideal solution.
When including kernel header code, I've not performed any reformatting (ie
they are not in GNU style).

One of the later patches adds functions to gdbserver regcache so that I can
add common functionality that works on gdb and gdbserver.

Given there are no working SVE systems available today, this was manually
tested on an Aarch64 SVE emulator running Ubuntu. In addition I've run make
check on X86 and Aarch64 builds for both unix and native-gdbserver.


Alan.

Alan Hayward (8):
  Add Aarch64 SVE target description
  Function for reading the Aarch64 SVE vector length.
  Add SVE register defines
  Enable SVE for GDB
  Add aarch64 psuedo help functions
  Aarch64 SVE pseudo register support
  Add methods to gdbserver regcache and raw_compare
  Ptrace support for Aarch64 SVE

 gdb/Makefile.in                     |   1 +
 gdb/aarch64-linux-nat.c             |  60 +++++-
 gdb/aarch64-linux-tdep.c            |   5 +-
 gdb/aarch64-tdep.c                  | 402 +++++++++++++++++++++++-------------
 gdb/aarch64-tdep.h                  |  12 +-
 gdb/arch/aarch64.c                  |  12 +-
 gdb/arch/aarch64.h                  |  37 +++-
 gdb/configure.nat                   |   2 +-
 gdb/doc/gdb.texinfo                 |   4 +
 gdb/features/aarch64-sve.c          | 158 ++++++++++++++
 gdb/gdbserver/Makefile.in           |   1 +
 gdb/gdbserver/configure.srv         |   1 +
 gdb/gdbserver/linux-aarch64-tdesc.c |   3 +-
 gdb/gdbserver/regcache.c            |  55 ++++-
 gdb/gdbserver/regcache.h            |   8 +
 gdb/nat/aarch64-sve-linux-ptrace.c  | 315 ++++++++++++++++++++++++++++
 gdb/nat/aarch64-sve-linux-ptrace.h  | 193 +++++++++++++++++
 gdb/regcache.c                      |  17 ++
 gdb/regcache.h                      |   2 +
 19 files changed, 1122 insertions(+), 166 deletions(-)
 create mode 100644 gdb/features/aarch64-sve.c
 create mode 100644 gdb/nat/aarch64-sve-linux-ptrace.c
 create mode 100644 gdb/nat/aarch64-sve-linux-ptrace.h

--
2.15.1 (Apple Git-101)

Reply | Threaded
Open this post in threaded view
|

[PATCH 1/8] Add Aarch64 SVE target description

Alan Hayward
This patch adds the SVE target description. However, no code will
yet use it - that comes in the later patches.

The create_feature_aarch64_sve function is not generated from XML.
This is because we need to know the sve vector size (VQ) in order
to size the registers correctly.

A VQ of 0 is used when the hardware does not support SVE.
(SVE hardware will always have a valid vector size). I considered
using a bool to indicate SVE in addition to the VQ. Whilst this
may be slightly more readable initially, I think it's a little
odd to have two variables, eg:
aarch64_create_target_description (bool sve_supported, long vq)

Alan.

2018-05-11  Alan Hayward  <[hidden email]>

gdb/
        * aarch64-linux-nat.c (aarch64_linux_read_description):
        Add parmeter zero.
        * aarch64-linux-tdep.c (aarch64_linux_core_read_description):
        Likewise.
        * aarch64-tdep.c (tdesc_aarch64_list): Add.
        (aarch64_read_description): Use VQ to index tdesc_aarch64_list.
        (aarch64_gdbarch_init): Add parmeter zero.
        * aarch64-tdep.h (aarch64_read_description): Add VQ parmeter.
        * arch/aarch64.c (aarch64_create_target_description): Check VQ.
        * arch/aarch64.h (aarch64_create_target_description): Add VQ.
        parmeter.
        * doc/gdb.texinfo: Describe SVE feature
        * features/aarch64-sve.c: New file.

gdbserver/
        * linux-aarch64-tdesc.c (aarch64_linux_read_description): Add
        null VQ.
---
 gdb/aarch64-linux-nat.c             |   3 +-
 gdb/aarch64-linux-tdep.c            |   5 +-
 gdb/aarch64-tdep.c                  |  30 +++++--
 gdb/aarch64-tdep.h                  |   2 +-
 gdb/arch/aarch64.c                  |  12 ++-
 gdb/arch/aarch64.h                  |   5 +-
 gdb/doc/gdb.texinfo                 |   4 +
 gdb/features/aarch64-sve.c          | 158 ++++++++++++++++++++++++++++++++++++
 gdb/gdbserver/linux-aarch64-tdesc.c |   3 +-
 9 files changed, 204 insertions(+), 18 deletions(-)
 create mode 100644 gdb/features/aarch64-sve.c

diff --git a/gdb/aarch64-linux-nat.c b/gdb/aarch64-linux-nat.c
index 908b83a49a..9a6ee91d2d 100644
--- a/gdb/aarch64-linux-nat.c
+++ b/gdb/aarch64-linux-nat.c
@@ -541,7 +541,8 @@ aarch64_linux_nat_target::read_description ()
   if (ret == 0)
     return tdesc_arm_with_neon;
   else
-    return aarch64_read_description ();
+    /* SVE not yet supported.  */
+    return aarch64_read_description (0);
 }
 
 /* Convert a native/host siginfo object, into/from the siginfo in the
diff --git a/gdb/aarch64-linux-tdep.c b/gdb/aarch64-linux-tdep.c
index 1f3e888e40..b84dcfa9ed 100644
--- a/gdb/aarch64-linux-tdep.c
+++ b/gdb/aarch64-linux-tdep.c
@@ -233,7 +233,8 @@ aarch64_linux_iterate_over_regset_sections (struct gdbarch *gdbarch,
       NULL, cb_data);
 }
 
-/* Implement the "core_read_description" gdbarch method.  */
+/* Implement the "core_read_description" gdbarch method.  SVE not yet
+   supported.  */
 
 static const struct target_desc *
 aarch64_linux_core_read_description (struct gdbarch *gdbarch,
@@ -244,7 +245,7 @@ aarch64_linux_core_read_description (struct gdbarch *gdbarch,
   if (target_auxv_search (target, AT_HWCAP, &aarch64_hwcap) != 1)
     return NULL;
 
-  return aarch64_read_description ();
+  return aarch64_read_description (0);
 }
 
 /* Implementation of `gdbarch_stap_is_single_operand', as defined in
diff --git a/gdb/aarch64-tdep.c b/gdb/aarch64-tdep.c
index 01566b475f..806a3dac55 100644
--- a/gdb/aarch64-tdep.c
+++ b/gdb/aarch64-tdep.c
@@ -70,6 +70,9 @@
 #define AARCH64_H0_REGNUM (AARCH64_S0_REGNUM + 32)
 #define AARCH64_B0_REGNUM (AARCH64_H0_REGNUM + 32)
 
+/* All possible aarch64 target descriptors.  */
+struct target_desc *tdesc_aarch64_list[AARCH64_MAX_SVE_VQ + 1];
+
 /* The standard register names, and all the valid aliases for them.  */
 static const struct
 {
@@ -2827,18 +2830,26 @@ aarch64_displaced_step_hw_singlestep (struct gdbarch *gdbarch,
   return 1;
 }
 
-/* Get the correct target description.  */
+/* Get the correct target description for the given VQ value.
+   If VQ is zero then it is assumed SVE is not supported.
+   (It is not possible to set VQ to zero on an SVE system).  */
 
 const target_desc *
-aarch64_read_description ()
+aarch64_read_description (long vq)
 {
-  static target_desc *aarch64_tdesc = NULL;
-  target_desc **tdesc = &aarch64_tdesc;
+  if (vq > AARCH64_MAX_SVE_VQ)
+    error (_("VQ is %ld, maximum supported value is %d"), vq,
+   AARCH64_MAX_SVE_VQ);
+
+  struct target_desc *tdesc = tdesc_aarch64_list[vq];
 
-  if (*tdesc == NULL)
-    *tdesc = aarch64_create_target_description ();
+  if (tdesc == NULL)
+    {
+      tdesc = aarch64_create_target_description (vq);
+      tdesc_aarch64_list[vq] = tdesc;
+    }
 
-  return *tdesc;
+  return tdesc;
 }
 
 /* Initialize the current architecture based on INFO.  If possible,
@@ -2864,7 +2875,8 @@ aarch64_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
 
   /* Ensure we always have a target descriptor.  */
   if (!tdesc_has_registers (tdesc))
-    tdesc = aarch64_read_description ();
+    /* SVE is not yet supported.  */
+    tdesc = aarch64_read_description (0);
 
   gdb_assert (tdesc);
 
@@ -3077,7 +3089,7 @@ When on, AArch64 specific debugging is enabled."),
   selftests::register_test ("aarch64-process-record",
     selftests::aarch64_process_record_test);
   selftests::record_xml_tdesc ("aarch64.xml",
-       aarch64_create_target_description ());
+       aarch64_create_target_description (0));
 #endif
 }
 
diff --git a/gdb/aarch64-tdep.h b/gdb/aarch64-tdep.h
index c806125fb7..c9fd7b3578 100644
--- a/gdb/aarch64-tdep.h
+++ b/gdb/aarch64-tdep.h
@@ -75,7 +75,7 @@ struct gdbarch_tdep
   int (*aarch64_syscall_record) (struct regcache *regcache, unsigned long svc_number);
 };
 
-const target_desc *aarch64_read_description ();
+const target_desc *aarch64_read_description (long vq);
 
 extern int aarch64_process_record (struct gdbarch *gdbarch,
                                struct regcache *regcache, CORE_ADDR addr);
diff --git a/gdb/arch/aarch64.c b/gdb/arch/aarch64.c
index b85e460b6b..d1ec5cedf8 100644
--- a/gdb/arch/aarch64.c
+++ b/gdb/arch/aarch64.c
@@ -21,11 +21,13 @@
 
 #include "../features/aarch64-core.c"
 #include "../features/aarch64-fpu.c"
+#include "../features/aarch64-sve.c"
 
-/* Create the aarch64 target description.  */
+/* Create the aarch64 target description.  A non zero VQ value indicates both
+   the presence of SVE and the SVE vector quotient.  */
 
 target_desc *
-aarch64_create_target_description ()
+aarch64_create_target_description (long vq)
 {
   target_desc *tdesc = allocate_target_description ();
 
@@ -36,7 +38,11 @@ aarch64_create_target_description ()
   long regnum = 0;
 
   regnum = create_feature_aarch64_core (tdesc, regnum);
-  regnum = create_feature_aarch64_fpu (tdesc, regnum);
+
+  if (vq == 0)
+    regnum = create_feature_aarch64_fpu (tdesc, regnum);
+  else
+    regnum = create_feature_aarch64_sve (tdesc, regnum, vq);
 
   return tdesc;
 }
diff --git a/gdb/arch/aarch64.h b/gdb/arch/aarch64.h
index 86185f596a..1846e04163 100644
--- a/gdb/arch/aarch64.h
+++ b/gdb/arch/aarch64.h
@@ -22,7 +22,7 @@
 
 #include "common/tdesc.h"
 
-target_desc *aarch64_create_target_description ();
+target_desc *aarch64_create_target_description (long vq);
 
 /* Register numbers of various important registers.  */
 enum aarch64_regnum
@@ -48,4 +48,7 @@ enum aarch64_regnum
 #define AARCH64_V_REGS_NUM 32
 #define AARCH64_NUM_REGS AARCH64_FPCR_REGNUM + 1
 
+/* Maximum supported VQ value.  Increase if required.  */
+#define AARCH64_MAX_SVE_VQ  16
+
 #endif /* ARCH_AARCH64_H */
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 28f083f96e..04f811032a 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -42134,6 +42134,10 @@ The @samp{org.gnu.gdb.aarch64.fpu} feature is optional.  If present,
 it should contain registers @samp{v0} through @samp{v31}, @samp{fpsr},
 and @samp{fpcr}.
 
+The @samp{org.gnu.gdb.aarch64.sve} feature is optional.  If present,
+it should contain registers @samp{z0} through @samp{z31}, @samp{p0}
+through @samp{p15}, @samp{ffr} and @samp{vg}.
+
 @node ARC Features
 @subsection ARC Features
 @cindex target descriptions, ARC Features
diff --git a/gdb/features/aarch64-sve.c b/gdb/features/aarch64-sve.c
new file mode 100644
index 0000000000..6442640a73
--- /dev/null
+++ b/gdb/features/aarch64-sve.c
@@ -0,0 +1,158 @@
+/* Copyright (C) 2017 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#include "common/tdesc.h"
+
+/* This function is NOT auto generated from xml.  Create the aarch64 with SVE
+   feature into RESULT, where SCALE is the number of 128 bit chunks in a Z
+   register.  */
+
+static int
+create_feature_aarch64_sve (struct target_desc *result, long regnum,
+    int scale)
+{
+  struct tdesc_feature *feature;
+  tdesc_type *element_type, *field_type;
+  tdesc_type_with_fields *type_with_fields;
+
+  feature = tdesc_create_feature (result, "org.gnu.gdb.aarch64.sve");
+
+  element_type = tdesc_named_type (feature, "ieee_double");
+  tdesc_create_vector (feature, "svevdf", element_type, 2 * scale);
+
+  element_type = tdesc_named_type (feature, "uint64");
+  tdesc_create_vector (feature, "svevdu", element_type, 2 * scale);
+
+  element_type = tdesc_named_type (feature, "int64");
+  tdesc_create_vector (feature, "svevds", element_type, 2 * scale);
+
+  element_type = tdesc_named_type (feature, "ieee_single");
+  tdesc_create_vector (feature, "svevsf", element_type, 4 * scale);
+
+  element_type = tdesc_named_type (feature, "uint32");
+  tdesc_create_vector (feature, "svevsu", element_type, 4 * scale);
+
+  element_type = tdesc_named_type (feature, "int32");
+  tdesc_create_vector (feature, "svevss", element_type, 4 * scale);
+
+  element_type = tdesc_named_type (feature, "uint16");
+  tdesc_create_vector (feature, "svevhu", element_type, 8 * scale);
+
+  element_type = tdesc_named_type (feature, "int16");
+  tdesc_create_vector (feature, "svevhs", element_type, 8 * scale);
+
+  element_type = tdesc_named_type (feature, "uint8");
+  tdesc_create_vector (feature, "svevbu", element_type, 16 * scale);
+
+  element_type = tdesc_named_type (feature, "int8");
+  tdesc_create_vector (feature, "svevbs", element_type, 16 * scale);
+
+  type_with_fields = tdesc_create_union (feature, "svevnd");
+  field_type = tdesc_named_type (feature, "svevdf");
+  tdesc_add_field (type_with_fields, "f", field_type);
+  field_type = tdesc_named_type (feature, "svevdu");
+  tdesc_add_field (type_with_fields, "u", field_type);
+  field_type = tdesc_named_type (feature, "svevds");
+  tdesc_add_field (type_with_fields, "s", field_type);
+
+  type_with_fields = tdesc_create_union (feature, "svevns");
+  field_type = tdesc_named_type (feature, "svevsf");
+  tdesc_add_field (type_with_fields, "f", field_type);
+  field_type = tdesc_named_type (feature, "svevsu");
+  tdesc_add_field (type_with_fields, "u", field_type);
+  field_type = tdesc_named_type (feature, "svevss");
+  tdesc_add_field (type_with_fields, "s", field_type);
+
+  type_with_fields = tdesc_create_union (feature, "svevnh");
+  field_type = tdesc_named_type (feature, "svevhu");
+  tdesc_add_field (type_with_fields, "u", field_type);
+  field_type = tdesc_named_type (feature, "svevhs");
+  tdesc_add_field (type_with_fields, "s", field_type);
+
+  type_with_fields = tdesc_create_union (feature, "svevnb");
+  field_type = tdesc_named_type (feature, "svevbu");
+  tdesc_add_field (type_with_fields, "u", field_type);
+  field_type = tdesc_named_type (feature, "svevbs");
+  tdesc_add_field (type_with_fields, "s", field_type);
+
+  type_with_fields = tdesc_create_union (feature, "svev");
+  field_type = tdesc_named_type (feature, "svevnd");
+  tdesc_add_field (type_with_fields, "d", field_type);
+  field_type = tdesc_named_type (feature, "svevns");
+  tdesc_add_field (type_with_fields, "s", field_type);
+  field_type = tdesc_named_type (feature, "svevnh");
+  tdesc_add_field (type_with_fields, "h", field_type);
+  field_type = tdesc_named_type (feature, "svevnb");
+  tdesc_add_field (type_with_fields, "b", field_type);
+
+  field_type = tdesc_named_type (feature, "uint8");
+  tdesc_create_vector (feature, "svep", field_type, 2 * scale);
+
+  tdesc_create_reg (feature, "z0", regnum++, 1, NULL, 128 * scale, "svev");
+  tdesc_create_reg (feature, "z1", regnum++, 1, NULL, 128 * scale, "svev");
+  tdesc_create_reg (feature, "z2", regnum++, 1, NULL, 128 * scale, "svev");
+  tdesc_create_reg (feature, "z3", regnum++, 1, NULL, 128 * scale, "svev");
+  tdesc_create_reg (feature, "z4", regnum++, 1, NULL, 128 * scale, "svev");
+  tdesc_create_reg (feature, "z5", regnum++, 1, NULL, 128 * scale, "svev");
+  tdesc_create_reg (feature, "z6", regnum++, 1, NULL, 128 * scale, "svev");
+  tdesc_create_reg (feature, "z7", regnum++, 1, NULL, 128 * scale, "svev");
+  tdesc_create_reg (feature, "z8", regnum++, 1, NULL, 128 * scale, "svev");
+  tdesc_create_reg (feature, "z9", regnum++, 1, NULL, 128 * scale, "svev");
+  tdesc_create_reg (feature, "z10", regnum++, 1, NULL, 128 * scale, "svev");
+  tdesc_create_reg (feature, "z11", regnum++, 1, NULL, 128 * scale, "svev");
+  tdesc_create_reg (feature, "z12", regnum++, 1, NULL, 128 * scale, "svev");
+  tdesc_create_reg (feature, "z13", regnum++, 1, NULL, 128 * scale, "svev");
+  tdesc_create_reg (feature, "z14", regnum++, 1, NULL, 128 * scale, "svev");
+  tdesc_create_reg (feature, "z15", regnum++, 1, NULL, 128 * scale, "svev");
+  tdesc_create_reg (feature, "z16", regnum++, 1, NULL, 128 * scale, "svev");
+  tdesc_create_reg (feature, "z17", regnum++, 1, NULL, 128 * scale, "svev");
+  tdesc_create_reg (feature, "z18", regnum++, 1, NULL, 128 * scale, "svev");
+  tdesc_create_reg (feature, "z19", regnum++, 1, NULL, 128 * scale, "svev");
+  tdesc_create_reg (feature, "z20", regnum++, 1, NULL, 128 * scale, "svev");
+  tdesc_create_reg (feature, "z21", regnum++, 1, NULL, 128 * scale, "svev");
+  tdesc_create_reg (feature, "z22", regnum++, 1, NULL, 128 * scale, "svev");
+  tdesc_create_reg (feature, "z23", regnum++, 1, NULL, 128 * scale, "svev");
+  tdesc_create_reg (feature, "z24", regnum++, 1, NULL, 128 * scale, "svev");
+  tdesc_create_reg (feature, "z25", regnum++, 1, NULL, 128 * scale, "svev");
+  tdesc_create_reg (feature, "z26", regnum++, 1, NULL, 128 * scale, "svev");
+  tdesc_create_reg (feature, "z27", regnum++, 1, NULL, 128 * scale, "svev");
+  tdesc_create_reg (feature, "z28", regnum++, 1, NULL, 128 * scale, "svev");
+  tdesc_create_reg (feature, "z29", regnum++, 1, NULL, 128 * scale, "svev");
+  tdesc_create_reg (feature, "z30", regnum++, 1, NULL, 128 * scale, "svev");
+  tdesc_create_reg (feature, "z31", regnum++, 1, NULL, 128 * scale, "svev");
+  tdesc_create_reg (feature, "fpsr", regnum++, 1, NULL, 32, "int");
+  tdesc_create_reg (feature, "fpcr", regnum++, 1, NULL, 32, "int");
+  tdesc_create_reg (feature, "p0", regnum++, 1, NULL, 16 * scale, "svep");
+  tdesc_create_reg (feature, "p1", regnum++, 1, NULL, 16 * scale, "svep");
+  tdesc_create_reg (feature, "p2", regnum++, 1, NULL, 16 * scale, "svep");
+  tdesc_create_reg (feature, "p3", regnum++, 1, NULL, 16 * scale, "svep");
+  tdesc_create_reg (feature, "p4", regnum++, 1, NULL, 16 * scale, "svep");
+  tdesc_create_reg (feature, "p5", regnum++, 1, NULL, 16 * scale, "svep");
+  tdesc_create_reg (feature, "p6", regnum++, 1, NULL, 16 * scale, "svep");
+  tdesc_create_reg (feature, "p7", regnum++, 1, NULL, 16 * scale, "svep");
+  tdesc_create_reg (feature, "p8", regnum++, 1, NULL, 16 * scale, "svep");
+  tdesc_create_reg (feature, "p9", regnum++, 1, NULL, 16 * scale, "svep");
+  tdesc_create_reg (feature, "p10", regnum++, 1, NULL, 16 * scale, "svep");
+  tdesc_create_reg (feature, "p11", regnum++, 1, NULL, 16 * scale, "svep");
+  tdesc_create_reg (feature, "p12", regnum++, 1, NULL, 16 * scale, "svep");
+  tdesc_create_reg (feature, "p13", regnum++, 1, NULL, 16 * scale, "svep");
+  tdesc_create_reg (feature, "p14", regnum++, 1, NULL, 16 * scale, "svep");
+  tdesc_create_reg (feature, "p15", regnum++, 1, NULL, 16 * scale, "svep");
+  tdesc_create_reg (feature, "ffr", regnum++, 1, NULL, 16 * scale, "svep");
+  tdesc_create_reg (feature, "vg", regnum++, 1, NULL, 64, "int");
+  return regnum;
+}
diff --git a/gdb/gdbserver/linux-aarch64-tdesc.c b/gdb/gdbserver/linux-aarch64-tdesc.c
index 9f7b9e5c85..3b36c47cfa 100644
--- a/gdb/gdbserver/linux-aarch64-tdesc.c
+++ b/gdb/gdbserver/linux-aarch64-tdesc.c
@@ -32,7 +32,8 @@ aarch64_linux_read_description ()
 
   if (*tdesc == NULL)
     {
-      *tdesc = aarch64_create_target_description ();
+      /* SVE not yet supported.  */
+      *tdesc = aarch64_create_target_description (0);
 
       init_target_desc (*tdesc);
 
--
2.15.1 (Apple Git-101)

Reply | Threaded
Open this post in threaded view
|

[PATCH 2/8] Function for reading the Aarch64 SVE vector length.

Alan Hayward
In reply to this post by Alan Hayward
Add a method for reading the SVE vector length using ptrace. This returns
0 for systems without SVE support.

Note the defines taken from Linux kernel headers in aarch64-sve-linux-ptrace.h.
See the covering email for details about this.

There are multiple ways of expressing the vector length. Thankfully these are
all wll defined. I've added convertors for going from one to the other.
Hopefully this will help to prevent future confusion.

2018-05-11  Alan Hayward  <[hidden email]>

gdb/
        * Makefile.in: Add new header.
        * gdb/arch/aarch64.h (sve_vg_from_vl): New macro.
        (sve_vl_from_vg): Likewise.
        (sve_vq_from_vl): Likewise.
        (sve_vl_from_vq): Likewise.
        (sve_vq_from_vg): Likewise.
        (sve_vg_from_vq): Likewise.
        * configure.nat: Add new c file.
        * nat/aarch64-sve-linux-ptrace.c: New file.
        * nat/aarch64-sve-linux-ptrace.h: New file.

gdbserver/
        * configure.srv: Add new c/h file.
---
 gdb/Makefile.in                    |  1 +
 gdb/arch/aarch64.h                 | 17 +++++++++
 gdb/configure.nat                  |  2 +-
 gdb/gdbserver/configure.srv        |  1 +
 gdb/nat/aarch64-sve-linux-ptrace.c | 52 +++++++++++++++++++++++++++
 gdb/nat/aarch64-sve-linux-ptrace.h | 73 ++++++++++++++++++++++++++++++++++++++
 6 files changed, 145 insertions(+), 1 deletion(-)
 create mode 100644 gdb/nat/aarch64-sve-linux-ptrace.c
 create mode 100644 gdb/nat/aarch64-sve-linux-ptrace.h

diff --git a/gdb/Makefile.in b/gdb/Makefile.in
index 87d74a7703..64042d1bd1 100644
--- a/gdb/Makefile.in
+++ b/gdb/Makefile.in
@@ -1478,6 +1478,7 @@ HFILES_NO_SRCDIR = \
  mi/mi-parse.h \
  nat/aarch64-linux.h \
  nat/aarch64-linux-hw-point.h \
+ nat/aarch64-sve-linux-ptrace.h \
  nat/amd64-linux-siginfo.h \
  nat/gdb_ptrace.h \
  nat/gdb_thread_db.h \
diff --git a/gdb/arch/aarch64.h b/gdb/arch/aarch64.h
index 1846e04163..af0b157c51 100644
--- a/gdb/arch/aarch64.h
+++ b/gdb/arch/aarch64.h
@@ -48,6 +48,23 @@ enum aarch64_regnum
 #define AARCH64_V_REGS_NUM 32
 #define AARCH64_NUM_REGS AARCH64_FPCR_REGNUM + 1
 
+/* There are a number of ways of expressing the current SVE vector size:
+
+   VL : Vector Length.
+ The number of bytes in an SVE Z register.
+   VQ : Vector Quotient.
+ The number of 128bit chunks in an SVE Z register.
+   VG : Vector Gradient.
+ The number of 64bit chunks in an SVE Z register.  */
+
+#define sve_vg_from_vl(vl) ((vl) / 8)
+#define sve_vl_from_vg(vg) ((vg) * 8)
+#define sve_vq_from_vl(vl) ((vl) / 0x10)
+#define sve_vl_from_vq(vq) ((vq) * 0x10)
+#define sve_vq_from_vg(vg) (sve_vq_from_vl (sve_vl_from_vg (vg)))
+#define sve_vg_from_vq(vq) (sve_vg_from_vl (sve_vl_from_vq (vq)))
+
+
 /* Maximum supported VQ value.  Increase if required.  */
 #define AARCH64_MAX_SVE_VQ  16
 
diff --git a/gdb/configure.nat b/gdb/configure.nat
index 6b0f44fede..d7d79adaca 100644
--- a/gdb/configure.nat
+++ b/gdb/configure.nat
@@ -228,7 +228,7 @@ case ${gdb_host} in
     aarch64)
  #  Host: AArch64 based machine running GNU/Linux
  NATDEPFILES="${NATDEPFILES} aarch64-linux-nat.o \
- aarch32-linux-nat.o aarch64-linux-hw-point.o aarch64-linux.o"
+ aarch32-linux-nat.o aarch64-linux-hw-point.o aarch64-linux.o aarch64-sve-linux-ptrace.o"
  ;;
     arm)
  # Host: ARM based machine running GNU/Linux
diff --git a/gdb/gdbserver/configure.srv b/gdb/gdbserver/configure.srv
index ffeefb9b92..8bf0dcc650 100644
--- a/gdb/gdbserver/configure.srv
+++ b/gdb/gdbserver/configure.srv
@@ -54,6 +54,7 @@ case "${target}" in
  srv_tgtobj="$srv_tgtobj arch/aarch64-insn.o"
  srv_tgtobj="$srv_tgtobj arch/aarch64.o"
  srv_tgtobj="$srv_tgtobj linux-aarch64-tdesc.o"
+ srv_tgtobj="$srv_tgtobj aarch64-sve-linux-ptrace.o"
  srv_tgtobj="${srv_tgtobj} $srv_linux_obj"
  srv_linux_regsets=yes
  srv_linux_thread_db=yes
diff --git a/gdb/nat/aarch64-sve-linux-ptrace.c b/gdb/nat/aarch64-sve-linux-ptrace.c
new file mode 100644
index 0000000000..9381786fda
--- /dev/null
+++ b/gdb/nat/aarch64-sve-linux-ptrace.c
@@ -0,0 +1,52 @@
+/* Common target dependent for AArch64 systems.
+
+   Copyright (C) 2017 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#include <sys/utsname.h>
+#include <sys/uio.h>
+#include "common-defs.h"
+#include "elf/external.h"
+#include "elf/common.h"
+#include "aarch64-sve-linux-ptrace.h"
+#include "arch/aarch64.h"
+
+/* Read VQ for the given tid using ptrace.  If SVE is not supported then zero
+   is returned (on a system that supports SVE, then VQ cannot be zeo).  */
+
+unsigned long
+aarch64_sve_get_vq (int tid)
+{
+  struct iovec iovec;
+  struct user_sve_header header;
+
+  iovec.iov_len = sizeof (header);
+  iovec.iov_base = &header;
+
+  /* Ptrace gives the vector length in bytes.  Convert it to VQ, the number of
+     128bit chunks in a Z register.  We use VQ because 128bits is the minimum
+     a Z register can increase in size.  */
+
+  if (ptrace (PTRACE_GETREGSET, tid, NT_ARM_SVE, &iovec) < 0)
+    /* SVE is not supported.  */
+    return 0;
+
+  long vq = sve_vq_from_vl (header.vl);
+  gdb_assert (sve_vl_valid (header.vl));
+
+  return vq;
+}
diff --git a/gdb/nat/aarch64-sve-linux-ptrace.h b/gdb/nat/aarch64-sve-linux-ptrace.h
new file mode 100644
index 0000000000..b318150ac1
--- /dev/null
+++ b/gdb/nat/aarch64-sve-linux-ptrace.h
@@ -0,0 +1,73 @@
+/* Common target dependent for AArch64 systems.
+
+   Copyright (C) 2017 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#ifndef AARCH64_SVE_LINUX_PTRACE_H
+#define AARCH64_SVE_LINUX_PTRACE_H
+
+/* Where indicated, this file contains defines and macros lifted directly from
+   the Linux kernel headers, with no modification.
+   Refer to Linux kernel documentation for details.  */
+
+#include <asm/sigcontext.h>
+#include <sys/utsname.h>
+#include <sys/ptrace.h>
+#include <asm/ptrace.h>
+
+/* Read VQ for the given tid using ptrace.  If SVE is not supported then zero
+   is returned (on a system that supports SVE, then VQ cannot be zeo).  */
+
+extern unsigned long aarch64_sve_get_vq (int tid);
+
+/* Structures and defines taken from sigcontext.h.  */
+
+#ifndef SVE_SIG_ZREGS_SIZE
+
+#define SVE_VQ_BYTES 16 /* number of bytes per quadword */
+
+#define SVE_VQ_MIN 1
+#define SVE_VQ_MAX 512
+
+#define SVE_VL_MIN (SVE_VQ_MIN * SVE_VQ_BYTES)
+#define SVE_VL_MAX (SVE_VQ_MAX * SVE_VQ_BYTES)
+
+#define SVE_NUM_ZREGS 32
+#define SVE_NUM_PREGS 16
+
+#define sve_vl_valid(vl) \
+ ((vl) % SVE_VQ_BYTES == 0 && (vl) >= SVE_VL_MIN && (vl) <= SVE_VL_MAX)
+
+#endif /* SVE_SIG_ZREGS_SIZE.  */
+
+
+/* Structures and defines taken from ptrace.h.  */
+
+#ifndef SVE_PT_SVE_ZREG_SIZE
+
+struct user_sve_header {
+ __u32 size; /* total meaningful regset content in bytes */
+ __u32 max_size; /* maxmium possible size for this thread */
+ __u16 vl; /* current vector length */
+ __u16 max_vl; /* maximum possible vector length */
+ __u16 flags;
+ __u16 __reserved;
+};
+
+#endif /* SVE_PT_SVE_ZREG_SIZE.  */
+
+#endif /* aarch64-sve-linux-ptrace.h */
--
2.15.1 (Apple Git-101)

Reply | Threaded
Open this post in threaded view
|

[PATCH 3/8] Add SVE register defines

Alan Hayward
In reply to this post by Alan Hayward
Add all the SVE register defines used by the later patches.

In order to prevent gaps in the register numbering, the Z registers
reuse the V register numbers (which become pseudos on SVE).

2018-05-11  Alan Hayward  <[hidden email]>

        * aarch64-tdep.c (aarch64_sve_register_names): New const
        var.
        * arch/aarch64.h (enum aarch64_regnum): Add SVE entries.
        (AARCH64_SVE_Z_REGS_NUM): New define.
        (AARCH64_SVE_P_REGS_NUM): Likewise.
        (AARCH64_SVE_NUM_REGS): Likewise.
---
 gdb/aarch64-tdep.c | 21 +++++++++++++++++++++
 gdb/arch/aarch64.h | 15 ++++++++++++++-
 2 files changed, 35 insertions(+), 1 deletion(-)

diff --git a/gdb/aarch64-tdep.c b/gdb/aarch64-tdep.c
index 806a3dac55..1dc31a43bd 100644
--- a/gdb/aarch64-tdep.c
+++ b/gdb/aarch64-tdep.c
@@ -156,6 +156,27 @@ static const char *const aarch64_v_register_names[] =
   "fpcr"
 };
 
+/* The SVE 'Z' and 'P' registers.  */
+static const char *const aarch64_sve_register_names[] =
+{
+  /* These registers must appear in consecutive RAW register number
+     order and they must begin with AARCH64_SVE_Z0_REGNUM! */
+  "z0", "z1", "z2", "z3",
+  "z4", "z5", "z6", "z7",
+  "z8", "z9", "z10", "z11",
+  "z12", "z13", "z14", "z15",
+  "z16", "z17", "z18", "z19",
+  "z20", "z21", "z22", "z23",
+  "z24", "z25", "z26", "z27",
+  "z28", "z29", "z30", "z31",
+  "fpsr", "fpcr",
+  "p0", "p1", "p2", "p3",
+  "p4", "p5", "p6", "p7",
+  "p8", "p9", "p10", "p11",
+  "p12", "p13", "p14", "p15",
+  "ffr", "vg"
+};
+
 /* AArch64 prologue cache structure.  */
 struct aarch64_prologue_cache
 {
diff --git a/gdb/arch/aarch64.h b/gdb/arch/aarch64.h
index af0b157c51..9855e6f286 100644
--- a/gdb/arch/aarch64.h
+++ b/gdb/arch/aarch64.h
@@ -24,7 +24,9 @@
 
 target_desc *aarch64_create_target_description (long vq);
 
-/* Register numbers of various important registers.  */
+/* Register numbers of various important registers.
+   Note that on SVE, the Z registers reuse the V register numbers and the V
+   registers become pseudo registers.  */
 enum aarch64_regnum
 {
   AARCH64_X0_REGNUM, /* First integer register.  */
@@ -35,8 +37,15 @@ enum aarch64_regnum
   AARCH64_CPSR_REGNUM, /* Current Program Status Register.  */
   AARCH64_V0_REGNUM, /* First fp/vec register.  */
   AARCH64_V31_REGNUM = AARCH64_V0_REGNUM + 31, /* Last fp/vec register.  */
+  AARCH64_SVE_Z0_REGNUM = AARCH64_V0_REGNUM, /* First SVE Z register.  */
+  AARCH64_SVE_Z31_REGNUM = AARCH64_V31_REGNUM,  /* Last SVE Z register.  */
   AARCH64_FPSR_REGNUM, /* Floating Point Status Register.  */
   AARCH64_FPCR_REGNUM, /* Floating Point Control Register.  */
+  AARCH64_SVE_P0_REGNUM, /* First SVE predicate register.  */
+  AARCH64_SVE_P15_REGNUM = AARCH64_SVE_P0_REGNUM + 15, /* Last SVE predicate
+   register.  */
+  AARCH64_SVE_FFR_REGNUM, /* SVE First Fault Register.  */
+  AARCH64_SVE_VG_REGNUM, /* SVE Vector Gradient.  */
 
   /* Other useful registers.  */
   AARCH64_LAST_X_ARG_REGNUM = AARCH64_X0_REGNUM + 7,
@@ -46,7 +55,11 @@ enum aarch64_regnum
 
 #define AARCH64_X_REGS_NUM 31
 #define AARCH64_V_REGS_NUM 32
+#define AARCH64_SVE_Z_REGS_NUM AARCH64_V_REGS_NUM
+#define AARCH64_SVE_P_REGS_NUM 16
 #define AARCH64_NUM_REGS AARCH64_FPCR_REGNUM + 1
+#define AARCH64_SVE_NUM_REGS AARCH64_SVE_VG_REGNUM + 1
+
 
 /* There are a number of ways of expressing the current SVE vector size:
 
--
2.15.1 (Apple Git-101)

Reply | Threaded
Open this post in threaded view
|

[PATCH 4/8] Enable SVE for GDB

Alan Hayward
In reply to this post by Alan Hayward
This patch enables SVE support for GDB by reading the VQ when
creating a target description.

It also ensures that SVE is taken into account when creating
the tdep structure, and stores the current VQ value directly
in tdep.

With this patch, gdb on an aarch64 system with SVE will now detect
SVE. The SVE registers will be displayed (but the contents will be
invalid). The following patches fill out the contents.

2018-05-101 Alan Hayward  <[hidden email]>

        * aarch64-linux-nat.c (aarch64_linux_read_description): Support SVE.
        * aarch64-tdep.c (aarch64_get_tdesc_vq): New function.
        (aarch64_gdbarch_init): Check for SVE.
        * aarch64-tdep.h (gdbarch_tdep::has_sve): New function.
---
 gdb/aarch64-linux-nat.c |  4 +--
 gdb/aarch64-tdep.c      | 73 +++++++++++++++++++++++++++++++++++++++----------
 gdb/aarch64-tdep.h      |  9 ++++++
 3 files changed, 69 insertions(+), 17 deletions(-)

diff --git a/gdb/aarch64-linux-nat.c b/gdb/aarch64-linux-nat.c
index 9a6ee91d2d..1270b7e0c0 100644
--- a/gdb/aarch64-linux-nat.c
+++ b/gdb/aarch64-linux-nat.c
@@ -32,6 +32,7 @@
 #include "aarch32-linux-nat.h"
 #include "nat/aarch64-linux.h"
 #include "nat/aarch64-linux-hw-point.h"
+#include "nat/aarch64-sve-linux-ptrace.h"
 
 #include "elf/external.h"
 #include "elf/common.h"
@@ -541,8 +542,7 @@ aarch64_linux_nat_target::read_description ()
   if (ret == 0)
     return tdesc_arm_with_neon;
   else
-    /* SVE not yet supported.  */
-    return aarch64_read_description (0);
+    return aarch64_read_description (aarch64_sve_get_vq (tid));
 }
 
 /* Convert a native/host siginfo object, into/from the siginfo in the
diff --git a/gdb/aarch64-tdep.c b/gdb/aarch64-tdep.c
index 1dc31a43bd..7893579d58 100644
--- a/gdb/aarch64-tdep.c
+++ b/gdb/aarch64-tdep.c
@@ -2873,6 +2873,26 @@ aarch64_read_description (long vq)
   return tdesc;
 }
 
+/* Return the VQ used when creating the target description TDESC.  */
+
+static long
+aarch64_get_tdesc_vq (const struct target_desc *tdesc)
+{
+  const struct tdesc_feature *feature_sve;
+
+  if (!tdesc_has_registers (tdesc))
+    return 0;
+
+  feature_sve = tdesc_find_feature (tdesc, "org.gnu.gdb.aarch64.sve");
+
+  if (feature_sve == nullptr)
+    return 0;
+
+  long vl = tdesc_register_size (feature_sve, aarch64_sve_register_names[0]);
+  return sve_vq_from_vl (vl);
+}
+
+
 /* Initialize the current architecture based on INFO.  If possible,
    re-use an architecture from ARCHES, which is a list of
    architectures already created during this debugging session.
@@ -2890,20 +2910,22 @@ aarch64_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
   const struct target_desc *tdesc = info.target_desc;
   int i;
   int valid_p = 1;
-  const struct tdesc_feature *feature;
+  const struct tdesc_feature *feature_core;
+  const struct tdesc_feature *feature_fpu;
+  const struct tdesc_feature *feature_sve;
   int num_regs = 0;
   int num_pseudo_regs = 0;
 
   /* Ensure we always have a target descriptor.  */
   if (!tdesc_has_registers (tdesc))
-    /* SVE is not yet supported.  */
     tdesc = aarch64_read_description (0);
-
   gdb_assert (tdesc);
 
-  feature = tdesc_find_feature (tdesc, "org.gnu.gdb.aarch64.core");
+  feature_core = tdesc_find_feature (tdesc, "org.gnu.gdb.aarch64.core");
+  feature_fpu = tdesc_find_feature (tdesc, "org.gnu.gdb.aarch64.fpu");
+  feature_sve = tdesc_find_feature (tdesc, "org.gnu.gdb.aarch64.sve");
 
-  if (feature == NULL)
+  if (feature_core == NULL)
     return NULL;
 
   tdesc_data = tdesc_data_alloc ();
@@ -2911,25 +2933,45 @@ aarch64_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
   /* Validate the descriptor provides the mandatory core R registers
      and allocate their numbers.  */
   for (i = 0; i < ARRAY_SIZE (aarch64_r_register_names); i++)
-    valid_p &=
-      tdesc_numbered_register (feature, tdesc_data, AARCH64_X0_REGNUM + i,
-       aarch64_r_register_names[i]);
+    valid_p &= tdesc_numbered_register (feature_core, tdesc_data,
+ AARCH64_X0_REGNUM + i,
+ aarch64_r_register_names[i]);
 
   num_regs = AARCH64_X0_REGNUM + i;
 
-  /* Look for the V registers.  */
-  feature = tdesc_find_feature (tdesc, "org.gnu.gdb.aarch64.fpu");
-  if (feature)
+  /* Add the V registers.  */
+  if (feature_fpu != NULL)
     {
+      gdb_assert (feature_sve == NULL);
+
       /* Validate the descriptor provides the mandatory V registers
-         and allocate their numbers.  */
+ and allocate their numbers.  */
       for (i = 0; i < ARRAY_SIZE (aarch64_v_register_names); i++)
- valid_p &=
-  tdesc_numbered_register (feature, tdesc_data, AARCH64_V0_REGNUM + i,
-   aarch64_v_register_names[i]);
+ valid_p &= tdesc_numbered_register (feature_fpu, tdesc_data,
+    AARCH64_V0_REGNUM + i,
+    aarch64_v_register_names[i]);
 
       num_regs = AARCH64_V0_REGNUM + i;
+    }
+
+  /* Add the SVE registers.  */
+  if (feature_sve != NULL)
+    {
+      gdb_assert (feature_fpu == NULL);
+
+      /* Validate the descriptor provides the mandatory sve registers
+ and allocate their numbers.  */
+      for (i = 0; i < ARRAY_SIZE (aarch64_sve_register_names); i++)
+ valid_p &= tdesc_numbered_register (feature_sve, tdesc_data,
+    AARCH64_SVE_Z0_REGNUM + i,
+    aarch64_sve_register_names[i]);
 
+      num_regs = AARCH64_SVE_Z0_REGNUM + i;
+      num_pseudo_regs += 32; /* add the Vn register pseudos.  */
+    }
+
+  if (feature_fpu != NULL || feature_sve != NULL)
+    {
       num_pseudo_regs += 32; /* add the Qn scalar register pseudos */
       num_pseudo_regs += 32; /* add the Dn scalar register pseudos */
       num_pseudo_regs += 32; /* add the Sn scalar register pseudos */
@@ -2969,6 +3011,7 @@ aarch64_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
   tdep->lowest_pc = 0x20;
   tdep->jb_pc = -1; /* Longjump support not enabled by default.  */
   tdep->jb_elt_size = 8;
+  tdep->vq = aarch64_get_tdesc_vq (tdesc);
 
   set_gdbarch_push_dummy_call (gdbarch, aarch64_push_dummy_call);
   set_gdbarch_frame_align (gdbarch, aarch64_frame_align);
diff --git a/gdb/aarch64-tdep.h b/gdb/aarch64-tdep.h
index c9fd7b3578..046de6228f 100644
--- a/gdb/aarch64-tdep.h
+++ b/gdb/aarch64-tdep.h
@@ -73,6 +73,15 @@ struct gdbarch_tdep
 
   /* syscall record.  */
   int (*aarch64_syscall_record) (struct regcache *regcache, unsigned long svc_number);
+
+  /* The VQ value for SVE targets, or zero if SVE is not supported.  */
+  long vq;
+
+  /* Returns true if the target supports SVE.  */
+  bool has_sve ()
+  {
+    return vq != 0;
+  }
 };
 
 const target_desc *aarch64_read_description (long vq);
--
2.15.1 (Apple Git-101)

Reply | Threaded
Open this post in threaded view
|

[PATCH 5/8] Add aarch64 psuedo help functions

Alan Hayward
In reply to this post by Alan Hayward
Reduce code copy/paste by adding two helper functions for
aarch64_pseudo_read_value and aarch64_pseudo_write
The patch does not change any functionality.

2018-05-11  Alan Hayward  <[hidden email]>

        * aarch64-tdep.c (aarch64_pseudo_read_value_2): New helper func.
        (aarch64_pseudo_write_2): Likewise.
        (aarch64_pseudo_read_value): Use helper.
        (aarch64_pseudo_write): Likewise.
---
 gdb/aarch64-tdep.c | 173 +++++++++++++++++------------------------------------
 1 file changed, 54 insertions(+), 119 deletions(-)

diff --git a/gdb/aarch64-tdep.c b/gdb/aarch64-tdep.c
index 7893579d58..003fefb3c9 100644
--- a/gdb/aarch64-tdep.c
+++ b/gdb/aarch64-tdep.c
@@ -2247,109 +2247,67 @@ aarch64_pseudo_register_reggroup_p (struct gdbarch *gdbarch, int regnum,
   return group == all_reggroup;
 }
 
+/* Inner version of aarch64_pseudo_read_value.  */
+
+static struct value *
+aarch64_pseudo_read_value_2 (readable_regcache *regcache, int regnum_offset,
+     int regsize, struct value *result_value)
+{
+  gdb_byte reg_buf[V_REGISTER_SIZE];
+  unsigned v_regnum = AARCH64_V0_REGNUM + regnum_offset;
+
+  if (regcache->raw_read (v_regnum, reg_buf) != REG_VALID)
+    mark_value_bytes_unavailable (result_value, 0,
+  TYPE_LENGTH (value_type (result_value)));
+  else
+    memcpy (value_contents_raw (result_value), reg_buf, regsize);
+  return result_value;
+ }
+
 /* Implement the "pseudo_register_read_value" gdbarch method.  */
 
 static struct value *
-aarch64_pseudo_read_value (struct gdbarch *gdbarch,
-   readable_regcache *regcache,
+aarch64_pseudo_read_value (struct gdbarch *gdbarch, readable_regcache *regcache,
    int regnum)
 {
-  gdb_byte reg_buf[V_REGISTER_SIZE];
-  struct value *result_value;
-  gdb_byte *buf;
+  struct value *result_value = allocate_value (register_type (gdbarch, regnum));
 
-  result_value = allocate_value (register_type (gdbarch, regnum));
   VALUE_LVAL (result_value) = lval_register;
   VALUE_REGNUM (result_value) = regnum;
-  buf = value_contents_raw (result_value);
 
   regnum -= gdbarch_num_regs (gdbarch);
 
   if (regnum >= AARCH64_Q0_REGNUM && regnum < AARCH64_Q0_REGNUM + 32)
-    {
-      enum register_status status;
-      unsigned v_regnum;
-
-      v_regnum = AARCH64_V0_REGNUM + regnum - AARCH64_Q0_REGNUM;
-      status = regcache->raw_read (v_regnum, reg_buf);
-      if (status != REG_VALID)
- mark_value_bytes_unavailable (result_value, 0,
-      TYPE_LENGTH (value_type (result_value)));
-      else
- memcpy (buf, reg_buf, Q_REGISTER_SIZE);
-      return result_value;
-    }
+    return aarch64_pseudo_read_value_2 (regcache, regnum - AARCH64_Q0_REGNUM,
+ Q_REGISTER_SIZE, result_value);
 
   if (regnum >= AARCH64_D0_REGNUM && regnum < AARCH64_D0_REGNUM + 32)
-    {
-      enum register_status status;
-      unsigned v_regnum;
-
-      v_regnum = AARCH64_V0_REGNUM + regnum - AARCH64_D0_REGNUM;
-      status = regcache->raw_read (v_regnum, reg_buf);
-      if (status != REG_VALID)
- mark_value_bytes_unavailable (result_value, 0,
-      TYPE_LENGTH (value_type (result_value)));
-      else
- memcpy (buf, reg_buf, D_REGISTER_SIZE);
-      return result_value;
-    }
+    return aarch64_pseudo_read_value_2 (regcache, regnum - AARCH64_D0_REGNUM,
+ D_REGISTER_SIZE, result_value);
 
   if (regnum >= AARCH64_S0_REGNUM && regnum < AARCH64_S0_REGNUM + 32)
-    {
-      enum register_status status;
-      unsigned v_regnum;
-
-      v_regnum = AARCH64_V0_REGNUM + regnum - AARCH64_S0_REGNUM;
-      status = regcache->raw_read (v_regnum, reg_buf);
-      if (status != REG_VALID)
- mark_value_bytes_unavailable (result_value, 0,
-      TYPE_LENGTH (value_type (result_value)));
-      else
- memcpy (buf, reg_buf, S_REGISTER_SIZE);
-      return result_value;
-    }
+    return aarch64_pseudo_read_value_2 (regcache, regnum - AARCH64_S0_REGNUM,
+ S_REGISTER_SIZE, result_value);
 
   if (regnum >= AARCH64_H0_REGNUM && regnum < AARCH64_H0_REGNUM + 32)
-    {
-      enum register_status status;
-      unsigned v_regnum;
-
-      v_regnum = AARCH64_V0_REGNUM + regnum - AARCH64_H0_REGNUM;
-      status = regcache->raw_read (v_regnum, reg_buf);
-      if (status != REG_VALID)
- mark_value_bytes_unavailable (result_value, 0,
-      TYPE_LENGTH (value_type (result_value)));
-      else
- memcpy (buf, reg_buf, H_REGISTER_SIZE);
-      return result_value;
-    }
+    return aarch64_pseudo_read_value_2 (regcache, regnum - AARCH64_H0_REGNUM,
+ H_REGISTER_SIZE, result_value);
 
   if (regnum >= AARCH64_B0_REGNUM && regnum < AARCH64_B0_REGNUM + 32)
-    {
-      enum register_status status;
-      unsigned v_regnum;
-
-      v_regnum = AARCH64_V0_REGNUM + regnum - AARCH64_B0_REGNUM;
-      status = regcache->raw_read (v_regnum, reg_buf);
-      if (status != REG_VALID)
- mark_value_bytes_unavailable (result_value, 0,
-      TYPE_LENGTH (value_type (result_value)));
-      else
- memcpy (buf, reg_buf, B_REGISTER_SIZE);
-      return result_value;
-    }
+    return aarch64_pseudo_read_value_2 (regcache, regnum - AARCH64_B0_REGNUM,
+ B_REGISTER_SIZE, result_value);
 
   gdb_assert_not_reached ("regnum out of bound");
 }
 
-/* Implement the "pseudo_register_write" gdbarch method.  */
+/* Inner version of aarch64_pseudo_write.  */
 
 static void
-aarch64_pseudo_write (struct gdbarch *gdbarch, struct regcache *regcache,
-      int regnum, const gdb_byte *buf)
+aarch64_pseudo_write_2 (struct regcache *regcache, int regnum_offset,
+ int regsize, const gdb_byte *buf)
 {
   gdb_byte reg_buf[V_REGISTER_SIZE];
+  unsigned v_regnum = AARCH64_V0_REGNUM + regnum_offset;
 
   /* Ensure the register buffer is zero, we want gdb writes of the
      various 'scalar' pseudo registers to behavior like architectural
@@ -2357,61 +2315,38 @@ aarch64_pseudo_write (struct gdbarch *gdbarch, struct regcache *regcache,
      zero.  */
   memset (reg_buf, 0, sizeof (reg_buf));
 
+  memcpy (reg_buf, buf, regsize);
+  regcache->raw_write (v_regnum, reg_buf);
+}
+
+/* Implement the "pseudo_register_write" gdbarch method.  */
+
+static void
+aarch64_pseudo_write (struct gdbarch *gdbarch, struct regcache *regcache,
+      int regnum, const gdb_byte *buf)
+{
+
   regnum -= gdbarch_num_regs (gdbarch);
 
   if (regnum >= AARCH64_Q0_REGNUM && regnum < AARCH64_Q0_REGNUM + 32)
-    {
-      /* pseudo Q registers */
-      unsigned v_regnum;
-
-      v_regnum = AARCH64_V0_REGNUM + regnum - AARCH64_Q0_REGNUM;
-      memcpy (reg_buf, buf, Q_REGISTER_SIZE);
-      regcache_raw_write (regcache, v_regnum, reg_buf);
-      return;
-    }
+    return aarch64_pseudo_write_2 (regcache, regnum - AARCH64_Q0_REGNUM,
+   Q_REGISTER_SIZE, buf);
 
   if (regnum >= AARCH64_D0_REGNUM && regnum < AARCH64_D0_REGNUM + 32)
-    {
-      /* pseudo D registers */
-      unsigned v_regnum;
-
-      v_regnum = AARCH64_V0_REGNUM + regnum - AARCH64_D0_REGNUM;
-      memcpy (reg_buf, buf, D_REGISTER_SIZE);
-      regcache_raw_write (regcache, v_regnum, reg_buf);
-      return;
-    }
+    return aarch64_pseudo_write_2 (regcache, regnum - AARCH64_D0_REGNUM,
+   D_REGISTER_SIZE, buf);
 
   if (regnum >= AARCH64_S0_REGNUM && regnum < AARCH64_S0_REGNUM + 32)
-    {
-      unsigned v_regnum;
-
-      v_regnum = AARCH64_V0_REGNUM + regnum - AARCH64_S0_REGNUM;
-      memcpy (reg_buf, buf, S_REGISTER_SIZE);
-      regcache_raw_write (regcache, v_regnum, reg_buf);
-      return;
-    }
+    return aarch64_pseudo_write_2 (regcache, regnum - AARCH64_S0_REGNUM,
+   S_REGISTER_SIZE, buf);
 
   if (regnum >= AARCH64_H0_REGNUM && regnum < AARCH64_H0_REGNUM + 32)
-    {
-      /* pseudo H registers */
-      unsigned v_regnum;
-
-      v_regnum = AARCH64_V0_REGNUM + regnum - AARCH64_H0_REGNUM;
-      memcpy (reg_buf, buf, H_REGISTER_SIZE);
-      regcache_raw_write (regcache, v_regnum, reg_buf);
-      return;
-    }
+    return aarch64_pseudo_write_2 (regcache, regnum - AARCH64_H0_REGNUM,
+   H_REGISTER_SIZE, buf);
 
   if (regnum >= AARCH64_B0_REGNUM && regnum < AARCH64_B0_REGNUM + 32)
-    {
-      /* pseudo B registers */
-      unsigned v_regnum;
-
-      v_regnum = AARCH64_V0_REGNUM + regnum - AARCH64_B0_REGNUM;
-      memcpy (reg_buf, buf, B_REGISTER_SIZE);
-      regcache_raw_write (regcache, v_regnum, reg_buf);
-      return;
-    }
+    return aarch64_pseudo_write_2 (regcache, regnum - AARCH64_B0_REGNUM,
+   B_REGISTER_SIZE, buf);
 
   gdb_assert_not_reached ("regnum out of bound");
 }
--
2.15.1 (Apple Git-101)

Reply | Threaded
Open this post in threaded view
|

[PATCH 6/8] Aarch64 SVE pseudo register support

Alan Hayward
In reply to this post by Alan Hayward
Add the functionality for reading/writing psuedo registers.

On SVE the V registers are pseudo registers. This is supported
by adding AARCH64_SVE_V0_REGNUM.

2018-05-11  Alan Hayward  <[hidden email]>

        * aarch64-tdep.c (AARCH64_SVE_V0_REGNUM): Add define.
        (aarch64_vnv_type): Add function.
        (aarch64_pseudo_register_name): Add V regs for SVE.
        (aarch64_pseudo_register_type): Likewise.
        (aarch64_pseudo_register_reggroup_p): Likewise.
        (aarch64_pseudo_read_value_2): Use V0 offset for SVE
        (aarch64_pseudo_read_value): Add V regs for SVE.
        (aarch64_pseudo_write_2): Use V0 offset for SVE
        (aarch64_pseudo_write): Add V regs for SVE.
        * aarch64-tdep.h (struct gdbarch_tdep): Add vnv_type.
---
 gdb/aarch64-tdep.c | 151 +++++++++++++++++++++++++++++++++++++++++++++--------
 gdb/aarch64-tdep.h |   1 +
 2 files changed, 130 insertions(+), 22 deletions(-)

diff --git a/gdb/aarch64-tdep.c b/gdb/aarch64-tdep.c
index 003fefb3c9..6a40a081cb 100644
--- a/gdb/aarch64-tdep.c
+++ b/gdb/aarch64-tdep.c
@@ -69,6 +69,7 @@
 #define AARCH64_S0_REGNUM (AARCH64_D0_REGNUM + 32)
 #define AARCH64_H0_REGNUM (AARCH64_S0_REGNUM + 32)
 #define AARCH64_B0_REGNUM (AARCH64_H0_REGNUM + 32)
+#define AARCH64_SVE_V0_REGNUM (AARCH64_B0_REGNUM + 32)
 
 /* All possible aarch64 target descriptors.  */
 struct target_desc *tdesc_aarch64_list[AARCH64_MAX_SVE_VQ + 1];
@@ -1766,6 +1767,33 @@ aarch64_vnb_type (struct gdbarch *gdbarch)
   return tdep->vnb_type;
 }
 
+/* Return the type for an AdvSISD V register.  */
+
+static struct type *
+aarch64_vnv_type (struct gdbarch *gdbarch)
+{
+  struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
+
+  if (tdep->vnv_type == NULL)
+    {
+      struct type *t;
+      struct type *elem;
+
+      t = arch_composite_type (gdbarch, "__gdb_builtin_type_vnv",
+       TYPE_CODE_UNION);
+
+      append_composite_type_field (t, "d", aarch64_vnd_type (gdbarch));
+      append_composite_type_field (t, "s", aarch64_vns_type (gdbarch));
+      append_composite_type_field (t, "h", aarch64_vnh_type (gdbarch));
+      append_composite_type_field (t, "b", aarch64_vnb_type (gdbarch));
+      append_composite_type_field (t, "q", aarch64_vnq_type (gdbarch));
+
+      tdep->vnv_type = t;
+    }
+
+  return tdep->vnv_type;
+}
+
 /* Implement the "dwarf2_reg_to_regnum" gdbarch method.  */
 
 static int
@@ -2114,6 +2142,8 @@ aarch64_gen_return_address (struct gdbarch *gdbarch,
 static const char *
 aarch64_pseudo_register_name (struct gdbarch *gdbarch, int regnum)
 {
+  struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
+
   static const char *const q_name[] =
     {
       "q0", "q1", "q2", "q3",
@@ -2191,6 +2221,25 @@ aarch64_pseudo_register_name (struct gdbarch *gdbarch, int regnum)
   if (regnum >= AARCH64_B0_REGNUM && regnum < AARCH64_B0_REGNUM + 32)
     return b_name[regnum - AARCH64_B0_REGNUM];
 
+  if (tdep->has_sve ())
+    {
+      static const char *const sve_v_name[] =
+ {
+  "v0", "v1", "v2", "v3",
+  "v4", "v5", "v6", "v7",
+  "v8", "v9", "v10", "v11",
+  "v12", "v13", "v14", "v15",
+  "v16", "v17", "v18", "v19",
+  "v20", "v21", "v22", "v23",
+  "v24", "v25", "v26", "v27",
+  "v28", "v29", "v30", "v31",
+ };
+
+      if (regnum >= AARCH64_SVE_V0_REGNUM
+  && regnum < AARCH64_SVE_V0_REGNUM + AARCH64_V_REGS_NUM)
+ return sve_v_name[regnum - AARCH64_SVE_V0_REGNUM];
+    }
+
   internal_error (__FILE__, __LINE__,
   _("aarch64_pseudo_register_name: bad register number %d"),
   regnum);
@@ -2201,6 +2250,8 @@ aarch64_pseudo_register_name (struct gdbarch *gdbarch, int regnum)
 static struct type *
 aarch64_pseudo_register_type (struct gdbarch *gdbarch, int regnum)
 {
+  struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
+
   regnum -= gdbarch_num_regs (gdbarch);
 
   if (regnum >= AARCH64_Q0_REGNUM && regnum < AARCH64_Q0_REGNUM + 32)
@@ -2218,6 +2269,10 @@ aarch64_pseudo_register_type (struct gdbarch *gdbarch, int regnum)
   if (regnum >= AARCH64_B0_REGNUM && regnum < AARCH64_B0_REGNUM + 32)
     return aarch64_vnb_type (gdbarch);
 
+  if (tdep->has_sve () && regnum >= AARCH64_SVE_V0_REGNUM
+      && regnum < AARCH64_SVE_V0_REGNUM + 32)
+    return aarch64_vnv_type (gdbarch);
+
   internal_error (__FILE__, __LINE__,
   _("aarch64_pseudo_register_type: bad register number %d"),
   regnum);
@@ -2229,6 +2284,8 @@ static int
 aarch64_pseudo_register_reggroup_p (struct gdbarch *gdbarch, int regnum,
     struct reggroup *group)
 {
+  struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
+
   regnum -= gdbarch_num_regs (gdbarch);
 
   if (regnum >= AARCH64_Q0_REGNUM && regnum < AARCH64_Q0_REGNUM + 32)
@@ -2243,6 +2300,9 @@ aarch64_pseudo_register_reggroup_p (struct gdbarch *gdbarch, int regnum,
     return group == all_reggroup || group == vector_reggroup;
   else if (regnum >= AARCH64_B0_REGNUM && regnum < AARCH64_B0_REGNUM + 32)
     return group == all_reggroup || group == vector_reggroup;
+  else if (tdep->has_sve () && regnum >= AARCH64_SVE_V0_REGNUM
+   && regnum < AARCH64_SVE_V0_REGNUM + 32)
+    return group == all_reggroup || group == vector_reggroup;
 
   return group == all_reggroup;
 }
@@ -2250,17 +2310,30 @@ aarch64_pseudo_register_reggroup_p (struct gdbarch *gdbarch, int regnum,
 /* Inner version of aarch64_pseudo_read_value.  */
 
 static struct value *
-aarch64_pseudo_read_value_2 (readable_regcache *regcache, int regnum_offset,
+aarch64_pseudo_read_value_2 (struct gdbarch *gdbarch,
+     readable_regcache *regcache, int regnum_offset,
      int regsize, struct value *result_value)
 {
-  gdb_byte reg_buf[V_REGISTER_SIZE];
+  struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
+  gdb_byte v_buf[V_REGISTER_SIZE], *reg_buf;
+  gdb_assert (AARCH64_V0_REGNUM == AARCH64_SVE_Z0_REGNUM);
   unsigned v_regnum = AARCH64_V0_REGNUM + regnum_offset;
 
+  /* Enough space to read a full vector register.  */
+  if (tdep->has_sve ())
+    reg_buf = (gdb_byte *) xmalloc (register_size (gdbarch, AARCH64_V0_REGNUM));
+  else
+    reg_buf = v_buf;
+
   if (regcache->raw_read (v_regnum, reg_buf) != REG_VALID)
     mark_value_bytes_unavailable (result_value, 0,
   TYPE_LENGTH (value_type (result_value)));
   else
     memcpy (value_contents_raw (result_value), reg_buf, regsize);
+
+  if (tdep->has_sve ())
+    xfree (reg_buf);
+
   return result_value;
  }
 
@@ -2270,6 +2343,7 @@ static struct value *
 aarch64_pseudo_read_value (struct gdbarch *gdbarch, readable_regcache *regcache,
    int regnum)
 {
+  struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
   struct value *result_value = allocate_value (register_type (gdbarch, regnum));
 
   VALUE_LVAL (result_value) = lval_register;
@@ -2278,45 +2352,67 @@ aarch64_pseudo_read_value (struct gdbarch *gdbarch, readable_regcache *regcache,
   regnum -= gdbarch_num_regs (gdbarch);
 
   if (regnum >= AARCH64_Q0_REGNUM && regnum < AARCH64_Q0_REGNUM + 32)
-    return aarch64_pseudo_read_value_2 (regcache, regnum - AARCH64_Q0_REGNUM,
+    return aarch64_pseudo_read_value_2 (gdbarch, regcache,
+ regnum - AARCH64_Q0_REGNUM,
  Q_REGISTER_SIZE, result_value);
 
   if (regnum >= AARCH64_D0_REGNUM && regnum < AARCH64_D0_REGNUM + 32)
-    return aarch64_pseudo_read_value_2 (regcache, regnum - AARCH64_D0_REGNUM,
+    return aarch64_pseudo_read_value_2 (gdbarch, regcache,
+ regnum - AARCH64_D0_REGNUM,
  D_REGISTER_SIZE, result_value);
 
   if (regnum >= AARCH64_S0_REGNUM && regnum < AARCH64_S0_REGNUM + 32)
-    return aarch64_pseudo_read_value_2 (regcache, regnum - AARCH64_S0_REGNUM,
+    return aarch64_pseudo_read_value_2 (gdbarch, regcache,
+ regnum - AARCH64_S0_REGNUM,
  S_REGISTER_SIZE, result_value);
 
   if (regnum >= AARCH64_H0_REGNUM && regnum < AARCH64_H0_REGNUM + 32)
-    return aarch64_pseudo_read_value_2 (regcache, regnum - AARCH64_H0_REGNUM,
+    return aarch64_pseudo_read_value_2 (gdbarch, regcache,
+ regnum - AARCH64_H0_REGNUM,
  H_REGISTER_SIZE, result_value);
 
   if (regnum >= AARCH64_B0_REGNUM && regnum < AARCH64_B0_REGNUM + 32)
-    return aarch64_pseudo_read_value_2 (regcache, regnum - AARCH64_B0_REGNUM,
+    return aarch64_pseudo_read_value_2 (gdbarch, regcache,
+ regnum - AARCH64_B0_REGNUM,
  B_REGISTER_SIZE, result_value);
 
+  if (tdep->has_sve () && regnum >= AARCH64_SVE_V0_REGNUM
+      && regnum < AARCH64_SVE_V0_REGNUM + 32)
+    return aarch64_pseudo_read_value_2 (gdbarch, regcache,
+ regnum - AARCH64_SVE_V0_REGNUM,
+ V_REGISTER_SIZE, result_value);
+
   gdb_assert_not_reached ("regnum out of bound");
 }
 
 /* Inner version of aarch64_pseudo_write.  */
 
 static void
-aarch64_pseudo_write_2 (struct regcache *regcache, int regnum_offset,
- int regsize, const gdb_byte *buf)
+aarch64_pseudo_write_2 (struct gdbarch *gdbarch, struct regcache *regcache,
+ int regnum_offset, int regsize, const gdb_byte *buf)
 {
-  gdb_byte reg_buf[V_REGISTER_SIZE];
+  struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
+  gdb_byte v_buf[V_REGISTER_SIZE], *reg_buf;
+  gdb_assert (AARCH64_V0_REGNUM == AARCH64_SVE_Z0_REGNUM);
   unsigned v_regnum = AARCH64_V0_REGNUM + regnum_offset;
 
+  /* Enough space to write a full vector register.  */
+  if (tdep->has_sve ())
+    reg_buf = (gdb_byte *) xmalloc (register_size (gdbarch, AARCH64_V0_REGNUM));
+  else
+    reg_buf = v_buf;
+
   /* Ensure the register buffer is zero, we want gdb writes of the
      various 'scalar' pseudo registers to behavior like architectural
      writes, register width bytes are written the remainder are set to
      zero.  */
-  memset (reg_buf, 0, sizeof (reg_buf));
+  memset (reg_buf, 0, register_size (gdbarch, AARCH64_V0_REGNUM));
 
   memcpy (reg_buf, buf, regsize);
   regcache->raw_write (v_regnum, reg_buf);
+
+  if (tdep->has_sve ())
+    xfree (reg_buf);
 }
 
 /* Implement the "pseudo_register_write" gdbarch method.  */
@@ -2325,28 +2421,39 @@ static void
 aarch64_pseudo_write (struct gdbarch *gdbarch, struct regcache *regcache,
       int regnum, const gdb_byte *buf)
 {
-
+  struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
   regnum -= gdbarch_num_regs (gdbarch);
 
   if (regnum >= AARCH64_Q0_REGNUM && regnum < AARCH64_Q0_REGNUM + 32)
-    return aarch64_pseudo_write_2 (regcache, regnum - AARCH64_Q0_REGNUM,
-   Q_REGISTER_SIZE, buf);
+    return aarch64_pseudo_write_2 (gdbarch, regcache,
+   regnum - AARCH64_Q0_REGNUM, Q_REGISTER_SIZE,
+   buf);
 
   if (regnum >= AARCH64_D0_REGNUM && regnum < AARCH64_D0_REGNUM + 32)
-    return aarch64_pseudo_write_2 (regcache, regnum - AARCH64_D0_REGNUM,
-   D_REGISTER_SIZE, buf);
+    return aarch64_pseudo_write_2 (gdbarch, regcache,
+   regnum - AARCH64_D0_REGNUM, D_REGISTER_SIZE,
+   buf);
 
   if (regnum >= AARCH64_S0_REGNUM && regnum < AARCH64_S0_REGNUM + 32)
-    return aarch64_pseudo_write_2 (regcache, regnum - AARCH64_S0_REGNUM,
-   S_REGISTER_SIZE, buf);
+    return aarch64_pseudo_write_2 (gdbarch, regcache,
+   regnum - AARCH64_S0_REGNUM, S_REGISTER_SIZE,
+   buf);
 
   if (regnum >= AARCH64_H0_REGNUM && regnum < AARCH64_H0_REGNUM + 32)
-    return aarch64_pseudo_write_2 (regcache, regnum - AARCH64_H0_REGNUM,
-   H_REGISTER_SIZE, buf);
+    return aarch64_pseudo_write_2 (gdbarch, regcache,
+   regnum - AARCH64_H0_REGNUM, H_REGISTER_SIZE,
+   buf);
 
   if (regnum >= AARCH64_B0_REGNUM && regnum < AARCH64_B0_REGNUM + 32)
-    return aarch64_pseudo_write_2 (regcache, regnum - AARCH64_B0_REGNUM,
-   B_REGISTER_SIZE, buf);
+    return aarch64_pseudo_write_2 (gdbarch, regcache,
+   regnum - AARCH64_B0_REGNUM, B_REGISTER_SIZE,
+   buf);
+
+  if (tdep->has_sve () && regnum >= AARCH64_SVE_V0_REGNUM
+      && regnum < AARCH64_SVE_V0_REGNUM + 32)
+    return aarch64_pseudo_write_2 (gdbarch, regcache,
+   regnum - AARCH64_SVE_V0_REGNUM,
+   V_REGISTER_SIZE, buf);
 
   gdb_assert_not_reached ("regnum out of bound");
 }
diff --git a/gdb/aarch64-tdep.h b/gdb/aarch64-tdep.h
index 046de6228f..f4c04d7867 100644
--- a/gdb/aarch64-tdep.h
+++ b/gdb/aarch64-tdep.h
@@ -70,6 +70,7 @@ struct gdbarch_tdep
   struct type *vns_type;
   struct type *vnh_type;
   struct type *vnb_type;
+  struct type *vnv_type;
 
   /* syscall record.  */
   int (*aarch64_syscall_record) (struct regcache *regcache, unsigned long svc_number);
--
2.15.1 (Apple Git-101)

Reply | Threaded
Open this post in threaded view
|

[PATCH 7/8] Add methods to gdbserver regcache and raw_compare

Alan Hayward
In reply to this post by Alan Hayward
Add additional functions to gdbserver regcache to make it more like gdb
regcache. This will allow the next patch to add a common function which
uses regcache.
The alternatives for the next patch would be to either duplicate the
common code in both gdb and gdbserver, or alternatively pass function
pointers for read register, write register, get status to the common code.

In addition, add a register compare function. This will be used in the
next patch. Alternatively instead of adding a new function, I could read
into a buffer and then compare.

2018-05-11  Alan Hayward  <[hidden email]>

gdb/
        * regcache.c (regcache::raw_compare): New function.
        * regcache.h (regcache::raw_compare): New declaration.

gdbserver/
        * regcache.c (register_data): New function.
        (supply_register): Call member function.
        (regcache::raw_supply): Replacement for supply_register.
        (collect_register): Call member function.
        (regcache::raw_collect): Replacement for collect_register.
        (regcache::get_register_status): New function.
        (regcache::raw_compare): Likewise.
        * regcache.h: (regcache::raw_supply): New declaration.
        * (regcache::raw_collect): Likewise.
        * (regcache::raw_compare): Likewise.
        * (regcache::get_register_status): Likewise.
---
 gdb/gdbserver/regcache.c | 55 ++++++++++++++++++++++++++++++++++++++----------
 gdb/gdbserver/regcache.h |  8 +++++++
 gdb/regcache.c           | 17 +++++++++++++++
 gdb/regcache.h           |  2 ++
 4 files changed, 71 insertions(+), 11 deletions(-)

diff --git a/gdb/gdbserver/regcache.c b/gdb/gdbserver/regcache.c
index 0718b9f9a0..88f0db0483 100644
--- a/gdb/gdbserver/regcache.c
+++ b/gdb/gdbserver/regcache.c
@@ -300,7 +300,7 @@ regcache_register_size (const struct regcache *regcache, int n)
 }
 
 static unsigned char *
-register_data (struct regcache *regcache, int n, int fetch)
+register_data (const struct regcache *regcache, int n, int fetch)
 {
   return (regcache->registers
   + find_register_by_number (regcache->tdesc, n).offset / 8);
@@ -312,23 +312,27 @@ register_data (struct regcache *regcache, int n, int fetch)
 
 void
 supply_register (struct regcache *regcache, int n, const void *buf)
+{
+  return regcache->raw_supply (n, buf);
+}
+
+void
+regcache::raw_supply (int n, const void *buf)
 {
   if (buf)
     {
-      memcpy (register_data (regcache, n, 0), buf,
-      register_size (regcache->tdesc, n));
+      memcpy (register_data (this, n, 0), buf, register_size (tdesc, n));
 #ifndef IN_PROCESS_AGENT
-      if (regcache->register_status != NULL)
- regcache->register_status[n] = REG_VALID;
+      if (register_status != NULL)
+ register_status[n] = REG_VALID;
 #endif
     }
   else
     {
-      memset (register_data (regcache, n, 0), 0,
-      register_size (regcache->tdesc, n));
+      memset (register_data (this, n, 0), 0, register_size (tdesc, n));
 #ifndef IN_PROCESS_AGENT
-      if (regcache->register_status != NULL)
- regcache->register_status[n] = REG_UNAVAILABLE;
+      if (register_status != NULL)
+ register_status[n] = REG_UNAVAILABLE;
 #endif
     }
 }
@@ -410,10 +414,16 @@ supply_register_by_name (struct regcache *regcache,
 void
 collect_register (struct regcache *regcache, int n, void *buf)
 {
-  memcpy (buf, register_data (regcache, n, 1),
-  register_size (regcache->tdesc, n));
+  regcache->raw_collect (n, buf);
+}
+
+void
+regcache::raw_collect (int n, void *buf) const
+{
+  memcpy (buf, register_data (this, n, 1), register_size (tdesc, n));
 }
 
+
 enum register_status
 regcache_raw_read_unsigned (struct regcache *regcache, int regnum,
     ULONGEST *val)
@@ -480,3 +490,26 @@ regcache_write_pc (struct regcache *regcache, CORE_ADDR pc)
 }
 
 #endif
+
+enum register_status
+regcache::get_register_status (int regnum) const
+{
+#ifndef IN_PROCESS_AGENT
+  gdb_assert (regnum >= 0 && regnum < tdesc->reg_defs.size ());
+  return (enum register_status) (register_status[regnum]);
+#else
+  return REG_VALID;
+#endif
+}
+
+/* Compare the contents of the register stored in the regcache (ignoring the
+   first OFFSET bytes) to the contents of BUF (without any offset).  Returns 0
+   if identical.  */
+
+int
+regcache::raw_compare (int regnum, const void *buf, int offset) const
+{
+  gdb_assert (register_size (tdesc, regnum) > offset);
+  return memcmp (buf, register_data (this, regnum, 1) + offset,
+ register_size (tdesc, regnum) - offset);
+}
diff --git a/gdb/gdbserver/regcache.h b/gdb/gdbserver/regcache.h
index 2c0df648f6..b3631bebd2 100644
--- a/gdb/gdbserver/regcache.h
+++ b/gdb/gdbserver/regcache.h
@@ -45,6 +45,14 @@ struct regcache
   /* One of REG_UNAVAILBLE or REG_VALID.  */
   unsigned char *register_status;
 #endif
+
+  void raw_supply (int regnum, const void *buf);
+
+  void raw_collect (int regnum, void *buf) const;
+
+  int raw_compare (int regnum, const void *buf, int offset) const;
+
+  enum register_status get_register_status (int regnum) const;
 };
 
 struct regcache *init_register_cache (struct regcache *regcache,
diff --git a/gdb/regcache.c b/gdb/regcache.c
index a3cc7743a2..92b9a69593 100644
--- a/gdb/regcache.c
+++ b/gdb/regcache.c
@@ -1197,6 +1197,23 @@ regcache::collect_regset (const struct regset *regset,
   transfer_regset (regset, NULL, regnum, NULL, buf, size);
 }
 
+/* Compare the contents of the register stored in the regcache (ignoring the
+   first OFFSET bytes) to the contents of BUF (without any offset).  Returns 0
+   if identical.  */
+
+int
+regcache::raw_compare (int regnum, const void *buf, int offset) const
+{
+  const char *regbuf;
+  size_t size;
+
+  gdb_assert (buf != NULL);
+  assert_regnum (regnum);
+
+  regbuf = (const char *) register_buffer (regnum);
+  size = m_descr->sizeof_register[regnum];
+  return memcmp (buf, regbuf + offset, size - offset);
+}
 
 /* Special handling for register PC.  */
 
diff --git a/gdb/regcache.h b/gdb/regcache.h
index d7bb8b5c93..61d6e33c98 100644
--- a/gdb/regcache.h
+++ b/gdb/regcache.h
@@ -357,6 +357,8 @@ public:
   void collect_regset (const struct regset *regset, int regnum,
        void *buf, size_t size) const;
 
+  int raw_compare (int regnum, const void *buf, int offset) const;
+
   ptid_t ptid () const
   {
     return m_ptid;
--
2.15.1 (Apple Git-101)

Reply | Threaded
Open this post in threaded view
|

[PATCH 8/8] Ptrace support for Aarch64 SVE

Alan Hayward
In reply to this post by Alan Hayward
Add support for reading and writing registers for Aarch64 SVE.
I've made this functionality common as it will be required for
gdbserver when gdbsever sve support is added.

Given that gdbserver does not yet call this function, I am
happy to remove the regcache commonise functions from the
previous patch. However, this would result in code in nat/
that does not compile for gdbserver. I wanted to avoid that.

We need to support the cases where the kernel only gives us a
fpsimd structure. This occurs when there is no active SVE state
in the kernel (for example, after starting a new process).

As per the covering email description, I've included large chunks
of linux kernel headers within an ifdef. Formatting of these macros
remains identical to the Kernel versions (ie not adapted to GNU style).

Added checks to make sure the vector length has not changed whilst
the process is running.

2018-05-11  Alan Hayward  <[hidden email]>

gdb/
        * aarch64-linux-nat.c (fetch_sveregs_from_thread): New function.
        (store_sveregs_to_thread): Likewise.
        (aarch64_linux_fetch_inferior_registers): Check for SVE.
        (aarch64_linux_store_inferior_registers): Likewise.
        * nat/aarch64-sve-linux-ptrace.c (aarch64_sve_get_sveregs): New
        function.
        (aarch64_sve_regs_copy_to_regcache): Likewise.
        (aarch64_sve_regs_copy_from_regcache): Likewise.
        * nat/aarch64-sve-linux-ptrace.h (aarch64_sve_get_sveregs): New
        declaration.
        (aarch64_sve_regs_copy_to_regcache): Likewise.
        (aarch64_sve_regs_copy_from_regcache): Likewise.
        (sve_context): Structure from Linux headers.
        (SVE_SIG_ZREGS_SIZE): Define from Linux headers.
        (SVE_SIG_ZREG_SIZE): Likewise.
        (SVE_SIG_PREG_SIZE): Likewise.
        (SVE_SIG_FFR_SIZE): Likewise.
        (SVE_SIG_REGS_OFFSET): Likewise.
        (SVE_SIG_ZREGS_OFFSET): Likewise.
        (SVE_SIG_ZREG_OFFSET): Likewise.
        (SVE_SIG_ZREGS_SIZE): Likewise.
        (SVE_SIG_PREGS_OFFSET): Likewise.
        (SVE_SIG_PREG_OFFSET): Likewise.
        (SVE_SIG_PREGS_SIZE): Likewise.
        (SVE_SIG_FFR_OFFSET): Likewise.
        (SVE_SIG_REGS_SIZE): Likewise.
        (SVE_SIG_CONTEXT_SIZE): Likewise.
        (SVE_PT_REGS_MASK): Likewise.
        (SVE_PT_REGS_FPSIMD): Likewise.
        (SVE_PT_REGS_SVE): Likewise.
        (SVE_PT_VL_INHERIT): Likewise.
        (SVE_PT_VL_ONEXEC): Likewise.
        (SVE_PT_REGS_OFFSET): Likewise.
        (SVE_PT_FPSIMD_OFFSET): Likewise.
        (SVE_PT_FPSIMD_SIZE): Likewise.
        (SVE_PT_SVE_ZREG_SIZE): Likewise.
        (SVE_PT_SVE_PREG_SIZE): Likewise.
        (SVE_PT_SVE_FFR_SIZE): Likewise.
        (SVE_PT_SVE_FPSR_SIZE): Likewise.
        (SVE_PT_SVE_FPCR_SIZE): Likewise.
        (__SVE_SIG_TO_PT): Likewise.
        (SVE_PT_SVE_OFFSET): Likewise.
        (SVE_PT_SVE_ZREGS_OFFSET): Likewise.
        (SVE_PT_SVE_ZREG_OFFSET): Likewise.
        (SVE_PT_SVE_ZREGS_SIZE): Likewise.
        (SVE_PT_SVE_PREGS_OFFSET): Likewise.
        (SVE_PT_SVE_PREG_OFFSET): Likewise.
        (SVE_PT_SVE_PREGS_SIZE): Likewise.
        (SVE_PT_SVE_FFR_OFFSET): Likewise.
        (SVE_PT_SVE_FPSR_OFFSET): Likewise.
        (SVE_PT_SVE_FPCR_OFFSET): Likewise.
        (SVE_PT_SVE_SIZE): Likewise.
        (SVE_PT_SIZE): Likewise.
        (HAS_SVE_STATE): New define.

gdbserver
        * Makefile.in: Add aarch64-sve-linux-ptrace.c.
---
 gdb/aarch64-linux-nat.c            |  57 +++++++-
 gdb/gdbserver/Makefile.in          |   1 +
 gdb/nat/aarch64-sve-linux-ptrace.c | 263 +++++++++++++++++++++++++++++++++++++
 gdb/nat/aarch64-sve-linux-ptrace.h | 120 +++++++++++++++++
 4 files changed, 439 insertions(+), 2 deletions(-)

diff --git a/gdb/aarch64-linux-nat.c b/gdb/aarch64-linux-nat.c
index 1270b7e0c0..bc570ade6d 100644
--- a/gdb/aarch64-linux-nat.c
+++ b/gdb/aarch64-linux-nat.c
@@ -388,19 +388,65 @@ store_fpregs_to_thread (const struct regcache *regcache)
     }
 }
 
+/* Fill GDB's register array with the sve register values
+   from the current thread.  */
+
+static void
+fetch_sveregs_from_thread (struct regcache *regcache)
+{
+  gdb_byte *base = aarch64_sve_get_sveregs (ptid_get_lwp (inferior_ptid));
+  aarch64_sve_regs_copy_to_regcache (regcache, base);
+  xfree (base);
+}
+
+/* Store to the current thread the valid sve register
+   values in the GDB's register array.  */
+
+static void
+store_sveregs_to_thread (struct regcache *regcache)
+{
+  gdb_byte *base;
+  int ret, tid;
+  struct iovec iovec;
+
+  tid = ptid_get_lwp (inferior_ptid);
+
+  /* Obtain a dump of SVE registers from ptrace.  */
+  base = aarch64_sve_get_sveregs (tid);
+
+  /* Overwrite with regcache state.  */
+  aarch64_sve_regs_copy_from_regcache (regcache, base);
+
+  /* Write back to the kernel.  */
+  iovec.iov_base = base;
+  iovec.iov_len = ((struct user_sve_header *) base)->size;
+  ret = ptrace (PTRACE_SETREGSET, tid, NT_ARM_SVE, &iovec);
+  xfree (base);
+
+  if (ret < 0)
+    perror_with_name (_("Unable to store sve registers"));
+}
+
 /* Implement the "fetch_registers" target_ops method.  */
 
 void
 aarch64_linux_nat_target::fetch_registers (struct regcache *regcache,
    int regno)
 {
+  struct gdbarch_tdep *tdep = gdbarch_tdep (regcache->arch ());
+
   if (regno == -1)
     {
       fetch_gregs_from_thread (regcache);
-      fetch_fpregs_from_thread (regcache);
+      if (tdep->has_sve ())
+ fetch_sveregs_from_thread (regcache);
+      else
+ fetch_fpregs_from_thread (regcache);
     }
   else if (regno < AARCH64_V0_REGNUM)
     fetch_gregs_from_thread (regcache);
+  else if (tdep->has_sve ())
+    fetch_sveregs_from_thread (regcache);
   else
     fetch_fpregs_from_thread (regcache);
 }
@@ -411,13 +457,20 @@ void
 aarch64_linux_nat_target::store_registers (struct regcache *regcache,
    int regno)
 {
+  struct gdbarch_tdep *tdep = gdbarch_tdep (regcache->arch ());
+
   if (regno == -1)
     {
       store_gregs_to_thread (regcache);
-      store_fpregs_to_thread (regcache);
+      if (tdep->has_sve ())
+ store_sveregs_to_thread (regcache);
+      else
+ store_fpregs_to_thread (regcache);
     }
   else if (regno < AARCH64_V0_REGNUM)
     store_gregs_to_thread (regcache);
+  else if (tdep->has_sve ())
+    store_sveregs_to_thread (regcache);
   else
     store_fpregs_to_thread (regcache);
 }
diff --git a/gdb/gdbserver/Makefile.in b/gdb/gdbserver/Makefile.in
index c377378809..22ff19a2d2 100644
--- a/gdb/gdbserver/Makefile.in
+++ b/gdb/gdbserver/Makefile.in
@@ -218,6 +218,7 @@ SFILES = \
  $(srcdir)/common/tdesc.c \
  $(srcdir)/common/vec.c \
  $(srcdir)/common/xml-utils.c \
+ $(srcdir)/nat/aarch64-sve-linux-ptrace.c \
  $(srcdir)/nat/linux-btrace.c \
  $(srcdir)/nat/linux-namespaces.c \
  $(srcdir)/nat/linux-osdata.c \
diff --git a/gdb/nat/aarch64-sve-linux-ptrace.c b/gdb/nat/aarch64-sve-linux-ptrace.c
index 9381786fda..84c7a41f40 100644
--- a/gdb/nat/aarch64-sve-linux-ptrace.c
+++ b/gdb/nat/aarch64-sve-linux-ptrace.c
@@ -25,6 +25,13 @@
 #include "aarch64-sve-linux-ptrace.h"
 #include "arch/aarch64.h"
 
+#ifndef GDBSERVER
+#include "defs.h"
+#endif
+#include "regcache.h"
+
+static bool vq_change_warned = false;
+
 /* Read VQ for the given tid using ptrace.  If SVE is not supported then zero
    is returned (on a system that supports SVE, then VQ cannot be zeo).  */
 
@@ -50,3 +57,259 @@ aarch64_sve_get_vq (int tid)
 
   return vq;
 }
+
+/* Read the current SVE register set using ptrace, allocating space as
+   required.  */
+
+gdb_byte *
+aarch64_sve_get_sveregs (int tid)
+{
+  int ret;
+  struct iovec iovec;
+  struct user_sve_header header;
+  long vq = aarch64_sve_get_vq (tid);
+
+  if (vq == 0)
+    perror_with_name (_("Unable to fetch sve register header"));
+
+  /* A ptrace call with NT_ARM_SVE will return a header followed by either a
+     dump of all the SVE and FP registers, or an fpsimd structure (identical to
+     the one returned by NT_FPREGSET) if the kernel has not yet executed any
+     SVE code.  Make sure we allocate enough space for a full SVE dump.  */
+
+  iovec.iov_len = SVE_PT_SIZE (vq, SVE_PT_REGS_SVE);
+  iovec.iov_base = xmalloc (iovec.iov_len);
+
+  ret = ptrace (PTRACE_GETREGSET, tid, NT_ARM_SVE, &iovec);
+  if (ret < 0)
+    perror_with_name (_("Unable to fetch sve registers"));
+
+  return (gdb_byte *) iovec.iov_base;
+}
+
+/* Put the registers from linux structure buf into regcache.  */
+
+void
+aarch64_sve_regs_copy_to_regcache (struct regcache *regcache, const void *buf)
+{
+  char *base = (char*) buf;
+  int i;
+  struct user_sve_header *header = (struct user_sve_header *) buf;
+  long vq, vg_regcache;
+
+  vq = sve_vq_from_vl (header->vl);
+
+  /* Sanity check the data in the header.  */
+  gdb_assert (sve_vl_valid (header->vl));
+  gdb_assert (SVE_PT_SIZE (vq, header->flags) == header->size);
+
+  regcache->raw_collect (AARCH64_SVE_VG_REGNUM, &vg_regcache);
+  if (vg_regcache == 0)
+    {
+      /* VG has not been set.  */
+      vg_regcache = sve_vg_from_vl (header->vl);
+      regcache->raw_supply (AARCH64_SVE_VG_REGNUM, &vg_regcache);
+    }
+  else if (vg_regcache != 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 (_("Vector length has changed (%ld to %d). "
+ "Vector registers may show incorrect data."),
+ vg_regcache, sve_vg_from_vl (header->vl));
+      vq_change_warned = true;
+    }
+
+  if (HAS_SVE_STATE (*header))
+    {
+      /* The register dump contains a set of SVE registers.  */
+
+      for (i = 0; i < AARCH64_SVE_Z_REGS_NUM; i++)
+ regcache->raw_supply (AARCH64_SVE_Z0_REGNUM + i,
+    base + SVE_PT_SVE_ZREG_OFFSET (vq, i));
+
+      for (i = 0; i < AARCH64_SVE_P_REGS_NUM; i++)
+ regcache->raw_supply (AARCH64_SVE_P0_REGNUM + i,
+    base + SVE_PT_SVE_PREG_OFFSET (vq, i));
+
+      regcache->raw_supply (AARCH64_SVE_FFR_REGNUM,
+  base + SVE_PT_SVE_FFR_OFFSET (vq));
+      regcache->raw_supply (AARCH64_FPSR_REGNUM,
+  base + SVE_PT_SVE_FPSR_OFFSET (vq));
+      regcache->raw_supply (AARCH64_FPCR_REGNUM,
+  base + SVE_PT_SVE_FPCR_OFFSET (vq));
+    }
+  else
+    {
+      /* There is no SVE state yet - the register dump contains a fpsimd
+ structure instead.  These registers still exist in the hardware, but
+ the kernel has not yet initialised them, and so they will be null.  */
+
+      char *zero_reg = (char *) alloca (SVE_PT_SVE_ZREG_SIZE (vq));
+      struct user_fpsimd_state *fpsimd
+ = (struct user_fpsimd_state *)(base + SVE_PT_FPSIMD_OFFSET);
+
+      /* Copy across the V registers from fpsimd structure to the Z registers,
+ ensuring the non overlapping state is set to null.  */
+
+      memset (zero_reg, 0, SVE_PT_SVE_ZREG_SIZE (vq));
+
+      for (i = 0; i <= AARCH64_SVE_Z_REGS_NUM; i++)
+ {
+  memcpy (zero_reg, &fpsimd->vregs[i], sizeof (__int128_t));
+  regcache->raw_supply (AARCH64_SVE_Z0_REGNUM + i, zero_reg);
+ }
+
+      regcache->raw_supply (AARCH64_FPSR_REGNUM, &fpsimd->fpsr);
+      regcache->raw_supply (AARCH64_FPCR_REGNUM, &fpsimd->fpcr);
+
+      /* Clear the SVE only registers.  */
+
+      for (i = 0; i <= AARCH64_SVE_P_REGS_NUM; i++)
+ regcache->raw_supply (AARCH64_SVE_P0_REGNUM + i, zero_reg);
+
+      regcache->raw_supply (AARCH64_SVE_FFR_REGNUM, zero_reg);
+    }
+}
+
+
+/* Put the registers from regcache into linux structure buf.  */
+
+void
+aarch64_sve_regs_copy_from_regcache (struct regcache *regcache, void *buf)
+{
+  struct user_sve_header *header = (struct user_sve_header *) buf;
+  char *base = (char*) buf;
+  long vq, vg_regcache;
+  int i;
+
+  vq = sve_vq_from_vl (header->vl);
+
+  /* Sanity check the data in the header.  */
+  gdb_assert (sve_vl_valid (header->vl));
+  gdb_assert (SVE_PT_SIZE (vq, header->flags) == header->size);
+
+  regcache->raw_collect (AARCH64_SVE_VG_REGNUM, &vg_regcache);
+  if (vg_regcache != 0 && vg_regcache != 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 (_("Vector length has changed. Cannot write back registers."));
+
+  if (!HAS_SVE_STATE (*header))
+    {
+      /* There is no SVE state yet - the register dump contains a fpsimd
+ structure instead.  Where possible we want to write the regcache data
+ back to the kernel using the fpsimd structure.  However, if we cannot
+ then we'll need to reformat the fpsimd into a full SVE structure,
+ resulting in the initialization of SVE state written back to the
+ kernel, which is why we try to avoid it.  */
+
+      int has_sve_state = 0;
+      char *zero_reg = (char *) alloca (SVE_PT_SVE_ZREG_SIZE (vq));
+      struct user_fpsimd_state *fpsimd
+ = (struct user_fpsimd_state *)(base + SVE_PT_FPSIMD_OFFSET);
+
+      memset (zero_reg, 0, SVE_PT_SVE_ZREG_SIZE (vq));
+
+      /* Check in the regcache if any of the Z registers are set after the
+ first 128 bits, or if any of the other SVE registers are set.  */
+
+      for (i = 0; i < AARCH64_SVE_Z_REGS_NUM; i++)
+ {
+  has_sve_state |= regcache->raw_compare (AARCH64_SVE_Z0_REGNUM + i,
+ zero_reg, sizeof (__int128_t));
+  if (has_sve_state != 0)
+    break;
+ }
+
+      if (has_sve_state == 0)
+ for (i = 0; i < AARCH64_SVE_P_REGS_NUM; i++)
+  {
+    has_sve_state |= regcache->raw_compare (AARCH64_SVE_P0_REGNUM + i,
+  zero_reg, 0);
+    if (has_sve_state != 0)
+      break;
+  }
+
+      if (has_sve_state == 0)
+  has_sve_state |= regcache->raw_compare (AARCH64_SVE_FFR_REGNUM,
+ zero_reg, 0);
+
+      /* If no SVE state exists, then use the existing fpsimd structure to
+ write out state and return.  */
+
+      if (has_sve_state == 0)
+ {
+  /* The collects of the Z registers will overflow the size of a vreg.
+     There is enough space in the structure to allow for this, but we
+     cannot overflow into the next register as we might not be
+     collecting every register.  */
+
+  for (i = 0; i < AARCH64_SVE_Z_REGS_NUM; i++)
+    {
+      if (REG_VALID == regcache->get_register_status (
+   AARCH64_SVE_Z0_REGNUM + i))
+ {
+  regcache->raw_collect (AARCH64_SVE_Z0_REGNUM + i, zero_reg);
+  memcpy (&fpsimd->vregs[i], zero_reg, sizeof (__int128_t));
+ }
+    }
+
+  if (REG_VALID == regcache->get_register_status (AARCH64_FPSR_REGNUM))
+    regcache->raw_collect (AARCH64_FPSR_REGNUM, &fpsimd->fpsr);
+  if (REG_VALID == regcache->get_register_status (AARCH64_FPCR_REGNUM))
+    regcache->raw_collect ( AARCH64_FPCR_REGNUM, &fpsimd->fpcr);
+
+  return;
+ }
+
+      /* Otherwise, reformat the fpsimd structure into a full SVE set, by
+ expanding the V registers (working backwards so we don't splat
+ registers before they are copied) and using null for everything else.
+ Note that enough space for a full SVE dump was originally allocated
+ for base.  */
+
+      header->flags |= SVE_PT_REGS_SVE;
+      header->size = SVE_PT_SIZE (vq, SVE_PT_REGS_SVE);
+
+      memcpy (base + SVE_PT_SVE_FPSR_OFFSET (vq), &fpsimd->fpsr,
+      sizeof (uint32_t));
+      memcpy (base + SVE_PT_SVE_FPCR_OFFSET (vq), &fpsimd->fpcr,
+      sizeof (uint32_t));
+
+      for (i = AARCH64_SVE_Z_REGS_NUM; i >= 0 ; i--)
+ {
+  memcpy (base + SVE_PT_SVE_ZREG_OFFSET (vq, i), &fpsimd->vregs[i],
+  sizeof (__int128_t));
+ }
+    }
+
+  /* Replace the kernel values with those from regcache.  */
+
+  for (i = 0; i < AARCH64_SVE_Z_REGS_NUM; i++)
+    if (REG_VALID == regcache->get_register_status (AARCH64_SVE_Z0_REGNUM + i))
+      regcache->raw_collect (AARCH64_SVE_Z0_REGNUM + i,
+   base + SVE_PT_SVE_ZREG_OFFSET (vq, i));
+
+  for (i = 0; i < AARCH64_SVE_P_REGS_NUM; i++)
+    if (REG_VALID == regcache->get_register_status (AARCH64_SVE_P0_REGNUM + i))
+      regcache->raw_collect (AARCH64_SVE_P0_REGNUM + i,
+   base + SVE_PT_SVE_PREG_OFFSET (vq, i));
+
+  if (REG_VALID == regcache->get_register_status (AARCH64_SVE_FFR_REGNUM))
+    regcache->raw_collect (AARCH64_SVE_FFR_REGNUM,
+ base + SVE_PT_SVE_FFR_OFFSET (vq));
+  if (REG_VALID == regcache->get_register_status (AARCH64_FPSR_REGNUM))
+    regcache->raw_collect (AARCH64_FPSR_REGNUM,
+ base + SVE_PT_SVE_FPSR_OFFSET (vq));
+  if (REG_VALID == regcache->get_register_status (AARCH64_FPCR_REGNUM))
+    regcache->raw_collect (AARCH64_FPCR_REGNUM,
+ base + SVE_PT_SVE_FPCR_OFFSET (vq));
+}
diff --git a/gdb/nat/aarch64-sve-linux-ptrace.h b/gdb/nat/aarch64-sve-linux-ptrace.h
index b318150ac1..eff90ea2ea 100644
--- a/gdb/nat/aarch64-sve-linux-ptrace.h
+++ b/gdb/nat/aarch64-sve-linux-ptrace.h
@@ -34,10 +34,31 @@
 
 extern unsigned long aarch64_sve_get_vq (int tid);
 
+/* Read the current SVE register set using ptrace, allocating space as
+   required.  */
+
+extern gdb_byte *aarch64_sve_get_sveregs (int tid);
+
+/* Put the registers from linux structure buf into regcache.  */
+
+extern void aarch64_sve_regs_copy_to_regcache (struct regcache *regcache,
+       const void *buf);
+
+/* Put the registers from regcache into linux structure buf.  */
+
+extern void aarch64_sve_regs_copy_from_regcache (struct regcache *regcache,
+ void *buf);
+
 /* Structures and defines taken from sigcontext.h.  */
 
 #ifndef SVE_SIG_ZREGS_SIZE
 
+struct sve_context {
+   struct _aarch64_ctx head;
+   __u16 vl;
+   __u16 __reserved[3];
+};
+
 #define SVE_VQ_BYTES 16 /* number of bytes per quadword */
 
 #define SVE_VQ_MIN 1
@@ -52,6 +73,35 @@ extern unsigned long aarch64_sve_get_vq (int tid);
 #define sve_vl_valid(vl) \
  ((vl) % SVE_VQ_BYTES == 0 && (vl) >= SVE_VL_MIN && (vl) <= SVE_VL_MAX)
 
+#define SVE_SIG_ZREG_SIZE(vq) ((__u32)(vq) * SVE_VQ_BYTES)
+#define SVE_SIG_PREG_SIZE(vq) ((__u32)(vq) * (SVE_VQ_BYTES / 8))
+#define SVE_SIG_FFR_SIZE(vq)  SVE_SIG_PREG_SIZE(vq)
+
+#define SVE_SIG_REGS_OFFSET               \
+   ((sizeof(struct sve_context) + (SVE_VQ_BYTES - 1)) \
+      / SVE_VQ_BYTES * SVE_VQ_BYTES)
+
+#define SVE_SIG_ZREGS_OFFSET  SVE_SIG_REGS_OFFSET
+#define SVE_SIG_ZREG_OFFSET(vq, n) \
+   (SVE_SIG_ZREGS_OFFSET + SVE_SIG_ZREG_SIZE(vq) * (n))
+#define SVE_SIG_ZREGS_SIZE(vq) \
+   (SVE_SIG_ZREG_OFFSET(vq, SVE_NUM_ZREGS) - SVE_SIG_ZREGS_OFFSET)
+
+#define SVE_SIG_PREGS_OFFSET(vq) \
+   (SVE_SIG_ZREGS_OFFSET + SVE_SIG_ZREGS_SIZE(vq))
+#define SVE_SIG_PREG_OFFSET(vq, n) \
+   (SVE_SIG_PREGS_OFFSET(vq) + SVE_SIG_PREG_SIZE(vq) * (n))
+#define SVE_SIG_PREGS_SIZE(vq) \
+   (SVE_SIG_PREG_OFFSET(vq, SVE_NUM_PREGS) - SVE_SIG_PREGS_OFFSET(vq))
+
+#define SVE_SIG_FFR_OFFSET(vq) \
+   (SVE_SIG_PREGS_OFFSET(vq) + SVE_SIG_PREGS_SIZE(vq))
+
+#define SVE_SIG_REGS_SIZE(vq) \
+   (SVE_SIG_FFR_OFFSET(vq) + SVE_SIG_FFR_SIZE(vq) - SVE_SIG_REGS_OFFSET)
+
+#define SVE_SIG_CONTEXT_SIZE(vq) (SVE_SIG_REGS_OFFSET + SVE_SIG_REGS_SIZE(vq))
+
 #endif /* SVE_SIG_ZREGS_SIZE.  */
 
 
@@ -68,6 +118,76 @@ struct user_sve_header {
  __u16 __reserved;
 };
 
+
+#define SVE_PT_REGS_MASK      1
+
+#define SVE_PT_REGS_FPSIMD    0
+#define SVE_PT_REGS_SVE       SVE_PT_REGS_MASK
+
+#define SVE_PT_VL_INHERIT     (PR_SVE_VL_INHERIT >> 16)
+#define SVE_PT_VL_ONEXEC      (PR_SVE_SET_VL_ONEXEC >> 16)
+
+#define SVE_PT_REGS_OFFSET             \
+   ((sizeof(struct sve_context) + (SVE_VQ_BYTES - 1)) \
+      / SVE_VQ_BYTES * SVE_VQ_BYTES)
+
+#define SVE_PT_FPSIMD_OFFSET     SVE_PT_REGS_OFFSET
+
+#define SVE_PT_FPSIMD_SIZE(vq, flags)  (sizeof(struct user_fpsimd_state))
+
+#define SVE_PT_SVE_ZREG_SIZE(vq) SVE_SIG_ZREG_SIZE(vq)
+#define SVE_PT_SVE_PREG_SIZE(vq) SVE_SIG_PREG_SIZE(vq)
+#define SVE_PT_SVE_FFR_SIZE(vq)     SVE_SIG_FFR_SIZE(vq)
+#define SVE_PT_SVE_FPSR_SIZE     sizeof(__u32)
+#define SVE_PT_SVE_FPCR_SIZE     sizeof(__u32)
+
+#define __SVE_SIG_TO_PT(offset) \
+   ((offset) - SVE_SIG_REGS_OFFSET + SVE_PT_REGS_OFFSET)
+
+#define SVE_PT_SVE_OFFSET     SVE_PT_REGS_OFFSET
+
+#define SVE_PT_SVE_ZREGS_OFFSET \
+   __SVE_SIG_TO_PT(SVE_SIG_ZREGS_OFFSET)
+#define SVE_PT_SVE_ZREG_OFFSET(vq, n) \
+   __SVE_SIG_TO_PT(SVE_SIG_ZREG_OFFSET(vq, n))
+#define SVE_PT_SVE_ZREGS_SIZE(vq) \
+   (SVE_PT_SVE_ZREG_OFFSET(vq, SVE_NUM_ZREGS) - SVE_PT_SVE_ZREGS_OFFSET)
+
+#define SVE_PT_SVE_PREGS_OFFSET(vq) \
+   __SVE_SIG_TO_PT(SVE_SIG_PREGS_OFFSET(vq))
+#define SVE_PT_SVE_PREG_OFFSET(vq, n) \
+   __SVE_SIG_TO_PT(SVE_SIG_PREG_OFFSET(vq, n))
+#define SVE_PT_SVE_PREGS_SIZE(vq) \
+   (SVE_PT_SVE_PREG_OFFSET(vq, SVE_NUM_PREGS) - \
+      SVE_PT_SVE_PREGS_OFFSET(vq))
+
+#define SVE_PT_SVE_FFR_OFFSET(vq) \
+   __SVE_SIG_TO_PT(SVE_SIG_FFR_OFFSET(vq))
+
+#define SVE_PT_SVE_FPSR_OFFSET(vq)           \
+   ((SVE_PT_SVE_FFR_OFFSET(vq) + SVE_PT_SVE_FFR_SIZE(vq) +  \
+         (SVE_VQ_BYTES - 1))        \
+      / SVE_VQ_BYTES * SVE_VQ_BYTES)
+#define SVE_PT_SVE_FPCR_OFFSET(vq) \
+   (SVE_PT_SVE_FPSR_OFFSET(vq) + SVE_PT_SVE_FPSR_SIZE)
+
+#define SVE_PT_SVE_SIZE(vq, flags)              \
+   ((SVE_PT_SVE_FPCR_OFFSET(vq) + SVE_PT_SVE_FPCR_SIZE      \
+         - SVE_PT_SVE_OFFSET + (SVE_VQ_BYTES - 1)) \
+      / SVE_VQ_BYTES * SVE_VQ_BYTES)
+
+#define SVE_PT_SIZE(vq, flags)                  \
+    (((flags) & SVE_PT_REGS_MASK) == SVE_PT_REGS_SVE ?      \
+        SVE_PT_SVE_OFFSET + SVE_PT_SVE_SIZE(vq, flags)   \
+      : SVE_PT_FPSIMD_OFFSET + SVE_PT_FPSIMD_SIZE(vq, flags))
+
 #endif /* SVE_PT_SVE_ZREG_SIZE.  */
 
+
+/* Indicates whether a SVE ptrace header is followed by SVE registers or a
+   fpsimd structure.  */
+
+#define HAS_SVE_STATE(header) ((header).flags && SVE_PT_REGS_SVE)
+
+
 #endif /* aarch64-sve-linux-ptrace.h */
--
2.15.1 (Apple Git-101)

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 1/8] Add Aarch64 SVE target description

Eli Zaretskii
In reply to this post by Alan Hayward
> From: Alan Hayward <[hidden email]>
> Cc: [hidden email], Alan Hayward <[hidden email]>
> Date: Fri, 11 May 2018 11:52:49 +0100
>
> * doc/gdb.texinfo: Describe SVE feature

OK for this part.

Thanks.
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 1/8] Add Aarch64 SVE target description

Alan Hayward

> On 11 May 2018, at 13:12, Eli Zaretskii <[hidden email]> wrote:
>
>> From: Alan Hayward <[hidden email]>
>> Cc: [hidden email], Alan Hayward <[hidden email]>
>> Date: Fri, 11 May 2018 11:52:49 +0100
>>
>> * doc/gdb.texinfo: Describe SVE feature
>
> OK for this part.
>

Thanks for the quick review!

Alan.

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 0/8] Add SVE support for Aarch64 GDB

Alan Hayward
In reply to this post by Alan Hayward
Ping.


Thanks,
Alan.

> On 11 May 2018, at 11:52, Alan Hayward <[hidden email]> wrote:
>
> This set of patches adds gdb support for SVE on AArch64.
>
> SVE is the new vector extension for Aarch64. SVE is different in that the
> length of a vector register is not fixed - it is can vary for different
> hardware implementors. All code compiled for SVE is vector length agnostic.
> The Linux kernel can then further restrict the vector length up to the
> supported maximum for the hardware. Potentially different process and
> threads can have different vector lengths, and they could change at any
> time. However, in practice, we expect the vector length to remain constant
> across the lifetime of a process.
> This set of patches assumes the vector length will not change and either
> warns once (when reading registers) or errors (when writing registers) if the
> vector length has changed. A future set of patches will offer full support.
>
> This series does not support gdbserver. However, where it makes sense I have
> commonised as much code as possible and have added the groundwork in
> gdbserver. Enabling gdbserver should be a small set of additional patches.
>
> Core files and watchpoints are not yet supported.
>
> The vector length is read from ptrace and is required to create the target
> description for the running process. There is no hardcoded SVE XML.
>
> The patches require recent linux header files for all the SVE ptrace macros.
> To allow builds for older headers, I've added all required macros and
> structures within ifdef checks. I'm not sure if this is the ideal solution.
> When including kernel header code, I've not performed any reformatting (ie
> they are not in GNU style).
>
> One of the later patches adds functions to gdbserver regcache so that I can
> add common functionality that works on gdb and gdbserver.
>
> Given there are no working SVE systems available today, this was manually
> tested on an Aarch64 SVE emulator running Ubuntu. In addition I've run make
> check on X86 and Aarch64 builds for both unix and native-gdbserver.
>
>
> Alan.
>
> Alan Hayward (8):
>  Add Aarch64 SVE target description
>  Function for reading the Aarch64 SVE vector length.
>  Add SVE register defines
>  Enable SVE for GDB
>  Add aarch64 psuedo help functions
>  Aarch64 SVE pseudo register support
>  Add methods to gdbserver regcache and raw_compare
>  Ptrace support for Aarch64 SVE
>
> gdb/Makefile.in                     |   1 +
> gdb/aarch64-linux-nat.c             |  60 +++++-
> gdb/aarch64-linux-tdep.c            |   5 +-
> gdb/aarch64-tdep.c                  | 402 +++++++++++++++++++++++-------------
> gdb/aarch64-tdep.h                  |  12 +-
> gdb/arch/aarch64.c                  |  12 +-
> gdb/arch/aarch64.h                  |  37 +++-
> gdb/configure.nat                   |   2 +-
> gdb/doc/gdb.texinfo                 |   4 +
> gdb/features/aarch64-sve.c          | 158 ++++++++++++++
> gdb/gdbserver/Makefile.in           |   1 +
> gdb/gdbserver/configure.srv         |   1 +
> gdb/gdbserver/linux-aarch64-tdesc.c |   3 +-
> gdb/gdbserver/regcache.c            |  55 ++++-
> gdb/gdbserver/regcache.h            |   8 +
> gdb/nat/aarch64-sve-linux-ptrace.c  | 315 ++++++++++++++++++++++++++++
> gdb/nat/aarch64-sve-linux-ptrace.h  | 193 +++++++++++++++++
> gdb/regcache.c                      |  17 ++
> gdb/regcache.h                      |   2 +
> 19 files changed, 1122 insertions(+), 166 deletions(-)
> create mode 100644 gdb/features/aarch64-sve.c
> create mode 100644 gdb/nat/aarch64-sve-linux-ptrace.c
> create mode 100644 gdb/nat/aarch64-sve-linux-ptrace.h
>
> --
> 2.15.1 (Apple Git-101)
>

Reply | Threaded
Open this post in threaded view
|

[PING 2][PATCH 0/8] Add SVE support for Aarch64 GDB

Alan Hayward
Ping ping.

Thanks,
Alan.

> On 22 May 2018, at 09:34, Alan Hayward <[hidden email]> wrote:
>
> Ping.
>
>
> Thanks,
> Alan.
>
>> On 11 May 2018, at 11:52, Alan Hayward <[hidden email]> wrote:
>>
>> This set of patches adds gdb support for SVE on AArch64.
>>
>> SVE is the new vector extension for Aarch64. SVE is different in that the
>> length of a vector register is not fixed - it is can vary for different
>> hardware implementors. All code compiled for SVE is vector length agnostic.
>> The Linux kernel can then further restrict the vector length up to the
>> supported maximum for the hardware. Potentially different process and
>> threads can have different vector lengths, and they could change at any
>> time. However, in practice, we expect the vector length to remain constant
>> across the lifetime of a process.
>> This set of patches assumes the vector length will not change and either
>> warns once (when reading registers) or errors (when writing registers) if the
>> vector length has changed. A future set of patches will offer full support.
>>
>> This series does not support gdbserver. However, where it makes sense I have
>> commonised as much code as possible and have added the groundwork in
>> gdbserver. Enabling gdbserver should be a small set of additional patches.
>>
>> Core files and watchpoints are not yet supported.
>>
>> The vector length is read from ptrace and is required to create the target
>> description for the running process. There is no hardcoded SVE XML.
>>
>> The patches require recent linux header files for all the SVE ptrace macros.
>> To allow builds for older headers, I've added all required macros and
>> structures within ifdef checks. I'm not sure if this is the ideal solution.
>> When including kernel header code, I've not performed any reformatting (ie
>> they are not in GNU style).
>>
>> One of the later patches adds functions to gdbserver regcache so that I can
>> add common functionality that works on gdb and gdbserver.
>>
>> Given there are no working SVE systems available today, this was manually
>> tested on an Aarch64 SVE emulator running Ubuntu. In addition I've run make
>> check on X86 and Aarch64 builds for both unix and native-gdbserver.
>>
>>
>> Alan.
>>
>> Alan Hayward (8):
>> Add Aarch64 SVE target description
>> Function for reading the Aarch64 SVE vector length.
>> Add SVE register defines
>> Enable SVE for GDB
>> Add aarch64 psuedo help functions
>> Aarch64 SVE pseudo register support
>> Add methods to gdbserver regcache and raw_compare
>> Ptrace support for Aarch64 SVE
>>
>> gdb/Makefile.in                     |   1 +
>> gdb/aarch64-linux-nat.c             |  60 +++++-
>> gdb/aarch64-linux-tdep.c            |   5 +-
>> gdb/aarch64-tdep.c                  | 402 +++++++++++++++++++++++-------------
>> gdb/aarch64-tdep.h                  |  12 +-
>> gdb/arch/aarch64.c                  |  12 +-
>> gdb/arch/aarch64.h                  |  37 +++-
>> gdb/configure.nat                   |   2 +-
>> gdb/doc/gdb.texinfo                 |   4 +
>> gdb/features/aarch64-sve.c          | 158 ++++++++++++++
>> gdb/gdbserver/Makefile.in           |   1 +
>> gdb/gdbserver/configure.srv         |   1 +
>> gdb/gdbserver/linux-aarch64-tdesc.c |   3 +-
>> gdb/gdbserver/regcache.c            |  55 ++++-
>> gdb/gdbserver/regcache.h            |   8 +
>> gdb/nat/aarch64-sve-linux-ptrace.c  | 315 ++++++++++++++++++++++++++++
>> gdb/nat/aarch64-sve-linux-ptrace.h  | 193 +++++++++++++++++
>> gdb/regcache.c                      |  17 ++
>> gdb/regcache.h                      |   2 +
>> 19 files changed, 1122 insertions(+), 166 deletions(-)
>> create mode 100644 gdb/features/aarch64-sve.c
>> create mode 100644 gdb/nat/aarch64-sve-linux-ptrace.c
>> create mode 100644 gdb/nat/aarch64-sve-linux-ptrace.h
>>
>> --
>> 2.15.1 (Apple Git-101)
>>
>

Reply | Threaded
Open this post in threaded view
|

Re: [PING 2][PATCH 0/8] Add SVE support for Aarch64 GDB

omjavaid
Hi Alan,

I would like to help you review and get these patches accepted upstream.

https://developer.arm.com/docs/ddi0584/latest/arm-architecture-reference-manual-supplement-the-scalable-vector-extension-sve-for-armv8-a

Do you think above link has the latest documents on SVE?

Also can we test these patches with QEMU or any other virtualization model?

What is the status of corresponding ptrace changes in kernel? Are they
already accepted which kernel versions will have that code?

I will go forward the say more on your patches in coming days.

Thanks!

On 29 May 2018 at 13:59, Alan Hayward <[hidden email]> wrote:

> Ping ping.
>
> Thanks,
> Alan.
>
>> On 22 May 2018, at 09:34, Alan Hayward <[hidden email]> wrote:
>>
>> Ping.
>>
>>
>> Thanks,
>> Alan.
>>
>>> On 11 May 2018, at 11:52, Alan Hayward <[hidden email]> wrote:
>>>
>>> This set of patches adds gdb support for SVE on AArch64.
>>>
>>> SVE is the new vector extension for Aarch64. SVE is different in that the
>>> length of a vector register is not fixed - it is can vary for different
>>> hardware implementors. All code compiled for SVE is vector length agnostic.
>>> The Linux kernel can then further restrict the vector length up to the
>>> supported maximum for the hardware. Potentially different process and
>>> threads can have different vector lengths, and they could change at any
>>> time. However, in practice, we expect the vector length to remain constant
>>> across the lifetime of a process.
>>> This set of patches assumes the vector length will not change and either
>>> warns once (when reading registers) or errors (when writing registers) if the
>>> vector length has changed. A future set of patches will offer full support.
>>>
>>> This series does not support gdbserver. However, where it makes sense I have
>>> commonised as much code as possible and have added the groundwork in
>>> gdbserver. Enabling gdbserver should be a small set of additional patches.
>>>
>>> Core files and watchpoints are not yet supported.
>>>
>>> The vector length is read from ptrace and is required to create the target
>>> description for the running process. There is no hardcoded SVE XML.
>>>
>>> The patches require recent linux header files for all the SVE ptrace macros.
>>> To allow builds for older headers, I've added all required macros and
>>> structures within ifdef checks. I'm not sure if this is the ideal solution.
>>> When including kernel header code, I've not performed any reformatting (ie
>>> they are not in GNU style).
>>>
>>> One of the later patches adds functions to gdbserver regcache so that I can
>>> add common functionality that works on gdb and gdbserver.
>>>
>>> Given there are no working SVE systems available today, this was manually
>>> tested on an Aarch64 SVE emulator running Ubuntu. In addition I've run make
>>> check on X86 and Aarch64 builds for both unix and native-gdbserver.
>>>
>>>
>>> Alan.
>>>
>>> Alan Hayward (8):
>>> Add Aarch64 SVE target description
>>> Function for reading the Aarch64 SVE vector length.
>>> Add SVE register defines
>>> Enable SVE for GDB
>>> Add aarch64 psuedo help functions
>>> Aarch64 SVE pseudo register support
>>> Add methods to gdbserver regcache and raw_compare
>>> Ptrace support for Aarch64 SVE
>>>
>>> gdb/Makefile.in                     |   1 +
>>> gdb/aarch64-linux-nat.c             |  60 +++++-
>>> gdb/aarch64-linux-tdep.c            |   5 +-
>>> gdb/aarch64-tdep.c                  | 402 +++++++++++++++++++++++-------------
>>> gdb/aarch64-tdep.h                  |  12 +-
>>> gdb/arch/aarch64.c                  |  12 +-
>>> gdb/arch/aarch64.h                  |  37 +++-
>>> gdb/configure.nat                   |   2 +-
>>> gdb/doc/gdb.texinfo                 |   4 +
>>> gdb/features/aarch64-sve.c          | 158 ++++++++++++++
>>> gdb/gdbserver/Makefile.in           |   1 +
>>> gdb/gdbserver/configure.srv         |   1 +
>>> gdb/gdbserver/linux-aarch64-tdesc.c |   3 +-
>>> gdb/gdbserver/regcache.c            |  55 ++++-
>>> gdb/gdbserver/regcache.h            |   8 +
>>> gdb/nat/aarch64-sve-linux-ptrace.c  | 315 ++++++++++++++++++++++++++++
>>> gdb/nat/aarch64-sve-linux-ptrace.h  | 193 +++++++++++++++++
>>> gdb/regcache.c                      |  17 ++
>>> gdb/regcache.h                      |   2 +
>>> 19 files changed, 1122 insertions(+), 166 deletions(-)
>>> create mode 100644 gdb/features/aarch64-sve.c
>>> create mode 100644 gdb/nat/aarch64-sve-linux-ptrace.c
>>> create mode 100644 gdb/nat/aarch64-sve-linux-ptrace.h
>>>
>>> --
>>> 2.15.1 (Apple Git-101)
>>>
>>
>
Reply | Threaded
Open this post in threaded view
|

Re: [PING 2][PATCH 0/8] Add SVE support for Aarch64 GDB

Alan Hayward


> On 29 May 2018, at 14:06, Omair Javaid <[hidden email]> wrote:
>
> Hi Alan,
>
> I would like to help you review and get these patches accepted upstream.
>

Thanks!


> https://developer.arm.com/docs/ddi0584/latest/arm-architecture-reference-manual-supplement-the-scalable-vector-extension-sve-for-armv8-a
>
> Do you think above link has the latest documents on SVE?


Yes, that’ll give you the latest docs for SVE.

At the last GNU cauldron I gave a summary of SVE, including the
latest status on Linux, and the changes required for gdb:
https://slideslive.com/38902367/supporting-variable-register-sizes-for-the-arm-scalable-vector-extension-sve-in-gdb
The regcache slide is out of date - it’s an early plan for
detached_regcache/readable_regcache etc. Not sure if the video
is ok, as I dare not watch it :)

Also, there is also a nice delve into some simple programming
using SVE here:
http://developer.arm.com/hpc/a-sneak-peek-into-sve-and-vla-programming

Binutils fully supports SVE already.

(I should have put all this in the patch summary).

>
> Also can we test these patches with QEMU or any other virtualization model?

SVE support for QEMU is being worked on by Linaro. The changelog for
the latest release (2.12) says preliminary SVE support. I can look into
this a little more if you want - but I suspect it won’t be good enough yet.
(You’ll need to emulate a full linux kernel - otherwise qemu will fail
when it hits ptrace).

I’ve been using Arm’s Fast Models, but that’s a paid for product and sadly
not something I can easily give you access to.

What happens with other ports to obscure architectures for which most people
/ reviewers don't have access too?
At a minimum, the existing buildbot setup will confirm that nothing is
broken for normal aarch64.

>
> What is the status of corresponding ptrace changes in kernel? Are they
> already accepted which kernel versions will have that code?

SVE support went into Linux 4.15 in a single batch patches.

Easiest way to check is to look for NT_ARM_SVE in elf.h:
https://github.com/torvalds/linux/blob/v4.15/include/uapi/linux/elf.h


>
> I will go forward the say more on your patches in coming days.

Note, because this patch set doesn’t support gdbserver, it makes the last
two patches a little strange. I now have gdbserver support working too
(it’s just a few small changes). When I get to V2 of the set, I’ll
probably add in the gdbserver changes.


Thanks,
Alan.


>
> Thanks!
>
> On 29 May 2018 at 13:59, Alan Hayward <[hidden email]> wrote:
>> Ping ping.
>>
>> Thanks,
>> Alan.
>>
>>> On 22 May 2018, at 09:34, Alan Hayward <[hidden email]> wrote:
>>>
>>> Ping.
>>>
>>>
>>> Thanks,
>>> Alan.
>>>
>>>> On 11 May 2018, at 11:52, Alan Hayward <[hidden email]> wrote:
>>>>
>>>> This set of patches adds gdb support for SVE on AArch64.
>>>>
>>>> SVE is the new vector extension for Aarch64. SVE is different in that the
>>>> length of a vector register is not fixed - it is can vary for different
>>>> hardware implementors. All code compiled for SVE is vector length agnostic.
>>>> The Linux kernel can then further restrict the vector length up to the
>>>> supported maximum for the hardware. Potentially different process and
>>>> threads can have different vector lengths, and they could change at any
>>>> time. However, in practice, we expect the vector length to remain constant
>>>> across the lifetime of a process.
>>>> This set of patches assumes the vector length will not change and either
>>>> warns once (when reading registers) or errors (when writing registers) if the
>>>> vector length has changed. A future set of patches will offer full support.
>>>>
>>>> This series does not support gdbserver. However, where it makes sense I have
>>>> commonised as much code as possible and have added the groundwork in
>>>> gdbserver. Enabling gdbserver should be a small set of additional patches.
>>>>
>>>> Core files and watchpoints are not yet supported.
>>>>
>>>> The vector length is read from ptrace and is required to create the target
>>>> description for the running process. There is no hardcoded SVE XML.
>>>>
>>>> The patches require recent linux header files for all the SVE ptrace macros.
>>>> To allow builds for older headers, I've added all required macros and
>>>> structures within ifdef checks. I'm not sure if this is the ideal solution.
>>>> When including kernel header code, I've not performed any reformatting (ie
>>>> they are not in GNU style).
>>>>
>>>> One of the later patches adds functions to gdbserver regcache so that I can
>>>> add common functionality that works on gdb and gdbserver.
>>>>
>>>> Given there are no working SVE systems available today, this was manually
>>>> tested on an Aarch64 SVE emulator running Ubuntu. In addition I've run make
>>>> check on X86 and Aarch64 builds for both unix and native-gdbserver.
>>>>
>>>>
>>>> Alan.
>>>>
>>>> Alan Hayward (8):
>>>> Add Aarch64 SVE target description
>>>> Function for reading the Aarch64 SVE vector length.
>>>> Add SVE register defines
>>>> Enable SVE for GDB
>>>> Add aarch64 psuedo help functions
>>>> Aarch64 SVE pseudo register support
>>>> Add methods to gdbserver regcache and raw_compare
>>>> Ptrace support for Aarch64 SVE
>>>>
>>>> gdb/Makefile.in                     |   1 +
>>>> gdb/aarch64-linux-nat.c             |  60 +++++-
>>>> gdb/aarch64-linux-tdep.c            |   5 +-
>>>> gdb/aarch64-tdep.c                  | 402 +++++++++++++++++++++++-------------
>>>> gdb/aarch64-tdep.h                  |  12 +-
>>>> gdb/arch/aarch64.c                  |  12 +-
>>>> gdb/arch/aarch64.h                  |  37 +++-
>>>> gdb/configure.nat                   |   2 +-
>>>> gdb/doc/gdb.texinfo                 |   4 +
>>>> gdb/features/aarch64-sve.c          | 158 ++++++++++++++
>>>> gdb/gdbserver/Makefile.in           |   1 +
>>>> gdb/gdbserver/configure.srv         |   1 +
>>>> gdb/gdbserver/linux-aarch64-tdesc.c |   3 +-
>>>> gdb/gdbserver/regcache.c            |  55 ++++-
>>>> gdb/gdbserver/regcache.h            |   8 +
>>>> gdb/nat/aarch64-sve-linux-ptrace.c  | 315 ++++++++++++++++++++++++++++
>>>> gdb/nat/aarch64-sve-linux-ptrace.h  | 193 +++++++++++++++++
>>>> gdb/regcache.c                      |  17 ++
>>>> gdb/regcache.h                      |   2 +
>>>> 19 files changed, 1122 insertions(+), 166 deletions(-)
>>>> create mode 100644 gdb/features/aarch64-sve.c
>>>> create mode 100644 gdb/nat/aarch64-sve-linux-ptrace.c
>>>> create mode 100644 gdb/nat/aarch64-sve-linux-ptrace.h
>>>>
>>>> --
>>>> 2.15.1 (Apple Git-101)
>>>>
>>>
>>

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 1/8] Add Aarch64 SVE target description

Simon Marchi-2
In reply to this post by Alan Hayward
On 2018-05-11 06:52 AM, Alan Hayward wrote:

> This patch adds the SVE target description. However, no code will
> yet use it - that comes in the later patches.
>
> The create_feature_aarch64_sve function is not generated from XML.
> This is because we need to know the sve vector size (VQ) in order
> to size the registers correctly.
>
> A VQ of 0 is used when the hardware does not support SVE.
> (SVE hardware will always have a valid vector size). I considered
> using a bool to indicate SVE in addition to the VQ. Whilst this
> may be slightly more readable initially, I think it's a little
> odd to have two variables, eg:
> aarch64_create_target_description (bool sve_supported, long vq)
>
> Alan.

Hi Alan,

This patch LGTM, I just noted some nits.

>  /* Initialize the current architecture based on INFO.  If possible,
> @@ -2864,7 +2875,8 @@ aarch64_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
>  
>    /* Ensure we always have a target descriptor.  */
>    if (!tdesc_has_registers (tdesc))
> -    tdesc = aarch64_read_description ();
> +    /* SVE is not yet supported.  */
> +    tdesc = aarch64_read_description (0);

When there there is a comment above the single statement branch, braces become required:

  if (!tdesc_has_registers (tdesc))
    {
      /* SVE is not yet supported.  */
      tdesc = aarch64_read_description (0);
    }

> diff --git a/gdb/arch/aarch64.c b/gdb/arch/aarch64.c
> index b85e460b6b..d1ec5cedf8 100644
> --- a/gdb/arch/aarch64.c
> +++ b/gdb/arch/aarch64.c
> @@ -21,11 +21,13 @@
>  
>  #include "../features/aarch64-core.c"
>  #include "../features/aarch64-fpu.c"
> +#include "../features/aarch64-sve.c"
>  
> -/* Create the aarch64 target description.  */
> +/* Create the aarch64 target description.  A non zero VQ value indicates both
> +   the presence of SVE and the SVE vector quotient.  */

What does "SVE vector quotient" mean?  Is there maybe a simpler way to say it?

Could you move this comment to the .h and put

/* See arch/aarch64.h.  */

here?

> diff --git a/gdb/features/aarch64-sve.c b/gdb/features/aarch64-sve.c
> new file mode 100644
> index 0000000000..6442640a73
> --- /dev/null
> +++ b/gdb/features/aarch64-sve.c
> @@ -0,0 +1,158 @@
> +/* Copyright (C) 2017 Free Software Foundation, Inc.

2018

Thanks,

Simon
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 2/8] Function for reading the Aarch64 SVE vector length.

Simon Marchi-2
In reply to this post by Alan Hayward
Hi Alan,

LGTM, with some nits.

On 2018-05-11 06:52 AM, Alan Hayward wrote:
> Add a method for reading the SVE vector length using ptrace. This returns
> 0 for systems without SVE support.
>
> Note the defines taken from Linux kernel headers in aarch64-sve-linux-ptrace.h.
> See the covering email for details about this.

Using ptrace to read the SVE registers (and the SVE register length) will work even
for kernels that didn't have these macros?

> There are multiple ways of expressing the vector length. Thankfully these are
> all wll defined. I've added convertors for going from one to the other.
> Hopefully this will help to prevent future confusion.
Copyright 2017 -> 2018 in the new files.

>
> 2018-05-11  Alan Hayward  <[hidden email]>
>
> gdb/
> * Makefile.in: Add new header.
> * gdb/arch/aarch64.h (sve_vg_from_vl): New macro.
> (sve_vl_from_vg): Likewise.
> (sve_vq_from_vl): Likewise.
> (sve_vl_from_vq): Likewise.
> (sve_vq_from_vg): Likewise.
> (sve_vg_from_vq): Likewise.
> * configure.nat: Add new c file.
> * nat/aarch64-sve-linux-ptrace.c: New file.
> * nat/aarch64-sve-linux-ptrace.h: New file.
>
> gdbserver/
> * configure.srv: Add new c/h file.
> ---
>  gdb/Makefile.in                    |  1 +
>  gdb/arch/aarch64.h                 | 17 +++++++++
>  gdb/configure.nat                  |  2 +-
>  gdb/gdbserver/configure.srv        |  1 +
>  gdb/nat/aarch64-sve-linux-ptrace.c | 52 +++++++++++++++++++++++++++
>  gdb/nat/aarch64-sve-linux-ptrace.h | 73 ++++++++++++++++++++++++++++++++++++++
>  6 files changed, 145 insertions(+), 1 deletion(-)
>  create mode 100644 gdb/nat/aarch64-sve-linux-ptrace.c
>  create mode 100644 gdb/nat/aarch64-sve-linux-ptrace.h
>
> diff --git a/gdb/Makefile.in b/gdb/Makefile.in
> index 87d74a7703..64042d1bd1 100644
> --- a/gdb/Makefile.in
> +++ b/gdb/Makefile.in
> @@ -1478,6 +1478,7 @@ HFILES_NO_SRCDIR = \
>   mi/mi-parse.h \
>   nat/aarch64-linux.h \
>   nat/aarch64-linux-hw-point.h \
> + nat/aarch64-sve-linux-ptrace.h \
>   nat/amd64-linux-siginfo.h \
>   nat/gdb_ptrace.h \
>   nat/gdb_thread_db.h \
> diff --git a/gdb/arch/aarch64.h b/gdb/arch/aarch64.h
> index 1846e04163..af0b157c51 100644
> --- a/gdb/arch/aarch64.h
> +++ b/gdb/arch/aarch64.h
> @@ -48,6 +48,23 @@ enum aarch64_regnum
>  #define AARCH64_V_REGS_NUM 32
>  #define AARCH64_NUM_REGS AARCH64_FPCR_REGNUM + 1
>  
> +/* There are a number of ways of expressing the current SVE vector size:
> +
> +   VL : Vector Length.
> + The number of bytes in an SVE Z register.
> +   VQ : Vector Quotient.
> + The number of 128bit chunks in an SVE Z register.
> +   VG : Vector Gradient.
> + The number of 64bit chunks in an SVE Z register.  */
> +
> +#define sve_vg_from_vl(vl) ((vl) / 8)
> +#define sve_vl_from_vg(vg) ((vg) * 8)
> +#define sve_vq_from_vl(vl) ((vl) / 0x10)
> +#define sve_vl_from_vq(vq) ((vq) * 0x10)
> +#define sve_vq_from_vg(vg) (sve_vq_from_vl (sve_vl_from_vg (vg)))
> +#define sve_vg_from_vq(vq) (sve_vg_from_vl (sve_vl_from_vq (vq)))
> +
> +
>  /* Maximum supported VQ value.  Increase if required.  */
>  #define AARCH64_MAX_SVE_VQ  16
>  
> diff --git a/gdb/configure.nat b/gdb/configure.nat
> index 6b0f44fede..d7d79adaca 100644
> --- a/gdb/configure.nat
> +++ b/gdb/configure.nat
> @@ -228,7 +228,7 @@ case ${gdb_host} in
>      aarch64)
>   #  Host: AArch64 based machine running GNU/Linux
>   NATDEPFILES="${NATDEPFILES} aarch64-linux-nat.o \
> - aarch32-linux-nat.o aarch64-linux-hw-point.o aarch64-linux.o"
> + aarch32-linux-nat.o aarch64-linux-hw-point.o aarch64-linux.o aarch64-sve-linux-ptrace.o"

Please wrap to 80 columns.

>   ;;
>      arm)
>   # Host: ARM based machine running GNU/Linux
> diff --git a/gdb/gdbserver/configure.srv b/gdb/gdbserver/configure.srv
> index ffeefb9b92..8bf0dcc650 100644
> --- a/gdb/gdbserver/configure.srv
> +++ b/gdb/gdbserver/configure.srv
> @@ -54,6 +54,7 @@ case "${target}" in
>   srv_tgtobj="$srv_tgtobj arch/aarch64-insn.o"
>   srv_tgtobj="$srv_tgtobj arch/aarch64.o"
>   srv_tgtobj="$srv_tgtobj linux-aarch64-tdesc.o"
> + srv_tgtobj="$srv_tgtobj aarch64-sve-linux-ptrace.o"
>   srv_tgtobj="${srv_tgtobj} $srv_linux_obj"
>   srv_linux_regsets=yes
>   srv_linux_thread_db=yes
> diff --git a/gdb/nat/aarch64-sve-linux-ptrace.c b/gdb/nat/aarch64-sve-linux-ptrace.c
> new file mode 100644
> index 0000000000..9381786fda
> --- /dev/null
> +++ b/gdb/nat/aarch64-sve-linux-ptrace.c
> @@ -0,0 +1,52 @@
> +/* Common target dependent for AArch64 systems.
> +
> +   Copyright (C) 2017 Free Software Foundation, Inc.
> +
> +   This file is part of GDB.
> +
> +   This program is free software; you can redistribute it and/or modify
> +   it under the terms of the GNU General Public License as published by
> +   the Free Software Foundation; either version 3 of the License, or
> +   (at your option) any later version.
> +
> +   This program is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +   GNU General Public License for more details.
> +
> +   You should have received a copy of the GNU General Public License
> +   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
> +
> +#include <sys/utsname.h>
> +#include <sys/uio.h>
> +#include "common-defs.h"
> +#include "elf/external.h"
> +#include "elf/common.h"
> +#include "aarch64-sve-linux-ptrace.h"
> +#include "arch/aarch64.h"
> +
> +/* Read VQ for the given tid using ptrace.  If SVE is not supported then zero
> +   is returned (on a system that supports SVE, then VQ cannot be zeo).  */

Here, put a reference to the .h as usual.

> +
> +unsigned long
> +aarch64_sve_get_vq (int tid)
> +{
> +  struct iovec iovec;
> +  struct user_sve_header header;
> +
> +  iovec.iov_len = sizeof (header);
> +  iovec.iov_base = &header;
> +
> +  /* Ptrace gives the vector length in bytes.  Convert it to VQ, the number of
> +     128bit chunks in a Z register.  We use VQ because 128bits is the minimum
> +     a Z register can increase in size.  */
> +
> +  if (ptrace (PTRACE_GETREGSET, tid, NT_ARM_SVE, &iovec) < 0)
> +    /* SVE is not supported.  */
> +    return 0;

Add braces here.

> +
> +  long vq = sve_vq_from_vl (header.vl);
> +  gdb_assert (sve_vl_valid (header.vl));

We should avoid gdb_assert for bad input values (including what we receive from the
kernel).  Could we display a warning and return 0?

> +
> +  return vq;
> +}
> diff --git a/gdb/nat/aarch64-sve-linux-ptrace.h b/gdb/nat/aarch64-sve-linux-ptrace.h
> new file mode 100644
> index 0000000000..b318150ac1
> --- /dev/null
> +++ b/gdb/nat/aarch64-sve-linux-ptrace.h
> @@ -0,0 +1,73 @@
> +/* Common target dependent for AArch64 systems.
> +
> +   Copyright (C) 2017 Free Software Foundation, Inc.
> +
> +   This file is part of GDB.
> +
> +   This program is free software; you can redistribute it and/or modify
> +   it under the terms of the GNU General Public License as published by
> +   the Free Software Foundation; either version 3 of the License, or
> +   (at your option) any later version.
> +
> +   This program is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +   GNU General Public License for more details.
> +
> +   You should have received a copy of the GNU General Public License
> +   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
> +
> +#ifndef AARCH64_SVE_LINUX_PTRACE_H
> +#define AARCH64_SVE_LINUX_PTRACE_H
> +
> +/* Where indicated, this file contains defines and macros lifted directly from
> +   the Linux kernel headers, with no modification.
> +   Refer to Linux kernel documentation for details.  */
> +
> +#include <asm/sigcontext.h>
> +#include <sys/utsname.h>
> +#include <sys/ptrace.h>
> +#include <asm/ptrace.h>
> +
> +/* Read VQ for the given tid using ptrace.  If SVE is not supported then zero
> +   is returned (on a system that supports SVE, then VQ cannot be zeo).  */

zeo -> zero.

> +extern unsigned long aarch64_sve_get_vq (int tid);
> +
> +/* Structures and defines taken from sigcontext.h.  */
> +
> +#ifndef SVE_SIG_ZREGS_SIZE
> +
> +#define SVE_VQ_BYTES 16 /* number of bytes per quadword */
> +
> +#define SVE_VQ_MIN 1
> +#define SVE_VQ_MAX 512
> +
> +#define SVE_VL_MIN (SVE_VQ_MIN * SVE_VQ_BYTES)
> +#define SVE_VL_MAX (SVE_VQ_MAX * SVE_VQ_BYTES)
> +
> +#define SVE_NUM_ZREGS 32
> +#define SVE_NUM_PREGS 16
> +
> +#define sve_vl_valid(vl) \
> + ((vl) % SVE_VQ_BYTES == 0 && (vl) >= SVE_VL_MIN && (vl) <= SVE_VL_MAX)
> +
> +#endif /* SVE_SIG_ZREGS_SIZE.  */
> +
> +
> +/* Structures and defines taken from ptrace.h.  */
> +
> +#ifndef SVE_PT_SVE_ZREG_SIZE
> +
> +struct user_sve_header {
> + __u32 size; /* total meaningful regset content in bytes */
> + __u32 max_size; /* maxmium possible size for this thread */
> + __u16 vl; /* current vector length */
> + __u16 max_vl; /* maximum possible vector length */
> + __u16 flags;
> + __u16 __reserved;
> +};
> +
> +#endif /* SVE_PT_SVE_ZREG_SIZE.  */
> +
> +#endif /* aarch64-sve-linux-ptrace.h */
>

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 4/8] Enable SVE for GDB

Simon Marchi-2
In reply to this post by Alan Hayward
On 2018-05-11 06:52 AM, Alan Hayward wrote:

> This patch enables SVE support for GDB by reading the VQ when
> creating a target description.
>
> It also ensures that SVE is taken into account when creating
> the tdep structure, and stores the current VQ value directly
> in tdep.
>
> With this patch, gdb on an aarch64 system with SVE will now detect
> SVE. The SVE registers will be displayed (but the contents will be
> invalid). The following patches fill out the contents.

LGTM, with some nits.

> @@ -2911,25 +2933,45 @@ aarch64_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
>    /* Validate the descriptor provides the mandatory core R registers
>       and allocate their numbers.  */
>    for (i = 0; i < ARRAY_SIZE (aarch64_r_register_names); i++)
> -    valid_p &=
> -      tdesc_numbered_register (feature, tdesc_data, AARCH64_X0_REGNUM + i,
> -       aarch64_r_register_names[i]);
> +    valid_p &= tdesc_numbered_register (feature_core, tdesc_data,
> + AARCH64_X0_REGNUM + i,
> + aarch64_r_register_names[i]);
>  
>    num_regs = AARCH64_X0_REGNUM + i;
>  
> -  /* Look for the V registers.  */
> -  feature = tdesc_find_feature (tdesc, "org.gnu.gdb.aarch64.fpu");
> -  if (feature)
> +  /* Add the V registers.  */
> +  if (feature_fpu != NULL)
>      {
> +      gdb_assert (feature_sve == NULL);

Again, if this situation can result from a bad input passed to GDB (a bad tdesc
sent by the remote), we shouldn't gdb_assert on it, but show an error message
and gracefully fail.

> +
>        /* Validate the descriptor provides the mandatory V registers
> -         and allocate their numbers.  */
> + and allocate their numbers.  */
>        for (i = 0; i < ARRAY_SIZE (aarch64_v_register_names); i++)
> - valid_p &=
> -  tdesc_numbered_register (feature, tdesc_data, AARCH64_V0_REGNUM + i,
> -   aarch64_v_register_names[i]);
> + valid_p &= tdesc_numbered_register (feature_fpu, tdesc_data,
> +    AARCH64_V0_REGNUM + i,
> +    aarch64_v_register_names[i]);
>  
>        num_regs = AARCH64_V0_REGNUM + i;
> +    }
> +
> +  /* Add the SVE registers.  */
> +  if (feature_sve != NULL)
> +    {
> +      gdb_assert (feature_fpu == NULL);

Same here.

> diff --git a/gdb/aarch64-tdep.h b/gdb/aarch64-tdep.h
> index c9fd7b3578..046de6228f 100644
> --- a/gdb/aarch64-tdep.h
> +++ b/gdb/aarch64-tdep.h
> @@ -73,6 +73,15 @@ struct gdbarch_tdep
>  
>    /* syscall record.  */
>    int (*aarch64_syscall_record) (struct regcache *regcache, unsigned long svc_number);
> +
> +  /* The VQ value for SVE targets, or zero if SVE is not supported.  */
> +  long vq;
> +
> +  /* Returns true if the target supports SVE.  */
> +  bool has_sve ()
> +  {
> +    return vq != 0;
> +  }

This method can be const.

Simon
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 5/8] Add aarch64 psuedo help functions

Simon Marchi-2
In reply to this post by Alan Hayward
psuedo -> pseudo in the title.

On 2018-05-11 06:52 AM, Alan Hayward wrote:

> Reduce code copy/paste by adding two helper functions for
> aarch64_pseudo_read_value and aarch64_pseudo_write
> The patch does not change any functionality.
>
> 2018-05-11  Alan Hayward  <[hidden email]>
>
> * aarch64-tdep.c (aarch64_pseudo_read_value_2): New helper func.
> (aarch64_pseudo_write_2): Likewise.
> (aarch64_pseudo_read_value): Use helper.
> (aarch64_pseudo_write): Likewise.
> ---
>  gdb/aarch64-tdep.c | 173 +++++++++++++++++------------------------------------
>  1 file changed, 54 insertions(+), 119 deletions(-)
>
> diff --git a/gdb/aarch64-tdep.c b/gdb/aarch64-tdep.c
> index 7893579d58..003fefb3c9 100644
> --- a/gdb/aarch64-tdep.c
> +++ b/gdb/aarch64-tdep.c
> @@ -2247,109 +2247,67 @@ aarch64_pseudo_register_reggroup_p (struct gdbarch *gdbarch, int regnum,
>    return group == all_reggroup;
>  }
>  
> +/* Inner version of aarch64_pseudo_read_value.  */
> +
> +static struct value *
> +aarch64_pseudo_read_value_2 (readable_regcache *regcache, int regnum_offset,
> +     int regsize, struct value *result_value)

It's not a big deal, but in other GDB parts we actually often use the _1 suffix
for the helper functions.

But otherwise, LGTM.

Simon

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 6/8] Aarch64 SVE pseudo register support

Simon Marchi-2
In reply to this post by Alan Hayward
On 2018-05-11 06:52 AM, Alan Hayward wrote:
> Add the functionality for reading/writing psuedo registers.

"pseudo"

>
> On SVE the V registers are pseudo registers. This is supported
> by adding AARCH64_SVE_V0_REGNUM.
>
> 2018-05-11  Alan Hayward  <[hidden email]>
>
> * aarch64-tdep.c (AARCH64_SVE_V0_REGNUM): Add define.
> (aarch64_vnv_type): Add function.
> (aarch64_pseudo_register_name): Add V regs for SVE.
> (aarch64_pseudo_register_type): Likewise.
> (aarch64_pseudo_register_reggroup_p): Likewise.
> (aarch64_pseudo_read_value_2): Use V0 offset for SVE
> (aarch64_pseudo_read_value): Add V regs for SVE.
> (aarch64_pseudo_write_2): Use V0 offset for SVE
> (aarch64_pseudo_write): Add V regs for SVE.
> * aarch64-tdep.h (struct gdbarch_tdep): Add vnv_type.
> ---
>  gdb/aarch64-tdep.c | 151 +++++++++++++++++++++++++++++++++++++++++++++--------
>  gdb/aarch64-tdep.h |   1 +
>  2 files changed, 130 insertions(+), 22 deletions(-)
>
> diff --git a/gdb/aarch64-tdep.c b/gdb/aarch64-tdep.c
> index 003fefb3c9..6a40a081cb 100644
> --- a/gdb/aarch64-tdep.c
> +++ b/gdb/aarch64-tdep.c
> @@ -69,6 +69,7 @@
>  #define AARCH64_S0_REGNUM (AARCH64_D0_REGNUM + 32)
>  #define AARCH64_H0_REGNUM (AARCH64_S0_REGNUM + 32)
>  #define AARCH64_B0_REGNUM (AARCH64_H0_REGNUM + 32)
> +#define AARCH64_SVE_V0_REGNUM (AARCH64_B0_REGNUM + 32)
>  
>  /* All possible aarch64 target descriptors.  */
>  struct target_desc *tdesc_aarch64_list[AARCH64_MAX_SVE_VQ + 1];
> @@ -1766,6 +1767,33 @@ aarch64_vnb_type (struct gdbarch *gdbarch)
>    return tdep->vnb_type;
>  }
>  
> +/* Return the type for an AdvSISD V register.  */
> +
> +static struct type *
> +aarch64_vnv_type (struct gdbarch *gdbarch)
> +{
> +  struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
> +
> +  if (tdep->vnv_type == NULL)
> +    {
> +      struct type *t;
> +      struct type *elem;
> +
> +      t = arch_composite_type (gdbarch, "__gdb_builtin_type_vnv",
> +       TYPE_CODE_UNION);
> +
> +      append_composite_type_field (t, "d", aarch64_vnd_type (gdbarch));
> +      append_composite_type_field (t, "s", aarch64_vns_type (gdbarch));
> +      append_composite_type_field (t, "h", aarch64_vnh_type (gdbarch));
> +      append_composite_type_field (t, "b", aarch64_vnb_type (gdbarch));
> +      append_composite_type_field (t, "q", aarch64_vnq_type (gdbarch));
> +
> +      tdep->vnv_type = t;
> +    }
> +
> +  return tdep->vnv_type;
> +}
> +
>  /* Implement the "dwarf2_reg_to_regnum" gdbarch method.  */
>  
>  static int
> @@ -2114,6 +2142,8 @@ aarch64_gen_return_address (struct gdbarch *gdbarch,
>  static const char *
>  aarch64_pseudo_register_name (struct gdbarch *gdbarch, int regnum)
>  {
> +  struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
> +
>    static const char *const q_name[] =
>      {
>        "q0", "q1", "q2", "q3",
> @@ -2191,6 +2221,25 @@ aarch64_pseudo_register_name (struct gdbarch *gdbarch, int regnum)
>    if (regnum >= AARCH64_B0_REGNUM && regnum < AARCH64_B0_REGNUM + 32)
>      return b_name[regnum - AARCH64_B0_REGNUM];
>  
> +  if (tdep->has_sve ())
> +    {
> +      static const char *const sve_v_name[] =
> + {
> +  "v0", "v1", "v2", "v3",
> +  "v4", "v5", "v6", "v7",
> +  "v8", "v9", "v10", "v11",
> +  "v12", "v13", "v14", "v15",
> +  "v16", "v17", "v18", "v19",
> +  "v20", "v21", "v22", "v23",
> +  "v24", "v25", "v26", "v27",
> +  "v28", "v29", "v30", "v31",
> + };
> +
> +      if (regnum >= AARCH64_SVE_V0_REGNUM
> +  && regnum < AARCH64_SVE_V0_REGNUM + AARCH64_V_REGS_NUM)
> + return sve_v_name[regnum - AARCH64_SVE_V0_REGNUM];
> +    }
> +
>    internal_error (__FILE__, __LINE__,
>    _("aarch64_pseudo_register_name: bad register number %d"),
>    regnum);
> @@ -2201,6 +2250,8 @@ aarch64_pseudo_register_name (struct gdbarch *gdbarch, int regnum)
>  static struct type *
>  aarch64_pseudo_register_type (struct gdbarch *gdbarch, int regnum)
>  {
> +  struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
> +
>    regnum -= gdbarch_num_regs (gdbarch);
>  
>    if (regnum >= AARCH64_Q0_REGNUM && regnum < AARCH64_Q0_REGNUM + 32)
> @@ -2218,6 +2269,10 @@ aarch64_pseudo_register_type (struct gdbarch *gdbarch, int regnum)
>    if (regnum >= AARCH64_B0_REGNUM && regnum < AARCH64_B0_REGNUM + 32)
>      return aarch64_vnb_type (gdbarch);
>  
> +  if (tdep->has_sve () && regnum >= AARCH64_SVE_V0_REGNUM
> +      && regnum < AARCH64_SVE_V0_REGNUM + 32)
> +    return aarch64_vnv_type (gdbarch);
> +
>    internal_error (__FILE__, __LINE__,
>    _("aarch64_pseudo_register_type: bad register number %d"),
>    regnum);
> @@ -2229,6 +2284,8 @@ static int
>  aarch64_pseudo_register_reggroup_p (struct gdbarch *gdbarch, int regnum,
>      struct reggroup *group)
>  {
> +  struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
> +
>    regnum -= gdbarch_num_regs (gdbarch);
>  
>    if (regnum >= AARCH64_Q0_REGNUM && regnum < AARCH64_Q0_REGNUM + 32)
> @@ -2243,6 +2300,9 @@ aarch64_pseudo_register_reggroup_p (struct gdbarch *gdbarch, int regnum,
>      return group == all_reggroup || group == vector_reggroup;
>    else if (regnum >= AARCH64_B0_REGNUM && regnum < AARCH64_B0_REGNUM + 32)
>      return group == all_reggroup || group == vector_reggroup;
> +  else if (tdep->has_sve () && regnum >= AARCH64_SVE_V0_REGNUM
> +   && regnum < AARCH64_SVE_V0_REGNUM + 32)
> +    return group == all_reggroup || group == vector_reggroup;
>  
>    return group == all_reggroup;
>  }
> @@ -2250,17 +2310,30 @@ aarch64_pseudo_register_reggroup_p (struct gdbarch *gdbarch, int regnum,
>  /* Inner version of aarch64_pseudo_read_value.  */
>  
>  static struct value *
> -aarch64_pseudo_read_value_2 (readable_regcache *regcache, int regnum_offset,
> +aarch64_pseudo_read_value_2 (struct gdbarch *gdbarch,
> +     readable_regcache *regcache, int regnum_offset,
>       int regsize, struct value *result_value)
>  {
> -  gdb_byte reg_buf[V_REGISTER_SIZE];
> +  struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
> +  gdb_byte v_buf[V_REGISTER_SIZE], *reg_buf;
> +  gdb_assert (AARCH64_V0_REGNUM == AARCH64_SVE_Z0_REGNUM);
>    unsigned v_regnum = AARCH64_V0_REGNUM + regnum_offset;
>  
> +  /* Enough space to read a full vector register.  */
> +  if (tdep->has_sve ())
> +    reg_buf = (gdb_byte *) xmalloc (register_size (gdbarch, AARCH64_V0_REGNUM));
> +  else
> +    reg_buf = v_buf;

If the size of a register (even with SVE) will always be reasonable to allocate on the
stack, maybe you could just use

  gdb_byte reg_buf[register_size (gdbarch, AARCH64_V0_REGNUM)];

(If there is always a register with number AARCH64_V0_REGNUM, that is)

Otherwise, it would be good to use an std::unique_ptr/gdb::unique_xmalloc_ptr to avoid
the manual xfree.

Same in aarch64_pseudo_write_2.

Simon
123