fix kvmclock bug
PATCH 1/1 fix kvmclock bug
For CPUs with unstable TSC, we null time offset between not just VCPU switches, but all preemptions of the kvm thread. This makes a bug much more likely where the kvmclock values are updated before a successful exit from virt, causing an underflow.
The null offsetting was added at : bf0fb4a42ba7eb362f4013bd2e93209666793e66 The underflow happens with this additional patch : cf839f5da2b0779b9ec8b990f851fb4e7d681da0
There is a secondary bug, which is that TSC fails to advance with real time on unstable TSC, but the fix is much more involved (it requires the TSC catchup code).
For now, this patch is sufficient to get things working again for me.
commit 1abe7e8806fd71ea802c6622ed3ce7821a18f271
Author: Zachary Amsden <[email protected]> Date: Sat Sep 18 13:58:37 2010 -1000
Fix kvmclock bug
If preempted after kvmclock values are updated, but before hardware virtualization is entered, the last tsc time as read by the guest is never set. It underflows the next time kvmclock is updated if there has not yet been a successful entry / exit into hardware virt.
Fix this by simply setting last_tsc to the newly read tsc value so that any computed nsec advance of kvmclock is nulled.
arch/x86/kvm/x86.c +1 -0
@@ -1101,6 +1101,7 @@ static int kvm_guest_time_update(struct kvm_vcpu *v) vcpu->hv_clock.tsc_timestamp = tsc_timestamp; vcpu->hv_clock.system_time = kernel_ns + v->kvm->arch.kvmclock_offset; vcpu->last_kernel_ns = kernel_ns; + vcpu->last_guest_tsc = tsc_timestamp; vcpu->hv_clock.flags =0 ;/*
Quoted 9 lines · Zachary Amsden
For CPUs with unstable TSC, we null time offset between not just VCPU switches, but all preemptions of the kvm thread. This makes a bug much more likely where the kvmclock values are updated before a successful exit from virt, causing an underflow.
The null offsetting was added at :
bf0fb4a42ba7eb362f4013bd2e93209666793e66The underflow happens with this additional patch :cf839f5da2b0779b9ec8b990f851fb4e7d681da0
Quoted 5 lines
There is a secondary bug, which is that TSC fails to advance with real time on unstable TSC, but the fix is much more involved (it requires the TSC catchup code).
For now, this patch is sufficient to get things working again for me.
...but not for me. I still face stuck (or infinitely slow) guests that want to use kvmclock once tsc_unstable gets set. Or is this patch addressing a different issue?
Jan
Quoted 14 lines · Jan Kiszka
Quoted 13 lines · Zachary Amsden
For CPUs with unstable TSC, we null time offset between not just VCPU switches, but all preemptions of the kvm thread. This makes a bug much more likely where the kvmclock values are updated before a successful exit from virt, causing an underflow.
The null offsetting was added at :
bf0fb4a42ba7eb362f4013bd2e93209666793e66The underflow happens with this additional patch :cf839f5da2b0779b9ec8b990f851fb4e7d681da0There is a secondary bug, which is that TSC fails to advance with real time on unstable TSC, but the fix is much more involved (it requires the TSC catchup code).
Quoted 5 lines
Quoted 1 line
For now, this patch is sufficient to get things working again for me.
...but not for me. I still face stuck (or infinitely slow) guests that want to use kvmclock once
tsc_unstablegets set. Or is this patch addressing a different issue?
Commit bfb3f332 ("TSC catchup mode") in kvm.git finally resolves the issue here.
That only leaves us with the likely wrong unstable declaration of the TSC after resume. And that raises the question for me if KVM is actually that much smarter than the Linux kernel in detecting TSC jumps. If something is missing, can't we improve the kernel's detection mechanism which already has suspend/resume support?
Jan
Quoted 17 lines · Jan Kiszka
Quoted 15 lines · Jan Kiszka
Quoted 13 lines · Zachary Amsden
For CPUs with unstable TSC, we null time offset between not just VCPU switches, but all preemptions of the kvm thread. This makes a bug much more likely where the kvmclock values are updated before a successful exit from virt, causing an underflow.
The null offsetting was added at :
bf0fb4a42ba7eb362f4013bd2e93209666793e66The underflow happens with this additional patch :cf839f5da2b0779b9ec8b990f851fb4e7d681da0There is a secondary bug, which is that TSC fails to advance with real time on unstable TSC, but the fix is much more involved (it requires the TSC catchup code).
Quoted 9 lines
Quoted 6 lines
Quoted 2 lines
For now, this patch is sufficient to get things working again for me.
...but not for me. I still face stuck (or infinitely slow) guests that want to use kvmclock once
tsc_unstablegets set. Or is this patch addressing a different issue?Commit
bfb3f332("TSC catchup mode") in kvm.git finally resolves the issue here.
Good to hear that!
Quoted 6 lines
That only leaves us with the likely wrong unstable declaration of the TSC after resume. And that raises the question for me if KVM is actually that much smarter than the Linux kernel in detecting TSC jumps. If something is missing, can't we improve the kernel's detection mechanism which already has suspend/resume support?
Linux must make the the conservative choice about TSC being declared unstable; if it is possible that it has become unstable, it is unstable. Unfortunately, this bodes not well for us, as most of the finer points of accuracy depend on having a stable TSC.
There's a bunch of places that declare TSC unstable, and where in the suspend / resume cycle that happens would depend on your actual hardware.
Zach
Quoted 8 lines · Zachary Amsden
Quoted 6 lines · Jan Kiszka
That only leaves us with the likely wrong unstable declaration of the TSC after resume. And that raises the question for me if KVM is actually that much smarter than the Linux kernel in detecting TSC jumps. If something is missing, can't we improve the kernel's detection mechanism which already has suspend/resume support?
Quoted 7 lines
Linux must make the the conservative choice about TSC being declared unstable; if it is possible that it has become unstable, it is unstable. Unfortunately, this bodes not well for us, as most of the finer points of accuracy depend on having a stable TSC.
There's a bunch of places that declare TSC unstable, and where in the suspend / resume cycle that happens would depend on your actual hardware.
It's absolutely clear where this happens: kvm_arch_vcpu_load. And it seems to happen as the TSC is reset due to suspend-to-RAM.
Again: Linux recovers from this and continues to use the TSC. KVM is more picky, so my question is if this is really required.
Jan
Signature · 2 lines
Siemens AG, Corporate Technology, CT T DE IT 1 Corporate Competence Center Embedded Linux
Quoted 5 lines · Zachary Amsden
commit
1abe7e8806fd71ea802c6622ed3ce7821a18f271Author: Zachary Amsden<[email protected]> Date: Sat Sep 18 13:58:37 2010 -1000Fix kvmclock bug
I think there's some redundancy here. Anyone who has tracked your kernel work knows you've done a lot of work on kvmclock, so "Fix bug" would be just as descriptive. Of course, if we fix something, it's because it was a bug, so "Fix" is all that's really necessary ("bug" would be reserved for commits that introduce bugs). In the case of kvmclock, it's hard to see how new features could be added, and it has a pretty bad history of bugs, so most readers would probably deduce that a commit fixes a bug. I don't really see why you wrote a subject line at all.
Signature · 2 lines
I have a truly marvellous patch that fixes the bug which this signature is too narrow to contain.