All of lore.kernel.org
 help / color / mirror / Atom feed
From: Zachary Amsden <zamsden@redhat.com>
To: kvm@vger.kernel.org
Cc: linux-kernel@vger.kernel.org, Zachary Amsden <zamsden@redhat.com>,
	Avi Kivity <avi@redhat.com>,
	Marcelo Tosatti <mtosatti@redhat.com>
Subject: [PATCH v2: kvm 2/4] Kill the confusing tsc_ref_khz and ref_freq variables.
Date: Mon, 28 Sep 2009 18:04:30 -1000	[thread overview]
Message-ID: <1254197072-889-3-git-send-email-zamsden@redhat.com> (raw)
In-Reply-To: <1254197072-889-2-git-send-email-zamsden@redhat.com>

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@redhat.com>
---
 arch/x86/kvm/x86.c |   30 ++++++++++++++++++++----------
 1 files changed, 20 insertions(+), 10 deletions(-)

diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 15d2ace..9cbd53a 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,18 @@ 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);
+	} else {
+		for_each_possible_cpu(cpu)
+			per_cpu(cpu_tsc_khz, cpu) = tsc_khz;
+	}
+	for_each_possible_cpu(cpu) {
+		printk(KERN_DEBUG "kvm: cpu %d = %ld khz\n",
+			cpu, per_cpu(cpu_tsc_khz, cpu));
 	}
 }
 
@@ -4698,6 +4700,14 @@ int kvm_arch_vcpu_reset(struct kvm_vcpu *vcpu)
 
 int kvm_arch_hardware_enable(void *garbage)
 {
+	/*
+	 * Since this may be called from a hotplug notifcation,
+	 * we can't get the CPU frequency directly.
+	 */
+	if (!boot_cpu_has(X86_FEATURE_CONSTANT_TSC)) {
+		int cpu = raw_smp_processor_id();
+		per_cpu(cpu_tsc_khz, cpu) = 0;
+	}
 	return kvm_x86_ops->hardware_enable(garbage);
 }
 
-- 
1.6.4.4


  reply	other threads:[~2009-09-29  4:06 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-09-29  4:04 Hotplug / TSC cleanup and fixing Zachary Amsden
2009-09-29  4:04 ` [PATCH v2: kvm 1/4] Code motion. Separate timer intialization into an indepedent function Zachary Amsden
2009-09-29  4:04   ` Zachary Amsden [this message]
2009-09-29  4:04     ` [PATCH v2: kvm 3/4] Fix printk name error in svm.c Zachary Amsden
2009-09-29  4:04       ` [PATCH v2: kvm 4/4] Fix hotplug of CPUs for KVM Zachary Amsden
2009-09-29  8:30         ` Avi Kivity
2009-09-29 15:51           ` Zachary Amsden
2009-09-29 21:38           ` [PATCH v4: kvm 1/4] Code motion. Separate timer intialization into an indepedent function Zachary Amsden
2009-09-29 21:38             ` [PATCH v4: kvm 2/4] Kill the confusing tsc_ref_khz and ref_freq variables Zachary Amsden
2009-09-29 21:38               ` [PATCH v4: kvm 3/4] Fix printk name error in svm.c Zachary Amsden
2009-09-29 21:38                 ` [PATCH v4: kvm 4/4] Fix hotplug of CPUs for KVM Zachary Amsden
2009-09-30 13:35                   ` Marcelo Tosatti
2009-10-08 23:18               ` [PATCH v4: kvm 2/4] Kill the confusing tsc_ref_khz and ref_freq variables Jan Kiszka
2009-10-09 20:27                 ` Zachary Amsden
2009-10-09 20:36                   ` Jan Kiszka
2009-10-09 20:36                     ` Zachary Amsden
2009-10-09 20:47                       ` Jan Kiszka
2009-10-09 20:47                         ` Zachary Amsden
2009-10-09 21:05                           ` Jan Kiszka
2009-10-10  2:26                         ` [PATCH] cpufreq: Make cpufreq_get always defined Zachary Amsden
2009-10-10  2:26                           ` [PATCH] KVM: Harden against cpufreq Zachary Amsden
2009-10-10  2:26                             ` [PATCH] kvm-kmod cpufreq_get fix Zachary Amsden
2009-10-12 19:41                             ` [PATCH] KVM: Harden against cpufreq Marcelo Tosatti
2009-09-30  8:45             ` [PATCH v4: kvm 1/4] Code motion. Separate timer intialization into an indepedent function Avi Kivity
2009-09-30 15:51               ` Zachary Amsden
2009-09-30 15:56                 ` Avi Kivity
2009-09-30 16:06                   ` Zachary Amsden
2009-09-30 16:11                     ` Avi Kivity
2009-09-30 16:16                       ` Zachary Amsden
2009-09-29  8:29     ` [PATCH v2: kvm 2/4] Kill the confusing tsc_ref_khz and ref_freq variables Avi Kivity

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1254197072-889-3-git-send-email-zamsden@redhat.com \
    --to=zamsden@redhat.com \
    --cc=avi@redhat.com \
    --cc=kvm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mtosatti@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.