linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Igor Mammedov <imammedo@redhat.com>
To: linux-kernel@vger.kernel.org
Cc: mtosatti@redhat.com, tglx@linutronix.de, mingo@redhat.com,
	hpa@zytor.com, kvm@vger.kernel.org, x86@kernel.org,
	pbonzini@redhat.com, eabatalov89@gmail.com
Subject: [PATCH 2/2] x86: kvmclock: register per-cpu kvmclock at earliest possible time
Date: Fri, 21 Jun 2013 11:01:21 +0200	[thread overview]
Message-ID: <1371805281-21305-3-git-send-email-imammedo@redhat.com> (raw)
In-Reply-To: <bug-59521-28872@https.bugzilla.kernel.org/>

printk's in cpu_init() might access per-cpu pv_clock before it's
registered. Which might lead to an incorrect last_value value or
big jumps in it depending on current contents of kvm's hv_clock.

Also ftrace by default uses local clock for time-stamping its
records, which might cause access to not yet registered per-cpu
kvmclock during cpu_init() execution:
   function_trace_call -> trace_function ->
       trace_buffer_lock_reserve -> ring_buffer_lock_reserve ->
           rb_reserve_next_event -> rb_time_stamp ->
               trace_clock_local -> sched_clock ->
                   paravirt_sched_clock ->
                       kvm_clock_read

Fix provides a simplified version of kvm_setup_secondary_clock(),
that could be safely used before cpu_init() and turns off tracing
for its underlying calls to prevent premature access to kvmclock
during its registration. So that any following usage of
sched_clock() would yield correct value.

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
 arch/x86/kernel/kvmclock.c |   11 +++++++++--
 arch/x86/kernel/smpboot.c  |    2 +-
 arch/x86/mm/pageattr.c     |    4 ++--
 3 files changed, 12 insertions(+), 5 deletions(-)

diff --git a/arch/x86/kernel/kvmclock.c b/arch/x86/kernel/kvmclock.c
index 3dd37eb..4660b59 100644
--- a/arch/x86/kernel/kvmclock.c
+++ b/arch/x86/kernel/kvmclock.c
@@ -185,13 +185,20 @@ static void kvm_restore_sched_clock_state(void)
 }
 
 #ifdef CONFIG_X86_LOCAL_APIC
-static void __cpuinit kvm_setup_secondary_clock(void)
+static void __cpuinit notrace kvm_setup_secondary_clock(void)
 {
 	/*
 	 * Now that the first cpu already had this clocksource initialized,
 	 * we shouldn't fail.
 	 */
-	WARN_ON(kvm_register_clock("secondary cpu clock"));
+	int cpu = stack_smp_processor_id();
+	int low, high;
+	struct pvclock_vcpu_time_info *src;
+
+	src = &hv_clock[cpu].pvti;
+	low = (int)slow_virt_to_phys(src) | 1;
+	high = ((u64)slow_virt_to_phys(src) >> 32);
+	native_write_msr_safe(msr_kvm_system_time, low, high);
 }
 #endif
 
diff --git a/arch/x86/kernel/smpboot.c b/arch/x86/kernel/smpboot.c
index 9c73b51..5e2b7cb 100644
--- a/arch/x86/kernel/smpboot.c
+++ b/arch/x86/kernel/smpboot.c
@@ -244,8 +244,8 @@ notrace static void __cpuinit start_secondary(void *unused)
 	 * fragile that we want to limit the things done here to the
 	 * most necessary things.
 	 */
-	cpu_init();
 	x86_cpuinit.early_percpu_clock_init();
+	cpu_init();
 	preempt_disable();
 	smp_callin();
 
diff --git a/arch/x86/mm/pageattr.c b/arch/x86/mm/pageattr.c
index bb32480..06cabff 100644
--- a/arch/x86/mm/pageattr.c
+++ b/arch/x86/mm/pageattr.c
@@ -330,7 +330,7 @@ static inline pgprot_t static_protections(pgprot_t prot, unsigned long address,
  * or when the present bit is not set. Otherwise we would return a
  * pointer to a nonexisting mapping.
  */
-pte_t *lookup_address(unsigned long address, unsigned int *level)
+pte_t notrace *lookup_address(unsigned long address, unsigned int *level)
 {
 	pgd_t *pgd = pgd_offset_k(address);
 	pud_t *pud;
@@ -374,7 +374,7 @@ EXPORT_SYMBOL_GPL(lookup_address);
  * unoptimized should increase the testing coverage for
  * the more obscure platforms.
  */
-phys_addr_t slow_virt_to_phys(void *__virt_addr)
+phys_addr_t notrace slow_virt_to_phys(void *__virt_addr)
 {
 	unsigned long virt_addr = (unsigned long)__virt_addr;
 	phys_addr_t phys_addr;
-- 
1.7.1


      parent reply	other threads:[~2013-06-21  9:02 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <bug-59521-28872@https.bugzilla.kernel.org/>
2013-06-10 16:31 ` [PATCH] x86: kvmclock: zero initialize pvclock shared memory area Igor Mammedov
2013-06-10 20:19   ` Marcelo Tosatti
2013-06-15 18:01     ` [PATCHv1] kvm guest: fix uninitialized kvmclock read by KVM guest Eugene Batalov
2013-06-18 22:21       ` Marcelo Tosatti
2013-06-19 13:05         ` Paolo Bonzini
     [not found]           ` <CAJF2t5sYHy9q9a7-fZauf1Z7_FkK1_DOP13GHji=8-vDUsnnsQ@mail.gmail.com>
2013-06-19 13:29             ` Paolo Bonzini
2013-06-20  8:30               ` Igor Mammedov
2013-06-20  8:35                 ` Paolo Bonzini
2013-06-21  9:01 ` [PATCH 0/2 v2] x86: kvmclock: Prevent uninitialized per-cpu kvmclock usage Igor Mammedov
2013-06-21  9:01 ` [PATCH 1/2] x86: kvmclock: zero initialize pvclock shared memory area Igor Mammedov
2013-06-21  9:01 ` Igor Mammedov [this message]

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=1371805281-21305-3-git-send-email-imammedo@redhat.com \
    --to=imammedo@redhat.com \
    --cc=eabatalov89@gmail.com \
    --cc=hpa@zytor.com \
    --cc=kvm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=mtosatti@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=tglx@linutronix.de \
    --cc=x86@kernel.org \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).