Patchwork MIPS: Translate breaks and traps into the appropriate signal

login
register
mail settings
Submitter Meador Inge
Date Jan. 10, 2013, 9:46 p.m.
Message ID <1357854407-7093-1-git-send-email-meadori@codesourcery.com>
Download mbox | patch
Permalink /patch/211159/
State New
Headers show

Comments

Meador Inge - Jan. 10, 2013, 9:46 p.m.
GCC and GAS are capable of generating traps or breaks to check for
division by zero.  Additionally, GAS is capable of generating traps
or breaks to check for overflow on certain division and multiplication
operations.  The Linux kernel translates these traps and breaks into
signals.  This patch implements the corresponding feature in QEMU.

Signed-off-by: Meador Inge <meadori@codesourcery.com>
---
 linux-user/main.c |   64 ++++++++++++++++++++++++++++++++++++++++++++++++++++-
 target-mips/cpu.h |    6 +++++
 2 files changed, 69 insertions(+), 1 deletion(-)
Stefan Weil - Jan. 10, 2013, 9:57 p.m.
Am 10.01.2013 22:46, schrieb Meador Inge:
> GCC and GAS are capable of generating traps or breaks to check for
> division by zero.  Additionally, GAS is capable of generating traps
> or breaks to check for overflow on certain division and multiplication
> operations.  The Linux kernel translates these traps and breaks into
> signals.  This patch implements the corresponding feature in QEMU.
>
> Signed-off-by: Meador Inge <meadori@codesourcery.com>
> ---
>   linux-user/main.c |   64 ++++++++++++++++++++++++++++++++++++++++++++++++++++-
>   target-mips/cpu.h |    6 +++++
>   2 files changed, 69 insertions(+), 1 deletion(-)
>
>


Hi,

please check your patch before submitting it to qemu-devel.
See also http://wiki.qemu.org/Contribute/SubmitAPatch.

Regards,

Stefan W.


$ scripts/checkpatch.pl 
0001-MIPS-Translate-breaks-and-traps-into-the-appropriate.patch
WARNING: braces {} are necessary for all arms of this statement
#62: FILE: linux-user/main.c:2329:
+                if (ret != 0)
[...]

WARNING: braces {} are necessary for all arms of this statement
#70: FILE: linux-user/main.c:2337:
+                if (code >= (1 << 10))
[...]

WARNING: braces {} are necessary for all arms of this statement
#73: FILE: linux-user/main.c:2340:
+                if (do_break(env, &info, code) != 0)
[...]

WARNING: braces {} are necessary for all arms of this statement
#83: FILE: linux-user/main.c:2350:
+                if (ret != 0)
[...]

WARNING: braces {} are necessary for all arms of this statement
#87: FILE: linux-user/main.c:2354:
+                if (!(trap_instr & 0xFC000000))
[...]

WARNING: braces {} are necessary for all arms of this statement
#90: FILE: linux-user/main.c:2357:
+                if (do_break(env, &info, code) != 0)
[...]

total: 0 errors, 6 warnings, 89 lines checked

0001-MIPS-Translate-breaks-and-traps-into-the-appropriate.patch has 
style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
Peter Maydell - Jan. 10, 2013, 10:12 p.m.
On 10 January 2013 21:46, Meador Inge <meadori@codesourcery.com> wrote:
> --- a/target-mips/cpu.h
> +++ b/target-mips/cpu.h
> @@ -620,6 +620,12 @@ enum {
>  /* Dummy exception for conditional stores.  */
>  #define EXCP_SC 0x100
>
> +/* Break codes */
> +enum {
> +    BRK_OVERFLOW = 6,
> +    BRK_DIVZERO = 7
> +};

This is an OS/ABI specific define, right? I don't think it
belongs in the target-mips header file. Since it only has one
user, I think you could reasonably just put it in linux-user/main.c.

-- PMM
Meador Inge - Jan. 10, 2013, 10:15 p.m.
On 01/10/2013 03:57 PM, Stefan Weil wrote:

> please check your patch before submitting it to qemu-devel.
> See also http://wiki.qemu.org/Contribute/SubmitAPatch.

Ah, thanks for the pointer.  I have fixed the style violations.
Meador Inge - Jan. 10, 2013, 10:27 p.m.
On 01/10/2013 04:12 PM, Peter Maydell wrote:

> This is an OS/ABI specific define, right? I don't think it
> belongs in the target-mips header file. Since it only has one
> user, I think you could reasonably just put it in linux-user/main.c.

The enum will only be used in the MIPS CPU loop.  I originally put it in
target-mips/cpu.h because that is where the exception codes
are defined.   However, the one user argument makes sense to me.  I
moved the enum definition.

Thanks for the review.

Patch

diff --git a/linux-user/main.c b/linux-user/main.c
index 9ade1bf..b9532e0 100644
--- a/linux-user/main.c
+++ b/linux-user/main.c
@@ -2182,6 +2182,26 @@  static int do_store_exclusive(CPUMIPSState *env)
     return segv;
 }
 
+static int do_break(CPUMIPSState *env, target_siginfo_t *info,
+                    unsigned int code)
+{
+    int ret = -1;
+
+    switch (code) {
+    case BRK_OVERFLOW:
+    case BRK_DIVZERO:
+        info->si_signo = TARGET_SIGFPE;
+        info->si_errno = 0;
+        info->si_code = (code == BRK_OVERFLOW) ? FPE_INTOVF : FPE_INTDIV;
+        queue_signal(env, info->si_signo, &*info);
+        ret = 0;
+    default:
+        break;
+    }
+
+    return ret;
+}
+
 void cpu_loop(CPUMIPSState *env)
 {
     target_siginfo_t info;
@@ -2297,8 +2317,50 @@  done_syscall:
             info.si_code = TARGET_ILL_ILLOPC;
             queue_signal(env, info.si_signo, &info);
             break;
+        /* The code below was inspired by the MIPS Linux kernel trap
+         * handling code in arch/mips/kernel/traps.c.
+         */
+        case EXCP_BREAK:
+            {
+                abi_ulong trap_instr;
+                unsigned int code;
+
+                ret = get_user_ual(trap_instr, env->active_tc.PC);
+                if (ret != 0)
+                    goto error;
+
+                /* As described in the original Linux kernel code, the
+                 * below checks on 'code' are to work around an old
+                 * assembly bug.
+                 */
+                code = ((trap_instr >> 6) & ((1 << 20) - 1));
+                if (code >= (1 << 10))
+                    code >>= 10;
+
+                if (do_break(env, &info, code) != 0)
+                    goto error;
+                break;
+            }
+        case EXCP_TRAP:
+            {
+                abi_ulong trap_instr;
+                unsigned int code = 0;
+
+                ret = get_user_ual(trap_instr, env->active_tc.PC);
+                if (ret != 0)
+                    goto error;
+
+                /* The immediate versions don't provide a code.  */
+                if (!(trap_instr & 0xFC000000))
+                    code = ((trap_instr >> 6) & ((1 << 10) - 1));
+
+                if (do_break(env, &info, code) != 0)
+                    goto error;
+                break;
+            }
+            break;
         default:
-            //        error:
+error:
             fprintf(stderr, "qemu: unhandled CPU exception 0x%x - aborting\n",
                     trapnr);
             cpu_dump_state(env, stderr, fprintf, 0);
diff --git a/target-mips/cpu.h b/target-mips/cpu.h
index 5963d62..c5fbe04 100644
--- a/target-mips/cpu.h
+++ b/target-mips/cpu.h
@@ -620,6 +620,12 @@  enum {
 /* Dummy exception for conditional stores.  */
 #define EXCP_SC 0x100
 
+/* Break codes */
+enum {
+    BRK_OVERFLOW = 6,
+    BRK_DIVZERO = 7
+};
+
 /*
  * This is an interrnally generated WAKE request line.
  * It is driven by the CPU itself. Raised when the MT