Messages in this thread | | | Date | Thu, 13 Aug 2009 19:45:55 -0300 | From | Kevin Winchester <> | Subject | Re: [PATCH 2/2] x86: Clear incorrectly forced X86_FEATURE_LAHF_LM flag |
| |
Brian Gerst wrote: > On Thu, Aug 13, 2009 at 10:54 AM, Kevin > Winchester<kjwinchester@gmail.com> wrote: >> 2009/8/13 Brian Gerst <brgerst@gmail.com>: >>> On Thu, Aug 13, 2009 at 8:31 AM, Borislav Petkov<borislav.petkov@amd.com> wrote: >>>> From: Kevin Winchester <kjwinchester@gmail.com> >>>> >>>> Due to an erratum with certain AMD Athlon 64 processors, the BIOS may >>>> need to force enable the LAHF_LM capability. Unfortunately, in at >>>> least one case, the BIOS does this even for processors that do not >>>> support the functionality. >>>> >>>> Add a specific check that will clear the feature bit for processors >>>> known not to support the LAHF/SAHF instructions. >>>> >>>> Borislav: turn off cpuid bit. >>>> >>>> Signed-off-by: Kevin Winchester <kjwinchester@gmail.com> >>>> Signed-off-by: Borislav Petkov <borislav.petkov@amd.com> >>>> --- >>>> arch/x86/kernel/cpu/amd.c | 16 ++++++++++++++++ >>>> 1 files changed, 16 insertions(+), 0 deletions(-) >>>> >>>> diff --git a/arch/x86/kernel/cpu/amd.c b/arch/x86/kernel/cpu/amd.c >>>> index e2485b0..9cd6fc7 100644 >>>> --- a/arch/x86/kernel/cpu/amd.c >>>> +++ b/arch/x86/kernel/cpu/amd.c >>>> @@ -400,6 +400,22 @@ static void __cpuinit init_amd(struct cpuinfo_x86 *c) >>>> level = cpuid_eax(1); >>>> if((level >= 0x0f48 && level < 0x0f50) || level >= 0x0f58) >>>> set_cpu_cap(c, X86_FEATURE_REP_GOOD); >>>> + >>>> + /* >>>> + * Some BIOSes incorrectly force this feature, but only K8 >>>> + * revision D (model = 0x14) and later actually support it. >>>> + */ >>>> + if (c->x86_model < 0x14) { >>> Shouldn't you test that the flag is actually set before trying to clear it? >>> >> Possibly. If there were some concern that: >> >> - The extra instructions would cause a performance impact, and the >> test was significantly faster than the clear. > > Testing a bit is cheap and MSR accesses are not. > >> - The extra instructions might actually cause more problems if the >> flag is not set. > > These MSRs don't exist on older cpus and will cause a fault, which is > handled at additional cost. >
I stand corrected. I was unaware of this, so I guess testing the flag first would be a good idea.
Thanks,
-- Kevin Winchester
| |