target/riscv: fix invalid sstc predicate in gdbstub

[PATCH] target/riscv: fix invalid sstc predicate in gdbstub

Abhigyan Kumar posted 1 patch 1 month, 1 week ago

Download mbox

Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20260530053200.902591-1-314abh@gmail.com

Maintainers: Palmer Dabbelt <palmer@dabbelt.com>, Alistair Francis <alistair.francis@wdc.com>, Weiwei Li <liwei1518@gmail.com>, Daniel Henrique Barboza <daniel.barboza@oss.qualcomm.com>, Liu Zhiwei <zhiwei_liu@linux.alibaba.com>, Chao Liu <chao.liu.zevorn@gmail.com>

target/riscv/csr.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

Expand all Fold all

[PATCH] target/riscv: fix invalid sstc predicate in gdbstub

Posted by Abhigyan Kumar 1 month, 1 week ago

sstc function incorrectly checks for avaibility of env->rdtime_fn.
This causes it to fail each time it's called because rdtime_fn is setup
later in the procedure (when timer devices are created). This prevents
stimecmp's addition to gdb.

This change ensures only the riscv_cpu_cfg(env) check is done. rdtime_fn
will always be NULL otherwise and fail.

Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3496
Signed-off-by: Abhigyan Kumar <314abh@gmail.com>
---
 target/riscv/csr.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index f5b0895fd..d72df945f 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -590,7 +590,7 @@ static RISCVException sstc(CPURISCVState *env, int csrno)
 {
     bool hmode_check = false;
 
-    if (!riscv_cpu_cfg(env)->ext_sstc || !env->rdtime_fn) {
+    if (!riscv_cpu_cfg(env)->ext_sstc) {
         return RISCV_EXCP_ILLEGAL_INST;
     }
 
-- 
2.54.0

Re: [PATCH] target/riscv: fix invalid sstc predicate in gdbstub

Posted by Daniel Henrique Barboza 1 month, 1 week ago


On 5/30/2026 2:32 AM, Abhigyan Kumar wrote:
> sstc function incorrectly checks for avaibility of env->rdtime_fn.
> This causes it to fail each time it's called because rdtime_fn is setup
> later in the procedure (when timer devices are created). This prevents
> stimecmp's addition to gdb.
> 
> This change ensures only the riscv_cpu_cfg(env) check is done. rdtime_fn
> will always be NULL otherwise and fail.

I am not sure we can do that - there are several boards that don't create
the timer that implements rdtime_fn (e.g. spike).  If we just remove the
check we'll break the sstc check for them.

I suggest using the 'env->debugger' flag to skip the rdtimer check if
we're running the gdbstub:

$ git diff
diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index 5514e0f455..63f4795ac7 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -590,7 +590,8 @@ static RISCVException sstc(CPURISCVState *env, int csrno)
  {
      bool hmode_check = false;

-    if (!riscv_cpu_cfg(env)->ext_sstc || !env->rdtime_fn) {
+    if (!riscv_cpu_cfg(env)->ext_sstc
+        || (!env->debugger && !env->rdtime_fn)) {
          return RISCV_EXCP_ILLEGAL_INST;
      }

Testing with gitlab 3496 this seems to work:


$ riscv64-unknown-elf-gdb
(gdb) target extended-remote :1234
(gdb)  p $stimecmp
$1 = 0
(gdb)


Thanks,
Daniel

> 
> Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3496
> Signed-off-by: Abhigyan Kumar <314abh@gmail.com>
> ---
>   target/riscv/csr.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/target/riscv/csr.c b/target/riscv/csr.c
> index f5b0895fd..d72df945f 100644
> --- a/target/riscv/csr.c
> +++ b/target/riscv/csr.c
> @@ -590,7 +590,7 @@ static RISCVException sstc(CPURISCVState *env, int csrno)
>   {
>       bool hmode_check = false;
>   
> -    if (!riscv_cpu_cfg(env)->ext_sstc || !env->rdtime_fn) {
> +    if (!riscv_cpu_cfg(env)->ext_sstc) {
>           return RISCV_EXCP_ILLEGAL_INST;
>       }
>