[PATCH] powerpc/64: Drop ppc_inst_as_str()

[Michael Ellerman]

Segher Boessenkool <segher at kernel.crashing.org> writes:
> Hi!
>
> On Tue, May 31, 2022 at 04:59:36PM +1000, Michael Ellerman wrote:
>> More problematically it doesn't compile at all with GCC 12, due to the
>> fact that it returns the char buffer declared inside the macro:
>
> It returns a pointer to a buffer on stack.  It is not valid C to access
> that buffer after the function has returned (and indeed it does not
> work, in general).

It's a statement expression though, not a function. So it doesn't return
as such, that would be obviously wrong.

But I'm not a language lawyer, so presumably it's not valid to refer to
the variable after it's gone out of scope.

Although we do use that same pattern in many places where the value of
the expression is a scalar type.

>> A simpler solution is to just print the value as an unsigned long. For
>> normal instructions the output is identical. For prefixed instructions
>> the value is printed as a single 64-bit quantity, whereas previously the
>> low half was printed first. But that is good enough for debug output,
>> especially as prefixed instructions will be rare in practice.
>
> Prefixed insns might be somewhat rare currently, but it will not stay
> that way.

These are all printing kernel instructions, not userspace. I should have
said that in the change log.

The kernel doesn't build for -mcpu=power10 because we haven't done any
changes for pcrel.

We will do that one day, but not soon.

> It is not hard to fix the problem here?  The only tricky part is that
> ppc_inst_as_ulong swaps the two halves for LE, for as far as I can see
> no reason at all :-(
>
> If it didn't it would be easy to detect prefixed insns (because they
> then are guaranteed to be > 0xffffffff), and it is easy to print them
> with a space between the two opcodes, with a utility function:
>
> void print_insn_bytes_nicely(unsigned long insn)
> {
> 	if (insn > 0xffffffff)
> 		printf("%08x ", insn >> 32);
> 	printf("%08x", insn & 0xffffffff);
> }

We don't want to do that because it can lead to interleaving messages
between different CPUs in the kernel log.

In the medium term there's some changes to printk that might land soon
(printbuf), which would mean we could more easily define a custom printk
formatter for printing prefixed instructions.

cheers