qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [MIPS][PATCH] Fix mfc0 and dmtc0 instructions on MIPS64


From: Aurelien Jarno
Subject: Re: [Qemu-devel] [MIPS][PATCH] Fix mfc0 and dmtc0 instructions on MIPS64
Date: Sun, 13 May 2007 19:05:04 +0200
User-agent: Mutt/1.5.13 (2006-08-11)

On Sun, May 13, 2007 at 04:57:42PM +0200, Aurelien Jarno wrote:
> Thiemo Seufer a écrit :
> > Aurelien Jarno wrote:
> >> Hi all,
> >>
> >> The patch below fixes the mfc0 and dmtc0 instructions for the 
> >> MIPS64 target:
> >>
> >> - The mfc0 instruction should return the 32 lowest bits of the 
> >>   coprocessor 0 register sign extended to 64-bit.
> > 
> > Agreed, and I think it doess already. (The places where you added
> > casts read fron 32bit wide registers anyway.)
> 
> Oops, I haven't seen before that those registers were declared as
> int32_t, so that the sign extension is already done.
> 
> Forget that part.
> 
> >> - The mtc0 instruction should do the same as the dmtc0 instruction for 
> >>   64-bit coprocessor registers instead of copying only the low 32 bits.
> > 
> > I'm not entirely sure about this, but it feels wrong, as mtc0 should
> > have the same behaviour as on 32bit CPUs. What prompted the change here?
> 
> The MIPS64 ISA manual:
> 
> Operation:
>    data  GPR[rt]
>    if (Width(CPR[0,rd,sel]) = 64) then
>         CPR[0,rd,sel]  data
>    else
>         CPR[0,rd,sel]  data31..0
>    endif
> 
> But it is also need if you need to run a 32-bit kernel on MIPS64. For
> example the EntryHi register is a 64-bit register, and the higher 32
> bits (and most notably the R part of this register) has to be filled.
> This part is initialised when an exception occurs, so even if a 32-bit
> kernel don't know about it, it already holds the correct values.
> 
> >> - The XContest register does not exists on MIPS32 CPU.
> > 
> > Indeed, but simply not wiring up the instruction decoding for 32bit
> > should be good enough, no need to #ifdef everything.
> > 
> 


Please find below an updated patch against the current CVS that take
into account the remarks you made.

Bye,
Aurelien


Index: op.c
===================================================================
RCS file: /sources/qemu/qemu/target-mips/op.c,v
retrieving revision 1.51
diff -u -d -p -r1.51 op.c
--- op.c        13 May 2007 14:07:26 -0000      1.51
+++ op.c        13 May 2007 16:44:43 -0000
@@ -1262,7 +1262,7 @@ void op_mtc0_entrylo0 (void)
 {
     /* Large physaddr not implemented */
     /* 1k pages not implemented */
-    env->CP0_EntryLo0 = (int32_t)T0 & 0x3FFFFFFF;
+    env->CP0_EntryLo0 = T0 & 0x3FFFFFFF;
     RETURN();
 }
 
@@ -1270,7 +1270,7 @@ void op_mtc0_entrylo1 (void)
 {
     /* Large physaddr not implemented */
     /* 1k pages not implemented */
-    env->CP0_EntryLo1 = (int32_t)T0 & 0x3FFFFFFF;
+    env->CP0_EntryLo1 = T0 & 0x3FFFFFFF;
     RETURN();
 }
 
@@ -1340,9 +1340,9 @@ void op_mtc0_status (void)
     uint32_t val, old;
     uint32_t mask = env->Status_rw_bitmask;
 
-    /* No reverse endianness, no MDMX/DSP, no 64bit ops,
-       no 64bit addressing implemented. */
-    val = (int32_t)T0 & mask;
+    /* No reverse endianness, no MDMX/DSP, no 64bit ops
+       implemented. */
+    val = T0 & mask;
     old = env->CP0_Status;
     if (!(val & (1 << CP0St_EXL)) &&
         !(val & (1 << CP0St_ERL)) &&
@@ -1397,7 +1397,7 @@ void op_mtc0_cause (void)
 
 void op_mtc0_epc (void)
 {
-    env->CP0_EPC = (int32_t)T0;
+    env->CP0_EPC = T0;
     RETURN();
 }
 
@@ -1426,7 +1426,7 @@ void op_mtc0_watchlo0 (void)
 {
     /* Watch exceptions for instructions, data loads, data stores
        not implemented. */
-    env->CP0_WatchLo = (int32_t)(T0 & ~0x7);
+    env->CP0_WatchLo = (T0 & ~0x7);
     RETURN();
 }
 
@@ -1455,7 +1455,7 @@ void op_mtc0_debug (void)
 
 void op_mtc0_depc (void)
 {
-    env->CP0_DEPC = (int32_t)T0;
+    env->CP0_DEPC = T0;
     RETURN();
 }
 
@@ -1491,7 +1491,7 @@ void op_mtc0_datahi (void)
 
 void op_mtc0_errorepc (void)
 {
-    env->CP0_ErrorEPC = (int32_t)T0;
+    env->CP0_ErrorEPC = T0;
     RETURN();
 }
 
@@ -1502,6 +1502,12 @@ void op_mtc0_desave (void)
 }
 
 #ifdef TARGET_MIPS64
+void op_mtc0_xcontext (void)
+{
+    env->CP0_XContext = (env->CP0_XContext & 0x1ffffffff) | (T0 & 
~0x1ffffffff);
+    RETURN();
+}
+
 void op_dmfc0_entrylo0 (void)
 {
     T0 = env->CP0_EntryLo0;
@@ -1567,60 +1573,6 @@ void op_dmfc0_errorepc (void)
     T0 = env->CP0_ErrorEPC;
     RETURN();
 }
-
-void op_dmtc0_entrylo0 (void)
-{
-    /* Large physaddr not implemented */
-    /* 1k pages not implemented */
-    env->CP0_EntryLo0 = T0 & 0x3FFFFFFF;
-    RETURN();
-}
-
-void op_dmtc0_entrylo1 (void)
-{
-    /* Large physaddr not implemented */
-    /* 1k pages not implemented */
-    env->CP0_EntryLo1 = T0 & 0x3FFFFFFF;
-    RETURN();
-}
-
-void op_dmtc0_context (void)
-{
-    env->CP0_Context = (env->CP0_Context & 0x007FFFFF) | (T0 & ~0x007FFFFF);
-    RETURN();
-}
-
-void op_dmtc0_epc (void)
-{
-    env->CP0_EPC = T0;
-    RETURN();
-}
-
-void op_dmtc0_watchlo0 (void)
-{
-    /* Watch exceptions for instructions, data loads, data stores
-       not implemented. */
-    env->CP0_WatchLo = T0 & ~0x7;
-    RETURN();
-}
-
-void op_dmtc0_xcontext (void)
-{
-    env->CP0_XContext = (env->CP0_XContext & 0xffffffff) | (T0 & ~0xffffffff);
-    RETURN();
-}
-
-void op_dmtc0_depc (void)
-{
-    env->CP0_DEPC = T0;
-    RETURN();
-}
-
-void op_dmtc0_errorepc (void)
-{
-    env->CP0_ErrorEPC = T0;
-    RETURN();
-}
 #endif /* TARGET_MIPS64 */
 
 /* CP1 functions */
Index: translate.c
===================================================================
RCS file: /sources/qemu/qemu/target-mips/translate.c,v
retrieving revision 1.77
diff -u -d -p -r1.77 translate.c
--- translate.c 13 May 2007 14:42:18 -0000      1.77
+++ translate.c 13 May 2007 16:44:44 -0000
@@ -2790,7 +2790,7 @@ static void gen_mtc0 (DisasContext *ctx,
         switch (sel) {
         case 0:
 #ifdef TARGET_MIPS64
-            /* Nothing writable in lower 32 bits */
+            gen_op_mtc0_xcontext();
             rn = "XContext";
             break;
 #endif
@@ -3583,15 +3583,15 @@ static void gen_dmtc0 (DisasContext *ctx
             rn = "Index";
             break;
         case 1:
-//            gen_op_dmtc0_mvpcontrol(); /* MT ASE */
+//            gen_op_mtc0_mvpcontrol(); /* MT ASE */
             rn = "MVPControl";
 //            break;
         case 2:
-//            gen_op_dmtc0_mvpconf0(); /* MT ASE */
+//            gen_op_mtc0_mvpconf0(); /* MT ASE */
             rn = "MVPConf0";
 //            break;
         case 3:
-//            gen_op_dmtc0_mvpconf1(); /* MT ASE */
+//            gen_op_mtc0_mvpconf1(); /* MT ASE */
             rn = "MVPConf1";
 //            break;
         default:
@@ -3605,31 +3605,31 @@ static void gen_dmtc0 (DisasContext *ctx
             rn = "Random";
             break;
         case 1:
-//            gen_op_dmtc0_vpecontrol(); /* MT ASE */
+//            gen_op_mtc0_vpecontrol(); /* MT ASE */
             rn = "VPEControl";
 //            break;
         case 2:
-//            gen_op_dmtc0_vpeconf0(); /* MT ASE */
+//            gen_op_mtc0_vpeconf0(); /* MT ASE */
             rn = "VPEConf0";
 //            break;
         case 3:
-//            gen_op_dmtc0_vpeconf1(); /* MT ASE */
+//            gen_op_mtc0_vpeconf1(); /* MT ASE */
             rn = "VPEConf1";
 //            break;
         case 4:
-//            gen_op_dmtc0_YQMask(); /* MT ASE */
+//            gen_op_mtc0_YQMask(); /* MT ASE */
             rn = "YQMask";
 //            break;
         case 5:
-//            gen_op_dmtc0_vpeschedule(); /* MT ASE */
+//            gen_op_mtc0_vpeschedule(); /* MT ASE */
             rn = "VPESchedule";
 //            break;
         case 6:
-//            gen_op_dmtc0_vpeschefback(); /* MT ASE */
+//            gen_op_mtc0_vpeschefback(); /* MT ASE */
             rn = "VPEScheFBack";
 //            break;
         case 7:
-//            gen_op_dmtc0_vpeopt(); /* MT ASE */
+//            gen_op_mtc0_vpeopt(); /* MT ASE */
             rn = "VPEOpt";
 //            break;
         default:
@@ -3639,35 +3639,35 @@ static void gen_dmtc0 (DisasContext *ctx
     case 2:
         switch (sel) {
         case 0:
-            gen_op_dmtc0_entrylo0();
+            gen_op_mtc0_entrylo0();
             rn = "EntryLo0";
             break;
         case 1:
-//            gen_op_dmtc0_tcstatus(); /* MT ASE */
+//            gen_op_mtc0_tcstatus(); /* MT ASE */
             rn = "TCStatus";
 //            break;
         case 2:
-//            gen_op_dmtc0_tcbind(); /* MT ASE */
+//            gen_op_mtc0_tcbind(); /* MT ASE */
             rn = "TCBind";
 //            break;
         case 3:
-//            gen_op_dmtc0_tcrestart(); /* MT ASE */
+//            gen_op_mtc0_tcrestart(); /* MT ASE */
             rn = "TCRestart";
 //            break;
         case 4:
-//            gen_op_dmtc0_tchalt(); /* MT ASE */
+//            gen_op_mtc0_tchalt(); /* MT ASE */
             rn = "TCHalt";
 //            break;
         case 5:
-//            gen_op_dmtc0_tccontext(); /* MT ASE */
+//            gen_op_mtc0_tccontext(); /* MT ASE */
             rn = "TCContext";
 //            break;
         case 6:
-//            gen_op_dmtc0_tcschedule(); /* MT ASE */
+//            gen_op_mtc0_tcschedule(); /* MT ASE */
             rn = "TCSchedule";
 //            break;
         case 7:
-//            gen_op_dmtc0_tcschefback(); /* MT ASE */
+//            gen_op_mtc0_tcschefback(); /* MT ASE */
             rn = "TCScheFBack";
 //            break;
         default:
@@ -3677,7 +3677,7 @@ static void gen_dmtc0 (DisasContext *ctx
     case 3:
         switch (sel) {
         case 0:
-            gen_op_dmtc0_entrylo1();
+            gen_op_mtc0_entrylo1();
             rn = "EntryLo1";
             break;
         default:
@@ -3687,11 +3687,11 @@ static void gen_dmtc0 (DisasContext *ctx
     case 4:
         switch (sel) {
         case 0:
-            gen_op_dmtc0_context();
+            gen_op_mtc0_context();
             rn = "Context";
             break;
         case 1:
-//           gen_op_dmtc0_contextconfig(); /* SmartMIPS ASE */
+//           gen_op_mtc0_contextconfig(); /* SmartMIPS ASE */
             rn = "ContextConfig";
 //           break;
         default:
@@ -3719,23 +3719,23 @@ static void gen_dmtc0 (DisasContext *ctx
             rn = "Wired";
             break;
         case 1:
-//            gen_op_dmtc0_srsconf0(); /* shadow registers */
+//            gen_op_mtc0_srsconf0(); /* shadow registers */
             rn = "SRSConf0";
 //            break;
         case 2:
-//            gen_op_dmtc0_srsconf1(); /* shadow registers */
+//            gen_op_mtc0_srsconf1(); /* shadow registers */
             rn = "SRSConf1";
 //            break;
         case 3:
-//            gen_op_dmtc0_srsconf2(); /* shadow registers */
+//            gen_op_mtc0_srsconf2(); /* shadow registers */
             rn = "SRSConf2";
 //            break;
         case 4:
-//            gen_op_dmtc0_srsconf3(); /* shadow registers */
+//            gen_op_mtc0_srsconf3(); /* shadow registers */
             rn = "SRSConf3";
 //            break;
         case 5:
-//            gen_op_dmtc0_srsconf4(); /* shadow registers */
+//            gen_op_mtc0_srsconf4(); /* shadow registers */
             rn = "SRSConf4";
 //            break;
         default:
@@ -3831,7 +3831,7 @@ static void gen_dmtc0 (DisasContext *ctx
     case 14:
         switch (sel) {
         case 0:
-            gen_op_dmtc0_epc();
+            gen_op_mtc0_epc();
             rn = "EPC";
             break;
         default:
@@ -3893,35 +3893,35 @@ static void gen_dmtc0 (DisasContext *ctx
     case 18:
         switch (sel) {
         case 0:
-            gen_op_dmtc0_watchlo0();
+            gen_op_mtc0_watchlo0();
             rn = "WatchLo";
             break;
         case 1:
-//            gen_op_dmtc0_watchlo1();
+//            gen_op_mtc0_watchlo1();
             rn = "WatchLo1";
 //            break;
         case 2:
-//            gen_op_dmtc0_watchlo2();
+//            gen_op_mtc0_watchlo2();
             rn = "WatchLo2";
 //            break;
         case 3:
-//            gen_op_dmtc0_watchlo3();
+//            gen_op_mtc0_watchlo3();
             rn = "WatchLo3";
 //            break;
         case 4:
-//            gen_op_dmtc0_watchlo4();
+//            gen_op_mtc0_watchlo4();
             rn = "WatchLo4";
 //            break;
         case 5:
-//            gen_op_dmtc0_watchlo5();
+//            gen_op_mtc0_watchlo5();
             rn = "WatchLo5";
 //            break;
         case 6:
-//            gen_op_dmtc0_watchlo6();
+//            gen_op_mtc0_watchlo6();
             rn = "WatchLo6";
 //            break;
         case 7:
-//            gen_op_dmtc0_watchlo7();
+//            gen_op_mtc0_watchlo7();
             rn = "WatchLo7";
 //            break;
         default:
@@ -3935,31 +3935,31 @@ static void gen_dmtc0 (DisasContext *ctx
             rn = "WatchHi";
             break;
         case 1:
-//            gen_op_dmtc0_watchhi1();
+//            gen_op_mtc0_watchhi1();
             rn = "WatchHi1";
 //            break;
         case 2:
-//            gen_op_dmtc0_watchhi2();
+//            gen_op_mtc0_watchhi2();
             rn = "WatchHi2";
 //            break;
         case 3:
-//            gen_op_dmtc0_watchhi3();
+//            gen_op_mtc0_watchhi3();
             rn = "WatchHi3";
 //            break;
         case 4:
-//            gen_op_dmtc0_watchhi4();
+//            gen_op_mtc0_watchhi4();
             rn = "WatchHi4";
 //            break;
         case 5:
-//            gen_op_dmtc0_watchhi5();
+//            gen_op_mtc0_watchhi5();
             rn = "WatchHi5";
 //            break;
         case 6:
-//            gen_op_dmtc0_watchhi6();
+//            gen_op_mtc0_watchhi6();
             rn = "WatchHi6";
 //            break;
         case 7:
-//            gen_op_dmtc0_watchhi7();
+//            gen_op_mtc0_watchhi7();
             rn = "WatchHi7";
 //            break;
         default:
@@ -3970,7 +3970,7 @@ static void gen_dmtc0 (DisasContext *ctx
         switch (sel) {
         case 0:
 #ifdef TARGET_MIPS64
-            gen_op_dmtc0_xcontext();
+            gen_op_mtc0_xcontext();
             rn = "XContext";
             break;
 #endif
@@ -4000,19 +4000,19 @@ static void gen_dmtc0 (DisasContext *ctx
             rn = "Debug";
             break;
         case 1:
-//            gen_op_dmtc0_tracecontrol(); /* PDtrace support */
+//            gen_op_mtc0_tracecontrol(); /* PDtrace support */
             rn = "TraceControl";
 //            break;
         case 2:
-//            gen_op_dmtc0_tracecontrol2(); /* PDtrace support */
+//            gen_op_mtc0_tracecontrol2(); /* PDtrace support */
             rn = "TraceControl2";
 //            break;
         case 3:
-//            gen_op_dmtc0_usertracedata(); /* PDtrace support */
+//            gen_op_mtc0_usertracedata(); /* PDtrace support */
             rn = "UserTraceData";
 //            break;
         case 4:
-//            gen_op_dmtc0_debug(); /* PDtrace support */
+//            gen_op_mtc0_debug(); /* PDtrace support */
             rn = "TraceBPC";
 //            break;
         default:
@@ -4024,7 +4024,7 @@ static void gen_dmtc0 (DisasContext *ctx
     case 24:
         switch (sel) {
         case 0:
-            gen_op_dmtc0_depc(); /* EJTAG support */
+            gen_op_mtc0_depc(); /* EJTAG support */
             rn = "DEPC";
             break;
         default:
@@ -4038,31 +4038,31 @@ static void gen_dmtc0 (DisasContext *ctx
             rn = "Performance0";
             break;
         case 1:
-//            gen_op_dmtc0_performance1();
+//            gen_op_mtc0_performance1();
             rn = "Performance1";
 //            break;
         case 2:
-//            gen_op_dmtc0_performance2();
+//            gen_op_mtc0_performance2();
             rn = "Performance2";
 //            break;
         case 3:
-//            gen_op_dmtc0_performance3();
+//            gen_op_mtc0_performance3();
             rn = "Performance3";
 //            break;
         case 4:
-//            gen_op_dmtc0_performance4();
+//            gen_op_mtc0_performance4();
             rn = "Performance4";
 //            break;
         case 5:
-//            gen_op_dmtc0_performance5();
+//            gen_op_mtc0_performance5();
             rn = "Performance5";
 //            break;
         case 6:
-//            gen_op_dmtc0_performance6();
+//            gen_op_mtc0_performance6();
             rn = "Performance6";
 //            break;
         case 7:
-//            gen_op_dmtc0_performance7();
+//            gen_op_mtc0_performance7();
             rn = "Performance7";
 //            break;
         default:
@@ -4127,7 +4127,7 @@ static void gen_dmtc0 (DisasContext *ctx
     case 30:
         switch (sel) {
         case 0:
-            gen_op_dmtc0_errorepc();
+            gen_op_mtc0_errorepc();
             rn = "ErrorEPC";
             break;
         default:


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




reply via email to

[Prev in Thread] Current Thread [Next in Thread]