Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1541053 > unrolled thread

[patch 1/2] x86/tsc: Validate TSC_ADJUST after resume

Started byThomas Gleixner <tglx@linutronix.de>
First post2016-12-13 14:20 +0100
Last post2016-12-15 12:00 +0100
Articles 4 — 3 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [patch 1/2] x86/tsc: Validate TSC_ADJUST after resume Thomas Gleixner <tglx@linutronix.de> - 2016-12-13 14:20 +0100
    Re: [patch 1/2] x86/tsc: Validate TSC_ADJUST after resume Peter Zijlstra <peterz@infradead.org> - 2016-12-13 14:30 +0100
      Re: [patch 1/2] x86/tsc: Validate TSC_ADJUST after resume Thomas Gleixner <tglx@linutronix.de> - 2016-12-13 14:30 +0100
    [tip:x86/timers] x86/tsc: Validate TSC_ADJUST after resume tip-bot for Thomas Gleixner <tipbot@zytor.com> - 2016-12-15 12:00 +0100

#1541053 — [patch 1/2] x86/tsc: Validate TSC_ADJUST after resume

FromThomas Gleixner <tglx@linutronix.de>
Date2016-12-13 14:20 +0100
Subject[patch 1/2] x86/tsc: Validate TSC_ADJUST after resume
Message-ID<sNV9U-1DV-5@gated-at.bofh.it>
Some 'feature' BIOSes fiddle with the TSC_ADJUST register during
suspend/resume which renders the TSC unusable.

Add sanity checks into the resume path and restore the
original value if it was adjusted.

Reported-by: Roland Scheidegger <rscheidegger_lists@hispeed.ch>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 arch/x86/include/asm/tsc.h |    4 ++--
 arch/x86/kernel/process.c  |    2 +-
 arch/x86/kernel/tsc.c      |    6 ++++++
 arch/x86/kernel/tsc_sync.c |    6 +++---
 arch/x86/power/cpu.c       |    1 +
 5 files changed, 13 insertions(+), 6 deletions(-)

--- a/arch/x86/include/asm/tsc.h
+++ b/arch/x86/include/asm/tsc.h
@@ -47,12 +47,12 @@ extern int tsc_clocksource_reliable;
  */
 #ifdef CONFIG_X86_TSC
 extern bool tsc_store_and_check_tsc_adjust(void);
-extern void tsc_verify_tsc_adjust(void);
+extern void tsc_verify_tsc_adjust(bool resume);
 extern void check_tsc_sync_source(int cpu);
 extern void check_tsc_sync_target(void);
 #else
 static inline bool tsc_store_and_check_tsc_adjust(void) { return false; }
-static inline void tsc_verify_tsc_adjust(void) { }
+static inline void tsc_verify_tsc_adjust(bool resume) { }
 static inline void check_tsc_sync_source(int cpu) { }
 static inline void check_tsc_sync_target(void) { }
 #endif
--- a/arch/x86/kernel/process.c
+++ b/arch/x86/kernel/process.c
@@ -277,7 +277,7 @@ void exit_idle(void)
 
 void arch_cpu_idle_enter(void)
 {
-	tsc_verify_tsc_adjust();
+	tsc_verify_tsc_adjust(false);
 	local_touch_nmi();
 	enter_idle();
 }
--- a/arch/x86/kernel/tsc.c
+++ b/arch/x86/kernel/tsc.c
@@ -1080,6 +1080,11 @@ static void detect_art(void)
 
 static struct clocksource clocksource_tsc;
 
+static void tsc_resume(struct clocksource *cs)
+{
+	tsc_verify_tsc_adjust(true);
+}
+
 /*
  * We used to compare the TSC to the cycle_last value in the clocksource
  * structure to avoid a nasty time-warp. This can be observed in a
@@ -1112,6 +1117,7 @@ static struct clocksource clocksource_ts
 	.flags                  = CLOCK_SOURCE_IS_CONTINUOUS |
 				  CLOCK_SOURCE_MUST_VERIFY,
 	.archdata               = { .vclock_mode = VCLOCK_TSC },
+	.resume			= tsc_resume,
 };
 
 void mark_tsc_unstable(char *reason)
--- a/arch/x86/kernel/tsc_sync.c
+++ b/arch/x86/kernel/tsc_sync.c
@@ -30,7 +30,7 @@ struct tsc_adjust {
 
 static DEFINE_PER_CPU(struct tsc_adjust, tsc_adjust);
 
-void tsc_verify_tsc_adjust(void)
+void tsc_verify_tsc_adjust(bool resume)
 {
 	struct tsc_adjust *adj = this_cpu_ptr(&tsc_adjust);
 	s64 curval;
@@ -39,7 +39,7 @@ void tsc_verify_tsc_adjust(void)
 		return;
 
 	/* Rate limit the MSR check */
-	if (time_before(jiffies, adj->nextcheck))
+	if (!resume && time_before(jiffies, adj->nextcheck))
 		return;
 
 	adj->nextcheck = jiffies + HZ;
@@ -51,7 +51,7 @@ void tsc_verify_tsc_adjust(void)
 	/* Restore the original value */
 	wrmsrl(MSR_IA32_TSC_ADJUST, adj->adjusted);
 
-	if (!adj->warned) {
+	if (!adj->warned || resume) {
 		pr_warn(FW_BUG "TSC ADJUST differs: CPU%u %lld --> %lld. Restoring\n",
 			smp_processor_id(), adj->adjusted, curval);
 		adj->warned = true;
--- a/arch/x86/power/cpu.c
+++ b/arch/x86/power/cpu.c
@@ -256,6 +256,7 @@ static void notrace __restore_processor_
 	mtrr_bp_restore();
 	perf_restore_debug_store();
 	msr_restore_context(ctxt);
+	tsc_verify_tsc_adjust(true);
 }
 
 /* Needed by apm.c */

[toc] | [next] | [standalone]

#1541057

FromPeter Zijlstra <peterz@infradead.org>
Date2016-12-13 14:30 +0100
Message-ID<sNVjz-1Hc-5@gated-at.bofh.it>
In reply to#1541053
On Tue, Dec 13, 2016 at 01:14:17PM -0000, Thomas Gleixner wrote:
> --- a/arch/x86/power/cpu.c
> +++ b/arch/x86/power/cpu.c
> @@ -256,6 +256,7 @@ static void notrace __restore_processor_
>  	mtrr_bp_restore();
>  	perf_restore_debug_store();
>  	msr_restore_context(ctxt);
> +	tsc_verify_tsc_adjust(true);
>  }

Should we do that sooner, as in before calling
restore_sched_clock_state() ? Otherwise we recompute the sched_clock
deltas vs the wrecked TSC and then fix it up through the ADJUST,
wrecking our sched clock again.

[toc] | [prev] | [next] | [standalone]

#1541058

FromThomas Gleixner <tglx@linutronix.de>
Date2016-12-13 14:30 +0100
Message-ID<sNVjA-1Hc-31@gated-at.bofh.it>
In reply to#1541057
On Tue, 13 Dec 2016, Peter Zijlstra wrote:

> On Tue, Dec 13, 2016 at 01:14:17PM -0000, Thomas Gleixner wrote:
> > --- a/arch/x86/power/cpu.c
> > +++ b/arch/x86/power/cpu.c
> > @@ -256,6 +256,7 @@ static void notrace __restore_processor_
> >  	mtrr_bp_restore();
> >  	perf_restore_debug_store();
> >  	msr_restore_context(ctxt);
> > +	tsc_verify_tsc_adjust(true);
> >  }
> 
> Should we do that sooner, as in before calling
> restore_sched_clock_state() ? Otherwise we recompute the sched_clock
> deltas vs the wrecked TSC and then fix it up through the ADJUST,
> wrecking our sched clock again.

Ah. Indeed. Will fix. That needs some thought on the secondary CPUs as
well...

Thanks,

	tglx

[toc] | [prev] | [next] | [standalone]

#1542636 — [tip:x86/timers] x86/tsc: Validate TSC_ADJUST after resume

Fromtip-bot for Thomas Gleixner <tipbot@zytor.com>
Date2016-12-15 12:00 +0100
Subject[tip:x86/timers] x86/tsc: Validate TSC_ADJUST after resume
Message-ID<sOBVv-4Kn-15@gated-at.bofh.it>
In reply to#1541053
Commit-ID:  6a369583178d0b89c2c3919c4456ee22fee0f249
Gitweb:     http://git.kernel.org/tip/6a369583178d0b89c2c3919c4456ee22fee0f249
Author:     Thomas Gleixner <tglx@linutronix.de>
AuthorDate: Tue, 13 Dec 2016 13:14:17 +0000
Committer:  Thomas Gleixner <tglx@linutronix.de>
CommitDate: Thu, 15 Dec 2016 11:44:29 +0100

x86/tsc: Validate TSC_ADJUST after resume

Some 'feature' BIOSes fiddle with the TSC_ADJUST register during
suspend/resume which renders the TSC unusable.

Add sanity checks into the resume path and restore the
original value if it was adjusted.

Reported-and-tested-by: Roland Scheidegger <rscheidegger_lists@hispeed.ch>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Cc: Bruce Schlobohm <bruce.schlobohm@intel.com>
Cc: Kevin Stanton <kevin.b.stanton@intel.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Allen Hung <allen_hung@dell.com>
Cc: Borislav Petkov <bp@alien8.de>
Link: http://lkml.kernel.org/r/20161213131211.317654500@linutronix.de
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>

---
 arch/x86/include/asm/tsc.h | 4 ++--
 arch/x86/kernel/process.c  | 2 +-
 arch/x86/kernel/tsc.c      | 6 ++++++
 arch/x86/kernel/tsc_sync.c | 6 +++---
 arch/x86/power/cpu.c       | 1 +
 5 files changed, 13 insertions(+), 6 deletions(-)

diff --git a/arch/x86/include/asm/tsc.h b/arch/x86/include/asm/tsc.h
index c054eaa..372ad0c 100644
--- a/arch/x86/include/asm/tsc.h
+++ b/arch/x86/include/asm/tsc.h
@@ -47,12 +47,12 @@ extern int tsc_clocksource_reliable;
  */
 #ifdef CONFIG_X86_TSC
 extern bool tsc_store_and_check_tsc_adjust(void);
-extern void tsc_verify_tsc_adjust(void);
+extern void tsc_verify_tsc_adjust(bool resume);
 extern void check_tsc_sync_source(int cpu);
 extern void check_tsc_sync_target(void);
 #else
 static inline bool tsc_store_and_check_tsc_adjust(void) { return false; }
-static inline void tsc_verify_tsc_adjust(void) { }
+static inline void tsc_verify_tsc_adjust(bool resume) { }
 static inline void check_tsc_sync_source(int cpu) { }
 static inline void check_tsc_sync_target(void) { }
 #endif
diff --git a/arch/x86/kernel/process.c b/arch/x86/kernel/process.c
index 4fe5dc8..a67e0f0 100644
--- a/arch/x86/kernel/process.c
+++ b/arch/x86/kernel/process.c
@@ -277,7 +277,7 @@ void exit_idle(void)
 
 void arch_cpu_idle_enter(void)
 {
-	tsc_verify_tsc_adjust();
+	tsc_verify_tsc_adjust(false);
 	local_touch_nmi();
 	enter_idle();
 }
diff --git a/arch/x86/kernel/tsc.c b/arch/x86/kernel/tsc.c
index 2bb8de4..bfb541a 100644
--- a/arch/x86/kernel/tsc.c
+++ b/arch/x86/kernel/tsc.c
@@ -1080,6 +1080,11 @@ static void detect_art(void)
 
 static struct clocksource clocksource_tsc;
 
+static void tsc_resume(struct clocksource *cs)
+{
+	tsc_verify_tsc_adjust(true);
+}
+
 /*
  * We used to compare the TSC to the cycle_last value in the clocksource
  * structure to avoid a nasty time-warp. This can be observed in a
@@ -1112,6 +1117,7 @@ static struct clocksource clocksource_tsc = {
 	.flags                  = CLOCK_SOURCE_IS_CONTINUOUS |
 				  CLOCK_SOURCE_MUST_VERIFY,
 	.archdata               = { .vclock_mode = VCLOCK_TSC },
+	.resume			= tsc_resume,
 };
 
 void mark_tsc_unstable(char *reason)
diff --git a/arch/x86/kernel/tsc_sync.c b/arch/x86/kernel/tsc_sync.c
index a75f696..94f2ce5 100644
--- a/arch/x86/kernel/tsc_sync.c
+++ b/arch/x86/kernel/tsc_sync.c
@@ -30,7 +30,7 @@ struct tsc_adjust {
 
 static DEFINE_PER_CPU(struct tsc_adjust, tsc_adjust);
 
-void tsc_verify_tsc_adjust(void)
+void tsc_verify_tsc_adjust(bool resume)
 {
 	struct tsc_adjust *adj = this_cpu_ptr(&tsc_adjust);
 	s64 curval;
@@ -39,7 +39,7 @@ void tsc_verify_tsc_adjust(void)
 		return;
 
 	/* Rate limit the MSR check */
-	if (time_before(jiffies, adj->nextcheck))
+	if (!resume && time_before(jiffies, adj->nextcheck))
 		return;
 
 	adj->nextcheck = jiffies + HZ;
@@ -51,7 +51,7 @@ void tsc_verify_tsc_adjust(void)
 	/* Restore the original value */
 	wrmsrl(MSR_IA32_TSC_ADJUST, adj->adjusted);
 
-	if (!adj->warned) {
+	if (!adj->warned || resume) {
 		pr_warn(FW_BUG "TSC ADJUST differs: CPU%u %lld --> %lld. Restoring\n",
 			smp_processor_id(), adj->adjusted, curval);
 		adj->warned = true;
diff --git a/arch/x86/power/cpu.c b/arch/x86/power/cpu.c
index 53cace2..66ade16 100644
--- a/arch/x86/power/cpu.c
+++ b/arch/x86/power/cpu.c
@@ -252,6 +252,7 @@ static void notrace __restore_processor_state(struct saved_context *ctxt)
 	fix_processor_context();
 
 	do_fpu_end();
+	tsc_verify_tsc_adjust(true);
 	x86_platform.restore_sched_clock_state();
 	mtrr_bp_restore();
 	perf_restore_debug_store();

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web