riscv: Separate FPU register size from core register size in gdbstub

[PATCH] riscv: Separate FPU register size from core register size in gdbstub

Posted by Keith Packard via 4 years, 3 months ago

The size of the FPU registers is dictated by the 'f' and 'd' features,
not the core processor register size. Processors with the 'd' feature
have 64-bit FPU registers. Processors without the 'd' feature but with
the 'f' feature have 32-bit FPU registers.

Signed-off-by: Keith Packard <keithp@keithp.com>
---
 configure              |  4 ++--
 target/riscv/gdbstub.c | 18 +++++++++---------
 2 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/configure b/configure
index a72a5def57..c21bff8d10 100755
--- a/configure
+++ b/configure
@@ -7709,13 +7709,13 @@ case "$target_name" in
     TARGET_BASE_ARCH=riscv
     TARGET_ABI_DIR=riscv
     mttcg=yes
-    gdb_xml_files="riscv-32bit-cpu.xml riscv-32bit-fpu.xml riscv-32bit-csr.xml riscv-32bit-virtual.xml"
+    gdb_xml_files="riscv-32bit-cpu.xml riscv-32bit-fpu.xml riscv-64bit-fpu.xml riscv-32bit-csr.xml riscv-32bit-virtual.xml"
   ;;
   riscv64)
     TARGET_BASE_ARCH=riscv
     TARGET_ABI_DIR=riscv
     mttcg=yes
-    gdb_xml_files="riscv-64bit-cpu.xml riscv-64bit-fpu.xml riscv-64bit-csr.xml riscv-64bit-virtual.xml"
+    gdb_xml_files="riscv-64bit-cpu.xml riscv-32bit-fpu.xml riscv-64bit-fpu.xml riscv-64bit-csr.xml riscv-64bit-virtual.xml"
   ;;
   sh4|sh4eb)
     TARGET_ARCH=sh4
diff --git a/target/riscv/gdbstub.c b/target/riscv/gdbstub.c
index 1a7947e019..c1803a5916 100644
--- a/target/riscv/gdbstub.c
+++ b/target/riscv/gdbstub.c
@@ -303,7 +303,10 @@ int riscv_cpu_gdb_write_register(CPUState *cs, uint8_t *mem_buf, int n)
 static int riscv_gdb_get_fpu(CPURISCVState *env, uint8_t *mem_buf, int n)
 {
     if (n < 32) {
-        return gdb_get_reg64(mem_buf, env->fpr[n]);
+        if (env->misa & RVD)
+            return gdb_get_reg64(mem_buf, env->fpr[n]);
+        if (env->misa & RVF)
+            return gdb_get_reg32(mem_buf, env->fpr[n]);
     /* there is hole between ft11 and fflags in fpu.xml */
     } else if (n < 36 && n > 32) {
         target_ulong val = 0;
@@ -403,23 +406,20 @@ void riscv_cpu_register_gdb_regs_for_features(CPUState *cs)
 {
     RISCVCPU *cpu = RISCV_CPU(cs);
     CPURISCVState *env = &cpu->env;
-#if defined(TARGET_RISCV32)
-    if (env->misa & RVF) {
+    if (env->misa & RVD) {
+        gdb_register_coprocessor(cs, riscv_gdb_get_fpu, riscv_gdb_set_fpu,
+                                 36, "riscv-64bit-fpu.xml", 0);
+    } else if (env->misa & RVF) {
         gdb_register_coprocessor(cs, riscv_gdb_get_fpu, riscv_gdb_set_fpu,
                                  36, "riscv-32bit-fpu.xml", 0);
     }
-
+#if defined(TARGET_RISCV32)
     gdb_register_coprocessor(cs, riscv_gdb_get_csr, riscv_gdb_set_csr,
                              240, "riscv-32bit-csr.xml", 0);
 
     gdb_register_coprocessor(cs, riscv_gdb_get_virtual, riscv_gdb_set_virtual,
                              1, "riscv-32bit-virtual.xml", 0);
 #elif defined(TARGET_RISCV64)
-    if (env->misa & RVF) {
-        gdb_register_coprocessor(cs, riscv_gdb_get_fpu, riscv_gdb_set_fpu,
-                                 36, "riscv-64bit-fpu.xml", 0);
-    }
-
     gdb_register_coprocessor(cs, riscv_gdb_get_csr, riscv_gdb_set_csr,
                              240, "riscv-64bit-csr.xml", 0);
 
-- 
2.25.0

Re: [PATCH] riscv: Separate FPU register size from core register size in gdbstub

Posted by no-reply@patchew.org 4 years, 3 months ago

Patchew URL: https://patchew.org/QEMU/20200128223955.464556-1-keithp@keithp.com/



Hi,

This series seems to have some coding style problems. See output below for
more information:

Type: series
Message-id: 20200128223955.464556-1-keithp@keithp.com
Subject: [PATCH] riscv: Separate FPU register size from core register size in gdbstub

=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 * [new tag]         patchew/20200128223955.464556-1-keithp@keithp.com -> patchew/20200128223955.464556-1-keithp@keithp.com
Switched to a new branch 'test'
f8704a8 riscv: Separate FPU register size from core register size in gdbstub

=== OUTPUT BEGIN ===
ERROR: braces {} are necessary for all arms of this statement
#44: FILE: target/riscv/gdbstub.c:306:
+        if (env->misa & RVD)
[...]

total: 1 errors, 0 warnings, 54 lines checked

Commit f8704a85f17a (riscv: Separate FPU register size from core register size in gdbstub) has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
=== OUTPUT END ===

Test command exited with code: 1


The full log is available at
http://patchew.org/logs/20200128223955.464556-1-keithp@keithp.com/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com

Re: [PATCH] riscv: Separate FPU register size from core register size in gdbstub

Posted by Alistair Francis 4 years, 3 months ago

On Tue, Jan 28, 2020 at 2:40 PM Keith Packard via <qemu-devel@nongnu.org> wrote:
>
> The size of the FPU registers is dictated by the 'f' and 'd' features,
> not the core processor register size. Processors with the 'd' feature
> have 64-bit FPU registers. Processors without the 'd' feature but with
> the 'f' feature have 32-bit FPU registers.
>
> Signed-off-by: Keith Packard <keithp@keithp.com>
> ---
>  configure              |  4 ++--
>  target/riscv/gdbstub.c | 18 +++++++++---------
>  2 files changed, 11 insertions(+), 11 deletions(-)
>
> diff --git a/configure b/configure
> index a72a5def57..c21bff8d10 100755
> --- a/configure
> +++ b/configure
> @@ -7709,13 +7709,13 @@ case "$target_name" in
>      TARGET_BASE_ARCH=riscv
>      TARGET_ABI_DIR=riscv
>      mttcg=yes
> -    gdb_xml_files="riscv-32bit-cpu.xml riscv-32bit-fpu.xml riscv-32bit-csr.xml riscv-32bit-virtual.xml"
> +    gdb_xml_files="riscv-32bit-cpu.xml riscv-32bit-fpu.xml riscv-64bit-fpu.xml riscv-32bit-csr.xml riscv-32bit-virtual.xml"
>    ;;
>    riscv64)
>      TARGET_BASE_ARCH=riscv
>      TARGET_ABI_DIR=riscv
>      mttcg=yes
> -    gdb_xml_files="riscv-64bit-cpu.xml riscv-64bit-fpu.xml riscv-64bit-csr.xml riscv-64bit-virtual.xml"
> +    gdb_xml_files="riscv-64bit-cpu.xml riscv-32bit-fpu.xml riscv-64bit-fpu.xml riscv-64bit-csr.xml riscv-64bit-virtual.xml"
>    ;;
>    sh4|sh4eb)
>      TARGET_ARCH=sh4
> diff --git a/target/riscv/gdbstub.c b/target/riscv/gdbstub.c
> index 1a7947e019..c1803a5916 100644
> --- a/target/riscv/gdbstub.c
> +++ b/target/riscv/gdbstub.c
> @@ -303,7 +303,10 @@ int riscv_cpu_gdb_write_register(CPUState *cs, uint8_t *mem_buf, int n)
>  static int riscv_gdb_get_fpu(CPURISCVState *env, uint8_t *mem_buf, int n)
>  {
>      if (n < 32) {
> -        return gdb_get_reg64(mem_buf, env->fpr[n]);
> +        if (env->misa & RVD)
> +            return gdb_get_reg64(mem_buf, env->fpr[n]);
> +        if (env->misa & RVF)
> +            return gdb_get_reg32(mem_buf, env->fpr[n]);

You need brackets around all if statements, besides that:

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

Alistair

>      /* there is hole between ft11 and fflags in fpu.xml */
>      } else if (n < 36 && n > 32) {
>          target_ulong val = 0;
> @@ -403,23 +406,20 @@ void riscv_cpu_register_gdb_regs_for_features(CPUState *cs)
>  {
>      RISCVCPU *cpu = RISCV_CPU(cs);
>      CPURISCVState *env = &cpu->env;
> -#if defined(TARGET_RISCV32)
> -    if (env->misa & RVF) {
> +    if (env->misa & RVD) {
> +        gdb_register_coprocessor(cs, riscv_gdb_get_fpu, riscv_gdb_set_fpu,
> +                                 36, "riscv-64bit-fpu.xml", 0);
> +    } else if (env->misa & RVF) {
>          gdb_register_coprocessor(cs, riscv_gdb_get_fpu, riscv_gdb_set_fpu,
>                                   36, "riscv-32bit-fpu.xml", 0);
>      }
> -
> +#if defined(TARGET_RISCV32)
>      gdb_register_coprocessor(cs, riscv_gdb_get_csr, riscv_gdb_set_csr,
>                               240, "riscv-32bit-csr.xml", 0);
>
>      gdb_register_coprocessor(cs, riscv_gdb_get_virtual, riscv_gdb_set_virtual,
>                               1, "riscv-32bit-virtual.xml", 0);
>  #elif defined(TARGET_RISCV64)
> -    if (env->misa & RVF) {
> -        gdb_register_coprocessor(cs, riscv_gdb_get_fpu, riscv_gdb_set_fpu,
> -                                 36, "riscv-64bit-fpu.xml", 0);
> -    }
> -
>      gdb_register_coprocessor(cs, riscv_gdb_get_csr, riscv_gdb_set_csr,
>                               240, "riscv-64bit-csr.xml", 0);
>
> --
> 2.25.0
>
>

Re: [PATCH] riscv: Separate FPU register size from core register size in gdbstub

Posted by Keith Packard via 4 years, 3 months ago

Alistair Francis <alistair23@gmail.com> writes:

> You need brackets around all if statements, besides that:

Sorry for the noise; I caught that and sent another version of this
patch.

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

Thanks for your review!

-- 
-keith

Re: [PATCH] riscv: Separate FPU register size from core register size in gdbstub

Posted by no-reply@patchew.org 4 years, 3 months ago

Patchew URL: https://patchew.org/QEMU/20200128223955.464556-1-keithp@keithp.com/



Hi,

This series seems to have some coding style problems. See output below for
more information:

Type: series
Message-id: 20200128223955.464556-1-keithp@keithp.com
Subject: [PATCH] riscv: Separate FPU register size from core register size in gdbstub

=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 - [tag update]      patchew/1580242161-20333-1-git-send-email-aleksandar.markovic@rt-rk.com -> patchew/1580242161-20333-1-git-send-email-aleksandar.markovic@rt-rk.com
 - [tag update]      patchew/20200128223955.464556-1-keithp@keithp.com -> patchew/20200128223955.464556-1-keithp@keithp.com
 * [new tag]         patchew/20200128231840.508986-1-keithp@keithp.com -> patchew/20200128231840.508986-1-keithp@keithp.com
 * [new tag]         patchew/20200128233216.515171-1-keithp@keithp.com -> patchew/20200128233216.515171-1-keithp@keithp.com
Switched to a new branch 'test'
a2c277b riscv: Separate FPU register size from core register size in gdbstub

=== OUTPUT BEGIN ===
ERROR: braces {} are necessary for all arms of this statement
#45: FILE: target/riscv/gdbstub.c:306:
+        if (env->misa & RVD)
[...]

total: 1 errors, 0 warnings, 54 lines checked

Commit a2c277b8eb39 (riscv: Separate FPU register size from core register size in gdbstub) has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
=== OUTPUT END ===

Test command exited with code: 1


The full log is available at
http://patchew.org/logs/20200128223955.464556-1-keithp@keithp.com/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com