[Qemu-devel] [PATCH] linux-user,target-ppc: fix use of MSR_LE

[Qemu-devel] [PATCH] linux-user,target-ppc: fix use of MSR_LE

[Laurent Vivier]

setup_frame()/setup_rt_frame()/restore_user_regs() are using
MSR_LE as the similar kernel functions do: as a bitmask.

But in QEMU, MSR_LE is a bit position, so change this
accordingly.

The previous code was doing nothing as MSR_LE is 0,
and "env->msr &= ~MSR_LE" doesn't change the value of msr.

And yes, a user process can change its endianness,
see linux kernel commit:

    fab5db9 [PATCH] powerpc: Implement support for setting little-endian mode via prctl

and prctl(2): PR_SET_ENDIAN, PR_GET_ENDIAN

Signed-off-by: Laurent Vivier <laurent@vivier.eu>
---
 linux-user/signal.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/linux-user/signal.c b/linux-user/signal.c
index a233bab..f1b597b 100644
--- a/linux-user/signal.c
+++ b/linux-user/signal.c
@@ -4588,7 +4588,7 @@ static void restore_user_regs(CPUPPCState *env,
 
     /* If doing signal return, restore the previous little-endian mode.  */
     if (sig)
-        env->msr = (env->msr & ~MSR_LE) | (msr & MSR_LE);
+        env->msr = (env->msr & ~(1ull << MSR_LE)) | (msr & (1ull << MSR_LE));
 
     /* Restore Altivec registers if necessary.  */
     if (env->insns_flags & PPC_ALTIVEC) {
@@ -4703,7 +4703,7 @@ static void setup_frame(int sig, struct target_sigaction *ka,
 #endif
 
     /* Signal handlers are entered in big-endian mode.  */
-    env->msr &= ~MSR_LE;
+    env->msr &= ~(1ull << MSR_LE);
 
     unlock_user_struct(frame, frame_addr, 1);
     return;
@@ -4798,7 +4798,7 @@ static void setup_rt_frame(int sig, struct target_sigaction *ka,
 #endif
 
     /* Signal handlers are entered in big-endian mode.  */
-    env->msr &= ~MSR_LE;
+    env->msr &= ~(1ull << MSR_LE);
 
     unlock_user_struct(rt_sf, rt_sf_addr, 1);
     return;
-- 
2.5.0

Re: [Qemu-devel] [PATCH] linux-user,target-ppc: fix use of MSR_LE

[Thomas Huth]

On 30.03.2016 18:36, Laurent Vivier wrote:
> setup_frame()/setup_rt_frame()/restore_user_regs() are using
> MSR_LE as the similar kernel functions do: as a bitmask.
> 
> But in QEMU, MSR_LE is a bit position, so change this
> accordingly.
> 
> The previous code was doing nothing as MSR_LE is 0,
> and "env->msr &= ~MSR_LE" doesn't change the value of msr.
> 
> And yes, a user process can change its endianness,
> see linux kernel commit:
> 
>     fab5db9 [PATCH] powerpc: Implement support for setting little-endian mode via prctl
> 
> and prctl(2): PR_SET_ENDIAN, PR_GET_ENDIAN
> 
> Signed-off-by: Laurent Vivier <laurent@vivier.eu>
> ---
>  linux-user/signal.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/linux-user/signal.c b/linux-user/signal.c
> index a233bab..f1b597b 100644
> --- a/linux-user/signal.c
> +++ b/linux-user/signal.c
> @@ -4588,7 +4588,7 @@ static void restore_user_regs(CPUPPCState *env,
>  
>      /* If doing signal return, restore the previous little-endian mode.  */
>      if (sig)
> -        env->msr = (env->msr & ~MSR_LE) | (msr & MSR_LE);
> +        env->msr = (env->msr & ~(1ull << MSR_LE)) | (msr & (1ull << MSR_LE));
>  
>      /* Restore Altivec registers if necessary.  */
>      if (env->insns_flags & PPC_ALTIVEC) {
> @@ -4703,7 +4703,7 @@ static void setup_frame(int sig, struct target_sigaction *ka,
>  #endif
>  
>      /* Signal handlers are entered in big-endian mode.  */
> -    env->msr &= ~MSR_LE;
> +    env->msr &= ~(1ull << MSR_LE);
>  
>      unlock_user_struct(frame, frame_addr, 1);
>      return;
> @@ -4798,7 +4798,7 @@ static void setup_rt_frame(int sig, struct target_sigaction *ka,
>  #endif
>  
>      /* Signal handlers are entered in big-endian mode.  */
> -    env->msr &= ~MSR_LE;
> +    env->msr &= ~(1ull << MSR_LE);
>  
>      unlock_user_struct(rt_sf, rt_sf_addr, 1);
>      return;
> 

Reviewed-by: Thomas Huth <huth@tuxfamily.org>