diff mbox series

[PULL,06/12] target/riscv: remove fflags, frm, and fcsr from riscv-*-fpu.xml

Message ID 20220923040704.428285-7-alistair.francis@opensource.wdc.com (mailing list archive)
State New, archived
Headers show
Series [PULL,01/12] hw/ssi: ibex_spi: fixup typos in ibex_spi_host | expand

Commit Message

Alistair Francis Sept. 23, 2022, 4:06 a.m. UTC 
From: Andrew Burgess <aburgess@redhat.com>

While testing some changes to GDB's handling for the RISC-V registers
fcsr, fflags, and frm, I spotted that QEMU includes these registers
twice in the target description it sends to GDB, once in the fpu
feature, and once in the csr feature.

Right now things basically work OK, QEMU maps these registers onto two
different register numbers, e.g. fcsr maps to both 68 and 73, and GDB
can use either of these to access the register.

However, GDB's target descriptions don't really work this way, each
register should appear just once in a target description, mapping the
register name onto the number GDB should use when accessing the
register on the target.  Duplicate register names actually result in
duplicate registers on the GDB side, however, as the registers have
the same name, the user can only access one of these registers.

Currently GDB has a hack in place, specifically for RISC-V, to spot
the duplicate copies of these three registers, and hide them from the
user, ensuring the user only ever sees a single copy of each.

In this commit I propose fixing this issue on the QEMU side, and in
the process, simplify the fpu register handling a little.

I think we should, remove fflags, frm, and fcsr from the two (32-bit
and 64-bit) fpu feature xml files.  These files will only contain the
32 core floating point register f0 to f31.  The fflags, frm, and fcsr
registers will continue to be advertised in the csr feature as they
currently are.

With that change made, I will simplify riscv_gdb_get_fpu and
riscv_gdb_set_fpu, removing the extra handling for the 3 status
registers.

Signed-off-by: Andrew Burgess <aburgess@redhat.com>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Message-Id: <0fbf2a5b12e3210ff3867d5cf7022b3f3462c9c8.1661934573.git.aburgess@redhat.com>
Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
---
 target/riscv/gdbstub.c      | 32 ++------------------------------
 gdb-xml/riscv-32bit-fpu.xml |  4 ----
 gdb-xml/riscv-64bit-fpu.xml |  4 ----
 3 files changed, 2 insertions(+), 38 deletions(-)
diff mbox series

Patch

diff --git a/target/riscv/gdbstub.c b/target/riscv/gdbstub.c
index 9ed049c29e..9974b7aac6 100644
--- a/target/riscv/gdbstub.c
+++ b/target/riscv/gdbstub.c
@@ -114,20 +114,6 @@  static int riscv_gdb_get_fpu(CPURISCVState *env, GByteArray *buf, int n)
         if (env->misa_ext & RVF) {
             return gdb_get_reg32(buf, env->fpr[n]);
         }
-    /* there is hole between ft11 and fflags in fpu.xml */
-    } else if (n < 36 && n > 32) {
-        target_ulong val = 0;
-        int result;
-        /*
-         * CSR_FFLAGS is at index 1 in csr_register, and gdb says it is FP
-         * register 33, so we recalculate the map index.
-         * This also works for CSR_FRM and CSR_FCSR.
-         */
-        result = riscv_csrrw_debug(env, n - 32, &val,
-                                   0, 0);
-        if (result == RISCV_EXCP_NONE) {
-            return gdb_get_regl(buf, val);
-        }
     }
     return 0;
 }
@@ -137,20 +123,6 @@  static int riscv_gdb_set_fpu(CPURISCVState *env, uint8_t *mem_buf, int n)
     if (n < 32) {
         env->fpr[n] = ldq_p(mem_buf); /* always 64-bit */
         return sizeof(uint64_t);
-    /* there is hole between ft11 and fflags in fpu.xml */
-    } else if (n < 36 && n > 32) {
-        target_ulong val = ldtul_p(mem_buf);
-        int result;
-        /*
-         * CSR_FFLAGS is at index 1 in csr_register, and gdb says it is FP
-         * register 33, so we recalculate the map index.
-         * This also works for CSR_FRM and CSR_FCSR.
-         */
-        result = riscv_csrrw_debug(env, n - 32, NULL,
-                                   val, -1);
-        if (result == RISCV_EXCP_NONE) {
-            return sizeof(target_ulong);
-        }
     }
     return 0;
 }
@@ -404,10 +376,10 @@  void riscv_cpu_register_gdb_regs_for_features(CPUState *cs)
     CPURISCVState *env = &cpu->env;
     if (env->misa_ext & RVD) {
         gdb_register_coprocessor(cs, riscv_gdb_get_fpu, riscv_gdb_set_fpu,
-                                 36, "riscv-64bit-fpu.xml", 0);
+                                 32, "riscv-64bit-fpu.xml", 0);
     } else if (env->misa_ext & RVF) {
         gdb_register_coprocessor(cs, riscv_gdb_get_fpu, riscv_gdb_set_fpu,
-                                 36, "riscv-32bit-fpu.xml", 0);
+                                 32, "riscv-32bit-fpu.xml", 0);
     }
     if (env->misa_ext & RVV) {
         gdb_register_coprocessor(cs, riscv_gdb_get_vector, riscv_gdb_set_vector,
diff --git a/gdb-xml/riscv-32bit-fpu.xml b/gdb-xml/riscv-32bit-fpu.xml
index 1eaae9119e..84a44ba8df 100644
--- a/gdb-xml/riscv-32bit-fpu.xml
+++ b/gdb-xml/riscv-32bit-fpu.xml
@@ -43,8 +43,4 @@ 
   <reg name="ft9" bitsize="32" type="ieee_single"/>
   <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-xml/riscv-64bit-fpu.xml b/gdb-xml/riscv-64bit-fpu.xml
index 794854cc01..9856a9d1d3 100644
--- a/gdb-xml/riscv-64bit-fpu.xml
+++ b/gdb-xml/riscv-64bit-fpu.xml
@@ -49,8 +49,4 @@ 
   <reg name="ft9" bitsize="64" type="riscv_double"/>
   <reg name="ft10" bitsize="64" type="riscv_double"/>
   <reg name="ft11" bitsize="64" type="riscv_double"/>
-
-  <reg name="fflags" bitsize="32" type="int" regnum="66"/>
-  <reg name="frm" bitsize="32" type="int" regnum="67"/>
-  <reg name="fcsr" bitsize="32" type="int" regnum="68"/>
 </feature>