From mboxrd@z Thu Jan  1 00:00:00 1970
From: Marcelo Tosatti <mtosatti@redhat.com>
Subject: Re: [patch 2/2] KVM: PIT: provide an option to disable interrupt
	reinjection
Date: Tue, 30 Dec 2008 14:13:16 -0200
Message-ID: <20081230161316.GA5588@amt.cnet>
References: <20081229173824.759371918@localhost.localdomain> <20081229174009.523082019@localhost.localdomain> <4959F301.2030104@redhat.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Cc: kvm@vger.kernel.org, sheng@linux.intel.com
To: Avi Kivity <avi@redhat.com>
Return-path: <kvm-owner@vger.kernel.org>
Received: from mx2.redhat.com ([66.187.237.31]:42754 "EHLO mx2.redhat.com"
	rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
	id S1751446AbYL3QNa (ORCPT <rfc822;kvm@vger.kernel.org>);
	Tue, 30 Dec 2008 11:13:30 -0500
Content-Disposition: inline
In-Reply-To: <4959F301.2030104@redhat.com>
Sender: kvm-owner@vger.kernel.org
List-ID: <kvm.vger.kernel.org>

On Tue, Dec 30, 2008 at 12:08:01PM +0200, Avi Kivity wrote:
> Marcelo Tosatti wrote:
>> Certain clocks (such as TSC) in older 2.6 guests overaccount for lost
>> ticks, causing severe time drift. Interrupt reinjection magnifies the
>> problem.
>>
>> Provide an option to disable it.
>>
>> 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
>> @@ -201,6 +201,9 @@ static int __pit_timer_fn(struct kvm_kpi
>>  	if (!atomic_inc_and_test(&pt->pending))
>>  		set_bit(KVM_REQ_PENDING_TIMER, &vcpu0->requests);
>>  +	if (pt->no_reinject)
>> +		atomic_set(&pt->pending, 1);
>> +
>>   
>
> What about moving this to the place where we atomic_inc()?  Any reason  
> not to?

The atomic_inc_and_test is right above, and we want to set
KVM_REQ_PENDING_TIMER on 0->1 transitions, so better keep it
separate. BTW the KVM_REQ_PENDING_TIMER optimization is broken
(atomic_inc_and_test returns true if the _new_ value is zero) but thats
another issue.

Can't see the improvement you're suggesting.

>> --- kvm.orig/arch/x86/kvm/i8254.h
>> +++ kvm/arch/x86/kvm/i8254.h
>> @@ -9,6 +9,7 @@ struct kvm_kpit_timer {
>>  	s64 period; /* unit: ns */
>>  	s64 scheduled;
>>  	atomic_t pending;
>> +	bool no_reinject;
>>  };
>>   
>
> Negative logic = !good.  Call it reinject and init to true.

Sure.