diff mbox series

[v3,10/21] target/riscv: Skip empty MIP updates

Message ID 20260923171116.31276-11-philmd@oss.qualcomm.com
State New
Headers show
Series accel/tcg: Make halt-to-exec transition explicit and remove cpu_exec_halt | expand

Commit Message

Philippe Mathieu-Daudé Sept. 23, 2026, 5:11 p.m. UTC 
Return early when the effective mask is zero. This avoids taking
the BQL and processing interrupts when riscv_cpu_update_mip()
cannot change env->mip.

Signed-off-by: Philippe Mathieu-Daudé <philmd@oss.qualcomm.com>
---
 target/riscv/tcg/cpu_helper.c | 3 +++
 1 file changed, 3 insertions(+)

Comments

Richard Henderson Sept. 23, 2026, 7:02 p.m. UTC | #1 
On 9/23/26 10:11, Philippe Mathieu-Daudé wrote:
> Return early when the effective mask is zero. This avoids taking
> the BQL and processing interrupts when riscv_cpu_update_mip()
> cannot change env->mip.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@oss.qualcomm.com>


This breaks the usage in riscv_cpu_set_mode.  One could argue that 
should be calling riscv_cpu_interrupt directly, but you'd want to 
examine locking as well.

>       /* No need to update mip for VSTIP */
>       mask = ((mask == MIP_VSTIP) && env->vstime_irq) ? 0 : mask;
> +    if (!mask) {
> +        return old;
> +    }

If this is kept in some way, this should be written as

     if (!mask || (mask == MIP_VSTIP && env->vstime_irq)) {
         return env->mip;
     }

Note that there is an existing bug in which the returned 'old' is 
fetched before the bql lock is taken.  Note as well that the existing 
mip update statement doesn't use 'old', but could if it were fetched at 
the right time.


r~
diff mbox series

Patch

diff --git a/target/riscv/tcg/cpu_helper.c b/target/riscv/tcg/cpu_helper.c
index 07d92226527..be64f9ee8f2 100644
--- a/target/riscv/tcg/cpu_helper.c
+++ b/target/riscv/tcg/cpu_helper.c
@@ -580,6 +580,9 @@  uint64_t riscv_cpu_update_mip(CPURISCVState *env, uint64_t mask, uint64_t value)
 
     /* No need to update mip for VSTIP */
     mask = ((mask == MIP_VSTIP) && env->vstime_irq) ? 0 : mask;
+    if (!mask) {
+        return old;
+    }
 
     BQL_LOCK_GUARD();