Subject: Re: [PATCH] powerpc/64: Drop ppc_inst_as_str()

From: Segher Boessenkool <[email-masked]>
Date: Fri, 3 Jun 2022 07:49:05 -0500

Hi!

On Fri, Jun 03, 2022 at 03:03:05PM +1000, Jordan Niethe wrote:
> On Thu, Jun 2, 2022 at 6:49 PM Segher Boessenkool
> <[email-masked]> wrote:
> > On Thu, Jun 02, 2022 at 01:01:04PM +1000, Jordan Niethe wrote:
> > > > What about the more fundamental thing?  Have the order of the two halves
> > > > of a prefixed insn as ulong not depend on endianness?  It really is two
> > > > opcodes, and the prefixed one is first, always, even in LE.
> > > The reason would be the value of as ulong is then used to write a
> > > prefixed instruction to
> > > memory with std.
> > > If both endiannesses had the halves the same one of them would store
> > > the suffix in front of the prefix.
> >
> > You cannot do such a (possibly) unaligned access from C though, not
> > without invoking undefined behaviour.  The compiler usually lets you get
> > away with it, but there are no guarantees.  You can make sure you only
> > ever do such an access from assembler code of course.
> 
> Would using inline assembly to do it be ok?

You cannot refer to the instruction words as one 8-byte integer in
memory (with "m" or similar), since such a thing is not valid C.  You
can input the address of it to the inline asm of course, and use a
clobber "memory", or also give it the actual memory as input, but as
bytes or words or such, with something like
  "m"(*(const u32 (*)[2]) ptr)

> > Swapping the two halves of a register costs at most one insn.  It is
> > harmful premature optimisation to make this single cycle advantage
> > override more important consideration (almost everything else :-) )
> 
> I'm not sure I follow. We are not doing this as an optimisation, but
> out of the necessity of writing
> the prefixed instruction to memory in a single instruction so that we
> don't end up with half an
> instruction in the kernel image.

Ah.  That does not necessitate having this different for LE at all
though!  The function that does the patching has to do it as one
atomic memory access (which an ld only is because prefixed insns cannot
cross 64-byte address boundaries btw), but that does not mean the kernel
has to use the packing into one u64 it needs for that anywhere else,
certainly not if that just complicates matters!

Segher