Date: Thu, 17 Mar 2011 23:55:31 +1100 (EST) From: Bruce Evans <brde@optusnet.com.au> To: Jung-uk Kim <jkim@FreeBSD.org> Cc: svn-src-head@FreeBSD.org, svn-src-all@FreeBSD.org, src-committers@FreeBSD.org Subject: Re: svn commit: r219698 - head/sys/i386/include Message-ID: <20110317234320.E1128@besplex.bde.org> In-Reply-To: <201103161609.p2GG98q6097329@svn.freebsd.org> References: <201103161609.p2GG98q6097329@svn.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Wed, 16 Mar 2011, Jung-uk Kim wrote:
> Log:
> Rework r219679. Always check CPU class at run-time to make it predictable.
> Unfortunately, it pulls in <machine/cputypes.h> but it is small enough and
> namespace pollution is minimal, I hope.
>
> Pointed out by: bde
Well, I don't like the namespace pollution, and the old code carefully
avoided it by using the tsc_present global.
> Modified: head/sys/i386/include/cpu.h
> ==============================================================================
> --- head/sys/i386/include/cpu.h Wed Mar 16 12:40:58 2011 (r219697)
> +++ head/sys/i386/include/cpu.h Wed Mar 16 16:09:08 2011 (r219698)
> @@ -39,6 +39,7 @@
> /*
> * Definitions unique to i386 cpu support.
> */
> +#include <machine/cputypes.h>
> #include <machine/psl.h>
> #include <machine/frame.h>
> #include <machine/segments.h>
> @@ -69,14 +70,13 @@ void swi_vm(void *);
> static __inline uint64_t
> get_cyclecount(void)
> {
> -#if defined(I486_CPU) || defined(KLD_MODULE)
> struct bintime bt;
>
> - binuptime(&bt);
> - return ((uint64_t)bt.sec << 56 | bt.frac >> 8);
> -#else
> + if (cpu_class == CPUCLASS_486) {
> + binuptime(&bt);
> + return ((uint64_t)bt.sec << 56 | bt.frac >> 8);
> + }
> return (rdtsc());
> -#endif
> }
>
> #endif
cpu_class shouldn't be used for anything, and might not be able to
correctly classify whether the CPU has a TSC. The cpu feature for
this should be used. Using it directly with the correct includes
would give considerably more namespace pollution (md_var.h for cpu_feature
and specialreg.h for CPUID_TSC).
Although I said that tsc_present should go, it seems to be the best
thing to use here. Rename it __cpu_feature_TSC to reflect that it is
exactly (cpu_feature & CPUID_TSC) and put underscores in its name so
as to start being even more careful about namespace pollution in cpu.h.
Or redeclare cpu_feature and CPUID_TSC.
Bruce
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20110317234320.E1128>