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

2020-02-10 Thread Palmer Dabbelt 

On Thu, 30 Jan 2020 14:13:16 PST (-0800), kei...@keithp.com wrote:

Palmer Dabbelt  writes:


This isn't working for me.  When I apply it on top of my for-master (which is
very close to master), I get 



warning: while parsing target description (at line 1): Could not load XML document 
"riscv-64bit-fpu.xml"


If you're building from an existing directory, I think you'll have to
force re-creation of riscv32-softmmu/gdbstub-xml.c


That worked, thanks.  It's in the queue, with a note in case I forget again :)



--
-keith

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

2020-01-30 Thread Keith Packard 
Palmer Dabbelt  writes:

> This isn't working for me.  When I apply it on top of my for-master (which is
> very close to master), I get 

> warning: while parsing target description (at line 1): Could not load XML 
> document "riscv-64bit-fpu.xml"

If you're building from an existing directory, I think you'll have to
force re-creation of riscv32-softmmu/gdbstub-xml.c

-- 
-keith


signature.asc
Description: PGP signature

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

2020-01-30 Thread Palmer Dabbelt 

On Tue, 28 Jan 2020 23:32:16 GMT (+), kei...@keithp.com 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 

---

v2:
Fix checkpatch formatting complaints.
---
 configure  |  4 ++--
 target/riscv/gdbstub.c | 20 +++-
 2 files changed, 13 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..1a72f7be9c 100644
--- a/target/riscv/gdbstub.c
+++ b/target/riscv/gdbstub.c
@@ -303,7 +303,12 @@ 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 +408,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);


This isn't working for me.  When I apply it on top of my for-master (which is
very close to master), I get 


   $ ./install/bin/qemu-system-riscv32 -M virt -m 256 -nographic -cpu rv32 -s -S 
-bios default -kernel vmlinux &
   $ ./install/bin/riscv32-unknown-elf-gdb --ex "target remote :1234"
   GNU gdb (GDB) 10.0.50.20200122-git
   Copyright (C) 2020 Free Software Foundation, Inc.
   License GPLv3+: GNU GPL version 3 or later 
   This is free software: you are free to change and redistribute it.
   There is NO WARRANTY, to the extent permitted by law.
   Type "show copying" and "show warranty" for details.
   This GDB was configured as "--host=x86_64-pc-linux-gnu 
--target=riscv32-unknown-elf".
   Type "show configuration" for configuration details.
   For bug reporting instructions, please see:
   .
   Find the GDB manual and other documentation resources online at:
   .
   
   For help, type "help".

   Type "apropos word" to search for commands related to "word".
   Remote debugging using :1234
   warning: while parsing target description (at line 1): Could not load XML document 
"riscv-64bit-fpu.xml"
   warning: Could not load XML target description; ignoring
   warning: No executable has been specified and target does not support
   determining executable automatically.  Try using the "file" command.
   Truncated register 16 in remote 'g' packet
   (gdb) q

The same setup works (for some definition of works, as I get 32-bit D
registers) before the patch, so I don't think it's just something silly in my
environment.

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

2020-01-28 Thread Alistair Francis 
On Tue, Jan 28, 2020 at 3:33 PM Keith Packard via  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 

Reviewed-by: Alistair Francis 

Alistair

>
> ---
>
> v2:
> Fix checkpatch formatting complaints.
> ---
>  configure  |  4 ++--
>  target/riscv/gdbstub.c | 20 +++-
>  2 files changed, 13 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..1a72f7be9c 100644
> --- a/target/riscv/gdbstub.c
> +++ b/target/riscv/gdbstub.c
> @@ -303,7 +303,12 @@ 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 +408,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
>
>

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

2020-01-28 Thread Keith Packard via 
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 

---

v2:
Fix checkpatch formatting complaints.
---
 configure  |  4 ++--
 target/riscv/gdbstub.c | 20 +++-
 2 files changed, 13 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..1a72f7be9c 100644
--- a/target/riscv/gdbstub.c
+++ b/target/riscv/gdbstub.c
@@ -303,7 +303,12 @@ 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 +408,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