target/riscv/gdbstub.c: use misa_mxl_max for GPR write width

[PATCH] target/riscv/gdbstub.c: use misa_mxl_max for GPR write width

Posted by heavenlydev 1 month, 2 weeks ago

From: Enrique Abma Romero <heavenlydev01@gmail.com>

riscv_cpu_gdb_write_register() decides whether to sign-truncate an
incoming GPR value to 32 bits based on env->xl, the hart current XLEN.
This is asymmetric with the rest of the gdbstub: both the read path
(riscv_cpu_gdb_read_register) and the target description size registers
off misa_mxl_max, the maximum XLEN. GDB therefore always exchanges
8-byte register fields for an RV64-max hart, but a write is truncated to
its low 32 bits whenever the hart happens to be executing in an RV32
privilege context (env->xl == MXL_RV32, e.g. S/U-mode with SXL/UXL
narrowed to 32). The upper 32 bits are silently dropped, so a debugger
cannot set the full 64-bit architectural register even though the CPU
state and the read path are 64-bit.

Key the write width off misa_mxl_max, matching the read path and the
advertised register size, so a full-width write is honored regardless of
the hart current privilege XLEN.

Signed-off-by: Enrique Abma Romero <heavenlydev01@gmail.com>
---
 target/riscv/gdbstub.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target/riscv/gdbstub.c b/target/riscv/gdbstub.c
index 9abbf5b..9284579 100644
--- a/target/riscv/gdbstub.c
+++ b/target/riscv/gdbstub.c
@@ -89,7 +89,7 @@ int riscv_cpu_gdb_write_register(CPUState *cs, uint8_t *mem_buf, int n)
     const size_t regsize = mcc->def->misa_mxl_max == MXL_RV32 ? 4 : 8;
     uint64_t tmp = ldn(env, mem_buf, regsize);
 
-    if (env->xl < MXL_RV64) {
+    if (mcc->def->misa_mxl_max < MXL_RV64) {
         tmp = (int32_t)tmp;
     }
 
-- 
2.39.5

Re: [PATCH] target/riscv/gdbstub.c: use misa_mxl_max for GPR write width

Posted by Richard Henderson 1 month, 2 weeks ago

On 8/8/26 05:15, heavenlydev wrote:
> From: Enrique Abma Romero <heavenlydev01@gmail.com>
> 
> riscv_cpu_gdb_write_register() decides whether to sign-truncate an
> incoming GPR value to 32 bits based on env->xl, the hart current XLEN.
> This is asymmetric with the rest of the gdbstub: both the read path
> (riscv_cpu_gdb_read_register) and the target description size registers
> off misa_mxl_max, the maximum XLEN. GDB therefore always exchanges
> 8-byte register fields for an RV64-max hart, but a write is truncated to
> its low 32 bits whenever the hart happens to be executing in an RV32
> privilege context (env->xl == MXL_RV32, e.g. S/U-mode with SXL/UXL
> narrowed to 32). The upper 32 bits are silently dropped, so a debugger
> cannot set the full 64-bit architectural register even though the CPU
> state and the read path are 64-bit.
> 
> Key the write width off misa_mxl_max, matching the read path and the
> advertised register size, so a full-width write is honored regardless of
> the hart current privilege XLEN.
> 
> Signed-off-by: Enrique Abma Romero <heavenlydev01@gmail.com>
> ---
>   target/riscv/gdbstub.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/target/riscv/gdbstub.c b/target/riscv/gdbstub.c
> index 9abbf5b..9284579 100644
> --- a/target/riscv/gdbstub.c
> +++ b/target/riscv/gdbstub.c
> @@ -89,7 +89,7 @@ int riscv_cpu_gdb_write_register(CPUState *cs, uint8_t *mem_buf, int n)
>       const size_t regsize = mcc->def->misa_mxl_max == MXL_RV32 ? 4 : 8;
>       uint64_t tmp = ldn(env, mem_buf, regsize);
>   
> -    if (env->xl < MXL_RV64) {
> +    if (mcc->def->misa_mxl_max < MXL_RV64) {
>           tmp = (int32_t)tmp;
>       }
>   

A 64-bit machine in 32-bit bit mode has all registers sign extended -- it's part of the 
specification.  We enforce this during execution here:

static void gen_set_gpr(DisasContext *ctx, int reg_num, TCGv t)
{
     if (reg_num != 0) {
         switch (get_ol(ctx)) {
         case MXL_RV32:
             tcg_gen_ext32s_tl(cpu_gpr[reg_num], t);
             break;

The debugger shouldn't be able to break this.


r~