Skip to contentSkip to conversation

x86: Disable traditional FPU instructions too

RFC In review Borislav Petkov 5 participants 10 messages Jan 27, 2014 – Jan 28, 2014 x86

lore
Share & tools
lore

Borislav Petkov

author maintainer
#0 lore raw

RFC 1/1 x86: Disable traditional FPU instructions too

Hey guys,

should we do the below? It looks like we don't disable the generation of *all* FPU instructions on x86_64 (commit message below has the rationale why).

We do -msoft-float on 32-bit only and Micha says that -msoft-float and -mno-80387 are the same and the gcc manpage says:

On machines where a function returns floating-point results in the 80387 register stack, some floating-point opcodes may be emitted even if -msoft-float is used.

and right after, it has also

-mno-fp-ret-in-387
Do not use the FPU registers for return values of functions.

The usual calling convention has functions return values of types "float" and "double" in an FPU register, even if there is no FPU. The idea is that the operating system should emulate an FPU.

The option -mno-fp-ret-in-387 causes such values to be returned in ordinary CPU registers instead.

Btw, there's this -mno-fp-regs switch too which forces passing of FP results of functions in integer registers...

So maybe we should do this:

---
From: Borislav Petkov <[email protected]>
Subject: [PATCH] x86: Disable generation of traditional x87 instructions

We recently had the case where wrongly used constant caused the generation of x87 instructions in kernel code unknowingly, wreaking all kinds of havoc. Disable the generation of those too. This will save people a lot of time when trying to debug such issues by erroring out of the build and not manifesting itself in all kinds of spectacular and funny ways at runtime.

Cc Jiri Kosina <[email protected]>
Cc Michael Matz <[email protected]>
Cc Michal Hocko <[email protected]>
Signed-off-by Borislav Petkov <[email protected]>
---
 arch/x86/Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
arch/x86/Makefile +1 -1
index 13b22e0f681d..dbd31182669c 100644
@@ -61,7 +61,7 @@ else
         KBUILD_CFLAGS += -m64
 
         # Don't autogenerate MMX or SSE instructions
-        KBUILD_CFLAGS += -mno-mmx -mno-sse
+        KBUILD_CFLAGS += -mno-mmx -mno-sse -mno-80387 -mno-fp-ret-in-387
 
 	# Use -mpreferred-stack-boundary=3 if supported.
 	KBUILD_CFLAGS += $(call cc-option,-mpreferred-stack-boundary=3)
Signature · 9 lines
1.8.5.2.192.g7794a68


-- 
Regards/Gruss,
    Boris.

Sent from a fat crate under my desk. Formatting is fine.
--
reviewer
#1 lore raw
Quoted 22 lines · Borislav Petkov

Hey guys,

should we do the below? It looks like we don't disable the generation of *all* FPU instructions on x86_64 (commit message below has the rationale why).

We do -msoft-float on 32-bit only and Micha says that -msoft-float and -mno-80387 are the same and the gcc manpage says:

On machines where a function returns floating-point results in the 80387 register stack, some floating-point opcodes may be emitted even if -msoft-float is used.

and right after, it has also

-mno-fp-ret-in-387
Do not use the FPU registers for return values of functions.

The usual calling convention has functions return values of types "float" and "double" in an FPU register, even if there is no FPU. The idea is that the operating system should emulate an FPU.

Quoted 6 lines

The option -mno-fp-ret-in-387 causes such values to be returned in ordinary CPU registers instead.

Btw, there's this -mno-fp-regs switch too which forces passing of FP results of functions in integer registers...

I don't think it'd hurt... although I think the above pretty much requires that the code contain actual floating-point types to ever be generated. The issue with MMX/SSE is that an autovectorizing compiler could decide to use them for *integer* code.

-mno-fp-ret-in-387 in particular will only ever apply if a function return type is a floating-point type.

-hpa

#2 lore raw
Quoted 3 lines · H. Peter Anvin

I don't think it'd hurt... although I think the above pretty much requires that the code contain actual floating-point types to ever be generated.

Unfortunately using float-typed constants (such as 1EexpL) are enough for this to happen.

Signature · 2 lines
Jiri Kosina
SUSE Labs
#3 lore raw
Quoted 8 lines · Jiri Kosina
Quoted 3 lines · H. Peter Anvin

I don't think it'd hurt... although I think the above pretty much requires that the code contain actual floating-point types to ever be generated.

Unfortunately using float-typed constants (such as 1EexpL) are enough for this to happen.

checkpatch would seem to be the right place to check for this?

-Andi

Signature · 1 line
[email protected] -- Speaking for myself only
reviewer
#4 lore raw
Quoted 4 lines · Andi Kleen
Quoted 2 lines · Jiri Kosina

On Mon, 27 Jan 2014, H. Peter Anvin wrote:

Quoted 9 lines
Quoted 6 lines
Quoted 3 lines

I don't think it'd hurt... although I think the above pretty much requires that the code contain actual floating-point types to ever be generated.

Unfortunately using float-typed constants (such as 1EexpL) are enough for this to happen.

checkpatch would seem to be the right place to check for this?

Ideally gcc should have a -no-float option...

-hpa

author maintainer
#5 lore raw
Quoted 1 line · H. Peter Anvin
Quoted 1 line

checkpatch would seem to be the right place to check for this?

checkpatch is not mandatory.

Quoted 1 line

Ideally gcc should have a -no-float option...

Yeah.

Signature · 5 lines
Regards/Gruss,
    Boris.

Sent from a fat crate under my desk. Formatting is fine.
--
author maintainer
#6 lore raw
In reply to H. Peter Anvin
Quoted 3 lines · H. Peter Anvin

I don't think it'd hurt... although I think the above pretty much requires that the code contain actual floating-point types to ever be generated.

Yes, that is supposed to catch usage of the 'E' floating constant, for example, in definitions. It is a build-time assertion, if you want. :)

Quoted 2 lines

The issue with MMX/SSE is that an autovectorizing compiler could decide to use them for *integer* code.

Right, and we don't want the FPU state handling if it can be helped.

Quoted 2 lines

-mno-fp-ret-in-387 in particular will only ever apply if a function return type is a floating-point type.

Yeah, this is for when using -msoft-float == -mno-80387 which still can generate FPU insns according to the manpage:

On machines where a function returns floating-point results in the 80387 register stack, some floating-point opcodes may be emitted even if -msoft-float is used.

Signature · 5 lines
Regards/Gruss,
    Boris.

Sent from a fat crate under my desk. Formatting is fine.
--
#7 lore raw

[CCing Ralf for MIPS.]

Quoted 7 lines · Borislav Petkov
Quoted 3 lines · H. Peter Anvin

I don't think it'd hurt... although I think the above pretty much requires that the code contain actual floating-point types to ever be generated.

Yes, that is supposed to catch usage of the 'E' floating constant, for example, in definitions. It is a build-time assertion, if you want. :)

And git grep says that mips abuses this notation:
arch/mips/include/asm/kvm_host.h:#define MS_TO_NS(x) (x * 1E6L)

I do not have cross-compiler set up to check whether this is a real issue because include/media/rc-core.h defines the same macro correctly so the real code might end up using the right variant.

Signature · 2 lines
Michal Hocko
SUSE Labs
reviewer
#8 lore raw

If that is intentional it needs a big huge freaking comment recording why it is not a bug. I would expect that to generate a FP multiply.

Quoted 20 lines · Michal Hocko

[CCing Ralf for MIPS.]

Quoted 3 lines · Borislav Petkov
Quoted 2 lines · H. Peter Anvin

I don't think it'd hurt... although I think the above pretty much requires that the code contain actual floating-point types to ever

be

Quoted 3 lines
Quoted 1 line

generated.

Yes, that is supposed to catch usage of the 'E' floating constant,

for

Quoted 1 line

example, in definitions. It is a build-time assertion, if you want.

:)

And git grep says that mips abuses this notation:
arch/mips/include/asm/kvm_host.h:#define MS_TO_NS(x) (x * 1E6L)

I do not have cross-compiler set up to check whether this is a real issue because include/media/rc-core.h defines the same macro correctly so the real code might end up using the right variant.

Signature · 1 line
Sent from my mobile phone.  Please pardon brevity and lack of formatting.
#9 lore raw
Quoted 2 lines · H. Peter Anvin

If that is intentional it needs a big huge freaking comment recording why it is not a bug.

Agreed.

Quoted 1 line

I would expect that to generate a FP multiply.

Exactly! And actually the very same construct in a module provided to reproduce an issue was a trigger to the discussed patch.

Quoted 24 lines · Michal Hocko
Quoted 20 lines

[CCing Ralf for MIPS.]

Quoted 3 lines · Borislav Petkov
Quoted 2 lines · H. Peter Anvin

I don't think it'd hurt... although I think the above pretty much requires that the code contain actual floating-point types to ever

be

Quoted 3 lines
Quoted 1 line

generated.

Yes, that is supposed to catch usage of the 'E' floating constant,

for

Quoted 1 line

example, in definitions. It is a build-time assertion, if you want.

:)

And git grep says that mips abuses this notation:
arch/mips/include/asm/kvm_host.h:#define MS_TO_NS(x) (x * 1E6L)

I do not have cross-compiler set up to check whether this is a real issue because include/media/rc-core.h defines the same macro correctly so the real code might end up using the right variant.

Signature · 1 line
Sent from my mobile phone.  Please pardon brevity and lack of formatting.
Signature · 2 lines
Michal Hocko
SUSE Labs