svm.c: tsc unsigned delta calculation
RFC PATCH 1/1 svm.c: tsc unsigned delta calculation
I've been playing around with smp guests on a couple amd systems and I've also seen some of the smp/locking issues which lead me to dig into some of the tsc code. The svm_cpu_load, the tsc_offset calc will generate a massively large tsc_offset if we're switching cpus and tsc_this is ahead of the host_tsc value (delta would normally be negative, but since it's unsigned, we get a huge positive number).
svm_vcpu_load()
...
rdtscll(tsc_this);
delta = vcpu->arch.host_tsc - tsc_this;
svm->vmcb->control.tsc_offset += delta;This is handled a little differently on Intel (in vmx.c) where there is a check:
if (tsc_this < vcpu->arch.host_tsc)
/* do delta and new offset calc */This check makes sense to me in that we only need to ensure that we don't go backwards which means that unless the new cpu is behind the current vcpu's host_tsc, we can skip a new delta calc. If the check doesn't make sense then we'll need to do the math with s64s.
Attached patch fixed the case where an idle guest was live-locked.
Signature · 13 lines
Ryan Harper Software Engineer; Linux Technology Center IBM Corp., Austin, Tx (512) 838-9253 T/L: 678-9253 [email protected] diffstat output: svm.c | 10 ++++++++-- 1 files changed, 8 insertions(+), 2 deletions(-) Signed-off-by: Ryan Harper <[email protected]> ---
arch/x86/kvm/svm.c +8 -2
@@ -685,8 +685,14 @@ static void svm_vcpu_load(struct kvm_vcpu *vcpu, int cpu)* increasing TSC. */ rdtscll (tsc_this); - delta = vcpu->arch.host_tsc - tsc_this; - svm->vmcb->control.tsc_offset += delta; +/* we only need to adjust this if the old tsc was ahead +* also, we'll generate a massively large u64 value if +* tsc_this is less than host_tsc because of unsigned math +*/ +if (tsc_this < vcpu->arch.host_tsc) { + delta = vcpu->arch.host_tsc - tsc_this; + svm->vmcb->control.tsc_offset += delta; + } vcpu->cpu = cpu;kvm_migrate_apic_timer (vcpu); }
-------------------------------------------------------------------------
This SF.net email is sponsored by the 2008 JavaOne(SM) Conference Don't miss this year's exciting event. There's still time to save $100. Use priority code J8TL2D2.
http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone
Quoted 7 lines · Ryan Harper
I've been playing around with smp guests on a couple amd systems and I've also seen some of the smp/locking issues which lead me to dig into some of the tsc code. The
svm_cpu_load, thetsc_offsetcalc will generate a massively largetsc_offsetif we're switching cpus andtsc_thisis ahead of thehost_tscvalue (delta would normally be negative, but since it's unsigned, we get a huge positive number).
Quoted 6 lines
svm_vcpu_load()
...rdtscll(tsc_this); delta = vcpu->arch.host_tsc - tsc_this; svm->vmcb->control.tsc_offset += delta;
This math will work out fine since the very large number will generate an overflow and the result will be identical to if we were using s64s. We're using u64s because that's how the tsc_offset is defined by hardware.
Quoted 6 lines
This is handled a little differently on Intel (in
vmx.c) where there is a check:if (tsc_this < vcpu->arch.host_tsc) /* do delta and new offset calc */
So what your patch really does is change the behavior of the tsc_offset to increase the guest's TSC by a potentially large amount depending on how out of sync the TSC is on CPU migration.
The question is why this would make things work out better for you..
Do you have Gerd's kvm-clock most recent patch applied?
Regards,
Anthony Liguori
Quoted 8 lines
This check makes sense to me in that we only need to ensure that we don't go backwards which means that unless the new cpu is behind the current vcpu's
host_tsc, we can skip a new delta calc. If the check doesn't make sense then we'll need to do the math with s64s.Attached patch fixed the case where an idle guest was live-locked.
-------------------------------------------------------------------------
This SF.net email is sponsored by the 2008 JavaOne(SM) Conference Don't miss this year's exciting event. There's still time to save $100. Use priority code J8TL2D2.
http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone
Quoted 11 lines · Ryan Harper
arch/x86/kvm/svm.c +1
-2@@ -685,8 +685,14 @@ static void svm_vcpu_load(struct kvm_vcpu *vcpu, int cpu)* increasing TSC. */ rdtscll (tsc_this); - delta = vcpu->arch.host_tsc - tsc_this; - svm->vmcb->control.tsc_offset += delta; +/* we only need to adjust this if the old tsc was ahead
arch/x86/kvm/svm.c static void svm_vcpu_load(struct kvm_vcpu *vcpu, int cpu) · line 698
Quoted 10 lines
arch/x86/kvm/svm.c +7
-0+* also, we'll generate a massively large u64 value if +* tsc_this is less than host_tsc because of unsigned math +*/ +if (tsc_this < vcpu->arch.host_tsc) { + delta = vcpu->arch.host_tsc - tsc_this; + svm->vmcb->control.tsc_offset += delta; + } vcpu->cpu = cpu;kvm_migrate_apic_timer (vcpu); }
Hmm, I think this can result in inaccurate guest time because it makes the tsc hopping. Does it fix the problem when you make delta an s64?
Joerg
Signature · 12 lines
| AMD Saxony Limited Liability Company & Co. KG
Operating | Wilschdorfer Landstr. 101, 01109 Dresden, Germany
System | Register Court Dresden: HRA 4896
Research | General Partner authorized to represent:
Center | AMD Saxony LLC (Wilmington, Delaware, US)
| General Manager of AMD Saxony LLC: Dr. Hans-R. Deppe, Thomas McCoy
-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/