[PATCH] powerpc/boot: Build wrapper for an appropriate CPU

Thu Mar 31 16:01:00 AEDT 2022

On Thu, 31 Mar 2022 at 02:05, Murilo Opsfelder Araújo
<mopsfelder at 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 at 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