[Qemu-devel] [PATCH, MIPS] Code translation improvement

[Qemu-devel] [PATCH, MIPS] Code translation improvement

[Aurelien Jarno]

Hi,

The patch below makes a few improvement to the code translation, and fixes
a small bug:

- The check for the bit ERL in CP0 Status register implies that the CPU
  is in kernel mode, so there is no need to further check for user mode.

- Checking for the bit UM in CP0 Status register is not enough to detect
  the user mode, the patch below changes that to a check for 
  MIPS_HFLAG_UM in hflags.

- Currently checking if CP0 is accessible, ie either the CPU is in kernel
  mode or the bit CU0 in CP0 Status register enabled, is done in the
  translated code. The patch below moves the check for user mode at 
  translation time to improve a bit the execution time. It also saves a 
  call to save_cpu_state() if the CPU is in kernel mode. A few CP0
  instructions then needs to explicitely save the cpu state, as they
  are modifying hflags.

Cheers,
Aurelien


Index: target-mips/helper.c
===================================================================
RCS file: /sources/qemu/qemu/target-mips/helper.c,v
retrieving revision 1.42
diff -u -d -p -r1.42 helper.c
--- target-mips/helper.c	28 May 2007 20:36:48 -0000	1.42
+++ target-mips/helper.c	14 Jun 2007 19:32:56 -0000
@@ -130,7 +130,7 @@ static int get_physical_address (CPUStat
 
     if (address <= (int32_t)0x7FFFFFFFUL) {
         /* useg */
-        if (!(env->CP0_Status & (1 << CP0St_ERL) && user_mode)) {
+        if (!(env->CP0_Status & (1 << CP0St_ERL))) {
             ret = env->map_address(env, physical, prot, address, rw, access_type);
         } else {
             *physical = address & 0xFFFFFFFF;
Index: target-mips/op.c
===================================================================
RCS file: /sources/qemu/qemu/target-mips/op.c,v
retrieving revision 1.65
diff -u -d -p -r1.65 op.c
--- target-mips/op.c	29 May 2007 16:52:56 -0000	1.65
+++ target-mips/op.c	14 Jun 2007 19:32:56 -0000
@@ -297,8 +297,8 @@ void op_addr_add (void)
    with Status_UX = 0 should be casted to 32-bit and sign extended.
    See the MIPS64 PRA manual, section 4.10. */
 #ifdef TARGET_MIPS64
-    if ((env->CP0_Status & (1 << CP0St_UM)) &&
-        !(env->CP0_Status & (1 << CP0St_UX)))
+    if ((env->hflags & MIPS_HFLAG_UM) && 
+        !(env->CP0_Status & (1 << CP0St_UX)))
         T0 = (int64_t)(int32_t)(T0 + T1);
     else
 #endif
@@ -1606,10 +1614,8 @@ void op_dmfc0_errorepc (void)
 
 void op_cp0_enabled(void)
 {
-    if (!(env->CP0_Status & (1 << CP0St_CU0)) &&
-	(env->hflags & MIPS_HFLAG_UM)) {
+    if (!(env->CP0_Status & (1 << CP0St_CU0)))
         CALL_FROM_TB2(do_raise_exception_err, EXCP_CpU, 0);
-    }
     RETURN();
 }
 
Index: target-mips/translate.c
===================================================================
RCS file: /sources/qemu/qemu/target-mips/translate.c,v
retrieving revision 1.93
diff -u -d -p -r1.93 translate.c
--- target-mips/translate.c	2 Jun 2007 00:25:12 -0000	1.93
+++ target-mips/translate.c	14 Jun 2007 19:32:56 -0000
@@ -2642,6 +2642,7 @@ static void gen_mtc0 (CPUState *env, Dis
     case 12:
         switch (sel) {
         case 0:
+            save_cpu_state(ctx, 0);
             gen_op_mtc0_status();
             /* BS_STOP isn't good enough here, hflags may have changed. */
             gen_save_pc(ctx->pc + 4);
@@ -2807,6 +2808,7 @@ static void gen_mtc0 (CPUState *env, Dis
     case 23:
         switch (sel) {
         case 0:
+            save_cpu_state(ctx, 0);
             gen_op_mtc0_debug(); /* EJTAG support */
             /* BS_STOP isn't good enough here, hflags may have changed. */
             gen_save_pc(ctx->pc + 4);
@@ -4147,6 +4149,7 @@ static void gen_cp0 (CPUState *env, Disa
         break;
     case OPC_ERET:
         opn = "eret";
+        save_cpu_state(ctx, 0);
         gen_op_eret();
         ctx->bstate = BS_EXCP;
         break;
@@ -4156,6 +4159,7 @@ static void gen_cp0 (CPUState *env, Disa
             MIPS_INVAL(opn);
             generate_exception(ctx, EXCP_RI);
         } else {
+            save_cpu_state(ctx, 0);
             gen_op_deret();
             ctx->bstate = BS_EXCP;
         }
@@ -5647,8 +5651,11 @@ static void decode_opc (CPUState *env, D
         }
         break;
     case OPC_CP0:
-        save_cpu_state(ctx, 1);
-        gen_op_cp0_enabled();
+        if (ctx->hflags & MIPS_HFLAG_UM)
+        {		
+            save_cpu_state(ctx, 1);
+            gen_op_cp0_enabled();
+        }
         op1 = MASK_CP0(ctx->opcode);
         switch (op1) {
         case OPC_MFC0:

-- 
  .''`.  Aurelien Jarno	            | GPG: 1024D/F1BCDB73
 : :' :  Debian developer           | Electrical Engineer
 `. `'   aurel32@debian.org         | aurelien@aurel32.net
   `-    people.debian.org/~aurel32 | www.aurel32.net

Re: [Qemu-devel] [PATCH, MIPS] Code translation improvement

[Thiemo Seufer]

Aurelien Jarno wrote:
> Hi,
> 
> The patch below makes a few improvement to the code translation, and fixes
> a small bug:
> 
> - The check for the bit ERL in CP0 Status register implies that the CPU
>   is in kernel mode, so there is no need to further check for user mode.

Applied.

> - Checking for the bit UM in CP0 Status register is not enough to detect
>   the user mode, the patch below changes that to a check for 
>   MIPS_HFLAG_UM in hflags.

Applied.

> - Currently checking if CP0 is accessible, ie either the CPU is in kernel
>   mode or the bit CU0 in CP0 Status register enabled, is done in the
>   translated code. The patch below moves the check for user mode at 
>   translation time to improve a bit the execution time. It also saves a 
>   call to save_cpu_state() if the CPU is in kernel mode. A few CP0
>   instructions then needs to explicitely save the cpu state, as they
>   are modifying hflags.

This part breaks my standard Linux boot test. In general the idea is
sound, though.


Thiemo