Re: [PATCH 2/3] linux-user/s390x: Clean up signal.c

2021-04-28 Thread David Hildenbrand

On 28.04.21 05:32, Richard Henderson wrote:

The "save" routines from the kernel, which are currently
commented out, are unnecessary in qemu.  We can copy from
env where the kernel needs special instructions.

Drop the return value from restore_sigregs, as it cannot fail.
Use __get_user return as input to trace, so that we properly
bswap the value for the host.

Reorder the function bodies to correspond to the kernel source.
Fix the use of host addresses where guest addresses are needed.
Drop the use of PSW_ADDR_AMODE, since we only support 64-bit.
Set psw.mask properly for the signal handler.
Use tswap_sigset in setup_rt_frame.


This would be a lot easier to review if this would be split up into 
individual patches.


--
Thanks,

David / dhildenb




[PATCH 2/3] linux-user/s390x: Clean up signal.c

2021-04-27 Thread Richard Henderson
The "save" routines from the kernel, which are currently
commented out, are unnecessary in qemu.  We can copy from
env where the kernel needs special instructions.

Drop the return value from restore_sigregs, as it cannot fail.
Use __get_user return as input to trace, so that we properly
bswap the value for the host.

Reorder the function bodies to correspond to the kernel source.
Fix the use of host addresses where guest addresses are needed.
Drop the use of PSW_ADDR_AMODE, since we only support 64-bit.
Set psw.mask properly for the signal handler.
Use tswap_sigset in setup_rt_frame.

Signed-off-by: Richard Henderson 
---
 linux-user/s390x/signal.c | 184 ++
 1 file changed, 89 insertions(+), 95 deletions(-)

diff --git a/linux-user/s390x/signal.c b/linux-user/s390x/signal.c
index e5bc4f0358..fb7065f243 100644
--- a/linux-user/s390x/signal.c
+++ b/linux-user/s390x/signal.c
@@ -32,7 +32,6 @@
 #define _SIGCONTEXT_NSIG_BPW64 /* FIXME: 31-bit mode -> 32 */
 #define _SIGCONTEXT_NSIG_WORDS  (_SIGCONTEXT_NSIG / _SIGCONTEXT_NSIG_BPW)
 #define _SIGMASK_COPY_SIZE(sizeof(unsigned long)*_SIGCONTEXT_NSIG_WORDS)
-#define PSW_ADDR_AMODE0xUL /* 0x8000UL for 
31-bit */
 #define S390_SYSCALL_OPCODE ((uint16_t)0x0a00)
 
 typedef struct {
@@ -106,23 +105,25 @@ get_sigframe(struct target_sigaction *ka, CPUS390XState 
*env, size_t frame_size)
 static void save_sigregs(CPUS390XState *env, target_sigregs *sregs)
 {
 int i;
-//save_access_regs(current->thread.acrs); FIXME
 
-/* Copy a 'clean' PSW mask to the user to avoid leaking
-   information about whether PER is currently on.  */
+/*
+ * Copy a 'clean' PSW mask to the user to avoid leaking
+ * information about whether PER is currently on.
+ */
 __put_user(env->psw.mask, &sregs->regs.psw.mask);
 __put_user(env->psw.addr, &sregs->regs.psw.addr);
+
 for (i = 0; i < 16; i++) {
 __put_user(env->regs[i], &sregs->regs.gprs[i]);
 }
 for (i = 0; i < 16; i++) {
 __put_user(env->aregs[i], &sregs->regs.acrs[i]);
 }
+
 /*
  * We have to store the fp registers to current->thread.fp_regs
  * to merge them with the emulated registers.
  */
-//save_fp_regs(¤t->thread.fp_regs); FIXME
 for (i = 0; i < 16; i++) {
 __put_user(*get_freg(env, i), &sregs->fpregs.fprs[i]);
 }
@@ -137,120 +138,124 @@ void setup_frame(int sig, struct target_sigaction *ka,
 frame_addr = get_sigframe(ka, env, sizeof(*frame));
 trace_user_setup_frame(env, frame_addr);
 if (!lock_user_struct(VERIFY_WRITE, frame, frame_addr, 0)) {
-goto give_sigsegv;
-}
-
-__put_user(set->sig[0], &frame->sc.oldmask[0]);
-
-save_sigregs(env, &frame->sregs);
-
-__put_user((abi_ulong)(unsigned long)&frame->sregs,
-   (abi_ulong *)&frame->sc.sregs);
-
-/* Set up to return from userspace.  If provided, use a stub
-   already in userspace.  */
-if (ka->sa_flags & TARGET_SA_RESTORER) {
-env->regs[14] = (unsigned long)
-ka->sa_restorer | PSW_ADDR_AMODE;
-} else {
-env->regs[14] = (frame_addr + offsetof(sigframe, retcode))
-| PSW_ADDR_AMODE;
-__put_user(S390_SYSCALL_OPCODE | TARGET_NR_sigreturn,
-   (uint16_t *)(frame->retcode));
+force_sigsegv(sig);
+return;
 }
 
 /* Set up backchain. */
 __put_user(env->regs[15], (abi_ulong *) frame);
 
+/* Create struct sigcontext on the signal stack. */
+for (int i = 0; i < ARRAY_SIZE(frame->sc.oldmask); ++i) {
+__put_user(set->sig[i], &frame->sc.oldmask[i]);
+}
+
+save_sigregs(env, &frame->sregs);
+__put_user(frame_addr + offsetof(sigframe, sregs), &frame->sc.sregs);
+
+/* Place signal number on stack to allow backtrace from handler.  */
+__put_user(sig, &frame->signo);
+
+/*
+ * Set up to return from userspace.
+ * If provided, use a stub already in userspace.
+ */
+if (ka->sa_flags & TARGET_SA_RESTORER) {
+env->regs[14] = ka->sa_restorer;
+} else {
+env->regs[14] = frame_addr + offsetof(sigframe, retcode);
+__put_user(S390_SYSCALL_OPCODE | TARGET_NR_sigreturn,
+   (uint16_t *)(frame->retcode));
+}
+
 /* Set up registers for signal handler */
 env->regs[15] = frame_addr;
-env->psw.addr = (target_ulong) ka->_sa_handler | PSW_ADDR_AMODE;
+
+/* Force default amode and default user address space control. */
+env->psw.mask = PSW_MASK_64 | PSW_MASK_32 | PSW_ASC_PRIMARY
+  | (env->psw.mask & ~PSW_MASK_ASC);
+env->psw.addr = ka->_sa_handler;
 
 env->regs[2] = sig; //map_signal(sig);
 env->regs[3] = frame_addr += offsetof(typeof(*frame), sc);
 
-/* We forgot to include these in the sigcontext.
-   To avoid breaking binary compatibility, they are passed as args. */
-env->regs[4] = 0; // FIXME: no clue... curre