Re: [PATCH 2/2] riscv: enable gdb/sim integration

Andrew Burgess
2021-02-04T10:45:31Z

* Mike Frysinger via Gdb-patches <gdb-patches@sourceware.org> [2021-01-12 06:18:42 -0500]:

> Now the simulator can be loaded via gdb using "target sim".
>
> gdb/:
>
> * configure.tgt (riscv*-*-*): Set gdb_sim.
>
> include/gdb/:
>
> * sim-riscv.h: New file.

On a style issue, you'll have noticed I'm sure that GDB patches all
include their changelog entries inside the commit message, something
you haven't been doing for sim/ patches.

As this patch does touch gdb/ it would be appreciated if you could
include the full ChangeLog for all parts in the commit message as is
GDB style, this should also cover the sim/* changes - otherwise it's
just going to look like a mistake was made.

> ---
> gdb/configure.tgt | 1 +
> include/gdb/sim-riscv.h | 99 +++++++++++++++++++++++++++++++++++++++++
> sim/riscv/ChangeLog | 6 +++
> sim/riscv/sim-main.c | 70 +++++++++++++++++++++++++++++

The gdb/ parts are approved with the above change made. The sim/
parts look good.

Thanks,
Andrew

> 4 files changed, 176 insertions(+)
> create mode 100644 include/gdb/sim-riscv.h
>
> diff --git a/gdb/configure.tgt b/gdb/configure.tgt
> index 6e0398387482..5440780065f0 100644
> --- a/gdb/configure.tgt
> +++ b/gdb/configure.tgt
> @@ -554,6 +554,7 @@ riscv*-*-linux*)
> riscv*-*-*)
> # Target: RISC-V architecture
> gdb_target_obs=""
> + gdb_sim=../sim/riscv/libsim.a
> ;;
>
> rl78-*-elf)
> diff --git a/include/gdb/sim-riscv.h b/include/gdb/sim-riscv.h
> new file mode 100644
> index 000000000000..23e5f766aa6c
> --- /dev/null
> +++ b/include/gdb/sim-riscv.h
> @@ -0,0 +1,99 @@
> +/* This file defines the interface between the RISC-V simulator and GDB.
> +
> + Copyright (C) 2005-2021 Free Software Foundation, Inc.
> + Contributed by Mike Frysinger.
> +
> + 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/>. */
> +
> +/* Order has to match gdb riscv-tdep list. */
> +enum sim_riscv_regnum {
> + SIM_RISCV_ZERO_REGNUM = 0,
> + SIM_RISCV_RA_REGNUM,
> + SIM_RISCV_SP_REGNUM,
> + SIM_RISCV_GP_REGNUM,
> + SIM_RISCV_TP_REGNUM,
> + SIM_RISCV_T0_REGNUM,
> + SIM_RISCV_T1_REGNUM,
> + SIM_RISCV_T2_REGNUM,
> + SIM_RISCV_S0_REGNUM,
> +#define SIM_RISCV_FP_REGNUM SIM_RISCV_S0_REGNUM
> + SIM_RISCV_S1_REGNUM,
> + SIM_RISCV_A0_REGNUM,
> + SIM_RISCV_A1_REGNUM,
> + SIM_RISCV_A2_REGNUM,
> + SIM_RISCV_A3_REGNUM,
> + SIM_RISCV_A4_REGNUM,
> + SIM_RISCV_A5_REGNUM,
> + SIM_RISCV_A6_REGNUM,
> + SIM_RISCV_A7_REGNUM,
> + SIM_RISCV_S2_REGNUM,
> + SIM_RISCV_S3_REGNUM,
> + SIM_RISCV_S4_REGNUM,
> + SIM_RISCV_S5_REGNUM,
> + SIM_RISCV_S6_REGNUM,
> + SIM_RISCV_S7_REGNUM,
> + SIM_RISCV_S8_REGNUM,
> + SIM_RISCV_S9_REGNUM,
> + SIM_RISCV_S10_REGNUM,
> + SIM_RISCV_S11_REGNUM,
> + SIM_RISCV_T3_REGNUM,
> + SIM_RISCV_T4_REGNUM,
> + SIM_RISCV_T5_REGNUM,
> + SIM_RISCV_T6_REGNUM,
> + SIM_RISCV_PC_REGNUM,
> + SIM_RISCV_FT0_REGNUM,
> +#define SIM_RISCV_FIRST_FP_REGNUM SIM_RISCV_FT0_REGNUM
> + SIM_RISCV_FT1_REGNUM,
> + SIM_RISCV_FT2_REGNUM,
> + SIM_RISCV_FT3_REGNUM,
> + SIM_RISCV_FT4_REGNUM,
> + SIM_RISCV_FT5_REGNUM,
> + SIM_RISCV_FT6_REGNUM,
> + SIM_RISCV_FT7_REGNUM,
> + SIM_RISCV_FS0_REGNUM,
> + SIM_RISCV_FS1_REGNUM,
> + SIM_RISCV_FA0_REGNUM,
> + SIM_RISCV_FA1_REGNUM,
> + SIM_RISCV_FA2_REGNUM,
> + SIM_RISCV_FA3_REGNUM,
> + SIM_RISCV_FA4_REGNUM,
> + SIM_RISCV_FA5_REGNUM,
> + SIM_RISCV_FA6_REGNUM,
> + SIM_RISCV_FA7_REGNUM,
> + SIM_RISCV_FS2_REGNUM,
> + SIM_RISCV_FS3_REGNUM,
> + SIM_RISCV_FS4_REGNUM,
> + SIM_RISCV_FS5_REGNUM,
> + SIM_RISCV_FS6_REGNUM,
> + SIM_RISCV_FS7_REGNUM,
> + SIM_RISCV_FS8_REGNUM,
> + SIM_RISCV_FS9_REGNUM,
> + SIM_RISCV_FS10_REGNUM,
> + SIM_RISCV_FS11_REGNUM,
> + SIM_RISCV_FT8_REGNUM,
> + SIM_RISCV_FT9_REGNUM,
> + SIM_RISCV_FT10_REGNUM,
> + SIM_RISCV_FT11_REGNUM,
> +#define SIM_RISCV_LAST_FP_REGNUM SIM_RISCV_FT11_REGNUM
> +
> +#define SIM_RISCV_FIRST_CSR_REGNUM SIM_RISCV_LAST_FP_REGNUM + 1
> +#define DECLARE_CSR(name, num, ...) SIM_RISCV_ ## num ## _REGNUM,
> +#include "opcode/riscv-opc.h"
> +#undef DECLARE_CSR
> +#define SIM_RISCV_LAST_CSR_REGNUM SIM_RISCV_LAST_REGNUM - 1
> +
> + SIM_RISCV_LAST_REGNUM
> +};
> diff --git a/sim/riscv/ChangeLog b/sim/riscv/ChangeLog
> index f152de1e4646..7d588c80babd 100644
> --- a/sim/riscv/ChangeLog
> +++ b/sim/riscv/ChangeLog
> @@ -1,3 +1,9 @@
> +2021-01-12 Mike Frysinger <vapier@gentoo.org>
> +
> + * sim-main.c: Include gdb/sim-riscv.h.
> + (reg_fetch, reg_store): Define.
> + (initialize_cpu): Assign reg_fetch & reg_store.
> +
> 2021-01-12 Mike Frysinger <vapier@gentoo.org>
>
> * Makefile.in, configure.ac, interp.c, machs.c, machs.h,
> diff --git a/sim/riscv/sim-main.c b/sim/riscv/sim-main.c
> index 15a0eb02ae26..2dd7167cc1d0 100644
> --- a/sim/riscv/sim-main.c
> +++ b/sim/riscv/sim-main.c
> @@ -31,6 +31,8 @@
>
> #include "opcode/riscv.h"
>
> +#include "gdb/sim-riscv.h"
> +
> #include "targ-vals.h"
> \f
> #define TRACE_REG(cpu, reg) \
> @@ -1019,6 +1021,72 @@ pc_set (sim_cpu *cpu, sim_cia pc)
> cpu->pc = pc;
> }
>
> +static int
> +reg_fetch (sim_cpu *cpu, int rn, unsigned char *buf, int len)
> +{
> + if (len <= 0 || len > sizeof (unsigned_word))
> + return -1;
> +
> + switch (rn)
> + {
> + case SIM_RISCV_ZERO_REGNUM:
> + memset (buf, 0, len);
> + return len;
> + case SIM_RISCV_RA_REGNUM ... SIM_RISCV_T6_REGNUM:
> + memcpy (buf, &cpu->regs[rn], len);
> + return len;
> + case SIM_RISCV_FIRST_FP_REGNUM ... SIM_RISCV_LAST_FP_REGNUM:
> + memcpy (buf, &cpu->fpregs[rn - SIM_RISCV_FIRST_FP_REGNUM], len);
> + return len;
> + case SIM_RISCV_PC_REGNUM:
> + memcpy (buf, &cpu->pc, len);
> + return len;
> +
> +#define DECLARE_CSR(name, num, ...) \
> + case SIM_RISCV_ ## num ## _REGNUM: \
> + memcpy (buf, &cpu->csr.name, len); \
> + return len;
> +#include "opcode/riscv-opc.h"
> +#undef DECLARE_CSR
> +
> + default:
> + return -1;
> + }
> +}
> +
> +static int
> +reg_store (sim_cpu *cpu, int rn, unsigned char *buf, int len)
> +{
> + if (len <= 0 || len > sizeof (unsigned_word))
> + return -1;
> +
> + switch (rn)
> + {
> + case SIM_RISCV_ZERO_REGNUM:
> + /* Ignore writes. */
> + return len;
> + case SIM_RISCV_RA_REGNUM ... SIM_RISCV_T6_REGNUM:
> + memcpy (&cpu->regs[rn], buf, len);
> + return len;
> + case SIM_RISCV_FIRST_FP_REGNUM ... SIM_RISCV_LAST_FP_REGNUM:
> + memcpy (&cpu->fpregs[rn - SIM_RISCV_FIRST_FP_REGNUM], buf, len);
> + return len;
> + case SIM_RISCV_PC_REGNUM:
> + memcpy (&cpu->pc, buf, len);
> + return len;
> +
> +#define DECLARE_CSR(name, num, ...) \
> + case SIM_RISCV_ ## num ## _REGNUM: \
> + memcpy (&cpu->csr.name, buf, len); \
> + return len;
> +#include "opcode/riscv-opc.h"
> +#undef DECLARE_CSR
> +
> + default:
> + return -1;
> + }
> +}
> +
> /* Initialize the state for a single cpu. Usuaully this involves clearing all
> registers back to their reset state. Should also hook up the fetch/store
> helper functions too. */
> @@ -1031,6 +1099,8 @@ void initialize_cpu (SIM_DESC sd, SIM_CPU *cpu, int mhartid)
>
> CPU_PC_FETCH (cpu) = pc_get;
> CPU_PC_STORE (cpu) = pc_set;
> + CPU_REG_FETCH (cpu) = reg_fetch;
> + CPU_REG_STORE (cpu) = reg_store;
>
> if (!riscv_hash[0])
> {
> --
> 2.28.0
>