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

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 2 of 2 — ← Prev page 1 [2]

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

On 03/11/2016 02:03 PM, Borislav Petkov wrote:
> I'm still wondering, though, why didn't the Quark people scream
> earlier... And who knows, it was probably b0rked even before the
> FPU rewrite.

I've actually got 4.0 running on my Quark board.  The FPU rewrite
dropped in just after that iirc.

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

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

On Fri, Mar 11, 2016 at 02:07:19PM -0800, Dave Hansen wrote:
> I've actually got 4.0 running on my Quark board.  The FPU rewrite
> dropped in just after that iirc.

4.2 or so... Ok, so it looks like we broke it then.

-- 
Regards/Gruss,
    Boris.

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

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

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

On Mar 11, 2016 2:20 PM, "Borislav Petkov" <bp@alien8.de> wrote:
>
> On Fri, Mar 11, 2016 at 02:07:19PM -0800, Dave Hansen wrote:
> > I've actually got 4.0 running on my Quark board.  The FPU rewrite
> > dropped in just after that iirc.
>
> 4.2 or so... Ok, so it looks like we broke it then.
>

For reference, what are the QEMU options and boot options you used to
trigger this?  I'm asking because I tested eagerfpu=on without fxsr a
few weeks ago in QEMU, and I didn't trigger it.  Maybe I needed to
force KVM off or something.  Off the top of my head, I'm guessing that
when I wrote "x86/fpu: Fix FNSAVE usage in eagerfpu mode", I was
inadvertently using a bastardize combination of FNSAVE and
FXRSTOR-of-init-state, KVM let the FXRSTOR through despite not
advertising it in CPUID, and it papered over the init issue because
the wrong init state format was hidden by using the wrong instruction
to load it.

Sigh.  Yet more reason for Intel to add chicken bits to *turn off* new features.

--Andy

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

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

On Sat, Mar 12, 2016 at 09:21:16AM -0800, Andy Lutomirski wrote:
> For reference, what are the QEMU options and boot options you used to
> trigger this?  I'm asking because I tested eagerfpu=on without fxsr a
> few weeks ago in QEMU, and I didn't trigger it.  Maybe I needed to
> force KVM off or something.

$ qemu-system-i386
-enable-kvm
-gdb tcp::1234
-cpu 486
-m 2048
-hda /home/boris/kvm/debian/sid-i386.img
-boot menu=off,order=c
-localtime
-net nic,model=rtl8139,macaddr=12:35:12:34:56:78
-net user,hostfwd=tcp::1235-:22
-usbdevice tablet
-kernel /home/boris/kernel/linux-2.6/arch/x86/boot/bzImage
-append "root=/dev/sda1 resume=/dev/sdb1 debug ignore_loglevel log_buf_len=16M earlyprintk=ttyS0,115200 console=ttyS0,115200 console=tty0 "
-monitor pty
-soundhw hda
-serial file:/home/boris/kvm/test-i386-1235.log
-snapshot
-name "Debian i386:1235"
-smp 1
-virtfs local,path=/tmp,mount_tag=tmp,security_model=none


I basically tried to get it as close to 486er as possible. I had also
CONFIG_M486=y for the guest kernel.

> Off the top of my head, I'm guessing that when I wrote "x86/fpu: Fix
> FNSAVE usage in eagerfpu mode", I was inadvertently using a bastardize
> combination of FNSAVE and FXRSTOR-of-init-state, KVM let the FXRSTOR
> through despite not advertising it in CPUID, and it papered over the
> init issue because the wrong init state format was hidden by using the
> wrong instruction to load it.

Yeah, I think qemu looks at CPUID bits for some stuff but probably not
all. And I also think we should go and fix such issues there so that we
can have as accurate an emulation as possible.

> Sigh. Yet more reason for Intel to add chicken bits to *turn off* new
> features.

Amen to that.

-- 
Regards/Gruss,
    Boris.

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

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

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

On Fri, Mar 11, 2016 at 10:32:43AM -0800, Linus Torvalds wrote:
> Obvious Ack to the patch, along with a "how did this ever work
> before?" comment..

I had a sarcastic sentence in the commit message which I deleted later:

"Apparently no one had tried the kernel on a 486er after the FPU
rewrite. Backwards compatibility is overrated."

:-)

I'm still wondering, though, why didn't the Quark people scream
earlier... And who knows, it was probably b0rked even before the
FPU rewrite.

-- 
Regards/Gruss,
    Boris.

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

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

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

On Fri, 2016-03-11 at 23:03 +0100, Borislav Petkov wrote:
> On Fri, Mar 11, 2016 at 10:32:43AM -0800, Linus Torvalds wrote:
> > Obvious Ack to the patch, along with a "how did this ever work
> > before?" comment..
> 
> I had a sarcastic sentence in the commit message which I deleted
> later:
> 
> "Apparently no one had tried the kernel on a 486er after the FPU
> rewrite. Backwards compatibility is overrated."
> 
> :-)
> 
> I'm still wondering, though, why didn't the Quark people scream
> earlier... And who knows, it was probably b0rked even before the
> FPU rewrite.
> 

Busy with the dayjob :) so I haven't updated on Galileo since 

4b696dcb1a55e40648ad0eec4af991c72f945a85 (Feb 28 or so)

but... we'll do a better job keep track of -next to catch this stuff
earlier

---
bod

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

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

On Sat, Mar 12, 2016 at 12:04:49PM +0000, Bryan O'Donoghue wrote:
> Busy with the dayjob :) so I haven't updated on Galileo since 
> 
> 4b696dcb1a55e40648ad0eec4af991c72f945a85 (Feb 28 or so)

That's 4.5-rc5-ish and the FPU rewrite came during 4.2. BUT!,

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

which is in tip currently, made eagerfpu default and we know that
eagerfpu=off makes the issue go away.

So the bug was there, just it wasn't being hit on Quark. Until
58122bf1d856...

-- 
Regards/Gruss,
    Boris.

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

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

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

* Borislav Petkov <bp@alien8.de> wrote:

> On Fri, Mar 11, 2016 at 10:32:43AM -0800, Linus Torvalds wrote:
> > Obvious Ack to the patch, along with a "how did this ever work
> > before?" comment..
> 
> I had a sarcastic sentence in the commit message which I deleted later:
> 
> "Apparently no one had tried the kernel on a 486er after the FPU
> rewrite. Backwards compatibility is overrated."
> 
> :-)
> 
> I'm still wondering, though, why didn't the Quark people scream
> earlier... And who knows, it was probably b0rked even before the
> FPU rewrite.

So I actually tested legacy FPU support during (and after) the big FPU rewrite - I 
even tried FPU emu and we made that work (it was broken for like years).

This particular breakage was made prominent recently, with switching old FPU CPUs 
over to eagerfpu context switching:

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

If you check the fix, it only affects eagerfpu code paths. So to see the bug with 
Quark (or ancient CPUs) you'd have to have booted it with eagerfpu=on - and most 
people avoid twiddling boot parameters if they can avoid it.

Thanks,

	Ingo

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

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

* Linus Torvalds <torvalds@linux-foundation.org> wrote:

> 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..

So the window for 'real' breakage was relatively short: this is an older bug but 
only became a serious bug with the following upcoming commit:

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

... but it's nice to have it fixed nevertheless!

Thanks,

	Ingo

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

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

* Ingo Molnar <mingo@kernel.org> wrote:

> * Linus Torvalds <torvalds@linux-foundation.org> wrote:
> 
> > 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..
> 
> So the window for 'real' breakage was relatively short: this is an older bug but 
> only became a serious bug with the following upcoming commit:
> 
>   58122bf1d856 x86/fpu: Default eagerfpu=on on all CPUs

And the reason for that is:

void fpu__clear(struct fpu *fpu)
{
        WARN_ON_FPU(fpu != &current->thread.fpu); /* Almost certainly an anomaly */

        if (!use_eager_fpu() || !static_cpu_has(X86_FEATURE_FPU)) {
                /* FPU state will be reallocated lazily at the first use. */
                fpu__drop(fpu);
        } else {
                if (!fpu->fpstate_active) {
                        fpu__activate_curr(fpu);
                        user_fpu_begin();
                }
                copy_init_fpstate_to_fpregs();
        }
}

i.e. we only execute the buggy sequence in the !eager_fpu case - and old FPUs were 
not eager-FPU, which hid the bug.

The other bug:

@@ -134,7 +134,7 @@ static void __init fpu__init_system_gene
         * 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);
 
was also hidden by the fact that it only affects eagerfpu case - but all previous 
eagerfpu bootups were for post-XSAVE CPUs.

Thanks,

	Ingo

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

#1356440 — [tip:x86/urgent] x86/fpu: Fix eager-FPU handling on legacy FPU machines

Commit-ID:  6e6867093de35141f0a76b66ac13f9f2e2c8e77a
Gitweb:     http://git.kernel.org/tip/6e6867093de35141f0a76b66ac13f9f2e2c8e77a
Author:     Borislav Petkov <bp@alien8.de>
AuthorDate: Fri, 11 Mar 2016 12:32:06 +0100
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Sat, 12 Mar 2016 16:13:55 +0100

x86/fpu: Fix eager-FPU handling on legacy FPU machines

i486 derived 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 in the 'eagerfpu' case.

So after we made eagerfpu the default for all CPU types:

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

these old FPU designs broke. 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-and-tested-by: Bryan O'Donoghue <pure.logic@nexus-software.ie>
Reported-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Signed-off-by: Borislav Petkov <bp@suse.de>
Acked-by: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Brian Gerst <brgerst@gmail.com>
Cc: Dave Hansen <dave.hansen@linux.intel.com>
Cc: Denys Vlasenko <dvlasenk@redhat.com>
Cc: Fenghua Yu <fenghua.yu@intel.com>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Oleg Nesterov <oleg@redhat.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Quentin Casasnovas <quentin.casasnovas@oracle.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Yu-cheng <yu-cheng.yu@intel.com>
Link: http://lkml.kernel.org/r/20160311113206.GD4312@pd.tnic
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 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 d25097c..d5804ad 100644
--- a/arch/x86/kernel/fpu/core.c
+++ b/arch/x86/kernel/fpu/core.c
@@ -409,8 +409,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 9ee7e30..bd08fb7 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();
 }

[toc] | [prev] | [standalone]

Page 2 of 2 — ← Prev page 1 [2]

Back to top | Article view | linux.kernel

csiph-web