linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/3] x86: stack alignment for boot code and clang
@ 2017-06-13  0:55 Matthias Kaehlcke
  2017-06-13  0:55 ` [PATCH 1/3] kbuild: Add cc-option-no-kbuild macro Matthias Kaehlcke
                   ` (3 more replies)
  0 siblings, 4 replies; 11+ messages in thread
From: Matthias Kaehlcke @ 2017-06-13  0:55 UTC (permalink / raw)
  To: Thomas Gleixner, Ingo Molnar, H . Peter Anvin, H . J . Lu,
	David Woodhouse, Masahiro Yamada, Michal Marek
  Cc: x86, linux-kbuild, linux-kernel, Michael Davidson, Greg Hackmann,
	Nick Desaulniers, Stephen Hines, Kees Cook, Arnd Bergmann,
	Bernhard.Rosenkranzer, Peter Foley, Behan Webster,
	Douglas Anderson, Matthias Kaehlcke

This series fixes an issue with the stack of the x86 boot code not
being aligned as intended. Further it adapts the Makefile to account
for the fact that clang uses a different option to configure the
stack alignment than gcc (-mstack-alignment=N vs
-mpreferred-stack-boundary=N)

Matthias Kaehlcke (3):
  kbuild: Add cc-option-no-kbuild macro
  x86/build: Use cc-option-no-kbuild for boot code compiler options
  x86/build: Specify stack alignment for clang

 arch/x86/Makefile      | 25 +++++++++++++++++--------
 scripts/Kbuild.include |  5 +++++
 2 files changed, 22 insertions(+), 8 deletions(-)

-- 
2.13.1.508.gb3defc5cc-goog

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 1/3] kbuild: Add cc-option-no-kbuild macro
  2017-06-13  0:55 [PATCH 0/3] x86: stack alignment for boot code and clang Matthias Kaehlcke
@ 2017-06-13  0:55 ` Matthias Kaehlcke
  2017-06-13  7:49   ` Michal Marek
  2017-06-13  0:55 ` [PATCH 2/3] x86/build: Use cc-option-no-kbuild for boot code compiler options Matthias Kaehlcke
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 11+ messages in thread
From: Matthias Kaehlcke @ 2017-06-13  0:55 UTC (permalink / raw)
  To: Thomas Gleixner, Ingo Molnar, H . Peter Anvin, H . J . Lu,
	David Woodhouse, Masahiro Yamada, Michal Marek
  Cc: x86, linux-kbuild, linux-kernel, Michael Davidson, Greg Hackmann,
	Nick Desaulniers, Stephen Hines, Kees Cook, Arnd Bergmann,
	Bernhard.Rosenkranzer, Peter Foley, Behan Webster,
	Douglas Anderson, Matthias Kaehlcke

cc-option uses KBUILD_CFLAGS and KBUILD_CPPFLAGS when it determines
whether an option is supported or not. This is fine for options used to
build the kernel itself, however some components like the x86 boot code
use a different set of flags.

Add the new macro cc-option-no-kbuild which does the same as cc-option
except that it has an additional parameter with the compiler options
which are used instead of KBUILD_CFLAGS and KBUILD_CPPFLAGS.

Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
---
 scripts/Kbuild.include | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/scripts/Kbuild.include b/scripts/Kbuild.include
index 61f87a99bf0a..d9fdc740105f 100644
--- a/scripts/Kbuild.include
+++ b/scripts/Kbuild.include
@@ -128,6 +128,11 @@ cc-option-yn = $(call try-run,\
 cc-option-align = $(subst -functions=0,,\
 	$(call cc-option,-falign-functions=0,-malign-functions=0))
 
+# cc-option-no-kbuild
+# Usage: cflags-no-kbuild-y += $(call cc-option-no-kbuild,<other flags>,-march=winchip-c6,-march=i586)
+cc-option-no-kbuild = $(call try-run,\
+	$(CC) -Werror $(filter-out $(GCC_PLUGINS_CFLAGS),$(1)) $(2) -c -x c /dev/null -o "$$TMP",$(2),$(3))
+
 # cc-disable-warning
 # Usage: cflags-y += $(call cc-disable-warning,unused-but-set-variable)
 cc-disable-warning = $(call try-run,\
-- 
2.13.1.508.gb3defc5cc-goog

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 2/3] x86/build: Use cc-option-no-kbuild for boot code compiler options
  2017-06-13  0:55 [PATCH 0/3] x86: stack alignment for boot code and clang Matthias Kaehlcke
  2017-06-13  0:55 ` [PATCH 1/3] kbuild: Add cc-option-no-kbuild macro Matthias Kaehlcke
@ 2017-06-13  0:55 ` Matthias Kaehlcke
  2017-06-13  0:55 ` [PATCH 3/3] x86/build: Specify stack alignment for clang Matthias Kaehlcke
  2017-06-13  6:39 ` [PATCH 0/3] x86: stack alignment for boot code and clang Ingo Molnar
  3 siblings, 0 replies; 11+ messages in thread
From: Matthias Kaehlcke @ 2017-06-13  0:55 UTC (permalink / raw)
  To: Thomas Gleixner, Ingo Molnar, H . Peter Anvin, H . J . Lu,
	David Woodhouse, Masahiro Yamada, Michal Marek
  Cc: x86, linux-kbuild, linux-kernel, Michael Davidson, Greg Hackmann,
	Nick Desaulniers, Stephen Hines, Kees Cook, Arnd Bergmann,
	Bernhard.Rosenkranzer, Peter Foley, Behan Webster,
	Douglas Anderson, Matthias Kaehlcke

cc-option uses KBUILD_CFLAGS and KBUILD_CPPFLAGS when checking if a
compiler option is available. These flags aren't used to build the boot
code, in consequence cc-option can yield wrong results. For example
-mpreferred-stack-boundary=2 is never set with a 64 bit compiler,
since the setting is only valid for 16 and 32 bit binaries. This
is also the case for 32 bit kernel builds, because the option -m32 is
added to KBUILD_CFLAGS after the assignment of REALMODE_CFLAGS.

Use cc-option-no-kbuild instead of cc-option for the boot mode options.
The macro receives the compiler options as parameter instead of using
KBUILD_C*FLAGS. For the boot code we pass REALMODE_CFLAGS.

Also use separate statements for the cc-option-no-kbuild checks instead
of performing them in the initial assignment of REALMODE_CFLAGS since
the variable is an input of the macro.

Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
---
 arch/x86/Makefile | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/arch/x86/Makefile b/arch/x86/Makefile
index bf240b920473..86b725d69423 100644
--- a/arch/x86/Makefile
+++ b/arch/x86/Makefile
@@ -24,10 +24,11 @@ REALMODE_CFLAGS	:= $(M16_CFLAGS) -g -Os -D__KERNEL__ \
 		   -DDISABLE_BRANCH_PROFILING \
 		   -Wall -Wstrict-prototypes -march=i386 -mregparm=3 \
 		   -fno-strict-aliasing -fomit-frame-pointer -fno-pic \
-		   -mno-mmx -mno-sse \
-		   $(call cc-option, -ffreestanding) \
-		   $(call cc-option, -fno-stack-protector) \
-		   $(call cc-option, -mpreferred-stack-boundary=2)
+		   -mno-mmx -mno-sse
+
+REALMODE_CFLAGS += $(call cc-option-no-kbuild, $(REALMODE_CFLAGS), -ffreestanding)
+REALMODE_CFLAGS += $(call cc-option-no-kbuild, $(REALMODE_CFLAGS), -fno-stack-protector)
+REALMODE_CFLAGS += $(call cc-option-no-kbuild, $(REALMODE_CFLAGS), -mpreferred-stack-boundary=2)
 export REALMODE_CFLAGS
 
 # BITS is used as extension for files which are available in a 32 bit
-- 
2.13.1.508.gb3defc5cc-goog

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 3/3] x86/build: Specify stack alignment for clang
  2017-06-13  0:55 [PATCH 0/3] x86: stack alignment for boot code and clang Matthias Kaehlcke
  2017-06-13  0:55 ` [PATCH 1/3] kbuild: Add cc-option-no-kbuild macro Matthias Kaehlcke
  2017-06-13  0:55 ` [PATCH 2/3] x86/build: Use cc-option-no-kbuild for boot code compiler options Matthias Kaehlcke
@ 2017-06-13  0:55 ` Matthias Kaehlcke
  2017-06-13  6:38   ` Ingo Molnar
  2017-06-13  6:39 ` [PATCH 0/3] x86: stack alignment for boot code and clang Ingo Molnar
  3 siblings, 1 reply; 11+ messages in thread
From: Matthias Kaehlcke @ 2017-06-13  0:55 UTC (permalink / raw)
  To: Thomas Gleixner, Ingo Molnar, H . Peter Anvin, H . J . Lu,
	David Woodhouse, Masahiro Yamada, Michal Marek
  Cc: x86, linux-kbuild, linux-kernel, Michael Davidson, Greg Hackmann,
	Nick Desaulniers, Stephen Hines, Kees Cook, Arnd Bergmann,
	Bernhard.Rosenkranzer, Peter Foley, Behan Webster,
	Douglas Anderson, Matthias Kaehlcke

For gcc stack alignment is configured with -mpreferred-stack-boundary=N,
clang has the option -mstack-alignment=N for that purpose. Use the same
alignment as for gcc.

If the alignment is not specified clang assumes an alignment of
16 bytes, as required by the standard ABI. However as mentioned in
d9b0cde91c60 ("x86-64, gcc: Use -mpreferred-stack-boundary=3 if
supported") the standard kernel entry on x86-64 leaves the stack
on an 8-byte boundary, as a consequence clang will keep the stack
misaligned.

Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
---
 arch/x86/Makefile | 18 +++++++++++++-----
 1 file changed, 13 insertions(+), 5 deletions(-)

diff --git a/arch/x86/Makefile b/arch/x86/Makefile
index 86b725d69423..7f6c33f4d428 100644
--- a/arch/x86/Makefile
+++ b/arch/x86/Makefile
@@ -11,6 +11,14 @@ else
         KBUILD_DEFCONFIG := $(ARCH)_defconfig
 endif
 
+# Handle different option names for specifying stack alignment with gcc and
+# clang.
+ifeq ($(cc-name),clang)
+	stack_align_opt := -mstack-alignment
+else
+	stack_align_opt := -mpreferred-stack-boundary
+endif
+
 # How to compile the 16-bit code.  Note we always compile for -march=i386;
 # that way we can complain to the user if the CPU is insufficient.
 #
@@ -28,7 +36,7 @@ REALMODE_CFLAGS	:= $(M16_CFLAGS) -g -Os -D__KERNEL__ \
 
 REALMODE_CFLAGS += $(call cc-option-no-kbuild, $(REALMODE_CFLAGS), -ffreestanding)
 REALMODE_CFLAGS += $(call cc-option-no-kbuild, $(REALMODE_CFLAGS), -fno-stack-protector)
-REALMODE_CFLAGS += $(call cc-option-no-kbuild, $(REALMODE_CFLAGS), -mpreferred-stack-boundary=2)
+REALMODE_CFLAGS += $(call cc-option-no-kbuild, $(REALMODE_CFLAGS), $(stack_align_opt)=2)
 export REALMODE_CFLAGS
 
 # BITS is used as extension for files which are available in a 32 bit
@@ -65,8 +73,8 @@ ifeq ($(CONFIG_X86_32),y)
         # with nonstandard options
         KBUILD_CFLAGS += -fno-pic
 
-        # prevent gcc from keeping the stack 16 byte aligned
-        KBUILD_CFLAGS += $(call cc-option,-mpreferred-stack-boundary=2)
+        # prevent the compiler from keeping the stack 16 byte aligned
+        KBUILD_CFLAGS += $(call cc-option,$(stack_align_opt)=2)
 
         # Disable unit-at-a-time mode on pre-gcc-4.0 compilers, it makes gcc use
         # a lot more stack due to the lack of sharing of stacklots:
@@ -98,8 +106,8 @@ else
         KBUILD_CFLAGS += $(call cc-option,-mno-80387)
         KBUILD_CFLAGS += $(call cc-option,-mno-fp-ret-in-387)
 
-	# Use -mpreferred-stack-boundary=3 if supported.
-	KBUILD_CFLAGS += $(call cc-option,-mpreferred-stack-boundary=3)
+	# Align the stack to 8 bytes if supported.
+	KBUILD_CFLAGS += $(call cc-option,$(stack_align_opt)=3)
 
 	# Use -mskip-rax-setup if supported.
 	KBUILD_CFLAGS += $(call cc-option,-mskip-rax-setup)
-- 
2.13.1.508.gb3defc5cc-goog

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH 3/3] x86/build: Specify stack alignment for clang
  2017-06-13  0:55 ` [PATCH 3/3] x86/build: Specify stack alignment for clang Matthias Kaehlcke
@ 2017-06-13  6:38   ` Ingo Molnar
  2017-06-13 17:25     ` Matthias Kaehlcke
  0 siblings, 1 reply; 11+ messages in thread
From: Ingo Molnar @ 2017-06-13  6:38 UTC (permalink / raw)
  To: Matthias Kaehlcke
  Cc: Thomas Gleixner, Ingo Molnar, H . Peter Anvin, H . J . Lu,
	David Woodhouse, Masahiro Yamada, Michal Marek, x86,
	linux-kbuild, linux-kernel, Michael Davidson, Greg Hackmann,
	Nick Desaulniers, Stephen Hines, Kees Cook, Arnd Bergmann,
	Bernhard.Rosenkranzer, Peter Foley, Behan Webster,
	Douglas Anderson


* Matthias Kaehlcke <mka@chromium.org> wrote:

> For gcc stack alignment is configured with -mpreferred-stack-boundary=N,
> clang has the option -mstack-alignment=N for that purpose. Use the same
> alignment as for gcc.
> 
> If the alignment is not specified clang assumes an alignment of
> 16 bytes, as required by the standard ABI. However as mentioned in
> d9b0cde91c60 ("x86-64, gcc: Use -mpreferred-stack-boundary=3 if
> supported") the standard kernel entry on x86-64 leaves the stack
> on an 8-byte boundary, as a consequence clang will keep the stack
> misaligned.
> 
> Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
> ---
>  arch/x86/Makefile | 18 +++++++++++++-----
>  1 file changed, 13 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/x86/Makefile b/arch/x86/Makefile
> index 86b725d69423..7f6c33f4d428 100644
> --- a/arch/x86/Makefile
> +++ b/arch/x86/Makefile
> @@ -11,6 +11,14 @@ else
>          KBUILD_DEFCONFIG := $(ARCH)_defconfig
>  endif
>  
> +# Handle different option names for specifying stack alignment with gcc and
> +# clang.
> +ifeq ($(cc-name),clang)
> +	stack_align_opt := -mstack-alignment
> +else
> +	stack_align_opt := -mpreferred-stack-boundary
> +endif

Nit: I'd name it cc_stack_align_opt or so, to make it clear this is a C compiler 
option.

> @@ -65,8 +73,8 @@ ifeq ($(CONFIG_X86_32),y)
>          # with nonstandard options
>          KBUILD_CFLAGS += -fno-pic
>  
> -        # prevent gcc from keeping the stack 16 byte aligned
> -        KBUILD_CFLAGS += $(call cc-option,-mpreferred-stack-boundary=2)
> +        # prevent the compiler from keeping the stack 16 byte aligned
> +        KBUILD_CFLAGS += $(call cc-option,$(stack_align_opt)=2)

So the comment appears inaccurate: the point isn't really to 'keep' the compiler 
from 16-byte alignment (there's nothing wrong with that, functionally), the point 
is to use a more optimal alignment, right?

>  
>          # Disable unit-at-a-time mode on pre-gcc-4.0 compilers, it makes gcc use
>          # a lot more stack due to the lack of sharing of stacklots:
> @@ -98,8 +106,8 @@ else
>          KBUILD_CFLAGS += $(call cc-option,-mno-80387)
>          KBUILD_CFLAGS += $(call cc-option,-mno-fp-ret-in-387)
>  
> -	# Use -mpreferred-stack-boundary=3 if supported.
> -	KBUILD_CFLAGS += $(call cc-option,-mpreferred-stack-boundary=3)
> +	# Align the stack to 8 bytes if supported.
> +	KBUILD_CFLAGS += $(call cc-option,$(stack_align_opt)=3)

Here too the reason should be outlined: performance, features or correctness?

Thanks,

	Ingo

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 0/3] x86: stack alignment for boot code and clang
  2017-06-13  0:55 [PATCH 0/3] x86: stack alignment for boot code and clang Matthias Kaehlcke
                   ` (2 preceding siblings ...)
  2017-06-13  0:55 ` [PATCH 3/3] x86/build: Specify stack alignment for clang Matthias Kaehlcke
@ 2017-06-13  6:39 ` Ingo Molnar
  3 siblings, 0 replies; 11+ messages in thread
From: Ingo Molnar @ 2017-06-13  6:39 UTC (permalink / raw)
  To: Matthias Kaehlcke
  Cc: Thomas Gleixner, Ingo Molnar, H . Peter Anvin, H . J . Lu,
	David Woodhouse, Masahiro Yamada, Michal Marek, x86,
	linux-kbuild, linux-kernel, Michael Davidson, Greg Hackmann,
	Nick Desaulniers, Stephen Hines, Kees Cook, Arnd Bergmann,
	Bernhard.Rosenkranzer, Peter Foley, Behan Webster,
	Douglas Anderson


* Matthias Kaehlcke <mka@chromium.org> wrote:

> This series fixes an issue with the stack of the x86 boot code not
> being aligned as intended. Further it adapts the Makefile to account
> for the fact that clang uses a different option to configure the
> stack alignment than gcc (-mstack-alignment=N vs
> -mpreferred-stack-boundary=N)
> 
> Matthias Kaehlcke (3):
>   kbuild: Add cc-option-no-kbuild macro
>   x86/build: Use cc-option-no-kbuild for boot code compiler options
>   x86/build: Specify stack alignment for clang
> 
>  arch/x86/Makefile      | 25 +++++++++++++++++--------
>  scripts/Kbuild.include |  5 +++++
>  2 files changed, 22 insertions(+), 8 deletions(-)

Looks good to me, modulo the minor comments I just made to the #3 patch.

This needs acks from the kbuild people before I can apply the fixes.

Thanks,

	Ingo

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/3] kbuild: Add cc-option-no-kbuild macro
  2017-06-13  0:55 ` [PATCH 1/3] kbuild: Add cc-option-no-kbuild macro Matthias Kaehlcke
@ 2017-06-13  7:49   ` Michal Marek
  2017-06-13  8:31     ` Arnd Bergmann
  0 siblings, 1 reply; 11+ messages in thread
From: Michal Marek @ 2017-06-13  7:49 UTC (permalink / raw)
  To: Matthias Kaehlcke
  Cc: Thomas Gleixner, Ingo Molnar, H . Peter Anvin, H . J . Lu,
	David Woodhouse, Masahiro Yamada, x86, linux-kbuild,
	linux-kernel, Michael Davidson, Greg Hackmann, Nick Desaulniers,
	Stephen Hines, Kees Cook, Arnd Bergmann, Bernhard.Rosenkranzer,
	Peter Foley, Behan Webster, Douglas Anderson

On 2017-06-13 02:55, Matthias Kaehlcke wrote:
> cc-option uses KBUILD_CFLAGS and KBUILD_CPPFLAGS when it determines
> whether an option is supported or not. This is fine for options used to
> build the kernel itself, however some components like the x86 boot code
> use a different set of flags.
> 
> Add the new macro cc-option-no-kbuild which does the same as cc-option
> except that it has an additional parameter with the compiler options
> which are used instead of KBUILD_CFLAGS and KBUILD_CPPFLAGS.
> 
> Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
> ---
>  scripts/Kbuild.include | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/scripts/Kbuild.include b/scripts/Kbuild.include
> index 61f87a99bf0a..d9fdc740105f 100644
> --- a/scripts/Kbuild.include
> +++ b/scripts/Kbuild.include
> @@ -128,6 +128,11 @@ cc-option-yn = $(call try-run,\
>  cc-option-align = $(subst -functions=0,,\
>  	$(call cc-option,-falign-functions=0,-malign-functions=0))
>  
> +# cc-option-no-kbuild
> +# Usage: cflags-no-kbuild-y += $(call cc-option-no-kbuild,<other flags>,-march=winchip-c6,-march=i586)
> +cc-option-no-kbuild = $(call try-run,\
> +	$(CC) -Werror $(filter-out $(GCC_PLUGINS_CFLAGS),$(1)) $(2) -c -x c /dev/null -o "$$TMP",$(2),$(3))

As this is a version of cc-option with an extrra argument, how about
implementing cc-option as a shorthand for cc-option-no-kbuild? It would
make it more obvious what cc-option-no-kbuild does differently (it's
probably just me, but I was unable to infer the semantics from its name).

Michal

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/3] kbuild: Add cc-option-no-kbuild macro
  2017-06-13  7:49   ` Michal Marek
@ 2017-06-13  8:31     ` Arnd Bergmann
  2017-06-13 10:13       ` Masahiro Yamada
  0 siblings, 1 reply; 11+ messages in thread
From: Arnd Bergmann @ 2017-06-13  8:31 UTC (permalink / raw)
  To: Michal Marek
  Cc: Matthias Kaehlcke, Thomas Gleixner, Ingo Molnar, H . Peter Anvin,
	H . J . Lu, David Woodhouse, Masahiro Yamada,
	the arch/x86 maintainers, Linux Kbuild mailing list,
	Linux Kernel Mailing List, Michael Davidson, Greg Hackmann,
	Nick Desaulniers, Stephen Hines, Kees Cook,
	Bernhard.Rosenkranzer, Peter Foley, Behan Webster,
	Douglas Anderson

On Tue, Jun 13, 2017 at 9:49 AM, Michal Marek <mmarek@suse.com> wrote:
> On 2017-06-13 02:55, Matthias Kaehlcke wrote:
>> cc-option uses KBUILD_CFLAGS and KBUILD_CPPFLAGS when it determines
>> whether an option is supported or not. This is fine for options used to
>> build the kernel itself, however some components like the x86 boot code
>> use a different set of flags.
>>
>> Add the new macro cc-option-no-kbuild which does the same as cc-option
>> except that it has an additional parameter with the compiler options
>> which are used instead of KBUILD_CFLAGS and KBUILD_CPPFLAGS.
>>
>> Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
>> ---
>>  scripts/Kbuild.include | 5 +++++
>>  1 file changed, 5 insertions(+)
>>
>> diff --git a/scripts/Kbuild.include b/scripts/Kbuild.include
>> index 61f87a99bf0a..d9fdc740105f 100644
>> --- a/scripts/Kbuild.include
>> +++ b/scripts/Kbuild.include
>> @@ -128,6 +128,11 @@ cc-option-yn = $(call try-run,\
>>  cc-option-align = $(subst -functions=0,,\
>>       $(call cc-option,-falign-functions=0,-malign-functions=0))
>>
>> +# cc-option-no-kbuild
>> +# Usage: cflags-no-kbuild-y += $(call cc-option-no-kbuild,<other flags>,-march=winchip-c6,-march=i586)
>> +cc-option-no-kbuild = $(call try-run,\
>> +     $(CC) -Werror $(filter-out $(GCC_PLUGINS_CFLAGS),$(1)) $(2) -c -x c /dev/null -o "$$TMP",$(2),$(3))
>
> As this is a version of cc-option with an extrra argument, how about
> implementing cc-option as a shorthand for cc-option-no-kbuild? It would
> make it more obvious what cc-option-no-kbuild does differently (it's
> probably just me, but I was unable to infer the semantics from its name).

Agreed, also the hostcc-option could be based on the same I think, if we
also make the $(CC) an argument of the low-level helper.

For reference, these seem to be the files that might be affected here,
as they override KBUILD_CFLAGS:

$ git grep -w KBUILD_CFLAGS.*:
arch/cris/Makefile:KBUILD_CFLAGS := $(subst
-fomit-frame-pointer,,$(KBUILD_CFLAGS)) -g
arch/ia64/Makefile:KBUILD_CFLAGS_KERNEL := -mconstant-gp
arch/m68k/Makefile:KBUILD_CFLAGS := $(subst
-fomit-frame-pointer,,$(KBUILD_CFLAGS)) -g
arch/mips/boot/compressed/Makefile:KBUILD_CFLAGS := $(filter-out -pg,
$(KBUILD_CFLAGS))
arch/mips/boot/compressed/Makefile:KBUILD_CFLAGS := $(filter-out
-fstack-protector, $(KBUILD_CFLAGS))
arch/mips/boot/compressed/Makefile:KBUILD_CFLAGS := $(KBUILD_CFLAGS)
-D__KERNEL__ \
arch/mips/vdso/Makefile:$(obj-vdso): KBUILD_CFLAGS := $(cflags-vdso)
$(native-abi)
arch/mips/vdso/Makefile:$(obj-vdso-o32): KBUILD_CFLAGS :=
$(cflags-vdso) -mabi=32
arch/mips/vdso/Makefile:$(obj-vdso-n32): KBUILD_CFLAGS :=
$(cflags-vdso) -mabi=n32
arch/s390/boot/compressed/Makefile:KBUILD_CFLAGS := -m64 -D__KERNEL__ -O2
arch/s390/kernel/vdso32/Makefile:KBUILD_CFLAGS_31 := $(filter-out
-m64,$(KBUILD_CFLAGS))
arch/s390/kernel/vdso64/Makefile:KBUILD_CFLAGS_64 := $(filter-out
-m64,$(KBUILD_CFLAGS))
arch/tile/kernel/vdso/Makefile:KBUILD_CFLAGS_32 := $(filter-out
-m64,$(KBUILD_CFLAGS))
arch/x86/boot/Makefile:KBUILD_CFLAGS    := $(REALMODE_CFLAGS) -D_SETUP
arch/x86/boot/compressed/Makefile:KBUILD_CFLAGS := -m$(BITS) -D__KERNEL__ -O2
arch/x86/entry/vdso/Makefile:$(vobjs): KBUILD_CFLAGS := $(filter-out
$(GCC_PLUGINS_CFLAGS),$(KBUILD_CFLAGS)) $(CFL)
arch/x86/entry/vdso/Makefile:KBUILD_CFLAGS_32 := $(filter-out
-m64,$(KBUILD_CFLAGS))
arch/x86/entry/vdso/Makefile:KBUILD_CFLAGS_32 := $(filter-out
-mcmodel=kernel,$(KBUILD_CFLAGS_32))
arch/x86/entry/vdso/Makefile:KBUILD_CFLAGS_32 := $(filter-out
-fno-pic,$(KBUILD_CFLAGS_32))
arch/x86/entry/vdso/Makefile:KBUILD_CFLAGS_32 := $(filter-out
-mfentry,$(KBUILD_CFLAGS_32))
arch/x86/entry/vdso/Makefile:KBUILD_CFLAGS_32 := $(filter-out
$(GCC_PLUGINS_CFLAGS),$(KBUILD_CFLAGS_32))
arch/x86/purgatory/Makefile:KBUILD_CFLAGS := -fno-strict-aliasing
-Wall -Wstrict-prototypes -fno-zero-initialized-in-bss -fno-builtin
-ffreestanding -c -MD -Os -mcmodel=large
arch/x86/realmode/rm/Makefile:KBUILD_CFLAGS     := $(REALMODE_CFLAGS)
-D_SETUP -D_WAKEUP \
drivers/firmware/efi/libstub/Makefile:KBUILD_CFLAGS
 := $(cflags-y) -DDISABLE_BRANCH_PROFILING \

        Arnd

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/3] kbuild: Add cc-option-no-kbuild macro
  2017-06-13  8:31     ` Arnd Bergmann
@ 2017-06-13 10:13       ` Masahiro Yamada
  2017-06-13 16:45         ` Matthias Kaehlcke
  0 siblings, 1 reply; 11+ messages in thread
From: Masahiro Yamada @ 2017-06-13 10:13 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Michal Marek, Matthias Kaehlcke, Thomas Gleixner, Ingo Molnar,
	H . Peter Anvin, H . J . Lu, David Woodhouse,
	the arch/x86 maintainers, Linux Kbuild mailing list,
	Linux Kernel Mailing List, Michael Davidson, Greg Hackmann,
	Nick Desaulniers, Stephen Hines, Kees Cook,
	Bernhard.Rosenkranzer, Peter Foley, Behan Webster,
	Douglas Anderson

2017-06-13 17:31 GMT+09:00 Arnd Bergmann <arnd@arndb.de>:
> On Tue, Jun 13, 2017 at 9:49 AM, Michal Marek <mmarek@suse.com> wrote:
>> On 2017-06-13 02:55, Matthias Kaehlcke wrote:
>>> cc-option uses KBUILD_CFLAGS and KBUILD_CPPFLAGS when it determines
>>> whether an option is supported or not. This is fine for options used to
>>> build the kernel itself, however some components like the x86 boot code
>>> use a different set of flags.
>>>
>>> Add the new macro cc-option-no-kbuild which does the same as cc-option
>>> except that it has an additional parameter with the compiler options
>>> which are used instead of KBUILD_CFLAGS and KBUILD_CPPFLAGS.
>>>
>>> Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
>>> ---
>>>  scripts/Kbuild.include | 5 +++++
>>>  1 file changed, 5 insertions(+)
>>>
>>> diff --git a/scripts/Kbuild.include b/scripts/Kbuild.include
>>> index 61f87a99bf0a..d9fdc740105f 100644
>>> --- a/scripts/Kbuild.include
>>> +++ b/scripts/Kbuild.include
>>> @@ -128,6 +128,11 @@ cc-option-yn = $(call try-run,\
>>>  cc-option-align = $(subst -functions=0,,\
>>>       $(call cc-option,-falign-functions=0,-malign-functions=0))
>>>
>>> +# cc-option-no-kbuild
>>> +# Usage: cflags-no-kbuild-y += $(call cc-option-no-kbuild,<other flags>,-march=winchip-c6,-march=i586)
>>> +cc-option-no-kbuild = $(call try-run,\
>>> +     $(CC) -Werror $(filter-out $(GCC_PLUGINS_CFLAGS),$(1)) $(2) -c -x c /dev/null -o "$$TMP",$(2),$(3))
>>
>> As this is a version of cc-option with an extrra argument, how about
>> implementing cc-option as a shorthand for cc-option-no-kbuild? It would
>> make it more obvious what cc-option-no-kbuild does differently (it's
>> probably just me, but I was unable to infer the semantics from its name).
>
> Agreed, also the hostcc-option could be based on the same I think, if we
> also make the $(CC) an argument of the low-level helper.
>

Agree.  One possible implementation:

cc-option-raw = $(call try-run,\
             $(1) -Werror $(2) $(3) -c -x c /dev/null -o "$$TMP",$(3),$(4))

cc-option = $(call cc-option-raw, $(CC), $(KBUILD_CPPFLAGS)
$(CC_OPTION_CFLAGS),\
                                                                $(1), $(2))

This will allow us to do:
hostcc-option = $(call cc-option-raw, $(HOSTCC), $(HOSTCFLAGS), $(1), $(2))



Suggestion for a better name is welcome...


-- 
Best Regards
Masahiro Yamada

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/3] kbuild: Add cc-option-no-kbuild macro
  2017-06-13 10:13       ` Masahiro Yamada
@ 2017-06-13 16:45         ` Matthias Kaehlcke
  0 siblings, 0 replies; 11+ messages in thread
From: Matthias Kaehlcke @ 2017-06-13 16:45 UTC (permalink / raw)
  To: Masahiro Yamada
  Cc: Arnd Bergmann, Michal Marek, Thomas Gleixner, Ingo Molnar,
	H . Peter Anvin, H . J . Lu, David Woodhouse,
	the arch/x86 maintainers, Linux Kbuild mailing list,
	Linux Kernel Mailing List, Michael Davidson, Greg Hackmann,
	Nick Desaulniers, Stephen Hines, Kees Cook,
	Bernhard.Rosenkranzer, Peter Foley, Behan Webster,
	Douglas Anderson

El Tue, Jun 13, 2017 at 07:13:55PM +0900 Masahiro Yamada ha dit:

> 2017-06-13 17:31 GMT+09:00 Arnd Bergmann <arnd@arndb.de>:
> > On Tue, Jun 13, 2017 at 9:49 AM, Michal Marek <mmarek@suse.com> wrote:
> >> On 2017-06-13 02:55, Matthias Kaehlcke wrote:
> >>> cc-option uses KBUILD_CFLAGS and KBUILD_CPPFLAGS when it determines
> >>> whether an option is supported or not. This is fine for options used to
> >>> build the kernel itself, however some components like the x86 boot code
> >>> use a different set of flags.
> >>>
> >>> Add the new macro cc-option-no-kbuild which does the same as cc-option
> >>> except that it has an additional parameter with the compiler options
> >>> which are used instead of KBUILD_CFLAGS and KBUILD_CPPFLAGS.
> >>>
> >>> Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
> >>> ---
> >>>  scripts/Kbuild.include | 5 +++++
> >>>  1 file changed, 5 insertions(+)
> >>>
> >>> diff --git a/scripts/Kbuild.include b/scripts/Kbuild.include
> >>> index 61f87a99bf0a..d9fdc740105f 100644
> >>> --- a/scripts/Kbuild.include
> >>> +++ b/scripts/Kbuild.include
> >>> @@ -128,6 +128,11 @@ cc-option-yn = $(call try-run,\
> >>>  cc-option-align = $(subst -functions=0,,\
> >>>       $(call cc-option,-falign-functions=0,-malign-functions=0))
> >>>
> >>> +# cc-option-no-kbuild
> >>> +# Usage: cflags-no-kbuild-y += $(call cc-option-no-kbuild,<other flags>,-march=winchip-c6,-march=i586)
> >>> +cc-option-no-kbuild = $(call try-run,\
> >>> +     $(CC) -Werror $(filter-out $(GCC_PLUGINS_CFLAGS),$(1)) $(2) -c -x c /dev/null -o "$$TMP",$(2),$(3))
> >>
> >> As this is a version of cc-option with an extrra argument, how about
> >> implementing cc-option as a shorthand for cc-option-no-kbuild? It would
> >> make it more obvious what cc-option-no-kbuild does differently (it's
> >> probably just me, but I was unable to infer the semantics from its name).
> >
> > Agreed, also the hostcc-option could be based on the same I think, if we
> > also make the $(CC) an argument of the low-level helper.
> >
> 
> Agree.  One possible implementation:
> 
> cc-option-raw = $(call try-run,\
>              $(1) -Werror $(2) $(3) -c -x c /dev/null -o "$$TMP",$(3),$(4))
> 
> cc-option = $(call cc-option-raw, $(CC), $(KBUILD_CPPFLAGS)
> $(CC_OPTION_CFLAGS),\
>                                                                 $(1), $(2))
> 
> This will allow us to do:
> hostcc-option = $(call cc-option-raw, $(HOSTCC), $(HOSTCFLAGS), $(1), $(2))

Looks good, thanks all for the suggestions.

> Suggestion for a better name is welcome...

Yeah, this tends to be the difficult part, I didn't like
the initial 'cc-option-no-kbuild' either ... 'cc-option-raw' seems
ok if nothing better pops up.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 3/3] x86/build: Specify stack alignment for clang
  2017-06-13  6:38   ` Ingo Molnar
@ 2017-06-13 17:25     ` Matthias Kaehlcke
  0 siblings, 0 replies; 11+ messages in thread
From: Matthias Kaehlcke @ 2017-06-13 17:25 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: Thomas Gleixner, Ingo Molnar, H . Peter Anvin, H . J . Lu,
	David Woodhouse, Masahiro Yamada, Michal Marek, x86,
	linux-kbuild, linux-kernel, Michael Davidson, Greg Hackmann,
	Nick Desaulniers, Stephen Hines, Kees Cook, Arnd Bergmann,
	Bernhard.Rosenkranzer, Peter Foley, Behan Webster,
	Douglas Anderson

El Tue, Jun 13, 2017 at 08:38:00AM +0200 Ingo Molnar ha dit:

> 
> * Matthias Kaehlcke <mka@chromium.org> wrote:
> 
> > For gcc stack alignment is configured with -mpreferred-stack-boundary=N,
> > clang has the option -mstack-alignment=N for that purpose. Use the same
> > alignment as for gcc.
> > 
> > If the alignment is not specified clang assumes an alignment of
> > 16 bytes, as required by the standard ABI. However as mentioned in
> > d9b0cde91c60 ("x86-64, gcc: Use -mpreferred-stack-boundary=3 if
> > supported") the standard kernel entry on x86-64 leaves the stack
> > on an 8-byte boundary, as a consequence clang will keep the stack
> > misaligned.
> > 
> > Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
> > ---
> >  arch/x86/Makefile | 18 +++++++++++++-----
> >  1 file changed, 13 insertions(+), 5 deletions(-)
> > 
> > diff --git a/arch/x86/Makefile b/arch/x86/Makefile
> > index 86b725d69423..7f6c33f4d428 100644
> > --- a/arch/x86/Makefile
> > +++ b/arch/x86/Makefile
> > @@ -11,6 +11,14 @@ else
> >          KBUILD_DEFCONFIG := $(ARCH)_defconfig
> >  endif
> >  
> > +# Handle different option names for specifying stack alignment with gcc and
> > +# clang.
> > +ifeq ($(cc-name),clang)
> > +	stack_align_opt := -mstack-alignment
> > +else
> > +	stack_align_opt := -mpreferred-stack-boundary
> > +endif
> 
> Nit: I'd name it cc_stack_align_opt or so, to make it clear this is a C compiler 
> option.

Sounds good

> > @@ -65,8 +73,8 @@ ifeq ($(CONFIG_X86_32),y)
> >          # with nonstandard options
> >          KBUILD_CFLAGS += -fno-pic
> >  
> > -        # prevent gcc from keeping the stack 16 byte aligned
> > -        KBUILD_CFLAGS += $(call cc-option,-mpreferred-stack-boundary=2)
> > +        # prevent the compiler from keeping the stack 16 byte aligned
> > +        KBUILD_CFLAGS += $(call cc-option,$(stack_align_opt)=2)
> 
> So the comment appears inaccurate: the point isn't really to 'keep' the compiler 
> from 16-byte alignment (there's nothing wrong with that, functionally), the point 
> is to use a more optimal alignment, right?

I don't know for sure what is the reason for the 4 byte alignment,
since there is no history for the option being added (apparently it
was added in 1999 by Artur Skawina). I suppose it is an optimization
to save stack space and reduce code size by reducing the need for
extra alignment instructions. Please correct me if I'm wrong.

> >          # Disable unit-at-a-time mode on pre-gcc-4.0 compilers, it makes gcc use
> >          # a lot more stack due to the lack of sharing of stacklots:
> > @@ -98,8 +106,8 @@ else
> >          KBUILD_CFLAGS += $(call cc-option,-mno-80387)
> >          KBUILD_CFLAGS += $(call cc-option,-mno-fp-ret-in-387)
> >  
> > -	# Use -mpreferred-stack-boundary=3 if supported.
> > -	KBUILD_CFLAGS += $(call cc-option,-mpreferred-stack-boundary=3)
> > +	# Align the stack to 8 bytes if supported.
> > +	KBUILD_CFLAGS += $(call cc-option,$(stack_align_opt)=3)
> 
> Here too the reason should be outlined: performance, features or correctness?

Probably the same as for 32-bit, plus the stack being aligned in any
case at an 8-byte boundary by the kernel entry. If you have more
wisdom to add please let me know.

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2017-06-13 17:25 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-06-13  0:55 [PATCH 0/3] x86: stack alignment for boot code and clang Matthias Kaehlcke
2017-06-13  0:55 ` [PATCH 1/3] kbuild: Add cc-option-no-kbuild macro Matthias Kaehlcke
2017-06-13  7:49   ` Michal Marek
2017-06-13  8:31     ` Arnd Bergmann
2017-06-13 10:13       ` Masahiro Yamada
2017-06-13 16:45         ` Matthias Kaehlcke
2017-06-13  0:55 ` [PATCH 2/3] x86/build: Use cc-option-no-kbuild for boot code compiler options Matthias Kaehlcke
2017-06-13  0:55 ` [PATCH 3/3] x86/build: Specify stack alignment for clang Matthias Kaehlcke
2017-06-13  6:38   ` Ingo Molnar
2017-06-13 17:25     ` Matthias Kaehlcke
2017-06-13  6:39 ` [PATCH 0/3] x86: stack alignment for boot code and clang Ingo Molnar

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).