Messages in this thread | | | Date | Mon, 4 Jul 2022 15:13:58 +0200 | From | Pali Rohár <> | Subject | Re: [PATCH] powerpc: e500: Fix compilation with gcc e500 compiler |
| |
On Monday 04 July 2022 14:07:10 Arnd Bergmann wrote: > On Mon, Jul 4, 2022 at 12:39 PM Pali Rohár <pali@kernel.org> wrote: > > On Monday 04 July 2022 20:23:29 Michael Ellerman wrote: > > > On 2 July 2022 7:44:05 pm AEST, "Pali Rohár" <pali@kernel.org> wrote: > > > >On Tuesday 24 May 2022 11:39:39 Pali Rohár wrote: > > > >> gcc e500 compiler does not support -mcpu=powerpc option. When it is > > > >> specified then gcc throws compile error: > > > >> > > > >> gcc: error: unrecognized argument in option ‘-mcpu=powerpc’ > > > >> gcc: note: valid arguments to ‘-mcpu=’ are: 8540 8548 native > > > >> > > > >> So do not set -mcpu=powerpc option when CONFIG_E500 is set. Correct option > > > >> -mcpu=8540 for CONFIG_E500 is set few lines below in that Makefile. > > > >> > > > >> Signed-off-by: Pali Rohár <pali@kernel.org> > > > >> Cc: stable@vger.kernel.org > > > > > > > >Michael, do you have any objections about this patch? > > > > > > I don't particularly like it :) > > > > > > From the discussion with Segher, it sounds like this is a problem with a specific build of gcc that you're using, not a general problem with gcc built with e500 support. > > > > Well, the "full" build of gcc for e500 cores with SPE does not support > > -mcpu=powerpc option. So I think this is a general problem. I do not > > think that this is "specific build" as this is the correct build of gcc > > for these processors with e500 cores. > > > > "stripped". build of gcc without SPE support for e500 cores does not > > have this problem... > > I can see a couple of problems with the CPU selection, but I don't think > this is a major one, as nobody should be using those SPE compilers for > building the kernel. Just use a modern powerpc-gcc build.
The point is to use same compiler for building kernel as for the all other parts of the system.
I just do not see reason why for kernel it is needed to build completely different toolchain and compiler.
> > > Keying it off CONFIG_E500 means it will fix your problem, but not anyone else who has a different non-e500 compiler that also doesn't support -mcpu=powerpc (for whatever reason). > > > > > > So I wonder if a better fix is to use cc-option when setting -mcpu=powerpc. > > > > > > > Comment for that code which adds -mpcu=powerpc says: > > > > they are needed to set a sane 32-bit cpu target for the 64-bit cross > > compiler which may default to the wrong ISA. > > > > So I'm not sure how to handle this in other way. GCC uses -mpcu=8540 > > option for specifying to compile code for e500 cores and seems that > > -mcpu=8540 is supported by all e500 compilers... > > > > Few lines below is code > > > > CFLAGS-$(CONFIG_E500) += $(call cc-option,-mcpu=8540 -msoft-float,-mcpu=powerpc) > > > > which for e500 kernel builds user either -mcpu=8540 or -mcpu=powerpc > > (probably as a fallback if -mcpu=8540 is not supported). > > The -mcpu=powerpc fallback can probably be skipped here, that must have been > for compilers predating the addition of -mcpu=8540, and even the oldest ones > support that now.
Ok, makes sense.
> > So for me it looks like that problematic code > > > > KBUILD_CFLAGS += -mcpu=powerpc > > KBUILD_AFLAGS += -mcpu=powerpc > > > > needs to be somehow skipped when compiling for CONFIG_E500. > >> My change which skips that code base on ifndef CONFIG_E500 should be > > fine as when CONFIG_E500 is disabled it does nothing and when it is > > enabled then code > > > > CFLAGS-$(CONFIG_E500) += $(call cc-option,-mcpu=8540 -msoft-float,-mcpu=powerpc) > > > > is called which sets -mcpu option suitable for e500. > > I think this part is indeed fishy, but adding another special case for E500 > seems to take it in the wrong direction. > > Nick added this in 4bf4f42a2feb ("powerpc/kbuild: Set default generic > machine type > for 32-bit compile") as a compile-time fix to prevent the default target from > getting used when the compiler supports both 64-bit and 32-bit. This is the > right idea, but it's inconsistent to pass different flags depending on the type > of toolchain, and it loses the more specific options. > > Another problem I see is that a kernel that is built for both E500 and E500MC > uses -mcpu=e500mc and may not actually work on the older ones either > (even with your patch).
That is probably truth, -mcpu=8540 should have been chosen. (Anyway it should have been called -mcpu=e500, no idea why gcc still name it 8540.)
> I think what you actually want is to set one option for each of the > possible CPU types: > > CFLAGS_CPU-$(CONFIG_PPC_BOOK3S_32) := -mcpu=powerpc > CFLAGS_CPU-$(CONFIG_PPC_85xx) := -mcpu=8540 > CFLAGS_CPU-$(CONFIG_PPC8xx) := -mcpu=860 > CFLAGS_CPU-$(CONFIG_PPC44x) := -mcpu=440 > CFLAGS_CPU-$(CONFIG_PPC40x) := -mcpu=405 > ifdef CONFIG_CPU_LITTLE_ENDIAN > CFLAGS_CPU-$(CONFIG_BOOK3S_64) := -mcpu=power8 > else > CFLAGS_CPU-$(CONFIG_BOOK3S_64) := -mcpu=power5 > endif > CFLAGS_CPU-$(CONFIG_BOOK3E_64) := -mcpu=powerpc64
Yes, this is something I would expect that in Makefile should be.
But what to do with fallback value?
> For the non-generic CPU types, there is also CONFIG_TARGET_CPU, > and the list above could just get folded into that instead. > > Arnd
| |