[PATCH] x86_64: prevent auto select of mwait_idle for AMD CPUs

[PATCH] x86_64: prevent auto select of mwait_idle for AMD CPUs

[Andreas Herrmann]

This fix is needed for AMD family 10h CPUs.

It prevents auto select of mwait_idle for AMD CPUs.
MWAIT does not enter C-states on family 10h and more
power saving is reached by entering C1 with
default_idle.

The patch also adds an idle=mwait command line option
to select mwait_idle for benchmarking.

Signed-off-by: Andreas Herrmann <andreas.herrmann3@amd.com>
---
 arch/x86_64/kernel/process.c    |    9 +++++++--
 arch/x86_64/kernel/setup.c      |    4 ++++
 include/asm-x86_64/cpufeature.h |    1 +
 3 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/arch/x86_64/kernel/process.c b/arch/x86_64/kernel/process.c
index d8d5ccc..18fe9de 100644
--- a/arch/x86_64/kernel/process.c
+++ b/arch/x86_64/kernel/process.c
@@ -271,7 +271,8 @@ static void mwait_idle(void)
 void __cpuinit select_idle_routine(const struct cpuinfo_x86 *c)
 {
 	static int printed;
-	if (cpu_has(c, X86_FEATURE_MWAIT)) {
+	if (cpu_has(c, X86_FEATURE_MWAIT) &&
+	    !cpu_has(c, X86_FEATURE_MWAIT_NO_CSTATE)) {
 		/*
 		 * Skip, if setup has overridden idle.
 		 * One CPU supports mwait => All CPUs supports mwait
@@ -291,7 +292,11 @@ static int __init idle_setup (char *str)
 	if (!strncmp(str, "poll", 4)) {
 		printk("using polling idle threads.\n");
 		pm_idle = poll_idle;
-	}
+	} else if (!strncmp(str, "mwait", 5))
+		if (boot_cpu_has(X86_FEATURE_MWAIT)) {
+			printk(KERN_INFO "using mwait in idle threads.\n");
+			pm_idle = mwait_idle;
+		}
 
 	boot_option_idle_override = 1;
 	return 1;
diff --git a/arch/x86_64/kernel/setup.c b/arch/x86_64/kernel/setup.c
index 3d98b69..b707025 100644
--- a/arch/x86_64/kernel/setup.c
+++ b/arch/x86_64/kernel/setup.c
@@ -612,6 +612,10 @@ static void __cpuinit init_amd(struct cpuinfo_x86 *c)
 
 	/* RDTSC can be speculated around */
 	clear_bit(X86_FEATURE_SYNC_RDTSC, &c->x86_capability);
+
+	/* prevent auto select of mwait_idle */
+	if (cpu_has(c, X86_FEATURE_MWAIT))
+		set_bit(X86_FEATURE_MWAIT_NO_CSTATE, c->x86_capability);
 }
 
 static void __cpuinit detect_ht(struct cpuinfo_x86 *c)
diff --git a/include/asm-x86_64/cpufeature.h b/include/asm-x86_64/cpufeature.h
index 0b3c686..9331d1a 100644
--- a/include/asm-x86_64/cpufeature.h
+++ b/include/asm-x86_64/cpufeature.h
@@ -70,6 +70,7 @@
 #define X86_FEATURE_ARCH_PERFMON (3*32+9) /* Intel Architectural PerfMon */
 #define X86_FEATURE_PEBS	(3*32+10) /* Precise-Event Based Sampling */
 #define X86_FEATURE_BTS		(3*32+11) /* Branch Trace Store */
+#define X86_FEATURE_MWAIT_NO_CSTATE (3*32+12) /* mwait does not enter c-state */
 
 /* Intel-defined CPU features, CPUID level 0x00000001 (ecx), word 4 */
 #define X86_FEATURE_XMM3	(4*32+ 0) /* Streaming SIMD Extensions-3 */
-- 
1.5.0.6

Re: [PATCH] x86_64: prevent auto select of mwait_idle for AMD CPUs

[Andreas Herrmann]

Actually I have also written patches to clear the MWAIT flag
for AMD CPUs.

But after re-reading of specs (also Intel's specs) I preferred
to keep the MWAIT flag but to introduce a MWAIT_NO_CSTATE flag.

I think this is the cleaner solution.


Regards,

Andreas

Re: [PATCH] x86_64: prevent auto select of mwait_idle for AMD CPUs

[Andi Kleen]

On Tuesday 10 April 2007 19:24:25 Andreas Herrmann wrote:
> This fix is needed for AMD family 10h CPUs.
> 
> It prevents auto select of mwait_idle for AMD CPUs.
> MWAIT does not enter C-states on family 10h and more
> power saving is reached by entering C1 with
> default_idle.
> 
> The patch also adds an idle=mwait command line option
> to select mwait_idle for benchmarking.

I already did my own patch earlier. Applied for reference.

-Andi
Don't use MWAIT on AMD Family 10/11

It doesn't put the CPU into deeper sleep states, so it's better to use the standard
idle loop to save power. But allow to reenable it anyways for benchmarking.

I also removed the obsolete idle=halt

Signed-off-by: Andi Kleen <ak@suse.de>

Index: linux/Documentation/kernel-parameters.txt
===================================================================
--- linux.orig/Documentation/kernel-parameters.txt
+++ linux/Documentation/kernel-parameters.txt
@@ -673,8 +673,15 @@ and is between 256 and 4096 characters. 
 	idebus=		[HW] (E)IDE subsystem - VLB/PCI bus speed
 			See Documentation/ide.txt.
 
-	idle=		[HW]
-			Format: idle=poll or idle=halt
+	idle=		[X86]
+			Format: idle=poll or idle=mwait
+			Poll forces a polling idle loop that can slightly improves the performance
+			of waking up a idle CPU, but will use a lot of power and make the system
+			run hot. Not recommended.
+			idle=mwait. On systems which support MONITOR/MWAIT but the kernel chose
+			to not use it because it doesn't save as much power as a normal idle
+			loop use the MONITOR/MWAIT idle loop anyways. Performance should be the same
+			as idle=poll.
 
 	ignore_loglevel	[KNL]
 			Ignore loglevel setting - this will print /all/
Index: linux/arch/i386/kernel/cpu/amd.c
===================================================================
--- linux.orig/arch/i386/kernel/cpu/amd.c
+++ linux/arch/i386/kernel/cpu/amd.c
@@ -22,6 +22,8 @@
 extern void vide(void);
 __asm__(".align 4\nvide: ret");
 
+int force_mwait __initdata;
+
 static void __cpuinit init_amd(struct cpuinfo_x86 *c)
 {
 	u32 l, h;
@@ -241,6 +243,9 @@ static void __cpuinit init_amd(struct cp
 
 	if (cpuid_eax(0x80000000) >= 0x80000006)
 		num_cache_leaves = 3;
+
+	if ((c->x86 == 0x10 || c->x86 == 0x11) && !force_mwait)
+		clear_bit(X86_FEATURE_MWAIT, &c->x86_capability);
 }
 
 static unsigned int __cpuinit amd_size_cache(struct cpuinfo_x86 * c, unsigned int size)
Index: linux/arch/i386/kernel/process.c
===================================================================
--- linux.orig/arch/i386/kernel/process.c
+++ linux/arch/i386/kernel/process.c
@@ -272,25 +272,24 @@ void __devinit select_idle_routine(const
 	}
 }
 
-static int __init idle_setup (char *str)
+static int __init idle_setup(char *str)
 {
-	if (!strncmp(str, "poll", 4)) {
+	if (!strcmp(str, "poll")) {
 		printk("using polling idle threads.\n");
 		pm_idle = poll_idle;
 #ifdef CONFIG_X86_SMP
 		if (smp_num_siblings > 1)
 			printk("WARNING: polling idle and HT enabled, performance may degrade.\n");
 #endif
-	} else if (!strncmp(str, "halt", 4)) {
-		printk("using halt in idle threads.\n");
-		pm_idle = default_idle;
-	}
+	} else if (!strcmp(str, "mwait"))
+		force_mwait = 1;
+	else
+		return -1;
 
 	boot_option_idle_override = 1;
-	return 1;
+	return 0;
 }
-
-__setup("idle=", idle_setup);
+early_param("idle", idle_setup);
 
 void show_regs(struct pt_regs * regs)
 {
Index: linux/arch/x86_64/kernel/process.c
===================================================================
--- linux.orig/arch/x86_64/kernel/process.c
+++ linux/arch/x86_64/kernel/process.c
@@ -288,16 +288,18 @@ void __cpuinit select_idle_routine(const
 
 static int __init idle_setup (char *str)
 {
-	if (!strncmp(str, "poll", 4)) {
+	if (!strcmp(str, "poll")) {
 		printk("using polling idle threads.\n");
 		pm_idle = poll_idle;
-	}
+	} else if (!strcmp(str, "mwait")) 
+		force_mwait = 1;
+	else
+		return -1;
 
 	boot_option_idle_override = 1;
-	return 1;
+	return 0;
 }
-
-__setup("idle=", idle_setup);
+early_param("idle", idle_setup);
 
 /* Prints also some state that isn't saved in the pt_regs */ 
 void __show_regs(struct pt_regs * regs)
Index: linux/arch/x86_64/kernel/setup.c
===================================================================
--- linux.orig/arch/x86_64/kernel/setup.c
+++ linux/arch/x86_64/kernel/setup.c
@@ -79,6 +79,8 @@ int bootloader_type;
 
 unsigned long saved_video_mode;
 
+int force_mwait __initdata;
+
 /* 
  * Early DMI memory
  */
@@ -604,6 +606,10 @@ static void __cpuinit init_amd(struct cp
 
 	/* RDTSC can be speculated around */
 	clear_bit(X86_FEATURE_SYNC_RDTSC, &c->x86_capability);
+
+	/* Family 10/11 don't support C states in MWAIT so don't use it */ 
+	if ((c->x86 == 0x10 || c->x86 == 0x11) && !force_mwait) 
+		clear_bit(X86_FEATURE_MWAIT, &c->x86_capability);
 }
 
 static void __cpuinit detect_ht(struct cpuinfo_x86 *c)
Index: linux/include/asm-i386/processor.h
===================================================================
--- linux.orig/include/asm-i386/processor.h
+++ linux/include/asm-i386/processor.h
@@ -779,4 +779,6 @@ extern int sysenter_setup(void);
 extern void cpu_set_gdt(int);
 extern void cpu_init(void);
 
+extern int force_mwait;
+
 #endif /* __ASM_I386_PROCESSOR_H */
Index: linux/include/asm-x86_64/proto.h
===================================================================
--- linux.orig/include/asm-x86_64/proto.h
+++ linux/include/asm-x86_64/proto.h
@@ -119,6 +119,8 @@ extern int gsi_irq_sharing(int gsi);
 
 extern void smp_local_timer_interrupt(void);
 
+extern int force_mwait;
+
 long do_arch_prctl(struct task_struct *task, int code, unsigned long addr);
 
 void i8254_timer_resume(void);

Re: [PATCH] x86_64: prevent auto select of mwait_idle for AMD CPUs

[Andreas Herrmann]

On Tue, Apr 10, 2007 at 08:11:18PM +0200, Andi Kleen wrote:
> On Tuesday 10 April 2007 19:24:25 Andreas Herrmann wrote:
> > This fix is needed for AMD family 10h CPUs.
> > 
> > It prevents auto select of mwait_idle for AMD CPUs.
> > MWAIT does not enter C-states on family 10h and more
> > power saving is reached by entering C1 with
> > default_idle.
> > 
> > The patch also adds an idle=mwait command line option
> > to select mwait_idle for benchmarking.
> 
> I already did my own patch earlier. Applied for reference.
> 
> -Andi

Great. Your patch is the better solution.


Regards,

Andreas