On Thursday 17 March 2011 08:55 am, Bruce Evans wrote:
> 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.
Actually, I think this function must be move to machdep.c as a real
function unless there is any serious objection. There is no reason
to pollute this file for awful things like this. I know it was done
because of performance reason in the past (such as CPU ticker,
performance measurement, I guess.) but it is rarely used now and it
does only one serious thing, i.e., early entropy seeding. There is a
minor usage in SCTP for debugging but moving it to machdep.c won't
hurt much, I think. What do you think?
Jung-uk Kim
_______________________________________________
svn-src-head@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
