[PATCH] KVM: SVM: Adjust tsc_offset only if tsc_unstable

[PATCH] KVM: SVM: Adjust tsc_offset only if tsc_unstable

[Joerg Roedel]

The tsc_offset adjustment in svm_vcpu_load is executed
unconditionally even if Linux considers the host tsc as
stable. This causes a Linux guest detecting an unstable tsc
in any case.
This patch removes the tsc_offset adjustment if the host tsc
is stable. The guest will now get the benefit of a stable
tsc too.

Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
---
 arch/x86/kvm/svm.c |   18 ++++++++++--------
 1 files changed, 10 insertions(+), 8 deletions(-)

diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
index 3de0b37..cb04378 100644
--- a/arch/x86/kvm/svm.c
+++ b/arch/x86/kvm/svm.c
@@ -767,14 +767,16 @@ static void svm_vcpu_load(struct kvm_vcpu *vcpu, int cpu)
 	if (unlikely(cpu != vcpu->cpu)) {
 		u64 delta;
 
-		/*
-		 * Make sure that the guest sees a monotonically
-		 * increasing TSC.
-		 */
-		delta = vcpu->arch.host_tsc - native_read_tsc();
-		svm->vmcb->control.tsc_offset += delta;
-		if (is_nested(svm))
-			svm->nested.hsave->control.tsc_offset += delta;
+		if (check_tsc_unstable()) {
+			/*
+			 * Make sure that the guest sees a monotonically
+			 * increasing TSC.
+			 */
+			delta = vcpu->arch.host_tsc - native_read_tsc();
+			svm->vmcb->control.tsc_offset += delta;
+			if (is_nested(svm))
+				svm->nested.hsave->control.tsc_offset += delta;
+		}
 		vcpu->cpu = cpu;
 		kvm_migrate_timers(vcpu);
 		svm->asid_generation = 0;
-- 
1.6.5.4

Re: [PATCH] KVM: SVM: Adjust tsc_offset only if tsc_unstable

[Zachary Amsden]

On 12/14/2009 01:22 AM, Joerg Roedel wrote:
> The tsc_offset adjustment in svm_vcpu_load is executed
> unconditionally even if Linux considers the host tsc as
> stable. This causes a Linux guest detecting an unstable tsc
> in any case.
> This patch removes the tsc_offset adjustment if the host tsc
> is stable. The guest will now get the benefit of a stable
> tsc too.
>
>    
Hi Joerg, I have a much more comprehensive series of patches to address 
this issue, please take a look.

Thanks,

Zach

Re: [PATCH] KVM: SVM: Adjust tsc_offset only if tsc_unstable

[Avi Kivity]

On 12/15/2009 05:38 AM, Zachary Amsden wrote:
> On 12/14/2009 01:22 AM, Joerg Roedel wrote:
>> The tsc_offset adjustment in svm_vcpu_load is executed
>> unconditionally even if Linux considers the host tsc as
>> stable. This causes a Linux guest detecting an unstable tsc
>> in any case.
>> This patch removes the tsc_offset adjustment if the host tsc
>> is stable. The guest will now get the benefit of a stable
>> tsc too.
>>
> Hi Joerg, I have a much more comprehensive series of patches to 
> address this issue, please take a look.
>

Despite this, I've applied the patch to have this fix in while we review 
your patch set (haven't started yet, sorry - still recovering from 
nested vmx).

-- 
error compiling committee.c: too many arguments to function

Re: [PATCH] KVM: SVM: Adjust tsc_offset only if tsc_unstable

[Zachary Amsden]

On 12/19/2009 11:59 PM, Avi Kivity wrote:
> On 12/15/2009 05:38 AM, Zachary Amsden wrote:
>> On 12/14/2009 01:22 AM, Joerg Roedel wrote:
>>> The tsc_offset adjustment in svm_vcpu_load is executed
>>> unconditionally even if Linux considers the host tsc as
>>> stable. This causes a Linux guest detecting an unstable tsc
>>> in any case.
>>> This patch removes the tsc_offset adjustment if the host tsc
>>> is stable. The guest will now get the benefit of a stable
>>> tsc too.
>>>
>> Hi Joerg, I have a much more comprehensive series of patches to 
>> address this issue, please take a look.
>>
>
> Despite this, I've applied the patch to have this fix in while we 
> review your patch set (haven't started yet, sorry - still recovering 
> from nested vmx).

No worries, I can always re-base.  My patch set needs some other fixed 
bits for sleep in C-states on Intel CPUs.

Choices are:

1) do nothing; don't support these CPUs
2) do nothing; don't support MWAIT based sleep into C3 on these CPUs, 
force different idle routine
3) resync the TSC on exit from C3
4) don't use TSC here at all

If the wakeup time for a CPU needs to be fast, I prefer approach #2;

However, if the load is so low that we can drop to C3, then we can 
consider the state of deep idle to be equivalent to bring a CPU down and 
then back up, which is a problem we already know how to solve; this is 
an argument for #3.

It is also pragmatic to suggest #4, now that there exists a patchset for 
doing intercept based TSC virtualization.

Zach