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

["Yang, Sheng" <sheng.yang@intel.com>]

On Tuesday 13 May 2008 00:40:05 Marcelo Tosatti wrote:
> 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?

Yeah. I also suggest using the consistent implement for all the 
_has_pending_timer. (in fact, if take pending counters as the interrupts 
which have to delay their injection, the explanation is well :) )

-- 
Thanks
Yang, Sheng
>
> 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;



-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft 
Defy all challenges. Microsoft(R) Visual Studio 2008. 
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/