From mboxrd@z Thu Jan  1 00:00:00 1970
From: Marcelo Tosatti <mtosatti@redhat.com>
Subject: Re: [patch 2/3] KVM: x86: add option to advance tscdeadline hrtimer
 expiration
Date: Tue, 16 Dec 2014 13:13:55 -0200
Message-ID: <20141216151355.GC32479@amt.cnet>
References: <20141215220605.806000829@redhat.com>
 <20141215220916.959811227@redhat.com>
 <549042EE.8090205@redhat.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Cc: kvm@vger.kernel.org, Luiz Capitulino <lcapitulino@redhat.com>,
	Rik van Riel <riel@redhat.com>,
	Radim Krcmar <rkrcmar@redhat.com>
To: Paolo Bonzini <pbonzini@redhat.com>
Return-path: <kvm-owner@vger.kernel.org>
Received: from mx1.redhat.com ([209.132.183.28]:50431 "EHLO mx1.redhat.com"
	rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
	id S1751053AbaLPPOK (ORCPT <rfc822;kvm@vger.kernel.org>);
	Tue, 16 Dec 2014 10:14:10 -0500
Received: from int-mx14.intmail.prod.int.phx2.redhat.com (int-mx14.intmail.prod.int.phx2.redhat.com [10.5.11.27])
	by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id sBGFE987010739
	(version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=FAIL)
	for <kvm@vger.kernel.org>; Tue, 16 Dec 2014 10:14:09 -0500
Content-Disposition: inline
In-Reply-To: <549042EE.8090205@redhat.com>
Sender: kvm-owner@vger.kernel.org
List-ID: <kvm.vger.kernel.org>

On Tue, Dec 16, 2014 at 03:34:22PM +0100, Paolo Bonzini wrote:
> On 15/12/2014 23:06, Marcelo Tosatti wrote:
> > For the hrtimer which emulates the tscdeadline timer in the guest,
> > add an option to advance expiration, and busy spin on VM-entry waiting
> > for the actual expiration time to elapse.
> > 
> > This allows achieving low latencies in cyclictest (or any scenario 
> > which requires strict timing regarding timer expiration).
> > 
> > Reduces average cyclictest latency from 12us to 8us
> > on Core i5 desktop.
> > 
> > Note: this option requires tuning to find the appropriate value 
> > for a particular hardware/guest combination. One method is to measure the 
> > average delay between apic_timer_fn and VM-entry. 
> > Another method is to start with 1000ns, and increase the value
> > in say 500ns increments until avg cyclictest numbers stop decreasing.
> > 
> > Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com>
> > 
> > Index: kvm/arch/x86/kvm/lapic.c
> > ===================================================================
> > --- kvm.orig/arch/x86/kvm/lapic.c
> > +++ kvm/arch/x86/kvm/lapic.c
> > @@ -33,6 +33,7 @@
> >  #include <asm/page.h>
> >  #include <asm/current.h>
> >  #include <asm/apicdef.h>
> > +#include <asm/delay.h>
> >  #include <linux/atomic.h>
> >  #include <linux/jump_label.h>
> >  #include "kvm_cache_regs.h"
> > @@ -1073,6 +1074,7 @@ static void apic_timer_expired(struct kv
> >  {
> >  	struct kvm_vcpu *vcpu = apic->vcpu;
> >  	wait_queue_head_t *q = &vcpu->wq;
> > +	struct kvm_timer *ktimer = &apic->lapic_timer;
> >  
> >  	/*
> >  	 * Note: KVM_REQ_PENDING_TIMER is implicitly checked in
> > @@ -1087,11 +1089,64 @@ static void apic_timer_expired(struct kv
> >  
> >  	if (waitqueue_active(q))
> >  		wake_up_interruptible(q);
> > +
> > +	if (apic_lvtt_tscdeadline(apic))
> > +		ktimer->expired_tscdeadline = ktimer->tscdeadline;
> > +}
> > +
> > +/*
> > + * On APICv, this test will cause a busy wait
> > + * during a higher-priority task.
> > + */
> > +
> > +static bool lapic_timer_int_injected(struct kvm_vcpu *vcpu)
> > +{
> > +	struct kvm_lapic *apic = vcpu->arch.apic;
> > +	u32 reg = kvm_apic_get_reg(apic, APIC_LVTT);
> > +
> > +	if (kvm_apic_hw_enabled(apic)) {
> > +		int vec = reg & APIC_VECTOR_MASK;
> > +
> > +		if (kvm_x86_ops->test_posted_interrupt)
> > +			return kvm_x86_ops->test_posted_interrupt(vcpu, vec);
> > +		else {
> > +			if (apic_test_vector(vec, apic->regs + APIC_ISR))
> > +				return true;
> > +		}
> > +	}
> > +	return false;
> > +}
> > +
> > +void wait_lapic_expire(struct kvm_vcpu *vcpu)
> > +{
> > +	struct kvm_lapic *apic = vcpu->arch.apic;
> > +	u64 guest_tsc, tsc_deadline;
> > +
> > +	if (!kvm_vcpu_has_lapic(vcpu))
> > +		return;
> > +
> > +	if (apic->lapic_timer.expired_tscdeadline == 0)
> > +		return;
> > +
> > +	if (!lapic_timer_int_injected(vcpu))
> > +		return;
> 
> By the time we get here, I think, if expired_tscdeadline != 0 we're sure
> that the interrupt has been injected.  It may be in IRR rather than ISR,
> but at least on APICv the last test should be redundant.
> 
> So perhaps you can get rid of patch 1 and check
> kvm_apic_vid_enabled(vcpu->kvm):
> 
> 	if (k_a_v_e(vcpu->kvm)
> 		return true;
> 	if (apic_test_vector(vec, apic->regs + APIC_ISR))
> 		return true;
> 
> Does this sound correct?

* expired_tscdeadline != 0.
* APIC timer interrupt delivery masked at LVTT register.

Implies expired_tscdeadline != 0 and interrupt not injected.