2015-08-14 07:15:09

by Ingo Molnar

[permalink] [raw]
Subject: [GIT PULL] x86 fixes

Linus,

Please pull the latest x86-urgent-for-linus git tree from:

git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git x86-urgent-for-linus

# HEAD: 4809146b86c3d41ce588fdb767d021e2a80600dd x86/ldt: Correct FPU emulation access to LDT

Two followup fixes related to the previous LDT fix.

Thanks,

Ingo

------------------>
Juergen Gross (2):
x86/ldt: Correct LDT access in single stepping logic
x86/ldt: Correct FPU emulation access to LDT


arch/x86/kernel/step.c | 4 ++--
arch/x86/math-emu/fpu_entry.c | 3 +--
arch/x86/math-emu/fpu_system.h | 21 ++++++++++++++++++---
arch/x86/math-emu/get_address.c | 3 +--
4 files changed, 22 insertions(+), 9 deletions(-)

diff --git a/arch/x86/kernel/step.c b/arch/x86/kernel/step.c
index 6273324186ac..0ccb53a9fcd9 100644
--- a/arch/x86/kernel/step.c
+++ b/arch/x86/kernel/step.c
@@ -28,11 +28,11 @@ unsigned long convert_ip_to_linear(struct task_struct *child, struct pt_regs *re
struct desc_struct *desc;
unsigned long base;

- seg &= ~7UL;
+ seg >>= 3;

mutex_lock(&child->mm->context.lock);
if (unlikely(!child->mm->context.ldt ||
- (seg >> 3) >= child->mm->context.ldt->size))
+ seg >= child->mm->context.ldt->size))
addr = -1L; /* bogus selector, access would fault */
else {
desc = &child->mm->context.ldt->entries[seg];
diff --git a/arch/x86/math-emu/fpu_entry.c b/arch/x86/math-emu/fpu_entry.c
index f37e84ab49f3..3d8f2e421466 100644
--- a/arch/x86/math-emu/fpu_entry.c
+++ b/arch/x86/math-emu/fpu_entry.c
@@ -29,7 +29,6 @@

#include <asm/uaccess.h>
#include <asm/traps.h>
-#include <asm/desc.h>
#include <asm/user.h>
#include <asm/fpu/internal.h>

@@ -181,7 +180,7 @@ void math_emulate(struct math_emu_info *info)
math_abort(FPU_info, SIGILL);
}

- code_descriptor = LDT_DESCRIPTOR(FPU_CS);
+ code_descriptor = FPU_get_ldt_descriptor(FPU_CS);
if (SEG_D_SIZE(code_descriptor)) {
/* The above test may be wrong, the book is not clear */
/* Segmented 32 bit protected mode */
diff --git a/arch/x86/math-emu/fpu_system.h b/arch/x86/math-emu/fpu_system.h
index 9ccecb61a4fa..5e044d506b7a 100644
--- a/arch/x86/math-emu/fpu_system.h
+++ b/arch/x86/math-emu/fpu_system.h
@@ -16,9 +16,24 @@
#include <linux/kernel.h>
#include <linux/mm.h>

-/* s is always from a cpu register, and the cpu does bounds checking
- * during register load --> no further bounds checks needed */
-#define LDT_DESCRIPTOR(s) (((struct desc_struct *)current->mm->context.ldt)[(s) >> 3])
+#include <asm/desc.h>
+#include <asm/mmu_context.h>
+
+static inline struct desc_struct FPU_get_ldt_descriptor(unsigned seg)
+{
+ static struct desc_struct zero_desc;
+ struct desc_struct ret = zero_desc;
+
+#ifdef CONFIG_MODIFY_LDT_SYSCALL
+ seg >>= 3;
+ mutex_lock(&current->mm->context.lock);
+ if (current->mm->context.ldt && seg < current->mm->context.ldt->size)
+ ret = current->mm->context.ldt->entries[seg];
+ mutex_unlock(&current->mm->context.lock);
+#endif
+ return ret;
+}
+
#define SEG_D_SIZE(x) ((x).b & (3 << 21))
#define SEG_G_BIT(x) ((x).b & (1 << 23))
#define SEG_GRANULARITY(x) (((x).b & (1 << 23)) ? 4096 : 1)
diff --git a/arch/x86/math-emu/get_address.c b/arch/x86/math-emu/get_address.c
index 6ef5e99380f9..d13cab2aec45 100644
--- a/arch/x86/math-emu/get_address.c
+++ b/arch/x86/math-emu/get_address.c
@@ -20,7 +20,6 @@
#include <linux/stddef.h>

#include <asm/uaccess.h>
-#include <asm/desc.h>

#include "fpu_system.h"
#include "exception.h"
@@ -158,7 +157,7 @@ static long pm_address(u_char FPU_modrm, u_char segment,
addr->selector = PM_REG_(segment);
}

- descriptor = LDT_DESCRIPTOR(PM_REG_(segment));
+ descriptor = FPU_get_ldt_descriptor(segment);
base_address = SEG_BASE_ADDR(descriptor);
address = base_address + offset;
limit = base_address


2015-08-14 18:25:18

by Linus Torvalds

[permalink] [raw]
Subject: Re: [GIT PULL] x86 fixes

On Fri, Aug 14, 2015 at 12:15 AM, Ingo Molnar <[email protected]> wrote:
>
> Please pull the latest x86-urgent-for-linus git tree from:

Nope. It seems to be an unmitigated disaster, as far as I can tell.

> +static inline struct desc_struct FPU_get_ldt_descriptor(unsigned seg)
> +{
> + static struct desc_struct zero_desc;
> + struct desc_struct ret = zero_desc;
> +
> +#ifdef CONFIG_MODIFY_LDT_SYSCALL
> + seg >>= 3;

So this seems to take the actual segment selector, and look it up in
the LDT. (Why only the LDT?)

However:

> + descriptor = FPU_get_ldt_descriptor(segment);

as far as I can tell, the "segment" here is the segment _register_
number, not the segment selector.

The segment selector is in "addr->selector", and furthermore I'm not
at all convinced that it is in the LDT to begin with. I'd expect the
common case to be that it's in the GDT, in fact. But what do I know..

Anyway, I may be embarrassingly wrong, and if I am, feel free to shout
bad words at me, but that code seems to be utter crap.

Not that the old code was particularly good either, but at least that
PM_REG_(segment) that *used* to exist there would translate segment
register numbers into actual selector values (even if it would get the
FS case wrong).

Now that said, I doubt anybody cares. Since we don't support the
original 80386, the only way to ever trigger FP emulation is by having
a 486SX or possibly a couple of even rarer clone chips. So it's not
like the fact that the code is completely wrong and crap actually
*matters*, but I still refuse to pull stuff that seems to be so
completely screwed up.

And this commit is even marked as "reviewed". Are you guys really
seeing something that I'm not? Am I somehow wrong in thinking it's
entirely broken?

Linus

2015-08-14 18:46:42

by Andy Lutomirski

[permalink] [raw]
Subject: Re: [GIT PULL] x86 fixes

On Fri, Aug 14, 2015 at 11:25 AM, Linus Torvalds
<[email protected]> wrote:
> On Fri, Aug 14, 2015 at 12:15 AM, Ingo Molnar <[email protected]> wrote:
>>
>> Please pull the latest x86-urgent-for-linus git tree from:
>
> Nope. It seems to be an unmitigated disaster, as far as I can tell.
>
>> +static inline struct desc_struct FPU_get_ldt_descriptor(unsigned seg)
>> +{
>> + static struct desc_struct zero_desc;
>> + struct desc_struct ret = zero_desc;
>> +
>> +#ifdef CONFIG_MODIFY_LDT_SYSCALL
>> + seg >>= 3;
>
> So this seems to take the actual segment selector, and look it up in
> the LDT. (Why only the LDT?)
>

Beats the crap out of me. Presumably it's because no one ever cared
what happened if the selectors were in the GDT. This makes me suspect
that no one has ever tested math emulation in combination with
set_thread_area.


> However:
>
>> + descriptor = FPU_get_ldt_descriptor(segment);
>
> as far as I can tell, the "segment" here is the segment _register_
> number, not the segment selector.
>
> The segment selector is in "addr->selector", and furthermore I'm not
> at all convinced that it is in the LDT to begin with. I'd expect the
> common case to be that it's in the GDT, in fact. But what do I know..
>
> Anyway, I may be embarrassingly wrong, and if I am, feel free to shout
> bad words at me, but that code seems to be utter crap.
>
> Not that the old code was particularly good either, but at least that
> PM_REG_(segment) that *used* to exist there would translate segment
> register numbers into actual selector values (even if it would get the
> FS case wrong).

Oh, crap. You're right, and I'm embarrassed that I missed that.

>
> Now that said, I doubt anybody cares. Since we don't support the
> original 80386, the only way to ever trigger FP emulation is by having
> a 486SX or possibly a couple of even rarer clone chips. So it's not
> like the fact that the code is completely wrong and crap actually
> *matters*, but I still refuse to pull stuff that seems to be so
> completely screwed up.
>
> And this commit is even marked as "reviewed". Are you guys really
> seeing something that I'm not? Am I somehow wrong in thinking it's
> entirely broken?

I think it's only slightly broken.

This bit:

if ((FPU_CS & 4) != 4) { /* Must be in the LDT */
/* Can only handle segmented addressing via the LDT
for now, and it must be 16 bit */
printk("FPU emulator: Unsupported addressing mode\n");
math_abort(FPU_info, SIGILL);
}

code_descriptor = FPU_get_ldt_descriptor(FPU_CS);

is buggy, but no buggier than the old code. FPU_CS is thing:

#define FPU_CS (*(unsigned short *) &(FPU_info->regs->cs))

so it's only the pm_address thing that's broken.

Sorry :(

--Andy

2015-08-14 18:57:49

by Linus Torvalds

[permalink] [raw]
Subject: Re: [GIT PULL] x86 fixes

On Fri, Aug 14, 2015 at 11:46 AM, Andy Lutomirski <[email protected]> wrote:
>
> I think it's only slightly broken.
>
> This bit:
>
> if ((FPU_CS & 4) != 4) { /* Must be in the LDT */
> /* Can only handle segmented addressing via the LDT
> for now, and it must be 16 bit */
> printk("FPU emulator: Unsupported addressing mode\n");
> math_abort(FPU_info, SIGILL);
> }
>
> code_descriptor = FPU_get_ldt_descriptor(FPU_CS);
>
> is buggy, but no buggier than the old code.

That code seems fine to me (and explicitly errors out when it's not in
the LDT). FPU_CS is actually the CS selector value.

So testing that for being in the LDT by checking bit #2, and then
using FPU_get_ldt_descriptor() on it actually seems *correct*.

It's the actual instruction data segment handling that looks entirely
broken, and was explicitly made *more* broken by that commit.

The FPU emulation code has two different kinds of segment defines:

- the PREFIX_xx_ defines are the segment register numbers (well,
prefix numbers)

- the FPU_CS/SS/DS defines are the current selector values for those.

and yes, it's confusing how it tends to use the variable name
"segment" for the prefix number, when in the rest of the kernel we
tend to always track the selector value.

Linus

2015-08-14 19:06:26

by Linus Torvalds

[permalink] [raw]
Subject: Re: [GIT PULL] x86 fixes

On Fri, Aug 14, 2015 at 11:57 AM, Linus Torvalds
<[email protected]> wrote:
>
> That code seems fine to me (and explicitly errors out when it's not in
> the LDT). FPU_CS is actually the CS selector value.
>
> So testing that for being in the LDT by checking bit #2, and then
> using FPU_get_ldt_descriptor() on it actually seems *correct*.
>
> It's the actual instruction data segment handling that looks entirely
> broken, and was explicitly made *more* broken by that commit.

Note that in practice, it's *probably* true that if CS ends up being
in the LDT (so we're running something odd like Wine), then *probably*
the data segments are going to be in the LDT too. So the old code that
unconditionally looked things up in the LDT probably worked in
practice, even if it was wrong.

The new code cannot *possibly* work at all, because even if the data
segment register is in the LDT, it uses the wrong thing to look up the
LDT entry, so it will get the wrong base.

But as mentioned, it will only *matter* on something like a 486SX, and
only when the whole "CS/DS didn't match the default flat segments"
case triggers, so not only do you have to run on a 486SX, you will
have to run something like Wine on it. So it sounds very very unlikely
that this bug matters in practice.

Linus

2015-08-14 19:14:39

by Andy Lutomirski

[permalink] [raw]
Subject: Re: [GIT PULL] x86 fixes

On Fri, Aug 14, 2015 at 11:57 AM, Linus Torvalds
<[email protected]> wrote:
> On Fri, Aug 14, 2015 at 11:46 AM, Andy Lutomirski <[email protected]> wrote:
>>
>> I think it's only slightly broken.
>>
>> This bit:
>>
>> if ((FPU_CS & 4) != 4) { /* Must be in the LDT */
>> /* Can only handle segmented addressing via the LDT
>> for now, and it must be 16 bit */
>> printk("FPU emulator: Unsupported addressing mode\n");
>> math_abort(FPU_info, SIGILL);
>> }
>>
>> code_descriptor = FPU_get_ldt_descriptor(FPU_CS);
>>
>> is buggy, but no buggier than the old code.
>
> That code seems fine to me (and explicitly errors out when it's not in
> the LDT). FPU_CS is actually the CS selector value.
>
> So testing that for being in the LDT by checking bit #2, and then
> using FPU_get_ldt_descriptor() on it actually seems *correct*.
>

By "buggy" I meant that it aborted if it was in the GDT but wasn't
flat. This'll break if anyone does an emulated FP op on TLS data.

--Andy

2015-08-14 19:18:33

by Andy Lutomirski

[permalink] [raw]
Subject: Re: [GIT PULL] x86 fixes

On Fri, Aug 14, 2015 at 12:06 PM, Linus Torvalds
<[email protected]> wrote:
> On Fri, Aug 14, 2015 at 11:57 AM, Linus Torvalds
> <[email protected]> wrote:
>>
>> That code seems fine to me (and explicitly errors out when it's not in
>> the LDT). FPU_CS is actually the CS selector value.
>>
>> So testing that for being in the LDT by checking bit #2, and then
>> using FPU_get_ldt_descriptor() on it actually seems *correct*.
>>
>> It's the actual instruction data segment handling that looks entirely
>> broken, and was explicitly made *more* broken by that commit.
>
> Note that in practice, it's *probably* true that if CS ends up being
> in the LDT (so we're running something odd like Wine), then *probably*
> the data segments are going to be in the LDT too. So the old code that
> unconditionally looked things up in the LDT probably worked in
> practice, even if it was wrong.
>
> The new code cannot *possibly* work at all, because even if the data
> segment register is in the LDT, it uses the wrong thing to look up the
> LDT entry, so it will get the wrong base.
>
> But as mentioned, it will only *matter* on something like a 486SX, and
> only when the whole "CS/DS didn't match the default flat segments"
> case triggers, so not only do you have to run on a 486SX, you will
> have to run something like Wine on it. So it sounds very very unlikely
> that this bug matters in practice.

Unless I'm missing something, it's literally a one-line fix -- just
put the missing PM_REG_(segment) back in.

--Andy

2015-08-14 19:38:19

by Linus Torvalds

[permalink] [raw]
Subject: Re: [GIT PULL] x86 fixes

On Fri, Aug 14, 2015 at 12:18 PM, Andy Lutomirski <[email protected]> wrote:
>
> Unless I'm missing something, it's literally a one-line fix -- just
> put the missing PM_REG_(segment) back in.

Please use address->selector instead. Which should get the fs case right too.

Linus

2015-08-17 08:01:51

by Ingo Molnar

[permalink] [raw]
Subject: Re: [GIT PULL] x86 fixes


(Sorry about the late reply, wasn't around on the weekend.)

* Linus Torvalds <[email protected]> wrote:

> Now that said, I doubt anybody cares. Since we don't support the original 80386,
> the only way to ever trigger FP emulation is by having a 486SX or possibly a
> couple of even rarer clone chips. [...]

Yeah. So when I re-wrote the FPU code I tried to test math-emu by booting with
'no387': it turned out that ever since the XSAVE code got merged upstream,
math-emu oopsed reliably during bootup with a NULL reference, because it wasn't
updated to the dynamic allocation logic in:

61c4628b5386 ("x86, fpu: split FPU state from task struct - v5")

That was 6 years ago, so anything v2.6.26 and later probably has 100% non-working
math-emu.

So when I re-introduced static allocations math-emu started working again, to a
limited degree: on a modern distro, trying to boot /bin/bash I got a prompt, but
various programs would segfault. I did not investigate it any deeper, I suppose
the FPU emulation does not go far enough for modern user-space, or maybe it has
more bugs.

So in reality nobody has cared about x86 math-emu in the last 6 years and we can
probably remove it for good. I kept it for nostalgic reasons, but I guess using
v2.4 kernels ought to be enough for those with nostalgia?

> [...] So it's not like the fact that the code is completely wrong and crap
> actually *matters*, but I still refuse to pull stuff that seems to be so
> completely screwed up.

That's true, my bad for merging it!

Any objections against removing all of math-emu in v4.3? This would simplify the
FPU code in various places beyond math-emu/.

Thanks,

Ingo

2015-08-17 10:59:27

by Denys Vlasenko

[permalink] [raw]
Subject: Re: [GIT PULL] x86 fixes

On 08/17/2015 10:01 AM, Ingo Molnar wrote:
>
> (Sorry about the late reply, wasn't around on the weekend.)
>
> * Linus Torvalds <[email protected]> wrote:
>
>> Now that said, I doubt anybody cares. Since we don't support the original 80386,
>> the only way to ever trigger FP emulation is by having a 486SX or possibly a
>> couple of even rarer clone chips. [...]
>
> Yeah. So when I re-wrote the FPU code I tried to test math-emu by booting with
> 'no387': it turned out that ever since the XSAVE code got merged upstream,
> math-emu oopsed reliably during bootup with a NULL reference, because it wasn't
> updated to the dynamic allocation logic in:
>
> 61c4628b5386 ("x86, fpu: split FPU state from task struct - v5")
>
> That was 6 years ago, so anything v2.6.26 and later probably has 100% non-working
> math-emu.
>
> So when I re-introduced static allocations math-emu started working again, to a
> limited degree: on a modern distro, trying to boot /bin/bash I got a prompt, but
> various programs would segfault. I did not investigate it any deeper, I suppose
> the FPU emulation does not go far enough for modern user-space, or maybe it has
> more bugs.
>
> So in reality nobody has cared about x86 math-emu in the last 6 years

Well... there is this completely crazy x86 javascript emulator by Fabrice Bellard:

http://bellard.org/jslinux/tech.html

"The CPU is close to a 486 compatible x86 without FPU. The lack of FPU
is not a problem when running Linux as Operating System because it
contains a FPU emulator."


I have it running linux 2.6.20 and busybox here:

http://busybox.net/live_bbox/live_bbox.html

(or rather, *you* will have it running linux 2.6.20 inside your browser,
after you click on that link)

2015-08-17 16:47:04

by Linus Torvalds

[permalink] [raw]
Subject: Re: [GIT PULL] x86 fixes

On Mon, Aug 17, 2015 at 1:01 AM, Ingo Molnar <[email protected]> wrote:
>
> Any objections against removing all of math-emu in v4.3? This would simplify the
> FPU code in various places beyond math-emu/.

Hmm. I guess we could just try. The fact that you argue that the FP
emulation likely hasn't worked for a few years is a pretty powerful
argument that nobody has cared. And if somebody does, and ends up
having a use case and willing to test, we can always re-animate the
zombie that is math emu support.

Considering that we don't support the old i386 any more, the only chip
I know of that needs it is the i486sx. However, there are probably
several clone chips that did the "we're a 486" thing by adding the few
integer instructions but not doing a FPU. And there might well be
various embedded versions of the 486 that might even be in use still.
But afaik most distributions have required Pentium of P6-level
capabilities for some time, and I guess any existing old systems
aren't going to upgrade their kernels anyway.

Linus

2015-08-17 16:57:26

by Linus Torvalds

[permalink] [raw]
Subject: Re: [GIT PULL] x86 fixes

On Mon, Aug 17, 2015 at 3:59 AM, Denys Vlasenko <[email protected]> wrote:
>
> I have it running linux 2.6.20 and busybox here:
>
> http://busybox.net/live_bbox/live_bbox.html
>
> (or rather, *you* will have it running linux 2.6.20 inside your browser,
> after you click on that link)

Heh. I'm not sure that's a very useful thing, but if somebody can make
a kvm image or something with an old distribution that is known to
work with FPU emulation, maybe we should verify that the current code
at least works.

Because even if we decide that just deleting it is the right thing for
long-term maintainability, it would be better if we delete it in a
state where it is known to work about as well as it ever did. So that
*if* we have to resurrect it, we know that it at least was working at
the point where it was deleted.

I hate deleting code because it got broken. In contrast, I don't mind
deleting code that no longer makes sense to maintain. The two are
supposed to be very different things.

Linus

2015-08-17 16:58:42

by H. Peter Anvin

[permalink] [raw]
Subject: Re: [GIT PULL] x86 fixes

That is not true. It *does* work, and I have tested it fairly recently.

On August 17, 2015 9:47:01 AM PDT, Linus Torvalds <[email protected]> wrote:
>On Mon, Aug 17, 2015 at 1:01 AM, Ingo Molnar <[email protected]> wrote:
>>
>> Any objections against removing all of math-emu in v4.3? This would
>simplify the
>> FPU code in various places beyond math-emu/.
>
>Hmm. I guess we could just try. The fact that you argue that the FP
>emulation likely hasn't worked for a few years is a pretty powerful
>argument that nobody has cared. And if somebody does, and ends up
>having a use case and willing to test, we can always re-animate the
>zombie that is math emu support.
>
>Considering that we don't support the old i386 any more, the only chip
>I know of that needs it is the i486sx. However, there are probably
>several clone chips that did the "we're a 486" thing by adding the few
>integer instructions but not doing a FPU. And there might well be
>various embedded versions of the 486 that might even be in use still.
>But afaik most distributions have required Pentium of P6-level
>capabilities for some time, and I guess any existing old systems
>aren't going to upgrade their kernels anyway.
>
> Linus

--
Sent from my Android device with K-9 Mail. Please excuse my brevity.

2015-08-17 17:17:28

by Linus Torvalds

[permalink] [raw]
Subject: Re: [GIT PULL] x86 fixes

On Mon, Aug 17, 2015 at 9:58 AM, H. Peter Anvin <[email protected]> wrote:
> That is not true. It *does* work, and I have tested it fairly recently.

Ok, so it's not too badly broken. Good.

Also, while it's been a long time since we needed FPU emulation on the
i486sx, I don't recall the details of any of the (much more modern)
IoT small cores. I *think* the base platforms are all at a Pentium
level (ie not just FPU, but MMX), but maybe there's some reason to
keep FP emulation alive for some platforms.

Linus

2015-08-17 21:03:47

by H. Peter Anvin

[permalink] [raw]
Subject: Re: [GIT PULL] x86 fixes

Let me see when I last treated this... but I thought it was much more recently than that.

On August 17, 2015 1:01:43 AM PDT, Ingo Molnar <[email protected]> wrote:
>
>(Sorry about the late reply, wasn't around on the weekend.)
>
>* Linus Torvalds <[email protected]> wrote:
>
>> Now that said, I doubt anybody cares. Since we don't support the
>original 80386,
>> the only way to ever trigger FP emulation is by having a 486SX or
>possibly a
>> couple of even rarer clone chips. [...]
>
>Yeah. So when I re-wrote the FPU code I tried to test math-emu by
>booting with
>'no387': it turned out that ever since the XSAVE code got merged
>upstream,
>math-emu oopsed reliably during bootup with a NULL reference, because
>it wasn't
>updated to the dynamic allocation logic in:
>
> 61c4628b5386 ("x86, fpu: split FPU state from task struct - v5")
>
>That was 6 years ago, so anything v2.6.26 and later probably has 100%
>non-working
>math-emu.
>
>So when I re-introduced static allocations math-emu started working
>again, to a
>limited degree: on a modern distro, trying to boot /bin/bash I got a
>prompt, but
>various programs would segfault. I did not investigate it any deeper, I
>suppose
>the FPU emulation does not go far enough for modern user-space, or
>maybe it has
>more bugs.
>
>So in reality nobody has cared about x86 math-emu in the last 6 years
>and we can
>probably remove it for good. I kept it for nostalgic reasons, but I
>guess using
>v2.4 kernels ought to be enough for those with nostalgia?
>
>> [...] So it's not like the fact that the code is completely wrong and
>crap
>> actually *matters*, but I still refuse to pull stuff that seems to be
>so
>> completely screwed up.
>
>That's true, my bad for merging it!
>
>Any objections against removing all of math-emu in v4.3? This would
>simplify the
>FPU code in various places beyond math-emu/.
>
>Thanks,
>
> Ingo

--
Sent from my Android device with K-9 Mail. Please excuse my brevity.

2015-08-17 22:18:06

by H. Peter Anvin

[permalink] [raw]
Subject: Re: [GIT PULL] x86 fixes

On 08/17/2015 10:17 AM, Linus Torvalds wrote:
> On Mon, Aug 17, 2015 at 9:58 AM, H. Peter Anvin <[email protected]> wrote:
>> That is not true. It *does* work, and I have tested it fairly recently.
>
> Ok, so it's not too badly broken. Good.
>
> Also, while it's been a long time since we needed FPU emulation on the
> i486sx, I don't recall the details of any of the (much more modern)
> IoT small cores. I *think* the base platforms are all at a Pentium
> level (ie not just FPU, but MMX), but maybe there's some reason to
> keep FP emulation alive for some platforms.
>

I just went back and looked at my records... I can guarantee that it
worked as of 743aa456c1834f76982af44e8b71d1a0b2a82e21.

-hpa

2015-08-17 23:47:42

by Bryan O'Donoghue

[permalink] [raw]
Subject: Re: [GIT PULL] x86 fixes

On 17/08/15 18:17, Linus Torvalds wrote:
> On Mon, Aug 17, 2015 at 9:58 AM, H. Peter Anvin <[email protected]> wrote:
>> That is not true. It *does* work, and I have tested it fairly recently.
>
> Ok, so it's not too badly broken. Good.
>
> Also, while it's been a long time since we needed FPU emulation on the
> i486sx, I don't recall the details of any of the (much more modern)
> IoT small cores. I *think* the base platforms are all at a Pentium
> level (ie not just FPU, but MMX), but maybe there's some reason to
> keep FP emulation alive for some platforms.
>
> Linus

Quark is pentium ISA - it has tsc, apic, fpu but, not mmx.

2015-08-17 23:59:39

by Andy Lutomirski

[permalink] [raw]
Subject: Re: [GIT PULL] x86 fixes

On Mon, Aug 17, 2015 at 1:01 AM, Ingo Molnar <[email protected]> wrote:
> So when I re-introduced static allocations math-emu started working again, to a
> limited degree: on a modern distro, trying to boot /bin/bash I got a prompt, but
> various programs would segfault. I did not investigate it any deeper, I suppose
> the FPU emulation does not go far enough for modern user-space, or maybe it has
> more bugs.
>

Were you testing with just no387 or did you run a VM with SSE2 and
such turned off?

There's a *lot* of userspace that incorrectly checks for instructions
without checking for the state support. I've filed bugs against
libgcc for this and they're still not fixed IIRC.

--Andy

2015-08-18 00:01:45

by H. Peter Anvin

[permalink] [raw]
Subject: Re: [GIT PULL] x86 fixes

I ran a hacked Qemu with FPU off.

On August 17, 2015 4:59:18 PM PDT, Andy Lutomirski <[email protected]> wrote:
>On Mon, Aug 17, 2015 at 1:01 AM, Ingo Molnar <[email protected]> wrote:
>> So when I re-introduced static allocations math-emu started working
>again, to a
>> limited degree: on a modern distro, trying to boot /bin/bash I got a
>prompt, but
>> various programs would segfault. I did not investigate it any deeper,
>I suppose
>> the FPU emulation does not go far enough for modern user-space, or
>maybe it has
>> more bugs.
>>
>
>Were you testing with just no387 or did you run a VM with SSE2 and
>such turned off?
>
>There's a *lot* of userspace that incorrectly checks for instructions
>without checking for the state support. I've filed bugs against
>libgcc for this and they're still not fixed IIRC.
>
>--Andy

--
Sent from my Android device with K-9 Mail. Please excuse my brevity.

2015-08-18 00:07:12

by H. Peter Anvin

[permalink] [raw]
Subject: Re: [GIT PULL] x86 fixes

User space does not need to treat for FPU instructions, except for performance reasons, because the kernel emulates the full x87 FPU. So it is localized to the kernel.

On August 17, 2015 4:59:18 PM PDT, Andy Lutomirski <[email protected]> wrote:
>On Mon, Aug 17, 2015 at 1:01 AM, Ingo Molnar <[email protected]> wrote:
>> So when I re-introduced static allocations math-emu started working
>again, to a
>> limited degree: on a modern distro, trying to boot /bin/bash I got a
>prompt, but
>> various programs would segfault. I did not investigate it any deeper,
>I suppose
>> the FPU emulation does not go far enough for modern user-space, or
>maybe it has
>> more bugs.
>>
>
>Were you testing with just no387 or did you run a VM with SSE2 and
>such turned off?
>
>There's a *lot* of userspace that incorrectly checks for instructions
>without checking for the state support. I've filed bugs against
>libgcc for this and they're still not fixed IIRC.
>
>--Andy

--
Sent from my Android device with K-9 Mail. Please excuse my brevity.

2015-08-18 00:19:31

by Andy Lutomirski

[permalink] [raw]
Subject: Re: [GIT PULL] x86 fixes

On Mon, Aug 17, 2015 at 5:06 PM, H. Peter Anvin <[email protected]> wrote:
> User space does not need to treat for FPU instructions, except for performance reasons, because the kernel emulates the full x87 FPU. So it is localized to the kernel.

But user space needs to avoid SSE2 and such, I suspect. In general,
I'd be surprised if things work well if we emulate the FPU (and set
CR0.em? I haven't checked out Linux's FPU emulation works) if user
code sees fancy instruction sets exposed and possibly even OSXSAVE.

None of this matters except for testing, since it's very unlikely that
any CPU exists that supports XSAVE, XMM, SSE2, etc but uses emulated
x87. But if we emulate such a beast, things could break, and I bet
that's what Ingo's seeing. (Also, lots of distros target "i686" these
days, and that might cause its own set of problems.)

--Andy

2015-08-18 05:57:15

by H. Peter Anvin

[permalink] [raw]
Subject: Re: [GIT PULL] x86 fixes

I used a very old userspace, and embedded systems are much more likely to use uclibc than glibc. However, if they try to use SSE without checking they will break on a hell of a lot more hardware.

On August 17, 2015 5:19:10 PM PDT, Andy Lutomirski <[email protected]> wrote:
>On Mon, Aug 17, 2015 at 5:06 PM, H. Peter Anvin <[email protected]> wrote:
>> User space does not need to treat for FPU instructions, except for
>performance reasons, because the kernel emulates the full x87 FPU. So
>it is localized to the kernel.
>
>But user space needs to avoid SSE2 and such, I suspect. In general,
>I'd be surprised if things work well if we emulate the FPU (and set
>CR0.em? I haven't checked out Linux's FPU emulation works) if user
>code sees fancy instruction sets exposed and possibly even OSXSAVE.
>
>None of this matters except for testing, since it's very unlikely that
>any CPU exists that supports XSAVE, XMM, SSE2, etc but uses emulated
>x87. But if we emulate such a beast, things could break, and I bet
>that's what Ingo's seeing. (Also, lots of distros target "i686" these
>days, and that might cause its own set of problems.)
>
>--Andy

--
Sent from my Android device with K-9 Mail. Please excuse my brevity.

2015-08-18 06:00:41

by H. Peter Anvin

[permalink] [raw]
Subject: Re: [GIT PULL] x86 fixes

I should make it clear: I was using a 486SX model in Qemu.

On August 17, 2015 5:19:10 PM PDT, Andy Lutomirski <[email protected]> wrote:
>On Mon, Aug 17, 2015 at 5:06 PM, H. Peter Anvin <[email protected]> wrote:
>> User space does not need to treat for FPU instructions, except for
>performance reasons, because the kernel emulates the full x87 FPU. So
>it is localized to the kernel.
>
>But user space needs to avoid SSE2 and such, I suspect. In general,
>I'd be surprised if things work well if we emulate the FPU (and set
>CR0.em? I haven't checked out Linux's FPU emulation works) if user
>code sees fancy instruction sets exposed and possibly even OSXSAVE.
>
>None of this matters except for testing, since it's very unlikely that
>any CPU exists that supports XSAVE, XMM, SSE2, etc but uses emulated
>x87. But if we emulate such a beast, things could break, and I bet
>that's what Ingo's seeing. (Also, lots of distros target "i686" these
>days, and that might cause its own set of problems.)
>
>--Andy

--
Sent from my Android device with K-9 Mail. Please excuse my brevity.

2015-08-18 07:55:25

by Ingo Molnar

[permalink] [raw]
Subject: Re: [GIT PULL] x86 fixes


* Andy Lutomirski <[email protected]> wrote:

> On Mon, Aug 17, 2015 at 1:01 AM, Ingo Molnar <[email protected]> wrote:
>
> > So when I re-introduced static allocations math-emu started working again, to
> > a limited degree: on a modern distro, trying to boot /bin/bash I got a prompt,
> > but various programs would segfault. I did not investigate it any deeper, I
> > suppose the FPU emulation does not go far enough for modern user-space, or
> > maybe it has more bugs.
>
> Were you testing with just no387 or did you run a VM with SSE2 and such turned
> off?
>
> There's a *lot* of userspace that incorrectly checks for instructions without
> checking for the state support. I've filed bugs against libgcc for this and
> they're still not fixed IIRC.

I tested relatively ancient user-space, 2007 era Fedora Core 6 - but I didn't
check whether it's truly SSE-less (it probably isn't).

Thanks,

Ingo

2015-08-18 07:57:52

by Ingo Molnar

[permalink] [raw]
Subject: Re: [GIT PULL] x86 fixes


* Linus Torvalds <[email protected]> wrote:

> On Mon, Aug 17, 2015 at 3:59 AM, Denys Vlasenko <[email protected]> wrote:
> >
> > I have it running linux 2.6.20 and busybox here:
> >
> > http://busybox.net/live_bbox/live_bbox.html
> >
> > (or rather, *you* will have it running linux 2.6.20 inside your browser,
> > after you click on that link)
>
> Heh. I'm not sure that's a very useful thing, but if somebody can make a kvm
> image or something with an old distribution that is known to work with FPU
> emulation, maybe we should verify that the current code at least works.
>
> Because even if we decide that just deleting it is the right thing for long-term
> maintainability, it would be better if we delete it in a state where it is known
> to work about as well as it ever did. So that *if* we have to resurrect it, we
> know that it at least was working at the point where it was deleted.
>
> I hate deleting code because it got broken. In contrast, I don't mind deleting
> code that no longer makes sense to maintain. The two are supposed to be very
> different things.

Absolutely agreed - it was my other motivator to try to fix math-emu in the FPU
series.

Another example is the low level assembly code which we are converting to C code,
we (tried to) fix and deobfuscate and fix it before moving to C for similar
reasons.

Thanks,

Ingo

2015-08-19 05:59:38

by Ingo Molnar

[permalink] [raw]
Subject: Re: [GIT PULL] x86 fixes


* H. Peter Anvin <[email protected]> wrote:

> On 08/17/2015 10:17 AM, Linus Torvalds wrote:
> > On Mon, Aug 17, 2015 at 9:58 AM, H. Peter Anvin <[email protected]> wrote:
> >>
> >> That is not true. It *does* work, and I have tested it fairly recently.
> >
> > Ok, so it's not too badly broken. Good.
> >
> > Also, while it's been a long time since we needed FPU emulation on the
> > i486sx, I don't recall the details of any of the (much more modern)
> > IoT small cores. I *think* the base platforms are all at a Pentium
> > level (ie not just FPU, but MMX), but maybe there's some reason to
> > keep FP emulation alive for some platforms.
> >
>
> I just went back and looked at my records... I can guarantee that it
> worked as of 743aa456c1834f76982af44e8b71d1a0b2a82e21.

So I went and built 743aa456c1834f76 with ARCH=i386 defconfig +MATH_EMULATION=y
and booted it on real hardware with and without 'no387':

- 743aa456c1834f76: boots fine to a generic distro
- 743aa456c1834f76 + no387: early crash

the early crash is similar to what I saw when doing the recent FPU changes (and
which crash I fixed):

[ 0.000000] Linux version 3.7.0+ (mingo@fomalhaut) (gcc version 4.9.2 20150212 (Red Hat 4.9.2-6) (GCC) ) #284 SMP Wed Aug 19 07:51:05 CEST 2015
...
[ 0.000000] Inode-cache hash table entries: 65536 (order: 6, 262144 bytes)
[ 0.000000] __ex_table already sorted, skipping sort
[ 0.000000] Initializing CPU#0
[ 0.000000] BUG: unable to handle kernel NULL pointer dereference at (null)
...
[ 0.000000] EIP is at kmem_cache_alloc+0x25/0x110
[ 0.000000] Call Trace:
[ 0.000000] [<c048a1fc>] ? sprintf+0x1c/0x20
[ 0.000000] [<c020a11f>] ? init_fpu+0x7f/0xc0
[ 0.000000] [<c0235aef>] ? print_prefix+0xcf/0x130
[ 0.000000] [<c08b0d20>] ? do_debug+0x160/0x160
[ 0.000000] [<c020a11f>] init_fpu+0x7f/0xc0
[ 0.000000] [<c0730df5>] math_emulate+0x6b5/0xc90
[ 0.000000] [<c08b0d58>] do_device_not_available+0x38/0x60
[ 0.000000] [<c08b0752>] error_code+0x5a/0x60
[ 0.000000] [<c08b0d20>] ? do_debug+0x160/0x160
[ 0.000000] [<c08a29e7>] ? fpu_init+0xd9/0xf7
[ 0.000000] [<c08a475e>] cpu_init+0x237/0x23f
[ 0.000000] [<c0b324f2>] trap_init+0x243/0x24b
[ 0.000000] [<c0b30759>] start_kernel+0x143/0x2d4
[ 0.000000] [<c0b302a0>] i386_start_kernel+0x76/0x7b

And I think this early crash bug was introduced 7+ years ago in March 2008, when
the FPU context area was separated from the task struct and its allocation went
dynamic:

61c4628b5386 ("x86, fpu: split FPU state from task struct - v5")

... but that's just a guess, I couldn't check that as kernels that far back don't
build and boot anymore with modern tooling.

Thanks,

Ingo

2015-08-19 06:15:27

by Ingo Molnar

[permalink] [raw]
Subject: Re: [GIT PULL] x86 fixes


* Ingo Molnar <[email protected]> wrote:

> So I went and built 743aa456c1834f76 with ARCH=i386 defconfig +MATH_EMULATION=y
> and booted it on real hardware with and without 'no387':
>
> - 743aa456c1834f76: boots fine to a generic distro
> - 743aa456c1834f76 + no387: early crash
>
> the early crash is similar to what I saw when doing the recent FPU changes (and
> which crash I fixed):

Hm, so that crash is back again with the latest kernel:

[ 0.000000] EIP is at kmem_cache_alloc+0x25/0x110
...
[ 0.000000] [<c020a11f>] init_fpu+0x7f/0xc0
[ 0.000000] [<c0730df5>] math_emulate+0x6b5/0xc90
[ 0.000000] [<c0227440>] ? early_serial_putc+0x60/0x60
[ 0.000000] [<c02359cc>] ? call_console_drivers.constprop.14+0x8c/0xe0
[ 0.000000] [<c0258c05>] ? up+0x25/0x50
[ 0.000000] [<c02366e5>] ? console_unlock+0x305/0x4d0
[ 0.000000] [<c0258b06>] ? process_srcu+0x56/0xb0
[ 0.000000] [<c08b0d20>] ? do_debug+0x160/0x160
[ 0.000000] [<c08b0d58>] do_device_not_available+0x38/0x60
[ 0.000000] [<c08b0752>] error_code+0x5a/0x60
[ 0.000000] [<c08b0d20>] ? do_debug+0x160/0x160
[ 0.000000] [<c08a29e7>] ? fpu_init+0xd9/0xf7
[ 0.000000] [<c08a475e>] cpu_init+0x237/0x23f
[ 0.000000] [<c0b324f2>] trap_init+0x243/0x24b
[ 0.000000] [<c0b30759>] start_kernel+0x143/0x2d4
[ 0.000000] [<c0b302a0>] i386_start_kernel+0x76/0x7b

I suspect due to:

5aaeb5c01c5b x86/fpu, sched: Introduce CONFIG_ARCH_WANTS_DYNAMIC_TASK_STRUCT and use it on x86
0c8c0f03e3a2 x86/fpu, sched: Dynamically allocate 'struct fpu'

Thanks,

Ingo

2015-08-19 06:51:03

by Ingo Molnar

[permalink] [raw]
Subject: Re: [GIT PULL] x86 fixes


* Ingo Molnar <[email protected]> wrote:

>
> * H. Peter Anvin <[email protected]> wrote:
>
> > On 08/17/2015 10:17 AM, Linus Torvalds wrote:
> > > On Mon, Aug 17, 2015 at 9:58 AM, H. Peter Anvin <[email protected]> wrote:
> > >>
> > >> That is not true. It *does* work, and I have tested it fairly recently.
> > >
> > > Ok, so it's not too badly broken. Good.
> > >
> > > Also, while it's been a long time since we needed FPU emulation on the
> > > i486sx, I don't recall the details of any of the (much more modern)
> > > IoT small cores. I *think* the base platforms are all at a Pentium
> > > level (ie not just FPU, but MMX), but maybe there's some reason to
> > > keep FP emulation alive for some platforms.
> > >
> >
> > I just went back and looked at my records... I can guarantee that it
> > worked as of 743aa456c1834f76982af44e8b71d1a0b2a82e21.
>
> So I went and built 743aa456c1834f76 with ARCH=i386 defconfig +MATH_EMULATION=y
> and booted it on real hardware with and without 'no387':
>
> - 743aa456c1834f76: boots fine to a generic distro
> - 743aa456c1834f76 + no387: early crash

Ah, so I was able to make math-emu work with the 'no387 nofxsr' boot options.

It turns out that the crash happens due 'no387' not turning off all modern FPU
features in cpufeatures, which confuses the FPU code. (This is a far less severe
bug than math emu not working at all.)

I get some instances of:

/etc/rc3.d/S99local: line 26: 1626 Illegal instruction

due to user-space presuming modern FPU capabilities:

Program received signal SIGILL, Illegal instruction.
0xb76be371 in RAND_SSLeay () from /lib/libcrypto.so.6

0xb76be371 <RAND_SSLeay+1297>: fucomip %st(1),%st

FUCOMPIP is a P6+ FPU instruction.

but otherwise it boots up to a shell prompt.

So I take back my claim, math-emu works. If math-emu grew support for some P6 era
FPU instructions it might even boot up generic distros without too much trouble.

Thanks,

Ingo

2015-08-19 10:00:47

by H. Peter Anvin

[permalink] [raw]
Subject: Re: [GIT PULL] x86 fixes

On 08/18/15 23:50, Ingo Molnar wrote:
>
> Ah, so I was able to make math-emu work with the 'no387 nofxsr' boot options.
>
> It turns out that the crash happens due 'no387' not turning off all modern FPU
> features in cpufeatures, which confuses the FPU code. (This is a far less severe
> bug than math emu not working at all.)
>
> I get some instances of:
>
> /etc/rc3.d/S99local: line 26: 1626 Illegal instruction
>
> due to user-space presuming modern FPU capabilities:
>

And I bet if CPUID actually reported the right thing it probably would
work okay. As I said, I tested this under Qemu which reported an
accurate (lack of) CPUID for a 486SX.

-hpa

2015-08-19 21:53:31

by H. Peter Anvin

[permalink] [raw]
Subject: Re: [GIT PULL] x86 fixes

On 08/18/2015 11:50 PM, Ingo Molnar wrote:
>
> So I take back my claim, math-emu works. If math-emu grew support for some P6 era
> FPU instructions it might even boot up generic distros without too much trouble.
>

For the record, I used a RedHat 4.1 (not Fedora or RHEL!) userspace.


https://plus.google.com/107672941537404681483/posts/2GYcSm5injN

-hpa

2015-08-19 22:33:52

by Linus Torvalds

[permalink] [raw]
Subject: Re: [GIT PULL] x86 fixes

On Wed, Aug 19, 2015 at 3:00 AM, H. Peter Anvin <[email protected]> wrote:
>
> And I bet if CPUID actually reported the right thing it probably would work
> okay. As I said, I tested this under Qemu which reported an accurate (lack
> of) CPUID for a 486SX.

While I agree that cpuid is a problem for FPU emulation on modern
CPU's, if this is due to "fucomip" then I think it's just that modern
distributions (and not-so-modern ones, for that matter) are compiled
with i686 support, so gcc just generates fucomip directly. So it's
just "plain FPU" code (no mmx, nothing like that), but it still fails.

The "set regular integer flags instructions" versions of floating
point compares are some of the bigger improvements to the legacy i87
instruction set, because the sequences to do FP compares without them
are just insane. I forget the details, but it's something like "store
status word to ax, then use sahf to get it into the flags register".
Crazy crazy crap. So it's no wonder that gcc wants to use a i686-only
instruction even for just regular FP code if at all possible.

I suspect it shouldn't be that hard to add f[u]compi[p] support to the
emulator.

But I also expect that most modern distributions are likely fairly
eager to use mmx etc, which sounds like a major pain to emulate.

Linus

2015-08-20 06:55:04

by H. Peter Anvin

[permalink] [raw]
Subject: Re: [GIT PULL] x86 fixes

Yes, and MMX, SSE et al didn't have the envision trap support, so you would have to do a full decide and emulation inside the #UD handler. However, the trap overhead for a lot of those instructions is extreme, as compared to the rather heavyweight x87 instructions (in terms of the ratio between the trap overhead and the actual emulation code.)

On August 19, 2015 3:33:50 PM PDT, Linus Torvalds <[email protected]> wrote:
>On Wed, Aug 19, 2015 at 3:00 AM, H. Peter Anvin <[email protected]> wrote:
>>
>> And I bet if CPUID actually reported the right thing it probably
>would work
>> okay. As I said, I tested this under Qemu which reported an accurate
>(lack
>> of) CPUID for a 486SX.
>
>While I agree that cpuid is a problem for FPU emulation on modern
>CPU's, if this is due to "fucomip" then I think it's just that modern
>distributions (and not-so-modern ones, for that matter) are compiled
>with i686 support, so gcc just generates fucomip directly. So it's
>just "plain FPU" code (no mmx, nothing like that), but it still fails.
>
>The "set regular integer flags instructions" versions of floating
>point compares are some of the bigger improvements to the legacy i87
>instruction set, because the sequences to do FP compares without them
>are just insane. I forget the details, but it's something like "store
>status word to ax, then use sahf to get it into the flags register".
>Crazy crazy crap. So it's no wonder that gcc wants to use a i686-only
>instruction even for just regular FP code if at all possible.
>
>I suspect it shouldn't be that hard to add f[u]compi[p] support to the
>emulator.
>
>But I also expect that most modern distributions are likely fairly
>eager to use mmx etc, which sounds like a major pain to emulate.
>
> Linus

--
Sent from my Android device with K-9 Mail. Please excuse my brevity.

2015-08-21 10:17:19

by Denys Vlasenko

[permalink] [raw]
Subject: Re: [GIT PULL] x86 fixes

On 08/19/2015 08:50 AM, Ingo Molnar wrote:
> I get some instances of:
>
> /etc/rc3.d/S99local: line 26: 1626 Illegal instruction
>
> due to user-space presuming modern FPU capabilities:
>
> Program received signal SIGILL, Illegal instruction.
> 0xb76be371 in RAND_SSLeay () from /lib/libcrypto.so.6
>
> 0xb76be371 <RAND_SSLeay+1297>: fucomip %st(1),%st
>
> FUCOMPIP is a P6+ FPU instruction.

I have a patch which adds support for emulating it. Will post now.