>Number:         36839
>Category:       port-i386
>Synopsis:       x86/x86/tsc.c should be switched to using MI kern_cctr.c
>Confidential:   no
>Severity:       non-critical
>Priority:       low
>Responsible:    port-i386-maintainer
>State:          open
>Class:          change-request
>Submitter-Id:   net
>Arrival-Date:   Sun Aug 26 05:45:00 +0000 2007
>Originator:     Izumi Tsutsui
>Release:        NetBSD 4.99.25
>Organization:
>Environment:
NetBSD-current around 20070822
Architecture: i386
Machine: i386

>Description:
When I was working on MI todr(9) and timecounter(9) support for alpha,
Frank Kardel (kardel@) suggested that we should split x86/x86/tsc.c
int an MD part (x86/x86/tsc.c) and an MI part (kern/kern_cctr.c):
http://mail-index.netbsd.org/port-alpha/2007/02/24/0004.html
and then I adapted the patch for alpha's PCC counters:
http://mail-index.netbsd.org/port-alpha/2007/07/08/0001.html

I've commit MI files (sys/kern/kern_cctr.c and sys/sys/cctr.h)
before commiting timecounter(9) support for alpha:
http://mail-index.netbsd.org/source-changes/2007/07/21/0012.html
http://mail-index.netbsd.org/source-changes/2007/07/21/0013.html
but changes for x86/x86/tsc.c (and amd64 and i386 MD part) have not
been committed due to lack of testers, so I'd file a PR about
these changes for a record.

>How-To-Repeat:
By code inspection.

>Fix:
Here is a diff based on the above Frank's patch (with some tweaks by me):

Index: arch/amd64/amd64/ipifuncs.c
===================================================================
RCS file: /cvsroot/src/sys/arch/amd64/amd64/ipifuncs.c,v
retrieving revision 1.9
diff -u -r1.9 ipifuncs.c
--- arch/amd64/amd64/ipifuncs.c	21 Mar 2007 06:36:43 -0000	1.9
+++ arch/amd64/amd64/ipifuncs.c	26 Aug 2007 04:59:12 -0000
@@ -83,7 +83,7 @@
 void (*ipifunc[X86_NIPI])(struct cpu_info *) =
 {
 	x86_64_ipi_halt,
-	tsc_calibrate_cpu,
+	cc_calibrate_cpu,
 	x86_64_ipi_flush_fpu,
 	x86_64_ipi_synch_fpu,
 	pmap_do_tlb_shootdown,
Index: arch/amd64/conf/files.amd64
===================================================================
RCS file: /cvsroot/src/sys/arch/amd64/conf/files.amd64,v
retrieving revision 1.41
diff -u -r1.41 files.amd64
--- arch/amd64/conf/files.amd64	6 Aug 2007 06:40:41 -0000	1.41
+++ arch/amd64/conf/files.amd64	26 Aug 2007 04:59:12 -0000
@@ -141,6 +141,7 @@
 file	arch/x86/isa/clock.c			isa
 
 # TSC timecounter support
+file	kern/kern_cctr.c
 file	arch/x86/x86/tsc.c
 
 # attribute used to represent the "keyboard controller"
Index: arch/amd64/include/cpu.h
===================================================================
RCS file: /cvsroot/src/sys/arch/amd64/include/cpu.h,v
retrieving revision 1.24
diff -u -r1.24 cpu.h
--- arch/amd64/include/cpu.h	21 May 2007 08:10:39 -0000	1.24
+++ arch/amd64/include/cpu.h	26 Aug 2007 04:59:12 -0000
@@ -55,14 +55,14 @@
 #include <sys/device.h>
 #include <sys/simplelock.h>
 #include <sys/cpu_data.h>
-#include <sys/cc_microtime.h>
+#include <sys/cctr.h>
 
 struct cpu_info {
 	struct device *ci_dev;
 	struct cpu_info *ci_self;
 	void *ci_self200;		/* self + 0x200, see lock_stubs.S */
 	struct cpu_data ci_data;	/* MI per-cpu data */
-	struct cc_microtime_state ci_cc;/* cc_microtime state */
+	struct cctr_state ci_cc;	/* cycle counter state */
 	struct cpu_info *ci_next;
 
 	struct lwp *ci_curlwp;
Index: arch/i386/conf/files.i386
===================================================================
RCS file: /cvsroot/src/sys/arch/i386/conf/files.i386,v
retrieving revision 1.312
diff -u -r1.312 files.i386
--- arch/i386/conf/files.i386	8 Jul 2007 01:13:26 -0000	1.312
+++ arch/i386/conf/files.i386	26 Aug 2007 04:59:12 -0000
@@ -260,6 +260,7 @@
 file	arch/x86/isa/clock.c		isa
 
 # TSC support
+file	kern/kern_cctr.c		i586_cpu | i686_cpu
 file	arch/x86/x86/tsc.c		i586_cpu | i686_cpu
 
 # Numeric Processing Extension; Math Co-processor
Index: arch/i386/i386/ipifuncs.c
===================================================================
RCS file: /cvsroot/src/sys/arch/i386/i386/ipifuncs.c,v
retrieving revision 1.17
diff -u -r1.17 ipifuncs.c
--- arch/i386/i386/ipifuncs.c	17 May 2007 14:51:20 -0000	1.17
+++ arch/i386/i386/ipifuncs.c	26 Aug 2007 04:59:12 -0000
@@ -88,7 +88,7 @@
 {
 	i386_ipi_halt,
 #if defined(I586_CPU) || defined(I686_CPU)
-	tsc_calibrate_cpu,	/* keep cycle counters synchronized */
+	cc_calibrate_cpu,	/* keep cycle counters synchronized */
 #else
 	0,
 #endif
Index: arch/i386/include/cpu.h
===================================================================
RCS file: /cvsroot/src/sys/arch/i386/include/cpu.h,v
retrieving revision 1.143
diff -u -r1.143 cpu.h
--- arch/i386/include/cpu.h	9 Jul 2007 20:52:17 -0000	1.143
+++ arch/i386/include/cpu.h	26 Aug 2007 04:59:12 -0000
@@ -57,7 +57,7 @@
 
 #include <sys/device.h>
 #include <sys/cpu_data.h>
-#include <sys/cc_microtime.h>
+#include <sys/cctr.h>
 
 #include <lib/libkern/libkern.h>	/* offsetof */
 
@@ -83,7 +83,7 @@
 	cpuid_t ci_cpuid;		/* our CPU ID */
 	u_int ci_apicid;		/* our APIC ID */
 	struct cpu_data ci_data;	/* MI per-cpu data */
-	struct cc_microtime_state ci_cc;/* cc_microtime state */
+	struct cctr_state ci_cc;	/* cycle counter state */
 
 	/*
 	 * Private members.
Index: arch/x86/include/cpu_counter.h
===================================================================
RCS file: /cvsroot/src/sys/arch/x86/include/cpu_counter.h,v
retrieving revision 1.1
diff -u -r1.1 cpu_counter.h
--- arch/x86/include/cpu_counter.h	7 Jul 2007 17:38:27 -0000	1.1
+++ arch/x86/include/cpu_counter.h	26 Aug 2007 04:59:12 -0000
@@ -43,6 +43,8 @@
  * x86 common functions for CPU counter.
  */
 
+#define cc_calibrate_mp(ci)	x86_broadcast_ipi(X86_IPI_MICROSET)
+
 static __inline uint64_t
 cpu_counter(void)
 {
Index: arch/x86/x86/tsc.c
===================================================================
RCS file: /cvsroot/src/sys/arch/x86/x86/tsc.c,v
retrieving revision 1.10
diff -u -r1.10 tsc.c
--- arch/x86/x86/tsc.c	16 Nov 2006 01:32:39 -0000	1.10
+++ arch/x86/x86/tsc.c	26 Aug 2007 04:59:12 -0000
@@ -88,8 +88,6 @@
 #include "opt_multiprocessor.h"
 #ifdef i386
 #include "opt_enhanced_speedstep.h"
-#endif
-#ifdef i386
 #include "opt_powernow_k7.h"
 #endif
 #include "opt_powernow_k8.h"
@@ -100,44 +98,21 @@
 #include <sys/time.h>
 #include <sys/timetc.h>
 #include <sys/kernel.h>
-#include <sys/power.h>
 #include <sys/reboot.h>	/* XXX for bootverbose */
 #include <machine/cpu.h>
 #include <machine/cpu_counter.h>
-#include <x86/x86/tsc.h>
 #include <machine/specialreg.h>
+#include <x86/x86/tsc.h>
 
 uint64_t	tsc_freq;
 u_int		tsc_present;
 int		tsc_is_broken = 0;
-
-static int64_t tsc_cal_val;  /* last calibrate time stamp */
-
-static timecounter_get_t tsc_get_timecount;
-static timecounter_pps_t tsc_calibrate;
-
-void tsc_calibrate_cpu(struct cpu_info *);
-
-static struct timecounter tsc_timecounter = {
-	tsc_get_timecount,	/* get_timecount */
-	tsc_calibrate,		/* once per second - used to calibrate cpu TSC */
- 	~0u,			/* counter_mask */
-	0,			/* frequency */
-	 "TSC",			/* name */
-#if (defined(ENHANCED_SPEEDSTEP) || defined(POWERNOW_K7) || defined(POWERNOW_K8))
-	-100,			/* don't pick TSC automatically */
-				/* if frequency changes might affect TSC */
-#else
-	800,			/* quality (adjusted in code) */
-#endif
-	NULL,
-	NULL,
-};
+int		tsc_qual = -100000;
 
 void
 init_TSC(void)
 {
-	u_int64_t tscval[2];
+	uint64_t tscval[2];
 
 	if (cpu_feature & CPUID_TSC)
 		tsc_present = 1;
@@ -157,6 +132,12 @@
 	} while (tscval[1] < tscval[0]);
 
 	tsc_freq = 10 * (tscval[1] - tscval[0]);
+#if defined(ENHANCED_SPEEDSTEP) || defined(POWERNOW_K7) || defined(POWERNOW_K8)
+	tsc_qual = -100;	/* don't pick TSC automatically */
+				/* if frequency changes might affect TSC */
+#else
+	tsc_qual = 800;		/* quality (adjusted in code) */
+#endif
 	if (bootverbose)
 		printf("TSC clock: %" PRId64 " Hz\n", tsc_freq);
 }
@@ -164,164 +145,8 @@
 void
 init_TSC_tc(void)
 {
-	if (tsc_present && tsc_freq != 0 && !tsc_is_broken) {
-		tsc_timecounter.tc_frequency = tsc_freq;
-		tc_init(&tsc_timecounter);
-	}
-}
-
-/* XXX make tsc_timecounter.tc_frequency settable by sysctl() */
-
-/*
- * pick up tick count scaled to reference tick count
- */
-static u_int
-tsc_get_timecount(struct timecounter *tc)
-{
-	struct cpu_info *ci = curcpu();
-	int64_t rcc, cc;
-	u_int gen;
-
-	if (ci->ci_cc.cc_denom == 0) {
-		/*
-		 * This is our first time here on this CPU.  Just
-		 * start with reasonable initial values.
-		 */
-	        ci->ci_cc.cc_cc    = cpu_counter32();
-		ci->ci_cc.cc_val   = 0;
-		if (ci->ci_cc.cc_gen == 0)
-			ci->ci_cc.cc_gen++;
-
-		ci->ci_cc.cc_denom = cpu_frequency(ci);
-		if (ci->ci_cc.cc_denom == 0)
-			ci->ci_cc.cc_denom = tsc_freq;
-		ci->ci_cc.cc_delta = ci->ci_cc.cc_denom;
-	}
 
-	/* read counter and re-read when the re-calibration
-	   strikes inbetween */
-	do {
-		/* pick up current generation number */
-		gen = ci->ci_cc.cc_gen;
-
-		/* determine local delta ticks */
-		cc = cpu_counter32() - ci->ci_cc.cc_cc;
-		if (cc < 0)
-			cc += 0x100000000LL;
-
-		/* scale to primary */
-		rcc = (cc * ci->ci_cc.cc_delta) / ci->ci_cc.cc_denom
-			+ ci->ci_cc.cc_val;
-	} while (gen == 0 || gen != ci->ci_cc.cc_gen);
-
-	return rcc;
-}
-
-/*
- * called once per second via the pps callback
- * for the calibration of the TSC counters.
- * it is called only for the PRIMARY cpu. all
- * other cpus are called via a broadcast IPI
- */
-static void
-tsc_calibrate(struct timecounter *tc)
-{
-	struct cpu_info *ci = curcpu();
-	
-	/* pick up reference ticks */
-	tsc_cal_val = cpu_counter32();
-
-#if defined(MULTIPROCESSOR)
-	x86_broadcast_ipi(X86_IPI_MICROSET);
-#endif
-
-	tsc_calibrate_cpu(ci);
-}
-
-/*
- * This routine is called about once per second directly by the master
- * processor and via an interprocessor interrupt for other processors.
- * It determines the CC frequency of each processor relative to the
- * master clock and the time this determination is made.  These values
- * are used by tsc_get_timecount() to interpolate the ticks between
- * timer interrupts.  Note that we assume the kernel variables have
- * been zeroed early in life.
- */
-void
-tsc_calibrate_cpu(struct cpu_info *ci)
-{
-	u_int   gen;
-	int64_t val;
-	int64_t delta, denom;
-	int s;
-#ifdef TIMECOUNTER_DEBUG
-	int64_t factor, old_factor;
-#endif
-	val = tsc_cal_val;
-
-	s = splhigh();
-	/* create next generation number */
-	gen = ci->ci_cc.cc_gen;
-	gen++;
-	if (gen == 0)
-		gen++;
-	/* update in progress */
-	ci->ci_cc.cc_gen = 0;
-
-	denom = ci->ci_cc.cc_cc;
-	ci->ci_cc.cc_cc = cpu_counter32();
-
-	if (ci->ci_cc.cc_denom == 0) {
-		/*
-		 * This is our first time here on this CPU.  Just
-		 * start with reasonable initial values.
-		 */
-		ci->ci_cc.cc_val = val;
-		ci->ci_cc.cc_denom = cpu_frequency(ci);
-		if (ci->ci_cc.cc_denom == 0)
-			ci->ci_cc.cc_denom = tsc_freq;
-		ci->ci_cc.cc_delta = ci->ci_cc.cc_denom;
-		ci->ci_cc.cc_gen = gen;
-		splx(s);
-		return;
+	if (tsc_present && tsc_freq != 0 && !tsc_is_broken) {
+		(void)cc_init(tsc_freq, "TSC", tsc_qual);
 	}
-
-#ifdef TIMECOUNTER_DEBUG
-	old_factor = (ci->ci_cc.cc_delta * 1000 ) / ci->ci_cc.cc_denom;
-#endif
-
-	/* local ticks per period */
-	denom = ci->ci_cc.cc_cc - denom;
-	if (denom < 0)
-		denom += 0x100000000LL;
-
-	ci->ci_cc.cc_denom = denom;
-
-	/* reference ticks per period */
-	delta = val - ci->ci_cc.cc_val;
-	if (delta < 0)
-		delta += 0x100000000LL;
-
-	ci->ci_cc.cc_val = val;
-	ci->ci_cc.cc_delta = delta;
-	
-	/* publish new generation number */
-	ci->ci_cc.cc_gen = gen;
-	splx(s);
-
-#ifdef TIMECOUNTER_DEBUG
-	factor = (delta * 1000) / denom - old_factor;
-	if (factor < 0)
-		factor = -factor;
-
-	if (factor > old_factor / 10)
-		printf("tsc_calibrate_cpu[%lu]: 10%% exceeded - delta %"
-		       PRId64 ", denom %" PRId64 ", factor %" PRId64
-		       ", old factor %" PRId64"\n", ci->ci_cpuid,
-		       delta, denom, (delta * 1000) / denom, old_factor);
-#if 0
-	printf("tsc_calibrate_cpu[%lu]: delta %" PRId64
-	       ", denom %" PRId64 ", factor %" PRId64 "\n", ci->ci_cpuid, delta, denom, (delta * 1000) / denom);
-#endif
-#endif /* TIMECOUNTER_DEBUG */
 }

---
Note <sys/cc_microtime.h> can be removed once the diff is applied.

---
Izumi Tsutsui