[PATCH v3 0/3] x86: stack alignment for boot code and clang

[PATCH v3 0/3] x86: stack alignment for boot code and clang

[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)

Collaterally the series adds the new kbuild macro __cc-option and
refactors the macros cc-option and hostcc-option to make use of
__cc-option.

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

 Makefile               |  2 +-
 arch/x86/Makefile      | 33 +++++++++++++++++++++++++--------
 scripts/Kbuild.include | 14 ++++++++++++--
 scripts/Makefile.host  |  6 ------
 4 files changed, 38 insertions(+), 17 deletions(-)

-- 
2.13.1.518.g3df882009-goog

[PATCH v3 1/3] kbuild: Add __cc-option macro

[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 which is a more generic version of
cc-option with additional parameters. One parameter is the compiler
with which the check should be performed, the other the compiler options
to be used instead KBUILD_C*FLAGS.

Refactor cc-option and hostcc-option to use __cc-option and move
hostcc-option to scripts/Kbuild.include.

Suggested-by: Arnd Bergmann <arnd@arndb.de>
Suggested-by: Masahiro Yamada <yamada.masahiro@socionext.com>
Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
---
Changes in v3:
- Changed the name of the new macro to __cc-option
- Removed existing hostcc-option macro from scripts/Makefile.host
- Added HOST_EXTRACFLAGS to hostcc-option
- Updated commit message

 Makefile               |  2 +-
 scripts/Kbuild.include | 14 ++++++++++++--
 scripts/Makefile.host  |  6 ------
 3 files changed, 13 insertions(+), 9 deletions(-)

diff --git a/Makefile b/Makefile
index 83f6d9972cab..b234bba6d652 100644
--- a/Makefile
+++ b/Makefile
@@ -303,7 +303,7 @@ CONFIG_SHELL := $(shell if [ -x "$$BASH" ]; then echo $$BASH; \
 
 HOSTCC       = gcc
 HOSTCXX      = g++
-HOSTCFLAGS   = -Wall -Wmissing-prototypes -Wstrict-prototypes -O2 -fomit-frame-pointer -std=gnu89
+HOSTCFLAGS   := -Wall -Wmissing-prototypes -Wstrict-prototypes -O2 -fomit-frame-pointer -std=gnu89
 HOSTCXXFLAGS = -O2
 
 ifeq ($(shell $(HOSTCC) -v 2>&1 | grep -c "clang version"), 1)
diff --git a/scripts/Kbuild.include b/scripts/Kbuild.include
index 61f87a99bf0a..5001bf9b6579 100644
--- a/scripts/Kbuild.include
+++ b/scripts/Kbuild.include
@@ -108,6 +108,11 @@ as-option = $(call try-run,\
 as-instr = $(call try-run,\
 	printf "%b\n" "$(1)" | $(CC) $(KBUILD_AFLAGS) -c -x assembler -o "$$TMP" -,$(2),$(3))
 
+# __cc-option
+# Usage: MY_CFLAGS += $(call __cc-option,$(CC),$(MY_CFLAGS),-march=winchip-c6,-march=i586)
+__cc-option = $(call try-run,\
+	$(1) -Werror $(2) $(3) -c -x c /dev/null -o "$$TMP",$(3),$(4))
+
 # Do not attempt to build with gcc plugins during cc-option tests.
 # (And this uses delayed resolution so the flags will be up to date.)
 CC_OPTION_CFLAGS = $(filter-out $(GCC_PLUGINS_CFLAGS),$(KBUILD_CFLAGS))
@@ -115,8 +120,13 @@ CC_OPTION_CFLAGS = $(filter-out $(GCC_PLUGINS_CFLAGS),$(KBUILD_CFLAGS))
 # cc-option
 # Usage: cflags-y += $(call cc-option,-march=winchip-c6,-march=i586)
 
-cc-option = $(call try-run,\
-	$(CC) -Werror $(KBUILD_CPPFLAGS) $(CC_OPTION_CFLAGS) $(1) -c -x c /dev/null -o "$$TMP",$(1),$(2))
+cc-option = $(call __cc-option, $(CC), $(KBUILD_CPPFLAGS) $(CC_OPTION_CFLAGS),\
+	$(1), $(2))
+
+# hostcc-option
+# Usage: cflags-y += $(call hostcc-option,-march=winchip-c6,-march=i586)
+hostcc-option = $(call __cc-option, $(HOSTCC),\
+	$(HOSTCFLAGS) $(HOST_EXTRACFLAGS), $(1), $(2))
 
 # cc-option-yn
 # Usage: flag := $(call cc-option-yn,-march=winchip-c6)
diff --git a/scripts/Makefile.host b/scripts/Makefile.host
index 45b5b1aaedbd..9cfd5c84d76f 100644
--- a/scripts/Makefile.host
+++ b/scripts/Makefile.host
@@ -20,12 +20,6 @@
 # Will compile qconf as a C++ program, and menu as a C program.
 # They are linked as C++ code to the executable qconf
 
-# hostcc-option
-# Usage: cflags-y += $(call hostcc-option,-march=winchip-c6,-march=i586)
-
-hostcc-option = $(call try-run,\
-	$(HOSTCC) $(HOSTCFLAGS) $(HOST_EXTRACFLAGS) $(1) -c -x c /dev/null -o "$$TMP",$(1),$(2))
-
 __hostprogs := $(sort $(hostprogs-y) $(hostprogs-m))
 host-cshlib := $(sort $(hostlibs-y) $(hostlibs-m))
 host-cxxshlib := $(sort $(hostcxxlibs-y) $(hostcxxlibs-m))
-- 
2.13.1.518.g3df882009-goog

Re: [PATCH v3 1/3] kbuild: Add __cc-option macro

[Masahiro Yamada]

2017-06-16 2:37 GMT+09:00 Matthias Kaehlcke <mka@chromium.org>:
> 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 which is a more generic version of
> cc-option with additional parameters. One parameter is the compiler
> with which the check should be performed, the other the compiler options
> to be used instead KBUILD_C*FLAGS.
>
> Refactor cc-option and hostcc-option to use __cc-option and move
> hostcc-option to scripts/Kbuild.include.
>
> Suggested-by: Arnd Bergmann <arnd@arndb.de>
> Suggested-by: Masahiro Yamada <yamada.masahiro@socionext.com>
> Signed-off-by: Matthias Kaehlcke <mka@chromium.org>


Acked-by: Masahiro Yamada <yamada.masahiro@socionext.com>



-- 
Best Regards
Masahiro Yamada

Re: [PATCH v3 1/3] kbuild: Add __cc-option macro

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 1965 bytes --]

Hi Matthias,

[auto build test ERROR on tip/auto-latest]
[also build test ERROR on v4.12-rc5 next-20170616]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Matthias-Kaehlcke/kbuild-Add-__cc-option-macro/20170616-082252
config: sh-sh7710voipgw_defconfig (attached as .config)
compiler: sh4-linux-gnu-gcc (Debian 6.1.1-9) 6.1.1 20160705
reproduce:
        wget https://raw.githubusercontent.com/01org/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=sh 

All errors (new ones prefixed by >>):

   {standard input}: Assembler messages:
>> {standard input}:672: Error: invalid operands for opcode
   {standard input}:673: Error: opcode not valid for this cpu variant
   {standard input}:674: Error: opcode not valid for this cpu variant
   {standard input}:675: Error: invalid operands for opcode
   {standard input}:1900: Error: invalid operands for opcode
   {standard input}:1901: Error: opcode not valid for this cpu variant
   {standard input}:1902: Error: opcode not valid for this cpu variant
   {standard input}:1903: Error: invalid operands for opcode
   {standard input}:2067: Error: invalid operands for opcode
   {standard input}:2068: Error: opcode not valid for this cpu variant
   {standard input}:2069: Error: opcode not valid for this cpu variant
   {standard input}:2070: Error: invalid operands for opcode
   {standard input}:2570: Error: invalid operands for opcode
   {standard input}:2571: Error: opcode not valid for this cpu variant
   {standard input}:2572: Error: opcode not valid for this cpu variant
   {standard input}:2573: Error: invalid operands for opcode

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 10115 bytes --]

Re: [PATCH v3 1/3] kbuild: Add __cc-option macro

[Masahiro Yamada]

Hi Matthias,


Sorry, the code I suggested introduced a slight behavior change.

With this patch, cc-option adds a space before the returned option.


Please see line 46 of arch/sh/Makefile.

ifeq ($(cflags-y),)


Now cflags-y contains a space, so
this conditional is now evaluated as false,
which was actually true without the patch.
This is the cause of the build error.


Could you remove whitespaces before $(1) and $(2) in cc-option?

Also, please fix hostcc-option as well.

Thanks!






2017-06-18 15:58 GMT+09:00 kbuild test robot <lkp@intel.com>:
> Hi Matthias,
>
> [auto build test ERROR on tip/auto-latest]
> [also build test ERROR on v4.12-rc5 next-20170616]
> [if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
>
> url:    https://github.com/0day-ci/linux/commits/Matthias-Kaehlcke/kbuild-Add-__cc-option-macro/20170616-082252
> config: sh-sh7710voipgw_defconfig (attached as .config)
> compiler: sh4-linux-gnu-gcc (Debian 6.1.1-9) 6.1.1 20160705
> reproduce:
>         wget https://raw.githubusercontent.com/01org/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
>         chmod +x ~/bin/make.cross
>         # save the attached .config to linux build tree
>         make.cross ARCH=sh
>
> All errors (new ones prefixed by >>):
>
>    {standard input}: Assembler messages:
>>> {standard input}:672: Error: invalid operands for opcode
>    {standard input}:673: Error: opcode not valid for this cpu variant
>    {standard input}:674: Error: opcode not valid for this cpu variant
>    {standard input}:675: Error: invalid operands for opcode
>    {standard input}:1900: Error: invalid operands for opcode
>    {standard input}:1901: Error: opcode not valid for this cpu variant
>    {standard input}:1902: Error: opcode not valid for this cpu variant
>    {standard input}:1903: Error: invalid operands for opcode
>    {standard input}:2067: Error: invalid operands for opcode
>    {standard input}:2068: Error: opcode not valid for this cpu variant
>    {standard input}:2069: Error: opcode not valid for this cpu variant
>    {standard input}:2070: Error: invalid operands for opcode
>    {standard input}:2570: Error: invalid operands for opcode
>    {standard input}:2571: Error: opcode not valid for this cpu variant
>    {standard input}:2572: Error: opcode not valid for this cpu variant
>    {standard input}:2573: Error: invalid operands for opcode
>
> ---
> 0-DAY kernel test infrastructure                Open Source Technology Center
> https://lists.01.org/pipermail/kbuild-all                   Intel Corporation



-- 
Best Regards
Masahiro Yamada

Re: [PATCH v3 1/3] kbuild: Add __cc-option macro

[Matthias Kaehlcke]

Hi Masahiro,

El Mon, Jun 19, 2017 at 11:00:42AM +0900 Masahiro Yamada ha dit:

> Sorry, the code I suggested introduced a slight behavior change.
> 
> With this patch, cc-option adds a space before the returned option.
> 
> 
> Please see line 46 of arch/sh/Makefile.
> 
> ifeq ($(cflags-y),)
> 
> 
> Now cflags-y contains a space, so
> this conditional is now evaluated as false,
> which was actually true without the patch.
> This is the cause of the build error.

Thanks for looking into this, I also didn't realize that the extra
space could be an issue.

> Could you remove whitespaces before $(1) and $(2) in cc-option?
> 
> Also, please fix hostcc-option as well.

Sure, I'll send out a new version soon.

> 2017-06-18 15:58 GMT+09:00 kbuild test robot <lkp@intel.com>:
> > Hi Matthias,
> >
> > [auto build test ERROR on tip/auto-latest]
> > [also build test ERROR on v4.12-rc5 next-20170616]
> > [if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
> >
> > url:    https://github.com/0day-ci/linux/commits/Matthias-Kaehlcke/kbuild-Add-__cc-option-macro/20170616-082252
> > config: sh-sh7710voipgw_defconfig (attached as .config)
> > compiler: sh4-linux-gnu-gcc (Debian 6.1.1-9) 6.1.1 20160705
> > reproduce:
> >         wget https://raw.githubusercontent.com/01org/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
> >         chmod +x ~/bin/make.cross
> >         # save the attached .config to linux build tree
> >         make.cross ARCH=sh
> >
> > All errors (new ones prefixed by >>):
> >
> >    {standard input}: Assembler messages:
> >>> {standard input}:672: Error: invalid operands for opcode
> >    {standard input}:673: Error: opcode not valid for this cpu variant
> >    {standard input}:674: Error: opcode not valid for this cpu variant
> >    {standard input}:675: Error: invalid operands for opcode
> >    {standard input}:1900: Error: invalid operands for opcode
> >    {standard input}:1901: Error: opcode not valid for this cpu variant
> >    {standard input}:1902: Error: opcode not valid for this cpu variant
> >    {standard input}:1903: Error: invalid operands for opcode
> >    {standard input}:2067: Error: invalid operands for opcode
> >    {standard input}:2068: Error: opcode not valid for this cpu variant
> >    {standard input}:2069: Error: opcode not valid for this cpu variant
> >    {standard input}:2070: Error: invalid operands for opcode
> >    {standard input}:2570: Error: invalid operands for opcode
> >    {standard input}:2571: Error: opcode not valid for this cpu variant
> >    {standard input}:2572: Error: opcode not valid for this cpu variant
> >    {standard input}:2573: Error: invalid operands for opcode
> >
> > ---
> > 0-DAY kernel test infrastructure                Open Source Technology Center
> > https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
> 
> 
> 

[PATCH v3 2/3] x86/build: Use __cc-option for boot code compiler options

[Matthias Kaehlcke]

cc-option is used to enable compiler options for the boot code if they
are available. The macro uses KBUILD_CFLAGS and KBUILD_CPPFLAGS for the
check, however 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 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 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>
---
Changes in v3:
- Use __cc-option instead of cc-option-raw
- Updated commit message

 arch/x86/Makefile | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/arch/x86/Makefile b/arch/x86/Makefile
index bf240b920473..b2dae639f778 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, $(CC), $(REALMODE_CFLAGS), -ffreestanding)
+REALMODE_CFLAGS += $(call __cc-option, $(CC), $(REALMODE_CFLAGS), -fno-stack-protector)
+REALMODE_CFLAGS += $(call __cc-option, $(CC), $(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.518.g3df882009-goog

[PATCH v3 3/3] x86/build: Specify stack alignment for clang

[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 with 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>
---
Changes in v3:
- Use __cc-option instead of cc-option-raw

 arch/x86/Makefile | 26 +++++++++++++++++++++-----
 1 file changed, 21 insertions(+), 5 deletions(-)

diff --git a/arch/x86/Makefile b/arch/x86/Makefile
index b2dae639f778..9406d3670452 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)
+	cc_stack_align_opt := -mstack-alignment
+else
+	cc_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, $(CC), $(REALMODE_CFLAGS), -ffreestanding)
 REALMODE_CFLAGS += $(call __cc-option, $(CC), $(REALMODE_CFLAGS), -fno-stack-protector)
-REALMODE_CFLAGS += $(call __cc-option, $(CC), $(REALMODE_CFLAGS), -mpreferred-stack-boundary=2)
+REALMODE_CFLAGS += $(call __cc-option, $(CC), $(REALMODE_CFLAGS), $(cc_stack_align_opt)=2)
 export REALMODE_CFLAGS
 
 # BITS is used as extension for files which are available in a 32 bit
@@ -65,8 +73,10 @@ 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)
+        # Align the stack to the register width instead of using the default
+        # alignment of 16 bytes. This reduces stack usage and the number of
+        # alignment instructions.
+        KBUILD_CFLAGS += $(call cc-option,$(cc_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 +108,14 @@ 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)
+        # By default gcc and clang use a stack alignment of 16 bytes for x86.
+        # However the standard kernel entry on x86-64 leaves the stack on an
+        # 8-byte boundary. If the compiler isn't informed about the actual
+        # alignment it will generate extra alignment instructions for the
+        # default alignment which keep the stack *mis*aligned.
+        # Furthermore an alignment to the register width reduces stack usage
+        # and the number of alignment instructions.
+        KBUILD_CFLAGS += $(call cc-option,$(cc_stack_align_opt)=3)
 
 	# Use -mskip-rax-setup if supported.
 	KBUILD_CFLAGS += $(call cc-option,-mskip-rax-setup)
-- 
2.13.1.518.g3df882009-goog