Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1354998 > unrolled thread

Got FPU related warning on Intel Quark during boot

Started byAndy Shevchenko <andy.shevchenko@gmail.com>
First post2016-03-10 11:50 +0100
Last post2016-03-12 16:20 +0100
Articles 20 on this page of 31 — 8 participants

Back to article view | Back to linux.kernel

Contents

  Got FPU related warning on Intel Quark during boot Andy Shevchenko <andy.shevchenko@gmail.com> - 2016-03-10 11:50 +0100
    Re: Got FPU related warning on Intel Quark during boot Ingo Molnar <mingo@kernel.org> - 2016-03-10 12:20 +0100
      Re: Got FPU related warning on Intel Quark during boot Andy Shevchenko <andy.shevchenko@gmail.com> - 2016-03-10 13:50 +0100
        Re: Got FPU related warning on Intel Quark during boot Borislav Petkov <bp@alien8.de> - 2016-03-10 14:00 +0100
          Re: Got FPU related warning on Intel Quark during boot Andy Shevchenko <andy.shevchenko@gmail.com> - 2016-03-10 14:40 +0100
            Re: Got FPU related warning on Intel Quark during boot Borislav Petkov <bp@alien8.de> - 2016-03-10 16:10 +0100
              Re: Got FPU related warning on Intel Quark during boot Andy Shevchenko <andy.shevchenko@gmail.com> - 2016-03-10 16:30 +0100
                Re: Got FPU related warning on Intel Quark during boot Bryan O'Donoghue <pure.logic@nexus-software.ie> - 2016-03-10 16:50 +0100
                  Re: Got FPU related warning on Intel Quark during boot Borislav Petkov <bp@alien8.de> - 2016-03-10 17:50 +0100
                    Re: Got FPU related warning on Intel Quark during boot Bryan O'Donoghue <pure.logic@nexus-software.ie> - 2016-03-10 18:20 +0100
                      Re: Got FPU related warning on Intel Quark during boot Borislav Petkov <bp@alien8.de> - 2016-03-10 20:10 +0100
                  Re: Got FPU related warning on Intel Quark during boot Andy Lutomirski <luto@amacapital.net> - 2016-03-11 02:40 +0100
                    Re: Got FPU related warning on Intel Quark during boot Bryan O'Donoghue <pure.logic@nexus-software.ie> - 2016-03-11 12:00 +0100
              Re: Got FPU related warning on Intel Quark during boot Andy Lutomirski <luto@amacapital.net> - 2016-03-11 02:50 +0100
                Re: Got FPU related warning on Intel Quark during boot Ingo Molnar <mingo@kernel.org> - 2016-03-11 10:10 +0100
                  Re: Got FPU related warning on Intel Quark during boot Borislav Petkov <bp@alien8.de> - 2016-03-11 10:50 +0100
                    Re: Got FPU related warning on Intel Quark during boot Bryan O'Donoghue <pure.logic@nexus-software.ie> - 2016-03-11 12:10 +0100
                      Re: Got FPU related warning on Intel Quark during boot Borislav Petkov <bp@alien8.de> - 2016-03-11 12:30 +0100
                        [PATCH] x86/FPU: Fix FPU handling on legacy FPU machines Borislav Petkov <bp@alien8.de> - 2016-03-11 12:40 +0100
                          Re: [PATCH] x86/FPU: Fix FPU handling on legacy FPU machines Linus Torvalds <torvalds@linux-foundation.org> - 2016-03-11 19:40 +0100
                            Re: [PATCH] x86/FPU: Fix FPU handling on legacy FPU machines Dave Hansen <dave.hansen@linux.intel.com> - 2016-03-11 23:10 +0100
                              Re: [PATCH] x86/FPU: Fix FPU handling on legacy FPU machines Borislav Petkov <bp@alien8.de> - 2016-03-11 23:30 +0100
                                Re: [PATCH] x86/FPU: Fix FPU handling on legacy FPU machines Andy Lutomirski <luto@amacapital.net> - 2016-03-12 18:30 +0100
                                  Re: [PATCH] x86/FPU: Fix FPU handling on legacy FPU machines Borislav Petkov <bp@alien8.de> - 2016-03-12 18:50 +0100
                            Re: [PATCH] x86/FPU: Fix FPU handling on legacy FPU machines Borislav Petkov <bp@alien8.de> - 2016-03-11 23:10 +0100
                              Re: [PATCH] x86/FPU: Fix FPU handling on legacy FPU machines Bryan O'Donoghue <pure.logic@nexus-software.ie> - 2016-03-12 13:10 +0100
                                Re: [PATCH] x86/FPU: Fix FPU handling on legacy FPU machines Borislav Petkov <bp@alien8.de> - 2016-03-12 13:30 +0100
                              Re: [PATCH] x86/FPU: Fix FPU handling on legacy FPU machines Ingo Molnar <mingo@kernel.org> - 2016-03-12 16:20 +0100
                            Re: [PATCH] x86/FPU: Fix FPU handling on legacy FPU machines Ingo Molnar <mingo@kernel.org> - 2016-03-12 16:10 +0100
                              Re: [PATCH] x86/FPU: Fix FPU handling on legacy FPU machines Ingo Molnar <mingo@kernel.org> - 2016-03-12 16:20 +0100
                          [tip:x86/urgent] x86/fpu: Fix eager-FPU handling on legacy FPU  machines tip-bot for Borislav Petkov <tipbot@zytor.com> - 2016-03-12 16:20 +0100

Page 1 of 2  [1] 2  Next page →

#1354998 — Got FPU related warning on Intel Quark during boot

FromAndy Shevchenko <andy.shevchenko@gmail.com>
Date2016-03-10 11:50 +0100
SubjectGot FPU related warning on Intel Quark during boot
Message-ID<rb6kh-5OL-9@gated-at.bofh.it>
Today tried first time after long break to boot Intel Quark SoC with
most recent linux-next. Got the following warning:

[   14.714533] WARNING: CPU: 0 PID: 823 at
arch/x86/include/asm/fpu/internal.h:163 fpu__clear+0x8c/0x160
[   14.726603] Modules linked in:
[   14.729910] CPU: 0 PID: 823 Comm: kworker/u2:0 Not tainted
4.5.0-rc7-next-20160310+ #137
[   14.738307]  00000000 00000000 ce691e20 c12b6fc9 ce691e50 c1049fd1
c1978c6c 00000000
[   14.747000]  00000337 c196b530 000000a3 c102050c 000000a3 ce587ac0
00000000 ce653000
[   14.755722]  ce691e64 c104a095 00000009 00000000 00000000 ce691e74
c102050c ce587500
[   14.764468] Call Trace:
[   14.767172]  [<c12b6fc9>] dump_stack+0x16/0x1d
[   14.771889]  [<c1049fd1>] __warn+0xd1/0xf0
[   14.776253]  [<c102050c>] ? fpu__clear+0x8c/0x160
[   14.781234]  [<c104a095>] warn_slowpath_null+0x25/0x30
[   14.786648]  [<c102050c>] fpu__clear+0x8c/0x160
[   14.791447]  [<c101f347>] flush_thread+0x57/0x60
[   14.796341]  [<c113a5cc>] flush_old_exec+0x4cc/0x600
[   14.801594]  [<c117ab20>] load_elf_binary+0x2b0/0x1060
[   14.807010]  [<c1111220>] ? get_user_pages_remote+0x50/0x60
[   14.812898]  [<c12c4687>] ? _copy_from_user+0x37/0x40
[   14.818236]  [<c1139f82>] search_binary_handler+0x62/0x150
[   14.824007]  [<c113b19c>] do_execveat_common+0x45c/0x600
[   14.829647]  [<c113b35f>] do_execve+0x1f/0x30
[   14.834289]  [<c1059941>] call_usermodehelper_exec_async+0x91/0xe0
[   14.840765]  [<c17f2310>] ret_from_kernel_thread+0x20/0x40
[   14.846540]  [<c10598b0>] ? umh_complete+0x40/0x40
[   14.851626] ---[ end trace 137ff5893f9b85bf ]---

Is it know issue? Or what could I try to fix it?

Reproducibility:  3 of 3.

-- 
With Best Regards,
Andy Shevchenko

[toc] | [next] | [standalone]

#1355033

FromIngo Molnar <mingo@kernel.org>
Date2016-03-10 12:20 +0100
Message-ID<rb6Nk-6gw-5@gated-at.bofh.it>
In reply to#1354998
I've Cc:-ed more FPU developers. Mail quoted below. I don't have a Quark system to 
test this on, but maybe others have an idea why this warning triggers?

My thinking is that it's related to:

  58122bf1d856 x86/fpu: Default eagerfpu=on on all CPUs

Thanks,

	Ingo

* Andy Shevchenko <andy.shevchenko@gmail.com> wrote:

> Today tried first time after long break to boot Intel Quark SoC with
> most recent linux-next. Got the following warning:
> 
> [   14.714533] WARNING: CPU: 0 PID: 823 at
> arch/x86/include/asm/fpu/internal.h:163 fpu__clear+0x8c/0x160
> [   14.726603] Modules linked in:
> [   14.729910] CPU: 0 PID: 823 Comm: kworker/u2:0 Not tainted
> 4.5.0-rc7-next-20160310+ #137
> [   14.738307]  00000000 00000000 ce691e20 c12b6fc9 ce691e50 c1049fd1
> c1978c6c 00000000
> [   14.747000]  00000337 c196b530 000000a3 c102050c 000000a3 ce587ac0
> 00000000 ce653000
> [   14.755722]  ce691e64 c104a095 00000009 00000000 00000000 ce691e74
> c102050c ce587500
> [   14.764468] Call Trace:
> [   14.767172]  [<c12b6fc9>] dump_stack+0x16/0x1d
> [   14.771889]  [<c1049fd1>] __warn+0xd1/0xf0
> [   14.776253]  [<c102050c>] ? fpu__clear+0x8c/0x160
> [   14.781234]  [<c104a095>] warn_slowpath_null+0x25/0x30
> [   14.786648]  [<c102050c>] fpu__clear+0x8c/0x160
> [   14.791447]  [<c101f347>] flush_thread+0x57/0x60
> [   14.796341]  [<c113a5cc>] flush_old_exec+0x4cc/0x600
> [   14.801594]  [<c117ab20>] load_elf_binary+0x2b0/0x1060
> [   14.807010]  [<c1111220>] ? get_user_pages_remote+0x50/0x60
> [   14.812898]  [<c12c4687>] ? _copy_from_user+0x37/0x40
> [   14.818236]  [<c1139f82>] search_binary_handler+0x62/0x150
> [   14.824007]  [<c113b19c>] do_execveat_common+0x45c/0x600
> [   14.829647]  [<c113b35f>] do_execve+0x1f/0x30
> [   14.834289]  [<c1059941>] call_usermodehelper_exec_async+0x91/0xe0
> [   14.840765]  [<c17f2310>] ret_from_kernel_thread+0x20/0x40
> [   14.846540]  [<c10598b0>] ? umh_complete+0x40/0x40
> [   14.851626] ---[ end trace 137ff5893f9b85bf ]---
> 
> Is it know issue? Or what could I try to fix it?
> 
> Reproducibility:  3 of 3.
> 
> -- 
> With Best Regards,
> Andy Shevchenko

[toc] | [prev] | [next] | [standalone]

#1355093

FromAndy Shevchenko <andy.shevchenko@gmail.com>
Date2016-03-10 13:50 +0100
Message-ID<rb8cq-74T-17@gated-at.bofh.it>
In reply to#1355033
On Thu, Mar 10, 2016 at 1:19 PM, Ingo Molnar <mingo@kernel.org> wrote:
>
> I've Cc:-ed more FPU developers. Mail quoted below. I don't have a Quark system to
> test this on, but maybe others have an idea why this warning triggers?
>
> My thinking is that it's related to:
>
>   58122bf1d856 x86/fpu: Default eagerfpu=on on all CPUs

He-he, my cursor stays on
        if (!use_eager_fpu() || !static_cpu_has(X86_FEATURE_FPU)) {
while I was having a lunch. So far got from datasheet that some kind
of FPU is present there.

eagerfpu=auto doesn't fix
eagerfpu=off fixes the issue

>> Today tried first time after long break to boot Intel Quark SoC with
>> most recent linux-next. Got the following warning:
>>
>> [   14.714533] WARNING: CPU: 0 PID: 823 at
>> arch/x86/include/asm/fpu/internal.h:163 fpu__clear+0x8c/0x160
>> [   14.726603] Modules linked in:
>> [   14.729910] CPU: 0 PID: 823 Comm: kworker/u2:0 Not tainted
>> 4.5.0-rc7-next-20160310+ #137
>> [   14.738307]  00000000 00000000 ce691e20 c12b6fc9 ce691e50 c1049fd1
>> c1978c6c 00000000
>> [   14.747000]  00000337 c196b530 000000a3 c102050c 000000a3 ce587ac0
>> 00000000 ce653000
>> [   14.755722]  ce691e64 c104a095 00000009 00000000 00000000 ce691e74
>> c102050c ce587500
>> [   14.764468] Call Trace:
>> [   14.767172]  [<c12b6fc9>] dump_stack+0x16/0x1d
>> [   14.771889]  [<c1049fd1>] __warn+0xd1/0xf0
>> [   14.776253]  [<c102050c>] ? fpu__clear+0x8c/0x160
>> [   14.781234]  [<c104a095>] warn_slowpath_null+0x25/0x30
>> [   14.786648]  [<c102050c>] fpu__clear+0x8c/0x160
>> [   14.791447]  [<c101f347>] flush_thread+0x57/0x60
>> [   14.796341]  [<c113a5cc>] flush_old_exec+0x4cc/0x600
>> [   14.801594]  [<c117ab20>] load_elf_binary+0x2b0/0x1060
>> [   14.807010]  [<c1111220>] ? get_user_pages_remote+0x50/0x60
>> [   14.812898]  [<c12c4687>] ? _copy_from_user+0x37/0x40
>> [   14.818236]  [<c1139f82>] search_binary_handler+0x62/0x150
>> [   14.824007]  [<c113b19c>] do_execveat_common+0x45c/0x600
>> [   14.829647]  [<c113b35f>] do_execve+0x1f/0x30
>> [   14.834289]  [<c1059941>] call_usermodehelper_exec_async+0x91/0xe0
>> [   14.840765]  [<c17f2310>] ret_from_kernel_thread+0x20/0x40
>> [   14.846540]  [<c10598b0>] ? umh_complete+0x40/0x40
>> [   14.851626] ---[ end trace 137ff5893f9b85bf ]---
>>
>> Is it know issue? Or what could I try to fix it?
>>
>> Reproducibility:  3 of 3.

-- 
With Best Regards,
Andy Shevchenko

[toc] | [prev] | [next] | [standalone]

#1355095

FromBorislav Petkov <bp@alien8.de>
Date2016-03-10 14:00 +0100
Message-ID<rb8m6-78l-5@gated-at.bofh.it>
In reply to#1355093
On Thu, Mar 10, 2016 at 02:48:09PM +0200, Andy Shevchenko wrote:
> He-he, my cursor stays on
>         if (!use_eager_fpu() || !static_cpu_has(X86_FEATURE_FPU)) {
> while I was having a lunch. So far got from datasheet that some kind
> of FPU is present there.
> 
> eagerfpu=auto doesn't fix
> eagerfpu=off fixes the issue

Looking at the stacktrace, does that quark thing have FXRSTOR?

$ grep -i fxsr /proc/cpuinfo

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.

[toc] | [prev] | [next] | [standalone]

#1355125

FromAndy Shevchenko <andy.shevchenko@gmail.com>
Date2016-03-10 14:40 +0100
Message-ID<rb8YO-7CH-19@gated-at.bofh.it>
In reply to#1355095
On Thu, Mar 10, 2016 at 2:56 PM, Borislav Petkov <bp@alien8.de> wrote:
> On Thu, Mar 10, 2016 at 02:48:09PM +0200, Andy Shevchenko wrote:
>> He-he, my cursor stays on
>>         if (!use_eager_fpu() || !static_cpu_has(X86_FEATURE_FPU)) {
>> while I was having a lunch. So far got from datasheet that some kind
>> of FPU is present there.
>>
>> eagerfpu=auto doesn't fix
>> eagerfpu=off fixes the issue
>
> Looking at the stacktrace, does that quark thing have FXRSTOR?
>
> $ grep -i fxsr /proc/cpuinfo

Looks like it lacks that one.

# grep -i fxsr /proc/cpuinfo; echo $?
1

-- 
With Best Regards,
Andy Shevchenko

[toc] | [prev] | [next] | [standalone]

#1355182

FromBorislav Petkov <bp@alien8.de>
Date2016-03-10 16:10 +0100
Message-ID<rbanU-es-11@gated-at.bofh.it>
In reply to#1355125
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);
+
 }
 
 /*

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.

[toc] | [prev] | [next] | [standalone]

#1355202

FromAndy Shevchenko <andy.shevchenko@gmail.com>
Date2016-03-10 16:30 +0100
Message-ID<rbaHg-q8-19@gated-at.bofh.it>
In reply to#1355182
On Thu, Mar 10, 2016 at 4:59 PM, Borislav Petkov <bp@alien8.de> 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);
> +

Obviously redundant line, otherwise it indeed works

Tested-by: Andy Shevchenko <andy.shevchenko@gmail.com>

>  }
>
>  /*



-- 
With Best Regards,
Andy Shevchenko

[toc] | [prev] | [next] | [standalone]

#1355223

FromBryan O'Donoghue <pure.logic@nexus-software.ie>
Date2016-03-10 16:50 +0100
Message-ID<rbb0B-AW-5@gated-at.bofh.it>
In reply to#1355202

[Multipart message — attachments visible in raw view] — view raw

On Thu, 2016-03-10 at 17:22 +0200, Andy Shevchenko wrote:
> On Thu, Mar 10, 2016 at 4:59 PM, Borislav Petkov <bp@alien8.de>
> 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);
> > +
> 
> Obviously redundant line, otherwise it indeed works
> 
> Tested-by: Andy Shevchenko <andy.shevchenko@gmail.com>
> 
> >  }
> > 
> >  /*
> 
> 
> 

It works but user-space FPU is broken; something's wrong with the
initial state of the FPU regs - it looks as though they aren't being
properly initialized and FPU context in the signal handler is wrong
too.

Linux 3.8.7:
/root@galileo:~# ./fpu
f is 10.000000 g is 10.100000
Double value is 0.000000
Double value is 0.100000
Double value is 0.200000
^Chandler value of variable is 0.300000
Double value is 0.300000
Double value is 0.400000

Linux-next + Boris' fix:
root@galileo:~# ./fpu
f is -nan g is -nan
Double value is 0.000000
Double value is 0.100000
Double value is 0.200000^C
handler value of variable is -nan
Double value is 0.300000
Double value is 0.400000^Z[1]+  Stopped

root@galileo:~# uname -aLinux galileo 4.5.0-rc7-next-20160310+ #185 Thu
Mar 10 15:11:10 GMT 2016 i586 GNU/Linux

[toc] | [prev] | [next] | [standalone]

#1355264

FromBorislav Petkov <bp@alien8.de>
Date2016-03-10 17:50 +0100
Message-ID<rbbWH-1kz-27@gated-at.bofh.it>
In reply to#1355223
On Thu, Mar 10, 2016 at 03:45:21PM +0000, Bryan O'Donoghue wrote:
> It works but user-space FPU is broken; something's wrong with the
> initial state of the FPU regs - it looks as though they aren't being
> properly initialized and FPU context in the signal handler is wrong
> too.

What does your test prog say when you boot linux-next with
"eagerfpu=off" and *without* my fix?

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.

[toc] | [prev] | [next] | [standalone]

#1355270

FromBryan O'Donoghue <pure.logic@nexus-software.ie>
Date2016-03-10 18:20 +0100
Message-ID<rbcpI-1KE-7@gated-at.bofh.it>
In reply to#1355264
On Thu, 2016-03-10 at 17:49 +0100, Borislav Petkov wrote:
> On Thu, Mar 10, 2016 at 03:45:21PM +0000, Bryan O'Donoghue wrote:
> > It works but user-space FPU is broken; something's wrong with the
> > initial state of the FPU regs - it looks as though they aren't
> > being
> > properly initialized and FPU context in the signal handler is wrong
> > too.
> 
> What does your test prog say when you boot linux-next with
> "eagerfpu=off" and *without* my fix?
> 

root@galileo:~# ./fpu 
f is 10.000000 g is 10.100000
Double value is 0.000000
Double value is 0.100000
Double value is 0.200000
^Chandler value of variable is 0.300000
Double value is 0.300000
Double value is 0.400000
^Z
[1]+  Stopped                 ./fpu
root@galileo:~#

[toc] | [prev] | [next] | [standalone]

#1355325

FromBorislav Petkov <bp@alien8.de>
Date2016-03-10 20:10 +0100
Message-ID<rbe8a-2SM-13@gated-at.bofh.it>
In reply to#1355270
On Thu, Mar 10, 2016 at 05:15:15PM +0000, Bryan O'Donoghue wrote:
> root@galileo:~# ./fpu 
> f is 10.000000 g is 10.100000

Hmm, ok, I can *actually* reproduce it in kvm+qemu with 486 CPU type
(which should be close to quark AFAIK).

Debugging continues...

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.

[toc] | [prev] | [next] | [standalone]

#1355525

FromAndy Lutomirski <luto@amacapital.net>
Date2016-03-11 02:40 +0100
Message-ID<rbkdA-74F-7@gated-at.bofh.it>
In reply to#1355223
On Thu, Mar 10, 2016 at 7:45 AM, Bryan O'Donoghue
<pure.logic@nexus-software.ie> wrote:
> On Thu, 2016-03-10 at 17:22 +0200, Andy Shevchenko wrote:
>> On Thu, Mar 10, 2016 at 4:59 PM, Borislav Petkov <bp@alien8.de>
>> 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);
>> > +
>>
>> Obviously redundant line, otherwise it indeed works
>>
>> Tested-by: Andy Shevchenko <andy.shevchenko@gmail.com>
>>
>> >  }
>> >
>> >  /*
>>
>>
>>
>
> It works but user-space FPU is broken; something's wrong with the
> initial state of the FPU regs - it looks as though they aren't being
> properly initialized and FPU context in the signal handler is wrong
> too.
>
> Linux 3.8.7:
> /root@galileo:~# ./fpu
> f is 10.000000 g is 10.100000
> Double value is 0.000000
> Double value is 0.100000
> Double value is 0.200000
> ^Chandler value of variable is 0.300000
> Double value is 0.300000
> Double value is 0.400000
>
> Linux-next + Boris' fix:
> root@galileo:~# ./fpu
> f is -nan g is -nan
> Double value is 0.000000
> Double value is 0.100000
> Double value is 0.200000^C
> handler value of variable is -nan
> Double value is 0.300000
> Double value is 0.400000^Z[1]+  Stopped
>

Just to check: are you running the exact same compiled binary on both
kernels?  Because your test case invokes undefined behavior, and I'm a
bit surprised you get anything sensible from it.  That being said, the
f = -nan part is worrisome.

--Andy

[toc] | [prev] | [next] | [standalone]

#1355819

FromBryan O'Donoghue <pure.logic@nexus-software.ie>
Date2016-03-11 12:00 +0100
Message-ID<rbsXx-4QJ-37@gated-at.bofh.it>
In reply to#1355525
On Thu, 2016-03-10 at 17:31 -0800, Andy Lutomirski wrote:
> On Thu, Mar 10, 2016 at 7:45 AM, Bryan O'Donoghue
> <pure.logic@nexus-software.ie> wrote:
> > On Thu, 2016-03-10 at 17:22 +0200, Andy Shevchenko wrote:
> > > On Thu, Mar 10, 2016 at 4:59 PM, Borislav Petkov <bp@alien8.de>
> > > 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);
> > > > +
> > > 
> > > Obviously redundant line, otherwise it indeed works
> > > 
> > > Tested-by: Andy Shevchenko <andy.shevchenko@gmail.com>
> > > 
> > > >  }
> > > > 
> > > >  /*
> > > 
> > > 
> > > 
> > 
> > It works but user-space FPU is broken; something's wrong with the
> > initial state of the FPU regs - it looks as though they aren't
> > being
> > properly initialized and FPU context in the signal handler is wrong
> > too.
> > 
> > Linux 3.8.7:
> > /root@galileo:~# ./fpu
> > f is 10.000000 g is 10.100000
> > Double value is 0.000000
> > Double value is 0.100000
> > Double value is 0.200000
> > ^Chandler value of variable is 0.300000
> > Double value is 0.300000
> > Double value is 0.400000
> > 
> > Linux-next + Boris' fix:
> > root@galileo:~# ./fpu
> > f is -nan g is -nan
> > Double value is 0.000000
> > Double value is 0.100000
> > Double value is 0.200000^C
> > handler value of variable is -nan
> > Double value is 0.300000
> > Double value is 0.400000^Z[1]+  Stopped
> > 
> 
> Just to check: are you running the exact same compiled binary on both
> kernels?  Because your test case invokes undefined behavior, and I'm
> a
> bit surprised you get anything sensible from it.  That being said,
> the
> f = -nan part is worrisome.
> 
> --Andy

It's the same binary yes.

[toc] | [prev] | [next] | [standalone]

#1355528

FromAndy Lutomirski <luto@amacapital.net>
Date2016-03-11 02:50 +0100
Message-ID<rbknf-79u-7@gated-at.bofh.it>
In reply to#1355182
On Thu, Mar 10, 2016 at 6:59 AM, Borislav Petkov <bp@alien8.de> 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();
}

[toc] | [prev] | [next] | [standalone]

#1355742

FromIngo Molnar <mingo@kernel.org>
Date2016-03-11 10:10 +0100
Message-ID<rbrf4-3Sw-19@gated-at.bofh.it>
In reply to#1355528
* Andy Lutomirski <luto@amacapital.net> wrote:

> On Thu, Mar 10, 2016 at 6:59 AM, Borislav Petkov <bp@alien8.de> 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

Indeed:

static inline void fpstate_init_fxstate(struct fxregs_state *fx)
{
        fx->cwd = 0x37f;
        fx->mxcsr = MXCSR_DEFAULT;
}

I assumed that the fxstate init is outside the legacy FPU context area, but they 
overlap: fx->cwd is the first two bytes, and fx->mxcsr overlaps the middle of the 
legacy area.

We do a later fpstate_init_fstate(), which does:

static inline void fpstate_init_fstate(struct fregs_state *fp)
{
        fp->cwd = 0xffff037fu;
        fp->swd = 0xffff0000u;
        fp->twd = 0xffffffffu;
        fp->fos = 0xffff0000u;
}

which accidentally overwrites the cwd bit - but AFAICS fx->mxcsw overlaps the 
first legacy FPU register?

So yes, this needs to be fixed too.

Thanks,

	Ingo

[toc] | [prev] | [next] | [standalone]

#1355770

FromBorislav Petkov <bp@alien8.de>
Date2016-03-11 10:50 +0100
Message-ID<rbrRN-489-17@gated-at.bofh.it>
In reply to#1355742
On Fri, Mar 11, 2016 at 10:08:40AM +0100, Ingo Molnar wrote:
> So yes, this needs to be fixed too.

Yes indeed. So the diff below seems to work with Bryan's simple test
case.

Bryan, can you confirm on your box pls?

---
diff --git a/arch/x86/kernel/fpu/core.c b/arch/x86/kernel/fpu/core.c
index dea8e76d60c6..8e37cc8a539a 100644
--- a/arch/x86/kernel/fpu/core.c
+++ b/arch/x86/kernel/fpu/core.c
@@ -474,8 +474,10 @@ 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);
 }
 
 /*
diff --git a/arch/x86/kernel/fpu/init.c b/arch/x86/kernel/fpu/init.c
index e12cc0ad368e..c835f61d5feb 100644
--- a/arch/x86/kernel/fpu/init.c
+++ b/arch/x86/kernel/fpu/init.c
@@ -134,7 +134,7 @@ 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);
+	fpstate_init(&init_fpstate);
 
 	fpu__init_system_mxcsr();
 }

---

Thanks.

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.

[toc] | [prev] | [next] | [standalone]

#1355826

FromBryan O'Donoghue <pure.logic@nexus-software.ie>
Date2016-03-11 12:10 +0100
Message-ID<rbt7d-5b2-47@gated-at.bofh.it>
In reply to#1355770
On Fri, 2016-03-11 at 10:48 +0100, Borislav Petkov wrote:
> On Fri, Mar 11, 2016 at 10:08:40AM +0100, Ingo Molnar wrote:
> > So yes, this needs to be fixed too.
> 
> Yes indeed. So the diff below seems to work with Bryan's simple test
> case.
> 
> Bryan, can you confirm on your box pls?
> 
> ---
> diff --git a/arch/x86/kernel/fpu/core.c b/arch/x86/kernel/fpu/core.c
> index dea8e76d60c6..8e37cc8a539a 100644
> --- a/arch/x86/kernel/fpu/core.c
> +++ b/arch/x86/kernel/fpu/core.c
> @@ -474,8 +474,10 @@ 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);
>  }
>  
>  /*
> diff --git a/arch/x86/kernel/fpu/init.c b/arch/x86/kernel/fpu/init.c
> index e12cc0ad368e..c835f61d5feb 100644
> --- a/arch/x86/kernel/fpu/init.c
> +++ b/arch/x86/kernel/fpu/init.c
> @@ -134,7 +134,7 @@ 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);
> +	fpstate_init(&init_fpstate);
>  
>  	fpu__init_system_mxcsr();
>  }
> 
> ---
> 
> Thanks.
> 

Hi Boris,

Looks good.

Tested-by: Bryan O'Donoghue <pure.logic@nexus-software.ie>

[toc] | [prev] | [next] | [standalone]

#1355839

FromBorislav Petkov <bp@alien8.de>
Date2016-03-11 12:30 +0100
Message-ID<rbtqx-5pl-1@gated-at.bofh.it>
In reply to#1355826
On Fri, Mar 11, 2016 at 11:02:04AM +0000, Bryan O'Donoghue wrote:
> Looks good.
> 
> Tested-by: Bryan O'Donoghue <pure.logic@nexus-software.ie>

Thanks Bryan!

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.

[toc] | [prev] | [next] | [standalone]

#1355848 — [PATCH] x86/FPU: Fix FPU handling on legacy FPU machines

FromBorislav Petkov <bp@alien8.de>
Date2016-03-11 12:40 +0100
Subject[PATCH] x86/FPU: Fix FPU handling on legacy FPU machines
Message-ID<rbtAe-5vz-15@gated-at.bofh.it>
In reply to#1355839
486 cores like Intel Quark support only the very old, legacy x87 FPU
(FSAVE/FRSTOR, CPUID bit FXSR is not set). And our FPU code wasn't
handling the saving and restoring there properly. First, Andy Shevchenko
reported a splat:

  WARNING: CPU: 0 PID: 823 at arch/x86/include/asm/fpu/internal.h:163 fpu__clear+0x8c/0x160

which was us trying to execute FXRSTOR on those machines even though
they don't support it.

After taking care of that, Bryan O'Donoghue reported that a simple FPU
test still failed because we weren't initializing the FPU state properly
on those machines.

Take care of all that.

Reported-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Reported-and-tested-by: Bryan O'Donoghue <pure.logic@nexus-software.ie>
Signed-off-by: Borislav Petkov <bp@suse.de>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Brian Gerst <brgerst@gmail.com>
Cc: Dave Hansen <dave.hansen@linux.intel.com>
Cc: Denys Vlasenko <dvlasenk@redhat.com>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Oleg Nesterov <oleg@redhat.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
---
 arch/x86/kernel/fpu/core.c | 4 +++-
 arch/x86/kernel/fpu/init.c | 2 +-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kernel/fpu/core.c b/arch/x86/kernel/fpu/core.c
index dea8e76d60c6..8e37cc8a539a 100644
--- a/arch/x86/kernel/fpu/core.c
+++ b/arch/x86/kernel/fpu/core.c
@@ -474,8 +474,10 @@ 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);
 }
 
 /*
diff --git a/arch/x86/kernel/fpu/init.c b/arch/x86/kernel/fpu/init.c
index e12cc0ad368e..c835f61d5feb 100644
--- a/arch/x86/kernel/fpu/init.c
+++ b/arch/x86/kernel/fpu/init.c
@@ -134,7 +134,7 @@ 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);
+	fpstate_init(&init_fpstate);
 
 	fpu__init_system_mxcsr();
 }
-- 
2.3.5


-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.

[toc] | [prev] | [next] | [standalone]

#1356126 — Re: [PATCH] x86/FPU: Fix FPU handling on legacy FPU machines

FromLinus Torvalds <torvalds@linux-foundation.org>
Date2016-03-11 19:40 +0100
SubjectRe: [PATCH] x86/FPU: Fix FPU handling on legacy FPU machines
Message-ID<rbA8G-1R9-25@gated-at.bofh.it>
In reply to#1355848
On Fri, Mar 11, 2016 at 3:32 AM, Borislav Petkov <bp@alien8.de> wrote:
> 486 cores like Intel Quark support only the very old, legacy x87 FPU
> (FSAVE/FRSTOR, CPUID bit FXSR is not set). And our FPU code wasn't
> handling the saving and restoring there properly. First, Andy Shevchenko
> reported a splat:
>
>   WARNING: CPU: 0 PID: 823 at arch/x86/include/asm/fpu/internal.h:163 fpu__clear+0x8c/0x160
>
> which was us trying to execute FXRSTOR on those machines even though
> they don't support it.
>
> After taking care of that, Bryan O'Donoghue reported that a simple FPU
> test still failed because we weren't initializing the FPU state properly
> on those machines.

Obvious Ack to the patch, along with a "how did this ever work
before?" comment..

           Linus

[toc] | [prev] | [next] | [standalone]

Page 1 of 2  [1] 2  Next page →

Back to top | Article view | linux.kernel

csiph-web