It seems that the CPUID lies about the monitor-line size, or at least our

interpretation of it isn't quite right.  So instead of allocating memory
and slicing it based on the parameters returned by CPUID, simply use a member
in struct cpu_info like basically all other OSes out there do.  Our struct
cpu_info is large enough to never cause any overlap.  This makes the
mwait-based idle loop actually work.  We still execute the CPUID instruction
to make sure monitor/mwait is properly supported by the hardware we're
running on.

ok sthen@, deraadt@, guenther@

6 files changed, 36 insertions, 86 deletions

diff --git a/sys/arch/amd64/amd64/cpu.c b/sys/arch/amd64/amd64/cpu.c
index db7efb817b3..4337d0e2483 100644
--- a/sys/arch/amd64/amd64/cpu.c
+++ b/sys/arch/amd64/amd64/cpu.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: cpu.c,v 1.81 2015/03/25 21:05:18 kettenis Exp $	*/
+/*	$OpenBSD: cpu.c,v 1.82 2015/04/18 22:16:21 kettenis Exp $	*/
 /* $NetBSD: cpu.c,v 1.1 2003/04/26 18:39:26 fvdl Exp $ */
 
 /*-
@@ -243,7 +243,6 @@ void
 cpu_idle_mwait_cycle(void)
 {
 	struct cpu_info *ci = curcpu();
-	volatile int *state = &ci->ci_mwait[0];
 
 	if ((read_rflags() & PSL_I) == 0)
 		panic("idle with interrupts blocked!");
@@ -262,18 +261,18 @@ cpu_idle_mwait_cycle(void)
 	 * something to the queue and called cpu_unidle() between
 	 * the check in sched_idle() and here.
 	 */
-	atomic_setbits_int(state, MWAIT_IDLING);
+	atomic_setbits_int(&ci->ci_mwait, MWAIT_IDLING);
 	if (ci->ci_schedstate.spc_whichqs == 0) {
-		monitor(state, 0, 0);
-		if ((*state & MWAIT_IDLING) == MWAIT_IDLING)
+		monitor(&ci->ci_mwait, 0, 0);
+		if ((ci->ci_mwait & MWAIT_IDLING) == MWAIT_IDLING)
 			mwait(0, 0);
 	}
 
 	/* done idling; let cpu_kick() know that an IPI is required */
-	atomic_clearbits_int(state, MWAIT_IDLING);
+	atomic_clearbits_int(&ci->ci_mwait, MWAIT_IDLING);
 }
 
-unsigned int mwait_size;
+u_int cpu_mwait_size;
 
 void
 cpu_init_mwait(struct cpu_softc *sc)
@@ -306,40 +305,15 @@ cpu_init_mwait(struct cpu_softc *sc)
 	    (largest & (sizeof(int)-1)))
 		printf(" (bogus)");
 	else
-		mwait_size = largest;
+		cpu_mwait_size = largest;
 	printf("\n");
-	/* XXX disable mwait: ACPI says not to use it on too many systems */
-	mwait_size = 0;
 }
 
 void
 cpu_enable_mwait(void)
 {
-	unsigned long area;
-	struct cpu_info *ci;
-	CPU_INFO_ITERATOR cii;
-
-	if (mwait_size == 0)
-		return;
-
-	/*
-	 * Allocate the area, with a bit extra so that we can align
-	 * to a multiple of mwait_size
-	 */
-	area = (unsigned long)malloc((ncpus * mwait_size) + mwait_size
-	    - sizeof(int), M_DEVBUF, M_NOWAIT|M_ZERO);
-	if (area == 0) {
-		printf("cpu0: mwait failed\n");
-	} else {
-		/* round to a multiple of mwait_size  */
-		area = ((area + mwait_size - sizeof(int)) / mwait_size)
-		    * mwait_size;
-		CPU_INFO_FOREACH(cii, ci) {
-			ci->ci_mwait = (int *)area;
-			area += mwait_size;
-		}
+	if (cpu_mwait_size > 0)
 		cpu_idle_cycle_fcn = &cpu_idle_mwait_cycle;
-	}
 }
 #endif /* MULTIPROCESSOR */
 
diff --git a/sys/arch/amd64/amd64/machdep.c b/sys/arch/amd64/amd64/machdep.c
index a519b911c97..f18e5bc7561 100644
--- a/sys/arch/amd64/amd64/machdep.c
+++ b/sys/arch/amd64/amd64/machdep.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: machdep.c,v 1.208 2015/03/25 21:05:18 kettenis Exp $	*/
+/*	$OpenBSD: machdep.c,v 1.209 2015/04/18 22:16:21 kettenis Exp $	*/
 /*	$NetBSD: machdep.c,v 1.3 2003/05/07 22:58:18 fvdl Exp $	*/
 
 /*-
@@ -671,15 +671,15 @@ cpu_kick(struct cpu_info *ci)
 {
 	/* only need to kick other CPUs */
 	if (ci != curcpu()) {
-		if (ci->ci_mwait != NULL) {
+		if (cpu_mwait_size > 0) {
 			/*
 			 * If not idling, then send an IPI, else
 			 * just clear the "keep idling" bit.
 			 */
-			if ((ci->ci_mwait[0] & MWAIT_IN_IDLE) == 0)
+			if ((ci->ci_mwait & MWAIT_IN_IDLE) == 0)
 				x86_send_ipi(ci, X86_IPI_NOP);
 			else
-				atomic_clearbits_int(&ci->ci_mwait[0],
+				atomic_clearbits_int(&ci->ci_mwait,
 				    MWAIT_KEEP_IDLING);
 		} else {
 			/* no mwait, so need an IPI */
@@ -704,12 +704,12 @@ signotify(struct proc *p)
 void
 cpu_unidle(struct cpu_info *ci)
 {
-	if (ci->ci_mwait != NULL) {
+	if (cpu_mwait_size > 0) {
 		/*
 		 * Just clear the "keep idling" bit; if it wasn't
 		 * idling then we didn't need to do anything anyway.
 		 */
-		atomic_clearbits_int(&ci->ci_mwait[0], MWAIT_KEEP_IDLING);
+		atomic_clearbits_int(&ci->ci_mwait, MWAIT_KEEP_IDLING);
 		return;
 	}
 
diff --git a/sys/arch/amd64/include/cpu.h b/sys/arch/amd64/include/cpu.h
index 780948bed4a..7caefde92ff 100644
--- a/sys/arch/amd64/include/cpu.h
+++ b/sys/arch/amd64/include/cpu.h
@@ -1,4 +1,4 @@
-/*	$OpenBSD: cpu.h,v 1.90 2015/01/15 15:30:17 sf Exp $	*/
+/*	$OpenBSD: cpu.h,v 1.91 2015/04/18 22:16:21 kettenis Exp $	*/
 /*	$NetBSD: cpu.h,v 1.1 2003/04/26 18:39:39 fvdl Exp $	*/
 
 /*-
@@ -109,8 +109,7 @@ struct cpu_info {
 	void (*cpu_setup)(struct cpu_info *);
 	void (*ci_info)(struct cpu_info *);
 
-	u_int		*ci_mwait;
-/* bits in ci_mwait[0] */
+	volatile u_int	ci_mwait;
 #define	MWAIT_IN_IDLE		0x1	/* don't need IPI to wake */
 #define	MWAIT_KEEP_IDLING	0x2	/* cleared by other cpus to wake me */
 #define	MWAIT_IDLING	(MWAIT_IN_IDLE | MWAIT_KEEP_IDLING)
@@ -191,6 +190,8 @@ extern struct cpu_info *cpu_info[MAXCPUS];
 void cpu_boot_secondary_processors(void);
 void cpu_init_idle_pcbs(void);    
 
+extern u_int cpu_mwait_size;
+
 void cpu_kick(struct cpu_info *);
 void cpu_unidle(struct cpu_info *);
 
diff --git a/sys/arch/i386/i386/cpu.c b/sys/arch/i386/i386/cpu.c
index c3ecb157fba..78cf7fd033f 100644
--- a/sys/arch/i386/i386/cpu.c
+++ b/sys/arch/i386/i386/cpu.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: cpu.c,v 1.61 2015/02/11 05:54:48 dlg Exp $	*/
+/*	$OpenBSD: cpu.c,v 1.62 2015/04/18 22:16:21 kettenis Exp $	*/
 /* $NetBSD: cpu.c,v 1.1.2.7 2000/06/26 02:04:05 sommerfeld Exp $ */
 
 /*-
@@ -789,7 +789,6 @@ void
 cpu_idle_mwait_cycle(void)
 {
 	struct cpu_info *ci = curcpu();
-	volatile int *state = &ci->ci_mwait[0];
 
 	if ((read_eflags() & PSL_I) == 0)
 		panic("idle with interrupts blocked!");
@@ -808,18 +807,18 @@ cpu_idle_mwait_cycle(void)
 	 * something to the queue and called cpu_unidle() between
 	 * the check in sched_idle() and here.
 	 */
-	atomic_setbits_int(state, MWAIT_IDLING);
+	atomic_setbits_int(&ci->ci_mwait, MWAIT_IDLING);
 	if (ci->ci_schedstate.spc_whichqs == 0) {
-		monitor(state, 0, 0);
-		if ((*state & MWAIT_IDLING) == MWAIT_IDLING)
+		monitor(&ci->ci_mwait, 0, 0);
+		if ((ci->ci_mwait & MWAIT_IDLING) == MWAIT_IDLING)
 			mwait(0, 0);
 	}
 
 	/* done idling; let cpu_kick() know that an IPI is required */
-	atomic_clearbits_int(state, MWAIT_IDLING);
+	atomic_clearbits_int(&ci->ci_mwait, MWAIT_IDLING);
 }
 
-unsigned int mwait_size;
+u_int cpu_mwait_size;
 
 void
 cpu_init_mwait(struct device *dv)
@@ -851,40 +850,15 @@ cpu_init_mwait(struct device *dv)
 	    (largest & (sizeof(int)-1)))
 		printf(" (bogus)");
 	else
-		mwait_size = largest;
+		cpu_mwait_size = largest;
 	printf("\n");
-	/* XXX disable mwait: ACPI says not to use it on too many systems */
-	mwait_size = 0;
 }
 
 void
 cpu_enable_mwait(void)
 {
-	unsigned long area;
-	struct cpu_info *ci;
-	CPU_INFO_ITERATOR cii;
-
-	if (mwait_size == 0)
-		return;
-
-	/*
-	 * Allocate the area, with a bit extra so that we can align
-	 * to a multiple of mwait_size
-	 */
-	area = (unsigned long)malloc((ncpus * mwait_size) + mwait_size
-	    - sizeof(int), M_DEVBUF, M_NOWAIT|M_ZERO);
-	if (area == 0) {
-		printf("cpu0: mwait failed\n");
-	} else {
-		/* round to a multiple of mwait_size  */
-		area = ((area + mwait_size - sizeof(int)) / mwait_size)
-		    * mwait_size;
-		CPU_INFO_FOREACH(cii, ci) {
-			ci->ci_mwait = (int *)area;
-			area += mwait_size;
-		}
+	if (cpu_mwait_size > 0)
 		cpu_idle_cycle_fcn = &cpu_idle_mwait_cycle;
-	}
 }
 #endif /* MULTIPROCESSOR */
 
diff --git a/sys/arch/i386/i386/machdep.c b/sys/arch/i386/i386/machdep.c
index 5027a0c0d5a..40f0a9bfac0 100644
--- a/sys/arch/i386/i386/machdep.c
+++ b/sys/arch/i386/i386/machdep.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: machdep.c,v 1.568 2015/04/12 18:37:53 mlarkin Exp $	*/
+/*	$OpenBSD: machdep.c,v 1.569 2015/04/18 22:16:21 kettenis Exp $	*/
 /*	$NetBSD: machdep.c,v 1.214 1996/11/10 03:16:17 thorpej Exp $	*/
 
 /*-
@@ -2532,15 +2532,15 @@ cpu_kick(struct cpu_info *ci)
 {
 	/* only need to kick other CPUs */
 	if (ci != curcpu()) {
-		if (ci->ci_mwait != NULL) {
+		if (cpu_mwait_size > 0) {
 			/*
 			 * If not idling, then send an IPI, else
 			 * just clear the "keep idling" bit.
 			 */
-			if ((ci->ci_mwait[0] & MWAIT_IN_IDLE) == 0)
+			if ((ci->ci_mwait & MWAIT_IN_IDLE) == 0)
 				i386_send_ipi(ci, I386_IPI_NOP);
 			else
-				atomic_clearbits_int(&ci->ci_mwait[0],
+				atomic_clearbits_int(&ci->ci_mwait,
 				    MWAIT_KEEP_IDLING);
 		} else {
 			/* no mwait, so need an IPI */
@@ -2565,12 +2565,12 @@ signotify(struct proc *p)
 void
 cpu_unidle(struct cpu_info *ci)
 {
-	if (ci->ci_mwait != NULL) {
+	if (cpu_mwait_size > 0) {
 		/*
 		 * Just clear the "keep idling" bit; if it wasn't
 		 * idling then we didn't need to do anything anyway.
 		 */
-		atomic_clearbits_int(&ci->ci_mwait[0], MWAIT_KEEP_IDLING);
+		atomic_clearbits_int(&ci->ci_mwait, MWAIT_KEEP_IDLING);
 		return;
 	}
 
diff --git a/sys/arch/i386/include/cpu.h b/sys/arch/i386/include/cpu.h
index 516bbe1f3ca..14033d378d0 100644
--- a/sys/arch/i386/include/cpu.h
+++ b/sys/arch/i386/include/cpu.h
@@ -1,4 +1,4 @@
-/*	$OpenBSD: cpu.h,v 1.138 2015/04/12 18:37:54 mlarkin Exp $	*/
+/*	$OpenBSD: cpu.h,v 1.139 2015/04/18 22:16:21 kettenis Exp $	*/
 /*	$NetBSD: cpu.h,v 1.35 1996/05/05 19:29:26 christos Exp $	*/
 
 /*-
@@ -126,8 +126,7 @@ struct cpu_info {
 	struct cpu_functions *ci_func;	/* start/stop functions */
 	void (*cpu_setup)(struct cpu_info *);	/* proc-dependant init */
 
-	u_int		*ci_mwait;
-/* bits in ci_mwait[0] */
+	volatile u_int	ci_mwait;
 #define	MWAIT_IN_IDLE		0x1	/* don't need IPI to wake */
 #define	MWAIT_KEEP_IDLING	0x2	/* cleared by other cpus to wake me */
 #define	MWAIT_IDLING		(MWAIT_IN_IDLE | MWAIT_KEEP_IDLING)
@@ -214,6 +213,8 @@ extern struct cpu_info	*cpu_info[MAXCPUS];
 extern void cpu_boot_secondary_processors(void);
 extern void cpu_init_idle_pcbs(void);
 
+extern u_int cpu_mwait_size;
+
 void cpu_kick(struct cpu_info *);
 void cpu_unidle(struct cpu_info *);