[PATCH 0/5] RISC-V Linux native port

[PATCH 0/5] RISC-V Linux native port

[Jim Wilson]

Here are my current patches to add the RISC-V Linux native support to
gdb.  These have been tested on a Hifive Unleashed running Fedora as
previously mentioned and are working reasonably well.

One caveat I should mention here is that this port requires multiple
Linux kernel patches and a GLIBC patch, which people may not already
have.  I'm not sure how to handle that.  I have a README.md in my
personal github repo that points at the patches.  There is 1 linux
kernel patch that was added in the previous merge window, 2 being
added in the current merge window, and one I haven't submitted yet for
FP register support because it needs some cleanup work before
upstreaming.   There is also a glibc patch that was just added last
week.

Jim

[PATCH 1/5] RISC-V: Make riscv_isa_xlen a global function.

[Jim Wilson]

This allows the function to be used from riscv OS files, which also need to
depend on XLEN size.

	gdb/
	* riscv-tdep.c (riscv_isa_xlen): Drop static.
	* riscv-tdep.h (riscv_isa_xlen): Add extern declaration.
---
 gdb/riscv-tdep.c | 2 +-
 gdb/riscv-tdep.h | 3 +++
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/gdb/riscv-tdep.c b/gdb/riscv-tdep.c
index abcac98016..20181896c5 100644
--- a/gdb/riscv-tdep.c
+++ b/gdb/riscv-tdep.c
@@ -346,7 +346,7 @@ riscv_has_feature (struct gdbarch *gdbarch, char feature)
    Possible return values are 4, 8, or 16 for RiscV variants RV32, RV64, or
    RV128.  */
 
-static int
+int
 riscv_isa_xlen (struct gdbarch *gdbarch)
 {
   switch (gdbarch_tdep (gdbarch)->abi.fields.base_len)
diff --git a/gdb/riscv-tdep.h b/gdb/riscv-tdep.h
index 4fc05976ba..b35266daf7 100644
--- a/gdb/riscv-tdep.h
+++ b/gdb/riscv-tdep.h
@@ -76,4 +76,7 @@ struct gdbarch_tdep
   unsigned core_features;
 };
 
+/* Return the width in bytes of the general purpose registers for GDBARCH.  */
+extern int riscv_isa_xlen (struct gdbarch *gdbarch);
+
 #endif /* RISCV_TDEP_H */
-- 
2.17.1

[PATCH 2/5] RISC-V: Add software single step support.

[Jim Wilson]

This adds software single step support that is needed by the linux native port.
This is modeled after equivalent code in the MIPS port.

This also fixes a few bugs in the compressed instruction decode support.  Some
instructions are RV32/RV64 specific, and this wasn't being checked.  Also, a
few instructions were accidentally using the non-compressed is_* function.

This has been tested on a HiFive Unleashed running Fedora, by putting a
breakpoint on start, typing stepi, and then holding down the return key until
it finishes, and observing that I see everything I expect to see along the way.
There is a problem in _dl_addr where I get into an infinite loop, but it seems
to be some synchronization code that doesn't agree with single step, so I have
to find the end of the loop, put a breakpoint there, continue, and then single
step again until the end.

	gdb/
	* riscv-tdep.c (enum opcode): Add jump, branch, lr, and sc opcodes.
	(decode_register_index_short): New.
	(decode_j_type_insn, decode_cj_type_insn): New.
	(decode_b_type_insn, decode_cb_type_insn): New.
	(riscv_insn::decode): Add support for jumps, branches, lr, and sc.  New
	local xlen.  Check xlen when decoding ambiguous compressed insns.  In
	compressed decode, use is_c_lui_insn instead of is_lui_insn, and
	is_c_sw_insn instead of is_sw_insn.
	(riscv_next_pc, riscv_next_pc_atomic_sequence): New.
	(riscv_software_single_step): New.
	* riscv-tdep.h (riscv_software_single_step): Declare.
---
 gdb/riscv-tdep.c | 250 +++++++++++++++++++++++++++++++++++++++++++++--
 gdb/riscv-tdep.h |   4 +
 2 files changed, 248 insertions(+), 6 deletions(-)

diff --git a/gdb/riscv-tdep.c b/gdb/riscv-tdep.c
index 20181896c5..1df1300100 100644
--- a/gdb/riscv-tdep.c
+++ b/gdb/riscv-tdep.c
@@ -880,6 +880,18 @@ public:
       LUI,
       SD,
       SW,
+      /* These are needed for software breakopint support.  */
+      JAL,
+      JALR,
+      BEQ,
+      BNE,
+      BLT,
+      BGE,
+      BLTU,
+      BGEU,
+      /* These are needed for stepping over atomic sequences.  */
+      LR,
+      SC,
 
       /* Other instructions are not interesting during the prologue scan, and
 	 are ignored.  */
@@ -936,6 +948,12 @@ private:
     return (opcode >> offset) & 0x1F;
   }
 
+  /* Extract 5 bit register field at OFFSET from instruction OPCODE.  */
+  int decode_register_index_short (unsigned long opcode, int offset)
+  {
+    return ((opcode >> offset) & 0x7) + 8;
+  }
+
   /* Helper for DECODE, decode 32-bit R-type instruction.  */
   void decode_r_type_insn (enum opcode opcode, ULONGEST ival)
   {
@@ -987,6 +1005,36 @@ private:
     m_imm.s = EXTRACT_UTYPE_IMM (ival);
   }
 
+  /* Helper for DECODE, decode 32-bit J-type instruction.  */
+  void decode_j_type_insn (enum opcode opcode, ULONGEST ival)
+  {
+    m_opcode = opcode;
+    m_rd = decode_register_index (ival, OP_SH_RD);
+    m_imm.s = EXTRACT_UJTYPE_IMM (ival);
+  }
+
+  /* Helper for DECODE, decode 32-bit J-type instruction.  */
+  void decode_cj_type_insn (enum opcode opcode, ULONGEST ival)
+  {
+    m_opcode = opcode;
+    m_imm.s = EXTRACT_RVC_J_IMM (ival);
+  }
+
+  void decode_b_type_insn (enum opcode opcode, ULONGEST ival)
+  {
+    m_opcode = opcode;
+    m_rs1 = decode_register_index (ival, OP_SH_RS1);
+    m_rs2 = decode_register_index (ival, OP_SH_RS2);
+    m_imm.s = EXTRACT_SBTYPE_IMM (ival);
+  }
+
+  void decode_cb_type_insn (enum opcode opcode, ULONGEST ival)
+  {
+    m_opcode = opcode;
+    m_rs1 = decode_register_index_short (ival, OP_SH_CRS1S);
+    m_imm.s = EXTRACT_RVC_B_IMM (ival);
+  }
+
   /* Fetch instruction from target memory at ADDR, return the content of
      the instruction, and update LEN with the instruction length.  */
   static ULONGEST fetch_instruction (struct gdbarch *gdbarch,
@@ -1083,32 +1131,83 @@ riscv_insn::decode (struct gdbarch *gdbarch, CORE_ADDR pc)
 	decode_s_type_insn (SD, ival);
       else if (is_sw_insn (ival))
 	decode_s_type_insn (SW, ival);
+      else if (is_jal_insn (ival))
+	decode_j_type_insn (JAL, ival);
+      else if (is_jalr_insn (ival))
+	decode_i_type_insn (JALR, ival);
+      else if (is_beq_insn (ival))
+	decode_b_type_insn (BEQ, ival);
+      else if (is_bne_insn (ival))
+	decode_b_type_insn (BNE, ival);
+      else if (is_blt_insn (ival))
+	decode_b_type_insn (BLT, ival);
+      else if (is_bge_insn (ival))
+	decode_b_type_insn (BGE, ival);
+      else if (is_bltu_insn (ival))
+	decode_b_type_insn (BLTU, ival);
+      else if (is_bgeu_insn (ival))
+	decode_b_type_insn (BGEU, ival);
+      else if (is_lr_w_insn (ival))
+	decode_r_type_insn (LR, ival);
+      else if (is_lr_d_insn (ival))
+	decode_r_type_insn (LR, ival);
+      else if (is_sc_w_insn (ival))
+	decode_r_type_insn (SC, ival);
+      else if (is_sc_d_insn (ival))
+	decode_r_type_insn (SC, ival);
       else
 	/* None of the other fields are valid in this case.  */
 	m_opcode = OTHER;
     }
   else if (m_length == 2)
     {
-      if (is_c_add_insn (ival))
+      int xlen = riscv_isa_xlen (gdbarch);
+
+      /* C_ADD and C_JALR have the same opcode.  If RS2 is 0, then this is a
+	 C_JALR.  So must try to match C_JALR first as it has more bits in
+	 mask.  */
+      if (is_c_jalr_insn (ival))
+	decode_cr_type_insn (JALR, ival);
+      else if (is_c_add_insn (ival))
 	decode_cr_type_insn (ADD, ival);
-      else if (is_c_addw_insn (ival))
+      /* C_ADDW is RV64 and RV128 only.  */
+      else if (xlen != 4 && is_c_addw_insn (ival))
 	decode_cr_type_insn (ADDW, ival);
       else if (is_c_addi_insn (ival))
 	decode_ci_type_insn (ADDI, ival);
-      else if (is_c_addiw_insn (ival))
+      /* C_ADDIW and C_JAL have the same opcode.  C_ADDIW is RV64 and RV128
+	 only and C_JAL is RV32 only.  */
+      else if (xlen != 4 && is_c_addiw_insn (ival))
 	decode_ci_type_insn (ADDIW, ival);
+      else if (xlen == 4 && is_c_jal_insn (ival))
+	decode_cj_type_insn (JAL, ival);
+      /* C_ADDI16SP and C_LUI have the same opcode.  If RD is 2, then this is a
+	 C_ADDI16SP.  So must try to match C_ADDI16SP first as it has more bits
+	 in mask.  */
       else if (is_c_addi16sp_insn (ival))
 	{
 	  m_opcode = ADDI;
 	  m_rd = m_rs1 = decode_register_index (ival, OP_SH_RD);
 	  m_imm.s = EXTRACT_RVC_ADDI16SP_IMM (ival);
 	}
-      else if (is_lui_insn (ival))
+      else if (is_c_lui_insn (ival))
 	m_opcode = OTHER;
-      else if (is_c_sd_insn (ival))
+      /* C_SD and C_FSW have the same opcode.  C_SD is RV64 and RV128 only,
+	 and C_FSW is RV32 only.  */
+      else if (xlen != 4 && is_c_sd_insn (ival))
 	m_opcode = OTHER;
-      else if (is_sw_insn (ival))
+      else if (is_c_sw_insn (ival))
 	m_opcode = OTHER;
+      /* C_JR and C_MV have the same opcode.  If RS2 is 0, then this is a C_JR.
+	 So must try to match C_JR first as it ahs more bits in mask.  */
+      else if (is_c_jr_insn (ival))
+	decode_cr_type_insn (JALR, ival);
+      else if (is_c_j_insn (ival))
+	decode_cj_type_insn (JAL, ival);
+      else if (is_c_beqz_insn (ival))
+	decode_cb_type_insn (BEQ, ival);
+      else if (is_c_bnez_insn (ival))
+	decode_cb_type_insn (BNE, ival);
       else
 	/* None of the other fields of INSN are valid in this case.  */
 	m_opcode = OTHER;
@@ -2610,6 +2709,145 @@ riscv_invalidate_inferior_data (struct inferior *inf)
     }
 }
 
+/* This decodes the current instruction and determines the address of the
+   next instruction.  */
+
+static CORE_ADDR
+riscv_next_pc (struct regcache *regcache, CORE_ADDR pc)
+{
+  struct gdbarch *gdbarch = regcache->arch ();
+  struct riscv_insn insn;
+  CORE_ADDR next_pc;
+
+  insn.decode (gdbarch, pc);
+  next_pc = pc + insn.length ();
+
+  if (insn.opcode () == riscv_insn::JAL)
+    next_pc = pc + insn.imm_signed ();
+  else if (insn.opcode () == riscv_insn::JALR)
+    {
+      LONGEST source;
+      regcache->cooked_read (insn.rs1 (), &source);
+      next_pc = (source + insn.imm_signed ()) & ~(CORE_ADDR) 0x1;
+    }
+  else if (insn.opcode () == riscv_insn::BEQ)
+    {
+      LONGEST src1, src2;
+      regcache->cooked_read (insn.rs1 (), &src1);
+      regcache->cooked_read (insn.rs2 (), &src2);
+      if (src1 == src2)
+	next_pc = pc + insn.imm_signed ();
+    }
+  else if (insn.opcode () == riscv_insn::BNE)
+    {
+      LONGEST src1, src2;
+      regcache->cooked_read (insn.rs1 (), &src1);
+      regcache->cooked_read (insn.rs2 (), &src2);
+      if (src1 != src2)
+	next_pc = pc + insn.imm_signed ();
+    }
+  else if (insn.opcode () == riscv_insn::BLT)
+    {
+      LONGEST src1, src2;
+      regcache->cooked_read (insn.rs1 (), &src1);
+      regcache->cooked_read (insn.rs2 (), &src2);
+      if (src1 < src2)
+	next_pc = pc + insn.imm_signed ();
+    }
+  else if (insn.opcode () == riscv_insn::BGE)
+    {
+      LONGEST src1, src2;
+      regcache->cooked_read (insn.rs1 (), &src1);
+      regcache->cooked_read (insn.rs2 (), &src2);
+      if (src1 >= src2)
+	next_pc = pc + insn.imm_signed ();
+    }
+  else if (insn.opcode () == riscv_insn::BLTU)
+    {
+      ULONGEST src1, src2;
+      regcache->cooked_read (insn.rs1 (), &src1);
+      regcache->cooked_read (insn.rs2 (), &src2);
+      if (src1 < src2)
+	next_pc = pc + insn.imm_signed ();
+    }
+  else if (insn.opcode () == riscv_insn::BGEU)
+    {
+      ULONGEST src1, src2;
+      regcache->cooked_read (insn.rs1 (), &src1);
+      regcache->cooked_read (insn.rs2 (), &src2);
+      if (src1 >= src2)
+	next_pc = pc + insn.imm_signed ();
+    }
+
+  return next_pc;
+}
+
+/* We can't put a breakpoint in the middle of a lr/sc atomic sequence, so look
+   for the end of the sequence and put the breakpoint there.  */
+
+static bool
+riscv_next_pc_atomic_sequence (struct regcache *regcache, CORE_ADDR pc,
+			       CORE_ADDR *next_pc)
+{
+  struct gdbarch *gdbarch = regcache->arch ();
+  struct riscv_insn insn;
+  CORE_ADDR cur_step_pc = pc;
+  CORE_ADDR last_addr = 0;
+
+  /* First instruction has to be a load reserved.  */
+  insn.decode (gdbarch, cur_step_pc);
+  if (insn.opcode () != riscv_insn::LR)
+    return false;
+  cur_step_pc = cur_step_pc + insn.length ();
+
+  /* Next instruction should be branch to exit.  */
+  insn.decode (gdbarch, cur_step_pc);
+  if (insn.opcode () != riscv_insn::BNE)
+    return false;
+  last_addr = cur_step_pc + insn.imm_signed ();
+  cur_step_pc = cur_step_pc + insn.length ();
+
+  /* Next instruction should be store conditional.  */
+  insn.decode (gdbarch, cur_step_pc);
+  if (insn.opcode () != riscv_insn::SC)
+    return false;
+  cur_step_pc = cur_step_pc + insn.length ();
+
+  /* Next instruction should be branch to start.  */
+  insn.decode (gdbarch, cur_step_pc);
+  if (insn.opcode () != riscv_insn::BNE)
+    return false;
+  if (pc != (cur_step_pc + insn.imm_signed ()))
+    return false;
+  cur_step_pc = cur_step_pc + insn.length ();
+
+  /* We should now be at the end of the sequence.  */
+  if (cur_step_pc != last_addr)
+    return false;
+
+  *next_pc = cur_step_pc;
+  return true;
+}
+
+/* This is called just before we want to resume the inferior, if we want to
+   single-step it but there is no hardware or kernel single-step support.  We
+   find the target of the coming instruction and breakpoint it.  */
+
+std::vector<CORE_ADDR>
+riscv_software_single_step (struct regcache *regcache)
+{
+  CORE_ADDR pc, next_pc;
+
+  pc = regcache_read_pc (regcache);
+
+  if (riscv_next_pc_atomic_sequence (regcache, pc, &next_pc))
+    return {next_pc};
+
+  next_pc = riscv_next_pc (regcache, pc);
+
+  return {next_pc};
+}
+
 void
 _initialize_riscv_tdep (void)
 {
diff --git a/gdb/riscv-tdep.h b/gdb/riscv-tdep.h
index b35266daf7..8591116922 100644
--- a/gdb/riscv-tdep.h
+++ b/gdb/riscv-tdep.h
@@ -79,4 +79,8 @@ struct gdbarch_tdep
 /* Return the width in bytes of the general purpose registers for GDBARCH.  */
 extern int riscv_isa_xlen (struct gdbarch *gdbarch);
 
+/* Single step based on where the current instruction will take us.  */
+extern std::vector<CORE_ADDR>
+riscv_software_single_step (struct regcache *regcache);
+
 #endif /* RISCV_TDEP_H */
-- 
2.17.1

[PATCH 3/5] RISC-V: Add linux target support.

[Jim Wilson]

Add initial target support for riscv*-linux*.

	gdb/
	* riscv-linux-tdep.c: New file.
---
 gdb/riscv-linux-tdep.c | 88 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 88 insertions(+)
 create mode 100644 gdb/riscv-linux-tdep.c

diff --git a/gdb/riscv-linux-tdep.c b/gdb/riscv-linux-tdep.c
new file mode 100644
index 0000000000..c98dba70ca
--- /dev/null
+++ b/gdb/riscv-linux-tdep.c
@@ -0,0 +1,88 @@
+/* Target-dependent code for GNU/Linux on RISC-V processors.
+   Copyright (C) 2018 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 "defs.h"
+#include "riscv-tdep.h"
+#include "osabi.h"
+#include "glibc-tdep.h"
+#include "linux-tdep.h"
+#include "solib-svr4.h"
+#include "regset.h"
+
+static const struct regcache_map_entry riscv_linux_gregmap[] =
+{
+  { 1,  RISCV_PC_REGNUM, 0 },
+  { 31, RISCV_RA_REGNUM, 0 }, /* x1 to x31 */
+  { 0 }
+};
+
+const struct regset riscv_linux_gregset =
+{
+  riscv_linux_gregmap, regcache_supply_regset, regcache_collect_regset
+};
+
+/* Define hook for core file support.  */
+
+static void
+riscv_linux_iterate_over_regset_sections (struct gdbarch *gdbarch,
+                                          iterate_over_regset_sections_cb *cb,
+                                          void *cb_data,
+                                          const struct regcache *regcache)
+{
+  cb (".reg", (32 * riscv_isa_xlen (gdbarch)),
+      &riscv_linux_gregset, NULL, cb_data);
+
+  /* TODO: Add FP register support.  */
+}
+
+/* Initialize RISC-V Linux ABI info.  */
+
+static void
+riscv_linux_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
+{
+  linux_init_abi (info, gdbarch);
+
+  set_gdbarch_software_single_step (gdbarch, riscv_software_single_step);
+
+  set_solib_svr4_fetch_link_map_offsets (gdbarch,
+					 (riscv_isa_xlen (gdbarch) == 4
+					  ? svr4_ilp32_fetch_link_map_offsets
+					  : svr4_lp64_fetch_link_map_offsets));
+
+  /* GNU/Linux uses SVR4-style shared libraries.  */
+  set_gdbarch_skip_trampoline_code (gdbarch, find_solib_trampoline_target);
+
+  /* GNU/Linux uses the dynamic linker included in the GNU C Library.  */
+  set_gdbarch_skip_solib_resolver (gdbarch, glibc_skip_solib_resolver);
+
+  /* Enable TLS support.  */
+  set_gdbarch_fetch_tls_load_module_address (gdbarch,
+                                             svr4_fetch_objfile_link_map);
+
+  set_gdbarch_iterate_over_regset_sections
+    (gdbarch, riscv_linux_iterate_over_regset_sections);
+}
+
+/* Initialize RISC-V Linux target support.  */
+
+void
+_initialize_riscv_linux_tdep (void)
+{
+  gdbarch_register_osabi (bfd_arch_riscv, 0, GDB_OSABI_LINUX,
+			  riscv_linux_init_abi);
+}
-- 
2.17.1

[PATCH 5/5] RISC-V: Add configure support riscv*-linux*.

[Jim Wilson]

This adds the target and native configure support, and the NEWS entries for
the new target and native configurations.

	gdb/
	* Makefile.in (ALLDEPFILES): Add riscv-linux-nat.c, riscv-linux-tdep.c.
	* NEWS: Mention new GNU/Linux RISC-V target.
	* configure.host: Add riscv*-*-linux*.
	* configure.nat: Add riscv*.
	* configure.tgt: Add riscv*-*-linux*.
---
 gdb/Makefile.in    | 4 ++++
 gdb/NEWS           | 8 ++++++++
 gdb/configure.host | 2 ++
 gdb/configure.nat  | 4 ++++
 gdb/configure.tgt  | 6 ++++++
 5 files changed, 24 insertions(+)

diff --git a/gdb/Makefile.in b/gdb/Makefile.in
index 8c744d70c0..280b3b1283 100644
--- a/gdb/Makefile.in
+++ b/gdb/Makefile.in
@@ -752,6 +752,8 @@ ALL_TARGET_OBS = \
 	ppc-sysv-tdep.o \
 	ppc64-tdep.o \
 	ravenscar-thread.o \
+	riscv-linux-nat.o \
+	riscv-linux-tdep.o \
 	riscv-tdep.o \
 	rl78-tdep.o \
 	rs6000-aix-tdep.o \
@@ -2300,6 +2302,8 @@ ALLDEPFILES = \
 	procfs.c \
 	ravenscar-thread.c \
 	remote-sim.c \
+	riscv-linux-nat.c \
+	riscv-linux-tdep.c \
 	riscv-tdep.c \
 	rl78-tdep.c \
 	rs6000-lynx178-tdep.c \
diff --git a/gdb/NEWS b/gdb/NEWS
index 669ed2d0eb..62cde1cde2 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -38,6 +38,14 @@ thread apply [all | COUNT | -COUNT] [FLAG]... COMMAND
   FLAG arguments allow to control what output to produce and how to handle
   errors raised when applying COMMAND to a thread.
 
+* New native configurations
+
+GNU/Linux/RISC-V		riscv*-*-linux*
+
+* New targets
+
+GNU/Linux/RISC-V		riscv*-*-linux*
+
 *** Changes in GDB 8.2
 
 * The 'set disassembler-options' command now supports specifying options
diff --git a/gdb/configure.host b/gdb/configure.host
index 6bcb8da74c..23a2f16399 100644
--- a/gdb/configure.host
+++ b/gdb/configure.host
@@ -149,6 +149,8 @@ powerpc64*-*-linux*)	gdb_host=ppc64-linux
 			;;
 powerpc*-*-linux*)	gdb_host=linux ;;
 
+riscv*-*-linux*)	gdb_host=linux ;;
+
 s390*-*-linux*)		gdb_host=linux ;;
 
 sh*-*-netbsdelf* | sh*-*-knetbsd*-gnu)
diff --git a/gdb/configure.nat b/gdb/configure.nat
index 7611266d86..feddeaa5e0 100644
--- a/gdb/configure.nat
+++ b/gdb/configure.nat
@@ -267,6 +267,10 @@ case ${gdb_host} in
 		# Host: PowerPC, running Linux
 		NATDEPFILES="${NATDEPFILES} ppc-linux-nat.o ppc-linux.o"
 		;;
+	    riscv*)
+		# Host: RISC-V, running Linux
+		NATDEPFILES="${NATDEPFILES} riscv-linux-nat.o"
+		;;
 	    s390)
 		# Host: S390, running Linux
 		NATDEPFILES="${NATDEPFILES} s390-linux-nat.o"
diff --git a/gdb/configure.tgt b/gdb/configure.tgt
index f197160896..5e3bd5de71 100644
--- a/gdb/configure.tgt
+++ b/gdb/configure.tgt
@@ -517,6 +517,12 @@ s390*-*-linux*)
 	build_gdbserver=yes
 	;;
 
+riscv*-*-linux*)
+	# Target: Linux/RISC-V
+	gdb_target_obs="riscv-linux-tdep.o riscv-tdep.o glibc-tdep.o \
+ 			linux-tdep.o solib-svr4.o symfile-mem.o linux-record.o"
+	;;
+
 riscv*-*-*)
 	# Target: RISC-V architecture
 	gdb_target_obs="riscv-tdep.o"
-- 
2.17.1

[PATCH 4/5] RISC-V: Add native linux support.

[Jim Wilson]

Add initial native support for riscv*-linux*.

	gdb/
	* riscv-linux-nat.c: New file.
---
 gdb/riscv-linux-nat.c | 277 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 277 insertions(+)
 create mode 100644 gdb/riscv-linux-nat.c

diff --git a/gdb/riscv-linux-nat.c b/gdb/riscv-linux-nat.c
new file mode 100644
index 0000000000..b525220759
--- /dev/null
+++ b/gdb/riscv-linux-nat.c
@@ -0,0 +1,277 @@
+/* Native-dependent code for GNU/Linux RISC-V.
+   Copyright (C) 2018 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 "defs.h"
+#include "regcache.h"
+#include "gregset.h"
+#include "linux-nat.h"
+#include "elf.h"
+#include "riscv-tdep.h"
+
+#include <sys/ptrace.h>
+
+class riscv_linux_nat_target final : public linux_nat_target
+{
+public:
+  /* Add our register access methods.  */
+  void fetch_registers (struct regcache *regcache, int regnum) override;
+  void store_registers (struct regcache *regcache, int regnum) override;
+};
+
+static riscv_linux_nat_target the_riscv_linux_nat_target;
+
+/* Copy general purpose register REGNUM (or all gp regs if REGNUM == -1)
+   from regset GREGS into REGCACHE.  */
+
+void
+supply_gregset_regnum (struct regcache *regcache, const prgregset_t *gregs,
+		       int regnum)
+{
+  int i;
+  const elf_greg_t *regp = *gregs;
+
+  if (regnum == -1)
+    {
+      /* We only support the integer registers and PC here.  */
+      for (i = RISCV_ZERO_REGNUM + 1; i < RISCV_PC_REGNUM; i++)
+	regcache->raw_supply (i, regp + i);
+
+      /* GDB stores PC in reg 32.  Linux kernel stores it in reg 0.  */
+      regcache->raw_supply (32, regp + 0);
+
+      /* Fill the inaccessible zero register with zero.  */
+      regcache->raw_supply_zeroed (0);
+    }
+  else if (regnum == RISCV_ZERO_REGNUM)
+    regcache->raw_supply_zeroed (0);
+  else if (regnum > RISCV_ZERO_REGNUM && regnum < RISCV_PC_REGNUM)
+    regcache->raw_supply (regnum, regp + regnum);
+  else if (regnum == RISCV_PC_REGNUM)
+    regcache->raw_supply (32, regp + 0);
+}
+
+/* Copy all general purpose registers from regset GREGS into REGCACHE.  */
+
+void
+supply_gregset (struct regcache *regcache, const prgregset_t *gregs)
+{
+  supply_gregset_regnum (regcache, gregs, -1);
+}
+
+/* Copy floating point register REGNUM (or all fp regs if REGNUM == -1)
+   from regset FPREGS into REGCACHE.  */
+
+void
+supply_fpregset_regnum (struct regcache *regcache, const prfpregset_t *fpregs,
+			int regnum)
+{
+  int i;
+
+  if (regnum == -1)
+    {
+      /* We only support the FP registers and FCSR here.  */
+      for (i = RISCV_FIRST_FP_REGNUM; i <= RISCV_LAST_FP_REGNUM; i++)
+	regcache->raw_supply (i, &fpregs->__d.__f[i - RISCV_FIRST_FP_REGNUM]);
+
+      regcache->raw_supply (RISCV_CSR_FCSR_REGNUM, &fpregs->__d.__fcsr);
+    }
+  else if (regnum >= RISCV_FIRST_FP_REGNUM && regnum <= RISCV_LAST_FP_REGNUM)
+    regcache->raw_supply (regnum,
+			  &fpregs->__d.__f[regnum - RISCV_FIRST_FP_REGNUM]);
+  else if (regnum == RISCV_CSR_FCSR_REGNUM)
+    regcache->raw_supply (RISCV_CSR_FCSR_REGNUM, &fpregs->__d.__fcsr);
+}
+
+/* Copy all floating point registers from regset FPREGS into REGCACHE.  */
+
+void
+supply_fpregset (struct regcache *regcache, const prfpregset_t *fpregs)
+{
+  supply_fpregset_regnum (regcache, fpregs, -1);
+}
+
+/* Copy general purpose register REGNUM (or all gp regs if REGNUM == -1)
+   from REGCACHE into regset GREGS.  */
+
+void
+fill_gregset (const struct regcache *regcache, prgregset_t *gregs, int regnum)
+{
+  elf_greg_t *regp = *gregs;
+
+  if (regnum == -1)
+    {
+      /* We only support the integer registers and PC here.  */
+      for (int i = RISCV_ZERO_REGNUM + 1; i < RISCV_PC_REGNUM; i++)
+	regcache->raw_collect (i, regp + i);
+
+      regcache->raw_collect (32, regp + 0);
+    }
+  else if (regnum == RISCV_ZERO_REGNUM)
+    /* Nothing to do here.  */
+    ;
+  else if (regnum > RISCV_ZERO_REGNUM && regnum < RISCV_PC_REGNUM)
+    regcache->raw_collect (regnum, regp + regnum);
+  else if (regnum == RISCV_PC_REGNUM)
+    regcache->raw_collect (32, regp + 0);
+}
+
+/* Copy floating point register REGNUM (or all fp regs if REGNUM == -1)
+   from REGCACHE into regset FPREGS.  */
+
+void
+fill_fpregset (const struct regcache *regcache, prfpregset_t *fpregs,
+	       int regnum)
+{
+  if (regnum == -1)
+    {
+      /* We only support the FP registers and FCSR here.  */
+      for (int i = RISCV_FIRST_FP_REGNUM; i <= RISCV_LAST_FP_REGNUM; i++)
+	regcache->raw_collect (i, &fpregs->__d.__f[i - RISCV_FIRST_FP_REGNUM]);
+
+      regcache->raw_collect (RISCV_CSR_FCSR_REGNUM, &fpregs->__d.__fcsr);
+    }
+  else if (regnum >= RISCV_FIRST_FP_REGNUM && regnum <= RISCV_LAST_FP_REGNUM)
+    regcache->raw_collect (regnum,
+			   &fpregs->__d.__f[regnum - RISCV_FIRST_FP_REGNUM]);
+  else if (regnum == RISCV_CSR_FCSR_REGNUM)
+    regcache->raw_collect (RISCV_CSR_FCSR_REGNUM, &fpregs->__d.__fcsr);
+}
+
+/* Fetch REGNUM (or all registers if REGNUM == -1) from the target
+   into REGCACHE using PTRACE_GETREGSET.  */
+
+void
+riscv_linux_nat_target::fetch_registers (struct regcache *regcache, int regnum)
+{
+  int tid;
+
+  tid = get_ptrace_pid (regcache->ptid());
+
+  if ((regnum >= RISCV_ZERO_REGNUM && regnum <= RISCV_PC_REGNUM)
+      || (regnum == -1))
+    {
+      struct iovec iov;
+      elf_gregset_t regs;
+
+      iov.iov_base = &regs;
+      iov.iov_len = sizeof (regs);
+
+      if (ptrace (PTRACE_GETREGSET, tid, NT_PRSTATUS,
+		  (PTRACE_TYPE_ARG3) &iov) == -1)
+	perror_with_name (_("Couldn't get registers"));
+      else
+	supply_gregset_regnum (regcache, &regs, regnum);
+    }
+
+  if ((regnum >= RISCV_FIRST_FP_REGNUM
+       && regnum <= RISCV_LAST_FP_REGNUM)
+      || (regnum == RISCV_CSR_FCSR_REGNUM)
+      || (regnum == -1))
+    {
+      struct iovec iov;
+      elf_fpregset_t regs;
+
+      iov.iov_base = &regs;
+      iov.iov_len = sizeof (regs);
+
+      if (ptrace (PTRACE_GETREGSET, tid, NT_PRFPREG,
+		  (PTRACE_TYPE_ARG3) &iov) == -1)
+	perror_with_name (_("Couldn't get registers"));
+      else
+	supply_fpregset_regnum (regcache, &regs, regnum);
+    }
+
+  if (regnum == RISCV_CSR_MISA_REGNUM)
+    {
+      /* TODO: Need to add a ptrace call for this.  */
+      regcache->raw_supply_zeroed (regnum);
+    }
+
+  /* Access to other CSRs has potential security issues, don't support them for
+     now.  */
+}
+
+/* Store REGNUM (or all registers if REGNUM == -1) to the target
+   from REGCACHE using PTRACE_SETREGSET.  */
+
+void
+riscv_linux_nat_target::store_registers (struct regcache *regcache, int regnum)
+{
+  int tid;
+
+  tid = get_ptrace_pid (regcache->ptid ());
+
+  if ((regnum >= RISCV_ZERO_REGNUM && regnum <= RISCV_PC_REGNUM)
+      || (regnum == -1))
+    {
+      struct iovec iov;
+      elf_gregset_t regs;
+
+      iov.iov_base = &regs;
+      iov.iov_len = sizeof (regs);
+
+      if (ptrace (PTRACE_GETREGSET, tid, NT_PRSTATUS,
+		  (PTRACE_TYPE_ARG3) &iov) == -1)
+	perror_with_name (_("Couldn't get registers"));
+      else
+	{
+	  fill_gregset (regcache, &regs, regnum);
+
+	  if (ptrace (PTRACE_SETREGSET, tid, NT_PRSTATUS,
+		      (PTRACE_TYPE_ARG3) &iov) == -1)
+	    perror_with_name (_("Couldn't set registers"));
+	}
+    }
+
+  if ((regnum >= RISCV_FIRST_FP_REGNUM
+       && regnum <= RISCV_LAST_FP_REGNUM)
+      || (regnum == RISCV_CSR_FCSR_REGNUM)
+      || (regnum == -1))
+    {
+      struct iovec iov;
+      elf_fpregset_t regs;
+
+      iov.iov_base = &regs;
+      iov.iov_len = sizeof (regs);
+
+      if (ptrace (PTRACE_GETREGSET, tid, NT_PRFPREG,
+		  (PTRACE_TYPE_ARG3) &iov) == -1)
+	perror_with_name (_("Couldn't get registers"));
+      else
+	{
+	  fill_fpregset (regcache, &regs, regnum);
+
+	  if (ptrace (PTRACE_SETREGSET, tid, NT_PRFPREG,
+		      (PTRACE_TYPE_ARG3) &iov) == -1)
+	    perror_with_name (_("Couldn't set registers"));
+	}
+    }
+
+  /* Access to CSRs has potential security issues, don't support them for
+     now.  */
+}
+
+/* Initialize RISC-V Linux native support.  */
+
+void
+_initialize_riscv_linux_nat (void)
+{
+  /* Register the target.  */
+  linux_target = &the_riscv_linux_nat_target;
+  add_inf_child_target (&the_riscv_linux_nat_target);
+}
-- 
2.17.1

Re: [PATCH 0/5] RISC-V Linux native port

[Andrew Burgess]

* Jim Wilson <jimw@sifive.com> [2018-08-07 19:12:26 -0700]:

> Here are my current patches to add the RISC-V Linux native support to
> gdb.

Excellent!

>        These have been tested on a Hifive Unleashed running Fedora as
> previously mentioned and are working reasonably well.
> 
> One caveat I should mention here is that this port requires multiple
> Linux kernel patches and a GLIBC patch, which people may not already
> have.  I'm not sure how to handle that.  I have a README.md in my
> personal github repo that points at the patches.

I think instructions on how to put together a working environment is
fine.  I guess in this case access to actual hardware is probably
required so the number of people testing is going to be pretty low.
However, a link would be nice, so we can know we're all looking at the
same set of instructions.

Thanks,
Andrew

>                                                   There is 1 linux
> kernel patch that was added in the previous merge window, 2 being
> added in the current merge window, and one I haven't submitted yet for
> FP register support because it needs some cleanup work before
> upstreaming.   There is also a glibc patch that was just added last
> week.

Re: [PATCH 1/5] RISC-V: Make riscv_isa_xlen a global function.

[Andrew Burgess]

* Jim Wilson <jimw@sifive.com> [2018-08-07 19:14:06 -0700]:

> This allows the function to be used from riscv OS files, which also need to
> depend on XLEN size.
> 
> 	gdb/
> 	* riscv-tdep.c (riscv_isa_xlen): Drop static.
> 	* riscv-tdep.h (riscv_isa_xlen): Add extern declaration.

Looks good.

Thanks,
Andrew



> ---
>  gdb/riscv-tdep.c | 2 +-
>  gdb/riscv-tdep.h | 3 +++
>  2 files changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/gdb/riscv-tdep.c b/gdb/riscv-tdep.c
> index abcac98016..20181896c5 100644
> --- a/gdb/riscv-tdep.c
> +++ b/gdb/riscv-tdep.c
> @@ -346,7 +346,7 @@ riscv_has_feature (struct gdbarch *gdbarch, char feature)
>     Possible return values are 4, 8, or 16 for RiscV variants RV32, RV64, or
>     RV128.  */
>  
> -static int
> +int
>  riscv_isa_xlen (struct gdbarch *gdbarch)
>  {
>    switch (gdbarch_tdep (gdbarch)->abi.fields.base_len)
> diff --git a/gdb/riscv-tdep.h b/gdb/riscv-tdep.h
> index 4fc05976ba..b35266daf7 100644
> --- a/gdb/riscv-tdep.h
> +++ b/gdb/riscv-tdep.h
> @@ -76,4 +76,7 @@ struct gdbarch_tdep
>    unsigned core_features;
>  };
>  
> +/* Return the width in bytes of the general purpose registers for GDBARCH.  */
> +extern int riscv_isa_xlen (struct gdbarch *gdbarch);
> +
>  #endif /* RISCV_TDEP_H */
> -- 
> 2.17.1
> 

Re: [PATCH 2/5] RISC-V: Add software single step support.

[Andrew Burgess]

* Jim Wilson <jimw@sifive.com> [2018-08-07 19:16:07 -0700]:

> This adds software single step support that is needed by the linux native port.
> This is modeled after equivalent code in the MIPS port.
> 
> This also fixes a few bugs in the compressed instruction decode support.  Some
> instructions are RV32/RV64 specific, and this wasn't being checked.  Also, a
> few instructions were accidentally using the non-compressed is_* function.
> 
> This has been tested on a HiFive Unleashed running Fedora, by putting a
> breakpoint on start, typing stepi, and then holding down the return key until
> it finishes, and observing that I see everything I expect to see along the way.
> There is a problem in _dl_addr where I get into an infinite loop, but it seems
> to be some synchronization code that doesn't agree with single step, so I have
> to find the end of the loop, put a breakpoint there, continue, and then single
> step again until the end.
> 
> 	gdb/
> 	* riscv-tdep.c (enum opcode): Add jump, branch, lr, and sc opcodes.
> 	(decode_register_index_short): New.
> 	(decode_j_type_insn, decode_cj_type_insn): New.
> 	(decode_b_type_insn, decode_cb_type_insn): New.
> 	(riscv_insn::decode): Add support for jumps, branches, lr, and sc.  New
> 	local xlen.  Check xlen when decoding ambiguous compressed insns.  In
> 	compressed decode, use is_c_lui_insn instead of is_lui_insn, and
> 	is_c_sw_insn instead of is_sw_insn.
> 	(riscv_next_pc, riscv_next_pc_atomic_sequence): New.
> 	(riscv_software_single_step): New.
> 	* riscv-tdep.h (riscv_software_single_step): Declare.

Looks good to me.

Thanks,
Andrew



> ---
>  gdb/riscv-tdep.c | 250 +++++++++++++++++++++++++++++++++++++++++++++--
>  gdb/riscv-tdep.h |   4 +
>  2 files changed, 248 insertions(+), 6 deletions(-)
> 
> diff --git a/gdb/riscv-tdep.c b/gdb/riscv-tdep.c
> index 20181896c5..1df1300100 100644
> --- a/gdb/riscv-tdep.c
> +++ b/gdb/riscv-tdep.c
> @@ -880,6 +880,18 @@ public:
>        LUI,
>        SD,
>        SW,
> +      /* These are needed for software breakopint support.  */
> +      JAL,
> +      JALR,
> +      BEQ,
> +      BNE,
> +      BLT,
> +      BGE,
> +      BLTU,
> +      BGEU,
> +      /* These are needed for stepping over atomic sequences.  */
> +      LR,
> +      SC,
>  
>        /* Other instructions are not interesting during the prologue scan, and
>  	 are ignored.  */
> @@ -936,6 +948,12 @@ private:
>      return (opcode >> offset) & 0x1F;
>    }
>  
> +  /* Extract 5 bit register field at OFFSET from instruction OPCODE.  */
> +  int decode_register_index_short (unsigned long opcode, int offset)
> +  {
> +    return ((opcode >> offset) & 0x7) + 8;
> +  }
> +
>    /* Helper for DECODE, decode 32-bit R-type instruction.  */
>    void decode_r_type_insn (enum opcode opcode, ULONGEST ival)
>    {
> @@ -987,6 +1005,36 @@ private:
>      m_imm.s = EXTRACT_UTYPE_IMM (ival);
>    }
>  
> +  /* Helper for DECODE, decode 32-bit J-type instruction.  */
> +  void decode_j_type_insn (enum opcode opcode, ULONGEST ival)
> +  {
> +    m_opcode = opcode;
> +    m_rd = decode_register_index (ival, OP_SH_RD);
> +    m_imm.s = EXTRACT_UJTYPE_IMM (ival);
> +  }
> +
> +  /* Helper for DECODE, decode 32-bit J-type instruction.  */
> +  void decode_cj_type_insn (enum opcode opcode, ULONGEST ival)
> +  {
> +    m_opcode = opcode;
> +    m_imm.s = EXTRACT_RVC_J_IMM (ival);
> +  }
> +
> +  void decode_b_type_insn (enum opcode opcode, ULONGEST ival)
> +  {
> +    m_opcode = opcode;
> +    m_rs1 = decode_register_index (ival, OP_SH_RS1);
> +    m_rs2 = decode_register_index (ival, OP_SH_RS2);
> +    m_imm.s = EXTRACT_SBTYPE_IMM (ival);
> +  }
> +
> +  void decode_cb_type_insn (enum opcode opcode, ULONGEST ival)
> +  {
> +    m_opcode = opcode;
> +    m_rs1 = decode_register_index_short (ival, OP_SH_CRS1S);
> +    m_imm.s = EXTRACT_RVC_B_IMM (ival);
> +  }
> +
>    /* Fetch instruction from target memory at ADDR, return the content of
>       the instruction, and update LEN with the instruction length.  */
>    static ULONGEST fetch_instruction (struct gdbarch *gdbarch,
> @@ -1083,32 +1131,83 @@ riscv_insn::decode (struct gdbarch *gdbarch, CORE_ADDR pc)
>  	decode_s_type_insn (SD, ival);
>        else if (is_sw_insn (ival))
>  	decode_s_type_insn (SW, ival);
> +      else if (is_jal_insn (ival))
> +	decode_j_type_insn (JAL, ival);
> +      else if (is_jalr_insn (ival))
> +	decode_i_type_insn (JALR, ival);
> +      else if (is_beq_insn (ival))
> +	decode_b_type_insn (BEQ, ival);
> +      else if (is_bne_insn (ival))
> +	decode_b_type_insn (BNE, ival);
> +      else if (is_blt_insn (ival))
> +	decode_b_type_insn (BLT, ival);
> +      else if (is_bge_insn (ival))
> +	decode_b_type_insn (BGE, ival);
> +      else if (is_bltu_insn (ival))
> +	decode_b_type_insn (BLTU, ival);
> +      else if (is_bgeu_insn (ival))
> +	decode_b_type_insn (BGEU, ival);
> +      else if (is_lr_w_insn (ival))
> +	decode_r_type_insn (LR, ival);
> +      else if (is_lr_d_insn (ival))
> +	decode_r_type_insn (LR, ival);
> +      else if (is_sc_w_insn (ival))
> +	decode_r_type_insn (SC, ival);
> +      else if (is_sc_d_insn (ival))
> +	decode_r_type_insn (SC, ival);
>        else
>  	/* None of the other fields are valid in this case.  */
>  	m_opcode = OTHER;
>      }
>    else if (m_length == 2)
>      {
> -      if (is_c_add_insn (ival))
> +      int xlen = riscv_isa_xlen (gdbarch);
> +
> +      /* C_ADD and C_JALR have the same opcode.  If RS2 is 0, then this is a
> +	 C_JALR.  So must try to match C_JALR first as it has more bits in
> +	 mask.  */
> +      if (is_c_jalr_insn (ival))
> +	decode_cr_type_insn (JALR, ival);
> +      else if (is_c_add_insn (ival))
>  	decode_cr_type_insn (ADD, ival);
> -      else if (is_c_addw_insn (ival))
> +      /* C_ADDW is RV64 and RV128 only.  */
> +      else if (xlen != 4 && is_c_addw_insn (ival))
>  	decode_cr_type_insn (ADDW, ival);
>        else if (is_c_addi_insn (ival))
>  	decode_ci_type_insn (ADDI, ival);
> -      else if (is_c_addiw_insn (ival))
> +      /* C_ADDIW and C_JAL have the same opcode.  C_ADDIW is RV64 and RV128
> +	 only and C_JAL is RV32 only.  */
> +      else if (xlen != 4 && is_c_addiw_insn (ival))
>  	decode_ci_type_insn (ADDIW, ival);
> +      else if (xlen == 4 && is_c_jal_insn (ival))
> +	decode_cj_type_insn (JAL, ival);
> +      /* C_ADDI16SP and C_LUI have the same opcode.  If RD is 2, then this is a
> +	 C_ADDI16SP.  So must try to match C_ADDI16SP first as it has more bits
> +	 in mask.  */
>        else if (is_c_addi16sp_insn (ival))
>  	{
>  	  m_opcode = ADDI;
>  	  m_rd = m_rs1 = decode_register_index (ival, OP_SH_RD);
>  	  m_imm.s = EXTRACT_RVC_ADDI16SP_IMM (ival);
>  	}
> -      else if (is_lui_insn (ival))
> +      else if (is_c_lui_insn (ival))
>  	m_opcode = OTHER;
> -      else if (is_c_sd_insn (ival))
> +      /* C_SD and C_FSW have the same opcode.  C_SD is RV64 and RV128 only,
> +	 and C_FSW is RV32 only.  */
> +      else if (xlen != 4 && is_c_sd_insn (ival))
>  	m_opcode = OTHER;
> -      else if (is_sw_insn (ival))
> +      else if (is_c_sw_insn (ival))
>  	m_opcode = OTHER;
> +      /* C_JR and C_MV have the same opcode.  If RS2 is 0, then this is a C_JR.
> +	 So must try to match C_JR first as it ahs more bits in mask.  */
> +      else if (is_c_jr_insn (ival))
> +	decode_cr_type_insn (JALR, ival);
> +      else if (is_c_j_insn (ival))
> +	decode_cj_type_insn (JAL, ival);
> +      else if (is_c_beqz_insn (ival))
> +	decode_cb_type_insn (BEQ, ival);
> +      else if (is_c_bnez_insn (ival))
> +	decode_cb_type_insn (BNE, ival);
>        else
>  	/* None of the other fields of INSN are valid in this case.  */
>  	m_opcode = OTHER;
> @@ -2610,6 +2709,145 @@ riscv_invalidate_inferior_data (struct inferior *inf)
>      }
>  }
>  
> +/* This decodes the current instruction and determines the address of the
> +   next instruction.  */
> +
> +static CORE_ADDR
> +riscv_next_pc (struct regcache *regcache, CORE_ADDR pc)
> +{
> +  struct gdbarch *gdbarch = regcache->arch ();
> +  struct riscv_insn insn;
> +  CORE_ADDR next_pc;
> +
> +  insn.decode (gdbarch, pc);
> +  next_pc = pc + insn.length ();
> +
> +  if (insn.opcode () == riscv_insn::JAL)
> +    next_pc = pc + insn.imm_signed ();
> +  else if (insn.opcode () == riscv_insn::JALR)
> +    {
> +      LONGEST source;
> +      regcache->cooked_read (insn.rs1 (), &source);
> +      next_pc = (source + insn.imm_signed ()) & ~(CORE_ADDR) 0x1;
> +    }
> +  else if (insn.opcode () == riscv_insn::BEQ)
> +    {
> +      LONGEST src1, src2;
> +      regcache->cooked_read (insn.rs1 (), &src1);
> +      regcache->cooked_read (insn.rs2 (), &src2);
> +      if (src1 == src2)
> +	next_pc = pc + insn.imm_signed ();
> +    }
> +  else if (insn.opcode () == riscv_insn::BNE)
> +    {
> +      LONGEST src1, src2;
> +      regcache->cooked_read (insn.rs1 (), &src1);
> +      regcache->cooked_read (insn.rs2 (), &src2);
> +      if (src1 != src2)
> +	next_pc = pc + insn.imm_signed ();
> +    }
> +  else if (insn.opcode () == riscv_insn::BLT)
> +    {
> +      LONGEST src1, src2;
> +      regcache->cooked_read (insn.rs1 (), &src1);
> +      regcache->cooked_read (insn.rs2 (), &src2);
> +      if (src1 < src2)
> +	next_pc = pc + insn.imm_signed ();
> +    }
> +  else if (insn.opcode () == riscv_insn::BGE)
> +    {
> +      LONGEST src1, src2;
> +      regcache->cooked_read (insn.rs1 (), &src1);
> +      regcache->cooked_read (insn.rs2 (), &src2);
> +      if (src1 >= src2)
> +	next_pc = pc + insn.imm_signed ();
> +    }
> +  else if (insn.opcode () == riscv_insn::BLTU)
> +    {
> +      ULONGEST src1, src2;
> +      regcache->cooked_read (insn.rs1 (), &src1);
> +      regcache->cooked_read (insn.rs2 (), &src2);
> +      if (src1 < src2)
> +	next_pc = pc + insn.imm_signed ();
> +    }
> +  else if (insn.opcode () == riscv_insn::BGEU)
> +    {
> +      ULONGEST src1, src2;
> +      regcache->cooked_read (insn.rs1 (), &src1);
> +      regcache->cooked_read (insn.rs2 (), &src2);
> +      if (src1 >= src2)
> +	next_pc = pc + insn.imm_signed ();
> +    }
> +
> +  return next_pc;
> +}
> +
> +/* We can't put a breakpoint in the middle of a lr/sc atomic sequence, so look
> +   for the end of the sequence and put the breakpoint there.  */
> +
> +static bool
> +riscv_next_pc_atomic_sequence (struct regcache *regcache, CORE_ADDR pc,
> +			       CORE_ADDR *next_pc)
> +{
> +  struct gdbarch *gdbarch = regcache->arch ();
> +  struct riscv_insn insn;
> +  CORE_ADDR cur_step_pc = pc;
> +  CORE_ADDR last_addr = 0;
> +
> +  /* First instruction has to be a load reserved.  */
> +  insn.decode (gdbarch, cur_step_pc);
> +  if (insn.opcode () != riscv_insn::LR)
> +    return false;
> +  cur_step_pc = cur_step_pc + insn.length ();
> +
> +  /* Next instruction should be branch to exit.  */
> +  insn.decode (gdbarch, cur_step_pc);
> +  if (insn.opcode () != riscv_insn::BNE)
> +    return false;
> +  last_addr = cur_step_pc + insn.imm_signed ();
> +  cur_step_pc = cur_step_pc + insn.length ();
> +
> +  /* Next instruction should be store conditional.  */
> +  insn.decode (gdbarch, cur_step_pc);
> +  if (insn.opcode () != riscv_insn::SC)
> +    return false;
> +  cur_step_pc = cur_step_pc + insn.length ();
> +
> +  /* Next instruction should be branch to start.  */
> +  insn.decode (gdbarch, cur_step_pc);
> +  if (insn.opcode () != riscv_insn::BNE)
> +    return false;
> +  if (pc != (cur_step_pc + insn.imm_signed ()))
> +    return false;
> +  cur_step_pc = cur_step_pc + insn.length ();
> +
> +  /* We should now be at the end of the sequence.  */
> +  if (cur_step_pc != last_addr)
> +    return false;
> +
> +  *next_pc = cur_step_pc;
> +  return true;
> +}
> +
> +/* This is called just before we want to resume the inferior, if we want to
> +   single-step it but there is no hardware or kernel single-step support.  We
> +   find the target of the coming instruction and breakpoint it.  */
> +
> +std::vector<CORE_ADDR>
> +riscv_software_single_step (struct regcache *regcache)
> +{
> +  CORE_ADDR pc, next_pc;
> +
> +  pc = regcache_read_pc (regcache);
> +
> +  if (riscv_next_pc_atomic_sequence (regcache, pc, &next_pc))
> +    return {next_pc};
> +
> +  next_pc = riscv_next_pc (regcache, pc);
> +
> +  return {next_pc};
> +}
> +
>  void
>  _initialize_riscv_tdep (void)
>  {
> diff --git a/gdb/riscv-tdep.h b/gdb/riscv-tdep.h
> index b35266daf7..8591116922 100644
> --- a/gdb/riscv-tdep.h
> +++ b/gdb/riscv-tdep.h
> @@ -79,4 +79,8 @@ struct gdbarch_tdep
>  /* Return the width in bytes of the general purpose registers for GDBARCH.  */
>  extern int riscv_isa_xlen (struct gdbarch *gdbarch);
>  
> +/* Single step based on where the current instruction will take us.  */
> +extern std::vector<CORE_ADDR>
> +riscv_software_single_step (struct regcache *regcache);
> +
>  #endif /* RISCV_TDEP_H */
> -- 
> 2.17.1
> 

Re: [PATCH 3/5] RISC-V: Add linux target support.

[Andrew Burgess]

* Jim Wilson <jimw@sifive.com> [2018-08-07 19:16:38 -0700]:

> Add initial target support for riscv*-linux*.
> 
> 	gdb/
> 	* riscv-linux-tdep.c: New file.

This seems fine to me, with a couple of minor issues...

> ---
>  gdb/riscv-linux-tdep.c | 88 ++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 88 insertions(+)
>  create mode 100644 gdb/riscv-linux-tdep.c
> 
> diff --git a/gdb/riscv-linux-tdep.c b/gdb/riscv-linux-tdep.c
> new file mode 100644
> index 0000000000..c98dba70ca
> --- /dev/null
> +++ b/gdb/riscv-linux-tdep.c
> @@ -0,0 +1,88 @@
> +/* Target-dependent code for GNU/Linux on RISC-V processors.
> +   Copyright (C) 2018 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 "defs.h"
> +#include "riscv-tdep.h"
> +#include "osabi.h"
> +#include "glibc-tdep.h"
> +#include "linux-tdep.h"
> +#include "solib-svr4.h"
> +#include "regset.h"
> +
> +static const struct regcache_map_entry riscv_linux_gregmap[] =
> +{
> +  { 1,  RISCV_PC_REGNUM, 0 },
> +  { 31, RISCV_RA_REGNUM, 0 }, /* x1 to x31 */
> +  { 0 }
> +};
> +
> +const struct regset riscv_linux_gregset =
> +{
> +  riscv_linux_gregmap, regcache_supply_regset, regcache_collect_regset
> +};

Could this not be static?

Also, I think 'riscv_linux_gregmap' and 'riscv_linux_gregset' should
both have comments against them, I'm pretty sure the expectation these
days is that every definition has a comment....

Otherwise, this looks great,

Thanks,
Andrew




> +
> +/* Define hook for core file support.  */
> +
> +static void
> +riscv_linux_iterate_over_regset_sections (struct gdbarch *gdbarch,
> +                                          iterate_over_regset_sections_cb *cb,
> +                                          void *cb_data,
> +                                          const struct regcache *regcache)
> +{
> +  cb (".reg", (32 * riscv_isa_xlen (gdbarch)),
> +      &riscv_linux_gregset, NULL, cb_data);
> +
> +  /* TODO: Add FP register support.  */
> +}
> +
> +/* Initialize RISC-V Linux ABI info.  */
> +
> +static void
> +riscv_linux_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
> +{
> +  linux_init_abi (info, gdbarch);
> +
> +  set_gdbarch_software_single_step (gdbarch, riscv_software_single_step);
> +
> +  set_solib_svr4_fetch_link_map_offsets (gdbarch,
> +					 (riscv_isa_xlen (gdbarch) == 4
> +					  ? svr4_ilp32_fetch_link_map_offsets
> +					  : svr4_lp64_fetch_link_map_offsets));
> +
> +  /* GNU/Linux uses SVR4-style shared libraries.  */
> +  set_gdbarch_skip_trampoline_code (gdbarch, find_solib_trampoline_target);
> +
> +  /* GNU/Linux uses the dynamic linker included in the GNU C Library.  */
> +  set_gdbarch_skip_solib_resolver (gdbarch, glibc_skip_solib_resolver);
> +
> +  /* Enable TLS support.  */
> +  set_gdbarch_fetch_tls_load_module_address (gdbarch,
> +                                             svr4_fetch_objfile_link_map);
> +
> +  set_gdbarch_iterate_over_regset_sections
> +    (gdbarch, riscv_linux_iterate_over_regset_sections);
> +}
> +
> +/* Initialize RISC-V Linux target support.  */
> +
> +void
> +_initialize_riscv_linux_tdep (void)
> +{
> +  gdbarch_register_osabi (bfd_arch_riscv, 0, GDB_OSABI_LINUX,
> +			  riscv_linux_init_abi);
> +}
> -- 
> 2.17.1
> 

Re: [PATCH 4/5] RISC-V: Add native linux support.

[Andrew Burgess]

* Jim Wilson <jimw@sifive.com> [2018-08-07 19:17:10 -0700]:

> Add initial native support for riscv*-linux*.
> 
> 	gdb/
> 	* riscv-linux-nat.c: New file.

This looks good with a couple of minor nits...

> ---
>  gdb/riscv-linux-nat.c | 277 ++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 277 insertions(+)
>  create mode 100644 gdb/riscv-linux-nat.c
> 
> diff --git a/gdb/riscv-linux-nat.c b/gdb/riscv-linux-nat.c
> new file mode 100644
> index 0000000000..b525220759
> --- /dev/null
> +++ b/gdb/riscv-linux-nat.c
> @@ -0,0 +1,277 @@
> +/* Native-dependent code for GNU/Linux RISC-V.
> +   Copyright (C) 2018 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 "defs.h"
> +#include "regcache.h"
> +#include "gregset.h"
> +#include "linux-nat.h"
> +#include "elf.h"
> +#include "riscv-tdep.h"
> +
> +#include <sys/ptrace.h>
> +
> +class riscv_linux_nat_target final : public linux_nat_target

This probably needs a comment.

> +{
> +public:
> +  /* Add our register access methods.  */
> +  void fetch_registers (struct regcache *regcache, int regnum) override;
> +  void store_registers (struct regcache *regcache, int regnum) override;
> +};
> +
> +static riscv_linux_nat_target the_riscv_linux_nat_target;
> +
> +/* Copy general purpose register REGNUM (or all gp regs if REGNUM == -1)
> +   from regset GREGS into REGCACHE.  */
> +
> +void
> +supply_gregset_regnum (struct regcache *regcache, const prgregset_t *gregs,
> +		       int regnum)

Can this be static?

> +{
> +  int i;
> +  const elf_greg_t *regp = *gregs;
> +
> +  if (regnum == -1)
> +    {
> +      /* We only support the integer registers and PC here.  */
> +      for (i = RISCV_ZERO_REGNUM + 1; i < RISCV_PC_REGNUM; i++)
> +	regcache->raw_supply (i, regp + i);
> +
> +      /* GDB stores PC in reg 32.  Linux kernel stores it in reg 0.  */
> +      regcache->raw_supply (32, regp + 0);
> +
> +      /* Fill the inaccessible zero register with zero.  */
> +      regcache->raw_supply_zeroed (0);
> +    }
> +  else if (regnum == RISCV_ZERO_REGNUM)
> +    regcache->raw_supply_zeroed (0);
> +  else if (regnum > RISCV_ZERO_REGNUM && regnum < RISCV_PC_REGNUM)
> +    regcache->raw_supply (regnum, regp + regnum);
> +  else if (regnum == RISCV_PC_REGNUM)
> +    regcache->raw_supply (32, regp + 0);
> +}
> +
> +/* Copy all general purpose registers from regset GREGS into REGCACHE.  */
> +
> +void
> +supply_gregset (struct regcache *regcache, const prgregset_t *gregs)
> +{
> +  supply_gregset_regnum (regcache, gregs, -1);
> +}
> +
> +/* Copy floating point register REGNUM (or all fp regs if REGNUM == -1)
> +   from regset FPREGS into REGCACHE.  */
> +
> +void
> +supply_fpregset_regnum (struct regcache *regcache, const prfpregset_t *fpregs,
> +			int regnum)

Can this be static?

> +{
> +  int i;
> +
> +  if (regnum == -1)
> +    {
> +      /* We only support the FP registers and FCSR here.  */
> +      for (i = RISCV_FIRST_FP_REGNUM; i <= RISCV_LAST_FP_REGNUM; i++)
> +	regcache->raw_supply (i, &fpregs->__d.__f[i - RISCV_FIRST_FP_REGNUM]);
> +
> +      regcache->raw_supply (RISCV_CSR_FCSR_REGNUM, &fpregs->__d.__fcsr);
> +    }
> +  else if (regnum >= RISCV_FIRST_FP_REGNUM && regnum <= RISCV_LAST_FP_REGNUM)
> +    regcache->raw_supply (regnum,
> +			  &fpregs->__d.__f[regnum - RISCV_FIRST_FP_REGNUM]);
> +  else if (regnum == RISCV_CSR_FCSR_REGNUM)
> +    regcache->raw_supply (RISCV_CSR_FCSR_REGNUM, &fpregs->__d.__fcsr);
> +}
> +
> +/* Copy all floating point registers from regset FPREGS into REGCACHE.  */
> +
> +void
> +supply_fpregset (struct regcache *regcache, const prfpregset_t *fpregs)
> +{
> +  supply_fpregset_regnum (regcache, fpregs, -1);
> +}
> +
> +/* Copy general purpose register REGNUM (or all gp regs if REGNUM == -1)
> +   from REGCACHE into regset GREGS.  */
> +
> +void
> +fill_gregset (const struct regcache *regcache, prgregset_t *gregs, int regnum)
> +{
> +  elf_greg_t *regp = *gregs;
> +
> +  if (regnum == -1)
> +    {
> +      /* We only support the integer registers and PC here.  */
> +      for (int i = RISCV_ZERO_REGNUM + 1; i < RISCV_PC_REGNUM; i++)
> +	regcache->raw_collect (i, regp + i);
> +
> +      regcache->raw_collect (32, regp + 0);
> +    }
> +  else if (regnum == RISCV_ZERO_REGNUM)
> +    /* Nothing to do here.  */
> +    ;
> +  else if (regnum > RISCV_ZERO_REGNUM && regnum < RISCV_PC_REGNUM)
> +    regcache->raw_collect (regnum, regp + regnum);
> +  else if (regnum == RISCV_PC_REGNUM)
> +    regcache->raw_collect (32, regp + 0);
> +}
> +
> +/* Copy floating point register REGNUM (or all fp regs if REGNUM == -1)
> +   from REGCACHE into regset FPREGS.  */
> +
> +void
> +fill_fpregset (const struct regcache *regcache, prfpregset_t *fpregs,
> +	       int regnum)
> +{
> +  if (regnum == -1)
> +    {
> +      /* We only support the FP registers and FCSR here.  */
> +      for (int i = RISCV_FIRST_FP_REGNUM; i <= RISCV_LAST_FP_REGNUM; i++)
> +	regcache->raw_collect (i, &fpregs->__d.__f[i - RISCV_FIRST_FP_REGNUM]);
> +
> +      regcache->raw_collect (RISCV_CSR_FCSR_REGNUM, &fpregs->__d.__fcsr);
> +    }
> +  else if (regnum >= RISCV_FIRST_FP_REGNUM && regnum <= RISCV_LAST_FP_REGNUM)
> +    regcache->raw_collect (regnum,
> +			   &fpregs->__d.__f[regnum - RISCV_FIRST_FP_REGNUM]);
> +  else if (regnum == RISCV_CSR_FCSR_REGNUM)
> +    regcache->raw_collect (RISCV_CSR_FCSR_REGNUM, &fpregs->__d.__fcsr);
> +}
> +
> +/* Fetch REGNUM (or all registers if REGNUM == -1) from the target
> +   into REGCACHE using PTRACE_GETREGSET.  */
> +
> +void
> +riscv_linux_nat_target::fetch_registers (struct regcache *regcache, int regnum)
> +{
> +  int tid;
> +
> +  tid = get_ptrace_pid (regcache->ptid());
> +
> +  if ((regnum >= RISCV_ZERO_REGNUM && regnum <= RISCV_PC_REGNUM)
> +      || (regnum == -1))
> +    {
> +      struct iovec iov;
> +      elf_gregset_t regs;
> +
> +      iov.iov_base = &regs;
> +      iov.iov_len = sizeof (regs);
> +
> +      if (ptrace (PTRACE_GETREGSET, tid, NT_PRSTATUS,
> +		  (PTRACE_TYPE_ARG3) &iov) == -1)
> +	perror_with_name (_("Couldn't get registers"));
> +      else
> +	supply_gregset_regnum (regcache, &regs, regnum);
> +    }
> +
> +  if ((regnum >= RISCV_FIRST_FP_REGNUM
> +       && regnum <= RISCV_LAST_FP_REGNUM)
> +      || (regnum == RISCV_CSR_FCSR_REGNUM)
> +      || (regnum == -1))
> +    {
> +      struct iovec iov;
> +      elf_fpregset_t regs;
> +
> +      iov.iov_base = &regs;
> +      iov.iov_len = sizeof (regs);
> +
> +      if (ptrace (PTRACE_GETREGSET, tid, NT_PRFPREG,
> +		  (PTRACE_TYPE_ARG3) &iov) == -1)
> +	perror_with_name (_("Couldn't get registers"));
> +      else
> +	supply_fpregset_regnum (regcache, &regs, regnum);
> +    }
> +
> +  if (regnum == RISCV_CSR_MISA_REGNUM)

Should this also have a 'regnum == -1' check?

> +    {
> +      /* TODO: Need to add a ptrace call for this.  */
> +      regcache->raw_supply_zeroed (regnum);
> +    }
> +
> +  /* Access to other CSRs has potential security issues, don't support them for
> +     now.  */

Should we add a warning if regnum is NOT -1, but IS for a CSR?  Would
this warning end up triggering all the time?  Maybe if it does it
should just trigger once?

I'm just thinking if a user specifically asks for a csr register, it
might be nice if GDB would say "can't" instead of silently failing...

> +}
> +
> +/* Store REGNUM (or all registers if REGNUM == -1) to the target
> +   from REGCACHE using PTRACE_SETREGSET.  */
> +
> +void
> +riscv_linux_nat_target::store_registers (struct regcache *regcache, int regnum)
> +{
> +  int tid;
> +
> +  tid = get_ptrace_pid (regcache->ptid ());
> +
> +  if ((regnum >= RISCV_ZERO_REGNUM && regnum <= RISCV_PC_REGNUM)
> +      || (regnum == -1))
> +    {
> +      struct iovec iov;
> +      elf_gregset_t regs;
> +
> +      iov.iov_base = &regs;
> +      iov.iov_len = sizeof (regs);
> +
> +      if (ptrace (PTRACE_GETREGSET, tid, NT_PRSTATUS,
> +		  (PTRACE_TYPE_ARG3) &iov) == -1)
> +	perror_with_name (_("Couldn't get registers"));
> +      else
> +	{
> +	  fill_gregset (regcache, &regs, regnum);
> +
> +	  if (ptrace (PTRACE_SETREGSET, tid, NT_PRSTATUS,
> +		      (PTRACE_TYPE_ARG3) &iov) == -1)
> +	    perror_with_name (_("Couldn't set registers"));
> +	}
> +    }
> +
> +  if ((regnum >= RISCV_FIRST_FP_REGNUM
> +       && regnum <= RISCV_LAST_FP_REGNUM)
> +      || (regnum == RISCV_CSR_FCSR_REGNUM)
> +      || (regnum == -1))
> +    {
> +      struct iovec iov;
> +      elf_fpregset_t regs;
> +
> +      iov.iov_base = &regs;
> +      iov.iov_len = sizeof (regs);
> +
> +      if (ptrace (PTRACE_GETREGSET, tid, NT_PRFPREG,
> +		  (PTRACE_TYPE_ARG3) &iov) == -1)
> +	perror_with_name (_("Couldn't get registers"));
> +      else
> +	{
> +	  fill_fpregset (regcache, &regs, regnum);
> +
> +	  if (ptrace (PTRACE_SETREGSET, tid, NT_PRFPREG,
> +		      (PTRACE_TYPE_ARG3) &iov) == -1)
> +	    perror_with_name (_("Couldn't set registers"));
> +	}
> +    }
> +
> +  /* Access to CSRs has potential security issues, don't support them for
> +     now.  */

As above, it might be nice if GDB produced some warning when the user
tries to write to an unsupported register...

> +}
> +
> +/* Initialize RISC-V Linux native support.  */
> +
> +void
> +_initialize_riscv_linux_nat (void)
> +{
> +  /* Register the target.  */
> +  linux_target = &the_riscv_linux_nat_target;
> +  add_inf_child_target (&the_riscv_linux_nat_target);
> +}
> -- 
> 2.17.1
> 

With the above issues fixed, this looks fine.

Thanks,
Andrew

Re: [PATCH 5/5] RISC-V: Add configure support riscv*-linux*.

[Andrew Burgess]

* Jim Wilson <jimw@sifive.com> [2018-08-07 19:17:34 -0700]:

> This adds the target and native configure support, and the NEWS entries for
> the new target and native configurations.
> 
> 	gdb/
> 	* Makefile.in (ALLDEPFILES): Add riscv-linux-nat.c, riscv-linux-tdep.c.
> 	* NEWS: Mention new GNU/Linux RISC-V target.
> 	* configure.host: Add riscv*-*-linux*.
> 	* configure.nat: Add riscv*.
> 	* configure.tgt: Add riscv*-*-linux*.

I'm not sure if I can approve changes outside of riscv-* files, but if
I could I'd approve these :)

Thanks,
Andrew



> ---
>  gdb/Makefile.in    | 4 ++++
>  gdb/NEWS           | 8 ++++++++
>  gdb/configure.host | 2 ++
>  gdb/configure.nat  | 4 ++++
>  gdb/configure.tgt  | 6 ++++++
>  5 files changed, 24 insertions(+)
> 
> diff --git a/gdb/Makefile.in b/gdb/Makefile.in
> index 8c744d70c0..280b3b1283 100644
> --- a/gdb/Makefile.in
> +++ b/gdb/Makefile.in
> @@ -752,6 +752,8 @@ ALL_TARGET_OBS = \
>  	ppc-sysv-tdep.o \
>  	ppc64-tdep.o \
>  	ravenscar-thread.o \
> +	riscv-linux-nat.o \
> +	riscv-linux-tdep.o \
>  	riscv-tdep.o \
>  	rl78-tdep.o \
>  	rs6000-aix-tdep.o \
> @@ -2300,6 +2302,8 @@ ALLDEPFILES = \
>  	procfs.c \
>  	ravenscar-thread.c \
>  	remote-sim.c \
> +	riscv-linux-nat.c \
> +	riscv-linux-tdep.c \
>  	riscv-tdep.c \
>  	rl78-tdep.c \
>  	rs6000-lynx178-tdep.c \
> diff --git a/gdb/NEWS b/gdb/NEWS
> index 669ed2d0eb..62cde1cde2 100644
> --- a/gdb/NEWS
> +++ b/gdb/NEWS
> @@ -38,6 +38,14 @@ thread apply [all | COUNT | -COUNT] [FLAG]... COMMAND
>    FLAG arguments allow to control what output to produce and how to handle
>    errors raised when applying COMMAND to a thread.
>  
> +* New native configurations
> +
> +GNU/Linux/RISC-V		riscv*-*-linux*
> +
> +* New targets
> +
> +GNU/Linux/RISC-V		riscv*-*-linux*
> +
>  *** Changes in GDB 8.2
>  
>  * The 'set disassembler-options' command now supports specifying options
> diff --git a/gdb/configure.host b/gdb/configure.host
> index 6bcb8da74c..23a2f16399 100644
> --- a/gdb/configure.host
> +++ b/gdb/configure.host
> @@ -149,6 +149,8 @@ powerpc64*-*-linux*)	gdb_host=ppc64-linux
>  			;;
>  powerpc*-*-linux*)	gdb_host=linux ;;
>  
> +riscv*-*-linux*)	gdb_host=linux ;;
> +
>  s390*-*-linux*)		gdb_host=linux ;;
>  
>  sh*-*-netbsdelf* | sh*-*-knetbsd*-gnu)
> diff --git a/gdb/configure.nat b/gdb/configure.nat
> index 7611266d86..feddeaa5e0 100644
> --- a/gdb/configure.nat
> +++ b/gdb/configure.nat
> @@ -267,6 +267,10 @@ case ${gdb_host} in
>  		# Host: PowerPC, running Linux
>  		NATDEPFILES="${NATDEPFILES} ppc-linux-nat.o ppc-linux.o"
>  		;;
> +	    riscv*)
> +		# Host: RISC-V, running Linux
> +		NATDEPFILES="${NATDEPFILES} riscv-linux-nat.o"
> +		;;
>  	    s390)
>  		# Host: S390, running Linux
>  		NATDEPFILES="${NATDEPFILES} s390-linux-nat.o"
> diff --git a/gdb/configure.tgt b/gdb/configure.tgt
> index f197160896..5e3bd5de71 100644
> --- a/gdb/configure.tgt
> +++ b/gdb/configure.tgt
> @@ -517,6 +517,12 @@ s390*-*-linux*)
>  	build_gdbserver=yes
>  	;;
>  
> +riscv*-*-linux*)
> +	# Target: Linux/RISC-V
> +	gdb_target_obs="riscv-linux-tdep.o riscv-tdep.o glibc-tdep.o \
> + 			linux-tdep.o solib-svr4.o symfile-mem.o linux-record.o"
> +	;;
> +
>  riscv*-*-*)
>  	# Target: RISC-V architecture
>  	gdb_target_obs="riscv-tdep.o"
> -- 
> 2.17.1
> 

Re: [PATCH 5/5] RISC-V: Add configure support riscv*-linux*.

[Tom Tromey]

>>>>> "Jim" == Jim Wilson <jimw@sifive.com> writes:

Jim> 	* Makefile.in (ALLDEPFILES): Add riscv-linux-nat.c, riscv-linux-tdep.c.

This doesn't mention the ALL_TARGET_OBS change.

Jim> 	* NEWS: Mention new GNU/Linux RISC-V target.

Eli should review this bit.

Jim> 	* configure.host: Add riscv*-*-linux*.
Jim> 	* configure.nat: Add riscv*.
Jim> 	* configure.tgt: Add riscv*-*-linux*.

These parts are ok.

Jim> diff --git a/gdb/Makefile.in b/gdb/Makefile.in
Jim> index 8c744d70c0..280b3b1283 100644
Jim> --- a/gdb/Makefile.in
Jim> +++ b/gdb/Makefile.in
Jim> @@ -752,6 +752,8 @@ ALL_TARGET_OBS = \
Jim>  	ppc-sysv-tdep.o \
Jim>  	ppc64-tdep.o \
Jim>  	ravenscar-thread.o \
Jim> +	riscv-linux-nat.o \
Jim> +	riscv-linux-tdep.o \
Jim>  	riscv-tdep.o \
Jim>  	rl78-tdep.o \
Jim>  	rs6000-aix-tdep.o \

I think only the tdep file should be listed here.

ALL_TARGET_OBS is used when --enable-targets=all is given.
In this case, we would not want the riscv-linux-nat.o object to be
linked in, since we're not necessarily building on linux.

Is riscv a 64-bit architecture?  (I don't know.)  If so, gdb still
splits 64-bit targets into a separate variable, I think in case one is
building on a 32-bit machine without a 64-bit integer type (or maybe if
you didn't want to use the extra memory to inflate a bunch of type
sizes, not sure).  In this case you'd want to add the tdep file to
ALL_64_TARGET_OBS instead.

I wonder if this 32/64 bit thing is needed any more.

Tom

Re: [PATCH 0/5] RISC-V Linux native port

[Jim Wilson]

On Wed, Aug 8, 2018 at 5:41 AM, Andrew Burgess
<andrew.burgess@embecosm.com> wrote:
> I think instructions on how to put together a working environment is
> fine.  I guess in this case access to actual hardware is probably
> required so the number of people testing is going to be pretty low.
> However, a link would be nice, so we can know we're all looking at the
> same set of instructions.

Where would I put those instructions?

Jim

Re: [PATCH 1/5] RISC-V: Make riscv_isa_xlen a global function.

[Jim Wilson]

On Wed, Aug 8, 2018 at 5:42 AM, Andrew Burgess
<andrew.burgess@embecosm.com> wrote:
> * Jim Wilson <jimw@sifive.com> [2018-08-07 19:14:06 -0700]:
>
>> This allows the function to be used from riscv OS files, which also need to
>> depend on XLEN size.
>>
>>       gdb/
>>       * riscv-tdep.c (riscv_isa_xlen): Drop static.
>>       * riscv-tdep.h (riscv_isa_xlen): Add extern declaration.
>
> Looks good.

Committed.

Jim

Re: [PATCH 2/5] RISC-V: Add software single step support.

[Jim Wilson]

On Wed, Aug 8, 2018 at 5:50 AM, Andrew Burgess
<andrew.burgess@embecosm.com> wrote:
> * Jim Wilson <jimw@sifive.com> [2018-08-07 19:16:07 -0700]:
>>       gdb/
>>       * riscv-tdep.c (enum opcode): Add jump, branch, lr, and sc opcodes.
>>       (decode_register_index_short): New.
>>       (decode_j_type_insn, decode_cj_type_insn): New.
>>       (decode_b_type_insn, decode_cb_type_insn): New.
>>       (riscv_insn::decode): Add support for jumps, branches, lr, and sc.  New
>>       local xlen.  Check xlen when decoding ambiguous compressed insns.  In
>>       compressed decode, use is_c_lui_insn instead of is_lui_insn, and
>>       is_c_sw_insn instead of is_sw_insn.
>>       (riscv_next_pc, riscv_next_pc_atomic_sequence): New.
>>       (riscv_software_single_step): New.
>>       * riscv-tdep.h (riscv_software_single_step): Declare.
>
> Looks good to me.

Committed.

Jim

Re: [PATCH 0/5] RISC-V Linux native port

[Andrew Burgess]

* Jim Wilson <jimw@sifive.com> [2018-08-08 10:41:52 -0700]:

> On Wed, Aug 8, 2018 at 5:41 AM, Andrew Burgess
> <andrew.burgess@embecosm.com> wrote:
> > I think instructions on how to put together a working environment is
> > fine.  I guess in this case access to actual hardware is probably
> > required so the number of people testing is going to be pretty low.
> > However, a link would be nice, so we can know we're all looking at the
> > same set of instructions.
> 
> Where would I put those instructions?

I was more suggesting at least having the link on the mailing list so
anyone whose interested can find the instructions.

I guess you could include the information in the commit message for
patch 5/5 as that's the point where building riscv/linux becomes
possible, so knowing how to do it would be useful.


Thanks,
Andrew

Re: [PATCH 3/5] RISC-V: Add linux target support.

[Jim Wilson]

On Wed, Aug 8, 2018 at 7:41 AM, Andrew Burgess
<andrew.burgess@embecosm.com> wrote:
>> +const struct regset riscv_linux_gregset =
>> +{
>> +  riscv_linux_gregmap, regcache_supply_regset, regcache_collect_regset
>> +};
>
> Could this not be static?
>
> Also, I think 'riscv_linux_gregmap' and 'riscv_linux_gregset' should
> both have comments against them, I'm pretty sure the expectation these
> days is that every definition has a comment....

I added the static and the comments.

Jim

Re: [PATCH 5/5] RISC-V: Add configure support riscv*-linux*.

[Eli Zaretskii]

> From: Tom Tromey <tom@tromey.com>
> Cc: gdb-patches@sourceware.org
> Date: Wed, 08 Aug 2018 11:29:26 -0600
> 
> >>>>> "Jim" == Jim Wilson <jimw@sifive.com> writes:
> 
> Jim> 	* Makefile.in (ALLDEPFILES): Add riscv-linux-nat.c, riscv-linux-tdep.c.
> 
> This doesn't mention the ALL_TARGET_OBS change.
> 
> Jim> 	* NEWS: Mention new GNU/Linux RISC-V target.
> 
> Eli should review this bit.

Sorry, missed that.  It's okay.

Thanks.

[PATCH 3/5] RISC-V: Add linux target support.

[Jim Wilson]

Add initial target support for riscv*-linux*.

	gdb/
	* riscv-linux-tdep.c: New file.
---
 gdb/riscv-linux-tdep.c | 94 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 94 insertions(+)
 create mode 100644 gdb/riscv-linux-tdep.c

diff --git a/gdb/riscv-linux-tdep.c b/gdb/riscv-linux-tdep.c
new file mode 100644
index 0000000000..47c2ab6d4f
--- /dev/null
+++ b/gdb/riscv-linux-tdep.c
@@ -0,0 +1,94 @@
+/* Target-dependent code for GNU/Linux on RISC-V processors.
+   Copyright (C) 2018 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 "defs.h"
+#include "riscv-tdep.h"
+#include "osabi.h"
+#include "glibc-tdep.h"
+#include "linux-tdep.h"
+#include "solib-svr4.h"
+#include "regset.h"
+
+/* Define the general register mapping.  The kernel puts the PC at offset 0,
+   gdb puts it at offset 32.  Register x0 is always 0 and can be ignored.
+   Registers x1 to x31 are in the same place.  */
+
+static const struct regcache_map_entry riscv_linux_gregmap[] =
+{
+  { 1,  RISCV_PC_REGNUM, 0 },
+  { 31, RISCV_RA_REGNUM, 0 }, /* x1 to x31 */
+  { 0 }
+};
+
+/* Define the general register regset.  */
+
+static const struct regset riscv_linux_gregset =
+{
+  riscv_linux_gregmap, regcache_supply_regset, regcache_collect_regset
+};
+
+/* Define hook for core file support.  */
+
+static void
+riscv_linux_iterate_over_regset_sections (struct gdbarch *gdbarch,
+                                          iterate_over_regset_sections_cb *cb,
+                                          void *cb_data,
+                                          const struct regcache *regcache)
+{
+  cb (".reg", (32 * riscv_isa_xlen (gdbarch)),
+      &riscv_linux_gregset, NULL, cb_data);
+
+  /* TODO: Add FP register support.  */
+}
+
+/* Initialize RISC-V Linux ABI info.  */
+
+static void
+riscv_linux_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
+{
+  linux_init_abi (info, gdbarch);
+
+  set_gdbarch_software_single_step (gdbarch, riscv_software_single_step);
+
+  set_solib_svr4_fetch_link_map_offsets (gdbarch,
+					 (riscv_isa_xlen (gdbarch) == 4
+					  ? svr4_ilp32_fetch_link_map_offsets
+					  : svr4_lp64_fetch_link_map_offsets));
+
+  /* GNU/Linux uses SVR4-style shared libraries.  */
+  set_gdbarch_skip_trampoline_code (gdbarch, find_solib_trampoline_target);
+
+  /* GNU/Linux uses the dynamic linker included in the GNU C Library.  */
+  set_gdbarch_skip_solib_resolver (gdbarch, glibc_skip_solib_resolver);
+
+  /* Enable TLS support.  */
+  set_gdbarch_fetch_tls_load_module_address (gdbarch,
+                                             svr4_fetch_objfile_link_map);
+
+  set_gdbarch_iterate_over_regset_sections
+    (gdbarch, riscv_linux_iterate_over_regset_sections);
+}
+
+/* Initialize RISC-V Linux target support.  */
+
+void
+_initialize_riscv_linux_tdep (void)
+{
+  gdbarch_register_osabi (bfd_arch_riscv, 0, GDB_OSABI_LINUX,
+			  riscv_linux_init_abi);
+}
-- 
2.17.1

Re: [PATCH 0/5] RISC-V Linux native port

[Jim Wilson]

On Wed, Aug 8, 2018 at 11:16 AM, Andrew Burgess
<andrew.burgess@embecosm.com> wrote:
> * Jim Wilson <jimw@sifive.com> [2018-08-08 10:41:52 -0700]:
> I was more suggesting at least having the link on the mailing list so
> anyone whose interested can find the instructions.

Makes sense.  You can find info in
    https://github.com/jim-wilson/riscv-linux-native-gdb/blob/jimw-riscv-linux-gdb/README.md
and my collected set of linux kernel patches is
    https://github.com/jim-wilson/riscv-linux-native-gdb/blob/jimw-riscv-linux-gdb/riscv-linux.txt

I'll have to update the info, as the second and third patches are in
the queue for this merge window, and the fourth patch needs to be
rewritten before I can upstream it.  And now that gdb patches are
going in upstream I will want to point people at the upstream gdb tree
for further work.

Jim

Re: [PATCH 1/5] RISC-V: Make riscv_isa_xlen a global function.

[Simon Marchi]

On 2018-08-07 22:14, Jim Wilson wrote:
> This allows the function to be used from riscv OS files, which also 
> need to
> depend on XLEN size.
> 
> 	gdb/
> 	* riscv-tdep.c (riscv_isa_xlen): Drop static.
> 	* riscv-tdep.h (riscv_isa_xlen): Add extern declaration.
> ---
>  gdb/riscv-tdep.c | 2 +-
>  gdb/riscv-tdep.h | 3 +++
>  2 files changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/gdb/riscv-tdep.c b/gdb/riscv-tdep.c
> index abcac98016..20181896c5 100644
> --- a/gdb/riscv-tdep.c
> +++ b/gdb/riscv-tdep.c
> @@ -346,7 +346,7 @@ riscv_has_feature (struct gdbarch *gdbarch, char 
> feature)
>     Possible return values are 4, 8, or 16 for RiscV variants RV32, 
> RV64, or
>     RV128.  */
> 
> -static int
> +int
>  riscv_isa_xlen (struct gdbarch *gdbarch)
>  {
>    switch (gdbarch_tdep (gdbarch)->abi.fields.base_len)

Just a note for further patches (you can fix this instance as an obvious 
patch if you want): when a function is extern, put the documentation in 
the .h, and

   /* See riscv-tdep.h.  */

in the header file.  This way, the two comments won't diverge over time.

Simon

Re: [PATCH 5/5] RISC-V: Add configure support riscv*-linux*.

[Palmer Dabbelt]

On Wed, 08 Aug 2018 10:29:26 PDT (-0700), tom@tromey.com wrote:
>>>>>> "Jim" == Jim Wilson <jimw@sifive.com> writes:
>
> Jim> 	* Makefile.in (ALLDEPFILES): Add riscv-linux-nat.c, riscv-linux-tdep.c.
>
> This doesn't mention the ALL_TARGET_OBS change.
>
> Jim> 	* NEWS: Mention new GNU/Linux RISC-V target.
>
> Eli should review this bit.
>
> Jim> 	* configure.host: Add riscv*-*-linux*.
> Jim> 	* configure.nat: Add riscv*.
> Jim> 	* configure.tgt: Add riscv*-*-linux*.
>
> These parts are ok.
>
> Jim> diff --git a/gdb/Makefile.in b/gdb/Makefile.in
> Jim> index 8c744d70c0..280b3b1283 100644
> Jim> --- a/gdb/Makefile.in
> Jim> +++ b/gdb/Makefile.in
> Jim> @@ -752,6 +752,8 @@ ALL_TARGET_OBS = \
> Jim>  	ppc-sysv-tdep.o \
> Jim>  	ppc64-tdep.o \
> Jim>  	ravenscar-thread.o \
> Jim> +	riscv-linux-nat.o \
> Jim> +	riscv-linux-tdep.o \
> Jim>  	riscv-tdep.o \
> Jim>  	rl78-tdep.o \
> Jim>  	rs6000-aix-tdep.o \
>
> I think only the tdep file should be listed here.
>
> ALL_TARGET_OBS is used when --enable-targets=all is given.
> In this case, we would not want the riscv-linux-nat.o object to be
> linked in, since we're not necessarily building on linux.
>
> Is riscv a 64-bit architecture?  (I don't know.)  If so, gdb still
> splits 64-bit targets into a separate variable, I think in case one is
> building on a 32-bit machine without a 64-bit integer type (or maybe if
> you didn't want to use the extra memory to inflate a bunch of type
> sizes, not sure).  In this case you'd want to add the tdep file to
> ALL_64_TARGET_OBS instead.

RISC-V is actually a family of ISAs, not one specific ISA.  There are 4 base 
ISAs: rv32e (16 32-bit registers), rv32i (32 32-bit registers), rv64i (32 
64-bit registers), and rv128i (32 128-bit revisers).  They all look very 
similar and we developed the ports together, so they share the vast majority of 
the source.  Since they're so similar we try to make sure that the various 
toolchain components always support all of them (aside from rv128i, which 
nobody has started serious work on and is only a placeholder in the spec).

To enable this support in BFD we decided to put both the 32-bit and 64-bit 
RISC-V targets in the list of 32-bit targets.  This allows toolchains 
configured with the 32-bit tuples to generate code for the 64-bit targets, 
which when coupled with multilib gives you a functioning setup.

I'd like to keep this behavior in GDB if possible, so "riscv32-unknown-elf-gdb" 
can debug rv64i-based targets.  One of my goals is to rely as little as 
possible on the tuple so users don't have to go find and build a whole bunch of 
them -- whenever I'm a user I find that to be a pain.

> I wonder if this 32/64 bit thing is needed any more.

Part of our rationale for doing this was that if you can build GCC 7 (the first 
version we support), then your host probably has support for 64-bit integer 
types.  Nobody else has seemed to notice yet, but there also aren't that many 
RISC-V users so we're far from a representative sample.

Re: [PATCH 5/5] RISC-V: Add configure support riscv*-linux*.

[Tom Tromey]

>>>>> "Palmer" == Palmer Dabbelt <palmer@sifive.com> writes:

Palmer> To enable this support in BFD we decided to put both the 32-bit and 64-bit 
Palmer> RISC-V targets in the list of 32-bit targets.  This allows toolchains 
Palmer> configured with the 32-bit tuples to generate code for the 64-bit targets, 
Palmer> which when coupled with multilib gives you a functioning setup.

Palmer> I'd like to keep this behavior in GDB if possible, so "riscv32-unknown-elf-gdb" 
Palmer> can debug rv64i-based targets.

My main concern is really whether --disable-64-bit-bfd still works.
I don't know if people still use this, but as long as it exists it seems
like it should continue to work.  If RISC-V works in this setup, then it
is totally fine by me for gdb to follow, assuming it works as well.

Maybe to really test it has to be a 32 bit build with
--disable-64-bit-bfd and --enable-targets=all.

Tom

Re: [PATCH 5/5] RISC-V: Add configure support riscv*-linux*.

[Tom Tromey]

>>>>> "Tom" == Tom Tromey <tom@tromey.com> writes:

Tom>   If RISC-V works in this setup, then it is totally fine by me for
Tom> gdb to follow, assuming it works as well.

That didn't come out quite like I intended.  I meant to say, if BFD
works in this setup, etc.

Tom

Re: [PATCH 4/5] RISC-V: Add native linux support.

[Jim Wilson]

On Wed, Aug 8, 2018 at 8:58 AM, Andrew Burgess
<andrew.burgess@embecosm.com> wrote:
>> +class riscv_linux_nat_target final : public linux_nat_target
>
> This probably needs a comment.

OK.

>> +void
>> +supply_gregset_regnum (struct regcache *regcache, const prgregset_t *gregs,
>> +                    int regnum)
>
> Can this be static?

Yes.

>> +void
>> +supply_fpregset_regnum (struct regcache *regcache, const prfpregset_t *fpregs,
>> +                     int regnum)
>
> Can this be static?

Yes.

>> +  if (regnum == RISCV_CSR_MISA_REGNUM)
>
> Should this also have a 'regnum == -1' check?

I was going to worry about CSR support later, but it is harmless to
add this now.  I did so.

>> +  /* Access to other CSRs has potential security issues, don't support them for
>> +     now.  */
>
> Should we add a warning if regnum is NOT -1, but IS for a CSR?  Would
> this warning end up triggering all the time?  Maybe if it does it
> should just trigger once?
>
> I'm just thinking if a user specifically asks for a csr register, it
> might be nice if GDB would say "can't" instead of silently failing...

Gdb is tracking register info via regcache, so it knows that the
register was not updated.  I get for instance

(gdb) print $mtvec
$1 = <unavailable>
(gdb) print $mtvec = 0x180
$2 = <unavailable>

So I don't think we need anything in the riscv-linux-nat.c file to cover this.

Jim

[PATCH 4/5] RISC-V: Add native linux support.

[Jim Wilson]

Add initial native support for riscv*-linux*.

	gdb/
	* riscv-linux-nat.c: New file.
---
 gdb/riscv-linux-nat.c | 280 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 280 insertions(+)
 create mode 100644 gdb/riscv-linux-nat.c

diff --git a/gdb/riscv-linux-nat.c b/gdb/riscv-linux-nat.c
new file mode 100644
index 0000000000..3faa9f9c1f
--- /dev/null
+++ b/gdb/riscv-linux-nat.c
@@ -0,0 +1,280 @@
+/* Native-dependent code for GNU/Linux RISC-V.
+   Copyright (C) 2018 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 "defs.h"
+#include "regcache.h"
+#include "gregset.h"
+#include "linux-nat.h"
+#include "elf.h"
+#include "riscv-tdep.h"
+
+#include <sys/ptrace.h>
+
+/* RISC-V Linux native additions to the default linux support.  */
+
+class riscv_linux_nat_target final : public linux_nat_target
+{
+public:
+  /* Add our register access methods.  */
+  void fetch_registers (struct regcache *regcache, int regnum) override;
+  void store_registers (struct regcache *regcache, int regnum) override;
+};
+
+static riscv_linux_nat_target the_riscv_linux_nat_target;
+
+/* Copy general purpose register REGNUM (or all gp regs if REGNUM == -1)
+   from regset GREGS into REGCACHE.  */
+
+static void
+supply_gregset_regnum (struct regcache *regcache, const prgregset_t *gregs,
+		       int regnum)
+{
+  int i;
+  const elf_greg_t *regp = *gregs;
+
+  if (regnum == -1)
+    {
+      /* We only support the integer registers and PC here.  */
+      for (i = RISCV_ZERO_REGNUM + 1; i < RISCV_PC_REGNUM; i++)
+	regcache->raw_supply (i, regp + i);
+
+      /* GDB stores PC in reg 32.  Linux kernel stores it in reg 0.  */
+      regcache->raw_supply (32, regp + 0);
+
+      /* Fill the inaccessible zero register with zero.  */
+      regcache->raw_supply_zeroed (0);
+    }
+  else if (regnum == RISCV_ZERO_REGNUM)
+    regcache->raw_supply_zeroed (0);
+  else if (regnum > RISCV_ZERO_REGNUM && regnum < RISCV_PC_REGNUM)
+    regcache->raw_supply (regnum, regp + regnum);
+  else if (regnum == RISCV_PC_REGNUM)
+    regcache->raw_supply (32, regp + 0);
+}
+
+/* Copy all general purpose registers from regset GREGS into REGCACHE.  */
+
+void
+supply_gregset (struct regcache *regcache, const prgregset_t *gregs)
+{
+  supply_gregset_regnum (regcache, gregs, -1);
+}
+
+/* Copy floating point register REGNUM (or all fp regs if REGNUM == -1)
+   from regset FPREGS into REGCACHE.  */
+
+static void
+supply_fpregset_regnum (struct regcache *regcache, const prfpregset_t *fpregs,
+			int regnum)
+{
+  int i;
+
+  if (regnum == -1)
+    {
+      /* We only support the FP registers and FCSR here.  */
+      for (i = RISCV_FIRST_FP_REGNUM; i <= RISCV_LAST_FP_REGNUM; i++)
+	regcache->raw_supply (i, &fpregs->__d.__f[i - RISCV_FIRST_FP_REGNUM]);
+
+      regcache->raw_supply (RISCV_CSR_FCSR_REGNUM, &fpregs->__d.__fcsr);
+    }
+  else if (regnum >= RISCV_FIRST_FP_REGNUM && regnum <= RISCV_LAST_FP_REGNUM)
+    regcache->raw_supply (regnum,
+			  &fpregs->__d.__f[regnum - RISCV_FIRST_FP_REGNUM]);
+  else if (regnum == RISCV_CSR_FCSR_REGNUM)
+    regcache->raw_supply (RISCV_CSR_FCSR_REGNUM, &fpregs->__d.__fcsr);
+}
+
+/* Copy all floating point registers from regset FPREGS into REGCACHE.  */
+
+void
+supply_fpregset (struct regcache *regcache, const prfpregset_t *fpregs)
+{
+  supply_fpregset_regnum (regcache, fpregs, -1);
+}
+
+/* Copy general purpose register REGNUM (or all gp regs if REGNUM == -1)
+   from REGCACHE into regset GREGS.  */
+
+void
+fill_gregset (const struct regcache *regcache, prgregset_t *gregs, int regnum)
+{
+  elf_greg_t *regp = *gregs;
+
+  if (regnum == -1)
+    {
+      /* We only support the integer registers and PC here.  */
+      for (int i = RISCV_ZERO_REGNUM + 1; i < RISCV_PC_REGNUM; i++)
+	regcache->raw_collect (i, regp + i);
+
+      regcache->raw_collect (32, regp + 0);
+    }
+  else if (regnum == RISCV_ZERO_REGNUM)
+    /* Nothing to do here.  */
+    ;
+  else if (regnum > RISCV_ZERO_REGNUM && regnum < RISCV_PC_REGNUM)
+    regcache->raw_collect (regnum, regp + regnum);
+  else if (regnum == RISCV_PC_REGNUM)
+    regcache->raw_collect (32, regp + 0);
+}
+
+/* Copy floating point register REGNUM (or all fp regs if REGNUM == -1)
+   from REGCACHE into regset FPREGS.  */
+
+void
+fill_fpregset (const struct regcache *regcache, prfpregset_t *fpregs,
+	       int regnum)
+{
+  if (regnum == -1)
+    {
+      /* We only support the FP registers and FCSR here.  */
+      for (int i = RISCV_FIRST_FP_REGNUM; i <= RISCV_LAST_FP_REGNUM; i++)
+	regcache->raw_collect (i, &fpregs->__d.__f[i - RISCV_FIRST_FP_REGNUM]);
+
+      regcache->raw_collect (RISCV_CSR_FCSR_REGNUM, &fpregs->__d.__fcsr);
+    }
+  else if (regnum >= RISCV_FIRST_FP_REGNUM && regnum <= RISCV_LAST_FP_REGNUM)
+    regcache->raw_collect (regnum,
+			   &fpregs->__d.__f[regnum - RISCV_FIRST_FP_REGNUM]);
+  else if (regnum == RISCV_CSR_FCSR_REGNUM)
+    regcache->raw_collect (RISCV_CSR_FCSR_REGNUM, &fpregs->__d.__fcsr);
+}
+
+/* Fetch REGNUM (or all registers if REGNUM == -1) from the target
+   into REGCACHE using PTRACE_GETREGSET.  */
+
+void
+riscv_linux_nat_target::fetch_registers (struct regcache *regcache, int regnum)
+{
+  int tid;
+
+  tid = get_ptrace_pid (regcache->ptid());
+
+  if ((regnum >= RISCV_ZERO_REGNUM && regnum <= RISCV_PC_REGNUM)
+      || (regnum == -1))
+    {
+      struct iovec iov;
+      elf_gregset_t regs;
+
+      iov.iov_base = &regs;
+      iov.iov_len = sizeof (regs);
+
+      if (ptrace (PTRACE_GETREGSET, tid, NT_PRSTATUS,
+		  (PTRACE_TYPE_ARG3) &iov) == -1)
+	perror_with_name (_("Couldn't get registers"));
+      else
+	supply_gregset_regnum (regcache, &regs, regnum);
+    }
+
+  if ((regnum >= RISCV_FIRST_FP_REGNUM
+       && regnum <= RISCV_LAST_FP_REGNUM)
+      || (regnum == RISCV_CSR_FCSR_REGNUM)
+      || (regnum == -1))
+    {
+      struct iovec iov;
+      elf_fpregset_t regs;
+
+      iov.iov_base = &regs;
+      iov.iov_len = sizeof (regs);
+
+      if (ptrace (PTRACE_GETREGSET, tid, NT_PRFPREG,
+		  (PTRACE_TYPE_ARG3) &iov) == -1)
+	perror_with_name (_("Couldn't get registers"));
+      else
+	supply_fpregset_regnum (regcache, &regs, regnum);
+    }
+
+  if ((regnum == RISCV_CSR_MISA_REGNUM)
+      || (regnum == -1))
+    {
+      /* TODO: Need to add a ptrace call for this.  */
+      regcache->raw_supply_zeroed (regnum);
+    }
+
+  /* Access to other CSRs has potential security issues, don't support them for
+     now.  */
+}
+
+/* Store REGNUM (or all registers if REGNUM == -1) to the target
+   from REGCACHE using PTRACE_SETREGSET.  */
+
+void
+riscv_linux_nat_target::store_registers (struct regcache *regcache, int regnum)
+{
+  int tid;
+
+  tid = get_ptrace_pid (regcache->ptid ());
+
+  if ((regnum >= RISCV_ZERO_REGNUM && regnum <= RISCV_PC_REGNUM)
+      || (regnum == -1))
+    {
+      struct iovec iov;
+      elf_gregset_t regs;
+
+      iov.iov_base = &regs;
+      iov.iov_len = sizeof (regs);
+
+      if (ptrace (PTRACE_GETREGSET, tid, NT_PRSTATUS,
+		  (PTRACE_TYPE_ARG3) &iov) == -1)
+	perror_with_name (_("Couldn't get registers"));
+      else
+	{
+	  fill_gregset (regcache, &regs, regnum);
+
+	  if (ptrace (PTRACE_SETREGSET, tid, NT_PRSTATUS,
+		      (PTRACE_TYPE_ARG3) &iov) == -1)
+	    perror_with_name (_("Couldn't set registers"));
+	}
+    }
+
+  if ((regnum >= RISCV_FIRST_FP_REGNUM
+       && regnum <= RISCV_LAST_FP_REGNUM)
+      || (regnum == RISCV_CSR_FCSR_REGNUM)
+      || (regnum == -1))
+    {
+      struct iovec iov;
+      elf_fpregset_t regs;
+
+      iov.iov_base = &regs;
+      iov.iov_len = sizeof (regs);
+
+      if (ptrace (PTRACE_GETREGSET, tid, NT_PRFPREG,
+		  (PTRACE_TYPE_ARG3) &iov) == -1)
+	perror_with_name (_("Couldn't get registers"));
+      else
+	{
+	  fill_fpregset (regcache, &regs, regnum);
+
+	  if (ptrace (PTRACE_SETREGSET, tid, NT_PRFPREG,
+		      (PTRACE_TYPE_ARG3) &iov) == -1)
+	    perror_with_name (_("Couldn't set registers"));
+	}
+    }
+
+  /* Access to CSRs has potential security issues, don't support them for
+     now.  */
+}
+
+/* Initialize RISC-V Linux native support.  */
+
+void
+_initialize_riscv_linux_nat (void)
+{
+  /* Register the target.  */
+  linux_target = &the_riscv_linux_nat_target;
+  add_inf_child_target (&the_riscv_linux_nat_target);
+}
-- 
2.17.1

Re: [PATCH 5/5] RISC-V: Add configure support riscv*-linux*.

[Jim Wilson]

On Wed, Aug 8, 2018 at 10:29 AM, Tom Tromey <tom@tromey.com> wrote:
>>>>>> "Jim" == Jim Wilson <jimw@sifive.com> writes:
>
> Jim>    * Makefile.in (ALLDEPFILES): Add riscv-linux-nat.c, riscv-linux-tdep.c.
>
> This doesn't mention the ALL_TARGET_OBS change.

This is something I noticed was missing at the last minute and managed
to screwed it up when i tried to fix it.  I added the missing
ChangeLog entry.

> Jim>    ravenscar-thread.o \
> Jim> +  riscv-linux-nat.o \
> Jim> +  riscv-linux-tdep.o \
> Jim>    riscv-tdep.o \
> Jim>    rl78-tdep.o \
> Jim>    rs6000-aix-tdep.o \
>
> I think only the tdep file should be listed here.

Yes.  I removed the riscv-linux-nat.o file from this list.

> Is riscv a 64-bit architecture?  (I don't know.)  If so, gdb still
> splits 64-bit targets into a separate variable, I think in case one is
> building on a 32-bit machine without a 64-bit integer type (or maybe if
> you didn't want to use the extra memory to inflate a bunch of type
> sizes, not sure).  In this case you'd want to add the tdep file to
> ALL_64_TARGET_OBS instead.
>
> I wonder if this 32/64 bit thing is needed any more.

To add to what Palmer said, the current riscv port is intended to
support both 32-bit and 64-bit.  It checks a hardware register or the
ELF header flags to get the word size, and then things like register
sizes are based on the word size.  However, I only have access to a
working 64-bit linux port, so the 32-bit linux support has not been
tested yet.  32-bit glibc support was just recently submitted
upstream, and we are hoping to get that resolved in time for the next
glibc release early next year.  The 32-bit linux kernel was booting
but not running init last I heard, so that needs some bug fixing too.
Once 32-bit linux support is sorted out I will start testing the gdb
support for that.  I am currently testing 32-bit linux binutils and
gcc support using a user-mode qemu and some old glibc sources, but I
don't think that I can do any useful gdb testing that way.

While we have support for both 32-bit and 64-bit in theory, we can
probably only support one target at the time, as this gets computed
early and I don't think that we support changes to it.  I think I'd
have to define two targets, and have two sets of initialization
functions, etc.  This is something I can't easily do when I don't have
access to the 32-bit linux target for testing yet.  We also don't have
any support for running 32-bit code on 64-bit systems yet, so this
would be an issue only for cross gdb, and we don't have gdbserver yet
so cross gdb isn't actually very useful at this time.  So this all
ends up being a moot question at the current time.  Only the riscv64
linux gdb support is usable today.

I have an ARM v7 chromebook with crouton Ubuntu chroot environment
that I can use for 32-bit builds.  I can try a build with
--disable-64-bit-bfd and see what happens.  This could take a little
time, it is a slow machine, and I'll have to get the machine set up
for builds again.

Jim

[PATCH 5/5] RISC-V: Add configure support for riscv*-linux*.

[Jim Wilson]

This adds the target and native configure support, and the NEWS entries for
the new target and native configurations.

	gdb/
	* Makefile.in (ALL_TARGET_OBS): Add riscv-linux-tdep.c.
	(ALLDEPFILES): Add riscv-linux-nat.c, riscv-linux-tdep.c.
	* NEWS: Mention new GNU/Linux RISC-V target.
	* configure.host: Add riscv*-*-linux*.
	* configure.nat: Add riscv*.
	* configure.tgt: Add riscv*-*-linux*.
---
 gdb/Makefile.in    | 3 +++
 gdb/NEWS           | 8 ++++++++
 gdb/configure.host | 2 ++
 gdb/configure.nat  | 4 ++++
 gdb/configure.tgt  | 6 ++++++
 5 files changed, 23 insertions(+)

diff --git a/gdb/Makefile.in b/gdb/Makefile.in
index 8c744d70c0..85b54244a5 100644
--- a/gdb/Makefile.in
+++ b/gdb/Makefile.in
@@ -752,6 +752,7 @@ ALL_TARGET_OBS = \
 	ppc-sysv-tdep.o \
 	ppc64-tdep.o \
 	ravenscar-thread.o \
+	riscv-linux-tdep.o \
 	riscv-tdep.o \
 	rl78-tdep.o \
 	rs6000-aix-tdep.o \
@@ -2300,6 +2301,8 @@ ALLDEPFILES = \
 	procfs.c \
 	ravenscar-thread.c \
 	remote-sim.c \
+	riscv-linux-nat.c \
+	riscv-linux-tdep.c \
 	riscv-tdep.c \
 	rl78-tdep.c \
 	rs6000-lynx178-tdep.c \
diff --git a/gdb/NEWS b/gdb/NEWS
index 669ed2d0eb..62cde1cde2 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -38,6 +38,14 @@ thread apply [all | COUNT | -COUNT] [FLAG]... COMMAND
   FLAG arguments allow to control what output to produce and how to handle
   errors raised when applying COMMAND to a thread.
 
+* New native configurations
+
+GNU/Linux/RISC-V		riscv*-*-linux*
+
+* New targets
+
+GNU/Linux/RISC-V		riscv*-*-linux*
+
 *** Changes in GDB 8.2
 
 * The 'set disassembler-options' command now supports specifying options
diff --git a/gdb/configure.host b/gdb/configure.host
index 6bcb8da74c..23a2f16399 100644
--- a/gdb/configure.host
+++ b/gdb/configure.host
@@ -149,6 +149,8 @@ powerpc64*-*-linux*)	gdb_host=ppc64-linux
 			;;
 powerpc*-*-linux*)	gdb_host=linux ;;
 
+riscv*-*-linux*)	gdb_host=linux ;;
+
 s390*-*-linux*)		gdb_host=linux ;;
 
 sh*-*-netbsdelf* | sh*-*-knetbsd*-gnu)
diff --git a/gdb/configure.nat b/gdb/configure.nat
index 7611266d86..feddeaa5e0 100644
--- a/gdb/configure.nat
+++ b/gdb/configure.nat
@@ -267,6 +267,10 @@ case ${gdb_host} in
 		# Host: PowerPC, running Linux
 		NATDEPFILES="${NATDEPFILES} ppc-linux-nat.o ppc-linux.o"
 		;;
+	    riscv*)
+		# Host: RISC-V, running Linux
+		NATDEPFILES="${NATDEPFILES} riscv-linux-nat.o"
+		;;
 	    s390)
 		# Host: S390, running Linux
 		NATDEPFILES="${NATDEPFILES} s390-linux-nat.o"
diff --git a/gdb/configure.tgt b/gdb/configure.tgt
index f197160896..5e3bd5de71 100644
--- a/gdb/configure.tgt
+++ b/gdb/configure.tgt
@@ -517,6 +517,12 @@ s390*-*-linux*)
 	build_gdbserver=yes
 	;;
 
+riscv*-*-linux*)
+	# Target: Linux/RISC-V
+	gdb_target_obs="riscv-linux-tdep.o riscv-tdep.o glibc-tdep.o \
+ 			linux-tdep.o solib-svr4.o symfile-mem.o linux-record.o"
+	;;
+
 riscv*-*-*)
 	# Target: RISC-V architecture
 	gdb_target_obs="riscv-tdep.o"
-- 
2.17.1

Re: [PATCH 5/5] RISC-V: Add configure support riscv*-linux*.

[Eli Zaretskii]

> From: Tom Tromey <tom@tromey.com>
> Cc: tom@tromey.com,  Jim Wilson <jimw@sifive.com>,  gdb-patches@sourceware.org
> Date: Wed, 08 Aug 2018 17:25:59 -0600
> 
> My main concern is really whether --disable-64-bit-bfd still works.
> I don't know if people still use this, but as long as it exists it seems
> like it should continue to work.  If RISC-V works in this setup, then it
> is totally fine by me for gdb to follow, assuming it works as well.
> 
> Maybe to really test it has to be a 32 bit build with
> --disable-64-bit-bfd and --enable-targets=all.

Btw, can you or someone else explain what exactly does
"--enable-64-bit-bfd" do and what practical effects should it have on
GDB?  I frequently wonder whether I want that switch when building
GDB, and I still don't know the answer.

Thanks.

Re: [PATCH 5/5] RISC-V: Add configure support for riscv*-linux*.

[Eli Zaretskii]

> From: Jim Wilson <jimw@sifive.com>
> Cc: Jim Wilson <jimw@sifive.com>
> Date: Wed,  8 Aug 2018 17:26:50 -0700
> 
> This adds the target and native configure support, and the NEWS entries for
> the new target and native configurations.
> 
> 	gdb/
> 	* Makefile.in (ALL_TARGET_OBS): Add riscv-linux-tdep.c.
> 	(ALLDEPFILES): Add riscv-linux-nat.c, riscv-linux-tdep.c.
> 	* NEWS: Mention new GNU/Linux RISC-V target.
> 	* configure.host: Add riscv*-*-linux*.
> 	* configure.nat: Add riscv*.
> 	* configure.tgt: Add riscv*-*-linux*.

OK for the NEWS part.

Thanks.

Re: [PATCH 0/5] RISC-V Linux native port

[Palmer Dabbelt]

On Wed, 08 Aug 2018 11:42:42 PDT (-0700), Jim Wilson wrote:
> On Wed, Aug 8, 2018 at 11:16 AM, Andrew Burgess
> <andrew.burgess@embecosm.com> wrote:
>> * Jim Wilson <jimw@sifive.com> [2018-08-08 10:41:52 -0700]:
>> I was more suggesting at least having the link on the mailing list so
>> anyone whose interested can find the instructions.
>
> Makes sense.  You can find info in
>     https://github.com/jim-wilson/riscv-linux-native-gdb/blob/jimw-riscv-linux-gdb/README.md
> and my collected set of linux kernel patches is
>     https://github.com/jim-wilson/riscv-linux-native-gdb/blob/jimw-riscv-linux-gdb/riscv-linux.txt
>
> I'll have to update the info, as the second and third patches are in
> the queue for this merge window, and the fourth patch needs to be
> rewritten before I can upstream it.  And now that gdb patches are
> going in upstream I will want to point people at the upstream gdb tree
> for further work.

The Linux patch queue is for-next, the last of Jim's accepted patches can be 
found here:

    https://git.kernel.org/pub/scm/linux/kernel/git/palmer/riscv-linux.git/commit/?h=for-next&id=b06d1415ddc43963761bcce379248bc329d314e4

The merge window opens on Monday, I usually tag Linux patch sets on Monday and 
then submit them on Wednesday.  Unless something goes wrong, I'd anticipate 
they'll all be merged to Linus' master by next Friday (not this week).

Thanks!
 
> Jim

Re: [PATCH 5/5] RISC-V: Add configure support riscv*-linux*.

[Jim Wilson]

On Wed, Aug 8, 2018 at 7:36 PM, Eli Zaretskii <eliz@gnu.org> wrote:
>> From: Tom Tromey <tom@tromey.com>
>> Cc: tom@tromey.com,  Jim Wilson <jimw@sifive.com>,  gdb-patches@sourceware.org
>> Date: Wed, 08 Aug 2018 17:25:59 -0600
>>
>> My main concern is really whether --disable-64-bit-bfd still works.
>> I don't know if people still use this, but as long as it exists it seems
>> like it should continue to work.  If RISC-V works in this setup, then it
>> is totally fine by me for gdb to follow, assuming it works as well.
>>
>> Maybe to really test it has to be a 32 bit build with
>> --disable-64-bit-bfd and --enable-targets=all.
>
> Btw, can you or someone else explain what exactly does
> "--enable-64-bit-bfd" do and what practical effects should it have on
> GDB?  I frequently wonder whether I want that switch when building
> GDB, and I still don't know the answer.

Since I'm looking at this at the moment...

BFD will automatically enable 64-bit support if you configure on a
host with 64-bit support, or configure for a target with 64-bit
support.  It will also enable 64-bit support if you use
--enable-64-bit-bfd.  64-bit support means you can use 64-bit integer
types such as long long on a 32-bit host or long on a 64-bit LP64
host.  If 64-bit support is enabled, and your compiler doesn't support
it, then you get a configure error.

If you use --disable-64-bit-bfd, then bfd will disallow use of 64-bit
integer types.  If you configure for a 64-bit target, you will get a
configure error.

As a practical matter, I think --enable/disable-64-bit-bfd doesn't
really do anything useful unless you are configuring with
--enable-targets=all.  In this case, 64-bit targets will be enabled by
default, and will fail to build on a 32-bit host with a compiler that
doesn't support long long.  You can work around that problem by using
--disable-64-bit-bfd which will get you all of the 32-bit targets, and
none of the 64-bit targets, which will allow the build to complete.
Of course, nowadays, finding a 32-bit compiler without long long
support would be hard, so this option combination is unlikely to be
useful to anyone.

I've been looking at the riscv{32,64} bfd support.  Since configuring
for riscv32 enables riscv64 support also, and riscv64 requires 64-bit
support, this means that there is no riscv bfd support at all when
using --disable-64-bit-bfd --enable-targets=all.  It looks like the
gdb riscv support is broken even without my patch, as we can't build a
riscv32-elf gdb if we don't have bfd.  I've got a build running to see
if I can force a build failure.

Jim

Re: [PATCH 5/5] RISC-V: Add configure support riscv*-linux*.

[Tom Tromey]

>>>>> "Jim" == Jim Wilson <jimw@sifive.com> writes:

Jim> I've been looking at the riscv{32,64} bfd support.  Since configuring
Jim> for riscv32 enables riscv64 support also, and riscv64 requires 64-bit
Jim> support, this means that there is no riscv bfd support at all when
Jim> using --disable-64-bit-bfd --enable-targets=all.  It looks like the
Jim> gdb riscv support is broken even without my patch, as we can't build a
Jim> riscv32-elf gdb if we don't have bfd.  I've got a build running to see
Jim> if I can force a build failure.

I just wanted to reiterate that I don't know how important any of this
really is.  The option exists, but I don't know if anybody relies on it.
Perhaps it could be removed instead.  Maybe a question for the binutils
list as well.

Tom

Re: [PATCH 5/5] RISC-V: Add configure support riscv*-linux*.

[Andreas Schwab]

On Aug 08 2018, Jim Wilson <jimw@sifive.com> wrote:

> I've been looking at the riscv{32,64} bfd support.  Since configuring
> for riscv32 enables riscv64 support also, and riscv64 requires 64-bit
> support, this means that there is no riscv bfd support at all when
> using --disable-64-bit-bfd --enable-targets=all.

Isn't that enforced by want64 in bfd/config.bfd?

Andreas.

-- 
Andreas Schwab, SUSE Labs, schwab@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."

Re: [PATCH 4/5] RISC-V: Add native linux support.

[Andrew Burgess]

* Jim Wilson <jimw@sifive.com> [2018-08-08 16:39:08 -0700]:

> Add initial native support for riscv*-linux*.
> 
> 	gdb/
> 	* riscv-linux-nat.c: New file.

Looks good to me.

Thanks,
Andrew


> ---
>  gdb/riscv-linux-nat.c | 280 ++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 280 insertions(+)
>  create mode 100644 gdb/riscv-linux-nat.c
> 
> diff --git a/gdb/riscv-linux-nat.c b/gdb/riscv-linux-nat.c
> new file mode 100644
> index 0000000000..3faa9f9c1f
> --- /dev/null
> +++ b/gdb/riscv-linux-nat.c
> @@ -0,0 +1,280 @@
> +/* Native-dependent code for GNU/Linux RISC-V.
> +   Copyright (C) 2018 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 "defs.h"
> +#include "regcache.h"
> +#include "gregset.h"
> +#include "linux-nat.h"
> +#include "elf.h"
> +#include "riscv-tdep.h"
> +
> +#include <sys/ptrace.h>
> +
> +/* RISC-V Linux native additions to the default linux support.  */
> +
> +class riscv_linux_nat_target final : public linux_nat_target
> +{
> +public:
> +  /* Add our register access methods.  */
> +  void fetch_registers (struct regcache *regcache, int regnum) override;
> +  void store_registers (struct regcache *regcache, int regnum) override;
> +};
> +
> +static riscv_linux_nat_target the_riscv_linux_nat_target;
> +
> +/* Copy general purpose register REGNUM (or all gp regs if REGNUM == -1)
> +   from regset GREGS into REGCACHE.  */
> +
> +static void
> +supply_gregset_regnum (struct regcache *regcache, const prgregset_t *gregs,
> +		       int regnum)
> +{
> +  int i;
> +  const elf_greg_t *regp = *gregs;
> +
> +  if (regnum == -1)
> +    {
> +      /* We only support the integer registers and PC here.  */
> +      for (i = RISCV_ZERO_REGNUM + 1; i < RISCV_PC_REGNUM; i++)
> +	regcache->raw_supply (i, regp + i);
> +
> +      /* GDB stores PC in reg 32.  Linux kernel stores it in reg 0.  */
> +      regcache->raw_supply (32, regp + 0);
> +
> +      /* Fill the inaccessible zero register with zero.  */
> +      regcache->raw_supply_zeroed (0);
> +    }
> +  else if (regnum == RISCV_ZERO_REGNUM)
> +    regcache->raw_supply_zeroed (0);
> +  else if (regnum > RISCV_ZERO_REGNUM && regnum < RISCV_PC_REGNUM)
> +    regcache->raw_supply (regnum, regp + regnum);
> +  else if (regnum == RISCV_PC_REGNUM)
> +    regcache->raw_supply (32, regp + 0);
> +}
> +
> +/* Copy all general purpose registers from regset GREGS into REGCACHE.  */
> +
> +void
> +supply_gregset (struct regcache *regcache, const prgregset_t *gregs)
> +{
> +  supply_gregset_regnum (regcache, gregs, -1);
> +}
> +
> +/* Copy floating point register REGNUM (or all fp regs if REGNUM == -1)
> +   from regset FPREGS into REGCACHE.  */
> +
> +static void
> +supply_fpregset_regnum (struct regcache *regcache, const prfpregset_t *fpregs,
> +			int regnum)
> +{
> +  int i;
> +
> +  if (regnum == -1)
> +    {
> +      /* We only support the FP registers and FCSR here.  */
> +      for (i = RISCV_FIRST_FP_REGNUM; i <= RISCV_LAST_FP_REGNUM; i++)
> +	regcache->raw_supply (i, &fpregs->__d.__f[i - RISCV_FIRST_FP_REGNUM]);
> +
> +      regcache->raw_supply (RISCV_CSR_FCSR_REGNUM, &fpregs->__d.__fcsr);
> +    }
> +  else if (regnum >= RISCV_FIRST_FP_REGNUM && regnum <= RISCV_LAST_FP_REGNUM)
> +    regcache->raw_supply (regnum,
> +			  &fpregs->__d.__f[regnum - RISCV_FIRST_FP_REGNUM]);
> +  else if (regnum == RISCV_CSR_FCSR_REGNUM)
> +    regcache->raw_supply (RISCV_CSR_FCSR_REGNUM, &fpregs->__d.__fcsr);
> +}
> +
> +/* Copy all floating point registers from regset FPREGS into REGCACHE.  */
> +
> +void
> +supply_fpregset (struct regcache *regcache, const prfpregset_t *fpregs)
> +{
> +  supply_fpregset_regnum (regcache, fpregs, -1);
> +}
> +
> +/* Copy general purpose register REGNUM (or all gp regs if REGNUM == -1)
> +   from REGCACHE into regset GREGS.  */
> +
> +void
> +fill_gregset (const struct regcache *regcache, prgregset_t *gregs, int regnum)
> +{
> +  elf_greg_t *regp = *gregs;
> +
> +  if (regnum == -1)
> +    {
> +      /* We only support the integer registers and PC here.  */
> +      for (int i = RISCV_ZERO_REGNUM + 1; i < RISCV_PC_REGNUM; i++)
> +	regcache->raw_collect (i, regp + i);
> +
> +      regcache->raw_collect (32, regp + 0);
> +    }
> +  else if (regnum == RISCV_ZERO_REGNUM)
> +    /* Nothing to do here.  */
> +    ;
> +  else if (regnum > RISCV_ZERO_REGNUM && regnum < RISCV_PC_REGNUM)
> +    regcache->raw_collect (regnum, regp + regnum);
> +  else if (regnum == RISCV_PC_REGNUM)
> +    regcache->raw_collect (32, regp + 0);
> +}
> +
> +/* Copy floating point register REGNUM (or all fp regs if REGNUM == -1)
> +   from REGCACHE into regset FPREGS.  */
> +
> +void
> +fill_fpregset (const struct regcache *regcache, prfpregset_t *fpregs,
> +	       int regnum)
> +{
> +  if (regnum == -1)
> +    {
> +      /* We only support the FP registers and FCSR here.  */
> +      for (int i = RISCV_FIRST_FP_REGNUM; i <= RISCV_LAST_FP_REGNUM; i++)
> +	regcache->raw_collect (i, &fpregs->__d.__f[i - RISCV_FIRST_FP_REGNUM]);
> +
> +      regcache->raw_collect (RISCV_CSR_FCSR_REGNUM, &fpregs->__d.__fcsr);
> +    }
> +  else if (regnum >= RISCV_FIRST_FP_REGNUM && regnum <= RISCV_LAST_FP_REGNUM)
> +    regcache->raw_collect (regnum,
> +			   &fpregs->__d.__f[regnum - RISCV_FIRST_FP_REGNUM]);
> +  else if (regnum == RISCV_CSR_FCSR_REGNUM)
> +    regcache->raw_collect (RISCV_CSR_FCSR_REGNUM, &fpregs->__d.__fcsr);
> +}
> +
> +/* Fetch REGNUM (or all registers if REGNUM == -1) from the target
> +   into REGCACHE using PTRACE_GETREGSET.  */
> +
> +void
> +riscv_linux_nat_target::fetch_registers (struct regcache *regcache, int regnum)
> +{
> +  int tid;
> +
> +  tid = get_ptrace_pid (regcache->ptid());
> +
> +  if ((regnum >= RISCV_ZERO_REGNUM && regnum <= RISCV_PC_REGNUM)
> +      || (regnum == -1))
> +    {
> +      struct iovec iov;
> +      elf_gregset_t regs;
> +
> +      iov.iov_base = &regs;
> +      iov.iov_len = sizeof (regs);
> +
> +      if (ptrace (PTRACE_GETREGSET, tid, NT_PRSTATUS,
> +		  (PTRACE_TYPE_ARG3) &iov) == -1)
> +	perror_with_name (_("Couldn't get registers"));
> +      else
> +	supply_gregset_regnum (regcache, &regs, regnum);
> +    }
> +
> +  if ((regnum >= RISCV_FIRST_FP_REGNUM
> +       && regnum <= RISCV_LAST_FP_REGNUM)
> +      || (regnum == RISCV_CSR_FCSR_REGNUM)
> +      || (regnum == -1))
> +    {
> +      struct iovec iov;
> +      elf_fpregset_t regs;
> +
> +      iov.iov_base = &regs;
> +      iov.iov_len = sizeof (regs);
> +
> +      if (ptrace (PTRACE_GETREGSET, tid, NT_PRFPREG,
> +		  (PTRACE_TYPE_ARG3) &iov) == -1)
> +	perror_with_name (_("Couldn't get registers"));
> +      else
> +	supply_fpregset_regnum (regcache, &regs, regnum);
> +    }
> +
> +  if ((regnum == RISCV_CSR_MISA_REGNUM)
> +      || (regnum == -1))
> +    {
> +      /* TODO: Need to add a ptrace call for this.  */
> +      regcache->raw_supply_zeroed (regnum);
> +    }
> +
> +  /* Access to other CSRs has potential security issues, don't support them for
> +     now.  */
> +}
> +
> +/* Store REGNUM (or all registers if REGNUM == -1) to the target
> +   from REGCACHE using PTRACE_SETREGSET.  */
> +
> +void
> +riscv_linux_nat_target::store_registers (struct regcache *regcache, int regnum)
> +{
> +  int tid;
> +
> +  tid = get_ptrace_pid (regcache->ptid ());
> +
> +  if ((regnum >= RISCV_ZERO_REGNUM && regnum <= RISCV_PC_REGNUM)
> +      || (regnum == -1))
> +    {
> +      struct iovec iov;
> +      elf_gregset_t regs;
> +
> +      iov.iov_base = &regs;
> +      iov.iov_len = sizeof (regs);
> +
> +      if (ptrace (PTRACE_GETREGSET, tid, NT_PRSTATUS,
> +		  (PTRACE_TYPE_ARG3) &iov) == -1)
> +	perror_with_name (_("Couldn't get registers"));
> +      else
> +	{
> +	  fill_gregset (regcache, &regs, regnum);
> +
> +	  if (ptrace (PTRACE_SETREGSET, tid, NT_PRSTATUS,
> +		      (PTRACE_TYPE_ARG3) &iov) == -1)
> +	    perror_with_name (_("Couldn't set registers"));
> +	}
> +    }
> +
> +  if ((regnum >= RISCV_FIRST_FP_REGNUM
> +       && regnum <= RISCV_LAST_FP_REGNUM)
> +      || (regnum == RISCV_CSR_FCSR_REGNUM)
> +      || (regnum == -1))
> +    {
> +      struct iovec iov;
> +      elf_fpregset_t regs;
> +
> +      iov.iov_base = &regs;
> +      iov.iov_len = sizeof (regs);
> +
> +      if (ptrace (PTRACE_GETREGSET, tid, NT_PRFPREG,
> +		  (PTRACE_TYPE_ARG3) &iov) == -1)
> +	perror_with_name (_("Couldn't get registers"));
> +      else
> +	{
> +	  fill_fpregset (regcache, &regs, regnum);
> +
> +	  if (ptrace (PTRACE_SETREGSET, tid, NT_PRFPREG,
> +		      (PTRACE_TYPE_ARG3) &iov) == -1)
> +	    perror_with_name (_("Couldn't set registers"));
> +	}
> +    }
> +
> +  /* Access to CSRs has potential security issues, don't support them for
> +     now.  */
> +}
> +
> +/* Initialize RISC-V Linux native support.  */
> +
> +void
> +_initialize_riscv_linux_nat (void)
> +{
> +  /* Register the target.  */
> +  linux_target = &the_riscv_linux_nat_target;
> +  add_inf_child_target (&the_riscv_linux_nat_target);
> +}
> -- 
> 2.17.1
> 

Re: [PATCH 5/5] RISC-V: Add configure support riscv*-linux*.

[Eli Zaretskii]

> From: Jim Wilson <jimw@sifive.com>
> Date: Wed, 8 Aug 2018 20:43:20 -0700
> Cc: Tom Tromey <tom@tromey.com>, Palmer Dabbelt <palmer@sifive.com>, gdb-patches@sourceware.org
> 
> Since I'm looking at this at the moment...
> 
> BFD will automatically enable 64-bit support if you configure on a
> host with 64-bit support, or configure for a target with 64-bit
> support.  It will also enable 64-bit support if you use
> --enable-64-bit-bfd.  64-bit support means you can use 64-bit integer
> types such as long long on a 32-bit host or long on a 64-bit LP64
> host.  If 64-bit support is enabled, and your compiler doesn't support
> it, then you get a configure error.
> 
> If you use --disable-64-bit-bfd, then bfd will disallow use of 64-bit
> integer types.  If you configure for a 64-bit target, you will get a
> configure error.
> 
> As a practical matter, I think --enable/disable-64-bit-bfd doesn't
> really do anything useful unless you are configuring with
> --enable-targets=all.  In this case, 64-bit targets will be enabled by
> default, and will fail to build on a 32-bit host with a compiler that
> doesn't support long long.

Thanks.  So you are saying that building on a 64-bit hosts will enable
this by default, while a 32-bit build with --enable-64-bit-bfd only
makes sense if --enable-targets=all is also used, is that right?

Re: [PATCH 5/5] RISC-V: Add configure support for riscv*-linux*.

[Tom Tromey]

>>>>> "Jim" == Jim Wilson <jimw@sifive.com> writes:

Jim> This adds the target and native configure support, and the NEWS entries for
Jim> the new target and native configurations.

Jim> 	gdb/
Jim> 	* Makefile.in (ALL_TARGET_OBS): Add riscv-linux-tdep.c.
Jim> 	(ALLDEPFILES): Add riscv-linux-nat.c, riscv-linux-tdep.c.
Jim> 	* NEWS: Mention new GNU/Linux RISC-V target.
Jim> 	* configure.host: Add riscv*-*-linux*.
Jim> 	* configure.nat: Add riscv*.
Jim> 	* configure.tgt: Add riscv*-*-linux*.

Thank you, this is ok.

Tom

Re: [PATCH 5/5] RISC-V: Add configure support riscv*-linux*.

[Jim Wilson]

On Thu, Aug 9, 2018 at 5:55 AM, Eli Zaretskii <eliz@gnu.org> wrote:
> Thanks.  So you are saying that building on a 64-bit hosts will enable
> this by default, while a 32-bit build with --enable-64-bit-bfd only
> makes sense if --enable-targets=all is also used, is that right?

I didn't actually test that, but trying it, yes, this is how it works.
If I'm on a 32-bit host, and use --enable-targets=all, I only get
32-bit targets enabled.  If I'm on a 32-bit host and use
--enable-targets=all --enable-64-bit-bfd then I get both 32-bit and
64-bit targets enabled.  This requires that the C/C++ compiler
supports long long.

FYI for my riscv build experiment, with an unpatched gdb i.e. only the
riscv bare metal support, a 32-bit hosted --enable-targets=all gdb
does build and run.  There is no bfd elf riscv support, only the bfd
cpu-riscv.o file was built.  The gdb riscv-tdep.o file was built and
linked in.  But without the bfd support I can't trigger it, so it
appears to be harmless dead code.  If I load a riscv32-elf binary,
info target just says elf32-little, and disassembling code gives me
ARM v7 instructions, which must be the default if it can't recognize
an object file.  I think that there can only be a problem if a gdb
target port is making direct calls into the bfd target elf support, in
which case that target port would have to be in the 64-bit object file
list.  The riscv linux port is not doing that currently, and I'm not
sure if it ever would need to.  But I can try another build after I
get my riscv linux patches all checked in.

Jim

Re: [PATCH 3/5] RISC-V: Add linux target support.

[Jim Wilson]

On Wed, Aug 8, 2018 at 11:34 AM, Jim Wilson <jimw@sifive.com> wrote:
> Add initial target support for riscv*-linux*.
>
>         gdb/
>         * riscv-linux-tdep.c: New file.

Committed.

Jim

Re: [PATCH 4/5] RISC-V: Add native linux support.

[Jim Wilson]

On Thu, Aug 9, 2018 at 1:40 AM, Andrew Burgess
<andrew.burgess@embecosm.com> wrote:
> * Jim Wilson <jimw@sifive.com> [2018-08-08 16:39:08 -0700]:
>
>> Add initial native support for riscv*-linux*.
>>
>>       gdb/
>>       * riscv-linux-nat.c: New file.
>
> Looks good to me.

Committed.

Jim

Re: [PATCH 5/5] RISC-V: Add configure support for riscv*-linux*.

[Jim Wilson]

On Thu, Aug 9, 2018 at 8:57 AM, Tom Tromey <tom@tromey.com> wrote:
>>>>>> "Jim" == Jim Wilson <jimw@sifive.com> writes:
>
> Jim> This adds the target and native configure support, and the NEWS entries for
> Jim> the new target and native configurations.
>
> Jim>    gdb/
> Jim>    * Makefile.in (ALL_TARGET_OBS): Add riscv-linux-tdep.c.
> Jim>    (ALLDEPFILES): Add riscv-linux-nat.c, riscv-linux-tdep.c.
> Jim>    * NEWS: Mention new GNU/Linux RISC-V target.
> Jim>    * configure.host: Add riscv*-*-linux*.
> Jim>    * configure.nat: Add riscv*.
> Jim>    * configure.tgt: Add riscv*-*-linux*.
>
> Thank you, this is ok.

Committed.

Jim

Re: [PATCH 0/5] RISC-V Linux native port

[Pedro Alves]

Hi Jim,

On 08/08/2018 03:12 AM, Jim Wilson wrote:
> Here are my current patches to add the RISC-V Linux native support to
> gdb.  These have been tested on a Hifive Unleashed running Fedora as
> previously mentioned and are working reasonably well.

Awesome.  There should be a gdb/NEWS entry for this.

Thanks,
Pedro Alves

Re: [PATCH 4/5] RISC-V: Add native linux support.

[Andreas Schwab]

On Aug 08 2018, Jim Wilson <jimw@sifive.com> wrote:

> +  if ((regnum == RISCV_CSR_MISA_REGNUM)
> +      || (regnum == -1))
> +    {
> +      /* TODO: Need to add a ptrace call for this.  */
> +      regcache->raw_supply_zeroed (regnum);

../../gdb/gdb/regcache.c:337: internal-error: void reg_buffer::assert_regnum(int) const: Assertion `regnum >= 0' failed.

Andreas.

-- 
Andreas Schwab, SUSE Labs, schwab@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."

Re: [PATCH 4/5] RISC-V: Add native linux support.

[Andrew Burgess]

* Andreas Schwab <schwab@suse.de> [2018-10-25 12:49:09 +0200]:

> On Aug 08 2018, Jim Wilson <jimw@sifive.com> wrote:
> 
> > +  if ((regnum == RISCV_CSR_MISA_REGNUM)
> > +      || (regnum == -1))
> > +    {
> > +      /* TODO: Need to add a ptrace call for this.  */
> > +      regcache->raw_supply_zeroed (regnum);
> 
> ../../gdb/gdb/regcache.c:337: internal-error: void reg_buffer::assert_regnum(int) const: Assertion `regnum >= 0' failed.

Thanks for the report.

I pushed the patch below to fix this issue.

Thanks,
Andrew

---

[PATCH] gdb/riscv: Use correct regnum in riscv_linux_nat_target::fetch_registers

In riscv_linux_nat_target::fetch_registers, if we are asked to supply
all registers (regnum parameter is -1), then we currently end up
calling regcache::raw_supply_zeroed with the regnum -1, which is
invalid.  Instead we should be passing the regnum of the specific
register we wish to supply zeroed, in this case RISCV_CSR_MISA_REGNUM.

I removed the extra { ... } block in line with the coding standard
while editing this area.

gdb/ChangeLog:

	* riscv-linux-nat.c (riscv_linux_nat_target::fetch_registers):
	Pass correct regnum to raw_supply_zeroed.
---
 gdb/ChangeLog         | 5 +++++
 gdb/riscv-linux-nat.c | 6 ++----
 2 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/gdb/riscv-linux-nat.c b/gdb/riscv-linux-nat.c
index 7dbfe651f2c..c09121d052b 100644
--- a/gdb/riscv-linux-nat.c
+++ b/gdb/riscv-linux-nat.c
@@ -201,10 +201,8 @@ riscv_linux_nat_target::fetch_registers (struct regcache *regcache, int regnum)
 
   if ((regnum == RISCV_CSR_MISA_REGNUM)
       || (regnum == -1))
-    {
-      /* TODO: Need to add a ptrace call for this.  */
-      regcache->raw_supply_zeroed (regnum);
-    }
+    /* TODO: Need to add a ptrace call for this.  */
+    regcache->raw_supply_zeroed (RISCV_CSR_MISA_REGNUM);
 
   /* Access to other CSRs has potential security issues, don't support them for
      now.  */
-- 
2.14.5

Re: [PATCH 4/5] RISC-V: Add native linux support.

[Pedro Alves]

On 10/25/2018 12:09 PM, Andrew Burgess wrote:
> 
> I removed the extra { ... } block in line with the coding standard
> while editing this area.

Actually, this applies here:

> Any two or more lines in code should be wrapped in braces, even if they are comments, as they look like separate statements: 
> if (i)
>   {
>     /* Return success.  */
>     return 0;
>   }
> 
> and not:
> 
> if (i)
>   /* Return success.  */
>   return 0;

From:

  https://sourceware.org/gdb/wiki/Internals%20GDB-C-Coding-Standards?highlight=%28coding%29%7C%28conventions%29#Whitespaces

Thanks,
Pedro Alves

Re: [PATCH 4/5] RISC-V: Add native linux support.

[Jim Wilson]

On Thu, Oct 25, 2018 at 3:49 AM Andreas Schwab <schwab@suse.de> wrote:
> On Aug 08 2018, Jim Wilson <jimw@sifive.com> wrote:
>
> > +  if ((regnum == RISCV_CSR_MISA_REGNUM)
> > +      || (regnum == -1))
> > +    {
> > +      /* TODO: Need to add a ptrace call for this.  */
> > +      regcache->raw_supply_zeroed (regnum);
>
> ../../gdb/gdb/regcache.c:337: internal-error: void reg_buffer::assert_regnum(int) const: Assertion `regnum >= 0' failed.

I just wrote a patch for that a couple of days ago but have been tied
up with other things and haven't had a chance to submit it yet.  It is
effectively the same as the patch Andrew Burgess wrote, though I kept
the braces.  I also have a patch to support signal handler frames, and
can now go up to where the signal occurred.  Step into signal handler
doesn't work, but I found a comment yesterday that says it isn't
expected to work with software single stepping, so I will just submit
what I already have, as apparently it is as good as it can be for now.

Jim

Re: [PATCH 4/5] RISC-V: Add native linux support.

[John Baldwin]

On 10/25/18 4:09 AM, Andrew Burgess wrote:
> diff --git a/gdb/riscv-linux-nat.c b/gdb/riscv-linux-nat.c
> index 7dbfe651f2c..c09121d052b 100644
> --- a/gdb/riscv-linux-nat.c
> +++ b/gdb/riscv-linux-nat.c
> @@ -201,10 +201,8 @@ riscv_linux_nat_target::fetch_registers (struct regcache *regcache, int regnum)
>  
>    if ((regnum == RISCV_CSR_MISA_REGNUM)
>        || (regnum == -1))
> -    {
> -      /* TODO: Need to add a ptrace call for this.  */
> -      regcache->raw_supply_zeroed (regnum);
> -    }
> +    /* TODO: Need to add a ptrace call for this.  */
> +    regcache->raw_supply_zeroed (RISCV_CSR_MISA_REGNUM);
>  
>    /* Access to other CSRs has potential security issues, don't support them for
>       now.  */

Oops, I just replied to Andrew directly on the commit, but probably better to reply
on the list:

Now that the MISA defaults to 0 if not present, would it better to just remove
this and not set it to 0 explicitly?  The FreeBSD native target for RISC-V
doesn't set MISA to anything at all.

-- 
John Baldwin

Re: [PATCH 4/5] RISC-V: Add native linux support.

[Jim Wilson]

On Thu, Oct 25, 2018 at 10:55 AM John Baldwin <jhb@freebsd.org> wrote:
> Now that the MISA defaults to 0 if not present, would it better to just remove
> this and not set it to 0 explicitly?  The FreeBSD native target for RISC-V
> doesn't set MISA to anything at all.

There is still the issue of FP register size, which comes from MISA,
unless perhaps we can get it from auxvec/hw-cap info.  I was going to
look into that latter, and if the auxvec/hw-cap stuff works, then
remove the remaining MISA support in the riscv-linux-nat.c file.

Jim

Re: [PATCH 4/5] RISC-V: Add native linux support.

[John Baldwin]

On 10/25/18 11:17 AM, Jim Wilson wrote:
> On Thu, Oct 25, 2018 at 10:55 AM John Baldwin <jhb@freebsd.org> wrote:
>> Now that the MISA defaults to 0 if not present, would it better to just remove
>> this and not set it to 0 explicitly?  The FreeBSD native target for RISC-V
>> doesn't set MISA to anything at all.
> 
> There is still the issue of FP register size, which comes from MISA,
> unless perhaps we can get it from auxvec/hw-cap info.  I was going to
> look into that latter, and if the auxvec/hw-cap stuff works, then
> remove the remaining MISA support in the riscv-linux-nat.c file.

Ok.  I do agree that auxvec is probably the right way to handle this, as what
really matters is what format the kernel exports.  You can find existing uses
of auxvec for this on 32-bit arm support where AT_HWCAP flags are tested for
both Linux and FreeBSD in the respective tdep.c files to determine which
floating point registers are available.  You are free to use the same code
in a nat.c file as well of course.

-- 
John Baldwin

Re: [PATCH 4/5] RISC-V: Add native linux support.

[Palmer Dabbelt]

On Thu, 25 Oct 2018 12:19:29 PDT (-0700), jhb@FreeBSD.org wrote:
> On 10/25/18 11:17 AM, Jim Wilson wrote:
>> On Thu, Oct 25, 2018 at 10:55 AM John Baldwin <jhb@freebsd.org> wrote:
>>> Now that the MISA defaults to 0 if not present, would it better to just remove
>>> this and not set it to 0 explicitly?  The FreeBSD native target for RISC-V
>>> doesn't set MISA to anything at all.
>>
>> There is still the issue of FP register size, which comes from MISA,
>> unless perhaps we can get it from auxvec/hw-cap info.  I was going to
>> look into that latter, and if the auxvec/hw-cap stuff works, then
>> remove the remaining MISA support in the riscv-linux-nat.c file.
>
> Ok.  I do agree that auxvec is probably the right way to handle this, as what
> really matters is what format the kernel exports.  You can find existing uses
> of auxvec for this on 32-bit arm support where AT_HWCAP flags are tested for
> both Linux and FreeBSD in the respective tdep.c files to determine which
> floating point registers are available.  You are free to use the same code
> in a nat.c file as well of course.

We have a very simple scheme here: there's a bit for every ISA extension that 
is set in HWCAP by the kernel when that extension is present as far as 
userspace is concerned.  The code is probably easier to understand

    https://git.kernel.org/pub/scm/linux/kernel/git/palmer/riscv-linux.git/tree/arch/riscv/kernel/cpufeature.c#n33

We should probably but this in an ABI document somewhere... :)

Re: [PATCH 4/5] RISC-V: Add native linux support.

[Andrew Burgess]

* Pedro Alves <palves@redhat.com> [2018-10-25 13:06:25 +0100]:

> On 10/25/2018 12:09 PM, Andrew Burgess wrote:
> > 
> > I removed the extra { ... } block in line with the coding standard
> > while editing this area.
> 
> Actually, this applies here:
> 
> > Any two or more lines in code should be wrapped in braces, even if they are comments, as they look like separate statements: 
> > if (i)
> >   {
> >     /* Return success.  */
> >     return 0;
> >   }
> > 
> > and not:
> > 
> > if (i)
> >   /* Return success.  */
> >   return 0;
> 
> From:
> 
>   https://sourceware.org/gdb/wiki/Internals%20GDB-C-Coding-Standards?highlight=%28coding%29%7C%28conventions%29#Whitespaces
> 

Sorry for this mistake.

I pushed the patch below to correct this.

Thanks,
Andrew

--

gdb/riscv: Add back missing braces in riscv-linux-nat.c

In this commit:

    commit ee67fd7f3f6ca78eede2862e309c0bcf266bbd7e
    Date:   Thu Oct 25 12:03:31 2018 +0100

        gdb/riscv: Use correct regnum in riscv_linux_nat_target::fetch_registers

I incorrectly removed a set of braces in violation of the GDB coding
standard.  This commit adds them back.

gdb/ChangeLog:

	* riscv-linux-nat.c (riscv_linux_nat_target::fetch_registers):
	Add missing braces.  No functional change.
---
 gdb/ChangeLog         | 5 +++++
 gdb/riscv-linux-nat.c | 6 ++++--
 2 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/gdb/riscv-linux-nat.c b/gdb/riscv-linux-nat.c
index c09121d052b..d51f6e30218 100644
--- a/gdb/riscv-linux-nat.c
+++ b/gdb/riscv-linux-nat.c
@@ -201,8 +201,10 @@ riscv_linux_nat_target::fetch_registers (struct regcache *regcache, int regnum)
 
   if ((regnum == RISCV_CSR_MISA_REGNUM)
       || (regnum == -1))
-    /* TODO: Need to add a ptrace call for this.  */
-    regcache->raw_supply_zeroed (RISCV_CSR_MISA_REGNUM);
+    {
+      /* TODO: Need to add a ptrace call for this.  */
+      regcache->raw_supply_zeroed (RISCV_CSR_MISA_REGNUM);
+    }
 
   /* Access to other CSRs has potential security issues, don't support them for
      now.  */
-- 
2.14.5

Re: [PATCH 4/5] RISC-V: Add native linux support.

[Andreas Schwab]

On Okt 25 2018, Jim Wilson <jimw@sifive.com> wrote:

> On Thu, Oct 25, 2018 at 10:55 AM John Baldwin <jhb@freebsd.org> wrote:
>> Now that the MISA defaults to 0 if not present, would it better to just remove
>> this and not set it to 0 explicitly?  The FreeBSD native target for RISC-V
>> doesn't set MISA to anything at all.
>
> There is still the issue of FP register size, which comes from MISA,
> unless perhaps we can get it from auxvec/hw-cap info.  I was going to
> look into that latter, and if the auxvec/hw-cap stuff works, then
> remove the remaining MISA support in the riscv-linux-nat.c file.

Note you need commit 0ddc77556c to get a correct AT_HWCAP.

Andreas.

-- 
Andreas Schwab, SUSE Labs, schwab@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."