Hi, > 2026年7月20日 17:04,Bibo Mao <[email protected]> 写道: > > > > On 2026/7/20 下午4:49, Miao Wang wrote: >> Hi, >>> 2026年7月20日 16:34,Bibo Mao <[email protected]> 写道: >>> >>> >>> >>> On 2026/7/16 上午4:48, Miao Wang via B4 Relay wrote: >>>> 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)); >>> This is a good method with async_run_on_cpu, there is one small issue with >>> irq == 0 interrupt injection where -irq/irq are both 0. >> Since according to Section 7.4.6. of Loongarch Reference Manual Vol1, >> bit [1:0] stands for the software interrupt SW1 and SW0, I expect that >> these two irqs should not be manipulated by hardware sources and thus >> the irq here should be larger than 2. Can you further explain under >> what condition the irq line 0 and 1 of a cpu core would be connected >> to external outputs. > loongarch_cpu_set_irq() is not only for external interrupts, percpu interrupt > injection also uses loongarch_cpu_set_irq() such as IRQ_TIMER etc.
The users of loongarch_cpu_set_irq() are limited. The main usage is as the callback of qdev_init_gpio_in(). Other usage is for the timer, where the irq number is IRQ_TIMER, i.e. 11. > With SW1 and SW0, if the CPU set bit0 of estat, it will inject SW0 to this > CPU itself. And de-assert the irq if the CPU clear bit0 of estat. When SW1 and/or SW0 are set by the guest code running in the emulated CPU, the handling of the software IRQs in the qemu side does not involve loongarch_cpu_set_irq(). As a result, I believe that it is safe to assume that the second parameter of loongarch_cpu_set_irq(), i.e. irq should not be smaller than 2. I also suggest enforce this check at the entrance of loongarch_cpu_set_irq(). Cheers, Miao Wang
