[PATCH] powerpc: e500: Fix compilation with gcc e500 compiler

[PATCH] powerpc: e500: Fix compilation with gcc e500 compiler

[Pali Rohár]

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
---
 arch/powerpc/Makefile | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/arch/powerpc/Makefile b/arch/powerpc/Makefile
index eb541e730d3c..87f9f29ac9d2 100644
--- a/arch/powerpc/Makefile
+++ b/arch/powerpc/Makefile
@@ -22,11 +22,16 @@ ifdef CONFIG_PPC32
 # or platform code sets later on the command line, but they are needed
 # to set a sane 32-bit cpu target for the 64-bit cross compiler which
 # may default to the wrong ISA.
+# Never set -mcpu=powerpc option for gcc e500 compiler because this
+# option is unsupported and throws error. The correct option for
+# CONFIG_E500 is -mcpu=8540 and it is set few lines below.
+ifndef CONFIG_E500
 KBUILD_CFLAGS		+= -mcpu=powerpc
 KBUILD_AFLAGS		+= -mcpu=powerpc
 endif
 endif
 endif
+endif
 
 ifdef CONFIG_PPC_BOOK3S_32
 KBUILD_CFLAGS		+= -mcpu=powerpc
-- 
2.20.1

Re: [PATCH] powerpc: e500: Fix compilation with gcc e500 compiler

[Segher Boessenkool]

Hi!

On Tue, May 24, 2022 at 11:39:39AM +0200, 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

What?  Are you using some modified version of GCC, perhaps?  No version
of GCC that isn't hamstrung can have this output.


Segher

Re: [PATCH] powerpc: e500: Fix compilation with gcc e500 compiler

[Pali Rohár]

On Tuesday 24 May 2022 12:59:55 Segher Boessenkool wrote:
> Hi!
> 
> On Tue, May 24, 2022 at 11:39:39AM +0200, 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
> 
> What?  Are you using some modified version of GCC, perhaps?

Hello! I'm using official gcc version, no special modification.

> No version of GCC that isn't hamstrung can have this output.

gcc for e500 cores has really this output when you pass -mcpu=powerpc.

Upstream gcc dropped support for e500 cores during development of
version 9. But you can still compile and install gcc 8.5.0 (last version
of gcc 8) which has this full e500 support.

Really, you can easily try it. Debian 10 (Buster) has gcc 8.3.0 in its
default installation and also provides packages with cross compilers.
Just run 'sudo apt install gcc-powerpc-linux-gnuspe' on desktop amd64
version of Debian 10, it will install e500 cross compiler.

-mcpu=8540 specify e500v1 and -mcpu=8548 specify e500v2

Re: [PATCH] powerpc: e500: Fix compilation with gcc e500 compiler

[Segher Boessenkool]

On Tue, May 24, 2022 at 08:12:55PM +0200, Pali Rohár wrote:
> On Tuesday 24 May 2022 12:59:55 Segher Boessenkool wrote:
> > On Tue, May 24, 2022 at 11:39:39AM +0200, 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
> > 
> > What?  Are you using some modified version of GCC, perhaps?
> 
> Hello! I'm using official gcc version, no special modification.
> 
> > No version of GCC that isn't hamstrung can have this output.
> 
> gcc for e500 cores has really this output when you pass -mcpu=powerpc.
> 
> Upstream gcc dropped support for e500 cores during development of
> version 9.

This isn't true.  The SPE instruction extension is no longer supported
(because it wasn't maintained).  Everything else still works.

> But you can still compile and install gcc 8.5.0 (last version
> of gcc 8) which has this full e500 support.
> 
> Really, you can easily try it. Debian 10 (Buster) has gcc 8.3.0 in its
> default installation and also provides packages with cross compilers.
> Just run 'sudo apt install gcc-powerpc-linux-gnuspe' on desktop amd64
> version of Debian 10, it will install e500 cross compiler.
> 
> -mcpu=8540 specify e500v1 and -mcpu=8548 specify e500v2

Aha.  Right, because this config forces -mspe it requires one of these
CPUs.

You can use a powerpc-linux compiler instead, and everything will just
work.  These CPUs are still supported, in all of GCC 9 .. GCC 12 :-)


Segher

Re: [PATCH] powerpc: e500: Fix compilation with gcc e500 compiler

[Pali Rohár]

On Tuesday 24 May 2022 13:52:47 Segher Boessenkool wrote:
> On Tue, May 24, 2022 at 08:12:55PM +0200, Pali Rohár wrote:
> > On Tuesday 24 May 2022 12:59:55 Segher Boessenkool wrote:
> > > On Tue, May 24, 2022 at 11:39:39AM +0200, 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
> > > 
> > > What?  Are you using some modified version of GCC, perhaps?
> > 
> > Hello! I'm using official gcc version, no special modification.
> > 
> > > No version of GCC that isn't hamstrung can have this output.
> > 
> > gcc for e500 cores has really this output when you pass -mcpu=powerpc.
> > 
> > Upstream gcc dropped support for e500 cores during development of
> > version 9.
> 
> This isn't true.  The SPE instruction extension is no longer supported
> (because it wasn't maintained).  Everything else still works.
>
> > But you can still compile and install gcc 8.5.0 (last version
> > of gcc 8) which has this full e500 support.
> > 
> > Really, you can easily try it. Debian 10 (Buster) has gcc 8.3.0 in its
> > default installation and also provides packages with cross compilers.
> > Just run 'sudo apt install gcc-powerpc-linux-gnuspe' on desktop amd64
> > version of Debian 10, it will install e500 cross compiler.
> > 
> > -mcpu=8540 specify e500v1 and -mcpu=8548 specify e500v2
> 
> Aha.  Right, because this config forces -mspe it requires one of these
> CPUs.
> 
> You can use a powerpc-linux compiler instead, and everything will just
> work.  These CPUs are still supported, in all of GCC 9 .. GCC 12 :-)
> 
> 
> Segher

Ok. I can use different "generic" powerpc compiler (It should work fine
as you said, as it has also -mcpu=8540 option). But I think that
compilation of kernel should be supported also by that gcc 8.5.0 e500
compiler.

It is really annoying if for compiling kernel is needed different
compiler than for compiling rest of the system (userspace and
bootloader). And for user applications it should be really used e500
SPE-capable compiler due to performance reasons.

Re: [PATCH] powerpc: e500: Fix compilation with gcc e500 compiler

[Segher Boessenkool]

On Tue, May 24, 2022 at 09:16:10PM +0200, Pali Rohár wrote:
> On Tuesday 24 May 2022 13:52:47 Segher Boessenkool wrote:
> > Aha.  Right, because this config forces -mspe it requires one of these
> > CPUs.
> > 
> > You can use a powerpc-linux compiler instead, and everything will just
> > work.  These CPUs are still supported, in all of GCC 9 .. GCC 12 :-)
> 
> Ok. I can use different "generic" powerpc compiler (It should work fine
> as you said, as it has also -mcpu=8540 option). But I think that
> compilation of kernel should be supported also by that gcc 8.5.0 e500
> compiler.

That linuxspe compiler you mean.  Sure, why not, the more the merrier,
as long as it doesn't get in the way of other stuff, I won't protest.

But please don't confuse people: you are talking about a
powerpc-linuxspe compiler, not e500, which is supported just fine by
current GCC trunk still, and does not have such limitations :-)


Segher

Re: [PATCH] powerpc: e500: Fix compilation with gcc e500 compiler

[Pali Rohár]

On Tuesday 24 May 2022 14:52:16 Segher Boessenkool wrote:
> On Tue, May 24, 2022 at 09:16:10PM +0200, Pali Rohár wrote:
> > On Tuesday 24 May 2022 13:52:47 Segher Boessenkool wrote:
> > > Aha.  Right, because this config forces -mspe it requires one of these
> > > CPUs.
> > > 
> > > You can use a powerpc-linux compiler instead, and everything will just
> > > work.  These CPUs are still supported, in all of GCC 9 .. GCC 12 :-)
> > 
> > Ok. I can use different "generic" powerpc compiler (It should work fine
> > as you said, as it has also -mcpu=8540 option). But I think that
> > compilation of kernel should be supported also by that gcc 8.5.0 e500
> > compiler.
> 
> That linuxspe compiler you mean.  Sure, why not, the more the merrier,
> as long as it doesn't get in the way of other stuff, I won't protest.
> 
> But please don't confuse people: you are talking about a
> powerpc-linuxspe compiler, not e500, which is supported just fine by
> current GCC trunk still, and does not have such limitations :-)
> 
> 
> Segher

I think the confusion is the calling "generic" or "stripped" compiler
without SPE support as e500 compiler.

The "true" e500 compiler has support for both both integer and floating
point ISA and not just subset or just one type.

Re: [PATCH] powerpc: e500: Fix compilation with gcc e500 compiler

[Pali Rohár]

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?

> ---
>  arch/powerpc/Makefile | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/arch/powerpc/Makefile b/arch/powerpc/Makefile
> index eb541e730d3c..87f9f29ac9d2 100644
> --- a/arch/powerpc/Makefile
> +++ b/arch/powerpc/Makefile
> @@ -22,11 +22,16 @@ ifdef CONFIG_PPC32
>  # or platform code sets later on the command line, but they are needed
>  # to set a sane 32-bit cpu target for the 64-bit cross compiler which
>  # may default to the wrong ISA.
> +# Never set -mcpu=powerpc option for gcc e500 compiler because this
> +# option is unsupported and throws error. The correct option for
> +# CONFIG_E500 is -mcpu=8540 and it is set few lines below.
> +ifndef CONFIG_E500
>  KBUILD_CFLAGS		+= -mcpu=powerpc
>  KBUILD_AFLAGS		+= -mcpu=powerpc
>  endif
>  endif
>  endif
> +endif
>  
>  ifdef CONFIG_PPC_BOOK3S_32
>  KBUILD_CFLAGS		+= -mcpu=powerpc
> -- 
> 2.20.1
> 

Re: [PATCH] powerpc: e500: Fix compilation with gcc e500 compiler

[Michael Ellerman]

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.

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.

cheers

Re: [PATCH] powerpc: e500: Fix compilation with gcc e500 compiler

[Pali Rohár]

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...

> 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.
> 
> cheers

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).

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.


If somebody has non-e500 compiler which does not support -mcpu=8540 nor
-mpcu=powerpc then it would not be able to compile kernel -- and this
behavior is correct, as without neither of these options, kernel binary
would not run on e500 core.

And for non-e500 builds with other non-e500 compiler, this my change
does not have any function change. So I think it should be safe.


Anyway, it you have any other idea how to fix and improve this, let me
know.

Re: [PATCH] powerpc: e500: Fix compilation with gcc e500 compiler

[Arnd Bergmann]

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.

> > 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.

> 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).

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

For the non-generic CPU types, there is also CONFIG_TARGET_CPU,
and the list above could just get folded into that instead.

       Arnd

Re: [PATCH] powerpc: e500: Fix compilation with gcc e500 compiler

[Pali Rohár]

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

Re: [PATCH] powerpc: e500: Fix compilation with gcc e500 compiler

[Arnd Bergmann]

On Mon, Jul 4, 2022 at 3:13 PM Pali Rohár <pali@kernel.org> wrote:
> On Monday 04 July 2022 14:07:10 Arnd Bergmann wrote:

> > 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?

Most of the fallback values can just be removed because we don't support
building with gcc versions older than 5.1.0 any more. The only one
that I think still needs a fallback is mtune=power9, which requires gcc-6.1
or higher. CONFIG_POWER9_CPU could similarly use a
"depends on GCC_VERSION > 60100".

        Arnd

Re: [PATCH] powerpc: e500: Fix compilation with gcc e500 compiler

[Pali Rohár]

On Monday 04 July 2022 15:22:03 Arnd Bergmann wrote:
> On Mon, Jul 4, 2022 at 3:13 PM Pali Rohár <pali@kernel.org> wrote:
> > On Monday 04 July 2022 14:07:10 Arnd Bergmann wrote:
> 
> > > 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?
> 
> Most of the fallback values can just be removed because we don't support
> building with gcc versions older than 5.1.0 any more. The only one
> that I think still needs a fallback is mtune=power9, which requires gcc-6.1
> or higher. CONFIG_POWER9_CPU could similarly use a
> "depends on GCC_VERSION > 60100".
> 
>         Arnd

And still what to do with 4bf4f42a2feb ("powerpc/kbuild: Set default
generic machine type for 32-bit compile")? I'm somehow lost there...

Re: [PATCH] powerpc: e500: Fix compilation with gcc e500 compiler

[Arnd Bergmann]

On Mon, Jul 4, 2022 at 3:29 PM Pali Rohár <pali@kernel.org> wrote:
>
> And still what to do with 4bf4f42a2feb ("powerpc/kbuild: Set default
> generic machine type for 32-bit compile")? I'm somehow lost there...

As far as I can tell, that is not needed, as long as every configuration
sets a specific -mcpu= option, the only reason it was required is that
there were some configs that relied on the compiler default, which
ended up being -mcpu=power8 or similar.

       Arnd

Re: [PATCH] powerpc: e500: Fix compilation with gcc e500 compiler

[Christophe Leroy]

Le 04/07/2022 à 15:43, Arnd Bergmann a écrit :
> On Mon, Jul 4, 2022 at 3:29 PM Pali Rohár <pali@kernel.org> wrote:
>>
>> And still what to do with 4bf4f42a2feb ("powerpc/kbuild: Set default
>> generic machine type for 32-bit compile")? I'm somehow lost there...
> 
> As far as I can tell, that is not needed, as long as every configuration
> sets a specific -mcpu= option, the only reason it was required is that
> there were some configs that relied on the compiler default, which
> ended up being -mcpu=power8 or similar.
> 


Is there any link between this discussion and the following patch 
submitted 3,5 years ago ?

https://patchwork.ozlabs.org/project/linuxppc-dev/patch/20181226141511.3ag7uf6rvdgzlmxt@decadent.org.uk/

Thanks
Christophe

Re: [PATCH] powerpc: e500: Fix compilation with gcc e500 compiler

[Pali Rohár]

On Thursday 07 July 2022 09:56:04 Christophe Leroy wrote:
> Le 04/07/2022 à 15:43, Arnd Bergmann a écrit :
> > On Mon, Jul 4, 2022 at 3:29 PM Pali Rohár <pali@kernel.org> wrote:
> >>
> >> And still what to do with 4bf4f42a2feb ("powerpc/kbuild: Set default
> >> generic machine type for 32-bit compile")? I'm somehow lost there...
> > 
> > As far as I can tell, that is not needed, as long as every configuration
> > sets a specific -mcpu= option, the only reason it was required is that
> > there were some configs that relied on the compiler default, which
> > ended up being -mcpu=power8 or similar.
> > 
> 
> 
> Is there any link between this discussion and the following patch 
> submitted 3,5 years ago ?
> 
> https://patchwork.ozlabs.org/project/linuxppc-dev/patch/20181226141511.3ag7uf6rvdgzlmxt@decadent.org.uk/
> 
> Thanks
> Christophe

Seems that above patch tried to fix same issue. I did not know about it.

Re: [PATCH] powerpc: e500: Fix compilation with gcc e500 compiler

[Pali Rohár]

On Monday 04 July 2022 15:13:58 Pali Rohár wrote:
> 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.

So what about this change?

diff --git a/arch/powerpc/Makefile b/arch/powerpc/Makefile
index a0cd70712061..74a608b5796a 100644
--- a/arch/powerpc/Makefile
+++ b/arch/powerpc/Makefile
@@ -15,22 +15,7 @@ HAS_BIARCH	:= $(call cc-option-yn, -m32)
 # Set default 32 bits cross compilers for vdso and boot wrapper
 CROSS32_COMPILE ?=
 
-ifeq ($(HAS_BIARCH),y)
-ifeq ($(CROSS32_COMPILE),)
-ifdef CONFIG_PPC32
-# These options will be overridden by any -mcpu option that the CPU
-# or platform code sets later on the command line, but they are needed
-# to set a sane 32-bit cpu target for the 64-bit cross compiler which
-# may default to the wrong ISA.
-KBUILD_CFLAGS		+= -mcpu=powerpc
-KBUILD_AFLAGS		+= -mcpu=powerpc
-endif
-endif
-endif
-
-ifdef CONFIG_PPC_BOOK3S_32
-KBUILD_CFLAGS		+= -mcpu=powerpc
-endif
+CFLAGS-$(CONFIG_PPC_BOOK3S_32) += -mcpu=powerpc
 
 # If we're on a ppc/ppc64/ppc64le machine use that defconfig, otherwise just use
 # ppc64_defconfig because we have nothing better to go on.
@@ -163,17 +148,14 @@ CFLAGS-$(CONFIG_PPC32)	+= $(call cc-option, $(MULTIPLEWORD))
 
 CFLAGS-$(CONFIG_PPC32)	+= $(call cc-option,-mno-readonly-in-sdata)
 
-ifdef CONFIG_PPC_BOOK3S_64
 ifdef CONFIG_CPU_LITTLE_ENDIAN
-CFLAGS-$(CONFIG_GENERIC_CPU) += -mcpu=power8
-CFLAGS-$(CONFIG_GENERIC_CPU) += $(call cc-option,-mtune=power9,-mtune=power8)
+CFLAGS-$(CONFIG_PPC_BOOK3S_64) += -mcpu=power8
+CFLAGS-$(CONFIG_PPC_BOOK3S_64) += $(call cc-option,-mtune=power9,-mtune=power8)
 else
-CFLAGS-$(CONFIG_GENERIC_CPU) += $(call cc-option,-mtune=power7,$(call cc-option,-mtune=power5))
-CFLAGS-$(CONFIG_GENERIC_CPU) += $(call cc-option,-mcpu=power5,-mcpu=power4)
-endif
-else ifdef CONFIG_PPC_BOOK3E_64
-CFLAGS-$(CONFIG_GENERIC_CPU) += -mcpu=powerpc64
+CFLAGS-$(CONFIG_PPC_BOOK3S_64) += $(call cc-option,-mtune=power7,$(call cc-option,-mtune=power5))
+CFLAGS-$(CONFIG_PPC_BOOK3S_64) += $(call cc-option,-mcpu=power5,-mcpu=power4)
 endif
+CFLAGS-$(CONFIG_PPC_BOOK3E_64) += -mcpu=powerpc64
 
 ifdef CONFIG_FUNCTION_TRACER
 CC_FLAGS_FTRACE := -pg
@@ -193,13 +175,8 @@ endif
 CFLAGS-$(CONFIG_E5500_CPU) += $(E5500_CPU)
 CFLAGS-$(CONFIG_E6500_CPU) += $(call cc-option,-mcpu=e6500,$(E5500_CPU))
 
-ifdef CONFIG_PPC32
-ifdef CONFIG_PPC_E500MC
-CFLAGS-y += $(call cc-option,-mcpu=e500mc,-mcpu=powerpc)
-else
+CFLAGS-$(CONFIG_PPC_E500MC) += $(call cc-option,-mcpu=e500mc,-mcpu=powerpc)
 CFLAGS-$(CONFIG_E500) += $(call cc-option,-mcpu=8540 -msoft-float,-mcpu=powerpc)
-endif
-endif
 
 asinstr := $(call as-instr,lis 9$(comma)foo@high,-DHAVE_AS_ATHIGH=1)
 


> 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

Re: [PATCH] powerpc: e500: Fix compilation with gcc e500 compiler

[Arnd Bergmann]

On Fri, Jul 8, 2022 at 7:14 PM Pali Rohár <pali@kernel.org> wrote:
>
> -ifeq ($(HAS_BIARCH),y)
> -ifeq ($(CROSS32_COMPILE),)
> -ifdef CONFIG_PPC32
> -# These options will be overridden by any -mcpu option that the CPU
> -# or platform code sets later on the command line, but they are needed
> -# to set a sane 32-bit cpu target for the 64-bit cross compiler which
> -# may default to the wrong ISA.
> -KBUILD_CFLAGS          += -mcpu=powerpc
> -KBUILD_AFLAGS          += -mcpu=powerpc
> -endif
> -endif

I think to remove these, we first need to ensure that /some/ option is
set for any of
the CPU options.
>
> -ifdef CONFIG_PPC32
> -ifdef CONFIG_PPC_E500MC
> -CFLAGS-y += $(call cc-option,-mcpu=e500mc,-mcpu=powerpc)
> -else
> +CFLAGS-$(CONFIG_PPC_E500MC) += $(call cc-option,-mcpu=e500mc,-mcpu=powerpc)
>  CFLAGS-$(CONFIG_E500) += $(call cc-option,-mcpu=8540 -msoft-float,-mcpu=powerpc)
> -endif
> -endif
>

And this part will not do what you want because CONFIG_PPC_E500MC is only
set when CONFIG_E500 is also set, so the -mcpu=e500mc option always
gets overridden by -mcpu=8540

         Arnd

Re: [PATCH] powerpc: e500: Fix compilation with gcc e500 compiler

[Christophe Leroy]

Le 08/07/2022 à 19:14, Pali Rohár a écrit :
> On Monday 04 July 2022 15:13:58 Pali Rohár wrote:
>> 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.
> 
> So what about this change?
> 
> diff --git a/arch/powerpc/Makefile b/arch/powerpc/Makefile
> index a0cd70712061..74a608b5796a 100644
> --- a/arch/powerpc/Makefile
> +++ b/arch/powerpc/Makefile
> @@ -15,22 +15,7 @@ HAS_BIARCH	:= $(call cc-option-yn, -m32)
>   # Set default 32 bits cross compilers for vdso and boot wrapper
>   CROSS32_COMPILE ?=
>   
> -ifeq ($(HAS_BIARCH),y)
> -ifeq ($(CROSS32_COMPILE),)
> -ifdef CONFIG_PPC32
> -# These options will be overridden by any -mcpu option that the CPU
> -# or platform code sets later on the command line, but they are needed
> -# to set a sane 32-bit cpu target for the 64-bit cross compiler which
> -# may default to the wrong ISA.
> -KBUILD_CFLAGS		+= -mcpu=powerpc
> -KBUILD_AFLAGS		+= -mcpu=powerpc
> -endif
> -endif
> -endif
> -
> -ifdef CONFIG_PPC_BOOK3S_32
> -KBUILD_CFLAGS		+= -mcpu=powerpc
> -endif
> +CFLAGS-$(CONFIG_PPC_BOOK3S_32) += -mcpu=powerpc

This comes too early, it is overriden by later CFLAGS-$(CONFIG_PPC32) := 
something

>   
>   # If we're on a ppc/ppc64/ppc64le machine use that defconfig, otherwise just use
>   # ppc64_defconfig because we have nothing better to go on.
> @@ -163,17 +148,14 @@ CFLAGS-$(CONFIG_PPC32)	+= $(call cc-option, $(MULTIPLEWORD))
>   
>   CFLAGS-$(CONFIG_PPC32)	+= $(call cc-option,-mno-readonly-in-sdata)
>   
> -ifdef CONFIG_PPC_BOOK3S_64
>   ifdef CONFIG_CPU_LITTLE_ENDIAN
> -CFLAGS-$(CONFIG_GENERIC_CPU) += -mcpu=power8
> -CFLAGS-$(CONFIG_GENERIC_CPU) += $(call cc-option,-mtune=power9,-mtune=power8)
> +CFLAGS-$(CONFIG_PPC_BOOK3S_64) += -mcpu=power8
> +CFLAGS-$(CONFIG_PPC_BOOK3S_64) += $(call cc-option,-mtune=power9,-mtune=power8)
>   else
> -CFLAGS-$(CONFIG_GENERIC_CPU) += $(call cc-option,-mtune=power7,$(call cc-option,-mtune=power5))
> -CFLAGS-$(CONFIG_GENERIC_CPU) += $(call cc-option,-mcpu=power5,-mcpu=power4)
> -endif
> -else ifdef CONFIG_PPC_BOOK3E_64
> -CFLAGS-$(CONFIG_GENERIC_CPU) += -mcpu=powerpc64
> +CFLAGS-$(CONFIG_PPC_BOOK3S_64) += $(call cc-option,-mtune=power7,$(call cc-option,-mtune=power5))
> +CFLAGS-$(CONFIG_PPC_BOOK3S_64) += $(call cc-option,-mcpu=power5,-mcpu=power4)

So before that change I got -mcpu=power9

Now I get -mtune=power7 -mcpu=power5 -mcpu=power9



>   endif
> +CFLAGS-$(CONFIG_PPC_BOOK3E_64) += -mcpu=powerpc64
>   
>   ifdef CONFIG_FUNCTION_TRACER
>   CC_FLAGS_FTRACE := -pg
> @@ -193,13 +175,8 @@ endif
>   CFLAGS-$(CONFIG_E5500_CPU) += $(E5500_CPU)
>   CFLAGS-$(CONFIG_E6500_CPU) += $(call cc-option,-mcpu=e6500,$(E5500_CPU))
>   
> -ifdef CONFIG_PPC32
> -ifdef CONFIG_PPC_E500MC
> -CFLAGS-y += $(call cc-option,-mcpu=e500mc,-mcpu=powerpc)
> -else
> +CFLAGS-$(CONFIG_PPC_E500MC) += $(call cc-option,-mcpu=e500mc,-mcpu=powerpc)

Before I got -mcpu=e6500

Now I get  -mcpu=powerpc64 -mcpu=e6500 -mcpu=e500mc -mcpu=8540

>   CFLAGS-$(CONFIG_E500) += $(call cc-option,-mcpu=8540 -msoft-float,-mcpu=powerpc)
> -endif
> -endif
>   
>   asinstr := $(call as-instr,lis 9$(comma)foo@high,-DHAVE_AS_ATHIGH=1)
>   
> 
> 
>> 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


Christophe

Re: [PATCH] powerpc: e500: Fix compilation with gcc e500 compiler

[Pali Rohár]

On Saturday 09 July 2022 09:16:13 Christophe Leroy wrote:
> Le 08/07/2022 à 19:14, Pali Rohár a écrit :
> > On Monday 04 July 2022 15:13:58 Pali Rohár wrote:
> >> 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.
> > 
> > So what about this change?
> > 
> > diff --git a/arch/powerpc/Makefile b/arch/powerpc/Makefile
> > index a0cd70712061..74a608b5796a 100644
> > --- a/arch/powerpc/Makefile
> > +++ b/arch/powerpc/Makefile
> > @@ -15,22 +15,7 @@ HAS_BIARCH	:= $(call cc-option-yn, -m32)
> >   # Set default 32 bits cross compilers for vdso and boot wrapper
> >   CROSS32_COMPILE ?=
> >   
> > -ifeq ($(HAS_BIARCH),y)
> > -ifeq ($(CROSS32_COMPILE),)
> > -ifdef CONFIG_PPC32
> > -# These options will be overridden by any -mcpu option that the CPU
> > -# or platform code sets later on the command line, but they are needed
> > -# to set a sane 32-bit cpu target for the 64-bit cross compiler which
> > -# may default to the wrong ISA.
> > -KBUILD_CFLAGS		+= -mcpu=powerpc
> > -KBUILD_AFLAGS		+= -mcpu=powerpc
> > -endif
> > -endif
> > -endif
> > -
> > -ifdef CONFIG_PPC_BOOK3S_32
> > -KBUILD_CFLAGS		+= -mcpu=powerpc
> > -endif
> > +CFLAGS-$(CONFIG_PPC_BOOK3S_32) += -mcpu=powerpc
> 
> This comes too early, it is overriden by later CFLAGS-$(CONFIG_PPC32) := 
> something
> 
> >   
> >   # If we're on a ppc/ppc64/ppc64le machine use that defconfig, otherwise just use
> >   # ppc64_defconfig because we have nothing better to go on.
> > @@ -163,17 +148,14 @@ CFLAGS-$(CONFIG_PPC32)	+= $(call cc-option, $(MULTIPLEWORD))
> >   
> >   CFLAGS-$(CONFIG_PPC32)	+= $(call cc-option,-mno-readonly-in-sdata)
> >   
> > -ifdef CONFIG_PPC_BOOK3S_64
> >   ifdef CONFIG_CPU_LITTLE_ENDIAN
> > -CFLAGS-$(CONFIG_GENERIC_CPU) += -mcpu=power8
> > -CFLAGS-$(CONFIG_GENERIC_CPU) += $(call cc-option,-mtune=power9,-mtune=power8)
> > +CFLAGS-$(CONFIG_PPC_BOOK3S_64) += -mcpu=power8
> > +CFLAGS-$(CONFIG_PPC_BOOK3S_64) += $(call cc-option,-mtune=power9,-mtune=power8)
> >   else
> > -CFLAGS-$(CONFIG_GENERIC_CPU) += $(call cc-option,-mtune=power7,$(call cc-option,-mtune=power5))
> > -CFLAGS-$(CONFIG_GENERIC_CPU) += $(call cc-option,-mcpu=power5,-mcpu=power4)
> > -endif
> > -else ifdef CONFIG_PPC_BOOK3E_64
> > -CFLAGS-$(CONFIG_GENERIC_CPU) += -mcpu=powerpc64
> > +CFLAGS-$(CONFIG_PPC_BOOK3S_64) += $(call cc-option,-mtune=power7,$(call cc-option,-mtune=power5))
> > +CFLAGS-$(CONFIG_PPC_BOOK3S_64) += $(call cc-option,-mcpu=power5,-mcpu=power4)
> 
> So before that change I got -mcpu=power9
> 
> Now I get -mtune=power7 -mcpu=power5 -mcpu=power9

I did it like Arnd wrote.

And seems that it does not work and now is fully out of the scope of the
original issue. Now I'm really lost here.

So I nobody comes with better solution, I would prefer to stick with my
original version which targets _only_ e500 cores.

Any other suggestion?

> 
> 
> >   endif
> > +CFLAGS-$(CONFIG_PPC_BOOK3E_64) += -mcpu=powerpc64
> >   
> >   ifdef CONFIG_FUNCTION_TRACER
> >   CC_FLAGS_FTRACE := -pg
> > @@ -193,13 +175,8 @@ endif
> >   CFLAGS-$(CONFIG_E5500_CPU) += $(E5500_CPU)
> >   CFLAGS-$(CONFIG_E6500_CPU) += $(call cc-option,-mcpu=e6500,$(E5500_CPU))
> >   
> > -ifdef CONFIG_PPC32
> > -ifdef CONFIG_PPC_E500MC
> > -CFLAGS-y += $(call cc-option,-mcpu=e500mc,-mcpu=powerpc)
> > -else
> > +CFLAGS-$(CONFIG_PPC_E500MC) += $(call cc-option,-mcpu=e500mc,-mcpu=powerpc)
> 
> Before I got -mcpu=e6500
> 
> Now I get  -mcpu=powerpc64 -mcpu=e6500 -mcpu=e500mc -mcpu=8540
> 
> >   CFLAGS-$(CONFIG_E500) += $(call cc-option,-mcpu=8540 -msoft-float,-mcpu=powerpc)
> > -endif
> > -endif
> >   
> >   asinstr := $(call as-instr,lis 9$(comma)foo@high,-DHAVE_AS_ATHIGH=1)
> >   
> > 
> > 
> >> 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
> 
> 
> Christophe

Re: [PATCH] powerpc: e500: Fix compilation with gcc e500 compiler

[Christophe Leroy]

Le 09/07/2022 à 12:23, Pali Rohár a écrit :
>>>    
>>> -ifdef CONFIG_PPC_BOOK3S_64
>>>    ifdef CONFIG_CPU_LITTLE_ENDIAN
>>> -CFLAGS-$(CONFIG_GENERIC_CPU) += -mcpu=power8
>>> -CFLAGS-$(CONFIG_GENERIC_CPU) += $(call cc-option,-mtune=power9,-mtune=power8)
>>> +CFLAGS-$(CONFIG_PPC_BOOK3S_64) += -mcpu=power8
>>> +CFLAGS-$(CONFIG_PPC_BOOK3S_64) += $(call cc-option,-mtune=power9,-mtune=power8)
>>>    else
>>> -CFLAGS-$(CONFIG_GENERIC_CPU) += $(call cc-option,-mtune=power7,$(call cc-option,-mtune=power5))
>>> -CFLAGS-$(CONFIG_GENERIC_CPU) += $(call cc-option,-mcpu=power5,-mcpu=power4)
>>> -endif
>>> -else ifdef CONFIG_PPC_BOOK3E_64
>>> -CFLAGS-$(CONFIG_GENERIC_CPU) += -mcpu=powerpc64
>>> +CFLAGS-$(CONFIG_PPC_BOOK3S_64) += $(call cc-option,-mtune=power7,$(call cc-option,-mtune=power5))
>>> +CFLAGS-$(CONFIG_PPC_BOOK3S_64) += $(call cc-option,-mcpu=power5,-mcpu=power4)
>>
>> So before that change I got -mcpu=power9
>>
>> Now I get -mtune=power7 -mcpu=power5 -mcpu=power9
> 
> I did it like Arnd wrote.
> 
> And seems that it does not work and now is fully out of the scope of the
> original issue. Now I'm really lost here.
> 
> So I nobody comes with better solution, I would prefer to stick with my
> original version which targets _only_ e500 cores.
> 
> Any other suggestion?

I sent a patch based on the TARGET_CPU logic, does it work for you ?

Christophe

Re: [PATCH] powerpc: e500: Fix compilation with gcc e500 compiler

[Pali Rohár]

On Sunday 10 July 2022 17:38:33 Christophe Leroy wrote:
> Le 09/07/2022 à 12:23, Pali Rohár a écrit :
> >>>    
> >>> -ifdef CONFIG_PPC_BOOK3S_64
> >>>    ifdef CONFIG_CPU_LITTLE_ENDIAN
> >>> -CFLAGS-$(CONFIG_GENERIC_CPU) += -mcpu=power8
> >>> -CFLAGS-$(CONFIG_GENERIC_CPU) += $(call cc-option,-mtune=power9,-mtune=power8)
> >>> +CFLAGS-$(CONFIG_PPC_BOOK3S_64) += -mcpu=power8
> >>> +CFLAGS-$(CONFIG_PPC_BOOK3S_64) += $(call cc-option,-mtune=power9,-mtune=power8)
> >>>    else
> >>> -CFLAGS-$(CONFIG_GENERIC_CPU) += $(call cc-option,-mtune=power7,$(call cc-option,-mtune=power5))
> >>> -CFLAGS-$(CONFIG_GENERIC_CPU) += $(call cc-option,-mcpu=power5,-mcpu=power4)
> >>> -endif
> >>> -else ifdef CONFIG_PPC_BOOK3E_64
> >>> -CFLAGS-$(CONFIG_GENERIC_CPU) += -mcpu=powerpc64
> >>> +CFLAGS-$(CONFIG_PPC_BOOK3S_64) += $(call cc-option,-mtune=power7,$(call cc-option,-mtune=power5))
> >>> +CFLAGS-$(CONFIG_PPC_BOOK3S_64) += $(call cc-option,-mcpu=power5,-mcpu=power4)
> >>
> >> So before that change I got -mcpu=power9
> >>
> >> Now I get -mtune=power7 -mcpu=power5 -mcpu=power9
> > 
> > I did it like Arnd wrote.
> > 
> > And seems that it does not work and now is fully out of the scope of the
> > original issue. Now I'm really lost here.
> > 
> > So I nobody comes with better solution, I would prefer to stick with my
> > original version which targets _only_ e500 cores.
> > 
> > Any other suggestion?
> 
> I sent a patch based on the TARGET_CPU logic, does it work for you ?
> 
> Christophe

Perfect, it works! Thank you.


Anyway, same problem is with arch/powerpc/boot/Makefile file when
building "uImage" target. There is hardcoded -mcpu=powerpc flag.

Re: [PATCH] powerpc: e500: Fix compilation with gcc e500 compiler

[Christophe Leroy]

Le 10/07/2022 à 19:57, Pali Rohár a écrit :
> On Sunday 10 July 2022 17:38:33 Christophe Leroy wrote:
>> Le 09/07/2022 à 12:23, Pali Rohár a écrit :
>>>>>     
>>>>> -ifdef CONFIG_PPC_BOOK3S_64
>>>>>     ifdef CONFIG_CPU_LITTLE_ENDIAN
>>>>> -CFLAGS-$(CONFIG_GENERIC_CPU) += -mcpu=power8
>>>>> -CFLAGS-$(CONFIG_GENERIC_CPU) += $(call cc-option,-mtune=power9,-mtune=power8)
>>>>> +CFLAGS-$(CONFIG_PPC_BOOK3S_64) += -mcpu=power8
>>>>> +CFLAGS-$(CONFIG_PPC_BOOK3S_64) += $(call cc-option,-mtune=power9,-mtune=power8)
>>>>>     else
>>>>> -CFLAGS-$(CONFIG_GENERIC_CPU) += $(call cc-option,-mtune=power7,$(call cc-option,-mtune=power5))
>>>>> -CFLAGS-$(CONFIG_GENERIC_CPU) += $(call cc-option,-mcpu=power5,-mcpu=power4)
>>>>> -endif
>>>>> -else ifdef CONFIG_PPC_BOOK3E_64
>>>>> -CFLAGS-$(CONFIG_GENERIC_CPU) += -mcpu=powerpc64
>>>>> +CFLAGS-$(CONFIG_PPC_BOOK3S_64) += $(call cc-option,-mtune=power7,$(call cc-option,-mtune=power5))
>>>>> +CFLAGS-$(CONFIG_PPC_BOOK3S_64) += $(call cc-option,-mcpu=power5,-mcpu=power4)
>>>>
>>>> So before that change I got -mcpu=power9
>>>>
>>>> Now I get -mtune=power7 -mcpu=power5 -mcpu=power9
>>>
>>> I did it like Arnd wrote.
>>>
>>> And seems that it does not work and now is fully out of the scope of the
>>> original issue. Now I'm really lost here.
>>>
>>> So I nobody comes with better solution, I would prefer to stick with my
>>> original version which targets _only_ e500 cores.
>>>
>>> Any other suggestion?
>>
>> I sent a patch based on the TARGET_CPU logic, does it work for you ?
>>
>> Christophe
> 
> Perfect, it works! Thank you.
> 

Thanks.

I sent a series including this patch with your comments taken into account.

Christophe

Re: [PATCH] powerpc: e500: Fix compilation with gcc e500 compiler

[Pali Rohár]

On Monday 04 July 2022 14:07:10 Arnd Bergmann wrote:
> 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).

Such configuration is not supported, see arch/powerpc/platforms/Kconfig.cputype:

config PPC_E500MC
	bool "e500mc Support"
	select PPC_FPU
	select COMMON_CLK
	depends on E500
	help
	  This must be enabled for running on e500mc (and derivatives
	  such as e5500/e6500), and must be disabled for running on
	  e500v1 or e500v2.

Based on this option you can enable either support for e500v1/e500v2 or
for e500mc. But not both.

Re: [PATCH] powerpc: e500: Fix compilation with gcc e500 compiler

[Arnd Bergmann]

On Fri, Jul 8, 2022 at 7:12 PM Pali Rohár <pali@kernel.org> wrote:
>
> On Monday 04 July 2022 14:07:10 Arnd Bergmann wrote:
> > 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).
>
> Such configuration is not supported, see arch/powerpc/platforms/Kconfig.cputype:
>
> config PPC_E500MC
>         bool "e500mc Support"
>         select PPC_FPU
>         select COMMON_CLK
>         depends on E500
>         help
>           This must be enabled for running on e500mc (and derivatives
>           such as e5500/e6500), and must be disabled for running on
>           e500v1 or e500v2.
>
> Based on this option you can enable either support for e500v1/e500v2 or
> for e500mc. But not both.

This looks like a bad decision in Kconfig though, as there is nothing
enforcing the rule: If you want support for E500MC, you have to select
PPC_85xx, which implies E500 and allows selecting any combination
of E500v1, E500v2 and E500MC based machines, but enabling
any E500MC based one breaks all the others.

If this is a hard dependency, I think it should be enforced by making
E500MC a separate top-level option in the "Processor Type" choice
statement. However, if they can actually coexist, the help text and
the Makefile need to be fixed.

       Arnd

Re: [PATCH] powerpc: e500: Fix compilation with gcc e500 compiler

[Christophe Leroy]

Le 08/07/2022 à 22:04, Arnd Bergmann a écrit :
> On Fri, Jul 8, 2022 at 7:12 PM Pali Rohár <pali@kernel.org> wrote:
>>
>> On Monday 04 July 2022 14:07:10 Arnd Bergmann wrote:
>>> 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).
>>
>> Such configuration is not supported, see arch/powerpc/platforms/Kconfig.cputype:
>>
>> config PPC_E500MC
>>          bool "e500mc Support"
>>          select PPC_FPU
>>          select COMMON_CLK
>>          depends on E500
>>          help
>>            This must be enabled for running on e500mc (and derivatives
>>            such as e5500/e6500), and must be disabled for running on
>>            e500v1 or e500v2.
>>
>> Based on this option you can enable either support for e500v1/e500v2 or
>> for e500mc. But not both.
> 
> This looks like a bad decision in Kconfig though, as there is nothing
> enforcing the rule: If you want support for E500MC, you have to select
> PPC_85xx, which implies E500 and allows selecting any combination
> of E500v1, E500v2 and E500MC based machines, but enabling
> any E500MC based one breaks all the others.
> 
> If this is a hard dependency, I think it should be enforced by making
> E500MC a separate top-level option in the "Processor Type" choice
> statement. However, if they can actually coexist, the help text and
> the Makefile need to be fixed.
> 

In cputable.c you have entries in the cpu_specs table. It show that when 
selecting PPC32 and E500MC, you exclude e500 and e500v2, allthough you 
then fallback on the generic e500 probably by mistake.

Seems to come from commit 3477e71d5319 ("powerpc/booke: Restrict SPE 
exception handlers to e200/e500 cores"), before that we had both e500 
and e500mc in the table at the same time.

But when e500mc was introduced by commit 3dfa8773674e ("powerpc/booke: 
Add support for new e500mc core"), it was already clear that it was not 
compatible with other e500, especially due to the size of the cacheline, 
which is hardcoded at buildtime on PPC32.

The comment in Kconfig.cputype was added my commit 9653018b615b 
("powerpc/e500: add paravirt QEMU platform")

And by the way, today you are able to build a kernel with an empty 
cpu_specs[] table if you select CONFIG_PPC_BOOK3E_64 and select neither 
CONFIG_CORENET_GENERIC nor CONFIG_PPC_QEMU_E500

static struct cpu_spec __initdata cpu_specs[] = {
#ifdef CONFIG_E500
#ifdef CONFIG_PPC32
#ifndef CONFIG_PPC_E500MC
	{	/* e500 */
		.pvr_mask		= 0xffff0000,
		.pvr_value		= 0x80200000,
		.cpu_name		= "e500",
		.cpu_features		= CPU_FTRS_E500,
	},
	{	/* e500v2 */
		.pvr_mask		= 0xffff0000,
		.pvr_value		= 0x80210000,
		.cpu_name		= "e500v2",
		.cpu_features		= CPU_FTRS_E500_2,
	},
#else
	{	/* e500mc */
		.pvr_mask		= 0xffff0000,
		.pvr_value		= 0x80230000,
		.cpu_name		= "e500mc",
		.cpu_features		= CPU_FTRS_E500MC,
	},
#endif /* CONFIG_PPC_E500MC */
#endif /* CONFIG_PPC32 */
#ifdef CONFIG_PPC_E500MC
	{	/* e5500 */
		.pvr_mask		= 0xffff0000,
		.pvr_value		= 0x80240000,
		.cpu_name		= "e5500",
		.cpu_features		= CPU_FTRS_E5500,
	},
	{	/* e6500 */
		.pvr_mask		= 0xffff0000,
		.pvr_value		= 0x80400000,
		.cpu_name		= "e6500",
		.cpu_features		= CPU_FTRS_E6500,
	},
#endif /* CONFIG_PPC_E500MC */
#ifdef CONFIG_PPC32
	{	/* default match */
		.pvr_mask		= 0x00000000,
		.pvr_value		= 0x00000000,
		.cpu_name		= "(generic E500 PPC)",
		.cpu_features		= CPU_FTRS_E500,
	}
#endif /* CONFIG_PPC32 */

Re: [PATCH] powerpc: e500: Fix compilation with gcc e500 compiler

[Christophe Leroy]

Le 08/07/2022 à 22:04, Arnd Bergmann a écrit :
> On Fri, Jul 8, 2022 at 7:12 PM Pali Rohár <pali@kernel.org> wrote:
>>
>> On Monday 04 July 2022 14:07:10 Arnd Bergmann wrote:
>>> 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).
>>
>> Such configuration is not supported, see arch/powerpc/platforms/Kconfig.cputype:
>>
>> config PPC_E500MC
>>          bool "e500mc Support"
>>          select PPC_FPU
>>          select COMMON_CLK
>>          depends on E500
>>          help
>>            This must be enabled for running on e500mc (and derivatives
>>            such as e5500/e6500), and must be disabled for running on
>>            e500v1 or e500v2.
>>
>> Based on this option you can enable either support for e500v1/e500v2 or
>> for e500mc. But not both.
> 
> This looks like a bad decision in Kconfig though, as there is nothing
> enforcing the rule: If you want support for E500MC, you have to select
> PPC_85xx, which implies E500 and allows selecting any combination
> of E500v1, E500v2 and E500MC based machines, but enabling
> any E500MC based one breaks all the others.
> 
> If this is a hard dependency, I think it should be enforced by making
> E500MC a separate top-level option in the "Processor Type" choice
> statement. However, if they can actually coexist, the help text and
> the Makefile need to be fixed.
> 

While looking at this discussion, I discovered that with GCC 12 from 
https://mirrors.edge.kernel.org/pub/tools/crosstool/ I'm unable to build 
a corenet64_smp_defconfig:

If I select the GENERIC_CPU or e5500 (with altivec) I get:

   CC      arch/powerpc/kernel/irq.o
{standard input}: Assembler messages:
{standard input}:3535: Error: unrecognized opcode: `wrteei'
{standard input}:5608: Error: unrecognized opcode: `wrteei'

   CC      arch/powerpc/kernel/pmc.o
{standard input}: Assembler messages:
{standard input}:42: Error: unrecognized opcode: `mfpmr'
{standard input}:53: Error: unrecognized opcode: `mtpmr'


If I select the e5500 (without altivec) or e6500 I get:

   CC      arch/powerpc/kernel/io.o
{standard input}: Assembler messages:
{standard input}:381: Error: unrecognized opcode: `eieio'
{standard input}:444: Error: unrecognized opcode: `eieio'
{standard input}:480: Error: unrecognized opcode: `eieio'
{standard input}:506: Error: unrecognized opcode: `eieio'
{standard input}:552: Error: unrecognized opcode: `eieio'
{standard input}:817: Error: unrecognized opcode: `eieio'
{standard input}:839: Error: unrecognized opcode: `eieio'
{standard input}:885: Error: unrecognized opcode: `eieio'
{standard input}:1106: Error: unrecognized opcode: `eieio'
{standard input}:1130: Error: unrecognized opcode: `eieio'
{standard input}:1174: Error: unrecognized opcode: `eieio'
{standard input}:1393: Error: unrecognized opcode: `eieio'
{standard input}:1417: Error: unrecognized opcode: `eieio'
{standard input}:1461: Error: unrecognized opcode: `eieio'



Christophe

Re: [PATCH] powerpc: e500: Fix compilation with gcc e500 compiler

[Segher Boessenkool]

On Sat, Jul 09, 2022 at 09:26:11AM +0000, Christophe Leroy wrote:
> If I select the GENERIC_CPU or e5500 (with altivec) I get:
> 
>    CC      arch/powerpc/kernel/irq.o
> {standard input}: Assembler messages:
> {standard input}:3535: Error: unrecognized opcode: `wrteei'
> {standard input}:5608: Error: unrecognized opcode: `wrteei'

What -mcpu= did it use here?

wrteei is not a PowerPC insn (it is BookE, instead), so it is not
recognised without an appropriate -mcpu=.

> If I select the e5500 (without altivec) or e6500 I get:
> 
>    CC      arch/powerpc/kernel/io.o
> {standard input}: Assembler messages:
> {standard input}:381: Error: unrecognized opcode: `eieio'

Same question.  eieio is a base PowerPC instruction, so this one is
"interesting" :-)


Segher

Re: [PATCH] powerpc: e500: Fix compilation with gcc e500 compiler

[Christophe Leroy]

Le 11/07/2022 à 18:14, Segher Boessenkool a écrit :
> On Sat, Jul 09, 2022 at 09:26:11AM +0000, Christophe Leroy wrote:
>> If I select the GENERIC_CPU or e5500 (with altivec) I get:
>>
>>     CC      arch/powerpc/kernel/irq.o
>> {standard input}: Assembler messages:
>> {standard input}:3535: Error: unrecognized opcode: `wrteei'
>> {standard input}:5608: Error: unrecognized opcode: `wrteei'
> 
> What -mcpu= did it use here?

-mcpu=powerpc64

> 
> wrteei is not a PowerPC insn (it is BookE, instead), so it is not
> recognised without an appropriate -mcpu=.
> 
>> If I select the e5500 (without altivec) or e6500 I get:
>>
>>     CC      arch/powerpc/kernel/io.o
>> {standard input}: Assembler messages:
>> {standard input}:381: Error: unrecognized opcode: `eieio'
> 
> Same question.  eieio is a base PowerPC instruction, so this one is
> "interesting" :-)

-mcpu=e500mc64 (for e5500)
-mcpu=e6500 (for e6500)

I had to replace 'eieio' instruction by 'mbar' instruction.


Seems like binutils added 'eieio' to e500 in 2010 via commit 
e01d869a3be, but it seems it is only for the 85xx, not for the others.

Christophe

Re: [PATCH] powerpc: e500: Fix compilation with gcc e500 compiler

[Segher Boessenkool]

Hi!

On Mon, Jul 11, 2022 at 05:32:09PM +0000, Christophe Leroy wrote:
> Le 11/07/2022 à 18:14, Segher Boessenkool a écrit :
> >>     CC      arch/powerpc/kernel/irq.o
> >> {standard input}: Assembler messages:
> >> {standard input}:3535: Error: unrecognized opcode: `wrteei'
> >> {standard input}:5608: Error: unrecognized opcode: `wrteei'
> > 
> > What -mcpu= did it use here?
> 
> -mcpu=powerpc64
> 
> > wrteei is not a PowerPC insn (it is BookE, instead), so it is not
> > recognised without an appropriate -mcpu=.
> > 
> >> If I select the e5500 (without altivec) or e6500 I get:
> >>
> >>     CC      arch/powerpc/kernel/io.o
> >> {standard input}: Assembler messages:
> >> {standard input}:381: Error: unrecognized opcode: `eieio'
> > 
> > Same question.  eieio is a base PowerPC instruction, so this one is
> > "interesting" :-)
> 
> -mcpu=e500mc64 (for e5500)
> -mcpu=e6500 (for e6500)
> 
> I had to replace 'eieio' instruction by 'mbar' instruction.

I saw some patches fly by...  you have it all fixed with that?

> Seems like binutils added 'eieio' to e500 in 2010 via commit 
> e01d869a3be, but it seems it is only for the 85xx, not for the others.

I believe the eieio instruction is disabled on some e500 models, because
it does not work correctly, so EIEIO_EN=1 cannot work, something like
that?


Segher

Re: [PATCH] powerpc: e500: Fix compilation with gcc e500 compiler

[Christophe Leroy]

Le 11/07/2022 à 23:48, Segher Boessenkool a écrit :
> Hi!
> 
> On Mon, Jul 11, 2022 at 05:32:09PM +0000, Christophe Leroy wrote:
>> Le 11/07/2022 à 18:14, Segher Boessenkool a écrit :
>>>>      CC      arch/powerpc/kernel/irq.o
>>>> {standard input}: Assembler messages:
>>>> {standard input}:3535: Error: unrecognized opcode: `wrteei'
>>>> {standard input}:5608: Error: unrecognized opcode: `wrteei'
>>>
>>> What -mcpu= did it use here?
>>
>> -mcpu=powerpc64
>>
>>> wrteei is not a PowerPC insn (it is BookE, instead), so it is not
>>> recognised without an appropriate -mcpu=.
>>>
>>>> If I select the e5500 (without altivec) or e6500 I get:
>>>>
>>>>      CC      arch/powerpc/kernel/io.o
>>>> {standard input}: Assembler messages:
>>>> {standard input}:381: Error: unrecognized opcode: `eieio'
>>>
>>> Same question.  eieio is a base PowerPC instruction, so this one is
>>> "interesting" :-)
>>
>> -mcpu=e500mc64 (for e5500)
>> -mcpu=e6500 (for e6500)
>>
>> I had to replace 'eieio' instruction by 'mbar' instruction.
> 
> I saw some patches fly by...  you have it all fixed with that?

Yes it fixed all build failures with GCC 12.


> 
>> Seems like binutils added 'eieio' to e500 in 2010 via commit
>> e01d869a3be, but it seems it is only for the 85xx, not for the others.
> 
> I believe the eieio instruction is disabled on some e500 models, because
> it does not work correctly, so EIEIO_EN=1 cannot work, something like
> that?

Don't know.

It is also disabled on 405 and 440.

That's new with GCC 12.

Christophe

Re: [PATCH] powerpc: e500: Fix compilation with gcc e500 compiler

[Segher Boessenkool]

On Tue, Jul 12, 2022 at 09:22:12AM +0000, Christophe Leroy wrote:
> Le 11/07/2022 à 23:48, Segher Boessenkool a écrit :
> > I believe the eieio instruction is disabled on some e500 models, because
> > it does not work correctly, so EIEIO_EN=1 cannot work, something like
> > that?
> 
> Don't know.
> 
> It is also disabled on 405 and 440.

BookE does not have the eieio insn.  Instead, it reuses the same opcode
for mbar, which has similar but different semantics.

e500 has that EIEIO_EN thing which makes the insn behave like eieio.

> That's new with GCC 12.

Yup.  In the past we used -many, but that just hides problems in the
best case, and causes more problems itself :-(

There are many mnemonics that cause a different instruction to be
emitted on different targets, and that causes a lot of wasted time
trying to find and fix the problems this causes.

If you hit any remaining problems related to this, please let me know!


Segher