Re: [patch 3/3] KVM: PIT: fix injection logic and count

[Avi Kivity <avi@qumranet.com>]

Marcelo Tosatti wrote:
> The PIT injection logic is problematic under the following cases:
>
> 1) If there is a higher priority vector to be delivered by the time
> kvm_pit_timer_intr_post is invoked ps->inject_pending won't be set. 
> This opens the possibility for missing many PIT event injections (say if
> guest executes hlt at this point).
>
> 2) ps->inject_pending is racy with more than two vcpus. Since there's no locking
> around read/dec of pt->pending, two vcpu's can inject two interrupts for a single
> pt->pending count.
>
> Fix 1 by using an irq ack notifier: only reinject when the previous irq 
> has been acked. Fix 2 with appropriate locking around manipulation of 
> pending count and irq_ack by the injection / ack paths.
>
> Also, count_load_time should be set at the time the count is reloaded,
> not when the interrupt is injected (BTW, LAPIC uses the same apparently
> broken scheme, could someone explain what was the reasoning behind
> that? kvm_apic_timer_intr_post).
>
> Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com>
>
> Index: kvm/arch/x86/kvm/i8254.c
> ===================================================================
> --- kvm.orig/arch/x86/kvm/i8254.c
> +++ kvm/arch/x86/kvm/i8254.c
> @@ -207,6 +207,8 @@ static int __pit_timer_fn(struct kvm_kpi
>  
>  	pt->timer.expires = ktime_add_ns(pt->timer.expires, pt->period);
>  	pt->scheduled = ktime_to_ns(pt->timer.expires);
> +	if (pt->period)
> +		ps->channels[0].count_load_time = pt->timer.expires;
>  
>  	return (pt->period == 0 ? 0 : 1);
>  }
>   

Sometimes the guest leaves the timer enabled but the output pin masked, 
(e.g. it doesn't use the timer bug doesn't bother to turn it off 
properly).  This results in extraneous interrupts, causing unnecessary 
vmexits and increased power usage.

But if we detect that the guest isn't processing the interrupts, we can 
turn the timer off, and after the next injection, calculate the number 
of missing interrupts, and turn the timer on again.  This will have to 
be done carefully (taking care of the guest adjusting the frequency 
during the period where we missed injections, for example).

Comments?  Questions?  Patches?

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