Re: [PATCH 1/6] KVM: SVM: Implement infrastructure for TSC_RATE_MSR

[Joerg Roedel <joro@8bytes.org>]

On Thu, Mar 24, 2011 at 11:51:59AM +0200, Avi Kivity wrote:
> On 03/24/2011 09:40 AM, Joerg Roedel wrote:
>> This patch enhances the kvm_amd module with functions to
>> support the TSC_RATE_MSR which can be used to set a given
>> tsc frequency for the guest vcpu.
>>
>>
>> @@ -1141,6 +1175,9 @@ static void svm_vcpu_load(struct kvm_vcpu *vcpu, int cpu)
>>
>>   	for (i = 0; i<  NR_HOST_SAVE_USER_MSRS; i++)
>>   		rdmsrl(host_save_user_msrs[i], svm->host_user_msrs[i]);
>> +
>> +	if (static_cpu_has(X86_FEATURE_TSCRATEMSR)&&  svm->tsc_scale.enabled)
>> +		wrmsrl(MSR_AMD64_TSC_RATIO, svm->tsc_scale.ratio);
>>   }
>>
>>   static void svm_vcpu_put(struct kvm_vcpu *vcpu)
>> @@ -1161,6 +1198,9 @@ static void svm_vcpu_put(struct kvm_vcpu *vcpu)
>>   #endif
>>   	for (i = 0; i<  NR_HOST_SAVE_USER_MSRS; i++)
>>   		wrmsrl(host_save_user_msrs[i], svm->host_user_msrs[i]);
>> +
>> +	if (static_cpu_has(X86_FEATURE_TSCRATEMSR)&&  svm->tsc_scale.enabled)
>> +		wrmsr(MSR_AMD64_TSC_RATIO, 0, 1);
>>   }
>
> This is a bit suboptimal.  We could do something like
>
> static DEFINE_PER_CPU(u64, current_tsc_scale);
>
>   static void svm_vcpu_load(...)
>   {
>       ...
>       if (svm->tsc_scale != __get_cpu_var(current_tsc_scale)) {
>           __get_cpu_var(current_tsc_scale) = svm->tsc_scale;
>           wrmsrl(MSR_AMD64_TSC_RATIO, svm->tsc_scale);
>       }
>   }
>
> with no changes to svm_vcpu_put() (note no tsc_scale.enabled either,  
> just put the unity value for disabled).

Right, this is a good optimization. Thanks for the hint, I'll change it.

Regards,

	Joerg