Re: rdmsr(MSR_TSC) vs rdtsc

[Andrew Doran <ad%netbsd.org@localhost>]

On Tue, Feb 01, 2011 at 06:47:31PM +0100, Manuel Bouyer wrote:
> On Mon, Jan 31, 2011 at 11:22:38AM +0000, Andrew Doran wrote:
> > > How could this be fixed ? I guess we'll have to use rdmsr(MSR_TSC)
> > > or cpu_counter() depending on the CPU ?
> > 
> > How about another routine like cpu_counter_serializing() or something
> > that does a test and picks which one to use?
> 
> maybe somthing lilke the attached patch ?

Looks fine to me, thanks.

> 
> -- 
> Manuel Bouyer <bouyer%antioche.eu.org@localhost>
>      NetBSD: 26 ans d'experience feront toujours la difference
> --

> Index: include/cpu_counter.h
> ===================================================================
> RCS file: /cvsroot/src/sys/arch/x86/include/cpu_counter.h,v
> retrieving revision 1.4
> diff -u -p -u -r1.4 cpu_counter.h
> --- include/cpu_counter.h     10 May 2008 16:12:32 -0000      1.4
> +++ include/cpu_counter.h     1 Feb 2011 17:35:50 -0000
> @@ -35,6 +35,7 @@
>  #ifdef _KERNEL
>  
>  uint64_t     cpu_counter(void);
> +uint64_t     cpu_counter_serializing(void);
>  uint32_t     cpu_counter32(void);
>  uint64_t     cpu_frequency(struct cpu_info *);
>  int          cpu_hascounter(void);
> Index: x86/cpu.c
> ===================================================================
> RCS file: /cvsroot/src/sys/arch/x86/x86/cpu.c,v
> retrieving revision 1.79
> diff -u -p -u -r1.79 cpu.c
> --- x86/cpu.c 11 Jan 2011 18:25:25 -0000      1.79
> +++ x86/cpu.c 1 Feb 2011 17:35:50 -0000
> @@ -1080,10 +1080,12 @@ cpu_get_tsc_freq(struct cpu_info *ci)
>       uint64_t last_tsc;
>  
>       if (cpu_hascounter()) {
> -             last_tsc = rdmsr(MSR_TSC);
> +             last_tsc = cpu_counter_serializing();
>               i8254_delay(100000);
> -             ci->ci_data.cpu_cc_freq = (rdmsr(MSR_TSC) - last_tsc) * 10;
> +             ci->ci_data.cpu_cc_freq =
> +                 (cpu_counter_serializing() - last_tsc) * 10;
>       }
> +     printf("\n");
>  }
>  
>  void
> Index: x86/tsc.c
> ===================================================================
> RCS file: /cvsroot/src/sys/arch/x86/x86/tsc.c,v
> retrieving revision 1.27
> diff -u -p -u -r1.27 tsc.c
> --- x86/tsc.c 21 Aug 2010 01:57:34 -0000      1.27
> +++ x86/tsc.c 1 Feb 2011 17:35:50 -0000
> @@ -177,13 +177,13 @@ tsc_read_bp(struct cpu_info *ci, uint64_
>  
>       /* Flag it and read our TSC. */
>       atomic_or_uint(&ci->ci_flags, CPUF_SYNCTSC);
> -     bptsc = rdmsr(MSR_TSC) >> 1;
> +     bptsc = cpu_counter_serializing() >> 1;
>  
>       /* Wait for remote to complete, and read ours again. */
>       while ((ci->ci_flags & CPUF_SYNCTSC) != 0) {
>               __insn_barrier();
>       }
> -     bptsc += (rdmsr(MSR_TSC) >> 1);
> +     bptsc += (cpu_counter_serializing() >> 1);
>  
>       /* Wait for the results to come in. */
>       while (tsc_sync_cpu == ci) {
> @@ -222,11 +222,11 @@ tsc_post_ap(struct cpu_info *ci)
>       while ((ci->ci_flags & CPUF_SYNCTSC) == 0) {
>               __insn_barrier();
>       }
> -     tsc = (rdmsr(MSR_TSC) >> 1);
> +     tsc = (cpu_counter_serializing() >> 1);
>  
>       /* Instruct primary to read its counter. */
>       atomic_and_uint(&ci->ci_flags, ~CPUF_SYNCTSC);
> -     tsc += (rdmsr(MSR_TSC) >> 1);
> +     tsc += (cpu_counter_serializing() >> 1);
>  
>       /* Post result.  Ensure the whole value goes out atomically. */
>       (void)atomic_swap_64(&tsc_sync_val, tsc);
> @@ -257,3 +257,12 @@ cpu_hascounter(void)
>  
>       return cpu_feature[0] & CPUID_TSC;
>  }
> +
> +uint64_t
> +cpu_counter_serializing(void)
> +{
> +     if (cpu_feature[0] & CPUID_MSR)
> +             return rdmsr(MSR_TSC);
> +     else
> +             return cpu_counter();
> +}