public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH 0/3] RISC-V fflags, frm, and fcsr (includes small tdesc change)
@ 2022-08-16 10:09 Andrew Burgess
  2022-08-16 10:09 ` [PATCH 1/3] gdb/riscv: improve (and fix) display of frm field in 'info registers' Andrew Burgess
                   ` (4 more replies)
  0 siblings, 5 replies; 7+ messages in thread
From: Andrew Burgess @ 2022-08-16 10:09 UTC (permalink / raw)
  To: gdb-patches; +Cc: Andrew Burgess

While testing on a RISC-V native Linux target, I noticed some issues
accessing the fflags and frm registers.  This series addresses these issues.

Patch #2 is a small extension to the target description mechanism
required to support patch #3.

Thanks,
Andrew

---

Andrew Burgess (3):
  gdb/riscv: improve (and fix) display of frm field in 'info registers'
  gdb: Add tdesc_found_register function to tdesc API
  gdb/riscv: better support for fflags and frm registers

 gdb/arch/riscv.h                              |  19 +-
 gdb/features/riscv/32bit-fpu.c                |   4 +-
 gdb/features/riscv/32bit-fpu.xml              |   2 -
 gdb/features/riscv/64bit-fpu.c                |   4 +-
 gdb/features/riscv/64bit-fpu.xml              |   2 -
 gdb/riscv-tdep.c                              | 236 +++++++++++++++---
 gdb/riscv-tdep.h                              |   6 +
 gdb/target-descriptions.c                     |  11 +
 gdb/target-descriptions.h                     |   4 +
 gdb/testsuite/gdb.arch/riscv-info-fcsr.c      |  22 ++
 gdb/testsuite/gdb.arch/riscv-info-fcsr.exp    | 149 +++++++++++
 .../gdb.arch/riscv-tdesc-fcsr-32.xml          |  75 ++++++
 .../gdb.arch/riscv-tdesc-fcsr-64.xml          |  79 ++++++
 .../gdb.arch/riscv-tdesc-loading-05.xml       |  77 ++++++
 .../gdb.arch/riscv-tdesc-loading-06.xml       |  75 ++++++
 gdb/testsuite/gdb.arch/riscv-tdesc-regs.exp   |  36 +++
 16 files changed, 755 insertions(+), 46 deletions(-)
 create mode 100644 gdb/testsuite/gdb.arch/riscv-info-fcsr.c
 create mode 100644 gdb/testsuite/gdb.arch/riscv-info-fcsr.exp
 create mode 100644 gdb/testsuite/gdb.arch/riscv-tdesc-fcsr-32.xml
 create mode 100644 gdb/testsuite/gdb.arch/riscv-tdesc-fcsr-64.xml
 create mode 100644 gdb/testsuite/gdb.arch/riscv-tdesc-loading-05.xml
 create mode 100644 gdb/testsuite/gdb.arch/riscv-tdesc-loading-06.xml

-- 
2.25.4


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

* [PATCH 1/3] gdb/riscv: improve (and fix) display of frm field in 'info registers'
  2022-08-16 10:09 [PATCH 0/3] RISC-V fflags, frm, and fcsr (includes small tdesc change) Andrew Burgess
@ 2022-08-16 10:09 ` Andrew Burgess
  2022-08-16 10:09 ` [PATCH 2/3] gdb: Add tdesc_found_register function to tdesc API Andrew Burgess
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 7+ messages in thread
From: Andrew Burgess @ 2022-08-16 10:09 UTC (permalink / raw)
  To: gdb-patches; +Cc: Andrew Burgess

On RISC-V the FCSR (float control/status register) is split into two
parts, FFLAGS (the flags) and FRM (the rounding mode).  Both of these
two fields are part of the FCSR register, but can also be accessed as
separate registers in their own right.  And so, we have three separate
registers, $fflags, $frm, and $fcsr, with the last of these being the
combination of the first two.

Here's how the bits of FCSR are split between FRM and FFLAGS:

         ,--------- FFLAGS
       |---|
    76543210 <----- FCSR
    |-|
     '--------------FRM

Here's how GDB currently displays these registers:

  (gdb) info registers $fflags $frm $fcsr
  fflags         0x0      RD:0 NV:0 DZ:0 OF:0 UF:0 NX:0
  frm            0x0      FRM:0 [RNE (round to nearest; ties to even)]
  fcsr           0x0      RD:0 NV:0 DZ:0 OF:0 UF:0 NX:0 FRM:0 [RNE (round to nearest; ties to even)]

Notice the 'RD' field which is present in both $fflags and $fcsr.
This field contains the value of the FRM field, which makes sense when
displaying the $fcsr, but makes no sense when displaying $fflags, as
the $fflags doesn't include the FRM field.

Additionally, the $fcsr already includes an FRM field, so the
information in 'RD' is duplicated.  Consider this:

  (gdb) set $frm = 0x3
  (gdb) info registers $fflags $frm $fcsr                             │
  fflags         0x0      RD:0 NV:0 DZ:0 OF:0 UF:0 NX:0
  frm            0x3      FRM:3 [RUP (Round up towards +INF)]
  fcsr           0x60     RD:3 NV:0 DZ:0 OF:0 UF:0 NX:0 FRM:3 [RUP (Round up towards +INF)]

See how the 'RD' field in $fflags still displays 0, while the 'RD' and
'FRM' fields in $fcsr show the same information.

The first change I propose in this commit is to remove the 'RD'
field.  After this change the output now looks like this:

  (gdb) info registers $fflags $frm $fcsr
  fflags         0x0      NV:0 DZ:0 OF:0 UF:0 NX:0
  frm            0x0      FRM:0 [RNE (round to nearest; ties to even)]
  fcsr           0x0      NV:0 DZ:0 OF:0 UF:0 NX:0 FRM:0 [RNE (round to nearest; ties to even)]

Next, I spotted that the text that goes along with the 'FRM' field was
not wrapped in the i18n markers for internationalisation, so I added
those.

Next, I spotted that:

  (gdb) set $frm=0x7
  (gdb) info registers $fflags $frm $fcsr
  fflags         0x0      RD:0 NV:0 DZ:0 OF:0 UF:0 NX:0
  frm            0x7      FRM:3 [RUP (Round up towards +INF)]
  fcsr           0xe0     RD:7 NV:0 DZ:0 OF:0 UF:0 NX:0 FRM:3 [RUP (Round up towards +INF)]

Notice that despite being a 3-bit field, FRM masks to 2-bits.
Checking the manual I can see that the FRM field is 3-bits, and is
defined for all 8 values.  That GDB masks to 2-bits is just a bug I
think, so I've fixed this.

Finally, the 'FRM' text for value 0x7 is wrong.  Currently we use the
text 'dynamic rounding mode' for value 0x7.  However, this is not
really correct.

A RISC-V instruction can either encode the rounding mode within the
instruction, or a RISC-V instruction can choose to use a global,
dynamic rounding mode.

So, for the rounding-mode field of an _instruction_ the value 0x7
indicates "dynamic round mode", the instruction should defer to the
rounding mode held in the FRM field of the $fcsr.

But it makes no sense for the FRM of $fcsr to itself be set to
0x7 (dynamic rounding mode), and indeed, section 11.2, "Floating-Point
Control and Status Register" of the RISC-V manual, says that a value
of 0x7 in the $fcsr FRM field is invalid, and if an instruction has
_its_ round-mode set to dynamic, and the FRM field is also set to 0x7,
then an illegal instruction exception is raised.

And so, I propose changing the text for value 0x7 of the FRM field to
be "INVALID[7] (Dynamic rounding mode)".  We already use the text
"INVALID[5]" and "INVALID[6]" for the two other invalid fields,
however, I think adding the extra "Dynamic round mode" hint might be
helpful.

I've added a new test that uses 'info registers' to check what GDB
prints for the three registers related to this patch.  There is one
slight oddity with this test - for the fflags and frm registers, the
test accepts both the "normal" output (as described above), but also
allows these registers to be reported as '<unavailable>'.

The reason why I accept <unavailable> is that currently, the RISC-V,
native Linux target advertises these registers in its target
description, but then doesn't support reading or writing of these
registers, this results in the registers being reported as
unavailable.

A later patch in this series will address this issue, and will remove
this check for <unavailable>.
---
 gdb/riscv-tdep.c                           |  24 ++--
 gdb/testsuite/gdb.arch/riscv-info-fcsr.c   |  22 +++
 gdb/testsuite/gdb.arch/riscv-info-fcsr.exp | 147 +++++++++++++++++++++
 3 files changed, 182 insertions(+), 11 deletions(-)
 create mode 100644 gdb/testsuite/gdb.arch/riscv-info-fcsr.c
 create mode 100644 gdb/testsuite/gdb.arch/riscv-info-fcsr.exp

diff --git a/gdb/riscv-tdep.c b/gdb/riscv-tdep.c
index 9ec430d8a10..93ee597af58 100644
--- a/gdb/riscv-tdep.c
+++ b/gdb/riscv-tdep.c
@@ -1172,8 +1172,7 @@ riscv_print_one_register_info (struct gdbarch *gdbarch,
 	      gdb_printf (file, "\t");
 	      if (regnum != RISCV_CSR_FRM_REGNUM)
 		gdb_printf (file,
-			    "RD:%01X NV:%d DZ:%d OF:%d UF:%d NX:%d",
-			    (int) ((d >> 5) & 0x7),
+			    "NV:%d DZ:%d OF:%d UF:%d NX:%d",
 			    (int) ((d >> 4) & 0x1),
 			    (int) ((d >> 3) & 0x1),
 			    (int) ((d >> 2) & 0x1),
@@ -1184,17 +1183,20 @@ riscv_print_one_register_info (struct gdbarch *gdbarch,
 		{
 		  static const char * const sfrm[] =
 		    {
-		      "RNE (round to nearest; ties to even)",
-		      "RTZ (Round towards zero)",
-		      "RDN (Round down towards -INF)",
-		      "RUP (Round up towards +INF)",
-		      "RMM (Round to nearest; ties to max magnitude)",
-		      "INVALID[5]",
-		      "INVALID[6]",
-		      "dynamic rounding mode",
+		      _("RNE (round to nearest; ties to even)"),
+		      _("RTZ (Round towards zero)"),
+		      _("RDN (Round down towards -INF)"),
+		      _("RUP (Round up towards +INF)"),
+		      _("RMM (Round to nearest; ties to max magnitude)"),
+		      _("INVALID[5]"),
+		      _("INVALID[6]"),
+		      /* A value of 0x7 indicates dynamic rounding mode when
+			 used within an instructions rounding-mode field, but
+			 is invalid within the FRM register.  */
+		      _("INVALID[7] (Dynamic rounding mode)"),
 		    };
 		  int frm = ((regnum == RISCV_CSR_FCSR_REGNUM)
-			     ? (d >> 5) : d) & 0x3;
+			     ? (d >> 5) : d) & 0x7;
 
 		  gdb_printf (file, "%sFRM:%i [%s]",
 			      (regnum == RISCV_CSR_FCSR_REGNUM
diff --git a/gdb/testsuite/gdb.arch/riscv-info-fcsr.c b/gdb/testsuite/gdb.arch/riscv-info-fcsr.c
new file mode 100644
index 00000000000..034c190692a
--- /dev/null
+++ b/gdb/testsuite/gdb.arch/riscv-info-fcsr.c
@@ -0,0 +1,22 @@
+/* This file is part of GDB, the GNU debugger.
+
+   Copyright 2022 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 (void)
+{
+  return 0;
+}
diff --git a/gdb/testsuite/gdb.arch/riscv-info-fcsr.exp b/gdb/testsuite/gdb.arch/riscv-info-fcsr.exp
new file mode 100644
index 00000000000..3d6095d7e55
--- /dev/null
+++ b/gdb/testsuite/gdb.arch/riscv-info-fcsr.exp
@@ -0,0 +1,147 @@
+# Copyright 2022 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 the formatting of the fcsr, fflags, and frm registers in the
+# output of the 'info registers' command.
+
+if {![istarget "riscv*-*-*"]} {
+    verbose "Skipping ${gdb_test_file_name}."
+    return
+}
+
+if { [gdb_skip_float_test] } {
+    untested "no floating point support"
+    return
+}
+
+standard_testfile
+
+if {[prepare_for_testing "failed to prepare" $testfile $srcfile debug]} {
+    return -1
+}
+
+if ![runto_main] then {
+   return 0
+}
+
+# Merge FFLAGS_VALUE and FRM_VALUE into a single hexadecimal value
+# that can be written to the fcsr register.  The two arguments should
+# be the value of each of the two fields within the fcsr register.
+proc merge_fflags_and_frm { fflags_value frm_value } {
+    set fcsr_value 0x[format %x [expr $fflags_value | ($frm_value << 5)]]
+    return $fcsr_value
+}
+
+# Use 'info registers' to check the current values of the fflags, frm,
+# and fcsr registers.  The value in fcsr should consist of the
+# FFLAGS_VALUE and FRM_VALUE, and the frm field of the fcsr register
+# should have the text FRM_STRING associated with it.
+proc check_fcsr { fflags_value frm_value frm_string } {
+    # Merge fflags and frm values into a single fcsr value.
+    set fcsr_value [merge_fflags_and_frm $fflags_value $frm_value]
+
+    # Build up all the patterns we will need for this test.
+    set frm_str_re [string_to_regexp "$frm_string"]
+    set frm_val_re [format %d ${frm_value}]
+
+    set nv [format %d [expr ($fflags_value >> 4) & 0x1]]
+    set dz [format %d [expr ($fflags_value >> 3) & 0x1]]
+    set of [format %d [expr ($fflags_value >> 2) & 0x1]]
+    set uf [format %d [expr ($fflags_value >> 1) & 0x1]]
+    set nx [format %d [expr ($fflags_value >> 0) & 0x1]]
+
+    set fflags_pattern "NV:${nv} DZ:${dz} OF:${of} UF:${uf} NX:${nx}"
+    set frm_pattern "FRM:${frm_val_re} \\\[${frm_str_re}\\\]"
+    set fcsr_pattern "${fflags_pattern} ${frm_pattern}"
+
+    # Now use 'info registers' to check the register values.
+    array set reg_counts {}
+    gdb_test_multiple "info registers \$fflags \$frm \$fcsr" "" {
+	-re "^info registers\[^\r\n\]+\r\n" {
+	    exp_continue
+	}
+
+	-re "^(fflags\|frm)\\s+<unavailable>\r\n" {
+	    # Currently, on some targets (e.g. RISC-V native Linux) the
+	    # fflags and frm registers show as being available, but are
+	    # unreadable, the result is these registers report
+	    # themselves as <unavailable>.  So long as fcsr is readable
+	    # (which is checked below), then for now we accept this.
+	    set reg_name $expect_out(1,string)
+	    incr reg_counts($reg_name)
+	    exp_continue
+	}
+
+	-re "^(frm)\\s+${frm_value}\\s+${frm_pattern}\r\n" {
+	    set reg_name $expect_out(1,string)
+	    incr reg_counts($reg_name)
+	    exp_continue
+	}
+
+	-re "^(fflags)\\s+${fflags_value}\\s+${fflags_pattern}\r\n" {
+	    set reg_name $expect_out(1,string)
+	    incr reg_counts($reg_name)
+	    exp_continue
+	}
+
+	-re "^(fcsr)\\s+${fcsr_value}\\s+${fcsr_pattern}\r\n" {
+	    set reg_name $expect_out(1,string)
+	    incr reg_counts($reg_name)
+	    exp_continue
+	}
+
+	-re "^$::gdb_prompt $" {
+	    pass $gdb_test_name
+	}
+    }
+
+    # Check that each register is seen only once.
+    foreach reg {fflags frm fcsr} {
+	gdb_assert { $reg_counts($reg) == 1 } \
+	    "check we saw $reg just once"
+    }
+}
+
+# Set the fcsr register based on FFLAGS_VALUE and FRM_VALUE, then
+# check that the value is displayed correctly in the 'info registers'
+# output.  FRM_STRING should appear in the 'info registers' output
+# next to the frm field.
+proc test_fcsr { fflags_value frm_value frm_string } {
+    # Merge fflags and frm values into a single fcsr value.
+    set fcsr_value [merge_fflags_and_frm $fflags_value $frm_value]
+
+    with_test_prefix "fcsr=${fcsr_value}" {
+	# Set the fcsr value directly.
+	gdb_test_no_output "set \$fcsr = ${fcsr_value}"
+
+	with_test_prefix "set through fcsr" {
+	    check_fcsr $fflags_value $frm_value $frm_string
+	}
+    }
+}
+
+# Check each valid value of the fflags register.
+for { set i 0 } { $i < 32 } { incr i } {
+    test_fcsr 0x[format %x $i] 0x0 "RNE (round to nearest; ties to even)"
+}
+
+# Check each valid value of the frm register.
+test_fcsr 0x0 0x1 "RTZ (Round towards zero)"
+test_fcsr 0x0 0x2 "RDN (Round down towards -INF)"
+test_fcsr 0x0 0x3 "RUP (Round up towards +INF)"
+test_fcsr 0x0 0x4 "RMM (Round to nearest; ties to max magnitude)"
+test_fcsr 0x0 0x5 "INVALID\[5\]"
+test_fcsr 0x0 0x6 "INVALID\[6\]"
+test_fcsr 0x0 0x7 "INVALID\[7\] (Dynamic rounding mode)"
-- 
2.25.4


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

* [PATCH 2/3] gdb: Add tdesc_found_register function to tdesc API
  2022-08-16 10:09 [PATCH 0/3] RISC-V fflags, frm, and fcsr (includes small tdesc change) Andrew Burgess
  2022-08-16 10:09 ` [PATCH 1/3] gdb/riscv: improve (and fix) display of frm field in 'info registers' Andrew Burgess
@ 2022-08-16 10:09 ` Andrew Burgess
  2022-08-16 10:09 ` [PATCH 3/3] gdb/riscv: better support for fflags and frm registers Andrew Burgess
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 7+ messages in thread
From: Andrew Burgess @ 2022-08-16 10:09 UTC (permalink / raw)
  To: gdb-patches; +Cc: Andrew Burgess

This commit adds a new function to the target description API within
GDB.  This new function is not used in this commit, but will be used
in the next commit, I'm splitting it out into a separate patch for
easier review.

What I want to do in the next commit is check to see if a target
description supplied a particular register, however, the register in
question could appear in one of two possible features.

The new function allows me to ask the tdesc_arch_data whether a
register was found and assigned a particular GDB register number once
all of the features have been checked.  I think this is a much simpler
solution than adding code such that, while checking each feature, I
spot if the register I'm processing is the one I care about.

No tests here as the new code is not used, but this code will be
exercised in the next commit.
---
 gdb/target-descriptions.c | 11 +++++++++++
 gdb/target-descriptions.h |  4 ++++
 2 files changed, 15 insertions(+)

diff --git a/gdb/target-descriptions.c b/gdb/target-descriptions.c
index d5d795875c2..044b171ecd2 100644
--- a/gdb/target-descriptions.c
+++ b/gdb/target-descriptions.c
@@ -849,6 +849,17 @@ tdesc_numbered_register_choices (const struct tdesc_feature *feature,
   return 0;
 }
 
+/* See target-descriptions.h.  */
+
+bool
+tdesc_found_register (struct tdesc_arch_data *data, int regno)
+{
+  gdb_assert (regno >= 0);
+
+  return (regno < data->arch_regs.size ()
+	  && data->arch_regs[regno].reg != nullptr);
+}
+
 /* Search FEATURE for a register named NAME, and return its size in
    bits.  The register must exist.  */
 
diff --git a/gdb/target-descriptions.h b/gdb/target-descriptions.h
index 3b90dedcd80..3ab0ae2542d 100644
--- a/gdb/target-descriptions.h
+++ b/gdb/target-descriptions.h
@@ -170,6 +170,10 @@ int tdesc_numbered_register_choices (const struct tdesc_feature *feature,
 				     struct tdesc_arch_data *data,
 				     int regno, const char *const names[]);
 
+/* Return true if DATA contains an entry for REGNO, a GDB register
+   number.  */
+
+extern bool tdesc_found_register (struct tdesc_arch_data *data, int regno);
 
 /* Accessors for target descriptions.  */
 
-- 
2.25.4


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

* [PATCH 3/3] gdb/riscv: better support for fflags and frm registers
  2022-08-16 10:09 [PATCH 0/3] RISC-V fflags, frm, and fcsr (includes small tdesc change) Andrew Burgess
  2022-08-16 10:09 ` [PATCH 1/3] gdb/riscv: improve (and fix) display of frm field in 'info registers' Andrew Burgess
  2022-08-16 10:09 ` [PATCH 2/3] gdb: Add tdesc_found_register function to tdesc API Andrew Burgess
@ 2022-08-16 10:09 ` Andrew Burgess
  2022-08-31 15:11   ` Andrew Burgess
  2022-08-16 16:26 ` [PATCH 0/3] RISC-V fflags, frm, and fcsr (includes small tdesc change) John Baldwin
  2022-08-31 15:33 ` Andrew Burgess
  4 siblings, 1 reply; 7+ messages in thread
From: Andrew Burgess @ 2022-08-16 10:09 UTC (permalink / raw)
  To: gdb-patches; +Cc: Andrew Burgess

First, some background on the RISC-V registers fflags, frm, and fcsr.

These three registers all relate to the floating-point status and
control mechanism on RISC-V.  The fcsr is the floatint-point control
status register, and consists of two parts, the flags (bits 0 to 4)
and the rounding-mode (bits 5 to 7).

The fcsr register is just one of many control/status registers (or
CSRs) available on RISC-V.  The fflags and frm registers are also
CSRs.  These CSRs are aliases for the relevant parts of the fcsr
register.  So fflags is an alias for bits 0 to 4 of fcsr, and frm is
an alias for bits 5 to 7 of fcsr.

This means that a user can change the floating-point rounding mode
either, by writing a complete new value into fcsr, or by writing just
the rounding mode into frm.

How this impacts on GDB is like this: a target description could,
legitimately include all three registers, fcsr, fflags, and frm.  The
QEMU target currently does this, and this makes sense.  The target is
emulating the complete system, and has all three CSRs available, so
why not tell GDB about this.

In contrast, the RISC-V native Linux target only has access to the
fcsr.  This is because the ptrace data structure that the kernel uses
for reading and writing floating point state only contains a copy of
the fcsr, after all, this one field really contains both the fflags
and frm fields, so why carry around duplicate data.

So, we might expect that the target description for the RISC-V native
Linux GDB would only contain the fcsr register.  Unfortunately, this
is not the case.  The RISC-V native Linux target uses GDB's builtin
target descriptions by calling riscv_lookup_target_description, this
will then add an fpu feature from gdb/features/riscv, either
32bit-fpu.xml or 64bit-fpu.xml.  The problem, is that these features
include an entry for fcsr, fflags, and frm.  This means that GDB
expects the target to handle reading and writing these registers.  And
the RISC-V native Linux target currently doesn't.

In riscv_linux_nat_target::store_registers and
riscv_linux_nat_target::fetch_registers only the fcsr register is
handled, this means that, for RISC-V native Linux, the fflags and frm
registers always show up as <unavailable> - they are present in the
target description, but the target doesn't know how to access the
registers.

A final complication relating to these floating pointer CSRs is which
target description feature the registers appear in.

These registers are CSRs, so it would seem sensible that these
registers should appear in the CSR target description feature.

However, when I first added RISC-V target description support, I was
using a RISC-V simulator that didn't support any CSRs other than the
floating point related ones.  This simulator bundled all the float
related CSRs into the fpu target feature.  This didn't feel completely
unreasonable to me, and so I had GDB check for these registers in
either target feature.

In this commit I make some changes relating to how GDB handles the
three floating point CSR:

1. Remove fflags and frm from 32bit-fpu.xml and 64bit-fpu.xml.  This
means that the default RISC-V target description (which RISC-V native
FreeBSD), and the target descriptions created for RISC-V native Linux,
will not include these registers.  There's nothing stopping some other
target (e.g. QEMU) from continuing to include all three of these CSRs,
the code in riscv-tdep.c continues to check for all three of these
registers, and will handle them correctly if they are present.

2. If a target supplied fcsr, but does not supply fflags and/or frm,
then RISC-V GDB will now create two pseudo registers in order to
emulate the two missing CSRs.  These new pseudo-registers do the
obvious thing of just reading and writing the fcsr register.

3. With the new pseudo-registers we can no longer make use of the GDB
register numbers RISCV_CSR_FFLAGS_REGNUM and RISCV_CSR_FRM_REGNUM.
These will be the numbers used if the target supplies the registers in
its target description, but, if GDB falls back to using
pseudo-registers, then new, unique numbers will be used.  To handle
this I've added riscv_gdbarch_tdep::fflags_regnum and
riscv_gdbarch_tdep::frm_regnum, I've then updated the RISC-V code to
compare against these fields.

When adding the pseudo-register support, it is important that the
pseudo-register numbers are calculated after the call to
tdesc_use_registers.  This is because we don't know the total number
of physical registers until after this call, and the psuedo-register
numbers must follow on from the real (target supplied) registers.

I've updated some tests to include more testing of the fflags and frm
registers, as well as adding a new test.
---
 gdb/arch/riscv.h                              |  19 +-
 gdb/features/riscv/32bit-fpu.c                |   4 +-
 gdb/features/riscv/32bit-fpu.xml              |   2 -
 gdb/features/riscv/64bit-fpu.c                |   4 +-
 gdb/features/riscv/64bit-fpu.xml              |   2 -
 gdb/riscv-tdep.c                              | 212 ++++++++++++++++--
 gdb/riscv-tdep.h                              |   6 +
 gdb/testsuite/gdb.arch/riscv-info-fcsr.exp    |  24 +-
 .../gdb.arch/riscv-tdesc-fcsr-32.xml          |  75 +++++++
 .../gdb.arch/riscv-tdesc-fcsr-64.xml          |  79 +++++++
 .../gdb.arch/riscv-tdesc-loading-05.xml       |  77 +++++++
 .../gdb.arch/riscv-tdesc-loading-06.xml       |  75 +++++++
 gdb/testsuite/gdb.arch/riscv-tdesc-regs.exp   |  36 +++
 13 files changed, 569 insertions(+), 46 deletions(-)
 create mode 100644 gdb/testsuite/gdb.arch/riscv-tdesc-fcsr-32.xml
 create mode 100644 gdb/testsuite/gdb.arch/riscv-tdesc-fcsr-64.xml
 create mode 100644 gdb/testsuite/gdb.arch/riscv-tdesc-loading-05.xml
 create mode 100644 gdb/testsuite/gdb.arch/riscv-tdesc-loading-06.xml

diff --git a/gdb/arch/riscv.h b/gdb/arch/riscv.h
index 0aef54638fe..8e814d506ee 100644
--- a/gdb/arch/riscv.h
+++ b/gdb/arch/riscv.h
@@ -55,11 +55,23 @@ struct riscv_gdbarch_features
   /* When true this target is RV32E.  */
   bool embedded = false;
 
+  /* Track if the target description has an fcsr, fflags, and frm
+     registers.  Some targets provide all these in their target
+     descriptions, while some only offer fcsr, while others don't even
+     offer that register.  If a target provides fcsr but not fflags and/or
+     frm, then we can emulate these registers as pseudo registers.  */
+  bool has_fcsr_reg = false;
+  bool has_fflags_reg = false;
+  bool has_frm_reg = false;
+
   /* Equality operator.  */
   bool operator== (const struct riscv_gdbarch_features &rhs) const
   {
     return (xlen == rhs.xlen && flen == rhs.flen
-	    && embedded == rhs.embedded && vlen == rhs.vlen);
+	    && embedded == rhs.embedded && vlen == rhs.vlen
+	    && has_fflags_reg == rhs.has_fflags_reg
+	    && has_frm_reg == rhs.has_frm_reg
+	    && has_fcsr_reg == rhs.has_fcsr_reg);
   }
 
   /* Inequality operator.  */
@@ -72,9 +84,12 @@ struct riscv_gdbarch_features
   std::size_t hash () const noexcept
   {
     std::size_t val = ((embedded ? 1 : 0) << 10
+		       | (has_fflags_reg ? 1 : 0) << 11
+		       | (has_frm_reg ? 1 : 0) << 12
+		       | (has_fcsr_reg ? 1 : 0) << 13
 		       | (xlen & 0x1f) << 5
 		       | (flen & 0x1f) << 0
-		       | (vlen & 0xfff) << 11);
+		       | (vlen & 0xfff) << 14);
     return val;
   }
 };
diff --git a/gdb/features/riscv/32bit-fpu.c b/gdb/features/riscv/32bit-fpu.c
index d92407fb9e0..e763fccfe4c 100644
--- a/gdb/features/riscv/32bit-fpu.c
+++ b/gdb/features/riscv/32bit-fpu.c
@@ -42,9 +42,7 @@ create_feature_riscv_32bit_fpu (struct target_desc *result, long regnum)
   tdesc_create_reg (feature, "ft9", regnum++, 1, NULL, 32, "ieee_single");
   tdesc_create_reg (feature, "ft10", regnum++, 1, NULL, 32, "ieee_single");
   tdesc_create_reg (feature, "ft11", regnum++, 1, NULL, 32, "ieee_single");
-  regnum = 66;
-  tdesc_create_reg (feature, "fflags", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "frm", regnum++, 1, NULL, 32, "int");
+  regnum = 68;
   tdesc_create_reg (feature, "fcsr", regnum++, 1, NULL, 32, "int");
   return regnum;
 }
diff --git a/gdb/features/riscv/32bit-fpu.xml b/gdb/features/riscv/32bit-fpu.xml
index ed8f14c09dc..7d87f9c057b 100644
--- a/gdb/features/riscv/32bit-fpu.xml
+++ b/gdb/features/riscv/32bit-fpu.xml
@@ -44,7 +44,5 @@
   <reg name="ft10" bitsize="32" type="ieee_single"/>
   <reg name="ft11" bitsize="32" type="ieee_single"/>
 
-  <reg name="fflags" bitsize="32" type="int" regnum="66"/>
-  <reg name="frm" bitsize="32" type="int" regnum="67"/>
   <reg name="fcsr" bitsize="32" type="int" regnum="68"/>
 </feature>
diff --git a/gdb/features/riscv/64bit-fpu.c b/gdb/features/riscv/64bit-fpu.c
index 4fd8ee1e41b..bf9b74b24dc 100644
--- a/gdb/features/riscv/64bit-fpu.c
+++ b/gdb/features/riscv/64bit-fpu.c
@@ -50,9 +50,7 @@ create_feature_riscv_64bit_fpu (struct target_desc *result, long regnum)
   tdesc_create_reg (feature, "ft9", regnum++, 1, NULL, 64, "riscv_double");
   tdesc_create_reg (feature, "ft10", regnum++, 1, NULL, 64, "riscv_double");
   tdesc_create_reg (feature, "ft11", regnum++, 1, NULL, 64, "riscv_double");
-  regnum = 66;
-  tdesc_create_reg (feature, "fflags", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "frm", regnum++, 1, NULL, 32, "int");
+  regnum = 68;
   tdesc_create_reg (feature, "fcsr", regnum++, 1, NULL, 32, "int");
   return regnum;
 }
diff --git a/gdb/features/riscv/64bit-fpu.xml b/gdb/features/riscv/64bit-fpu.xml
index ff42b4a21fb..68f523f0d60 100644
--- a/gdb/features/riscv/64bit-fpu.xml
+++ b/gdb/features/riscv/64bit-fpu.xml
@@ -50,7 +50,5 @@
   <reg name="ft10" bitsize="64" type="riscv_double"/>
   <reg name="ft11" bitsize="64" type="riscv_double"/>
 
-  <reg name="fflags" bitsize="32" type="int" regnum="66"/>
-  <reg name="frm" bitsize="32" type="int" regnum="67"/>
   <reg name="fcsr" bitsize="32" type="int" regnum="68"/>
 </feature>
diff --git a/gdb/riscv-tdep.c b/gdb/riscv-tdep.c
index 93ee597af58..30f29eaa285 100644
--- a/gdb/riscv-tdep.c
+++ b/gdb/riscv-tdep.c
@@ -933,6 +933,72 @@ riscv_register_name (struct gdbarch *gdbarch, int regnum)
   return name;
 }
 
+/* Implement gdbarch_pseudo_register_read.  Read pseudo-register REGNUM
+   from REGCACHE and place the register value into BUF.  BUF is sized
+   based on the type of register REGNUM, all of BUF should be written too,
+   the result should be sign or zero extended as appropriate.  */
+
+static enum register_status
+riscv_pseudo_register_read (struct gdbarch *gdbarch,
+			    readable_regcache *regcache,
+			    int regnum, gdb_byte *buf)
+{
+  riscv_gdbarch_tdep *tdep = gdbarch_tdep<riscv_gdbarch_tdep> (gdbarch);
+
+  if (regnum == tdep->fflags_regnum || regnum == tdep->frm_regnum)
+    {
+      /* Clear BUF.  */
+      memset (buf, 0, register_size (gdbarch, regnum));
+
+      /* Read the first byte of the fcsr register, this contains both frm
+	 and fflags.  */
+      enum register_status status
+	= regcache->raw_read_part (RISCV_CSR_FCSR_REGNUM, 0, 1, buf);
+
+      if (status != REG_VALID)
+	return status;
+
+      /* Extract the appropriate parts.  */
+      if (regnum == tdep->fflags_regnum)
+	buf[0] &= 0x1f;
+      else if (regnum == tdep->frm_regnum)
+	buf[0] = (buf[0] >> 5) & 0x7;
+
+      return REG_VALID;
+    }
+
+  return REG_UNKNOWN;
+}
+
+/* Implement gdbarch_pseudo_register_write.  Write the contents of BUF into
+   pseudo-register REGNUM in REGCACHE.  BUF is sized based on the type of
+   register REGNUM.  */
+
+static void
+riscv_pseudo_register_write (struct gdbarch *gdbarch,
+			     struct regcache *regcache, int regnum,
+			     const gdb_byte *buf)
+{
+  riscv_gdbarch_tdep *tdep = gdbarch_tdep<riscv_gdbarch_tdep> (gdbarch);
+
+  if (regnum == tdep->fflags_regnum || regnum == tdep->frm_regnum)
+    {
+      int fcsr_regnum = RISCV_CSR_FCSR_REGNUM;
+      gdb_byte raw_buf[register_size (gdbarch, fcsr_regnum)];
+
+      regcache->raw_read (fcsr_regnum, raw_buf);
+
+      if (regnum == tdep->fflags_regnum)
+	raw_buf[0] = (raw_buf[0] & ~0x1f) | (buf[0] & 0x1f);
+      else if (regnum == tdep->frm_regnum)
+	raw_buf[0] = (raw_buf[0] & ~(0x7 << 5)) | ((buf[0] & 0x7) << 5);
+
+      regcache->raw_write (fcsr_regnum, raw_buf);
+    }
+  else
+    gdb_assert_not_reached ("unknown pseudo register %d", regnum);
+}
+
 /* Implement the cannot_store_register gdbarch method.  The zero register
    (x0) is read-only on RISC-V.  */
 
@@ -1096,6 +1162,7 @@ riscv_print_one_register_info (struct gdbarch *gdbarch,
   else
     {
       struct value_print_options opts;
+      riscv_gdbarch_tdep *tdep = gdbarch_tdep<riscv_gdbarch_tdep> (gdbarch);
 
       /* Print the register in hex.  */
       get_formatted_print_options (&opts, 'x');
@@ -1162,15 +1229,13 @@ riscv_print_one_register_info (struct gdbarch *gdbarch,
 		}
 	    }
 	  else if (regnum == RISCV_CSR_FCSR_REGNUM
-		   || regnum == RISCV_CSR_FFLAGS_REGNUM
-		   || regnum == RISCV_CSR_FRM_REGNUM)
+		   || regnum == tdep->fflags_regnum
+		   || regnum == tdep->frm_regnum)
 	    {
-	      LONGEST d;
-
-	      d = value_as_long (val);
+	      LONGEST d = value_as_long (val);
 
 	      gdb_printf (file, "\t");
-	      if (regnum != RISCV_CSR_FRM_REGNUM)
+	      if (regnum != tdep->frm_regnum)
 		gdb_printf (file,
 			    "NV:%d DZ:%d OF:%d UF:%d NX:%d",
 			    (int) ((d >> 4) & 0x1),
@@ -1179,7 +1244,7 @@ riscv_print_one_register_info (struct gdbarch *gdbarch,
 			    (int) ((d >> 1) & 0x1),
 			    (int) ((d >> 0) & 0x1));
 
-	      if (regnum != RISCV_CSR_FFLAGS_REGNUM)
+	      if (regnum != tdep->fflags_regnum)
 		{
 		  static const char * const sfrm[] =
 		    {
@@ -1284,13 +1349,15 @@ static int
 riscv_register_reggroup_p (struct gdbarch  *gdbarch, int regnum,
 			   const struct reggroup *reggroup)
 {
+  riscv_gdbarch_tdep *tdep = gdbarch_tdep<riscv_gdbarch_tdep> (gdbarch);
+
   /* Used by 'info registers' and 'info registers <groupname>'.  */
 
   if (gdbarch_register_name (gdbarch, regnum) == NULL
       || gdbarch_register_name (gdbarch, regnum)[0] == '\0')
     return 0;
 
-  if (regnum > RISCV_LAST_REGNUM)
+  if (regnum > RISCV_LAST_REGNUM && regnum < gdbarch_num_regs (gdbarch))
     {
       /* Any extra registers from the CSR tdesc_feature (identified in
 	 riscv_tdesc_unknown_reg) are removed from the save/restore groups
@@ -1331,8 +1398,8 @@ riscv_register_reggroup_p (struct gdbarch  *gdbarch, int regnum,
   else if (reggroup == float_reggroup)
     return (riscv_is_fp_regno_p (regnum)
 	    || regnum == RISCV_CSR_FCSR_REGNUM
-	    || regnum == RISCV_CSR_FFLAGS_REGNUM
-	    || regnum == RISCV_CSR_FRM_REGNUM);
+	    || regnum == tdep->fflags_regnum
+	    || regnum == tdep->frm_regnum);
   else if (reggroup == general_reggroup)
     return regnum < RISCV_FIRST_FP_REGNUM;
   else if (reggroup == restore_reggroup || reggroup == save_reggroup)
@@ -1340,8 +1407,8 @@ riscv_register_reggroup_p (struct gdbarch  *gdbarch, int regnum,
       if (riscv_has_fp_regs (gdbarch))
 	return (regnum <= RISCV_LAST_FP_REGNUM
 		|| regnum == RISCV_CSR_FCSR_REGNUM
-		|| regnum == RISCV_CSR_FFLAGS_REGNUM
-		|| regnum == RISCV_CSR_FRM_REGNUM);
+		|| regnum == tdep->fflags_regnum
+		|| regnum == tdep->frm_regnum);
       else
 	return regnum < RISCV_FIRST_FP_REGNUM;
     }
@@ -1361,6 +1428,45 @@ riscv_register_reggroup_p (struct gdbarch  *gdbarch, int regnum,
     return 0;
 }
 
+/* Return the name for pseudo-register REGNUM for GDBARCH.  */
+
+static const char *
+riscv_pseudo_register_name (struct gdbarch *gdbarch, int regnum)
+{
+  riscv_gdbarch_tdep *tdep = gdbarch_tdep<riscv_gdbarch_tdep> (gdbarch);
+
+  if (regnum == tdep->fflags_regnum)
+    return "fflags";
+  else if (regnum == tdep->frm_regnum)
+    return "frm";
+  else
+    gdb_assert_not_reached ("unknown pseudo register number %d", regnum);
+}
+
+/* Return the type for pseudo-register REGNUM for GDBARCH.  */
+
+static struct type *
+riscv_pseudo_register_type (struct gdbarch *gdbarch, int regnum)
+{
+  riscv_gdbarch_tdep *tdep = gdbarch_tdep<riscv_gdbarch_tdep> (gdbarch);
+
+  if (regnum == tdep->fflags_regnum || regnum == tdep->frm_regnum)
+   return builtin_type (gdbarch)->builtin_int32;
+  else
+    gdb_assert_not_reached ("unknown pseudo register number %d", regnum);
+}
+
+/* Return true (non-zero) if pseudo-register REGNUM from GDBARCH is a
+   member of REGGROUP, otherwise return false (zero).  */
+
+static int
+riscv_pseudo_register_reggroup_p (struct gdbarch *gdbarch, int regnum,
+				  const struct reggroup *reggroup)
+{
+  /* The standard function will also work for pseudo-registers.  */
+  return riscv_register_reggroup_p (gdbarch, regnum, reggroup);
+}
+
 /* Implement the print_registers_info gdbarch method.  This is used by
    'info registers' and 'info all-registers'.  */
 
@@ -3713,6 +3819,13 @@ riscv_gdbarch_init (struct gdbarch_info info,
       return NULL;
     }
 
+  if (tdesc_found_register (tdesc_data.get (), RISCV_CSR_FFLAGS_REGNUM))
+    features.has_fflags_reg = true;
+  if (tdesc_found_register (tdesc_data.get (), RISCV_CSR_FRM_REGNUM))
+    features.has_frm_reg = true;
+  if (tdesc_found_register (tdesc_data.get (), RISCV_CSR_FCSR_REGNUM))
+    features.has_fcsr_reg = true;
+
   /* Have a look at what the supplied (if any) bfd object requires of the
      target, then check that this matches with what the target is
      providing.  */
@@ -3811,21 +3924,64 @@ riscv_gdbarch_init (struct gdbarch_info info,
      just a little easier.  */
   set_gdbarch_num_regs (gdbarch, RISCV_LAST_REGNUM + 1);
 
-  /* We don't have to provide the count of 0 here (its the default) but
-     include this line to make it explicit that, right now, we don't have
-     any pseudo registers on RISC-V.  */
-  set_gdbarch_num_pseudo_regs (gdbarch, 0);
-
   /* Some specific register numbers GDB likes to know about.  */
   set_gdbarch_sp_regnum (gdbarch, RISCV_SP_REGNUM);
   set_gdbarch_pc_regnum (gdbarch, RISCV_PC_REGNUM);
 
   set_gdbarch_print_registers_info (gdbarch, riscv_print_registers_info);
 
+  set_tdesc_pseudo_register_name (gdbarch, riscv_pseudo_register_name);
+  set_tdesc_pseudo_register_type (gdbarch, riscv_pseudo_register_type);
+  set_tdesc_pseudo_register_reggroup_p (gdbarch,
+					riscv_pseudo_register_reggroup_p);
+  set_gdbarch_pseudo_register_read (gdbarch, riscv_pseudo_register_read);
+  set_gdbarch_pseudo_register_write (gdbarch, riscv_pseudo_register_write);
+
   /* Finalise the target description registers.  */
   tdesc_use_registers (gdbarch, tdesc, std::move (tdesc_data),
 		       riscv_tdesc_unknown_reg);
 
+  /* Calculate the number of pseudo registers we need.  The fflags and frm
+     registers are sub-fields of the fcsr CSR register (csr3).  However,
+     these registers can also be accessed directly as separate CSR
+     registers (fflags is csr1, and frm is csr2).  And so, some targets
+     might choose to offer direct access to all three registers in the
+     target description, while other targets might choose to only offer
+     access to fcsr.
+
+     As we scan the target description we spot which of fcsr, fflags, and
+     frm are available.  If fcsr is available but either of fflags and/or
+     frm are not available, then we add pseudo-registers to provide the
+     missing functionality.
+
+     This has to be done after the call to tdesc_use_registers as we don't
+     know the final register number until after that call, and the pseudo
+     register numbers need to be after the physical registers.  */
+  int num_pseudo_regs = 0;
+  int next_pseudo_regnum = gdbarch_num_regs (gdbarch);
+
+  if (features.has_fflags_reg)
+    tdep->fflags_regnum = RISCV_CSR_FFLAGS_REGNUM;
+  else if (features.has_fcsr_reg)
+    {
+      tdep->fflags_regnum = next_pseudo_regnum;
+      pending_aliases.emplace_back ("csr1", (void *) &tdep->fflags_regnum);
+      next_pseudo_regnum++;
+      num_pseudo_regs++;
+    }
+
+  if (features.has_frm_reg)
+    tdep->frm_regnum = RISCV_CSR_FRM_REGNUM;
+  else if (features.has_fcsr_reg)
+    {
+      tdep->frm_regnum = next_pseudo_regnum;
+      pending_aliases.emplace_back ("csr2", (void *) &tdep->frm_regnum);
+      next_pseudo_regnum++;
+      num_pseudo_regs++;
+    }
+
+  set_gdbarch_num_pseudo_regs (gdbarch, num_pseudo_regs);
+
   /* Override the register type callback setup by the target description
      mechanism.  This allows us to provide special type for floating point
      registers.  */
@@ -4033,8 +4189,12 @@ riscv_supply_regset (const struct regset *regset,
   if (regnum == -1 || regnum == RISCV_ZERO_REGNUM)
     regcache->raw_supply_zeroed (RISCV_ZERO_REGNUM);
 
-  if (regnum == -1 || regnum == RISCV_CSR_FFLAGS_REGNUM
-      || regnum == RISCV_CSR_FRM_REGNUM)
+  struct gdbarch *gdbarch = regcache->arch ();
+  riscv_gdbarch_tdep *tdep = gdbarch_tdep<riscv_gdbarch_tdep> (gdbarch);
+
+  if (regnum == -1
+      || regnum == tdep->fflags_regnum
+      || regnum == tdep->frm_regnum)
     {
       int fcsr_regnum = RISCV_CSR_FCSR_REGNUM;
 
@@ -4048,6 +4208,12 @@ riscv_supply_regset (const struct regset *regset,
 	 registers.  */
       if (regcache->get_register_status (fcsr_regnum) == REG_VALID)
 	{
+	  /* If we have an fcsr register then we should have fflags and frm
+	     too, either provided by the target, or provided as a pseudo
+	     register by GDB.  */
+	  gdb_assert (tdep->fflags_regnum >= 0);
+	  gdb_assert (tdep->frm_regnum >= 0);
+
 	  ULONGEST fcsr_val;
 	  regcache->raw_read (fcsr_regnum, &fcsr_val);
 
@@ -4056,14 +4222,14 @@ riscv_supply_regset (const struct regset *regset,
 	  ULONGEST frm_val = (fcsr_val >> 5) & 0x7;
 
 	  /* And supply these if needed.  */
-	  if (regnum == -1 || regnum == RISCV_CSR_FFLAGS_REGNUM)
-	    regcache->raw_supply_integer (RISCV_CSR_FFLAGS_REGNUM,
+	  if (regnum == -1 || regnum == tdep->fflags_regnum)
+	    regcache->raw_supply_integer (tdep->fflags_regnum,
 					  (gdb_byte *) &fflags_val,
 					  sizeof (fflags_val),
 					  /* is_signed */ false);
 
-	  if (regnum == -1 || regnum == RISCV_CSR_FRM_REGNUM)
-	    regcache->raw_supply_integer (RISCV_CSR_FRM_REGNUM,
+	  if (regnum == -1 || regnum == tdep->frm_regnum)
+	    regcache->raw_supply_integer (tdep->frm_regnum,
 					  (gdb_byte *)&frm_val,
 					  sizeof (fflags_val),
 					  /* is_signed */ false);
diff --git a/gdb/riscv-tdep.h b/gdb/riscv-tdep.h
index 826a002ef92..3ae126f4d8a 100644
--- a/gdb/riscv-tdep.h
+++ b/gdb/riscv-tdep.h
@@ -90,6 +90,12 @@ struct riscv_gdbarch_tdep : gdbarch_tdep_base
   /* ISA-specific data types.  */
   struct type *riscv_fpreg_d_type = nullptr;
 
+  /* The location of these registers, set to -2 by default so we don't
+     match against -1 which is frequently used to mean "all registers",
+     e.g. in the regcache supply/collect code.  */
+  int fflags_regnum = -2;
+  int frm_regnum = -2;
+
   /* 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
diff --git a/gdb/testsuite/gdb.arch/riscv-info-fcsr.exp b/gdb/testsuite/gdb.arch/riscv-info-fcsr.exp
index 3d6095d7e55..bf319650db3 100644
--- a/gdb/testsuite/gdb.arch/riscv-info-fcsr.exp
+++ b/gdb/testsuite/gdb.arch/riscv-info-fcsr.exp
@@ -73,17 +73,6 @@ proc check_fcsr { fflags_value frm_value frm_string } {
 	    exp_continue
 	}
 
-	-re "^(fflags\|frm)\\s+<unavailable>\r\n" {
-	    # Currently, on some targets (e.g. RISC-V native Linux) the
-	    # fflags and frm registers show as being available, but are
-	    # unreadable, the result is these registers report
-	    # themselves as <unavailable>.  So long as fcsr is readable
-	    # (which is checked below), then for now we accept this.
-	    set reg_name $expect_out(1,string)
-	    incr reg_counts($reg_name)
-	    exp_continue
-	}
-
 	-re "^(frm)\\s+${frm_value}\\s+${frm_pattern}\r\n" {
 	    set reg_name $expect_out(1,string)
 	    incr reg_counts($reg_name)
@@ -129,6 +118,19 @@ proc test_fcsr { fflags_value frm_value frm_string } {
 	with_test_prefix "set through fcsr" {
 	    check_fcsr $fflags_value $frm_value $frm_string
 	}
+
+	# Reset fcsr register back to zero.
+	gdb_test_no_output "set \$fcsr = 0x0" \
+	    "reset fcsr back to 0x0"
+	gdb_test "p/x \$fcsr" " = 0x0"
+
+	# Now set fcsr value through fflags and frm.
+	gdb_test_no_output "set \$fflags = ${fflags_value}"
+	gdb_test_no_output "set \$frm = ${frm_value}"
+
+	with_test_prefix "set through fflags and frm" {
+	    check_fcsr $fflags_value $frm_value $frm_string
+	}
     }
 }
 
diff --git a/gdb/testsuite/gdb.arch/riscv-tdesc-fcsr-32.xml b/gdb/testsuite/gdb.arch/riscv-tdesc-fcsr-32.xml
new file mode 100644
index 00000000000..844f8e354d9
--- /dev/null
+++ b/gdb/testsuite/gdb.arch/riscv-tdesc-fcsr-32.xml
@@ -0,0 +1,75 @@
+<?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="fcsr" bitsize="32" type="int"/>
+  </feature>
+</target>
diff --git a/gdb/testsuite/gdb.arch/riscv-tdesc-fcsr-64.xml b/gdb/testsuite/gdb.arch/riscv-tdesc-fcsr-64.xml
new file mode 100644
index 00000000000..130e308b903
--- /dev/null
+++ b/gdb/testsuite/gdb.arch/riscv-tdesc-fcsr-64.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="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="fcsr" bitsize="32" type="int"/>
+  </feature>
+</target>
diff --git a/gdb/testsuite/gdb.arch/riscv-tdesc-loading-05.xml b/gdb/testsuite/gdb.arch/riscv-tdesc-loading-05.xml
new file mode 100644
index 00000000000..62b472edba6
--- /dev/null
+++ b/gdb/testsuite/gdb.arch/riscv-tdesc-loading-05.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"/>
+  </feature>
+  <feature name="org.gnu.gdb.riscv.csr">
+    <reg name="fcsr" bitsize="32" type="int"/>
+  </feature>
+</target>
diff --git a/gdb/testsuite/gdb.arch/riscv-tdesc-loading-06.xml b/gdb/testsuite/gdb.arch/riscv-tdesc-loading-06.xml
new file mode 100644
index 00000000000..844f8e354d9
--- /dev/null
+++ b/gdb/testsuite/gdb.arch/riscv-tdesc-loading-06.xml
@@ -0,0 +1,75 @@
+<?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="fcsr" bitsize="32" type="int"/>
+  </feature>
+</target>
diff --git a/gdb/testsuite/gdb.arch/riscv-tdesc-regs.exp b/gdb/testsuite/gdb.arch/riscv-tdesc-regs.exp
index 7f6bc45039f..4bacaaef99c 100644
--- a/gdb/testsuite/gdb.arch/riscv-tdesc-regs.exp
+++ b/gdb/testsuite/gdb.arch/riscv-tdesc-regs.exp
@@ -137,3 +137,39 @@ foreach rgroup {x_all all save restore general system csr} {
     }
     array unset reg_counts
 }
+
+# Next load a target description that contains fcsr, but not fflags or
+# frm.  Then check that GDB provides an fflags and frm registers using
+# the pseudo-register mechanism.
+if { $xlen == 4 } {
+    set xml_tdesc "riscv-tdesc-fcsr-32.xml"
+} else {
+    set xml_tdesc "riscv-tdesc-fcsr-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 target description that lacks fflags and frm"
+
+foreach reg {fflags frm} {
+    gdb_test_multiple "info registers $reg" "" {
+	-re "^info registers $reg\r\n" {
+	    exp_continue
+	}
+
+	-wrap -re "^Invalid register `$reg`" {
+	    fail $gdb_test_name
+	}
+
+	-wrap -re "^$reg\\s+\[^\r\n\]+" {
+	    pass $gdb_test_name
+	}
+    }
+}
-- 
2.25.4


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

* Re: [PATCH 0/3] RISC-V fflags, frm, and fcsr (includes small tdesc change)
  2022-08-16 10:09 [PATCH 0/3] RISC-V fflags, frm, and fcsr (includes small tdesc change) Andrew Burgess
                   ` (2 preceding siblings ...)
  2022-08-16 10:09 ` [PATCH 3/3] gdb/riscv: better support for fflags and frm registers Andrew Burgess
@ 2022-08-16 16:26 ` John Baldwin
  2022-08-31 15:33 ` Andrew Burgess
  4 siblings, 0 replies; 7+ messages in thread
From: John Baldwin @ 2022-08-16 16:26 UTC (permalink / raw)
  To: Andrew Burgess, gdb-patches

On 8/16/22 3:09 AM, Andrew Burgess via Gdb-patches wrote:
> While testing on a RISC-V native Linux target, I noticed some issues
> accessing the fflags and frm registers.  This series addresses these issues.
> 
> Patch #2 is a small extension to the target description mechanism
> required to support patch #3.
> 
> Thanks,
> Andrew

I only skimmed the patches, but I agree that the idea of using pseudo registers
for frm and fflags unless a remote target provides them explicitly is the
right approach.

-- 
John Baldwin

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

* Re: [PATCH 3/3] gdb/riscv: better support for fflags and frm registers
  2022-08-16 10:09 ` [PATCH 3/3] gdb/riscv: better support for fflags and frm registers Andrew Burgess
@ 2022-08-31 15:11   ` Andrew Burgess
  0 siblings, 0 replies; 7+ messages in thread
From: Andrew Burgess @ 2022-08-31 15:11 UTC (permalink / raw)
  To: gdb-patches


I found a small issue with the previous version of this patch, in
riscv_supply_regset, if either fflags and/or frm was a pseudo-register
then GDB would try to "supply" the pseudo-register into the register
cache, which resulted in an assertion - only real (non-psuedo) registers
should be supplied into a register cache.

The version below includes a fix for this issue.

Thanks,
Andrew

---

commit 4749b84b51bcaa59acfb8bda5d9e134432528cf9
Author: Andrew Burgess <aburgess@redhat.com>
Date:   Thu Aug 11 18:38:53 2022 +0100

    gdb/riscv: better support for fflags and frm registers
    
    First, some background on the RISC-V registers fflags, frm, and fcsr.
    
    These three registers all relate to the floating-point status and
    control mechanism on RISC-V.  The fcsr is the floatint-point control
    status register, and consists of two parts, the flags (bits 0 to 4)
    and the rounding-mode (bits 5 to 7).
    
    The fcsr register is just one of many control/status registers (or
    CSRs) available on RISC-V.  The fflags and frm registers are also
    CSRs.  These CSRs are aliases for the relevant parts of the fcsr
    register.  So fflags is an alias for bits 0 to 4 of fcsr, and frm is
    an alias for bits 5 to 7 of fcsr.
    
    This means that a user can change the floating-point rounding mode
    either, by writing a complete new value into fcsr, or by writing just
    the rounding mode into frm.
    
    How this impacts on GDB is like this: a target description could,
    legitimately include all three registers, fcsr, fflags, and frm.  The
    QEMU target currently does this, and this makes sense.  The target is
    emulating the complete system, and has all three CSRs available, so
    why not tell GDB about this.
    
    In contrast, the RISC-V native Linux target only has access to the
    fcsr.  This is because the ptrace data structure that the kernel uses
    for reading and writing floating point state only contains a copy of
    the fcsr, after all, this one field really contains both the fflags
    and frm fields, so why carry around duplicate data.
    
    So, we might expect that the target description for the RISC-V native
    Linux GDB would only contain the fcsr register.  Unfortunately, this
    is not the case.  The RISC-V native Linux target uses GDB's builtin
    target descriptions by calling riscv_lookup_target_description, this
    will then add an fpu feature from gdb/features/riscv, either
    32bit-fpu.xml or 64bit-fpu.xml.  The problem, is that these features
    include an entry for fcsr, fflags, and frm.  This means that GDB
    expects the target to handle reading and writing these registers.  And
    the RISC-V native Linux target currently doesn't.
    
    In riscv_linux_nat_target::store_registers and
    riscv_linux_nat_target::fetch_registers only the fcsr register is
    handled, this means that, for RISC-V native Linux, the fflags and frm
    registers always show up as <unavailable> - they are present in the
    target description, but the target doesn't know how to access the
    registers.
    
    A final complication relating to these floating pointer CSRs is which
    target description feature the registers appear in.
    
    These registers are CSRs, so it would seem sensible that these
    registers should appear in the CSR target description feature.
    
    However, when I first added RISC-V target description support, I was
    using a RISC-V simulator that didn't support any CSRs other than the
    floating point related ones.  This simulator bundled all the float
    related CSRs into the fpu target feature.  This didn't feel completely
    unreasonable to me, and so I had GDB check for these registers in
    either target feature.
    
    In this commit I make some changes relating to how GDB handles the
    three floating point CSR:
    
    1. Remove fflags and frm from 32bit-fpu.xml and 64bit-fpu.xml.  This
    means that the default RISC-V target description (which RISC-V native
    FreeBSD), and the target descriptions created for RISC-V native Linux,
    will not include these registers.  There's nothing stopping some other
    target (e.g. QEMU) from continuing to include all three of these CSRs,
    the code in riscv-tdep.c continues to check for all three of these
    registers, and will handle them correctly if they are present.
    
    2. If a target supplied fcsr, but does not supply fflags and/or frm,
    then RISC-V GDB will now create two pseudo registers in order to
    emulate the two missing CSRs.  These new pseudo-registers do the
    obvious thing of just reading and writing the fcsr register.
    
    3. With the new pseudo-registers we can no longer make use of the GDB
    register numbers RISCV_CSR_FFLAGS_REGNUM and RISCV_CSR_FRM_REGNUM.
    These will be the numbers used if the target supplies the registers in
    its target description, but, if GDB falls back to using
    pseudo-registers, then new, unique numbers will be used.  To handle
    this I've added riscv_gdbarch_tdep::fflags_regnum and
    riscv_gdbarch_tdep::frm_regnum, I've then updated the RISC-V code to
    compare against these fields.
    
    When adding the pseudo-register support, it is important that the
    pseudo-register numbers are calculated after the call to
    tdesc_use_registers.  This is because we don't know the total number
    of physical registers until after this call, and the psuedo-register
    numbers must follow on from the real (target supplied) registers.
    
    I've updated some tests to include more testing of the fflags and frm
    registers, as well as adding a new test.

diff --git a/gdb/arch/riscv.h b/gdb/arch/riscv.h
index 0aef54638fe..8e814d506ee 100644
--- a/gdb/arch/riscv.h
+++ b/gdb/arch/riscv.h
@@ -55,11 +55,23 @@ struct riscv_gdbarch_features
   /* When true this target is RV32E.  */
   bool embedded = false;
 
+  /* Track if the target description has an fcsr, fflags, and frm
+     registers.  Some targets provide all these in their target
+     descriptions, while some only offer fcsr, while others don't even
+     offer that register.  If a target provides fcsr but not fflags and/or
+     frm, then we can emulate these registers as pseudo registers.  */
+  bool has_fcsr_reg = false;
+  bool has_fflags_reg = false;
+  bool has_frm_reg = false;
+
   /* Equality operator.  */
   bool operator== (const struct riscv_gdbarch_features &rhs) const
   {
     return (xlen == rhs.xlen && flen == rhs.flen
-	    && embedded == rhs.embedded && vlen == rhs.vlen);
+	    && embedded == rhs.embedded && vlen == rhs.vlen
+	    && has_fflags_reg == rhs.has_fflags_reg
+	    && has_frm_reg == rhs.has_frm_reg
+	    && has_fcsr_reg == rhs.has_fcsr_reg);
   }
 
   /* Inequality operator.  */
@@ -72,9 +84,12 @@ struct riscv_gdbarch_features
   std::size_t hash () const noexcept
   {
     std::size_t val = ((embedded ? 1 : 0) << 10
+		       | (has_fflags_reg ? 1 : 0) << 11
+		       | (has_frm_reg ? 1 : 0) << 12
+		       | (has_fcsr_reg ? 1 : 0) << 13
 		       | (xlen & 0x1f) << 5
 		       | (flen & 0x1f) << 0
-		       | (vlen & 0xfff) << 11);
+		       | (vlen & 0xfff) << 14);
     return val;
   }
 };
diff --git a/gdb/features/riscv/32bit-fpu.c b/gdb/features/riscv/32bit-fpu.c
index d92407fb9e0..e763fccfe4c 100644
--- a/gdb/features/riscv/32bit-fpu.c
+++ b/gdb/features/riscv/32bit-fpu.c
@@ -42,9 +42,7 @@ create_feature_riscv_32bit_fpu (struct target_desc *result, long regnum)
   tdesc_create_reg (feature, "ft9", regnum++, 1, NULL, 32, "ieee_single");
   tdesc_create_reg (feature, "ft10", regnum++, 1, NULL, 32, "ieee_single");
   tdesc_create_reg (feature, "ft11", regnum++, 1, NULL, 32, "ieee_single");
-  regnum = 66;
-  tdesc_create_reg (feature, "fflags", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "frm", regnum++, 1, NULL, 32, "int");
+  regnum = 68;
   tdesc_create_reg (feature, "fcsr", regnum++, 1, NULL, 32, "int");
   return regnum;
 }
diff --git a/gdb/features/riscv/32bit-fpu.xml b/gdb/features/riscv/32bit-fpu.xml
index ed8f14c09dc..7d87f9c057b 100644
--- a/gdb/features/riscv/32bit-fpu.xml
+++ b/gdb/features/riscv/32bit-fpu.xml
@@ -44,7 +44,5 @@
   <reg name="ft10" bitsize="32" type="ieee_single"/>
   <reg name="ft11" bitsize="32" type="ieee_single"/>
 
-  <reg name="fflags" bitsize="32" type="int" regnum="66"/>
-  <reg name="frm" bitsize="32" type="int" regnum="67"/>
   <reg name="fcsr" bitsize="32" type="int" regnum="68"/>
 </feature>
diff --git a/gdb/features/riscv/64bit-fpu.c b/gdb/features/riscv/64bit-fpu.c
index 4fd8ee1e41b..bf9b74b24dc 100644
--- a/gdb/features/riscv/64bit-fpu.c
+++ b/gdb/features/riscv/64bit-fpu.c
@@ -50,9 +50,7 @@ create_feature_riscv_64bit_fpu (struct target_desc *result, long regnum)
   tdesc_create_reg (feature, "ft9", regnum++, 1, NULL, 64, "riscv_double");
   tdesc_create_reg (feature, "ft10", regnum++, 1, NULL, 64, "riscv_double");
   tdesc_create_reg (feature, "ft11", regnum++, 1, NULL, 64, "riscv_double");
-  regnum = 66;
-  tdesc_create_reg (feature, "fflags", regnum++, 1, NULL, 32, "int");
-  tdesc_create_reg (feature, "frm", regnum++, 1, NULL, 32, "int");
+  regnum = 68;
   tdesc_create_reg (feature, "fcsr", regnum++, 1, NULL, 32, "int");
   return regnum;
 }
diff --git a/gdb/features/riscv/64bit-fpu.xml b/gdb/features/riscv/64bit-fpu.xml
index ff42b4a21fb..68f523f0d60 100644
--- a/gdb/features/riscv/64bit-fpu.xml
+++ b/gdb/features/riscv/64bit-fpu.xml
@@ -50,7 +50,5 @@
   <reg name="ft10" bitsize="64" type="riscv_double"/>
   <reg name="ft11" bitsize="64" type="riscv_double"/>
 
-  <reg name="fflags" bitsize="32" type="int" regnum="66"/>
-  <reg name="frm" bitsize="32" type="int" regnum="67"/>
   <reg name="fcsr" bitsize="32" type="int" regnum="68"/>
 </feature>
diff --git a/gdb/riscv-tdep.c b/gdb/riscv-tdep.c
index 93ee597af58..9df527e13e7 100644
--- a/gdb/riscv-tdep.c
+++ b/gdb/riscv-tdep.c
@@ -933,6 +933,72 @@ riscv_register_name (struct gdbarch *gdbarch, int regnum)
   return name;
 }
 
+/* Implement gdbarch_pseudo_register_read.  Read pseudo-register REGNUM
+   from REGCACHE and place the register value into BUF.  BUF is sized
+   based on the type of register REGNUM, all of BUF should be written too,
+   the result should be sign or zero extended as appropriate.  */
+
+static enum register_status
+riscv_pseudo_register_read (struct gdbarch *gdbarch,
+			    readable_regcache *regcache,
+			    int regnum, gdb_byte *buf)
+{
+  riscv_gdbarch_tdep *tdep = gdbarch_tdep<riscv_gdbarch_tdep> (gdbarch);
+
+  if (regnum == tdep->fflags_regnum || regnum == tdep->frm_regnum)
+    {
+      /* Clear BUF.  */
+      memset (buf, 0, register_size (gdbarch, regnum));
+
+      /* Read the first byte of the fcsr register, this contains both frm
+	 and fflags.  */
+      enum register_status status
+	= regcache->raw_read_part (RISCV_CSR_FCSR_REGNUM, 0, 1, buf);
+
+      if (status != REG_VALID)
+	return status;
+
+      /* Extract the appropriate parts.  */
+      if (regnum == tdep->fflags_regnum)
+	buf[0] &= 0x1f;
+      else if (regnum == tdep->frm_regnum)
+	buf[0] = (buf[0] >> 5) & 0x7;
+
+      return REG_VALID;
+    }
+
+  return REG_UNKNOWN;
+}
+
+/* Implement gdbarch_pseudo_register_write.  Write the contents of BUF into
+   pseudo-register REGNUM in REGCACHE.  BUF is sized based on the type of
+   register REGNUM.  */
+
+static void
+riscv_pseudo_register_write (struct gdbarch *gdbarch,
+			     struct regcache *regcache, int regnum,
+			     const gdb_byte *buf)
+{
+  riscv_gdbarch_tdep *tdep = gdbarch_tdep<riscv_gdbarch_tdep> (gdbarch);
+
+  if (regnum == tdep->fflags_regnum || regnum == tdep->frm_regnum)
+    {
+      int fcsr_regnum = RISCV_CSR_FCSR_REGNUM;
+      gdb_byte raw_buf[register_size (gdbarch, fcsr_regnum)];
+
+      regcache->raw_read (fcsr_regnum, raw_buf);
+
+      if (regnum == tdep->fflags_regnum)
+	raw_buf[0] = (raw_buf[0] & ~0x1f) | (buf[0] & 0x1f);
+      else if (regnum == tdep->frm_regnum)
+	raw_buf[0] = (raw_buf[0] & ~(0x7 << 5)) | ((buf[0] & 0x7) << 5);
+
+      regcache->raw_write (fcsr_regnum, raw_buf);
+    }
+  else
+    gdb_assert_not_reached ("unknown pseudo register %d", regnum);
+}
+
 /* Implement the cannot_store_register gdbarch method.  The zero register
    (x0) is read-only on RISC-V.  */
 
@@ -1096,6 +1162,7 @@ riscv_print_one_register_info (struct gdbarch *gdbarch,
   else
     {
       struct value_print_options opts;
+      riscv_gdbarch_tdep *tdep = gdbarch_tdep<riscv_gdbarch_tdep> (gdbarch);
 
       /* Print the register in hex.  */
       get_formatted_print_options (&opts, 'x');
@@ -1162,15 +1229,13 @@ riscv_print_one_register_info (struct gdbarch *gdbarch,
 		}
 	    }
 	  else if (regnum == RISCV_CSR_FCSR_REGNUM
-		   || regnum == RISCV_CSR_FFLAGS_REGNUM
-		   || regnum == RISCV_CSR_FRM_REGNUM)
+		   || regnum == tdep->fflags_regnum
+		   || regnum == tdep->frm_regnum)
 	    {
-	      LONGEST d;
-
-	      d = value_as_long (val);
+	      LONGEST d = value_as_long (val);
 
 	      gdb_printf (file, "\t");
-	      if (regnum != RISCV_CSR_FRM_REGNUM)
+	      if (regnum != tdep->frm_regnum)
 		gdb_printf (file,
 			    "NV:%d DZ:%d OF:%d UF:%d NX:%d",
 			    (int) ((d >> 4) & 0x1),
@@ -1179,7 +1244,7 @@ riscv_print_one_register_info (struct gdbarch *gdbarch,
 			    (int) ((d >> 1) & 0x1),
 			    (int) ((d >> 0) & 0x1));
 
-	      if (regnum != RISCV_CSR_FFLAGS_REGNUM)
+	      if (regnum != tdep->fflags_regnum)
 		{
 		  static const char * const sfrm[] =
 		    {
@@ -1284,13 +1349,15 @@ static int
 riscv_register_reggroup_p (struct gdbarch  *gdbarch, int regnum,
 			   const struct reggroup *reggroup)
 {
+  riscv_gdbarch_tdep *tdep = gdbarch_tdep<riscv_gdbarch_tdep> (gdbarch);
+
   /* Used by 'info registers' and 'info registers <groupname>'.  */
 
   if (gdbarch_register_name (gdbarch, regnum) == NULL
       || gdbarch_register_name (gdbarch, regnum)[0] == '\0')
     return 0;
 
-  if (regnum > RISCV_LAST_REGNUM)
+  if (regnum > RISCV_LAST_REGNUM && regnum < gdbarch_num_regs (gdbarch))
     {
       /* Any extra registers from the CSR tdesc_feature (identified in
 	 riscv_tdesc_unknown_reg) are removed from the save/restore groups
@@ -1331,8 +1398,8 @@ riscv_register_reggroup_p (struct gdbarch  *gdbarch, int regnum,
   else if (reggroup == float_reggroup)
     return (riscv_is_fp_regno_p (regnum)
 	    || regnum == RISCV_CSR_FCSR_REGNUM
-	    || regnum == RISCV_CSR_FFLAGS_REGNUM
-	    || regnum == RISCV_CSR_FRM_REGNUM);
+	    || regnum == tdep->fflags_regnum
+	    || regnum == tdep->frm_regnum);
   else if (reggroup == general_reggroup)
     return regnum < RISCV_FIRST_FP_REGNUM;
   else if (reggroup == restore_reggroup || reggroup == save_reggroup)
@@ -1340,8 +1407,8 @@ riscv_register_reggroup_p (struct gdbarch  *gdbarch, int regnum,
       if (riscv_has_fp_regs (gdbarch))
 	return (regnum <= RISCV_LAST_FP_REGNUM
 		|| regnum == RISCV_CSR_FCSR_REGNUM
-		|| regnum == RISCV_CSR_FFLAGS_REGNUM
-		|| regnum == RISCV_CSR_FRM_REGNUM);
+		|| regnum == tdep->fflags_regnum
+		|| regnum == tdep->frm_regnum);
       else
 	return regnum < RISCV_FIRST_FP_REGNUM;
     }
@@ -1361,6 +1428,45 @@ riscv_register_reggroup_p (struct gdbarch  *gdbarch, int regnum,
     return 0;
 }
 
+/* Return the name for pseudo-register REGNUM for GDBARCH.  */
+
+static const char *
+riscv_pseudo_register_name (struct gdbarch *gdbarch, int regnum)
+{
+  riscv_gdbarch_tdep *tdep = gdbarch_tdep<riscv_gdbarch_tdep> (gdbarch);
+
+  if (regnum == tdep->fflags_regnum)
+    return "fflags";
+  else if (regnum == tdep->frm_regnum)
+    return "frm";
+  else
+    gdb_assert_not_reached ("unknown pseudo register number %d", regnum);
+}
+
+/* Return the type for pseudo-register REGNUM for GDBARCH.  */
+
+static struct type *
+riscv_pseudo_register_type (struct gdbarch *gdbarch, int regnum)
+{
+  riscv_gdbarch_tdep *tdep = gdbarch_tdep<riscv_gdbarch_tdep> (gdbarch);
+
+  if (regnum == tdep->fflags_regnum || regnum == tdep->frm_regnum)
+   return builtin_type (gdbarch)->builtin_int32;
+  else
+    gdb_assert_not_reached ("unknown pseudo register number %d", regnum);
+}
+
+/* Return true (non-zero) if pseudo-register REGNUM from GDBARCH is a
+   member of REGGROUP, otherwise return false (zero).  */
+
+static int
+riscv_pseudo_register_reggroup_p (struct gdbarch *gdbarch, int regnum,
+				  const struct reggroup *reggroup)
+{
+  /* The standard function will also work for pseudo-registers.  */
+  return riscv_register_reggroup_p (gdbarch, regnum, reggroup);
+}
+
 /* Implement the print_registers_info gdbarch method.  This is used by
    'info registers' and 'info all-registers'.  */
 
@@ -3713,6 +3819,13 @@ riscv_gdbarch_init (struct gdbarch_info info,
       return NULL;
     }
 
+  if (tdesc_found_register (tdesc_data.get (), RISCV_CSR_FFLAGS_REGNUM))
+    features.has_fflags_reg = true;
+  if (tdesc_found_register (tdesc_data.get (), RISCV_CSR_FRM_REGNUM))
+    features.has_frm_reg = true;
+  if (tdesc_found_register (tdesc_data.get (), RISCV_CSR_FCSR_REGNUM))
+    features.has_fcsr_reg = true;
+
   /* Have a look at what the supplied (if any) bfd object requires of the
      target, then check that this matches with what the target is
      providing.  */
@@ -3811,21 +3924,64 @@ riscv_gdbarch_init (struct gdbarch_info info,
      just a little easier.  */
   set_gdbarch_num_regs (gdbarch, RISCV_LAST_REGNUM + 1);
 
-  /* We don't have to provide the count of 0 here (its the default) but
-     include this line to make it explicit that, right now, we don't have
-     any pseudo registers on RISC-V.  */
-  set_gdbarch_num_pseudo_regs (gdbarch, 0);
-
   /* Some specific register numbers GDB likes to know about.  */
   set_gdbarch_sp_regnum (gdbarch, RISCV_SP_REGNUM);
   set_gdbarch_pc_regnum (gdbarch, RISCV_PC_REGNUM);
 
   set_gdbarch_print_registers_info (gdbarch, riscv_print_registers_info);
 
+  set_tdesc_pseudo_register_name (gdbarch, riscv_pseudo_register_name);
+  set_tdesc_pseudo_register_type (gdbarch, riscv_pseudo_register_type);
+  set_tdesc_pseudo_register_reggroup_p (gdbarch,
+					riscv_pseudo_register_reggroup_p);
+  set_gdbarch_pseudo_register_read (gdbarch, riscv_pseudo_register_read);
+  set_gdbarch_pseudo_register_write (gdbarch, riscv_pseudo_register_write);
+
   /* Finalise the target description registers.  */
   tdesc_use_registers (gdbarch, tdesc, std::move (tdesc_data),
 		       riscv_tdesc_unknown_reg);
 
+  /* Calculate the number of pseudo registers we need.  The fflags and frm
+     registers are sub-fields of the fcsr CSR register (csr3).  However,
+     these registers can also be accessed directly as separate CSR
+     registers (fflags is csr1, and frm is csr2).  And so, some targets
+     might choose to offer direct access to all three registers in the
+     target description, while other targets might choose to only offer
+     access to fcsr.
+
+     As we scan the target description we spot which of fcsr, fflags, and
+     frm are available.  If fcsr is available but either of fflags and/or
+     frm are not available, then we add pseudo-registers to provide the
+     missing functionality.
+
+     This has to be done after the call to tdesc_use_registers as we don't
+     know the final register number until after that call, and the pseudo
+     register numbers need to be after the physical registers.  */
+  int num_pseudo_regs = 0;
+  int next_pseudo_regnum = gdbarch_num_regs (gdbarch);
+
+  if (features.has_fflags_reg)
+    tdep->fflags_regnum = RISCV_CSR_FFLAGS_REGNUM;
+  else if (features.has_fcsr_reg)
+    {
+      tdep->fflags_regnum = next_pseudo_regnum;
+      pending_aliases.emplace_back ("csr1", (void *) &tdep->fflags_regnum);
+      next_pseudo_regnum++;
+      num_pseudo_regs++;
+    }
+
+  if (features.has_frm_reg)
+    tdep->frm_regnum = RISCV_CSR_FRM_REGNUM;
+  else if (features.has_fcsr_reg)
+    {
+      tdep->frm_regnum = next_pseudo_regnum;
+      pending_aliases.emplace_back ("csr2", (void *) &tdep->frm_regnum);
+      next_pseudo_regnum++;
+      num_pseudo_regs++;
+    }
+
+  set_gdbarch_num_pseudo_regs (gdbarch, num_pseudo_regs);
+
   /* Override the register type callback setup by the target description
      mechanism.  This allows us to provide special type for floating point
      registers.  */
@@ -4033,8 +4189,12 @@ riscv_supply_regset (const struct regset *regset,
   if (regnum == -1 || regnum == RISCV_ZERO_REGNUM)
     regcache->raw_supply_zeroed (RISCV_ZERO_REGNUM);
 
-  if (regnum == -1 || regnum == RISCV_CSR_FFLAGS_REGNUM
-      || regnum == RISCV_CSR_FRM_REGNUM)
+  struct gdbarch *gdbarch = regcache->arch ();
+  riscv_gdbarch_tdep *tdep = gdbarch_tdep<riscv_gdbarch_tdep> (gdbarch);
+
+  if (regnum == -1
+      || regnum == tdep->fflags_regnum
+      || regnum == tdep->frm_regnum)
     {
       int fcsr_regnum = RISCV_CSR_FCSR_REGNUM;
 
@@ -4048,6 +4208,12 @@ riscv_supply_regset (const struct regset *regset,
 	 registers.  */
       if (regcache->get_register_status (fcsr_regnum) == REG_VALID)
 	{
+	  /* If we have an fcsr register then we should have fflags and frm
+	     too, either provided by the target, or provided as a pseudo
+	     register by GDB.  */
+	  gdb_assert (tdep->fflags_regnum >= 0);
+	  gdb_assert (tdep->frm_regnum >= 0);
+
 	  ULONGEST fcsr_val;
 	  regcache->raw_read (fcsr_regnum, &fcsr_val);
 
@@ -4055,15 +4221,19 @@ riscv_supply_regset (const struct regset *regset,
 	  ULONGEST fflags_val = fcsr_val & 0x1f;
 	  ULONGEST frm_val = (fcsr_val >> 5) & 0x7;
 
-	  /* And supply these if needed.  */
-	  if (regnum == -1 || regnum == RISCV_CSR_FFLAGS_REGNUM)
-	    regcache->raw_supply_integer (RISCV_CSR_FFLAGS_REGNUM,
+	  /* And supply these if needed.  We can only supply real
+	     registers, so don't try to supply fflags or frm if they are
+	     implemented as pseudo-registers.  */
+	  if ((regnum == -1 || regnum == tdep->fflags_regnum)
+	      && tdep->fflags_regnum < gdbarch_num_regs (gdbarch))
+	    regcache->raw_supply_integer (tdep->fflags_regnum,
 					  (gdb_byte *) &fflags_val,
 					  sizeof (fflags_val),
 					  /* is_signed */ false);
 
-	  if (regnum == -1 || regnum == RISCV_CSR_FRM_REGNUM)
-	    regcache->raw_supply_integer (RISCV_CSR_FRM_REGNUM,
+	  if ((regnum == -1 || regnum == tdep->frm_regnum)
+	      && tdep->frm_regnum < gdbarch_num_regs (gdbarch))
+	    regcache->raw_supply_integer (tdep->frm_regnum,
 					  (gdb_byte *)&frm_val,
 					  sizeof (fflags_val),
 					  /* is_signed */ false);
diff --git a/gdb/riscv-tdep.h b/gdb/riscv-tdep.h
index 826a002ef92..3ae126f4d8a 100644
--- a/gdb/riscv-tdep.h
+++ b/gdb/riscv-tdep.h
@@ -90,6 +90,12 @@ struct riscv_gdbarch_tdep : gdbarch_tdep_base
   /* ISA-specific data types.  */
   struct type *riscv_fpreg_d_type = nullptr;
 
+  /* The location of these registers, set to -2 by default so we don't
+     match against -1 which is frequently used to mean "all registers",
+     e.g. in the regcache supply/collect code.  */
+  int fflags_regnum = -2;
+  int frm_regnum = -2;
+
   /* 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
diff --git a/gdb/testsuite/gdb.arch/riscv-info-fcsr.exp b/gdb/testsuite/gdb.arch/riscv-info-fcsr.exp
index 3d6095d7e55..bf319650db3 100644
--- a/gdb/testsuite/gdb.arch/riscv-info-fcsr.exp
+++ b/gdb/testsuite/gdb.arch/riscv-info-fcsr.exp
@@ -73,17 +73,6 @@ proc check_fcsr { fflags_value frm_value frm_string } {
 	    exp_continue
 	}
 
-	-re "^(fflags\|frm)\\s+<unavailable>\r\n" {
-	    # Currently, on some targets (e.g. RISC-V native Linux) the
-	    # fflags and frm registers show as being available, but are
-	    # unreadable, the result is these registers report
-	    # themselves as <unavailable>.  So long as fcsr is readable
-	    # (which is checked below), then for now we accept this.
-	    set reg_name $expect_out(1,string)
-	    incr reg_counts($reg_name)
-	    exp_continue
-	}
-
 	-re "^(frm)\\s+${frm_value}\\s+${frm_pattern}\r\n" {
 	    set reg_name $expect_out(1,string)
 	    incr reg_counts($reg_name)
@@ -129,6 +118,19 @@ proc test_fcsr { fflags_value frm_value frm_string } {
 	with_test_prefix "set through fcsr" {
 	    check_fcsr $fflags_value $frm_value $frm_string
 	}
+
+	# Reset fcsr register back to zero.
+	gdb_test_no_output "set \$fcsr = 0x0" \
+	    "reset fcsr back to 0x0"
+	gdb_test "p/x \$fcsr" " = 0x0"
+
+	# Now set fcsr value through fflags and frm.
+	gdb_test_no_output "set \$fflags = ${fflags_value}"
+	gdb_test_no_output "set \$frm = ${frm_value}"
+
+	with_test_prefix "set through fflags and frm" {
+	    check_fcsr $fflags_value $frm_value $frm_string
+	}
     }
 }
 
diff --git a/gdb/testsuite/gdb.arch/riscv-tdesc-fcsr-32.xml b/gdb/testsuite/gdb.arch/riscv-tdesc-fcsr-32.xml
new file mode 100644
index 00000000000..844f8e354d9
--- /dev/null
+++ b/gdb/testsuite/gdb.arch/riscv-tdesc-fcsr-32.xml
@@ -0,0 +1,75 @@
+<?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="fcsr" bitsize="32" type="int"/>
+  </feature>
+</target>
diff --git a/gdb/testsuite/gdb.arch/riscv-tdesc-fcsr-64.xml b/gdb/testsuite/gdb.arch/riscv-tdesc-fcsr-64.xml
new file mode 100644
index 00000000000..130e308b903
--- /dev/null
+++ b/gdb/testsuite/gdb.arch/riscv-tdesc-fcsr-64.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="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="fcsr" bitsize="32" type="int"/>
+  </feature>
+</target>
diff --git a/gdb/testsuite/gdb.arch/riscv-tdesc-loading-05.xml b/gdb/testsuite/gdb.arch/riscv-tdesc-loading-05.xml
new file mode 100644
index 00000000000..62b472edba6
--- /dev/null
+++ b/gdb/testsuite/gdb.arch/riscv-tdesc-loading-05.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"/>
+  </feature>
+  <feature name="org.gnu.gdb.riscv.csr">
+    <reg name="fcsr" bitsize="32" type="int"/>
+  </feature>
+</target>
diff --git a/gdb/testsuite/gdb.arch/riscv-tdesc-loading-06.xml b/gdb/testsuite/gdb.arch/riscv-tdesc-loading-06.xml
new file mode 100644
index 00000000000..844f8e354d9
--- /dev/null
+++ b/gdb/testsuite/gdb.arch/riscv-tdesc-loading-06.xml
@@ -0,0 +1,75 @@
+<?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="fcsr" bitsize="32" type="int"/>
+  </feature>
+</target>
diff --git a/gdb/testsuite/gdb.arch/riscv-tdesc-regs.exp b/gdb/testsuite/gdb.arch/riscv-tdesc-regs.exp
index 7f6bc45039f..4bacaaef99c 100644
--- a/gdb/testsuite/gdb.arch/riscv-tdesc-regs.exp
+++ b/gdb/testsuite/gdb.arch/riscv-tdesc-regs.exp
@@ -137,3 +137,39 @@ foreach rgroup {x_all all save restore general system csr} {
     }
     array unset reg_counts
 }
+
+# Next load a target description that contains fcsr, but not fflags or
+# frm.  Then check that GDB provides an fflags and frm registers using
+# the pseudo-register mechanism.
+if { $xlen == 4 } {
+    set xml_tdesc "riscv-tdesc-fcsr-32.xml"
+} else {
+    set xml_tdesc "riscv-tdesc-fcsr-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 target description that lacks fflags and frm"
+
+foreach reg {fflags frm} {
+    gdb_test_multiple "info registers $reg" "" {
+	-re "^info registers $reg\r\n" {
+	    exp_continue
+	}
+
+	-wrap -re "^Invalid register `$reg`" {
+	    fail $gdb_test_name
+	}
+
+	-wrap -re "^$reg\\s+\[^\r\n\]+" {
+	    pass $gdb_test_name
+	}
+    }
+}
diff --git a/gdb/testsuite/gdb.base/float.exp b/gdb/testsuite/gdb.base/float.exp
index 62e8346928b..7fff2b90727 100644
--- a/gdb/testsuite/gdb.base/float.exp
+++ b/gdb/testsuite/gdb.base/float.exp
@@ -111,11 +111,18 @@ if { [is_aarch64_target] } then {
 } elseif [istarget "sparc*-*-*"] then {
     gdb_test "info float" "f0.*f1.*f31.*d0.*d30.*" "info float"
 } elseif [istarget "riscv*-*-*"] then {
-    # RISC-V may or may not have an FPU
+    # RISC-V may or may not have an FPU.  Additionally, the order of
+    # fcsr relative to fflags and frm can change depending on whether
+    # the fflags and frm registers are implemented as real registers
+    # (supplied in the target description) or pseudo-registers
+    # (supplied by GDB as a view into fcsr).
     gdb_test_multiple "info float" "info float" {
 	-re "ft0.*ft1.*ft11.*fflags.*frm.*fcsr.*$gdb_prompt $" {
 	      pass "info float (with FPU)"
 	  }
+	-re "ft0.*ft1.*ft11.*fcsr.*fflags.*frm.*$gdb_prompt $" {
+	      pass "info float (with FPU)"
+	  }
 	-re "No floating.point info available for this processor.*$gdb_prompt $" {
 	      pass "info float (without FPU)"
 	}


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

* Re: [PATCH 0/3] RISC-V fflags, frm, and fcsr (includes small tdesc change)
  2022-08-16 10:09 [PATCH 0/3] RISC-V fflags, frm, and fcsr (includes small tdesc change) Andrew Burgess
                   ` (3 preceding siblings ...)
  2022-08-16 16:26 ` [PATCH 0/3] RISC-V fflags, frm, and fcsr (includes small tdesc change) John Baldwin
@ 2022-08-31 15:33 ` Andrew Burgess
  4 siblings, 0 replies; 7+ messages in thread
From: Andrew Burgess @ 2022-08-31 15:33 UTC (permalink / raw)
  To: gdb-patches

Andrew Burgess <aburgess@redhat.com> writes:

> While testing on a RISC-V native Linux target, I noticed some issues
> accessing the fflags and frm registers.  This series addresses these issues.
>
> Patch #2 is a small extension to the target description mechanism
> required to support patch #3.

I've now pushed this series.

Thanks,
Andrew


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

end of thread, other threads:[~2022-08-31 15:33 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-08-16 10:09 [PATCH 0/3] RISC-V fflags, frm, and fcsr (includes small tdesc change) Andrew Burgess
2022-08-16 10:09 ` [PATCH 1/3] gdb/riscv: improve (and fix) display of frm field in 'info registers' Andrew Burgess
2022-08-16 10:09 ` [PATCH 2/3] gdb: Add tdesc_found_register function to tdesc API Andrew Burgess
2022-08-16 10:09 ` [PATCH 3/3] gdb/riscv: better support for fflags and frm registers Andrew Burgess
2022-08-31 15:11   ` Andrew Burgess
2022-08-16 16:26 ` [PATCH 0/3] RISC-V fflags, frm, and fcsr (includes small tdesc change) John Baldwin
2022-08-31 15:33 ` 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).