Skip site navigation (1)Skip section navigation (2)
Date:      Wed, 22 Oct 2008 12:41:19 -0400
From:      Jung-uk Kim <jkim@FreeBSD.org>
To:        John Baldwin <jhb@FreeBSD.org>
Cc:        svn-src-head@FreeBSD.org, svn-src-all@FreeBSD.org, src-committers@FreeBSD.org
Subject:   Re: svn commit: r184146 - in head/sys: amd64/amd64 amd64/include i386/i386 i386/include
Message-ID:  <200810221241.22455.jkim@FreeBSD.org>
In-Reply-To: <200810221202.40803.jhb@freebsd.org>
References:  <200810220001.m9M01rLq011948@svn.freebsd.org> <200810221141.52036.jkim@FreeBSD.org> <200810221202.40803.jhb@freebsd.org>

next in thread | previous in thread | raw e-mail | index | archive | help
On Wednesday 22 October 2008 12:02 pm, John Baldwin wrote:
> On Wednesday 22 October 2008 11:41:36 am Jung-uk Kim wrote:
> > On Wednesday 22 October 2008 11:01 am, John Baldwin wrote:
> > > On Tuesday 21 October 2008 08:01:53 pm Jung-uk Kim wrote:
> > > > Author: jkim
> > > > Date: Wed Oct 22 00:01:53 2008
> > > > New Revision: 184146
> > > > URL: http://svn.freebsd.org/changeset/base/184146
> > > >
> > > > Log:
> > > >   Set kern.timecounter.invariant_tsc to 1 for AMD CPU family
> > > > 10h and higher even if BIOS does not advertise it.
> > > >
> > > > Modified:
> > > >   head/sys/amd64/amd64/identcpu.c
> > > >   head/sys/amd64/include/specialreg.h
> > > >   head/sys/i386/i386/identcpu.c
> > > >   head/sys/i386/include/specialreg.h
> > > >
> > > > Modified: head/sys/amd64/amd64/identcpu.c
> > >
> > > ===============================================================
> > >==== ===========
> > >
> > > > --- head/sys/amd64/amd64/identcpu.c	Tue Oct 21 23:36:28
> > > > 2008	(r184145) +++ head/sys/amd64/amd64/identcpu.c	Wed Oct 22
> > > > 00:01:53 2008	(r184146) @@ -348,7 +348,9 @@
> > > > printcpuinfo(void) cpu_feature &= ~CPUID_HTT;
> > > >
> > > >  			if (!tsc_is_invariant &&
> > > > -			    (amd_pminfo & AMDPM_TSC_INVARIANT)) {
> > > > +			    (strcmp(cpu_vendor, "AuthenticAMD") == 0 &&
> > > > +			    ((amd_pminfo & AMDPM_TSC_INVARIANT) != 0 ||
> > > > +			    AMD64_CPU_FAMILY(cpu_id) >= 0x10))) {
> > > >  				tsc_is_invariant = 1;
> > > >  				printf("\n  P-state invariant TSC");
> > > >  			}
> > >
> > > Perhaps a simpler approach might be to just set
> > > AMDPM_TSC_INVARIANT for AMD64_CPU_FAMILY(cpu_id) >= 0x10?
> >
> > It seems Athlon X2 (Family 0Fh, Model 6Bh, Stepping G2) that I
> > bought also has invariant TSC although it is not advertised by
> > CPUID.  If BIOS manufacturer decide to set the bit later, it
> > should be honored IMO.
>
> Err, yeah, my suggestion is something like this:
>
> 	/*
> 	 * These CPUs have an invariant TSC even though they don't
> 	 * always advertise it.
> 	 */
> 	if (strcmp(cpu_vendor, "AuthenticAMD") == 0 &&
> 	    AMD64_CPU_FAMILY(cpu_id) >= 0x10)
> 		amd_pminfo |= AMDPM_TSC_INVARIANT;
>
> 	if (!tsc_is_invariant &&
> 	    (amd_pminfo & AMDPM_TSC_INVARIANT)) {
> 		tsc_is_invariant;
> 		printf(...);
> 	}
>
> That will still support any other CPUs that grow the CPUID flag at
> some point, it just fakes the CPUID flag for CPUs that we know have
> it but just aren't advertising it.

Hmm, interesting idea.  But please read this:

http://docs.freebsd.org/cgi/mid.cgi?200810211605.46927.jkim

If Linux people are actually correct, then:

	if (amd_pminfo & AMDPM_TSC_INVARIANT)
		smp_tsc = 1;

I like to use it later if it is really true.

Besides, I hate to set/clear CPUIDs like this.  For that, I believe we 
should have a single feature flag for all amd64/i386 CPUs regardless 
of vendors, e.g., CPU_CAP_LONGMODE, CPU_CAP_SSE3, 
CPU_CAP_INVARIANT_TSC, etc. and we should leave the feature variables 
as is.

Jung-uk Kim



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200810221241.22455.jkim>