| Message ID | 1470194197.12584.45.camel@kernel.crashing.org |
|---|---|
| State | New |
| Headers | show |
On Wed, 2016-08-03 at 13:16 +1000, Benjamin Herrenschmidt wrote: > 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. > > Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org> > --- 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. > linux-user/ppc/syscall_nr.h | 2 ++ > linux-user/signal.c | 75 +++++++++++++++++++++++++-------- > ------------ > 2 files changed, 44 insertions(+), 33 deletions(-) > > diff --git a/linux-user/ppc/syscall_nr.h b/linux- > user/ppc/syscall_nr.h > index 46ed8a6..afa3654 100644 > --- a/linux-user/ppc/syscall_nr.h > +++ b/linux-user/ppc/syscall_nr.h > @@ -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 > diff --git a/linux-user/signal.c b/linux-user/signal.c > index 9a4d894..af80a3e 100644 > --- a/linux-user/signal.c > +++ b/linux-user/signal.c > @@ -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 > @@ -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) > -#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. */ > @@ -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- > >mc_vregs.altivec[i]; > > __put_user(avr->u64[0], &vreg->u64[0]); > __put_user(avr->u64[1], &vreg->u64[1]); > @@ -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); > +#else > + vrsave = (uint32_t *)&frame->mc_vregs.altivec[32]; > +#endif > + __put_user((uint32_t)env->spr[SPR_VRSAVE], vrsave); > } > > /* Save floating point registers. */ > @@ -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- > >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- > >mc_vregs.altivec[32].u32[3])); > +#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. */ > @@ -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) > { > @@ -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); > @@ -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); > -#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); > > @@ -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); > @@ -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, > @@ -4914,6 +4920,7 @@ sigsegv: > > } > > +#if !defined(TARGET_PPC64) > long do_sigreturn(CPUPPCState *env) > { > struct target_sigcontext *sc = NULL; > @@ -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) > @@ -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
diff --git a/linux-user/ppc/syscall_nr.h b/linux-user/ppc/syscall_nr.h index 46ed8a6..afa3654 100644 --- a/linux-user/ppc/syscall_nr.h +++ b/linux-user/ppc/syscall_nr.h @@ -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 diff --git a/linux-user/signal.c b/linux-user/signal.c index 9a4d894..af80a3e 100644 --- a/linux-user/signal.c +++ b/linux-user/signal.c @@ -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 @@ -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) -#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. */ @@ -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->mc_vregs.altivec[i]; __put_user(avr->u64[0], &vreg->u64[0]); __put_user(avr->u64[1], &vreg->u64[1]); @@ -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); +#else + vrsave = (uint32_t *)&frame->mc_vregs.altivec[32]; +#endif + __put_user((uint32_t)env->spr[SPR_VRSAVE], vrsave); } /* Save floating point registers. */ @@ -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->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->mc_vregs.altivec[32].u32[3])); +#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. */ @@ -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) { @@ -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); @@ -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); -#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); @@ -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); @@ -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, @@ -4914,6 +4920,7 @@ sigsegv: } +#if !defined(TARGET_PPC64) long do_sigreturn(CPUPPCState *env) { struct target_sigcontext *sc = NULL; @@ -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) @@ -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
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. Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org> --- linux-user/ppc/syscall_nr.h | 2 ++ linux-user/signal.c | 75 +++++++++++++++++++++++++-------------------- 2 files changed, 44 insertions(+), 33 deletions(-)