Re: [PATCH] powerpc/kexec: Don't use .machine ppc64 in trampoline_64.S

[Segher Boessenkool <segher@kernel.crashing.org>]

Hi!

On Mon, Mar 15, 2021 at 02:41:59PM +1100, Michael Ellerman wrote:
> The ".machine" directive allows changing the machine for which code is
> being generated. It's equivalent to passing an -mcpu option on the
> command line.
> 
> Although it can be useful, it's generally a bad idea because it adds
> another way to influence code generation separate from the flags
> passed via the build system. ie. if we need to build different pieces
> of code with different flags we should do that via our Makefiles, not
> using ".machine".

It does not influence code generation.  It says which instructions are
valid, instead.  There are a few cases where the same mnemonic will
generate a different binary encoding depending on machine selected,
maybe you mean that?

It is *normal* to use .machine push/pop and a specific .machine around
instructions that require a machine other than what you are building
for.  The compiler does this itself, and it is the recommended way to
use "foreign" instructions in inline assembler.

That said...

> However as best as I can tell the ".machine" directive in
> trampoline_64.S is not necessary at all.
> 
> It was added in commit 0d97631392c2 ("powerpc: Add purgatory for
> kexec_file_load() implementation."), which created the file based on
> the kexec-tools purgatory. It may be/have-been necessary in the
> kexec-tools version, but we have a completely different build system,
> and we already pass the desired CPU flags, eg:
> 
>   gcc ... -m64 -Wl,-a64 -mabi=elfv2 -Wa,-maltivec -Wa,-mpower4 -Wa,-many
>   ... arch/powerpc/purgatory/trampoline_64.S
> 
> So drop the ".machine" directive and rely on the assembler flags.

> -	.machine ppc64

Please make sure to test this on a big endian config.

A ppc64le-linux assembler defaults to power8.  A ppc64-linux assembler
defaults to power3 (that is the same as .machine ppc64).  Or maybe it
makes it power4?  I get lost :-)

It certainly *should* work, but, test please :-)

(And with a *default* powerpc64-linux config, not one that defaults to
power7 or power8 or similar!  Arnd's toolchains at
<https://mirrors.edge.kernel.org/pub/tools/crosstool/>
are fine for this.)


Reviewed-by: Segher Boessenkool <segher@kernel.crashing.org>


Segher