Re: Got FPU related warning on Intel Quark during boot

[Andy Lutomirski]

On Thu, Mar 10, 2016 at 6:59 AM, Borislav Petkov <bp@xxxxxxxxx> wrote:
> On Thu, Mar 10, 2016 at 03:31:43PM +0200, Andy Shevchenko wrote:
>> Looks like it lacks that one.
>>
>> # grep -i fxsr /proc/cpuinfo; echo $?
>> 1
>
> Ok, so looking at where the warning comes from:
>
> [   14.714533] WARNING: CPU: 0 PID: 823 at arch/x86/include/asm/fpu/internal.h:163 fpu__clear+0x8c/0x160
>
> static inline void copy_kernel_to_fxregs(struct fxregs_state *fx)
> {
>         int err;
>
>         if (config_enabled(CONFIG_X86_32)) {
>                 err = check_insn(fxrstor %[fx], "=m" (*fx), [fx] "m" (*fx));
>                       ^^^^^^^^^^^^^^^^^
>         } else {
>
>         ...
>
>         /* Copying from a kernel buffer to FPU registers should never fail: */
>         WARN_ON_FPU(err);
>
>
> and the stacktrace is pretty clear:
>
> flush_thread
> |-> fpu__clear(&tsk->thread.fpu);
>     |-> we are eager by default here:
>
>         if (!use_eager_fpu() || !static_cpu_has(X86_FEATURE_FPU)) {
>                 /* FPU state will be reallocated lazily at the first use. */
>                 fpu__drop(fpu);
>         } else {
>
>                 --> we're in that branch.
>
>                 copy_init_fpstate_to_fpregs();
>                 |-> copy_kernel_to_fxregs()
>
>
> I think we should use FRSTOR on quark, i.e., copy_kernel_to_fregs().
>
> Does this untested wild guess even work?
>
> ---
> diff --git a/arch/x86/kernel/fpu/core.c b/arch/x86/kernel/fpu/core.c
> index dea8e76d60c6..bbafe5e8a1a6 100644
> --- a/arch/x86/kernel/fpu/core.c
> +++ b/arch/x86/kernel/fpu/core.c
> @@ -474,8 +474,11 @@ static inline void copy_init_fpstate_to_fpregs(void)
>  {
>         if (use_xsave())
>                 copy_kernel_to_xregs(&init_fpstate.xsave, -1);
> -       else
> +       else if (static_cpu_has(X86_FEATURE_FXSR))
>                 copy_kernel_to_fxregs(&init_fpstate.fxsave);
> +       else
> +               copy_kernel_to_fregs(&init_fpstate.fsave);
> +
>  }
>

This looks wrong, too:

/*
 * Once per bootup FPU initialization sequences that will run on most x86 CPUs:
 */
static void __init fpu__init_system_generic(void)
{
    /*
     * Set up the legacy init FPU context. (xstate init might overwrite this
     * with a more modern format, if the CPU supports it.)
     */
    fpstate_init_fxstate(&init_fpstate.fxsave);  <-- wrong format on
pre-FXSR CPUs

    fpu__init_system_mxcsr();
}