Re: FW: [kvm-commits] KVM: Move interrupt injection out of interruptdisabled section

[Avi Kivity <avi-atKUWr5tajBWk0Htik3J/w@public.gmane.org>]

Dong, Eddie wrote:
>>>> previous irq injection, next injection will be refused with IRQ
>>>> window enabled. This is because current implementation will inject
>>>> exception earlier than irq injection and vmx_intr_assist doesn;t
>>>> know if previous injected event is external irq (and thus
>>>> overwrite) or exception. Guest will see lower priority irq get
>>>> injected rather than higher priority IRQ which
>>>> arrives later.
>>>>
>>>>
>>>>         
>> Well, the same thing happens with the code before this commit,
>> no?  If a
>>     
>
> Why? 
> The philosiphy here is that the vCPU may receive new virtual irqs,
> but it will not try to injected. From guest point of view, an old irq
> (say irq_lo) arrives earlier and right before the virtual instruction
> boundary,
> and thus get responsed, while the new high priority irq (irq_hi) arrives
> a little bit later than irq_lo, but right after the instruction boundary
> and thus couldn't preempt irq_lo.
>
> When guest response to irq_lo, eflag.if=0, so irq_hi can't be responsed
> at that time although it see irq_hi in IRR.
>
> That align with native where it uses micro-code.
>
> If we enable pirq before physical irq is delivered, current code has 
> obvious issue (see below comments too).
> If we insist to move virq deliver out of irq disable. Then we need to
> avoid 2nd injection of irq. But a big side effect is that frequently 
> higher priority irq such as NIC/IDE irq will arrive right after lower
> priority irq which will obviously hurt various benchmark and even
> correctness.
>
> I will argue the necessaity of this code movement. A VM
> Exit/Resume itself probably will block system responsibility 
> (can't deliver irq) in the level of 1K cycles, interrupt injection is
> clearly far less than this granularity. Probably less than 100cycles.
>
>   

The reason for the code movement is that interrupt injection in real 
mode on Intel must be emulated, which means writing into guest memory, 
which means sleeping operations once swap is enabled.  A side benefit is 
removing the lapic calculations from the critical section.

But I still don't understand your object.  The interrupt has to be 
committed sometime.  We move the commit point earlier by the time it 
takes to ->prepare_guest_switch() (normally zero, but sometimes some 
thousand cycles), and kvm_load_guest_fpu() (usually zero as well).  The 
timing is sometimes worse, but there was always a window where a high 
priority interrupt is ignored over low priority interrupts.

[we can try to improve it by using vm86 interrupt redirection which may 
allow event injection using VT instead of writing to the guest stack].

>   
>> high priority interrupt arrives after injection, it will have to wait.
>> The difference is that before this commit, it woke up with the IPI and
>> now it notices with KVM_REQ_INTR.
>>     
>
> When physical IRQ is disabled, the physical IPI will not interrupt 
> pCPU, it get delivered only after we resume to guest.
>
>   

But the effect is the same: the high priority interrupt is delayed.

>> The window grew larger, but not by much typically.
>>     
>
>
> Maybe not much typical, but will frequently happen.
> With this commit, now the window becomes huge since 
> vcpu deschedule may happen :-(
>
>   

If the vcpu is descheduled, then both interrupts are delayed, by much 
more than it takes to process a single one.  If we're descheduled then 
interrupt priority is a very small problem with respect to timing.

> Another serious issue in current code is that due to previous 
> injection, the irq is already consumed, (IRR->ISR). So if there
> are higher priority IRQs arrive later after the previous injection,
> now guest will see 2 IRQs consumed, i.e. 2 ISR bits are set.
>
>   

I don't understand why.  If an event is pending in 
VM_ENTRY_INTR_INFO_FIELD, then we don't call kvm_cpu_get_interrupt() at all.

> I will guess this one is the major problem source if no-kvm-irqchip
> & kvm_irqchip still has difference. And I bet it should cause many
> issues.
>   
Well, I can thing of a workaround by using vm86 interrupt redirection.  
But I'm not convinced it is necessary.

-- 
error compiling committee.c: too many arguments to function


-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/