Re: [Qemu-devel] [PATCH][MIPS] Fix [ls][wd][lr] instructions

2007-10-06 Thread Aurelien Jarno
On Wed, Sep 26, 2007 at 11:23:30AM +0200, Aurelien Jarno wrote:
 Hi,
 
 As written in the MIPS TODO file, the lwl, lwr, ldl, ldr, swl, swr,
 sdl and sdr instructions are not correctly implemented. In case of 
 exception the BadVAddr register gets the aligned address instead of the
 unaligned original address.
 
 In addition to that, the store instructions are generating the wrong
 exception, AdEl instead of AdEs, because the current implementation 
 first do a load.
 
 The patch below fixes that by accessing the bytes one by one, starting
 by the unaligned original address.
 

The patch doesn't apply anymore to the current CVS. Please find below an
updated version.

Index: Makefile.target
===
RCS file: /sources/qemu/qemu/Makefile.target,v
retrieving revision 1.204
diff -u -d -p -r1.204 Makefile.target
--- Makefile.target 4 Oct 2007 21:53:54 -   1.204
+++ Makefile.target 6 Oct 2007 10:52:41 -
@@ -641,7 +641,7 @@ endif
 ifeq ($(TARGET_BASE_ARCH), mips)
 helper.o: cpu.h exec-all.h
 op.o: op_template.c fop_template.c op_mem.c exec.h cpu.h
-op_helper.o: op_helper_mem.c exec.h softmmu_template.h cpu.h
+op_helper.o: exec.h softmmu_template.h cpu.h
 translate.o: translate_init.c exec-all.h disas.h
 endif
 
Index: target-mips/exec.h
===
RCS file: /sources/qemu/qemu/target-mips/exec.h,v
retrieving revision 1.36
diff -u -d -p -r1.36 exec.h
--- target-mips/exec.h  30 Sep 2007 01:58:33 -  1.36
+++ target-mips/exec.h  6 Oct 2007 10:52:42 -
@@ -100,36 +100,6 @@ void fpu_dump_state(CPUState *env, FILE 
 int (*fpu_fprintf)(FILE *f, const char *fmt, ...),
 int flags);
 void dump_sc (void);
-void do_lwl_raw (uint32_t);
-void do_lwr_raw (uint32_t);
-uint32_t do_swl_raw (uint32_t);
-uint32_t do_swr_raw (uint32_t);
-#if defined(TARGET_MIPSN32) || defined(TARGET_MIPS64)
-void do_ldl_raw (uint64_t);
-void do_ldr_raw (uint64_t);
-uint64_t do_sdl_raw (uint64_t);
-uint64_t do_sdr_raw (uint64_t);
-#endif
-#if !defined(CONFIG_USER_ONLY)
-void do_lwl_user (uint32_t);
-void do_lwl_kernel (uint32_t);
-void do_lwr_user (uint32_t);
-void do_lwr_kernel (uint32_t);
-uint32_t do_swl_user (uint32_t);
-uint32_t do_swl_kernel (uint32_t);
-uint32_t do_swr_user (uint32_t);
-uint32_t do_swr_kernel (uint32_t);
-#if defined(TARGET_MIPSN32) || defined(TARGET_MIPS64)
-void do_ldl_user (uint64_t);
-void do_ldl_kernel (uint64_t);
-void do_ldr_user (uint64_t);
-void do_ldr_kernel (uint64_t);
-uint64_t do_sdl_user (uint64_t);
-uint64_t do_sdl_kernel (uint64_t);
-uint64_t do_sdr_user (uint64_t);
-uint64_t do_sdr_kernel (uint64_t);
-#endif
-#endif
 void do_pmon (int function);
 
 void dump_sc (void);
Index: target-mips/op_helper.c
===
RCS file: /sources/qemu/qemu/target-mips/op_helper.c,v
retrieving revision 1.63
diff -u -d -p -r1.63 op_helper.c
--- target-mips/op_helper.c 30 Sep 2007 01:58:33 -  1.63
+++ target-mips/op_helper.c 6 Oct 2007 10:52:42 -
@@ -62,18 +62,6 @@ void do_raise_exception_direct (uint32_t
 do_raise_exception_direct_err (exception, 0);
 }
 
-#define MEMSUFFIX _raw
-#include op_helper_mem.c
-#undef MEMSUFFIX
-#if !defined(CONFIG_USER_ONLY)
-#define MEMSUFFIX _user
-#include op_helper_mem.c
-#undef MEMSUFFIX
-#define MEMSUFFIX _kernel
-#include op_helper_mem.c
-#undef MEMSUFFIX
-#endif
-
 #if defined(TARGET_MIPSN32) || defined(TARGET_MIPS64)
 #if TARGET_LONG_BITS  HOST_LONG_BITS
 /* Those might call libgcc functions.  */
Index: target-mips/op_helper_mem.c
===
RCS file: target-mips/op_helper_mem.c
diff -N target-mips/op_helper_mem.c
--- target-mips/op_helper_mem.c 30 Sep 2007 01:58:33 -  1.9
+++ /dev/null   1 Jan 1970 00:00:00 -
@@ -1,301 +0,0 @@
-#undef DEBUG_OP
-
-#ifdef TARGET_WORDS_BIGENDIAN
-#define GET_LMASK(v) ((v)  3)
-#else
-#define GET_LMASK(v) (((v)  3) ^ 3)
-#endif
-
-void glue(do_lwl, MEMSUFFIX) (uint32_t tmp)
-{
-#if defined (DEBUG_OP)
-target_ulong sav = T0;
-#endif
-
-switch (GET_LMASK(T0)) {
-case 0:
-T0 = (int32_t)tmp;
-break;
-case 1:
-T0 = (int32_t)((tmp  8) | (T1  0x00FF));
-break;
-case 2:
-T0 = (int32_t)((tmp  16) | (T1  0x));
-break;
-case 3:
-T0 = (int32_t)((tmp  24) | (T1  0x00FF));
-break;
-}
-#if defined (DEBUG_OP)
-if (logfile) {
-fprintf(logfile, %s:  TARGET_FMT_lx  - %08x  TARGET_FMT_lx  =  
TARGET_FMT_lx \n,
-__func__, sav, tmp, T1, T0);
-}
-#endif
-RETURN();
-}
-
-void glue(do_lwr, MEMSUFFIX) (uint32_t tmp)
-{
-#if defined (DEBUG_OP)
-target_ulong sav = T0;
-#endif
-
-switch (GET_LMASK(T0)) {
-case 0:
-T0 = (int32_t)((tmp  24) | (T1  0xFF00));
-break;
-case 1:
-T0 = 

Re: [Qemu-devel] [PATCH][MIPS] Fix [ls][wd][lr] instructions

2007-09-26 Thread Fabrice Bellard

Aurelien Jarno wrote:

Hi,

As written in the MIPS TODO file, the lwl, lwr, ldl, ldr, swl, swr,
sdl and sdr instructions are not correctly implemented. In case of 
exception the BadVAddr register gets the aligned address instead of the

unaligned original address.

In addition to that, the store instructions are generating the wrong
exception, AdEl instead of AdEs, because the current implementation 
first do a load.


The patch below fixes that by accessing the bytes one by one, starting
by the unaligned original address.

Bye,
Aurelien


[...]

BTW, you could simplify a lot the implementation :

switch (GET_LMASK(T0)) {
case 0:
T0 = (int32_t)tmp;
break;
case 1:
T0 = (int32_t)((tmp  8) | (T1  0x00FF));
break;
case 2:
T0 = (int32_t)((tmp  16) | (T1  0x));
break;
case 3:
T0 = (int32_t)((tmp  24) | (T1  0x00FF));
break;
}

-

v = GET_LMASK(T0);
if (v == 0) {
T0 = tmp;
} else {
TO = (int32_t)((tmp  (8 * v)) | (T1  ((1  (v * 8)) - 1)));
}

Fabrice.




Re: [Qemu-devel] [PATCH][MIPS] Fix [ls][wd][lr] instructions

2007-09-26 Thread Fabrice Bellard

Aurelien Jarno wrote:

Hi,

As written in the MIPS TODO file, the lwl, lwr, ldl, ldr, swl, swr,
sdl and sdr instructions are not correctly implemented. In case of 
exception the BadVAddr register gets the aligned address instead of the

unaligned original address.

In addition to that, the store instructions are generating the wrong
exception, AdEl instead of AdEs, because the current implementation 
first do a load.


The patch below fixes that by accessing the bytes one by one, starting
by the unaligned original address.

 [...]

It would be a lot more efficient to add specific code in the MIPS 
exception handling.


Fabrice.




Re: [Qemu-devel] [PATCH][MIPS] Fix [ls][wd][lr] instructions

2007-09-26 Thread Aurelien Jarno
Fabrice Bellard a écrit :
 Aurelien Jarno wrote:
 Hi,

 As written in the MIPS TODO file, the lwl, lwr, ldl, ldr, swl, swr,
 sdl and sdr instructions are not correctly implemented. In case of 
 exception the BadVAddr register gets the aligned address instead of the
 unaligned original address.

 In addition to that, the store instructions are generating the wrong
 exception, AdEl instead of AdEs, because the current implementation 
 first do a load.

 The patch below fixes that by accessing the bytes one by one, starting
 by the unaligned original address.

 Bye,
 Aurelien
 
 [...]
 
 BTW, you could simplify a lot the implementation :
 
  switch (GET_LMASK(T0)) {
  case 0:
  T0 = (int32_t)tmp;
  break;
  case 1:
  T0 = (int32_t)((tmp  8) | (T1  0x00FF));
  break;
  case 2:
  T0 = (int32_t)((tmp  16) | (T1  0x));
  break;
  case 3:
  T0 = (int32_t)((tmp  24) | (T1  0x00FF));
  break;
  }
 
 -
 
 v = GET_LMASK(T0);
 if (v == 0) {
   T0 = tmp;
 } else {
   TO = (int32_t)((tmp  (8 * v)) | (T1  ((1  (v * 8)) - 1)));
 }
 

Agreed, but this code is actually removed and not added by the patch I
posted.

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