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

Radim Krcmar
2014-12-12T18:35:52Z

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.)