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

[Charlie Jenkins <charlie@rivosinc.com>]

On Tue, May 06, 2025 at 12:37:09PM +0000, Sameer Natu wrote:
> From: Sameer Natu <snatu@whileone.in>
> 
> A v4 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.

I should have left this on earlier versions of this patch, but this
commit message isn't very useful. The commit message should contain what
the patch does (add vector registers and CSRs), while meta-data like this
would be more useful as a cover letter or as a note. You can leave this
as a note to this patch by placing this text under the "---".

The version numbers got messed up too, I believe this is v5, there were
2 v3s.

> 
> Co-Authored-By: Greg Savin <greg.savin@sifive.com>
> ---

Place the note here and add another "---" after:

---
>  gdb/arch/riscv.c             | 180 ++++++++++++++++++++++++++++++++++-
>  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, 726 insertions(+), 7 deletions(-)
> 
> diff --git a/gdb/arch/riscv.c b/gdb/arch/riscv.c
> index a6188ea3a8c..f1f5d83d60a 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

All of the uses of RISCV_V0_REGNUM in this file have been removed. This
definition along with this whole ifdef block can be removed.

Please remove this block. When you send the next version you can also
append my reviewed and testing tags after the co-authored-by tag since
these changes are small.

Reviewed-by: Charlie Jenkins <charlie@rivosinc.com>
Tested-by: Charlie Jenkins <charlie@rivosinc.com>

- Charlie