[PATCH] powerpc/pseries: fix endian problems with LE migration

[Michael Ellerman]

On Thu, 2015-01-22 at 16:37 +1100, Cyril Bur wrote:
> On Wed, 2015-01-21 at 14:33 +1100, Michael Ellerman wrote:
> > On Wed, 2015-01-21 at 13:32 +1100, Cyril Bur wrote:
> > > The need to handle ibm,suspend_me specially from within ppc_rtas has left an
> > > endian bug exposed as rtas_ibm_suspend_me actually performs HCALLs and should
> > > have its params in CPU endian.
> > 
> > That needs a much better explanation.

> I've tried to write that out neatly and orderly. Here's how that went:

Yep that's much better.

Minor points of style that I like are to use "()" on functions to make it
clear, eg. ppc_rtas(), and to put strings in quotes, eg. "ibm,suspend_me".

I've fixed it up and incorporated your testing notes, end result is:

Author: cyrilbur at gmail.com <cyrilbur at gmail.com>
Date:   Wed Jan 21 13:32:00 2015 +1100

    powerpc/pseries: Fix endian problems with LE migration
    
    RTAS events require arguments be passed in big endian while hypercalls
    have their arguments passed in registers and the values should therefore
    be in CPU endian.
    
    The "ibm,suspend_me" 'RTAS' call makes a sequence of hypercalls to setup
    one true RTAS call. This means that "ibm,suspend_me" is handled
    specially in the ppc_rtas() syscall.
    
    The ppc_rtas() syscall has its arguments in big endian and can therefore
    pass these arguments directly to the RTAS call. "ibm,suspend_me" is
    handled specially from within ppc_rtas() (by calling rtas_ibm_suspend_me())
    which has left an endian bug on little endian systems due to the
    requirement of hypercalls. The return value from rtas_ibm_suspend_me()
    gets returned in cpu endian, and is left unconverted, also a bug on
    little endian systems.
    
    rtas_ibm_suspend_me() does not actually make use of the rtas_args that
    it is passed. This patch removes the convoluted use of the rtas_args
    struct to pass params to rtas_ibm_suspend_me() in favour of passing what
    it needs as actual arguments. This patch also ensures the two callers of
    rtas_ibm_suspend_me() pass function parameters in cpu endian and in the
    case of ppc_rtas(), converts the return value.
    
    migrate_store() (the other caller of rtas_ibm_suspend_me()) is from a
    sysfs file which deals with everything in cpu endian so this function
    only underwent cleanup.
    
    This patch has been tested with KVM both LE and BE and on PowerVM both
    LE and BE. Under QEMU/KVM the migration happens without touching these
    code pathes.
    
    For PowerVM there is no obvious regression on BE and the LE code path
    now provides the correct parameters to the hypervisor.
    
    Signed-off-by: Cyril Bur <cyrilbur at gmail.com>
    Signed-off-by: Michael Ellerman <mpe at ellerman.id.au>


cheers