[PATCH] [PATCH v3] RISC-V: support for vector register accesses via ptrace() in RISC-V Linux native

[PATCH] [PATCH v3] RISC-V: support for vector register accesses via ptrace() in RISC-V Linux native

[snatu]

From: Sameer Natu <snatu@whileone.in>

A v3 re-spin of the original patch.
Tested with latest kernel 6.14.2 on RISCV QEMU.
Removed Magic Numbers from v2 patch and worked on review comments of v2 patch.

---
 gdb/arch/riscv.c             | 188 ++++++++++++++++++++++++++++++++++-
 gdb/nat/riscv-linux-tdesc.c  |  68 +++++++++++++
 gdb/nat/riscv-linux-tdesc.h  |  24 +++++
 gdb/riscv-linux-nat.c        | 163 ++++++++++++++++++++++++++++++
 gdb/riscv-linux-tdep.c       | 133 +++++++++++++++++++++++++
 gdb/riscv-tdep.c             |  49 ++++++++-
 gdb/riscv-tdep.h             |  14 +++
 gdbserver/linux-riscv-low.cc | 110 ++++++++++++++++++++
 include/elf/common.h         |   1 +
 9 files changed, 743 insertions(+), 7 deletions(-)

diff --git a/gdb/arch/riscv.c b/gdb/arch/riscv.c
index a6188ea3a8c..14fc85631e3 100644
--- a/gdb/arch/riscv.c
+++ b/gdb/arch/riscv.c
@@ -25,12 +25,38 @@
 #include "../features/riscv/64bit-fpu.c"
 #include "../features/riscv/rv32e-xregs.c"
 
+#include "opcode/riscv-opc.h"
+
 #ifndef GDBSERVER
 #define STATIC_IN_GDB static
 #else
 #define STATIC_IN_GDB
 #endif
 
+#ifdef GDBSERVER
+/* Work around issue where trying to include riscv-tdep.h (to get access to canonical RISCV_V0_REGNUM declaration
+   from that header) is problamtic for gdbserver build.  */
+//#include "riscv-tdep.h"
+#define RISCV_VSTART 73
+#define RISCV_VXSAT 74
+#define RISCV_VXRM 75
+#define RISCV_VCSR 80
+#define RISCV_VL 3169 
+#define RISCV_VTYPE 3170
+#define RISCV_VLENB 3171
+#define RISCV_V0_REGNUM 4162   
+#else
+#include "riscv-tdep.h"
+#include "defs.h"
+#endif
+
+static int
+create_feature_riscv_vector_from_features (struct target_desc *result,
+					   long regnum,
+					   const struct riscv_gdbarch_features
+					   features);
+
+
 /* See arch/riscv.h.  */
 
 STATIC_IN_GDB target_desc_up
@@ -83,15 +109,169 @@ riscv_create_target_description (const struct riscv_gdbarch_features features)
   else if (features.flen == 8)
     regnum = create_feature_riscv_64bit_fpu (tdesc.get (), regnum);
 
-  /* Currently GDB only supports vector features coming from remote
-     targets.  We don't support creating vector features on native targets
-     (yet).  */
   if (features.vlen != 0)
-    error (_("unable to create vector feature"));
+    regnum =
+      create_feature_riscv_vector_from_features (tdesc.get (),
+						 RISCV_V0_REGNUM, features);
 
   return tdesc;
 }
 
+
+
+/* Usually, these target_desc instances are static for an architecture, and expressable
+   in XML format, but this is a special case where length of a RISC-V vector register
+   is not architecturally fixed to a constant (the maximuim width is a defined constant,
+   but it's nice to tailor a target description the actual VLENB) */
+static int
+create_feature_riscv_vector_from_features (struct target_desc *result,
+					   long regnum,
+					   const struct riscv_gdbarch_features
+					   features)
+{
+  struct tdesc_feature *feature;
+  unsigned long bitsize;
+
+  feature = tdesc_create_feature (result, "org.gnu.gdb.riscv.vector");
+  tdesc_type *element_type;
+
+  /* if VLENB is present (which we know it is present if execution reaches this function),
+     then we know by definition that it is at least 4 bytes wide */
+  
+  element_type = tdesc_named_type (feature, "uint8");
+  tdesc_create_vector (feature, "bytes", element_type, features.vlen);
+
+  element_type = tdesc_named_type (feature, "uint16");
+  tdesc_create_vector (feature, "shorts", element_type, features.vlen / 2);
+
+  element_type = tdesc_named_type (feature, "uint32");
+  tdesc_create_vector (feature, "words", element_type, features.vlen / 4);
+
+  /* Need VLENB value checks for element chunks larger than 4 bytes */
+  
+  if (features.vlen >= 8)
+    {
+      element_type = tdesc_named_type (feature, "uint64");
+      tdesc_create_vector (feature, "longs", element_type, features.vlen / 8);
+    }
+
+  /* QEMU and OpenOCD include the quads width in their target descriptions, so we're
+     following that precedent, even if it's not particularly useful in practice, yet */
+  
+  if (features.vlen >= 16)
+    {
+      element_type = tdesc_named_type (feature, "uint128");
+      tdesc_create_vector (feature, "quads", element_type,
+			   features.vlen / 16);
+    }
+
+  tdesc_type_with_fields *type_with_fields;
+  type_with_fields = tdesc_create_union (feature, "riscv_vector");
+  tdesc_type *field_type;
+
+  if (features.vlen >= 16)
+    {
+      field_type = tdesc_named_type (feature, "quads");
+      tdesc_add_field (type_with_fields, "q", field_type);
+    }
+  if (features.vlen >= 8)
+    {
+      field_type = tdesc_named_type (feature, "longs");
+      tdesc_add_field (type_with_fields, "l", field_type);
+    }
+
+  /* Again, we know vlenb is >= 4, so no if guards needed for words/shorts/bytes */
+  
+  field_type = tdesc_named_type (feature, "words");
+  tdesc_add_field (type_with_fields, "w", field_type);
+  
+  field_type = tdesc_named_type (feature, "shorts");
+  tdesc_add_field (type_with_fields, "s", field_type);
+  
+  field_type = tdesc_named_type (feature, "bytes");
+  tdesc_add_field (type_with_fields, "b", field_type);
+
+  /* Register vector and CSR definitions using stable magic regnums to 
+     ensure compatibility across GDB and gdbserver builds.  */
+  tdesc_create_reg (feature, "vstart", RISCV_VSTART, 1, NULL, features.xlen * 8, "int");
+  tdesc_create_reg (feature, "vxsat", RISCV_VXSAT, 1, NULL, features.xlen * 8, "int");
+  tdesc_create_reg (feature, "vxrm", RISCV_VXRM, 1, NULL, features.xlen * 8, "int");  
+  tdesc_create_reg (feature, "vcsr", RISCV_VCSR, 1, NULL, features.xlen * 8, "int");
+  tdesc_create_reg (feature, "vl", RISCV_VL, 1, NULL, features.xlen * 8, "int");
+  tdesc_create_reg (feature, "vtype", RISCV_VTYPE, 1, NULL, features.xlen * 8, "int");
+  tdesc_create_reg (feature, "vlenb", RISCV_VLENB, 1, NULL, features.xlen * 8, "int");
+
+  bitsize = features.vlen * 8;
+  tdesc_create_reg (feature, "v0", regnum++, 1, NULL, bitsize,
+		    "riscv_vector");
+  tdesc_create_reg (feature, "v1", regnum++, 1, NULL, bitsize,
+		    "riscv_vector");
+  tdesc_create_reg (feature, "v2", regnum++, 1, NULL, bitsize,
+		    "riscv_vector");
+  tdesc_create_reg (feature, "v3", regnum++, 1, NULL, bitsize,
+		    "riscv_vector");
+  tdesc_create_reg (feature, "v4", regnum++, 1, NULL, bitsize,
+		    "riscv_vector");
+  tdesc_create_reg (feature, "v5", regnum++, 1, NULL, bitsize,
+		    "riscv_vector");
+  tdesc_create_reg (feature, "v6", regnum++, 1, NULL, bitsize,
+		    "riscv_vector");
+  tdesc_create_reg (feature, "v7", regnum++, 1, NULL, bitsize,
+		    "riscv_vector");
+  tdesc_create_reg (feature, "v8", regnum++, 1, NULL, bitsize,
+		    "riscv_vector");
+  tdesc_create_reg (feature, "v9", regnum++, 1, NULL, bitsize,
+		    "riscv_vector");
+  tdesc_create_reg (feature, "v10", regnum++, 1, NULL, bitsize,
+		    "riscv_vector");
+  tdesc_create_reg (feature, "v11", regnum++, 1, NULL, bitsize,
+		    "riscv_vector");
+  tdesc_create_reg (feature, "v12", regnum++, 1, NULL, bitsize,
+		    "riscv_vector");
+  tdesc_create_reg (feature, "v13", regnum++, 1, NULL, bitsize,
+		    "riscv_vector");
+  tdesc_create_reg (feature, "v14", regnum++, 1, NULL, bitsize,
+		    "riscv_vector");
+  tdesc_create_reg (feature, "v15", regnum++, 1, NULL, bitsize,
+		    "riscv_vector");
+  tdesc_create_reg (feature, "v16", regnum++, 1, NULL, bitsize,
+		    "riscv_vector");
+  tdesc_create_reg (feature, "v17", regnum++, 1, NULL, bitsize,
+		    "riscv_vector");
+  tdesc_create_reg (feature, "v18", regnum++, 1, NULL, bitsize,
+		    "riscv_vector");
+  tdesc_create_reg (feature, "v19", regnum++, 1, NULL, bitsize,
+		    "riscv_vector");
+  tdesc_create_reg (feature, "v20", regnum++, 1, NULL, bitsize,
+		    "riscv_vector");
+  tdesc_create_reg (feature, "v21", regnum++, 1, NULL, bitsize,
+		    "riscv_vector");
+  tdesc_create_reg (feature, "v22", regnum++, 1, NULL, bitsize,
+		    "riscv_vector");
+  tdesc_create_reg (feature, "v23", regnum++, 1, NULL, bitsize,
+		    "riscv_vector");
+  tdesc_create_reg (feature, "v24", regnum++, 1, NULL, bitsize,
+		    "riscv_vector");
+  tdesc_create_reg (feature, "v25", regnum++, 1, NULL, bitsize,
+		    "riscv_vector");
+  tdesc_create_reg (feature, "v26", regnum++, 1, NULL, bitsize,
+		    "riscv_vector");
+  tdesc_create_reg (feature, "v27", regnum++, 1, NULL, bitsize,
+		    "riscv_vector");
+  tdesc_create_reg (feature, "v28", regnum++, 1, NULL, bitsize,
+		    "riscv_vector");
+  tdesc_create_reg (feature, "v29", regnum++, 1, NULL, bitsize,
+		    "riscv_vector");
+  tdesc_create_reg (feature, "v30", regnum++, 1, NULL, bitsize,
+		    "riscv_vector");
+  tdesc_create_reg (feature, "v31", regnum++, 1, NULL, bitsize,
+		    "riscv_vector");
+
+
+  return regnum;
+}
+
+
 #ifndef GDBSERVER
 
 /* Wrapper used by std::unordered_map to generate hash for feature set.  */
diff --git a/gdb/nat/riscv-linux-tdesc.c b/gdb/nat/riscv-linux-tdesc.c
index 254a25ccefe..80d1ca64ba9 100644
--- a/gdb/nat/riscv-linux-tdesc.c
+++ b/gdb/nat/riscv-linux-tdesc.c
@@ -22,14 +22,18 @@
 #include "elf/common.h"
 #include "nat/gdb_ptrace.h"
 #include "nat/riscv-linux-tdesc.h"
+#include "gdbsupport/gdb_setjmp.h"
 
 #include <sys/uio.h>
+#include <signal.h>
 
 /* Work around glibc header breakage causing ELF_NFPREG not to be usable.  */
 #ifndef NFPREG
 # define NFPREG 33
 #endif
 
+static unsigned long safe_read_vlenb ();
+
 /* See nat/riscv-linux-tdesc.h.  */
 
 struct riscv_gdbarch_features
@@ -78,5 +82,69 @@ riscv_linux_read_features (int tid)
       break;
     }
 
+  features.vlen = safe_read_vlenb ();
+
   return features;
 }
+
+static SIGJMP_BUF sigill_guard_jmp_buf;
+
+static void
+sigill_guard (int sig)
+{
+  /* this will gets us back to caller deeper in the call stack, with an indication that
+     an illegal instruction condition was encountered */
+  SIGLONGJMP (sigill_guard_jmp_buf, -1);
+
+  /* control won't get here */
+}
+
+
+
+static unsigned long
+safe_read_vlenb ()
+{
+  /* Surrounding the attempt here to read VLENB CSR to have a signal handler set up
+     to trap illegal instruction condition (SIGILL), and if a trap happens during this call,
+     get control back within this function and return 0 in that case.
+   */
+  unsigned long vlenb = 0;
+  struct sigaction our_action = { 0 };
+  struct sigaction original_action;
+  int sysresult;
+
+
+  our_action.sa_handler = sigill_guard;
+
+  sysresult = sigaction (SIGILL, &our_action, &original_action);
+  if (sysresult != 0)
+    {
+      perror
+	("Error installing temporary SIGILL handler in safe_read_vlenb()");
+    }
+
+  if (SIGSETJMP (sigill_guard_jmp_buf, 1) == 0)
+    {
+    asm ("csrr %0, vlenb":"=r" (vlenb));
+    }
+  else
+    {
+      /* Must've generated an illegal instruction condition; we'll figure this means
+         no vector unit is present */
+      vlenb = 0;
+    }
+
+
+  if (sysresult == 0)
+    {
+      /* re-install former handler */
+      sysresult = sigaction (SIGILL, &original_action, NULL);
+      if (sysresult != 0)
+	{
+	  perror
+	    ("Error re-installing original SIGILL handler in safe_read_vlenb()");
+	}
+
+    }
+  return vlenb;
+}
diff --git a/gdb/nat/riscv-linux-tdesc.h b/gdb/nat/riscv-linux-tdesc.h
index de10d91caae..76f8d2f0cdd 100644
--- a/gdb/nat/riscv-linux-tdesc.h
+++ b/gdb/nat/riscv-linux-tdesc.h
@@ -20,9 +20,33 @@
 #define GDB_NAT_RISCV_LINUX_TDESC_H
 
 #include "arch/riscv.h"
+#include "asm/ptrace.h"
 
 /* Determine XLEN and FLEN for the LWP identified by TID, and return a
    corresponding features object.  */
 struct riscv_gdbarch_features riscv_linux_read_features (int tid);
 
+#define RISCV_MAX_VLENB (8192)
+
+/* Some branches and/or commits of linux kernel named this "struct __riscv_v_state",
+   and later it was changed to "struct __riscv_v_ext_state",
+   so using a macro to stand-in for that struct type to make it easier to modify
+   in a single place, if compiling against one of those older Linux kernel commits */
+#ifndef RISCV_VECTOR_STATE_T
+#define RISCV_VECTOR_STATE_T struct __riscv_v_ext_state
+#endif
+
+/* Struct for use in ptrace() calls for vector CSRs/registers */
+struct __riscv_vregs
+{
+  RISCV_VECTOR_STATE_T vstate;
+  gdb_byte data[RISCV_MAX_VLENB * 32]; /* data will arrive packed, VLENB bytes per element, not necessarily RISCV_MAX_VLENB bytes per element */
+};
+
+#define VCSR_MASK_VXSAT 0x1
+#define VCSR_POS_VXSAT 0
+#define VCSR_MASK_VXRM 0x3
+#define VCSR_POS_VXRM 1
+
+
 #endif /* GDB_NAT_RISCV_LINUX_TDESC_H */
diff --git a/gdb/riscv-linux-nat.c b/gdb/riscv-linux-nat.c
index 8846329afc6..000a9de8abf 100644
--- a/gdb/riscv-linux-nat.c
+++ b/gdb/riscv-linux-nat.c
@@ -21,6 +21,7 @@
 #include "linux-nat.h"
 #include "riscv-tdep.h"
 #include "inferior.h"
+#include "regset.h"
 
 #include "elf/common.h"
 
@@ -124,6 +125,114 @@ supply_fpregset_regnum (struct regcache *regcache, const prfpregset_t *fpregs,
     }
 }
 
+#define MEMBER_SIZE(type, member) sizeof(((type *) 0)->member)
+
+static const regcache_map_entry riscv_linux_vregmap[] =
+{
+  { 1, RISCV_CSR_VSTART_REGNUM, MEMBER_SIZE(struct __riscv_vregs, vstate.vstart) },
+  { 1, RISCV_CSR_VL_REGNUM, MEMBER_SIZE(struct __riscv_vregs, vstate.vl) },
+  { 1, RISCV_CSR_VTYPE_REGNUM, MEMBER_SIZE(struct __riscv_vregs, vstate.vtype) },
+  { 1, RISCV_CSR_VCSR_REGNUM, MEMBER_SIZE(struct __riscv_vregs, vstate.vcsr) },
+  /* struct __riscv_vregs member "datap" is a pointer that doesn't correspond
+     to a register value.  In the context of ptrace(), member is always zero,
+     with V0..V31 values inline after that.  So, skipping datap */
+  { 1, REGCACHE_MAP_SKIP, MEMBER_SIZE(struct __riscv_vregs, vstate.datap) },
+  /* Here's V0..V31.  Specifying 0 as size leads to a call to register_size()
+     for size determination */
+  { 32, RISCV_V0_REGNUM, 0 },
+  { 0 },  /* count==0 represents termination of entries */
+};
+
+/* Define the vector register regset.  */
+
+static const struct regset riscv_linux_vregset =
+{
+  riscv_linux_vregmap,
+  regcache_supply_regset /* Other RISC-V regsets use riscv_supply_regset here; not sure that'd be correct for this case */,
+  regcache_collect_regset
+};
+
+
+/* Supply RISC-V vector register values (including inferred CSRs) to the GDB regcache.  */
+
+static void
+supply_vregset_regnum (struct regcache *regcache,
+		       const struct __riscv_vregs *vregs, int regnum)
+{
+  const gdb_byte *buf;
+  int vlenb = register_size (regcache->arch (), RISCV_V0_REGNUM);
+
+  regcache_supply_regset (&riscv_linux_vregset, regcache, regnum, vregs, sizeof(*vregs));  
+
+  if (regnum == -1 || regnum == RISCV_CSR_VLENB_REGNUM)
+    {
+      /* we already have a local copy above, use that (widened for XLEN padding) */
+      uint64_t xlen_safe_vlenb = vlenb;
+      buf = (gdb_byte *) & xlen_safe_vlenb;
+      regcache->raw_supply (RISCV_CSR_VLENB_REGNUM, buf);
+    }
+
+  if (regnum == -1 || regnum == RISCV_CSR_VXSAT_REGNUM)
+    {
+      /*  this CSR is not part of vregs->vstate literally, but we can infer a value from vcsr */
+      uint64_t vxsat = ((vregs->vstate.vcsr >> VCSR_POS_VXSAT) & VCSR_MASK_VXSAT);
+      buf = (gdb_byte *) & vxsat;
+      regcache->raw_supply (RISCV_CSR_VXSAT_REGNUM, buf);
+    }
+
+  if (regnum == -1 || regnum == RISCV_CSR_VXRM_REGNUM)
+    {
+      /*  this CSR is not part of vregs->vstate literally, but we can infer a value from vcsr */
+      uint64_t vxrm = ((vregs->vstate.vcsr >> VCSR_POS_VXRM) & VCSR_MASK_VXRM);
+      buf = (gdb_byte *) & vxrm;
+      regcache->raw_supply (RISCV_CSR_VXRM_REGNUM, buf);
+    }
+}
+
+/* Collect RISC-V vector register values (including inferred CSRs) from the GDB regcache.  */
+static void
+fill_vregset (const struct regcache *regcache, struct __riscv_vregs *vregs,
+	      int regnum)
+{
+  regcache_collect_regset (&riscv_linux_vregset, regcache, regnum, vregs, sizeof(*vregs));    
+
+  if (regnum == -1 || regnum == RISCV_CSR_VCSR_REGNUM || regnum == RISCV_CSR_VXSAT_REGNUM
+      || regnum == RISCV_CSR_VXRM_REGNUM)
+    {
+      uint64_t vxsat_from_regcache;
+      uint64_t vxrm_from_regcache;      
+
+      if ( ! (regnum == -1 || regnum == RISCV_CSR_VCSR_REGNUM) )
+	{
+	  /* We don't already have the VCSR value, from the earlier regcache_collect_regset call, so let's get it now.  */
+	  regcache_collect_regset (&riscv_linux_vregset, regcache, RISCV_CSR_VCSR_REGNUM, vregs, sizeof(*vregs));    	  
+	}
+
+      if (regnum == RISCV_CSR_VXSAT_REGNUM)
+	{
+	  /* Overwrite VCSR with the VXSAT bit here.  */
+          gdb_byte *buf = (gdb_byte *) &vxsat_from_regcache;
+	  regcache->raw_collect (RISCV_CSR_VXSAT_REGNUM, buf);
+	  vregs->vstate.vcsr &= ~((uint64_t) VCSR_MASK_VXSAT << VCSR_POS_VXSAT);
+	  vregs->vstate.vcsr |= ((vxsat_from_regcache & VCSR_MASK_VXSAT) << VCSR_POS_VXSAT);
+	}
+
+      if (regnum == RISCV_CSR_VXRM_REGNUM)
+	{
+	  /* Overwrite VCSR with the VXRM bit here.  */
+          gdb_byte *buf = (gdb_byte *) &vxsat_from_regcache;
+	  regcache->raw_collect (RISCV_CSR_VXRM_REGNUM, buf);
+	  vregs->vstate.vcsr &= ~((uint64_t) VCSR_MASK_VXRM << VCSR_POS_VXRM);	  
+	  vregs->vstate.vcsr |= ((vxrm_from_regcache & VCSR_MASK_VXRM) << VCSR_POS_VXRM);
+	}
+      
+    }
+
+  /* VLENB register is not writable, so that's why nothing is collected here for that register.  */
+
+}
+
+
 /* Copy all floating point registers from regset FPREGS into REGCACHE.  */
 
 void
@@ -254,6 +363,31 @@ riscv_linux_nat_target::fetch_registers (struct regcache *regcache, int regnum)
 	supply_fpregset_regnum (regcache, &regs, regnum);
     }
 
+  /* if Linux kernel was not configured to support RISC-V vectors, then
+     the ptrace call will return -1, and we just won't get vector registers,
+     but in that case it wouldn't be an error that needs user attention.
+   */
+  if ((regnum >= RISCV_V0_REGNUM && regnum <= RISCV_V31_REGNUM)
+      || (regnum == RISCV_CSR_VSTART_REGNUM)
+      || (regnum == RISCV_CSR_VL_REGNUM)
+      || (regnum == RISCV_CSR_VTYPE_REGNUM)
+      || (regnum == RISCV_CSR_VCSR_REGNUM)
+      || (regnum == RISCV_CSR_VLENB_REGNUM)
+      || (regnum == RISCV_CSR_VXSAT_REGNUM)
+      || (regnum == RISCV_CSR_VXRM_REGNUM)
+      || (regnum == -1))
+    {
+      struct iovec iov;
+      struct __riscv_vregs vregs;
+
+      iov.iov_base = &vregs;
+      iov.iov_len = sizeof (vregs);
+
+      if (ptrace (PTRACE_GETREGSET, tid, NT_RISCV_VECTOR,
+		  (PTRACE_TYPE_ARG3) & iov) == 0)
+	supply_vregset_regnum (regcache, &vregs, regnum);
+    }
+
   if ((regnum == RISCV_CSR_MISA_REGNUM)
       || (regnum == -1))
     {
@@ -323,6 +457,35 @@ riscv_linux_nat_target::store_registers (struct regcache *regcache, int regnum)
 	}
     }
 
+  /* VLENB isn't writable, so we'll skip considering that one, if it's being
+     specified alone */
+  if ((regnum >= RISCV_V0_REGNUM && regnum <= RISCV_V31_REGNUM)
+      || (regnum == RISCV_CSR_VSTART_REGNUM)
+      || (regnum == RISCV_CSR_VL_REGNUM)
+      || (regnum == RISCV_CSR_VTYPE_REGNUM)
+      || (regnum == RISCV_CSR_VCSR_REGNUM)
+      || (regnum == RISCV_CSR_VXSAT_REGNUM)
+      || (regnum == RISCV_CSR_VXRM_REGNUM)
+      || (regnum == -1))
+    {
+      struct iovec iov;
+      struct __riscv_vregs vregs;
+
+      iov.iov_base = &vregs;
+      iov.iov_len = sizeof (vregs);
+
+      if (ptrace (PTRACE_GETREGSET, tid, NT_RISCV_VECTOR,
+		  (PTRACE_TYPE_ARG3) & iov) == 0)
+	{
+	  fill_vregset (regcache, &vregs, regnum);
+
+	  if (ptrace (PTRACE_SETREGSET, tid, NT_RISCV_VECTOR,
+		      (PTRACE_TYPE_ARG3) & iov) == -1)
+	    perror_with_name (_("Couldn't set vector registers"));
+	}
+    }
+
+
   /* Access to CSRs has potential security issues, don't support them for
      now.  */
 }
diff --git a/gdb/riscv-linux-tdep.c b/gdb/riscv-linux-tdep.c
index 4c0c65c1457..44c7d7c8115 100644
--- a/gdb/riscv-linux-tdep.c
+++ b/gdb/riscv-linux-tdep.c
@@ -31,6 +31,10 @@
 
 #define RISCV_NR_rt_sigreturn 139
 
+/* Magic number written to the head.magic field of struct __sc_riscv_v_state that kernel
+   places in the reserved area of struct sigcontext.  Comes from <asm/sigcontext.h> */
+#define RVV_MAGIC 0x53465457
+
 /* Define the general register mapping.  The kernel puts the PC at offset 0,
    gdb puts it at offset 32.  Register x0 is always 0 and can be ignored.
    Registers x1 to x31 are in the same place.  */
@@ -119,8 +123,123 @@ static const struct tramp_frame riscv_linux_sigframe = {
      mcontext_t uc_mcontext;
    }; */
 
+
+/* Read RVV magic and size fields from address REGS_BASE and return true if a valid
+   RISC-V vector context header is found, otherwise return false. THIS_FRAME is used
+   for the architecture and byte-order when reading memory. VLEN and XLEN are the
+   v-register and x-register sizes (in bytes) and are used for validation.  */
+
+static bool
+riscv_linux_vector_sigframe_header_check (frame_info_ptr this_frame,
+					  int vlen, int xlen,
+					  CORE_ADDR regs_base)
+{
+  uint32_t rvv_magic;
+  uint32_t rvv_size;
+  bool info_good = false;
+
+  /* If vector information is available, then we should see this structure at this address:
+     struct __riscv_ctx_hdr {
+     __u32 magic;  (RVV_MAGIC).
+     __u32 size;   (size of struct __sc_riscv_v_state + vector register data size (32*VLENB))
+     } head;
+   */
+
+  rvv_magic =
+    get_frame_memory_unsigned (this_frame, regs_base, sizeof (rvv_magic));
+  regs_base += sizeof (rvv_magic);
+  rvv_size =
+    get_frame_memory_unsigned (this_frame, regs_base, sizeof (rvv_magic));
+  regs_base += sizeof (rvv_size);
+
+
+  info_good = (rvv_magic == RVV_MAGIC);
+  if (!info_good)
+    {
+      /* Not an error, because kernels can be configured without CONFIG_VECTOR, but worth noting if frame debug
+         setting is turned on */
+      frame_debug_printf
+        ("Did not find RISC-V vector information in ucontext (kernel not built with CONFIG_VECTOR?)");
+
+      return false;
+    }
+
+  if (frame_debug)
+    {
+      uint32_t expected_rvv_size;
+
+      frame_debug_printf
+	("Located RISC-V vector information in signal frame ucontext (info size %u)",
+	 rvv_size);
+
+      /* sanity check the reported size; should be sizeof(uint32_t) + sizeof(uint32_t) + 5 * XLENB + 32 * vlen */
+      expected_rvv_size = sizeof (uint32_t) /* magic */  +
+	sizeof (uint32_t) /* size */  +
+	5 * xlen /* vstart, vl, vtype, vcsr, and datap */  +
+	32 * vlen;		/* v0..v31 values */
+
+      if (rvv_size != expected_rvv_size)
+	{
+          warning (_("Size in RISC-V vector information header in ucontext (%u) differs from expected size (%u)."),
+             rvv_size, expected_rvv_size);
+
+          /* Additional debug details if frame_debug is on.  */
+	  frame_debug_printf ("Detailed size mismatch: expected %u based on VLEN=%d and XLEN=%d",
+             expected_rvv_size, vlen, xlen);
+	}
+    }
+
+  return info_good;
+}
+
+static CORE_ADDR
+riscv_linux_sigframe_vector_init (frame_info_ptr this_frame,
+				  struct trad_frame_cache *this_cache,
+				  CORE_ADDR regs_base, int xlen, int vlen)
+{
+  int vfieldidx;
+  CORE_ADDR p_datap;
+  CORE_ADDR datap;
+
+  /* vstart, vl, vtype, vcsr, and datap are XLEN sized fields (unsigned long) from this point.  */
+  vfieldidx = 0;
+  trad_frame_set_reg_addr (this_cache, RISCV_CSR_VSTART_REGNUM,
+			   regs_base + (vfieldidx * xlen));
+  vfieldidx++;
+  trad_frame_set_reg_addr (this_cache, RISCV_CSR_VL_REGNUM,
+			   regs_base + (vfieldidx * xlen));
+
+  vfieldidx++;
+  trad_frame_set_reg_addr (this_cache, RISCV_CSR_VTYPE_REGNUM,
+			   regs_base + (vfieldidx * xlen));
+
+  vfieldidx++;
+  trad_frame_set_reg_addr (this_cache, RISCV_CSR_VCSR_REGNUM,
+			   regs_base + (vfieldidx * xlen));
+
+  /* for the datap member, there is one level of memory indirection to get the address of
+     the block of values for v0..v31 */
+  vfieldidx++;
+  p_datap = regs_base + (vfieldidx * xlen);
+  datap = get_frame_memory_unsigned (this_frame, p_datap, xlen);
+  regs_base = datap;
+  for (int i = 0; i < 32; i++)
+    {
+      trad_frame_set_reg_addr (this_cache, RISCV_V0_REGNUM + i,
+			       regs_base + (i * vlen));
+    }
+  regs_base += 32 * vlen;
+
+  return regs_base;
+}
+
+
 #define SIGFRAME_SIGINFO_SIZE		128
 #define UCONTEXT_MCONTEXT_OFFSET	176
+#define MCONTEXT_VECTOR_OFFSET		784	/* offset of struct mcontext's __reserved field,
+						   which is where the struct __sc_riscv_v_state is overlaid */
+#define RISCV_CONTEXT_HEADER_SIZE	8	/* size of struct __riscv_ctx_hdr {__u32 magic;  __u32 size; } */
+
 
 static void
 riscv_linux_sigframe_init (const struct tramp_frame *self,
@@ -131,6 +250,7 @@ riscv_linux_sigframe_init (const struct tramp_frame *self,
   struct gdbarch *gdbarch = get_frame_arch (this_frame);
   int xlen = riscv_isa_xlen (gdbarch);
   int flen = riscv_isa_flen (gdbarch);
+  int vlen = riscv_isa_vlen (gdbarch);
   CORE_ADDR frame_sp = get_frame_sp (this_frame);
   CORE_ADDR mcontext_base;
   CORE_ADDR regs_base;
@@ -154,6 +274,19 @@ riscv_linux_sigframe_init (const struct tramp_frame *self,
   regs_base += 32 * flen;
   trad_frame_set_reg_addr (this_cache, RISCV_CSR_FCSR_REGNUM, regs_base);
 
+  /* Handle the vector registers, if present. */
+  if (vlen > 0)
+    {
+      regs_base = mcontext_base + MCONTEXT_VECTOR_OFFSET;
+      if (riscv_linux_vector_sigframe_header_check
+	  (this_frame, vlen, xlen, regs_base))
+	{
+	  regs_base += RISCV_CONTEXT_HEADER_SIZE;	/* advance past the header */
+	  riscv_linux_sigframe_vector_init (this_frame, this_cache, regs_base,
+					    xlen, vlen);
+	}
+    }
+
   /* Choice of the bottom of the sigframe is somewhat arbitrary.  */
   trad_frame_set_id (this_cache, frame_id_build (frame_sp, func));
 }
diff --git a/gdb/riscv-tdep.c b/gdb/riscv-tdep.c
index 91f6dffebe1..eb276d55f80 100644
--- a/gdb/riscv-tdep.c
+++ b/gdb/riscv-tdep.c
@@ -47,6 +47,7 @@
 #include "remote.h"
 #include "target-descriptions.h"
 #include "dwarf2/frame.h"
+#include "dwarf2/expr.h"
 #include "user-regs.h"
 #include "valprint.h"
 #include "opcode/riscv-opc.h"
@@ -650,6 +651,14 @@ struct riscv_vector_feature : public riscv_register_feature
       { RISCV_V0_REGNUM + 29, { "v29" } },
       { RISCV_V0_REGNUM + 30, { "v30" } },
       { RISCV_V0_REGNUM + 31, { "v31" } },
+      /* vector CSRs */
+      { RISCV_CSR_VSTART_REGNUM, { "vstart" } },
+      { RISCV_CSR_VXSAT_REGNUM, { "vxsat" } },
+      { RISCV_CSR_VXRM_REGNUM, { "vxrm" } },
+      { RISCV_CSR_VL_REGNUM, { "vl" } },
+      { RISCV_CSR_VTYPE_REGNUM, { "vtype" } },
+      { RISCV_CSR_VCSR_REGNUM, { "vcsr" } },
+      { RISCV_CSR_VLENB_REGNUM, { "vlenb" } },
     };
   }
 
@@ -681,10 +690,16 @@ struct riscv_vector_feature : public riscv_register_feature
 	return true;
       }
 
-    /* Check all of the vector registers are present.  */
+    /* Check all of the vector registers are present.  We also
+       check that the vector CSRs are present too, though if these
+       are missing this is not fatal.  */
     for (const auto &reg : m_registers)
       {
-	if (!reg.check (tdesc_data, feature_vector, aliases))
+       bool found = reg.check (tdesc_data, feature_vector, aliases);
+
+       bool is_ctrl_reg_p = !(reg.regnum >= RISCV_V0_REGNUM && reg.regnum <= RISCV_V31_REGNUM);
+
+       if (!found && !is_ctrl_reg_p)
 	  return false;
       }
 
@@ -694,6 +709,12 @@ struct riscv_vector_feature : public riscv_register_feature
     int vector_bitsize = -1;
     for (const auto &reg : m_registers)
       {
+
+	bool is_ctrl_reg_p = !(reg.regnum >= RISCV_V0_REGNUM && reg.regnum <= RISCV_V31_REGNUM);	
+
+	if (is_ctrl_reg_p)
+	  continue;
+
 	int reg_bitsize = -1;
 	for (const char *name : reg.names)
 	  {
@@ -816,6 +837,16 @@ riscv_abi_embedded (struct gdbarch *gdbarch)
   return tdep->abi_features.embedded;
 }
 
+/* See riscv-tdep.h.  */
+
+int
+riscv_isa_vlen (struct gdbarch *gdbarch)
+{
+  riscv_gdbarch_tdep *tdep = gdbarch_tdep<riscv_gdbarch_tdep> (gdbarch);
+  return tdep->isa_features.vlen;
+}
+
+
 /* Return true if the target for GDBARCH has floating point hardware.  */
 
 static bool
@@ -1467,7 +1498,19 @@ riscv_register_reggroup_p (struct gdbarch  *gdbarch, int regnum,
       return 0;
     }
   else if (reggroup == vector_reggroup)
-    return (regnum >= RISCV_V0_REGNUM && regnum <= RISCV_V31_REGNUM);
+    {
+      if (regnum >= RISCV_V0_REGNUM && regnum <= RISCV_V31_REGNUM)
+	return 1;
+      if (regnum == RISCV_CSR_VSTART_REGNUM
+	  || regnum == RISCV_CSR_VXSAT_REGNUM
+	  || regnum == RISCV_CSR_VXRM_REGNUM
+	  || regnum == RISCV_CSR_VL_REGNUM
+	  || regnum == RISCV_CSR_VTYPE_REGNUM
+	  || regnum == RISCV_CSR_VCSR_REGNUM
+	  || regnum == RISCV_CSR_VLENB_REGNUM)
+	return 1;
+      return 0;
+    }
   else
     return 0;
 }
diff --git a/gdb/riscv-tdep.h b/gdb/riscv-tdep.h
index ad1e9596b83..7b41dfbdcbc 100644
--- a/gdb/riscv-tdep.h
+++ b/gdb/riscv-tdep.h
@@ -46,6 +46,15 @@ enum
   RISCV_LAST_FP_REGNUM = 64,	/* Last Floating Point Register */
 
   RISCV_FIRST_CSR_REGNUM = 65,  /* First CSR */
+
+  RISCV_VSTART = 73,    /* Vector start position.  */
+  RISCV_VXSAT = 74,     /* Fixed-Point Saturate Flag.  */
+  RISCV_VXRM = 75,      /* Fixed-Point Rounding Mode.  */  
+  RISCV_VCSR = 80,      /* Vector control and status register.  */
+  RISCV_VL = 3169,      /* Vector length.  */
+  RISCV_VTYPE = 3170,    /* Vector data type register.  */
+  RISCV_VLENB = 3171,    /* VLEN/8 (vector register length in bytes) */
+
 #define DECLARE_CSR(name, num, class, define_version, abort_version) \
   RISCV_ ## num ## _REGNUM = RISCV_FIRST_CSR_REGNUM + num,
 #include "opcode/riscv-opc.h"
@@ -150,6 +159,11 @@ extern int riscv_abi_flen (struct gdbarch *gdbarch);
    argument registers.  */
 extern bool riscv_abi_embedded (struct gdbarch *gdbarch);
 
+/* Return the width in bytes of the hardware vector registers for
+   GDBARCH.  If this architecture has no vector registers, then
+   return 0.  */
+extern int riscv_isa_vlen (struct gdbarch *gdbarch);
+
 /* Single step based on where the current instruction will take us.  */
 extern std::vector<CORE_ADDR> riscv_software_single_step
   (struct regcache *regcache);
diff --git a/gdbserver/linux-riscv-low.cc b/gdbserver/linux-riscv-low.cc
index 8c742f406a2..ac83d6b0808 100644
--- a/gdbserver/linux-riscv-low.cc
+++ b/gdbserver/linux-riscv-low.cc
@@ -161,6 +161,113 @@ riscv_store_fpregset (struct regcache *regcache, const void *buf)
   supply_register_by_name (regcache, "fcsr", regbuf);
 }
 
+/* Collect vector registers from REGCACHE into BUF.  */
+
+static void
+riscv_fill_vregset (struct regcache *regcache, void *buf)
+{
+  const struct target_desc *tdesc = regcache->tdesc;
+  int regno = find_regno (tdesc, "v0");
+  int vlenb = register_size (regcache->tdesc, regno);
+  uint64_t u64_vlenb = vlenb;	/* pad to max XLEN for buffer conversion */
+  uint64_t u64_vxsat = 0;
+  uint64_t u64_vxrm = 0;
+  uint64_t u64_vcsr = 0;
+  gdb_byte *regbuf;
+  int i;
+
+  /* Since vxsat and equivalent bits in vcsr are aliases (and same for vxrm), we have a dilemma.
+     For this gdb -> gdbserver topology, if the aliased pairs have values that disagree, then
+     which value should take precedence?  We don't know which alias was most
+     recently assigned.  We're just getting a block of register values including vxsat, vxrm,
+     and vcsr.  We have to impose some kind of rule for predictable resolution to resolve any inconsistency.
+     For now, let's say that vxsat and vxrm take precedence, and those values will be applied to the
+     corresponding fields in vcsr.  Reconcile these 3 interdependent registers now:
+  */
+  regbuf = (gdb_byte *) & u64_vcsr;
+  collect_register_by_name (regcache, "vcsr", regbuf);
+  regbuf = (gdb_byte *) & u64_vxsat;
+  collect_register_by_name (regcache, "vxsat", regbuf);
+  regbuf = (gdb_byte *) & u64_vxrm;
+  collect_register_by_name (regcache, "vxrm", regbuf);
+  
+  u64_vcsr &= ~((uint64_t)VCSR_MASK_VXSAT << VCSR_POS_VXSAT);
+  u64_vcsr |= ((u64_vxsat & VCSR_MASK_VXSAT) << VCSR_POS_VXSAT);
+  u64_vcsr &= ~((uint64_t)VCSR_MASK_VXRM << VCSR_POS_VXRM);	  
+  u64_vcsr |= ((u64_vxrm & VCSR_MASK_VXRM) << VCSR_POS_VXRM);
+
+  /* Replace the original vcsr value with the "cooked" value */
+  regbuf = (gdb_byte *) & u64_vcsr;  
+  supply_register_by_name (regcache, "vcsr", regbuf);
+
+  /* Now stage the ptrace buffer (it'll receive the cooked vcsr value) */
+
+  regbuf = (gdb_byte *) buf + offsetof (struct __riscv_vregs, vstate.vstart);
+  collect_register_by_name (regcache, "vstart", regbuf);
+  regbuf = (gdb_byte *) buf + offsetof (struct __riscv_vregs, vstate.vl);
+  collect_register_by_name (regcache, "vl", regbuf);
+  regbuf = (gdb_byte *) buf + offsetof (struct __riscv_vregs, vstate.vtype);
+  collect_register_by_name (regcache, "vtype", regbuf);
+  regbuf = (gdb_byte *) buf + offsetof (struct __riscv_vregs, vstate.vcsr);
+  collect_register_by_name (regcache, "vcsr", regbuf);
+  regbuf = (gdb_byte *) & u64_vlenb;
+  collect_register_by_name (regcache, "vlenb", regbuf);
+
+
+  regbuf = (gdb_byte *) buf + offsetof (struct __riscv_vregs, data);
+  for (i = 0; i < 32; i++, regbuf += vlenb)
+    collect_register (regcache, regno + i, regbuf);
+}
+
+/* Supply vector registers from BUF into REGCACHE.  */
+
+static void
+riscv_store_vregset (struct regcache *regcache, const void *buf)
+{
+  const struct target_desc *tdesc = regcache->tdesc;
+  int regno = find_regno (tdesc, "v0");
+  int vlenb = register_size (regcache->tdesc, regno);
+  uint64_t u64_vlenb = vlenb;	/* pad to max XLEN for buffer conversion */
+  uint64_t vcsr;
+  uint64_t vxsat;
+  uint64_t vxrm;  
+  const gdb_byte *regbuf;
+  int i;
+
+  regbuf =
+    (const gdb_byte *) buf + offsetof (struct __riscv_vregs, vstate.vstart);
+  supply_register_by_name (regcache, "vstart", regbuf);
+  regbuf =
+    (const gdb_byte *) buf + offsetof (struct __riscv_vregs, vstate.vl);
+  supply_register_by_name (regcache, "vl", regbuf);
+  regbuf =
+    (const gdb_byte *) buf + offsetof (struct __riscv_vregs, vstate.vtype);
+  supply_register_by_name (regcache, "vtype", regbuf);
+  regbuf =
+    (const gdb_byte *) buf + offsetof (struct __riscv_vregs, vstate.vcsr);
+  supply_register_by_name (regcache, "vcsr", regbuf);
+  /* also store off a non-byte-wise copy of vcsr, to derive values for vxsat and vxrm */
+  vcsr = *(uint64_t*)regbuf;
+  /* vlenb isn't part of vstate, but we have already inferred its value by running code on this
+     hart, and we're assuming homogeneous VLENB if it's an SMP system */
+  regbuf = (gdb_byte *) & u64_vlenb;
+  supply_register_by_name (regcache, "vlenb", regbuf);
+
+  /* vxsat and vxrm, are not part of vstate, so we have to extract from VCSR
+     value */
+  vxsat = ((vcsr >> VCSR_POS_VXSAT) & VCSR_MASK_VXSAT);  
+  regbuf = (gdb_byte *) &vxsat;
+  supply_register_by_name (regcache, "vxsat", regbuf);
+  vxrm = ((vcsr >> VCSR_POS_VXRM) & VCSR_MASK_VXRM);  
+  regbuf = (gdb_byte *) &vxrm;
+  supply_register_by_name (regcache, "vxrm", regbuf);
+
+  /* v0..v31 */
+  regbuf = (const gdb_byte *) buf + offsetof (struct __riscv_vregs, data);
+  for (i = 0; i < 32; i++, regbuf += vlenb)
+    supply_register (regcache, regno + i, regbuf);
+}
+
 /* RISC-V/Linux regsets.  FPRs are optional and come in different sizes,
    so define multiple regsets for them marking them all as OPTIONAL_REGS
    rather than FP_REGS, so that "regsets_fetch_inferior_registers" picks
@@ -178,6 +285,9 @@ static struct regset_info riscv_regsets[] = {
   { PTRACE_GETREGSET, PTRACE_SETREGSET, NT_FPREGSET,
     sizeof (struct __riscv_mc_f_ext_state), OPTIONAL_REGS,
     riscv_fill_fpregset, riscv_store_fpregset },
+  { PTRACE_GETREGSET, PTRACE_SETREGSET, NT_RISCV_VECTOR,
+    sizeof (struct __riscv_vregs), OPTIONAL_REGS,
+    riscv_fill_vregset, riscv_store_vregset },
   NULL_REGSET
 };
 
diff --git a/include/elf/common.h b/include/elf/common.h
index fd032d1e03e..7003f987b94 100644
--- a/include/elf/common.h
+++ b/include/elf/common.h
@@ -758,6 +758,7 @@
 					/*   note name must be "LINUX".  */
 #define NT_RISCV_CSR    0x900		/* RISC-V Control and Status Registers */
 					/*   note name must be "LINUX".  */
+#define NT_RISCV_VECTOR 0x901           /* RISC-V Vector Registers.  */
 #define NT_SIGINFO	0x53494749	/* Fields of siginfo_t.  */
 #define NT_FILE		0x46494c45	/* Description of mapped files.  */
 
-- 
2.43.0

Re: [PATCH] [PATCH v3] RISC-V: support for vector register accesses via ptrace() in RISC-V Linux native

[Andrew Burgess]

snatu@whileone.in writes:

> From: Sameer Natu <snatu@whileone.in>
>
> A v3 re-spin of the original patch.
> Tested with latest kernel 6.14.2 on RISCV QEMU.
> Removed Magic Numbers from v2 patch and worked on review comments of v2 patch.

Thanks for picking this up.  It would be amazing to see vector support
land in GDB.

I'm a little rushed right now, I'll try to do a proper review soon, but
I do have some immediate questions related to register numbering, see
below...

> diff --git a/gdb/arch/riscv.c b/gdb/arch/riscv.c
> index a6188ea3a8c..14fc85631e3 100644
> --- a/gdb/arch/riscv.c
> +++ b/gdb/arch/riscv.c
> @@ -25,12 +25,38 @@
>  #include "../features/riscv/64bit-fpu.c"
>  #include "../features/riscv/rv32e-xregs.c"
>  
> +#include "opcode/riscv-opc.h"
> +
>  #ifndef GDBSERVER
>  #define STATIC_IN_GDB static
>  #else
>  #define STATIC_IN_GDB
>  #endif
>  
> +#ifdef GDBSERVER
> +/* Work around issue where trying to include riscv-tdep.h (to get access to canonical RISCV_V0_REGNUM declaration
> +   from that header) is problamtic for gdbserver build.  */
> +//#include "riscv-tdep.h"
> +#define RISCV_VSTART 73
> +#define RISCV_VXSAT 74
> +#define RISCV_VXRM 75
> +#define RISCV_VCSR 80
> +#define RISCV_VL 3169 
> +#define RISCV_VTYPE 3170
> +#define RISCV_VLENB 3171
> +#define RISCV_V0_REGNUM 4162   
> +#else
> +#include "riscv-tdep.h"
> +#include "defs.h"
> +#endif

I'm curious why these constants are needed in here at all?  It's been a
while since I worked on this corner of the code, but my understanding is
that the register numbering in the target description shouldn't have to
match any particular number at all, it's just a unique id to tell the
registers apart.

The riscv-tdep.c file will then map the unique id assigned here to GDB's
internal register numbering.

Now, there is a bit of a nit here; the existing xml files do include a
fixed numbering, and this was to work around some issues with early tool
versions that didn't send an XML target description, but instead assumed
a fixed numbering.  Thus, they relied on GDB always asking for register
number X when asking for e.g. fflags register.

My understanding is that QEMU has correctly been sending XML
descriptions for risc-v for a while now, so the fixed register numbering
should no longer be needed, but critically, I believe any version of
QEMU that has vector register support, has XML target description
support, so we really should be free to use any register numbering we
like here.

Like I said, it's been a while, so maybe I'm forgetting something.  If I
am, could you explain more why the fixed numbering is needed here.

> diff --git a/gdb/riscv-tdep.h b/gdb/riscv-tdep.h
> index ad1e9596b83..7b41dfbdcbc 100644
> --- a/gdb/riscv-tdep.h
> +++ b/gdb/riscv-tdep.h
> @@ -46,6 +46,15 @@ enum
>    RISCV_LAST_FP_REGNUM = 64,	/* Last Floating Point Register */
>  
>    RISCV_FIRST_CSR_REGNUM = 65,  /* First CSR */
> +
> +  RISCV_VSTART = 73,    /* Vector start position.  */
> +  RISCV_VXSAT = 74,     /* Fixed-Point Saturate Flag.  */
> +  RISCV_VXRM = 75,      /* Fixed-Point Rounding Mode.  */  
> +  RISCV_VCSR = 80,      /* Vector control and status register.  */
> +  RISCV_VL = 3169,      /* Vector length.  */
> +  RISCV_VTYPE = 3170,    /* Vector data type register.  */
> +  RISCV_VLENB = 3171,    /* VLEN/8 (vector register length in bytes) */
> +
>  #define DECLARE_CSR(name, num, class, define_version, abort_version) \
>    RISCV_ ## num ## _REGNUM = RISCV_FIRST_CSR_REGNUM + num,

Notice that RISCV_FIRST_CSR_REGNUM is 65, so vstart -> vcsr will overlap
with the RISCV_CSR_*_REGNUM constants.  Why not use the existing
constants?

Then vl/vtype/vlenb don't seem to match with the existing CSR constants,
unless I'm doing something wrong.  What's that all about?

What I suspect here is that this is evidence that what I say above
(about not neededing fixed numbering) is correct.  In the generated
target description we're using these incorrect(?) register numbers.  But
in riscv-tdep.c we map these to the correct CSR numbers.  Fetching
register state will be done using the actual CSR number, so it'll all
work out.

Anyway, any additional insights into the register numbering in this
patch would be great.

Thanks,
Andrew

Re: [PATCH] [PATCH v3] RISC-V: support for vector register accesses via ptrace() in RISC-V Linux native

[Charlie Jenkins]

On Thu, Apr 24, 2025 at 12:19:14PM +0000, snatu@whileone.in wrote:
> From: Sameer Natu <snatu@whileone.in>
> 
> A v3 re-spin of the original patch.
> Tested with latest kernel 6.14.2 on RISCV QEMU.
> Removed Magic Numbers from v2 patch and worked on review comments of v2 patch.

Thanks for working on this!

Can you add a co-developed-by tag for the original author?

You also don't need to have [PATCH] twice in the header!

There are a handful of erroneous spaces at the end of lines.

I tested this patch and I noticed that the vector instructions are not
being decoded.

Breakpoint 1, vector () at main.S:4
4               vsetvli t0, a0, e32, m4, ta, ma
1: x/i $pc
=> 0x55555555566c <vector>:     .insn   4, 0x0d2572d

> 
> ---
>  gdb/arch/riscv.c             | 188 ++++++++++++++++++++++++++++++++++-
>  gdb/nat/riscv-linux-tdesc.c  |  68 +++++++++++++
>  gdb/nat/riscv-linux-tdesc.h  |  24 +++++
>  gdb/riscv-linux-nat.c        | 163 ++++++++++++++++++++++++++++++
>  gdb/riscv-linux-tdep.c       | 133 +++++++++++++++++++++++++
>  gdb/riscv-tdep.c             |  49 ++++++++-
>  gdb/riscv-tdep.h             |  14 +++
>  gdbserver/linux-riscv-low.cc | 110 ++++++++++++++++++++
>  include/elf/common.h         |   1 +
>  9 files changed, 743 insertions(+), 7 deletions(-)
> 
> diff --git a/gdb/arch/riscv.c b/gdb/arch/riscv.c
> index a6188ea3a8c..14fc85631e3 100644
> --- a/gdb/arch/riscv.c
> +++ b/gdb/arch/riscv.c
> @@ -25,12 +25,38 @@
>  #include "../features/riscv/64bit-fpu.c"
>  #include "../features/riscv/rv32e-xregs.c"
>  
> +#include "opcode/riscv-opc.h"
> +
>  #ifndef GDBSERVER
>  #define STATIC_IN_GDB static
>  #else
>  #define STATIC_IN_GDB
>  #endif
>  
> +#ifdef GDBSERVER
> +/* Work around issue where trying to include riscv-tdep.h (to get access to canonical RISCV_V0_REGNUM declaration
> +   from that header) is problamtic for gdbserver build.  */
> +//#include "riscv-tdep.h"
> +#define RISCV_VSTART 73
> +#define RISCV_VXSAT 74
> +#define RISCV_VXRM 75
> +#define RISCV_VCSR 80
> +#define RISCV_VL 3169 
> +#define RISCV_VTYPE 3170
> +#define RISCV_VLENB 3171
> +#define RISCV_V0_REGNUM 4162   
> +#else
> +#include "riscv-tdep.h"
> +#include "defs.h"
> +#endif
> +

As Andrew said, it seems valid to delete these hard-coded addresses.


> +static int
> +create_feature_riscv_vector_from_features (struct target_desc *result,
> +					   long regnum,
> +					   const struct riscv_gdbarch_features
> +					   features);
> +
> +
>  /* See arch/riscv.h.  */
>  
>  STATIC_IN_GDB target_desc_up
> @@ -83,15 +109,169 @@ riscv_create_target_description (const struct riscv_gdbarch_features features)
>    else if (features.flen == 8)
>      regnum = create_feature_riscv_64bit_fpu (tdesc.get (), regnum);
>  
> -  /* Currently GDB only supports vector features coming from remote
> -     targets.  We don't support creating vector features on native targets
> -     (yet).  */
>    if (features.vlen != 0)
> -    error (_("unable to create vector feature"));
> +    regnum =
> +      create_feature_riscv_vector_from_features (tdesc.get (),
> +						 RISCV_V0_REGNUM, features);
>  

After deleting the block, RISCV_V0_REGNUM can be replaced with regnum.

- Charlie

Re: [PATCH] [PATCH v3] RISC-V: support for vector register accesses via ptrace() in RISC-V Linux native

[Andrew Burgess]

Charlie Jenkins <charlie@rivosinc.com> writes:

> On Thu, Apr 24, 2025 at 12:19:14PM +0000, snatu@whileone.in wrote:
>> From: Sameer Natu <snatu@whileone.in>
>> 
>> A v3 re-spin of the original patch.
>> Tested with latest kernel 6.14.2 on RISCV QEMU.
>> Removed Magic Numbers from v2 patch and worked on review comments of v2 patch.
>
> Thanks for working on this!
>
> Can you add a co-developed-by tag for the original author?
>
> You also don't need to have [PATCH] twice in the header!
>
> There are a handful of erroneous spaces at the end of lines.
>
> I tested this patch and I noticed that the vector instructions are not
> being decoded.
>
> Breakpoint 1, vector () at main.S:4
> 4               vsetvli t0, a0, e32, m4, ta, ma
> 1: x/i $pc
> => 0x55555555566c <vector>:     .insn   4, 0x0d2572d

Does an `objdump` built from the same tree decode these instructions?
Wondering if this is a problem with the objdump disassembler library, or
a problem with the way GDB uses that library.

Thanks,
Andrew

Re: [PATCH] [PATCH v3] RISC-V: support for vector register accesses via ptrace() in RISC-V Linux native

[Charlie Jenkins]

On Mon, Apr 28, 2025 at 06:12:16PM +0100, Andrew Burgess wrote:
> Charlie Jenkins <charlie@rivosinc.com> writes:
> 
> > On Thu, Apr 24, 2025 at 12:19:14PM +0000, snatu@whileone.in wrote:
> >> From: Sameer Natu <snatu@whileone.in>
> >> 
> >> A v3 re-spin of the original patch.
> >> Tested with latest kernel 6.14.2 on RISCV QEMU.
> >> Removed Magic Numbers from v2 patch and worked on review comments of v2 patch.
> >
> > Thanks for working on this!
> >
> > Can you add a co-developed-by tag for the original author?
> >
> > You also don't need to have [PATCH] twice in the header!
> >
> > There are a handful of erroneous spaces at the end of lines.
> >
> > I tested this patch and I noticed that the vector instructions are not
> > being decoded.
> >
> > Breakpoint 1, vector () at main.S:4
> > 4               vsetvli t0, a0, e32, m4, ta, ma
> > 1: x/i $pc
> > => 0x55555555566c <vector>:     .insn   4, 0x0d2572d
> 
> Does an `objdump` built from the same tree decode these instructions?
> Wondering if this is a problem with the objdump disassembler library, or
> a problem with the way GDB uses that library.

Yes, `objdump -d` from the same tree dumps the instruction. Perhaps some
entry needs to be added to `riscv_gdbarch_init()` in gdb/riscv-tdep.c?

- Charlie

> 
> Thanks,
> Andrew
> 

Re: [PATCH] [PATCH v3] RISC-V: support for vector register accesses via ptrace() in RISC-V Linux native

[Charlie Jenkins]

On Mon, Apr 28, 2025 at 12:35:27PM -0700, Charlie Jenkins wrote:
> On Mon, Apr 28, 2025 at 06:12:16PM +0100, Andrew Burgess wrote:
> > Charlie Jenkins <charlie@rivosinc.com> writes:
> > 
> > > On Thu, Apr 24, 2025 at 12:19:14PM +0000, snatu@whileone.in wrote:
> > >> From: Sameer Natu <snatu@whileone.in>
> > >> 
> > >> A v3 re-spin of the original patch.
> > >> Tested with latest kernel 6.14.2 on RISCV QEMU.
> > >> Removed Magic Numbers from v2 patch and worked on review comments of v2 patch.
> > >
> > > Thanks for working on this!
> > >
> > > Can you add a co-developed-by tag for the original author?
> > >
> > > You also don't need to have [PATCH] twice in the header!
> > >
> > > There are a handful of erroneous spaces at the end of lines.
> > >
> > > I tested this patch and I noticed that the vector instructions are not
> > > being decoded.
> > >
> > > Breakpoint 1, vector () at main.S:4
> > > 4               vsetvli t0, a0, e32, m4, ta, ma
> > > 1: x/i $pc
> > > => 0x55555555566c <vector>:     .insn   4, 0x0d2572d
> > 
> > Does an `objdump` built from the same tree decode these instructions?
> > Wondering if this is a problem with the objdump disassembler library, or
> > a problem with the way GDB uses that library.
> 
> Yes, `objdump -d` from the same tree dumps the instruction. Perhaps some
> entry needs to be added to `riscv_gdbarch_init()` in gdb/riscv-tdep.c?

This isn't relevant for this patch so we can worry about this later :).

- Charlie

> 
> - Charlie
> 
> > 
> > Thanks,
> > Andrew
> >