Skip site navigation (1)Skip section navigation (2)
Date:      Sat, 03 Nov 2001 06:23:19 +0900 (JST)
From:      Mitsuru IWASAKI <iwasaki@jp.FreeBSD.org>
To:        jhb@FreeBSD.org
Cc:        cvs-all@FreeBSD.org, cvs-committers@FreeBSD.org
Subject:   Re: cvs commit: src/sys/conf files src/sys/dev/acpica acpi.c src
Message-ID:  <20011103.062319.41627011.iwasaki@jp.FreeBSD.org>
In-Reply-To: <XFMail.011102010133.jhb@FreeBSD.org>
References:  <200111011634.fA1GY8M18688@freefall.freebsd.org> <XFMail.011102010133.jhb@FreeBSD.org>

next in thread | previous in thread | raw e-mail | index | archive | help
Hi,

> On 01-Nov-01 Mitsuru IWASAKI wrote:
> > iwasaki     2001/11/01 08:34:08 PST
> > 
> >   Modified files:
> >     sys/conf             files 
> >     sys/dev/acpica       acpi.c 
> >     sys/dev/syscons      syscons.c 
> >     sys/i386/apm         apm.c 
> >     sys/i386/isa         clock.c 
> >     sys/sys              kernel.h 
> >   Added files:
> >     sys/kern             subr_power.c 
> >     sys/sys              power.h 
> >   Log:
> >   Some fix for the recent apm module changes.
> >    - Now that apm loadable module can inform its existence to other kernel
> >      components  (e.g. i386/isa/clock.c:startrtclock()'s TCS hack).
> >    - Exchange priority of SI_SUB_CPU and SI_SUB_KLD for above purpose.
> >    - Add simple arbitration mechanism for APM vs. ACPI.  This prevents
> >      the kernel enables both of them.
> >    - Remove obsolete `#ifdef DEV_APM' related code.
> >    - Add abstracted interface for Powermanagement operations.  Public apm(4)
> >      functions, such as apm_suspend(), should be replaced new interfaces.
> >      Currently only power_pm_suspend (successor of apm_suspend) is
> > implemented.
> >   
> >   Reviewed by:        peter, arch@ and audit@
> 
> Hmm, I actually really don't like swapping those SYSINIT's.  Probably what you
> should do is do whwat the alpha does and initalize the timecounters during the
> SI_SUB_CLOCKS sysinit.  If SI_SUB_CLOCKS is before SI_SUB_KLD, then you should
> create a SI_SUB_TC after SI_SUB_KLD and initialize timecounters there. 

Ok, like this?

Index: i386/isa/clock.c
===================================================================
RCS file: /home/ncvs/src/sys/i386/isa/clock.c,v
retrieving revision 1.178
diff -u -r1.178 clock.c
--- i386/isa/clock.c	1 Nov 2001 16:33:36 -0000	1.178
+++ i386/isa/clock.c	2 Nov 2001 12:07:27 -0000
@@ -788,8 +788,6 @@
 	}
 
 	set_timer_freq(timer_freq, hz);
-	i8254_timecounter.tc_frequency = timer_freq;
-	tc_init(&i8254_timecounter);
 
 #ifndef CLK_USE_TSC_CALIBRATION
 	if (tsc_freq != 0) {
@@ -814,37 +812,6 @@
 			printf("TSC clock: %u Hz (Method B)\n", tsc_freq);
 #endif
 	}
-
-#if !defined(SMP)
-	/*
-	 * We can not use the TSC in SMP mode, until we figure out a
-	 * cheap (impossible), reliable and precise (yeah right!)  way
-	 * to synchronize the TSCs of all the CPUs.
-	 * Curse Intel for leaving the counter out of the I/O APIC.
-	 */
-
-	/*
-	 * We can not use the TSC if we support APM. Precise timekeeping
-	 * on an APM'ed machine is at best a fools pursuit, since 
-	 * any and all of the time spent in various SMM code can't 
-	 * be reliably accounted for.  Reading the RTC is your only
-	 * source of reliable time info.  The i8254 looses too of course
-	 * but we need to have some kind of time...
-	 * We don't know at this point whether APM is going to be used
-	 * or not, nor when it might be activated.  Play it safe.
-	 */
-	if (power_pm_get_type() == POWER_PM_TYPE_APM) {
-		if (bootverbose)
-			printf("TSC initialization skipped: APM enabled.\n");
-		return;
-	}
-
-	if (tsc_present && tsc_freq != 0 && !tsc_is_broken) {
-		tsc_timecounter.tc_frequency = tsc_freq;
-		tc_init(&tsc_timecounter);
-	}
-
-#endif /* !defined(SMP) */
 }
 
 /*
@@ -992,6 +959,34 @@
 	int apic_8254_trial;
 	void *clkdesc;
 #endif /* APIC_IO */
+
+	i8254_timecounter.tc_frequency = timer_freq;
+	tc_init(&i8254_timecounter);
+#if !defined(SMP)
+	/*
+	 * We can not use the TSC in SMP mode, until we figure out a
+	 * cheap (impossible), reliable and precise (yeah right!)  way
+	 * to synchronize the TSCs of all the CPUs.
+	 * Curse Intel for leaving the counter out of the I/O APIC.
+	 */
+
+	/*
+	 * We can not use the TSC if we support APM. Precise timekeeping
+	 * on an APM'ed machine is at best a fools pursuit, since 
+	 * any and all of the time spent in various SMM code can't 
+	 * be reliably accounted for.  Reading the RTC is your only
+	 * source of reliable time info.  The i8254 looses too of course
+	 * but we need to have some kind of time...
+	 * We don't know at this point whether APM is going to be used
+	 * or not, nor when it might be activated.  Play it safe.
+	 */
+	if (power_pm_get_type() != POWER_PM_TYPE_APM &&
+	    tsc_present && tsc_freq != 0 && !tsc_is_broken) {
+		tsc_timecounter.tc_frequency = tsc_freq;
+		tc_init(&tsc_timecounter);
+	}
+
+#endif /* !defined(SMP) */
 
 	if (statclock_disable) {
 		/*
Index: sys/kernel.h
===================================================================
RCS file: /home/ncvs/src/sys/sys/kernel.h,v
retrieving revision 1.96
diff -u -r1.96 kernel.h
--- sys/kernel.h	1 Nov 2001 16:34:06 -0000	1.96
+++ sys/kernel.h	2 Nov 2001 11:19:03 -0000
@@ -119,8 +119,8 @@
 	SI_SUB_WITNESS		= 0x1A80000,	/* witness initialization */
 	SI_SUB_LOCK		= 0x1B00000,	/* lockmgr locks */
 	SI_SUB_EVENTHANDLER	= 0x1C00000,	/* eventhandler init */
-	SI_SUB_KLD		= 0x2000000,	/* KLD and module setup */
-	SI_SUB_CPU		= 0x2100000,	/* CPU resource(s)*/
+	SI_SUB_CPU		= 0x2000000,	/* CPU resource(s)*/
+	SI_SUB_KLD		= 0x2100000,	/* KLD and module setup */
 	SI_SUB_INTRINSIC	= 0x2200000,	/* proc 0*/
 	SI_SUB_VM_CONF		= 0x2300000,	/* config VM, set limits*/
 	SI_SUB_RUN_QUEUE	= 0x2400000,	/* set up run queue*/

But I won't commit this unless weird problem is reported :-)
However, if someone commit this I don't complain about it.

> Logically, we need the CPU up and initialized before we start trying to figure
> out what modules we have and how to init them.  At least IMO.

Hmmm, really?  I checked all of SI_SUB_KLD sysinits and modevent functions
briefly, but I didn't see any dependency on SI_SUB_CPU sysinits.
I believe that kernel linker should not depend on CPU initialization...

Thanks

To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe cvs-all" in the body of the message




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