From: Miao Wang <[email protected]>
The CSR_ESTAT register of a CPU can be read and written by both the
CPU thread and other threads (e.g., the interrupt controller thread).
Currently the possible readers and writers of CSR_ESTAT are:
- Readers
- tcg generated by trans_csrrd(, CSR_ESTAT)
- loongarch_cpu_has_work()
- Writers
- tcg generated by trans_csrxchg(, CSR_ESTAT)
- helper_csrwr_estat()
- helper_csrrd_msgir()
- loongarch_cpu_set_irq()
- loongarch_cpu_do_interrupt()
- loongarch_cpu_exec_interrupt()
The access from the CPU thread is not synchronized with the access from
other threads, which may lead to data races. The above readers and
writers shall all run on the corresponding CPU thread except for
loongarch_cpu_set_irq(). To fix this, the access to CSR_ESTAT in
loongarch_cpu_set_irq() is moved to the CPU thread by using
async_run_on_cpu().
The data race has been identified while running the test cases from
dracut, which is using QEMU to boot a LoongArch guest. By running the
tests repeatedly (about 30 times) in the following conditions, the
guest will hang in the middle of booting:
- Host architecture: LoongArch64 or Aarch64
- Guest kernel: 7.1.3+deb14-loong64
- Number of vCPUs: 1 or 2
- CPU Features: max, la464,msgint=off,ptw=off, or la464,msgint=off,ptw=on
- Accelerator: tcg
When the guest hangs, the guest kernel log shows various errors related
to RCU stalls or other Soft Lockup or Hard Lockup issues. When running
with 1 vCPU and the guest hangs, the guest kernel directly hangs without
any messages and stucks at idle_exit.
With this patch, the guest can boot successfully without any hangs
during repeated runs of the test cases.
Signed-off-by: Miao Wang <[email protected]>
---
Changes in v2:
- Simplify the changes to move the access to CSR_ESTAT from the only
unsynchronized loongarch_cpu_set_irq() to the CPU thread using
async_run_on_cpu() to avoid the race condition.
- Link to v1:
https://lore.kernel.org/qemu-devel/[email protected]
---
target/loongarch/cpu.c | 27 +++++++++++++++++++--------
1 file changed, 19 insertions(+), 8 deletions(-)
diff --git a/target/loongarch/cpu.c b/target/loongarch/cpu.c
index
fb03424ffa8cd5e5d250531cc2a36fa789fc25a9..126c3ad276cc44311942542e086bcabe41d91b63
100644
--- a/target/loongarch/cpu.c
+++ b/target/loongarch/cpu.c
@@ -57,12 +57,27 @@ static vaddr loongarch_cpu_get_pc(CPUState *cs)
#ifndef CONFIG_USER_ONLY
#include "hw/loongarch/virt.h"
+static void do_set_cpu_estat(CPUState *cs, run_on_cpu_data data)
+{
+ CPULoongArchState *env = cpu_env(cs);
+ CPUSysState *sys = env_sys(env);
+
+ int irq = data.host_int;
+ int level = irq >= 0 ? 1 : 0;
+ irq = level ? irq : -irq;
+
+ 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);
+ }
+}
+
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);
if (irq < 0 || irq >= N_IRQS) {
return;
@@ -71,12 +86,8 @@ 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()) {
- 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);
- }
+ async_run_on_cpu(cs, do_set_cpu_estat,
+ RUN_ON_CPU_HOST_INT(level ? irq : -irq));
