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

[Jan Kiszka <jan.kiszka@siemens.com>]

Avi Kivity wrote:
> 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.

Right now, you only call enable_nmi_window() if that window is currently
closed - and that's not the common case I'm worried about.

> 
> 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.

Yeah, probably. I'm just wondering now if we can set
exit-on-interrupt-window while the vcpu state is interruptible (ie.
_before_ the injection). There is some entry check like this for NMIs,
but maybe no for interrupts. Need to check.

Jan

-- 
Siemens AG, Corporate Technology, CT SE 2 ES-OS
Corporate Competence Center Embedded Linux