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