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

2011-09-12 Thread Marcelo Tosatti

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  h...@univention.de
 Univention GmbHLinux for Your Businessfon: +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 ITBusiness 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 Amsden2010-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 h...@univention.de
 --- a/arch/x86/kvm/vmx.c  2011-09-05 14:17:54.0 +0200
 +++ b/arch/x86/kvm/vmx.c  2011-09-05 14:18:03.0 +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.0 +0200
 +++ b/arch/x86/kvm/svm.c  2011-09-05 14:18:00.0 +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;
   }



--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2011-09-05 Thread Philipp Hahn
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  h...@univention.de
Univention GmbHLinux for Your Businessfon: +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 ITBusiness 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 Amsden2010-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 h...@univention.de
--- a/arch/x86/kvm/vmx.c	2011-09-05 14:17:54.0 +0200
+++ b/arch/x86/kvm/vmx.c	2011-09-05 14:18:03.0 +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.0 +0200
+++ b/arch/x86/kvm/svm.c	2011-09-05 14:18:00.0 +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;
 	}


signature.asc
Description: This is a digitally signed message part.