Skip to contentSkip to conversation

ppc: Fix signal delivery in 64-bit usermode qemu

PATCH In review Benjamin Herrenschmidt 1 participant 2 messages Aug 3, 2016

lore
Share & tools
lore
#0 lore raw

PATCH 3/3 ppc: Fix signal delivery in 64-bit usermode qemu

There were a number of bugs in the implementation. The structure alignment was wrong for 64-bit. Also 64-bit only does RT signals.

Finally on 64-bit, we need to put a pointer to the (aligned) vector registers in the frame.

This is still missing support for VSX which I will add separately.

---
 linux-user/ppc/syscall_nr.h |  2 ++
 linux-user/signal.c         | 75 +++++++++++++++++++++++++--------------------
 2 files changed, 44 insertions(+), 33 deletions(-)
linux-user/ppc/syscall_nr.h +2 -0
index 46ed8a6..afa3654 100644
@@ -120,7 +120,9 @@
 #define TARGET_NR_sysinfo                116
 #define TARGET_NR_ipc                    117
 #define TARGET_NR_fsync                  118
+#if !defined(TARGET_PPC64)
 #define TARGET_NR_sigreturn              119
+#endif
 #define TARGET_NR_clone                  120
 #define TARGET_NR_setdomainname          121
 #define TARGET_NR_uname                  122
linux-user/signal.c +42 -33

171 lines not on this page · raw message

#1 lore raw
Quoted 10 lines · Benjamin Herrenschmidt

There were a number of bugs in the implementation. The structure alignment was wrong for 64-bit. Also 64-bit only does RT signals.

Finally on 64-bit, we need to put a pointer to the (aligned) vector registers in the frame.

This is still missing support for VSX which I will add separately.

---

Dont apply this just yet ... the patch is correct but doesn't fix all the bugs :-) There's some endian crap too. Will post a new one.

Quoted 228 lines

 linux-user/ppc/syscall_nr.h |  2 ++
 linux-user/signal.c         | 75 +++++++++++++++++++++++++--------
------------

 2 files changed, 44 insertions(+), 33 deletions(-)
linux- renamed from linux-user/ppc/syscall_nr.h +0 -0

user/ppc/syscall_nr.h
index 46ed8a6..afa3654 100644

linux-user/ppc/syscall_nr.h +2 -0
@@ -120,7 +120,9 @@
  #define TARGET_NR_sysinfo                116
  #define TARGET_NR_ipc                    117
  #define TARGET_NR_fsync                  118
+#if !defined(TARGET_PPC64)
  #define TARGET_NR_sigreturn              119
+#endif
  #define TARGET_NR_clone                  120
  #define TARGET_NR_setdomainname          121
  #define TARGET_NR_uname                  122
linux-user/signal.c +5 -0
index 9a4d894..af80a3e 100644
@@ -4408,7 +4408,12 @@ struct target_mcontext {
      target_ulong mc_gregs[48];
      /* Includes fpscr.  */
      uint64_t mc_fregs[33];
+#if defined(TARGET_PPC64)
+    /* Pointer to the vector regs */
+    target_ulong v_regs;
+#else
      target_ulong mc_pad[2];
+#endif
      /* We need to handle Altivec and SPE at the same time, which no

        kernel needs to do.  Fortunately, the kernel defines this bit to
        be Altivec-register-large all the time, rather than trying to

linux-user/signal.c +0 -0
@@ -4418,15 +4423,24 @@ struct target_mcontext {
          uint32_t spe[33];
          /* Altivec vector registers.  The packing of VSCR and VRSAVE

            varies depending on whether we're PPC64 or not: PPC64 splits
-           them apart; PPC32 stuffs them together.  */ +           them apart; PPC32 stuffs them together.
+           We also need to account for the VSX registers on PPC64 +        */
 #if defined(TARGET_PPC64)

linux-user/signal.c +3 -1
-#define QEMU_NVRREG 34
+#define QEMU_NVRREG (34 + 16)
+        /* On ppc64, we need to align to 16 bytes by hand */
+        target_ulong pad;

 #else
+        /* On ppc32, we are already aligned to 16 bytes */ #define QEMU_NVRREG 33
 #endif
-        ppc_avr_t altivec[QEMU_NVRREG];
+        /* We cannot use ppc_avr_t here as we do *not* want the implied
+         * 16-bytes alignment that would result from it. This would have
+         * the effect of making. The 32-bit variant is already aligned.
+         */
+        uint64_t altivec[QEMU_NVRREG][2];
 #undef QEMU_NVRREG
-    } mc_vregs __attribute__((__aligned__(16)));
+    } mc_vregs;
 };

 /* See arch/powerpc/include/asm/sigcontext.h.  */

linux-user/signal.c +0 -0
@@ -4606,9 +4620,10 @@ static void save_user_regs(CPUPPCState *env,
 struct target_mcontext *frame)


     /* Save Altivec registers if necessary.  */ if (env->insns_flags & PPC_ALTIVEC) {
+        uint32_t *vrsave;
         for (i = 0; i < ARRAY_SIZE(env->avr); i++) { ppc_avr_t *avr = &env->avr[i];
-            ppc_avr_t *vreg = &frame->mc_vregs.altivec[i]; +            ppc_avr_t *vreg = (ppc_avr_t *)&frame-

Quoted 1 line

mc_vregs.altivec[i];


             __put_user(avr->u64[0], &vreg->u64[0]); __put_user(avr->u64[1], &vreg->u64[1]);

linux-user/signal.c +0 -0
@@ -4616,8 +4631,14 @@ static void save_user_regs(CPUPPCState *env,
 struct target_mcontext *frame)
          /* Set MSR_VR in the saved MSR value to indicate that

            frame->mc_vregs contains valid data.  */
         msr |= MSR_VR;
-        __put_user((uint32_t)env->spr[SPR_VRSAVE],
-                   &frame->mc_vregs.altivec[32].u32[3]); +#if defined(TARGET_PPC64)
+        vrsave = (uint32_t *)&frame->mc_vregs.altivec[33]; +        /* 64-bit needs to put a pointer to the vectors in the frame */
+        __put_user(h2g(frame->mc_vregs.altivec), &frame->v_regs);

linux-user/signal.c +4 -0
+#else
+        vrsave = (uint32_t *)&frame->mc_vregs.altivec[32];
+#endif
+        __put_user((uint32_t)env->spr[SPR_VRSAVE], vrsave);

     }

     /* Save floating point registers.  */

linux-user/signal.c +0 -0
@@ -4697,17 +4718,22 @@ static void restore_user_regs(CPUPPCState
 *env,


     /* Restore Altivec registers if necessary.  */ if (env->insns_flags & PPC_ALTIVEC) {
+        uint32_t *vrsave;
         for (i = 0; i < ARRAY_SIZE(env->avr); i++) { ppc_avr_t *avr = &env->avr[i];
-            ppc_avr_t *vreg = &frame->mc_vregs.altivec[i]; +            ppc_avr_t *vreg = (ppc_avr_t *)&frame-

Quoted 1 line

mc_vregs.altivec[i];


             __get_user(avr->u64[0], &vreg->u64[0]); __get_user(avr->u64[1], &vreg->u64[1]); }
         /* Set MSR_VEC in the saved MSR value to indicate that frame->mc_vregs contains valid data.  */ -        __get_user(env->spr[SPR_VRSAVE],
-                   (target_ulong *)(&frame-

Quoted 1 line

mc_vregs.altivec[32].u32[3]));

linux-user/signal.c +6 -0
+#if defined(TARGET_PPC64)
+        vrsave = (uint32_t *)&frame->mc_vregs.altivec[33];
+#else
+        vrsave = (uint32_t *)&frame->mc_vregs.altivec[32];
+#endif
+        __get_user(env->spr[SPR_VRSAVE], vrsave);

     }

     /* Restore floating point registers.  */

linux-user/signal.c +0 -0
@@ -4738,6 +4764,7 @@ static void restore_user_regs(CPUPPCState *env,
      }
  }


+#if !defined(TARGET_PPC64)
 static void setup_frame(int sig, struct target_sigaction *ka, target_sigset_t *set, CPUPPCState *env) {

linux-user/signal.c +0 -3
@@ -4745,9 +4772,6 @@ static void setup_frame(int sig, struct
 target_sigaction *ka,
      struct target_sigcontext *sc;
      target_ulong frame_addr, newsp;
      int err = 0;
-#if defined(TARGET_PPC64)
-    struct image_info *image = ((TaskState *)thread_cpu->opaque)-
 >info;
-#endif


     frame_addr = get_sigframe(ka, env, sizeof(*frame)); trace_user_setup_frame(env, frame_addr);

linux-user/signal.c +0 -0
@@ -4757,11 +4781,7 @@ static void setup_frame(int sig, struct
 target_sigaction *ka,


     __put_user(ka->_sa_handler, &sc->handler);
     __put_user(set->sig[0], &sc->oldmask);

linux-user/signal.c +0 -3
-#if TARGET_ABI_BITS == 64
-    __put_user(set->sig[0] >> 32, &sc->_unused[3]);
-#else

     __put_user(set->sig[1], &sc->_unused[3]);
-#endif
     __put_user(h2g(&frame->mctx), &sc->regs);
     __put_user(sig, &sc->signal);

linux-user/signal.c +0 -0
@@ -4790,22 +4810,7 @@ static void setup_frame(int sig, struct
 target_sigaction *ka,
      env->gpr[3] = sig;
      env->gpr[4] = frame_addr + offsetof(struct target_sigframe,
 sctx);


-#if defined(TARGET_PPC64)
-    if (get_ppc64_abi(image) < 2) {
-        /* ELFv1 PPC64 function pointers are pointers to OPD entries. */
-        struct target_func_ptr *handler =
-            (struct target_func_ptr *)g2h(ka->_sa_handler); -        env->nip = tswapl(handler->entry);
-        env->gpr[2] = tswapl(handler->toc);
-    } else {
-        /* ELFv2 PPC64 function pointers are entry points, but R12 -         * must also be set */
-        env->nip = tswapl((target_ulong) ka->_sa_handler); -        env->gpr[12] = env->nip;
-    }
-#else
     env->nip = (target_ulong) ka->_sa_handler;
-#endif

     /* Signal handlers are entered in big-endian mode.  */ env->msr &= ~(1ull << MSR_LE);

linux-user/signal.c +1 -0
@@ -4817,6 +4822,7 @@ sigsegv:
      unlock_user_struct(frame, frame_addr, 1);
      force_sig(TARGET_SIGSEGV);
  }
+#endif /* !defined(TARGET_PPC64) */


 static void setup_rt_frame(int sig, struct target_sigaction *ka, target_siginfo_t *info,

linux-user/signal.c +0 -0
@@ -4914,6 +4920,7 @@ sigsegv:


 }

+#if !defined(TARGET_PPC64)
 long do_sigreturn(CPUPPCState *env)
 {
     struct target_sigcontext *sc = NULL;

linux-user/signal.c +1 -0
@@ -4950,6 +4957,7 @@ sigsegv:
      force_sig(TARGET_SIGSEGV);
      return 0;
  }
+#endif /* !defined(TARGET_PPC64) */


 /* See arch/powerpc/kernel/signal_32.c.  */
 static int do_setcontext(struct target_ucontext *ucp, CPUPPCState *env, int sig)

linux-user/signal.c +2 -1
@@ -5893,7 +5901,8 @@ static void handle_pending_signal(CPUArchState
 *cpu_env, int sig,
  #endif
          /* prepare the stack frame of the virtual CPU */
  #if defined(TARGET_ABI_MIPSN32) || defined(TARGET_ABI_MIPSN64) \
-    || defined(TARGET_OPENRISC) || defined(TARGET_TILEGX)
+        || defined(TARGET_OPENRISC) || defined(TARGET_TILEGX) \
+        || defined(TARGET_PPC64)
          /* These targets do not have traditional signals.  */
          setup_rt_frame(sig, sa, &k->info, &target_old_set, cpu_env);

 #else