powerpc patches for new Kconfig language

RFC PATCH In review Nicholas Piggin 2 participants 5 messages Apr 30, 2018 – May 7, 2018

Subsystems (2)

PowerPC Kbuild

lore

Share & tools

lore

Nicholas Piggin

author reviewer

2018-04-30 01:24 UTC #0 lore raw

cover powerpc patches for new Kconfig language

I was taking a look at this nice new Kconfig feature here:

https://git.kernel.org/pub/scm/linux/kernel/git/masahiroy/linux-kbuild.git/log/?h=kconfig-shell-v3

And came up with some patches for it for powerpc. Hopefully after some review, these patches could be acked by powerpc maintainers and go into the kbuild tree (or the problems solved a different way).

Thanks,
Nick

Nicholas Piggin (2):
powerpc/kbuild: Use flags variables rather than overriding LD/CC/AS powerpc/kbuild: move -mprofile-kernel check to Kconfig

 arch/powerpc/Kconfig                          | 16 +----------
 arch/powerpc/Makefile                         | 28 ++++++-------------
 .../tools/gcc-check-mprofile-kernel.sh        |  4 +--
 3 files changed, 12 insertions(+), 36 deletions(-)

Signature · 1 line

2.17.0

Nicholas Piggin

author reviewer

2018-04-30 01:24 UTC #1 lore raw

RFC PATCH 1/2 powerpc/kbuild: Use flags variables rather than overriding LD/CC/AS

The powerpc toolchain can compile combinations of 32/64 bit and big/little endian, so it's convenient to consider, e.g.,

CC -m64 -mbig-endian

To be the C compiler for the purpose of invoking it to build target artifacts.

Rather than override, use kbuild defined variables to pass these flags around. Importantly, they must be passed to things like cc-option, so the usual cflags-y is not sufficient. This multitude of inconsistently named variables is a mess, but it's marginally better than overriding the toolchain because it matches what other architectures do.

This allows powerpc builds to work with the new kconfig macro language branch. XXX: not exactly sure why it fails in the first place.

XXX: 32-bit builds with 64-bit toolchain gain some additional options like -funit-at-a-time from cc-option. Unclear why that is. Build appears to be correct otherwise.

---
 arch/powerpc/Makefile | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

arch/powerpc/Makefile +7 -7

index 95813df90801..046b5dde9ff5 100644
@@ -74,13 +74,15 @@ endif
 endif
 
 ifeq ($(CONFIG_CPU_LITTLE_ENDIAN),y)
-override LD	+= -EL
+KBUILD_CPPFLAGS	+= -mlittle-endian
+LDFLAGS		+= -EL
 LDEMULATION	:= lppc
 GNUTARGET	:= powerpcle
 MULTIPLEWORD	:= -mno-multiple
 KBUILD_CFLAGS_MODULE += $(call cc-option,-mno-save-toc-indirect)
 else
-override LD	+= -EB
+KBUILD_CPPFLAGS += $(call cc-option,-mbig-endian)
+LDFLAGS		+= -EB
 LDEMULATION	:= ppc
 GNUTARGET	:= powerpc
 MULTIPLEWORD	:= -mmultiple

@@ -93,8 +95,6 @@ aflags-$(CONFIG_CPU_BIG_ENDIAN)		+= $(call cc-option,-mabi=elfv1)
 aflags-$(CONFIG_CPU_LITTLE_ENDIAN)	+= -mabi=elfv2
 endif
 
-cflags-$(CONFIG_CPU_LITTLE_ENDIAN)	+= -mlittle-endian
-cflags-$(CONFIG_CPU_BIG_ENDIAN)		+= $(call cc-option,-mbig-endian)
 ifneq ($(cc-name),clang)
   cflags-$(CONFIG_CPU_LITTLE_ENDIAN)	+= -mno-strict-align
 endif

@@ -103,9 +103,9 @@ aflags-$(CONFIG_CPU_BIG_ENDIAN)		+= $(call cc-option,-mbig-endian)
 aflags-$(CONFIG_CPU_LITTLE_ENDIAN)	+= -mlittle-endian
 
 ifeq ($(HAS_BIARCH),y)
-override AS	+= -a$(BITS)
-override LD	+= -m elf$(BITS)$(LDEMULATION)
-override CC	+= -m$(BITS)
+KBUILD_CPPFLAGS	+= -m$(BITS)
+KBUILD_AFLAGS	+= -m$(BITS) -Wl,-a$(BITS)
+LDFLAGS		+= -m elf$(BITS)$(LDEMULATION)
 KBUILD_ARFLAGS	+= --target=elf$(BITS)-$(GNUTARGET)
 endif

Signature · 1 line

2.17.0

Masahiro Yamada

2018-05-07 05:26 UTC #2 lore raw

Re: [RFC PATCH 1/2] powerpc/kbuild: Use flags variables rather than overriding LD/CC/AS

Hi.

2018-04-30 10:23 GMT+09:00 Nicholas Piggin <[email protected]>:

Quoted 7 lines

The powerpc toolchain can compile combinations of 32/64 bit and big/little endian, so it's convenient to consider, e.g.,
CC -m64 -mbig-endian
To be the C compiler for the purpose of invoking it to build target artifacts.

Right, but this is not possible in the new Kconfig scheme because CPU_BIG/LITTLE_ENDIAN can be turned on/off from Kconfig menus.

Quoted 10 lines

Rather than override, use kbuild defined variables to pass these flags around. Importantly, they must be passed to things like cc-option, so the usual cflags-y is not sufficient. This multitude of inconsistently named variables is a mess, but it's marginally better than overriding the toolchain because it matches what other architectures do.
This allows powerpc builds to work with the new kconfig macro language branch. XXX: not exactly sure why it fails in the first place.

Without this patch, 'scripts/kconfig/conf --syncconfig Kconfig' continues eternally.

This is because the change of environment variable $(CC) will trigger syncconfig.

So, this patch is the right thing to do.

Quoted 12 lines

XXX: 32-bit builds with 64-bit toolchain gain some additional options like -funit-at-a-time from cc-option. Unclear why that is. Build appears to be correct otherwise.
---
 arch/powerpc/Makefile | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)
arch/powerpc/Makefile +0 -0
index 95813df90801..046b5dde9ff5 100644

arch/powerpc/Makefile endif · line 77

Quoted 5 lines

arch/powerpc/Makefile +1 -1

@@ -74,13 +74,15 @@ endif
 endif
 
 ifeq ($(CONFIG_CPU_LITTLE_ENDIAN),y)
-override LD    += -EL
+KBUILD_CPPFLAGS        += -mlittle-endian

IMHO, I personally prefer

KBUILD_CFLAGS += -mlittle-endian
KBUILD_AFLAGS += -mlittle-endian

like the current arch/powerpc/Makefile add the flag to cflags-y / aflags-y separately.

Only the difference would be whether -mlittle-endian is passed to pre-processing the linker script, though.

Quoted 22 lines

arch/powerpc/Makefile +3 -3

+LDFLAGS                += -EL
 LDEMULATION    := lppc
 GNUTARGET      := powerpcle
 MULTIPLEWORD   := -mno-multiple
 KBUILD_CFLAGS_MODULE += $(call cc-option,-mno-save-toc-indirect)
 else
-override LD    += -EB
+KBUILD_CPPFLAGS += $(call cc-option,-mbig-endian)
+LDFLAGS                += -EB
 LDEMULATION    := ppc
 GNUTARGET      := powerpc
 MULTIPLEWORD   := -mmultiple

@@ -93,8 +95,6 @@ aflags-$(CONFIG_CPU_BIG_ENDIAN)               += $(call cc-option,-mabi=elfv1)
 aflags-$(CONFIG_CPU_LITTLE_ENDIAN)     += -mabi=elfv2
 endif
 
-cflags-$(CONFIG_CPU_LITTLE_ENDIAN)     += -mlittle-endian
-cflags-$(CONFIG_CPU_BIG_ENDIAN)                += $(call cc-option,-mbig-endian)
 ifneq ($(cc-name),clang)
   cflags-$(CONFIG_CPU_LITTLE_ENDIAN)   += -mno-strict-align
 endif

arch/powerpc/Makefile aflags-$(CONFIG_CPU_BIG_ENDIAN) += $(call cc-option,-mbig-endian) · line 106

Quoted 7 lines

arch/powerpc/Makefile +1 -3

@@ -103,9 +103,9 @@ aflags-$(CONFIG_CPU_BIG_ENDIAN)             += $(call cc-option,-mbig-endian)
 aflags-$(CONFIG_CPU_LITTLE_ENDIAN)     += -mlittle-endian
 
 ifeq ($(HAS_BIARCH),y)
-override AS    += -a$(BITS)
-override LD    += -m elf$(BITS)$(LDEMULATION)
-override CC    += -m$(BITS)
+KBUILD_CPPFLAGS        += -m$(BITS)

Do you mean this?

KBUILD_CFLAGS += -m$(BITS)

arch/powerpc/Makefile aflags-$(CONFIG_CPU_BIG_ENDIAN) += $(call cc-option,-mbig-endian) · line 107

Quoted 1 line

arch/powerpc/Makefile +1 -0
+KBUILD_AFLAGS  += -m$(BITS) -Wl,-a$(BITS)

Both KBUILD_CPPFLAGS and KBUILD_AFLAGS are added to orig_a_flags in scripts/Makefile.lib

So, -m$(BITS) will be doubled for *.S files.

Quoted 4 lines

arch/powerpc/Makefile +1 -0

+LDFLAGS                += -m elf$(BITS)$(LDEMULATION)
 KBUILD_ARFLAGS += --target=elf$(BITS)-$(GNUTARGET)
 endif

Signature · 2 lines

Best Regards
Masahiro Yamada

Nicholas Piggin

author reviewer

2018-05-07 09:50 UTC #3 lore raw

Re: [RFC PATCH 1/2] powerpc/kbuild: Use flags variables rather than overriding LD/CC/AS

Quoted 29 lines · On Mon, 7 May 2018 14:25:12 +0900 Masahiro Yamada…

Hi.
2018-04-30 10:23 GMT+09:00 Nicholas Piggin <[email protected]>:
Quoted 7 lines
The powerpc toolchain can compile combinations of 32/64 bit and big/little endian, so it's convenient to consider, e.g.,
CC -m64 -mbig-endian
To be the C compiler for the purpose of invoking it to build target artifacts.
Right, but this is not possible in the new Kconfig scheme because CPU_BIG/LITTLE_ENDIAN can be turned on/off from Kconfig menus.
Quoted 10 lines
Rather than override, use kbuild defined variables to pass these flags around. Importantly, they must be passed to things like cc-option, so the usual cflags-y is not sufficient. This multitude of inconsistently named variables is a mess, but it's marginally better than overriding the toolchain because it matches what other architectures do.
This allows powerpc builds to work with the new kconfig macro language branch. XXX: not exactly sure why it fails in the first place.

Quoted 7 lines

Without this patch, 'scripts/kconfig/conf --syncconfig Kconfig' continues eternally.
This is because the change of environment variable $(CC) will trigger syncconfig.
So, this patch is the right thing to do.

Thanks for looking, yes I like it better. I'm still having some trouble with 32-bit cross compilation with 64-bit toolchain. I'm trying to work through this, may need some help though.

Quoted 21 lines

Quoted 12 lines
XXX: 32-bit builds with 64-bit toolchain gain some additional options like -funit-at-a-time from cc-option. Unclear why that is. Build appears to be correct otherwise.
---
 arch/powerpc/Makefile | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)
arch/powerpc/Makefile +0 -0
index 95813df90801..046b5dde9ff5 100644
arch/powerpc/Makefile endif · line 77
Quoted 5 lines
arch/powerpc/Makefile +1 -1
@@ -74,13 +74,15 @@ endif
 endif
 
 ifeq ($(CONFIG_CPU_LITTLE_ENDIAN),y)
-override LD    += -EL
+KBUILD_CPPFLAGS        += -mlittle-endian  
IMHO, I personally prefer

Quoted 5 lines

KBUILD_CFLAGS += -mlittle-endian
KBUILD_AFLAGS += -mlittle-endian
like the current arch/powerpc/Makefile add the flag to cflags-y / aflags-y separately.

The problem I found is that we need this option to be invoked with 'cc-option'. I know this is probably not the right way to go about it though, but arm64 seems to have the same hack.

Any suggestions for how to solve that? Should the cc-option take KBUILD_CFLAGS as well?

Quoted 39 lines

Only the difference would be whether -mlittle-endian is passed to pre-processing the linker script, though.

Quoted 22 lines

arch/powerpc/Makefile +3 -3

+LDFLAGS                += -EL
 LDEMULATION    := lppc
 GNUTARGET      := powerpcle
 MULTIPLEWORD   := -mno-multiple
 KBUILD_CFLAGS_MODULE += $(call cc-option,-mno-save-toc-indirect)
 else
-override LD    += -EB
+KBUILD_CPPFLAGS += $(call cc-option,-mbig-endian)
+LDFLAGS                += -EB
 LDEMULATION    := ppc
 GNUTARGET      := powerpc
 MULTIPLEWORD   := -mmultiple

@@ -93,8 +95,6 @@ aflags-$(CONFIG_CPU_BIG_ENDIAN)               += $(call cc-option,-mabi=elfv1)
 aflags-$(CONFIG_CPU_LITTLE_ENDIAN)     += -mabi=elfv2
 endif
 
-cflags-$(CONFIG_CPU_LITTLE_ENDIAN)     += -mlittle-endian
-cflags-$(CONFIG_CPU_BIG_ENDIAN)                += $(call cc-option,-mbig-endian)
 ifneq ($(cc-name),clang)
   cflags-$(CONFIG_CPU_LITTLE_ENDIAN)   += -mno-strict-align
 endif

arch/powerpc/Makefile aflags-$(CONFIG_CPU_BIG_ENDIAN) += $(call cc-option,-mbig-endian) · line 106

Quoted 7 lines

arch/powerpc/Makefile +1 -3

@@ -103,9 +103,9 @@ aflags-$(CONFIG_CPU_BIG_ENDIAN)             += $(call cc-option,-mbig-endian)
 aflags-$(CONFIG_CPU_LITTLE_ENDIAN)     += -mlittle-endian
 
 ifeq ($(HAS_BIARCH),y)
-override AS    += -a$(BITS)
-override LD    += -m elf$(BITS)$(LDEMULATION)
-override CC    += -m$(BITS)
+KBUILD_CPPFLAGS        += -m$(BITS)

Do you mean this?

Quoted 1 line

KBUILD_CFLAGS += -m$(BITS)

IIRC it was the same cc-option problem.

Quoted 7 lines

arch/powerpc/Makefile aflags-$(CONFIG_CPU_BIG_ENDIAN) += $(call cc-option,-mbig-endian) · line 107
Quoted 1 line
arch/powerpc/Makefile +1 -0
+KBUILD_AFLAGS  += -m$(BITS) -Wl,-a$(BITS)  
Both KBUILD_CPPFLAGS and KBUILD_AFLAGS are added to orig_a_flags in scripts/Makefile.lib
So, -m$(BITS) will be doubled for *.S files.

Yes, I haven't got things quite right yet.

Thanks,
Nick

Nicholas Piggin

author reviewer

2018-04-30 01:24 UTC #4 lore raw

RFC PATCH 2/2 powerpc/kbuild: move -mprofile-kernel check to Kconfig

In reply to Nicholas Piggin

This eliminates the workaround that requires disabling -mprofile-kernel by default in Kconfig.

---
 arch/powerpc/Kconfig                            | 16 +---------------
 arch/powerpc/Makefile                           | 14 ++------------
 arch/powerpc/tools/gcc-check-mprofile-kernel.sh |  4 ++--
 3 files changed, 5 insertions(+), 29 deletions(-)

arch/powerpc/Kconfig +1 ~~-15~~

index 512fcc177c87..af527f894f9b 100644
@@ -460,23 +460,9 @@ config LD_HEAD_STUB_CATCH
 
 	  If unsure, say "N".
 
-config DISABLE_MPROFILE_KERNEL
-	bool "Disable use of mprofile-kernel for kernel tracing"
-	depends on PPC64 && CPU_LITTLE_ENDIAN
-	default y
-	help
-	  Selecting this options disables use of the mprofile-kernel ABI for
-	  kernel tracing. That will cause options such as live patching
-	  (CONFIG_LIVEPATCH) which depend on CONFIG_DYNAMIC_FTRACE_WITH_REGS to
-	  be disabled also.
-
-	  If you have a toolchain which supports mprofile-kernel, then you can
-	  disable this. Otherwise leave it enabled. If you're not sure, say
-	  "Y".
-
 config MPROFILE_KERNEL
 	depends on PPC64 && CPU_LITTLE_ENDIAN
-	def_bool !DISABLE_MPROFILE_KERNEL
+	def_bool $(success $(srctree)/arch/powerpc/tools/gcc-check-mprofile-kernel.sh $(CC) -I$(srctree)/include -D__KERNEL__)
 
 config IOMMU_HELPER
 	def_bool PPC64

arch/powerpc/Makefile +2 ~~-12~~

index 046b5dde9ff5..efab4d66043b 100644
@@ -158,18 +158,8 @@ CFLAGS-$(CONFIG_GENERIC_CPU) += -mcpu=powerpc64
 endif
 
 ifdef CONFIG_MPROFILE_KERNEL
-    ifeq ($(shell $(srctree)/arch/powerpc/tools/gcc-check-mprofile-kernel.sh $(CC) -I$(srctree)/include -D__KERNEL__),OK)
-        CC_FLAGS_FTRACE := -pg -mprofile-kernel
-        KBUILD_CPPFLAGS += -DCC_USING_MPROFILE_KERNEL
-    else
-        # If the user asked for mprofile-kernel but the toolchain doesn't
-        # support it, emit a warning and deliberately break the build later
-        # with mprofile-kernel-not-supported. We would prefer to make this an
-        # error right here, but then the user would never be able to run
-        # oldconfig to change their configuration.
-        $(warning Compiler does not support mprofile-kernel, set CONFIG_DISABLE_MPROFILE_KERNEL)
-        CC_FLAGS_FTRACE := -mprofile-kernel-not-supported
-    endif
+	CC_FLAGS_FTRACE := -pg -mprofile-kernel
+	KBUILD_CPPFLAGS += -DCC_USING_MPROFILE_KERNEL
 endif
 
 CFLAGS-$(CONFIG_CELL_CPU) += $(call cc-option,-mcpu=cell)

arch/powerpc/tools/gcc-check-mprofile-kernel.sh +2 -2

index 061f8035bdbe..ec4486a9c4a3 100755
@@ -10,13 +10,13 @@ set -o pipefail
 # Test whether the compile option -mprofile-kernel exists and generates
 # profiling code (ie. a call to _mcount()).
 echo "int func() { return 0; }" | \
-    $* -S -x c -O2 -p -mprofile-kernel - -o - 2> /dev/null | \
+    $* -m64 -S -x c -O2 -p -mprofile-kernel - -o - 2> /dev/null | \
     grep -q "_mcount"
 
 # Test whether the notrace attribute correctly suppresses calls to _mcount().
 
 echo -e "#include <linux/compiler.h>\nnotrace int func() { return 0; }" | \
-    $* -S -x c -O2 -p -mprofile-kernel - -o - 2> /dev/null | \
+    $* -m64 -S -x c -O2 -p -mprofile-kernel - -o - 2> /dev/null | \
     grep -q "_mcount" && \
     exit 1

Signature · 1 line

2.17.0