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

[Charlie Jenkins <charlie@rivosinc.com>]

On Mon, Apr 28, 2025 at 07:20:44AM +0000, Sameer Natu wrote:
> From: Sameer Natu <snatu@whileone.in>
> 
> A v3 re-spin of the original patch.

v4 now :)

> Tested with latest kernel 6.14.2 on RISCV QEMU.
> Removed Magic Numbers from v2 patch and worked on review comments of v2 patch.
> 
> Co-Authored-By: Greg Savin <greg.savin@sifive.com>
> 
> ---
>  gdb/arch/riscv.c             | 182 ++++++++++++++++++++++++++++++++++-
>  gdb/nat/riscv-linux-tdesc.c  |  68 +++++++++++++
>  gdb/nat/riscv-linux-tdesc.h  |  24 +++++
>  gdb/riscv-linux-nat.c        | 162 +++++++++++++++++++++++++++++++
>  gdb/riscv-linux-tdep.c       | 133 +++++++++++++++++++++++++
>  gdb/riscv-tdep.c             |  49 +++++++++-
>  gdb/riscv-tdep.h             |   6 ++
>  gdbserver/linux-riscv-low.cc | 110 +++++++++++++++++++++
>  include/elf/common.h         |   1 +
>  9 files changed, 728 insertions(+), 7 deletions(-)
> 
> diff --git a/gdb/arch/riscv.c b/gdb/arch/riscv.c
> index a6188ea3a8c..1a9699b8f79 100644
> --- a/gdb/arch/riscv.c
> +++ b/gdb/arch/riscv.c
> @@ -25,12 +25,30 @@
>  #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.  */
> +#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 +101,171 @@ 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 (),
> +						 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.  */
> +#if 0
> +  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");
> +#endif

This prevents gdb from printing the csrs! I meant that you should
replace all of the macros like "RISCV_VSTART" with "regnum++".

- Charlie