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

Marcelo Tosatti
2008-05-12T16:40:05Z

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;