[Qemu-devel] [PATCH v2 0/2] linux-user: Fix MIPS16/microMIPS signal handling

[Qemu-devel] [PATCH v2 0/2] linux-user: Fix MIPS16/microMIPS signal handling

[Kwok Cheung Yeung]

These patches fix various issues related to signal handling in user mode
emulation for the MIPS architecture.

- When a MIPS16/microMIPS signal handler is called, the program segfaults
because the PC is set to an invalid address.
- When returning from a signal handler, the ISA mode is not set to that
of the resume instruction.
- When the faulting instruction is in a branch delay slot, the resume
address is set to that of the instruction rather than the branch,
resulting in incorrect behaviour.  The flag indicating that the
instruction is in a delay slot is also not cleared.

v1 -> v2:
- Add fixes for signal return and delay slot instructions
- Refactor code

Kwok Cheung Yeung (2):
  linux-user: Fix MIPS ISA transitions during signal handling
  linux-user: Save the correct resume address for MIPS signal handling

 linux-user/signal.c  | 16 +++++++++++++++-
 target-mips/cpu.h    |  1 +
 target-mips/helper.c |  4 ++--
 3 files changed, 18 insertions(+), 3 deletions(-)

-- 
1.8.1.2

[Qemu-devel] [PATCH v2 1/2] linux-user: Fix MIPS ISA transitions during signal handling

[Kwok Cheung Yeung]

Processors supporting the MIPS16 or microMIPS ISAs set bit 0 in target
addresses to indicate that the target is written using a compressed ISA.

During signal handling, when jumping to or returning from a signal
handler, bit 0 of the destination PC is inspected and MIPS_HFLAG_M16 in
hflags cleared or set accordingly.  Bit 0 of the PC is then cleared.

Signed-off-by: Kwok Cheung Yeung <kcy@codesourcery.com>
---
 linux-user/signal.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/linux-user/signal.c b/linux-user/signal.c
index 1055507..dc34ae7 100644
--- a/linux-user/signal.c
+++ b/linux-user/signal.c
@@ -2620,6 +2620,15 @@ get_sigframe(struct target_sigaction *ka, CPUMIPSState *regs, size_t frame_size)
     return (sp - frame_size) & ~7;
 }
 
+static void mips_set_hflags_isa_mode_from_pc(CPUMIPSState *env)
+{
+    if (env->insn_flags & (ASE_MIPS16 | ASE_MICROMIPS)) {
+        env->hflags &= ~MIPS_HFLAG_M16;
+        env->hflags |= (env->active_tc.PC & 1) << MIPS_HFLAG_M16_SHIFT;
+        env->active_tc.PC &= ~(target_ulong) 1;
+    }
+}
+
 # if defined(TARGET_ABI_MIPSO32)
 /* compare linux/arch/mips/kernel/signal.c:setup_frame() */
 static void setup_frame(int sig, struct target_sigaction * ka,
@@ -2662,6 +2671,7 @@ static void setup_frame(int sig, struct target_sigaction * ka,
     * since it returns to userland using eret
     * we cannot do this here, and we must set PC directly */
     regs->active_tc.PC = regs->active_tc.gpr[25] = ka->_sa_handler;
+    mips_set_hflags_isa_mode_from_pc(regs);
     unlock_user_struct(frame, frame_addr, 1);
     return;
 
@@ -2709,6 +2719,7 @@ long do_sigreturn(CPUMIPSState *regs)
 #endif
 
     regs->active_tc.PC = regs->CP0_EPC;
+    mips_set_hflags_isa_mode_from_pc(regs);
     /* I am not sure this is right, but it seems to work
     * maybe a problem with nested signals ? */
     regs->CP0_EPC = 0;
@@ -2771,6 +2782,7 @@ static void setup_rt_frame(int sig, struct target_sigaction *ka,
     * since it returns to userland using eret
     * we cannot do this here, and we must set PC directly */
     env->active_tc.PC = env->active_tc.gpr[25] = ka->_sa_handler;
+    mips_set_hflags_isa_mode_from_pc(env);
     unlock_user_struct(frame, frame_addr, 1);
     return;
 
@@ -2804,6 +2816,7 @@ long do_rt_sigreturn(CPUMIPSState *env)
         goto badframe;
 
     env->active_tc.PC = env->CP0_EPC;
+    mips_set_hflags_isa_mode_from_pc(env);
     /* I am not sure this is right, but it seems to work
     * maybe a problem with nested signals ? */
     env->CP0_EPC = 0;
-- 
1.8.1.2

[Qemu-devel] [PATCH v2 2/2] linux-user: Save the correct resume address for MIPS signal handling

[Kwok Cheung Yeung]

The current ISA mode needs to be saved in bit 0 of the resume address.
If the current instruction happens to be in a branch delay slot, then
the address of the preceding jump instruction should be stored instead.
exception_resume_pc already does both of these tasks, so it is
made available and reused.

MIPS_HFLAG_BMASK in hflags is cleared, otherwise QEMU may treat the
first instruction of the signal handler as a delay slot instruction.

Signed-off-by: Kwok Cheung Yeung <kcy@codesourcery.com>
---
 linux-user/signal.c  | 3 ++-
 target-mips/cpu.h    | 1 +
 target-mips/helper.c | 4 ++--
 3 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/linux-user/signal.c b/linux-user/signal.c
index dc34ae7..5da8452 100644
--- a/linux-user/signal.c
+++ b/linux-user/signal.c
@@ -2528,7 +2528,8 @@ setup_sigcontext(CPUMIPSState *regs, struct target_sigcontext *sc)
     int err = 0;
     int i;
 
-    err |= __put_user(regs->active_tc.PC, &sc->sc_pc);
+    err |= __put_user(exception_resume_pc(regs), &sc->sc_pc);
+    regs->hflags &= ~MIPS_HFLAG_BMASK;
 
     __put_user(0, &sc->sc_regs[0]);
     for (i = 1; i < 32; ++i) {
diff --git a/target-mips/cpu.h b/target-mips/cpu.h
index cedf03d..6e761e0 100644
--- a/target-mips/cpu.h
+++ b/target-mips/cpu.h
@@ -668,6 +668,7 @@ void r4k_invalidate_tlb (CPUMIPSState *env, int idx, int use_extra);
 hwaddr cpu_mips_translate_address (CPUMIPSState *env, target_ulong address,
 		                               int rw);
 #endif
+target_ulong exception_resume_pc (CPUMIPSState *env);
 
 static inline void cpu_get_tb_cpu_state(CPUMIPSState *env, target_ulong *pc,
                                         target_ulong *cs_base, int *flags)
diff --git a/target-mips/helper.c b/target-mips/helper.c
index 3a54acf..36929dd 100644
--- a/target-mips/helper.c
+++ b/target-mips/helper.c
@@ -366,8 +366,7 @@ static const char * const excp_names[EXCP_LAST + 1] = {
     [EXCP_CACHE] = "cache error",
 };
 
-#if !defined(CONFIG_USER_ONLY)
-static target_ulong exception_resume_pc (CPUMIPSState *env)
+target_ulong exception_resume_pc (CPUMIPSState *env)
 {
     target_ulong bad_pc;
     target_ulong isa_mode;
@@ -383,6 +382,7 @@ static target_ulong exception_resume_pc (CPUMIPSState *env)
     return bad_pc;
 }
 
+#if !defined(CONFIG_USER_ONLY)
 static void set_hflags_for_handler (CPUMIPSState *env)
 {
     /* Exception handlers are entered in 32-bit mode.  */
-- 
1.8.1.2

Re: [Qemu-devel] [PATCH v2 0/2] linux-user: Fix MIPS16/microMIPS signal handling

[Aurelien Jarno]

On Fri, May 17, 2013 at 02:51:19PM -0700, Kwok Cheung Yeung wrote:
> These patches fix various issues related to signal handling in user mode
> emulation for the MIPS architecture.
> 
> - When a MIPS16/microMIPS signal handler is called, the program segfaults
> because the PC is set to an invalid address.
> - When returning from a signal handler, the ISA mode is not set to that
> of the resume instruction.
> - When the faulting instruction is in a branch delay slot, the resume
> address is set to that of the instruction rather than the branch,
> resulting in incorrect behaviour.  The flag indicating that the
> instruction is in a delay slot is also not cleared.
> 
> v1 -> v2:
> - Add fixes for signal return and delay slot instructions
> - Refactor code
> 
> Kwok Cheung Yeung (2):
>   linux-user: Fix MIPS ISA transitions during signal handling
>   linux-user: Save the correct resume address for MIPS signal handling
> 
>  linux-user/signal.c  | 16 +++++++++++++++-
>  target-mips/cpu.h    |  1 +
>  target-mips/helper.c |  4 ++--
>  3 files changed, 18 insertions(+), 3 deletions(-)
> 

Thanks, both queued for 1.6.

-- 
Aurelien Jarno                          GPG: 1024D/F1BCDB73
aurelien@aurel32.net                 http://www.aurel32.net