[PATCH 0/2] Forcibly enable some MISC_ENABLE features on Intel

[PATCH 0/2] Forcibly enable some MISC_ENABLE features on Intel

[Andy Lutomirski]

Intel allows BIOS or the OS to enable or disable some CPU fueatures via
IA32_MISC_ENABLE.  I have machines that don't enable fast strings or
monitor/mwait in BIOS, so do it on bootup instead.

The Intel SDM volume 3, appendix B.1 says that the OS should not touch
the monitor enable bit if SSE3 is not present, which presumably means
that the OS may touch that bit if SSE3 is present.  In any case, these
patches seem to work.

Andy Lutomirski (2):
  x86: Enable fast strings on Intel if BIOS hasn't already
  x86: Enable monitor/mwait on Intel if BIOS hasn't already

 arch/x86/kernel/cpu/intel.c |   37 +++++++++++++++++++++++++++++++++++--
 1 files changed, 35 insertions(+), 2 deletions(-)

-- 
1.7.6

[PATCH 1/2] x86: Enable fast strings on Intel if BIOS hasn't already

[Andy Lutomirski]

I have two Sandy Bridge systems and neither one enables fast strings
in BIOS.  I can't find anything in the manual that says that only
BIOS should enable or disable the feature, so try to enable it at
bootup.

Signed-off-by: Andy Lutomirski <luto@mit.edu>
---
 arch/x86/kernel/cpu/intel.c |   17 +++++++++++++++--
 1 files changed, 15 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kernel/cpu/intel.c b/arch/x86/kernel/cpu/intel.c
index ed6086e..6b20fef 100644
--- a/arch/x86/kernel/cpu/intel.c
+++ b/arch/x86/kernel/cpu/intel.c
@@ -30,6 +30,7 @@
 static void __cpuinit early_init_intel(struct cpuinfo_x86 *c)
 {
 	u64 misc_enable;
+	bool allow_fast_string = true;
 
 	/* Unmask CPUID levels if masked: */
 	if (c->x86 > 6 || (c->x86 == 6 && c->x86_model >= 0xd)) {
@@ -118,10 +119,11 @@ static void __cpuinit early_init_intel(struct cpuinfo_x86 *c)
 	 * (model 2) with the same problem.
 	 */
 	if (c->x86 == 15) {
-		rdmsrl(MSR_IA32_MISC_ENABLE, misc_enable);
+		allow_fast_string = false;
+		printk(KERN_INFO "kmemcheck: Disabling fast string operations\n");
 
+		rdmsrl(MSR_IA32_MISC_ENABLE, misc_enable);
 		if (misc_enable & MSR_IA32_MISC_ENABLE_FAST_STRING) {
-			printk(KERN_INFO "kmemcheck: Disabling fast string operations\n");
 
 			misc_enable &= ~MSR_IA32_MISC_ENABLE_FAST_STRING;
 			wrmsrl(MSR_IA32_MISC_ENABLE, misc_enable);
@@ -135,6 +137,17 @@ static void __cpuinit early_init_intel(struct cpuinfo_x86 *c)
 	 */
 	if (c->x86 > 6 || (c->x86 == 6 && c->x86_model >= 0xd)) {
 		rdmsrl(MSR_IA32_MISC_ENABLE, misc_enable);
+
+		if (allow_fast_string &&
+		    !(misc_enable & MSR_IA32_MISC_ENABLE_FAST_STRING)) {
+			misc_enable |= MSR_IA32_MISC_ENABLE_FAST_STRING;
+			wrmsrl(MSR_IA32_MISC_ENABLE, misc_enable);
+			printk(KERN_INFO "Enabled fast string operations\n");
+
+			/* Re-read to make sure it stuck. */
+			rdmsrl(MSR_IA32_MISC_ENABLE, misc_enable);
+		}
+
 		if (!(misc_enable & MSR_IA32_MISC_ENABLE_FAST_STRING)) {
 			printk(KERN_INFO "Disabled fast string operations\n");
 			setup_clear_cpu_cap(X86_FEATURE_REP_GOOD);
-- 
1.7.6

RE: [PATCH 1/2] x86: Enable fast strings on Intel if BIOS hasn't already

[Yu, Fenghua]

> -----Original Message-----
> From: Andy Lutomirski [mailto:luto@MIT.EDU]
> Sent: Wednesday, August 03, 2011 8:47 AM
> To: x86@kernel.org; linux-kernel@vger.kernel.org
> Cc: Yu, Fenghua; Andy Lutomirski
> Subject: [PATCH 1/2] x86: Enable fast strings on Intel if BIOS hasn't
> already
> 
> I have two Sandy Bridge systems and neither one enables fast strings
> in BIOS.  I can't find anything in the manual that says that only
> BIOS should enable or disable the feature, so try to enable it at
> bootup.
> 
> Signed-off-by: Andy Lutomirski <luto@mit.edu>
> ---
>  arch/x86/kernel/cpu/intel.c |   17 +++++++++++++++--
>  1 files changed, 15 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/x86/kernel/cpu/intel.c b/arch/x86/kernel/cpu/intel.c
> index ed6086e..6b20fef 100644
> --- a/arch/x86/kernel/cpu/intel.c
> +++ b/arch/x86/kernel/cpu/intel.c
> @@ -30,6 +30,7 @@
>  static void __cpuinit early_init_intel(struct cpuinfo_x86 *c)
>  {
>  	u64 misc_enable;
> +	bool allow_fast_string = true;
> 
>  	/* Unmask CPUID levels if masked: */
>  	if (c->x86 > 6 || (c->x86 == 6 && c->x86_model >= 0xd)) {
> @@ -118,10 +119,11 @@ static void __cpuinit early_init_intel(struct
> cpuinfo_x86 *c)
>  	 * (model 2) with the same problem.
>  	 */
>  	if (c->x86 == 15) {
> -		rdmsrl(MSR_IA32_MISC_ENABLE, misc_enable);
> +		allow_fast_string = false;
> +		printk(KERN_INFO "kmemcheck: Disabling fast string
> operations\n");

Please don't move this printk() here. Keep it in the original place. If fast string bit is 0 already, we don't need to print the info.

> 
> +		rdmsrl(MSR_IA32_MISC_ENABLE, misc_enable);
>  		if (misc_enable & MSR_IA32_MISC_ENABLE_FAST_STRING) {
> -			printk(KERN_INFO "kmemcheck: Disabling fast string
> operations\n");
> 
>  			misc_enable &= ~MSR_IA32_MISC_ENABLE_FAST_STRING;
>  			wrmsrl(MSR_IA32_MISC_ENABLE, misc_enable);
> @@ -135,6 +137,17 @@ static void __cpuinit early_init_intel(struct
> cpuinfo_x86 *c)

You also need to change the comment right before this code. With your patch, the comment is out-of-date.

>  	 */
>  	if (c->x86 > 6 || (c->x86 == 6 && c->x86_model >= 0xd)) {
>  		rdmsrl(MSR_IA32_MISC_ENABLE, misc_enable);
> +
> +		if (allow_fast_string &&
> +		    !(misc_enable & MSR_IA32_MISC_ENABLE_FAST_STRING)) {
> +			misc_enable |= MSR_IA32_MISC_ENABLE_FAST_STRING;
> +			wrmsrl(MSR_IA32_MISC_ENABLE, misc_enable);
> +			printk(KERN_INFO "Enabled fast string operations\n");
> +
> +			/* Re-read to make sure it stuck. */
> +			rdmsrl(MSR_IA32_MISC_ENABLE, misc_enable);
> +		}
> +
>  		if (!(misc_enable & MSR_IA32_MISC_ENABLE_FAST_STRING)) {
>  			printk(KERN_INFO "Disabled fast string
> operations\n");
>  			setup_clear_cpu_cap(X86_FEATURE_REP_GOOD);
> --
> 1.7.6

Re: [PATCH 1/2] x86: Enable fast strings on Intel if BIOS hasn't already

[Andrew Lutomirski]

On Wed, Aug 3, 2011 at 12:34 PM, Yu, Fenghua <fenghua.yu@intel.com> wrote:
>
>
>> -----Original Message-----
>> From: Andy Lutomirski [mailto:luto@MIT.EDU]
>> Sent: Wednesday, August 03, 2011 8:47 AM
>> To: x86@kernel.org; linux-kernel@vger.kernel.org
>> Cc: Yu, Fenghua; Andy Lutomirski
>> Subject: [PATCH 1/2] x86: Enable fast strings on Intel if BIOS hasn't
>> already
>>
>> I have two Sandy Bridge systems and neither one enables fast strings
>> in BIOS.  I can't find anything in the manual that says that only
>> BIOS should enable or disable the feature, so try to enable it at
>> bootup.
>>
>> Signed-off-by: Andy Lutomirski <luto@mit.edu>
>> ---
>>  arch/x86/kernel/cpu/intel.c |   17 +++++++++++++++--
>>  1 files changed, 15 insertions(+), 2 deletions(-)
>>
>> diff --git a/arch/x86/kernel/cpu/intel.c b/arch/x86/kernel/cpu/intel.c
>> index ed6086e..6b20fef 100644
>> --- a/arch/x86/kernel/cpu/intel.c
>> +++ b/arch/x86/kernel/cpu/intel.c
>> @@ -30,6 +30,7 @@
>>  static void __cpuinit early_init_intel(struct cpuinfo_x86 *c)
>>  {
>>       u64 misc_enable;
>> +     bool allow_fast_string = true;
>>
>>       /* Unmask CPUID levels if masked: */
>>       if (c->x86 > 6 || (c->x86 == 6 && c->x86_model >= 0xd)) {
>> @@ -118,10 +119,11 @@ static void __cpuinit early_init_intel(struct
>> cpuinfo_x86 *c)
>>        * (model 2) with the same problem.
>>        */
>>       if (c->x86 == 15) {
>> -             rdmsrl(MSR_IA32_MISC_ENABLE, misc_enable);
>> +             allow_fast_string = false;
>> +             printk(KERN_INFO "kmemcheck: Disabling fast string
>> operations\n");
>
> Please don't move this printk() here. Keep it in the original place. If fast string bit is 0 already, we don't need to print the info.

I'll rework it.

>
>>
>> +             rdmsrl(MSR_IA32_MISC_ENABLE, misc_enable);
>>               if (misc_enable & MSR_IA32_MISC_ENABLE_FAST_STRING) {
>> -                     printk(KERN_INFO "kmemcheck: Disabling fast string
>> operations\n");
>>
>>                       misc_enable &= ~MSR_IA32_MISC_ENABLE_FAST_STRING;
>>                       wrmsrl(MSR_IA32_MISC_ENABLE, misc_enable);
>> @@ -135,6 +137,17 @@ static void __cpuinit early_init_intel(struct
>> cpuinfo_x86 *c)
>
> You also need to change the comment right before this code. With your patch, the comment is out-of-date.

Will do.

--Andy

[PATCH 2/2] x86: Enable monitor/mwait on Intel if BIOS hasn't already

[Andy Lutomirski]

I have a Sandy Bridge machine with an Intel BIOS that enables
monitor/mwait on all but the boot CPU.  With this patch, intel_idle
works.

Signed-off-by: Andy Lutomirski <luto@mit.edu>
---
 arch/x86/kernel/cpu/intel.c |   20 ++++++++++++++++++++
 1 files changed, 20 insertions(+), 0 deletions(-)

diff --git a/arch/x86/kernel/cpu/intel.c b/arch/x86/kernel/cpu/intel.c
index 6b20fef..a3339a1 100644
--- a/arch/x86/kernel/cpu/intel.c
+++ b/arch/x86/kernel/cpu/intel.c
@@ -487,6 +487,26 @@ static void __cpuinit init_intel(struct cpuinfo_x86 *c)
 			wrmsrl(MSR_IA32_ENERGY_PERF_BIAS, epb);
 		}
 	}
+
+	/* Enable monitor/mwait if BIOS didn't do it for us. */
+	if (!cpu_has(c, X86_FEATURE_MWAIT) && cpu_has(c, X86_FEATURE_XMM3)
+	    && c->x86 >= 6 && !(c->x86 == 6 && c->x86_model < 0x1c)
+	    && !(c->x86 == 0xf && c->x86_model < 3)) {
+		u64 misc_enable;
+		rdmsrl(MSR_IA32_MISC_ENABLE, misc_enable);
+		misc_enable |= MSR_IA32_MISC_ENABLE_MWAIT;
+
+		/*
+		 * Some non-SSE3 cpus will #GP.  We check for that,
+		 * but it can't hurt to be safe.
+		 */
+		wrmsr_safe(MSR_IA32_MISC_ENABLE, (u32)misc_enable,
+			   (u32)(misc_enable >> 32));
+
+		/* Re-read monitor capability. */
+		if (cpuid_ecx(1) & 0x8)
+			set_cpu_cap(c, X86_FEATURE_MWAIT);
+	}
 }
 
 #ifdef CONFIG_X86_32
-- 
1.7.6

Re: [PATCH 2/2] x86: Enable monitor/mwait on Intel if BIOS hasn't already

[Len Brown]

On Wed, 3 Aug 2011, Andy Lutomirski wrote:

> I have a Sandy Bridge machine with an Intel BIOS that enables
> monitor/mwait on all but the boot CPU.  With this patch, intel_idle
> works.

What machine has this BIOS bug, and are you running
the latest version of the BIOS?

thanks,
Len Brown, Intel Open Source Technolgy Center

> Signed-off-by: Andy Lutomirski <luto@mit.edu>
> ---
>  arch/x86/kernel/cpu/intel.c |   20 ++++++++++++++++++++
>  1 files changed, 20 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/x86/kernel/cpu/intel.c b/arch/x86/kernel/cpu/intel.c
> index 6b20fef..a3339a1 100644
> --- a/arch/x86/kernel/cpu/intel.c
> +++ b/arch/x86/kernel/cpu/intel.c
> @@ -487,6 +487,26 @@ static void __cpuinit init_intel(struct cpuinfo_x86 *c)
>  			wrmsrl(MSR_IA32_ENERGY_PERF_BIAS, epb);
>  		}
>  	}
> +
> +	/* Enable monitor/mwait if BIOS didn't do it for us. */
> +	if (!cpu_has(c, X86_FEATURE_MWAIT) && cpu_has(c, X86_FEATURE_XMM3)
> +	    && c->x86 >= 6 && !(c->x86 == 6 && c->x86_model < 0x1c)
> +	    && !(c->x86 == 0xf && c->x86_model < 3)) {
> +		u64 misc_enable;
> +		rdmsrl(MSR_IA32_MISC_ENABLE, misc_enable);
> +		misc_enable |= MSR_IA32_MISC_ENABLE_MWAIT;
> +
> +		/*
> +		 * Some non-SSE3 cpus will #GP.  We check for that,
> +		 * but it can't hurt to be safe.
> +		 */
> +		wrmsr_safe(MSR_IA32_MISC_ENABLE, (u32)misc_enable,
> +			   (u32)(misc_enable >> 32));
> +
> +		/* Re-read monitor capability. */
> +		if (cpuid_ecx(1) & 0x8)
> +			set_cpu_cap(c, X86_FEATURE_MWAIT);
> +	}
>  }
>  
>  #ifdef CONFIG_X86_32
> -- 
> 1.7.6
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
> 

Re: [PATCH 2/2] x86: Enable monitor/mwait on Intel if BIOS hasn't already

[Andrew Lutomirski]

On Wed, Aug 3, 2011 at 12:15 PM, Len Brown <lenb@kernel.org> wrote:
> On Wed, 3 Aug 2011, Andy Lutomirski wrote:
>
>> I have a Sandy Bridge machine with an Intel BIOS that enables
>> monitor/mwait on all but the boot CPU.  With this patch, intel_idle
>> works.
>
> What machine has this BIOS bug, and are you running
> the latest version of the BIOS?

Intel DQ67SW, version SWQ6710H.86A.0051.2011.0413.1154.  This is not
quite up-to-date -- I'll test a newer BIOS.

(Enabling monitor/mwait on some but not all CPUs is obnoxious, too.  I
think it's just dumb luck that Linux notices at all and doesn't
crash.)

--Andy

Re: [PATCH 2/2] x86: Enable monitor/mwait on Intel if BIOS hasn't already

[Andrew Lutomirski]

On Wed, Aug 3, 2011 at 12:22 PM, Andrew Lutomirski <luto@mit.edu> wrote:
> On Wed, Aug 3, 2011 at 12:15 PM, Len Brown <lenb@kernel.org> wrote:
>> On Wed, 3 Aug 2011, Andy Lutomirski wrote:
>>
>>> I have a Sandy Bridge machine with an Intel BIOS that enables
>>> monitor/mwait on all but the boot CPU.  With this patch, intel_idle
>>> works.
>>
>> What machine has this BIOS bug, and are you running
>> the latest version of the BIOS?
>
> Intel DQ67SW, version SWQ6710H.86A.0051.2011.0413.1154.  This is not
> quite up-to-date -- I'll test a newer BIOS.

SWQ6710H.86A.0053.2011.0615.1535 has the same problem.  It fixed ASPM, though :)

Should I redo the patch with a printk to warn about the firmware bug?

--Andy

>
> (Enabling monitor/mwait on some but not all CPUs is obnoxious, too.  I
> think it's just dumb luck that Linux notices at all and doesn't
> crash.)
>
> --Andy
>

Re: [PATCH 2/2] x86: Enable monitor/mwait on Intel if BIOS hasn't already

[Len Brown]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 785 bytes --]

> >> What machine has this BIOS bug, and are you running
> >> the latest version of the BIOS?
> >
> > Intel DQ67SW, version SWQ6710H.86A.0051.2011.0413.1154.  This is not
> > quite up-to-date -- I'll test a newer BIOS.
> 
> SWQ6710H.86A.0053.2011.0615.1535 has the same problem.  It fixed ASPM, though :)
> 
> Should I redo the patch with a printk to warn about the firmware bug?

I would hope that a patch that _only_ prints [Firmware bug]
should be enough.  This isn't the kind of workaround
that we should really have to carry in the kernel.

This is still a very new platform, and my guess is that
they are actively updating the BIOS as we speak.
I'll see if I can figure out who to talk to
in order to make sure of that:-)

thanks,
Len Brown, Intel Open Source Technology Center

Re: [PATCH 2/2] x86: Enable monitor/mwait on Intel if BIOS hasn't already

[Andrew Lutomirski]

On Wed, Aug 3, 2011 at 3:17 PM, Len Brown <lenb@kernel.org> wrote:
>> >> What machine has this BIOS bug, and are you running
>> >> the latest version of the BIOS?
>> >
>> > Intel DQ67SW, version SWQ6710H.86A.0051.2011.0413.1154.  This is not
>> > quite up-to-date -- I'll test a newer BIOS.
>>
>> SWQ6710H.86A.0053.2011.0615.1535 has the same problem.  It fixed ASPM, though :)
>>
>> Should I redo the patch with a printk to warn about the firmware bug?
>
> I would hope that a patch that _only_ prints [Firmware bug]
> should be enough.  This isn't the kind of workaround
> that we should really have to carry in the kernel.
>
> This is still a very new platform, and my guess is that
> they are actively updating the BIOS as we speak.
> I'll see if I can figure out who to talk to
> in order to make sure of that:-)

I can add a strange data point:  with TXT enabled in BIOS, fast
strings are disabled (on all CPUs, I think) and monitor/mwait is
disabled on the boot CPU.  With TXT disabled, both features are
enabled.

I'll see if my laptop does the same thing.

--Andy

Re: [PATCH 2/2] x86: Enable monitor/mwait on Intel if BIOS hasn't already

[Matthew Garrett]

On Wed, Aug 03, 2011 at 03:17:59PM -0400, Len Brown wrote:
> > >> What machine has this BIOS bug, and are you running
> > >> the latest version of the BIOS?
> > >
> > > Intel DQ67SW, version SWQ6710H.86A.0051.2011.0413.1154.  This is not
> > > quite up-to-date -- I'll test a newer BIOS.
> > 
> > SWQ6710H.86A.0053.2011.0615.1535 has the same problem.  It fixed ASPM, though :)
> > 
> > Should I redo the patch with a printk to warn about the firmware bug?
> 
> I would hope that a patch that _only_ prints [Firmware bug]
> should be enough.  This isn't the kind of workaround
> that we should really have to carry in the kernel.

We can't trust BIOS authors to get things right. Where we can reliably 
fix up things that are broken, we should do so.

-- 
Matthew Garrett | mjg59@srcf.ucam.org

Re: [PATCH 2/2] x86: Enable monitor/mwait on Intel if BIOS hasn't already

[Len Brown]

> We can't trust BIOS authors to get things right. Where we can reliably 
> fix up things that are broken, we should do so.

How about if we can get a BIOS fix for this brand new system,
then we shall not clutter the kernel with a workaround,
otherwise we shall.

thanks,
-Len

Re: [PATCH 2/2] x86: Enable monitor/mwait on Intel if BIOS hasn't already

[Matthew Garrett]

On Thu, Aug 04, 2011 at 05:54:04PM -0400, Len Brown wrote:
> > We can't trust BIOS authors to get things right. Where we can reliably 
> > fix up things that are broken, we should do so.
> 
> How about if we can get a BIOS fix for this brand new system,
> then we shall not clutter the kernel with a workaround,
> otherwise we shall.

It's a pretty good rule of thumb that if a BIOS bug is present in one 
BIOS it's present in others, but the users will just never notice.

-- 
Matthew Garrett | mjg59@srcf.ucam.org

Re: [PATCH 2/2] x86: Enable monitor/mwait on Intel if BIOS hasn't already

[Ingo Molnar]

* Matthew Garrett <mjg59@srcf.ucam.org> wrote:

> On Thu, Aug 04, 2011 at 05:54:04PM -0400, Len Brown wrote:
> > > We can't trust BIOS authors to get things right. Where we can reliably 
> > > fix up things that are broken, we should do so.
> > 
> > How about if we can get a BIOS fix for this brand new system, 
> > then we shall not clutter the kernel with a workaround, otherwise 
> > we shall.
> 
> It's a pretty good rule of thumb that if a BIOS bug is present in 
> one BIOS it's present in others, but the users will just never 
> notice.

Yes. There's no harm really from working around a BIOS bug and 
warning about it - when the BIOS gets fixed the warning message will 
go away.

Btw., few people risk upgrading their BIOSen on already purchased 
hardware. I almost never do it, i only do it if it's impossible to 
continue with a current BIOS.

Thanks,

	Ingo

Re: [PATCH 2/2] x86: Enable monitor/mwait on Intel if BIOS hasn't already

[Andrew Lutomirski]

On Fri, Aug 5, 2011 at 5:43 AM, Ingo Molnar <mingo@elte.hu> wrote:
>
> * Matthew Garrett <mjg59@srcf.ucam.org> wrote:
>
>> On Thu, Aug 04, 2011 at 05:54:04PM -0400, Len Brown wrote:
>> > > We can't trust BIOS authors to get things right. Where we can reliably
>> > > fix up things that are broken, we should do so.
>> >
>> > How about if we can get a BIOS fix for this brand new system,
>> > then we shall not clutter the kernel with a workaround, otherwise
>> > we shall.
>>
>> It's a pretty good rule of thumb that if a BIOS bug is present in
>> one BIOS it's present in others, but the users will just never
>> notice.
>
> Yes. There's no harm really from working around a BIOS bug and
> warning about it - when the BIOS gets fixed the warning message will
> go away.
>
> Btw., few people risk upgrading their BIOSen on already purchased
> hardware. I almost never do it, i only do it if it's impossible to
> continue with a current BIOS.

I do it, but I understand the sentiment.  I've destroyed laptops with
BIOS upgrades, and the new Intel BIOS seems to insist on spewing
garbage into my keyboard input when Linux (but not grub) starts.

Updates patches coming.

>
> Thanks,
>
>        Ingo
>