[PATCH v3] target/loongarch: Fix SWI interrupt delivery via CSR_ESTAT

Bibo Mao posted 1 patch 2 weeks, 3 days ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20260806120315.1059413-1-maobibo@loongson.cn
Maintainers: Song Gao <17746591750@163.com>, Bibo Mao <maobibo@loongson.cn>, Xianglai Li <lixianglai@loongson.cn>
target/loongarch/cpu.c            | 26 ++++++++++++++++++++------
target/loongarch/internals.h      |  1 +
target/loongarch/tcg/csr_helper.c | 10 ++++++++++
3 files changed, 31 insertions(+), 6 deletions(-)
[PATCH v3] target/loongarch: Fix SWI interrupt delivery via CSR_ESTAT
Posted by Bibo Mao 2 weeks, 3 days ago
In TCG mode, helper_csrwr_estat() updates CSR.ESTAT.IS[1:0] (SWI0/SWI1)
when the guest writes CSR_ESTAT, but it did not update the CPU interrupt
request state. As a result, software interrupts could be observed as pending
in CSR.ESTAT while no interrupt exception was taken.

Update CPU_INTERRUPT_HARD after modifying CSR_ESTAT, matching the behavior of
loongarch_cpu_set_irq(). The helper runs without the Big QEMU Lock (BQL), so
take the BQL while calling cpu_interrupt().

Fixes: 5b1dedfe848b ("target/loongarch: Add LoongArch CSR instruction")
Reported-by: Andrew S. Rightenburg <andrew@rail5.org>
Signed-off-by: Andrew S. Rightenburg <andrew@rail5.org>
Signed-off-by: Bibo Mao <maobibo@loongson.cn>
Reviewed-by:  Song Gao <17746591750@163.com>
---
v2 ... v3:
  1. Add old value of CSR estat for loongarch_cpu_update_irq(), trigger
     interrupt if old == 0 and new != 0, the similiar with interrupt
     clear logic.
  2. Use simpler method (sys->CSR_ESTAT != old_v) check whether to update
     irq in helper_csrwr_estat().

v1 ... v2:
  1. Add common function loongarch_cpu_update_irq() called by
     loongarch_cpu_set_irq() and helper_csrwr_estat().
---
 target/loongarch/cpu.c            | 26 ++++++++++++++++++++------
 target/loongarch/internals.h      |  1 +
 target/loongarch/tcg/csr_helper.c | 10 ++++++++++
 3 files changed, 31 insertions(+), 6 deletions(-)

diff --git a/target/loongarch/cpu.c b/target/loongarch/cpu.c
index fb03424ffa..7bc77cf67e 100644
--- a/target/loongarch/cpu.c
+++ b/target/loongarch/cpu.c
@@ -57,12 +57,29 @@ static vaddr loongarch_cpu_get_pc(CPUState *cs)
 #ifndef CONFIG_USER_ONLY
 #include "hw/loongarch/virt.h"
 
+void loongarch_cpu_update_irq(LoongArchCPU *cpu, uint64_t old)
+{
+    CPULoongArchState *env = &cpu->env;
+    CPUState *cs = CPU(cpu);
+    CPUSysState *sys = env_sys(env);
+
+    if (FIELD_EX64(sys->CSR_ESTAT, CSR_ESTAT, IS)) {
+        if (!FIELD_EX64(old, CSR_ESTAT, IS)) {
+            cpu_interrupt(cs, CPU_INTERRUPT_HARD);
+        }
+    } else {
+        if (FIELD_EX64(old, CSR_ESTAT, IS)) {
+            cpu_reset_interrupt(cs, CPU_INTERRUPT_HARD);
+        }
+    }
+}
+
 void loongarch_cpu_set_irq(void *opaque, int irq, int level)
 {
     LoongArchCPU *cpu = opaque;
     CPULoongArchState *env = &cpu->env;
-    CPUState *cs = CPU(cpu);
     CPUSysState *sys = env_sys(env);
+    uint64_t old;
 
     if (irq < 0 || irq >= N_IRQS) {
         return;
@@ -71,12 +88,9 @@ void loongarch_cpu_set_irq(void *opaque, int irq, int level)
     if (kvm_enabled()) {
         kvm_loongarch_set_interrupt(cpu, irq, level);
     } else if (tcg_enabled()) {
+        old = sys->CSR_ESTAT;
         sys->CSR_ESTAT = deposit64(sys->CSR_ESTAT, irq, 1, level != 0);
-        if (FIELD_EX64(sys->CSR_ESTAT, CSR_ESTAT, IS)) {
-            cpu_interrupt(cs, CPU_INTERRUPT_HARD);
-        } else {
-            cpu_reset_interrupt(cs, CPU_INTERRUPT_HARD);
-        }
+        loongarch_cpu_update_irq(cpu, old);
     }
 }
 
diff --git a/target/loongarch/internals.h b/target/loongarch/internals.h
index e01dbed40f..0b670f9466 100644
--- a/target/loongarch/internals.h
+++ b/target/loongarch/internals.h
@@ -31,6 +31,7 @@ void restore_fp_status(CPULoongArchState *env);
 #ifndef CONFIG_USER_ONLY
 extern const VMStateDescription vmstate_loongarch_cpu;
 
+void loongarch_cpu_update_irq(LoongArchCPU *cpu, uint64_t old);
 void loongarch_cpu_set_irq(void *opaque, int irq, int level);
 
 void loongarch_constant_timer_cb(void *opaque);
diff --git a/target/loongarch/tcg/csr_helper.c b/target/loongarch/tcg/csr_helper.c
index 7dc33bc180..155482efc6 100644
--- a/target/loongarch/tcg/csr_helper.c
+++ b/target/loongarch/tcg/csr_helper.c
@@ -106,6 +106,16 @@ target_ulong helper_csrwr_estat(CPULoongArchState *env, target_ulong val)
 
     /* Only IS[1:0] can be written */
     sys->CSR_ESTAT = deposit64(sys->CSR_ESTAT, 0, 2, val);
+    /*
+     * Software interrupts (SWI0/SWI1) are latched in CSR.ESTAT.IS[1:0].
+     * Make sure the CPU interrupt request state tracks the pending bits,
+     * matching the behavior of loongarch_cpu_set_irq().
+     */
+    if (sys->CSR_ESTAT != old_v) {
+        bql_lock();
+        loongarch_cpu_update_irq(env_archcpu(env), old_v);
+        bql_unlock();
+    }
 
     return old_v;
 }

base-commit: b428fe036233cbd15d37e3c027ab6ca4d3661a80
-- 
2.54.0