All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] powerpc/boot: Build wrapper for an appropriate CPU
@ 2022-03-30 11:24 Joel Stanley
  2022-03-30 11:33 ` Christophe Leroy
                   ` (2 more replies)
  0 siblings, 3 replies; 13+ messages in thread
From: Joel Stanley @ 2022-03-30 11:24 UTC (permalink / raw)
  To: linuxppc-dev

Currently the boot wrapper lacks a -mcpu option, so it will be built for
the toolchain's default cpu. This is a problem if the toolchain defaults
to a cpu with newer instructions.

We could wire in TARGET_CPU but instead use the oldest supported option
so the wrapper runs anywhere.

The GCC documentation stays that -mcpu=powerpc64le will give us a
generic 64 bit powerpc machine:

 -mcpu=powerpc, -mcpu=powerpc64, and -mcpu=powerpc64le specify pure
 32-bit PowerPC (either endian), 64-bit big endian PowerPC and 64-bit
 little endian PowerPC architecture machine types, with an appropriate,
 generic processor model assumed for scheduling purposes.

So do that for each of the three machines.

This bug was found when building the kernel with a toolchain that
defaulted to powre10, resulting in a pcrel enabled wrapper which fails
to link:

 arch/powerpc/boot/wrapper.a(crt0.o): in function `p_base':
 (.text+0x150): call to `platform_init' lacks nop, can't restore toc; (toc save/adjust stub)
 (.text+0x154): call to `start' lacks nop, can't restore toc; (toc save/adjust stub)
 powerpc64le-buildroot-linux-gnu-ld: final link failed: bad value

Even with tha bug worked around the resulting kernel would crash on a
power9 box:

 $ qemu-system-ppc64 -nographic -nodefaults -M powernv9 -kernel arch/powerpc/boot/zImage.epapr -serial mon:stdio
 [    7.069331356,5] INIT: Starting kernel at 0x20010020, fdt at 0x3068c628 25694 bytes
 [    7.130374661,3] ***********************************************
 [    7.131072886,3] Fatal Exception 0xe40 at 00000000200101e4    MSR 9000000000000001
 [    7.131290613,3] CFAR : 000000002001027c MSR  : 9000000000000001
 [    7.131433759,3] SRR0 : 0000000020010050 SRR1 : 9000000000000001
 [    7.131577775,3] HSRR0: 00000000200101e4 HSRR1: 9000000000000001
 [    7.131733687,3] DSISR: 00000000         DAR  : 0000000000000000
 [    7.131905162,3] LR   : 0000000020010280 CTR  : 0000000000000000
 [    7.132068356,3] CR   : 44002004         XER  : 00000000

Link: https://github.com/linuxppc/issues/issues/400
Signed-off-by: Joel Stanley <joel@jms.id.au>
---
Tested:

 - ppc64le_defconfig
 - pseries and powernv qemu, for power8, power9, power10 cpus
 - buildroot compiler that defaults to -mcpu=power10 (gcc 10.3.0, ld 2.36.1)
 -  RHEL9 cross compilers (gcc 11.2.1-1, ld 2.35.2-17.el9)

All decompressed and made it into the kernel ok.

ppc64_defconfig did not work, as we've got a regression when the wrapper
is built for big endian. It hasn't worked for zImage.pseries for a long
time (at least v4.14), and broke some time between v5.4 and v5.17 for
zImage.epapr.

 arch/powerpc/boot/Makefile | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/boot/Makefile b/arch/powerpc/boot/Makefile
index 9993c6256ad2..1f5cc401bfc0 100644
--- a/arch/powerpc/boot/Makefile
+++ b/arch/powerpc/boot/Makefile
@@ -38,9 +38,13 @@ BOOTCFLAGS    := -Wall -Wundef -Wstrict-prototypes -Wno-trigraphs \
 		 $(LINUXINCLUDE)
 
 ifdef CONFIG_PPC64_BOOT_WRAPPER
-BOOTCFLAGS	+= -m64
+ifdef CONFIG_CPU_LITTLE_ENDIAN
+BOOTCFLAGS	+= -m64 -mcpu=powerpc64le
 else
-BOOTCFLAGS	+= -m32
+BOOTCFLAGS	+= -m64 -mcpu=powerpc64
+endif
+else
+BOOTCFLAGS	+= -m32 -mcpu=powerpc
 endif
 
 BOOTCFLAGS	+= -isystem $(shell $(BOOTCC) -print-file-name=include)
-- 
2.35.1


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH] powerpc/boot: Build wrapper for an appropriate CPU
  2022-03-30 11:24 [PATCH] powerpc/boot: Build wrapper for an appropriate CPU Joel Stanley
@ 2022-03-30 11:33 ` Christophe Leroy
  2022-03-30 11:39   ` Joel Stanley
  2022-03-31  2:05 ` Murilo Opsfelder Araújo
  2022-05-15 10:12 ` Michael Ellerman
  2 siblings, 1 reply; 13+ messages in thread
From: Christophe Leroy @ 2022-03-30 11:33 UTC (permalink / raw)
  To: Joel Stanley, linuxppc-dev



Le 30/03/2022 à 13:24, Joel Stanley a écrit :
> Currently the boot wrapper lacks a -mcpu option, so it will be built for
> the toolchain's default cpu. This is a problem if the toolchain defaults
> to a cpu with newer instructions.
> 
> We could wire in TARGET_CPU but instead use the oldest supported option
> so the wrapper runs anywhere.
> 
> The GCC documentation stays that -mcpu=powerpc64le will give us a
> generic 64 bit powerpc machine:
> 
>   -mcpu=powerpc, -mcpu=powerpc64, and -mcpu=powerpc64le specify pure
>   32-bit PowerPC (either endian), 64-bit big endian PowerPC and 64-bit
>   little endian PowerPC architecture machine types, with an appropriate,
>   generic processor model assumed for scheduling purposes.
> 
> So do that for each of the three machines.
> 
> This bug was found when building the kernel with a toolchain that
> defaulted to powre10, resulting in a pcrel enabled wrapper which fails
> to link:
> 
>   arch/powerpc/boot/wrapper.a(crt0.o): in function `p_base':
>   (.text+0x150): call to `platform_init' lacks nop, can't restore toc; (toc save/adjust stub)
>   (.text+0x154): call to `start' lacks nop, can't restore toc; (toc save/adjust stub)
>   powerpc64le-buildroot-linux-gnu-ld: final link failed: bad value
> 
> Even with tha bug worked around the resulting kernel would crash on a
> power9 box:
> 
>   $ qemu-system-ppc64 -nographic -nodefaults -M powernv9 -kernel arch/powerpc/boot/zImage.epapr -serial mon:stdio
>   [    7.069331356,5] INIT: Starting kernel at 0x20010020, fdt at 0x3068c628 25694 bytes
>   [    7.130374661,3] ***********************************************
>   [    7.131072886,3] Fatal Exception 0xe40 at 00000000200101e4    MSR 9000000000000001
>   [    7.131290613,3] CFAR : 000000002001027c MSR  : 9000000000000001
>   [    7.131433759,3] SRR0 : 0000000020010050 SRR1 : 9000000000000001
>   [    7.131577775,3] HSRR0: 00000000200101e4 HSRR1: 9000000000000001
>   [    7.131733687,3] DSISR: 00000000         DAR  : 0000000000000000
>   [    7.131905162,3] LR   : 0000000020010280 CTR  : 0000000000000000
>   [    7.132068356,3] CR   : 44002004         XER  : 00000000
> 
> Link: https://github.com/linuxppc/issues/issues/400
> Signed-off-by: Joel Stanley <joel@jms.id.au>
> ---
> Tested:
> 
>   - ppc64le_defconfig
>   - pseries and powernv qemu, for power8, power9, power10 cpus
>   - buildroot compiler that defaults to -mcpu=power10 (gcc 10.3.0, ld 2.36.1)
>   -  RHEL9 cross compilers (gcc 11.2.1-1, ld 2.35.2-17.el9)
> 
> All decompressed and made it into the kernel ok.
> 
> ppc64_defconfig did not work, as we've got a regression when the wrapper
> is built for big endian. It hasn't worked for zImage.pseries for a long
> time (at least v4.14), and broke some time between v5.4 and v5.17 for
> zImage.epapr.
> 
>   arch/powerpc/boot/Makefile | 8 ++++++--
>   1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/powerpc/boot/Makefile b/arch/powerpc/boot/Makefile
> index 9993c6256ad2..1f5cc401bfc0 100644
> --- a/arch/powerpc/boot/Makefile
> +++ b/arch/powerpc/boot/Makefile
> @@ -38,9 +38,13 @@ BOOTCFLAGS    := -Wall -Wundef -Wstrict-prototypes -Wno-trigraphs \
>   		 $(LINUXINCLUDE)
>   
>   ifdef CONFIG_PPC64_BOOT_WRAPPER
> -BOOTCFLAGS	+= -m64
> +ifdef CONFIG_CPU_LITTLE_ENDIAN
> +BOOTCFLAGS	+= -m64 -mcpu=powerpc64le
>   else
> -BOOTCFLAGS	+= -m32
> +BOOTCFLAGS	+= -m64 -mcpu=powerpc64
> +endif
> +else
> +BOOTCFLAGS	+= -m32 -mcpu=powerpc

How does that interracts with the following lines ? Isn't it an issue to 
have two -mcpu ?

arch/powerpc/boot/Makefile:$(obj)/4xx.o: BOOTCFLAGS += -mcpu=405
arch/powerpc/boot/Makefile:$(obj)/ebony.o: BOOTCFLAGS += -mcpu=440
arch/powerpc/boot/Makefile:$(obj)/cuboot-hotfoot.o: BOOTCFLAGS += -mcpu=405
arch/powerpc/boot/Makefile:$(obj)/cuboot-taishan.o: BOOTCFLAGS += -mcpu=440
arch/powerpc/boot/Makefile:$(obj)/cuboot-katmai.o: BOOTCFLAGS += -mcpu=440
arch/powerpc/boot/Makefile:$(obj)/cuboot-acadia.o: BOOTCFLAGS += -mcpu=405
arch/powerpc/boot/Makefile:$(obj)/treeboot-iss4xx.o: BOOTCFLAGS += -mcpu=405
arch/powerpc/boot/Makefile:$(obj)/treeboot-currituck.o: BOOTCFLAGS += 
-mcpu=405
arch/powerpc/boot/Makefile:$(obj)/treeboot-akebono.o: BOOTCFLAGS += 
-mcpu=405


>   endif
>   
>   BOOTCFLAGS	+= -isystem $(shell $(BOOTCC) -print-file-name=include)

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] powerpc/boot: Build wrapper for an appropriate CPU
  2022-03-30 11:33 ` Christophe Leroy
@ 2022-03-30 11:39   ` Joel Stanley
  2022-03-31 23:00     ` Segher Boessenkool
  0 siblings, 1 reply; 13+ messages in thread
From: Joel Stanley @ 2022-03-30 11:39 UTC (permalink / raw)
  To: Christophe Leroy; +Cc: linuxppc-dev

On Wed, 30 Mar 2022 at 11:33, Christophe Leroy
<christophe.leroy@csgroup.eu> wrote:
>
>
>
> Le 30/03/2022 à 13:24, Joel Stanley a écrit :
> > Currently the boot wrapper lacks a -mcpu option, so it will be built for
> > the toolchain's default cpu. This is a problem if the toolchain defaults
> > to a cpu with newer instructions.
> >
> > We could wire in TARGET_CPU but instead use the oldest supported option
> > so the wrapper runs anywhere.
> >
> > The GCC documentation stays that -mcpu=powerpc64le will give us a
> > generic 64 bit powerpc machine:
> >
> >   -mcpu=powerpc, -mcpu=powerpc64, and -mcpu=powerpc64le specify pure
> >   32-bit PowerPC (either endian), 64-bit big endian PowerPC and 64-bit
> >   little endian PowerPC architecture machine types, with an appropriate,
> >   generic processor model assumed for scheduling purposes.
> >
> > So do that for each of the three machines.
> >
> > This bug was found when building the kernel with a toolchain that
> > defaulted to powre10, resulting in a pcrel enabled wrapper which fails
> > to link:
> >
> >   arch/powerpc/boot/wrapper.a(crt0.o): in function `p_base':
> >   (.text+0x150): call to `platform_init' lacks nop, can't restore toc; (toc save/adjust stub)
> >   (.text+0x154): call to `start' lacks nop, can't restore toc; (toc save/adjust stub)
> >   powerpc64le-buildroot-linux-gnu-ld: final link failed: bad value
> >
> > Even with tha bug worked around the resulting kernel would crash on a
> > power9 box:
> >
> >   $ qemu-system-ppc64 -nographic -nodefaults -M powernv9 -kernel arch/powerpc/boot/zImage.epapr -serial mon:stdio
> >   [    7.069331356,5] INIT: Starting kernel at 0x20010020, fdt at 0x3068c628 25694 bytes
> >   [    7.130374661,3] ***********************************************
> >   [    7.131072886,3] Fatal Exception 0xe40 at 00000000200101e4    MSR 9000000000000001
> >   [    7.131290613,3] CFAR : 000000002001027c MSR  : 9000000000000001
> >   [    7.131433759,3] SRR0 : 0000000020010050 SRR1 : 9000000000000001
> >   [    7.131577775,3] HSRR0: 00000000200101e4 HSRR1: 9000000000000001
> >   [    7.131733687,3] DSISR: 00000000         DAR  : 0000000000000000
> >   [    7.131905162,3] LR   : 0000000020010280 CTR  : 0000000000000000
> >   [    7.132068356,3] CR   : 44002004         XER  : 00000000
> >
> > Link: https://github.com/linuxppc/issues/issues/400
> > Signed-off-by: Joel Stanley <joel@jms.id.au>
> > ---
> > Tested:
> >
> >   - ppc64le_defconfig
> >   - pseries and powernv qemu, for power8, power9, power10 cpus
> >   - buildroot compiler that defaults to -mcpu=power10 (gcc 10.3.0, ld 2.36.1)
> >   -  RHEL9 cross compilers (gcc 11.2.1-1, ld 2.35.2-17.el9)
> >
> > All decompressed and made it into the kernel ok.
> >
> > ppc64_defconfig did not work, as we've got a regression when the wrapper
> > is built for big endian. It hasn't worked for zImage.pseries for a long
> > time (at least v4.14), and broke some time between v5.4 and v5.17 for
> > zImage.epapr.
> >
> >   arch/powerpc/boot/Makefile | 8 ++++++--
> >   1 file changed, 6 insertions(+), 2 deletions(-)
> >
> > diff --git a/arch/powerpc/boot/Makefile b/arch/powerpc/boot/Makefile
> > index 9993c6256ad2..1f5cc401bfc0 100644
> > --- a/arch/powerpc/boot/Makefile
> > +++ b/arch/powerpc/boot/Makefile
> > @@ -38,9 +38,13 @@ BOOTCFLAGS    := -Wall -Wundef -Wstrict-prototypes -Wno-trigraphs \
> >                $(LINUXINCLUDE)
> >
> >   ifdef CONFIG_PPC64_BOOT_WRAPPER
> > -BOOTCFLAGS   += -m64
> > +ifdef CONFIG_CPU_LITTLE_ENDIAN
> > +BOOTCFLAGS   += -m64 -mcpu=powerpc64le
> >   else
> > -BOOTCFLAGS   += -m32
> > +BOOTCFLAGS   += -m64 -mcpu=powerpc64
> > +endif
> > +else
> > +BOOTCFLAGS   += -m32 -mcpu=powerpc
>
> How does that interracts with the following lines ? Isn't it an issue to
> have two -mcpu ?
>
> arch/powerpc/boot/Makefile:$(obj)/4xx.o: BOOTCFLAGS += -mcpu=405
> arch/powerpc/boot/Makefile:$(obj)/ebony.o: BOOTCFLAGS += -mcpu=440
> arch/powerpc/boot/Makefile:$(obj)/cuboot-hotfoot.o: BOOTCFLAGS += -mcpu=405
> arch/powerpc/boot/Makefile:$(obj)/cuboot-taishan.o: BOOTCFLAGS += -mcpu=440
> arch/powerpc/boot/Makefile:$(obj)/cuboot-katmai.o: BOOTCFLAGS += -mcpu=440
> arch/powerpc/boot/Makefile:$(obj)/cuboot-acadia.o: BOOTCFLAGS += -mcpu=405
> arch/powerpc/boot/Makefile:$(obj)/treeboot-iss4xx.o: BOOTCFLAGS += -mcpu=405
> arch/powerpc/boot/Makefile:$(obj)/treeboot-currituck.o: BOOTCFLAGS +=
> -mcpu=405
> arch/powerpc/boot/Makefile:$(obj)/treeboot-akebono.o: BOOTCFLAGS +=
> -mcpu=405

Good point, I didn't test the other wrappers.

Last one wins as far as -mcpu lines goes, from a quick test. But it
might lead to less confusion if I dropped the -mcpu=powerpc change.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] powerpc/boot: Build wrapper for an appropriate CPU
  2022-03-30 11:24 [PATCH] powerpc/boot: Build wrapper for an appropriate CPU Joel Stanley
  2022-03-30 11:33 ` Christophe Leroy
@ 2022-03-31  2:05 ` Murilo Opsfelder Araújo
  2022-03-31  5:01   ` Joel Stanley
  2022-03-31 23:09   ` Segher Boessenkool
  2022-05-15 10:12 ` Michael Ellerman
  2 siblings, 2 replies; 13+ messages in thread
From: Murilo Opsfelder Araújo @ 2022-03-31  2:05 UTC (permalink / raw)
  To: Joel Stanley, linuxppc-dev

Hi, Joel.

On 3/30/22 08:24, Joel Stanley wrote:
> Currently the boot wrapper lacks a -mcpu option, so it will be built for
> the toolchain's default cpu. This is a problem if the toolchain defaults
> to a cpu with newer instructions.
> 
> We could wire in TARGET_CPU but instead use the oldest supported option
> so the wrapper runs anywhere.
> 
> The GCC documentation stays that -mcpu=powerpc64le will give us a
> generic 64 bit powerpc machine:
> 
>   -mcpu=powerpc, -mcpu=powerpc64, and -mcpu=powerpc64le specify pure
>   32-bit PowerPC (either endian), 64-bit big endian PowerPC and 64-bit
>   little endian PowerPC architecture machine types, with an appropriate,
>   generic processor model assumed for scheduling purposes.
> 
> So do that for each of the three machines.
> 
> This bug was found when building the kernel with a toolchain that
> defaulted to powre10, resulting in a pcrel enabled wrapper which fails
> to link:
> 
>   arch/powerpc/boot/wrapper.a(crt0.o): in function `p_base':
>   (.text+0x150): call to `platform_init' lacks nop, can't restore toc; (toc save/adjust stub)
>   (.text+0x154): call to `start' lacks nop, can't restore toc; (toc save/adjust stub)
>   powerpc64le-buildroot-linux-gnu-ld: final link failed: bad value
> 
> Even with tha bug worked around the resulting kernel would crash on a
> power9 box:
> 
>   $ qemu-system-ppc64 -nographic -nodefaults -M powernv9 -kernel arch/powerpc/boot/zImage.epapr -serial mon:stdio
>   [    7.069331356,5] INIT: Starting kernel at 0x20010020, fdt at 0x3068c628 25694 bytes
>   [    7.130374661,3] ***********************************************
>   [    7.131072886,3] Fatal Exception 0xe40 at 00000000200101e4    MSR 9000000000000001
>   [    7.131290613,3] CFAR : 000000002001027c MSR  : 9000000000000001
>   [    7.131433759,3] SRR0 : 0000000020010050 SRR1 : 9000000000000001
>   [    7.131577775,3] HSRR0: 00000000200101e4 HSRR1: 9000000000000001
>   [    7.131733687,3] DSISR: 00000000         DAR  : 0000000000000000
>   [    7.131905162,3] LR   : 0000000020010280 CTR  : 0000000000000000
>   [    7.132068356,3] CR   : 44002004         XER  : 00000000
> 
> Link: https://github.com/linuxppc/issues/issues/400
> Signed-off-by: Joel Stanley <joel@jms.id.au>
> ---
> Tested:
> 
>   - ppc64le_defconfig
>   - pseries and powernv qemu, for power8, power9, power10 cpus
>   - buildroot compiler that defaults to -mcpu=power10 (gcc 10.3.0, ld 2.36.1)
>   -  RHEL9 cross compilers (gcc 11.2.1-1, ld 2.35.2-17.el9)
> 
> All decompressed and made it into the kernel ok.
> 
> ppc64_defconfig did not work, as we've got a regression when the wrapper
> is built for big endian. It hasn't worked for zImage.pseries for a long
> time (at least v4.14), and broke some time between v5.4 and v5.17 for
> zImage.epapr.
> 
>   arch/powerpc/boot/Makefile | 8 ++++++--
>   1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/powerpc/boot/Makefile b/arch/powerpc/boot/Makefile
> index 9993c6256ad2..1f5cc401bfc0 100644
> --- a/arch/powerpc/boot/Makefile
> +++ b/arch/powerpc/boot/Makefile
> @@ -38,9 +38,13 @@ BOOTCFLAGS    := -Wall -Wundef -Wstrict-prototypes -Wno-trigraphs \
>   		 $(LINUXINCLUDE)
>   
>   ifdef CONFIG_PPC64_BOOT_WRAPPER
> -BOOTCFLAGS	+= -m64
> +ifdef CONFIG_CPU_LITTLE_ENDIAN
> +BOOTCFLAGS	+= -m64 -mcpu=powerpc64le
>   else
> -BOOTCFLAGS	+= -m32
> +BOOTCFLAGS	+= -m64 -mcpu=powerpc64
> +endif
> +else
> +BOOTCFLAGS	+= -m32 -mcpu=powerpc
>   endif
>   
>   BOOTCFLAGS	+= -isystem $(shell $(BOOTCC) -print-file-name=include)

I think it was a fortunate coincidence that the default cpu type of your gcc is
compatible with your system.  If the distro gcc moves its default to a newer cpu
type than your system, this bug would happen again.

The command "gcc -v |& grep with-cpu" will show you the default cpu type for 32
and 64-bit that gcc was configured.

Considering the CONFIG_TARGET_CPU for BOOTCFLAGS would bring some level of
consistency between CFLAGS and BOOTCFLAGS regarding -mcpu value.

We could mimic the behaviour from arch/powerpc/Makefile:

     166 ifdef config_ppc_book3s_64
     167 ifdef config_cpu_little_endian
     168 cflags-$(config_generic_cpu) += -mcpu=power8
     169 cflags-$(config_generic_cpu) += $(call cc-option,-mtune=power9,-mtune=power8)
     170 else
     171 cflags-$(config_generic_cpu) += $(call cc-option,-mtune=power7,$(call cc-option,-mtune=power5))
     172 cflags-$(config_generic_cpu) += $(call cc-option,-mcpu=power5,-mcpu=power4)
     173 endif
     174 else ifdef config_ppc_book3e_64
     175 cflags-$(config_generic_cpu) += -mcpu=powerpc64
     176 endif
     ...
     185 CFLAGS-$(CONFIG_TARGET_CPU_BOOL) += $(call cc-option,-mcpu=$(CONFIG_TARGET_CPU))

Cheers!

-- 
Murilo

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] powerpc/boot: Build wrapper for an appropriate CPU
  2022-03-31  2:05 ` Murilo Opsfelder Araújo
@ 2022-03-31  5:01   ` Joel Stanley
  2022-03-31 15:19     ` Murilo Opsfelder Araújo
  2022-03-31 23:09   ` Segher Boessenkool
  1 sibling, 1 reply; 13+ messages in thread
From: Joel Stanley @ 2022-03-31  5:01 UTC (permalink / raw)
  To: Murilo Opsfelder Araújo; +Cc: linuxppc-dev

On Thu, 31 Mar 2022 at 02:05, Murilo Opsfelder Araújo
<mopsfelder@gmail.com> wrote:
>
> Hi, Joel.
>
> On 3/30/22 08:24, Joel Stanley wrote:
> > Currently the boot wrapper lacks a -mcpu option, so it will be built for
> > the toolchain's default cpu. This is a problem if the toolchain defaults
> > to a cpu with newer instructions.
> >
> > We could wire in TARGET_CPU but instead use the oldest supported option
> > so the wrapper runs anywhere.
> >
> > The GCC documentation stays that -mcpu=powerpc64le will give us a
> > generic 64 bit powerpc machine:
> >
> >   -mcpu=powerpc, -mcpu=powerpc64, and -mcpu=powerpc64le specify pure
> >   32-bit PowerPC (either endian), 64-bit big endian PowerPC and 64-bit
> >   little endian PowerPC architecture machine types, with an appropriate,
> >   generic processor model assumed for scheduling purposes.
> >
> > So do that for each of the three machines.
> >
> > This bug was found when building the kernel with a toolchain that
> > defaulted to powre10, resulting in a pcrel enabled wrapper which fails
> > to link:
> >
> >   arch/powerpc/boot/wrapper.a(crt0.o): in function `p_base':
> >   (.text+0x150): call to `platform_init' lacks nop, can't restore toc; (toc save/adjust stub)
> >   (.text+0x154): call to `start' lacks nop, can't restore toc; (toc save/adjust stub)
> >   powerpc64le-buildroot-linux-gnu-ld: final link failed: bad value
> >
> > Even with tha bug worked around the resulting kernel would crash on a
> > power9 box:
> >
> >   $ qemu-system-ppc64 -nographic -nodefaults -M powernv9 -kernel arch/powerpc/boot/zImage.epapr -serial mon:stdio
> >   [    7.069331356,5] INIT: Starting kernel at 0x20010020, fdt at 0x3068c628 25694 bytes
> >   [    7.130374661,3] ***********************************************
> >   [    7.131072886,3] Fatal Exception 0xe40 at 00000000200101e4    MSR 9000000000000001
> >   [    7.131290613,3] CFAR : 000000002001027c MSR  : 9000000000000001
> >   [    7.131433759,3] SRR0 : 0000000020010050 SRR1 : 9000000000000001
> >   [    7.131577775,3] HSRR0: 00000000200101e4 HSRR1: 9000000000000001
> >   [    7.131733687,3] DSISR: 00000000         DAR  : 0000000000000000
> >   [    7.131905162,3] LR   : 0000000020010280 CTR  : 0000000000000000
> >   [    7.132068356,3] CR   : 44002004         XER  : 00000000
> >
> > Link: https://github.com/linuxppc/issues/issues/400
> > Signed-off-by: Joel Stanley <joel@jms.id.au>
> > ---
> > Tested:
> >
> >   - ppc64le_defconfig
> >   - pseries and powernv qemu, for power8, power9, power10 cpus
> >   - buildroot compiler that defaults to -mcpu=power10 (gcc 10.3.0, ld 2.36.1)
> >   -  RHEL9 cross compilers (gcc 11.2.1-1, ld 2.35.2-17.el9)
> >
> > All decompressed and made it into the kernel ok.
> >
> > ppc64_defconfig did not work, as we've got a regression when the wrapper
> > is built for big endian. It hasn't worked for zImage.pseries for a long
> > time (at least v4.14), and broke some time between v5.4 and v5.17 for
> > zImage.epapr.
> >
> >   arch/powerpc/boot/Makefile | 8 ++++++--
> >   1 file changed, 6 insertions(+), 2 deletions(-)
> >
> > diff --git a/arch/powerpc/boot/Makefile b/arch/powerpc/boot/Makefile
> > index 9993c6256ad2..1f5cc401bfc0 100644
> > --- a/arch/powerpc/boot/Makefile
> > +++ b/arch/powerpc/boot/Makefile
> > @@ -38,9 +38,13 @@ BOOTCFLAGS    := -Wall -Wundef -Wstrict-prototypes -Wno-trigraphs \
> >                $(LINUXINCLUDE)
> >
> >   ifdef CONFIG_PPC64_BOOT_WRAPPER
> > -BOOTCFLAGS   += -m64
> > +ifdef CONFIG_CPU_LITTLE_ENDIAN
> > +BOOTCFLAGS   += -m64 -mcpu=powerpc64le
> >   else
> > -BOOTCFLAGS   += -m32
> > +BOOTCFLAGS   += -m64 -mcpu=powerpc64
> > +endif
> > +else
> > +BOOTCFLAGS   += -m32 -mcpu=powerpc
> >   endif
> >
> >   BOOTCFLAGS  += -isystem $(shell $(BOOTCC) -print-file-name=include)
>
> I think it was a fortunate coincidence that the default cpu type of your gcc is
> compatible with your system.  If the distro gcc moves its default to a newer cpu
> type than your system, this bug would happen again.

Perhaps I needed to be clear in my commit message: that's the exact
bug I'm looking to avoid. I have a buildroot toolchain that was built
for -mcpu=power10.

I think you're suggesting the -mcpu=powerpc64 option will change it 's
behavior depending on the default. From my reading of the man page, I
don't think that's true.

I did a little test using my buildroot compiler which has
with-cpu=power10. I used the presence of PCREL relocations as evidence
that it was build for power10.

$ powerpc64le-buildroot-linux-gnu-gcc -mcpu=power10 -c test.c
$ readelf -r test.o |grep -c PCREL
24
$ powerpc64le-buildroot-linux-gnu-gcc -c test.c
$ readelf -r test.o |grep -c PCREL
24
$ powerpc64le-buildroot-linux-gnu-gcc -mcpu=powerpc64le -c test.c
$ readelf -r test.o |grep -c PCREL
0

>
> The command "gcc -v |& grep with-cpu" will show you the default cpu type for 32
> and 64-bit that gcc was configured.

Just a headss up: this gives me no output for the 64 bit compilers on my laptop:

$ powerpc64le-linux-gnu-gcc -v |& grep  with-cpu
$ echo $?
1

$ powerpc64-linux-gnu-gcc -v |& grep  with-cpu
$ echo $?
1

It reports --with-cpu=default32 for the 32 bit compiler.

>
> Considering the CONFIG_TARGET_CPU for BOOTCFLAGS would bring some level of
> consistency between CFLAGS and BOOTCFLAGS regarding -mcpu value.
>
> We could mimic the behaviour from arch/powerpc/Makefile:

This was the inspiration for my change. I first took it verbatim, and
then did a bit of reading about what -mcpu actually sets. Reading the
GCC source it seems powerpc64le is equivalent to power8. powerpc64 is
less clear.

So I a agree with your suggestion. Hopefully my patch has the equivalent result.


>
>      166 ifdef config_ppc_book3s_64
>      167 ifdef config_cpu_little_endian
>      168 cflags-$(config_generic_cpu) += -mcpu=power8
>      169 cflags-$(config_generic_cpu) += $(call cc-option,-mtune=power9,-mtune=power8)
>      170 else
>      171 cflags-$(config_generic_cpu) += $(call cc-option,-mtune=power7,$(call cc-option,-mtune=power5))
>      172 cflags-$(config_generic_cpu) += $(call cc-option,-mcpu=power5,-mcpu=power4)
>      173 endif
>      174 else ifdef config_ppc_book3e_64
>      175 cflags-$(config_generic_cpu) += -mcpu=powerpc64
>      176 endif
>      ...
>      185 CFLAGS-$(CONFIG_TARGET_CPU_BOOL) += $(call cc-option,-mcpu=$(CONFIG_TARGET_CPU))
>
> Cheers!
>
> --
> Murilo

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] powerpc/boot: Build wrapper for an appropriate CPU
  2022-03-31  5:01   ` Joel Stanley
@ 2022-03-31 15:19     ` Murilo Opsfelder Araújo
  2022-03-31 23:44       ` Segher Boessenkool
  0 siblings, 1 reply; 13+ messages in thread
From: Murilo Opsfelder Araújo @ 2022-03-31 15:19 UTC (permalink / raw)
  To: Joel Stanley; +Cc: linuxppc-dev

Hi, Joel.

On 3/31/22 02:01, Joel Stanley wrote:
> On Thu, 31 Mar 2022 at 02:05, Murilo Opsfelder Araújo
> <mopsfelder@gmail.com> wrote:
>>
>> Hi, Joel.
>>
>> On 3/30/22 08:24, Joel Stanley wrote:
>>> Currently the boot wrapper lacks a -mcpu option, so it will be built for
>>> the toolchain's default cpu. This is a problem if the toolchain defaults
>>> to a cpu with newer instructions.
>>>
>>> We could wire in TARGET_CPU but instead use the oldest supported option
>>> so the wrapper runs anywhere.
>>>
>>> The GCC documentation stays that -mcpu=powerpc64le will give us a
>>> generic 64 bit powerpc machine:
>>>
>>>    -mcpu=powerpc, -mcpu=powerpc64, and -mcpu=powerpc64le specify pure
>>>    32-bit PowerPC (either endian), 64-bit big endian PowerPC and 64-bit
>>>    little endian PowerPC architecture machine types, with an appropriate,
>>>    generic processor model assumed for scheduling purposes.
>>>
>>> So do that for each of the three machines.
>>>
>>> This bug was found when building the kernel with a toolchain that
>>> defaulted to powre10, resulting in a pcrel enabled wrapper which fails
>>> to link:
>>>
>>>    arch/powerpc/boot/wrapper.a(crt0.o): in function `p_base':
>>>    (.text+0x150): call to `platform_init' lacks nop, can't restore toc; (toc save/adjust stub)
>>>    (.text+0x154): call to `start' lacks nop, can't restore toc; (toc save/adjust stub)
>>>    powerpc64le-buildroot-linux-gnu-ld: final link failed: bad value
>>>
>>> Even with tha bug worked around the resulting kernel would crash on a
>>> power9 box:
>>>
>>>    $ qemu-system-ppc64 -nographic -nodefaults -M powernv9 -kernel arch/powerpc/boot/zImage.epapr -serial mon:stdio
>>>    [    7.069331356,5] INIT: Starting kernel at 0x20010020, fdt at 0x3068c628 25694 bytes
>>>    [    7.130374661,3] ***********************************************
>>>    [    7.131072886,3] Fatal Exception 0xe40 at 00000000200101e4    MSR 9000000000000001
>>>    [    7.131290613,3] CFAR : 000000002001027c MSR  : 9000000000000001
>>>    [    7.131433759,3] SRR0 : 0000000020010050 SRR1 : 9000000000000001
>>>    [    7.131577775,3] HSRR0: 00000000200101e4 HSRR1: 9000000000000001
>>>    [    7.131733687,3] DSISR: 00000000         DAR  : 0000000000000000
>>>    [    7.131905162,3] LR   : 0000000020010280 CTR  : 0000000000000000
>>>    [    7.132068356,3] CR   : 44002004         XER  : 00000000
>>>
>>> Link: https://github.com/linuxppc/issues/issues/400
>>> Signed-off-by: Joel Stanley <joel@jms.id.au>
>>> ---
>>> Tested:
>>>
>>>    - ppc64le_defconfig
>>>    - pseries and powernv qemu, for power8, power9, power10 cpus
>>>    - buildroot compiler that defaults to -mcpu=power10 (gcc 10.3.0, ld 2.36.1)
>>>    -  RHEL9 cross compilers (gcc 11.2.1-1, ld 2.35.2-17.el9)
>>>
>>> All decompressed and made it into the kernel ok.
>>>
>>> ppc64_defconfig did not work, as we've got a regression when the wrapper
>>> is built for big endian. It hasn't worked for zImage.pseries for a long
>>> time (at least v4.14), and broke some time between v5.4 and v5.17 for
>>> zImage.epapr.
>>>
>>>    arch/powerpc/boot/Makefile | 8 ++++++--
>>>    1 file changed, 6 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/arch/powerpc/boot/Makefile b/arch/powerpc/boot/Makefile
>>> index 9993c6256ad2..1f5cc401bfc0 100644
>>> --- a/arch/powerpc/boot/Makefile
>>> +++ b/arch/powerpc/boot/Makefile
>>> @@ -38,9 +38,13 @@ BOOTCFLAGS    := -Wall -Wundef -Wstrict-prototypes -Wno-trigraphs \
>>>                 $(LINUXINCLUDE)
>>>
>>>    ifdef CONFIG_PPC64_BOOT_WRAPPER
>>> -BOOTCFLAGS   += -m64
>>> +ifdef CONFIG_CPU_LITTLE_ENDIAN
>>> +BOOTCFLAGS   += -m64 -mcpu=powerpc64le
>>>    else
>>> -BOOTCFLAGS   += -m32
>>> +BOOTCFLAGS   += -m64 -mcpu=powerpc64
>>> +endif
>>> +else
>>> +BOOTCFLAGS   += -m32 -mcpu=powerpc
>>>    endif
>>>
>>>    BOOTCFLAGS  += -isystem $(shell $(BOOTCC) -print-file-name=include)
>>
>> I think it was a fortunate coincidence that the default cpu type of your gcc is
>> compatible with your system.  If the distro gcc moves its default to a newer cpu
>> type than your system, this bug would happen again.
> 
> Perhaps I needed to be clear in my commit message: that's the exact
> bug I'm looking to avoid. I have a buildroot toolchain that was built
> for -mcpu=power10.
> 
> I think you're suggesting the -mcpu=powerpc64 option will change it 's
> behavior depending on the default. From my reading of the man page, I
> don't think that's true.

Looking at GCC source code (gcc/config/rs6000/rs6000.h:287), -mcpu=powerpc64 seems
to select "rs64" as the default cpu type.

     284 /* Define generic processor types based upon current deployment.  */
     285 #define PROCESSOR_COMMON    PROCESSOR_PPC601
     286 #define PROCESSOR_POWERPC   PROCESSOR_PPC604
     287 #define PROCESSOR_POWERPC64 PROCESSOR_RS64A

Then in gcc/config/rs6000/linux64.h:77 it sets the 64-bit default with power8.

     74 #undef  PROCESSOR_DEFAULT
     75 #define PROCESSOR_DEFAULT PROCESSOR_POWER7
     76 #undef  PROCESSOR_DEFAULT64
     77 #define PROCESSOR_DEFAULT64 PROCESSOR_POWER8

My understanding is that the default cpu type for -mcpu=powerpc64 can change.
If that change is unlikely to happen, that's a separate discussion.

> 
> I did a little test using my buildroot compiler which has
> with-cpu=power10. I used the presence of PCREL relocations as evidence
> that it was build for power10.
> 
> $ powerpc64le-buildroot-linux-gnu-gcc -mcpu=power10 -c test.c
> $ readelf -r test.o |grep -c PCREL
> 24

It respected the -mcpu=power10 you provided.

> $ powerpc64le-buildroot-linux-gnu-gcc -c test.c
> $ readelf -r test.o |grep -c PCREL
> 24

You didn't provide -mcpu, the default --with-cpu=power10 was used.

> $ powerpc64le-buildroot-linux-gnu-gcc -mcpu=powerpc64le -c test.c
> $ readelf -r test.o |grep -c PCREL
> 0

It likely defaulted to power8.

And that's my concern.  We're relying on the compiler default cpu type.

If gcc defaults -mcpu=powerpc64le to power10, you're going to have
the same problem again.  It happens that the power8 default cpu type
is compatible to your system by coincidence.

> 
>>
>> The command "gcc -v |& grep with-cpu" will show you the default cpu type for 32
>> and 64-bit that gcc was configured.
> 
> Just a headss up: this gives me no output for the 64 bit compilers on my laptop:
> 
> $ powerpc64le-linux-gnu-gcc -v |& grep  with-cpu
> $ echo $?
> 1
> 
> $ powerpc64-linux-gnu-gcc -v |& grep  with-cpu
> $ echo $?
> 1
> 
> It reports --with-cpu=default32 for the 32 bit compiler.
> 

This is what native gcc version 11.2.1 reports on Fedora 35 ppc64le:

     --with-cpu-32=power8 --with-cpu-64=power8

>>
>> Considering the CONFIG_TARGET_CPU for BOOTCFLAGS would bring some level of
>> consistency between CFLAGS and BOOTCFLAGS regarding -mcpu value.
>>
>> We could mimic the behaviour from arch/powerpc/Makefile:
> 
> This was the inspiration for my change. I first took it verbatim, and
> then did a bit of reading about what -mcpu actually sets. Reading the
> GCC source it seems powerpc64le is equivalent to power8. powerpc64 is
> less clear.

In gcc/config/rs6000/rs6000-cpus.def, they are set to different processors:

     254 RS6000_CPU ("powerpc64", PROCESSOR_POWERPC64, MASK_PPC_GFXOPT | MASK_POWERPC64)
     255 RS6000_CPU ("powerpc64le", PROCESSOR_POWER8, MASK_POWERPC64 | ISA_2_7_MASKS_SERVER

> 
> So I a agree with your suggestion. Hopefully my patch has the equivalent result.

My suggestion was to explicitly set -mcpu=power8 instead of -mcpu=powerpc64le.

We can set -mcpu=power8 for generic cpu, and then override it with target cpu.

That would be consistent with arch/powerpc/Makefile.

> 
> 
>>
>>       166 ifdef config_ppc_book3s_64
>>       167 ifdef config_cpu_little_endian
>>       168 cflags-$(config_generic_cpu) += -mcpu=power8
>>       169 cflags-$(config_generic_cpu) += $(call cc-option,-mtune=power9,-mtune=power8)
>>       170 else
>>       171 cflags-$(config_generic_cpu) += $(call cc-option,-mtune=power7,$(call cc-option,-mtune=power5))
>>       172 cflags-$(config_generic_cpu) += $(call cc-option,-mcpu=power5,-mcpu=power4)
>>       173 endif
>>       174 else ifdef config_ppc_book3e_64
>>       175 cflags-$(config_generic_cpu) += -mcpu=powerpc64
>>       176 endif
>>       ...
>>       185 CFLAGS-$(CONFIG_TARGET_CPU_BOOL) += $(call cc-option,-mcpu=$(CONFIG_TARGET_CPU))
>>
>> Cheers!
>>
>> --
>> Murilo

Cheers!

-- 
Murilo

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] powerpc/boot: Build wrapper for an appropriate CPU
  2022-03-30 11:39   ` Joel Stanley
@ 2022-03-31 23:00     ` Segher Boessenkool
  0 siblings, 0 replies; 13+ messages in thread
From: Segher Boessenkool @ 2022-03-31 23:00 UTC (permalink / raw)
  To: Joel Stanley; +Cc: linuxppc-dev

On Wed, Mar 30, 2022 at 11:39:13AM +0000, Joel Stanley wrote:
> Last one wins as far as -mcpu lines goes, from a quick test.

This is true yes.  It is true for GCC options in general.


Segher

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] powerpc/boot: Build wrapper for an appropriate CPU
  2022-03-31  2:05 ` Murilo Opsfelder Araújo
  2022-03-31  5:01   ` Joel Stanley
@ 2022-03-31 23:09   ` Segher Boessenkool
  1 sibling, 0 replies; 13+ messages in thread
From: Segher Boessenkool @ 2022-03-31 23:09 UTC (permalink / raw)
  To: Murilo Opsfelder Araújo; +Cc: linuxppc-dev, Joel Stanley

On Wed, Mar 30, 2022 at 11:05:19PM -0300, Murilo Opsfelder Araújo wrote:
> I think it was a fortunate coincidence that the default cpu type of your 
> gcc is
> compatible with your system.  If the distro gcc moves its default to a 
> newer cpu
> type than your system, this bug would happen again.

Indeed.  But why would you use a GCC from a distro that requires p10
when you target something older?

> The command "gcc -v |& grep with-cpu" will show you the default cpu type 
> for 32
> and 64-bit that gcc was configured.

Only if it was configured with --with-cpu*.  Most people do not.  If
someone builds compilers with non-default defaults like this, they had
better communicate that clearly to all their users, to avoid confusion
and disappointments.

There should be some easy way to show this default with GCC, but there
currently is none.  I'll see what I can do (just to make my own life
easier, we frequently get bug reports from people who use a different
-mcpu= than what they think they do :-) )


Segher

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] powerpc/boot: Build wrapper for an appropriate CPU
  2022-03-31 15:19     ` Murilo Opsfelder Araújo
@ 2022-03-31 23:44       ` Segher Boessenkool
  2022-04-06  7:08         ` Joel Stanley
  0 siblings, 1 reply; 13+ messages in thread
From: Segher Boessenkool @ 2022-03-31 23:44 UTC (permalink / raw)
  To: Murilo Opsfelder Araújo; +Cc: linuxppc-dev, Joel Stanley

On Thu, Mar 31, 2022 at 12:19:52PM -0300, Murilo Opsfelder Araújo wrote:
> My understanding is that the default cpu type for -mcpu=powerpc64 can 
> change.

Different subtargets (Linux, AIX, Darwin, the various BSDs, bare ELF,
etc.) have different default CPUs.  It also can be set at configure time
for most subtargets.

Linux can be built with compilers not targetting *-linux*, so it would
be best to specify a specific CPU always.

> >I did a little test using my buildroot compiler which has
> >with-cpu=power10. I used the presence of PCREL relocations as evidence
> >that it was build for power10.
> >
> >$ powerpc64le-buildroot-linux-gnu-gcc -mcpu=power10 -c test.c
> >$ readelf -r test.o |grep -c PCREL
> >24
> 
> It respected the -mcpu=power10 you provided.

Of course.

> And that's my concern.  We're relying on the compiler default cpu type.

That is not the compiler default.  It is the default from who built the
compiler.  It can vary wildly and unpredictably.

The actual compiler default will not change so easily at all, basically
only when its subtarget drops support for an older CPU.

> If gcc defaults -mcpu=powerpc64le to power10, you're going to have
> the same problem again.

That will not happen before power10 is the minimum supported CPU.
Anything else is madness.

> It happens that the power8 default cpu type
> is compatible to your system by coincidence.

No, power8 is (and always was) the minimum supported CPU type for
powerpc64le-linux.

> In gcc/config/rs6000/rs6000-cpus.def, they are set to different processors:
> 
>     254 RS6000_CPU ("powerpc64", PROCESSOR_POWERPC64, MASK_PPC_GFXOPT | 
>     MASK_POWERPC64)
>     255 RS6000_CPU ("powerpc64le", PROCESSOR_POWER8, MASK_POWERPC64 | 
>     ISA_2_7_MASKS_SERVER

Those can and will change though, over time.  But -mcpu=powerpc64 (etc.)
always will mean what the current documentation says it does:
     '-mcpu=powerpc', '-mcpu=powerpc64', and '-mcpu=powerpc64le' specify
     pure 32-bit PowerPC (either endian), 64-bit big endian PowerPC and
     64-bit little endian PowerPC architecture machine types, with an
     appropriate, generic processor model assumed for scheduling
     purposes.

> My suggestion was to explicitly set -mcpu=power8 instead of 
> -mcpu=powerpc64le.

That is implied anyway, it is the minimum supported for
powerpc64le-linux.  Using -mcpu=powerpc64le might schedule better for
newer CPUs, in the future (but the code will always work on all still
supported CPUs).


Segher

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] powerpc/boot: Build wrapper for an appropriate CPU
  2022-03-31 23:44       ` Segher Boessenkool
@ 2022-04-06  7:08         ` Joel Stanley
  2022-04-07 18:43           ` Murilo Opsfelder Araújo
  0 siblings, 1 reply; 13+ messages in thread
From: Joel Stanley @ 2022-04-06  7:08 UTC (permalink / raw)
  To: Segher Boessenkool, Murilo Opsfelder Araújo; +Cc: linuxppc-dev

On Thu, 31 Mar 2022 at 23:46, Segher Boessenkool
<segher@kernel.crashing.org> wrote:
>
> On Thu, Mar 31, 2022 at 12:19:52PM -0300, Murilo Opsfelder Araújo wrote:
> > My understanding is that the default cpu type for -mcpu=powerpc64 can
> > change.
>
> Different subtargets (Linux, AIX, Darwin, the various BSDs, bare ELF,
> etc.) have different default CPUs.  It also can be set at configure time
> for most subtargets.
>
> Linux can be built with compilers not targetting *-linux*, so it would
> be best to specify a specific CPU always.
>
> > >I did a little test using my buildroot compiler which has
> > >with-cpu=power10. I used the presence of PCREL relocations as evidence
> > >that it was build for power10.
> > >
> > >$ powerpc64le-buildroot-linux-gnu-gcc -mcpu=power10 -c test.c
> > >$ readelf -r test.o |grep -c PCREL
> > >24
> >
> > It respected the -mcpu=power10 you provided.
>
> Of course.
>
> > And that's my concern.  We're relying on the compiler default cpu type.
>
> That is not the compiler default.  It is the default from who built the
> compiler.  It can vary wildly and unpredictably.
>
> The actual compiler default will not change so easily at all, basically
> only when its subtarget drops support for an older CPU.
>
> > If gcc defaults -mcpu=powerpc64le to power10, you're going to have
> > the same problem again.
>
> That will not happen before power10 is the minimum supported CPU.
> Anything else is madness.

Murilo, does Segher's explanation address your concerns?

I believe the patch I sent fixes the problem that you're worried
about. It should be compatible into the future too.

Cheers,

Joel

>
> > It happens that the power8 default cpu type
> > is compatible to your system by coincidence.
>
> No, power8 is (and always was) the minimum supported CPU type for
> powerpc64le-linux.
>
> > In gcc/config/rs6000/rs6000-cpus.def, they are set to different processors:
> >
> >     254 RS6000_CPU ("powerpc64", PROCESSOR_POWERPC64, MASK_PPC_GFXOPT |
> >     MASK_POWERPC64)
> >     255 RS6000_CPU ("powerpc64le", PROCESSOR_POWER8, MASK_POWERPC64 |
> >     ISA_2_7_MASKS_SERVER
>
> Those can and will change though, over time.  But -mcpu=powerpc64 (etc.)
> always will mean what the current documentation says it does:
>      '-mcpu=powerpc', '-mcpu=powerpc64', and '-mcpu=powerpc64le' specify
>      pure 32-bit PowerPC (either endian), 64-bit big endian PowerPC and
>      64-bit little endian PowerPC architecture machine types, with an
>      appropriate, generic processor model assumed for scheduling
>      purposes.
>
> > My suggestion was to explicitly set -mcpu=power8 instead of
> > -mcpu=powerpc64le.
>
> That is implied anyway, it is the minimum supported for
> powerpc64le-linux.  Using -mcpu=powerpc64le might schedule better for
> newer CPUs, in the future (but the code will always work on all still
> supported CPUs).
>
>
> Segher

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] powerpc/boot: Build wrapper for an appropriate CPU
  2022-04-06  7:08         ` Joel Stanley
@ 2022-04-07 18:43           ` Murilo Opsfelder Araújo
  2022-04-07 20:33             ` Segher Boessenkool
  0 siblings, 1 reply; 13+ messages in thread
From: Murilo Opsfelder Araújo @ 2022-04-07 18:43 UTC (permalink / raw)
  To: Joel Stanley, Segher Boessenkool; +Cc: linuxppc-dev

On 4/6/22 04:08, Joel Stanley wrote:
> On Thu, 31 Mar 2022 at 23:46, Segher Boessenkool
> <segher@kernel.crashing.org> wrote:
>>
>> On Thu, Mar 31, 2022 at 12:19:52PM -0300, Murilo Opsfelder Araújo wrote:
>>> My understanding is that the default cpu type for -mcpu=powerpc64 can
>>> change.
>>
>> Different subtargets (Linux, AIX, Darwin, the various BSDs, bare ELF,
>> etc.) have different default CPUs.  It also can be set at configure time
>> for most subtargets.
>>
>> Linux can be built with compilers not targetting *-linux*, so it would
>> be best to specify a specific CPU always.
>>
>>>> I did a little test using my buildroot compiler which has
>>>> with-cpu=power10. I used the presence of PCREL relocations as evidence
>>>> that it was build for power10.
>>>>
>>>> $ powerpc64le-buildroot-linux-gnu-gcc -mcpu=power10 -c test.c
>>>> $ readelf -r test.o |grep -c PCREL
>>>> 24
>>>
>>> It respected the -mcpu=power10 you provided.
>>
>> Of course.
>>
>>> And that's my concern.  We're relying on the compiler default cpu type.
>>
>> That is not the compiler default.  It is the default from who built the
>> compiler.  It can vary wildly and unpredictably.
>>
>> The actual compiler default will not change so easily at all, basically
>> only when its subtarget drops support for an older CPU.
>>
>>> If gcc defaults -mcpu=powerpc64le to power10, you're going to have
>>> the same problem again.
>>
>> That will not happen before power10 is the minimum supported CPU.
>> Anything else is madness.
> 
> Murilo, does Segher's explanation address your concerns?

The comment:

"Different subtargets (Linux, AIX, Darwin, the various BSDs, bare ELF,
etc.) have different default CPUs.  It also can be set at configure time
for most subtargets.

Linux can be built with compilers not targetting *-linux*, so it would
be best to specify a specific CPU always."

made me think that it's better to specify -mcpu=power8 instead of -mcpu=powerpc64le
because of such compilers not targetting *-linux*.

Did I understand Segher's comment correctly?  To be honest, I don't know
how much concerned we should be about this scenario.

Just for the sake of consistency, if we decide to go with -mcpu=powerpc64le,
then I think we should also change arch/powerpc/Makefile CFLAGS.
Otherwise, we could follow what we already have in the tree and use
-mcpu=power8 in BOOTCLAGS, too.

Practically speaking, either way works for us.  In any case:

Reviewed-by: Murilo Opsfelder Araujo <muriloo@linux.ibm.com>

> 
> I believe the patch I sent fixes the problem that you're worried
> about. It should be compatible into the future too.
> 
> Cheers,
> 
> Joel
> 
>>
>>> It happens that the power8 default cpu type
>>> is compatible to your system by coincidence.
>>
>> No, power8 is (and always was) the minimum supported CPU type for
>> powerpc64le-linux.
>>
>>> In gcc/config/rs6000/rs6000-cpus.def, they are set to different processors:
>>>
>>>      254 RS6000_CPU ("powerpc64", PROCESSOR_POWERPC64, MASK_PPC_GFXOPT |
>>>      MASK_POWERPC64)
>>>      255 RS6000_CPU ("powerpc64le", PROCESSOR_POWER8, MASK_POWERPC64 |
>>>      ISA_2_7_MASKS_SERVER
>>
>> Those can and will change though, over time.  But -mcpu=powerpc64 (etc.)
>> always will mean what the current documentation says it does:
>>       '-mcpu=powerpc', '-mcpu=powerpc64', and '-mcpu=powerpc64le' specify
>>       pure 32-bit PowerPC (either endian), 64-bit big endian PowerPC and
>>       64-bit little endian PowerPC architecture machine types, with an
>>       appropriate, generic processor model assumed for scheduling
>>       purposes.
>>
>>> My suggestion was to explicitly set -mcpu=power8 instead of
>>> -mcpu=powerpc64le.
>>
>> That is implied anyway, it is the minimum supported for
>> powerpc64le-linux.  Using -mcpu=powerpc64le might schedule better for
>> newer CPUs, in the future (but the code will always work on all still
>> supported CPUs).
>>
>>
>> Segher

-- 
Murilo

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] powerpc/boot: Build wrapper for an appropriate CPU
  2022-04-07 18:43           ` Murilo Opsfelder Araújo
@ 2022-04-07 20:33             ` Segher Boessenkool
  0 siblings, 0 replies; 13+ messages in thread
From: Segher Boessenkool @ 2022-04-07 20:33 UTC (permalink / raw)
  To: Murilo Opsfelder Araújo; +Cc: linuxppc-dev, Joel Stanley

On Thu, Apr 07, 2022 at 03:43:20PM -0300, Murilo Opsfelder Araújo wrote:
> On 4/6/22 04:08, Joel Stanley wrote:
> >On Thu, 31 Mar 2022 at 23:46, Segher Boessenkool
> ><segher@kernel.crashing.org> wrote:
> >>On Thu, Mar 31, 2022 at 12:19:52PM -0300, Murilo Opsfelder Araújo wrote:
> >>>My understanding is that the default cpu type for -mcpu=powerpc64 can
> >>>change.
> >>
> >>Different subtargets (Linux, AIX, Darwin, the various BSDs, bare ELF,
> >>etc.) have different default CPUs.  It also can be set at configure time
> >>for most subtargets.
> >>
> >>Linux can be built with compilers not targetting *-linux*, so it would
> >>be best to specify a specific CPU always.
> >>
> >>>>I did a little test using my buildroot compiler which has
> >>>>with-cpu=power10. I used the presence of PCREL relocations as evidence
> >>>>that it was build for power10.
> >>>>
> >>>>$ powerpc64le-buildroot-linux-gnu-gcc -mcpu=power10 -c test.c
> >>>>$ readelf -r test.o |grep -c PCREL
> >>>>24
> >>>
> >>>It respected the -mcpu=power10 you provided.
> >>
> >>Of course.
> >>
> >>>And that's my concern.  We're relying on the compiler default cpu type.
> >>
> >>That is not the compiler default.  It is the default from who built the
> >>compiler.  It can vary wildly and unpredictably.
> >>
> >>The actual compiler default will not change so easily at all, basically
> >>only when its subtarget drops support for an older CPU.
> >>
> >>>If gcc defaults -mcpu=powerpc64le to power10, you're going to have
> >>>the same problem again.
> >>
> >>That will not happen before power10 is the minimum supported CPU.
> >>Anything else is madness.
> >
> >Murilo, does Segher's explanation address your concerns?
> 
> The comment:
> 
> "Different subtargets (Linux, AIX, Darwin, the various BSDs, bare ELF,
> etc.) have different default CPUs.  It also can be set at configure time
> for most subtargets.
> 
> Linux can be built with compilers not targetting *-linux*, so it would
> be best to specify a specific CPU always."
> 
> made me think that it's better to specify -mcpu=power8 instead of 
> -mcpu=powerpc64le
> because of such compilers not targetting *-linux*.
> 
> Did I understand Segher's comment correctly?  To be honest, I don't know
> how much concerned we should be about this scenario.

That is the long and short of it, yes.  This matters for reproducible
builds if you care for more exotic compilers (which are very common for
32 bit, but not so much for 64 bit).

> Just for the sake of consistency, if we decide to go with -mcpu=powerpc64le,
> then I think we should also change arch/powerpc/Makefile CFLAGS.
> Otherwise, we could follow what we already have in the tree and use
> -mcpu=power8 in BOOTCLAGS, too.
> 
> Practically speaking, either way works for us.  In any case:

+1

> Reviewed-by: Murilo Opsfelder Araujo <muriloo@linux.ibm.com>
Reviewed-by: Segher Boessenkool <segher@kernel.crashing.org>


Segher

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] powerpc/boot: Build wrapper for an appropriate CPU
  2022-03-30 11:24 [PATCH] powerpc/boot: Build wrapper for an appropriate CPU Joel Stanley
  2022-03-30 11:33 ` Christophe Leroy
  2022-03-31  2:05 ` Murilo Opsfelder Araújo
@ 2022-05-15 10:12 ` Michael Ellerman
  2 siblings, 0 replies; 13+ messages in thread
From: Michael Ellerman @ 2022-05-15 10:12 UTC (permalink / raw)
  To: linuxppc-dev, Joel Stanley

On Wed, 30 Mar 2022 21:54:37 +1030, Joel Stanley wrote:
> Currently the boot wrapper lacks a -mcpu option, so it will be built for
> the toolchain's default cpu. This is a problem if the toolchain defaults
> to a cpu with newer instructions.
> 
> We could wire in TARGET_CPU but instead use the oldest supported option
> so the wrapper runs anywhere.
> 
> [...]

Applied to powerpc/next.

[1/1] powerpc/boot: Build wrapper for an appropriate CPU
      https://git.kernel.org/powerpc/c/40a75584e526cc489234dac0897cd599e6013483

cheers

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2022-05-15 10:18 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-03-30 11:24 [PATCH] powerpc/boot: Build wrapper for an appropriate CPU Joel Stanley
2022-03-30 11:33 ` Christophe Leroy
2022-03-30 11:39   ` Joel Stanley
2022-03-31 23:00     ` Segher Boessenkool
2022-03-31  2:05 ` Murilo Opsfelder Araújo
2022-03-31  5:01   ` Joel Stanley
2022-03-31 15:19     ` Murilo Opsfelder Araújo
2022-03-31 23:44       ` Segher Boessenkool
2022-04-06  7:08         ` Joel Stanley
2022-04-07 18:43           ` Murilo Opsfelder Araújo
2022-04-07 20:33             ` Segher Boessenkool
2022-03-31 23:09   ` Segher Boessenkool
2022-05-15 10:12 ` Michael Ellerman

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.