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

On 2026-08-05 8:56, Bibo Mao wrote:
> applied to loongarch-next with small modification.
> 
> diff --git a/target/loongarch/tcg/csr_helper.c b/target/loongarch/tcg/ 
> csr_helper.c
> index 7dc33bc180..d8d54d81e0 100644
> --- a/target/loongarch/tcg/csr_helper.c
> +++ b/target/loongarch/tcg/csr_helper.c
> @@ -103,9 +103,22 @@ target_ulong helper_csrwr_estat(CPULoongArchState 
> *env, target_ulong val)
>   {
>       CPUSysState *sys = env_sys(env);
>       int64_t old_v = sys->CSR_ESTAT;
> +    CPUState *cs = env_cpu(env);
> 
>       /* 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().
> +     */
> +    bql_lock();
> +    if (FIELD_EX64(sys->CSR_ESTAT, CSR_ESTAT, IS)) {
> +        cpu_interrupt(cs, CPU_INTERRUPT_HARD);
> +    } else {
> +        cpu_reset_interrupt(cs, CPU_INTERRUPT_HARD);
> +    }
> +    bql_unlock();
> 
>       return old_v;
>   }
> 
> On 2026/4/15 下午7:17, Andrew S. Rightenburg via qemu development wrote:
>> 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().

Eh I don't understand this justification about taking the BQL...

If we can not call loongarch_cpu_set_irq() directly, we should extract
the common code in a common method and re-use, not duplicate the same
code.

>>
>> Fixes: 5b1dedfe848b ("target/loongarch: Add LoongArch CSR instruction")
>> Reported-by: Andrew S. Rightenburg <[email protected]>
>> Signed-off-by: Andrew S. Rightenburg <[email protected]>
>> ---
>>   target/loongarch/tcg/csr_helper.c | 12 ++++++++++++
>>   1 file changed, 12 insertions(+)
>>
>> diff --git a/target/loongarch/tcg/csr_helper.c b/target/loongarch/tcg/ 
>> csr_helper.c
>> index cd35ca93c7..0a5ed52eb6 100644
>> --- a/target/loongarch/tcg/csr_helper.c
>> +++ b/target/loongarch/tcg/csr_helper.c
>> @@ -98,10 +98,22 @@ target_ulong helper_csrrd_msgir(CPULoongArchState 
>> *env)
>>   target_ulong helper_csrwr_estat(CPULoongArchState *env, target_ulong 
>> val)
>>   {
>>       int64_t old_v = env->CSR_ESTAT;
>> +    CPUState *cs = env_cpu(env);
>>       /* Only IS[1:0] can be written */
>>       env->CSR_ESTAT = deposit64(env->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 (FIELD_EX64(env->CSR_ESTAT, CSR_ESTAT, IS)) {
>> +        bql_lock();
>> +        cpu_interrupt(cs, CPU_INTERRUPT_HARD);
>> +        bql_unlock();
>> +    }
>> +
>>       return old_v;
>>   }
>>
> 
>