PMTMR running too fast

PMTMR running too fast

[Ian Campbell]

[-- Attachment #1: Type: text/plain, Size: 1552 bytes --]

Hi,

In older kernels arch/i386/kernel/timers/timer_pm.c:verify_pmtmr_rate
contained a check for sensible PMTMR rate and disabled that clocksource
if it was found to be out of spec[0]. This check seems to have been lost
in the transition to drivers/clocksource/acpi_pm.c, the removal is in
61743fe445213b87fb55a389c8d073785323ca3e "Time: i386 Conversion - part
4: Remove Old timer_opts Code"[1] and the check is not present in the
replacement 5d0cf410e94b1f1ff852c3f210d22cc6c5a27ffa "Time: i386
Clocksource Drivers"[2].

Is there a specific reason the check was removed (I couldn't see on in
the archives) or was it simply overlooked? Without it I need to pass
clocksource=tsc to have 2.6.18 work correctly on an older K6 system with
an Aladdin chipset (will dig out the precise details if required). Would
a patch to reintroduce the check be acceptable or would some sort of
blacklist based solution be more acceptable?

Cheers,
Ian.

[0] "PM-Timer running at invalid rate: 200% of normal - aborting." from
http://www.kernel.org/git/?p=linux/kernel/git/tglx/history.git;a=commit;h=6d58b1286c7ac88741374c158867f564e602b288
see also http://bugme.osdl.org/show_bug.cgi?id=2375
[1]
http://www.kernel.org/git/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=61743fe445213b87fb55a389c8d073785323ca3e
[2]
http://www.kernel.org/git/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=5d0cf410e94b1f1ff852c3f210d22cc6c5a27ffa

-- 
Ian Campbell

Any time things appear to be going better, you have overlooked something.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: PMTMR running too fast

[john stultz]

On Sun, 2006-12-03 at 13:50 +0000, Ian Campbell wrote:
> In older kernels arch/i386/kernel/timers/timer_pm.c:verify_pmtmr_rate
> contained a check for sensible PMTMR rate and disabled that clocksource
> if it was found to be out of spec[0]. This check seems to have been lost
> in the transition to drivers/clocksource/acpi_pm.c, the removal is in
> 61743fe445213b87fb55a389c8d073785323ca3e "Time: i386 Conversion - part
> 4: Remove Old timer_opts Code"[1] and the check is not present in the
> replacement 5d0cf410e94b1f1ff852c3f210d22cc6c5a27ffa "Time: i386
> Clocksource Drivers"[2].

Fedora has a bug covering this:
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=211902


> Is there a specific reason the check was removed (I couldn't see on in
> the archives) or was it simply overlooked? Without it I need to pass
> clocksource=tsc to have 2.6.18 work correctly on an older K6 system with
> an Aladdin chipset (will dig out the precise details if required). Would
> a patch to reintroduce the check be acceptable or would some sort of
> blacklist based solution be more acceptable?

If I recall correctly, it was pulled because there was some question as
to if it was actually needed (x86_64 didn't need it) and it slows down
the boot time (although not by much). 

I'm fine just re-adding it. Although if the number of affected systems
are small we could just blacklist it (Ian, mind sending dmidecode
output?).

Andi, your thoughts?

thanks
-john

Re: PMTMR running too fast

[Ian Campbell]

[-- Attachment #1.1: Type: text/plain, Size: 1725 bytes --]

On Mon, 2006-12-04 at 11:19 -0800, john stultz wrote:
> On Sun, 2006-12-03 at 13:50 +0000, Ian Campbell wrote:
> > In older kernels arch/i386/kernel/timers/timer_pm.c:verify_pmtmr_rate
> > contained a check for sensible PMTMR rate and disabled that clocksource
> > if it was found to be out of spec[0]. This check seems to have been lost
> > in the transition to drivers/clocksource/acpi_pm.c, the removal is in
> > 61743fe445213b87fb55a389c8d073785323ca3e "Time: i386 Conversion - part
> > 4: Remove Old timer_opts Code"[1] and the check is not present in the
> > replacement 5d0cf410e94b1f1ff852c3f210d22cc6c5a27ffa "Time: i386
> > Clocksource Drivers"[2].
> 
> Fedora has a bug covering this:
> https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=211902

> > Is there a specific reason the check was removed (I couldn't see on in
> > the archives) or was it simply overlooked? Without it I need to pass
> > clocksource=tsc to have 2.6.18 work correctly on an older K6 system with
> > an Aladdin chipset (will dig out the precise details if required). Would
> > a patch to reintroduce the check be acceptable or would some sort of
> > blacklist based solution be more acceptable?
> 
> If I recall correctly, it was pulled because there was some question as
> to if it was actually needed (x86_64 didn't need it) and it slows down
> the boot time (although not by much). 
> 
> I'm fine just re-adding it. Although if the number of affected systems
> are small we could just blacklist it (Ian, mind sending dmidecode
> output?).

dmidecode.txt is attached.

Cheers,
Ian.

-- 
Ian Campbell

Felson's Law:
	To steal ideas from one person is plagiarism; to steal from
	many is research.

[-- Attachment #1.2: dmidecode.txt --]
[-- Type: text/plain, Size: 9779 bytes --]

# dmidecode 2.6
SMBIOS 2.2 present.
32 structures occupying 798 bytes.
Table at 0x000F0800.
Handle 0x0000
	DMI type 0, 19 bytes.
	BIOS Information
		Vendor: Award Software International, Inc.
		Version: 4.51 PG
		Release Date: 09/19/00
		Address: 0xE0000
		Runtime Size: 128 kB
		ROM Size: 128 kB
		Characteristics:
			ISA is supported
			PCI is supported
			PNP is supported
			APM is supported
			BIOS is upgradeable
			BIOS shadowing is allowed
			ESCD support is available
			Boot from CD is supported
			Selectable boot is supported
			BIOS ROM is socketed
			EDD is supported
			5.25"/360 KB floppy services are supported (int 13h)
			5.25"/1.2 MB floppy services are supported (int 13h)
			3.5"/720 KB floppy services are supported (int 13h)
			3.5"/2.88 MB floppy services are supported (int 13h)
			Print screen service is supported (int 5h)
			8042 keyboard services are supported (int 9h)
			Serial services are supported (int 14h)
			Printer services are supported (int 17h)
			CGA/mono video services are supported (int 10h)
			ACPI is supported
			USB legacy is supported
			AGP is supported
			LS-120 boot is supported
			ATAPI Zip drive boot is supported
Handle 0x0001
	DMI type 1, 25 bytes.
	System Information
		Manufacturer:  
		Product Name:  
		Version:  
		Serial Number:  
		UUID: Not Settable
		Wake-up Type: Reserved
Handle 0x0002
	DMI type 2, 8 bytes.
	Base Board Information
		Manufacturer:  
		Product Name: ALADDIN5
		Version:  
		Serial Number:  
Handle 0x0003
	DMI type 3, 13 bytes.
	Chassis Information
		Manufacturer:  
		Type: Unknown
		Lock: Not Present
		Version:  
		Serial Number:  
		Asset Tag:  
		Boot-up State: Unknown
		Power Supply State: Unknown
		Thermal State: Unknown
		Security Status: Unknown
Handle 0x0004
	DMI type 4, 32 bytes.
	Processor Information
		Socket Designation: Socket 7
		Type: Central Processor
		Family: K5
		Manufacturer: AMD
		ID: 91 05 00 00 BF 21 80 00
		Signature: Family 5, Model 9, Stepping 1
		Flags:
			FPU (Floating-point unit on-chip)
			VME (Virtual mode extension)
			DE (Debugging extension)
			PSE (Page size extension)
			TSC (Time stamp counter)
			MSR (Model specific registers)
			MCE (Machine check exception)
			CX8 (CMPXCHG8 instruction supported)
			PGE (Page global enable)
			MMX (MMX technology supported)
		Version: AMD-K6
		Voltage: 2.4 V
		External Clock: 100 MHz
		Max Speed: 500 MHz
		Current Speed: 450 MHz
		Status: Populated, Enabled
		Upgrade: ZIF Socket
		L1 Cache Handle: 0x0000
		L2 Cache Handle: 0x0000
		L3 Cache Handle: 0x0000
Handle 0x0005
	DMI type 5, 28 bytes.
	Memory Controller Information
		Error Detecting Method: 8-bit Parity
		Error Correcting Capabilities:
			None
		Supported Interleave: One-way Interleave
		Current Interleave: One-way Interleave
		Maximum Memory Module Size: 512 MB
		Maximum Total Memory Size: 3072 MB
		Supported Speeds:
			70 ns
			60 ns
		Supported Memory Types:
			Standard
			FPM
			EDO
			SDRAM
		Memory Module Voltage: 5.0 V 3.3 V
		Associated Memory Slots: 6
			0x0006
			0x0007
			0x0008
			0x0009
			0x000A
			0x000B
		Enabled Error Correcting Capabilities: None
Handle 0x0006
	DMI type 6, 12 bytes.
	Memory Module Information
		Socket Designation: A0
		Bank Connections: 0 1
		Current Speed: 60 ns
		Type: Parity ECC DIMM SDRAM
		Installed Size: 128 MB (Single-bank Connection)
		Enabled Size: 128 MB (Single-bank Connection)
		Error Status: OK
Handle 0x0007
	DMI type 6, 12 bytes.
	Memory Module Information
		Socket Designation: A1
		Bank Connections: 0 1
		Current Speed: 60 ns
		Type: Parity ECC DIMM SDRAM
		Installed Size: 128 MB (Single-bank Connection)
		Enabled Size: 128 MB (Single-bank Connection)
		Error Status: OK
Handle 0x0008
	DMI type 6, 12 bytes.
	Memory Module Information
		Socket Designation: A2
		Bank Connections: 2 3
		Current Speed: 60 ns
		Type: FPM Parity ECC
		Installed Size: 64 MB (Single-bank Connection)
		Enabled Size: 64 MB (Single-bank Connection)
		Error Status: OK
Handle 0x0009
	DMI type 6, 12 bytes.
	Memory Module Information
		Socket Designation: A3
		Bank Connections: 2 3
		Current Speed: 60 ns
		Type: FPM Parity ECC
		Installed Size: 64 MB (Single-bank Connection)
		Enabled Size: 64 MB (Single-bank Connection)
		Error Status: OK
Handle 0x000A
	DMI type 6, 12 bytes.
	Memory Module Information
		Socket Designation: A4
		Bank Connections: 4 5
		Current Speed: 60 ns
		Type: Unknown
		Installed Size: Not Installed
		Enabled Size: Not Installed
		Error Status: OK
Handle 0x000B
	DMI type 6, 12 bytes.
	Memory Module Information
		Socket Designation: A5
		Bank Connections: 4 5
		Current Speed: 60 ns
		Type: Unknown
		Installed Size: Not Installed
		Enabled Size: Not Installed
		Error Status: OK
Handle 0x000C
	DMI type 7, 19 bytes.
	Cache Information
		Socket Designation: Internal Cache
		Configuration: Enabled, Not Socketed, Level 1
		Operational Mode: Write Back
		Location: Internal
		Installed Size: 64 KB
		Maximum Size: 64 KB
		Supported SRAM Types:
			Synchronous
		Installed SRAM Type: Synchronous
		Speed: Unknown
		Error Correction Type: Unknown
		System Type: Unknown
		Associativity: Unknown
Handle 0x000D
	DMI type 7, 19 bytes.
	Cache Information
		Socket Designation: External Cache
		Configuration: Enabled, Not Socketed, Level 2
		Operational Mode: Write Back
		Location: External
		Installed Size: 512 KB
		Maximum Size: 1024 KB
		Supported SRAM Types:
			Synchronous
		Installed SRAM Type: Synchronous
		Speed: Unknown
		Error Correction Type: Unknown
		System Type: Unknown
		Associativity: Unknown
Handle 0x000E
	DMI type 8, 9 bytes.
	Port Connector Information
		Internal Reference Designator: PRIMARY IDE
		Internal Connector Type: On Board IDE
		External Reference Designator:  
		External Connector Type: None
		Port Type: Other
Handle 0x000F
	DMI type 8, 9 bytes.
	Port Connector Information
		Internal Reference Designator: SECONDARY IDE
		Internal Connector Type: On Board IDE
		External Reference Designator:  
		External Connector Type: None
		Port Type: Other
Handle 0x0010
	DMI type 8, 9 bytes.
	Port Connector Information
		Internal Reference Designator: FDD
		Internal Connector Type: On Board Floppy
		External Reference Designator:  
		External Connector Type: None
		Port Type: 8251 FIFO Compatible
Handle 0x0011
	DMI type 8, 9 bytes.
	Port Connector Information
		Internal Reference Designator: COM1
		Internal Connector Type: 9 Pin Dual Inline (pin 10 cut)
		External Reference Designator:  
		External Connector Type: DB-9 male
		Port Type: Serial Port 16550 Compatible
Handle 0x0012
	DMI type 8, 9 bytes.
	Port Connector Information
		Internal Reference Designator: COM2
		Internal Connector Type: 9 Pin Dual Inline (pin 10 cut)
		External Reference Designator:  
		External Connector Type: DB-9 male
		Port Type: Serial Port 16550 Compatible
Handle 0x0013
	DMI type 8, 9 bytes.
	Port Connector Information
		Internal Reference Designator: LPT1
		Internal Connector Type: DB-25 female
		External Reference Designator:  
		External Connector Type: DB-25 female
		Port Type: Parallel Port ECP/EPP
Handle 0x0014
	DMI type 8, 9 bytes.
	Port Connector Information
		Internal Reference Designator: Keyboard
		Internal Connector Type: Other
		External Reference Designator:  
		External Connector Type: PS/2
		Port Type: Keyboard Port
Handle 0x0015
	DMI type 8, 9 bytes.
	Port Connector Information
		Internal Reference Designator: PS/2 Mouse
		Internal Connector Type: PS/2
		External Reference Designator: No Detected
		External Connector Type: PS/2
		Port Type: Mouse Port
Handle 0x0016
	DMI type 9, 13 bytes.
	System Slot Information
		Designation: ISA
		Type: 16-bit ISA
		Current Usage: Unknown
		Length: Long
		Characteristics:
			5.0 V is provided
Handle 0x0017
	DMI type 9, 13 bytes.
	System Slot Information
		Designation: ISA
		Type: 16-bit ISA
		Current Usage: Unknown
		Length: Long
		Characteristics:
			5.0 V is provided
Handle 0x0018
	DMI type 9, 13 bytes.
	System Slot Information
		Designation: PCI
		Type: 32-bit PCI
		Current Usage: In Use
		Length: Long
		ID: 8
		Characteristics:
			5.0 V is provided
Handle 0x0019
	DMI type 9, 13 bytes.
	System Slot Information
		Designation: PCI
		Type: 32-bit PCI
		Current Usage: In Use
		Length: Long
		ID: 9
		Characteristics:
			5.0 V is provided
Handle 0x001A
	DMI type 9, 13 bytes.
	System Slot Information
		Designation: PCI
		Type: 32-bit PCI
		Current Usage: In Use
		Length: Long
		ID: 10
		Characteristics:
			5.0 V is provided
Handle 0x001B
	DMI type 9, 13 bytes.
	System Slot Information
		Designation: PCI
		Type: 32-bit PCI
		Current Usage: Available
		Length: Long
		ID: 11
		Characteristics:
			5.0 V is provided
Handle 0x001C
	DMI type 9, 13 bytes.
	System Slot Information
		Designation: PCI
		Type: 32-bit PCI
		Current Usage: In Use
		Length: Long
		ID: 12
		Characteristics:
			5.0 V is provided
Handle 0x001D
	DMI type 9, 13 bytes.
	System Slot Information
		Designation: AGP
		Type: 32-bit AGP
		Current Usage: In Use
		Length: Long
		ID: 0
		Characteristics:
			5.0 V is provided
Handle 0x001E
	DMI type 8, 9 bytes.
	Port Connector Information
		Internal Reference Designator: USB
		Internal Connector Type: None
		External Reference Designator:  
		External Connector Type: Other
		Port Type: USB
Handle 0x001F
	DMI type 13, 22 bytes.
	BIOS Language Information
		Installable Languages: 3
			n|US|iso8859-1
			r|CA|iso8859-1
			a|JP|unicode
		Currently Installed Language: n|US|iso8859-1

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: PMTMR running too fast

[john stultz]

On Mon, 2006-12-04 at 19:40 +0000, Ian Campbell wrote:
> On Mon, 2006-12-04 at 11:19 -0800, john stultz wrote:
> > On Sun, 2006-12-03 at 13:50 +0000, Ian Campbell wrote:
> > > In older kernels arch/i386/kernel/timers/timer_pm.c:verify_pmtmr_rate
> > > contained a check for sensible PMTMR rate and disabled that clocksource
> > > if it was found to be out of spec[0]. This check seems to have been lost
> > > in the transition to drivers/clocksource/acpi_pm.c, the removal is in
> > > 61743fe445213b87fb55a389c8d073785323ca3e "Time: i386 Conversion - part
> > > 4: Remove Old timer_opts Code"[1] and the check is not present in the
> > > replacement 5d0cf410e94b1f1ff852c3f210d22cc6c5a27ffa "Time: i386
> > > Clocksource Drivers"[2].
> > 
> > Fedora has a bug covering this:
> > https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=211902
> 
> > > Is there a specific reason the check was removed (I couldn't see on in
> > > the archives) or was it simply overlooked? Without it I need to pass
> > > clocksource=tsc to have 2.6.18 work correctly on an older K6 system with
> > > an Aladdin chipset (will dig out the precise details if required). Would
> > > a patch to reintroduce the check be acceptable or would some sort of
> > > blacklist based solution be more acceptable?
> > 
> > If I recall correctly, it was pulled because there was some question as
> > to if it was actually needed (x86_64 didn't need it) and it slows down
> > the boot time (although not by much). 
> > 
> > I'm fine just re-adding it. Although if the number of affected systems
> > are small we could just blacklist it (Ian, mind sending dmidecode
> > output?).

I don't have a dev box to test on at the moment, but here's a quick hack
attempt at re-adding the code. Does the following work for you? 

thanks
-john


diff --git a/drivers/clocksource/acpi_pm.c b/drivers/clocksource/acpi_pm.c
index 7fcb77a..3379b5f 100644
--- a/drivers/clocksource/acpi_pm.c
+++ b/drivers/clocksource/acpi_pm.c
@@ -21,9 +21,12 @@ #include <linux/errno.h>
 #include <linux/init.h>
 #include <linux/pci.h>
 #include <asm/io.h>
+#include "mach_timer.h"
 
 /* Number of PMTMR ticks expected during calibration run */
 #define PMTMR_TICKS_PER_SEC 3579545
+#define PMTMR_EXPECTED_RATE \
+  ((CALIBRATE_LATCH * (PMTMR_TICKS_PER_SEC >> 10)) / (CLOCK_TICK_RATE>>10))
 
 /*
  * The I/O port the PMTMR resides at.
@@ -142,6 +145,36 @@ DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_SE
 			acpi_pm_check_graylist);
 #endif
 
+#ifndef CONFIG_X86_64
+/*
+ * Some boards have the PMTMR running way too fast. We check
+ * the PMTMR rate against PIT channel 2 to catch these cases.
+ */
+static int verify_pmtmr_rate(void)
+{
+	u32 value1, value2;
+	unsigned long count, delta;
+
+	mach_prepare_counter();
+	value1 = read_pmtmr();
+	mach_countup(&count);
+	value2 = read_pmtmr();
+	delta = (value2 - value1) & ACPI_PM_MASK;
+
+	/* Check that the PMTMR delta is within 5% of what we expect */
+	if (delta < (PMTMR_EXPECTED_RATE * 19) / 20 ||
+	    delta > (PMTMR_EXPECTED_RATE * 21) / 20) {
+		printk(KERN_INFO "PM-Timer running at invalid rate: %lu%% "
+			"of normal - aborting.\n",
+			100UL * delta / PMTMR_EXPECTED_RATE);
+		return -1;
+	}
+
+	return 0;
+}
+#else
+#define verify_pmtmr_rate() (0)
+#endif
 
 static int __init init_acpi_pm_clocksource(void)
 {
@@ -173,6 +206,9 @@ static int __init init_acpi_pm_clocksour
 	return -ENODEV;
 
 pm_good:
+	if (verify_pmtmr_rate() != 0)
+		return -ENODEV;
+
 	return clocksource_register(&clocksource_acpi_pm);
 }
 

Re: PMTMR running too fast

[Ian Campbell]

[-- Attachment #1: Type: text/plain, Size: 2413 bytes --]

On Mon, 2006-12-04 at 12:14 -0800, john stultz wrote:
> On Mon, 2006-12-04 at 19:40 +0000, Ian Campbell wrote:
> > On Mon, 2006-12-04 at 11:19 -0800, john stultz wrote:
> > > On Sun, 2006-12-03 at 13:50 +0000, Ian Campbell wrote:
> > > > In older kernels arch/i386/kernel/timers/timer_pm.c:verify_pmtmr_rate
> > > > contained a check for sensible PMTMR rate and disabled that clocksource
> > > > if it was found to be out of spec[0]. This check seems to have been lost
> > > > in the transition to drivers/clocksource/acpi_pm.c, the removal is in
> > > > 61743fe445213b87fb55a389c8d073785323ca3e "Time: i386 Conversion - part
> > > > 4: Remove Old timer_opts Code"[1] and the check is not present in the
> > > > replacement 5d0cf410e94b1f1ff852c3f210d22cc6c5a27ffa "Time: i386
> > > > Clocksource Drivers"[2].
> > > 
> > > Fedora has a bug covering this:
> > > https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=211902
> > 
> > > > Is there a specific reason the check was removed (I couldn't see on in
> > > > the archives) or was it simply overlooked? Without it I need to pass
> > > > clocksource=tsc to have 2.6.18 work correctly on an older K6 system with
> > > > an Aladdin chipset (will dig out the precise details if required). Would
> > > > a patch to reintroduce the check be acceptable or would some sort of
> > > > blacklist based solution be more acceptable?
> > > 
> > > If I recall correctly, it was pulled because there was some question as
> > > to if it was actually needed (x86_64 didn't need it) and it slows down
> > > the boot time (although not by much). 
> > > 
> > > I'm fine just re-adding it. Although if the number of affected systems
> > > are small we could just blacklist it (Ian, mind sending dmidecode
> > > output?).
> 
> I don't have a dev box to test on at the moment, but here's a quick hack
> attempt at re-adding the code. Does the following work for you? 

I get:
        PM-Timer running at invalid rate: 200% of normal - aborting.
        ...
        Time: pit clocksource has been installed.
        
Without the clocksource parameter.

Should tsc be preferred to pit though?

/sys/devices/system/clocksource/clocksource0/available_clocksource now
shows "jiffies tsc pit" and current_clocksource is "pit".

Thanks,
Ian.

-- 
Ian Campbell

love, n.:
	When it's growing, you don't mind watering it with a few tears.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: PMTMR running too fast

[john stultz]

On Tue, 2006-12-05 at 07:41 +0000, Ian Campbell wrote:
> On Mon, 2006-12-04 at 12:14 -0800, john stultz wrote:
> > I don't have a dev box to test on at the moment, but here's a quick hack
> > attempt at re-adding the code. Does the following work for you? 
> 
> I get:
>         PM-Timer running at invalid rate: 200% of normal - aborting.
>         ...
>         Time: pit clocksource has been installed.
>         
> Without the clocksource parameter.

Great!

> Should tsc be preferred to pit though?

Depends on your system. If C2/C3 or cpufreq state changes are detected,
we mark the tsc as unstable. 

It sounds as if from your earlier email the TSC works fine, so we might
want to look at what's making the system think its not ok. I probably
need to add a message as to why it was disqualified. However, that's a
separate issue from the last patch.

Thanks for the testing!

-john

Re: PMTMR running too fast

[Ian Campbell]

[-- Attachment #1: Type: text/plain, Size: 818 bytes --]

On Tue, 2006-12-05 at 11:34 -0800, john stultz wrote:
> On Tue, 2006-12-05 at 07:41 +0000, Ian Campbell wrote:
> > Should tsc be preferred to pit though?
> 
> Depends on your system. If C2/C3 or cpufreq state changes are detected,
> we mark the tsc as unstable. 

I'm not using them on purpose but I'll check it out.

> It sounds as if from your earlier email the TSC works fine, so we might
> want to look at what's making the system think its not ok. I probably
> need to add a message as to why it was disqualified. However, that's a
> separate issue from the last patch.

I'll have a play, see if I can figure it out.

> Thanks for the testing!

Thanks for the fix!

Ian.

-- 
Ian Campbell

* Turken thinks little kids are absolutely adorable... especialyy when
  they're someone elses.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: PMTMR running too fast

[Andi Kleen]

> 
> > Is there a specific reason the check was removed (I couldn't see on in
> > the archives) or was it simply overlooked? Without it I need to pass
> > clocksource=tsc to have 2.6.18 work correctly on an older K6 system with
> > an Aladdin chipset (will dig out the precise details if required). Would
> > a patch to reintroduce the check be acceptable or would some sort of
> > blacklist based solution be more acceptable?
> 
> If I recall correctly, it was pulled because there was some question as
> to if it was actually needed (x86_64 didn't need it) and it slows down
> the boot time (although not by much). 
> 
> I'm fine just re-adding it. Although if the number of affected systems
> are small we could just blacklist it (Ian, mind sending dmidecode
> output?).
> 
> Andi, your thoughts?

Doing a check at boot time is fine for me. Just I don't want the
"read pmtmr three times at runtime" code anywhere near x86-64

I don't think the boot time check needs DMI guarding

But BTW the check is not necessarily enough -- there is at least one
NF3 machine around where the PIT timer ticks at a wrong frequency.
Safer would be probably to calibrate against RTC which is afaik used
by Windows too (so it's likely to be ok) 

-Andi

Re: PMTMR running too fast

[john stultz]

On Wed, 2006-12-06 at 17:44 +0100, Andi Kleen wrote:
> >
> > > Is there a specific reason the check was removed (I couldn't see on in
> > > the archives) or was it simply overlooked? Without it I need to pass
> > > clocksource=tsc to have 2.6.18 work correctly on an older K6 system with
> > > an Aladdin chipset (will dig out the precise details if required). Would
> > > a patch to reintroduce the check be acceptable or would some sort of
> > > blacklist based solution be more acceptable?
> >
> > If I recall correctly, it was pulled because there was some question as
> > to if it was actually needed (x86_64 didn't need it) and it slows down
> > the boot time (although not by much).
> >
> > I'm fine just re-adding it. Although if the number of affected systems
> > are small we could just blacklist it (Ian, mind sending dmidecode
> > output?).
> >
> > Andi, your thoughts?
> 
> Doing a check at boot time is fine for me. Just I don't want the
> "read pmtmr three times at runtime" code anywhere near x86-64

:) This change fully disqualifies the ACPI PM if its running at the
wrong frequency, so no worries there.

> I don't think the boot time check needs DMI guarding

DMI guarding? I'm not following...

> But BTW the check is not necessarily enough -- there is at least one
> NF3 machine around where the PIT timer ticks at a wrong frequency.
> Safer would be probably to calibrate against RTC which is afaik used
> by Windows too (so it's likely to be ok)

Hmm.. I might lean towards pushing the patch closer to as it is, as
we're just restoring functionality that was there before in 2.6.17.  The
NF3 system already needs bits to correct for the PIT frequency, so it
seems that code could also correct the mach_countup() routines.

Even so, I do agree with you that moving to utilize more "widely tested"
hardware for calibration would be a good thing.

thanks
-john

Re: PMTMR running too fast

[Ian Campbell]

[-- Attachment #1: Type: text/plain, Size: 382 bytes --]

On Tue, 2006-12-05 at 11:34 -0800, john stultz wrote:
> 
> > Should tsc be preferred to pit though?
> 
> Depends on your system. If C2/C3 or cpufreq state changes are
> detected, we mark the tsc as unstable.  

Turns out I was seeing C2 states. I'll stick with PIT for now.

Thanks,
Ian.

-- 
Ian Campbell

If ignorance is bliss, why aren't there more happy people?

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: PMTMR running too fast

[Andi Kleen]

> > I don't think the boot time check needs DMI guarding
> 
> DMI guarding? I'm not following...

DMI guarding = Making it conditional on a DMI check

-Andi