[RFC PATCH 0/2] powerpc patches for new Kconfig language

[RFC PATCH 0/2] powerpc patches for new Kconfig language

[Nicholas Piggin]

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

-- 
2.17.0

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

[Nicholas Piggin]

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

diff --git a/arch/powerpc/Makefile b/arch/powerpc/Makefile
index 95813df90801..046b5dde9ff5 100644
--- a/arch/powerpc/Makefile
+++ b/arch/powerpc/Makefile
@@ -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
 
-- 
2.17.0

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

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

diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index 512fcc177c87..af527f894f9b 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -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
diff --git a/arch/powerpc/Makefile b/arch/powerpc/Makefile
index 046b5dde9ff5..efab4d66043b 100644
--- a/arch/powerpc/Makefile
+++ b/arch/powerpc/Makefile
@@ -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)
diff --git a/arch/powerpc/tools/gcc-check-mprofile-kernel.sh b/arch/powerpc/tools/gcc-check-mprofile-kernel.sh
index 061f8035bdbe..ec4486a9c4a3 100755
--- a/arch/powerpc/tools/gcc-check-mprofile-kernel.sh
+++ b/arch/powerpc/tools/gcc-check-mprofile-kernel.sh
@@ -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
 
-- 
2.17.0

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

[Masahiro Yamada]

Hi.


2018-04-30 10:23 GMT+09:00 Nicholas Piggin <npiggin@gmail.com>:
> 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.


> 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.



> 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(-)
>
> diff --git a/arch/powerpc/Makefile b/arch/powerpc/Makefile
> index 95813df90801..046b5dde9ff5 100644
> --- a/arch/powerpc/Makefile
> +++ b/arch/powerpc/Makefile
> @@ -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.




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


Do you mean this?

KBUILD_CFLAGS        += -m$(BITS)


> +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.


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



-- 
Best Regards
Masahiro Yamada

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

[Nicholas Piggin]

On Mon, 7 May 2018 14:25:12 +0900
Masahiro Yamada <yamada.masahiro@socionext.com> wrote:

> Hi.
> 
> 
> 2018-04-30 10:23 GMT+09:00 Nicholas Piggin <npiggin@gmail.com>:
> > 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.
> 
> 
> > 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.

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.

> > 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(-)
> >
> > diff --git a/arch/powerpc/Makefile b/arch/powerpc/Makefile
> > index 95813df90801..046b5dde9ff5 100644
> > --- a/arch/powerpc/Makefile
> > +++ b/arch/powerpc/Makefile
> > @@ -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.

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?


> Only the difference would be whether -mlittle-endian
> is passed to pre-processing the linker script, though.
> 
> 
> 
> 
> > +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)  
> 
> 
> Do you mean this?
> 
> KBUILD_CFLAGS        += -m$(BITS)

IIRC it was the same cc-option problem.

> > +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