On Monday 04 July 2022 14:07:10 Arnd Bergmann wrote:

On Mon, Jul 4, 2022 at 12:39 PM Pali Rohár [off-list ref] wrote:

quoted

On Monday 04 July 2022 20:23:29 Michael Ellerman wrote:

quoted

On 2 July 2022 7:44:05 pm AEST, "Pali Rohár" [off-list ref] wrote:

quoted

On Tuesday 24 May 2022 11:39:39 Pali Rohár wrote:

quoted

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.

quoted

quoted

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.

quoted

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.

quoted

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