Skip to contentSkip to conversation

target/riscv/tcg: clear itrigger_enabled during trigger_reset_hold()

PATCH In review Daniel Henrique Barboza 3 participants 4 messages Nov 11 RISC-V

lore
Share & tools
lore

Daniel Henrique Barboza

author
#0 lore raw

v2 cover target/riscv/tcg: clear itrigger_enabled during trigger_reset_hold()

Hi,

This version just changes the "Fixes" tag in the commit msg. Sashiko pointed out that it pointed out the wrong commit in v1.

No other changes made.

Changes in v2:
- changed "Fixes" tag in commit msg
- v1 link: https://lore.kernel.org/qemu-devel/[email protected]/

Daniel Henrique Barboza (1):
target/riscv/tcg: clear itrigger_enabled during trigger_reset_hold()

 target/riscv/tcg/debug.c | 1 +
 1 file changed, 1 insertion(+)
Signature · 1 line
2.43.0
author
#1 lore raw

PATCH v2 1/1 target/riscv/tcg: clear itrigger_enabled during trigger_reset_hold()

We're leaving env->itrigger_enabled untouched during reset() while at the same time resetting triggers to AD_MATCH (type 2).

So, in case we reset the CPU during an itrigger session, translation code will enter itrigger_match helper every time (via ctx->itrigger), and the helper will do nothing since there's no type 3 triggers set. And we'll keep doing that until something else clears env->itrigger_enabled. This is a niche case that it's a bit hard to hit but it will cost a bit of performance for no good reason.

Found while reviewing an itrigger related patch.

Fixes 577f02869488 ("target/riscv: Add itrigger_enabled field to CPURISCVState").
Signed-off-by Daniel Henrique Barboza <[email protected]>
---
 target/riscv/tcg/debug.c | 1 +
 1 file changed, 1 insertion(+)
target/riscv/tcg/debug.c +1 -0
index 8b58cbd07a..988bc815d7 100644
@@ -1213,4 +1213,5 @@ void riscv_trigger_reset_hold(CPURISCVState *env)
     }
 
     env->mcontext = 0;
+    env->itrigger_enabled = false;
 }
Signature · 1 line
2.43.0
Reviewed-by on v2 1/1
#2 lore raw
Quoted 10 lines · On Wed, 11 Nov 2026 04:06:00 -0300 Daniel Henrique Barboza…

We're leaving env->itrigger_enabled untouched during reset() while at the same time resetting triggers to AD_MATCH (type 2).

So, in case we reset the CPU during an itrigger session, translation code will enter itrigger_match helper every time (via ctx->itrigger), and the helper will do nothing since there's no type 3 triggers set. And we'll keep doing that until something else clears env->itrigger_enabled. This is a niche case that it's a bit hard to hit but it will cost a bit of performance for no good reason.

Quoted 4 lines

Found while reviewing an itrigger related patch.

Fixes 577f02869488 ("target/riscv: Add itrigger_enabled field to CPURISCVState").
Signed-off-by Daniel Henrique Barboza <[email protected]>
Reviewed-by Chao Liu <[email protected]>

Thanks,
Chao

Quoted 19 lines
---
 target/riscv/tcg/debug.c | 1 +
 1 file changed, 1 insertion(+)
target/riscv/tcg/debug.c +1 -0
index 8b58cbd07a..988bc815d7 100644
@@ -1213,4 +1213,5 @@ void riscv_trigger_reset_hold(CPURISCVState *env)
     }
 
     env->mcontext = 0;
+    env->itrigger_enabled = false;
 }
Signature · 1 line
2.43.0

Sent using hkml (https://github.com/sjp38/hackermail)

Reviewed-by on v2 1/1
#3 lore raw
In reply to Daniel Henrique Barboza
Quoted 10 lines · Daniel Henrique Barboza

We're leaving env->itrigger_enabled untouched during reset() while at the same time resetting triggers to AD_MATCH (type 2).

So, in case we reset the CPU during an itrigger session, translation code will enter itrigger_match helper every time (via ctx->itrigger), and the helper will do nothing since there's no type 3 triggers set. And we'll keep doing that until something else clears env->itrigger_enabled. This is a niche case that it's a bit hard to hit but it will cost a bit of performance for no good reason.

Quoted 4 lines

Found while reviewing an itrigger related patch.

Fixes 577f02869488 ("target/riscv: Add itrigger_enabled field to CPURISCVState").
Signed-off-by Daniel Henrique Barboza <[email protected]>
Reviewed-by Nutty Liu <[email protected]>

Thanks,
Nutty

Quoted 14 lines
---
  target/riscv/tcg/debug.c | 1 +
  1 file changed, 1 insertion(+)
target/riscv/tcg/debug.c +1 -0
index 8b58cbd07a..988bc815d7 100644
@@ -1213,4 +1213,5 @@ void riscv_trigger_reset_hold(CPURISCVState *env)
      }
 
      env->mcontext = 0;
+    env->itrigger_enabled = false;
  }