| Message ID | 20200128233216.515171-1-keithp@keithp.com |
|---|---|
| State | New |
| Headers | show |
| Series | riscv: Separate FPU register size from core register size in gdbstub [v2] | expand |
On Tue, Jan 28, 2020 at 3:33 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> Reviewed-by: Alistair Francis <alistair.francis@wdc.com> 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 > >
On Tue, 28 Jan 2020 23:32:16 GMT (+0000), keithp@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 <keithp@keithp.com> > > --- > > 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 <http://gnu.org/licenses/gpl.html> 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: <http://www.gnu.org/software/gdb/bugs/>. Find the GDB manual and other documentation resources online at: <http://www.gnu.org/software/gdb/documentation/>. 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.
Palmer Dabbelt <palmerdabbelt@google.com> 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
On Thu, 30 Jan 2020 14:13:16 PST (-0800), keithp@keithp.com wrote: > Palmer Dabbelt <palmerdabbelt@google.com> 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
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);
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> --- v2: Fix checkpatch formatting complaints. --- configure | 4 ++-- target/riscv/gdbstub.c | 20 +++++++++++--------- 2 files changed, 13 insertions(+), 11 deletions(-)