On Wed, Aug 31, 2022 at 10:43 AM Andrew Burgess <aburg...@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 <aburg...@redhat.com>
Reviewed-by: Alistair Francis <alistair.fran...@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
>
>
