Re: [patch 2/2] KVM: x86: add option to advance tscdeadline hrtimer expiration

[Radim Krcmar <rkrcmar@redhat.com>]

2014-12-10 15:57-0500, Marcelo Tosatti:
> 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 cyclictest avg latency by 50%.
> 
> 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
> @@ -1087,11 +1089,58 @@ static void apic_timer_expired(struct kv
>  
>  	if (waitqueue_active(q))
>  		wake_up_interruptible(q);
> +
> +	if (ktimer->timer_mode_mask == APIC_LVT_TIMER_TSCDEADLINE)

timer_mode != timer_mode_mask.  (Please use apic_lvtt_tscdeadline().)

(So the code never waited for tsc_deadline >= guest_tsc ...
 I suppose it was possible to achieve lower latencies thanks to that.)

> +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_lvtt_tscdeadline(apic))
> +		return;

(It is better to check expired_tscdeadline here and zero it later.)

> +
> +	if (!lapic_timer_int_injected(vcpu))
> +		return;
> +
> +	tsc_deadline = apic->lapic_timer.expired_tscdeadline;
> +	guest_tsc = kvm_x86_ops->read_l1_tsc(vcpu, native_read_tsc());
> +
> +	while (guest_tsc < tsc_deadline) {
> +		int delay = min(tsc_deadline - guest_tsc, 1000ULL);
> +
> +		ndelay(delay);

The delay is in nanoseconds, but you feed it a difference in TSC.
(And usually overestimate the time;  cpu_relax() loop seems easiest.)