[PATCH v5 3/3] gdbserver: Add RISC-V/Linux support

Andrew Burgess andrew.burgess@embecosm.com
Sat Feb 8 00:17:00 GMT 2020


* Maciej W. Rozycki <macro@wdc.com> [2020-02-01 20:21:18 +0000]:

> Implement RISC-V/Linux support for both RV64 and RV32 systems, including 
> XML target description handling based on features determined, GPR and 
> FPR regset support including dynamic sizing of the latter, and software 
> breakpoint handling.  Define two NT_FPREGSET regsets of a different size 
> matching the FPR sizes supported for generic `gdbserver' code to pick 
> from according to what the OS supplies.
> 
> Also handle a glibc bug where ELF_NFPREG is defined in terms of NFPREG,
> however NFPREG is nowhere defined.
> 
> 	gdb/
> 	* arch/riscv.h (riscv_create_target_description): Remove `const' 
> 	qualifier from the return type.
> 	* arch/riscv.c (riscv_create_target_description): Likewise.
> 	* nat/riscv-linux-tdesc.h (riscv_linux_read_description): 
> 	Likewise.
> 	* nat/riscv-linux-tdesc.c (riscv_linux_read_description): 
> 	Likewise.
> 	* configure.tgt <riscv*-*-linux*>: Set build_gdbserver=yes.
> 
> 	gdb/gdbserver/
> 	* linux-riscv-low.c: New file.
> 	* Makefile.in (SFILES): Add linux-riscv-low.c, arch/riscv.c, and 
> 	nat/riscv-linux-tdesc.c.
> 	* configure.srv <riscv*-*-linux*> (srv_tgtobj)
> 	(srv_linux_regsets, srv_linux_usrregs, srv_linux_thread_db): 
> 	Define.

Maciej,

I took a look through, and I'm happy with the changes in
gdb/gdbserver/*.  But I wondered if we could try something slightly
different for the gdb/arch/* and gdb/nat/* changes.

I'll follow up to this mail with a patch that I'd like to apply before
we apply your patch #3, it tweaks the target description lookup API
slightly.  The result is that your patch #3 would drop all changes to
gdb/arch/ and gdb/nat/, then the riscv_arch_setup function would look
like this:

  static void
  riscv_arch_setup ()
  {
    static const char *expedite_regs[] = { "sp", "pc", NULL };

    const riscv_gdbarch_features features
      = riscv_linux_read_features (lwpid_of (current_thread));
    target_desc *tdesc = riscv_create_target_description (features);

    if (!tdesc->expedite_regs)
      init_target_desc (tdesc, expedite_regs);
    current_process ()->tdesc = tdesc;
  }

The feature lookup and target description fetch are now separate
calls.

The benefit of this is that GDB gets to retain the const in its API,
while gdbserver gets a non-const API to call.

If you're happy with this approach then feel free to push both these
patches, or let me know and I'll push them both.

Thanks,
Andrew




> ---
> Changes from v4:
> 
> - Add 128-bit FP regset.
> 
> Changes from v3:
> 
> - Actually include changes from v2.
> 
> Changes from v2:
> 
> - Advance through the buffer accessed in `riscv_fill_fpregset' and 
>   `riscv_store_fpregset' so as to avoid doing a variable multiplication in 
>   a loop; rename the `regset' variable to `regbuf'.
> 
> - Use OPTIONAL_REGS rather than FP_REGS as the type for the floating-point 
>   regsets.
> 
> - Add comments throughout.
> 
> Changes from v1:
> 
> - Make `gdbserver' selected for automatic build in a RISC-V/Linux/native
>   GDB configuration (thanks, Jim, for pointing this out!).
> 
> - Remove most of `riscv_arch_setup' and use `riscv_linux_read_description' 
>   from 2/3 instead.
> 
> - Stop using `elf_fpregset_t*' in favour to just a raw `gdb_byte *' buffer 
>   and size the regset according to the FPR size in `riscv_fill_fpregset' 
>   and `riscv_store_fpregset'.
> 
> - Define 2 NT_FPREGSET regsets of a different size for generic `gdbserver' 
>   code to pick from according to what the OS supplies.
> ---
>  gdb/arch/riscv.c                |    2 
>  gdb/arch/riscv.h                |    4 
>  gdb/configure.tgt               |    1 
>  gdb/gdbserver/Makefile.in       |    3 
>  gdb/gdbserver/configure.srv     |    7 +
>  gdb/gdbserver/linux-riscv-low.c |  277 ++++++++++++++++++++++++++++++++++++++++
>  gdb/nat/riscv-linux-tdesc.c     |    2 
>  gdb/nat/riscv-linux-tdesc.h     |    2 
>  8 files changed, 293 insertions(+), 5 deletions(-)
> 
> gdb-riscv-gdbserver-linux.diff
> Index: binutils-gdb/gdb/arch/riscv.c
> ===================================================================
> --- binutils-gdb.orig/gdb/arch/riscv.c
> +++ binutils-gdb/gdb/arch/riscv.c
> @@ -43,7 +43,7 @@ static std::unordered_map<riscv_gdbarch_
>  
>  /* See arch/riscv.h.  */
>  
> -const target_desc *
> +target_desc *
>  riscv_create_target_description (struct riscv_gdbarch_features features)
>  {
>    /* Have we seen this feature set before?  If we have return the same
> Index: binutils-gdb/gdb/arch/riscv.h
> ===================================================================
> --- binutils-gdb.orig/gdb/arch/riscv.h
> +++ binutils-gdb/gdb/arch/riscv.h
> @@ -69,7 +69,7 @@ struct riscv_gdbarch_features
>  /* Create and return a target description that is compatible with
>     FEATURES.  */
>  
> -const target_desc *riscv_create_target_description
> -	(struct riscv_gdbarch_features features);
> +target_desc *riscv_create_target_description
> +  (struct riscv_gdbarch_features features);
>  
>  #endif /* ARCH_RISCV_H */
> Index: binutils-gdb/gdb/configure.tgt
> ===================================================================
> --- binutils-gdb.orig/gdb/configure.tgt
> +++ binutils-gdb/gdb/configure.tgt
> @@ -553,6 +553,7 @@ riscv*-*-linux*)
>  	# Target: Linux/RISC-V
>  	gdb_target_obs="riscv-linux-tdep.o glibc-tdep.o \
>   			linux-tdep.o solib-svr4.o symfile-mem.o linux-record.o"
> +	build_gdbserver=yes
>  	;;
>  
>  riscv*-*-*)
> Index: binutils-gdb/gdb/gdbserver/Makefile.in
> ===================================================================
> --- binutils-gdb.orig/gdb/gdbserver/Makefile.in
> +++ binutils-gdb/gdb/gdbserver/Makefile.in
> @@ -177,6 +177,7 @@ SFILES = \
>  	$(srcdir)/linux-mips-low.c \
>  	$(srcdir)/linux-nios2-low.c \
>  	$(srcdir)/linux-ppc-low.c \
> +	$(srcdir)/linux-riscv-low.c \
>  	$(srcdir)/linux-s390-low.c \
>  	$(srcdir)/linux-sh-low.c \
>  	$(srcdir)/linux-sparc-low.c \
> @@ -203,6 +204,7 @@ SFILES = \
>  	$(srcdir)/../arch/arm-get-next-pcs.c \
>  	$(srcdir)/../arch/arm-linux.c \
>  	$(srcdir)/../arch/ppc-linux-common.c \
> +	$(srcdir)/../arch/riscv.c \
>  	$(srcdir)/../../gdbsupport/btrace-common.c \
>  	$(srcdir)/../../gdbsupport/buffer.c \
>  	$(srcdir)/../../gdbsupport/cleanups.c \
> @@ -236,6 +238,7 @@ SFILES = \
>  	$(srcdir)/../nat/linux-personality.c \
>  	$(srcdir)/../nat/mips-linux-watch.c \
>  	$(srcdir)/../nat/ppc-linux.c \
> +	$(srcdir)/../nat/riscv-linux-tdesc.c \
>  	$(srcdir)/../nat/fork-inferior.c \
>  	$(srcdir)/../target/waitstatus.c
>  
> Index: binutils-gdb/gdb/gdbserver/configure.srv
> ===================================================================
> --- binutils-gdb.orig/gdb/gdbserver/configure.srv
> +++ binutils-gdb/gdb/gdbserver/configure.srv
> @@ -267,6 +267,13 @@ case "${target}" in
>  			srv_xmlfiles="${srv_xmlfiles} rs6000/power-fpu.xml"
>  			srv_lynxos=yes
>  			;;
> +  riscv*-*-linux*)	srv_tgtobj="arch/riscv.o nat/riscv-linux-tdesc.o"
> +			srv_tgtobj="${srv_tgtobj} linux-riscv-low.o"
> +			srv_tgtobj="${srv_tgtobj} ${srv_linux_obj}"
> +			srv_linux_regsets=yes
> +			srv_linux_usrregs=yes
> +			srv_linux_thread_db=yes
> +			;;
>    s390*-*-linux*)	srv_regobj="s390-linux32.o"
>  			srv_regobj="${srv_regobj} s390-linux32v1.o"
>  			srv_regobj="${srv_regobj} s390-linux32v2.o"
> Index: binutils-gdb/gdb/gdbserver/linux-riscv-low.c
> ===================================================================
> --- /dev/null
> +++ binutils-gdb/gdb/gdbserver/linux-riscv-low.c
> @@ -0,0 +1,277 @@
> +/* GNU/Linux/RISC-V specific low level interface, for the remote server
> +   for GDB.
> +   Copyright (C) 2020 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 "server.h"
> +
> +#include "linux-low.h"
> +#include "tdesc.h"
> +#include "elf/common.h"
> +#include "nat/riscv-linux-tdesc.h"
> +#include "opcode/riscv.h"
> +
> +/* Work around glibc header breakage causing ELF_NFPREG not to be usable.  */
> +#ifndef NFPREG
> +# define NFPREG 33
> +#endif
> +
> +/* Implementation of linux_target_ops method "arch_setup".  */
> +
> +static void
> +riscv_arch_setup ()
> +{
> +  static const char *expedite_regs[] = { "sp", "pc", NULL };
> +  target_desc *tdesc;
> +
> +  tdesc = riscv_linux_read_description (lwpid_of (current_thread));
> +  if (!tdesc->expedite_regs)
> +    init_target_desc (tdesc, expedite_regs);
> +  current_process ()->tdesc = tdesc;
> +}
> +
> +/* Collect GPRs from REGCACHE into BUF.  */
> +
> +static void
> +riscv_fill_gregset (struct regcache *regcache, void *buf)
> +{
> +  const struct target_desc *tdesc = regcache->tdesc;
> +  elf_gregset_t *regset = (elf_gregset_t *) buf;
> +  int regno = find_regno (tdesc, "zero");
> +  int i;
> +
> +  collect_register_by_name (regcache, "pc", *regset);
> +  for (i = 1; i < ARRAY_SIZE (*regset); i++)
> +    collect_register (regcache, regno + i, *regset + i);
> +}
> +
> +/* Supply GPRs from BUF into REGCACHE.  */
> +
> +static void
> +riscv_store_gregset (struct regcache *regcache, const void *buf)
> +{
> +  const elf_gregset_t *regset = (const elf_gregset_t *) buf;
> +  const struct target_desc *tdesc = regcache->tdesc;
> +  int regno = find_regno (tdesc, "zero");
> +  int i;
> +
> +  supply_register_by_name (regcache, "pc", *regset);
> +  supply_register_zeroed (regcache, regno);
> +  for (i = 1; i < ARRAY_SIZE (*regset); i++)
> +    supply_register (regcache, regno + i, *regset + i);
> +}
> +
> +/* Collect FPRs from REGCACHE into BUF.  */
> +
> +static void
> +riscv_fill_fpregset (struct regcache *regcache, void *buf)
> +{
> +  const struct target_desc *tdesc = regcache->tdesc;
> +  int regno = find_regno (tdesc, "ft0");
> +  int flen = register_size (regcache->tdesc, regno);
> +  gdb_byte *regbuf = (gdb_byte *) buf;
> +  int i;
> +
> +  for (i = 0; i < ELF_NFPREG - 1; i++, regbuf += flen)
> +    collect_register (regcache, regno + i, regbuf);
> +  collect_register_by_name (regcache, "fcsr", regbuf);
> +}
> +
> +/* Supply FPRs from BUF into REGCACHE.  */
> +
> +static void
> +riscv_store_fpregset (struct regcache *regcache, const void *buf)
> +{
> +  const struct target_desc *tdesc = regcache->tdesc;
> +  int regno = find_regno (tdesc, "ft0");
> +  int flen = register_size (regcache->tdesc, regno);
> +  const gdb_byte *regbuf = (const gdb_byte *) buf;
> +  int i;
> +
> +  for (i = 0; i < ELF_NFPREG - 1; i++, regbuf += flen)
> +    supply_register (regcache, regno + i, regbuf);
> +  supply_register_by_name (regcache, "fcsr", 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
> +   the right one according to size.  */
> +static struct regset_info riscv_regsets[] = {
> +  { PTRACE_GETREGSET, PTRACE_SETREGSET, NT_PRSTATUS,
> +    sizeof (elf_gregset_t), GENERAL_REGS,
> +    riscv_fill_gregset, riscv_store_gregset },
> +  { PTRACE_GETREGSET, PTRACE_SETREGSET, NT_FPREGSET,
> +    sizeof (struct __riscv_mc_q_ext_state), OPTIONAL_REGS,
> +    riscv_fill_fpregset, riscv_store_fpregset },
> +  { PTRACE_GETREGSET, PTRACE_SETREGSET, NT_FPREGSET,
> +    sizeof (struct __riscv_mc_d_ext_state), OPTIONAL_REGS,
> +    riscv_fill_fpregset, riscv_store_fpregset },
> +  { PTRACE_GETREGSET, PTRACE_SETREGSET, NT_FPREGSET,
> +    sizeof (struct __riscv_mc_f_ext_state), OPTIONAL_REGS,
> +    riscv_fill_fpregset, riscv_store_fpregset },
> +  NULL_REGSET
> +};
> +
> +/* RISC-V/Linux regset information.  */
> +static struct regsets_info riscv_regsets_info =
> +  {
> +    riscv_regsets, /* regsets */
> +    0, /* num_regsets */
> +    NULL, /* disabled_regsets */
> +  };
> +
> +/* Definition of linux_target_ops data member "regs_info".  */
> +static struct regs_info riscv_regs =
> +  {
> +    NULL, /* regset_bitmap */
> +    NULL, /* usrregs */
> +    &riscv_regsets_info,
> +  };
> +
> +/* Implementation of linux_target_ops method "regs_info".  */
> +
> +static const struct regs_info *
> +riscv_regs_info ()
> +{
> +  return &riscv_regs;
> +}
> +
> +/* Implementation of linux_target_ops method "fetch_register".  */
> +
> +static int
> +riscv_fetch_register (struct regcache *regcache, int regno)
> +{
> +  const struct target_desc *tdesc = regcache->tdesc;
> +
> +  if (regno != find_regno (tdesc, "zero"))
> +    return 0;
> +  supply_register_zeroed (regcache, regno);
> +  return 1;
> +}
> +
> +/* Implementation of linux_target_ops method "get_pc".  */
> +
> +static CORE_ADDR
> +riscv_get_pc (struct regcache *regcache)
> +{
> +  elf_gregset_t regset;
> +
> +  if (sizeof (regset[0]) == 8)
> +    return linux_get_pc_64bit (regcache);
> +  else
> +    return linux_get_pc_32bit (regcache);
> +}
> +
> +/* Implementation of linux_target_ops method "set_pc".  */
> +
> +static void
> +riscv_set_pc (struct regcache *regcache, CORE_ADDR newpc)
> +{
> +  elf_gregset_t regset;
> +
> +  if (sizeof (regset[0]) == 8)
> +    linux_set_pc_64bit (regcache, newpc);
> +  else
> +    linux_set_pc_32bit (regcache, newpc);
> +}
> +
> +/* Correct in either endianness.  */
> +static const uint16_t riscv_ibreakpoint[] = { 0x0073, 0x0010 };
> +static const uint16_t riscv_cbreakpoint = 0x9002;
> +
> +/* Implementation of linux_target_ops method "breakpoint_kind_from_pc".  */
> +
> +static int
> +riscv_breakpoint_kind_from_pc (CORE_ADDR *pcptr)
> +{
> +  union
> +    {
> +      gdb_byte bytes[2];
> +      uint16_t insn;
> +    }
> +  buf;
> +
> +  if (target_read_memory (*pcptr, buf.bytes, sizeof (buf.insn)) == 0
> +      && riscv_insn_length (buf.insn == sizeof (riscv_ibreakpoint)))
> +    return sizeof (riscv_ibreakpoint);
> +  else
> +    return sizeof (riscv_cbreakpoint);
> +}
> +
> +/* Implementation of linux_target_ops method "sw_breakpoint_from_kind".  */
> +
> +static const gdb_byte *
> +riscv_sw_breakpoint_from_kind (int kind, int *size)
> +{
> +  *size = kind;
> +  switch (kind)
> +    {
> +      case sizeof (riscv_ibreakpoint):
> +	return (const gdb_byte *) &riscv_ibreakpoint;
> +      default:
> +	return (const gdb_byte *) &riscv_cbreakpoint;
> +    }
> +}
> +
> +/* Implementation of linux_target_ops method "breakpoint_at".  */
> +
> +static int
> +riscv_breakpoint_at (CORE_ADDR pc)
> +{
> +  union
> +    {
> +      gdb_byte bytes[2];
> +      uint16_t insn;
> +    }
> +  buf;
> +
> +  if (target_read_memory (pc, buf.bytes, sizeof (buf.insn)) == 0
> +      && (buf.insn == riscv_cbreakpoint
> +	  || (buf.insn == riscv_ibreakpoint[0]
> +	      && target_read_memory (pc + sizeof (buf.insn), buf.bytes,
> +				     sizeof (buf.insn)) == 0
> +	      && buf.insn == riscv_ibreakpoint[1])))
> +    return 1;
> +  else
> +    return 0;
> +}
> +
> +/* RISC-V/Linux target operations.  */
> +struct linux_target_ops the_low_target =
> +{
> +  riscv_arch_setup,
> +  riscv_regs_info,
> +  NULL, /* cannot_fetch_register */
> +  NULL, /* cannot_store_register */
> +  riscv_fetch_register,
> +  riscv_get_pc,
> +  riscv_set_pc,
> +  riscv_breakpoint_kind_from_pc,
> +  riscv_sw_breakpoint_from_kind,
> +  NULL, /* get_next_pcs */
> +  0,    /* decr_pc_after_break */
> +  riscv_breakpoint_at,
> +};
> +
> +/* Initialize the RISC-V/Linux target.  */
> +
> +void
> +initialize_low_arch ()
> +{
> +  initialize_regsets_info (&riscv_regsets_info);
> +}
> Index: binutils-gdb/gdb/nat/riscv-linux-tdesc.c
> ===================================================================
> --- binutils-gdb.orig/gdb/nat/riscv-linux-tdesc.c
> +++ binutils-gdb/gdb/nat/riscv-linux-tdesc.c
> @@ -33,7 +33,7 @@
>  
>  /* Determine XLEN and FLEN and return a corresponding target description.  */
>  
> -const struct target_desc *
> +struct target_desc *
>  riscv_linux_read_description (int tid)
>  {
>    struct riscv_gdbarch_features features;
> Index: binutils-gdb/gdb/nat/riscv-linux-tdesc.h
> ===================================================================
> --- binutils-gdb.orig/gdb/nat/riscv-linux-tdesc.h
> +++ binutils-gdb/gdb/nat/riscv-linux-tdesc.h
> @@ -22,6 +22,6 @@
>  struct target_desc;
>  
>  /* Return a target description for the LWP identified by TID.  */
> -const struct target_desc *riscv_linux_read_description (int tid);
> +struct target_desc *riscv_linux_read_description (int tid);
>  
>  #endif /* NAT_RISCV_LINUX_TDESC_H */



More information about the Gdb-patches mailing list