Skip to contentSkip to conversation

x86, TSC: Add a software TSC offset

PATCH In review Borislav Petkov 5 participants 20 messages Jul 19, 2014 – Jul 22, 2014 x86

lore
Share & tools
lore

Borislav Petkov

author maintainer
#0 lore raw

PATCH 1/1 x86, TSC: Add a software TSC offset

From: Borislav Petkov <[email protected]>

There are machines which do have stable and always-running TSCs but the last get started at different points in time by the platform, causing the TSCs to have a small constant diff.

It has been tried a couple of times to resync those during that sync check but the procedure is error prone and flaky, and not 100% successful.

So, instead of doing that, let's not touch the TSCs at all but save a per-CPU TSC offset which we add to the TSC value we've read from the Time-Stamp Counter. The hope is thus to still salvage the TSC on those machines.

For that to work, we need to populate the TSC AUX MSR with the core ID prior to doing the TSC sync check so that RDTSCP can give us the correct core number and we can add the offset atomically. And yes, we need a X86_FEATURE_RDTSCP CPU for the whole deal to work. Older ones simply lose.

See also comment above tsc_sync.c::compute_tsc_offset() for more details.

Signed-off-by Borislav Petkov <[email protected]>
---
 arch/x86/include/asm/cpufeature.h |   1 +
 arch/x86/include/asm/tsc.h        |  19 +++++++
 arch/x86/kernel/cpu/common.c      |   2 +
 arch/x86/kernel/tsc_sync.c        | 117 ++++++++++++++++++++++++++++++++++----
 4 files changed, 129 insertions(+), 10 deletions(-)
arch/x86/include/asm/cpufeature.h +1 -0
index bb9b258d60e7..8c27e55372fb 100644
@@ -244,6 +244,7 @@
 #define X86_BUG_11AP		X86_BUG(5) /* Bad local APIC aka 11AP */
 #define X86_BUG_FXSAVE_LEAK	X86_BUG(6) /* FXSAVE leaks FOP/FIP/FOP */
 #define X86_BUG_CLFLUSH_MONITOR	X86_BUG(7) /* AAI65, CLFLUSH required before MONITOR */
+#define X86_BUG_TSC_OFFSET	X86_BUG(8) /* CPU has skewed but stable TSCs */
 
 #if defined(__KERNEL__) && !defined(__ASSEMBLY__)
arch/x86/include/asm/tsc.h +19 -0
index 94605c0e9cee..a91f439738f9 100644
@@ -9,6 +9,9 @@
 #define NS_SCALE	10 /* 2^10, carefully chosen */
 #define US_SCALE	32 /* 2^32, arbitralrily chosen */
 
+
+DECLARE_PER_CPU(long long, tsc_offset);
+
 /*
  * Standard way to access the cycle counter.
  */

@@ -32,6 +35,22 @@ static inline cycles_t get_cycles(void)
 	return ret;
 }
 
+static inline cycles_t get_cycles_aux(void)
+{
+	unsigned long long ret = 0;
+	int cpu;
+
+#ifndef CONFIG_X86_TSC
+	if (!cpu_has_tsc)
+		return 0;
+#endif
+	if (static_cpu_has_safe(X86_FEATURE_RDTSCP)) {
+		rdtscpll(ret, cpu);
+		return ret + per_cpu(tsc_offset, cpu);
+	} else
+		return get_cycles();
+}
+
 static __always_inline cycles_t vget_cycles(void)
 {
 	/*
arch/x86/kernel/cpu/common.c +2 -0
index ce31eeada362..c67300281790 100644
@@ -962,6 +962,8 @@ static void identify_cpu(struct cpuinfo_x86 *c)
 #ifdef CONFIG_NUMA
 	numa_add_cpu(smp_processor_id());
 #endif
+	if (cpu_has(c, X86_FEATURE_RDTSCP))
+		write_rdtscp_aux(smp_processor_id());
 }
 
 #ifdef CONFIG_X86_64
arch/x86/kernel/tsc_sync.c +107 -10

203 lines not on this page · raw message

Signature · 5 lines
Regards/Gruss,
    Boris.

Sent from a fat crate under my desk. Formatting is fine.
--
author maintainer
#1 lore raw
Quoted 15 lines · Borislav Petkov

From: Borislav Petkov <[email protected]>

There are machines which do have stable and always-running TSCs but the last get started at different points in time by the platform, causing the TSCs to have a small constant diff.

It has been tried a couple of times to resync those during that sync check but the procedure is error prone and flaky, and not 100% successful.

So, instead of doing that, let's not touch the TSCs at all but save a per-CPU TSC offset which we add to the TSC value we've read from the Time-Stamp Counter. The hope is thus to still salvage the TSC on those machines.

Quoted 7 lines

For that to work, we need to populate the TSC AUX MSR with the core ID prior to doing the TSC sync check so that RDTSCP can give us the correct core number and we can add the offset atomically. And yes, we need a X86_FEATURE_RDTSCP CPU for the whole deal to work. Older ones simply lose.

See also comment above tsc_sync.c::compute_tsc_offset() for more details.

And here's how it looks like: So I'm injecting a TSC diff locally because I don't have a machine which has that problem, Peter has a WSM for that.

So here's the case where the target CPU has started its TSC earlier than the source CPU:

[    0.264966] x86: Booting SMP configuration:
[    0.265151] .... node  #0, CPUs:      #1
[    0.281610] 1, tsc1: 37576107984
[    0.281611] updating with 600000

This is the error injection into the TSC of CPU1 with +600K cycles.

[    0.281990] 1, tsc2: 37576716684

...

[    0.284259] TSCs of [CPU#0 -> CPU#1] 599193 cycles out of sync, saving offset.
[    0.284756] CPU1, saved offset: -599193

We save a negative offset, and we also see the time it took us to do a RMW on the TSC :-)

Then we run the sync test again, this time we read the TSC and add the negative offset.

[    0.287156] TSC synchronization [CPU#0 -> CPU#1]: passed
[    0.287385] x86: Booted up 1 node, 2 CPUs

And now the case where the target CPU starts later than the source (I'd expect this to be the common case):

[    0.264850] x86: Booting SMP configuration:
[    0.265036] .... node  #0, CPUs:      #1
[    0.281476] identify_cpu: Setting TSC_AUX MSR, cpu 1
[    0.281495] 1, tsc1: 56268738505
[    0.281497] updating with -12345678

injection

[    0.273772] 1, tsc2: 56256402112

...

[    0.284183] TSCs of [CPU#0 -> CPU#1] 12345363 cycles out of sync, saving offset.
[    0.276608] CPU1, saved offset: 12345363
[    0.287057] TSC synchronization [CPU#0 -> CPU#1]: passed
[    0.287288] x86: Booted up 1 node, 2 CPUs

We also state that we have this "workaround" enabled in /proc/cpuinfo:

processor : 1

...

flags : fpu vme de pse tsc msr pae mce cx8 apic sep mtrr pge mca cmov pat pse36 clflush mmx fxsr sse sse2 ht syscall nx mmxext fxsr_opt pdpe1gb rdtscp lm constant_tsc rep_good nopl nonstop_tsc extd_apicid aperfmperf pni monitor ssse3 cx16 popcnt lahf_lm cmp_legacy svm extapic cr8_legacy abm sse4a misalignsse 3dnowprefetch ibs skinit wdt arat hw_pstate npt lbrv svm_lock nrip_save pausefilter
bugs : fxsave_leak tsc_offset

			      ^^^^^^^^^^

bogomips : 3193.18
TLB size : 1024 4K pages
clflush size : 64
cache_alignment : 64
address sizes : 36 bits physical, 48 bits virtual
power management: ts ttp tm stc 100mhzsteps hwpstate

The whole deal needs more testing now.

Signature · 5 lines
Regards/Gruss,
    Boris.

Sent from a fat crate under my desk. Formatting is fine.
--
#2 lore raw
In reply to Borislav Petkov
Quoted 15 lines · Borislav Petkov

From: Borislav Petkov <[email protected]>

There are machines which do have stable and always-running TSCs but the last get started at different points in time by the platform, causing the TSCs to have a small constant diff.

It has been tried a couple of times to resync those during that sync check but the procedure is error prone and flaky, and not 100% successful.

So, instead of doing that, let's not touch the TSCs at all but save a per-CPU TSC offset which we add to the TSC value we've read from the Time-Stamp Counter. The hope is thus to still salvage the TSC on those machines.

Quoted 5 lines

For that to work, we need to populate the TSC AUX MSR with the core ID prior to doing the TSC sync check so that RDTSCP can give us the correct core number and we can add the offset atomically. And yes, we need a X86_FEATURE_RDTSCP CPU for the whole deal to work. Older ones simply lose.

I'm sure I'm missing something, but where is tsc_offset used outside the tsc startup code? Is it somehow getting programmed into the TSC config registers?

--Andy

author maintainer
#3 lore raw
Quoted 3 lines · Andy Lutomirski

I'm sure I'm missing something, but where is tsc_offset used outside the tsc startup code? Is it somehow getting programmed into the TSC config registers?

Nah, we're programming the CPU number into the TSC_AUX MSR. However, that approach might become obsolete for a simpler, preempt-disable version which Peter suggested today. It should be comparatively cheap and work on all CPUs, not only RDTSCP-supporting ones:

get_cycles() {
	preempt_disable();
	rdtscll() + this_cpu_read(tsc_offset);
	preempt_disable()
}

So, to answer your question, the TSC offset will be used in get_cycles(). So even on machines with a skewed TSC, we will have a stable TSC counter by adding the per-cpu offset each time we read the TSC.

We'll see how it plays out in testing. :)

Signature · 5 lines
Regards/Gruss,
    Boris.

Sent from a fat crate under my desk. Formatting is fine.
--
#4 lore raw
Quoted 16 lines · Borislav Petkov
Quoted 3 lines · Andy Lutomirski

I'm sure I'm missing something, but where is tsc_offset used outside the tsc startup code? Is it somehow getting programmed into the TSC config registers?

Nah, we're programming the CPU number into the TSC_AUX MSR. However, that approach might become obsolete for a simpler, preempt-disable version which Peter suggested today. It should be comparatively cheap and work on all CPUs, not only RDTSCP-supporting ones:

get_cycles() {
        preempt_disable();
        rdtscll() + this_cpu_read(tsc_offset);
        preempt_disable()
}
Quoted 5 lines

So, to answer your question, the TSC offset will be used in get_cycles(). So even on machines with a skewed TSC, we will have a stable TSC counter by adding the per-cpu offset each time we read the TSC.

We'll see how it plays out in testing. :)

How will this be compatible with the vdso?

Also, IIRC, rdtscp does not need rdtsc_barrier(), whereas rdtsc does. Getting this wrong will be a significant slowdown.

--Andy

Quoted 7 lines

--
Regards/Gruss,
Boris.

Sent from a fat crate under my desk. Formatting is fine. --

Signature · 2 lines
Andy Lutomirski
AMA Capital Management, LLC
author maintainer
#5 lore raw
Quoted 1 line · Andy Lutomirski

How will this be compatible with the vdso?

I've never thought about it yet. How compatible would you want it to be and what do you expect from it?

Remember, this is only attempting to be a hardware workaround for a smallish number of systems out there. Most of current machines should have stable and synched TSCs.

Quoted 2 lines

Also, IIRC, rdtscp does not need rdtsc_barrier(), whereas rdtsc does. Getting this wrong will be a significant slowdown.

This is too cryptic for me - get_cycles doesn't barrier around the TSC now either. Again, we will most likely end up not using RDTSCP anyway.

Signature · 5 lines
Regards/Gruss,
    Boris.

Sent from a fat crate under my desk. Formatting is fine.
--
#6 lore raw
Quoted 5 lines · Borislav Petkov
Quoted 1 line · Andy Lutomirski

How will this be compatible with the vdso?

I've never thought about it yet. How compatible would you want it to be and what do you expect from it?

I expect that users of __vdso_clock_gettime (e.g. glibc) will get the correct time :) They use vread_tsc, and they can't use preempt_disable, because they're in userspace. They also can't directly access per-cpu variables.

Turning off vdso tsc support on these machines would be an option.

Quoted 4 lines

Remember, this is only attempting to be a hardware workaround for a smallish number of systems out there. Most of current machines should have stable and synched TSCs.

I actually own one of these systems. It's a Sandy Bridge Core-i7 Extreme or something like that.

Quoted 6 lines
Quoted 3 lines

Also, IIRC, rdtscp does not need rdtsc_barrier(), whereas rdtsc does. Getting this wrong will be a significant slowdown.

This is too cryptic for me - get_cycles doesn't barrier around the TSC now either. Again, we will most likely end up not using RDTSCP anyway.

I wonder if that's a bug in get_cycles.

The basic issue is that rdtsc is not ordered with respect to nearby loads, so it's fairly easy to see it behaving non-monotonically across CPUs. rdtscp is ordered, but it's a little slower.

--Andy

#7 lore raw
Quoted 7 lines · Andy Lutomirski
Quoted 5 lines · Borislav Petkov
Quoted 1 line · Andy Lutomirski

How will this be compatible with the vdso?

I've never thought about it yet. How compatible would you want it to be and what do you expect from it?

Quoted 6 lines

I expect that users of __vdso_clock_gettime (e.g. glibc) will get the correct time :) They use vread_tsc, and they can't use preempt_disable, because they're in userspace. They also can't directly access per-cpu variables.

Turning off vdso tsc support on these machines would be an option.

Correct.

So if we make the TSC usable that way we need to mark it unusable for the VDSO and force vdso_*gettime* into the kernel. That will be way better than forcing the whole machine onto HPET.

And once we are using the real syscall, the whole thing just works ...

Thanks,

tglx

#8 lore raw
Quoted 15 lines · Thomas Gleixner
Quoted 7 lines · Andy Lutomirski
Quoted 5 lines · Borislav Petkov
Quoted 1 line · Andy Lutomirski

How will this be compatible with the vdso?

I've never thought about it yet. How compatible would you want it to be and what do you expect from it?

Quoted 6 lines

I expect that users of __vdso_clock_gettime (e.g. glibc) will get the correct time :) They use vread_tsc, and they can't use preempt_disable, because they're in userspace. They also can't directly access per-cpu variables.

Turning off vdso tsc support on these machines would be an option.

Quoted 7 lines

Correct.

So if we make the TSC usable that way we need to mark it unusable for the VDSO and force vdso_*gettime* into the kernel. That will be way better than forcing the whole machine onto HPET.

And once we are using the real syscall, the whole thing just works ...

So all it needs is to do

    clocksource_tsc.archdata.vclock_mode = VCLOCK_NONE;

if we do the magic per cpu offset trick to make the fcked up TSC usable.

Thanks,

tglx

#9 lore raw
Quoted 22 lines · Thomas Gleixner
Quoted 21 lines · Thomas Gleixner
Quoted 7 lines · Andy Lutomirski
Quoted 5 lines · Borislav Petkov
Quoted 1 line · Andy Lutomirski

How will this be compatible with the vdso?

I've never thought about it yet. How compatible would you want it to be and what do you expect from it?

Quoted 6 lines

I expect that users of __vdso_clock_gettime (e.g. glibc) will get the correct time :) They use vread_tsc, and they can't use preempt_disable, because they're in userspace. They also can't directly access per-cpu variables.

Turning off vdso tsc support on these machines would be an option.

Correct.

So if we make the TSC usable that way we need to mark it unusable for the VDSO and force vdso_*gettime* into the kernel. That will be way better than forcing the whole machine onto HPET.

Quoted 8 lines
Quoted 1 line

And once we are using the real syscall, the whole thing just works ...

So all it needs is to do

    clocksource_tsc.archdata.vclock_mode = VCLOCK_NONE;

if we do the magic per cpu offset trick to make the fcked up TSC usable.

Just make sure this happens every time the tsc clocksource is selected -- it's possible to go back and forth.

Quoted 5 lines

Thanks,

tglx

Signature · 2 lines
Andy Lutomirski
AMA Capital Management, LLC
author maintainer
#10 lore raw
In reply to Andy Lutomirski
Quoted 6 lines · Andy Lutomirski

I expect that users of __vdso_clock_gettime (e.g. glibc) will get the correct time :) They use vread_tsc, and they can't use preempt_disable, because they're in userspace. They also can't directly access per-cpu variables.

Turning off vdso tsc support on these machines would be an option.

Right, this is what I was going to propose to tglx on IRC. Or we can try to come up with something working for the vdso too, say RDTSCP :-)

But that still won't work as it needs the per-cpu variables. So I guess vdso loses...

Quoted 2 lines

I actually own one of these systems. It's a Sandy Bridge Core-i7 Extreme or something like that.

Ha, cool, so I've got my tester! :-)

Quoted 5 lines

I wonder if that's a bug in get_cycles.

The basic issue is that rdtsc is not ordered with respect to nearby loads, so it's fairly easy to see it behaving non-monotonically across CPUs. rdtscp is ordered, but it's a little slower.

Yah, that I know. But I don't see get_cycles() having the barriers. So it might be a bug. We can certainly try to "fix" it and see what happens :-)

arch/x86/include/asm/tsc.h +2 -0
index 94605c0e9cee..ad7d5e449c0b 100644
@@ -27,7 +27,9 @@ static inline cycles_t get_cycles(void)
 	if (!cpu_has_tsc)
 		return 0;
 #endif
+	rdtsc_barrier();
 	rdtscll(ret);
+	rdtsc_barrier();
 
 	return ret;
 }
Signature · 5 lines
Regards/Gruss,
    Boris.

Sent from a fat crate under my desk. Formatting is fine.
--
#11 lore raw
Quoted 11 lines · Borislav Petkov
Quoted 6 lines · Andy Lutomirski

I expect that users of __vdso_clock_gettime (e.g. glibc) will get the correct time :) They use vread_tsc, and they can't use preempt_disable, because they're in userspace. They also can't directly access per-cpu variables.

Turning off vdso tsc support on these machines would be an option.

Right, this is what I was going to propose to tglx on IRC. Or we can try to come up with something working for the vdso too, say RDTSCP :-)

Quoted 7 lines

But that still won't work as it needs the per-cpu variables. So I guess vdso loses...

Quoted 2 lines

I actually own one of these systems. It's a Sandy Bridge Core-i7 Extreme or something like that.

Ha, cool, so I've got my tester! :-)

Ha. Ha ha. Muahaha. Because IIRC this box is synced until the first time it suspends.

Quoted 16 lines
Quoted 6 lines

I wonder if that's a bug in get_cycles.

The basic issue is that rdtsc is not ordered with respect to nearby loads, so it's fairly easy to see it behaving non-monotonically across CPUs. rdtscp is ordered, but it's a little slower.

Yah, that I know. But I don't see get_cycles() having the barriers. So it might be a bug. We can certainly try to "fix" it and see what happens :-)

arch/x86/include/asm/tsc.h +0 -0
index 94605c0e9cee..ad7d5e449c0b 100644

arch/x86/include/asm/tsc.h static inline cycles_t get_cycles(void) · line 33

Quoted 7 lines
arch/x86/include/asm/tsc.h +2 -0
@@ -27,7 +27,9 @@ static inline cycles_t get_cycles(void)
        if (!cpu_has_tsc)
                return 0;
 #endif
+       rdtsc_barrier();
        rdtscll(ret);
+       rdtsc_barrier();

Only the first of these is necessary. There was a long thread on this a couple of years ago, and the conclusion was that the code in vread_tsc in vclock_gettime.c is correct.

--Andy

Quoted 9 lines
arch/x86/include/asm/tsc.h +0 -1
        return ret;
 }
 
--

Regards/Gruss,
Boris.

Sent from a fat crate under my desk. Formatting is fine. --

Signature · 2 lines
Andy Lutomirski
AMA Capital Management, LLC
author maintainer
#12 lore raw
Quoted 2 lines · Andy Lutomirski

Ha. Ha ha. Muahaha. Because IIRC this box is synced until the first time it suspends.

Sweet, TSCs get fumbled in some S-state or maybe SMI... Who TF knows.

Well, I'm thinking upon resume, we run through smpboot which should do the tsc sync check again. Will have to test to see.

Quoted 8 lines
Quoted 8 lines
arch/x86/include/asm/tsc.h +0 -0
index 94605c0e9cee..ad7d5e449c0b 100644
@@ -27,7 +27,9 @@ static inline cycles_t get_cycles(void)
        if (!cpu_has_tsc)
                return 0;
 #endif
Quoted 8 lines

arch/x86/include/asm/tsc.h static inline cycles_t get_cycles(void) · line 33

Quoted 4 lines
arch/x86/include/asm/tsc.h +2 -0
+       rdtsc_barrier();
        rdtscll(ret);
+       rdtsc_barrier();

Only the first of these is necessary. There was a long thread on this a couple of years ago, and the conclusion was that the code in vread_tsc in vclock_gettime.c is correct.

* The various CPU manuals are unclear
* as to whether rdtsc can be reordered with later loads,
* but no one has ever seen it happen.

until some future uarch proves you wrong. :-)

I guess we can try with one pre-fence only first although if we're doing one already, I can't imagine the post-one to be all that expensive since we've retired the whole inflight crap already anyway.

Signature · 5 lines
Regards/Gruss,
    Boris.

Sent from a fat crate under my desk. Formatting is fine.
--
#13 lore raw
Quoted 8 lines · Borislav Petkov
Quoted 2 lines · Andy Lutomirski

Ha. Ha ha. Muahaha. Because IIRC this box is synced until the first time it suspends.

Sweet, TSCs get fumbled in some S-state or maybe SMI... Who TF knows.

Well, I'm thinking upon resume, we run through smpboot which should do the tsc sync check again. Will have to test to see.

I have some reason to believe that this is almost an intentional bug on the part of the BIOS vendor.

Quoted 22 lines
Quoted 17 lines

arch/x86/include/asm/tsc.h static inline cycles_t get_cycles(void) · line 33

Quoted 13 lines
arch/x86/include/asm/tsc.h +2 -0
index 94605c0e9cee..ad7d5e449c0b 100644
@@ -27,7 +27,9 @@ static inline cycles_t get_cycles(void)
        if (!cpu_has_tsc)
                return 0;
 #endif
+       rdtsc_barrier();
        rdtscll(ret);
+       rdtsc_barrier();

Only the first of these is necessary. There was a long thread on this a couple of years ago, and the conclusion was that the code in vread_tsc in vclock_gettime.c is correct.

* The various CPU manuals are unclear
* as to whether rdtsc can be reordered with later loads,
* but no one has ever seen it happen.

Quoted 6 lines

until some future uarch proves you wrong. :-)

I guess we can try with one pre-fence only first although if we're doing one already, I can't imagine the post-one to be all that expensive since we've retired the whole inflight crap already anyway.

IIRC it was actually quite expensive, at least on Sandy Bridge. Maybe AMD is different.

Anyway, if some future uarch breaks this, I could resurrect my old
hack: do a TSC-dependent load prior to returning. Loads are ordered, and the hackish load can't be reordered wrt RDTSC due to data-dependency, so we're in business :)

--Andy

author maintainer
#14 lore raw
Quoted 2 lines · Andy Lutomirski

I have some reason to believe that this is almost an intentional bug on the part of the BIOS vendor.

Hiding SMIs or some other dumb, I-know-how-to-do-stuff-better-than-you BIOS gunk.

Quoted 1 line

IIRC it was actually quite expensive, at least on Sandy Bridge.

Hmm, strange. So after the first fence and when you get to retire the second fence, you will have only the RDTSC eligible for retirement and everything that's coming behind it in program order can still go out-of-order. Unless the second fence flushes more stuff. I most likely am missing something.

Quoted 6 lines

Maybe AMD is different.

Anyway, if some future uarch breaks this, I could resurrect my old
hack: do a TSC-dependent load prior to returning. Loads are ordered, and the hackish load can't be reordered wrt RDTSC due to data-dependency, so we're in business :)

:-)

Signature · 5 lines
Regards/Gruss,
    Boris.

Sent from a fat crate under my desk. Formatting is fine.
--
#15 lore raw
In reply to Andy Lutomirski
Quoted 7 lines · Andy Lutomirski
Quoted 4 lines
Quoted 2 lines

I actually own one of these systems. It's a Sandy Bridge Core-i7 Extreme or something like that.

Ha, cool, so I've got my tester! :-)

Ha. Ha ha. Muahaha. Because IIRC this box is synced until the first time it suspends.

Oh cute, one of those :-) Yes some BIOSes write 'random' crap into the TSC when doing the S states jig. We have some tsc suspend/resume hooks to correct the worst of that, but yeah screwy that.

#16 lore raw
In reply to Andy Lutomirski
Quoted 6 lines · Andy Lutomirski
Quoted 3 lines

Remember, this is only attempting to be a hardware workaround for a smallish number of systems out there. Most of current machines should have stable and synched TSCs.

I actually own one of these systems. It's a Sandy Bridge Core-i7 Extreme or something like that.

If it is (and it sounds like it is) a single socket, then your unsynced TSC is likely due to SMM fuckery and the TSCs will drift further and further apart as (run)time increases due to SMM activity.

The problem Borislav is talking about is multi socket systems, which due to (failed) board layout get the CPUs powered up 'wrong' and the TSCs between sockets is offset because of this, its a fixed offset and stable forever after (until power cycle etc..).

I have one WSM-EP that does this (occasionally).

His initial idea was to re-write the TSC value to match, but since writing the TSC is expensive (in the 1000s of cycles range) getting an offset adjustment of 10s of cycles in just right is nigh on impossible.

#17 lore raw
In reply to Borislav Petkov

Your patch inspired me to write this hack. I was curious to know how the TSCs of my boxes were with respect to each other, and wanted to get an idea. Maybe there's a better way, but I decided to waste an hour and write this hack up.

Here's what it does. It creates the file /sys/kernel/debug/rdtsc_test, and when you read it, it does some whacky things.

1) A table is set up with the number of possible CPUs. The cable consists of: index, TSC count, CPU.

2) A atomic variable is set to the number of online CPUS.

3) An IPI is sent to each of the other CPUs to run the test.

4) The test decrements the atomic, and then spins until it reaches zero.

5) The caller of smp_call_function() then calls the test iself, being the last to decrement the counter causing it to go to zero and all CPUs then fight for a spinlock.

6) When the spin lock is taken, it records which place it was in (order of spinlock taken, and records its own TSC. Then it releases the lock.

7) It then records in the table where its position is, its TSC counter and CPU number.

Finally, the read will show the results of the table. Looks something like this:

# cat /debug/rdtsc_test 
    0)  1305910016816  (cpu:5)
    1)  1305910017550  (cpu:7)
    2)  1305910017712  (cpu:1)
    3)  1305910017910  (cpu:6)
    4)  1305910018042  (cpu:2)
    5)  1305910018226  (cpu:3)
    6)  1305910018416  (cpu:4)
    7)  1305910018540  (cpu:0)

As long as the TSC counts are in order of the index, the TSC is moving forward nicely. If they are not in order, then the TSCs are not in sync.

Yes, this is a hack, but I think it's a somewhat useful hack.

Not-for-inclusion-by Steven Rostedt <[email protected]>
---
 kernel/Makefile     |    1 
 kernel/rdtsc_test.c |  127 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 128 insertions(+)
kernel/Makefile +1 -0
Index: linux-trace.git/kernel/Makefile
===================================================================
@@ -28,6 +28,7 @@
 obj-y += irq/
 obj-y += rcu/
 
+obj-$(CONFIG_DEBUG_FS) += rdtsc_test.o
 obj-$(CONFIG_CHECKPOINT_RESTORE) += kcmp.o
 obj-$(CONFIG_FREEZER) += freezer.o
 obj-$(CONFIG_PROFILING) += profile.o
kernel/rdtsc_test.c new file +127 -0

127 lines not on this page · raw message

author maintainer
#18 lore raw
Quoted 41 lines · Steven Rostedt

Your patch inspired me to write this hack. I was curious to know how the TSCs of my boxes were with respect to each other, and wanted to get an idea. Maybe there's a better way, but I decided to waste an hour and write this hack up.

Here's what it does. It creates the file /sys/kernel/debug/rdtsc_test, and when you read it, it does some whacky things.

1) A table is set up with the number of possible CPUs. The cable consists of: index, TSC count, CPU.

2) A atomic variable is set to the number of online CPUS.

3) An IPI is sent to each of the other CPUs to run the test.

4) The test decrements the atomic, and then spins until it reaches zero.

5) The caller of smp_call_function() then calls the test iself, being the last to decrement the counter causing it to go to zero and all CPUs then fight for a spinlock.

6) When the spin lock is taken, it records which place it was in (order of spinlock taken, and records its own TSC. Then it releases the lock.

7) It then records in the table where its position is, its TSC counter and CPU number.

Finally, the read will show the results of the table. Looks something like this:

# cat /debug/rdtsc_test 
    0)  1305910016816  (cpu:5)
    1)  1305910017550  (cpu:7)
    2)  1305910017712  (cpu:1)
    3)  1305910017910  (cpu:6)
    4)  1305910018042  (cpu:2)
    5)  1305910018226  (cpu:3)
    6)  1305910018416  (cpu:4)
    7)  1305910018540  (cpu:0)
Quoted 4 lines

As long as the TSC counts are in order of the index, the TSC is moving forward nicely. If they are not in order, then the TSCs are not in sync.

Yes, this is a hack, but I think it's a somewhat useful hack.

I think so too, especially if one would like to take a look at the TSCs and how they're looking like after, say, suspend cycle, a long machine runtime when there is suspicion that some SMM might've been entered or so...

However, I think you can do all that from luserspace, even though you'd have to do more dancing like pinning threads to cpus and those threads would have to synchronize back on rdtsc_start after having read the TSC, i.e.

	while(atomic_read(&rdtsc_start) != num_online_cpus())
		cpu_relax();

so that you can make sure they all have read the TSC at least once and nothing gets reordered.

It might work if I'm not missing something.

Doing it in the kernel is much easier with all that ring0 functionality present. :-)

Signature · 5 lines
Regards/Gruss,
    Boris.

Sent from a fat crate under my desk. Formatting is fine.
--
author maintainer
#19 lore raw
In reply to Borislav Petkov

Ok, here's the preempt-version we were talking about.

Please don't look at the vdso hunk - I had to make it build. Will fix properly later once we've established whether this actually makes sense at all first.

:-)

--

arch/x86/include/asm/cpufeature.h +1 -0
index bb9b258d60e7..8c27e55372fb 100644
@@ -244,6 +244,7 @@
 #define X86_BUG_11AP		X86_BUG(5) /* Bad local APIC aka 11AP */
 #define X86_BUG_FXSAVE_LEAK	X86_BUG(6) /* FXSAVE leaks FOP/FIP/FOP */
 #define X86_BUG_CLFLUSH_MONITOR	X86_BUG(7) /* AAI65, CLFLUSH required before MONITOR */
+#define X86_BUG_TSC_OFFSET	X86_BUG(8) /* CPU has skewed but stable TSCs */
 
 #if defined(__KERNEL__) && !defined(__ASSEMBLY__)
arch/x86/include/asm/tsc.h +5 -2
index 94605c0e9cee..904bc182a16b 100644
@@ -4,11 +4,11 @@
 #ifndef _ASM_X86_TSC_H
 #define _ASM_X86_TSC_H
 
-#include <asm/processor.h>
-
 #define NS_SCALE	10 /* 2^10, carefully chosen */
 #define US_SCALE	32 /* 2^32, arbitralrily chosen */
 
+DECLARE_PER_CPU(long long, tsc_offset);
+
 /*
  * Standard way to access the cycle counter.
  */

@@ -27,7 +27,10 @@ static inline cycles_t get_cycles(void)
 	if (!cpu_has_tsc)
 		return 0;
 #endif
+	preempt_disable();
 	rdtscll(ret);
+	ret += this_cpu_read_8(tsc_offset);
+	preempt_enable();
 
 	return ret;
 }
arch/x86/kernel/tsc_sync.c +105 -8

181 lines not on this page · raw message

arch/x86/vdso/vdso32/vclock_gettime.c +3 -0
index 175cc72c0f68..d5cba62bbf46 100644
@@ -25,6 +25,9 @@
 
 #define BUILD_VDSO32_64
 
+#undef this_cpu_read_8
+#define this_cpu_read_8(dummy) (0)
+
 #endif
 
 #include "../vclock_gettime.c"
Signature · 5 lines
Regards/Gruss,
    Boris.

Sent from a fat crate under my desk. Formatting is fine.
--