This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH v5 3/3] gdbserver: Add RISC-V/Linux support
* 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 */