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

On 5/8/26 09:33, Bibo Mao wrote:
> 
> 
> On 2026/8/5 下午3:19, Philippe Mathieu-Daudé wrote:
>> 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...
> I guess there is g_assert(bql_locked()) in cpu_interrupt() -:)

Hmm OK, thanks.

>>
>> 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.
> yes, I think it is better to call loongarch_cpu_set_irq() directly to 
> avoid duplicated code.
> 
> Regards
> Bibo Mao