| Message ID | 9a8f28ccbe44cd323a01e9a23b531cb869185a21.1312630712.git.luto@mit.edu (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
> -----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 -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> -----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 -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> -----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 -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
* 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 -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 > -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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);
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(-)