Re: [PATCH v4: kvm 2/4] Kill the confusing tsc_ref_khz and ref_freq variables.

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 10/09/2009 10:36 AM, Jan Kiszka wrote:
Zachary Amsden wrote:
On 10/08/2009 01:18 PM, Jan Kiszka wrote:
Zachary Amsden wrote:

They are globals, not clearly protected by any ordering or locking, and
vulnerable to various startup races.

Instead, for variable TSC machines, register the cpufreq notifier and
get
the TSC frequency directly from the cpufreq machinery.  Not only is it
always right, it is also perfectly accurate, as no error prone
measurement
is required.

On such machines, when a new CPU online is brought online, it isn't
clear what
frequency it will start with, and it may not correspond to the
reference, thus
in hardware_enable we clear the cpu_tsc_khz variable to zero and make
sure
it is set before running on a VCPU.

Signed-off-by: Zachary Amsden<zamsden@xxxxxxxxxx>
---
   arch/x86/kvm/x86.c |   26 ++++++++++++++++----------
   1 files changed, 16 insertions(+), 10 deletions(-)

diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 15d2ace..de4ce8f 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -1326,6 +1326,8 @@ out:
   void kvm_arch_vcpu_load(struct kvm_vcpu *vcpu, int cpu)
   {
       kvm_x86_ops->vcpu_load(vcpu, cpu);
+    if (unlikely(per_cpu(cpu_tsc_khz, cpu) == 0))
+        per_cpu(cpu_tsc_khz, cpu) = cpufreq_quick_get(cpu);
       kvm_request_guest_time_update(vcpu);
   }

@@ -3061,9 +3063,6 @@ static void bounce_off(void *info)
       /* nothing */
   }

-static unsigned int  ref_freq;
-static unsigned long tsc_khz_ref;
-
   static int kvmclock_cpufreq_notifier(struct notifier_block *nb,
unsigned long val,
                        void *data)
   {
@@ -3072,14 +3071,11 @@ static int kvmclock_cpufreq_notifier(struct
notifier_block *nb, unsigned long va
       struct kvm_vcpu *vcpu;
       int i, send_ipi = 0;

-    if (!ref_freq)
-        ref_freq = freq->old;
-
       if (val == CPUFREQ_PRECHANGE&&   freq->old>   freq->new)
           return 0;
       if (val == CPUFREQ_POSTCHANGE&&   freq->old<   freq->new)
           return 0;
-    per_cpu(cpu_tsc_khz, freq->cpu) = cpufreq_scale(tsc_khz_ref,
ref_freq, freq->new);
+    per_cpu(cpu_tsc_khz, freq->cpu) = freq->new;

       spin_lock(&kvm_lock);
       list_for_each_entry(kvm,&vm_list, vm_list) {
@@ -3120,12 +3116,14 @@ static void kvm_timer_init(void)
   {
       int cpu;

-    for_each_possible_cpu(cpu)
-        per_cpu(cpu_tsc_khz, cpu) = tsc_khz;
       if (!boot_cpu_has(X86_FEATURE_CONSTANT_TSC)) {
-        tsc_khz_ref = tsc_khz;
           cpufreq_register_notifier(&kvmclock_cpufreq_notifier_block,
                         CPUFREQ_TRANSITION_NOTIFIER);
+        for_each_online_cpu(cpu)
+            per_cpu(cpu_tsc_khz, cpu) = cpufreq_get(cpu);

This doesn't build for !CONFIG_CPU_FREQ.

And did it before?
Yes, because cpufreq_get, which is undefined without CONFIG_CPU_FREQ,
did not exist so far. One may argue that this is a deficit of the
cpufreq API. However, it needs fixing.

I'll send a patch today. I'm going with API deficient and will fix that, but this means I will also require a fix for kvm-kmod :(

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

[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux