Subject: Re: apic timing/clock problem in 4.0_RC3, MacBook 2x2.0
To: None <port-i386@netbsd.org>
From: Marco Trillo <marcotrillo@gmail.com>
List: port-i386
Date: 11/10/2007 15:17:36
Hi,

On 11/10/07, Marco Trillo <marcotrillo@gmail.com> wrote:
> Hi,
>
> On 11/10/07, Juan RP <juan@xtrarom.org> wrote:
> > On Sat, 10 Nov 2007 11:30:16 +0100
> > "Marco Trillo" <marcotrillo@gmail.com> wrote:
> > > More news about the issue: as I reported before, that patch indeed
> > > appears to resolve all the issues (thanks to Ryan Lortie for finding
> > > the cause of the problem).
> > > The problem is that adding this line to a generic kernel is quirky and
> > > may affect negatively to other (non-MacBook) systems.
> > >
> > > There was some discussion on FreeBSD lists about how to implement this
> > > in a portable and clean way. It seems that the MacBook can be detected
> > > using a SMBIOS 'system product' string:
> > >
> > <http://lists.freebsd.org/pipermail/freebsd-current/2007-November/079643.html>.
> > >
> > > What do you think of applying a similar patch to NetBSD?
> > >
> > > By the way, I sent a PR describing the issue and the fix as
> > > "port-i386/37305".
> >
> I'm experimenting with this

OK, the following patch uses the SMBIOS product identification to
disable the 'legacy USB SMI' bit only when a MacBook is found.
In addition, defining the USB_SMI_DISABLE preprocessor symbol causes
it to be always disabled -- this is to make it easy to enable it in a
system which is suspected to have the problem.

The patch also causes the SMBIOS support to be compiled always, and
not only when ipmi(4) is compiled (I guess that ipmi was the only user
of the SMBIOS stuff so it was not compiled by default). Some SMBIOS
strings such as vendor, machine and version are printed if available.

The dmesg looks like this:

NetBSD 4.0_RC2 (GENERIC.MP) #3: Sat Nov 10 15:50:13 CET 2007
	mtrillo@ununoctium.lan:/usr/src/sys/arch/i386/compile/GENERIC.MP
total memory = 480 MB
rbus: rbus_min_start set to 0x40000000
avail memory = 462 MB
timecounter: Timecounters tick every 10.000 msec
SMBIOS rev. 2.4 @ 0xe7440 (37 entries)
SMBIOS system identification:
	vendor:<Apple Computer, Inc.>
	product:<MacBook1,1>
	version:<1.0>
rtclock: disabling SMI generation by legacy USB circuit
[snip]
cpu0 at mainbus0: apid 0 (boot processor)
cpu0: Intel Pentium M (Yonah) (686-class), 1997.45 MHz, id [snip]
cpu0: calibrating local timer
cpu0: apic clock running at 166 MHz
cpu0: 64 page colors
cpu1 at mainbus0: apid 1 (application processor)
cpu1: starting
cpu1: Intel Pentium M (Yonah) (686-class), 1997.33 MHz, id 0x6e8
[snip]

The patch:

Index: src/sys/arch/i386/i386/autoconf.c
===================================================================
RCS file: /cvsroot/src/sys/arch/i386/i386/autoconf.c,v
retrieving revision 1.84
diff -u -r1.84 autoconf.c
--- src/sys/arch/i386/i386/autoconf.c	7 Jun 2006 22:37:58 -0000	1.84
+++ src/sys/arch/i386/i386/autoconf.c	10 Nov 2007 15:02:54 -0000
@@ -97,12 +97,12 @@
 void
 cpu_configure(void)
 {
-
-	startrtclock();
-
 #if NBIOS32 > 0
 	bios32_init();
 #endif
+	
+	startrtclock();
+
 #ifdef PCIBIOS
 	pcibios_init();
 #endif
Index: src/sys/arch/i386/i386/bios32.c
===================================================================
RCS file: /cvsroot/src/sys/arch/i386/i386/bios32.c,v
retrieving revision 1.11
diff -u -r1.11 bios32.c
--- src/sys/arch/i386/i386/bios32.c	1 Oct 2006 18:37:54 -0000	1.11
+++ src/sys/arch/i386/i386/bios32.c	10 Nov 2007 15:02:54 -0000
@@ -109,12 +109,11 @@

 #include <uvm/uvm.h>

-#include "ipmi.h"
-
 #define	BIOS32_START	0xe0000
 #define	BIOS32_SIZE	0x20000
 #define	BIOS32_END	(BIOS32_START + BIOS32_SIZE - 0x10)

+bool smbios = false;
 struct bios32_entry bios32_entry;
 struct smbios_entry smbios_entry;

@@ -127,6 +126,7 @@
 	paddr_t entry = 0;
 	caddr_t p;
 	unsigned char cksum;
+	struct smbtable systbl;
 	int i;

 	for (p = (caddr_t)ISA_HOLE_VADDR(BIOS32_START);
@@ -162,7 +162,6 @@
 		bios32_entry.offset = (caddr_t)ISA_HOLE_VADDR(entry);
 		bios32_entry.segment = GSEL(GCODE_SEL, SEL_KPL);
 	}
-#if NIPMI > 0
 	/* see if we have SMBIOS extentions */
 	for (p = ISA_HOLE_VADDR(SMBIOS_START);
 	    p < (caddr_t)ISA_HOLE_VADDR(SMBIOS_END); p+= 16) {
@@ -207,9 +206,30 @@
 			    sh->majrev, sh->minrev, (u_long)sh->addr,
 			    sh->count);

+		systbl.cookie = 0;
+		if (smbios_find_table(SMBIOS_TYPE_SYSTEM, &systbl)) {
+			char id[64];
+			
+			memset(id, 0, sizeof(id));
+			printf("SMBIOS system identification:\n");
+
+			if (smbios_get_string(&systbl, SMBIOS_SYS_VENDOR,
+					      id, sizeof(id) - 1))
+				printf("\tvendor:<%s>\n", id);
+			if (smbios_get_string(&systbl, SMBIOS_SYS_PRODUCT,
+					      id, sizeof(id) - 1))
+				printf("\tproduct:<%s>\n", id);
+			if (smbios_get_string(&systbl, SMBIOS_SYS_VERSION,
+					      id, sizeof(id) - 1))
+				printf("\tversion:<%s>\n", id);
+			if (smbios_get_string(&systbl, SMBIOS_SYS_SERIAL,
+					      id, sizeof(id) - 1))
+				printf("\tserial:<%s>\n", id);
+		}
+
+		smbios = true;
 		break;
 	}
-#endif

 }

@@ -258,7 +278,6 @@
 	return (1);
 }

-#if NIPMI > 0
 /*
  * smbios_find_table() takes a caller supplied smbios struct type and
  * a pointer to a handle (struct smbtable) returning one if the structure
@@ -342,4 +361,4 @@

 	return ret;
 }
-#endif
+
Index: src/sys/arch/x86/include/smbiosvar.h
===================================================================
RCS file: /cvsroot/src/sys/arch/x86/include/smbiosvar.h,v
retrieving revision 1.1
diff -u -r1.1 smbiosvar.h
--- src/sys/arch/x86/include/smbiosvar.h	1 Oct 2006 18:37:55 -0000	1.1
+++ src/sys/arch/x86/include/smbiosvar.h	10 Nov 2007 15:02:54 -0000
@@ -1,4 +1,5 @@
 /*	$NetBSD: smbiosvar.h,v 1.1 2006/10/01 18:37:55 bouyer Exp $ */
+
 /*
  * Copyright (c) 2006 Gordon Willem Klok <gklok@cogeco.ca>
  * Copyright (c) 2005 Jordan Hargrave
@@ -144,7 +145,13 @@
  * SMBIOS Structure Type 1 "System Information"
  * DMTF Specification DSP0134 Section 3.3.2 p.g. 35
  */
-
+enum {
+	SMBIOS_SYS,
+	SMBIOS_SYS_VENDOR,
+	SMBIOS_SYS_PRODUCT,
+	SMBIOS_SYS_VERSION,
+	SMBIOS_SYS_SERIAL
+};
 struct smbios_sys {
 /* SMBIOS spec 2.0+ */
 	u_int8_t	vendor;		/* string */
@@ -201,6 +208,8 @@
         u_int8_t        smipmi_irq;             /* IRQ if applicable */
 } __packed;

+extern bool smbios; // SMBIOS is present
+
 int smbios_find_table(u_int8_t, struct smbtable *);
 char *smbios_get_string(struct smbtable *, u_int8_t, char *, size_t);

Index: src/sys/arch/x86/isa/clock.c
===================================================================
RCS file: /cvsroot/src/sys/arch/x86/isa/clock.c,v
retrieving revision 1.7
diff -u -r1.7 clock.c
--- src/sys/arch/x86/isa/clock.c	16 Nov 2006 01:32:39 -0000	1.7
+++ src/sys/arch/x86/isa/clock.c	10 Nov 2007 15:02:56 -0000
@@ -121,7 +121,7 @@
  */

 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: clock.c,v 1.7 2006/11/16 01:32:39 christos Exp $");
+__KERNEL_RCSID(0, "$NetBSD$");

 /* #define CLOCKDEBUG */
 /* #define CLOCK_PARANOIA */
@@ -146,6 +146,7 @@
 #include <dev/ic/mc146818reg.h>
 #include <dev/ic/i8253reg.h>
 #include <i386/isa/nvram.h>
+#include <x86/include/smbiosvar.h>
 #include <x86/x86/tsc.h>
 #include <dev/clock_subr.h>
 #include <machine/specialreg.h>
@@ -334,6 +335,7 @@
 initrtclock(u_long freq)
 {
 	u_long tval;
+	
 	/*
 	 * Compute timer_count, the count-down count the timer will be
 	 * set to.  Also, correctly round
@@ -353,11 +355,49 @@
 	rtclock_init = 1;
 }

+static void
+preparertclock(void)
+{
+#ifdef USB_SMI_DISABLE
+	bool usb_smi_disable = true;
+#else
+	bool usb_smi_disable = false;
+#endif
+	char product[64];
+	struct smbtable tbl;
+
+	if (smbios) {
+		tbl.cookie = 0;
+		memset(product, 0, sizeof(product));
+		if (smbios_find_table(SMBIOS_TYPE_SYSTEM, &tbl) &&
+		    smbios_get_string(&tbl, SMBIOS_SYS_PRODUCT,
+				      product, sizeof(product) - 1))
+			/* known affected machines */
+			usb_smi_disable = usb_smi_disable ||
+				 (strcmp(product, "MacBook1,1") == 0);
+	}
+
+	/*
+	 * On some machines, the LEGACY_USB feature
+	 * of the Intel ICH(7) -- which "causes legacy
+	 * USB circuit to generate SMI" -- botches the
+	 * clock calibration if enabled, causing timing
+	 * and SMP problems. This has been detected at
+	 * least on MacBook1,1 products.
+	 */
+	if (usb_smi_disable) {
+		aprint_verbose("rtclock: disabling SMI generation "
+			       "by legacy USB circuit\n");
+		outl(0x430, inl(0x430) & ~8);
+	}
+}
+
 void
 startrtclock(void)
 {
 	int s;

+	preparertclock();
 	if (!rtclock_init)
 		initrtclock(TIMER_FREQ);