[PATCH 0/8] RISC-V target description and register handling fixes

public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed

* [PATCH 0/8] RISC-V target description and register handling fixes
@ 2020-06-16 17:14 Andrew Burgess
  2020-06-16 17:14 ` [PATCH 1/8] gdb/riscv: Improved register alias name creation Andrew Burgess
                   ` (9 more replies)
  0 siblings, 10 replies; 13+ messages in thread
From: Andrew Burgess @ 2020-06-16 17:14 UTC (permalink / raw)
  To: gdb-patches; +Cc: Nelson Chu, Jim Wilson, Tom Tromey, palmer, Andrew Burgess

This series includes a mixed bag of changes around RISC-V's target
description and general register handling.  This includes a fix for an
issue Tom reported here:

  https://sourceware.org/pipermail/gdb-patches/2020-June/169323.html

Though most of the changes are RISC-V only, patch #6 does make a
change to the generic target description handling code so is probably
worth additional review.

Feedback welcome,

Thanks,
Andrew

---

Andrew Burgess (8):
  gdb/riscv: Improved register alias name creation
  gdb/riscv: Fix whitespace error
  gdb/riscv: Take CSR names from target description
  gdb/riscv: Remove CSR feature file
  gdb/riscv: Improve support for matching against target descriptions
  gdb: Extend target description processing of unknown registers
  gdb/riscv: Record information about unknown tdesc registers
  gdb/riscv: Loop over all registers for 'info all-registers'

 gdb/ChangeLog                                 |  85 +++
 gdb/features/Makefile                         |   6 -
 gdb/features/riscv/32bit-csr.c                | 253 ---------
 gdb/features/riscv/32bit-csr.xml              | 251 ---------
 gdb/features/riscv/64bit-csr.c                | 253 ---------
 gdb/features/riscv/64bit-csr.xml              | 186 -------
 gdb/features/riscv/rebuild-csr-xml.sh         |  37 --
 gdb/riscv-tdep.c                              | 519 ++++++++++++------
 gdb/riscv-tdep.h                              |  15 +
 gdb/target-descriptions.c                     |  31 +-
 gdb/target-descriptions.h                     |  27 +-
 gdb/testsuite/ChangeLog                       |  27 +
 .../gdb.arch/riscv-tdesc-loading-01.xml       |  83 +++
 .../gdb.arch/riscv-tdesc-loading-02.xml       |  81 +++
 .../gdb.arch/riscv-tdesc-loading-03.xml       |  79 +++
 .../gdb.arch/riscv-tdesc-loading-04.xml       |  77 +++
 .../gdb.arch/riscv-tdesc-loading.exp          |  39 ++
 .../gdb.arch/riscv-tdesc-regs-32.xml          |  89 +++
 .../gdb.arch/riscv-tdesc-regs-64.xml          |  93 ++++
 gdb/testsuite/gdb.arch/riscv-tdesc-regs.c     |  22 +
 gdb/testsuite/gdb.arch/riscv-tdesc-regs.exp   | 123 +++++
 21 files changed, 1235 insertions(+), 1141 deletions(-)
 delete mode 100644 gdb/features/riscv/32bit-csr.c
 delete mode 100644 gdb/features/riscv/32bit-csr.xml
 delete mode 100644 gdb/features/riscv/64bit-csr.c
 delete mode 100644 gdb/features/riscv/64bit-csr.xml
 delete mode 100755 gdb/features/riscv/rebuild-csr-xml.sh
 create mode 100644 gdb/testsuite/gdb.arch/riscv-tdesc-loading-01.xml
 create mode 100644 gdb/testsuite/gdb.arch/riscv-tdesc-loading-02.xml
 create mode 100644 gdb/testsuite/gdb.arch/riscv-tdesc-loading-03.xml
 create mode 100644 gdb/testsuite/gdb.arch/riscv-tdesc-loading-04.xml
 create mode 100644 gdb/testsuite/gdb.arch/riscv-tdesc-loading.exp
 create mode 100644 gdb/testsuite/gdb.arch/riscv-tdesc-regs-32.xml
 create mode 100644 gdb/testsuite/gdb.arch/riscv-tdesc-regs-64.xml
 create mode 100644 gdb/testsuite/gdb.arch/riscv-tdesc-regs.c
 create mode 100644 gdb/testsuite/gdb.arch/riscv-tdesc-regs.exp

-- 
2.25.4


^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH 1/8] gdb/riscv: Improved register alias name creation
  2020-06-16 17:14 [PATCH 0/8] RISC-V target description and register handling fixes Andrew Burgess
@ 2020-06-16 17:14 ` Andrew Burgess
  2020-06-18 20:36   ` Tom Tromey
  2020-06-16 17:14 ` [PATCH 2/8] gdb/riscv: Fix whitespace error Andrew Burgess
                   ` (8 subsequent siblings)
  9 siblings, 1 reply; 13+ messages in thread
From: Andrew Burgess @ 2020-06-16 17:14 UTC (permalink / raw)
  To: gdb-patches; +Cc: Nelson Chu, Jim Wilson, Tom Tromey, palmer, Andrew Burgess

This commit does two things:

 1. Makes use of the DECLARE_CSR_ALIAS definitions in riscv-opc.h to
 add additional aliases for CSRs.

 2. Only creates aliases for registers that are actually present on
 the target (as announced in the target XML description).

This means that the 'csr%d' aliases that exist will only be created
for those CSRs the target actually has, which is a nice improvement,
as accessing one of the CSRs that didn't exist would cause GDB to
crash with this error:

  valprint.c:1560: internal-error: bool maybe_negate_by_bytes(const gdb_byte*, unsigned int, bfd_endian, gdb::byte_vector*): Assertion `len > 0' failed.

When we look at the DECLARE_CSR_ALIAS lines in riscv-opc.h, these can
be split into three groups:

 DECLARE_CSR_ALIAS(misa, 0xf10, CSR_CLASS_I, PRIV_SPEC_CLASS_1P9, PRIV_SPEC_CLASS_1P9P1)

The 'misa' register used to exist of offset 0xf10, but was moved to
its current offset (0x301) in with privilege spec 1.9.1.  We don't
want GDB to create an alias called 'misa' as we will already have a
'misa' register created by the DECLARE_CSR(misa ....) call earlier in
riscv-opc.h

 DECLARE_CSR_ALIAS(ubadaddr, CSR_UTVAL, CSR_CLASS_I, PRIV_SPEC_CLASS_1P9, PRIV_SPEC_CLASS_1P10)
 DECLARE_CSR_ALIAS(sbadaddr, CSR_STVAL, CSR_CLASS_I, PRIV_SPEC_CLASS_1P9, PRIV_SPEC_CLASS_1P10)
 DECLARE_CSR_ALIAS(sptbr, CSR_SATP, CSR_CLASS_I, PRIV_SPEC_CLASS_1P9, PRIV_SPEC_CLASS_1P10)
 DECLARE_CSR_ALIAS(mbadaddr, CSR_MTVAL, CSR_CLASS_I, PRIV_SPEC_CLASS_1P9, PRIV_SPEC_CLASS_1P10)
 DECLARE_CSR_ALIAS(mucounteren, CSR_MCOUNTINHIBIT, CSR_CLASS_I, PRIV_SPEC_CLASS_1P9, PRIV_SPEC_CLASS_1P10)

These aliases are all CSRs that were removed in privilege spec 1.10,
and whose addresses were reused by new CSRs.  The names meaning of the
old names is totally different to the new CSRs that have taken their
place.  I don't believe we should add these as aliases into GDB.  If
the new CSR exists in the target then that should be enough.

 DECLARE_CSR_ALIAS(dscratch, CSR_DSCRATCH0, CSR_CLASS_I, PRIV_SPEC_CLASS_1P9, PRIV_SPEC_CLASS_1P11)

In privilege spec 1.11 the 'dscratch' register was renamed to
'dscratch0', however the meaning of the register didn't change.
Adding the 'dscratch' alias makes sense I think.

Looking then at the final PRIV_SPEC_CLASS_* field for each alias then
we can see that currently we only want to take the alias from
PRIV_SPEC_CLASS_1P11.  For now then this is what I'm using to filter
the aliases within GDB.

In the future there's no telling how DECLARE_CSR_ALIAS will be used.
I've heard it said that future RISC-V privilege specs will not reuse
CSR offsets again.  But it could happen.  We just don't know.

If / when it does we may need to revisit how aliases are created for
GDB, but for now this seems to be OK.

gdb/ChangeLog:

	* riscv-tdep.c (riscv_create_csr_aliases): Handle csr aliases from
	riscv-opc.h.
	(class riscv_pending_register_alias): New class.
	(riscv_check_tdesc_feature): Take vector of pending aliases and
	populate it as appropriate.
	(riscv_setup_register_aliases): Delete.
	(riscv_gdbarch_init): Create vector of pending aliases and pass it
	to riscv_check_tdesc_feature in all cases.  Use the vector to
	create the register aliases.

gdb/testsuite/ChangeLog:

	* gdb.arch/riscv-tdesc-regs-32.xml: New file.
	* gdb.arch/riscv-tdesc-regs-64.xml: New file.
	* gdb.arch/riscv-tdesc-regs.c: New file.
	* gdb.arch/riscv-tdesc-regs.exp: New file.
---
 gdb/ChangeLog                                 | 12 +++
 gdb/riscv-tdep.c                              | 95 +++++++++++++------
 gdb/testsuite/ChangeLog                       |  7 ++
 .../gdb.arch/riscv-tdesc-regs-32.xml          | 89 +++++++++++++++++
 .../gdb.arch/riscv-tdesc-regs-64.xml          | 93 ++++++++++++++++++
 gdb/testsuite/gdb.arch/riscv-tdesc-regs.c     | 22 +++++
 gdb/testsuite/gdb.arch/riscv-tdesc-regs.exp   | 81 ++++++++++++++++
 7 files changed, 370 insertions(+), 29 deletions(-)
 create mode 100644 gdb/testsuite/gdb.arch/riscv-tdesc-regs-32.xml
 create mode 100644 gdb/testsuite/gdb.arch/riscv-tdesc-regs-64.xml
 create mode 100644 gdb/testsuite/gdb.arch/riscv-tdesc-regs.c
 create mode 100644 gdb/testsuite/gdb.arch/riscv-tdesc-regs.exp

diff --git a/gdb/riscv-tdep.c b/gdb/riscv-tdep.c
index fa43c8d02f2..64ec9b4a257 100644
--- a/gdb/riscv-tdep.c
+++ b/gdb/riscv-tdep.c
@@ -258,6 +258,16 @@ riscv_create_csr_aliases ()
       int csr_num = reg.regnum - RISCV_FIRST_CSR_REGNUM;
       const char *alias = xstrprintf ("csr%d", csr_num);
       reg.names.push_back (alias);
+
+      /* Setup the other csr aliases.  We don't use a switch table here in
+         case there are multiple aliases with the same value.  Also filter
+         based on ABRT_VER in order to avoid a very old alias for misa that
+         duplicates the name "misa" but at a different CSR address.  */
+#define DECLARE_CSR_ALIAS(NAME,VALUE,CLASS,DEF_VER,ABRT_VER)	 \
+      if (csr_num == VALUE && ABRT_VER >= PRIV_SPEC_CLASS_1P11)  \
+        reg.names.push_back ( # NAME );
+#include "opcode/riscv-opc.h"
+#undef DECLARE_CSR_ALIAS
     }
 }
 
@@ -2945,6 +2955,37 @@ riscv_find_default_target_description (const struct gdbarch_info info)
   return riscv_lookup_target_description (features);
 }
 
+/* Information about a register alias that needs to be set up for this
+   target.  These are collected when the target's XML description is
+   analysed, and then processed later, once the gdbarch has been created.  */
+
+class riscv_pending_register_alias
+{
+public:
+  /* Constructor.  */
+
+  riscv_pending_register_alias (const char *name, const void *baton)
+    : m_name (name),
+      m_baton (baton)
+  { /* Nothing.  */ }
+
+  /* Convert this into a user register for GDBARCH.  */
+
+  void create (struct gdbarch *gdbarch) const
+  {
+    user_reg_add (gdbarch, m_name, value_of_riscv_user_reg, m_baton);
+  }
+
+private:
+  /* The name for this alias.  */
+  const char *m_name;
+
+  /* The baton value for passing to user_reg_add.  This must point to some
+     data that will live for at least as long as the gdbarch object to
+     which the user register is attached.  */
+  const void *m_baton;
+};
+
 /* All of the registers in REG_SET are checked for in FEATURE, TDESC_DATA
    is updated with the register numbers for each register as listed in
    REG_SET.  If any register marked as required in REG_SET is not found in
@@ -2953,7 +2994,8 @@ riscv_find_default_target_description (const struct gdbarch_info info)
 static bool
 riscv_check_tdesc_feature (struct tdesc_arch_data *tdesc_data,
                            const struct tdesc_feature *feature,
-                           const struct riscv_register_feature *reg_set)
+                           const struct riscv_register_feature *reg_set,
+                           std::vector<riscv_pending_register_alias> *aliases)
 {
   for (const auto &reg : reg_set->registers)
     {
@@ -2965,7 +3007,15 @@ riscv_check_tdesc_feature (struct tdesc_arch_data *tdesc_data,
 	    tdesc_numbered_register (feature, tdesc_data, reg.regnum, name);
 
 	  if (found)
-	    break;
+            {
+              /* We know that the target description mentions this
+                 register.  In RISCV_REGISTER_NAME we ensure that GDB
+                 always uses the first name for each register, so here we
+                 add aliases for all of the remaining names.  */
+              for (int i = 0; i < reg.names.size (); ++i)
+		aliases->emplace_back (reg.names[i], (void *)&reg.regnum);
+              break;
+            }
 	}
 
       if (!found && reg.required_p)
@@ -2993,24 +3043,6 @@ riscv_add_reggroups (struct gdbarch *gdbarch)
   reggroup_add (gdbarch, csr_reggroup);
 }
 
-/* Create register aliases for all the alternative names that exist for
-   registers in REG_SET.  */
-
-static void
-riscv_setup_register_aliases (struct gdbarch *gdbarch,
-                              const struct riscv_register_feature *reg_set)
-{
-  for (auto &reg : reg_set->registers)
-    {
-      /* The first item in the names list is the preferred name for the
-         register, this is what RISCV_REGISTER_NAME returns, and so we
-         don't need to create an alias with that name here.  */
-      for (int i = 1; i < reg.names.size (); ++i)
-        user_reg_add (gdbarch, reg.names[i], value_of_riscv_user_reg,
-                      &reg.regnum);
-    }
-}
-
 /* Implement the "dwarf2_reg_to_regnum" gdbarch method.  */
 
 static int
@@ -3114,10 +3146,12 @@ riscv_gdbarch_init (struct gdbarch_info info,
     return NULL;
 
   struct tdesc_arch_data *tdesc_data = tdesc_data_alloc ();
+  std::vector<riscv_pending_register_alias> pending_aliases;
 
   bool valid_p = riscv_check_tdesc_feature (tdesc_data,
                                             feature_cpu,
-                                            &riscv_xreg_feature);
+                                            &riscv_xreg_feature,
+                                            &pending_aliases);
   if (valid_p)
     {
       /* Check that all of the core cpu registers have the same bitsize.  */
@@ -3137,7 +3171,8 @@ riscv_gdbarch_init (struct gdbarch_info info,
   if (feature_fpu != NULL)
     {
       valid_p &= riscv_check_tdesc_feature (tdesc_data, feature_fpu,
-                                            &riscv_freg_feature);
+                                            &riscv_freg_feature,
+                                            &pending_aliases);
 
       /* Search for the first floating point register (by any alias), to
          determine the bitsize.  */
@@ -3173,11 +3208,13 @@ riscv_gdbarch_init (struct gdbarch_info info,
 
   if (feature_virtual)
     riscv_check_tdesc_feature (tdesc_data, feature_virtual,
-                               &riscv_virtual_feature);
+                               &riscv_virtual_feature,
+                               &pending_aliases);
 
   if (feature_csr)
     riscv_check_tdesc_feature (tdesc_data, feature_csr,
-                               &riscv_csr_feature);
+                               &riscv_csr_feature,
+                               &pending_aliases);
 
   if (!valid_p)
     {
@@ -3315,11 +3352,11 @@ riscv_gdbarch_init (struct gdbarch_info info,
      want, ignoring what the target tells us.  */
   set_gdbarch_register_reggroup_p (gdbarch, riscv_register_reggroup_p);
 
-  /* Create register aliases for alternative register names.  */
-  riscv_setup_register_aliases (gdbarch, &riscv_xreg_feature);
-  if (riscv_has_fp_regs (gdbarch))
-    riscv_setup_register_aliases (gdbarch, &riscv_freg_feature);
-  riscv_setup_register_aliases (gdbarch, &riscv_csr_feature);
+  /* Create register aliases for alternative register names.  We only
+     create aliases for registers which were mentioned in the target
+     description.  */
+  for (const auto &alias : pending_aliases)
+    alias.create (gdbarch);
 
   /* Compile command hooks.  */
   set_gdbarch_gcc_target_options (gdbarch, riscv_gcc_target_options);
diff --git a/gdb/testsuite/gdb.arch/riscv-tdesc-regs-32.xml b/gdb/testsuite/gdb.arch/riscv-tdesc-regs-32.xml
new file mode 100644
index 00000000000..90406a0c676
--- /dev/null
+++ b/gdb/testsuite/gdb.arch/riscv-tdesc-regs-32.xml
@@ -0,0 +1,89 @@
+<?xml version="1.0"?>
+<!DOCTYPE target SYSTEM "gdb-target.dtd">
+<target>
+  <architecture>riscv</architecture>
+  <feature name="org.gnu.gdb.riscv.cpu">
+    <reg name="zero" bitsize="32" type="int"/>
+    <reg name="ra" bitsize="32" type="code_ptr"/>
+    <reg name="sp" bitsize="32" type="data_ptr"/>
+    <reg name="gp" bitsize="32" type="data_ptr"/>
+    <reg name="tp" bitsize="32" type="data_ptr"/>
+    <reg name="t0" bitsize="32" type="int"/>
+    <reg name="t1" bitsize="32" type="int"/>
+    <reg name="t2" bitsize="32" type="int"/>
+    <reg name="fp" bitsize="32" type="data_ptr"/>
+    <reg name="s1" bitsize="32" type="int"/>
+    <reg name="a0" bitsize="32" type="int"/>
+    <reg name="a1" bitsize="32" type="int"/>
+    <reg name="a2" bitsize="32" type="int"/>
+    <reg name="a3" bitsize="32" type="int"/>
+    <reg name="a4" bitsize="32" type="int"/>
+    <reg name="a5" bitsize="32" type="int"/>
+    <reg name="a6" bitsize="32" type="int"/>
+    <reg name="a7" bitsize="32" type="int"/>
+    <reg name="s2" bitsize="32" type="int"/>
+    <reg name="s3" bitsize="32" type="int"/>
+    <reg name="s4" bitsize="32" type="int"/>
+    <reg name="s5" bitsize="32" type="int"/>
+    <reg name="s6" bitsize="32" type="int"/>
+    <reg name="s7" bitsize="32" type="int"/>
+    <reg name="s8" bitsize="32" type="int"/>
+    <reg name="s9" bitsize="32" type="int"/>
+    <reg name="s10" bitsize="32" type="int"/>
+    <reg name="s11" bitsize="32" type="int"/>
+    <reg name="t3" bitsize="32" type="int"/>
+    <reg name="t4" bitsize="32" type="int"/>
+    <reg name="t5" bitsize="32" type="int"/>
+    <reg name="t6" bitsize="32" type="int"/>
+    <reg name="pc" bitsize="32" type="code_ptr"/>
+  </feature>
+  <feature name="org.gnu.gdb.riscv.fpu">
+    <reg name="ft0" bitsize="32" type="float"/>
+    <reg name="ft1" bitsize="32" type="float"/>
+    <reg name="ft2" bitsize="32" type="float"/>
+    <reg name="ft3" bitsize="32" type="float"/>
+    <reg name="ft4" bitsize="32" type="float"/>
+    <reg name="ft5" bitsize="32" type="float"/>
+    <reg name="ft6" bitsize="32" type="float"/>
+    <reg name="ft7" bitsize="32" type="float"/>
+    <reg name="fs0" bitsize="32" type="float"/>
+    <reg name="fs1" bitsize="32" type="float"/>
+    <reg name="fa0" bitsize="32" type="float"/>
+    <reg name="fa1" bitsize="32" type="float"/>
+    <reg name="fa2" bitsize="32" type="float"/>
+    <reg name="fa3" bitsize="32" type="float"/>
+    <reg name="fa4" bitsize="32" type="float"/>
+    <reg name="fa5" bitsize="32" type="float"/>
+    <reg name="fa6" bitsize="32" type="float"/>
+    <reg name="fa7" bitsize="32" type="float"/>
+    <reg name="fs2" bitsize="32" type="float"/>
+    <reg name="fs3" bitsize="32" type="float"/>
+    <reg name="fs4" bitsize="32" type="float"/>
+    <reg name="fs5" bitsize="32" type="float"/>
+    <reg name="fs6" bitsize="32" type="float"/>
+    <reg name="fs7" bitsize="32" type="float"/>
+    <reg name="fs8" bitsize="32" type="float"/>
+    <reg name="fs9" bitsize="32" type="float"/>
+    <reg name="fs10" bitsize="32" type="float"/>
+    <reg name="fs11" bitsize="32" type="float"/>
+    <reg name="ft8" bitsize="32" type="float"/>
+    <reg name="ft9" bitsize="32" type="float"/>
+    <reg name="ft10" bitsize="32" type="float"/>
+    <reg name="ft11" bitsize="32" type="float"/>
+    <!-- The following 3 registers are duplicated. -->
+    <reg name="fflags" bitsize="32" type="int"/>
+    <reg name="frm" bitsize="32" type="int"/>
+    <reg name="fcsr" bitsize="32" type="int"/>
+  </feature>
+  <feature name="org.gnu.gdb.riscv.csr">
+    <!-- The following 3 registers are duplicated. -->
+    <reg name="fflags" bitsize="32" type="int"/>
+    <reg name="frm" bitsize="32" type="int"/>
+    <reg name="fcsr" bitsize="32" type="int"/>
+    <!-- The following is a CSR unknown to GDB. -->
+    <reg name="unknown_csr" bitsize="32" type="int"/>
+    <!-- The following is now known as 'dscratch0' in the official
+         RISC-V spec, but GDB should NOT rename this register. -->
+    <reg name="dscratch" bitsize="32" type="int"/>
+  </feature>
+</target>
diff --git a/gdb/testsuite/gdb.arch/riscv-tdesc-regs-64.xml b/gdb/testsuite/gdb.arch/riscv-tdesc-regs-64.xml
new file mode 100644
index 00000000000..b2422aeb866
--- /dev/null
+++ b/gdb/testsuite/gdb.arch/riscv-tdesc-regs-64.xml
@@ -0,0 +1,93 @@
+<?xml version="1.0"?>
+<!DOCTYPE target SYSTEM "gdb-target.dtd">
+<target>
+  <architecture>riscv</architecture>
+  <feature name="org.gnu.gdb.riscv.cpu">
+    <reg name="zero" bitsize="64" type="int"/>
+    <reg name="ra" bitsize="64" type="code_ptr"/>
+    <reg name="sp" bitsize="64" type="data_ptr"/>
+    <reg name="gp" bitsize="64" type="data_ptr"/>
+    <reg name="tp" bitsize="64" type="data_ptr"/>
+    <reg name="t0" bitsize="64" type="int"/>
+    <reg name="t1" bitsize="64" type="int"/>
+    <reg name="t2" bitsize="64" type="int"/>
+    <reg name="fp" bitsize="64" type="data_ptr"/>
+    <reg name="s1" bitsize="64" type="int"/>
+    <reg name="a0" bitsize="64" type="int"/>
+    <reg name="a1" bitsize="64" type="int"/>
+    <reg name="a2" bitsize="64" type="int"/>
+    <reg name="a3" bitsize="64" type="int"/>
+    <reg name="a4" bitsize="64" type="int"/>
+    <reg name="a5" bitsize="64" type="int"/>
+    <reg name="a6" bitsize="64" type="int"/>
+    <reg name="a7" bitsize="64" type="int"/>
+    <reg name="s2" bitsize="64" type="int"/>
+    <reg name="s3" bitsize="64" type="int"/>
+    <reg name="s4" bitsize="64" type="int"/>
+    <reg name="s5" bitsize="64" type="int"/>
+    <reg name="s6" bitsize="64" type="int"/>
+    <reg name="s7" bitsize="64" type="int"/>
+    <reg name="s8" bitsize="64" type="int"/>
+    <reg name="s9" bitsize="64" type="int"/>
+    <reg name="s10" bitsize="64" type="int"/>
+    <reg name="s11" bitsize="64" type="int"/>
+    <reg name="t3" bitsize="64" type="int"/>
+    <reg name="t4" bitsize="64" type="int"/>
+    <reg name="t5" bitsize="64" type="int"/>
+    <reg name="t6" bitsize="64" type="int"/>
+    <reg name="pc" bitsize="64" type="code_ptr"/>
+  </feature>
+  <feature name="org.gnu.gdb.riscv.fpu">
+    <union id="riscv_double">
+      <field name="float" type="ieee_single"/>
+      <field name="double" type="ieee_double"/>
+    </union>
+    <reg name="ft0" bitsize="64" type="riscv_double"/>
+    <reg name="ft1" bitsize="64" type="riscv_double"/>
+    <reg name="ft2" bitsize="64" type="riscv_double"/>
+    <reg name="ft3" bitsize="64" type="riscv_double"/>
+    <reg name="ft4" bitsize="64" type="riscv_double"/>
+    <reg name="ft5" bitsize="64" type="riscv_double"/>
+    <reg name="ft6" bitsize="64" type="riscv_double"/>
+    <reg name="ft7" bitsize="64" type="riscv_double"/>
+    <reg name="fs0" bitsize="64" type="riscv_double"/>
+    <reg name="fs1" bitsize="64" type="riscv_double"/>
+    <reg name="fa0" bitsize="64" type="riscv_double"/>
+    <reg name="fa1" bitsize="64" type="riscv_double"/>
+    <reg name="fa2" bitsize="64" type="riscv_double"/>
+    <reg name="fa3" bitsize="64" type="riscv_double"/>
+    <reg name="fa4" bitsize="64" type="riscv_double"/>
+    <reg name="fa5" bitsize="64" type="riscv_double"/>
+    <reg name="fa6" bitsize="64" type="riscv_double"/>
+    <reg name="fa7" bitsize="64" type="riscv_double"/>
+    <reg name="fs2" bitsize="64" type="riscv_double"/>
+    <reg name="fs3" bitsize="64" type="riscv_double"/>
+    <reg name="fs4" bitsize="64" type="riscv_double"/>
+    <reg name="fs5" bitsize="64" type="riscv_double"/>
+    <reg name="fs6" bitsize="64" type="riscv_double"/>
+    <reg name="fs7" bitsize="64" type="riscv_double"/>
+    <reg name="fs8" bitsize="64" type="riscv_double"/>
+    <reg name="fs9" bitsize="64" type="riscv_double"/>
+    <reg name="fs10" bitsize="64" type="riscv_double"/>
+    <reg name="fs11" bitsize="64" type="riscv_double"/>
+    <reg name="ft8" bitsize="64" type="riscv_double"/>
+    <reg name="ft9" bitsize="64" type="riscv_double"/>
+    <reg name="ft10" bitsize="64" type="riscv_double"/>
+    <reg name="ft11" bitsize="64" type="riscv_double"/>
+    <!-- The following 3 registers are duplicated. -->
+    <reg name="fflags" bitsize="32" type="int"/>
+    <reg name="frm" bitsize="32" type="int"/>
+    <reg name="fcsr" bitsize="32" type="int"/>
+  </feature>
+  <feature name="org.gnu.gdb.riscv.csr">
+    <!-- The following 3 registers are duplicated. -->
+    <reg name="fflags" bitsize="32" type="int"/>
+    <reg name="frm" bitsize="32" type="int"/>
+    <reg name="fcsr" bitsize="32" type="int"/>
+    <!-- The following is a CSR unknown to GDB. -->
+    <reg name="unknown_csr" bitsize="32" type="int"/>
+    <!-- The following is now known as 'dscratch0' in the official
+         RISC-V spec, but GDB should NOT rename this register. -->
+    <reg name="dscratch" bitsize="32" type="int"/>
+  </feature>
+</target>
diff --git a/gdb/testsuite/gdb.arch/riscv-tdesc-regs.c b/gdb/testsuite/gdb.arch/riscv-tdesc-regs.c
new file mode 100644
index 00000000000..f4825c8a7c1
--- /dev/null
+++ b/gdb/testsuite/gdb.arch/riscv-tdesc-regs.c
@@ -0,0 +1,22 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2020 Free Software Foundation, Inc.
+
+   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/>.  */
+
+int
+main ()
+{
+  return 0;
+}
diff --git a/gdb/testsuite/gdb.arch/riscv-tdesc-regs.exp b/gdb/testsuite/gdb.arch/riscv-tdesc-regs.exp
new file mode 100644
index 00000000000..46e64d62a2b
--- /dev/null
+++ b/gdb/testsuite/gdb.arch/riscv-tdesc-regs.exp
@@ -0,0 +1,81 @@
+# Copyright 2020 Free Software Foundation, Inc.
+#
+# 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/>.
+
+# Various tests to check which register names are available after
+# loading a new target description file.
+
+if {![istarget "riscv*-*-*"]} {
+    verbose "Skipping ${gdb_test_file_name}."
+    return
+}
+
+standard_testfile
+
+if { [prepare_for_testing "failed to prepare" ${testfile} ${srcfile} \
+	  {debug quiet}] } {
+    unsupported "failed to compile"
+    return -1
+}
+
+if { ![runto_main] } {
+    untested "failed to runto main"
+    return -1
+}
+
+# First, figure out if we are 32-bit or 64-bit.
+set xlen [get_valueof "/d" "sizeof (\$a0)" 0]
+set flen [get_valueof "/d" "sizeof (\$fa0)" 0]
+
+gdb_assert { $xlen != 0 && $flen != 0 } "read xlen and flen"
+
+# We only handle 32-bit or 64-bit x-registers.
+if { $xlen != 4 && $xlen != 8 } {
+    unsupported "unknown x-register size"
+    return -1
+}
+
+# If FLEN is 1 then the target doesn't have floating point support
+# (the register $fa0 was not recognised).  Otherwise, we can only
+# proceed if FLEN equals XLEN, otherwise we'd need more test XML
+# files.
+if { $flen != 1 && $flen != $xlen } {
+    unsupport "unknown xlen/flen combination"
+    return -1
+}
+
+if { $xlen == 4 } {
+    set xml_tdesc "riscv-tdesc-regs-32.xml"
+} else {
+    set xml_tdesc "riscv-tdesc-regs-64.xml"
+}
+set xml_tdesc "${srcdir}/${subdir}/${xml_tdesc}"
+
+# Maybe copy the target over if we're remote testing.
+if {[is_remote host]} {
+    set remote_file [remote_download host $xml_tdesc]
+} else {
+    set remote_file $xml_tdesc
+}
+
+gdb_test_no_output "set tdesc filename $remote_file" \
+    "load the new target description"
+
+# Check that an alias for an unknown CSR will give a suitable error.
+gdb_test "info registers \$csr0" "Invalid register `csr0'"
+
+# Check we can access the dscratch register using either of its names.
+gdb_test "info registers \$dscratch0" "dscratch0\[ \t\]+.*"
+gdb_test "info registers \$dscratch" "dscratch\[ \t\]+.*"
+
-- 
2.25.4


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/8] gdb/riscv: Improved register alias name creation
  2020-06-16 17:14 ` [PATCH 1/8] gdb/riscv: Improved register alias name creation Andrew Burgess
@ 2020-06-18 20:36   ` Tom Tromey
  0 siblings, 0 replies; 13+ messages in thread
From: Tom Tromey @ 2020-06-18 20:36 UTC (permalink / raw)
  To: Andrew Burgess; +Cc: gdb-patches, Tom Tromey

>>>>> "Andrew" == Andrew Burgess <andrew.burgess@embecosm.com> writes:

Andrew> +		aliases->emplace_back (reg.names[i], (void *)&reg.regnum);

I'd add a space before the & here.

Otherwise this seems fine, as far as I understand it :)

Tom

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH 2/8] gdb/riscv: Fix whitespace error
  2020-06-16 17:14 [PATCH 0/8] RISC-V target description and register handling fixes Andrew Burgess
  2020-06-16 17:14 ` [PATCH 1/8] gdb/riscv: Improved register alias name creation Andrew Burgess
@ 2020-06-16 17:14 ` Andrew Burgess
  2020-06-16 17:14 ` [PATCH 3/8] gdb/riscv: Take CSR names from target description Andrew Burgess
                   ` (7 subsequent siblings)
  9 siblings, 0 replies; 13+ messages in thread
From: Andrew Burgess @ 2020-06-16 17:14 UTC (permalink / raw)
  To: gdb-patches; +Cc: Nelson Chu, Jim Wilson, Tom Tromey, palmer, Andrew Burgess

Should be 'std::vector<type>' not 'std::vector <type>'.

gdb/ChangeLog:

	* riscv-tdep.c (struct riscv_register_feature): Fix whitespace
	errors.
---
 gdb/ChangeLog    | 5 +++++
 gdb/riscv-tdep.c | 4 ++--
 2 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/gdb/riscv-tdep.c b/gdb/riscv-tdep.c
index 64ec9b4a257..e394263a591 100644
--- a/gdb/riscv-tdep.c
+++ b/gdb/riscv-tdep.c
@@ -115,7 +115,7 @@ struct riscv_register_feature
     /* List of names for this register.  The first name in this list is the
        preferred name, the name GDB should use when describing this
        register.  */
-    std::vector <const char *> names;
+    std::vector<const char *> names;
 
     /* When true this register is required in this feature set.  */
     bool required_p;
@@ -127,7 +127,7 @@ struct riscv_register_feature
 
   /* List of all the registers that we expect that we might find in this
      register set.  */
-  std::vector <struct register_info> registers;
+  std::vector<struct register_info> registers;
 };
 
 /* The general x-registers feature set.  */
-- 
2.25.4


^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH 3/8] gdb/riscv: Take CSR names from target description
  2020-06-16 17:14 [PATCH 0/8] RISC-V target description and register handling fixes Andrew Burgess
  2020-06-16 17:14 ` [PATCH 1/8] gdb/riscv: Improved register alias name creation Andrew Burgess
  2020-06-16 17:14 ` [PATCH 2/8] gdb/riscv: Fix whitespace error Andrew Burgess
@ 2020-06-16 17:14 ` Andrew Burgess
  2020-06-16 17:14 ` [PATCH 4/8] gdb/riscv: Remove CSR feature file Andrew Burgess
                   ` (6 subsequent siblings)
  9 siblings, 0 replies; 13+ messages in thread
From: Andrew Burgess @ 2020-06-16 17:14 UTC (permalink / raw)
  To: gdb-patches; +Cc: Nelson Chu, Jim Wilson, Tom Tromey, palmer, Andrew Burgess

First, consider the RISC-V register $x1.  This register has an alias
$ra.  When GDB processes an incoming target description we allow the
target to use either register name to describe the target.

However, within GDB's UI we want to use the $ra alias in preference to
the $x1 architecture name.

To achieve this GDB overrides the tdesc_register_name callback with
riscv_register_name.  In riscv_register_name we ensure that we always
return the preferred name, so in this case "ra".

To ensure the user can still access the register as $x1 if they want
to, when in riscv_check_tdesc_feature we spot that the target has
supplied the register, we add aliases for every name except the
preferred one, so in this case we add the alias "x1".

This scheme seems to work quite well, the targets have the flexibility
to be architecture focused if they wish (using x0 - x31) while GDB is
still using the ABI names ra, sp, gp, etc.

When this code was originally added there was an attempt made to
include the CSRs in the same scheme.  At the time the CSRs only had
two names, one pulled from riscv-opc.h, and one generated in GDB that
had the pattern csr%d.

The idea was that if the remote targets description described the CSRs
as csr%d then GDB would rename these back to the real CSR name.  This
code was only included because if followed the same pattern as the
x-regs and f-regs, not because I was actually aware of any target that
did this.

However, recent changes to add additional CSR aliases has made me
rethink the position here.

Lets consider the CSR $dscratch0.  This register has an alias
'csr1970' (1970 is 0x7b2, which is the offset of the CSR register into
the CSR address space).  However, this register was originally called
just 'dscratch', and so, after recent commits, this register also has
the alias 'dscratch'.

As the riscv-opc.h file calls this register 'dscratch0' GDB's
preferred name for this register is 'dscratch0'.

So, if the remote target description includes the register
'dscratch0', then GDB will add the aliases 'dscratch', and 'csr1970'.
In the UI GDB will describe the register as 'dscratch0', and all it
good.

The problem I see in this case is where the target describes the
register as 'dscratch'.  In this case GDB will still spot the register
and add the aliases 'dscratch', and 'csr1970', GDB will then give the
register the preferred name 'dscratch0'.

I don't like this.  For the CSRs I think that we should stick with the
naming scheme offered by the remote target description.  As the RISC-V
specification evolves and CSR register names evolve, insisting on
referring to registers by the most up to date name makes it harder for
a target to provide a consistent target description for an older
version of the RISC-V architecture spec.

In this precise case the target offers 'dscratch', which is from an
older version of the RISC-V specification, the newer version of the
spec has two registers 'dscratch0' and 'dscratch1'.  If we insist on
using 'dscratch0' it is then a little "weird" (or seems so to me) when
'dscratch1' is missing.

This patch makes a distinction between the x and f registers and the
other register sets.  For x and f we still make use of the renaming
scheme, forcing GDB to prefer the ABI name.  But after this patch the
CSR register group, and also the virtual register group, will always
prefer to use the name given in the target description, adding other
names as aliases, but not making any other name the preferred name.

gdb/ChangeLog:

	* riscv-tdep.c (struct riscv_register_feature::register_info): Fix
	whitespace error for declaration of names member variable.
	(struct riscv_register_feature): Add new prefer_first_name member
	variable, and fix whitespace error in declaration of registers.
	(riscv_xreg_feature): Initialize prefer_first_name field.
	(riscv_freg_feature): Likewise.
	(riscv_virtual_feature): Likewise.
	(riscv_csr_feature): Likewise.
	(riscv_register_name): Expand on comments.  Remove register name
	modifications for CSR and virtual registers.

gdb/testsuite/ChangeLog:

	* gdb.arch/riscv-tdesc-regs.exp: Extend test case.
---
 gdb/ChangeLog                               | 13 ++++
 gdb/riscv-tdep.c                            | 75 +++++++++++++--------
 gdb/testsuite/ChangeLog                     |  4 ++
 gdb/testsuite/gdb.arch/riscv-tdesc-regs.exp | 39 ++++++++++-
 4 files changed, 101 insertions(+), 30 deletions(-)

diff --git a/gdb/riscv-tdep.c b/gdb/riscv-tdep.c
index e394263a591..b136cbdfbf4 100644
--- a/gdb/riscv-tdep.c
+++ b/gdb/riscv-tdep.c
@@ -125,6 +125,20 @@ struct riscv_register_feature
      within the target description.  */
   const char *name;
 
+  /* For x-regs and f-regs we always force GDB to use the first name from
+     the REGISTERS.NAMES vector, it is therefore important that we create
+     user-register aliases for all of the remaining names at indexes 1+ in
+     the names vector.
+
+     For CSRs we take a different approach, we prefer whatever name the
+     target description uses, in this case we want to create user-register
+     aliases for any other names that aren't the target description
+     provided name.
+
+     When this flag is true we are dealing with the first case, and when
+     this is false we are dealing with the latter.  */
+  bool prefer_first_name;
+
   /* List of all the registers that we expect that we might find in this
      register set.  */
   std::vector<struct register_info> registers;
@@ -134,7 +148,7 @@ struct riscv_register_feature
 
 static const struct riscv_register_feature riscv_xreg_feature =
 {
- "org.gnu.gdb.riscv.cpu",
+ "org.gnu.gdb.riscv.cpu", true,
  {
    { RISCV_ZERO_REGNUM + 0, { "zero", "x0" }, true },
    { RISCV_ZERO_REGNUM + 1, { "ra", "x1" }, true },
@@ -176,7 +190,7 @@ static const struct riscv_register_feature riscv_xreg_feature =
 
 static const struct riscv_register_feature riscv_freg_feature =
 {
- "org.gnu.gdb.riscv.fpu",
+ "org.gnu.gdb.riscv.fpu", true,
  {
    { RISCV_FIRST_FP_REGNUM + 0, { "ft0", "f0" }, true },
    { RISCV_FIRST_FP_REGNUM + 1, { "ft1", "f1" }, true },
@@ -226,7 +240,7 @@ static const struct riscv_register_feature riscv_freg_feature =
 
 static const struct riscv_register_feature riscv_virtual_feature =
 {
- "org.gnu.gdb.riscv.virtual",
+ "org.gnu.gdb.riscv.virtual", false,
  {
    { RISCV_PRIV_REGNUM, { "priv" }, false }
  }
@@ -238,7 +252,7 @@ static const struct riscv_register_feature riscv_virtual_feature =
 
 static struct riscv_register_feature riscv_csr_feature =
 {
- "org.gnu.gdb.riscv.csr",
+ "org.gnu.gdb.riscv.csr", false,
  {
 #define DECLARE_CSR(NAME,VALUE,CLASS,DEFINE_VER,ABORT_VER) \
   { RISCV_ ## VALUE ## _REGNUM, { # NAME }, false },
@@ -472,7 +486,7 @@ value_of_riscv_user_reg (struct frame_info *frame, const void *baton)
 
 /* Implement the register_name gdbarch method.  This is used instead of
    the function supplied by calling TDESC_USE_REGISTERS so that we can
-   ensure the preferred names are offered.  */
+   ensure the preferred names are offered for x-regs and f-regs.  */
 
 static const char *
 riscv_register_name (struct gdbarch *gdbarch, int regnum)
@@ -484,12 +498,18 @@ riscv_register_name (struct gdbarch *gdbarch, int regnum)
   if (name == NULL || name[0] == '\0')
     return NULL;
 
+  /* We want GDB to use the ABI names for registers even if the target
+     gives us a target description with the architectural name.  For
+     example we want to see 'ra' instead of 'x1' whatever the target
+     description called it.  */
   if (regnum >= RISCV_ZERO_REGNUM && regnum < RISCV_FIRST_FP_REGNUM)
     {
       gdb_assert (regnum < riscv_xreg_feature.registers.size ());
       return riscv_xreg_feature.registers[regnum].names[0];
     }
 
+  /* Like with the x-regs we prefer the abi names for the floating point
+     registers.  */
   if (regnum >= RISCV_FIRST_FP_REGNUM && regnum <= RISCV_LAST_FP_REGNUM)
     {
       if (riscv_has_fp_regs (gdbarch))
@@ -502,28 +522,18 @@ riscv_register_name (struct gdbarch *gdbarch, int regnum)
         return NULL;
     }
 
-  /* Check that there's no gap between the set of registers handled above,
-     and the set of registers handled next.  */
-  gdb_assert ((RISCV_LAST_FP_REGNUM + 1) == RISCV_FIRST_CSR_REGNUM);
-
-  if (regnum >= RISCV_FIRST_CSR_REGNUM && regnum <= RISCV_LAST_CSR_REGNUM)
-    {
-#define DECLARE_CSR(NAME,VALUE,CLASS,DEFINE_VER,ABORT_VER) \
-      case RISCV_ ## VALUE ## _REGNUM: return # NAME;
-
-      switch (regnum)
-	{
-#include "opcode/riscv-opc.h"
-	}
-#undef DECLARE_CSR
-    }
+  /* The remaining registers are different.  For all other registers on the
+     machine we prefer to see the names that the target description
+     provides.  This is particularly important for CSRs which might be
+     renamed over time.  If GDB keeps track of the "latest" name, but a
+     particular target provides an older name then we don't want to force
+     users to see the newer name in register output.
 
-  if (regnum == RISCV_PRIV_REGNUM)
-    return "priv";
+     The other case that reaches here are any registers that the target
+     provided that GDB is completely unaware of.  For these we have no
+     choice but to accept the target description name.
 
-  /* It is possible that that the target provides some registers that GDB
-     is unaware of, in that case just return the NAME from the target
-     description.  */
+     Just accept whatever name TDESC_REGISTER_NAME returned.  */
   return name;
 }
 
@@ -3003,8 +3013,8 @@ riscv_check_tdesc_feature (struct tdesc_arch_data *tdesc_data,
 
       for (const char *name : reg.names)
 	{
-	  found =
-	    tdesc_numbered_register (feature, tdesc_data, reg.regnum, name);
+	  found = tdesc_numbered_register (feature, tdesc_data, reg.regnum,
+					   name);
 
 	  if (found)
             {
@@ -3012,8 +3022,15 @@ riscv_check_tdesc_feature (struct tdesc_arch_data *tdesc_data,
                  register.  In RISCV_REGISTER_NAME we ensure that GDB
                  always uses the first name for each register, so here we
                  add aliases for all of the remaining names.  */
-              for (int i = 0; i < reg.names.size (); ++i)
-		aliases->emplace_back (reg.names[i], (void *)&reg.regnum);
+	      bool prefer_first_name = reg_set->prefer_first_name;
+	      int start_index = prefer_first_name ? 1 : 0;
+	      for (int i = start_index; i < reg.names.size (); ++i)
+                {
+		  const char *alias = reg.names[i];
+                  if (alias == name && !prefer_first_name)
+		    continue;
+		  aliases->emplace_back (alias, (void *)&reg.regnum);
+                }
               break;
             }
 	}
diff --git a/gdb/testsuite/gdb.arch/riscv-tdesc-regs.exp b/gdb/testsuite/gdb.arch/riscv-tdesc-regs.exp
index 46e64d62a2b..63ac8fb7abc 100644
--- a/gdb/testsuite/gdb.arch/riscv-tdesc-regs.exp
+++ b/gdb/testsuite/gdb.arch/riscv-tdesc-regs.exp
@@ -14,7 +14,8 @@
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
 # Various tests to check which register names are available after
-# loading a new target description file.
+# loading a new target description file, and which registers show up
+# in the output of the 'info registers' command.
 
 if {![istarget "riscv*-*-*"]} {
     verbose "Skipping ${gdb_test_file_name}."
@@ -79,3 +80,39 @@ gdb_test "info registers \$csr0" "Invalid register `csr0'"
 gdb_test "info registers \$dscratch0" "dscratch0\[ \t\]+.*"
 gdb_test "info registers \$dscratch" "dscratch\[ \t\]+.*"
 
+foreach rgroup {all save restore} {
+    # Now use 'info registers all' to see how many times the floating
+    # point status registers show up in the output.
+    array set reg_counts {}
+    set test "info registers $rgroup"
+    gdb_test_multiple $test $test {
+	-re ".*info registers all\r\n" {
+	    verbose -log "Skip to first register"
+	    exp_continue
+	}
+	-re "^(\[^ \t\]+)\[ \t\]+\[^\r\n\]+\r\n" {
+	    set reg $expect_out(1,string)
+	    incr reg_counts($reg)
+	exp_continue
+	}
+	-re "^$gdb_prompt $" {
+	    # Done.
+	}
+    }
+
+    foreach reg {dscratch} {
+	if { [info exists reg_counts($reg) ] } {
+	    set count $reg_counts($reg)
+	} else {
+	    set count 0
+	}
+	if {$reg == "dscratch" && $rgroup != "all"} {
+	    gdb_assert {$count == 0} \
+		"register $reg not seen in reggroup $rgroup"
+	} else {
+	    gdb_assert {$count == 1} \
+		"register $reg seen once in reggroup $rgroup"
+	}
+    }
+    array unset reg_counts
+}
-- 
2.25.4


^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH 4/8] gdb/riscv: Remove CSR feature file
  2020-06-16 17:14 [PATCH 0/8] RISC-V target description and register handling fixes Andrew Burgess
                   ` (2 preceding siblings ...)
  2020-06-16 17:14 ` [PATCH 3/8] gdb/riscv: Take CSR names from target description Andrew Burgess
@ 2020-06-16 17:14 ` Andrew Burgess
  2020-06-16 17:14 ` [PATCH 5/8] gdb/riscv: Improve support for matching against target descriptions Andrew Burgess
                   ` (5 subsequent siblings)
  9 siblings, 0 replies; 13+ messages in thread
From: Andrew Burgess @ 2020-06-16 17:14 UTC (permalink / raw)
  To: gdb-patches; +Cc: Nelson Chu, Jim Wilson, Tom Tromey, palmer, Andrew Burgess

There is currently a bug in the RISC-V CSR/FPU feature files.  The
CSRs containing the FPU status registers are mentioned in both the FPU
feature file and the CSR feature file.

My original thinking when adding the FPU feature file was that it made
more sense to group the FPU status registers with the other FPU
state.  This opened up the possibility of debugging very
simple (possibly simulator only) targets that had little more than CPU
and FPU available for GDB to access.

When I then added code to automatically generate the CSR XML file I
forgot to filter out the FPU status CSRs, so these registers were
mentioned twice.

Now for GDB's default RISC-V target descriptions this doesn't actually
matter.  I did consider adding the CSRs to the default target
description, but in the end I didn't bother.  The reasoning again was
simplicity; the default target description is only to be used when the
target doesn't supply its own description, and NOT supplying the CSRs
actually serves to encourage targets to supply an accurate
description.  Combine this with the fact that the CSRs change from
revision to revision, sometimes in non-backward compatible ways, then
having a "default" set of CSRs just feels like a path to confusion and
complaints.

However, having a broken CSR XML file in the GDB source tree has had
one negative effect, QEMU has copied this file into its source tree,
and is using this as its description that it passes to GDB.  That is
QEMU announces the FPU status registers twice, once in the FPU
feature, and once in the CSR feature.

This commit starts along the path back to sanity by deleting the
default CSR XML files from within GDB.  These files were not used in
any way by current GDB, so there is absolutely no loss of
functionality with this change.

gdb/ChangeLog:

	* features/Makefile: Remove all references to the deleted files
	below.
	* features/riscv/32bit-csr.c: Deleted.
	* features/riscv/32bit-csr.xml: Deleted.
	* features/riscv/64bit-csr.c: Deleted.
	* features/riscv/64bit-csr.xml: Deleted.
	* features/riscv/rebuild-csr-xml.sh: Deleted.
---
 gdb/ChangeLog                         |  10 +
 gdb/features/Makefile                 |   6 -
 gdb/features/riscv/32bit-csr.c        | 253 --------------------------
 gdb/features/riscv/32bit-csr.xml      | 251 -------------------------
 gdb/features/riscv/64bit-csr.c        | 253 --------------------------
 gdb/features/riscv/64bit-csr.xml      | 186 -------------------
 gdb/features/riscv/rebuild-csr-xml.sh |  37 ----
 7 files changed, 10 insertions(+), 986 deletions(-)
 delete mode 100644 gdb/features/riscv/32bit-csr.c
 delete mode 100644 gdb/features/riscv/32bit-csr.xml
 delete mode 100644 gdb/features/riscv/64bit-csr.c
 delete mode 100644 gdb/features/riscv/64bit-csr.xml
 delete mode 100755 gdb/features/riscv/rebuild-csr-xml.sh

diff --git a/gdb/features/Makefile b/gdb/features/Makefile
index cc65baa6eda..88236e5e946 100644
--- a/gdb/features/Makefile
+++ b/gdb/features/Makefile
@@ -232,10 +232,8 @@ FEATURE_XMLFILES = aarch64-core.xml \
 	i386/64bit-sse.xml \
 	i386/x32-core.xml \
 	riscv/32bit-cpu.xml \
-	riscv/32bit-csr.xml \
 	riscv/32bit-fpu.xml \
 	riscv/64bit-cpu.xml \
-	riscv/64bit-csr.xml \
 	riscv/64bit-fpu.xml \
 	rx.xml \
 	tic6x-c6xp.xml \
@@ -268,10 +266,6 @@ $(FEATURE_CFILES): %.c: %.xml.tmp
 	echo "</target>" >> $@.tmp
 	sh ../../move-if-change $@.tmp $@
 
-# Regenerate RISC-V CSR feature lists.
-riscv/32bit-csr.xml riscv/64bit-csr.xml: ../../include/opcode/riscv-opc.h
-	./riscv/rebuild-csr-xml.sh ../../include/opcode/riscv-opc.h ./riscv
-
 # 'all' doesn't build the C files, so don't delete them in 'clean'
 # either.
 clean-cfiles:
diff --git a/gdb/features/riscv/32bit-csr.c b/gdb/features/riscv/32bit-csr.c
deleted file mode 100644
index d306bc8d293..00000000000
--- a/gdb/features/riscv/32bit-csr.c
+++ /dev/null
@@ -1,253 +0,0 @@
-/* THIS FILE IS GENERATED.  -*- buffer-read-only: t -*- vi:set ro:
-  Original: 32bit-csr.xml */
-
-#include "gdbsupport/tdesc.h"
-
-static int
-create_feature_riscv_32bit_csr (struct target_desc *result, long regnum)
-{
-  struct tdesc_feature *feature;
-
-  feature = tdesc_create_feature (result, "org.gnu.gdb.riscv.csr");
-  tdesc_create_reg (feature, "ustatus", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "uie", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "utvec", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "uscratch", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "uepc", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "ucause", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "utval", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "uip", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "fflags", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "frm", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "fcsr", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "cycle", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "time", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "instret", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "hpmcounter3", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "hpmcounter4", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "hpmcounter5", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "hpmcounter6", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "hpmcounter7", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "hpmcounter8", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "hpmcounter9", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "hpmcounter10", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "hpmcounter11", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "hpmcounter12", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "hpmcounter13", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "hpmcounter14", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "hpmcounter15", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "hpmcounter16", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "hpmcounter17", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "hpmcounter18", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "hpmcounter19", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "hpmcounter20", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "hpmcounter21", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "hpmcounter22", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "hpmcounter23", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "hpmcounter24", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "hpmcounter25", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "hpmcounter26", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "hpmcounter27", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "hpmcounter28", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "hpmcounter29", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "hpmcounter30", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "hpmcounter31", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "cycleh", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "timeh", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "instreth", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "hpmcounter3h", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "hpmcounter4h", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "hpmcounter5h", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "hpmcounter6h", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "hpmcounter7h", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "hpmcounter8h", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "hpmcounter9h", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "hpmcounter10h", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "hpmcounter11h", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "hpmcounter12h", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "hpmcounter13h", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "hpmcounter14h", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "hpmcounter15h", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "hpmcounter16h", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "hpmcounter17h", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "hpmcounter18h", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "hpmcounter19h", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "hpmcounter20h", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "hpmcounter21h", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "hpmcounter22h", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "hpmcounter23h", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "hpmcounter24h", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "hpmcounter25h", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "hpmcounter26h", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "hpmcounter27h", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "hpmcounter28h", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "hpmcounter29h", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "hpmcounter30h", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "hpmcounter31h", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "sstatus", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "sedeleg", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "sideleg", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "sie", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "stvec", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "scounteren", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "sscratch", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "sepc", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "scause", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "stval", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "sip", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "satp", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "mvendorid", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "marchid", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "mimpid", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "mhartid", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "mstatus", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "misa", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "medeleg", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "mideleg", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "mie", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "mtvec", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "mcounteren", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "mscratch", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "mepc", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "mcause", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "mtval", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "mip", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "pmpcfg0", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "pmpcfg1", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "pmpcfg2", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "pmpcfg3", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "pmpaddr0", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "pmpaddr1", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "pmpaddr2", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "pmpaddr3", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "pmpaddr4", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "pmpaddr5", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "pmpaddr6", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "pmpaddr7", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "pmpaddr8", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "pmpaddr9", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "pmpaddr10", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "pmpaddr11", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "pmpaddr12", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "pmpaddr13", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "pmpaddr14", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "pmpaddr15", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "mcycle", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "minstret", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "mhpmcounter3", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "mhpmcounter4", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "mhpmcounter5", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "mhpmcounter6", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "mhpmcounter7", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "mhpmcounter8", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "mhpmcounter9", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "mhpmcounter10", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "mhpmcounter11", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "mhpmcounter12", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "mhpmcounter13", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "mhpmcounter14", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "mhpmcounter15", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "mhpmcounter16", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "mhpmcounter17", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "mhpmcounter18", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "mhpmcounter19", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "mhpmcounter20", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "mhpmcounter21", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "mhpmcounter22", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "mhpmcounter23", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "mhpmcounter24", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "mhpmcounter25", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "mhpmcounter26", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "mhpmcounter27", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "mhpmcounter28", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "mhpmcounter29", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "mhpmcounter30", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "mhpmcounter31", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "mcycleh", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "minstreth", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "mhpmcounter3h", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "mhpmcounter4h", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "mhpmcounter5h", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "mhpmcounter6h", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "mhpmcounter7h", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "mhpmcounter8h", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "mhpmcounter9h", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "mhpmcounter10h", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "mhpmcounter11h", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "mhpmcounter12h", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "mhpmcounter13h", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "mhpmcounter14h", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "mhpmcounter15h", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "mhpmcounter16h", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "mhpmcounter17h", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "mhpmcounter18h", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "mhpmcounter19h", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "mhpmcounter20h", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "mhpmcounter21h", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "mhpmcounter22h", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "mhpmcounter23h", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "mhpmcounter24h", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "mhpmcounter25h", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "mhpmcounter26h", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "mhpmcounter27h", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "mhpmcounter28h", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "mhpmcounter29h", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "mhpmcounter30h", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "mhpmcounter31h", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "mhpmevent3", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "mhpmevent4", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "mhpmevent5", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "mhpmevent6", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "mhpmevent7", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "mhpmevent8", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "mhpmevent9", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "mhpmevent10", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "mhpmevent11", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "mhpmevent12", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "mhpmevent13", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "mhpmevent14", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "mhpmevent15", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "mhpmevent16", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "mhpmevent17", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "mhpmevent18", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "mhpmevent19", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "mhpmevent20", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "mhpmevent21", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "mhpmevent22", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "mhpmevent23", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "mhpmevent24", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "mhpmevent25", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "mhpmevent26", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "mhpmevent27", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "mhpmevent28", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "mhpmevent29", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "mhpmevent30", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "mhpmevent31", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "tselect", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "tdata1", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "tdata2", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "tdata3", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "dcsr", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "dpc", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "dscratch", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "hstatus", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "hedeleg", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "hideleg", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "hie", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "htvec", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "hscratch", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "hepc", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "hcause", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "hbadaddr", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "hip", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "mbase", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "mbound", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "mibase", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "mibound", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "mdbase", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "mdbound", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "mucounteren", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "mscounteren", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "mhcounteren", regnum++, 1, NULL, 32, "int");
-  return regnum;
-}
diff --git a/gdb/features/riscv/32bit-csr.xml b/gdb/features/riscv/32bit-csr.xml
deleted file mode 100644
index 8173eebda1c..00000000000
--- a/gdb/features/riscv/32bit-csr.xml
+++ /dev/null
@@ -1,251 +0,0 @@
-<?xml version="1.0"?>
-<!-- Copyright (C) 2018-2020 Free Software Foundation, Inc.
-
-     Copying and distribution of this file, with or without modification,
-     are permitted in any medium without royalty provided the copyright
-     notice and this notice are preserved.  -->
-
-<!DOCTYPE feature SYSTEM "gdb-target.dtd">
-<feature name="org.gnu.gdb.riscv.csr">
-  <reg name="ustatus" bitsize="32"/>
-  <reg name="uie" bitsize="32"/>
-  <reg name="utvec" bitsize="32"/>
-  <reg name="uscratch" bitsize="32"/>
-  <reg name="uepc" bitsize="32"/>
-  <reg name="ucause" bitsize="32"/>
-  <reg name="utval" bitsize="32"/>
-  <reg name="uip" bitsize="32"/>
-  <reg name="fflags" bitsize="32"/>
-  <reg name="frm" bitsize="32"/>
-  <reg name="fcsr" bitsize="32"/>
-  <reg name="cycle" bitsize="32"/>
-  <reg name="time" bitsize="32"/>
-  <reg name="instret" bitsize="32"/>
-  <reg name="hpmcounter3" bitsize="32"/>
-  <reg name="hpmcounter4" bitsize="32"/>
-  <reg name="hpmcounter5" bitsize="32"/>
-  <reg name="hpmcounter6" bitsize="32"/>
-  <reg name="hpmcounter7" bitsize="32"/>
-  <reg name="hpmcounter8" bitsize="32"/>
-  <reg name="hpmcounter9" bitsize="32"/>
-  <reg name="hpmcounter10" bitsize="32"/>
-  <reg name="hpmcounter11" bitsize="32"/>
-  <reg name="hpmcounter12" bitsize="32"/>
-  <reg name="hpmcounter13" bitsize="32"/>
-  <reg name="hpmcounter14" bitsize="32"/>
-  <reg name="hpmcounter15" bitsize="32"/>
-  <reg name="hpmcounter16" bitsize="32"/>
-  <reg name="hpmcounter17" bitsize="32"/>
-  <reg name="hpmcounter18" bitsize="32"/>
-  <reg name="hpmcounter19" bitsize="32"/>
-  <reg name="hpmcounter20" bitsize="32"/>
-  <reg name="hpmcounter21" bitsize="32"/>
-  <reg name="hpmcounter22" bitsize="32"/>
-  <reg name="hpmcounter23" bitsize="32"/>
-  <reg name="hpmcounter24" bitsize="32"/>
-  <reg name="hpmcounter25" bitsize="32"/>
-  <reg name="hpmcounter26" bitsize="32"/>
-  <reg name="hpmcounter27" bitsize="32"/>
-  <reg name="hpmcounter28" bitsize="32"/>
-  <reg name="hpmcounter29" bitsize="32"/>
-  <reg name="hpmcounter30" bitsize="32"/>
-  <reg name="hpmcounter31" bitsize="32"/>
-  <reg name="cycleh" bitsize="32"/>
-  <reg name="timeh" bitsize="32"/>
-  <reg name="instreth" bitsize="32"/>
-  <reg name="hpmcounter3h" bitsize="32"/>
-  <reg name="hpmcounter4h" bitsize="32"/>
-  <reg name="hpmcounter5h" bitsize="32"/>
-  <reg name="hpmcounter6h" bitsize="32"/>
-  <reg name="hpmcounter7h" bitsize="32"/>
-  <reg name="hpmcounter8h" bitsize="32"/>
-  <reg name="hpmcounter9h" bitsize="32"/>
-  <reg name="hpmcounter10h" bitsize="32"/>
-  <reg name="hpmcounter11h" bitsize="32"/>
-  <reg name="hpmcounter12h" bitsize="32"/>
-  <reg name="hpmcounter13h" bitsize="32"/>
-  <reg name="hpmcounter14h" bitsize="32"/>
-  <reg name="hpmcounter15h" bitsize="32"/>
-  <reg name="hpmcounter16h" bitsize="32"/>
-  <reg name="hpmcounter17h" bitsize="32"/>
-  <reg name="hpmcounter18h" bitsize="32"/>
-  <reg name="hpmcounter19h" bitsize="32"/>
-  <reg name="hpmcounter20h" bitsize="32"/>
-  <reg name="hpmcounter21h" bitsize="32"/>
-  <reg name="hpmcounter22h" bitsize="32"/>
-  <reg name="hpmcounter23h" bitsize="32"/>
-  <reg name="hpmcounter24h" bitsize="32"/>
-  <reg name="hpmcounter25h" bitsize="32"/>
-  <reg name="hpmcounter26h" bitsize="32"/>
-  <reg name="hpmcounter27h" bitsize="32"/>
-  <reg name="hpmcounter28h" bitsize="32"/>
-  <reg name="hpmcounter29h" bitsize="32"/>
-  <reg name="hpmcounter30h" bitsize="32"/>
-  <reg name="hpmcounter31h" bitsize="32"/>
-  <reg name="sstatus" bitsize="32"/>
-  <reg name="sedeleg" bitsize="32"/>
-  <reg name="sideleg" bitsize="32"/>
-  <reg name="sie" bitsize="32"/>
-  <reg name="stvec" bitsize="32"/>
-  <reg name="scounteren" bitsize="32"/>
-  <reg name="sscratch" bitsize="32"/>
-  <reg name="sepc" bitsize="32"/>
-  <reg name="scause" bitsize="32"/>
-  <reg name="stval" bitsize="32"/>
-  <reg name="sip" bitsize="32"/>
-  <reg name="satp" bitsize="32"/>
-  <reg name="mvendorid" bitsize="32"/>
-  <reg name="marchid" bitsize="32"/>
-  <reg name="mimpid" bitsize="32"/>
-  <reg name="mhartid" bitsize="32"/>
-  <reg name="mstatus" bitsize="32"/>
-  <reg name="misa" bitsize="32"/>
-  <reg name="medeleg" bitsize="32"/>
-  <reg name="mideleg" bitsize="32"/>
-  <reg name="mie" bitsize="32"/>
-  <reg name="mtvec" bitsize="32"/>
-  <reg name="mcounteren" bitsize="32"/>
-  <reg name="mscratch" bitsize="32"/>
-  <reg name="mepc" bitsize="32"/>
-  <reg name="mcause" bitsize="32"/>
-  <reg name="mtval" bitsize="32"/>
-  <reg name="mip" bitsize="32"/>
-  <reg name="pmpcfg0" bitsize="32"/>
-  <reg name="pmpcfg1" bitsize="32"/>
-  <reg name="pmpcfg2" bitsize="32"/>
-  <reg name="pmpcfg3" bitsize="32"/>
-  <reg name="pmpaddr0" bitsize="32"/>
-  <reg name="pmpaddr1" bitsize="32"/>
-  <reg name="pmpaddr2" bitsize="32"/>
-  <reg name="pmpaddr3" bitsize="32"/>
-  <reg name="pmpaddr4" bitsize="32"/>
-  <reg name="pmpaddr5" bitsize="32"/>
-  <reg name="pmpaddr6" bitsize="32"/>
-  <reg name="pmpaddr7" bitsize="32"/>
-  <reg name="pmpaddr8" bitsize="32"/>
-  <reg name="pmpaddr9" bitsize="32"/>
-  <reg name="pmpaddr10" bitsize="32"/>
-  <reg name="pmpaddr11" bitsize="32"/>
-  <reg name="pmpaddr12" bitsize="32"/>
-  <reg name="pmpaddr13" bitsize="32"/>
-  <reg name="pmpaddr14" bitsize="32"/>
-  <reg name="pmpaddr15" bitsize="32"/>
-  <reg name="mcycle" bitsize="32"/>
-  <reg name="minstret" bitsize="32"/>
-  <reg name="mhpmcounter3" bitsize="32"/>
-  <reg name="mhpmcounter4" bitsize="32"/>
-  <reg name="mhpmcounter5" bitsize="32"/>
-  <reg name="mhpmcounter6" bitsize="32"/>
-  <reg name="mhpmcounter7" bitsize="32"/>
-  <reg name="mhpmcounter8" bitsize="32"/>
-  <reg name="mhpmcounter9" bitsize="32"/>
-  <reg name="mhpmcounter10" bitsize="32"/>
-  <reg name="mhpmcounter11" bitsize="32"/>
-  <reg name="mhpmcounter12" bitsize="32"/>
-  <reg name="mhpmcounter13" bitsize="32"/>
-  <reg name="mhpmcounter14" bitsize="32"/>
-  <reg name="mhpmcounter15" bitsize="32"/>
-  <reg name="mhpmcounter16" bitsize="32"/>
-  <reg name="mhpmcounter17" bitsize="32"/>
-  <reg name="mhpmcounter18" bitsize="32"/>
-  <reg name="mhpmcounter19" bitsize="32"/>
-  <reg name="mhpmcounter20" bitsize="32"/>
-  <reg name="mhpmcounter21" bitsize="32"/>
-  <reg name="mhpmcounter22" bitsize="32"/>
-  <reg name="mhpmcounter23" bitsize="32"/>
-  <reg name="mhpmcounter24" bitsize="32"/>
-  <reg name="mhpmcounter25" bitsize="32"/>
-  <reg name="mhpmcounter26" bitsize="32"/>
-  <reg name="mhpmcounter27" bitsize="32"/>
-  <reg name="mhpmcounter28" bitsize="32"/>
-  <reg name="mhpmcounter29" bitsize="32"/>
-  <reg name="mhpmcounter30" bitsize="32"/>
-  <reg name="mhpmcounter31" bitsize="32"/>
-  <reg name="mcycleh" bitsize="32"/>
-  <reg name="minstreth" bitsize="32"/>
-  <reg name="mhpmcounter3h" bitsize="32"/>
-  <reg name="mhpmcounter4h" bitsize="32"/>
-  <reg name="mhpmcounter5h" bitsize="32"/>
-  <reg name="mhpmcounter6h" bitsize="32"/>
-  <reg name="mhpmcounter7h" bitsize="32"/>
-  <reg name="mhpmcounter8h" bitsize="32"/>
-  <reg name="mhpmcounter9h" bitsize="32"/>
-  <reg name="mhpmcounter10h" bitsize="32"/>
-  <reg name="mhpmcounter11h" bitsize="32"/>
-  <reg name="mhpmcounter12h" bitsize="32"/>
-  <reg name="mhpmcounter13h" bitsize="32"/>
-  <reg name="mhpmcounter14h" bitsize="32"/>
-  <reg name="mhpmcounter15h" bitsize="32"/>
-  <reg name="mhpmcounter16h" bitsize="32"/>
-  <reg name="mhpmcounter17h" bitsize="32"/>
-  <reg name="mhpmcounter18h" bitsize="32"/>
-  <reg name="mhpmcounter19h" bitsize="32"/>
-  <reg name="mhpmcounter20h" bitsize="32"/>
-  <reg name="mhpmcounter21h" bitsize="32"/>
-  <reg name="mhpmcounter22h" bitsize="32"/>
-  <reg name="mhpmcounter23h" bitsize="32"/>
-  <reg name="mhpmcounter24h" bitsize="32"/>
-  <reg name="mhpmcounter25h" bitsize="32"/>
-  <reg name="mhpmcounter26h" bitsize="32"/>
-  <reg name="mhpmcounter27h" bitsize="32"/>
-  <reg name="mhpmcounter28h" bitsize="32"/>
-  <reg name="mhpmcounter29h" bitsize="32"/>
-  <reg name="mhpmcounter30h" bitsize="32"/>
-  <reg name="mhpmcounter31h" bitsize="32"/>
-  <reg name="mcountinhibit" bitsize="32"/>
-  <reg name="mhpmevent3" bitsize="32"/>
-  <reg name="mhpmevent4" bitsize="32"/>
-  <reg name="mhpmevent5" bitsize="32"/>
-  <reg name="mhpmevent6" bitsize="32"/>
-  <reg name="mhpmevent7" bitsize="32"/>
-  <reg name="mhpmevent8" bitsize="32"/>
-  <reg name="mhpmevent9" bitsize="32"/>
-  <reg name="mhpmevent10" bitsize="32"/>
-  <reg name="mhpmevent11" bitsize="32"/>
-  <reg name="mhpmevent12" bitsize="32"/>
-  <reg name="mhpmevent13" bitsize="32"/>
-  <reg name="mhpmevent14" bitsize="32"/>
-  <reg name="mhpmevent15" bitsize="32"/>
-  <reg name="mhpmevent16" bitsize="32"/>
-  <reg name="mhpmevent17" bitsize="32"/>
-  <reg name="mhpmevent18" bitsize="32"/>
-  <reg name="mhpmevent19" bitsize="32"/>
-  <reg name="mhpmevent20" bitsize="32"/>
-  <reg name="mhpmevent21" bitsize="32"/>
-  <reg name="mhpmevent22" bitsize="32"/>
-  <reg name="mhpmevent23" bitsize="32"/>
-  <reg name="mhpmevent24" bitsize="32"/>
-  <reg name="mhpmevent25" bitsize="32"/>
-  <reg name="mhpmevent26" bitsize="32"/>
-  <reg name="mhpmevent27" bitsize="32"/>
-  <reg name="mhpmevent28" bitsize="32"/>
-  <reg name="mhpmevent29" bitsize="32"/>
-  <reg name="mhpmevent30" bitsize="32"/>
-  <reg name="mhpmevent31" bitsize="32"/>
-  <reg name="tselect" bitsize="32"/>
-  <reg name="tdata1" bitsize="32"/>
-  <reg name="tdata2" bitsize="32"/>
-  <reg name="tdata3" bitsize="32"/>
-  <reg name="dcsr" bitsize="32"/>
-  <reg name="dpc" bitsize="32"/>
-  <reg name="dscratch0" bitsize="32"/>
-  <reg name="dscratch1" bitsize="32"/>
-  <reg name="hstatus" bitsize="32"/>
-  <reg name="hedeleg" bitsize="32"/>
-  <reg name="hideleg" bitsize="32"/>
-  <reg name="hie" bitsize="32"/>
-  <reg name="htvec" bitsize="32"/>
-  <reg name="hscratch" bitsize="32"/>
-  <reg name="hepc" bitsize="32"/>
-  <reg name="hcause" bitsize="32"/>
-  <reg name="hbadaddr" bitsize="32"/>
-  <reg name="hip" bitsize="32"/>
-  <reg name="mbase" bitsize="32"/>
-  <reg name="mbound" bitsize="32"/>
-  <reg name="mibase" bitsize="32"/>
-  <reg name="mibound" bitsize="32"/>
-  <reg name="mdbase" bitsize="32"/>
-  <reg name="mdbound" bitsize="32"/>
-  <reg name="mscounteren" bitsize="32"/>
-  <reg name="mhcounteren" bitsize="32"/>
-</feature>
diff --git a/gdb/features/riscv/64bit-csr.c b/gdb/features/riscv/64bit-csr.c
deleted file mode 100644
index e46a77f3051..00000000000
--- a/gdb/features/riscv/64bit-csr.c
+++ /dev/null
@@ -1,253 +0,0 @@
-/* THIS FILE IS GENERATED.  -*- buffer-read-only: t -*- vi:set ro:
-  Original: 64bit-csr.xml */
-
-#include "gdbsupport/tdesc.h"
-
-static int
-create_feature_riscv_64bit_csr (struct target_desc *result, long regnum)
-{
-  struct tdesc_feature *feature;
-
-  feature = tdesc_create_feature (result, "org.gnu.gdb.riscv.csr");
-  tdesc_create_reg (feature, "ustatus", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "uie", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "utvec", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "uscratch", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "uepc", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "ucause", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "utval", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "uip", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "fflags", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "frm", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "fcsr", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "cycle", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "time", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "instret", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "hpmcounter3", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "hpmcounter4", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "hpmcounter5", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "hpmcounter6", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "hpmcounter7", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "hpmcounter8", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "hpmcounter9", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "hpmcounter10", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "hpmcounter11", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "hpmcounter12", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "hpmcounter13", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "hpmcounter14", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "hpmcounter15", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "hpmcounter16", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "hpmcounter17", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "hpmcounter18", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "hpmcounter19", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "hpmcounter20", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "hpmcounter21", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "hpmcounter22", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "hpmcounter23", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "hpmcounter24", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "hpmcounter25", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "hpmcounter26", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "hpmcounter27", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "hpmcounter28", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "hpmcounter29", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "hpmcounter30", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "hpmcounter31", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "cycleh", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "timeh", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "instreth", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "hpmcounter3h", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "hpmcounter4h", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "hpmcounter5h", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "hpmcounter6h", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "hpmcounter7h", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "hpmcounter8h", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "hpmcounter9h", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "hpmcounter10h", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "hpmcounter11h", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "hpmcounter12h", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "hpmcounter13h", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "hpmcounter14h", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "hpmcounter15h", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "hpmcounter16h", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "hpmcounter17h", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "hpmcounter18h", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "hpmcounter19h", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "hpmcounter20h", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "hpmcounter21h", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "hpmcounter22h", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "hpmcounter23h", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "hpmcounter24h", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "hpmcounter25h", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "hpmcounter26h", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "hpmcounter27h", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "hpmcounter28h", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "hpmcounter29h", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "hpmcounter30h", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "hpmcounter31h", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "sstatus", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "sedeleg", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "sideleg", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "sie", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "stvec", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "scounteren", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "sscratch", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "sepc", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "scause", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "stval", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "sip", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "satp", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "mvendorid", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "marchid", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "mimpid", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "mhartid", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "mstatus", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "misa", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "medeleg", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "mideleg", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "mie", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "mtvec", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "mcounteren", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "mscratch", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "mepc", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "mcause", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "mtval", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "mip", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "pmpcfg0", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "pmpcfg1", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "pmpcfg2", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "pmpcfg3", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "pmpaddr0", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "pmpaddr1", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "pmpaddr2", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "pmpaddr3", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "pmpaddr4", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "pmpaddr5", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "pmpaddr6", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "pmpaddr7", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "pmpaddr8", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "pmpaddr9", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "pmpaddr10", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "pmpaddr11", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "pmpaddr12", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "pmpaddr13", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "pmpaddr14", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "pmpaddr15", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "mcycle", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "minstret", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "mhpmcounter3", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "mhpmcounter4", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "mhpmcounter5", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "mhpmcounter6", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "mhpmcounter7", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "mhpmcounter8", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "mhpmcounter9", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "mhpmcounter10", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "mhpmcounter11", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "mhpmcounter12", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "mhpmcounter13", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "mhpmcounter14", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "mhpmcounter15", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "mhpmcounter16", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "mhpmcounter17", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "mhpmcounter18", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "mhpmcounter19", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "mhpmcounter20", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "mhpmcounter21", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "mhpmcounter22", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "mhpmcounter23", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "mhpmcounter24", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "mhpmcounter25", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "mhpmcounter26", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "mhpmcounter27", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "mhpmcounter28", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "mhpmcounter29", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "mhpmcounter30", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "mhpmcounter31", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "mcycleh", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "minstreth", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "mhpmcounter3h", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "mhpmcounter4h", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "mhpmcounter5h", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "mhpmcounter6h", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "mhpmcounter7h", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "mhpmcounter8h", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "mhpmcounter9h", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "mhpmcounter10h", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "mhpmcounter11h", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "mhpmcounter12h", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "mhpmcounter13h", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "mhpmcounter14h", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "mhpmcounter15h", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "mhpmcounter16h", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "mhpmcounter17h", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "mhpmcounter18h", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "mhpmcounter19h", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "mhpmcounter20h", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "mhpmcounter21h", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "mhpmcounter22h", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "mhpmcounter23h", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "mhpmcounter24h", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "mhpmcounter25h", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "mhpmcounter26h", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "mhpmcounter27h", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "mhpmcounter28h", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "mhpmcounter29h", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "mhpmcounter30h", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "mhpmcounter31h", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "mhpmevent3", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "mhpmevent4", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "mhpmevent5", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "mhpmevent6", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "mhpmevent7", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "mhpmevent8", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "mhpmevent9", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "mhpmevent10", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "mhpmevent11", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "mhpmevent12", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "mhpmevent13", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "mhpmevent14", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "mhpmevent15", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "mhpmevent16", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "mhpmevent17", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "mhpmevent18", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "mhpmevent19", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "mhpmevent20", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "mhpmevent21", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "mhpmevent22", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "mhpmevent23", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "mhpmevent24", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "mhpmevent25", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "mhpmevent26", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "mhpmevent27", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "mhpmevent28", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "mhpmevent29", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "mhpmevent30", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "mhpmevent31", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "tselect", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "tdata1", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "tdata2", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "tdata3", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "dcsr", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "dpc", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "dscratch", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "hstatus", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "hedeleg", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "hideleg", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "hie", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "htvec", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "hscratch", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "hepc", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "hcause", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "hbadaddr", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "hip", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "mbase", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "mbound", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "mibase", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "mibound", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "mdbase", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "mdbound", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "mucounteren", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "mscounteren", regnum++, 1, NULL, 64, "int");
-  tdesc_create_reg (feature, "mhcounteren", regnum++, 1, NULL, 64, "int");
-  return regnum;
-}
diff --git a/gdb/features/riscv/64bit-csr.xml b/gdb/features/riscv/64bit-csr.xml
deleted file mode 100644
index ed28964ac35..00000000000
--- a/gdb/features/riscv/64bit-csr.xml
+++ /dev/null
@@ -1,186 +0,0 @@
-<?xml version="1.0"?>
-<!-- Copyright (C) 2018-2020 Free Software Foundation, Inc.
-
-     Copying and distribution of this file, with or without modification,
-     are permitted in any medium without royalty provided the copyright
-     notice and this notice are preserved.  -->
-
-<!DOCTYPE feature SYSTEM "gdb-target.dtd">
-<feature name="org.gnu.gdb.riscv.csr">
-  <reg name="ustatus" bitsize="64"/>
-  <reg name="uie" bitsize="64"/>
-  <reg name="utvec" bitsize="64"/>
-  <reg name="uscratch" bitsize="64"/>
-  <reg name="uepc" bitsize="64"/>
-  <reg name="ucause" bitsize="64"/>
-  <reg name="utval" bitsize="64"/>
-  <reg name="uip" bitsize="64"/>
-  <reg name="fflags" bitsize="64"/>
-  <reg name="frm" bitsize="64"/>
-  <reg name="fcsr" bitsize="64"/>
-  <reg name="cycle" bitsize="64"/>
-  <reg name="time" bitsize="64"/>
-  <reg name="instret" bitsize="64"/>
-  <reg name="hpmcounter3" bitsize="64"/>
-  <reg name="hpmcounter4" bitsize="64"/>
-  <reg name="hpmcounter5" bitsize="64"/>
-  <reg name="hpmcounter6" bitsize="64"/>
-  <reg name="hpmcounter7" bitsize="64"/>
-  <reg name="hpmcounter8" bitsize="64"/>
-  <reg name="hpmcounter9" bitsize="64"/>
-  <reg name="hpmcounter10" bitsize="64"/>
-  <reg name="hpmcounter11" bitsize="64"/>
-  <reg name="hpmcounter12" bitsize="64"/>
-  <reg name="hpmcounter13" bitsize="64"/>
-  <reg name="hpmcounter14" bitsize="64"/>
-  <reg name="hpmcounter15" bitsize="64"/>
-  <reg name="hpmcounter16" bitsize="64"/>
-  <reg name="hpmcounter17" bitsize="64"/>
-  <reg name="hpmcounter18" bitsize="64"/>
-  <reg name="hpmcounter19" bitsize="64"/>
-  <reg name="hpmcounter20" bitsize="64"/>
-  <reg name="hpmcounter21" bitsize="64"/>
-  <reg name="hpmcounter22" bitsize="64"/>
-  <reg name="hpmcounter23" bitsize="64"/>
-  <reg name="hpmcounter24" bitsize="64"/>
-  <reg name="hpmcounter25" bitsize="64"/>
-  <reg name="hpmcounter26" bitsize="64"/>
-  <reg name="hpmcounter27" bitsize="64"/>
-  <reg name="hpmcounter28" bitsize="64"/>
-  <reg name="hpmcounter29" bitsize="64"/>
-  <reg name="hpmcounter30" bitsize="64"/>
-  <reg name="hpmcounter31" bitsize="64"/>
-  <reg name="sstatus" bitsize="64"/>
-  <reg name="sedeleg" bitsize="64"/>
-  <reg name="sideleg" bitsize="64"/>
-  <reg name="sie" bitsize="64"/>
-  <reg name="stvec" bitsize="64"/>
-  <reg name="scounteren" bitsize="64"/>
-  <reg name="sscratch" bitsize="64"/>
-  <reg name="sepc" bitsize="64"/>
-  <reg name="scause" bitsize="64"/>
-  <reg name="stval" bitsize="64"/>
-  <reg name="sip" bitsize="64"/>
-  <reg name="satp" bitsize="64"/>
-  <reg name="mvendorid" bitsize="64"/>
-  <reg name="marchid" bitsize="64"/>
-  <reg name="mimpid" bitsize="64"/>
-  <reg name="mhartid" bitsize="64"/>
-  <reg name="mstatus" bitsize="64"/>
-  <reg name="misa" bitsize="64"/>
-  <reg name="medeleg" bitsize="64"/>
-  <reg name="mideleg" bitsize="64"/>
-  <reg name="mie" bitsize="64"/>
-  <reg name="mtvec" bitsize="64"/>
-  <reg name="mcounteren" bitsize="64"/>
-  <reg name="mscratch" bitsize="64"/>
-  <reg name="mepc" bitsize="64"/>
-  <reg name="mcause" bitsize="64"/>
-  <reg name="mtval" bitsize="64"/>
-  <reg name="mip" bitsize="64"/>
-  <reg name="pmpcfg0" bitsize="64"/>
-  <reg name="pmpcfg2" bitsize="64"/>
-  <reg name="pmpaddr0" bitsize="64"/>
-  <reg name="pmpaddr1" bitsize="64"/>
-  <reg name="pmpaddr2" bitsize="64"/>
-  <reg name="pmpaddr3" bitsize="64"/>
-  <reg name="pmpaddr4" bitsize="64"/>
-  <reg name="pmpaddr5" bitsize="64"/>
-  <reg name="pmpaddr6" bitsize="64"/>
-  <reg name="pmpaddr7" bitsize="64"/>
-  <reg name="pmpaddr8" bitsize="64"/>
-  <reg name="pmpaddr9" bitsize="64"/>
-  <reg name="pmpaddr10" bitsize="64"/>
-  <reg name="pmpaddr11" bitsize="64"/>
-  <reg name="pmpaddr12" bitsize="64"/>
-  <reg name="pmpaddr13" bitsize="64"/>
-  <reg name="pmpaddr14" bitsize="64"/>
-  <reg name="pmpaddr15" bitsize="64"/>
-  <reg name="mcycle" bitsize="64"/>
-  <reg name="minstret" bitsize="64"/>
-  <reg name="mhpmcounter3" bitsize="64"/>
-  <reg name="mhpmcounter4" bitsize="64"/>
-  <reg name="mhpmcounter5" bitsize="64"/>
-  <reg name="mhpmcounter6" bitsize="64"/>
-  <reg name="mhpmcounter7" bitsize="64"/>
-  <reg name="mhpmcounter8" bitsize="64"/>
-  <reg name="mhpmcounter9" bitsize="64"/>
-  <reg name="mhpmcounter10" bitsize="64"/>
-  <reg name="mhpmcounter11" bitsize="64"/>
-  <reg name="mhpmcounter12" bitsize="64"/>
-  <reg name="mhpmcounter13" bitsize="64"/>
-  <reg name="mhpmcounter14" bitsize="64"/>
-  <reg name="mhpmcounter15" bitsize="64"/>
-  <reg name="mhpmcounter16" bitsize="64"/>
-  <reg name="mhpmcounter17" bitsize="64"/>
-  <reg name="mhpmcounter18" bitsize="64"/>
-  <reg name="mhpmcounter19" bitsize="64"/>
-  <reg name="mhpmcounter20" bitsize="64"/>
-  <reg name="mhpmcounter21" bitsize="64"/>
-  <reg name="mhpmcounter22" bitsize="64"/>
-  <reg name="mhpmcounter23" bitsize="64"/>
-  <reg name="mhpmcounter24" bitsize="64"/>
-  <reg name="mhpmcounter25" bitsize="64"/>
-  <reg name="mhpmcounter26" bitsize="64"/>
-  <reg name="mhpmcounter27" bitsize="64"/>
-  <reg name="mhpmcounter28" bitsize="64"/>
-  <reg name="mhpmcounter29" bitsize="64"/>
-  <reg name="mhpmcounter30" bitsize="64"/>
-  <reg name="mhpmcounter31" bitsize="64"/>
-  <reg name="mcountinhibit" bitsize="64"/>
-  <reg name="mhpmevent3" bitsize="64"/>
-  <reg name="mhpmevent4" bitsize="64"/>
-  <reg name="mhpmevent5" bitsize="64"/>
-  <reg name="mhpmevent6" bitsize="64"/>
-  <reg name="mhpmevent7" bitsize="64"/>
-  <reg name="mhpmevent8" bitsize="64"/>
-  <reg name="mhpmevent9" bitsize="64"/>
-  <reg name="mhpmevent10" bitsize="64"/>
-  <reg name="mhpmevent11" bitsize="64"/>
-  <reg name="mhpmevent12" bitsize="64"/>
-  <reg name="mhpmevent13" bitsize="64"/>
-  <reg name="mhpmevent14" bitsize="64"/>
-  <reg name="mhpmevent15" bitsize="64"/>
-  <reg name="mhpmevent16" bitsize="64"/>
-  <reg name="mhpmevent17" bitsize="64"/>
-  <reg name="mhpmevent18" bitsize="64"/>
-  <reg name="mhpmevent19" bitsize="64"/>
-  <reg name="mhpmevent20" bitsize="64"/>
-  <reg name="mhpmevent21" bitsize="64"/>
-  <reg name="mhpmevent22" bitsize="64"/>
-  <reg name="mhpmevent23" bitsize="64"/>
-  <reg name="mhpmevent24" bitsize="64"/>
-  <reg name="mhpmevent25" bitsize="64"/>
-  <reg name="mhpmevent26" bitsize="64"/>
-  <reg name="mhpmevent27" bitsize="64"/>
-  <reg name="mhpmevent28" bitsize="64"/>
-  <reg name="mhpmevent29" bitsize="64"/>
-  <reg name="mhpmevent30" bitsize="64"/>
-  <reg name="mhpmevent31" bitsize="64"/>
-  <reg name="tselect" bitsize="64"/>
-  <reg name="tdata1" bitsize="64"/>
-  <reg name="tdata2" bitsize="64"/>
-  <reg name="tdata3" bitsize="64"/>
-  <reg name="dcsr" bitsize="64"/>
-  <reg name="dpc" bitsize="64"/>
-  <reg name="dscratch0" bitsize="64"/>
-  <reg name="dscratch1" bitsize="64"/>
-  <reg name="hstatus" bitsize="64"/>
-  <reg name="hedeleg" bitsize="64"/>
-  <reg name="hideleg" bitsize="64"/>
-  <reg name="hie" bitsize="64"/>
-  <reg name="htvec" bitsize="64"/>
-  <reg name="hscratch" bitsize="64"/>
-  <reg name="hepc" bitsize="64"/>
-  <reg name="hcause" bitsize="64"/>
-  <reg name="hbadaddr" bitsize="64"/>
-  <reg name="hip" bitsize="64"/>
-  <reg name="mbase" bitsize="64"/>
-  <reg name="mbound" bitsize="64"/>
-  <reg name="mibase" bitsize="64"/>
-  <reg name="mibound" bitsize="64"/>
-  <reg name="mdbase" bitsize="64"/>
-  <reg name="mdbound" bitsize="64"/>
-  <reg name="mscounteren" bitsize="64"/>
-  <reg name="mhcounteren" bitsize="64"/>
-</feature>
diff --git a/gdb/features/riscv/rebuild-csr-xml.sh b/gdb/features/riscv/rebuild-csr-xml.sh
deleted file mode 100755
index bff79b0371f..00000000000
--- a/gdb/features/riscv/rebuild-csr-xml.sh
+++ /dev/null
@@ -1,37 +0,0 @@
-#! /bin/bash
-
-RISCV_OPC_FILE=$1
-RISCV_FEATURE_DIR=$2
-
-function gen_csr_xml ()
-{
-    bitsize=$1
-
-    cat <<EOF
-<?xml version="1.0"?>
-<!-- Copyright (C) 2018-2020 Free Software Foundation, Inc.
-
-     Copying and distribution of this file, with or without modification,
-     are permitted in any medium without royalty provided the copyright
-     notice and this notice are preserved.  -->
-
-<!DOCTYPE feature SYSTEM "gdb-target.dtd">
-<feature name="org.gnu.gdb.riscv.csr">
-EOF
-
-if [ "$bitsize" = "64" ]; then
-    grep "^DECLARE_CSR(" ${RISCV_OPC_FILE} \
-        | sed /CSR_CLASS_.*_32/d \
-        | sed -e "s!DECLARE_CSR(\(.*\), .*, .*, .*, .*!  <reg name=\"\1\" bitsize=\"$bitsize\"/>!"
-
-    echo "</feature>"
-else
-    grep "^DECLARE_CSR(" ${RISCV_OPC_FILE} \
-        | sed -e "s!DECLARE_CSR(\(.*\), .*, .*, .*, .*!  <reg name=\"\1\" bitsize=\"$bitsize\"/>!"
-
-    echo "</feature>"
-fi
-}
-
-gen_csr_xml 32 > ${RISCV_FEATURE_DIR}/32bit-csr.xml
-gen_csr_xml 64 > ${RISCV_FEATURE_DIR}/64bit-csr.xml
-- 
2.25.4


^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH 5/8] gdb/riscv: Improve support for matching against target descriptions
  2020-06-16 17:14 [PATCH 0/8] RISC-V target description and register handling fixes Andrew Burgess
                   ` (3 preceding siblings ...)
  2020-06-16 17:14 ` [PATCH 4/8] gdb/riscv: Remove CSR feature file Andrew Burgess
@ 2020-06-16 17:14 ` Andrew Burgess
  2020-06-16 17:14 ` [PATCH 6/8] gdb: Extend target description processing of unknown registers Andrew Burgess
                   ` (4 subsequent siblings)
  9 siblings, 0 replies; 13+ messages in thread
From: Andrew Burgess @ 2020-06-16 17:14 UTC (permalink / raw)
  To: gdb-patches; +Cc: Nelson Chu, Jim Wilson, Tom Tromey, palmer, Andrew Burgess

For the RISC-V target it is desirable if the three floating pointer
status CSRs fflags, frm, and fcsr can be placed into either the FPU
feature or the CSR feature.  This allows different targets to build
the features in a way that better reflects their target.

The change to support this within GDB is fairly simple, so this is
done in this commit, and some tests added to check this new
functionality.

gdb/ChangeLog:

	* riscv-tdep.c (value_of_riscv_user_reg): Moved to here from later
	in the file.
	(class riscv_pending_register_alias): Likewise.
	(riscv_register_feature::register_info): Change 'required_p' field
	to 'required', and change its type.  Add 'check' member function.
	(riscv_register_feature::register_info::check): Define new member
	function.
	(riscv_xreg_feature): Change initialisation of 'required' field.
	(riscv_freg_feature): Likewise.
	(riscv_virtual_feature): Likewise.
	(riscv_csr_feature): Likewise.
	(riscv_check_tdesc_feature): Take extra parameter, the csr
	tdesc_feature, rewrite the function to use the new
	riscv_register_feature::register_info::check function.
	(riscv_gdbarch_init): Pass the csr tdesc_feature where needed.

gdb/testsuite/ChangeLog:

	* gdb.arch/riscv-tdesc-loading-01.xml: New file.
	* gdb.arch/riscv-tdesc-loading-02.xml: New file.
	* gdb.arch/riscv-tdesc-loading-03.xml: New file.
	* gdb.arch/riscv-tdesc-loading-04.xml: New file.
	* gdb.arch/riscv-tdesc-loading.exp: New file.
---
 gdb/ChangeLog                                 |  18 +
 gdb/riscv-tdep.c                              | 335 ++++++++++--------
 gdb/testsuite/ChangeLog                       |   8 +
 .../gdb.arch/riscv-tdesc-loading-01.xml       |  83 +++++
 .../gdb.arch/riscv-tdesc-loading-02.xml       |  81 +++++
 .../gdb.arch/riscv-tdesc-loading-03.xml       |  79 +++++
 .../gdb.arch/riscv-tdesc-loading-04.xml       |  77 ++++
 .../gdb.arch/riscv-tdesc-loading.exp          |  39 ++
 8 files changed, 578 insertions(+), 142 deletions(-)
 create mode 100644 gdb/testsuite/gdb.arch/riscv-tdesc-loading-01.xml
 create mode 100644 gdb/testsuite/gdb.arch/riscv-tdesc-loading-02.xml
 create mode 100644 gdb/testsuite/gdb.arch/riscv-tdesc-loading-03.xml
 create mode 100644 gdb/testsuite/gdb.arch/riscv-tdesc-loading-04.xml
 create mode 100644 gdb/testsuite/gdb.arch/riscv-tdesc-loading.exp

diff --git a/gdb/riscv-tdep.c b/gdb/riscv-tdep.c
index b136cbdfbf4..f161c1ac625 100644
--- a/gdb/riscv-tdep.c
+++ b/gdb/riscv-tdep.c
@@ -101,6 +101,66 @@ struct riscv_unwind_cache
 
 static reggroup *csr_reggroup = NULL;
 
+/* Callback function for user_reg_add.  */
+
+static struct value *
+value_of_riscv_user_reg (struct frame_info *frame, const void *baton)
+{
+  const int *reg_p = (const int *) baton;
+  return value_of_register (*reg_p, frame);
+}
+
+/* Information about a register alias that needs to be set up for this
+   target.  These are collected when the target's XML description is
+   analysed, and then processed later, once the gdbarch has been created.  */
+
+class riscv_pending_register_alias
+{
+public:
+  /* Constructor.  */
+
+  riscv_pending_register_alias (const char *name, const void *baton)
+    : m_name (name),
+      m_baton (baton)
+  { /* Nothing.  */ }
+
+  /* Convert this into a user register for GDBARCH.  */
+
+  void create (struct gdbarch *gdbarch) const
+  {
+    user_reg_add (gdbarch, m_name, value_of_riscv_user_reg, m_baton);
+  }
+
+private:
+  /* The name for this alias.  */
+  const char *m_name;
+
+  /* The baton value for passing to user_reg_add.  This must point to some
+     data that will live for at least as long as the gdbarch object to
+     which the user register is attached.  */
+  const void *m_baton;
+};
+
+/* Registers in the RISCV_REGISTER_FEATURE lists below are either optional,
+   or required.  For example the $pc register is always going to be a
+   required register, you can't do much debugging without that.  In
+   contrast, most of the CSRs are optional, GDB doesn't require them in
+   order to have a useful debug session.  This enum models the difference
+   between these register types.  */
+
+enum riscv_register_required_status
+{
+  /* This register is optional within this feature.  */
+  RISCV_REG_OPTIONAL,
+
+  /* This register is required within this feature.  */
+  RISCV_REG_REQUIRED,
+
+  /* This register is required, the register must either be in this
+     feature, or it could appear within the CSR feature.  */
+  RISCV_REG_REQUIRED_MAYBE_CSR
+};
+
 /* A set of registers that we expect to find in a tdesc_feature.  These
    are use in RISCV_GDBARCH_INIT when processing the target description.  */
 
@@ -117,8 +177,18 @@ struct riscv_register_feature
        register.  */
     std::vector<const char *> names;
 
-    /* When true this register is required in this feature set.  */
-    bool required_p;
+    /* Is this register required within this feature?  In some cases the
+       register could be required, but might also be in the CSR feature.  */
+    riscv_register_required_status required;
+
+    /* Look in FEATURE for a register with a name from this classes names
+       list.  If the register is found then register its number with
+       TDESC_DATA and add all its aliases to the ALIASES list.  REG_SET is
+       used to help create the aliases.  */
+    bool check (struct tdesc_arch_data *tdesc_data,
+		const struct tdesc_feature *feature,
+		const struct riscv_register_feature *reg_set,
+		std::vector<riscv_pending_register_alias> *aliases) const;
   };
 
   /* The name for this feature.  This is the name used to find this feature
@@ -144,45 +214,79 @@ struct riscv_register_feature
   std::vector<struct register_info> registers;
 };
 
+/* See description in the class declaration above.  */
+
+bool
+riscv_register_feature::register_info::check
+	(struct tdesc_arch_data *tdesc_data,
+	 const struct tdesc_feature *feature,
+	 const struct riscv_register_feature *reg_set,
+	 std::vector<riscv_pending_register_alias> *aliases) const
+{
+  for (const char *name : this->names)
+    {
+      bool found = tdesc_numbered_register (feature, tdesc_data,
+					    this->regnum, name);
+      if (found)
+	{
+	  /* We know that the target description mentions this
+	     register.  In RISCV_REGISTER_NAME we ensure that GDB
+	     always uses the first name for each register, so here we
+	     add aliases for all of the remaining names.  */
+	  bool prefer_first_name = reg_set->prefer_first_name;
+	  int start_index = prefer_first_name ? 1 : 0;
+	  for (int i = start_index; i < this->names.size (); ++i)
+	    {
+	      const char *alias = this->names[i];
+	      if (alias == name && !prefer_first_name)
+		continue;
+	      aliases->emplace_back (alias, (void *)&this->regnum);
+	    }
+	  return true;
+	}
+    }
+  return false;
+}
+
 /* The general x-registers feature set.  */
 
 static const struct riscv_register_feature riscv_xreg_feature =
 {
  "org.gnu.gdb.riscv.cpu", true,
  {
-   { RISCV_ZERO_REGNUM + 0, { "zero", "x0" }, true },
-   { RISCV_ZERO_REGNUM + 1, { "ra", "x1" }, true },
-   { RISCV_ZERO_REGNUM + 2, { "sp", "x2" }, true },
-   { RISCV_ZERO_REGNUM + 3, { "gp", "x3" }, true },
-   { RISCV_ZERO_REGNUM + 4, { "tp", "x4" }, true },
-   { RISCV_ZERO_REGNUM + 5, { "t0", "x5" }, true },
-   { RISCV_ZERO_REGNUM + 6, { "t1", "x6" }, true },
-   { RISCV_ZERO_REGNUM + 7, { "t2", "x7" }, true },
-   { RISCV_ZERO_REGNUM + 8, { "fp", "x8", "s0" }, true },
-   { RISCV_ZERO_REGNUM + 9, { "s1", "x9" }, true },
-   { RISCV_ZERO_REGNUM + 10, { "a0", "x10" }, true },
-   { RISCV_ZERO_REGNUM + 11, { "a1", "x11" }, true },
-   { RISCV_ZERO_REGNUM + 12, { "a2", "x12" }, true },
-   { RISCV_ZERO_REGNUM + 13, { "a3", "x13" }, true },
-   { RISCV_ZERO_REGNUM + 14, { "a4", "x14" }, true },
-   { RISCV_ZERO_REGNUM + 15, { "a5", "x15" }, true },
-   { RISCV_ZERO_REGNUM + 16, { "a6", "x16" }, true },
-   { RISCV_ZERO_REGNUM + 17, { "a7", "x17" }, true },
-   { RISCV_ZERO_REGNUM + 18, { "s2", "x18" }, true },
-   { RISCV_ZERO_REGNUM + 19, { "s3", "x19" }, true },
-   { RISCV_ZERO_REGNUM + 20, { "s4", "x20" }, true },
-   { RISCV_ZERO_REGNUM + 21, { "s5", "x21" }, true },
-   { RISCV_ZERO_REGNUM + 22, { "s6", "x22" }, true },
-   { RISCV_ZERO_REGNUM + 23, { "s7", "x23" }, true },
-   { RISCV_ZERO_REGNUM + 24, { "s8", "x24" }, true },
-   { RISCV_ZERO_REGNUM + 25, { "s9", "x25" }, true },
-   { RISCV_ZERO_REGNUM + 26, { "s10", "x26" }, true },
-   { RISCV_ZERO_REGNUM + 27, { "s11", "x27" }, true },
-   { RISCV_ZERO_REGNUM + 28, { "t3", "x28" }, true },
-   { RISCV_ZERO_REGNUM + 29, { "t4", "x29" }, true },
-   { RISCV_ZERO_REGNUM + 30, { "t5", "x30" }, true },
-   { RISCV_ZERO_REGNUM + 31, { "t6", "x31" }, true },
-   { RISCV_ZERO_REGNUM + 32, { "pc" }, true }
+   { RISCV_ZERO_REGNUM + 0, { "zero", "x0" }, RISCV_REG_REQUIRED },
+   { RISCV_ZERO_REGNUM + 1, { "ra", "x1" }, RISCV_REG_REQUIRED },
+   { RISCV_ZERO_REGNUM + 2, { "sp", "x2" }, RISCV_REG_REQUIRED },
+   { RISCV_ZERO_REGNUM + 3, { "gp", "x3" }, RISCV_REG_REQUIRED },
+   { RISCV_ZERO_REGNUM + 4, { "tp", "x4" }, RISCV_REG_REQUIRED },
+   { RISCV_ZERO_REGNUM + 5, { "t0", "x5" }, RISCV_REG_REQUIRED },
+   { RISCV_ZERO_REGNUM + 6, { "t1", "x6" }, RISCV_REG_REQUIRED },
+   { RISCV_ZERO_REGNUM + 7, { "t2", "x7" }, RISCV_REG_REQUIRED },
+   { RISCV_ZERO_REGNUM + 8, { "fp", "x8", "s0" }, RISCV_REG_REQUIRED },
+   { RISCV_ZERO_REGNUM + 9, { "s1", "x9" }, RISCV_REG_REQUIRED },
+   { RISCV_ZERO_REGNUM + 10, { "a0", "x10" }, RISCV_REG_REQUIRED },
+   { RISCV_ZERO_REGNUM + 11, { "a1", "x11" }, RISCV_REG_REQUIRED },
+   { RISCV_ZERO_REGNUM + 12, { "a2", "x12" }, RISCV_REG_REQUIRED },
+   { RISCV_ZERO_REGNUM + 13, { "a3", "x13" }, RISCV_REG_REQUIRED },
+   { RISCV_ZERO_REGNUM + 14, { "a4", "x14" }, RISCV_REG_REQUIRED },
+   { RISCV_ZERO_REGNUM + 15, { "a5", "x15" }, RISCV_REG_REQUIRED },
+   { RISCV_ZERO_REGNUM + 16, { "a6", "x16" }, RISCV_REG_REQUIRED },
+   { RISCV_ZERO_REGNUM + 17, { "a7", "x17" }, RISCV_REG_REQUIRED },
+   { RISCV_ZERO_REGNUM + 18, { "s2", "x18" }, RISCV_REG_REQUIRED },
+   { RISCV_ZERO_REGNUM + 19, { "s3", "x19" }, RISCV_REG_REQUIRED },
+   { RISCV_ZERO_REGNUM + 20, { "s4", "x20" }, RISCV_REG_REQUIRED },
+   { RISCV_ZERO_REGNUM + 21, { "s5", "x21" }, RISCV_REG_REQUIRED },
+   { RISCV_ZERO_REGNUM + 22, { "s6", "x22" }, RISCV_REG_REQUIRED },
+   { RISCV_ZERO_REGNUM + 23, { "s7", "x23" }, RISCV_REG_REQUIRED },
+   { RISCV_ZERO_REGNUM + 24, { "s8", "x24" }, RISCV_REG_REQUIRED },
+   { RISCV_ZERO_REGNUM + 25, { "s9", "x25" }, RISCV_REG_REQUIRED },
+   { RISCV_ZERO_REGNUM + 26, { "s10", "x26" }, RISCV_REG_REQUIRED },
+   { RISCV_ZERO_REGNUM + 27, { "s11", "x27" }, RISCV_REG_REQUIRED },
+   { RISCV_ZERO_REGNUM + 28, { "t3", "x28" }, RISCV_REG_REQUIRED },
+   { RISCV_ZERO_REGNUM + 29, { "t4", "x29" }, RISCV_REG_REQUIRED },
+   { RISCV_ZERO_REGNUM + 30, { "t5", "x30" }, RISCV_REG_REQUIRED },
+   { RISCV_ZERO_REGNUM + 31, { "t6", "x31" }, RISCV_REG_REQUIRED },
+   { RISCV_ZERO_REGNUM + 32, { "pc" }, RISCV_REG_REQUIRED }
  }
 };
 
@@ -192,42 +296,42 @@ static const struct riscv_register_feature riscv_freg_feature =
 {
  "org.gnu.gdb.riscv.fpu", true,
  {
-   { RISCV_FIRST_FP_REGNUM + 0, { "ft0", "f0" }, true },
-   { RISCV_FIRST_FP_REGNUM + 1, { "ft1", "f1" }, true },
-   { RISCV_FIRST_FP_REGNUM + 2, { "ft2", "f2" }, true },
-   { RISCV_FIRST_FP_REGNUM + 3, { "ft3", "f3" }, true },
-   { RISCV_FIRST_FP_REGNUM + 4, { "ft4", "f4" }, true },
-   { RISCV_FIRST_FP_REGNUM + 5, { "ft5", "f5" }, true },
-   { RISCV_FIRST_FP_REGNUM + 6, { "ft6", "f6" }, true },
-   { RISCV_FIRST_FP_REGNUM + 7, { "ft7", "f7" }, true },
-   { RISCV_FIRST_FP_REGNUM + 8, { "fs0", "f8" }, true },
-   { RISCV_FIRST_FP_REGNUM + 9, { "fs1", "f9" }, true },
-   { RISCV_FIRST_FP_REGNUM + 10, { "fa0", "f10" }, true },
-   { RISCV_FIRST_FP_REGNUM + 11, { "fa1", "f11" }, true },
-   { RISCV_FIRST_FP_REGNUM + 12, { "fa2", "f12" }, true },
-   { RISCV_FIRST_FP_REGNUM + 13, { "fa3", "f13" }, true },
-   { RISCV_FIRST_FP_REGNUM + 14, { "fa4", "f14" }, true },
-   { RISCV_FIRST_FP_REGNUM + 15, { "fa5", "f15" }, true },
-   { RISCV_FIRST_FP_REGNUM + 16, { "fa6", "f16" }, true },
-   { RISCV_FIRST_FP_REGNUM + 17, { "fa7", "f17" }, true },
-   { RISCV_FIRST_FP_REGNUM + 18, { "fs2", "f18" }, true },
-   { RISCV_FIRST_FP_REGNUM + 19, { "fs3", "f19" }, true },
-   { RISCV_FIRST_FP_REGNUM + 20, { "fs4", "f20" }, true },
-   { RISCV_FIRST_FP_REGNUM + 21, { "fs5", "f21" }, true },
-   { RISCV_FIRST_FP_REGNUM + 22, { "fs6", "f22" }, true },
-   { RISCV_FIRST_FP_REGNUM + 23, { "fs7", "f23" }, true },
-   { RISCV_FIRST_FP_REGNUM + 24, { "fs8", "f24" }, true },
-   { RISCV_FIRST_FP_REGNUM + 25, { "fs9", "f25" }, true },
-   { RISCV_FIRST_FP_REGNUM + 26, { "fs10", "f26" }, true },
-   { RISCV_FIRST_FP_REGNUM + 27, { "fs11", "f27" }, true },
-   { RISCV_FIRST_FP_REGNUM + 28, { "ft8", "f28" }, true },
-   { RISCV_FIRST_FP_REGNUM + 29, { "ft9", "f29" }, true },
-   { RISCV_FIRST_FP_REGNUM + 30, { "ft10", "f30" }, true },
-   { RISCV_FIRST_FP_REGNUM + 31, { "ft11", "f31" }, true },
-
-   { RISCV_CSR_FFLAGS_REGNUM, { "fflags" }, true },
-   { RISCV_CSR_FRM_REGNUM, { "frm" }, true },
-   { RISCV_CSR_FCSR_REGNUM, { "fcsr" }, true },
+   { RISCV_FIRST_FP_REGNUM + 0, { "ft0", "f0" }, RISCV_REG_REQUIRED },
+   { RISCV_FIRST_FP_REGNUM + 1, { "ft1", "f1" }, RISCV_REG_REQUIRED },
+   { RISCV_FIRST_FP_REGNUM + 2, { "ft2", "f2" }, RISCV_REG_REQUIRED },
+   { RISCV_FIRST_FP_REGNUM + 3, { "ft3", "f3" }, RISCV_REG_REQUIRED },
+   { RISCV_FIRST_FP_REGNUM + 4, { "ft4", "f4" }, RISCV_REG_REQUIRED },
+   { RISCV_FIRST_FP_REGNUM + 5, { "ft5", "f5" }, RISCV_REG_REQUIRED },
+   { RISCV_FIRST_FP_REGNUM + 6, { "ft6", "f6" }, RISCV_REG_REQUIRED },
+   { RISCV_FIRST_FP_REGNUM + 7, { "ft7", "f7" }, RISCV_REG_REQUIRED },
+   { RISCV_FIRST_FP_REGNUM + 8, { "fs0", "f8" }, RISCV_REG_REQUIRED },
+   { RISCV_FIRST_FP_REGNUM + 9, { "fs1", "f9" }, RISCV_REG_REQUIRED },
+   { RISCV_FIRST_FP_REGNUM + 10, { "fa0", "f10" }, RISCV_REG_REQUIRED },
+   { RISCV_FIRST_FP_REGNUM + 11, { "fa1", "f11" }, RISCV_REG_REQUIRED },
+   { RISCV_FIRST_FP_REGNUM + 12, { "fa2", "f12" }, RISCV_REG_REQUIRED },
+   { RISCV_FIRST_FP_REGNUM + 13, { "fa3", "f13" }, RISCV_REG_REQUIRED },
+   { RISCV_FIRST_FP_REGNUM + 14, { "fa4", "f14" }, RISCV_REG_REQUIRED },
+   { RISCV_FIRST_FP_REGNUM + 15, { "fa5", "f15" }, RISCV_REG_REQUIRED },
+   { RISCV_FIRST_FP_REGNUM + 16, { "fa6", "f16" }, RISCV_REG_REQUIRED },
+   { RISCV_FIRST_FP_REGNUM + 17, { "fa7", "f17" }, RISCV_REG_REQUIRED },
+   { RISCV_FIRST_FP_REGNUM + 18, { "fs2", "f18" }, RISCV_REG_REQUIRED },
+   { RISCV_FIRST_FP_REGNUM + 19, { "fs3", "f19" }, RISCV_REG_REQUIRED },
+   { RISCV_FIRST_FP_REGNUM + 20, { "fs4", "f20" }, RISCV_REG_REQUIRED },
+   { RISCV_FIRST_FP_REGNUM + 21, { "fs5", "f21" }, RISCV_REG_REQUIRED },
+   { RISCV_FIRST_FP_REGNUM + 22, { "fs6", "f22" }, RISCV_REG_REQUIRED },
+   { RISCV_FIRST_FP_REGNUM + 23, { "fs7", "f23" }, RISCV_REG_REQUIRED },
+   { RISCV_FIRST_FP_REGNUM + 24, { "fs8", "f24" }, RISCV_REG_REQUIRED },
+   { RISCV_FIRST_FP_REGNUM + 25, { "fs9", "f25" }, RISCV_REG_REQUIRED },
+   { RISCV_FIRST_FP_REGNUM + 26, { "fs10", "f26" }, RISCV_REG_REQUIRED },
+   { RISCV_FIRST_FP_REGNUM + 27, { "fs11", "f27" }, RISCV_REG_REQUIRED },
+   { RISCV_FIRST_FP_REGNUM + 28, { "ft8", "f28" }, RISCV_REG_REQUIRED },
+   { RISCV_FIRST_FP_REGNUM + 29, { "ft9", "f29" }, RISCV_REG_REQUIRED },
+   { RISCV_FIRST_FP_REGNUM + 30, { "ft10", "f30" }, RISCV_REG_REQUIRED },
+   { RISCV_FIRST_FP_REGNUM + 31, { "ft11", "f31" }, RISCV_REG_REQUIRED },
+
+   { RISCV_CSR_FFLAGS_REGNUM, { "fflags", "csr1" }, RISCV_REG_REQUIRED_MAYBE_CSR },
+   { RISCV_CSR_FRM_REGNUM, { "frm", "csr2" }, RISCV_REG_REQUIRED_MAYBE_CSR },
+   { RISCV_CSR_FCSR_REGNUM, { "fcsr", "csr3" }, RISCV_REG_REQUIRED_MAYBE_CSR },
 
  }
 };
@@ -242,7 +346,7 @@ static const struct riscv_register_feature riscv_virtual_feature =
 {
  "org.gnu.gdb.riscv.virtual", false,
  {
-   { RISCV_PRIV_REGNUM, { "priv" }, false }
+   { RISCV_PRIV_REGNUM, { "priv" }, RISCV_REG_OPTIONAL }
  }
 };
 
@@ -255,7 +359,7 @@ static struct riscv_register_feature riscv_csr_feature =
  "org.gnu.gdb.riscv.csr", false,
  {
 #define DECLARE_CSR(NAME,VALUE,CLASS,DEFINE_VER,ABORT_VER) \
-  { RISCV_ ## VALUE ## _REGNUM, { # NAME }, false },
+  { RISCV_ ## VALUE ## _REGNUM, { # NAME }, RISCV_REG_OPTIONAL },
 #include "opcode/riscv-opc.h"
 #undef DECLARE_CSR
  }
@@ -475,15 +579,6 @@ riscv_sw_breakpoint_from_kind (struct gdbarch *gdbarch, int kind, int *size)
     }
 }
 
-/* Callback function for user_reg_add.  */
-
-static struct value *
-value_of_riscv_user_reg (struct frame_info *frame, const void *baton)
-{
-  const int *reg_p = (const int *) baton;
-  return value_of_register (*reg_p, frame);
-}
-
 /* Implement the register_name gdbarch method.  This is used instead of
    the function supplied by calling TDESC_USE_REGISTERS so that we can
    ensure the preferred names are offered for x-regs and f-regs.  */
@@ -2965,37 +3060,6 @@ riscv_find_default_target_description (const struct gdbarch_info info)
   return riscv_lookup_target_description (features);
 }
 
-/* Information about a register alias that needs to be set up for this
-   target.  These are collected when the target's XML description is
-   analysed, and then processed later, once the gdbarch has been created.  */
-
-class riscv_pending_register_alias
-{
-public:
-  /* Constructor.  */
-
-  riscv_pending_register_alias (const char *name, const void *baton)
-    : m_name (name),
-      m_baton (baton)
-  { /* Nothing.  */ }
-
-  /* Convert this into a user register for GDBARCH.  */
-
-  void create (struct gdbarch *gdbarch) const
-  {
-    user_reg_add (gdbarch, m_name, value_of_riscv_user_reg, m_baton);
-  }
-
-private:
-  /* The name for this alias.  */
-  const char *m_name;
-
-  /* The baton value for passing to user_reg_add.  This must point to some
-     data that will live for at least as long as the gdbarch object to
-     which the user register is attached.  */
-  const void *m_baton;
-};
-
 /* All of the registers in REG_SET are checked for in FEATURE, TDESC_DATA
    is updated with the register numbers for each register as listed in
    REG_SET.  If any register marked as required in REG_SET is not found in
@@ -3003,40 +3067,26 @@ class riscv_pending_register_alias
 
 static bool
 riscv_check_tdesc_feature (struct tdesc_arch_data *tdesc_data,
-                           const struct tdesc_feature *feature,
+                           const struct tdesc_feature *main_feature,
+                           const struct tdesc_feature *csr_feature,
                            const struct riscv_register_feature *reg_set,
                            std::vector<riscv_pending_register_alias> *aliases)
 {
   for (const auto &reg : reg_set->registers)
     {
-      bool found = false;
+      bool found = reg.check (tdesc_data, main_feature, reg_set, aliases);
 
-      for (const char *name : reg.names)
+      if (!found && reg.required != RISCV_REG_OPTIONAL)
 	{
-	  found = tdesc_numbered_register (feature, tdesc_data, reg.regnum,
-					   name);
-
-	  if (found)
-            {
-              /* We know that the target description mentions this
-                 register.  In RISCV_REGISTER_NAME we ensure that GDB
-                 always uses the first name for each register, so here we
-                 add aliases for all of the remaining names.  */
-	      bool prefer_first_name = reg_set->prefer_first_name;
-	      int start_index = prefer_first_name ? 1 : 0;
-	      for (int i = start_index; i < reg.names.size (); ++i)
-                {
-		  const char *alias = reg.names[i];
-                  if (alias == name && !prefer_first_name)
-		    continue;
-		  aliases->emplace_back (alias, (void *)&reg.regnum);
-                }
-              break;
-            }
+	  if (reg.required == RISCV_REG_REQUIRED_MAYBE_CSR
+	      && csr_feature != nullptr)
+	    {
+	      gdb_assert (main_feature != csr_feature);
+	      found = reg.check (tdesc_data, csr_feature,  reg_set, aliases);
+	    }
+	  if (!found)
+	    return false;
 	}
-
-      if (!found && reg.required_p)
-	return false;
     }
 
   return true;
@@ -3166,7 +3216,7 @@ riscv_gdbarch_init (struct gdbarch_info info,
   std::vector<riscv_pending_register_alias> pending_aliases;
 
   bool valid_p = riscv_check_tdesc_feature (tdesc_data,
-                                            feature_cpu,
+                                            feature_cpu, feature_csr,
                                             &riscv_xreg_feature,
                                             &pending_aliases);
   if (valid_p)
@@ -3188,6 +3238,7 @@ riscv_gdbarch_init (struct gdbarch_info info,
   if (feature_fpu != NULL)
     {
       valid_p &= riscv_check_tdesc_feature (tdesc_data, feature_fpu,
+					    feature_csr,
                                             &riscv_freg_feature,
                                             &pending_aliases);
 
@@ -3224,12 +3275,12 @@ riscv_gdbarch_init (struct gdbarch_info info,
     }
 
   if (feature_virtual)
-    riscv_check_tdesc_feature (tdesc_data, feature_virtual,
+    riscv_check_tdesc_feature (tdesc_data, feature_virtual, feature_csr,
                                &riscv_virtual_feature,
                                &pending_aliases);
 
   if (feature_csr)
-    riscv_check_tdesc_feature (tdesc_data, feature_csr,
+    riscv_check_tdesc_feature (tdesc_data, feature_csr, nullptr,
                                &riscv_csr_feature,
                                &pending_aliases);
 
diff --git a/gdb/testsuite/gdb.arch/riscv-tdesc-loading-01.xml b/gdb/testsuite/gdb.arch/riscv-tdesc-loading-01.xml
new file mode 100644
index 00000000000..39bde201c63
--- /dev/null
+++ b/gdb/testsuite/gdb.arch/riscv-tdesc-loading-01.xml
@@ -0,0 +1,83 @@
+<?xml version="1.0"?>
+<!DOCTYPE target SYSTEM "gdb-target.dtd">
+<target>
+  <architecture>riscv</architecture>
+  <feature name="org.gnu.gdb.riscv.cpu">
+    <reg name="zero" bitsize="64" type="int"/>
+    <reg name="ra" bitsize="64" type="code_ptr"/>
+    <reg name="sp" bitsize="64" type="data_ptr"/>
+    <reg name="gp" bitsize="64" type="data_ptr"/>
+    <reg name="tp" bitsize="64" type="data_ptr"/>
+    <reg name="t0" bitsize="64" type="int"/>
+    <reg name="t1" bitsize="64" type="int"/>
+    <reg name="t2" bitsize="64" type="int"/>
+    <reg name="fp" bitsize="64" type="data_ptr"/>
+    <reg name="s1" bitsize="64" type="int"/>
+    <reg name="a0" bitsize="64" type="int"/>
+    <reg name="a1" bitsize="64" type="int"/>
+    <reg name="a2" bitsize="64" type="int"/>
+    <reg name="a3" bitsize="64" type="int"/>
+    <reg name="a4" bitsize="64" type="int"/>
+    <reg name="a5" bitsize="64" type="int"/>
+    <reg name="a6" bitsize="64" type="int"/>
+    <reg name="a7" bitsize="64" type="int"/>
+    <reg name="s2" bitsize="64" type="int"/>
+    <reg name="s3" bitsize="64" type="int"/>
+    <reg name="s4" bitsize="64" type="int"/>
+    <reg name="s5" bitsize="64" type="int"/>
+    <reg name="s6" bitsize="64" type="int"/>
+    <reg name="s7" bitsize="64" type="int"/>
+    <reg name="s8" bitsize="64" type="int"/>
+    <reg name="s9" bitsize="64" type="int"/>
+    <reg name="s10" bitsize="64" type="int"/>
+    <reg name="s11" bitsize="64" type="int"/>
+    <reg name="t3" bitsize="64" type="int"/>
+    <reg name="t4" bitsize="64" type="int"/>
+    <reg name="t5" bitsize="64" type="int"/>
+    <reg name="t6" bitsize="64" type="int"/>
+    <reg name="pc" bitsize="64" type="code_ptr"/>
+  </feature>
+  <feature name="org.gnu.gdb.riscv.fpu">
+    <union id="riscv_double">
+      <field name="float" type="ieee_single"/>
+      <field name="double" type="ieee_double"/>
+    </union>
+    <reg name="ft0" bitsize="64" type="riscv_double"/>
+    <reg name="ft1" bitsize="64" type="riscv_double"/>
+    <reg name="ft2" bitsize="64" type="riscv_double"/>
+    <reg name="ft3" bitsize="64" type="riscv_double"/>
+    <reg name="ft4" bitsize="64" type="riscv_double"/>
+    <reg name="ft5" bitsize="64" type="riscv_double"/>
+    <reg name="ft6" bitsize="64" type="riscv_double"/>
+    <reg name="ft7" bitsize="64" type="riscv_double"/>
+    <reg name="fs0" bitsize="64" type="riscv_double"/>
+    <reg name="fs1" bitsize="64" type="riscv_double"/>
+    <reg name="fa0" bitsize="64" type="riscv_double"/>
+    <reg name="fa1" bitsize="64" type="riscv_double"/>
+    <reg name="fa2" bitsize="64" type="riscv_double"/>
+    <reg name="fa3" bitsize="64" type="riscv_double"/>
+    <reg name="fa4" bitsize="64" type="riscv_double"/>
+    <reg name="fa5" bitsize="64" type="riscv_double"/>
+    <reg name="fa6" bitsize="64" type="riscv_double"/>
+    <reg name="fa7" bitsize="64" type="riscv_double"/>
+    <reg name="fs2" bitsize="64" type="riscv_double"/>
+    <reg name="fs3" bitsize="64" type="riscv_double"/>
+    <reg name="fs4" bitsize="64" type="riscv_double"/>
+    <reg name="fs5" bitsize="64" type="riscv_double"/>
+    <reg name="fs6" bitsize="64" type="riscv_double"/>
+    <reg name="fs7" bitsize="64" type="riscv_double"/>
+    <reg name="fs8" bitsize="64" type="riscv_double"/>
+    <reg name="fs9" bitsize="64" type="riscv_double"/>
+    <reg name="fs10" bitsize="64" type="riscv_double"/>
+    <reg name="fs11" bitsize="64" type="riscv_double"/>
+    <reg name="ft8" bitsize="64" type="riscv_double"/>
+    <reg name="ft9" bitsize="64" type="riscv_double"/>
+    <reg name="ft10" bitsize="64" type="riscv_double"/>
+    <reg name="ft11" bitsize="64" type="riscv_double"/>
+  </feature>
+  <feature name="org.gnu.gdb.riscv.csr">
+    <reg name="fflags" bitsize="32" type="int"/>
+    <reg name="frm" bitsize="32" type="int"/>
+    <reg name="fcsr" bitsize="32" type="int"/>
+  </feature>
+</target>
diff --git a/gdb/testsuite/gdb.arch/riscv-tdesc-loading-02.xml b/gdb/testsuite/gdb.arch/riscv-tdesc-loading-02.xml
new file mode 100644
index 00000000000..db0771b863a
--- /dev/null
+++ b/gdb/testsuite/gdb.arch/riscv-tdesc-loading-02.xml
@@ -0,0 +1,81 @@
+<?xml version="1.0"?>
+<!DOCTYPE target SYSTEM "gdb-target.dtd">
+<target>
+  <architecture>riscv</architecture>
+  <feature name="org.gnu.gdb.riscv.cpu">
+    <reg name="zero" bitsize="64" type="int"/>
+    <reg name="ra" bitsize="64" type="code_ptr"/>
+    <reg name="sp" bitsize="64" type="data_ptr"/>
+    <reg name="gp" bitsize="64" type="data_ptr"/>
+    <reg name="tp" bitsize="64" type="data_ptr"/>
+    <reg name="t0" bitsize="64" type="int"/>
+    <reg name="t1" bitsize="64" type="int"/>
+    <reg name="t2" bitsize="64" type="int"/>
+    <reg name="fp" bitsize="64" type="data_ptr"/>
+    <reg name="s1" bitsize="64" type="int"/>
+    <reg name="a0" bitsize="64" type="int"/>
+    <reg name="a1" bitsize="64" type="int"/>
+    <reg name="a2" bitsize="64" type="int"/>
+    <reg name="a3" bitsize="64" type="int"/>
+    <reg name="a4" bitsize="64" type="int"/>
+    <reg name="a5" bitsize="64" type="int"/>
+    <reg name="a6" bitsize="64" type="int"/>
+    <reg name="a7" bitsize="64" type="int"/>
+    <reg name="s2" bitsize="64" type="int"/>
+    <reg name="s3" bitsize="64" type="int"/>
+    <reg name="s4" bitsize="64" type="int"/>
+    <reg name="s5" bitsize="64" type="int"/>
+    <reg name="s6" bitsize="64" type="int"/>
+    <reg name="s7" bitsize="64" type="int"/>
+    <reg name="s8" bitsize="64" type="int"/>
+    <reg name="s9" bitsize="64" type="int"/>
+    <reg name="s10" bitsize="64" type="int"/>
+    <reg name="s11" bitsize="64" type="int"/>
+    <reg name="t3" bitsize="64" type="int"/>
+    <reg name="t4" bitsize="64" type="int"/>
+    <reg name="t5" bitsize="64" type="int"/>
+    <reg name="t6" bitsize="64" type="int"/>
+    <reg name="pc" bitsize="64" type="code_ptr"/>
+  </feature>
+  <feature name="org.gnu.gdb.riscv.fpu">
+    <union id="riscv_double">
+      <field name="float" type="ieee_single"/>
+      <field name="double" type="ieee_double"/>
+    </union>
+    <reg name="ft0" bitsize="64" type="riscv_double"/>
+    <reg name="ft1" bitsize="64" type="riscv_double"/>
+    <reg name="ft2" bitsize="64" type="riscv_double"/>
+    <reg name="ft3" bitsize="64" type="riscv_double"/>
+    <reg name="ft4" bitsize="64" type="riscv_double"/>
+    <reg name="ft5" bitsize="64" type="riscv_double"/>
+    <reg name="ft6" bitsize="64" type="riscv_double"/>
+    <reg name="ft7" bitsize="64" type="riscv_double"/>
+    <reg name="fs0" bitsize="64" type="riscv_double"/>
+    <reg name="fs1" bitsize="64" type="riscv_double"/>
+    <reg name="fa0" bitsize="64" type="riscv_double"/>
+    <reg name="fa1" bitsize="64" type="riscv_double"/>
+    <reg name="fa2" bitsize="64" type="riscv_double"/>
+    <reg name="fa3" bitsize="64" type="riscv_double"/>
+    <reg name="fa4" bitsize="64" type="riscv_double"/>
+    <reg name="fa5" bitsize="64" type="riscv_double"/>
+    <reg name="fa6" bitsize="64" type="riscv_double"/>
+    <reg name="fa7" bitsize="64" type="riscv_double"/>
+    <reg name="fs2" bitsize="64" type="riscv_double"/>
+    <reg name="fs3" bitsize="64" type="riscv_double"/>
+    <reg name="fs4" bitsize="64" type="riscv_double"/>
+    <reg name="fs5" bitsize="64" type="riscv_double"/>
+    <reg name="fs6" bitsize="64" type="riscv_double"/>
+    <reg name="fs7" bitsize="64" type="riscv_double"/>
+    <reg name="fs8" bitsize="64" type="riscv_double"/>
+    <reg name="fs9" bitsize="64" type="riscv_double"/>
+    <reg name="fs10" bitsize="64" type="riscv_double"/>
+    <reg name="fs11" bitsize="64" type="riscv_double"/>
+    <reg name="ft8" bitsize="64" type="riscv_double"/>
+    <reg name="ft9" bitsize="64" type="riscv_double"/>
+    <reg name="ft10" bitsize="64" type="riscv_double"/>
+    <reg name="ft11" bitsize="64" type="riscv_double"/>
+    <reg name="fflags" bitsize="32" type="int"/>
+    <reg name="frm" bitsize="32" type="int"/>
+    <reg name="fcsr" bitsize="32" type="int"/>
+  </feature>
+</target>
diff --git a/gdb/testsuite/gdb.arch/riscv-tdesc-loading-03.xml b/gdb/testsuite/gdb.arch/riscv-tdesc-loading-03.xml
new file mode 100644
index 00000000000..b4af362c196
--- /dev/null
+++ b/gdb/testsuite/gdb.arch/riscv-tdesc-loading-03.xml
@@ -0,0 +1,79 @@
+<?xml version="1.0"?>
+<!DOCTYPE target SYSTEM "gdb-target.dtd">
+<target>
+  <architecture>riscv</architecture>
+  <feature name="org.gnu.gdb.riscv.cpu">
+    <reg name="zero" bitsize="32" type="int"/>
+    <reg name="ra" bitsize="32" type="code_ptr"/>
+    <reg name="sp" bitsize="32" type="data_ptr"/>
+    <reg name="gp" bitsize="32" type="data_ptr"/>
+    <reg name="tp" bitsize="32" type="data_ptr"/>
+    <reg name="t0" bitsize="32" type="int"/>
+    <reg name="t1" bitsize="32" type="int"/>
+    <reg name="t2" bitsize="32" type="int"/>
+    <reg name="fp" bitsize="32" type="data_ptr"/>
+    <reg name="s1" bitsize="32" type="int"/>
+    <reg name="a0" bitsize="32" type="int"/>
+    <reg name="a1" bitsize="32" type="int"/>
+    <reg name="a2" bitsize="32" type="int"/>
+    <reg name="a3" bitsize="32" type="int"/>
+    <reg name="a4" bitsize="32" type="int"/>
+    <reg name="a5" bitsize="32" type="int"/>
+    <reg name="a6" bitsize="32" type="int"/>
+    <reg name="a7" bitsize="32" type="int"/>
+    <reg name="s2" bitsize="32" type="int"/>
+    <reg name="s3" bitsize="32" type="int"/>
+    <reg name="s4" bitsize="32" type="int"/>
+    <reg name="s5" bitsize="32" type="int"/>
+    <reg name="s6" bitsize="32" type="int"/>
+    <reg name="s7" bitsize="32" type="int"/>
+    <reg name="s8" bitsize="32" type="int"/>
+    <reg name="s9" bitsize="32" type="int"/>
+    <reg name="s10" bitsize="32" type="int"/>
+    <reg name="s11" bitsize="32" type="int"/>
+    <reg name="t3" bitsize="32" type="int"/>
+    <reg name="t4" bitsize="32" type="int"/>
+    <reg name="t5" bitsize="32" type="int"/>
+    <reg name="t6" bitsize="32" type="int"/>
+    <reg name="pc" bitsize="32" type="code_ptr"/>
+  </feature>
+  <feature name="org.gnu.gdb.riscv.fpu">
+    <reg name="ft0" bitsize="32" type="float"/>
+    <reg name="ft1" bitsize="32" type="float"/>
+    <reg name="ft2" bitsize="32" type="float"/>
+    <reg name="ft3" bitsize="32" type="float"/>
+    <reg name="ft4" bitsize="32" type="float"/>
+    <reg name="ft5" bitsize="32" type="float"/>
+    <reg name="ft6" bitsize="32" type="float"/>
+    <reg name="ft7" bitsize="32" type="float"/>
+    <reg name="fs0" bitsize="32" type="float"/>
+    <reg name="fs1" bitsize="32" type="float"/>
+    <reg name="fa0" bitsize="32" type="float"/>
+    <reg name="fa1" bitsize="32" type="float"/>
+    <reg name="fa2" bitsize="32" type="float"/>
+    <reg name="fa3" bitsize="32" type="float"/>
+    <reg name="fa4" bitsize="32" type="float"/>
+    <reg name="fa5" bitsize="32" type="float"/>
+    <reg name="fa6" bitsize="32" type="float"/>
+    <reg name="fa7" bitsize="32" type="float"/>
+    <reg name="fs2" bitsize="32" type="float"/>
+    <reg name="fs3" bitsize="32" type="float"/>
+    <reg name="fs4" bitsize="32" type="float"/>
+    <reg name="fs5" bitsize="32" type="float"/>
+    <reg name="fs6" bitsize="32" type="float"/>
+    <reg name="fs7" bitsize="32" type="float"/>
+    <reg name="fs8" bitsize="32" type="float"/>
+    <reg name="fs9" bitsize="32" type="float"/>
+    <reg name="fs10" bitsize="32" type="float"/>
+    <reg name="fs11" bitsize="32" type="float"/>
+    <reg name="ft8" bitsize="32" type="float"/>
+    <reg name="ft9" bitsize="32" type="float"/>
+    <reg name="ft10" bitsize="32" type="float"/>
+    <reg name="ft11" bitsize="32" type="float"/>
+  </feature>
+  <feature name="org.gnu.gdb.riscv.csr">
+    <reg name="fflags" bitsize="32" type="int"/>
+    <reg name="frm" bitsize="32" type="int"/>
+    <reg name="fcsr" bitsize="32" type="int"/>
+  </feature>
+</target>
diff --git a/gdb/testsuite/gdb.arch/riscv-tdesc-loading-04.xml b/gdb/testsuite/gdb.arch/riscv-tdesc-loading-04.xml
new file mode 100644
index 00000000000..d44d7e27d87
--- /dev/null
+++ b/gdb/testsuite/gdb.arch/riscv-tdesc-loading-04.xml
@@ -0,0 +1,77 @@
+<?xml version="1.0"?>
+<!DOCTYPE target SYSTEM "gdb-target.dtd">
+<target>
+  <architecture>riscv</architecture>
+  <feature name="org.gnu.gdb.riscv.cpu">
+    <reg name="zero" bitsize="32" type="int"/>
+    <reg name="ra" bitsize="32" type="code_ptr"/>
+    <reg name="sp" bitsize="32" type="data_ptr"/>
+    <reg name="gp" bitsize="32" type="data_ptr"/>
+    <reg name="tp" bitsize="32" type="data_ptr"/>
+    <reg name="t0" bitsize="32" type="int"/>
+    <reg name="t1" bitsize="32" type="int"/>
+    <reg name="t2" bitsize="32" type="int"/>
+    <reg name="fp" bitsize="32" type="data_ptr"/>
+    <reg name="s1" bitsize="32" type="int"/>
+    <reg name="a0" bitsize="32" type="int"/>
+    <reg name="a1" bitsize="32" type="int"/>
+    <reg name="a2" bitsize="32" type="int"/>
+    <reg name="a3" bitsize="32" type="int"/>
+    <reg name="a4" bitsize="32" type="int"/>
+    <reg name="a5" bitsize="32" type="int"/>
+    <reg name="a6" bitsize="32" type="int"/>
+    <reg name="a7" bitsize="32" type="int"/>
+    <reg name="s2" bitsize="32" type="int"/>
+    <reg name="s3" bitsize="32" type="int"/>
+    <reg name="s4" bitsize="32" type="int"/>
+    <reg name="s5" bitsize="32" type="int"/>
+    <reg name="s6" bitsize="32" type="int"/>
+    <reg name="s7" bitsize="32" type="int"/>
+    <reg name="s8" bitsize="32" type="int"/>
+    <reg name="s9" bitsize="32" type="int"/>
+    <reg name="s10" bitsize="32" type="int"/>
+    <reg name="s11" bitsize="32" type="int"/>
+    <reg name="t3" bitsize="32" type="int"/>
+    <reg name="t4" bitsize="32" type="int"/>
+    <reg name="t5" bitsize="32" type="int"/>
+    <reg name="t6" bitsize="32" type="int"/>
+    <reg name="pc" bitsize="32" type="code_ptr"/>
+  </feature>
+  <feature name="org.gnu.gdb.riscv.fpu">
+    <reg name="ft0" bitsize="32" type="float"/>
+    <reg name="ft1" bitsize="32" type="float"/>
+    <reg name="ft2" bitsize="32" type="float"/>
+    <reg name="ft3" bitsize="32" type="float"/>
+    <reg name="ft4" bitsize="32" type="float"/>
+    <reg name="ft5" bitsize="32" type="float"/>
+    <reg name="ft6" bitsize="32" type="float"/>
+    <reg name="ft7" bitsize="32" type="float"/>
+    <reg name="fs0" bitsize="32" type="float"/>
+    <reg name="fs1" bitsize="32" type="float"/>
+    <reg name="fa0" bitsize="32" type="float"/>
+    <reg name="fa1" bitsize="32" type="float"/>
+    <reg name="fa2" bitsize="32" type="float"/>
+    <reg name="fa3" bitsize="32" type="float"/>
+    <reg name="fa4" bitsize="32" type="float"/>
+    <reg name="fa5" bitsize="32" type="float"/>
+    <reg name="fa6" bitsize="32" type="float"/>
+    <reg name="fa7" bitsize="32" type="float"/>
+    <reg name="fs2" bitsize="32" type="float"/>
+    <reg name="fs3" bitsize="32" type="float"/>
+    <reg name="fs4" bitsize="32" type="float"/>
+    <reg name="fs5" bitsize="32" type="float"/>
+    <reg name="fs6" bitsize="32" type="float"/>
+    <reg name="fs7" bitsize="32" type="float"/>
+    <reg name="fs8" bitsize="32" type="float"/>
+    <reg name="fs9" bitsize="32" type="float"/>
+    <reg name="fs10" bitsize="32" type="float"/>
+    <reg name="fs11" bitsize="32" type="float"/>
+    <reg name="ft8" bitsize="32" type="float"/>
+    <reg name="ft9" bitsize="32" type="float"/>
+    <reg name="ft10" bitsize="32" type="float"/>
+    <reg name="ft11" bitsize="32" type="float"/>
+    <reg name="fflags" bitsize="32" type="int"/>
+    <reg name="frm" bitsize="32" type="int"/>
+    <reg name="fcsr" bitsize="32" type="int"/>
+  </feature>
+</target>
diff --git a/gdb/testsuite/gdb.arch/riscv-tdesc-loading.exp b/gdb/testsuite/gdb.arch/riscv-tdesc-loading.exp
new file mode 100644
index 00000000000..e11117280c3
--- /dev/null
+++ b/gdb/testsuite/gdb.arch/riscv-tdesc-loading.exp
@@ -0,0 +1,39 @@
+# Copyright 2020 Free Software Foundation, Inc.
+#
+# 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/>.
+
+# Check that we can load different RISC-V target descriptions.
+
+if {![istarget "riscv*-*-*"]} {
+    verbose "Skipping ${gdb_test_file_name}."
+    return
+}
+
+clean_restart
+
+# Run over every test XML file and check the target description can be
+# loaded.
+foreach filename [lsort [glob $srcdir/$subdir/riscv-tdesc-loading-*.xml]] {
+    if {[is_remote host]} {
+       set test_path [remote_download host $filename]
+    } else {
+       set test_path $filename
+    }
+
+    # Currently it is expected that all of the target descriptions in
+    # this test will load successfully, so we expect no additonal
+    # output from GDB.
+    gdb_test_no_output "set tdesc filename $test_path" \
+	"check [file tail $filename]"
+}
-- 
2.25.4


^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH 6/8] gdb: Extend target description processing of unknown registers
  2020-06-16 17:14 [PATCH 0/8] RISC-V target description and register handling fixes Andrew Burgess
                   ` (4 preceding siblings ...)
  2020-06-16 17:14 ` [PATCH 5/8] gdb/riscv: Improve support for matching against target descriptions Andrew Burgess
@ 2020-06-16 17:14 ` Andrew Burgess
  2020-06-16 17:14 ` [PATCH 7/8] gdb/riscv: Record information about unknown tdesc registers Andrew Burgess
                   ` (3 subsequent siblings)
  9 siblings, 0 replies; 13+ messages in thread
From: Andrew Burgess @ 2020-06-16 17:14 UTC (permalink / raw)
  To: gdb-patches; +Cc: Nelson Chu, Jim Wilson, Tom Tromey, palmer, Andrew Burgess

This commit adds a new step to the processing of a target description
done in tdesc_use_registers, this new step is about how unknown
registers are processed.

Currently an architecture looks through the target description and
calls tdesc_numbered_register for each register is was expecting (or
hoping) to find.  This builds up a map from GDB's register numbers to
the tdesc_reg object.  Later the architecture calls
tdesc_use_registers.

In tdesc_use_registers we build a hash with keys being all the
tdesc_reg object pointers, from this hash we remove all of the
tdesc_reg objects that were assigned register numbers using
tdesc_numbered_register.

Finally we walk through all of the tdesc_reg objects, and if it was
not already assigned a number we assign that register the next
available number.

The problem with this is that the architecture has no visibility of
which unknown registers exist, and which tdesc_feature the register
came from, in some cases this might be important.

For example, on RISC-V GDB overrides the use of
tdesc_register_reggroup_p, with riscv_register_reggroup_p to modify
some of the register group choices.  In this function GDB wants to
treat all registers from a particular feature in a certain way.  This
is fine for registers that GDB knows might be in that feature, but for
unknown registers the RISC-V parts of GDB have no easy way to figure
out which unknown registers exist, and what numbers they were
assigned.

We could figure this information out by probing the register
structures after calling tdesc_use_registers, but this would be
horrible, much better to have tdesc_use_registers tell the
architecture about unknown registers.

This is what this commit does.  A new phase of tdesc_use_registers,
just before the unknown registers are assigned a number, we loop over
each tdesc_reg object, if it has not been assigned a number then we
figure out what number would be assigned and then call back into the
architecture passing the tdesc_feature, register name, and the
proposed register number.

The architecture is free to return the proposed register number, or it
can return a different number (which has a result identical to having
called tdesc_numbered_register).  Alternatively the architecture can
return -1 to indicate the register should be numbered later.

After calling the callback for every tdesc_reg object any registers
still don't have a number assigned (because the architecture returned
-1), then a new register number is assigned, which might be different
from the proposed number that was suggested earlier.

This commit adds the general target-description parts of this
mechanism.  No targets are currently using this code.  The RISC-V
target will make use of this in the next commit.

There should be no user visible changes after this commit.

gdb/ChangeLog:

	* target-descriptions.c (tdesc_use_registers): Add new parameter a
	callback, use the callback (when not null) to help number unknown
	registers.
	* target-descriptions.h (tdesc_unknown_register_ftype): New typedef.
	(tdesc_use_registers): Add extra parameter to declaration.
---
 gdb/ChangeLog             |  8 ++++++++
 gdb/target-descriptions.c | 31 ++++++++++++++++++++++++++++++-
 gdb/target-descriptions.h | 27 ++++++++++++++++++++++++++-
 3 files changed, 64 insertions(+), 2 deletions(-)

diff --git a/gdb/target-descriptions.c b/gdb/target-descriptions.c
index 20a3a640f4f..9a661c8487b 100644
--- a/gdb/target-descriptions.c
+++ b/gdb/target-descriptions.c
@@ -1056,7 +1056,8 @@ set_tdesc_pseudo_register_reggroup_p
 void
 tdesc_use_registers (struct gdbarch *gdbarch,
 		     const struct target_desc *target_desc,
-		     struct tdesc_arch_data *early_data)
+		     struct tdesc_arch_data *early_data,
+		     tdesc_unknown_register_ftype unk_reg_cb)
 {
   int num_regs = gdbarch_num_regs (gdbarch);
   struct tdesc_arch_data *data;
@@ -1105,6 +1106,34 @@ tdesc_use_registers (struct gdbarch *gdbarch,
   while (data->arch_regs.size () < num_regs)
     data->arch_regs.emplace_back (nullptr, nullptr);

+  /* First we give the target a chance to number previously unknown
+     registers.  This allows targets to record the numbers assigned based
+     on which feature the register was from.  */
+  if (unk_reg_cb != NULL)
+    {
+      for (const tdesc_feature_up &feature : target_desc->features)
+	for (const tdesc_reg_up &reg : feature->registers)
+	  if (htab_find (reg_hash, reg.get ()) != NULL)
+	    {
+	      int regno = unk_reg_cb (gdbarch, feature.get (),
+				      reg->name.c_str (), num_regs);
+	      gdb_assert (regno == -1 || regno >= num_regs);
+	      if (regno != -1)
+		{
+		  while (regno >= data->arch_regs.size ())
+		    data->arch_regs.emplace_back (nullptr, nullptr);
+		  data->arch_regs[regno] = tdesc_arch_reg (reg.get (), NULL);
+		  num_regs = regno + 1;
+		  htab_remove_elt (reg_hash, reg.get ());
+		}
+	    }
+    }
+
+  /* Ensure the array was sized correctly above.  */
+  gdb_assert (data->arch_regs.size () == num_regs);
+
+  /* Now in a final pass we assign register numbers to any remaining
+     unnumbered registers.  */
   for (const tdesc_feature_up &feature : target_desc->features)
     for (const tdesc_reg_up &reg : feature->registers)
       if (htab_find (reg_hash, reg.get ()) != NULL)
diff --git a/gdb/target-descriptions.h b/gdb/target-descriptions.h
index 96d283fb379..6d842bf07ed 100644
--- a/gdb/target-descriptions.h
+++ b/gdb/target-descriptions.h
@@ -80,6 +80,30 @@ void set_tdesc_pseudo_register_reggroup_p
   (struct gdbarch *gdbarch,
    gdbarch_register_reggroup_p_ftype *pseudo_reggroup_p);

+/* Pointer to a function that should be called for each unknown register in
+   a target description, used by TDESC_USE_REGISTERS.
+
+   GDBARCH is the architecture the target description is for, FEATURE is
+   the feature the unknown register is in, and REG_NAME is the name of the
+   register from the target description.  The POSSIBLE_REGNUM is a proposed
+   (GDB internal) number for this register.
+
+   The callback function can return, (-1) to indicate that the register
+   should not be assigned POSSIBLE_REGNUM now (though it might be later),
+   GDB will number the register automatically later on.  Return
+   POSSIBLE_REGNUM (or greater) to have this register assigned that number.
+   Returning a value less that POSSIBLE_REGNUM is also acceptable, but take
+   care not to clash with a register number that has already been
+   assigned.
+
+   The callback will always be called on the registers in the order they
+   appear in the target description.  This means all unknown registers
+   within a single feature will be called one after another.  */
+
+typedef int (*tdesc_unknown_register_ftype)
+	(struct gdbarch *gdbarch, tdesc_feature *feature,
+	 const char *reg_name, int possible_regnum);
+
 /* Update GDBARCH to use the TARGET_DESC for registers.  TARGET_DESC
    may be GDBARCH's target description or (if GDBARCH does not have
    one which describes registers) another target description
@@ -95,7 +119,8 @@ void set_tdesc_pseudo_register_reggroup_p

 void tdesc_use_registers (struct gdbarch *gdbarch,
 			  const struct target_desc *target_desc,
-			  struct tdesc_arch_data *early_data);
+			  struct tdesc_arch_data *early_data,
+			  tdesc_unknown_register_ftype unk_reg_cb = NULL);

 /* Allocate initial data for validation of a target description during
    gdbarch initialization.  */
-- 
2.25.4

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH 7/8] gdb/riscv: Record information about unknown tdesc registers
  2020-06-16 17:14 [PATCH 0/8] RISC-V target description and register handling fixes Andrew Burgess
                   ` (5 preceding siblings ...)
  2020-06-16 17:14 ` [PATCH 6/8] gdb: Extend target description processing of unknown registers Andrew Burgess
@ 2020-06-16 17:14 ` Andrew Burgess
  2020-06-16 17:14 ` [PATCH 8/8] gdb/riscv: Loop over all registers for 'info all-registers' Andrew Burgess
                   ` (2 subsequent siblings)
  9 siblings, 0 replies; 13+ messages in thread
From: Andrew Burgess @ 2020-06-16 17:14 UTC (permalink / raw)
  To: gdb-patches; +Cc: Nelson Chu, Jim Wilson, Tom Tromey, palmer, Andrew Burgess

Making use of the previous commit, record information about unknown
registers in the target description, and use this to resolve two
issues.

1. Some targets (QEMU) are reporting three register fflags, frm, and
   fcsr, twice, once in the FPU feature, and once in the CSR feature.
   GDB does create two registers with identical names, but this
   is (sort of) fine, we only ever use the first one, and as both
   registers access the same target state things basically work OK.
   The only real problem is that the register names show up twice in
   'info registers all' output.

   In this commit we spot the duplicates of these registers and then
   return NULL when asked for the name of these registers.  This
   causes GDB to hide these registers from the user, fixing this
   problem.

2. Some targets (QEMU) advertise CSRs that GDB then can't read.  The
   problem is these targets also say these CSRs are part of the
   save/restore register groups.

   This means that before an inferior call GDB tries to save all of
   these CSRs, and a failure to read one causes the inferior call to
   be abandoned.

   We already work around this issue to some degree, known CSRs are
   removed from the save/restore groups, despite what the target might
   say.  However, any unknown CSRs are (currently) not removed in this
   way.

   After this commit we keep a log of the register numbers for all
   unknown CSRs, then when asked about the register groups, we
   override the group information for unknown CSRs, removing them from
   the save and restore groups.

gdb/ChangeLog:

	* riscv-tdep.c (riscv_register_name): Return NULL for duplicate
	fflags, frm, and fcsr registers.
	(riscv_register_reggroup_p): Remove unknown CSRs from save and
	restore groups.
	(riscv_tdesc_unknown_reg): New function.
	(riscv_gdbarch_init): Pass riscv_tdesc_unknown_reg to
	tdesc_use_registers.
	* riscv-tdep.h (struct gdbarch_tdep): Add
	unknown_csrs_first_regnum, unknown_csrs_count,
	duplicate_fflags_regnum, duplicate_frm_regnum, and
	duplicate_fcsr_regnum fields.

gdb/testsuite/ChangeLog:

	* gdb.arch/riscv-tdesc-regs.exp: Extend test case.
---
 gdb/ChangeLog                               |  14 +++
 gdb/riscv-tdep.c                            | 110 +++++++++++++++++++-
 gdb/riscv-tdep.h                            |  15 +++
 gdb/testsuite/ChangeLog                     |   4 +
 gdb/testsuite/gdb.arch/riscv-tdesc-regs.exp |   5 +-
 5 files changed, 145 insertions(+), 3 deletions(-)

diff --git a/gdb/riscv-tdep.c b/gdb/riscv-tdep.c
index f161c1ac625..37ea0ee81a4 100644
--- a/gdb/riscv-tdep.c
+++ b/gdb/riscv-tdep.c
@@ -617,6 +617,22 @@ riscv_register_name (struct gdbarch *gdbarch, int regnum)
         return NULL;
     }
 
+  /* Some targets (QEMU) are reporting these three registers twice, once
+     in the FPU feature, and once in the CSR feature.  Both of these read
+     the same underlying state inside the target, but naming the register
+     twice in the target description results in GDB having two registers
+     with the same name, only one of which can ever be accessed, but both
+     will show up in 'info register all'.  Unless, we identify the
+     duplicate copies of these registers (in riscv_tdesc_unknown_reg) and
+     then hide the registers here by giving them no name.  */
+  struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
+  if (tdep->duplicate_fflags_regnum == regnum)
+    return NULL;
+  if (tdep->duplicate_frm_regnum == regnum)
+    return NULL;
+  if (tdep->duplicate_fcsr_regnum == regnum)
+    return NULL;
+
   /* The remaining registers are different.  For all other registers on the
      machine we prefer to see the names that the target description
      provides.  This is particularly important for CSRs which might be
@@ -968,6 +984,19 @@ riscv_register_reggroup_p (struct gdbarch  *gdbarch, int regnum,
 
   if (regnum > RISCV_LAST_REGNUM)
     {
+      /* Any extra registers from the CSR tdesc_feature (identified in
+	 riscv_tdesc_unknown_reg) are removed from the save/restore groups
+	 as some targets (QEMU) report CSRs which then can't be read.  */
+      struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
+      if ((reggroup == restore_reggroup || reggroup == save_reggroup)
+	  && regnum >= tdep->unknown_csrs_first_regnum
+	  && regnum < (tdep->unknown_csrs_first_regnum
+		       + tdep->unknown_csrs_count))
+	return 0;
+
+      /* This is some other unknown register from the target description.
+	 In this case we trust whatever the target description says about
+	 which groups this register should be in.  */
       int ret = tdesc_register_in_reggroup_p (gdbarch, regnum, reggroup);
       if (ret != -1)
         return ret;
@@ -3166,6 +3195,85 @@ riscv_gcc_target_options (struct gdbarch *gdbarch)
   return target_options;
 }
 
+/* Call back from tdesc_use_registers, called for each unknown register
+   found in the target description.
+
+   See target-description.h (typedef tdesc_unknown_register_ftype) for a
+   discussion of the arguments and return values.  */
+
+static int
+riscv_tdesc_unknown_reg (struct gdbarch *gdbarch, tdesc_feature *feature,
+			 const char *reg_name, int possible_regnum)
+{
+  /* At one point in time GDB had an incorrect default target description
+     that duplicated the fflags, frm, and fcsr registers in both the FPU
+     and CSR register sets.
+
+     Some targets (QEMU) copied these target descriptions into their source
+     tree, and so we're currently stuck working with some targets that
+     declare the same registers twice.
+
+     There's not much we can do about this any more.  Assuming the target
+     will direct a request for either register number to the correct
+     underlying hardware register then it doesn't matter which one GDB
+     uses, so long as we (GDB) are consistent (so that we don't end up with
+     invalid cache misses).
+
+     As we always scan the FPU registers first, then the CSRs, if the
+     target has included the offending registers in both sets then we will
+     always see the FPU copies here, as the CSR versions will replace them
+     in the register list.
+
+     To prevent these duplicates showing up in any of the register list,
+     record their register numbers here.  */
+  if (strcmp (tdesc_feature_name (feature), riscv_freg_feature.name) == 0)
+    {
+      struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
+      int *regnum_ptr = nullptr;
+
+      if (strcmp (reg_name, "fflags") == 0)
+	regnum_ptr = &tdep->duplicate_fflags_regnum;
+      else if (strcmp (reg_name, "frm") == 0)
+	regnum_ptr = &tdep->duplicate_frm_regnum;
+      else if (strcmp (reg_name, "fcsr") == 0)
+	regnum_ptr = &tdep->duplicate_fcsr_regnum;
+
+      if (regnum_ptr != nullptr)
+	{
+	  /* This means the register appears more than twice in the target
+	     description.  Just let GDB add this as another register.
+	     We'll have duplicates in the register name list, but there's
+	     not much more we can do.  */
+	  if (*regnum_ptr != -1)
+	    return -1;
+
+	  /* Record the number assigned to this register, then return the
+	     number (so it actually gets assigned to this register).  */
+	  *regnum_ptr = possible_regnum;
+	  return possible_regnum;
+	}
+    }
+
+  /* Any unknown registers in the CSR feature are recorded within a single
+     block so we can easily identify these registers when making choices
+     about register groups in riscv_register_reggroup_p.  */
+  if (strcmp (tdesc_feature_name (feature), riscv_csr_feature.name) == 0)
+    {
+      struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
+      if (tdep->unknown_csrs_first_regnum == -1)
+	tdep->unknown_csrs_first_regnum = possible_regnum;
+      gdb_assert (tdep->unknown_csrs_first_regnum
+		  + tdep->unknown_csrs_count == possible_regnum);
+      tdep->unknown_csrs_count++;
+      return possible_regnum;
+    }
+
+  /* Some other unknown register.  Don't assign this a number now, it will
+     be assigned a number automatically later by the target description
+     handling code.  */
+  return -1;
+}
+
 /* Implement the gnu_triplet_regexp method.  A single compiler supports both
    32-bit and 64-bit code, and may be named riscv32 or riscv64 or (not
    recommended) riscv.  */
@@ -3403,7 +3511,7 @@ riscv_gdbarch_init (struct gdbarch_info info,
   set_gdbarch_print_registers_info (gdbarch, riscv_print_registers_info);
 
   /* Finalise the target description registers.  */
-  tdesc_use_registers (gdbarch, tdesc, tdesc_data);
+  tdesc_use_registers (gdbarch, tdesc, tdesc_data, riscv_tdesc_unknown_reg);
 
   /* Override the register type callback setup by the target description
      mechanism.  This allows us to provide special type for floating point
diff --git a/gdb/riscv-tdep.h b/gdb/riscv-tdep.h
index e415fb4a7a1..0ff555b0632 100644
--- a/gdb/riscv-tdep.h
+++ b/gdb/riscv-tdep.h
@@ -79,6 +79,21 @@ struct gdbarch_tdep
 
   /* ISA-specific data types.  */
   struct type *riscv_fpreg_d_type = nullptr;
+
+  /* Use for tracking unknown CSRs in the target description.
+     UNKNOWN_CSRS_FIRST_REGNUM is the number assigned to the first unknown
+     CSR.  All other unknown CSRs will be assigned sequential numbers after
+     this, with UNKNOWN_CSRS_COUNT being the total number of unknown CSRs.  */
+  int unknown_csrs_first_regnum = -1;
+  int unknown_csrs_count = 0;
+
+  /* Some targets (QEMU) are reporting three registers twice in the target
+     description they send.  These three register numbers, when not set to
+     -1, are for the duplicate copies of these registers.  */
+  int duplicate_fflags_regnum = -1;
+  int duplicate_frm_regnum = -1;
+  int duplicate_fcsr_regnum = -1;
+
 };
 
 
diff --git a/gdb/testsuite/gdb.arch/riscv-tdesc-regs.exp b/gdb/testsuite/gdb.arch/riscv-tdesc-regs.exp
index 63ac8fb7abc..9feddbad074 100644
--- a/gdb/testsuite/gdb.arch/riscv-tdesc-regs.exp
+++ b/gdb/testsuite/gdb.arch/riscv-tdesc-regs.exp
@@ -100,13 +100,14 @@ foreach rgroup {all save restore} {
 	}
     }
 
-    foreach reg {dscratch} {
+    foreach reg {fflags frm fcsr unknown_csr dscratch} {
 	if { [info exists reg_counts($reg) ] } {
 	    set count $reg_counts($reg)
 	} else {
 	    set count 0
 	}
-	if {$reg == "dscratch" && $rgroup != "all"} {
+	if {($reg == "unknown_csr" || $reg == "dscratch") \
+		&& $rgroup != "all"} {
 	    gdb_assert {$count == 0} \
 		"register $reg not seen in reggroup $rgroup"
 	} else {
-- 
2.25.4


^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH 8/8] gdb/riscv: Loop over all registers for 'info all-registers'
  2020-06-16 17:14 [PATCH 0/8] RISC-V target description and register handling fixes Andrew Burgess
                   ` (6 preceding siblings ...)
  2020-06-16 17:14 ` [PATCH 7/8] gdb/riscv: Record information about unknown tdesc registers Andrew Burgess
@ 2020-06-16 17:14 ` Andrew Burgess
  2020-06-17  1:31 ` [PATCH 0/8] RISC-V target description and register handling fixes Nelson Chu
  2020-06-18 20:45 ` Tom Tromey
  9 siblings, 0 replies; 13+ messages in thread
From: Andrew Burgess @ 2020-06-16 17:14 UTC (permalink / raw)
  To: gdb-patches; +Cc: Nelson Chu, Jim Wilson, Tom Tromey, palmer, Andrew Burgess

Currently the 'info all-registers' command only loops over those
registers that are known to GDB.  Any registers that are unknown, that
is, are mentioned in the target description, but are not something GDB
otherwise knows, will not be displayed.

This feels wrong, so this commit fixes this mistake.  The output of
'info all-registers' now matches 'info registers all'.

gdb/ChangeLog:

	* riscv-tdep.c (riscv_print_registers_info): Loop over all
	registers, not just the known core set of registers.

gdb/testsuite/ChangeLog:

	* gdb.arch/riscv-tdesc-regs.exp: New test cases.
---
 gdb/ChangeLog                               |  5 +++++
 gdb/riscv-tdep.c                            |  2 +-
 gdb/testsuite/ChangeLog                     |  4 ++++
 gdb/testsuite/gdb.arch/riscv-tdesc-regs.exp | 10 +++++++---
 4 files changed, 17 insertions(+), 4 deletions(-)

diff --git a/gdb/riscv-tdep.c b/gdb/riscv-tdep.c
index 37ea0ee81a4..0028572d844 100644
--- a/gdb/riscv-tdep.c
+++ b/gdb/riscv-tdep.c
@@ -1071,7 +1071,7 @@ riscv_print_registers_info (struct gdbarch *gdbarch,
       else
 	reggroup = general_reggroup;
 
-      for (regnum = 0; regnum <= RISCV_LAST_REGNUM; ++regnum)
+      for (regnum = 0; regnum < gdbarch_num_cooked_regs (gdbarch); ++regnum)
 	{
 	  /* Zero never changes, so might as well hide by default.  */
 	  if (regnum == RISCV_ZERO_REGNUM && !print_all)
diff --git a/gdb/testsuite/gdb.arch/riscv-tdesc-regs.exp b/gdb/testsuite/gdb.arch/riscv-tdesc-regs.exp
index 9feddbad074..1be32e0e8a1 100644
--- a/gdb/testsuite/gdb.arch/riscv-tdesc-regs.exp
+++ b/gdb/testsuite/gdb.arch/riscv-tdesc-regs.exp
@@ -80,11 +80,15 @@ gdb_test "info registers \$csr0" "Invalid register `csr0'"
 gdb_test "info registers \$dscratch0" "dscratch0\[ \t\]+.*"
 gdb_test "info registers \$dscratch" "dscratch\[ \t\]+.*"
 
-foreach rgroup {all save restore} {
+foreach rgroup {x_all all save restore} {
     # Now use 'info registers all' to see how many times the floating
     # point status registers show up in the output.
     array set reg_counts {}
-    set test "info registers $rgroup"
+    if {$rgroup == "x_all"} {
+	set test "info all-registers"
+    } else {
+	set test "info registers $rgroup"
+    }
     gdb_test_multiple $test $test {
 	-re ".*info registers all\r\n" {
 	    verbose -log "Skip to first register"
@@ -107,7 +111,7 @@ foreach rgroup {all save restore} {
 	    set count 0
 	}
 	if {($reg == "unknown_csr" || $reg == "dscratch") \
-		&& $rgroup != "all"} {
+		&& $rgroup != "all" && $rgroup != "x_all"} {
 	    gdb_assert {$count == 0} \
 		"register $reg not seen in reggroup $rgroup"
 	} else {
-- 
2.25.4


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 0/8] RISC-V target description and register handling fixes
  2020-06-16 17:14 [PATCH 0/8] RISC-V target description and register handling fixes Andrew Burgess
                   ` (7 preceding siblings ...)
  2020-06-16 17:14 ` [PATCH 8/8] gdb/riscv: Loop over all registers for 'info all-registers' Andrew Burgess
@ 2020-06-17  1:31 ` Nelson Chu
  2020-06-18 20:45 ` Tom Tromey
  9 siblings, 0 replies; 13+ messages in thread
From: Nelson Chu @ 2020-06-17  1:31 UTC (permalink / raw)
  To: Andrew Burgess; +Cc: gdb-patches, Jim Wilson, Tom Tromey, Palmer Dabbelt

Hi Andrew,

These are great!  Thank you, Jim and Tom very much.  You make the CSR
support more sense now.  The code seems to be changed dramatically.  I
will spend time to figure them out, and then see what needs to be
modified in binutils, including my recent patch to support the missing
debug CSR.

Thanks
Nelson

On Wed, Jun 17, 2020 at 1:14 AM Andrew Burgess
<andrew.burgess@embecosm.com> wrote:
>
> This series includes a mixed bag of changes around RISC-V's target
> description and general register handling.  This includes a fix for an
> issue Tom reported here:
>
>   https://sourceware.org/pipermail/gdb-patches/2020-June/169323.html
>
> Though most of the changes are RISC-V only, patch #6 does make a
> change to the generic target description handling code so is probably
> worth additional review.
>
> Feedback welcome,
>
> Thanks,
> Andrew
>
> ---
>
> Andrew Burgess (8):
>   gdb/riscv: Improved register alias name creation
>   gdb/riscv: Fix whitespace error
>   gdb/riscv: Take CSR names from target description
>   gdb/riscv: Remove CSR feature file
>   gdb/riscv: Improve support for matching against target descriptions
>   gdb: Extend target description processing of unknown registers
>   gdb/riscv: Record information about unknown tdesc registers
>   gdb/riscv: Loop over all registers for 'info all-registers'

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 0/8] RISC-V target description and register handling fixes
  2020-06-16 17:14 [PATCH 0/8] RISC-V target description and register handling fixes Andrew Burgess
                   ` (8 preceding siblings ...)
  2020-06-17  1:31 ` [PATCH 0/8] RISC-V target description and register handling fixes Nelson Chu
@ 2020-06-18 20:45 ` Tom Tromey
  2020-06-18 20:54   ` Andrew Burgess
  9 siblings, 1 reply; 13+ messages in thread
From: Tom Tromey @ 2020-06-18 20:45 UTC (permalink / raw)
  To: Andrew Burgess; +Cc: gdb-patches, Tom Tromey

>>>>> "Andrew" == Andrew Burgess <andrew.burgess@embecosm.com> writes:

Andrew> This series includes a mixed bag of changes around RISC-V's target
Andrew> description and general register handling.  This includes a fix for an
Andrew> issue Tom reported here:

Andrew>   https://sourceware.org/pipermail/gdb-patches/2020-June/169323.html

Andrew> Though most of the changes are RISC-V only, patch #6 does make a
Andrew> change to the generic target description handling code so is probably
Andrew> worth additional review.

Thank you for doing this.

I read through this and sent a nit.  Seeing as it mostly touches RISC-V
code, I didn't have much more to add.  I think it all seems fine.

Tom

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 0/8] RISC-V target description and register handling fixes
  2020-06-18 20:45 ` Tom Tromey
@ 2020-06-18 20:54   ` Andrew Burgess
  0 siblings, 0 replies; 13+ messages in thread
From: Andrew Burgess @ 2020-06-18 20:54 UTC (permalink / raw)
  To: Tom Tromey; +Cc: gdb-patches

* Tom Tromey <tom@tromey.com> [2020-06-18 14:45:40 -0600]:

> >>>>> "Andrew" == Andrew Burgess <andrew.burgess@embecosm.com> writes:
> 
> Andrew> This series includes a mixed bag of changes around RISC-V's target
> Andrew> description and general register handling.  This includes a fix for an
> Andrew> issue Tom reported here:
> 
> Andrew>   https://sourceware.org/pipermail/gdb-patches/2020-June/169323.html
> 
> Andrew> Though most of the changes are RISC-V only, patch #6 does make a
> Andrew> change to the generic target description handling code so is probably
> Andrew> worth additional review.
> 
> Thank you for doing this.
> 
> I read through this and sent a nit.  Seeing as it mostly touches RISC-V
> code, I didn't have much more to add.  I think it all seems fine.

Thanks for your feedback.

I'll fix the nit before merging some time next week - I want to make
sure any other RISC-V folk have had a chance to comment if they wish.

Thanks,
Andrew

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2020-06-18 20:54 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-06-16 17:14 [PATCH 0/8] RISC-V target description and register handling fixes Andrew Burgess
2020-06-16 17:14 ` [PATCH 1/8] gdb/riscv: Improved register alias name creation Andrew Burgess
2020-06-18 20:36   ` Tom Tromey
2020-06-16 17:14 ` [PATCH 2/8] gdb/riscv: Fix whitespace error Andrew Burgess
2020-06-16 17:14 ` [PATCH 3/8] gdb/riscv: Take CSR names from target description Andrew Burgess
2020-06-16 17:14 ` [PATCH 4/8] gdb/riscv: Remove CSR feature file Andrew Burgess
2020-06-16 17:14 ` [PATCH 5/8] gdb/riscv: Improve support for matching against target descriptions Andrew Burgess
2020-06-16 17:14 ` [PATCH 6/8] gdb: Extend target description processing of unknown registers Andrew Burgess
2020-06-16 17:14 ` [PATCH 7/8] gdb/riscv: Record information about unknown tdesc registers Andrew Burgess
2020-06-16 17:14 ` [PATCH 8/8] gdb/riscv: Loop over all registers for 'info all-registers' Andrew Burgess
2020-06-17  1:31 ` [PATCH 0/8] RISC-V target description and register handling fixes Nelson Chu
2020-06-18 20:45 ` Tom Tromey
2020-06-18 20:54   ` Andrew Burgess

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).