[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <CAObL_7GA4BDvVfERqdK3XnTnwRdG+Zdp9EjZPtEQjWFvnSSO5Q@mail.gmail.com>
Date: Tue, 9 Aug 2011 11:12:38 -0400
From: Andrew Lutomirski <luto@....edu>
To: Ingo Molnar <mingo@...e.hu>
Cc: "Yu, Fenghua" <fenghua.yu@...el.com>,
"x86@...nel.org" <x86@...nel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
Matthew Garrett <mjg59@...f.ucam.org>,
Len Brown <lenb@...nel.org>,
"linux-acpi@...r.kernel.org" <linux-acpi@...r.kernel.org>
Subject: Re: [PATCH v2 1/2] x86: Enable fast strings on Intel if BIOS hasn't already
On Tue, Aug 9, 2011 at 11:00 AM, Ingo Molnar <mingo@...e.hu> wrote:
>
> * Yu, Fenghua <fenghua.yu@...el.com> wrote:
>
>> > -----Original Message-----
>> > From: amluto@...il.com [mailto:amluto@...il.com] On Behalf Of Andrew
>> > Lutomirski
>> > Sent: Saturday, August 06, 2011 4:32 PM
>> > To: Yu, Fenghua
>> > Cc: x86@...nel.org; linux-kernel@...r.kernel.org; Matthew Garrett; Len
>> > Brown; linux-acpi@...r.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@...el.com>
>> > wrote:
>> > >> -----Original Message-----
>> > >> From: Andy Lutomirski [mailto:luto@....EDU]
>> > >> Sent: Saturday, August 06, 2011 4:43 AM
>> > >> To: x86@...nel.org; linux-kernel@...r.kernel.org
>> > >> Cc: Yu, Fenghua; Matthew Garrett; Len Brown; linux-
>> > >> acpi@...r.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@....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-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists