[1/2] powerpc/kexec: Reset secondary cpu endianess before kexec

If the target kernel does not inlcude the FIXUP_ENDIAN check, coming
from a different-endian kernel will cause the target kernel to panic.
All ppc64 kernels can handle starting in big-endian mode, so return to
big-endian before branching into the target kernel.

This mainly affects pseries as secondaries on powernv are returned to
OPAL.

Signed-off-by: Samuel Mendoza-Jonas <sam.mj@au1.ibm.com>
---
 arch/powerpc/kernel/misc_64.S | 15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

On Wed, 2015-07-08 at 11:23 +1000, Samuel Mendoza-Jonas wrote:
> If the target kernel does not inlcude the FIXUP_ENDIAN check, coming
> from a different-endian kernel will cause the target kernel to panic.
> All ppc64 kernels can handle starting in big-endian mode, so return to
> big-endian before branching into the target kernel.
> 
> This mainly affects pseries as secondaries on powernv are returned to
> OPAL.
> 
> Signed-off-by: Samuel Mendoza-Jonas <sam.mj@au1.ibm.com>
> ---
>  arch/powerpc/kernel/misc_64.S | 15 +++++++++++++--
>  1 file changed, 13 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/powerpc/kernel/misc_64.S b/arch/powerpc/kernel/misc_64.S
> index 4e314b9..c73e587 100644
> --- a/arch/powerpc/kernel/misc_64.S
> +++ b/arch/powerpc/kernel/misc_64.S
> @@ -475,9 +475,20 @@ _GLOBAL(kexec_wait)
>  #ifdef CONFIG_KEXEC          /* use no memory without kexec */
>       lwz     r4,0(r5)
>       cmpwi   0,r4,0
> -     bnea    0x60
> +     beq     99b
> +     mfmsr   r9              /* Check endianess */
> +     clrldi. r10,r9,63
> +     beqa    0x60            /* Already big-endian */
> +     bcl     20,31,$+4
> +     mflr    r10
> +     addi    r10,r10,28
> +     mfmsr   r11
> +     xori    r11,r11,1
> +     mtsrr0  r10
> +     mtsrr1  r11
> +     rfid
> +     .long   0x62000048      /* ba 0x60 */
>  #endif
> -     b       99b

Could you put a book3s ifdef around this?  The low bit of MSR is reserved on 
book3e (and rfid doesn't exist).  Granted the bit should be zero, at least on 
FSL book3e, but it's better to be explicit about code that is subarch-
specific.

And yes, I have book3e kexec patches coming.

Also, it would be better to use label subtraction rather than hardcoding 
"28", and the bcl instruction would be more readable as "bl <label>".

-Scott

On Tue, Jul 07, 2015 at 09:35:38PM -0500, Scott Wood wrote:
> 
> Also, it would be better to use label subtraction rather than hardcoding 
> "28", and the bcl instruction would be more readable as "bl <label>".

I agree about using labels, but "bcl 20,31,foo" is not the same thing
as "bl foo".  The former is a form of bl that doesn't perturb the link
stack and is therefore better for performance when you're not going to
do a matching blr later (not that performance is at all critical
here).

Paul.

On Wed, 2015-07-08 at 14:04 +1000, Paul Mackerras wrote:
> On Tue, Jul 07, 2015 at 09:35:38PM -0500, Scott Wood wrote:
> > 
> > Also, it would be better to use label subtraction rather than hardcoding 
> > "28", and the bcl instruction would be more readable as "bl <label>".
> 
> I agree about using labels, but "bcl 20,31,foo" is not the same thing
> as "bl foo".  The former is a form of bl that doesn't perturb the link
> stack and is therefore better for performance when you're not going to
> do a matching blr later (not that performance is at all critical
> here).

If performance mattered I would have complained about the extra mfmsr. :-)

I see that some other parts of the kernel are using that bcl instruction, but 
if it is actually worthwhile in those places, it'd be nice to at least stick 
it in a macro for readability...

-Scott

On Tue, Jul 07, 2015 at 11:22:08PM -0500, Scott Wood wrote:
> > I agree about using labels, but "bcl 20,31,foo" is not the same thing
> > as "bl foo".  The former is a form of bl that doesn't perturb the link
> > stack and is therefore better for performance when you're not going to
> > do a matching blr later (not that performance is at all critical
> > here).
> 
> If performance mattered I would have complained about the extra mfmsr. :-)
> 
> I see that some other parts of the kernel are using that bcl instruction, but 
> if it is actually worthwhile in those places, it'd be nice to at least stick 
> it in a macro for readability...

How is that going to help?  It's just idiom; replacing it with some other
idiom only means people already used to the 20,31 thing (which is quite old
by now, btw.; 74xx days) will have to learn the new thing.

Unless you can think of a nice short name that can even be made an extended
mnemonic?


Segher

On Thu, 2015-07-09 at 10:42 -0500, Segher Boessenkool wrote:
> On Tue, Jul 07, 2015 at 11:22:08PM -0500, Scott Wood wrote:
> > > I agree about using labels, but "bcl 20,31,foo" is not the same thing
> > > as "bl foo".  The former is a form of bl that doesn't perturb the link
> > > stack and is therefore better for performance when you're not going to
> > > do a matching blr later (not that performance is at all critical
> > > here).
> > 
> > If performance mattered I would have complained about the extra mfmsr. :-)
> > 
> > I see that some other parts of the kernel are using that bcl instruction, 
> > but 
> > if it is actually worthwhile in those places, it'd be nice to at least 
> > stick 
> > it in a macro for readability...
> 
> How is that going to help?  It's just idiom; replacing it with some other
> idiom only means people already used to the 20,31 thing (which is quite old
> by now, btw.; 74xx days) will have to learn the new thing.

That's fine for people who are used to it (especially if it's the only number-
based bc they routinely come across), but I had to look up what it meant.  
Simplified mnemonics exist for a reason.

> Unless you can think of a nice short name that can even be made an extended
> mnemonic?

For a macro I was thinking "BL_NORET" but for a mnemonic it'd probably be 
shortened to something like "blnr".

-Scott