On Wed, Nov 11, 2015 at 11:27:22AM +1100, Benjamin Herrenschmidt wrote:
> XXX This patch needs double checking... It fixed 32-bit userspace
> but I'm not sure it's right. I wonder whether msr_is_64bit() should
> be applied to env->msr, not msr, but I need to double check the
> architecture.

Hrm, I'm not really sure where I'd look in the arch, but
msr_is_64bit(env->msr) seems like it would make more sense to me.
The current logic means that rfi, ostensibly a 32-bit instruction will
have different behaviour depending on the upper bits of SRR1, which
seems a unexpected.

> 
> Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> ---
>  target-ppc/excp_helper.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/target-ppc/excp_helper.c b/target-ppc/excp_helper.c
> index c1d6605..00fae60 100644
> --- a/target-ppc/excp_helper.c
> +++ b/target-ppc/excp_helper.c
> @@ -878,13 +878,13 @@ static inline void do_rfi(CPUPPCState *env, target_ulong nip, target_ulong msr,
>      CPUState *cs = CPU(ppc_env_get_cpu(env));
>  
>  #if defined(TARGET_PPC64)
> +    msr = msr & msrm;
>      if (msr_is_64bit(env, msr)) {
>          nip = (uint64_t)nip;
> -        msr &= (uint64_t)msrm;
>      } else {
>          nip = (uint32_t)nip;
> -        msr = (uint32_t)(msr & msrm);
>          if (keep_msrh) {
> +	    msr &= 0xffffffff;
>              msr |= env->msr & ~((uint64_t)0xFFFFFFFF);
>          }
>      }

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson