[8/8] AARCH64 SVE: Enable AARCH64 SVE in gdbserver

Message ID D46B0E8C.15503%alan.hayward@arm.com
State New, archived
Headers

Commit Message

Alan Hayward Dec. 5, 2016, 12:31 p.m. UTC
  This is part of a series adding AARCH64 SVE support to gdb and gdbserver.

This patch enables SVE support for the AARCH64 version of gdbserver and the
remote size of gdb.

Similar to the gdb code in the previous patch, in gdbserver the value of VG
is read prior to finding the current target description. A target
descriptor
is stored for each possible value of VG. The first time a particular value
of VG is used, then the target description is copied across from a default
version (with an implied VG value of 1) and the register sizes are scaled
to
the current VG value.

In gdbserver, when finding the current register cache the VG value is read
again. If it has changed since the last then aarch64_validate_tdesc () will
return false, causing the regcache to be destroyed and the target
descriptor
to be re-read. 

VG is included in the SVE register set and is marked as an expediated
register
(added in a previous patch).

In the gdb remote side, on receipt on a stop-reply the VG value in the stop
reply is compared against the currently known VG value. If there is a
difference, then the current target description and register cache is
cleared
and a new target description is found (using the new VG value).

Tested on x86 and aarch64.
Ok to commit?

Alan.
  

Comments

Yao Qi Dec. 14, 2016, 4:51 p.m. UTC | #1
On 16-12-05 12:31:08, Alan Hayward wrote:
> 
> VG is included in the SVE register set and is marked as an expediated
> register
> (added in a previous patch).
> 
> In the gdb remote side, on receipt on a stop-reply the VG value in the stop
> reply is compared against the currently known VG value. If there is a
> difference, then the current target description and register cache is
> cleared
> and a new target description is found (using the new VG value).

This only works when target descriptions change among different VG
values (VG != 0).  If the program starts with VG = 0, the target
description is still the normal aarch64 one, which doesn't 
understand register number of VG.

(gdb) c
Continuing.
Remote sent bad register number 0x55: 55:0000000000000000;thread:p2ad9.2ad9;core:5;
Packet: 'T0b1d:0000000000000000;1f:0000000000000000;20:0000000000000000;55:0000000000000000;thread:p2ad9.2ad9;core:5;'

I applied your patches, hack aarch64_read_vg a little bit, let it
return some random even number, like 0, 2, 4, 8.

Looks adding a new expedite register doesn't help detecting architecture
change.  We may need to add a new stop reason "tdesc:id", "id" is an
integer, the id of a known target description.

>  int aarch64_validate_tdesc (struct thread_info *thread)
>  {
> -  return 1;
> +  int tid = ptid_get_lwp (thread_to_gdb_id (thread));
> +  long vg = aarch64_read_vg (tid);
> +  struct regcache *regcache = inferior_regcache_data (thread);
> +  struct process_info *proc;
> +
> +  /* Non SVE targets always validate as true.  */
> +  if (vg == 0)
> +    return 1;

This function doesn't handle the case VG = 2 changed to VG = 0.  What
we need to do is to compare tdesc, like this,

    return (proc->tdesc == aarch64_get_tdesc(vg));

which is simpler.
> +
> +  if (regcache)
> +    return (register_size (regcache->tdesc, AARCH64_SVE_Z0_REGNO)
> +	    == sve_vl_from_vg (vg));
> +
> +  proc = get_thread_process (thread);
> +  return (register_size (proc->tdesc, AARCH64_SVE_Z0_REGNO)
> +	  == sve_vl_from_vg (vg));
>  }
>
  
Yao Qi Dec. 14, 2016, 5:15 p.m. UTC | #2
On 16-12-14 16:51:24, Yao Qi wrote:
> We may need to add a new stop reason "tdesc:id", "id" is an
> integer, the id of a known target description.

Please ignore this comment.  I thought I delete it.
  

Patch

diff --git a/gdb/aarch64-tdep.c b/gdb/aarch64-tdep.c
index 
e1cdee9e35534e1910c31feca5208ff1cd5bc8b5..84aa0300af1ae3ebd366df6361f5e0049
710bf4b 100644
--- a/gdb/aarch64-tdep.c
+++ b/gdb/aarch64-tdep.c
@@ -2772,14 +2772,38 @@  aarch64_displaced_step_hw_singlestep (struct
gdbarch *gdbarch,
   return 1;
 }

-/* Implement the "target_description_changed_p" gdbarch method.  */
+/* Implement the "target_description_changed_p" gdbarch method.
+   Return true if the VG value in the registers list does not match the VG
+   value in the regcache.  */
 static int
 aarch64_target_description_changed_p (struct gdbarch *gdbarch,
 				      ptid_t ptid,
 				      void *registers)
 {
   VEC(cached_reg_t) *registerlist = (VEC(cached_reg_t)*)registers;
-  return 0;
+  cached_reg_t *reg;
+  int ix;
+  int regcache_has_vg = (gdbarch_num_regs (gdbarch) >
AARCH64_SVE_VG_REGNUM);
+
+  for (ix = 0; VEC_iterate (cached_reg_t, registerlist, ix, reg); ix++)
+    if (reg->num == AARCH64_SVE_VG_REGNUM)
+      {
+	if (!regcache_has_vg)
+	  /* No VG in regcache.  */
+	  return 1;
+
+	struct regcache *regcache = get_thread_arch_regcache (ptid, gdbarch);
+
+	if (regcache_register_status (regcache, AARCH64_SVE_VG_REGNUM)
+	    != REG_VALID)
+	  return 1;
+
+	return regcache_raw_compare (regcache, AARCH64_SVE_VG_REGNUM,
+				     reg->data, 0);
+      }
+
+  /* VG is not in registers list.  */
+  return regcache_has_vg;
 }

 /* Implement the "target_get_tdep_info" gdbarch method.  */
@@ -2787,7 +2811,16 @@  void *
 aarch64_target_get_tdep_info (void *registers)
 {
   VEC(cached_reg_t) *regcache = (VEC(cached_reg_t)*)registers;
-  return NULL;
+  cached_reg_t *reg;
+  int ix;
+
+  for (ix = 0; VEC_iterate (cached_reg_t, regcache, ix, reg); ix++)
+    if (reg->num == AARCH64_SVE_VG_REGNUM)
+      {
+	return reg->data;
+      }
+
+  return 0;
 }

 /* Get the correct target description for the given VG value.
diff --git a/gdb/gdbserver/linux-aarch64-low.c
b/gdb/gdbserver/linux-aarch64-low.c
index 
bcc7f9184668b512008359be2d11b149bed81c59..bb28baf24bfd032fc05e9a5edb9d3f74e
8bcf7a3 100644
--- a/gdb/gdbserver/linux-aarch64-low.c
+++ b/gdb/gdbserver/linux-aarch64-low.c
@@ -164,9 +164,17 @@  struct user_sve_header {

 #endif

+/* Maximum supported VG value.  Increase if required.  */
+#define AARCH64_TDEP_MAX_SVE_VG  32
+
 /* Defined in auto-generated files.  */
 void init_registers_aarch64 (void);
+void init_registers_aarch64_sve (void);
 extern const struct target_desc *tdesc_aarch64;
+/* Aarch64 sve target descriptor pre-VG scaling.  */
+extern const struct target_desc *tdesc_aarch64_sve;
+/* All possible aarch64 sve target descriptors.  */
+struct target_desc tdesc_aarch64_sve_list[AARCH64_TDEP_MAX_SVE_VG + 1];

 #ifdef HAVE_SYS_REG_H
 #include <sys/reg.h>
@@ -208,6 +216,83 @@  struct arch_process_info
   struct aarch64_debug_reg_state debug_reg_state;
 };

+/* Read VG for the given tid using ptrace.  */
+
+static unsigned long
+aarch64_read_vg (int tid)
+{
+  int ret;
+  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 VG - which
is the
+     number of 64bit chunks.  We use VG because that is the value that
appears
+     as a DWARF register.  */
+
+  ret = ptrace (PTRACE_GETREGSET, tid, NT_ARM_SVE, &iovec);
+  if (ret >= 0)
+    {
+      gdb_assert (sve_vl_valid (header.vl));
+      return sve_vg_from_vl (header.vl);
+    }
+  /* SVE is not supported.  */
+  return 0;
+}
+
+static int
+aarch64_target_adjusted_register (int id)
+{
+  return ((id >= AARCH64_SVE_Z0_REGNO
+	   && id < AARCH64_SVE_Z0_REGNO + AARCH64_SVE_Z_REGS_NUM)
+	  || (id >= AARCH64_SVE_P0_REGNO
+	      && id < AARCH64_SVE_P0_REGNO + AARCH64_SVE_P_REGS_NUM)
+	  || id == AARCH64_SVE_FFR_REGNO);
+}
+
+/* Get the correct target description for the given VG value.
+   If VG is zero then it is assumed SVE is not supported.
+   (It is not possible to set VG to zero on an SVE system).  */
+static const struct target_desc *
+aarch64_get_tdesc (long vg)
+{
+  if (vg > AARCH64_TDEP_MAX_SVE_VG)
+    error (_("VG is %ld, maximum supported value is %d"), vg,
+	   AARCH64_TDEP_MAX_SVE_VG);
+  struct target_desc *tdesc;
+
+  if (vg == 0)
+    return tdesc_aarch64;
+
+  tdesc = &tdesc_aarch64_sve_list[vg];
+
+  if (tdesc->num_registers == 0)
+    {
+      int i, reg_def_size;
+
+      /* The target descriptor for the given VG value has not been
accessed
+	 yet.  Copy all fields across from the default tdesc_aarch64_sve and
+	 then update all the register sizes.  */
+
+      tdesc->num_registers = tdesc_aarch64_sve->num_registers;
+      tdesc->expedite_regs = tdesc_aarch64_sve->expedite_regs;
+      tdesc->xmltarget = tdesc_aarch64_sve->xmltarget;
+
+      reg_def_size = tdesc->num_registers * sizeof (struct reg);
+      tdesc->reg_defs = (struct reg*) xmalloc (reg_def_size);
+      memcpy (tdesc->reg_defs, tdesc_aarch64_sve->reg_defs, reg_def_size);
+      for (i=0; i<tdesc->num_registers; i++)
+	if (aarch64_target_adjusted_register (i))
+	  tdesc->reg_defs[i].size *= vg;
+
+      init_target_desc (tdesc);
+    }
+
+  return tdesc;
+}
+
 /* Return true if the size of register 0 is 8 byte.  */

 static int
@@ -822,7 +907,7 @@  aarch64_linux_read_description (void)
   is_elf64 = linux_pid_exe_is_elf_64_file (tid, &machine);

   if (is_elf64)
-    return tdesc_aarch64;
+    return aarch64_get_tdesc (aarch64_read_vg (tid));
   else
     return tdesc_arm_with_neon;
 }
@@ -3344,7 +3429,22 @@  aarch64_supports_hardware_single_step (void)

 int aarch64_validate_tdesc (struct thread_info *thread)
 {
-  return 1;
+  int tid = ptid_get_lwp (thread_to_gdb_id (thread));
+  long vg = aarch64_read_vg (tid);
+  struct regcache *regcache = inferior_regcache_data (thread);
+  struct process_info *proc;
+
+  /* Non SVE targets always validate as true.  */
+  if (vg == 0)
+    return 1;
+
+  if (regcache)
+    return (register_size (regcache->tdesc, AARCH64_SVE_Z0_REGNO)
+	    == sve_vl_from_vg (vg));
+
+  proc = get_thread_process (thread);
+  return (register_size (proc->tdesc, AARCH64_SVE_Z0_REGNO)
+	  == sve_vl_from_vg (vg));
 }

 struct linux_target_ops the_low_target =
@@ -3391,6 +3491,7 @@  void
 initialize_low_arch (void)
 {
   init_registers_aarch64 ();
+  init_registers_aarch64_sve ();

   initialize_low_arch_aarch32 ();

diff --git a/gdb/gdbserver/regcache.c b/gdb/gdbserver/regcache.c
index 
752148042f26e1a23c9d0b1b7fb5d14cab2bf538..cef61ac0695cafefaca667ab39269f4f0
b85663c 100644
--- a/gdb/gdbserver/regcache.c
+++ b/gdb/gdbserver/regcache.c
@@ -28,6 +28,16 @@  get_thread_regcache (struct thread_info *thread, int
fetch)
 {
   struct regcache *regcache;

+  if (!target_validate_tdesc (thread))
+    {
+      /* Clear regcache.  */
+      free_register_cache (inferior_regcache_data (thread));
+      set_inferior_regcache_data (thread, NULL);
+      regcache = NULL;
+
+      target_arch_setup ();
+    }
+
   regcache = inferior_regcache_data (thread);

   /* Threads' regcaches are created lazily, because biarch targets add
diff --git a/gdb/remote.c b/gdb/remote.c
index 
03485d5a202f0c8a571a3ae8405d8d5b4577a8d0..03c2f88ff4b6519bff83c557e564db36c
8b340e9 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -7192,10 +7192,20 @@  process_stop_reply (struct stop_reply *stop_reply,
       && status->kind != TARGET_WAITKIND_NO_RESUMED)
     {
       struct private_thread_info *remote_thr;
+      void *regcache = stop_reply->regcache;

       /* Expedited registers.  */
-      if (stop_reply->regcache)
+      if (regcache)
 	{
+	  struct gdbarch *gdbarch = target_gdbarch ();
+	  if (gdbarch_target_description_changed_p (gdbarch, ptid, regcache))
+	    {
+	      void *info = gdbarch_target_get_tdep_info (gdbarch, regcache);
+	      registers_changed ();
+	      target_clear_description ();
+	      target_find_description (info);
+	    }
+
 	  struct regcache *regcache
 	    = get_thread_arch_regcache (ptid, target_gdbarch ());
 	  cached_reg_t *reg;
diff --git a/gdb/target-descriptions.h b/gdb/target-descriptions.h
index 
217293a4f4e29dccf191508efab1f6a439623aac..fb8e9e1d1046242d1a2ec5282011c5910
2b3476f 100644
--- a/gdb/target-descriptions.h
+++ b/gdb/target-descriptions.h
@@ -37,7 +37,7 @@  struct inferior;
 /* Fetch the current inferior's description, and switch its current
    architecture to one which incorporates that description.  */

-void target_find_description (void);
+void target_find_description (void *tdep_info = NULL);

 /* Discard any description fetched from the target for the current
    inferior, and switch the current architecture to one with no target
diff --git a/gdb/target-descriptions.c b/gdb/target-descriptions.c
index 
62155a5a94f423ee88af739e3070ff78b6f75661..f5c107c277a4b93c24e7b25a6323c79d5
cf1beae 100644
--- a/gdb/target-descriptions.c
+++ b/gdb/target-descriptions.c
@@ -314,7 +314,7 @@  static char *tdesc_filename_cmd_string;
    architecture to one which incorporates that description.  */

 void
-target_find_description (void)
+target_find_description (void *tdep_info)
 {
   /* If we've already fetched a description from the target, don't do
      it again.  This allows a target to fetch the description early,
@@ -353,6 +353,8 @@  target_find_description (void)

       gdbarch_info_init (&info);
       info.target_desc = current_target_desc;
+      info.tdep_info = tdep_info;
+
       if (!gdbarch_update_p (info))
 	warning (_("Architecture rejected target-supplied description"));
       else