target/loongarch/cpu.c | 8 +++---- target/loongarch/internals.h | 27 +++++++++++++++++++++ target/loongarch/tcg/csr_helper.c | 28 +++++++++++++++++----- target/loongarch/tcg/helper.h | 2 ++ .../tcg/insn_trans/trans_privileged.c.inc | 13 ++++++++-- target/loongarch/tcg/tcg_cpu.c | 12 ++++------ 6 files changed, 71 insertions(+), 19 deletions(-)
From: Miao Wang <shankerwangmiao@gmail.com>
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. To fix this, use atomic
operations to read and write CSR_ESTAT.
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 <shankerwangmiao@gmail.com>
---
target/loongarch/cpu.c | 8 +++----
target/loongarch/internals.h | 27 +++++++++++++++++++++
target/loongarch/tcg/csr_helper.c | 28 +++++++++++++++++-----
target/loongarch/tcg/helper.h | 2 ++
.../tcg/insn_trans/trans_privileged.c.inc | 13 ++++++++--
target/loongarch/tcg/tcg_cpu.c | 12 ++++------
6 files changed, 71 insertions(+), 19 deletions(-)
diff --git a/target/loongarch/cpu.c b/target/loongarch/cpu.c
index fb03424ffa8cd5e5d250531cc2a36fa789fc25a9..be53307997d60ceaa404a6187eedf96dc51d82ff 100644
--- a/target/loongarch/cpu.c
+++ b/target/loongarch/cpu.c
@@ -71,8 +71,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_csr_estat_deposit64(sys, irq, 1, level != 0);
+ if (FIELD_EX64(cpu_csr_estat_get(sys), CSR_ESTAT, IS)) {
cpu_interrupt(cs, CPU_INTERRUPT_HARD);
} else {
cpu_reset_interrupt(cs, CPU_INTERRUPT_HARD);
@@ -87,7 +87,7 @@ bool cpu_loongarch_hw_interrupts_pending(CPULoongArchState *env)
uint32_t status;
CPUSysState *sys = env_sys(env);
- pending = FIELD_EX64(sys->CSR_ESTAT, CSR_ESTAT, IS);
+ pending = FIELD_EX64(cpu_csr_estat_get(sys), CSR_ESTAT, IS);
status = FIELD_EX64(sys->CSR_ECFG, CSR_ECFG, LIE);
return (pending & status) != 0;
@@ -641,7 +641,7 @@ static void loongarch_cpu_reset_hold(Object *obj, ResetType type)
sys->CSR_ECFG = FIELD_DP64(sys->CSR_ECFG, CSR_ECFG, VS, 0);
sys->CSR_ECFG = FIELD_DP64(sys->CSR_ECFG, CSR_ECFG, LIE, 0);
- sys->CSR_ESTAT = sys->CSR_ESTAT & (~MAKE_64BIT_MASK(0, 2));
+ qatomic_and(&sys->CSR_ESTAT, ~MAKE_64BIT_MASK(0, 2));
sys->CSR_RVACFG = FIELD_DP64(sys->CSR_RVACFG, CSR_RVACFG, RBITS, 0);
sys->CSR_CPUID = cs->cpu_index;
sys->CSR_TCFG = FIELD_DP64(sys->CSR_TCFG, CSR_TCFG, EN, 0);
diff --git a/target/loongarch/internals.h b/target/loongarch/internals.h
index e01dbed40f6c7d02d2180d9dd5e17dacdd31755a..b94d229b2e0a4d48a5b712ad5ee364fb4a402055 100644
--- a/target/loongarch/internals.h
+++ b/target/loongarch/internals.h
@@ -28,6 +28,33 @@ int ieee_ex_to_loongarch(int xcpt);
void restore_fp_status(CPULoongArchState *env);
#endif
+static inline uint64_t cpu_csr_estat_get(CPUSysState *sys)
+{
+ return qatomic_read(&sys->CSR_ESTAT);
+}
+
+#define cpu_csr_estat_set(sys, field, val) \
+ do { \
+ uint64_t _estat = cpu_csr_estat_get(sys); \
+ uint64_t _new = FIELD_DP64(_estat, CSR_ESTAT, field, (val)); \
+ if (qatomic_cmpxchg(&(sys)->CSR_ESTAT, _estat, _new) == _estat) { \
+ break; \
+ } \
+ } while (1)
+
+static inline uint64_t cpu_csr_estat_deposit64(CPUSysState *sys, int start,
+ int len, uint64_t val)
+{
+ do {
+ uint64_t _estat = cpu_csr_estat_get(sys);
+ uint64_t _new = deposit64(_estat, start, len, val);
+ if (qatomic_cmpxchg(&(sys)->CSR_ESTAT, _estat, _new) == _estat) {
+ return _estat;
+ }
+ } while (1);
+}
+
+
#ifndef CONFIG_USER_ONLY
extern const VMStateDescription vmstate_loongarch_cpu;
diff --git a/target/loongarch/tcg/csr_helper.c b/target/loongarch/tcg/csr_helper.c
index 7dc33bc1802e5d4efc72648174bbf5e9baeaea54..023a6b1dc44f0df38c8c2f46b0b07eda88ddb3de 100644
--- a/target/loongarch/tcg/csr_helper.c
+++ b/target/loongarch/tcg/csr_helper.c
@@ -90,7 +90,7 @@ target_ulong helper_csrrd_msgir(CPULoongArchState *env)
return irq;
}
- sys->CSR_ESTAT = FIELD_DP64(sys->CSR_ESTAT, CSR_ESTAT, MSGINT, 0);
+ cpu_csr_estat_set(sys, MSGINT, 0);
} else {
/* bit 31 set 1 for no invalid irq */
irq = BIT(31);
@@ -99,15 +99,31 @@ target_ulong helper_csrrd_msgir(CPULoongArchState *env)
return irq;
}
-target_ulong helper_csrwr_estat(CPULoongArchState *env, target_ulong val)
+target_ulong helper_csrrd_estat(CPULoongArchState *env)
{
CPUSysState *sys = env_sys(env);
- int64_t old_v = sys->CSR_ESTAT;
+ return cpu_csr_estat_get(sys);
+}
- /* Only IS[1:0] can be written */
- sys->CSR_ESTAT = deposit64(sys->CSR_ESTAT, 0, 2, val);
+target_ulong helper_csrwr_estat(CPULoongArchState *env, target_ulong val)
+{
+ CPUSysState *sys = env_sys(env);
+ return cpu_csr_estat_deposit64(sys, 0, 2, val);
+}
- return old_v;
+target_ulong helper_csrxhg_estat(CPULoongArchState *env, target_ulong val,
+ target_ulong mask)
+{
+ CPUSysState *sys = env_sys(env);
+ mask &= MAKE_64BIT_MASK(0, 2);
+ val &= mask;
+ do {
+ uint64_t _estat = cpu_csr_estat_get(sys);
+ uint64_t _new = (_estat & ~mask) | val;
+ if (qatomic_cmpxchg(&(sys)->CSR_ESTAT, _estat, _new) == _estat) {
+ return _estat;
+ }
+ } while (1);
}
target_ulong helper_csrwr_asid(CPULoongArchState *env, target_ulong val)
diff --git a/target/loongarch/tcg/helper.h b/target/loongarch/tcg/helper.h
index 8a6c62f11618d7125122831f4a9fe0d0fab73364..58089e84c60fa49524b4705981d0f677cfb439d2 100644
--- a/target/loongarch/tcg/helper.h
+++ b/target/loongarch/tcg/helper.h
@@ -101,8 +101,10 @@ DEF_HELPER_1(csrrd_pgd, i64, env)
DEF_HELPER_1(csrrd_cpuid, i64, env)
DEF_HELPER_1(csrrd_tval, i64, env)
DEF_HELPER_1(csrrd_msgir, i64, env)
+DEF_HELPER_1(csrrd_estat, i64, env)
DEF_HELPER_2(csrwr_stlbps, i64, env, tl)
DEF_HELPER_2(csrwr_estat, i64, env, tl)
+DEF_HELPER_3(csrxhg_estat, i64, env, tl, tl)
DEF_HELPER_2(csrwr_asid, i64, env, tl)
DEF_HELPER_2(csrwr_tcfg, i64, env, tl)
DEF_HELPER_2(csrwr_ticlr, i64, env, tl)
diff --git a/target/loongarch/tcg/insn_trans/trans_privileged.c.inc b/target/loongarch/tcg/insn_trans/trans_privileged.c.inc
index 6728ce5ec9387d1f66d68cf36abbe6e0997c639a..e6e9072518767479f66bae1cf1c72844df4343d4 100644
--- a/target/loongarch/tcg/insn_trans/trans_privileged.c.inc
+++ b/target/loongarch/tcg/insn_trans/trans_privileged.c.inc
@@ -75,7 +75,7 @@ static bool set_csr_trans_func(unsigned int csr_num, GenCSRRead readfn,
void loongarch_csr_translate_init(void)
{
SET_CSR_FUNC(STLBPS, NULL, gen_helper_csrwr_stlbps);
- SET_CSR_FUNC(ESTAT, NULL, gen_helper_csrwr_estat);
+ SET_CSR_FUNC(ESTAT, gen_helper_csrrd_estat, gen_helper_csrwr_estat);
SET_CSR_FUNC(ASID, NULL, gen_helper_csrwr_asid);
SET_CSR_FUNC(PGD, gen_helper_csrrd_pgd, NULL);
SET_CSR_FUNC(PWCL, NULL, gen_helper_csrwr_pwcl);
@@ -187,7 +187,16 @@ static bool trans_csrxchg(DisasContext *ctx, arg_csrxchg *a)
return false;
}
- /* So far only readonly csrs have readfn. */
+ if (a->csr == LOONGARCH_CSR_ESTAT) {
+ src1 = gpr_src(ctx, a->rd, EXT_NONE);
+ mask = gpr_src(ctx, a->rj, EXT_NONE);
+ oldv = tcg_temp_new();
+ gen_helper_csrxhg_estat(oldv, tcg_env, src1, mask);
+ gen_set_gpr(a->rd, oldv, EXT_NONE);
+ return true;
+ }
+
+ /* So far only readonly csrs have readfn, except ESTAT. */
assert(csr->readfn == NULL);
src1 = gpr_src(ctx, a->rd, EXT_NONE);
diff --git a/target/loongarch/tcg/tcg_cpu.c b/target/loongarch/tcg/tcg_cpu.c
index 4b1d44a16445841de8b9a7d69d8f389c357e6abf..64a28c7dcbbce2f2ce47640471602485344ca8e3 100644
--- a/target/loongarch/tcg/tcg_cpu.c
+++ b/target/loongarch/tcg/tcg_cpu.c
@@ -160,10 +160,8 @@ static void loongarch_cpu_do_interrupt(CPUState *cs)
sys->CSR_TLBRERA = FIELD_DP64(sys->CSR_TLBRERA, CSR_TLBRERA,
PC, (env->pc >> 2));
} else {
- sys->CSR_ESTAT = FIELD_DP64(sys->CSR_ESTAT, CSR_ESTAT, ECODE,
- EXCODE_MCODE(cause));
- sys->CSR_ESTAT = FIELD_DP64(sys->CSR_ESTAT, CSR_ESTAT, ESUBCODE,
- EXCODE_SUBCODE(cause));
+ cpu_csr_estat_set(sys, ECODE, EXCODE_MCODE(cause));
+ cpu_csr_estat_set(sys, ESUBCODE, EXCODE_SUBCODE(cause));
sys->CSR_PRMD = FIELD_DP64(sys->CSR_PRMD, CSR_PRMD, PPLV,
FIELD_EX64(sys->CSR_CRMD, CSR_CRMD, PLV));
sys->CSR_PRMD = FIELD_DP64(sys->CSR_PRMD, CSR_PRMD, PIE,
@@ -181,7 +179,7 @@ static void loongarch_cpu_do_interrupt(CPUState *cs)
if (cs->exception_index == EXCCODE_INT) {
/* Interrupt */
uint32_t vector = 0;
- uint32_t pending = FIELD_EX64(sys->CSR_ESTAT, CSR_ESTAT, IS);
+ uint32_t pending = FIELD_EX64(cpu_csr_estat_get(sys), CSR_ESTAT, IS);
pending &= FIELD_EX64(sys->CSR_ECFG, CSR_ECFG, LIE);
/* Find the highest-priority interrupt. */
@@ -195,7 +193,7 @@ static void loongarch_cpu_do_interrupt(CPUState *cs)
TARGET_FMT_lx "\n",
__func__, env->pc, sys->CSR_ERA,
cause, sys->CSR_BADV, sys->CSR_DERA, vector,
- sys->CSR_ECFG, sys->CSR_ESTAT);
+ sys->CSR_ECFG, cpu_csr_estat_get(sys));
qemu_plugin_vcpu_interrupt_cb(cs, last_pc);
} else {
if (tlbfill) {
@@ -210,7 +208,7 @@ static void loongarch_cpu_do_interrupt(CPUState *cs)
"BADI " TARGET_FMT_lx " SYS_NUM " TARGET_FMT_lu
" cpu %d asid " TARGET_FMT_lx "\n", __func__, env->pc,
tlbfill ? sys->CSR_TLBRERA : sys->CSR_ERA,
- cause, tlbfill ? "(refill)" : "", sys->CSR_ESTAT,
+ cause, tlbfill ? "(refill)" : "", cpu_csr_estat_get(sys),
sys->CSR_ECFG,
tlbfill ? sys->CSR_TLBRBADV : sys->CSR_BADV,
sys->CSR_BADI, env->gpr[11], cs->cpu_index,
---
base-commit: 499039798cdad7d86b787fec0eaf1da4151c0f05
change-id: 20260707-loong-race-63639e8f5afb
Best regards,
--
Miao Wang <shankerwangmiao@gmail.com>
On 2026/7/14 下午11:40, Miao Wang via B4 Relay wrote:
> From: Miao Wang <shankerwangmiao@gmail.com>
>
> 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()
yes, there is contention problem when injecting interrupt to vCPU in TCG
mode, only that it is a little strange to use atomic method on CSR estat
register.
From another side, only loongarch_cpu_set_irq() is called from other
threads, the other APIs are called from vCPU thread itself.
How about using method like KVM, adding software state
irq_pending/irq_clear in structure CPULoongArchState and using atomic
method, add cpu_sync_interrupt() and it is called function
cpu_loongarch_hw_interrupts_pending(). When vCPU thread is kicked with
new interrupt and executes cpu_loongarch_hw_interrupts_pending(), it
synchronize software intterupt irq_pending/irq_clear to CS ESTAT register.
Regards
Bibo Mao
>
> The access from the CPU thread is not synchronized with the access from
> other threads, which may lead to data races. To fix this, use atomic
> operations to read and write CSR_ESTAT.
>
> 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 <shankerwangmiao@gmail.com>
> ---
> target/loongarch/cpu.c | 8 +++----
> target/loongarch/internals.h | 27 +++++++++++++++++++++
> target/loongarch/tcg/csr_helper.c | 28 +++++++++++++++++-----
> target/loongarch/tcg/helper.h | 2 ++
> .../tcg/insn_trans/trans_privileged.c.inc | 13 ++++++++--
> target/loongarch/tcg/tcg_cpu.c | 12 ++++------
> 6 files changed, 71 insertions(+), 19 deletions(-)
>
> diff --git a/target/loongarch/cpu.c b/target/loongarch/cpu.c
> index fb03424ffa8cd5e5d250531cc2a36fa789fc25a9..be53307997d60ceaa404a6187eedf96dc51d82ff 100644
> --- a/target/loongarch/cpu.c
> +++ b/target/loongarch/cpu.c
> @@ -71,8 +71,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_csr_estat_deposit64(sys, irq, 1, level != 0);
> + if (FIELD_EX64(cpu_csr_estat_get(sys), CSR_ESTAT, IS)) {
> cpu_interrupt(cs, CPU_INTERRUPT_HARD);
> } else {
> cpu_reset_interrupt(cs, CPU_INTERRUPT_HARD);
> @@ -87,7 +87,7 @@ bool cpu_loongarch_hw_interrupts_pending(CPULoongArchState *env)
> uint32_t status;
> CPUSysState *sys = env_sys(env);
>
> - pending = FIELD_EX64(sys->CSR_ESTAT, CSR_ESTAT, IS);
> + pending = FIELD_EX64(cpu_csr_estat_get(sys), CSR_ESTAT, IS);
> status = FIELD_EX64(sys->CSR_ECFG, CSR_ECFG, LIE);
>
> return (pending & status) != 0;
> @@ -641,7 +641,7 @@ static void loongarch_cpu_reset_hold(Object *obj, ResetType type)
> sys->CSR_ECFG = FIELD_DP64(sys->CSR_ECFG, CSR_ECFG, VS, 0);
> sys->CSR_ECFG = FIELD_DP64(sys->CSR_ECFG, CSR_ECFG, LIE, 0);
>
> - sys->CSR_ESTAT = sys->CSR_ESTAT & (~MAKE_64BIT_MASK(0, 2));
> + qatomic_and(&sys->CSR_ESTAT, ~MAKE_64BIT_MASK(0, 2));
> sys->CSR_RVACFG = FIELD_DP64(sys->CSR_RVACFG, CSR_RVACFG, RBITS, 0);
> sys->CSR_CPUID = cs->cpu_index;
> sys->CSR_TCFG = FIELD_DP64(sys->CSR_TCFG, CSR_TCFG, EN, 0);
> diff --git a/target/loongarch/internals.h b/target/loongarch/internals.h
> index e01dbed40f6c7d02d2180d9dd5e17dacdd31755a..b94d229b2e0a4d48a5b712ad5ee364fb4a402055 100644
> --- a/target/loongarch/internals.h
> +++ b/target/loongarch/internals.h
> @@ -28,6 +28,33 @@ int ieee_ex_to_loongarch(int xcpt);
> void restore_fp_status(CPULoongArchState *env);
> #endif
>
> +static inline uint64_t cpu_csr_estat_get(CPUSysState *sys)
> +{
> + return qatomic_read(&sys->CSR_ESTAT);
> +}
> +
> +#define cpu_csr_estat_set(sys, field, val) \
> + do { \
> + uint64_t _estat = cpu_csr_estat_get(sys); \
> + uint64_t _new = FIELD_DP64(_estat, CSR_ESTAT, field, (val)); \
> + if (qatomic_cmpxchg(&(sys)->CSR_ESTAT, _estat, _new) == _estat) { \
> + break; \
> + } \
> + } while (1)
> +
> +static inline uint64_t cpu_csr_estat_deposit64(CPUSysState *sys, int start,
> + int len, uint64_t val)
> +{
> + do {
> + uint64_t _estat = cpu_csr_estat_get(sys);
> + uint64_t _new = deposit64(_estat, start, len, val);
> + if (qatomic_cmpxchg(&(sys)->CSR_ESTAT, _estat, _new) == _estat) {
> + return _estat;
> + }
> + } while (1);
> +}
> +
> +
> #ifndef CONFIG_USER_ONLY
> extern const VMStateDescription vmstate_loongarch_cpu;
>
> diff --git a/target/loongarch/tcg/csr_helper.c b/target/loongarch/tcg/csr_helper.c
> index 7dc33bc1802e5d4efc72648174bbf5e9baeaea54..023a6b1dc44f0df38c8c2f46b0b07eda88ddb3de 100644
> --- a/target/loongarch/tcg/csr_helper.c
> +++ b/target/loongarch/tcg/csr_helper.c
> @@ -90,7 +90,7 @@ target_ulong helper_csrrd_msgir(CPULoongArchState *env)
> return irq;
> }
>
> - sys->CSR_ESTAT = FIELD_DP64(sys->CSR_ESTAT, CSR_ESTAT, MSGINT, 0);
> + cpu_csr_estat_set(sys, MSGINT, 0);
> } else {
> /* bit 31 set 1 for no invalid irq */
> irq = BIT(31);
> @@ -99,15 +99,31 @@ target_ulong helper_csrrd_msgir(CPULoongArchState *env)
> return irq;
> }
>
> -target_ulong helper_csrwr_estat(CPULoongArchState *env, target_ulong val)
> +target_ulong helper_csrrd_estat(CPULoongArchState *env)
> {
> CPUSysState *sys = env_sys(env);
> - int64_t old_v = sys->CSR_ESTAT;
> + return cpu_csr_estat_get(sys);
> +}
>
> - /* Only IS[1:0] can be written */
> - sys->CSR_ESTAT = deposit64(sys->CSR_ESTAT, 0, 2, val);
> +target_ulong helper_csrwr_estat(CPULoongArchState *env, target_ulong val)
> +{
> + CPUSysState *sys = env_sys(env);
> + return cpu_csr_estat_deposit64(sys, 0, 2, val);
> +}
>
> - return old_v;
> +target_ulong helper_csrxhg_estat(CPULoongArchState *env, target_ulong val,
> + target_ulong mask)
> +{
> + CPUSysState *sys = env_sys(env);
> + mask &= MAKE_64BIT_MASK(0, 2);
> + val &= mask;
> + do {
> + uint64_t _estat = cpu_csr_estat_get(sys);
> + uint64_t _new = (_estat & ~mask) | val;
> + if (qatomic_cmpxchg(&(sys)->CSR_ESTAT, _estat, _new) == _estat) {
> + return _estat;
> + }
> + } while (1);
> }
>
> target_ulong helper_csrwr_asid(CPULoongArchState *env, target_ulong val)
> diff --git a/target/loongarch/tcg/helper.h b/target/loongarch/tcg/helper.h
> index 8a6c62f11618d7125122831f4a9fe0d0fab73364..58089e84c60fa49524b4705981d0f677cfb439d2 100644
> --- a/target/loongarch/tcg/helper.h
> +++ b/target/loongarch/tcg/helper.h
> @@ -101,8 +101,10 @@ DEF_HELPER_1(csrrd_pgd, i64, env)
> DEF_HELPER_1(csrrd_cpuid, i64, env)
> DEF_HELPER_1(csrrd_tval, i64, env)
> DEF_HELPER_1(csrrd_msgir, i64, env)
> +DEF_HELPER_1(csrrd_estat, i64, env)
> DEF_HELPER_2(csrwr_stlbps, i64, env, tl)
> DEF_HELPER_2(csrwr_estat, i64, env, tl)
> +DEF_HELPER_3(csrxhg_estat, i64, env, tl, tl)
> DEF_HELPER_2(csrwr_asid, i64, env, tl)
> DEF_HELPER_2(csrwr_tcfg, i64, env, tl)
> DEF_HELPER_2(csrwr_ticlr, i64, env, tl)
> diff --git a/target/loongarch/tcg/insn_trans/trans_privileged.c.inc b/target/loongarch/tcg/insn_trans/trans_privileged.c.inc
> index 6728ce5ec9387d1f66d68cf36abbe6e0997c639a..e6e9072518767479f66bae1cf1c72844df4343d4 100644
> --- a/target/loongarch/tcg/insn_trans/trans_privileged.c.inc
> +++ b/target/loongarch/tcg/insn_trans/trans_privileged.c.inc
> @@ -75,7 +75,7 @@ static bool set_csr_trans_func(unsigned int csr_num, GenCSRRead readfn,
> void loongarch_csr_translate_init(void)
> {
> SET_CSR_FUNC(STLBPS, NULL, gen_helper_csrwr_stlbps);
> - SET_CSR_FUNC(ESTAT, NULL, gen_helper_csrwr_estat);
> + SET_CSR_FUNC(ESTAT, gen_helper_csrrd_estat, gen_helper_csrwr_estat);
> SET_CSR_FUNC(ASID, NULL, gen_helper_csrwr_asid);
> SET_CSR_FUNC(PGD, gen_helper_csrrd_pgd, NULL);
> SET_CSR_FUNC(PWCL, NULL, gen_helper_csrwr_pwcl);
> @@ -187,7 +187,16 @@ static bool trans_csrxchg(DisasContext *ctx, arg_csrxchg *a)
> return false;
> }
>
> - /* So far only readonly csrs have readfn. */
> + if (a->csr == LOONGARCH_CSR_ESTAT) {
> + src1 = gpr_src(ctx, a->rd, EXT_NONE);
> + mask = gpr_src(ctx, a->rj, EXT_NONE);
> + oldv = tcg_temp_new();
> + gen_helper_csrxhg_estat(oldv, tcg_env, src1, mask);
> + gen_set_gpr(a->rd, oldv, EXT_NONE);
> + return true;
> + }
> +
> + /* So far only readonly csrs have readfn, except ESTAT. */
> assert(csr->readfn == NULL);
>
> src1 = gpr_src(ctx, a->rd, EXT_NONE);
> diff --git a/target/loongarch/tcg/tcg_cpu.c b/target/loongarch/tcg/tcg_cpu.c
> index 4b1d44a16445841de8b9a7d69d8f389c357e6abf..64a28c7dcbbce2f2ce47640471602485344ca8e3 100644
> --- a/target/loongarch/tcg/tcg_cpu.c
> +++ b/target/loongarch/tcg/tcg_cpu.c
> @@ -160,10 +160,8 @@ static void loongarch_cpu_do_interrupt(CPUState *cs)
> sys->CSR_TLBRERA = FIELD_DP64(sys->CSR_TLBRERA, CSR_TLBRERA,
> PC, (env->pc >> 2));
> } else {
> - sys->CSR_ESTAT = FIELD_DP64(sys->CSR_ESTAT, CSR_ESTAT, ECODE,
> - EXCODE_MCODE(cause));
> - sys->CSR_ESTAT = FIELD_DP64(sys->CSR_ESTAT, CSR_ESTAT, ESUBCODE,
> - EXCODE_SUBCODE(cause));
> + cpu_csr_estat_set(sys, ECODE, EXCODE_MCODE(cause));
> + cpu_csr_estat_set(sys, ESUBCODE, EXCODE_SUBCODE(cause));
> sys->CSR_PRMD = FIELD_DP64(sys->CSR_PRMD, CSR_PRMD, PPLV,
> FIELD_EX64(sys->CSR_CRMD, CSR_CRMD, PLV));
> sys->CSR_PRMD = FIELD_DP64(sys->CSR_PRMD, CSR_PRMD, PIE,
> @@ -181,7 +179,7 @@ static void loongarch_cpu_do_interrupt(CPUState *cs)
> if (cs->exception_index == EXCCODE_INT) {
> /* Interrupt */
> uint32_t vector = 0;
> - uint32_t pending = FIELD_EX64(sys->CSR_ESTAT, CSR_ESTAT, IS);
> + uint32_t pending = FIELD_EX64(cpu_csr_estat_get(sys), CSR_ESTAT, IS);
> pending &= FIELD_EX64(sys->CSR_ECFG, CSR_ECFG, LIE);
>
> /* Find the highest-priority interrupt. */
> @@ -195,7 +193,7 @@ static void loongarch_cpu_do_interrupt(CPUState *cs)
> TARGET_FMT_lx "\n",
> __func__, env->pc, sys->CSR_ERA,
> cause, sys->CSR_BADV, sys->CSR_DERA, vector,
> - sys->CSR_ECFG, sys->CSR_ESTAT);
> + sys->CSR_ECFG, cpu_csr_estat_get(sys));
> qemu_plugin_vcpu_interrupt_cb(cs, last_pc);
> } else {
> if (tlbfill) {
> @@ -210,7 +208,7 @@ static void loongarch_cpu_do_interrupt(CPUState *cs)
> "BADI " TARGET_FMT_lx " SYS_NUM " TARGET_FMT_lu
> " cpu %d asid " TARGET_FMT_lx "\n", __func__, env->pc,
> tlbfill ? sys->CSR_TLBRERA : sys->CSR_ERA,
> - cause, tlbfill ? "(refill)" : "", sys->CSR_ESTAT,
> + cause, tlbfill ? "(refill)" : "", cpu_csr_estat_get(sys),
> sys->CSR_ECFG,
> tlbfill ? sys->CSR_TLBRBADV : sys->CSR_BADV,
> sys->CSR_BADI, env->gpr[11], cs->cpu_index,
>
> ---
> base-commit: 499039798cdad7d86b787fec0eaf1da4151c0f05
> change-id: 20260707-loong-race-63639e8f5afb
>
> Best regards,
>
Hi, > 2026年7月15日 11:16，Bibo Mao <maobibo@loongson.cn> 写道： > > > > On 2026/7/14 下午11:40, Miao Wang via B4 Relay wrote: >> From: Miao Wang <shankerwangmiao@gmail.com> >> 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() > > yes, there is contention problem when injecting interrupt to vCPU in TCG mode, only that it is a little strange to use atomic method on CSR estat register. > > From another side, only loongarch_cpu_set_irq() is called from other threads, the other APIs are called from vCPU thread itself. > > How about using method like KVM, adding software state irq_pending/irq_clear in structure CPULoongArchState and using atomic method, add cpu_sync_interrupt() and it is called function > cpu_loongarch_hw_interrupts_pending(). When vCPU thread is kicked with new interrupt and executes cpu_loongarch_hw_interrupts_pending(), it synchronize software intterupt irq_pending/irq_clear to CS ESTAT register. > I understand that adding an additional field which is protected by atomic operations to store the pending changes to ESTAT in loongarch_cpu_set_irq() and merging it into ESTAT afterwards in cpu_loongarch_hw_interrupts_pending() is simpler and cleaner. However, there might be a problem here. In loongarch_cpu_set_irq(), without reading ESTAT, it cannot decide whether to call cpu_interrupt() or cpu_reset_interrupt(). If ESTAT should be read in loongarch_cpu_set_irq(), then ESTAT should still be protected by atomic operations. Cheers, Miao Wang
On 2026/7/15 下午4:29, Miao Wang wrote:
> Hi,
>
>> 2026年7月15日 11:16，Bibo Mao <maobibo@loongson.cn> 写道：
>>
>>
>>
>> On 2026/7/14 下午11:40, Miao Wang via B4 Relay wrote:
>>> From: Miao Wang <shankerwangmiao@gmail.com>
>>> 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()
>>
>> yes, there is contention problem when injecting interrupt to vCPU in TCG mode, only that it is a little strange to use atomic method on CSR estat register.
>>
>> From another side, only loongarch_cpu_set_irq() is called from other threads, the other APIs are called from vCPU thread itself.
>>
>> How about using method like KVM, adding software state irq_pending/irq_clear in structure CPULoongArchState and using atomic method, add cpu_sync_interrupt() and it is called function
>> cpu_loongarch_hw_interrupts_pending(). When vCPU thread is kicked with new interrupt and executes cpu_loongarch_hw_interrupts_pending(), it synchronize software intterupt irq_pending/irq_clear to CS ESTAT register.
>>
>
> I understand that adding an additional field which is protected by atomic
> operations to store the pending changes to ESTAT in loongarch_cpu_set_irq()
> and merging it into ESTAT afterwards in cpu_loongarch_hw_interrupts_pending()
> is simpler and cleaner.
>
> However, there might be a problem here. In loongarch_cpu_set_irq(), without
> reading ESTAT, it cannot decide whether to call cpu_interrupt() or
> cpu_reset_interrupt(). If ESTAT should be read in loongarch_cpu_set_irq(), then
> ESTAT should still be protected by atomic operations.
Is it ok to judge from software pending interrupt rather than ESTAT?
something like this,
if (level == 1) {
cpu_interrupt(cs, CPU_INTERRUPT_HARD);
} else if (!env->pending_int) {
cpu_reset_interrupt(cs, CPU_INTERRUPT_HARD);
}
Regards
Bibo Mao
>
> Cheers,
>
> Miao Wang
>
Hi,
> 2026年7月15日 16:58，Bibo Mao <maobibo@loongson.cn> 写道：
>
>
>
> On 2026/7/15 下午4:29, Miao Wang wrote:
>> Hi,
>>> 2026年7月15日 11:16，Bibo Mao <maobibo@loongson.cn> 写道：
>>>
>>>
>>>
>>> On 2026/7/14 下午11:40, Miao Wang via B4 Relay wrote:
>>>> From: Miao Wang <shankerwangmiao@gmail.com>
>>>> 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()
>>>
>>> yes, there is contention problem when injecting interrupt to vCPU in TCG mode, only that it is a little strange to use atomic method on CSR estat register.
>>>
>>> From another side, only loongarch_cpu_set_irq() is called from other threads, the other APIs are called from vCPU thread itself.
>>>
>>> How about using method like KVM, adding software state irq_pending/irq_clear in structure CPULoongArchState and using atomic method, add cpu_sync_interrupt() and it is called function
>>> cpu_loongarch_hw_interrupts_pending(). When vCPU thread is kicked with new interrupt and executes cpu_loongarch_hw_interrupts_pending(), it synchronize software intterupt irq_pending/irq_clear to CS ESTAT register.
>>>
>> I understand that adding an additional field which is protected by atomic
>> operations to store the pending changes to ESTAT in loongarch_cpu_set_irq()
>> and merging it into ESTAT afterwards in cpu_loongarch_hw_interrupts_pending()
>> is simpler and cleaner.
>> However, there might be a problem here. In loongarch_cpu_set_irq(), without
>> reading ESTAT, it cannot decide whether to call cpu_interrupt() or
>> cpu_reset_interrupt(). If ESTAT should be read in loongarch_cpu_set_irq(), then
>> ESTAT should still be protected by atomic operations.
> Is it ok to judge from software pending interrupt rather than ESTAT? something like this,
> if (level == 1) {
> cpu_interrupt(cs, CPU_INTERRUPT_HARD);
> } else if (!env->pending_int) {
> cpu_reset_interrupt(cs, CPU_INTERRUPT_HARD);
> }
If there is such a field (say, pending_int) in the cpu state, we may have two
options. The first is to store pending changes to ESTAT in that field. The
second is to store the expected result of changed ESTAT[15:0]. If it is the
first case, then by judging from only pending_int, the already set bits in ESTAT
might be ignored in your ``else if" case. If it is the second case, then the
added pending_int field becomes a shadow of ESTAT[15:0] and we should precisely
handle the synchronization between pending_int and ESTAT[15:0] and thus the
changes would not be simpler than to simply protect ESTAT with atomic operations.
I just came up with another idea. What if we always cpu_interrupt() in
loongarch_cpu_set_irq(), and call cpu_reset_interrupt() in
cpu_loongarch_hw_interrupts_pending() after merging the changes into ESTAT and
if CSR_ESTAT[15:0] is zero. The pseudocode is:
loongarch_cpu_set_irq(cs, irq, level)
{
atomically_record_changes_to_estat(&cs->pending_int, irq, level);
cpu_interrupt(cs, CPU_INTERRUPT_HARD);
}
cpu_loongarch_hw_interrupts_pending(cs)
{
changes = atomically_fetch_and_clear_changes_to_estat(&cs->pending_int);
merge_changes_to_estat(&cs->CSR_ESTAT, changes);
if (pending interrupts in CSR_ESTAT[15:0]) {
return true;
} else {
cpu_reset_interrupt(cs, CPU_INTERRUPT_HARD);
return false;
}
}
Cheers,
Miao Wang
© 2016 - 2026 Red Hat, Inc.