Re: [PATCH] KVM: VMX: Fix race between pending IRQ and NMI

[Avi Kivity <avi@redhat.com>]

Jan Kiszka wrote:
>> The nmi handler could change the tpr to mask the preempted interrupt;
>> but the code would not notice that.  Once the interrupt was injected the
>> guest would see an interrupt at a higher priority than it has programmed
>> the hardware to allow.
>>     
>
> I consider this a bit far fetch. What sane NMI handler would fiddle with
> the APIC? It would be fairly tricky to properly synchronize this with
> the rest of the OS.
>   

Sure, this is not a realistic guest.

>> Basically, once we commit to an interrupt via kvm_cpu_get_interrupt(),
>> we must inject it before the any instruction gets executed.
>>
>> I don't think any real guest would notice, though.
>>
>>     
>
> Well, I have no problems with your approach (when also applied on the
> user space irqchip path) of keeping the order *if* we can ensure that
> only the first instruction of the IRQ handler is executed and we will
> then inject the NMI. Otherwise this opens a prio inversion between IRQs
> and NMIs. The point is that, unless I'm overseeing some detail right
> now, your approach will inject the pending NMI only once the guest
> /happens/ to exit the VM, right? If yes, then it's a no-go IMHO, also
> for keeping this property with the queue approach.
>   

enable_nmi_window() should cause an exit once the interrupt has been 
injected (likely before the first interrupt handler instruction was 
executed, but after the stack frame was created).  So the nmi will not 
be delayed.

But I think I see a bigger issue - if we inject an regular interrupt 
while another is pending, then we will encounter this problem.  Looks 
like we have to enable the interrupt window after injecting an interrupt 
if there are still pending interrupts.

-- 
I have a truly marvellous patch that fixes the bug which this
signature is too narrow to contain.