Re: [RFC PATCH 5/6] x86/fpu: Fix CPUID-less FPU detection

[Andy Lutomirski]

On Tue, Dec 6, 2016 at 1:40 AM, Borislav Petkov <bp@xxxxxxxxx> wrote:
> On Mon, Dec 05, 2016 at 05:01:14PM -0800, Andy Lutomirski wrote:
>> The old code didn't work at all because it adjusted the current caps
>> instead of the forced caps.  Anything it did would be undone later
>> during cpu identification.  Fix that and, while we're at it, improve
>> the logging and don't bother running it if CPUID is available.
>>
>> Signed-off-by: Andy Lutomirski <luto@xxxxxxxxxx>
>> ---
>>  arch/x86/kernel/fpu/init.c | 28 ++++++++++++++++------------
>>  1 file changed, 16 insertions(+), 12 deletions(-)
>>
>> diff --git a/arch/x86/kernel/fpu/init.c b/arch/x86/kernel/fpu/init.c
>> index 60dece392b3a..75e1bf3b0319 100644
>> --- a/arch/x86/kernel/fpu/init.c
>> +++ b/arch/x86/kernel/fpu/init.c
>> @@ -50,29 +50,33 @@ void fpu__init_cpu(void)
>>
>>  /*
>>   * The earliest FPU detection code.
>> - *
>> - * Set the X86_FEATURE_FPU CPU-capability bit based on
>> - * trying to execute an actual sequence of FPU instructions:
>>   */
>>  static void fpu__init_system_early_generic(struct cpuinfo_x86 *c)
>>  {
>> -     unsigned long cr0;
>> -     u16 fsw, fcw;
>> +     if (!boot_cpu_has(X86_FEATURE_CPUID) &&
>> +         !test_bit(X86_FEATURE_FPU, (unsigned long *)cpu_caps_cleared)) {
>
> Flip test and save an indentation level.

How?  There's that bit at the bottom to worry about.

>
>> +             /*
>> +              * Set the X86_FEATURE_FPU CPU-capability bit based on
>> +              * trying to execute an actual sequence of FPU instructions:
>> +              */
>>
>> -     fsw = fcw = 0xffff;
>> +             unsigned long cr0;
>> +             u16 fsw, fcw;
>>
>> -     cr0 = read_cr0();
>> -     cr0 &= ~(X86_CR0_TS | X86_CR0_EM);
>> -     write_cr0(cr0);
>> +             fsw = fcw = 0xffff;
>> +
>> +             cr0 = read_cr0();
>> +             cr0 &= ~(X86_CR0_TS | X86_CR0_EM);
>> +             write_cr0(cr0);
>>
>> -     if (!test_bit(X86_FEATURE_FPU, (unsigned long *)cpu_caps_cleared)) {
>>               asm volatile("fninit ; fnstsw %0 ; fnstcw %1"
>>                            : "+m" (fsw), "+m" (fcw));
>> +             pr_info("x86/fpu: FSW=0x%04hx FCW=0x%04hx\n", fsw, fcw);
>
> Why do we want those in dmesg? Maybe KERN_DEBUG?

For debugging, since this code appears busted in every version of
Linux I looked at.  It's certainly been busted for quite a few years.
And this line won't print on any CPU with CPUID, so it's not going to
cause widespread spam.  I kind of line the idea of being able to ask
users of these ancient CPUs to just send in their logs.

--Andy