Re: [BUG, PATCH-2.6.32] Fix a possible backwards warp of kvmclock

[Marcelo Tosatti <mtosatti@redhat.com>]

Philipp,

Thanks for debugging this issue. We'll be fowarding the fix
for inclusion in the official 2.6.32 tree.

On Mon, Sep 05, 2011 at 04:06:57PM +0200, Philipp Hahn wrote:
> Hello,
> 
> (cc:-ing lost of people who reported similar bugs on kvm-devel)
> > Changing clock in KVM host may cause VM to hang
> > 2.6.32 guest with paravirt clock enabled hangs on 2.6.37.6 host (w 
> qemu-kvm-0.13.0)
> 
> I found a bug regarding PV-clock in the KVM-kernel module. The attached patch 
> solves the problem of the guest being very slow after a reboot. Can you 
> please have a look and give it a try to see if it solves your problem as 
> well.
> 
> Since the fix is only relevant for the stable 2.6.32 tree, where the code is 
> quiet different, please have a look and forward to stable@ as appropriate.
> 
> Sincerely
> Philipp
> 
> PS: This bug is tracked in our German Bugzilla at 
> <https://forge.univention.org/bugzilla/show_bug.cgi?id=23258>
> -- 
> Philipp Hahn           Open Source Software Engineer      hahn@univention.de
> Univention GmbH        Linux for Your Business        fon: +49 421 22 232- 0
> Mary-Somerville-Str.1  D-28359 Bremen                 fax: +49 421 22 232-99
>                                                    http://www.univention.de/
> ----------------------------------------------------------------------------
> Treffen Sie Univention auf der IT&Business vom 20. bis 22. September 2011
> auf dem Gemeinschaftsstand der Open Source Business Alliance in Stuttgart in
> Halle 3 Stand 3D27-7.

> Bug #23257: Reset tsc_timestamp on TSC writes
> 
> vcpu->last_guest_tsc is updated in vcpu_enter_guest() and kvm_arch_vcpu_put()
> by getting the last value of the TSC from the guest.
> On reset, the SeaBIOS resets the TSC to 0, which triggers a bug on the next
> call to kvm_write_guest_time(): Since vcpu->hw_clock.tsc_timestamp still
> contains the old value before the reset, "max_kernel_ns = vcpu->last_guest_tsc
> - vcpu->hw_clock.tsc_timestamp" gets negative. Since the variable is u64, it
>  gets translated to a large positive value.
> 
> [9333.197080]
> vcpu->last_guest_tsc        =209_328_760_015           ←
> vcpu->hv_clock.tsc_timestamp=209_328_708_109
> vcpu->last_kernel_ns        =9_333_179_830_643
> kernel_ns                   =9_333_197_073_429
> max_kernel_ns               =9_333_179_847_943         ←
> 
> [9336.910995]
> vcpu->last_guest_tsc        =9_438_510_584             ←
> vcpu->hv_clock.tsc_timestamp=211_080_593_143
> vcpu->last_kernel_ns        =9_333_763_732_907
> kernel_ns                   =9_336_910_990_771
> max_kernel_ns               =6_148_296_831_006_663_830 ←
> 
> For completeness, here are the values for my 3 GHz CPU:
> vcpu->hv_clock.tsc_shift         =-1
> vcpu->hv_clock.tsc_to_system_mul =2_863_019_502
> 
> This makes the guest kernel crawl very slowly when clocksource=kvmclock is
> used: sleeps take way longer than expected and don't match wall clock any more.
> The times printed with printk() don't match real time and the reboot often
> stalls for long times.
> 
> In linux-git this isn't a problem, since on every MSR_IA32_TSC write
> vcpu->arch.hv_clock.tsc_timestamp is reset to 0, which disables above logic.
> The code there is only in arch/x86/kvm/x86.c, since much of the kvm-clock
> related code has been refactured for 2.6.37:
> 	99e3e30a arch/x86/kvm/x86.c (Zachary Amsden            2010-08-19 22:07:17 -1000 1084)  vcpu->arch.hv_clock.tsc_timestamp = 0;                                                      
> Since 1d5f066e0b63271b67eac6d3752f8aa96adcbddb from 2.6.37 was back-ported to
> 2.6.32.40 as ad2088cabe0fd7f633f38ba106025d33ed9a2105, the following patch is
> needed to add the needed reset logic to 2.6.32 as well.
> 
> Signed-off-by: Philipp Hahn <hahn@univention.de>
> --- a/arch/x86/kvm/vmx.c	2011-09-05 14:17:54.000000000 +0200
> +++ b/arch/x86/kvm/vmx.c	2011-09-05 14:18:03.000000000 +0200
> @@ -1067,6 +1067,7 @@ static int vmx_set_msr(struct kvm_vcpu *
>  	case MSR_IA32_TSC:
>  		rdtscll(host_tsc);
>  		guest_write_tsc(data, host_tsc);
> +		vcpu->arch.hv_clock.tsc_timestamp = 0;
>  		break;
>  	case MSR_IA32_CR_PAT:
>  		if (vmcs_config.vmentry_ctrl & VM_ENTRY_LOAD_IA32_PAT) {
> --- a/arch/x86/kvm/svm.c	2011-09-05 14:17:57.000000000 +0200
> +++ b/arch/x86/kvm/svm.c	2011-09-05 14:18:00.000000000 +0200
> @@ -2256,6 +2256,7 @@ static int svm_set_msr(struct kvm_vcpu *
>  		}
>  
>  		svm->vmcb->control.tsc_offset = tsc_offset + g_tsc_offset;
> +		vcpu->arch.hv_clock.tsc_timestamp = 0;
>  
>  		break;
>  	}