| Message ID | d4df724691351531bf46d685d654689e5dfa0d74.1657549153.git.christophe.leroy@csgroup.eu (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [v1,1/5] powerpc/32: Do not allow selection of e5500 or e6500 CPUs on PPC32 | expand |
On Mon, Jul 11, 2022 at 04:19:30PM +0200, 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. More fundamentally, the *only* thing you should check biarch for is for determining if you can use both -m32 and -m64 with the same compiler. Everything behaves identically in a biarch and a non-biarch compiler, other than the latter screams bloody murder if you try to change the architecture from 32 to 64 bit or vice versa. > 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 <pali@kernel.org> > Tested-by: Pali Rohár <pali@kernel.org> > Reviewed-by: Arnd Bergmann <arnd@arndb.de> > Cc: Segher Boessenkool <segher@kernel.crashing.org> > Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu> Reviewed-by: Segher Boessenkool <segher@kernel.crashing.org> Looks good to me. This untangles/demystifies quite some code :-) Segher
On Monday 11 July 2022 16:19:30 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. Hello! Exactly same issue is still in file arch/powerpc/boot/Makefile: ifdef CONFIG_PPC64_BOOT_WRAPPER ifdef CONFIG_CPU_LITTLE_ENDIAN BOOTCFLAGS += -m64 -mcpu=powerpc64le else BOOTCFLAGS += -m64 -mcpu=powerpc64 endif else BOOTCFLAGS += -m32 -mcpu=powerpc endif It cause compile error: make ARCH=powerpc CROSS_COMPILE=powerpc-linux-gnuspe- mpc85xx_smp_defconfig uImage ... BOOTAS arch/powerpc/boot/crt0.o powerpc-linux-gnuspe-gcc: error: unrecognized argument in option ‘-mcpu=powerpc’ powerpc-linux-gnuspe-gcc: note: valid arguments to ‘-mcpu=’ are: 8540 8548 native make[1]: *** [arch/powerpc/boot/Makefile:231: arch/powerpc/boot/crt0.o] Error 1 > Reported-by: Pali Rohár <pali@kernel.org> > Tested-by: Pali Rohár <pali@kernel.org> > Reviewed-by: Arnd Bergmann <arnd@arndb.de> > Cc: Segher Boessenkool <segher@kernel.crashing.org> > Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu> > --- > arch/powerpc/Makefile | 26 +------------------------- > arch/powerpc/platforms/Kconfig.cputype | 21 ++++++++++++++++++--- > 2 files changed, 19 insertions(+), 28 deletions(-)
On Thursday 18 August 2022 19:46:34 Pali Rohár wrote: > On Monday 11 July 2022 16:19:30 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. > > Hello! Exactly same issue is still in file arch/powerpc/boot/Makefile: > > ifdef CONFIG_PPC64_BOOT_WRAPPER > ifdef CONFIG_CPU_LITTLE_ENDIAN > BOOTCFLAGS += -m64 -mcpu=powerpc64le > else > BOOTCFLAGS += -m64 -mcpu=powerpc64 > endif > else > BOOTCFLAGS += -m32 -mcpu=powerpc > endif > > It cause compile error: > > make ARCH=powerpc CROSS_COMPILE=powerpc-linux-gnuspe- mpc85xx_smp_defconfig uImage > ... > BOOTAS arch/powerpc/boot/crt0.o > powerpc-linux-gnuspe-gcc: error: unrecognized argument in option ‘-mcpu=powerpc’ > powerpc-linux-gnuspe-gcc: note: valid arguments to ‘-mcpu=’ are: 8540 8548 native > make[1]: *** [arch/powerpc/boot/Makefile:231: arch/powerpc/boot/crt0.o] Error 1 Now I have sent patch for this issue: https://patchwork.ozlabs.org/project/linuxppc-dev/patch/20220820105200.30425-1-pali@kernel.org/ > > Reported-by: Pali Rohár <pali@kernel.org> > > Tested-by: Pali Rohár <pali@kernel.org> > > Reviewed-by: Arnd Bergmann <arnd@arndb.de> > > Cc: Segher Boessenkool <segher@kernel.crashing.org> > > Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu> > > --- > > arch/powerpc/Makefile | 26 +------------------------- > > arch/powerpc/platforms/Kconfig.cputype | 21 ++++++++++++++++++--- > > 2 files changed, 19 insertions(+), 28 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 9805a2c717b9..bfea5d8452f8 100644 --- a/arch/powerpc/platforms/Kconfig.cputype +++ b/arch/powerpc/platforms/Kconfig.cputype @@ -135,9 +135,9 @@ 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 && !PPC_85xx config CELL_CPU bool "Cell Broadband Engine" @@ -196,11 +196,23 @@ config G4_CPU depends on PPC_BOOK3S_32 select ALTIVEC +config E500_CPU + bool "e500 (8540)" + depends on PPC_85xx && !PPC_E500MC + +config E500MC_CPU + bool "e500mc" + depends on PPC_85xx && PPC_E500MC + +config TOOLCHAIN_DEFAULT_CPU + bool "Rely on the toolchain's implicit default CPU" + depends on PPC32 + endchoice config TARGET_CPU_BOOL bool - default !GENERIC_CPU + default !GENERIC_CPU && !TOOLCHAIN_DEFAULT_CPU config TARGET_CPU string @@ -215,6 +227,9 @@ config TARGET_CPU default "e300c2" if E300C2_CPU default "e300c3" if E300C3_CPU default "G4" if G4_CPU + default "8540" if E500_CPU + default "e500mc" if E500MC_CPU + default "powerpc" if POWERPC_CPU config PPC_BOOK3S def_bool y
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 <pali@kernel.org> Tested-by: Pali Rohár <pali@kernel.org> Reviewed-by: Arnd Bergmann <arnd@arndb.de> Cc: Segher Boessenkool <segher@kernel.crashing.org> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu> --- arch/powerpc/Makefile | 26 +------------------------- arch/powerpc/platforms/Kconfig.cputype | 21 ++++++++++++++++++--- 2 files changed, 19 insertions(+), 28 deletions(-)