[PATCH 0/2] target/riscv: improvements to GDB target descriptions

[PATCH 0/2] target/riscv: improvements to GDB target descriptions

[Andrew Burgess]

I was running some GDB tests against QEMU, and noticed some oddities
with the target description QEMU sends, the following two patches
address these issues.

Thanks,
Andrew

---

Andrew Burgess (2):
  target/riscv: remove fflags, frm, and fcsr from riscv-*-fpu.xml
  target/riscv: remove fixed numbering from GDB xml feature files

 gdb-xml/riscv-32bit-cpu.xml |  6 +-----
 gdb-xml/riscv-32bit-fpu.xml | 10 +---------
 gdb-xml/riscv-64bit-cpu.xml |  6 +-----
 gdb-xml/riscv-64bit-fpu.xml | 10 +---------
 target/riscv/gdbstub.c      | 32 ++------------------------------
 5 files changed, 6 insertions(+), 58 deletions(-)

-- 
2.25.4

[PATCH 1/2] target/riscv: remove fflags, frm, and fcsr from riscv-*-fpu.xml

[Andrew Burgess]

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>
---
 gdb-xml/riscv-32bit-fpu.xml |  4 ----
 gdb-xml/riscv-64bit-fpu.xml |  4 ----
 target/riscv/gdbstub.c      | 32 ++------------------------------
 3 files changed, 2 insertions(+), 38 deletions(-)

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>
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,
-- 
2.25.4

[PATCH 2/2] target/riscv: remove fixed numbering from GDB xml feature files

[Andrew Burgess]

The fixed register numbering in the various GDB feature files for
RISC-V only exists because these files were originally copied from the
GDB source tree.

However, the fixed numbering only exists in the GDB source tree so
that GDB, when it connects to a target that doesn't provide a target
description, will use a specific numbering scheme.

That numbering scheme is designed to be compatible with the first
versions of QEMU (for RISC-V), that didn't send a target description,
and relied on a fixed numbering scheme.

Because of the way that QEMU manages its target descriptions,
recording the number of registers in each feature, and just relying on
GDB's numbering starting from 0, then I propose that we remove all the
fixed numbering from the RISC-V feature xml files, and just rely on
the standard numbering scheme.  Plenty of other targets manage their
xml files this way, e.g. ARM, AArch64, Loongarch, m68k, rx, and s390.

Signed-off-by: Andrew Burgess <aburgess@redhat.com>
---
 gdb-xml/riscv-32bit-cpu.xml | 6 +-----
 gdb-xml/riscv-32bit-fpu.xml | 6 +-----
 gdb-xml/riscv-64bit-cpu.xml | 6 +-----
 gdb-xml/riscv-64bit-fpu.xml | 6 +-----
 4 files changed, 4 insertions(+), 20 deletions(-)

diff --git a/gdb-xml/riscv-32bit-cpu.xml b/gdb-xml/riscv-32bit-cpu.xml
index 0d07aaec85..466f2c0648 100644
--- a/gdb-xml/riscv-32bit-cpu.xml
+++ b/gdb-xml/riscv-32bit-cpu.xml
@@ -5,13 +5,9 @@
      are permitted in any medium without royalty provided the copyright
      notice and this notice are preserved.  -->
 
-<!-- Register numbers are hard-coded in order to maintain backward
-     compatibility with older versions of tools that didn't use xml
-     register descriptions.  -->
-
 <!DOCTYPE feature SYSTEM "gdb-target.dtd">
 <feature name="org.gnu.gdb.riscv.cpu">
-  <reg name="zero" bitsize="32" type="int" regnum="0"/>
+  <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"/>
diff --git a/gdb-xml/riscv-32bit-fpu.xml b/gdb-xml/riscv-32bit-fpu.xml
index 84a44ba8df..24aa087031 100644
--- a/gdb-xml/riscv-32bit-fpu.xml
+++ b/gdb-xml/riscv-32bit-fpu.xml
@@ -5,13 +5,9 @@
      are permitted in any medium without royalty provided the copyright
      notice and this notice are preserved.  -->
 
-<!-- Register numbers are hard-coded in order to maintain backward
-     compatibility with older versions of tools that didn't use xml
-     register descriptions.  -->
-
 <!DOCTYPE feature SYSTEM "gdb-target.dtd">
 <feature name="org.gnu.gdb.riscv.fpu">
-  <reg name="ft0" bitsize="32" type="ieee_single" regnum="33"/>
+  <reg name="ft0" bitsize="32" type="ieee_single"/>
   <reg name="ft1" bitsize="32" type="ieee_single"/>
   <reg name="ft2" bitsize="32" type="ieee_single"/>
   <reg name="ft3" bitsize="32" type="ieee_single"/>
diff --git a/gdb-xml/riscv-64bit-cpu.xml b/gdb-xml/riscv-64bit-cpu.xml
index b8aa424ae4..c4d83de09b 100644
--- a/gdb-xml/riscv-64bit-cpu.xml
+++ b/gdb-xml/riscv-64bit-cpu.xml
@@ -5,13 +5,9 @@
      are permitted in any medium without royalty provided the copyright
      notice and this notice are preserved.  -->
 
-<!-- Register numbers are hard-coded in order to maintain backward
-     compatibility with older versions of tools that didn't use xml
-     register descriptions.  -->
-
 <!DOCTYPE feature SYSTEM "gdb-target.dtd">
 <feature name="org.gnu.gdb.riscv.cpu">
-  <reg name="zero" bitsize="64" type="int" regnum="0"/>
+  <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"/>
diff --git a/gdb-xml/riscv-64bit-fpu.xml b/gdb-xml/riscv-64bit-fpu.xml
index 9856a9d1d3..d0f17f9984 100644
--- a/gdb-xml/riscv-64bit-fpu.xml
+++ b/gdb-xml/riscv-64bit-fpu.xml
@@ -5,10 +5,6 @@
      are permitted in any medium without royalty provided the copyright
      notice and this notice are preserved.  -->
 
-<!-- Register numbers are hard-coded in order to maintain backward
-     compatibility with older versions of tools that didn't use xml
-     register descriptions.  -->
-
 <!DOCTYPE feature SYSTEM "gdb-target.dtd">
 <feature name="org.gnu.gdb.riscv.fpu">
 
@@ -17,7 +13,7 @@
     <field name="double" type="ieee_double"/>
   </union>
 
-  <reg name="ft0" bitsize="64" type="riscv_double" regnum="33"/>
+  <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"/>
-- 
2.25.4

Re: [PATCH 1/2] target/riscv: remove fflags, frm, and fcsr from riscv-*-fpu.xml

[Alistair Francis]

On Wed, Aug 31, 2022 at 10:43 AM Andrew Burgess <aburgess@redhat.com> wrote:
>
> 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>

Alistair

> ---
>  gdb-xml/riscv-32bit-fpu.xml |  4 ----
>  gdb-xml/riscv-64bit-fpu.xml |  4 ----
>  target/riscv/gdbstub.c      | 32 ++------------------------------
>  3 files changed, 2 insertions(+), 38 deletions(-)
>
> 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>
> 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,
> --
> 2.25.4
>
>

Re: [PATCH 2/2] target/riscv: remove fixed numbering from GDB xml feature files

[Alistair Francis]

On Wed, Aug 31, 2022 at 10:45 AM Andrew Burgess <aburgess@redhat.com> wrote:
>
> The fixed register numbering in the various GDB feature files for
> RISC-V only exists because these files were originally copied from the
> GDB source tree.
>
> However, the fixed numbering only exists in the GDB source tree so
> that GDB, when it connects to a target that doesn't provide a target
> description, will use a specific numbering scheme.
>
> That numbering scheme is designed to be compatible with the first
> versions of QEMU (for RISC-V), that didn't send a target description,
> and relied on a fixed numbering scheme.
>
> Because of the way that QEMU manages its target descriptions,
> recording the number of registers in each feature, and just relying on
> GDB's numbering starting from 0, then I propose that we remove all the
> fixed numbering from the RISC-V feature xml files, and just rely on
> the standard numbering scheme.  Plenty of other targets manage their
> xml files this way, e.g. ARM, AArch64, Loongarch, m68k, rx, and s390.
>
> Signed-off-by: Andrew Burgess <aburgess@redhat.com>

Acked-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  gdb-xml/riscv-32bit-cpu.xml | 6 +-----
>  gdb-xml/riscv-32bit-fpu.xml | 6 +-----
>  gdb-xml/riscv-64bit-cpu.xml | 6 +-----
>  gdb-xml/riscv-64bit-fpu.xml | 6 +-----
>  4 files changed, 4 insertions(+), 20 deletions(-)
>
> diff --git a/gdb-xml/riscv-32bit-cpu.xml b/gdb-xml/riscv-32bit-cpu.xml
> index 0d07aaec85..466f2c0648 100644
> --- a/gdb-xml/riscv-32bit-cpu.xml
> +++ b/gdb-xml/riscv-32bit-cpu.xml
> @@ -5,13 +5,9 @@
>       are permitted in any medium without royalty provided the copyright
>       notice and this notice are preserved.  -->
>
> -<!-- Register numbers are hard-coded in order to maintain backward
> -     compatibility with older versions of tools that didn't use xml
> -     register descriptions.  -->
> -
>  <!DOCTYPE feature SYSTEM "gdb-target.dtd">
>  <feature name="org.gnu.gdb.riscv.cpu">
> -  <reg name="zero" bitsize="32" type="int" regnum="0"/>
> +  <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"/>
> diff --git a/gdb-xml/riscv-32bit-fpu.xml b/gdb-xml/riscv-32bit-fpu.xml
> index 84a44ba8df..24aa087031 100644
> --- a/gdb-xml/riscv-32bit-fpu.xml
> +++ b/gdb-xml/riscv-32bit-fpu.xml
> @@ -5,13 +5,9 @@
>       are permitted in any medium without royalty provided the copyright
>       notice and this notice are preserved.  -->
>
> -<!-- Register numbers are hard-coded in order to maintain backward
> -     compatibility with older versions of tools that didn't use xml
> -     register descriptions.  -->
> -
>  <!DOCTYPE feature SYSTEM "gdb-target.dtd">
>  <feature name="org.gnu.gdb.riscv.fpu">
> -  <reg name="ft0" bitsize="32" type="ieee_single" regnum="33"/>
> +  <reg name="ft0" bitsize="32" type="ieee_single"/>
>    <reg name="ft1" bitsize="32" type="ieee_single"/>
>    <reg name="ft2" bitsize="32" type="ieee_single"/>
>    <reg name="ft3" bitsize="32" type="ieee_single"/>
> diff --git a/gdb-xml/riscv-64bit-cpu.xml b/gdb-xml/riscv-64bit-cpu.xml
> index b8aa424ae4..c4d83de09b 100644
> --- a/gdb-xml/riscv-64bit-cpu.xml
> +++ b/gdb-xml/riscv-64bit-cpu.xml
> @@ -5,13 +5,9 @@
>       are permitted in any medium without royalty provided the copyright
>       notice and this notice are preserved.  -->
>
> -<!-- Register numbers are hard-coded in order to maintain backward
> -     compatibility with older versions of tools that didn't use xml
> -     register descriptions.  -->
> -
>  <!DOCTYPE feature SYSTEM "gdb-target.dtd">
>  <feature name="org.gnu.gdb.riscv.cpu">
> -  <reg name="zero" bitsize="64" type="int" regnum="0"/>
> +  <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"/>
> diff --git a/gdb-xml/riscv-64bit-fpu.xml b/gdb-xml/riscv-64bit-fpu.xml
> index 9856a9d1d3..d0f17f9984 100644
> --- a/gdb-xml/riscv-64bit-fpu.xml
> +++ b/gdb-xml/riscv-64bit-fpu.xml
> @@ -5,10 +5,6 @@
>       are permitted in any medium without royalty provided the copyright
>       notice and this notice are preserved.  -->
>
> -<!-- Register numbers are hard-coded in order to maintain backward
> -     compatibility with older versions of tools that didn't use xml
> -     register descriptions.  -->
> -
>  <!DOCTYPE feature SYSTEM "gdb-target.dtd">
>  <feature name="org.gnu.gdb.riscv.fpu">
>
> @@ -17,7 +13,7 @@
>      <field name="double" type="ieee_double"/>
>    </union>
>
> -  <reg name="ft0" bitsize="64" type="riscv_double" regnum="33"/>
> +  <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"/>
> --
> 2.25.4
>
>

Re: [PATCH 2/2] target/riscv: remove fixed numbering from GDB xml feature files

[Palmer Dabbelt]

On Wed, 31 Aug 2022 01:41:23 PDT (-0700), aburgess@redhat.com wrote:
> The fixed register numbering in the various GDB feature files for
> RISC-V only exists because these files were originally copied from the
> GDB source tree.
>
> However, the fixed numbering only exists in the GDB source tree so
> that GDB, when it connects to a target that doesn't provide a target
> description, will use a specific numbering scheme.
>
> That numbering scheme is designed to be compatible with the first
> versions of QEMU (for RISC-V), that didn't send a target description,
> and relied on a fixed numbering scheme.
>
> Because of the way that QEMU manages its target descriptions,
> recording the number of registers in each feature, and just relying on
> GDB's numbering starting from 0, then I propose that we remove all the
> fixed numbering from the RISC-V feature xml files, and just rely on
> the standard numbering scheme.  Plenty of other targets manage their
> xml files this way, e.g. ARM, AArch64, Loongarch, m68k, rx, and s390.
>
> Signed-off-by: Andrew Burgess <aburgess@redhat.com>
> ---
>  gdb-xml/riscv-32bit-cpu.xml | 6 +-----
>  gdb-xml/riscv-32bit-fpu.xml | 6 +-----
>  gdb-xml/riscv-64bit-cpu.xml | 6 +-----
>  gdb-xml/riscv-64bit-fpu.xml | 6 +-----
>  4 files changed, 4 insertions(+), 20 deletions(-)
>
> diff --git a/gdb-xml/riscv-32bit-cpu.xml b/gdb-xml/riscv-32bit-cpu.xml
> index 0d07aaec85..466f2c0648 100644
> --- a/gdb-xml/riscv-32bit-cpu.xml
> +++ b/gdb-xml/riscv-32bit-cpu.xml
> @@ -5,13 +5,9 @@
>       are permitted in any medium without royalty provided the copyright
>       notice and this notice are preserved.  -->
>
> -<!-- Register numbers are hard-coded in order to maintain backward
> -     compatibility with older versions of tools that didn't use xml
> -     register descriptions.  -->
> -
>  <!DOCTYPE feature SYSTEM "gdb-target.dtd">
>  <feature name="org.gnu.gdb.riscv.cpu">
> -  <reg name="zero" bitsize="32" type="int" regnum="0"/>
> +  <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"/>
> diff --git a/gdb-xml/riscv-32bit-fpu.xml b/gdb-xml/riscv-32bit-fpu.xml
> index 84a44ba8df..24aa087031 100644
> --- a/gdb-xml/riscv-32bit-fpu.xml
> +++ b/gdb-xml/riscv-32bit-fpu.xml
> @@ -5,13 +5,9 @@
>       are permitted in any medium without royalty provided the copyright
>       notice and this notice are preserved.  -->
>
> -<!-- Register numbers are hard-coded in order to maintain backward
> -     compatibility with older versions of tools that didn't use xml
> -     register descriptions.  -->
> -
>  <!DOCTYPE feature SYSTEM "gdb-target.dtd">
>  <feature name="org.gnu.gdb.riscv.fpu">
> -  <reg name="ft0" bitsize="32" type="ieee_single" regnum="33"/>
> +  <reg name="ft0" bitsize="32" type="ieee_single"/>
>    <reg name="ft1" bitsize="32" type="ieee_single"/>
>    <reg name="ft2" bitsize="32" type="ieee_single"/>
>    <reg name="ft3" bitsize="32" type="ieee_single"/>
> diff --git a/gdb-xml/riscv-64bit-cpu.xml b/gdb-xml/riscv-64bit-cpu.xml
> index b8aa424ae4..c4d83de09b 100644
> --- a/gdb-xml/riscv-64bit-cpu.xml
> +++ b/gdb-xml/riscv-64bit-cpu.xml
> @@ -5,13 +5,9 @@
>       are permitted in any medium without royalty provided the copyright
>       notice and this notice are preserved.  -->
>
> -<!-- Register numbers are hard-coded in order to maintain backward
> -     compatibility with older versions of tools that didn't use xml
> -     register descriptions.  -->
> -
>  <!DOCTYPE feature SYSTEM "gdb-target.dtd">
>  <feature name="org.gnu.gdb.riscv.cpu">
> -  <reg name="zero" bitsize="64" type="int" regnum="0"/>
> +  <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"/>
> diff --git a/gdb-xml/riscv-64bit-fpu.xml b/gdb-xml/riscv-64bit-fpu.xml
> index 9856a9d1d3..d0f17f9984 100644
> --- a/gdb-xml/riscv-64bit-fpu.xml
> +++ b/gdb-xml/riscv-64bit-fpu.xml
> @@ -5,10 +5,6 @@
>       are permitted in any medium without royalty provided the copyright
>       notice and this notice are preserved.  -->
>
> -<!-- Register numbers are hard-coded in order to maintain backward
> -     compatibility with older versions of tools that didn't use xml
> -     register descriptions.  -->
> -
>  <!DOCTYPE feature SYSTEM "gdb-target.dtd">
>  <feature name="org.gnu.gdb.riscv.fpu">
>
> @@ -17,7 +13,7 @@
>      <field name="double" type="ieee_double"/>
>    </union>
>
> -  <reg name="ft0" bitsize="64" type="riscv_double" regnum="33"/>
> +  <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"/>

Reviewed-by: Palmer Dabbelt <palmer@rivosinc.com>

Thanks.

Re: [PATCH 0/2] target/riscv: improvements to GDB target descriptions

[Alistair Francis]

On Wed, Aug 31, 2022 at 6:43 PM Andrew Burgess <aburgess@redhat.com> wrote:
>
> I was running some GDB tests against QEMU, and noticed some oddities
> with the target description QEMU sends, the following two patches
> address these issues.
>
> Thanks,
> Andrew
>
> ---
>
> Andrew Burgess (2):
>   target/riscv: remove fflags, frm, and fcsr from riscv-*-fpu.xml
>   target/riscv: remove fixed numbering from GDB xml feature files

Thanks!

Applied to riscv-to-apply.next

Alistair

>
>  gdb-xml/riscv-32bit-cpu.xml |  6 +-----
>  gdb-xml/riscv-32bit-fpu.xml | 10 +---------
>  gdb-xml/riscv-64bit-cpu.xml |  6 +-----
>  gdb-xml/riscv-64bit-fpu.xml | 10 +---------
>  target/riscv/gdbstub.c      | 32 ++------------------------------
>  5 files changed, 6 insertions(+), 58 deletions(-)
>
> --
> 2.25.4
>
>