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

[PATCH v2 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.

Changes from v1:
 - Display FW_WARN messages.
 - Don't change the kmemcheck message.
 - Improve the fast string comment.
 - Improve the changelogs.

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 |   50 ++++++++++++++++++++++++++++++++++++++++--
 1 files changed, 47 insertions(+), 3 deletions(-)

-- 
1.7.6

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

[Andy Lutomirski]

Intel SDM volume 3A, 8.4.2 says:

  Software can disable fast-string operation by clearing the
  fast-string-enable bit (bit 0) of IA32_MISC_ENABLE MSR.
  However, Intel recomments that system software always enable
  fast-string operation.

The Intel DQ67SW board (with latest BIOS) disables fast string
operations if TXT is enabled.  A Lenovo X220 disables it regardless
of TXT setting.  I doubt I'm the only person with a dumb BIOS like
this.

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

diff --git a/arch/x86/kernel/cpu/intel.c b/arch/x86/kernel/cpu/intel.c
index ed6086e..c80ab41 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,8 +119,9 @@ 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;
 
+		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");
 
@@ -130,11 +132,28 @@ static void __cpuinit early_init_intel(struct cpuinfo_x86 *c)
 #endif
 
 	/*
-	 * If fast string is not enabled in IA32_MISC_ENABLE for any reason,
-	 * clear the fast string and enhanced fast string CPU capabilities.
+	 * If BIOS didn't enable fast string operation, try to enable
+	 * it ourselves.  If that fails, then clear the fast string
+	 * and enhanced fast string CPU capabilities.
 	 */
 	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;
+			wrmsr_safe(MSR_IA32_MISC_ENABLE, (u32)misc_enable,
+				   (u32)(misc_enable >> 32));
+
+			/* 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_WARNING FW_WARN "CPU #%d: "
+				       "IA32_MISC_ENABLE.FAST_STRING_ENABLE "
+				       "was not set", c->cpu_index);
+		}
+
 		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

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

[Andy Lutomirski]

My Intel DQ67SW (latest BIOS) disables monitor/mwait on the boot CPU
if TXT is enabled.  We're lucky that the system works at all, since
the feature is still enabled on other CPUs.

The obvious fix is to just re-enable it ourselves.

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

diff --git a/arch/x86/kernel/cpu/intel.c b/arch/x86/kernel/cpu/intel.c
index c80ab41..5a0150c 100644
--- a/arch/x86/kernel/cpu/intel.c
+++ b/arch/x86/kernel/cpu/intel.c
@@ -493,6 +493,31 @@ 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);
+
+			printk(KERN_WARNING FW_WARN "CPU #%d: "
+			       "IA32_MISC_ENABLE.ENABLE_MONITOR_FSM "
+			       "was not set\n", c->cpu_index);
+		}
+	}
 }
 
 #ifdef CONFIG_X86_32
-- 
1.7.6

RE: [PATCH v2 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: Saturday, August 06, 2011 4:43 AM
> To: x86@kernel.org; linux-kernel@vger.kernel.org
> Cc: Yu, Fenghua; Matthew Garrett; Len Brown; linux-
> acpi@vger.kernel.org; Ingo Molnar; Andy Lutomirski
> Subject: [PATCH v2 1/2] x86: Enable fast strings on Intel if BIOS
> hasn't already
> 
> Intel SDM volume 3A, 8.4.2 says:
> 
>   Software can disable fast-string operation by clearing the
>   fast-string-enable bit (bit 0) of IA32_MISC_ENABLE MSR.
>   However, Intel recomments that system software always enable
>   fast-string operation.
> 
> The Intel DQ67SW board (with latest BIOS) disables fast string
> operations if TXT is enabled.  A Lenovo X220 disables it regardless
> of TXT setting.  I doubt I'm the only person with a dumb BIOS like
> this.
> 
> Signed-off-by: Andy Lutomirski <luto@mit.edu>
> ---
>  arch/x86/kernel/cpu/intel.c |   25 ++++++++++++++++++++++---
>  1 files changed, 22 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/x86/kernel/cpu/intel.c b/arch/x86/kernel/cpu/intel.c
> index ed6086e..c80ab41 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,8 +119,9 @@ 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;
> 
> +		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");
> 
> @@ -130,11 +132,28 @@ static void __cpuinit early_init_intel(struct
> cpuinfo_x86 *c)
>  #endif
> 
>  	/*
> -	 * If fast string is not enabled in IA32_MISC_ENABLE for any
> reason,
> -	 * clear the fast string and enhanced fast string CPU
> capabilities.
> +	 * If BIOS didn't enable fast string operation, try to enable
> +	 * it ourselves.  If that fails, then clear the fast string
> +	 * and enhanced fast string CPU capabilities.
>  	 */
>  	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;
> +			wrmsr_safe(MSR_IA32_MISC_ENABLE, (u32)misc_enable,
> +				   (u32)(misc_enable >> 32));
> +
> +			/* 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_WARNING FW_WARN "CPU #%d: "
> +				       "IA32_MISC_ENABLE.FAST_STRING_ENABLE "
> +				       "was not set", c->cpu_index);
This printk is redundant because the same info is dumped in the below printk. Plus it's not firmware's issue if we can not set fast string bit 0 in MISC_ENABLE register. So I don't think FW_WARN is right.

I would suggest to remove this printk (and the if () statement) and change KERN_INFO to KERN_WARNING in the following printk.

> +		}
> +
>  		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 v2 1/2] x86: Enable fast strings on Intel if BIOS hasn't already

[Andrew Lutomirski]

On Sat, Aug 6, 2011 at 3:33 PM, Yu, Fenghua <fenghua.yu@intel.com> wrote:
>> -----Original Message-----
>> From: Andy Lutomirski [mailto:luto@MIT.EDU]
>> Sent: Saturday, August 06, 2011 4:43 AM
>> To: x86@kernel.org; linux-kernel@vger.kernel.org
>> Cc: Yu, Fenghua; Matthew Garrett; Len Brown; linux-
>> acpi@vger.kernel.org; Ingo Molnar; Andy Lutomirski
>> Subject: [PATCH v2 1/2] x86: Enable fast strings on Intel if BIOS
>> hasn't already
>>
>> Intel SDM volume 3A, 8.4.2 says:
>>
>>   Software can disable fast-string operation by clearing the
>>   fast-string-enable bit (bit 0) of IA32_MISC_ENABLE MSR.
>>   However, Intel recomments that system software always enable
>>   fast-string operation.
>>
>> The Intel DQ67SW board (with latest BIOS) disables fast string
>> operations if TXT is enabled.  A Lenovo X220 disables it regardless
>> of TXT setting.  I doubt I'm the only person with a dumb BIOS like
>> this.
>>
>> Signed-off-by: Andy Lutomirski <luto@mit.edu>
>> ---
>>  arch/x86/kernel/cpu/intel.c |   25 ++++++++++++++++++++++---
>>  1 files changed, 22 insertions(+), 3 deletions(-)
>>
>> diff --git a/arch/x86/kernel/cpu/intel.c b/arch/x86/kernel/cpu/intel.c
>> index ed6086e..c80ab41 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,8 +119,9 @@ 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;
>>
>> +             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");
>>
>> @@ -130,11 +132,28 @@ static void __cpuinit early_init_intel(struct
>> cpuinfo_x86 *c)
>>  #endif
>>
>>       /*
>> -      * If fast string is not enabled in IA32_MISC_ENABLE for any
>> reason,
>> -      * clear the fast string and enhanced fast string CPU
>> capabilities.
>> +      * If BIOS didn't enable fast string operation, try to enable
>> +      * it ourselves.  If that fails, then clear the fast string
>> +      * and enhanced fast string CPU capabilities.
>>        */
>>       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;
>> +                     wrmsr_safe(MSR_IA32_MISC_ENABLE, (u32)misc_enable,
>> +                                (u32)(misc_enable >> 32));
>> +
>> +                     /* 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_WARNING FW_WARN "CPU #%d: "
>> +                                    "IA32_MISC_ENABLE.FAST_STRING_ENABLE "
>> +                                    "was not set", c->cpu_index);
> This printk is redundant because the same info is dumped in the below printk. Plus it's not firmware's issue if we can not set fast string bit 0 in MISC_ENABLE register. So I don't think FW_WARN is right.

Huh?  This is the success path -- the patch prints the warning if
flipping the bit worked...

>
> I would suggest to remove this printk (and the if () statement) and change KERN_INFO to KERN_WARNING in the following printk.
>
>> +             }
>> +
>>               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);

...and this (which could be more clear) is displayed if flipping the
bit did not work, in which case the problem isn't BIOS's fault.

--Andy

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

[Yu, Fenghua]

> -----Original Message-----
> From: amluto@gmail.com [mailto:amluto@gmail.com] On Behalf Of Andrew
> Lutomirski
> Sent: Saturday, August 06, 2011 4:32 PM
> To: Yu, Fenghua
> Cc: x86@kernel.org; linux-kernel@vger.kernel.org; Matthew Garrett; Len
> Brown; linux-acpi@vger.kernel.org; Ingo Molnar
> Subject: Re: [PATCH v2 1/2] x86: Enable fast strings on Intel if BIOS
> hasn't already
> 
> On Sat, Aug 6, 2011 at 3:33 PM, Yu, Fenghua <fenghua.yu@intel.com>
> wrote:
> >> -----Original Message-----
> >> From: Andy Lutomirski [mailto:luto@MIT.EDU]
> >> Sent: Saturday, August 06, 2011 4:43 AM
> >> To: x86@kernel.org; linux-kernel@vger.kernel.org
> >> Cc: Yu, Fenghua; Matthew Garrett; Len Brown; linux-
> >> acpi@vger.kernel.org; Ingo Molnar; Andy Lutomirski
> >> Subject: [PATCH v2 1/2] x86: Enable fast strings on Intel if BIOS
> >> hasn't already
> >>
> >> Intel SDM volume 3A, 8.4.2 says:
> >>
> >>   Software can disable fast-string operation by clearing the
> >>   fast-string-enable bit (bit 0) of IA32_MISC_ENABLE MSR.
> >>   However, Intel recomments that system software always enable
> >>   fast-string operation.
> >>
> >> The Intel DQ67SW board (with latest BIOS) disables fast string
> >> operations if TXT is enabled.  A Lenovo X220 disables it regardless
> >> of TXT setting.  I doubt I'm the only person with a dumb BIOS like
> >> this.
> >>
> >> Signed-off-by: Andy Lutomirski <luto@mit.edu>
> >> ---
> >>  arch/x86/kernel/cpu/intel.c |   25 ++++++++++++++++++++++---
> >>  1 files changed, 22 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/arch/x86/kernel/cpu/intel.c
> b/arch/x86/kernel/cpu/intel.c
> >> index ed6086e..c80ab41 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,8 +119,9 @@ 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;
> >>
> >> +             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");
> >>
> >> @@ -130,11 +132,28 @@ static void __cpuinit early_init_intel(struct
> >> cpuinfo_x86 *c)
> >>  #endif
> >>
> >>       /*
> >> -      * If fast string is not enabled in IA32_MISC_ENABLE for any
> >> reason,
> >> -      * clear the fast string and enhanced fast string CPU
> >> capabilities.
> >> +      * If BIOS didn't enable fast string operation, try to enable
> >> +      * it ourselves.  If that fails, then clear the fast string
> >> +      * and enhanced fast string CPU capabilities.
> >>        */
> >>       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;
> >> +                     wrmsr_safe(MSR_IA32_MISC_ENABLE,
> (u32)misc_enable,
> >> +                                (u32)(misc_enable >> 32));
> >> +
> >> +                     /* 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_WARNING FW_WARN "CPU #%d:
> "
> >> +
>  "IA32_MISC_ENABLE.FAST_STRING_ENABLE "
> >> +                                    "was not set", c->cpu_index);
> > This printk is redundant because the same info is dumped in the below
> printk. Plus it's not firmware's issue if we can not set fast string
> bit 0 in MISC_ENABLE register. So I don't think FW_WARN is right.
> 
> Huh?  This is the success path -- the patch prints the warning if
> flipping the bit worked...

Ok. I see.

Still I would suggest to remove this printk. This info will be printed on every single CPU if BIOS doesn't enable fast string. This could flood the log buffer on big machines or many threads machines. And the info doesn't really provide useful message because we enable fast string in OS anyway. Reporting BIOS issue by this info probably will be viewed as low priority or ignored. 

The following info is more important because it reports an abnormal behavior.

Thanks.

-Fenghua

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

[Yu, Fenghua]

> -----Original Message-----
> From: Yu, Fenghua
> Sent: Sunday, August 07, 2011 4:45 PM
> To: 'Andrew Lutomirski'
> Cc: x86@kernel.org; linux-kernel@vger.kernel.org; Matthew Garrett; Len
> Brown; linux-acpi@vger.kernel.org; Ingo Molnar
> Subject: RE: [PATCH v2 1/2] x86: Enable fast strings on Intel if BIOS
> hasn't already
> 
> > -----Original Message-----
> > From: amluto@gmail.com [mailto:amluto@gmail.com] On Behalf Of Andrew
> > Lutomirski
> > Sent: Saturday, August 06, 2011 4:32 PM
> > To: Yu, Fenghua
> > Cc: x86@kernel.org; linux-kernel@vger.kernel.org; Matthew Garrett;
> Len
> > Brown; linux-acpi@vger.kernel.org; Ingo Molnar
> > Subject: Re: [PATCH v2 1/2] x86: Enable fast strings on Intel if BIOS
> > hasn't already
> >
> > On Sat, Aug 6, 2011 at 3:33 PM, Yu, Fenghua <fenghua.yu@intel.com>
> > wrote:
> > >> -----Original Message-----
> > >> From: Andy Lutomirski [mailto:luto@MIT.EDU]
> > >> Sent: Saturday, August 06, 2011 4:43 AM
> > >> To: x86@kernel.org; linux-kernel@vger.kernel.org
> > >> Cc: Yu, Fenghua; Matthew Garrett; Len Brown; linux-
> > >> acpi@vger.kernel.org; Ingo Molnar; Andy Lutomirski
> > >> Subject: [PATCH v2 1/2] x86: Enable fast strings on Intel if BIOS
> > >> hasn't already
> > >>
> > >> Intel SDM volume 3A, 8.4.2 says:
> > >>
> > >>   Software can disable fast-string operation by clearing the
> > >>   fast-string-enable bit (bit 0) of IA32_MISC_ENABLE MSR.
> > >>   However, Intel recomments that system software always enable
> > >>   fast-string operation.
> > >>
> > >> The Intel DQ67SW board (with latest BIOS) disables fast string
> > >> operations if TXT is enabled.  A Lenovo X220 disables it
> regardless
> > >> of TXT setting.  I doubt I'm the only person with a dumb BIOS like
> > >> this.
> > >>
> > >> Signed-off-by: Andy Lutomirski <luto@mit.edu>
> > >> ---
> > >>  arch/x86/kernel/cpu/intel.c |   25 ++++++++++++++++++++++---
> > >>  1 files changed, 22 insertions(+), 3 deletions(-)
> > >>
> > >> diff --git a/arch/x86/kernel/cpu/intel.c
> > b/arch/x86/kernel/cpu/intel.c
> > >> index ed6086e..c80ab41 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,8 +119,9 @@ 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;
> > >>
> > >> +             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");
> > >>
> > >> @@ -130,11 +132,28 @@ static void __cpuinit
> early_init_intel(struct
> > >> cpuinfo_x86 *c)
> > >>  #endif
> > >>
> > >>       /*
> > >> -      * If fast string is not enabled in IA32_MISC_ENABLE for any
> > >> reason,
> > >> -      * clear the fast string and enhanced fast string CPU
> > >> capabilities.
> > >> +      * If BIOS didn't enable fast string operation, try to
> enable
> > >> +      * it ourselves.  If that fails, then clear the fast string
> > >> +      * and enhanced fast string CPU capabilities.
> > >>        */
> > >>       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;
> > >> +                     wrmsr_safe(MSR_IA32_MISC_ENABLE,
> > (u32)misc_enable,
> > >> +                                (u32)(misc_enable >> 32));
> > >> +
> > >> +                     /* 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_WARNING FW_WARN "CPU
> #%d:
> > "
> > >> +
> >  "IA32_MISC_ENABLE.FAST_STRING_ENABLE "
> > >> +                                    "was not set", c->cpu_index);
> > > This printk is redundant because the same info is dumped in the
> below
> > printk. Plus it's not firmware's issue if we can not set fast string
> > bit 0 in MISC_ENABLE register. So I don't think FW_WARN is right.
> >
> > Huh?  This is the success path -- the patch prints the warning if
> > flipping the bit worked...
> 
> Ok. I see.
> 
> Still I would suggest to remove this printk. This info will be printed
> on every single CPU if BIOS doesn't enable fast string. This could
> flood the log buffer on big machines or many threads machines. And the

On the bottom line, you need to use printk_once to reduce repeated message on each CPU.

Thanks.
 
-Fenghua

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

[Ingo Molnar]

* Yu, Fenghua <fenghua.yu@intel.com> wrote:

> > -----Original Message-----
> > From: amluto@gmail.com [mailto:amluto@gmail.com] On Behalf Of Andrew
> > Lutomirski
> > Sent: Saturday, August 06, 2011 4:32 PM
> > To: Yu, Fenghua
> > Cc: x86@kernel.org; linux-kernel@vger.kernel.org; Matthew Garrett; Len
> > Brown; linux-acpi@vger.kernel.org; Ingo Molnar
> > Subject: Re: [PATCH v2 1/2] x86: Enable fast strings on Intel if BIOS
> > hasn't already
> > 
> > On Sat, Aug 6, 2011 at 3:33 PM, Yu, Fenghua <fenghua.yu@intel.com>
> > wrote:
> > >> -----Original Message-----
> > >> From: Andy Lutomirski [mailto:luto@MIT.EDU]
> > >> Sent: Saturday, August 06, 2011 4:43 AM
> > >> To: x86@kernel.org; linux-kernel@vger.kernel.org
> > >> Cc: Yu, Fenghua; Matthew Garrett; Len Brown; linux-
> > >> acpi@vger.kernel.org; Ingo Molnar; Andy Lutomirski
> > >> Subject: [PATCH v2 1/2] x86: Enable fast strings on Intel if BIOS
> > >> hasn't already
> > >>
> > >> Intel SDM volume 3A, 8.4.2 says:
> > >>
> > >>   Software can disable fast-string operation by clearing the
> > >>   fast-string-enable bit (bit 0) of IA32_MISC_ENABLE MSR.
> > >>   However, Intel recomments that system software always enable
> > >>   fast-string operation.
> > >>
> > >> The Intel DQ67SW board (with latest BIOS) disables fast string
> > >> operations if TXT is enabled.  A Lenovo X220 disables it regardless
> > >> of TXT setting.  I doubt I'm the only person with a dumb BIOS like
> > >> this.
> > >>
> > >> Signed-off-by: Andy Lutomirski <luto@mit.edu>
> > >> ---
> > >>  arch/x86/kernel/cpu/intel.c |   25 ++++++++++++++++++++++---
> > >>  1 files changed, 22 insertions(+), 3 deletions(-)
> > >>
> > >> diff --git a/arch/x86/kernel/cpu/intel.c
> > b/arch/x86/kernel/cpu/intel.c
> > >> index ed6086e..c80ab41 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,8 +119,9 @@ 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;
> > >>
> > >> +             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");
> > >>
> > >> @@ -130,11 +132,28 @@ static void __cpuinit early_init_intel(struct
> > >> cpuinfo_x86 *c)
> > >>  #endif
> > >>
> > >>       /*
> > >> -      * If fast string is not enabled in IA32_MISC_ENABLE for any
> > >> reason,
> > >> -      * clear the fast string and enhanced fast string CPU
> > >> capabilities.
> > >> +      * If BIOS didn't enable fast string operation, try to enable
> > >> +      * it ourselves.  If that fails, then clear the fast string
> > >> +      * and enhanced fast string CPU capabilities.
> > >>        */
> > >>       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;
> > >> +                     wrmsr_safe(MSR_IA32_MISC_ENABLE,
> > (u32)misc_enable,
> > >> +                                (u32)(misc_enable >> 32));
> > >> +
> > >> +                     /* 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_WARNING FW_WARN "CPU #%d:
> > "
> > >> +
> >  "IA32_MISC_ENABLE.FAST_STRING_ENABLE "
> > >> +                                    "was not set", c->cpu_index);
> > > This printk is redundant because the same info is dumped in the below
> > printk. Plus it's not firmware's issue if we can not set fast string
> > bit 0 in MISC_ENABLE register. So I don't think FW_WARN is right.
> > 
> > Huh?  This is the success path -- the patch prints the warning if
> > flipping the bit worked...
> 
> Ok. I see.
> 
> Still I would suggest to remove this printk. This info will be 
> printed on every single CPU if BIOS doesn't enable fast string. 

just do printk_once(). We want to inform the user that something's 
going on.

Thanks,

	Ingo

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

[Andrew Lutomirski]

On Tue, Aug 9, 2011 at 11:00 AM, Ingo Molnar <mingo@elte.hu> wrote:
>
> * Yu, Fenghua <fenghua.yu@intel.com> wrote:
>
>> > -----Original Message-----
>> > From: amluto@gmail.com [mailto:amluto@gmail.com] On Behalf Of Andrew
>> > Lutomirski
>> > Sent: Saturday, August 06, 2011 4:32 PM
>> > To: Yu, Fenghua
>> > Cc: x86@kernel.org; linux-kernel@vger.kernel.org; Matthew Garrett; Len
>> > Brown; linux-acpi@vger.kernel.org; Ingo Molnar
>> > Subject: Re: [PATCH v2 1/2] x86: Enable fast strings on Intel if BIOS
>> > hasn't already
>> >
>> > On Sat, Aug 6, 2011 at 3:33 PM, Yu, Fenghua <fenghua.yu@intel.com>
>> > wrote:
>> > >> -----Original Message-----
>> > >> From: Andy Lutomirski [mailto:luto@MIT.EDU]
>> > >> Sent: Saturday, August 06, 2011 4:43 AM
>> > >> To: x86@kernel.org; linux-kernel@vger.kernel.org
>> > >> Cc: Yu, Fenghua; Matthew Garrett; Len Brown; linux-
>> > >> acpi@vger.kernel.org; Ingo Molnar; Andy Lutomirski
>> > >> Subject: [PATCH v2 1/2] x86: Enable fast strings on Intel if BIOS
>> > >> hasn't already
>> > >>
>> > >> Intel SDM volume 3A, 8.4.2 says:
>> > >>
>> > >>   Software can disable fast-string operation by clearing the
>> > >>   fast-string-enable bit (bit 0) of IA32_MISC_ENABLE MSR.
>> > >>   However, Intel recomments that system software always enable
>> > >>   fast-string operation.
>> > >>
>> > >> The Intel DQ67SW board (with latest BIOS) disables fast string
>> > >> operations if TXT is enabled.  A Lenovo X220 disables it regardless
>> > >> of TXT setting.  I doubt I'm the only person with a dumb BIOS like
>> > >> this.
>> > >>
>> > >> Signed-off-by: Andy Lutomirski <luto@mit.edu>
>> > >> ---
>> > >>  arch/x86/kernel/cpu/intel.c |   25 ++++++++++++++++++++++---
>> > >>  1 files changed, 22 insertions(+), 3 deletions(-)
>> > >>
>> > >> diff --git a/arch/x86/kernel/cpu/intel.c
>> > b/arch/x86/kernel/cpu/intel.c
>> > >> index ed6086e..c80ab41 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,8 +119,9 @@ 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;
>> > >>
>> > >> +             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");
>> > >>
>> > >> @@ -130,11 +132,28 @@ static void __cpuinit early_init_intel(struct
>> > >> cpuinfo_x86 *c)
>> > >>  #endif
>> > >>
>> > >>       /*
>> > >> -      * If fast string is not enabled in IA32_MISC_ENABLE for any
>> > >> reason,
>> > >> -      * clear the fast string and enhanced fast string CPU
>> > >> capabilities.
>> > >> +      * If BIOS didn't enable fast string operation, try to enable
>> > >> +      * it ourselves.  If that fails, then clear the fast string
>> > >> +      * and enhanced fast string CPU capabilities.
>> > >>        */
>> > >>       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;
>> > >> +                     wrmsr_safe(MSR_IA32_MISC_ENABLE,
>> > (u32)misc_enable,
>> > >> +                                (u32)(misc_enable >> 32));
>> > >> +
>> > >> +                     /* 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_WARNING FW_WARN "CPU #%d:
>> > "
>> > >> +
>> >  "IA32_MISC_ENABLE.FAST_STRING_ENABLE "
>> > >> +                                    "was not set", c->cpu_index);
>> > > This printk is redundant because the same info is dumped in the below
>> > printk. Plus it's not firmware's issue if we can not set fast string
>> > bit 0 in MISC_ENABLE register. So I don't think FW_WARN is right.
>> >
>> > Huh?  This is the success path -- the patch prints the warning if
>> > flipping the bit worked...
>>
>> Ok. I see.
>>
>> Still I would suggest to remove this printk. This info will be
>> printed on every single CPU if BIOS doesn't enable fast string.
>
> just do printk_once(). We want to inform the user that something's
> going on.

Already there in v3.

--Andy

>
> Thanks,
>
>        Ingo
>