Since commit 4bf4f42a2feb ("powerpc/kbuild: Set default generic
machine type for 32-bit compile"), when building a 32 bits kernel
with a bi-arch version of GCC, or when building a book3s/32 kernel,
the option -mcpu=powerpc is passed to GCC at all time, relying on it
being eventually overriden by a subsequent -mcpu=xxxx.
But when building the same kernel with a 32 bits only version of GCC,
that is not done, relying on gcc being built with the expected default
CPU.
This logic has two problems. First, it is a bit fragile to rely on
whether the GCC version is bi-arch or not, because today we can have
bi-arch versions of GCC configured with a 32 bits default. Second,
there are some versions of GCC which don't support -mcpu=powerpc,
for instance for e500 SPE-only versions.
So, stop relying on this approximative logic and allow the user to
decide whether he/she wants to use the toolchain's default CPU or if
he/she wants to set one, and allow only possible CPUs based on the
selected target.
Reported-by: Pali Rohár <[email protected]>
Cc: Arnd Bergmann <[email protected]>
Cc: Segher Boessenkool <[email protected]>
Signed-off-by: Christophe Leroy <[email protected]>
---
arch/powerpc/Makefile | 26 +-------------------------
arch/powerpc/platforms/Kconfig.cputype | 24 ++++++++++++++++++++----
2 files changed, 21 insertions(+), 29 deletions(-)
diff --git a/arch/powerpc/Makefile b/arch/powerpc/Makefile
index a0cd70712061..d54e1fe03551 100644
--- a/arch/powerpc/Makefile
+++ b/arch/powerpc/Makefile
@@ -15,23 +15,6 @@ HAS_BIARCH := $(call cc-option-yn, -m32)
# Set default 32 bits cross compilers for vdso and boot wrapper
CROSS32_COMPILE ?=
-ifeq ($(HAS_BIARCH),y)
-ifeq ($(CROSS32_COMPILE),)
-ifdef CONFIG_PPC32
-# These options will be overridden by any -mcpu option that the CPU
-# or platform code sets later on the command line, but they are needed
-# to set a sane 32-bit cpu target for the 64-bit cross compiler which
-# may default to the wrong ISA.
-KBUILD_CFLAGS += -mcpu=powerpc
-KBUILD_AFLAGS += -mcpu=powerpc
-endif
-endif
-endif
-
-ifdef CONFIG_PPC_BOOK3S_32
-KBUILD_CFLAGS += -mcpu=powerpc
-endif
-
# If we're on a ppc/ppc64/ppc64le machine use that defconfig, otherwise just use
# ppc64_defconfig because we have nothing better to go on.
uname := $(shell uname -m)
@@ -183,6 +166,7 @@ endif
endif
CFLAGS-$(CONFIG_TARGET_CPU_BOOL) += $(call cc-option,-mcpu=$(CONFIG_TARGET_CPU))
+AFLAGS-$(CONFIG_TARGET_CPU_BOOL) += $(call cc-option,-mcpu=$(CONFIG_TARGET_CPU))
# Altivec option not allowed with e500mc64 in GCC.
ifdef CONFIG_ALTIVEC
@@ -193,14 +177,6 @@ endif
CFLAGS-$(CONFIG_E5500_CPU) += $(E5500_CPU)
CFLAGS-$(CONFIG_E6500_CPU) += $(call cc-option,-mcpu=e6500,$(E5500_CPU))
-ifdef CONFIG_PPC32
-ifdef CONFIG_PPC_E500MC
-CFLAGS-y += $(call cc-option,-mcpu=e500mc,-mcpu=powerpc)
-else
-CFLAGS-$(CONFIG_E500) += $(call cc-option,-mcpu=8540 -msoft-float,-mcpu=powerpc)
-endif
-endif
-
asinstr := $(call as-instr,lis 9$(comma)foo@high,-DHAVE_AS_ATHIGH=1)
KBUILD_CPPFLAGS += -I $(srctree)/arch/$(ARCH) $(asinstr)
diff --git a/arch/powerpc/platforms/Kconfig.cputype b/arch/powerpc/platforms/Kconfig.cputype
index 383ed4fe6013..197653442d9f 100644
--- a/arch/powerpc/platforms/Kconfig.cputype
+++ b/arch/powerpc/platforms/Kconfig.cputype
@@ -117,7 +117,8 @@ endchoice
choice
prompt "CPU selection"
- default GENERIC_CPU
+ default GENERIC_CPU if PPC64
+ default POWERPC_CPU if PPC32
help
This will create a kernel which is optimised for a particular CPU.
The resulting kernel may not run on other CPUs, so use this with care.
@@ -135,9 +136,13 @@ config GENERIC_CPU
select ARCH_HAS_FAST_MULTIPLIER
select PPC_64S_HASH_MMU
-config GENERIC_CPU
+config POWERPC_CPU
bool "Generic 32 bits powerpc"
- depends on PPC32 && !PPC_8xx
+ depends on PPC32 && !PPC_8xx && !E500
+
+config TOOLCHAIN_DEFAULT_CPU
+ bool "Rely on the toolchain's implicit default CPU"
+ depends on PPC32
config CELL_CPU
bool "Cell Broadband Engine"
@@ -196,11 +201,19 @@ config G4_CPU
depends on PPC_BOOK3S_32
select ALTIVEC
+config 8540_CPU
+ bool "8540"
+ depends on PPC_85xx && !PPC_E500MC
+
+config E500MC_CPU
+ bool "e500mc"
+ depends on PPC_85xx && PPC_E500MC
+
endchoice
config TARGET_CPU_BOOL
bool
- default !GENERIC_CPU
+ default !GENERIC_CPU && !TOOLCHAIN_DEFAULT_CPU
config TARGET_CPU
string
@@ -215,6 +228,9 @@ config TARGET_CPU
default "e300c2" if E300C2_CPU
default "e300c3" if E300C3_CPU
default "G4" if G4_CPU
+ default "8540" if 8540_CPU
+ default "e500mc" if E500MC_CPU
+ default "powerpc" if POWERPC_CPU
config PPC_BOOK3S
def_bool y
--
2.36.1
On Sunday 10 July 2022 19:36:58 Christophe Leroy wrote:
> Since commit 4bf4f42a2feb ("powerpc/kbuild: Set default generic
> machine type for 32-bit compile"), when building a 32 bits kernel
> with a bi-arch version of GCC, or when building a book3s/32 kernel,
> the option -mcpu=powerpc is passed to GCC at all time, relying on it
> being eventually overriden by a subsequent -mcpu=xxxx.
>
> But when building the same kernel with a 32 bits only version of GCC,
> that is not done, relying on gcc being built with the expected default
> CPU.
>
> This logic has two problems. First, it is a bit fragile to rely on
> whether the GCC version is bi-arch or not, because today we can have
> bi-arch versions of GCC configured with a 32 bits default. Second,
> there are some versions of GCC which don't support -mcpu=powerpc,
> for instance for e500 SPE-only versions.
>
> So, stop relying on this approximative logic and allow the user to
> decide whether he/she wants to use the toolchain's default CPU or if
> he/she wants to set one, and allow only possible CPUs based on the
> selected target.
>
> Reported-by: Pali Rohár <[email protected]>
> Cc: Arnd Bergmann <[email protected]>
> Cc: Segher Boessenkool <[email protected]>
> Signed-off-by: Christophe Leroy <[email protected]>
Tested-by: Pali Rohár <[email protected]>
Works fine with SPE-based gcc for e500v2 platform Freescale P2020.
I have small suggestion for naming convention, see below.
> ---
> arch/powerpc/Makefile | 26 +-------------------------
> arch/powerpc/platforms/Kconfig.cputype | 24 ++++++++++++++++++++----
> 2 files changed, 21 insertions(+), 29 deletions(-)
>
> diff --git a/arch/powerpc/Makefile b/arch/powerpc/Makefile
> index a0cd70712061..d54e1fe03551 100644
> --- a/arch/powerpc/Makefile
> +++ b/arch/powerpc/Makefile
> @@ -15,23 +15,6 @@ HAS_BIARCH := $(call cc-option-yn, -m32)
> # Set default 32 bits cross compilers for vdso and boot wrapper
> CROSS32_COMPILE ?=
>
> -ifeq ($(HAS_BIARCH),y)
> -ifeq ($(CROSS32_COMPILE),)
> -ifdef CONFIG_PPC32
> -# These options will be overridden by any -mcpu option that the CPU
> -# or platform code sets later on the command line, but they are needed
> -# to set a sane 32-bit cpu target for the 64-bit cross compiler which
> -# may default to the wrong ISA.
> -KBUILD_CFLAGS += -mcpu=powerpc
> -KBUILD_AFLAGS += -mcpu=powerpc
> -endif
> -endif
> -endif
> -
> -ifdef CONFIG_PPC_BOOK3S_32
> -KBUILD_CFLAGS += -mcpu=powerpc
> -endif
> -
> # If we're on a ppc/ppc64/ppc64le machine use that defconfig, otherwise just use
> # ppc64_defconfig because we have nothing better to go on.
> uname := $(shell uname -m)
> @@ -183,6 +166,7 @@ endif
> endif
>
> CFLAGS-$(CONFIG_TARGET_CPU_BOOL) += $(call cc-option,-mcpu=$(CONFIG_TARGET_CPU))
> +AFLAGS-$(CONFIG_TARGET_CPU_BOOL) += $(call cc-option,-mcpu=$(CONFIG_TARGET_CPU))
>
> # Altivec option not allowed with e500mc64 in GCC.
> ifdef CONFIG_ALTIVEC
> @@ -193,14 +177,6 @@ endif
> CFLAGS-$(CONFIG_E5500_CPU) += $(E5500_CPU)
> CFLAGS-$(CONFIG_E6500_CPU) += $(call cc-option,-mcpu=e6500,$(E5500_CPU))
>
> -ifdef CONFIG_PPC32
> -ifdef CONFIG_PPC_E500MC
> -CFLAGS-y += $(call cc-option,-mcpu=e500mc,-mcpu=powerpc)
> -else
> -CFLAGS-$(CONFIG_E500) += $(call cc-option,-mcpu=8540 -msoft-float,-mcpu=powerpc)
> -endif
> -endif
> -
> asinstr := $(call as-instr,lis 9$(comma)foo@high,-DHAVE_AS_ATHIGH=1)
>
> KBUILD_CPPFLAGS += -I $(srctree)/arch/$(ARCH) $(asinstr)
> diff --git a/arch/powerpc/platforms/Kconfig.cputype b/arch/powerpc/platforms/Kconfig.cputype
> index 383ed4fe6013..197653442d9f 100644
> --- a/arch/powerpc/platforms/Kconfig.cputype
> +++ b/arch/powerpc/platforms/Kconfig.cputype
> @@ -117,7 +117,8 @@ endchoice
>
> choice
> prompt "CPU selection"
> - default GENERIC_CPU
> + default GENERIC_CPU if PPC64
> + default POWERPC_CPU if PPC32
> help
> This will create a kernel which is optimised for a particular CPU.
> The resulting kernel may not run on other CPUs, so use this with care.
> @@ -135,9 +136,13 @@ config GENERIC_CPU
> select ARCH_HAS_FAST_MULTIPLIER
> select PPC_64S_HASH_MMU
>
> -config GENERIC_CPU
> +config POWERPC_CPU
> bool "Generic 32 bits powerpc"
> - depends on PPC32 && !PPC_8xx
> + depends on PPC32 && !PPC_8xx && !E500
> +
> +config TOOLCHAIN_DEFAULT_CPU
> + bool "Rely on the toolchain's implicit default CPU"
> + depends on PPC32
>
> config CELL_CPU
> bool "Cell Broadband Engine"
> @@ -196,11 +201,19 @@ config G4_CPU
> depends on PPC_BOOK3S_32
> select ALTIVEC
>
> +config 8540_CPU
> + bool "8540"
> + depends on PPC_85xx && !PPC_E500MC
MPC8540 is name of the first (Motorola) SoC which had e500(v1) core.
e500 cores are used also in new Freescale P1/P2 series and also in
BSC913x and C29x products.
So I would suggest to call this 'config E500_CPU' with description
'bool "e500 (8540)"' because all other *_CPU options are named by the
CPU core, not by the SoC product which core contains. There is e.g.
E5500_CPU, E300C2_CPU or E500MC_CPU.
It is confusing that gcc named this option "-mcpu=8540" and not
"-mcpu=e500". For example GNU AS call this option "-me500" and I read
somewhere that clang/llvm wanted to add "-mcpu=e500" as an alias.
For example with this change:
diff --git a/arch/powerpc/platforms/Kconfig.cputype b/arch/powerpc/platforms/Kconfig.cputype
index 6c2b2d2c8bfc..8c8dd3c066f9 100644
--- a/arch/powerpc/platforms/Kconfig.cputype
+++ b/arch/powerpc/platforms/Kconfig.cputype
@@ -202,8 +202,8 @@ config G4_CPU
depends on PPC_BOOK3S_32
select ALTIVEC
-config 8540_CPU
- bool "8540"
+config E500_CPU
+ bool "e500 (8540)"
depends on PPC_85xx && !PPC_E500MC
config E500MC_CPU
@@ -229,7 +229,7 @@ config TARGET_CPU
default "e300c2" if E300C2_CPU
default "e300c3" if E300C3_CPU
default "G4" if G4_CPU
- default "8540" if 8540_CPU
+ default "8540" if E500_CPU
default "e500mc" if E500MC_CPU
default "powerpc" if POWERPC_CPU
> +
> +config E500MC_CPU
> + bool "e500mc"
> + depends on PPC_85xx && PPC_E500MC
> +
> endchoice
>
> config TARGET_CPU_BOOL
> bool
> - default !GENERIC_CPU
> + default !GENERIC_CPU && !TOOLCHAIN_DEFAULT_CPU
>
> config TARGET_CPU
> string
> @@ -215,6 +228,9 @@ config TARGET_CPU
> default "e300c2" if E300C2_CPU
> default "e300c3" if E300C3_CPU
> default "G4" if G4_CPU
> + default "8540" if 8540_CPU
> + default "e500mc" if E500MC_CPU
> + default "powerpc" if POWERPC_CPU
>
> config PPC_BOOK3S
> def_bool y
> --
> 2.36.1
>
On Sun, Jul 10, 2022 at 7:36 PM Christophe Leroy
<[email protected]> wrote:
>
> Since commit 4bf4f42a2feb ("powerpc/kbuild: Set default generic
> machine type for 32-bit compile"), when building a 32 bits kernel
> with a bi-arch version of GCC, or when building a book3s/32 kernel,
> the option -mcpu=powerpc is passed to GCC at all time, relying on it
> being eventually overriden by a subsequent -mcpu=xxxx.
>
> But when building the same kernel with a 32 bits only version of GCC,
> that is not done, relying on gcc being built with the expected default
> CPU.
>
> This logic has two problems. First, it is a bit fragile to rely on
> whether the GCC version is bi-arch or not, because today we can have
> bi-arch versions of GCC configured with a 32 bits default. Second,
> there are some versions of GCC which don't support -mcpu=powerpc,
> for instance for e500 SPE-only versions.
>
> So, stop relying on this approximative logic and allow the user to
> decide whether he/she wants to use the toolchain's default CPU or if
> he/she wants to set one, and allow only possible CPUs based on the
> selected target.
>
> Reported-by: Pali Rohár <[email protected]>
> Cc: Arnd Bergmann <[email protected]>
> Cc: Segher Boessenkool <[email protected]>
> Signed-off-by: Christophe Leroy <[email protected]>
Looks good to me,
Reviewed-by: Arnd Bergmann <[email protected]>