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

2007-05-13 Thread Thiemo Seufer
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.)

 - 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 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.


Thiemo




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

2007-05-13 Thread Aurelien Jarno
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.
 

Ok.

Bye,
Aurelien

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




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

2007-05-13 Thread Aurelien Jarno
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.c13 May 2007 14:07:26 -  1.51
+++ op.c13 May 2007 16:44:43 -
@@ -1262,7 +1262,7 @@ void op_mtc0_entrylo0 (void)
 {
 /* Large physaddr not implemented */
 /* 1k pages not implemented */
-env-CP0_EntryLo0 = (int32_t)T0  0x3FFF;
+env-CP0_EntryLo0 = T0  0x3FFF;
 RETURN();
 }
 
@@ -1270,7 +1270,7 @@ void op_mtc0_entrylo1 (void)
 {
 /* Large physaddr not implemented */
 /* 1k pages not implemented */
-env-CP0_EntryLo1 = (int32_t)T0  0x3FFF;
+env-CP0_EntryLo1 = T0  0x3FFF;
 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  0x1) | (T0  
~0x1);
+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  0x3FFF;
-RETURN();
-}
-
-void op_dmtc0_entrylo1 (void)
-{
-/* Large physaddr not implemented */
-/* 1k pages not implemented */
-env-CP0_EntryLo1 = T0  0x3FFF;
-RETURN();
-}
-
-void op_dmtc0_context (void)
-{
-env-CP0_Context = (env-CP0_Context  0x007F) | (T0  ~0x007F);
-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  0x) | (T0  ~0x);
-RETURN();
-}
-
-void op_dmtc0_depc (void)
-{
-env-CP0_DEPC = T0;
-RETURN();
-}
-
-void op_dmtc0_errorepc (void)

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

2007-05-12 Thread Aurelien Jarno

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.
- The mtc0 instruction should do the same as the dmtc0 instruction for 
  64-bit coprocessor registers instead of copying only the low 32 bits.
- The XContest register does not exists on MIPS32 CPU.

Note that this patch does not fix the mfc0 and dmtc0 instructions 
for the EntryHi coprocessor 0 register, as it would break the MIPS64
target. I will send the fix in a later patch.

Bye,
Aurelien


--- target-mips/cpu.h   7 May 2007 13:55:33 -   1.31
+++ target-mips/cpu.h   12 May 2007 00:40:00 -
@@ -212,7 +212,9 @@ struct CPUMIPSState {
 target_ulong CP0_LLAddr;
 target_ulong CP0_WatchLo;
 int32_t CP0_WatchHi;
+#ifdef TARGET_MIPS64
 target_ulong CP0_XContext;
+#endif
 int32_t CP0_Framemask;
 int32_t CP0_Debug;
 #define CPDB_DBD   31
--- target-mips/op.c11 May 2007 17:08:26 -  1.49
+++ target-mips/op.c12 May 2007 00:40:01 -
@@ -1006,7 +1006,7 @@ void op_jnz_T2 (void)
 /* CP0 functions */
 void op_mfc0_index (void)
 {
-T0 = env-CP0_Index;
+T0 = (int32_t)env-CP0_Index;
 RETURN();
 }
 
@@ -1036,25 +1036,25 @@ void op_mfc0_context (void)
 
 void op_mfc0_pagemask (void)
 {
-T0 = env-CP0_PageMask;
+T0 = (int32_t)env-CP0_PageMask;
 RETURN();
 }
 
 void op_mfc0_pagegrain (void)
 {
-T0 = env-CP0_PageGrain;
+T0 = (int32_t)env-CP0_PageGrain;
 RETURN();
 }
 
 void op_mfc0_wired (void)
 {
-T0 = env-CP0_Wired;
+T0 = (int32_t)env-CP0_Wired;
 RETURN();
 }
 
 void op_mfc0_hwrena (void)
 {
-T0 = env-CP0_HWREna;
+T0 = (int32_t)env-CP0_HWREna;
 RETURN();
 }
 
@@ -1078,37 +1078,37 @@ void op_mfc0_entryhi (void)
 
 void op_mfc0_compare (void)
 {
-T0 = env-CP0_Compare;
+T0 = (int32_t)env-CP0_Compare;
 RETURN();
 }
 
 void op_mfc0_status (void)
 {
-T0 = env-CP0_Status;
+T0 = (int32_t)env-CP0_Status;
 RETURN();
 }
 
 void op_mfc0_intctl (void)
 {
-T0 = env-CP0_IntCtl;
+T0 = (int32_t)env-CP0_IntCtl;
 RETURN();
 }
 
 void op_mfc0_srsctl (void)
 {
-T0 = env-CP0_SRSCtl;
+T0 = (int32_t)env-CP0_SRSCtl;
 RETURN();
 }
 
 void op_mfc0_srsmap (void)
 {
-T0 = env-CP0_SRSMap;
+T0 = (int32_t)env-CP0_SRSMap;
 RETURN();
 }
 
 void op_mfc0_cause (void)
 {
-T0 = env-CP0_Cause;
+T0 = (int32_t)env-CP0_Cause;
 RETURN();
 }
 
@@ -1120,55 +1120,55 @@ void op_mfc0_epc (void)
 
 void op_mfc0_prid (void)
 {
-T0 = env-CP0_PRid;
+T0 = (int32_t)env-CP0_PRid;
 RETURN();
 }
 
 void op_mfc0_ebase (void)
 {
-T0 = env-CP0_EBase;
+T0 = (int32_t)env-CP0_EBase;
 RETURN();
 }
 
 void op_mfc0_config0 (void)
 {
-T0 = env-CP0_Config0;
+T0 = (int32_t)env-CP0_Config0;
 RETURN();
 }
 
 void op_mfc0_config1 (void)
 {
-T0 = env-CP0_Config1;
+T0 = (int32_t)env-CP0_Config1;
 RETURN();
 }
 
 void op_mfc0_config2 (void)
 {
-T0 = env-CP0_Config2;
+T0 = (int32_t)env-CP0_Config2;
 RETURN();
 }
 
 void op_mfc0_config3 (void)
 {
-T0 = env-CP0_Config3;
+T0 = (int32_t)env-CP0_Config3;
 RETURN();
 }
 
 void op_mfc0_config6 (void)
 {
-T0 = env-CP0_Config6;
+T0 = (int32_t)env-CP0_Config6;
 RETURN();
 }
 
 void op_mfc0_config7 (void)
 {
-T0 = env-CP0_Config7;
+T0 = (int32_t)env-CP0_Config7;
 RETURN();
 }
 
 void op_mfc0_lladdr (void)
 {
-T0 = (int32_t)env-CP0_LLAddr  4;
+T0 = (int32_t)(env-CP0_LLAddr  4);
 RETURN();
 }
 
@@ -1180,25 +1180,19 @@ void op_mfc0_watchlo0 (void)
 
 void op_mfc0_watchhi0 (void)
 {
-T0 = env-CP0_WatchHi;
-RETURN();
-}
-
-void op_mfc0_xcontext (void)
-{
-T0 = (int32_t)env-CP0_XContext;
+T0 = (int32_t)env-CP0_WatchHi;
 RETURN();
 }
 
 void op_mfc0_framemask (void)
 {
-T0 = env-CP0_Framemask;
+T0 = (int32_t)env-CP0_Framemask;
 RETURN();
 }
 
 void op_mfc0_debug (void)
 {
-T0 = env-CP0_Debug;
+T0 = (int32_t)env-CP0_Debug;
 if (env-hflags  MIPS_HFLAG_DM)
 T0 |= 1  CP0DB_DM;
 RETURN();
@@ -1212,31 +1206,31 @@ void op_mfc0_depc (void)
 
 void op_mfc0_performance0 (void)
 {
-T0 = env-CP0_Performance0;
+T0 = (int32_t)env-CP0_Performance0;
 RETURN();
 }
 
 void op_mfc0_taglo (void)
 {
-T0 = env-CP0_TagLo;
+T0 = (int32_t)env-CP0_TagLo;
 RETURN();
 }
 
 void op_mfc0_datalo (void)
 {
-T0 = env-CP0_DataLo;
+T0 = (int32_t)env-CP0_DataLo;
 RETURN();
 }
 
 void op_mfc0_taghi (void)
 {
-T0 = env-CP0_TagHi;
+T0 = (int32_t)env-CP0_TagHi;
 RETURN();
 }
 
 void op_mfc0_datahi (void)
 {
-T0 = env-CP0_DataHi;
+T0 = (int32_t)env-CP0_DataHi;
 RETURN();
 }
 
@@ -1248,7 +1242,7 @@ void op_mfc0_errorepc (void)
 
 void op_mfc0_desave (void)
 {
-T0 = env-CP0_DESAVE;
+T0 = (int32_t)env-CP0_DESAVE;
 RETURN();
 }
 
@@ -1262,7 +1256,7 @@ void op_mtc0_entrylo0 (void)
 {
 /* Large