Re: [PATCH v2] target/i386: Give IRQs a chance when resetting HF_INHIBIT_IRQ_MASK

["Philippe Mathieu-Daudé" <philmd@linaro.org>]

On 15/4/24 11:32, Paolo Bonzini wrote:
> On Mon, Apr 15, 2024 at 8:50 AM Ruihan Li <lrh2000@pku.edu.cn> wrote:
>>
>> When emulated with QEMU, interrupts will never come in the following
>> loop. However, if the NOP instruction is uncommented, interrupts will
>> fire as normal.
>>
>>          loop:
>>                  cli
>>                  call do_sti
>>                  jmp loop
>>
>>          do_sti:
>>                  sti
>>                  # nop
>>                  ret
>>
>> This behavior is different from that of a real processor. For example,
>> if KVM is enabled, interrupts will always fire regardless of whether the
>> NOP instruction is commented or not. Also, the Intel Software Developer
>> Manual states that after the STI instruction is executed, the interrupt
>> inhibit should end as soon as the next instruction (e.g., the RET
>> instruction if the NOP instruction is commented) is executed.
> 
> Thanks, interesting bug!
> 
> What do you think about writing this:
> 
>>       /* If several instructions disable interrupts, only the first does it.  */
>>       if (inhibit && !(s->flags & HF_INHIBIT_IRQ_MASK)) {
>>           gen_set_hflag(s, HF_INHIBIT_IRQ_MASK);
>> -    } else {
>> +        inhibit_reset = false;
>> +    } else if (!inhibit && (s->flags & HF_INHIBIT_IRQ_MASK)) {
>>           gen_reset_hflag(s, HF_INHIBIT_IRQ_MASK);
>> +        inhibit_reset = true;
>> +    } else {
>> +        inhibit_reset = false;
>>       }
> 
> in a slightly simpler manner:
> 
>      inhibit_reset = false;
>      if (s->flags & HF_INHIBIT_IRQ_MASK) {
>          gen_reset_hflag(s, HF_INHIBIT_IRQ_MASK);
>          inhibit_reset = true;
>      } else if (inhibit) {
>          gen_set_hflag(s, HF_INHIBIT_IRQ_MASK);
>      }
> 
> No need to submit v3, I can do the change myself when applying.

Cc: qemu-stable@nongnu.org