On Thu, Aug 03, 2017 at 07:56:11AM +0800, Adam Steen wrote:
> On Mon, Jul 31, 2017 at 3:58 PM, Mike Belopuhov <[email protected]> wrote:
> > On Mon, Jul 31, 2017 at 09:48 +0800, Adam Steen wrote:
> >> Ted Unangst  wrote:
> >> > we don't currently export this info, but we could add some sysctls. 
> >> > there's
> >> > some cpufeatures stuff there, but generally stuff isn't exported until
> >> > somebody finds a use for it... it shouldn't be too hard to add something 
> >> > to
> >> > amd64/machdep.c sysctl if you're interested.
> >>
> >> I am interested, as i need the info, i will look into it and hopefully
> >> come back with a patch.
> >
> > This is a bad idea because TSC as the time source is only usable
> > by OpenBSD on Skylake and Kaby Lake CPUs since they encode the TSC
> > frequency in the CPUID. All older CPUs have their TSCs measured
> > against the PIT. Currently the measurement done by the kernel isn't
> > very precise and if TSC is selected as a timecounter, the machine
> > would be gaining time on a pace that cannot be corrected by our NTP
> > daemon. (IIRC, about an hour a day on my Haswell running with NTP).
> >
> > To be able to use TSC as a timecounter source on OpenBSD or Solo5
> > you'd have to improve the in-kernel measurement of the TSC frequency
> > first. I've tried to perform 10 measurements and take an average and
> > it does improve accuracy, however I believe we need to poach another
> > bit from Linux and re-calibrate TSC via HPET:
> >
> >  
> > http://elixir.free-electrons.com/linux/v4.12.4/source/arch/x86/kernel/tsc.c#L409
> >
> > I think this is the most sane thing we can do. Here's a complete
> > procedure that Linux kernel undertakes:
> >
> >  
> > http://elixir.free-electrons.com/linux/v4.12.4/source/arch/x86/kernel/tsc.c#L751
> >
> > Regards,
> > Mike
> 
> Hi Mike and Ted
> 
> I understand using the tsc as a timecounter on non Skylake and
> Kabylake processors is inaccurate, but this i my first real foray into
> kernel programming, so wanted to started of slow. below is a diff to
> expose if the tsc is invariant and the tsc frequency via sysctl
> machdep. I would like to get this commited first and then move on to
> improving the in-kernel measurement of the tsc frequency as Mike
> describes above.
> 
> Sorry its taken a while to get back to you I have been working with
> Mike Larkin on vmm and my port of Solo5/ukvm.
> 
> Cheers
> Adam
> 
> comments?
> 

Everything in these sysctls can be obtained from CPUID on the processors you
want (skylake and later), and since that can be called in any CPL, why is
a kernel interface needed for this? The only thing that would be missing is
the tsc frequency on older-than-skylake cpus, but I don't think this is what
you are after, is it? (and even if you wanted this information on < skylake,
as mikeb points out, the accuracy of that value would then be very suspect and
probably not usable anyway).

-ml

> Index: sys/arch/amd64/amd64/identcpu.c
> ===================================================================
> RCS file: /cvs/src/sys/arch/amd64/amd64/identcpu.c,v
> retrieving revision 1.87
> diff -u -p -u -p -r1.87 identcpu.c
> --- sys/arch/amd64/amd64/identcpu.c 20 Jun 2017 05:34:41 -0000 1.87
> +++ sys/arch/amd64/amd64/identcpu.c 2 Aug 2017 23:45:54 -0000
> @@ -63,6 +63,8 @@ struct timecounter tsc_timecounter = {
>   tsc_get_timecount, NULL, ~0u, 0, "tsc", -1000, NULL
>  };
> 
> +u_int64_t amd64_tsc_freq = 0;
> +int amd64_has_invariant_tsc;
>  int amd64_has_xcrypt;
>  #ifdef CRYPTO
>  int amd64_has_pclmul;
> @@ -566,9 +568,12 @@ identifycpu(struct cpu_info *ci)
>   /* Check if it's an invariant TSC */
>   if (cpu_apmi_edx & CPUIDEDX_ITSC)
>   ci->ci_flags |= CPUF_INVAR_TSC;
> +
> +        amd64_has_invariant_tsc = (ci->ci_flags & CPUF_INVAR_TSC) != 0;
>   }
> 
>   ci->ci_tsc_freq = cpu_tsc_freq(ci);
> +    amd64_tsc_freq = ci->ci_tsc_freq;
> 
>   amd_cpu_cacheinfo(ci);
> 
> Index: sys/arch/amd64/amd64/machdep.c
> ===================================================================
> RCS file: /cvs/src/sys/arch/amd64/amd64/machdep.c,v
> retrieving revision 1.231
> diff -u -p -u -p -r1.231 machdep.c
> --- sys/arch/amd64/amd64/machdep.c 12 Jul 2017 06:26:32 -0000 1.231
> +++ sys/arch/amd64/amd64/machdep.c 2 Aug 2017 23:45:54 -0000
> @@ -425,7 +425,9 @@ int
>  cpu_sysctl(int *name, u_int namelen, void *oldp, size_t *oldlenp, void *newp,
>      size_t newlen, struct proc *p)
>  {
> +    extern u_int64_t amd64_tsc_freq;
>   extern int amd64_has_xcrypt;
> + extern int amd64_has_invariant_tsc;
>   dev_t consdev;
>   dev_t dev;
>   int val, error;
> @@ -496,6 +498,10 @@ cpu_sysctl(int *name, u_int namelen, voi
>   pckbc_release_console();
>   return (error);
>  #endif
> +    case CPU_TSCFREQ:
> +        return (sysctl_rdquad(oldp, oldlenp, newp, amd64_tsc_freq));
> + case CPU_INVARIANTTSC:
> + return (sysctl_rdint(oldp, oldlenp, newp, amd64_has_invariant_tsc));
>   default:
>   return (EOPNOTSUPP);
>   }
> Index: sys/arch/amd64/include/cpu.h
> ===================================================================
> RCS file: /cvs/src/sys/arch/amd64/include/cpu.h,v
> retrieving revision 1.113
> diff -u -p -u -p -r1.113 cpu.h
> --- sys/arch/amd64/include/cpu.h 12 Jul 2017 06:26:32 -0000 1.113
> +++ sys/arch/amd64/include/cpu.h 2 Aug 2017 23:45:56 -0000
> @@ -429,7 +429,9 @@ void mp_setperf_init(void);
>  #define CPU_XCRYPT 12 /* supports VIA xcrypt in userland */
>  #define CPU_LIDACTION 14 /* action caused by lid close */
>  #define CPU_FORCEUKBD 15 /* Force ukbd(4) as console keyboard */
> -#define CPU_MAXID 16 /* number of valid machdep ids */
> +#define CPU_TSCFREQ 16 /* tsc frequency */
> +#define CPU_INVARIANTTSC 17 /* has invariant tsc */
> +#define CPU_MAXID 18 /* number of valid machdep ids */
> 
>  #define CTL_MACHDEP_NAMES { \
>   { 0, 0 }, \
> @@ -448,6 +450,8 @@ void mp_setperf_init(void);
>   { 0, 0 }, \
>   { "lidaction", CTLTYPE_INT }, \
>   { "forceukbd", CTLTYPE_INT }, \
> + { "tscfreq", CTLTYPE_QUAD }, \
> + { "invarianttsc", CTLTYPE_INT }, \
>  }
> 
>  /*
> 

Reply via email to