Re: [kvm-devel] [patch 1/2] KVM: hlt emulation should take in-kernel APIC/PIT timers into account

[Marcelo T. <mto...@re...>]

On Sat, May 10, 2008 at 10:12:02AM +0800, Yang, Sheng wrote:
> > Did you have kvm.git commit 8ae6dc90ac84d9734e343210c8ec709f50cd9d89
> > when testing this?
> >
> > I believe it should fix that issue, because "ps->inject_pending" won't
> > be set by kvm_pit_timer_intr_post() if the IRQ is masked. Please correct
> > me if I'm wrong.
> 
> Oh, sorry, I missed that commit. But... It just solved an half of the problem. 
> LAPIC suffered from it as well, and the current HLT emulation still didn't 
> work... And I can't find something like inject_pending in LAPIC timer.
> 
> I have to say, I think my method is more preciously, directly and efficient... 
> It also can be extended easily if we got more clock sources (though I don't 
> think this would happen in near future...). In fact, I think take care of 
> pending counts is some kind of *wrong concept*... We should take care of the 
> window, or when the increment of pending counters happened, CMIIW. And it got 
> nothing to do with the current counter number (yeah, I realized it after saw 
> the hlt behaviour in XP, not before ;) ).

Sheng,

The problem is that you don't want to emulate hlt if you have a pending
timer _and_ the guest is accepting events. So for example if there are
two apic timers pending, you inject one of them, guest execute's hlt, we
end up in vcpu_block().

Does this work for you?

diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
index 7652f88..d41e34c 100644
--- a/arch/x86/kvm/lapic.c
+++ b/arch/x86/kvm/lapic.c
@@ -961,7 +961,7 @@ int apic_has_pending_timer(struct kvm_vcpu *vcpu)
 {
 	struct kvm_lapic *lapic = vcpu->arch.apic;
 
-	if (lapic)
+	if (lapic && apic_enabled(lapic) && apic_lvt_enabled(lapic, APIC_LVTT))
 		return atomic_read(&lapic->timer.pending);
 
 	return 0;