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

Radim Krcmar
2015-01-05T18:12:40Z

2014-12-23 15:58-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 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>

Reviewed-by: Radim Krčmář <rkrcmar@redhat.com>

(Other patches weren't touched, so my previous Reviewed-by holds.)

> +++ kvm/arch/x86/kvm/x86.c
> @@ -108,6 +108,10 @@ EXPORT_SYMBOL_GPL(kvm_max_guest_tsc_khz)
> static u32 tsc_tolerance_ppm = 250;
> module_param(tsc_tolerance_ppm, uint, S_IRUGO | S_IWUSR);
>
> +/* lapic timer advance (tscdeadline mode only) in nanoseconds */
> +unsigned int lapic_timer_advance_ns = 0;
> +module_param(lapic_timer_advance_ns, uint, S_IRUGO | S_IWUSR);
> +
> static bool backwards_tsc_observed = false;
>
> #define KVM_NR_SHARED_MSRS 16
> @@ -5625,6 +5629,10 @@ static void kvm_timer_init(void)
> __register_hotcpu_notifier(&kvmclock_cpu_notifier_block);
> cpu_notifier_register_done();
>
> + if (check_tsc_unstable() && lapic_timer_advance_ns) {
> + pr_info("kvm: unstable TSC, disabling lapic_timer_advance_ns\n");
> + lapic_timer_advance_ns = 0;

Does unstable TSC invalidate this feature?
(lapic_timer_advance_ns can be overridden, so we don't differentiate
workflows that calibrate after starting with 0.)

And cover letter is a bit misleading: The condition does nothing to
guarantee TSC based __delay() loop. (Right now, __delay() = delay_tsc()
whenever the hardware has TSC, regardless of stability, thus always.)