x86/tsc: Clean up rdtsc helpers
Subsystems (15)
Andy Lutomirski
maintainerv2 cover x86/tsc: Clean up rdtsc helpers
My sincere apologies for the spam. I send an unholy mixture of the real patch set and an old poorly split-up patch set, and the result is incomprehensible. Here's what I meant to send.
After the some recent threads about rdtsc barriers, I remembered that our RDTSC wrappers are a big mess. Let's clean it up.
Currently we have rdtscl, rdtscll, native_read_tsc, paravirt_read_tsc, and rdtsc_barrier. For people who haven't noticed rdtsc_barrier and who haven't carefully read the docs, there's no indication that all of the other accessors have a giant ordering gotcha. The macro forms are ugly, and the paravirt implementation is completely pointless.
rdtscl is particularly awful. It reads the low bits. There are no performance critical users of just the low bits anywhere in the kernel.
Clean it up. After this patch set, there are exactly three functions. rdtsc_unordered() is a function that does a raw RDTSC and returns a 64-bit number. rdtsc_ordered() is a function that does a properly ordered RDTSC for general-purpose use. barrier_before_rdtsc() is exactly what it sounds like.
Andy Lutomirski (17):
x86/tsc: Inline native_read_tsc and remove __native_read_tsc x86/msr/kvm: Remove vget_cycles()
x86/tsc/paravirt: Remove the read_tsc and read_tscp paravirt hooks x86/tsc: Replace rdtscll with native_read_tsc
x86/tsc: Remove the rdtscp and rdtscpll macros
x86/tsc: Use the full 64-bit tsc in tsc_delay
x86/cpu/amd: Use the full 64-bit TSC to detect the 2.6.2 bug
baycom_epp: Replace rdtscl() with native_read_tsc() staging/lirc_serial: Remove TSC-based timing input/joystick/analog: Switch from rdtscl() to native_read_tsc() drivers/input/gameport: Replace rdtscl() with native_read_tsc() x86/tsc: Remove rdtscl()
x86/tsc: Rename native_read_tsc() to rdtsc_unordered() x86/tsc: Move rdtsc_barrier() and rename it to barrier_before_rdtsc()
x86: Add rdtsc_ordered() and use it in trivial call sites x86/tsc: Use rdtsc_ordered() in check_tsc_warp() and drop extra
barriers
x86/tsc: In read_tsc, use rdtsc_ordered() instead of get_cycles()
arch/x86/boot/compressed/aslr.c | 2 +- arch/x86/entry/vdso/vclock_gettime.c | 16 +----- arch/x86/include/asm/barrier.h | 11 ---- arch/x86/include/asm/msr.h | 63 +++++++++++++++------- arch/x86/include/asm/paravirt.h | 34 ------------ arch/x86/include/asm/paravirt_types.h | 2 - arch/x86/include/asm/pvclock.h | 8 +-- arch/x86/include/asm/stackprotector.h | 2 +- arch/x86/include/asm/tsc.h | 18 +------ arch/x86/kernel/apb_timer.c | 8 +-- arch/x86/kernel/apic/apic.c | 8 +-- arch/x86/kernel/cpu/amd.c | 6 +-- arch/x86/kernel/cpu/mcheck/mce.c | 4 +- arch/x86/kernel/espfix_64.c | 2 +- arch/x86/kernel/hpet.c | 4 +- arch/x86/kernel/paravirt.c | 2 - arch/x86/kernel/paravirt_patch_32.c | 2 - arch/x86/kernel/trace_clock.c | 7 +-- arch/x86/kernel/tsc.c | 12 ++--- arch/x86/kernel/tsc_sync.c | 14 +++-- arch/x86/kvm/lapic.c | 4 +- arch/x86/kvm/svm.c | 4 +- arch/x86/kvm/vmx.c | 4 +- arch/x86/kvm/x86.c | 26 +++------ arch/x86/lib/delay.c | 13 ++--- arch/x86/um/asm/barrier.h | 4 +- arch/x86/xen/enlighten.c | 3 -- drivers/input/gameport/gameport.c | 4 +- drivers/input/joystick/analog.c | 4 +- drivers/net/hamradio/baycom_epp.c | 2 +- drivers/staging/media/lirc/lirc_serial.c | 63 ++-------------------- drivers/thermal/intel_powerclamp.c | 4 +- .../power/cpupower/debug/kernel/cpufreq-test_tsc.c | 4 +- 33 files changed, 112 insertions(+), 252 deletions(-)
Signature · 1 line
2.4.2
Andy Lutomirski
maintainerPATCH v2 1/17 x86/tsc: Inline native_read_tsc and remove __native_read_tsc
In cdc7957d1954 ("x86: move native_read_tsc() offline"), native_read_tsc was moved out of line, presumably for some now-obsolete vDSO-related reason. Undo it.
The entire rdtsc, shl, or sequence is only 11 bytes, and calls via rdtscl and similar helpers were already inlined.
Signed-off-by Andy Lutomirski <[email protected]>
--- arch/x86/entry/vdso/vclock_gettime.c | 2 +- arch/x86/include/asm/msr.h | 8 +++----- arch/x86/include/asm/pvclock.h | 2 +- arch/x86/include/asm/stackprotector.h | 2 +- arch/x86/include/asm/tsc.h | 2 +- arch/x86/kernel/apb_timer.c | 4 ++-- arch/x86/kernel/tsc.c | 6 ------ 7 files changed, 9 insertions(+), 17 deletions(-)
arch/x86/entry/vdso/vclock_gettime.c +1 -1
@@ -186,7 +186,7 @@ notrace static cycle_t vread_tsc(void)* but no one has ever seen it happen. */ rdtsc_barrier (); - ret = (cycle_t )__native_read_tsc (); + ret = (cycle_t )native_read_tsc (); last = gtod->cycle_last;
arch/x86/include/asm/msr.h +3 -5
@@ -106,12 +106,10 @@ notrace static inline int native_write_msr_safe(unsigned int msr,return err; } -extern unsigned long long native_read_tsc (void ); -extern int rdmsr_safe_regs (u32 regs[8 ]);extern int wrmsr_safe_regs (u32 regs[8 ]); -static __always_inline unsigned long long __native_read_tsc (void ) +static __always_inline unsigned long long native_read_tsc (void ) {DECLARE_ARGS (val, low, high);
@@ -181,10 +179,10 @@ static inline int rdmsrl_safe(unsigned msr, unsigned long long *p) }#define rdtscl (low) \ - ((low) = (u32)__native_read_tsc()) + ((low) = (u32)native_read_tsc())#define rdtscll (val) \ - ((val) = __native_read_tsc()) + ((val) = native_read_tsc())#define rdpmc (counter, low, high) \ do { \
arch/x86/include/asm/pvclock.h +1 -1
@@ -62,7 +62,7 @@ static inline u64 pvclock_scale_delta(u64 delta, u32 mul_frac, int shift)static __always_inline u64 pvclock_get_nsec_offset(const struct pvclock_vcpu_time_info *src) { -u64 delta =__native_read_tsc () - src->tsc_timestamp; +u64 delta =native_read_tsc () - src->tsc_timestamp;return pvclock_scale_delta (delta, src->tsc_to_system_mul, src->tsc_shift); }
arch/x86/include/asm/stackprotector.h +1 -1
@@ -72,7 +72,7 @@ static __always_inline void boot_init_stack_canary(void)* on during the bootup the random pool has true entropy too. */ get_random_bytes (&canary,sizeof (canary)); - tsc =__native_read_tsc (); + tsc =native_read_tsc (); canary += tsc + (tsc <<32UL ); current->stack_canary = canary;
arch/x86/include/asm/tsc.h +1 -1
@@ -42,7 +42,7 @@ static __always_inline cycles_t vget_cycles(void)if (!cpu_has_tsc)return 0 ;#endif -return (cycles_t )__native_read_tsc (); +return (cycles_t )native_read_tsc (); }extern void tsc_init (void );
arch/x86/kernel/apb_timer.c +2 -2
@@ -390,13 +390,13 @@ unsigned long apbt_quick_calibrate(void) old =dw_apb_clocksource_read (clocksource_apbt); old += loop; - t1 =__native_read_tsc (); + t1 =native_read_tsc ();do { new =dw_apb_clocksource_read (clocksource_apbt); }while (new < old); - t2 =__native_read_tsc (); + t2 =native_read_tsc (); shift =5 ;if (unlikely (loop >> shift ==0 )) {
arch/x86/kernel/tsc.c +0 -6
@@ -308,12 +308,6 @@ unsigned long long sched_clock(void) __attribute__((alias("native_sched_clock")));#endif -unsigned long long native_read_tsc (void ) -{ -return __native_read_tsc (); -} -EXPORT_SYMBOL (native_read_tsc); -int check_tsc_unstable (void ) {return tsc_unstable;
Signature · 1 line
2.4.2
Andy Lutomirski
maintainerPATCH v2 2/17 x86/msr/kvm: Remove vget_cycles()
In reply to Andy LutomirskiThe only caller was kvm's read_tsc. The only difference between vget_cycles and native_read_tsc was that vget_cycles returned zero instead of crashing on TSC-less systems. KVM's already checks vclock_mode before calling that function, so the extra check is unnecessary.
(Off-topic, but the whole KVM clock host implementation is gross.
IMO it should be rewritten.)
Signed-off-by Andy Lutomirski <[email protected]>
--- arch/x86/include/asm/tsc.h | 13 ------------- arch/x86/kvm/x86.c | 2 +- 2 files changed, 1 insertion(+), 14 deletions(-)
arch/x86/include/asm/tsc.h +0 -13
@@ -32,19 +32,6 @@ static inline cycles_t get_cycles(void)return ret; } -static __always_inline cycles_t vget_cycles(void) -{ -/* -* We only do VDSOs on TSC capable CPUs, so this shouldn't -* access boot_cpu_data (which is not VDSO-safe): -*/ -#ifndef CONFIG_X86_TSC -if (!cpu_has_tsc) -return 0 ; -#endif -return (cycles_t )native_read_tsc (); -} -extern void tsc_init (void );extern void mark_tsc_unstable (char *reason);extern int unsynchronized_tsc (void );
arch/x86/kvm/x86.c +1 -1
@@ -1430,7 +1430,7 @@ static cycle_t read_tsc(void)* but no one has ever seen it happen. */ rdtsc_barrier (); - ret = (cycle_t )vget_cycles (); + ret = (cycle_t )native_read_tsc (); last = pvclock_gtod_data.clock.cycle_last;
Signature · 1 line
2.4.2
Andy Lutomirski
maintainerPATCH v2 3/17 x86/tsc/paravirt: Remove the read_tsc and read_tscp paravirt hooks
In reply to Andy LutomirskiWe've had read_tsc and read_tscp paravirt hooks since the very beginning of paravirt, i.e., d3561b7fa0fb ("[PATCH] paravirt: header and stubs for paravirtualisation"). AFAICT the only paravirt guest implementation that ever replaced these calls was vmware, and it's gone. Arguably even vmware shouldn't have hooked rdtsc -- we fully support systems that don't have a TSC at all, so there's no point for a paravirt implementation to pretend that we have a TSC but to replace it.
I also doubt that these hooks actually worked. Calls to rdtscl and rdtscll, which respected the hooks, were used seemingly interchangeably with native_read_tsc, which did not.
Just remove them. If anyone ever needs them again, they can try to make a case for why they need them.
Before, on a paravirt config:
text data bss dec hex filename
13426505 1827056 14508032 29761593 1c62039 vmlinux
After:
text data bss dec hex filename
13426617 1827056 14508032 29761705 1c620a9 vmlinux
Signed-off-by Andy Lutomirski <[email protected]>
--- arch/x86/include/asm/msr.h | 16 ++++++++-------- arch/x86/include/asm/paravirt.h | 34 ---------------------------------- arch/x86/include/asm/paravirt_types.h | 2 -- arch/x86/kernel/paravirt.c | 2 -- arch/x86/kernel/paravirt_patch_32.c | 2 -- arch/x86/xen/enlighten.c | 3 --- 6 files changed, 8 insertions(+), 51 deletions(-)
arch/x86/include/asm/msr.h +8 -8
@@ -178,12 +178,6 @@ static inline int rdmsrl_safe(unsigned msr, unsigned long long *p)return err; } -#define rdtscl (low) \ - ((low) = (u32)native_read_tsc()) - -#define rdtscll (val) \ - ((val) = native_read_tsc()) -#define rdpmc (counter, low, high) \ do { \ u64 _l = native_read_pmc((counter)); \
@@ -193,6 +187,14 @@ do { \#define rdpmcl (counter, val) ((val) = native_read_pmc(counter)) +#endif /* !CONFIG_PARAVIRT */ + +#define rdtscl (low) \ + ((low) = (u32)native_read_tsc()) + +#define rdtscll (val) \ + ((val) = native_read_tsc()) +#define rdtscp (low, high, aux) \ do { \ unsigned long long _val = native_read_tscp(&(aux)); \
@@ -202,8 +204,6 @@ do { \#define rdtscpll (val, aux) (val) = native_read_tscp(&(aux)) -#endif /* !CONFIG_PARAVIRT */ -/* * 64-bit version of wrmsr_safe(): */
arch/x86/include/asm/paravirt.h +0 -34
@@ -174,19 +174,6 @@ static inline int rdmsrl_safe(unsigned msr, unsigned long long *p)return err; } -static inline u64 paravirt_read_tsc (void ) -{ -return PVOP_CALL0 (u64, pv_cpu_ops.read_tsc); -} - -#define rdtscl (low) \ -do { \ - u64 _l = paravirt_read_tsc(); \ - low = (int)_l; \ -} while (0) - -#define rdtscll (val) (val = paravirt_read_tsc()) -static inline unsigned long long paravirt_sched_clock (void ) { return PVOP_CALL0(unsigned long long, pv_time_ops.sched_clock);
@@ -215,27 +202,6 @@ do { \#define rdpmcl (counter, val) ((val) = paravirt_read_pmc(counter)) -static inline unsigned long long paravirt_rdtscp (unsigned int *aux) -{ -return PVOP_CALL1 (u64, pv_cpu_ops.read_tscp, aux); -} - -#define rdtscp (low, high, aux) \ -do { \ - int __aux; \ - unsigned long __val = paravirt_rdtscp(&__aux); \ - (low) = (u32)__val; \ - (high) = (u32)(__val >> 32); \ - (aux) = __aux; \ -} while (0) - -#define rdtscpll (val, aux) \ -do { \ - unsigned long __aux; \ - val = paravirt_rdtscp(&__aux); \ - (aux) = __aux; \ -} while (0) -static inline void paravirt_alloc_ldt (struct desc_struct *ldt,unsigned entries) {PVOP_VCALL2 (pv_cpu_ops.alloc_ldt, ldt, entries);
arch/x86/include/asm/paravirt_types.h +0 -2
@@ -156,9 +156,7 @@ struct pv_cpu_ops {u64 (*read_msr)(unsigned int msr,int *err);int (*write_msr)(unsigned int msr,unsigned low,unsigned high); -u64 (*read_tsc)(void);u64 (*read_pmc)(int counter); -unsigned long long (*read_tscp)(unsigned int *aux);#ifdef CONFIG_X86_32 /*
arch/x86/kernel/paravirt.c +0 -2
@@ -351,9 +351,7 @@ __visible struct pv_cpu_ops pv_cpu_ops = { .wbinvd = native_wbinvd, .read_msr = native_read_msr_safe, .write_msr = native_write_msr_safe, - .read_tsc = native_read_tsc, .read_pmc = native_read_pmc, - .read_tscp = native_read_tscp, .load_tr_desc = native_load_tr_desc, .set_ldt = native_set_ldt, .load_gdt = native_load_gdt,
arch/x86/kernel/paravirt_patch_32.c +0 -2
@@ -10,7 +10,6 @@ DEF_NATIVE(pv_mmu_ops, read_cr2, "mov %cr2, %eax");DEF_NATIVE (pv_mmu_ops, write_cr3,"mov %eax, %cr3" );DEF_NATIVE (pv_mmu_ops, read_cr3,"mov %cr3, %eax" );DEF_NATIVE (pv_cpu_ops, clts,"clts" ); -DEF_NATIVE (pv_cpu_ops, read_tsc,"rdtsc" );#if defined(CONFIG_PARAVIRT_SPINLOCKS ) && defined(CONFIG_QUEUED_SPINLOCKS )DEF_NATIVE (pv_lock_ops, queued_spin_unlock,"movb $0, (%eax)" );
@@ -52,7 +51,6 @@ unsigned native_patch(u8 type, u16 clobbers, void *ibuf,PATCH_SITE (pv_mmu_ops, read_cr3);PATCH_SITE (pv_mmu_ops, write_cr3);PATCH_SITE (pv_cpu_ops, clts); -PATCH_SITE (pv_cpu_ops, read_tsc);#if defined(CONFIG_PARAVIRT_SPINLOCKS ) && defined(CONFIG_QUEUED_SPINLOCKS )case PARAVIRT_PATCH (pv_lock_ops.queued_spin_unlock):if (pv_is_native_spin_unlock ()) {
arch/x86/xen/enlighten.c +0 -3
@@ -1175,11 +1175,8 @@ static const struct pv_cpu_ops xen_cpu_ops __initconst = { .read_msr = xen_read_msr_safe, .write_msr = xen_write_msr_safe, - .read_tsc = native_read_tsc, .read_pmc = native_read_pmc, - .read_tscp = native_read_tscp, - .iret = xen_iret, #ifdef CONFIG_X86_64 .usergs_sysret32 = xen_sysret32,
Signature · 1 line
2.4.2
Andy Lutomirski
maintainerPATCH v2 4/17 x86/tsc: Replace rdtscll with native_read_tsc
In reply to Andy LutomirskiNow that the read_tsc paravirt hook is gone, rdtscll() is just a wrapper around native_read_tsc(). Unwrap it.
Signed-off-by Andy Lutomirski <[email protected]>
--- arch/x86/boot/compressed/aslr.c | 2 +- arch/x86/include/asm/msr.h | 3 --- arch/x86/include/asm/tsc.h | 5 +---- arch/x86/kernel/apb_timer.c | 4 ++-- arch/x86/kernel/apic/apic.c | 8 ++++---- arch/x86/kernel/cpu/mcheck/mce.c | 4 ++-- arch/x86/kernel/espfix_64.c | 2 +- arch/x86/kernel/hpet.c | 4 ++-- arch/x86/kernel/trace_clock.c | 2 +- arch/x86/kernel/tsc.c | 4 ++-- arch/x86/kvm/vmx.c | 2 +- arch/x86/lib/delay.c | 2 +- drivers/thermal/intel_powerclamp.c | 4 ++-- tools/power/cpupower/debug/kernel/cpufreq-test_tsc.c | 4 ++-- 14 files changed, 22 insertions(+), 28 deletions(-)
arch/x86/boot/compressed/aslr.c +1 -1
@@ -82,7 +82,7 @@ static unsigned long get_random_long(void)if (has_cpuflag (X86_FEATURE_TSC )) {debug_putstr (" RDTSC" ); -rdtscll (raw); + raw =native_read_tsc (); random ^= raw; use_i8254 = false;
arch/x86/include/asm/msr.h +0 -3
@@ -192,9 +192,6 @@ do { \#define rdtscl (low) \ ((low) = (u32)native_read_tsc()) -#define rdtscll (val) \ - ((val) = native_read_tsc()) -#define rdtscp (low, high, aux) \ do { \ unsigned long long _val = native_read_tscp(&(aux)); \
arch/x86/include/asm/tsc.h +1 -4
@@ -21,15 +21,12 @@ extern void disable_TSC(void);static inline cycles_t get_cycles (void ) { -unsigned long long ret =0 ; -#ifndef CONFIG_X86_TSC if (!cpu_has_tsc)return 0 ;#endif -rdtscll (ret); -return ret; +return native_read_tsc (); }extern void tsc_init (void );
arch/x86/kernel/apb_timer.c +2 -2
@@ -263,7 +263,7 @@ static int apbt_clocksource_register(void)/* Verify whether apbt counter works */ t1 =dw_apb_clocksource_read (clocksource_apbt); -rdtscll (start); + start =native_read_tsc ();/* * We don't know the TSC frequency yet, but waiting for
@@ -273,7 +273,7 @@ static int apbt_clocksource_register(void)*/ do {rep_nop (); -rdtscll (now); + now =native_read_tsc (); }while ((now - start) <200000UL );/* APBT is the only always on clocksource, it has to work! */
arch/x86/kernel/apic/apic.c +4 -4
@@ -457,7 +457,7 @@ static int lapic_next_deadline(unsigned long delta, {u64 tsc; -rdtscll (tsc); + tsc =native_read_tsc ();wrmsrl (MSR_IA32_TSC_DEADLINE , tsc + (((u64 ) delta) *TSC_DIVISOR ));return 0 ; }
@@ -592,7 +592,7 @@ static void __init lapic_cal_handler(struct clock_event_device *dev)unsigned long pm =acpi_pm_read_early ();if (cpu_has_tsc) -rdtscll (tsc); + tsc =native_read_tsc ();switch (lapic_cal_loops++) {case 0 :
@@ -1209,7 +1209,7 @@ void setup_local_APIC(void)long long max_loops = cpu_khz ? cpu_khz :1000000 ;if (cpu_has_tsc) -rdtscll (tsc); + tsc =native_read_tsc ();if (disable_apic) {disable_ioapic_support ();
@@ -1293,7 +1293,7 @@ void setup_local_APIC(void) }if (queued) {if (cpu_has_tsc && cpu_khz) { -rdtscll (ntsc); + ntsc =native_read_tsc (); max_loops = (cpu_khz <<10 ) - (ntsc - tsc); }else max_loops--;
arch/x86/kernel/cpu/mcheck/mce.c +2 -2
@@ -125,7 +125,7 @@ void mce_setup(struct mce *m) {memset (m,0 ,sizeof (struct mce )); m->cpu = m->extcpu =smp_processor_id (); -rdtscll (m->tsc); + m->tsc =native_read_tsc ();/* We hope get_seconds stays lockless */ m->time =get_seconds (); m->cpuvendor = boot_cpu_data.x86_vendor;
@@ -1784,7 +1784,7 @@ static void collect_tscs(void *data) {unsigned long *cpu_tsc = (unsigned long *)data; -rdtscll (cpu_tsc[smp_processor_id ()]); + cpu_tsc[smp_processor_id ()] =native_read_tsc (); }static int mce_apei_read_done;
arch/x86/kernel/espfix_64.c +1 -1
@@ -110,7 +110,7 @@ static void init_espfix_random(void)*/ if (!arch_get_random_long (&rand)) {/* The constant is an arbitrary large prime */ -rdtscll (rand); + rand =native_read_tsc (); rand *=0xc345c6b72fd16123UL ; }
arch/x86/kernel/hpet.c +2 -2
@@ -734,7 +734,7 @@ static int hpet_clocksource_register(void)/* Verify whether hpet counter works */ t1 =hpet_readl (HPET_COUNTER ); -rdtscll (start); + start =native_read_tsc ();/* * We don't know the TSC frequency yet, but waiting for
@@ -744,7 +744,7 @@ static int hpet_clocksource_register(void)*/ do {rep_nop (); -rdtscll (now); + now =native_read_tsc (); }while ((now - start) <200000UL );if (t1 ==hpet_readl (HPET_COUNTER )) {
arch/x86/kernel/trace_clock.c +1 -1
@@ -15,7 +15,7 @@ u64 notrace trace_clock_x86_tsc(void)u64 ret;rdtsc_barrier (); -rdtscll (ret); + ret =native_read_tsc ();return ret; }
arch/x86/kernel/tsc.c +2 -2
@@ -248,7 +248,7 @@ static void set_cyc2ns_scale(unsigned long cpu_khz, int cpu) data =cyc2ns_write_begin (cpu); -rdtscll (tsc_now); + tsc_now =native_read_tsc (); ns_now =cycles_2_ns (tsc_now);/*
@@ -290,7 +290,7 @@ u64 native_sched_clock(void) }/* read the Time Stamp Counter: */ -rdtscll (tsc_now); + tsc_now =native_read_tsc ();/* return the value in ns */ return cycles_2_ns (tsc_now);
arch/x86/kvm/vmx.c +1 -1
@@ -2236,7 +2236,7 @@ static u64 guest_read_tsc(void) {u64 host_tsc, tsc_offset; -rdtscll (host_tsc); + host_tsc =native_read_tsc (); tsc_offset =vmcs_read64 (TSC_OFFSET );return host_tsc + tsc_offset; }
arch/x86/lib/delay.c +1 -1
@@ -100,7 +100,7 @@ void use_tsc_delay(void)int read_current_timer (unsigned long *timer_val) {if (delay_fn == delay_tsc) { -rdtscll (*timer_val); + *timer_val =native_read_tsc ();return 0 ; }return -1 ;
drivers/thermal/intel_powerclamp.c +2 -2
@@ -340,7 +340,7 @@ static bool powerclamp_adjust_controls(unsigned int target_ratio,/* check result for the last window */ msr_now =pkg_state_counter (); -rdtscll (tsc_now); + tsc_now =native_read_tsc ();/* calculate pkg cstate vs tsc ratio */ if (!msr_last || !tsc_last)
@@ -482,7 +482,7 @@ static void poll_pkg_cstate(struct work_struct *dummy)u64 val64; msr_now =pkg_state_counter (); -rdtscll (tsc_now); + tsc_now =native_read_tsc (); jiffies_now = jiffies;/* calculate pkg cstate vs tsc ratio */
tools/power/cpupower/debug/kernel/cpufreq-test_tsc.c +2 -2
@@ -81,11 +81,11 @@ static int __init cpufreq_test_tsc(void)printk (KERN_DEBUG "start--> \n" ); then =read_pmtmr (); -rdtscll (then_tsc); + then_tsc =native_read_tsc ();for (i=0 ;i<20 ;i++) {mdelay (100 ); now =read_pmtmr (); -rdtscll (now_tsc); + now_tsc =native_read_tsc (); diff = (now - then) &0xFFFFFF ; diff_tsc = now_tsc - then_tsc;printk (KERN_DEBUG "t1: %08u t2: %08u diff_pmtmr: %08u diff_tsc: %016llu\n" , then, now, diff, diff_tsc);
Signature · 1 line
2.4.2
Andy Lutomirski
maintainerPATCH v2 5/17 x86/tsc: Remove the rdtscp and rdtscpll macros
In reply to Andy LutomirskiThey have no users. Leave native_read_tscp, which seems potentially useful despite also having no callers.
Signed-off-by Andy Lutomirski <[email protected]>
--- arch/x86/include/asm/msr.h | 9 --------- 1 file changed, 9 deletions(-)
arch/x86/include/asm/msr.h +0 -9
@@ -192,15 +192,6 @@ do { \#define rdtscl (low) \ ((low) = (u32)native_read_tsc()) -#define rdtscp (low, high, aux) \ -do { \ - unsigned long long _val = native_read_tscp(&(aux)); \ - (low) = (u32)_val; \ - (high) = (u32)(_val >> 32); \ -} while (0) - -#define rdtscpll (val, aux) (val) = native_read_tscp(&(aux)) -/* * 64-bit version of wrmsr_safe(): */
Signature · 1 line
2.4.2
Andy Lutomirski
maintainerPATCH v2 6/17 x86/tsc: Use the full 64-bit tsc in tsc_delay
In reply to Andy LutomirskiAs a very minor optimization, tsc_delay was only using the low 32 bits of the TSC. It's a delay function, so just use the whole thing.
Signed-off-by Andy Lutomirski <[email protected]>
--- arch/x86/lib/delay.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
arch/x86/lib/delay.c +4 -4
@@ -49,16 +49,16 @@ static void delay_loop(unsigned long loops)/* TSC based delay: */ static void delay_tsc (unsigned long __loops) { -u32 bclock, now, loops = __loops; +u64 bclock, now, loops = __loops;int cpu;preempt_disable (); cpu =smp_processor_id ();rdtsc_barrier (); -rdtscl (bclock); + bclock =native_read_tsc ();for (;;) {rdtsc_barrier (); -rdtscl (now); + now =native_read_tsc ();if ((now - bclock) >= loops)break ;
@@ -80,7 +80,7 @@ static void delay_tsc(unsigned long __loops) loops -= (now - bclock); cpu =smp_processor_id ();rdtsc_barrier (); -rdtscl (bclock); + bclock =native_read_tsc (); } }preempt_enable ();
Signature · 1 line
2.4.2
Andy Lutomirski
maintainerPATCH v2 7/17 x86/cpu/amd: Use the full 64-bit TSC to detect the 2.6.2 bug
In reply to Andy LutomirskiThis code is timing 100k indirect calls, so the added overhead of counting the number of cycles elapsed as a 64-bit number should be insignificant. Drop the optimization of using a 32-bit count.
Signed-off-by Andy Lutomirski <[email protected]>
--- arch/x86/kernel/cpu/amd.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
arch/x86/kernel/cpu/amd.c +3 -3
@@ -107,7 +107,7 @@ static void init_amd_k6(struct cpuinfo_x86 *c)const int K6_BUG_LOOP =1000000 ;int n;void (*f_vide)(void ); -unsigned long d, d2; +u64 d, d2;printk (KERN_INFO "AMD K6 stepping B detected - " );
@@ -118,10 +118,10 @@ static void init_amd_k6(struct cpuinfo_x86 *c) n =K6_BUG_LOOP ; f_vide = vide; -rdtscl (d); + d =native_read_tsc ();while (n--)f_vide (); -rdtscl (d2); + d2 =native_read_tsc (); d = d2-d;if (d >20 *K6_BUG_LOOP )
Signature · 1 line
2.4.2
Andy Lutomirski
maintainerPATCH v2 8/17 baycom_epp: Replace rdtscl() with native_read_tsc()
In reply to Andy LutomirskiThis is only used if BAYCOM_DEBUG is defined.
Cc Thomas Sailer <[email protected]>
Signed-off-by Andy Lutomirski <[email protected]>
--- drivers/net/hamradio/baycom_epp.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/net/hamradio/baycom_epp.c +1 -1
@@ -638,7 +638,7 @@ static int receive(struct net_device *dev, int cnt)#define GETTICK (x) \ ({ \ if (cpu_has_tsc) \ - rdtscl(x); \ + x = (unsigned int)native_read_tsc(); \ })#else /* __i386__ */ #define GETTICK (x)
Signature · 1 line
2.4.2
Re: [PATCH v2 08/17] baycom_epp: Replace rdtscl() with native_read_tsc()
Hello,
please add a line what is the difference between V1 and V2.
re,
wh
Am 13.06.2015 01:44, schrieb Andy Lutomirski:
Quoted 22 lines
This is only used if
BAYCOM_DEBUGis defined.Cc Thomas Sailer <[email protected]>Signed-off-by Andy Lutomirski <[email protected]>--- drivers/net/hamradio/baycom_epp.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)drivers/net/hamradio/baycom_epp.c +1
-1@@ -638,7 +638,7 @@ static int receive(struct net_device *dev, int cnt)#define GETTICK (x) \ ({ \ if (cpu_has_tsc) \ - rdtscl(x); \ + x = (unsigned int)native_read_tsc(); \ })#else /* __i386__ */ #define GETTICK (x)
Re: [PATCH v2 08/17] baycom_epp: Replace rdtscl() with native_read_tsc()
Quoted 3 lines · walter harms
Hello,
please add a line what is the difference between V1 and V2.
There is none. v1 was screwed up elsewhere in the series.
This is part of a larger series to clean up the rdtsc inlines and macros. I'm hoping for an ack so this can go in through -tip.
Thanks,
Andy
Quoted 26 lines
re,
whAm 13.06.2015 01:44, schrieb Andy Lutomirski:
Quoted 22 lines
This is only used if
BAYCOM_DEBUGis defined.Cc Thomas Sailer <[email protected]>Signed-off-by Andy Lutomirski <[email protected]>--- drivers/net/hamradio/baycom_epp.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)drivers/net/hamradio/baycom_epp.c +1
-1@@ -638,7 +638,7 @@ static int receive(struct net_device *dev, int cnt)#define GETTICK (x) \ ({ \ if (cpu_has_tsc) \ - rdtscl(x); \ + x = (unsigned int)native_read_tsc(); \ })#else /* __i386__ */ #define GETTICK (x)
Signature · 2 lines
Andy Lutomirski AMA Capital Management, LLC
Andy Lutomirski
maintainerPATCH v2 9/17 staging/lirc_serial: Remove TSC-based timing
In reply to Andy LutomirskiIt wasn't compiled in by default. I suspect that the driver was and still is broken, though -- it's calling udelay with a parameter that's derived from loops_per_jiffy.
Cc Jarod Wilson <[email protected]>
Cc Greg Kroah-Hartman <[email protected]>
Signed-off-by Andy Lutomirski <[email protected]>
--- drivers/staging/media/lirc/lirc_serial.c | 63 ++------------------------------ 1 file changed, 4 insertions(+), 59 deletions(-)
drivers/staging/media/lirc/lirc_serial.c +4 -59
@@ -327,9 +327,6 @@ static void safe_udelay(unsigned long usecs)* time */ -/* So send_pulse can quickly convert microseconds to clocks */ -static unsigned long conv_us_to_clocks; -static int init_timing_params (unsigned int new_duty_cycle,unsigned int new_freq) {
@@ -344,7 +341,6 @@ static int init_timing_params(unsigned int new_duty_cycle,/* How many clocks in a microsecond?, avoiding long long divide */ work = loops_per_sec; work *=4295 ;/* 4295 = 2^32 / 1e6 */ - conv_us_to_clocks = work >>32 ;/* * Carrier period in clocks, approach good up to 32GHz clock,
@@ -357,10 +353,9 @@ static int init_timing_params(unsigned int new_duty_cycle, pulse_width = period * duty_cycle /100 ; space_width = period - pulse_width;dprintk ("in init_timing_params, freq=%d, duty_cycle=%d, " -"clk/jiffy=%ld, pulse=%ld, space=%ld, " -"conv_us_to_clocks=%ld\n" , +"clk/jiffy=%ld, pulse=%ld, space=%ld\n" , freq, duty_cycle,__this_cpu_read (cpu_info.loops_per_jiffy), - pulse_width, space_width, conv_us_to_clocks); + pulse_width, space_width); return 0; }#else /* ! USE_RDTSC */
@@ -431,63 +426,14 @@ static long send_pulse_irdeo(unsigned long length)return ret; } -#ifdef USE_RDTSC -/* Version that uses Pentium rdtsc instruction to measure clocks */ - -/* -* This version does sub-microsecond timing using rdtsc instruction, -* and does away with the fudged LIRC_SERIAL_TRANSMITTER_LATENCY -* Implicitly i586 architecture... - Steve -*/ - -static long send_pulse_homebrew_softcarrier (unsigned long length) -{ -int flag; -unsigned long target, start, now; - -/* Get going quick as we can */ -rdtscl (start); -on (); -/* Convert length from microseconds to clocks */ - length *= conv_us_to_clocks; -/* And loop till time is up - flipping at right intervals */ - now = start; - target = pulse_width; - flag =1 ; -/* -* FIXME: This looks like a hard busy wait, without even an occasional, -* polite, cpu_relax() call. There's got to be a better way? -* -* The i2c code has the result of a lot of bit-banging work, I wonder if -* there's something there which could be helpful here. -*/ -while ((now - start) < length) { -/* Delay till flip time */ -do { -rdtscl (now); - }while ((now - start) < target); - -/* flip */ -if (flag) { -rdtscl (now); -off (); - target += space_width; - }else { -rdtscl (now);on (); - target += pulse_width; - } - flag = !flag; - } -rdtscl (now); -return ((now - start) - length) / conv_us_to_clocks; -} -#else /* ! USE_RDTSC */ /* Version using udelay() */ /* * here we use fixed point arithmetic, with 8 * fractional bits. that gets us within 0.1% or so of the right average * frequency, albeit with some jitter in pulse length - Steve +* +* This should use ndelay instead. */ /* To match 8 fractional bits used for pulse/space length */
@@ -520,7 +466,6 @@ static long send_pulse_homebrew_softcarrier(unsigned long length) }return (actual-length) >>8 ; } -#endif /* USE_RDTSC */ static long send_pulse_homebrew (unsigned long length) {
Signature · 1 line
2.4.2
Re: [PATCH v2 09/17] staging/lirc_serial: Remove TSC-based timing
This one should go to Mauro and [email protected] instead of Greg and [email protected].
regards,
dan carpenter
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 09/17] staging/lirc_serial: Remove TSC-based timing
In reply to Andy LutomirskiHi Mauro, etc:
Are you okay with this change landing in the tip tree?
--Andy
Quoted 124 lines · Andy Lutomirski
It wasn't compiled in by default. I suspect that the driver was and still is broken, though -- it's calling udelay with a parameter that's derived from
loops_per_jiffy.Cc Jarod Wilson <[email protected]>Cc Greg Kroah-Hartman <[email protected]>Signed-off-by Andy Lutomirski <[email protected]>--- drivers/staging/media/lirc/lirc_serial.c | 63 ++------------------------------ 1 file changed, 4 insertions(+), 59 deletions(-)drivers/staging/media/lirc/lirc_serial.c +4
-60@@ -327,9 +327,6 @@ static void safe_udelay(unsigned long usecs)* time */ -/* So send_pulse can quickly convert microseconds to clocks */ -static unsigned long conv_us_to_clocks; -static int init_timing_params (unsigned int new_duty_cycle,unsigned int new_freq) {@@ -344,7 +341,6 @@ static int init_timing_params(unsigned int new_duty_cycle,/* How many clocks in a microsecond?, avoiding long long divide */ work = loops_per_sec; work *=4295 ;/* 4295 = 2^32 / 1e6 */ - conv_us_to_clocks = work >>32 ;/* * Carrier period in clocks, approach good up to 32GHz clock, @@ -357,10 +353,9 @@ static int init_timing_params(unsigned int new_duty_cycle, pulse_width = period * duty_cycle /100 ; space_width = period - pulse_width;dprintk ("in init_timing_params, freq=%d, duty_cycle=%d, " -"clk/jiffy=%ld, pulse=%ld, space=%ld, " -"conv_us_to_clocks=%ld\n" , +"clk/jiffy=%ld, pulse=%ld, space=%ld\n" , freq, duty_cycle,__this_cpu_read (cpu_info.loops_per_jiffy), - pulse_width, space_width, conv_us_to_clocks); + pulse_width, space_width); return 0; }#else /* ! USE_RDTSC */ @@ -431,63 +426,14 @@ static long send_pulse_irdeo(unsigned long length)return ret; } -#ifdef USE_RDTSC -/* Version that uses Pentium rdtsc instruction to measure clocks */ - -/* -* This version does sub-microsecond timing using rdtsc instruction, -* and does away with the fudged LIRC_SERIAL_TRANSMITTER_LATENCY -* Implicitly i586 architecture... - Steve -*/ - -static long send_pulse_homebrew_softcarrier (unsigned long length) -{ -int flag; -unsigned long target, start, now; - -/* Get going quick as we can */ -rdtscl (start); -on (); -/* Convert length from microseconds to clocks */ - length *= conv_us_to_clocks; -/* And loop till time is up - flipping at right intervals */ - now = start; - target = pulse_width; - flag =1 ; -/* -* FIXME: This looks like a hard busy wait, without even an occasional, -* polite, cpu_relax() call. There's got to be a better way? -* -* The i2c code has the result of a lot of bit-banging work, I wonder if -* there's something there which could be helpful here. -*/ -while ((now - start) < length) { -/* Delay till flip time */ -do { -rdtscl (now); - }while ((now - start) < target); - -/* flip */ -if (flag) { -rdtscl (now); -off (); - target += space_width; - }else { -rdtscl (now);on (); - target += pulse_width; - } - flag = !flag; - } -rdtscl (now); -return ((now - start) - length) / conv_us_to_clocks; -} -#else /* ! USE_RDTSC */ /* Version using udelay() */ /* * here we use fixed point arithmetic, with 8 * fractional bits. that gets us within 0.1% or so of the right average * frequency, albeit with some jitter in pulse length - Steve +* +* This should use ndelay instead. */ /* To match 8 fractional bits used for pulse/space length */ @@ -520,7 +466,6 @@ static long send_pulse_homebrew_softcarrier(unsigned long length) }return (actual-length) >>8 ; } -#endif /* USE_RDTSC */ static long send_pulse_homebrew (unsigned long length) { --2.4.2
Signature · 4 lines
Andy Lutomirski AMA Capital Management, LLC -- To unsubscribe from this list: send the line "unsubscribe linux-media" in
Andy Lutomirski
maintainerPATCH v2 10/17 input/joystick/analog: Switch from rdtscl() to native_read_tsc()
In reply to Andy LutomirskiThis timing code is hideous, and this doesn't help. It gets rid of one of the last users of rdtscl, though.
Cc Dmitry Torokhov <[email protected]>
Signed-off-by Andy Lutomirski <[email protected]>
--- drivers/input/joystick/analog.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/input/joystick/analog.c +2 -2
@@ -143,7 +143,7 @@ struct analog_port {#include <linux/i8253.h> -#define GET_TIME (x) do { if (cpu_has_tsc) rdtscl(x); else x = get_time_pit(); } while (0) +#define GET_TIME (x) do { if (cpu_has_tsc) x = (unsigned int)native_read_tsc(); else x = get_time_pit(); } while (0)#define DELTA (x,y) (cpu_has_tsc ? ((y) - (x)) : ((x) - (y) + ((x) < (y) ? PIT_TICK_RATE / HZ : 0)))#define TIME_NAME (cpu_has_tsc?"TSC":"PIT")static unsigned int get_time_pit (void )
@@ -160,7 +160,7 @@ static unsigned int get_time_pit(void) return count; }#elif defined(__x86_64__) -#define GET_TIME (x) rdtscl(x) +#define GET_TIME (x) do { x = (unsigned int)native_read_tsc(); } while (0)#define DELTA (x,y) ((y)-(x))#define TIME_NAME "TSC"#elif defined(__alpha__) || defined(CONFIG_MN10300) || defined(CONFIG_ARM) || defined(CONFIG_ARM64) || defined(CONFIG_TILE)
Signature · 1 line
2.4.2
Re: [PATCH v2 10/17] input/joystick/analog: Switch from rdtscl() to native_read_tsc()
Quoted 6 lines · Andy Lutomirski
This timing code is hideous, and this doesn't help. It gets rid of one of the last users of rdtscl, though.
Cc Dmitry Torokhov <[email protected]>Signed-off-by Andy Lutomirski <[email protected]>
Acked-by Dmitry Torokhov <[email protected]>
Quoted 29 lines
--- drivers/input/joystick/analog.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)drivers/input/joystick/analog.c +2
-2@@ -143,7 +143,7 @@ struct analog_port {#include <linux/i8253.h> -#define GET_TIME (x) do { if (cpu_has_tsc) rdtscl(x); else x = get_time_pit(); } while (0) +#define GET_TIME (x) do { if (cpu_has_tsc) x = (unsigned int)native_read_tsc(); else x = get_time_pit(); } while (0)#define DELTA (x,y) (cpu_has_tsc ? ((y) - (x)) : ((x) - (y) + ((x) < (y) ? PIT_TICK_RATE / HZ : 0)))#define TIME_NAME (cpu_has_tsc?"TSC":"PIT")static unsigned int get_time_pit (void )@@ -160,7 +160,7 @@ static unsigned int get_time_pit(void) return count; }#elif defined(__x86_64__) -#define GET_TIME (x) rdtscl(x) +#define GET_TIME (x) do { x = (unsigned int)native_read_tsc(); } while (0)#define DELTA (x,y) ((y)-(x))#define TIME_NAME "TSC"#elif defined(__alpha__) || defined(CONFIG_MN10300) || defined(CONFIG_ARM) || defined(CONFIG_ARM64) || defined(CONFIG_TILE)Signature · 1 line
2.4.2
Signature · 1 line
Dmitry
Re: [PATCH v2 10/17] input/joystick/analog: Switch from rdtscl() to native_read_tsc()
Is it okay for -tip to take this and the other one you just acked?
Thanks,
Andy
Quoted 42 lines · Dmitry Torokhov
Quoted 6 lines · Andy Lutomirski
This timing code is hideous, and this doesn't help. It gets rid of one of the last users of rdtscl, though.
Cc Dmitry Torokhov <[email protected]>Signed-off-by Andy Lutomirski <[email protected]>Acked-by Dmitry Torokhov <[email protected]>Quoted 27 lines
--- drivers/input/joystick/analog.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)drivers/input/joystick/analog.c +2
-3@@ -143,7 +143,7 @@ struct analog_port {#include <linux/i8253.h> -#define GET_TIME (x) do { if (cpu_has_tsc) rdtscl(x); else x = get_time_pit(); } while (0) +#define GET_TIME (x) do { if (cpu_has_tsc) x = (unsigned int)native_read_tsc(); else x = get_time_pit(); } while (0)#define DELTA (x,y) (cpu_has_tsc ? ((y) - (x)) : ((x) - (y) + ((x) < (y) ? PIT_TICK_RATE / HZ : 0)))#define TIME_NAME (cpu_has_tsc?"TSC":"PIT")static unsigned int get_time_pit (void )@@ -160,7 +160,7 @@ static unsigned int get_time_pit(void) return count; }#elif defined(__x86_64__) -#define GET_TIME (x) rdtscl(x) +#define GET_TIME (x) do { x = (unsigned int)native_read_tsc(); } while (0)#define DELTA (x,y) ((y)-(x))#define TIME_NAME "TSC"#elif defined(__alpha__) || defined(CONFIG_MN10300) || defined(CONFIG_ARM) || defined(CONFIG_ARM64) || defined(CONFIG_TILE) --Quoted 2 lines
2.4.2
--
Dmitry
Signature · 2 lines
Andy Lutomirski AMA Capital Management, LLC
Dmitry Torokhov
maintainerRe: [PATCH v2 10/17] input/joystick/analog: Switch from rdtscl() to native_read_tsc()
Quoted 1 line · Andy Lutomirski
Is it okay for -tip to take this and the other one you just acked?
Yeah, that's fine.
Thanks.
Signature · 1 line
Dmitry
Andy Lutomirski
maintainerPATCH v2 11/17 drivers/input/gameport: Replace rdtscl() with native_read_tsc()
In reply to Andy LutomirskiIt's unclear to me why this code exists in the first place.
Cc Dmitry Torokhov <[email protected]>
Signed-off-by Andy Lutomirski <[email protected]>
--- drivers/input/gameport/gameport.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/input/gameport/gameport.c +2 -2
@@ -149,9 +149,9 @@ static int old_gameport_measure_speed(struct gameport *gameport)for (i =0 ; i <50 ; i++) {local_irq_save (flags); -rdtscl (t1); + t1 =native_read_tsc ();for (t =0 ; t <50 ; t++)gameport_read (gameport); -rdtscl (t2); + t2 =native_read_tsc ();local_irq_restore (flags);udelay (i *10 );if (t2 - t1 < tx) tx = t2 - t1;
Signature · 1 line
2.4.2
Re: [PATCH v2 11/17] drivers/input/gameport: Replace rdtscl() with native_read_tsc()
Quoted 5 lines · Andy Lutomirski
It's unclear to me why this code exists in the first place.
Cc Dmitry Torokhov <[email protected]>Signed-off-by Andy Lutomirski <[email protected]>
Acked-by Dmitry Torokhov <[email protected]>
Quoted 23 lines
--- drivers/input/gameport/gameport.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)drivers/input/gameport/gameport.c +2
-2@@ -149,9 +149,9 @@ static int old_gameport_measure_speed(struct gameport *gameport)for (i =0 ; i <50 ; i++) {local_irq_save (flags); -rdtscl (t1); + t1 =native_read_tsc ();for (t =0 ; t <50 ; t++)gameport_read (gameport); -rdtscl (t2); + t2 =native_read_tsc ();local_irq_restore (flags);udelay (i *10 );if (t2 - t1 < tx) tx = t2 - t1;Signature · 1 line
2.4.2
Signature · 1 line
Dmitry
Andy Lutomirski
maintainerPATCH v2 12/17 x86/tsc: Remove rdtscl()
In reply to Andy LutomirskiIt has no more callers, and it was never a very sensible interface to begin with. Users of the TSC should either read all 64 bits or explicitly throw out the high bits.
Signed-off-by Andy Lutomirski <[email protected]>
--- arch/x86/include/asm/msr.h | 3 --- 1 file changed, 3 deletions(-)
arch/x86/include/asm/msr.h +0 -3
@@ -189,9 +189,6 @@ do { \#endif /* !CONFIG_PARAVIRT */ -#define rdtscl (low) \ - ((low) = (u32)native_read_tsc()) -/* * 64-bit version of wrmsr_safe(): */
Signature · 1 line
2.4.2
Andy Lutomirski
maintainerPATCH v2 13/17 x86/tsc: Rename native_read_tsc() to rdtsc_unordered()
In reply to Andy LutomirskiNow that there is no paravirt TSC, the "native" is inappropriate. The fact that rdtsc is not ordered can catch people by surprise, so call it rdtsc_unordered().
Signed-off-by Andy Lutomirski <[email protected]>
--- arch/x86/boot/compressed/aslr.c | 2 +- arch/x86/entry/vdso/vclock_gettime.c | 2 +- arch/x86/include/asm/msr.h | 11 ++++++++++- arch/x86/include/asm/pvclock.h | 2 +- arch/x86/include/asm/stackprotector.h | 2 +- arch/x86/include/asm/tsc.h | 2 +- arch/x86/kernel/apb_timer.c | 8 ++++---- arch/x86/kernel/apic/apic.c | 8 ++++---- arch/x86/kernel/cpu/amd.c | 4 ++-- arch/x86/kernel/cpu/mcheck/mce.c | 4 ++-- arch/x86/kernel/espfix_64.c | 2 +- arch/x86/kernel/hpet.c | 4 ++-- arch/x86/kernel/trace_clock.c | 2 +- arch/x86/kernel/tsc.c | 4 ++-- arch/x86/kvm/lapic.c | 4 ++-- arch/x86/kvm/svm.c | 4 ++-- arch/x86/kvm/vmx.c | 4 ++-- arch/x86/kvm/x86.c | 12 ++++++------ arch/x86/lib/delay.c | 8 ++++---- drivers/input/gameport/gameport.c | 4 ++-- drivers/input/joystick/analog.c | 4 ++-- drivers/net/hamradio/baycom_epp.c | 2 +- drivers/thermal/intel_powerclamp.c | 4 ++-- tools/power/cpupower/debug/kernel/cpufreq-test_tsc.c | 4 ++-- 24 files changed, 58 insertions(+), 49 deletions(-)
arch/x86/boot/compressed/aslr.c +1 -1
@@ -82,7 +82,7 @@ static unsigned long get_random_long(void)if (has_cpuflag (X86_FEATURE_TSC )) {debug_putstr (" RDTSC" ); - raw =native_read_tsc (); + raw =rdtsc_unordered (); random ^= raw; use_i8254 = false;
arch/x86/entry/vdso/vclock_gettime.c +1 -1
@@ -186,7 +186,7 @@ notrace static cycle_t vread_tsc(void)* but no one has ever seen it happen. */ rdtsc_barrier (); - ret = (cycle_t )native_read_tsc (); + ret = (cycle_t )rdtsc_unordered (); last = gtod->cycle_last;
arch/x86/include/asm/msr.h +10 -1
@@ -109,7 +109,16 @@ notrace static inline int native_write_msr_safe(unsigned int msr,extern int rdmsr_safe_regs (u32 regs[8 ]);extern int wrmsr_safe_regs (u32 regs[8 ]); -static __always_inline unsigned long long native_read_tsc (void ) +/** +* rdtsc_unordered() - returns the current TSC without ordering constraints +* +* rdtsc_unordered() returns the result of RDTSC as a 64-bit integer. The +* only ordering constraint it supplies is the ordering implied by +* "asm volatile": it will put the RDTSC in the place you expect. The +* CPU can and will speculatively execute that RDTSC, though, so the +* results can be non-monotonic if compared on different CPUs. +*/ +static __always_inline unsigned long long rdtsc_unordered (void ) {DECLARE_ARGS (val, low, high);
arch/x86/include/asm/pvclock.h +1 -1
@@ -62,7 +62,7 @@ static inline u64 pvclock_scale_delta(u64 delta, u32 mul_frac, int shift)static __always_inline u64 pvclock_get_nsec_offset(const struct pvclock_vcpu_time_info *src) { -u64 delta =native_read_tsc () - src->tsc_timestamp; +u64 delta =rdtsc_unordered () - src->tsc_timestamp;return pvclock_scale_delta (delta, src->tsc_to_system_mul, src->tsc_shift); }
arch/x86/include/asm/stackprotector.h +1 -1
@@ -72,7 +72,7 @@ static __always_inline void boot_init_stack_canary(void)* on during the bootup the random pool has true entropy too. */ get_random_bytes (&canary,sizeof (canary)); - tsc =native_read_tsc (); + tsc =rdtsc_unordered (); canary += tsc + (tsc <<32UL ); current->stack_canary = canary;
arch/x86/include/asm/tsc.h +1 -1
@@ -26,7 +26,7 @@ static inline cycles_t get_cycles(void)return 0 ;#endif -return native_read_tsc (); +return rdtsc_unordered (); }extern void tsc_init (void );
arch/x86/kernel/apb_timer.c +4 -4
@@ -263,7 +263,7 @@ static int apbt_clocksource_register(void)/* Verify whether apbt counter works */ t1 =dw_apb_clocksource_read (clocksource_apbt); - start =native_read_tsc (); + start =rdtsc_unordered ();/* * We don't know the TSC frequency yet, but waiting for
@@ -273,7 +273,7 @@ static int apbt_clocksource_register(void)*/ do {rep_nop (); - now =native_read_tsc (); + now =rdtsc_unordered (); }while ((now - start) <200000UL );/* APBT is the only always on clocksource, it has to work! */
@@ -390,13 +390,13 @@ unsigned long apbt_quick_calibrate(void) old =dw_apb_clocksource_read (clocksource_apbt); old += loop; - t1 =native_read_tsc (); + t1 =rdtsc_unordered ();do { new =dw_apb_clocksource_read (clocksource_apbt); }while (new < old); - t2 =native_read_tsc (); + t2 =rdtsc_unordered (); shift =5 ;if (unlikely (loop >> shift ==0 )) {
arch/x86/kernel/apic/apic.c +4 -4
@@ -457,7 +457,7 @@ static int lapic_next_deadline(unsigned long delta, {u64 tsc; - tsc =native_read_tsc (); + tsc =rdtsc_unordered ();wrmsrl (MSR_IA32_TSC_DEADLINE , tsc + (((u64 ) delta) *TSC_DIVISOR ));return 0 ; }
@@ -592,7 +592,7 @@ static void __init lapic_cal_handler(struct clock_event_device *dev)unsigned long pm =acpi_pm_read_early ();if (cpu_has_tsc) - tsc =native_read_tsc (); + tsc =rdtsc_unordered ();switch (lapic_cal_loops++) {case 0 :
@@ -1209,7 +1209,7 @@ void setup_local_APIC(void)long long max_loops = cpu_khz ? cpu_khz :1000000 ;if (cpu_has_tsc) - tsc =native_read_tsc (); + tsc =rdtsc_unordered ();if (disable_apic) {disable_ioapic_support ();
@@ -1293,7 +1293,7 @@ void setup_local_APIC(void) }if (queued) {if (cpu_has_tsc && cpu_khz) { - ntsc =native_read_tsc (); + ntsc =rdtsc_unordered (); max_loops = (cpu_khz <<10 ) - (ntsc - tsc); }else max_loops--;
arch/x86/kernel/cpu/amd.c +2 -2
@@ -118,10 +118,10 @@ static void init_amd_k6(struct cpuinfo_x86 *c) n =K6_BUG_LOOP ; f_vide = vide; - d =native_read_tsc (); + d =rdtsc_unordered ();while (n--)f_vide (); - d2 =native_read_tsc (); + d2 =rdtsc_unordered (); d = d2-d;if (d >20 *K6_BUG_LOOP )
arch/x86/kernel/cpu/mcheck/mce.c +2 -2
@@ -125,7 +125,7 @@ void mce_setup(struct mce *m) {memset (m,0 ,sizeof (struct mce )); m->cpu = m->extcpu =smp_processor_id (); - m->tsc =native_read_tsc (); + m->tsc =rdtsc_unordered ();/* We hope get_seconds stays lockless */ m->time =get_seconds (); m->cpuvendor = boot_cpu_data.x86_vendor;
@@ -1784,7 +1784,7 @@ static void collect_tscs(void *data) {unsigned long *cpu_tsc = (unsigned long *)data; - cpu_tsc[smp_processor_id ()] =native_read_tsc (); + cpu_tsc[smp_processor_id ()] =rdtsc_unordered (); }static int mce_apei_read_done;
arch/x86/kernel/espfix_64.c +1 -1
@@ -110,7 +110,7 @@ static void init_espfix_random(void)*/ if (!arch_get_random_long (&rand)) {/* The constant is an arbitrary large prime */ - rand =native_read_tsc (); + rand =rdtsc_unordered (); rand *=0xc345c6b72fd16123UL ; }
arch/x86/kernel/hpet.c +2 -2
@@ -734,7 +734,7 @@ static int hpet_clocksource_register(void)/* Verify whether hpet counter works */ t1 =hpet_readl (HPET_COUNTER ); - start =native_read_tsc (); + start =rdtsc_unordered ();/* * We don't know the TSC frequency yet, but waiting for
@@ -744,7 +744,7 @@ static int hpet_clocksource_register(void)*/ do {rep_nop (); - now =native_read_tsc (); + now =rdtsc_unordered (); }while ((now - start) <200000UL );if (t1 ==hpet_readl (HPET_COUNTER )) {
arch/x86/kernel/trace_clock.c +1 -1
@@ -15,7 +15,7 @@ u64 notrace trace_clock_x86_tsc(void)u64 ret;rdtsc_barrier (); - ret =native_read_tsc (); + ret =rdtsc_unordered ();return ret; }
arch/x86/kernel/tsc.c +2 -2
@@ -248,7 +248,7 @@ static void set_cyc2ns_scale(unsigned long cpu_khz, int cpu) data =cyc2ns_write_begin (cpu); - tsc_now =native_read_tsc (); + tsc_now =rdtsc_unordered (); ns_now =cycles_2_ns (tsc_now);/*
@@ -290,7 +290,7 @@ u64 native_sched_clock(void) }/* read the Time Stamp Counter: */ - tsc_now =native_read_tsc (); + tsc_now =rdtsc_unordered ();/* return the value in ns */ return cycles_2_ns (tsc_now);
arch/x86/kvm/lapic.c +2 -2
@@ -1148,7 +1148,7 @@ void wait_lapic_expire(struct kvm_vcpu *vcpu) tsc_deadline = apic->lapic_timer.expired_tscdeadline; apic->lapic_timer.expired_tscdeadline =0 ; - guest_tsc = kvm_x86_ops->read_l1_tsc (vcpu,native_read_tsc ()); + guest_tsc = kvm_x86_ops->read_l1_tsc (vcpu,rdtsc_unordered ());trace_kvm_wait_lapic_expire (vcpu->vcpu_id, guest_tsc - tsc_deadline);/* __delay is delay_tsc whenever the hardware has TSC, thus always. */
@@ -1216,7 +1216,7 @@ static void start_apic_timer(struct kvm_lapic *apic)local_irq_save (flags); now = apic->lapic_timer.timer.base->get_time (); - guest_tsc = kvm_x86_ops->read_l1_tsc (vcpu,native_read_tsc ()); + guest_tsc = kvm_x86_ops->read_l1_tsc (vcpu,rdtsc_unordered ());if (likely (tscdeadline > guest_tsc)) { ns = (tscdeadline - guest_tsc) *1000000ULL ;do_div (ns, this_tsc_khz);
arch/x86/kvm/svm.c +2 -2
@@ -1077,7 +1077,7 @@ static u64 svm_compute_tsc_offset(struct kvm_vcpu *vcpu, u64 target_tsc) {u64 tsc; - tsc =svm_scale_tsc (vcpu,native_read_tsc ()); + tsc =svm_scale_tsc (vcpu,rdtsc_unordered ());return target_tsc - tsc; }
@@ -3074,7 +3074,7 @@ static int svm_get_msr(struct kvm_vcpu *vcpu, unsigned ecx, u64 *data)switch (ecx) {case MSR_IA32_TSC : { *data = svm->vmcb->control.tsc_offset + -svm_scale_tsc (vcpu,native_read_tsc ()); +svm_scale_tsc (vcpu,rdtsc_unordered ());break ; }
arch/x86/kvm/vmx.c +2 -2
@@ -2236,7 +2236,7 @@ static u64 guest_read_tsc(void) {u64 host_tsc, tsc_offset; - host_tsc =native_read_tsc (); + host_tsc =rdtsc_unordered (); tsc_offset =vmcs_read64 (TSC_OFFSET );return host_tsc + tsc_offset; }
@@ -2317,7 +2317,7 @@ static void vmx_adjust_tsc_offset(struct kvm_vcpu *vcpu, s64 adjustment, bool hostatic u64 vmx_compute_tsc_offset (struct kvm_vcpu *vcpu,u64 target_tsc) { -return target_tsc -native_read_tsc (); +return target_tsc -rdtsc_unordered (); }static bool guest_cpuid_has_vmx (struct kvm_vcpu *vcpu)
arch/x86/kvm/x86.c +6 -6
@@ -1430,7 +1430,7 @@ static cycle_t read_tsc(void)* but no one has ever seen it happen. */ rdtsc_barrier (); - ret = (cycle_t )native_read_tsc (); + ret = (cycle_t )rdtsc_unordered (); last = pvclock_gtod_data.clock.cycle_last;
@@ -1621,7 +1621,7 @@ static int kvm_guest_time_update(struct kvm_vcpu *v)return 1 ; }if (!use_master_clock) { - host_tsc =native_read_tsc (); + host_tsc =rdtsc_unordered (); kernel_ns =get_kernel_ns (); }
@@ -2945,7 +2945,7 @@ void kvm_arch_vcpu_load(struct kvm_vcpu *vcpu, int cpu)if (unlikely (vcpu->cpu != cpu) ||check_tsc_unstable ()) {s64 tsc_delta = !vcpu->arch.last_host_tsc ?0 : -native_read_tsc () - vcpu->arch.last_host_tsc; +rdtsc_unordered () - vcpu->arch.last_host_tsc;if (tsc_delta <0 )mark_tsc_unstable ("KVM discovered backwards TSC" );if (check_tsc_unstable ()) {
@@ -2973,7 +2973,7 @@ void kvm_arch_vcpu_put(struct kvm_vcpu *vcpu) { kvm_x86_ops->vcpu_put (vcpu);kvm_put_guest_fpu (vcpu); - vcpu->arch.last_host_tsc =native_read_tsc (); + vcpu->arch.last_host_tsc =rdtsc_unordered (); } static int kvm_vcpu_ioctl_get_lapic(struct kvm_vcpu *vcpu,
@@ -6388,7 +6388,7 @@ static int vcpu_enter_guest(struct kvm_vcpu *vcpu)hw_breakpoint_restore (); vcpu->arch.last_guest_tsc = kvm_x86_ops->read_l1_tsc (vcpu, -native_read_tsc ()); +rdtsc_unordered ()); vcpu->mode =OUTSIDE_GUEST_MODE ;smp_wmb ();
@@ -7186,7 +7186,7 @@ int kvm_arch_hardware_enable(void)if (ret !=0 )return ret; - local_tsc =native_read_tsc (); + local_tsc =rdtsc_unordered (); stable = !check_tsc_unstable ();list_for_each_entry (kvm, &vm_list, vm_list) {kvm_for_each_vcpu (i, vcpu, kvm) {
arch/x86/lib/delay.c +4 -4
@@ -55,10 +55,10 @@ static void delay_tsc(unsigned long __loops)preempt_disable (); cpu =smp_processor_id ();rdtsc_barrier (); - bclock =native_read_tsc (); + bclock =rdtsc_unordered ();for (;;) {rdtsc_barrier (); - now =native_read_tsc (); + now =rdtsc_unordered ();if ((now - bclock) >= loops)break ;
@@ -80,7 +80,7 @@ static void delay_tsc(unsigned long __loops) loops -= (now - bclock); cpu =smp_processor_id ();rdtsc_barrier (); - bclock =native_read_tsc (); + bclock =rdtsc_unordered (); } }preempt_enable ();
@@ -100,7 +100,7 @@ void use_tsc_delay(void)int read_current_timer (unsigned long *timer_val) {if (delay_fn == delay_tsc) { - *timer_val =native_read_tsc (); + *timer_val =rdtsc_unordered ();return 0 ; }return -1 ;
drivers/input/gameport/gameport.c +2 -2
@@ -149,9 +149,9 @@ static int old_gameport_measure_speed(struct gameport *gameport)for (i =0 ; i <50 ; i++) {local_irq_save (flags); - t1 =native_read_tsc (); + t1 =rdtsc_unordered ();for (t =0 ; t <50 ; t++)gameport_read (gameport); - t2 =native_read_tsc (); + t2 =rdtsc_unordered ();local_irq_restore (flags);udelay (i *10 );if (t2 - t1 < tx) tx = t2 - t1;
drivers/input/joystick/analog.c +2 -2
@@ -143,7 +143,7 @@ struct analog_port {#include <linux/i8253.h> -#define GET_TIME (x) do { if (cpu_has_tsc) x = (unsigned int)native_read_tsc(); else x = get_time_pit(); } while (0) +#define GET_TIME (x) do { if (cpu_has_tsc) x = (unsigned int)rdtsc_unordered(); else x = get_time_pit(); } while (0)#define DELTA (x,y) (cpu_has_tsc ? ((y) - (x)) : ((x) - (y) + ((x) < (y) ? PIT_TICK_RATE / HZ : 0)))#define TIME_NAME (cpu_has_tsc?"TSC":"PIT")static unsigned int get_time_pit (void )
@@ -160,7 +160,7 @@ static unsigned int get_time_pit(void) return count; }#elif defined(__x86_64__) -#define GET_TIME (x) do { x = (unsigned int)native_read_tsc(); } while (0) +#define GET_TIME (x) do { x = (unsigned int)rdtsc_unordered(); } while (0)#define DELTA (x,y) ((y)-(x))#define TIME_NAME "TSC"#elif defined(__alpha__) || defined(CONFIG_MN10300) || defined(CONFIG_ARM) || defined(CONFIG_ARM64) || defined(CONFIG_TILE)
drivers/net/hamradio/baycom_epp.c +1 -1
@@ -638,7 +638,7 @@ static int receive(struct net_device *dev, int cnt)#define GETTICK (x) \ ({ \ if (cpu_has_tsc) \ - x = (unsigned int)native_read_tsc(); \ + x = (unsigned int)rdtsc_unordered(); \ })#else /* __i386__ */ #define GETTICK (x)
drivers/thermal/intel_powerclamp.c +2 -2
@@ -340,7 +340,7 @@ static bool powerclamp_adjust_controls(unsigned int target_ratio,/* check result for the last window */ msr_now =pkg_state_counter (); - tsc_now =native_read_tsc (); + tsc_now =rdtsc_unordered ();/* calculate pkg cstate vs tsc ratio */ if (!msr_last || !tsc_last)
@@ -482,7 +482,7 @@ static void poll_pkg_cstate(struct work_struct *dummy)u64 val64; msr_now =pkg_state_counter (); - tsc_now =native_read_tsc (); + tsc_now =rdtsc_unordered (); jiffies_now = jiffies;/* calculate pkg cstate vs tsc ratio */
tools/power/cpupower/debug/kernel/cpufreq-test_tsc.c +2 -2
@@ -81,11 +81,11 @@ static int __init cpufreq_test_tsc(void)printk (KERN_DEBUG "start--> \n" ); then =read_pmtmr (); - then_tsc =native_read_tsc (); + then_tsc =rdtsc_unordered ();for (i=0 ;i<20 ;i++) {mdelay (100 ); now =read_pmtmr (); - now_tsc =native_read_tsc (); + now_tsc =rdtsc_unordered (); diff = (now - then) &0xFFFFFF ; diff_tsc = now_tsc - then_tsc;printk (KERN_DEBUG "t1: %08u t2: %08u diff_pmtmr: %08u diff_tsc: %016llu\n" , then, now, diff, diff_tsc);
Signature · 1 line
2.4.2
Re: [PATCH v2 13/17] x86/tsc: Rename native_read_tsc() to rdtsc_unordered()
Quoted 5 lines · Andy Lutomirski
Now that there is no paravirt TSC, the "native" is inappropriate. The fact that rdtsc is not ordered can catch people by surprise, so call it
rdtsc_unordered().Signed-off-by Andy Lutomirski <[email protected]>
...
Quoted 5 lines
arch/x86/include/asm/msr.h +0
-1@@ -109,7 +109,16 @@ notrace static inline int native_write_msr_safe(unsigned int msr,extern int rdmsr_safe_regs (u32 regs[8 ]);extern int wrmsr_safe_regs (u32 regs[8 ]); -static __always_inline unsigned long long native_read_tsc (void )
arch/x86/include/asm/msr.h notrace static inline int native_write_msr_safe(unsigned int msr, · line 121
Quoted 10 lines
arch/x86/include/asm/msr.h +10
-0+/** +* rdtsc_unordered() - returns the current TSC without ordering constraints +* +* rdtsc_unordered() returns the result of RDTSC as a 64-bit integer. The +* only ordering constraint it supplies is the ordering implied by +* "asm volatile": it will put the RDTSC in the place you expect. The +* CPU can and will speculatively execute that RDTSC, though, so the +* results can be non-monotonic if compared on different CPUs. +*/ +static __always_inline unsigned long long rdtsc_unordered (void )
I like the rdtsc_ordered() thing because it wraps the barrier and people cannot just forget it. But let's call this not rdtsc_unordered() but simply
rdtsc()
The "_unordered" suffix is unnecessary IMO since this function is a simple wrapper around the hw insn and we do that naming scheme with all such wrappers.
Signature · 5 lines
Regards/Gruss,
Boris.
ECO tip #101: Trim your mails when you reply.
--Re: [PATCH v2 13/17] x86/tsc: Rename native_read_tsc() to rdtsc_unordered()
Quoted 29 lines · Borislav Petkov
Quoted 5 lines · Andy Lutomirski
Now that there is no paravirt TSC, the "native" is inappropriate. The fact that rdtsc is not ordered can catch people by surprise, so call it
rdtsc_unordered().Signed-off-by Andy Lutomirski <[email protected]>...
Quoted 5 lines
arch/x86/include/asm/msr.h +0
-1@@ -109,7 +109,16 @@ notrace static inline int native_write_msr_safe(unsigned int msr,extern int rdmsr_safe_regs (u32 regs[8 ]);extern int wrmsr_safe_regs (u32 regs[8 ]); -static __always_inline unsigned long long native_read_tsc (void )arch/x86/include/asm/msr.h notrace static inline int native_write_msr_safe(unsigned int msr, · line 121
Quoted 10 lines
arch/x86/include/asm/msr.h +10
-0+/** +* rdtsc_unordered() - returns the current TSC without ordering constraints +* +* rdtsc_unordered() returns the result of RDTSC as a 64-bit integer. The +* only ordering constraint it supplies is the ordering implied by +* "asm volatile": it will put the RDTSC in the place you expect. The +* CPU can and will speculatively execute that RDTSC, though, so the +* results can be non-monotonic if compared on different CPUs. +*/ +static __always_inline unsigned long long rdtsc_unordered (void )I like the
rdtsc_ordered()thing because it wraps the barrier and people cannot just forget it. But let's call this notrdtsc_unordered()but simply
Quoted 5 lines
rdtsc()The "
_unordered" suffix is unnecessary IMO since this function is a simple wrapper around the hw insn and we do that naming scheme with all such wrappers.
I could go either way here. rdtsc() is weird because people seem to forget about the barrier. Maybe I'm being unnecessarily paranoid.
--Andy
Quoted 7 lines
--
Regards/Gruss,
Boris.ECO tip #101: Trim your mails when you reply.
--
Signature · 2 lines
Andy Lutomirski AMA Capital Management, LLC
Re: [PATCH v2 13/17] x86/tsc: Rename native_read_tsc() to rdtsc_unordered()
Quoted 2 lines · Andy Lutomirski
I could go either way here.
rdtsc()is weird because people seem to forget about the barrier.
Yeah, our job is only to give them enough rope...
Quoted 1 line
Maybe I'm being unnecessarily paranoid.
You can always slap a comment above the rdtsc() definition saying that people should use the _ordered() version if they wanna order. :)
Signature · 5 lines
Regards/Gruss,
Boris.
ECO tip #101: Trim your mails when you reply.
--Andy Lutomirski
maintainerPATCH v2 14/17 x86/tsc: Move rdtsc_barrier() and rename it to barrier_before_rdtsc()
In reply to Andy Lutomirskirdtsc_barrier() (i.e. MFENCE or LFENCE depending on vendor) is supported by the docs as a barrier immediately before RDTSC. There is neither empirical evidence that it's needed at all after RDTSC nor is there any reason to believe that the type of fence to put after RDTSC (if any) would be the same as a fence before RDTSC.
Rename the function accordingly. While we're at it, move it so it lives with the rest of the RDTSC inlines.
There are some callers that look odd now. That's fine -- they are odd. A subsequent patch will fix them up.
Signed-off-by Andy Lutomirski <[email protected]>
--- arch/x86/entry/vdso/vclock_gettime.c | 2 +- arch/x86/include/asm/barrier.h | 11 ----------- arch/x86/include/asm/msr.h | 21 +++++++++++++++++++++ arch/x86/include/asm/pvclock.h | 6 +++--- arch/x86/kernel/trace_clock.c | 2 +- arch/x86/kernel/tsc_sync.c | 8 ++++---- arch/x86/kvm/x86.c | 2 +- arch/x86/lib/delay.c | 6 +++--- arch/x86/um/asm/barrier.h | 4 +--- 9 files changed, 35 insertions(+), 27 deletions(-)
arch/x86/entry/vdso/vclock_gettime.c +1 -1
@@ -185,7 +185,7 @@ notrace static cycle_t vread_tsc(void)* as to whether rdtsc can be reordered with later loads, * but no one has ever seen it happen. */ -rdtsc_barrier (); +barrier_before_rdtsc (); ret = (cycle_t )rdtsc_unordered (); last = gtod->cycle_last;
arch/x86/include/asm/barrier.h +0 -11
@@ -91,15 +91,4 @@ do { \#define smp_mb__before_atomic () barrier()#define smp_mb__after_atomic () barrier() -/* -* Stop RDTSC speculation. This is needed when you need to use RDTSC -* (or get_cycles or vread that possibly accesses the TSC) in a defined -* code region. -*/ -static __always_inline void rdtsc_barrier(void) -{ -alternative_2 ("" ,"mfence" ,X86_FEATURE_MFENCE_RDTSC , -"lfence" ,X86_FEATURE_LFENCE_RDTSC ); -} -#endif /* _ASM_X86_BARRIER_H */
arch/x86/include/asm/msr.h +21 -0
@@ -127,6 +127,27 @@ static __always_inline unsigned long long rdtsc_unordered(void)return EAX_EDX_VAL (val, low, high); } +/** +* barrier_before_rdtsc() - memory barrier before rdtsc_unordered +* +* The RDTSC instruction is not ordered relative to memory access. The +* Intel SDM and the AMD APM are both vague on this point, but +* empirically an RDTSC instruction can be speculatively executed before +* prior loads. An RDTSC immediately after an appropriate barrier +* appears to be ordered as a normal load, that is, it provides the same +* ordering guarantees as reading from a global memory location that +* some other imaginary CPU is updating continuously with a time stamp. +* +* For applications that require monotonicity across multiple CPUs or +* that need RDTSC to time code in a defined region, this barrier can be +* used before rdtsc_unordered(). +*/ +static __always_inline void barrier_before_rdtsc(void) +{ +alternative_2 ("" ,"mfence" ,X86_FEATURE_MFENCE_RDTSC , +"lfence" ,X86_FEATURE_LFENCE_RDTSC ); +} +static inline unsigned long long native_read_pmc (int counter) {DECLARE_ARGS (val, low, high);
arch/x86/include/asm/pvclock.h +3 -3
@@ -79,14 +79,14 @@ unsigned __pvclock_read_cycles(const struct pvclock_vcpu_time_info *src,/* Note: emulated platforms which do not advertise SSE2 support * result in kvmclock not using the necessary RDTSC barriers. * Without barriers, it is possible that RDTSC instruction reads from -* the time stamp counter outside rdtsc_barrier protected section +* the time stamp counter outside barrier_before_rdtsc protected section * below, resulting in violation of monotonicity. */ -rdtsc_barrier (); +barrier_before_rdtsc (); offset =pvclock_get_nsec_offset (src); ret = src->system_time + offset; ret_flags = src->flags; -rdtsc_barrier (); +barrier_before_rdtsc (); *cycles = ret; *flags = ret_flags;
arch/x86/kernel/trace_clock.c +1 -1
@@ -14,7 +14,7 @@ u64 notrace trace_clock_x86_tsc(void) {u64 ret; -rdtsc_barrier (); +barrier_before_rdtsc (); ret =rdtsc_unordered ();return ret;
arch/x86/kernel/tsc_sync.c +4 -4
@@ -46,9 +46,9 @@ static void check_tsc_warp(unsigned int timeout)cycles_t start, now, prev, end;int i; -rdtsc_barrier (); +barrier_before_rdtsc (); start =get_cycles (); -rdtsc_barrier (); +barrier_before_rdtsc ();/* * The measurement runs for 'timeout' msecs: */
@@ -63,9 +63,9 @@ static void check_tsc_warp(unsigned int timeout)*/ arch_spin_lock (&sync_lock); prev = last_tsc; -rdtsc_barrier (); +barrier_before_rdtsc (); now =get_cycles (); -rdtsc_barrier (); +barrier_before_rdtsc (); last_tsc = now;arch_spin_unlock (&sync_lock);
arch/x86/kvm/x86.c +1 -1
@@ -1429,7 +1429,7 @@ static cycle_t read_tsc(void)* as to whether rdtsc can be reordered with later loads, * but no one has ever seen it happen. */ -rdtsc_barrier (); +barrier_before_rdtsc (); ret = (cycle_t )rdtsc_unordered (); last = pvclock_gtod_data.clock.cycle_last;
arch/x86/lib/delay.c +3 -3
@@ -54,10 +54,10 @@ static void delay_tsc(unsigned long __loops)preempt_disable (); cpu =smp_processor_id (); -rdtsc_barrier (); +barrier_before_rdtsc (); bclock =rdtsc_unordered ();for (;;) { -rdtsc_barrier (); +barrier_before_rdtsc (); now =rdtsc_unordered ();if ((now - bclock) >= loops)break ;
@@ -79,7 +79,7 @@ static void delay_tsc(unsigned long __loops)if (unlikely (cpu !=smp_processor_id ())) { loops -= (now - bclock); cpu =smp_processor_id (); -rdtsc_barrier (); +barrier_before_rdtsc (); bclock =rdtsc_unordered (); } }
arch/x86/um/asm/barrier.h +1 -3
@@ -49,10 +49,8 @@* Stop RDTSC speculation. This is needed when you need to use RDTSC * (or get_cycles or vread that possibly accesses the TSC) in a defined * code region. -* -* (Could use an alternative three way for this if there was one.) */ -static inline void rdtsc_barrier (void ) +static inline void barrier_before_rdtsc (void ) {alternative_2 ("" ,"mfence" ,X86_FEATURE_MFENCE_RDTSC ,"lfence" ,X86_FEATURE_LFENCE_RDTSC );
Signature · 1 line
2.4.2
Andy Lutomirski
maintainerPATCH v2 15/17 x86: Add rdtsc_ordered() and use it in trivial call sites
In reply to Andy Lutomirskibarrier_before_rdtsc(); rdtsc_unordered() is an unnecessary mouthful and requires more thought than should be necessary. Add an rdtsc_ordered() helper and replace the trivial call sites with it.
This should not change generated code.
Signed-off-by Andy Lutomirski <[email protected]>
--- arch/x86/entry/vdso/vclock_gettime.c | 16 ++-------------- arch/x86/include/asm/msr.h | 14 ++++++++++++++ arch/x86/kernel/trace_clock.c | 7 +------ arch/x86/kvm/x86.c | 16 ++-------------- arch/x86/lib/delay.c | 9 +++------ 5 files changed, 22 insertions(+), 40 deletions(-)
arch/x86/entry/vdso/vclock_gettime.c +2 -14
@@ -175,20 +175,8 @@ static notrace cycle_t vread_pvclock(int *mode) notrace static cycle_t vread_tsc(void) { -cycle_t ret; -u64 last; - -/* -* Empirically, a fence (of type that depends on the CPU) -* before rdtsc is enough to ensure that rdtsc is ordered -* with respect to loads. The various CPU manuals are unclear -* as to whether rdtsc can be reordered with later loads, -* but no one has ever seen it happen. -*/ -barrier_before_rdtsc (); - ret = (cycle_t )rdtsc_unordered (); - - last = gtod->cycle_last; +cycle_t ret = (cycle_t )rdtsc_ordered (); +u64 last = gtod->cycle_last;if (likely (ret >= last))return ret;
arch/x86/include/asm/msr.h +14 -0
@@ -148,6 +148,20 @@ static __always_inline void barrier_before_rdtsc(void) "lfence", X86_FEATURE_LFENCE_RDTSC); } +/** +* rdtsc_ordered() - read the current TSC in program order +* +* rdtsc_ordered() returns the result of RDTSC as a 64-bit integer. +* It is ordered like a load to a global in-memory counter. It should +* be impossible to observe non-monotonic rdtsc_unordered() behavior +* across multiple CPUs as long as the TSC is synced. +*/ +static __always_inline unsigned long long rdtsc_ordered (void ) +{ +barrier_before_rdtsc (); +return rdtsc_unordered (); +} +static inline unsigned long long native_read_pmc (int counter) {DECLARE_ARGS (val, low, high);
arch/x86/kernel/trace_clock.c +1 -6
@@ -12,10 +12,5 @@*/ u64 notrace trace_clock_x86_tsc(void) { -u64 ret; - -barrier_before_rdtsc (); - ret =rdtsc_unordered (); - -return ret; +return rdtsc_ordered (); }
arch/x86/kvm/x86.c +2 -14
@@ -1419,20 +1419,8 @@ EXPORT_SYMBOL_GPL(kvm_write_tsc);static cycle_t read_tsc (void ) { -cycle_t ret; -u64 last; - -/* -* Empirically, a fence (of type that depends on the CPU) -* before rdtsc is enough to ensure that rdtsc is ordered -* with respect to loads. The various CPU manuals are unclear -* as to whether rdtsc can be reordered with later loads, -* but no one has ever seen it happen. -*/ -barrier_before_rdtsc (); - ret = (cycle_t )rdtsc_unordered (); - - last = pvclock_gtod_data.clock.cycle_last; +cycle_t ret = (cycle_t )rdtsc_ordered (); +u64 last = pvclock_gtod_data.clock.cycle_last;if (likely (ret >= last))return ret;
arch/x86/lib/delay.c +3 -6
@@ -54,11 +54,9 @@ static void delay_tsc(unsigned long __loops)preempt_disable (); cpu =smp_processor_id (); -barrier_before_rdtsc (); - bclock =rdtsc_unordered (); + bclock =rdtsc_ordered ();for (;;) { -barrier_before_rdtsc (); - now =rdtsc_unordered (); + now =rdtsc_ordered ();if ((now - bclock) >= loops)break ;
@@ -79,8 +77,7 @@ static void delay_tsc(unsigned long __loops)if (unlikely (cpu !=smp_processor_id ())) { loops -= (now - bclock); cpu =smp_processor_id (); -barrier_before_rdtsc (); - bclock =rdtsc_unordered (); + bclock =rdtsc_ordered (); } }preempt_enable ();
Signature · 1 line
2.4.2
Re: [PATCH v2 15/17] x86: Add rdtsc_ordered() and use it in trivial call sites
Quoted 8 lines · Andy Lutomirski
barrier_before_rdtsc();rdtsc_unordered()is an unnecessary mouthful and requires more thought than should be necessary. Add anrdtsc_ordered()helper and replace the trivial call sites with it.This should not change generated code.
Signed-off-by Andy Lutomirski <[email protected]>---
...
Quoted 12 lines
arch/x86/include/asm/msr.h +4
-0@@ -148,6 +148,20 @@ static __always_inline void barrier_before_rdtsc(void) "lfence", X86_FEATURE_LFENCE_RDTSC); } +/** +* rdtsc_ordered() - read the current TSC in program order +* +* rdtsc_ordered() returns the result of RDTSC as a 64-bit integer.
arch/x86/include/asm/msr.h static __always_inline void barrier_before_rdtsc(void) · line 164
Quoted 10 lines
arch/x86/include/asm/msr.h +10
-0+* It is ordered like a load to a global in-memory counter. It should +* be impossible to observe non-monotonic rdtsc_unordered() behavior +* across multiple CPUs as long as the TSC is synced. +*/ +static __always_inline unsigned long long rdtsc_ordered (void ) +{ +barrier_before_rdtsc (); +return rdtsc_unordered (); +} +
I don't see the final tree state with all those applied (too lazy to apply them) but why not simply kill barrier_before_rdtsc() and inline the alternative into rdtsc_ordered()?
I mean, I don't see usage for it somewhere else...
Signature · 5 lines
Regards/Gruss,
Boris.
ECO tip #101: Trim your mails when you reply.
--Re: [PATCH v2 15/17] x86: Add rdtsc_ordered() and use it in trivial call sites
Quoted 39 lines · Borislav Petkov
Quoted 8 lines · Andy Lutomirski
barrier_before_rdtsc();rdtsc_unordered()is an unnecessary mouthful and requires more thought than should be necessary. Add anrdtsc_ordered()helper and replace the trivial call sites with it.This should not change generated code.
Signed-off-by Andy Lutomirski <[email protected]>---
...
Quoted 12 lines
arch/x86/include/asm/msr.h +4
-0@@ -148,6 +148,20 @@ static __always_inline void barrier_before_rdtsc(void) "lfence", X86_FEATURE_LFENCE_RDTSC); } +/** +* rdtsc_ordered() - read the current TSC in program order +* +* rdtsc_ordered() returns the result of RDTSC as a 64-bit integer. arch/x86/include/asm/msr.h static __always_inline void barrier_before_rdtsc(void) · line 164
Quoted 10 lines
arch/x86/include/asm/msr.h +10
-0+* It is ordered like a load to a global in-memory counter. It should +* be impossible to observe non-monotonic rdtsc_unordered() behavior +* across multiple CPUs as long as the TSC is synced. +*/ +static __always_inline unsigned long long rdtsc_ordered (void ) +{ +barrier_before_rdtsc (); +return rdtsc_unordered (); +} +I don't see the final tree state with all those applied (too lazy to apply them) but why not simply kill
barrier_before_rdtsc()and inline the alternative intordtsc_ordered()?
Quoted 1 line
I mean, I don't see usage for it somewhere else...
There's the awful, awful code in kvmclock. I'll fix that as part of v3 and get rid of barrier_before_rdtsc().
--Andy
Andy Lutomirski
maintainerPATCH v2 16/17 x86/tsc: Use rdtsc_ordered() in check_tsc_warp() and drop extra barriers
In reply to Andy LutomirskiUsing get_cycles was unnecessary: check_tsc_warp() is not called on TSC-less systems. Replace barrier_before_rdtsc(); get_cycles() with rdtsc_ordered().
While we're at it, make the somewhat more dangerous change of removing barrier_before_rdtsc after RDTSC in the TSC warp check code. This should be okay, though -- the vDSO TSC code doesn't have that barrier, so, if removing the barrier from the warp check would cause us to detect a warp that we otherwise wouldn't detect, then we have a genuine bug.
Signed-off-by Andy Lutomirski <[email protected]>
--- arch/x86/kernel/tsc_sync.c | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-)
arch/x86/kernel/tsc_sync.c +6 -8
@@ -39,16 +39,15 @@ static cycles_t max_warp;static int nr_warps;/* -* TSC-warp measurement loop running on both CPUs: +* TSC-warp measurement loop running on both CPUs. This is not called +* if there is no TSC. */ static void check_tsc_warp (unsigned int timeout) {cycles_t start, now, prev, end;int i; -barrier_before_rdtsc (); - start =get_cycles (); -barrier_before_rdtsc (); + start =rdtsc_ordered ();/* * The measurement runs for 'timeout' msecs: */
@@ -63,9 +62,7 @@ static void check_tsc_warp(unsigned int timeout)*/ arch_spin_lock (&sync_lock); prev = last_tsc; -barrier_before_rdtsc (); - now =get_cycles (); -barrier_before_rdtsc (); + now =rdtsc_ordered (); last_tsc = now;arch_spin_unlock (&sync_lock);
@@ -126,7 +123,7 @@ void check_tsc_sync_source(int cpu)/* * No need to check if we already know that the TSC is not -* synchronized: +* synchronized or if we have no TSC. */ if (unsynchronized_tsc ())return ;
@@ -190,6 +187,7 @@ void check_tsc_sync_target(void) {int cpus =2 ; +/* Also aborts if there is no TSC. */ if (unsynchronized_tsc () || tsc_clocksource_reliable)return ;
Signature · 1 line
2.4.2
Andy Lutomirski
maintainerPATCH v2 17/17 x86/tsc: In read_tsc, use rdtsc_ordered() instead of get_cycles()
In reply to Andy LutomirskiThere are two logical changes here. First, this removes a check for cpu_has_tsc. That check is unnecessary, as we don't register the TSC as a clocksource on systems that have no TSC. Second, it adds a barrier, thus preventing observable non-monotonicity.
I suspect that the missing barrier was never a problem in practice because system calls themselves were heavy enough barriers to prevent user code from observing time warps due to speculation. (Without the corresponding barrier in the vDSO, however, non-monotonicity is easy to detect.)
Signed-off-by Andy Lutomirski <[email protected]>
--- arch/x86/kernel/tsc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
arch/x86/kernel/tsc.c +1 -1
@@ -961,7 +961,7 @@ static struct clocksource clocksource_tsc;*/ static cycle_t read_tsc (struct clocksource *cs) { -return (cycle_t )get_cycles (); +return (cycle_t )rdtsc_ordered (); }/*
Signature · 1 line
2.4.2