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

[Radim Krcmar <rkrcmar@redhat.com>]

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