[Qemu-devel] Pending MIPS patches

[Qemu-devel] Pending MIPS patches

[Dirk Behme]

Hi,

just fyi below my list of pending MIPS patches. Not sure if 
all patches are ready for inclusion, but maybe they are 
helpful for somebody. I try to track which patches are 
already applied and which not. Please feel free to add, 
comment, discuss etc anything.

Best regards

Dirk

Pending MIPS patches (bugs & improvements):
===========================================

1. [PATCH] Huge TLB performance improvement
http://lists.gnu.org/archive/html/qemu-devel/2006-03/msg00034.html

2. [PATCH][MIPS] add "lwu" instruction
http://lists.gnu.org/archive/html/qemu-devel/2006-04/msg00326.html

3. [PATCH] Add special MIPS multiply instructions
http://lists.gnu.org/archive/html/qemu-devel/2006-04/msg00375.html

4. [PATCH][MIPS] Enforce aligned pc
http://lists.gnu.org/archive/html/qemu-devel/2006-04/msg00484.html

5.[PATCH 5-6/8] Mips improvements

http://lists.gnu.org/archive/html/qemu-devel/2006-05/msg00314.html
http://lists.gnu.org/archive/html/qemu-devel/2006-05/msg00315.html

Note: Patch 7/8 was eaten by list.

6. [PATCH] Fix initial value for MIPS CP0 Config Register
http://lists.gnu.org/archive/html/qemu-devel/2006-05/msg00372.html

7. [PATCH] Restructure MIPS r4k code
http://lists.gnu.org/archive/html/qemu-devel/2006-05/msg00429.html

Note: Pending until additional MIPS machines are added. 
Comment from
Fabrice: I prefer to wait until there is another mips 
machine (e.g. Malta).
Currently I see no reason to move the mips code as it is 
very small.

8. [PATCH] Add mips-user signal handling
http://lists.gnu.org/archive/html/qemu-devel/2006-06/msg00319.html

9. [PATCH] Update MIPS status register with EXL and ERL bits 
at exception
http://lists.gnu.org/archive/html/qemu-devel/2006-06/msg00329.html

Re: [Qemu-devel] Pending MIPS patches

[Fabrice Bellard]

Dirk Behme wrote:
> Hi,
> 
> just fyi below my list of pending MIPS patches. Not sure if all patches 
> are ready for inclusion, but maybe they are helpful for somebody. I try 
> to track which patches are already applied and which not. Please feel 
> free to add, comment, discuss etc anything.
> 
> Best regards
> 
> Dirk
> 
> Pending MIPS patches (bugs & improvements):
> ===========================================
> 
> 1. [PATCH] Huge TLB performance improvement
> http://lists.gnu.org/archive/html/qemu-devel/2006-03/msg00034.html

Need to verify that it does not slow down x86 target.

> 2. [PATCH][MIPS] add "lwu" instruction
> http://lists.gnu.org/archive/html/qemu-devel/2006-04/msg00326.html

On which MIPS CPU is it defined ? Need to track instruction sets exactly 
to be able to select a given MIPS CPU at compile time or dynamically.

> 3. [PATCH] Add special MIPS multiply instructions
> http://lists.gnu.org/archive/html/qemu-devel/2006-04/msg00375.html

Same remark.

> 4. [PATCH][MIPS] Enforce aligned pc
> http://lists.gnu.org/archive/html/qemu-devel/2006-04/msg00484.html

Can it happen on a real MIPS ? If not, an assert should be used for example.

> 5.[PATCH 5-6/8] Mips improvements

OK. Maybe expanding only the performance critical CP0 insn would have 
sufficed ?

> http://lists.gnu.org/archive/html/qemu-devel/2006-05/msg00314.html
> http://lists.gnu.org/archive/html/qemu-devel/2006-05/msg00315.html
> 
> Note: Patch 7/8 was eaten by list.
> 
> 6. [PATCH] Fix initial value for MIPS CP0 Config Register
> http://lists.gnu.org/archive/html/qemu-devel/2006-05/msg00372.html

Already done.

> 7. [PATCH] Restructure MIPS r4k code
> http://lists.gnu.org/archive/html/qemu-devel/2006-05/msg00429.html
> Note: Pending until additional MIPS machines are added. Comment from
> Fabrice: I prefer to wait until there is another mips machine (e.g. Malta).
> Currently I see no reason to move the mips code as it is very small.
> 
> 8. [PATCH] Add mips-user signal handling
> http://lists.gnu.org/archive/html/qemu-devel/2006-06/msg00319.html

OK. Need to check the handling of "env->PC += 4" in linux-user/main.c. I 
want to be sure there is not a better solution. Adding setup_rt_frame 
would be good too.

> 9. [PATCH] Update MIPS status register with EXL and ERL bits at exception
> http://lists.gnu.org/archive/html/qemu-devel/2006-06/msg00329.html

OK but the following lines are suspicious:

          } else {
              env->CP0_ErrorEPC = env->PC;
          }
-        env->hflags = MIPS_HFLAG_ERL;
+        env->hflags |= MIPS_HFLAG_ERL;
+       env->CP0_Status &= (1 << CP0St_ERL);
          pc = 0xBFC00000;


Fabrice.

Re: [Qemu-devel] Pending MIPS patches

[Marius Groeger]

On Sun, 25 Jun 2006, Fabrice Bellard wrote:

>>  2. [PATCH][MIPS] add "lwu" instruction
>>  http://lists.gnu.org/archive/html/qemu-devel/2006-04/msg00326.html
>
> On which MIPS CPU is it defined ? Need to track instruction sets exactly to 
> be able to select a given MIPS CPU at compile time or dynamically.

Since the initial patch came from me: "lwu" is part of MIPS III.

>>  4. [PATCH][MIPS] Enforce aligned pc
>>  http://lists.gnu.org/archive/html/qemu-devel/2006-04/msg00484.html
>
> Can it happen on a real MIPS ? If not, an assert should be used for example.

Again this is one of mine. Yes, it can happen. You're free to load any 
crap into the link register GPR31. The documentation of "JR - jump 
register" says:

   If these low-order bits are not zero, an address exception will
   occur when the jump target instruction is subsequently fetched.

BTW, I'm referring to the document

   64-Bit TX System RISC TX49/H2, TX49/H3, TX49/H4 Core Architecture
   Rev 1.0

The TX49 uses an R4K core.

Regards,
Marius

-- 
Marius Groeger <mgroeger@sysgo.com>
SYSGO AG                      Embedded and Real-Time Software
Voice: +49 6136 9948 0                  FAX: +49 6136 9948 10
www.sysgo.com | www.elinos.com | www.osek.de | www.pikeos.com

Re: [Qemu-devel] Pending MIPS patches

[Dirk Behme]

Fabrice Bellard wrote:
>> 2. [PATCH][MIPS] add "lwu" instruction
>> http://lists.gnu.org/archive/html/qemu-devel/2006-04/msg00326.html
> 
> 
> On which MIPS CPU is it defined ? Need to track instruction sets exactly 
> to be able to select a given MIPS CPU at compile time or dynamically.
> 
>> 3. [PATCH] Add special MIPS multiply instructions
>> http://lists.gnu.org/archive/html/qemu-devel/2006-04/msg00375.html
> 
> 
> Same remark.

These are NEC VR54xx specific extensions to the MIPS 
instruction set.

They are used if you use GCC's -march=vr5400 option. See

www.necelam.com/docs/files/1375_V2.pdf

as well.

>> 9. [PATCH] Update MIPS status register with EXL and ERL bits at exception
>> http://lists.gnu.org/archive/html/qemu-devel/2006-06/msg00329.html
> 
> 
> OK but the following lines are suspicious:

Yes, I see. I mixed two fixes, let me explain below:

> -        env->hflags = MIPS_HFLAG_ERL;
> +        env->hflags |= MIPS_HFLAG_ERL;

This is a typo fix for a missing "or" (it should be "|=" 
instead of only "="). hflags is used as a shadow of the 
status register and there not the whole value should be set, 
only the corresponding flag. See for example correct usage 
of  MIPS_HFLAG_EXL or MIPS_HFLAG_ERL in helper.c or op.c of 
target-mips. There, they are correctly used everywhere with 
"|=" or "&= ~", except at this place.

> +       env->CP0_Status &= (1 << CP0St_ERL);

This is part of the "Update status register EXL and ERL 
flags directly, entering or leaving exception, not only hflags".

So this two added lines fix the missing "or" and set the 
flag in the status register as well, not only in hflags.

If you like you can apply the single "|=" line as a typo 
fix, and the rest as an additional fix to update status 
register as well with ERL and EXL.

Regards

Dirk

Btw: Many thanks for commenting all patches!

Re: [Qemu-devel] Pending MIPS patches

[Fabrice Bellard]

Dirk Behme wrote:
> Fabrice Bellard wrote:
> 
>>> 2. [PATCH][MIPS] add "lwu" instruction
>>> http://lists.gnu.org/archive/html/qemu-devel/2006-04/msg00326.html
>>
>>
>>
>> On which MIPS CPU is it defined ? Need to track instruction sets 
>> exactly to be able to select a given MIPS CPU at compile time or 
>> dynamically.
>>
>>> 3. [PATCH] Add special MIPS multiply instructions
>>> http://lists.gnu.org/archive/html/qemu-devel/2006-04/msg00375.html
>>
>>
>>
>> Same remark.
> 
> 
> These are NEC VR54xx specific extensions to the MIPS instruction set.
> 
> They are used if you use GCC's -march=vr5400 option. See
> 
> www.necelam.com/docs/files/1375_V2.pdf
> 
> as well.

Can you add some kind of define or dynamic processor definnition in your 
patch so that we can keep track of the exact instruction set ?

Fabrice.

MIPS instruction set configuration, was: Re: [Qemu-devel] Pending MIPS patches

[Dirk Behme]

Fabrice Bellard wrote:

>>>> 3. [PATCH] Add special MIPS multiply instructions
>>>> http://lists.gnu.org/archive/html/qemu-devel/2006-04/msg00375.html
>>>
>>> Same remark.
>>
>> These are NEC VR54xx specific extensions to the MIPS instruction set.
>>
>> They are used if you use GCC's -march=vr5400 option. See
>>
>> www.necelam.com/docs/files/1375_V2.pdf
>>
>> as well.
>  
> Can you add some kind of define or dynamic processor definnition in your 
> patch so that we can keep track of the exact instruction set ?

Yes, I will update the patch. Any ideas or proposals from 
anybody how to do this the best way? Are there already 
examples from other architectures?

I think a define to be able to completely select/unselect it 
at compile time combined with possibility for dynamic 
runtime selection would be the best?

Regards

Dirk

[Qemu-devel] Re: MIPS instruction set configuration

[Marius Groeger]

On Tue, 27 Jun 2006, Dirk Behme wrote:

> Fabrice Bellard wrote:
>
>>>>> 3. [PATCH] Add special MIPS multiply instructions
>>>>> http://lists.gnu.org/archive/html/qemu-devel/2006-04/msg00375.html
>>>> 
>>>> Same remark.
>>> 
>>> These are NEC VR54xx specific extensions to the MIPS instruction set.
>>> 
>>> They are used if you use GCC's -march=vr5400 option. See
>>> 
>>> www.necelam.com/docs/files/1375_V2.pdf
>>> 
>>> as well.
>>  Can you add some kind of define or dynamic processor definnition in your 
>> patch so that we can keep track of the exact instruction set ?
>
> Yes, I will update the patch. Any ideas or proposals from anybody how to do 
> this the best way? Are there already examples from other architectures?
>
> I think a define to be able to completely select/unselect it at compile time 
> combined with possibility for dynamic runtime selection would be the best?

I'm trying to make sense of a compile-time switch -- for the use to 
select you vr5400 based platform, I can't think of anything else than 
a new -M option (ie. a new machine definition). So the full set of 
possible insns must be present and be available depending on the 
machine (-M ...) at runtime.

Or is it possible to compile qemu for a specific "-M machine"?

Regards,
Marius

-- 
Marius Groeger <mgroeger@sysgo.com>
SYSGO AG                      Embedded and Real-Time Software
Voice: +49 6136 9948 0                  FAX: +49 6136 9948 10
www.sysgo.com | www.elinos.com | www.osek.de | www.pikeos.com

Re: [Qemu-devel] Re: MIPS instruction set configuration

[Fabrice Bellard]

Marius Groeger wrote:
> On Tue, 27 Jun 2006, Dirk Behme wrote:
> 
>> Fabrice Bellard wrote:
>>
>>>>>> 3. [PATCH] Add special MIPS multiply instructions
>>>>>> http://lists.gnu.org/archive/html/qemu-devel/2006-04/msg00375.html
>>>>>
>>>>>
>>>>> Same remark.
>>>>
>>>>
>>>> These are NEC VR54xx specific extensions to the MIPS instruction set.
>>>>
>>>> They are used if you use GCC's -march=vr5400 option. See
>>>>
>>>> www.necelam.com/docs/files/1375_V2.pdf
>>>>
>>>> as well.
>>>
>>>  Can you add some kind of define or dynamic processor definnition in 
>>> your patch so that we can keep track of the exact instruction set ?
>>
>>
>> Yes, I will update the patch. Any ideas or proposals from anybody how 
>> to do this the best way? Are there already examples from other 
>> architectures?
>>
>> I think a define to be able to completely select/unselect it at 
>> compile time combined with possibility for dynamic runtime selection 
>> would be the best?
> 
> 
> I'm trying to make sense of a compile-time switch -- for the use to 
> select you vr5400 based platform, I can't think of anything else than a 
> new -M option (ie. a new machine definition). So the full set of 
> possible insns must be present and be available depending on the machine 
> (-M ...) at runtime.
> 
> Or is it possible to compile qemu for a specific "-M machine"?

You should add a runtime selection system : see the ARM and PowerPC 
targets (I would prefer a parameter to cpu_init(). It was not done that 
way on PowerPC for legacy reasons). Each machine should be able to 
select the processor it needs (and allow the user to change it if 
needed, but it is not the main point). There is no good reason to make 
the selection at compile time because the translator can efficiently 
handle any CPU differences at runtime.

Regards,

Fabrice.

[Qemu-devel] [PATCH] MIPS instruction set configuration

[Dirk Behme]

[-- Attachment #1: Type: text/plain, Size: 2675 bytes --]

Fabrice Bellard wrote:
> You should add a runtime selection system : see the ARM and PowerPC 
> targets (I would prefer a parameter to cpu_init(). It was not done that 
> way on PowerPC for legacy reasons). Each machine should be able to 
> select the processor it needs (and allow the user to change it if 
> needed, but it is not the main point). There is no good reason to make 
> the selection at compile time because the translator can efficiently 
> handle any CPU differences at runtime.

Find in the attachment a first proposal for runtime
instruction set configuration for MIPS target. Please
comment, correct etc.

Some notes:

- As first step, only three options for R4K, NEC (partly,
see below) and FPU are used. The different MIPS ISA levels
aren't used at the moment and are there for future use.
Fabrice mentioned that he likes to split the different MIPS
intstruction set configurations clearly.

- As I understand it, MIPS III is an extension of MIPS II,
MIPS IV is an extension of MIPS III etc. Therefore I used
definitions for ISAx which include the smaller ones as well.

- With the

#ifdef MIPS_USES_xxx
#define mips_uses_xxx()  (env->features & MIPS_FEATURE_xxx)
#else
#define mips_uses_xxx()  0
#endif

logic it should be possible to enable or disable a feature
at compile time by setting the corresponding macro *or* at
runtime by adding machines with different features to
cpu_mips_set_model().

- If mips_uses_xxx()  is set to 0 at compile time, I think
gcc should optimize away the if(mips_uses_xxx()) parts in
translate.c. Therefore I think we can remove some #if
defined (MIPS_USES_xxx).

- An cpu_mips_set_model() is introduced similiar to the ARM
one. Because in mips-defs.h there was a hardcoded CPU, some
reorganization for runtime selection was necessary.

- Later, it should be possible to move more options to
runtime configuration, like endianess and the #if defined
(MIPS_CONFIGx) in cpu_mips_init(). For the moment, to keep
patch small I concentrated only on instruction set. I don't
want to mix to much instruction set configuration and
machine runtime selection for the moment.

- cpu_mips_init() moved from translate.c to helper.c and was
extended there by set_feature() and cpu_mips_set_model()
(same file location like used by ARM). Setting of FPU
options moved from cpu_mips_init() to model configuration
cpu_mips_set_model().

- My patch for NEC instruction set extension (the main
reason for the whole stuff ;) ) will be send and updated if
you like the configuration in this patch and something like
this is applied. For the moment, there's only a stub in
decode_opc() which should generate an exception.

Best regards

Dirk



[-- Attachment #2: mips_isa_config_patch.txt --]
[-- Type: text/plain, Size: 14347 bytes --]

--- ./target-mips/cpu.h_orig	2006-07-02 14:44:59.000000000 +0200
+++ ./target-mips/cpu.h	2006-07-02 17:32:41.000000000 +0200
@@ -210,6 +210,8 @@ struct CPUMIPSState {
     int bcond;                   /* Branch condition (if needed)       */
 
     int halted; /* TRUE if the CPU is in suspend state */
+    
+    uint32_t features;           /* Internal CPU feature flags */
 
     CPU_COMMON
 };
@@ -275,5 +277,58 @@ enum {
 int cpu_mips_exec(CPUMIPSState *s);
 CPUMIPSState *cpu_mips_init(void);
 uint32_t cpu_mips_get_clock (void);
+void cpu_mips_set_model(CPUMIPSState *env, uint32_t id);
+
+enum mips_features {
+    MIPS_FEATURE_ISA1 = 0x1, /* MIPS I   instruction set architecture */
+    MIPS_FEATURE_ISA2 = 0x3, /* MIPS II  instruction set architecture */
+    MIPS_FEATURE_ISA3 = 0x7, /* MIPS III instruction set architecture */
+    MIPS_FEATURE_ISA4 = 0xF, /* MIPS IV  instruction set architecture */
+    MIPS_FEATURE_R4K_EXT = 0x10, /* instruction set extension for MIPS R4K */
+    MIPS_FEATURE_NEC_EXT = 0x20, /* instruction set extension for NEC CPUs */
+    MIPS_FEATURE_FPU = 0x40,  /* floating point instruction set */
+};
+
+#ifdef MIPS_USES_ISA1
+#define mips_uses_isa1()  (env->features & MIPS_FEATURE_ISA1)
+#else
+#define mips_uses_isa1()  0
+#endif
+
+#ifdef MIPS_USES_ISA2
+#define mips_uses_isa2()  (env->features & MIPS_FEATURE_ISA2)
+#else
+#define mips_uses_isa2()  0
+#endif
+
+#ifdef MIPS_USES_ISA3
+#define mips_uses_isa3()  (env->features & MIPS_FEATURE_ISA3)
+#else
+#define mips_uses_isa3()  0
+#endif
+
+#ifdef MIPS_USES_ISA4
+#define mips_uses_isa4()  (env->features & MIPS_FEATURE_ISA4)
+#else
+#define mips_uses_isa4()  0
+#endif
+
+#ifdef MIPS_USES_R4K_EXT
+#define mips_uses_r4k_ext()  (env->features & MIPS_FEATURE_R4K_EXT)
+#else
+#define mips_uses_r4k_ext()  0
+#endif
+
+#ifdef MIPS_FEATURE_NEC_EXT
+#define mips_uses_nec_ext()  (env->features & MIPS_FEATURE_NEC_EXT)
+#else
+#define mips_uses_nec_ext()  0
+#endif
+
+#ifdef MIPS_USES_FPU
+#define mips_uses_fpu()  (env->features & MIPS_FEATURE_FPU)
+#else
+#define mips_uses_fpu()  0
+#endif
 
 #endif /* !defined (__MIPS_CPU_H__) */
--- ./target-mips/helper.c_orig	2006-07-02 15:30:49.000000000 +0200
+++ ./target-mips/helper.c	2006-07-02 17:41:01.000000000 +0200
@@ -430,3 +430,69 @@ void do_interrupt (CPUState *env)
     }
     env->exception_index = EXCP_NONE;
 }
+
+CPUMIPSState *cpu_mips_init (void)
+{
+    CPUMIPSState *env;
+
+    env = qemu_mallocz(sizeof(CPUMIPSState));
+    if (!env)
+        return NULL;
+    cpu_exec_init(env);
+    tlb_flush(env, 1);
+    /* Minimal init */
+    env->PC = 0xBFC00000;
+#if defined (MIPS_USES_R4K_TLB)
+    env->CP0_random = MIPS_TLB_NB - 1;
+#endif
+    env->CP0_Wired = 0;
+    env->CP0_Config0 = MIPS_CONFIG0;
+#if defined (MIPS_CONFIG1)
+        env->CP0_Config1 = MIPS_CONFIG1;
+#endif
+#if defined (MIPS_CONFIG2)
+        env->CP0_Config2 = MIPS_CONFIG2;
+#endif
+#if defined (MIPS_CONFIG3)
+        env->CP0_Config3 = MIPS_CONFIG3;
+#endif
+    env->CP0_Status = (1 << CP0St_CU0) | (1 << CP0St_BEV);
+    env->CP0_WatchLo = 0;
+    env->hflags = MIPS_HFLAG_ERL;
+    /* Count register increments in debug mode, EJTAG version 1 */
+    env->CP0_Debug = (1 << CP0DB_CNT) | (0x1 << CP0DB_VER);
+    env->exception_index = EXCP_NONE;
+#if defined(CONFIG_USER_ONLY)
+    env->hflags |= MIPS_HFLAG_UM;
+#endif
+    return env;
+}
+
+static inline void set_feature(CPUMIPSState *env, int feature)
+{
+    env->features |= feature;
+}
+
+void cpu_mips_set_model(CPUMIPSState *env, uint32_t id)
+{
+    env->CP0_PRid = id;
+    env->features = 0;
+    switch (id) {
+    case MIPS_R4Kc:
+        set_feature(env, MIPS_FEATURE_ISA3);
+        set_feature(env, MIPS_FEATURE_R4K_EXT);
+        set_feature(env, MIPS_FEATURE_FPU);
+        break;
+    default:
+        cpu_abort(env, "Bad CPU ID: %x\n", id);
+        break;
+    }
+
+    if(mips_uses_fpu()) {
+        env->fcr0 = MIPS_FCR0;
+#if defined (MIPS_CONFIG1)
+	env->CP0_Config1 |= (1 << CP0C1_FP);
+#endif
+    }
+}
+
--- ./target-mips/translate.c_orig	2006-07-02 17:06:18.000000000 +0200
+++ ./target-mips/translate.c	2006-07-02 17:40:17.000000000 +0200
@@ -1887,7 +1887,7 @@ static void gen_blikely(DisasContext *ct
     gen_set_label(l1);
 }
 
-static void decode_opc (DisasContext *ctx)
+static void decode_opc (CPUState *env, DisasContext *ctx)
 {
     int32_t offset;
     int rs, rt, rd, sa;
@@ -1927,7 +1927,17 @@ static void decode_opc (DisasContext *ct
             gen_arith(ctx, op1 | EXT_SPECIAL, rd, rs, rt);
             break;
         case 0x18 ... 0x1B: /* MULT / DIV */
-            gen_muldiv(ctx, op1 | EXT_SPECIAL, rs, rt);
+	    if(!sa) {
+                gen_muldiv(ctx, op1 | EXT_SPECIAL, rs, rt);
+	    } else {
+	        if(mips_uses_nec_ext()) {
+	            op1 = ctx->opcode & 0x7FF;
+	            /* tbd: call handler for special NEC instructions */
+                } else {
+	            MIPS_INVAL("NEC extension");
+                    generate_exception(ctx, EXCP_RI);
+                }
+            }
             break;
         case 0x08 ... 0x09: /* Jumps */
             gen_compute_branch(ctx, op1 | EXT_SPECIAL, rs, rd, sa);
@@ -1985,19 +1995,32 @@ static void decode_opc (DisasContext *ct
     case 0x1C:          /* Special2 opcode */
         op1 = ctx->opcode & 0x3F;
         switch (op1) {
-#if defined (MIPS_USES_R4K_EXT)
         /* Those instructions are not part of MIPS32 core */
         case 0x00 ... 0x01: /* Multiply and add/sub */
         case 0x04 ... 0x05:
-            gen_muldiv(ctx, op1 | EXT_SPECIAL2, rs, rt);
+	    if(mips_uses_r4k_ext()) {
+                gen_muldiv(ctx, op1 | EXT_SPECIAL2, rs, rt);
+            } else {
+	        MIPS_INVAL("r4k extension");
+                generate_exception(ctx, EXCP_RI);
+            }
             break;
         case 0x02:          /* MUL */
-            gen_arith(ctx, op1 | EXT_SPECIAL2, rd, rs, rt);
+	    if(mips_uses_r4k_ext()) {
+                gen_arith(ctx, op1 | EXT_SPECIAL2, rd, rs, rt);
+            } else {
+	        MIPS_INVAL("r4k extension");
+                generate_exception(ctx, EXCP_RI);
+            }
             break;
         case 0x20 ... 0x21: /* CLO / CLZ */
-            gen_cl(ctx, op1 | EXT_SPECIAL2, rd, rs);
+	    if(mips_uses_r4k_ext()) {
+                gen_cl(ctx, op1 | EXT_SPECIAL2, rd, rs);
+            } else {
+	        MIPS_INVAL("r4k extension");
+                generate_exception(ctx, EXCP_RI);
+            }
             break;
-#endif
         case 0x3F:          /* SDBBP */
             /* XXX: not clear which exception should be raised
              *      when in debug mode...
@@ -2074,43 +2097,43 @@ static void decode_opc (DisasContext *ct
     case 0x35: /* LDC1 */
     case 0x39: /* SWC1 */
     case 0x3D: /* SDC1 */
-#if defined(MIPS_USES_FPU)
-        gen_op_cp1_enabled();
-        gen_flt_ldst(ctx, op, rt, rs, imm);
-#else
-        generate_exception_err(ctx, EXCP_CpU, 1);
-#endif
+        if(mips_uses_fpu()) {
+            gen_op_cp1_enabled();
+            gen_flt_ldst(ctx, op, rt, rs, imm);
+        } else {
+            generate_exception_err(ctx, EXCP_CpU, 1);
+	}
         break;
 
     case 0x11:          /* CP1 opcode */
-#if defined(MIPS_USES_FPU)
-        gen_op_cp1_enabled();
-        op1 = ((ctx->opcode >> 21) & 0x1F);
-        switch (op1) {
-        case 0x00: /* mfc1 */
-        case 0x02: /* cfc1 */
-        case 0x04: /* mtc1 */
-        case 0x06: /* ctc1 */
-            gen_cp1(ctx, op1 | EXT_CP1, rt, rd);
-            break;
-        case 0x08: /* bc */
-            gen_compute_branch1(ctx, rt, imm << 2);
-            return;
-        case 0x10: /* 16: fmt=single fp */
-        case 0x11: /* 17: fmt=double fp */
-        case 0x14: /* 20: fmt=32bit fixed */
-        case 0x15: /* 21: fmt=64bit fixed */
-            gen_farith(ctx, op1, rt, rd, sa, ctx->opcode & 0x3f);
-            break;
-        default:
-            generate_exception_err(ctx, EXCP_RI, 1);
+        if(mips_uses_fpu()) {
+            gen_op_cp1_enabled();
+            op1 = ((ctx->opcode >> 21) & 0x1F);
+            switch (op1) {
+            case 0x00: /* mfc1 */
+            case 0x02: /* cfc1 */
+            case 0x04: /* mtc1 */
+            case 0x06: /* ctc1 */
+                gen_cp1(ctx, op1 | EXT_CP1, rt, rd);
+                break;
+            case 0x08: /* bc */
+                gen_compute_branch1(ctx, rt, imm << 2);
+                return;
+            case 0x10: /* 16: fmt=single fp */
+            case 0x11: /* 17: fmt=double fp */
+            case 0x14: /* 20: fmt=32bit fixed */
+            case 0x15: /* 21: fmt=64bit fixed */
+                gen_farith(ctx, op1, rt, rd, sa, ctx->opcode & 0x3f);
+                break;
+            default:
+                generate_exception_err(ctx, EXCP_RI, 1);
+                break;
+            }
             break;
+        } else {
+            generate_exception_err(ctx, EXCP_CpU, 1);
         }
         break;
-#else
-        generate_exception_err(ctx, EXCP_CpU, 1);
-#endif
-        break;
 
     /* COP2.  */
     case 0x32: /* LWC2 */
@@ -2262,7 +2285,7 @@ int gen_intermediate_code_internal (CPUS
             gen_opc_instr_start[lj] = 1;
         }
         ctx.opcode = ldl_code(ctx.pc);
-        decode_opc(&ctx);
+        decode_opc(env, &ctx);
         ctx.pc += 4;
 
         if (env->singlestep_enabled)
@@ -2400,44 +2423,3 @@ void cpu_dump_state (CPUState *env, FILE
     fpu_dump_state(env, f, cpu_fprintf, flags);
 #endif
 }
-
-CPUMIPSState *cpu_mips_init (void)
-{
-    CPUMIPSState *env;
-
-    env = qemu_mallocz(sizeof(CPUMIPSState));
-    if (!env)
-        return NULL;
-    cpu_exec_init(env);
-    tlb_flush(env, 1);
-    /* Minimal init */
-    env->PC = 0xBFC00000;
-#if defined (MIPS_USES_R4K_TLB)
-    env->CP0_random = MIPS_TLB_NB - 1;
-#endif
-    env->CP0_Wired = 0;
-    env->CP0_Config0 = MIPS_CONFIG0;
-#if defined (MIPS_CONFIG1)
-        env->CP0_Config1 = MIPS_CONFIG1;
-#endif
-#if defined (MIPS_CONFIG2)
-        env->CP0_Config2 = MIPS_CONFIG2;
-#endif
-#if defined (MIPS_CONFIG3)
-        env->CP0_Config3 = MIPS_CONFIG3;
-#endif
-    env->CP0_Status = (1 << CP0St_CU0) | (1 << CP0St_BEV);
-    env->CP0_WatchLo = 0;
-    env->hflags = MIPS_HFLAG_ERL;
-    /* Count register increments in debug mode, EJTAG version 1 */
-    env->CP0_Debug = (1 << CP0DB_CNT) | (0x1 << CP0DB_VER);
-    env->CP0_PRid = MIPS_CPU;
-    env->exception_index = EXCP_NONE;
-#if defined(CONFIG_USER_ONLY)
-    env->hflags |= MIPS_HFLAG_UM;
-#endif
-#ifdef MIPS_USES_FPU
-    env->fcr0 = MIPS_FCR0;	
-#endif
-    return env;
-}
--- ./target-mips/mips-defs.h_orig	2006-07-02 15:22:18.000000000 +0200
+++ ./target-mips/mips-defs.h	2006-07-02 17:01:35.000000000 +0200
@@ -9,10 +9,6 @@
 #define MIPS_R4Kc 0x00018000
 #define MIPS_R4Kp 0x00018300
 
-/* Emulate MIPS R4Kc for now */
-#define MIPS_CPU MIPS_R4Kc
-
-#if (MIPS_CPU == MIPS_R4Kc)
 /* 32 bits target */
 #define TARGET_LONG_BITS 32
 /* real pages are variable size... */
@@ -40,28 +36,13 @@
 /* 16 TLBs, 64 sets Icache, 16 bytes Icache line, 2-way Icache,
  * 64 sets Dcache, 16 bytes Dcache line, 2-way Dcache,
  * no performance counters, watch registers present, no code compression,
- * EJTAG present, FPU enable bit depending on MIPS_USES_FPU
+ * EJTAG present, FPU bit is set depending on runtime FPU selection
  */
 #define MIPS_CONFIG1                                            \
 ((15 << CP0C1_MMU) |                                            \
  (0x000 << CP0C1_IS) | (0x3 << CP0C1_IL) | (0x01 << CP0C1_IA) | \
  (0x000 << CP0C1_DS) | (0x3 << CP0C1_DL) | (0x01 << CP0C1_DA) | \
  (0 << CP0C1_PC) | (1 << CP0C1_WR) | (0 << CP0C1_CA) |          \
- (1 << CP0C1_EP) | (MIPS_USES_FPU << CP0C1_FP))
-#elif (MIPS_CPU == MIPS_R4Kp)
-/* 32 bits target */
-#define TARGET_LONG_BITS 32
-/* real pages are variable size... */
-#define TARGET_PAGE_BITS 12
-/* Uses MIPS R4Kx enhancements to MIPS32 architecture */
-#define MIPS_USES_R4K_EXT
-/* Uses MIPS R4Km FPM MMU model */
-#define MIPS_USES_R4K_FPM
-#else
-#error "MIPS CPU not defined"
-/* Remainder for other flags */
-//#define TARGET_MIPS64
-//#define MIPS_USES_FPU
-#endif
+ (1 << CP0C1_EP))
 
 #endif /* !defined (__QEMU_MIPS_DEFS_H__) */
--- ./vl.c_orig	2006-07-02 16:31:19.000000000 +0200
+++ ./vl.c	2006-07-02 16:38:57.000000000 +0200
@@ -5455,7 +5455,7 @@ void register_machines(void)
     qemu_register_machine(&core99_machine);
     qemu_register_machine(&prep_machine);
 #elif defined(TARGET_MIPS)
-    qemu_register_machine(&mips_machine);
+    qemu_register_machine(&mips_r4kc_machine);
 #elif defined(TARGET_SPARC)
 #ifdef TARGET_SPARC64
     qemu_register_machine(&sun4u_machine);
--- ./hw/mips_r4k.c_orig	2006-07-02 16:29:14.000000000 +0200
+++ ./hw/mips_r4k.c	2006-07-02 17:34:00.000000000 +0200
@@ -189,10 +189,10 @@ CPUReadMemoryFunc *io_read[] = {
     &io_readl,
 };
 
-void mips_r4k_init (int ram_size, int vga_ram_size, int boot_device,
-                    DisplayState *ds, const char **fd_filename, int snapshot,
-                    const char *kernel_filename, const char *kernel_cmdline,
-                    const char *initrd_filename)
+void mips_r4kc_init (int ram_size, int vga_ram_size, int boot_device,
+                     DisplayState *ds, const char **fd_filename, int snapshot,
+                     const char *kernel_filename, const char *kernel_cmdline,
+                     const char *initrd_filename)
 {
     char buf[1024];
     int64_t entry = 0;
@@ -203,6 +203,7 @@ void mips_r4k_init (int ram_size, int vg
     long kernel_size;
 
     env = cpu_init();
+    cpu_mips_set_model(env, MIPS_R4Kc);
     register_savevm("cpu", 0, 3, cpu_save, cpu_load, env);
 
     /* allocate RAM */
@@ -284,8 +285,8 @@ void mips_r4k_init (int ram_size, int vg
     }
 }
 
-QEMUMachine mips_machine = {
-    "mips",
-    "mips r4k platform",
-    mips_r4k_init,
+QEMUMachine mips_r4kc_machine = {
+    "r4kc",
+    "mips r4kc platform",
+    mips_r4kc_init,
 };
--- ./vl.h_orig	2006-07-02 16:31:24.000000000 +0200
+++ ./vl.h	2006-07-02 16:39:18.000000000 +0200
@@ -913,7 +913,7 @@ extern QEMUMachine core99_machine;
 extern QEMUMachine heathrow_machine;
 
 /* mips_r4k.c */
-extern QEMUMachine mips_machine;
+extern QEMUMachine mips_r4kc_machine;
 
 /* shix.c */
 extern QEMUMachine shix_machine;

Re: [Qemu-devel] [PATCH] MIPS instruction set configuration

[Thiemo Seufer]

Dirk Behme wrote:
> Fabrice Bellard wrote:
> >You should add a runtime selection system : see the ARM and PowerPC 
> >targets (I would prefer a parameter to cpu_init(). It was not done that 
> >way on PowerPC for legacy reasons). Each machine should be able to 
> >select the processor it needs (and allow the user to change it if 
> >needed, but it is not the main point).

It might be interesting for MIPS to decouple Machine and CPU somewhat.
E.g. the Malta board supports a number of different 32- and 64-bit CPUs.

> >There is no good reason to make 
> >the selection at compile time because the translator can efficiently 
> >handle any CPU differences at runtime.

I'm a bit dubious about this argument, each instruction needs to be
checked agains a tuple of values. How much performance loss would be
acceptable?

> Find in the attachment a first proposal for runtime
> instruction set configuration for MIPS target. Please
> comment, correct etc.
> 
> Some notes:
> 
> - As first step, only three options for R4K, NEC (partly,
> see below) and FPU are used. The different MIPS ISA levels
> aren't used at the moment and are there for future use.
> Fabrice mentioned that he likes to split the different MIPS
> intstruction set configurations clearly.
>
> - As I understand it, MIPS III is an extension of MIPS II,
> MIPS IV is an extension of MIPS III etc. Therefore I used
> definitions for ISAx which include the smaller ones as well.

Unfortunately it is not that simple. We have the upward-compatible ISAs:

       --> III -> IV -> V -> 64 -> 64R2     (64-bit)
      /                       ^      ^
      |                      /      /
I -> II -----------------> 32 -> 32R2       (32-bit)

plus CPU-specific extensions. For pre-MIPS32 the CP0 instructions are
also CPU-specific, but they are needed for useful CPU emulation. The
GNU toolchain solves this by declaring some specific CPUs to ISA models
(R3000 for MIPS-I, R6000 for MIPS-II, and R4000 for MIPS-III), and
handles their instructions as if they were part of the ISA. Qemu should
IMHO use the same scheme.

The floating point instructions need also some finer granularity, most
ISAs extended the FP instruction set as well.

All of the above still doesn't handle the ASEs, like MIPS16, MIPS3D,
the DSP extensions, etc.

Binutils uses a ISA-ASE-CPU-HAS_FP tuple, since this is at least good
enough for code generation I figure it is also ok for emulation.

> @@ -275,5 +277,58 @@ enum {
>  int cpu_mips_exec(CPUMIPSState *s);
>  CPUMIPSState *cpu_mips_init(void);
>  uint32_t cpu_mips_get_clock (void);
> +void cpu_mips_set_model(CPUMIPSState *env, uint32_t id);
> +
> +enum mips_features {
> +    MIPS_FEATURE_ISA1 = 0x1, /* MIPS I   instruction set architecture */
> +    MIPS_FEATURE_ISA2 = 0x3, /* MIPS II  instruction set architecture */
> +    MIPS_FEATURE_ISA3 = 0x7, /* MIPS III instruction set architecture */
> +    MIPS_FEATURE_ISA4 = 0xF, /* MIPS IV  instruction set architecture */
> +    MIPS_FEATURE_R4K_EXT = 0x10, /* instruction set extension for MIPS R4K */

Those "extensions" seem to be unimplemented, maybe this was intended to
cover partial support for MIPS32R2, IOW, for a 4KE.

> +    MIPS_FEATURE_NEC_EXT = 0x20, /* instruction set extension for NEC CPUs */

This should name the specific CPU core (vr5400 or vr5500?). NEC built
a range of MIPS CPUs (e.g. vr41xx, or R12000) with different capabilities.

[snip]
> +void cpu_mips_set_model(CPUMIPSState *env, uint32_t id)
> +{
> +    env->CP0_PRid = id;
> +    env->features = 0;
> +    switch (id) {
> +    case MIPS_R4Kc:
> +        set_feature(env, MIPS_FEATURE_ISA3);
> +        set_feature(env, MIPS_FEATURE_R4K_EXT);
> +        set_feature(env, MIPS_FEATURE_FPU);
> +        break;

What's the meaning of "R4Kc" here?
- R4000: We don't have 64bit (ISA III) support.
- 4kc: This one has neither ISA III nor FPU.


Thiemo

Re: [Qemu-devel] [PATCH] MIPS instruction set configuration

[Fabrice Bellard]

Thiemo Seufer wrote:
> Dirk Behme wrote:
> 
>>Fabrice Bellard wrote:
>>
>>>You should add a runtime selection system : see the ARM and PowerPC 
>>>targets (I would prefer a parameter to cpu_init(). It was not done that 
>>>way on PowerPC for legacy reasons). Each machine should be able to 
>>>select the processor it needs (and allow the user to change it if 
>>>needed, but it is not the main point).
> 
> 
> It might be interesting for MIPS to decouple Machine and CPU somewhat.
> E.g. the Malta board supports a number of different 32- and 64-bit CPUs.

Each machine can add specific support for that (for example a -cpu 
option). It is likely to come at least for the PC machines.

>>>There is no good reason to make 
>>>the selection at compile time because the translator can efficiently 
>>>handle any CPU differences at runtime.
> 
> 
> I'm a bit dubious about this argument, each instruction needs to be
> checked agains a tuple of values. How much performance loss would be
> acceptable?

It slows down the translator a bit, but most of the time is spent in the 
generated code, not in the translator (otherwise there is no point in 
doing dynamic translation !).

>[...]
> Unfortunately it is not that simple. We have the upward-compatible ISAs:
 >[...]

I add suggest one more parameter to cpu_mips_set_model() to specify 
optional features. A function converting a CPU "string id" into an id + 
features would be interesting too.

Fabrice.

Re: [Qemu-devel] [PATCH] MIPS instruction set configuration

[Thiemo Seufer]

Fabrice Bellard wrote:
[snip]
> >>>There is no good reason to make 
> >>>the selection at compile time because the translator can efficiently 
> >>>handle any CPU differences at runtime.
> >
> >
> >I'm a bit dubious about this argument, each instruction needs to be
> >checked agains a tuple of values. How much performance loss would be
> >acceptable?
> 
> It slows down the translator a bit, but most of the time is spent in the 
> generated code, not in the translator (otherwise there is no point in 
> doing dynamic translation !).

Well, based on that theory I moved the CP0 register handling to dynamic
translation. The patch was rejected because I had no effective method
to assess the performance gain. So I wonder what the accepted tradeoffs
for the implementation are.


Thiemo

Re: [Qemu-devel] [PATCH] MIPS instruction set configuration

[Dirk Behme]

Fabrice Bellard wrote:

> Each machine can add specific support for that (for example a -cpu 
> option). It is likely to come at least for the PC machines.
...
> I add suggest one more parameter to cpu_mips_set_model() to specify 
> optional features. A function converting a CPU "string id" into an id + 
> features would be interesting too.

Fabrice, do you will accept the patch if I remove the 
MIPS_FEATURE_ISAx options and convert MIPS_FEATURE_NEC_EXT 
to MIPS_FEATURE_NEC_VR5400 as described in my previous mail?

I consider this patch as a *first* step from hardcoded and 
compile time selected MIPS machine to flexible configuration 
as proposed by you and Thiemo. Other patches, for example 
with (really nice!) features above, can follow then and take 
this first patch as basis. With this, several small patches 
will improve MIPS step by step. I'd like to avoid a 
'perfect, but unreadable, big & never ready' patch. And, 
with several small patches, more people can contribute ;)

Many thanks for the comments

Dirk

Re: [Qemu-devel] [PATCH] MIPS instruction set configuration

[Fabrice Bellard]

Dirk Behme wrote:
> Fabrice Bellard wrote:
> 
>> Each machine can add specific support for that (for example a -cpu 
>> option). It is likely to come at least for the PC machines.
> 
> ...
> 
>> I add suggest one more parameter to cpu_mips_set_model() to specify 
>> optional features. A function converting a CPU "string id" into an id 
>> + features would be interesting too.
> 
> 
> Fabrice, do you will accept the patch if I remove the MIPS_FEATURE_ISAx 
> options and convert MIPS_FEATURE_NEC_EXT to MIPS_FEATURE_NEC_VR5400 as 
> described in my previous mail?

Yes. Even without I can accept it as it is better than what we 
previously had.

Fabrice.

Re: [Qemu-devel] [PATCH] MIPS instruction set configuration

[Dirk Behme]

[-- Attachment #1: Type: text/plain, Size: 879 bytes --]

Fabrice Bellard wrote:
> Dirk Behme wrote:
> 
>> Fabrice Bellard wrote:
>>
>>> Each machine can add specific support for that (for example a -cpu 
>>> option). It is likely to come at least for the PC machines.
>>
>>> I add suggest one more parameter to cpu_mips_set_model() to specify 
>>> optional features. A function converting a CPU "string id" into an id 
>>> + features would be interesting too.
>>
>> Fabrice, do you will accept the patch if I remove the 
>> MIPS_FEATURE_ISAx options and convert MIPS_FEATURE_NEC_EXT to 
>> MIPS_FEATURE_NEC_VR5400 as described in my previous mail?
> 
> Yes. Even without I can accept it as it is better than what we 
> previously had.

Please find as promised in attachment the updated version of
this patch. It removes MIPS_FEATURE_ISAx for the moment and
renames MIPS_FEATURE_NEC_EXT to  MIPS_FEATURE_NEC_VR5400.

Best regards

Dirk


[-- Attachment #2: mips_isa_config_patch.txt --]
[-- Type: text/plain, Size: 13498 bytes --]

--- ./hw/mips_r4k.c_orig	2006-07-08 07:16:50.000000000 +0200
+++ ./hw/mips_r4k.c	2006-07-08 07:16:45.000000000 +0200
@@ -189,10 +189,10 @@ CPUReadMemoryFunc *io_read[] = {
     &io_readl,
 };
 
-void mips_r4k_init (int ram_size, int vga_ram_size, int boot_device,
-                    DisplayState *ds, const char **fd_filename, int snapshot,
-                    const char *kernel_filename, const char *kernel_cmdline,
-                    const char *initrd_filename)
+void mips_r4kc_init (int ram_size, int vga_ram_size, int boot_device,
+                     DisplayState *ds, const char **fd_filename, int snapshot,
+                     const char *kernel_filename, const char *kernel_cmdline,
+                     const char *initrd_filename)
 {
     char buf[1024];
     int64_t entry = 0;
@@ -203,6 +203,7 @@ void mips_r4k_init (int ram_size, int vg
     long kernel_size;
 
     env = cpu_init();
+    cpu_mips_set_model(env, MIPS_R4Kc);
     register_savevm("cpu", 0, 3, cpu_save, cpu_load, env);
 
     /* allocate RAM */
@@ -284,8 +285,8 @@ void mips_r4k_init (int ram_size, int vg
     }
 }
 
-QEMUMachine mips_machine = {
-    "mips",
-    "mips r4k platform",
-    mips_r4k_init,
+QEMUMachine mips_r4kc_machine = {
+    "r4kc",
+    "mips r4kc platform",
+    mips_r4kc_init,
 };
--- ./vl.h_orig	2006-07-08 07:16:51.000000000 +0200
+++ ./vl.h	2006-07-08 07:16:45.000000000 +0200
@@ -917,7 +917,7 @@ extern QEMUMachine core99_machine;
 extern QEMUMachine heathrow_machine;
 
 /* mips_r4k.c */
-extern QEMUMachine mips_machine;
+extern QEMUMachine mips_r4kc_machine;
 
 /* shix.c */
 extern QEMUMachine shix_machine;
--- ./target-mips/cpu.h_orig	2006-07-08 07:16:50.000000000 +0200
+++ ./target-mips/cpu.h	2006-07-08 07:23:52.000000000 +0200
@@ -210,6 +210,8 @@ struct CPUMIPSState {
     int bcond;                   /* Branch condition (if needed)       */
 
     int halted; /* TRUE if the CPU is in suspend state */
+    
+    uint32_t features;           /* Internal CPU feature flags */
 
     CPU_COMMON
 };
@@ -275,5 +277,30 @@ enum {
 int cpu_mips_exec(CPUMIPSState *s);
 CPUMIPSState *cpu_mips_init(void);
 uint32_t cpu_mips_get_clock (void);
+void cpu_mips_set_model(CPUMIPSState *env, uint32_t id);
+
+enum mips_features {
+    MIPS_FEATURE_R4K_EXT  = 0x1, /* instruction set extension for MIPS R4K */
+    MIPS_FEATURE_NEC_VR5400 = 0x2, /* instruction set extension for NEC VR5400 CPUs */
+    MIPS_FEATURE_FPU = 0x4,  /* floating point instruction set */
+};
+
+#ifdef MIPS_USES_R4K_EXT
+#define mips_uses_r4k_ext()  (env->features & MIPS_FEATURE_R4K_EXT)
+#else
+#define mips_uses_r4k_ext()  0
+#endif
+
+#ifdef MIPS_USES_NEC_VR5400
+#define mips_uses_nec_vr5400()  (env->features & MIPS_FEATURE_NEC_VR5400)
+#else
+#define mips_uses_nec_vr5400()  0
+#endif
+
+#ifdef MIPS_USES_FPU
+#define mips_uses_fpu()  (env->features & MIPS_FEATURE_FPU)
+#else
+#define mips_uses_fpu()  0
+#endif
 
 #endif /* !defined (__MIPS_CPU_H__) */
--- ./target-mips/translate.c_orig	2006-07-08 07:16:50.000000000 +0200
+++ ./target-mips/translate.c	2006-07-08 07:22:48.000000000 +0200
@@ -1887,7 +1887,7 @@ static void gen_blikely(DisasContext *ct
     gen_set_label(l1);
 }
 
-static void decode_opc (DisasContext *ctx)
+static void decode_opc (CPUState *env, DisasContext *ctx)
 {
     int32_t offset;
     int rs, rt, rd, sa;
@@ -1927,7 +1927,17 @@ static void decode_opc (DisasContext *ct
             gen_arith(ctx, op1 | EXT_SPECIAL, rd, rs, rt);
             break;
         case 0x18 ... 0x1B: /* MULT / DIV */
-            gen_muldiv(ctx, op1 | EXT_SPECIAL, rs, rt);
+	    if(!sa) {
+                gen_muldiv(ctx, op1 | EXT_SPECIAL, rs, rt);
+	    } else {
+	        if(mips_uses_nec_vr5400()) {
+	            op1 = ctx->opcode & 0x7FF;
+	            /* tbd: call handler for special NEC instructions */
+                } else {
+	            MIPS_INVAL("NEC extension");
+                    generate_exception(ctx, EXCP_RI);
+                }
+            }
             break;
         case 0x08 ... 0x09: /* Jumps */
             gen_compute_branch(ctx, op1 | EXT_SPECIAL, rs, rd, sa);
@@ -1985,19 +1995,32 @@ static void decode_opc (DisasContext *ct
     case 0x1C:          /* Special2 opcode */
         op1 = ctx->opcode & 0x3F;
         switch (op1) {
-#if defined (MIPS_USES_R4K_EXT)
         /* Those instructions are not part of MIPS32 core */
         case 0x00 ... 0x01: /* Multiply and add/sub */
         case 0x04 ... 0x05:
-            gen_muldiv(ctx, op1 | EXT_SPECIAL2, rs, rt);
+	    if(mips_uses_r4k_ext()) {
+                gen_muldiv(ctx, op1 | EXT_SPECIAL2, rs, rt);
+            } else {
+	        MIPS_INVAL("r4k extension");
+                generate_exception(ctx, EXCP_RI);
+            }
             break;
         case 0x02:          /* MUL */
-            gen_arith(ctx, op1 | EXT_SPECIAL2, rd, rs, rt);
+	    if(mips_uses_r4k_ext()) {
+                gen_arith(ctx, op1 | EXT_SPECIAL2, rd, rs, rt);
+            } else {
+	        MIPS_INVAL("r4k extension");
+                generate_exception(ctx, EXCP_RI);
+            }
             break;
         case 0x20 ... 0x21: /* CLO / CLZ */
-            gen_cl(ctx, op1 | EXT_SPECIAL2, rd, rs);
+	    if(mips_uses_r4k_ext()) {
+                gen_cl(ctx, op1 | EXT_SPECIAL2, rd, rs);
+            } else {
+	        MIPS_INVAL("r4k extension");
+                generate_exception(ctx, EXCP_RI);
+            }
             break;
-#endif
         case 0x3F:          /* SDBBP */
             /* XXX: not clear which exception should be raised
              *      when in debug mode...
@@ -2074,43 +2097,43 @@ static void decode_opc (DisasContext *ct
     case 0x35: /* LDC1 */
     case 0x39: /* SWC1 */
     case 0x3D: /* SDC1 */
-#if defined(MIPS_USES_FPU)
-        gen_op_cp1_enabled();
-        gen_flt_ldst(ctx, op, rt, rs, imm);
-#else
-        generate_exception_err(ctx, EXCP_CpU, 1);
-#endif
+        if(mips_uses_fpu()) {
+            gen_op_cp1_enabled();
+            gen_flt_ldst(ctx, op, rt, rs, imm);
+        } else {
+            generate_exception_err(ctx, EXCP_CpU, 1);
+	}
         break;
 
     case 0x11:          /* CP1 opcode */
-#if defined(MIPS_USES_FPU)
-        gen_op_cp1_enabled();
-        op1 = ((ctx->opcode >> 21) & 0x1F);
-        switch (op1) {
-        case 0x00: /* mfc1 */
-        case 0x02: /* cfc1 */
-        case 0x04: /* mtc1 */
-        case 0x06: /* ctc1 */
-            gen_cp1(ctx, op1 | EXT_CP1, rt, rd);
-            break;
-        case 0x08: /* bc */
-            gen_compute_branch1(ctx, rt, imm << 2);
-            return;
-        case 0x10: /* 16: fmt=single fp */
-        case 0x11: /* 17: fmt=double fp */
-        case 0x14: /* 20: fmt=32bit fixed */
-        case 0x15: /* 21: fmt=64bit fixed */
-            gen_farith(ctx, op1, rt, rd, sa, ctx->opcode & 0x3f);
-            break;
-        default:
-            generate_exception_err(ctx, EXCP_RI, 1);
+        if(mips_uses_fpu()) {
+            gen_op_cp1_enabled();
+            op1 = ((ctx->opcode >> 21) & 0x1F);
+            switch (op1) {
+            case 0x00: /* mfc1 */
+            case 0x02: /* cfc1 */
+            case 0x04: /* mtc1 */
+            case 0x06: /* ctc1 */
+                gen_cp1(ctx, op1 | EXT_CP1, rt, rd);
+                break;
+            case 0x08: /* bc */
+                gen_compute_branch1(ctx, rt, imm << 2);
+                return;
+            case 0x10: /* 16: fmt=single fp */
+            case 0x11: /* 17: fmt=double fp */
+            case 0x14: /* 20: fmt=32bit fixed */
+            case 0x15: /* 21: fmt=64bit fixed */
+                gen_farith(ctx, op1, rt, rd, sa, ctx->opcode & 0x3f);
+                break;
+            default:
+                generate_exception_err(ctx, EXCP_RI, 1);
+                break;
+            }
             break;
+        } else {
+            generate_exception_err(ctx, EXCP_CpU, 1);
         }
         break;
-#else
-        generate_exception_err(ctx, EXCP_CpU, 1);
-#endif
-        break;
 
     /* COP2.  */
     case 0x32: /* LWC2 */
@@ -2262,7 +2285,7 @@ int gen_intermediate_code_internal (CPUS
             gen_opc_instr_start[lj] = 1;
         }
         ctx.opcode = ldl_code(ctx.pc);
-        decode_opc(&ctx);
+        decode_opc(env, &ctx);
         ctx.pc += 4;
 
         if (env->singlestep_enabled)
@@ -2400,44 +2423,3 @@ void cpu_dump_state (CPUState *env, FILE
     fpu_dump_state(env, f, cpu_fprintf, flags);
 #endif
 }
-
-CPUMIPSState *cpu_mips_init (void)
-{
-    CPUMIPSState *env;
-
-    env = qemu_mallocz(sizeof(CPUMIPSState));
-    if (!env)
-        return NULL;
-    cpu_exec_init(env);
-    tlb_flush(env, 1);
-    /* Minimal init */
-    env->PC = 0xBFC00000;
-#if defined (MIPS_USES_R4K_TLB)
-    env->CP0_random = MIPS_TLB_NB - 1;
-#endif
-    env->CP0_Wired = 0;
-    env->CP0_Config0 = MIPS_CONFIG0;
-#if defined (MIPS_CONFIG1)
-        env->CP0_Config1 = MIPS_CONFIG1;
-#endif
-#if defined (MIPS_CONFIG2)
-        env->CP0_Config2 = MIPS_CONFIG2;
-#endif
-#if defined (MIPS_CONFIG3)
-        env->CP0_Config3 = MIPS_CONFIG3;
-#endif
-    env->CP0_Status = (1 << CP0St_CU0) | (1 << CP0St_BEV);
-    env->CP0_WatchLo = 0;
-    env->hflags = MIPS_HFLAG_ERL;
-    /* Count register increments in debug mode, EJTAG version 1 */
-    env->CP0_Debug = (1 << CP0DB_CNT) | (0x1 << CP0DB_VER);
-    env->CP0_PRid = MIPS_CPU;
-    env->exception_index = EXCP_NONE;
-#if defined(CONFIG_USER_ONLY)
-    env->hflags |= MIPS_HFLAG_UM;
-#endif
-#ifdef MIPS_USES_FPU
-    env->fcr0 = MIPS_FCR0;	
-#endif
-    return env;
-}
--- ./target-mips/helper.c_orig	2006-07-08 07:16:50.000000000 +0200
+++ ./target-mips/helper.c	2006-07-08 07:22:04.000000000 +0200
@@ -430,3 +430,68 @@ void do_interrupt (CPUState *env)
     }
     env->exception_index = EXCP_NONE;
 }
+
+CPUMIPSState *cpu_mips_init (void)
+{
+    CPUMIPSState *env;
+
+    env = qemu_mallocz(sizeof(CPUMIPSState));
+    if (!env)
+        return NULL;
+    cpu_exec_init(env);
+    tlb_flush(env, 1);
+    /* Minimal init */
+    env->PC = 0xBFC00000;
+#if defined (MIPS_USES_R4K_TLB)
+    env->CP0_random = MIPS_TLB_NB - 1;
+#endif
+    env->CP0_Wired = 0;
+    env->CP0_Config0 = MIPS_CONFIG0;
+#if defined (MIPS_CONFIG1)
+        env->CP0_Config1 = MIPS_CONFIG1;
+#endif
+#if defined (MIPS_CONFIG2)
+        env->CP0_Config2 = MIPS_CONFIG2;
+#endif
+#if defined (MIPS_CONFIG3)
+        env->CP0_Config3 = MIPS_CONFIG3;
+#endif
+    env->CP0_Status = (1 << CP0St_CU0) | (1 << CP0St_BEV);
+    env->CP0_WatchLo = 0;
+    env->hflags = MIPS_HFLAG_ERL;
+    /* Count register increments in debug mode, EJTAG version 1 */
+    env->CP0_Debug = (1 << CP0DB_CNT) | (0x1 << CP0DB_VER);
+    env->exception_index = EXCP_NONE;
+#if defined(CONFIG_USER_ONLY)
+    env->hflags |= MIPS_HFLAG_UM;
+#endif
+    return env;
+}
+
+static inline void set_feature(CPUMIPSState *env, int feature)
+{
+    env->features |= feature;
+}
+
+void cpu_mips_set_model(CPUMIPSState *env, uint32_t id)
+{
+    env->CP0_PRid = id;
+    env->features = 0;
+    switch (id) {
+    case MIPS_R4Kc:
+        set_feature(env, MIPS_FEATURE_R4K_EXT);
+        set_feature(env, MIPS_FEATURE_FPU);
+        break;
+    default:
+        cpu_abort(env, "Bad CPU ID: %x\n", id);
+        break;
+    }
+
+    if(mips_uses_fpu()) {
+        env->fcr0 = MIPS_FCR0;
+#if defined (MIPS_CONFIG1)
+	env->CP0_Config1 |= (1 << CP0C1_FP);
+#endif
+    }
+}
+
--- ./target-mips/mips-defs.h_orig	2006-07-08 07:16:50.000000000 +0200
+++ ./target-mips/mips-defs.h	2006-07-08 07:16:45.000000000 +0200
@@ -9,10 +9,6 @@
 #define MIPS_R4Kc 0x00018000
 #define MIPS_R4Kp 0x00018300
 
-/* Emulate MIPS R4Kc for now */
-#define MIPS_CPU MIPS_R4Kc
-
-#if (MIPS_CPU == MIPS_R4Kc)
 /* 32 bits target */
 #define TARGET_LONG_BITS 32
 /* real pages are variable size... */
@@ -40,28 +36,13 @@
 /* 16 TLBs, 64 sets Icache, 16 bytes Icache line, 2-way Icache,
  * 64 sets Dcache, 16 bytes Dcache line, 2-way Dcache,
  * no performance counters, watch registers present, no code compression,
- * EJTAG present, FPU enable bit depending on MIPS_USES_FPU
+ * EJTAG present, FPU bit is set depending on runtime FPU selection
  */
 #define MIPS_CONFIG1                                            \
 ((15 << CP0C1_MMU) |                                            \
  (0x000 << CP0C1_IS) | (0x3 << CP0C1_IL) | (0x01 << CP0C1_IA) | \
  (0x000 << CP0C1_DS) | (0x3 << CP0C1_DL) | (0x01 << CP0C1_DA) | \
  (0 << CP0C1_PC) | (1 << CP0C1_WR) | (0 << CP0C1_CA) |          \
- (1 << CP0C1_EP) | (MIPS_USES_FPU << CP0C1_FP))
-#elif (MIPS_CPU == MIPS_R4Kp)
-/* 32 bits target */
-#define TARGET_LONG_BITS 32
-/* real pages are variable size... */
-#define TARGET_PAGE_BITS 12
-/* Uses MIPS R4Kx enhancements to MIPS32 architecture */
-#define MIPS_USES_R4K_EXT
-/* Uses MIPS R4Km FPM MMU model */
-#define MIPS_USES_R4K_FPM
-#else
-#error "MIPS CPU not defined"
-/* Remainder for other flags */
-//#define TARGET_MIPS64
-//#define MIPS_USES_FPU
-#endif
+ (1 << CP0C1_EP))
 
 #endif /* !defined (__QEMU_MIPS_DEFS_H__) */
--- ./vl.c_orig	2006-07-08 07:16:50.000000000 +0200
+++ ./vl.c	2006-07-08 07:16:45.000000000 +0200
@@ -5455,7 +5455,7 @@ void register_machines(void)
     qemu_register_machine(&core99_machine);
     qemu_register_machine(&prep_machine);
 #elif defined(TARGET_MIPS)
-    qemu_register_machine(&mips_machine);
+    qemu_register_machine(&mips_r4kc_machine);
 #elif defined(TARGET_SPARC)
 #ifdef TARGET_SPARC64
     qemu_register_machine(&sun4u_machine);

Re: [Qemu-devel] [PATCH] MIPS instruction set configuration

[Dirk Behme]

Thiemo Seufer wrote:
> Dirk Behme wrote:
>>- As I understand it, MIPS III is an extension of MIPS II,
>>MIPS IV is an extension of MIPS III etc. Therefore I used
>>definitions for ISAx which include the smaller ones as well.
> 
> Unfortunately it is not that simple. We have the upward-compatible ISAs:

Mmph ;)  What do you think about this: Looking at the recent 
MIPS code, we have a #define MIPS_USES_R4K_EXT and a #define 
MIPS_USES_FPU, nothing else. No splitting of the different 
ISAs and FPU instructions at all. And, looking at your 
explanation, it seems to be really tricky to do this 
splitting. So I'll remove the  MIPS_FEATURE_ISAx definitions 
from the patch (as mentioned, they were unused and only 
example), this will change nothing compared to the existing 
code.

I'd prefer to make changes and improvements in small 
readable pieces without changing too much. One small 
improvement after the other, not a big 'perfect', but 
unreadable, patch. So the splitting of the ISA levels is 
something for an other patch.

The improvement of this patch is to convert the compile time 
switches MIPS_USES_R4K_EXT, MIPS_USES_FPU to compile *and* 
runtime switches and to make machine runtime selectable (and 
introduce MIPS_FEATURE_NEC_xxx).

>>+enum mips_features {
>>+    MIPS_FEATURE_ISA1 = 0x1, /* MIPS I   instruction set architecture */
>>+    MIPS_FEATURE_ISA2 = 0x3, /* MIPS II  instruction set architecture */
>>+    MIPS_FEATURE_ISA3 = 0x7, /* MIPS III instruction set architecture */
>>+    MIPS_FEATURE_ISA4 = 0xF, /* MIPS IV  instruction set architecture */

I'll remove this.

>>+    MIPS_FEATURE_R4K_EXT = 0x10, /* instruction set extension for MIPS R4K */
> 
> 
> Those "extensions" seem to be unimplemented, maybe this was intended to
> cover partial support for MIPS32R2, IOW, for a 4KE.

Sorry, what do you mean here? I only took the existing 
#define MIPS_USES_R4K_EXT.

>>+    MIPS_FEATURE_NEC_EXT = 0x20, /* instruction set extension for NEC CPUs */
> 
> This should name the specific CPU core (vr5400 or vr5500?). NEC built
> a range of MIPS CPUs (e.g. vr41xx, or R12000) with different capabilities.

Yes, I'll change this.

However, on long term, there is a 'keep code simple' vs 
'split instructions technically correct' tradeoff. Adding 
later switches for e.g. vr41xx and R12000 will be okay from 
clean instruction split point of view, but additional 
switches will mess up the code and may be doublicate some 
stuff. While one MIPS_FEATURE_NEC_EXT which contains all NEC 
extensions will be simple, but wrong from instruction splitting.

But I'll convert this to MIPS_FEATURE_NEC_VR5400.

>>+void cpu_mips_set_model(CPUMIPSState *env, uint32_t id)
>>+{
>>+    env->CP0_PRid = id;
>>+    env->features = 0;
>>+    switch (id) {
>>+    case MIPS_R4Kc:
>>+        set_feature(env, MIPS_FEATURE_ISA3);
>>+        set_feature(env, MIPS_FEATURE_R4K_EXT);
>>+        set_feature(env, MIPS_FEATURE_FPU);
>>+        break;
> 
> 
> What's the meaning of "R4Kc" here?
> - R4000: We don't have 64bit (ISA III) support.
> - 4kc: This one has neither ISA III nor FPU.

That's easy: I don't know ;) If you look at existing 
mips-defs.h it is the name of the only hardcoded machine we 
have at the  moment. It enables R4K extensions and FPU. It 
was my goal to change no functionality, only to make first 
step to runtime selection. lI'll remove the ISA option.

Best regards

Dirk

Re: [Qemu-devel] [PATCH] MIPS instruction set configuration

[Thiemo Seufer]

Dirk Behme wrote:
> Thiemo Seufer wrote:
> >Dirk Behme wrote:
> >>- As I understand it, MIPS III is an extension of MIPS II,
> >>MIPS IV is an extension of MIPS III etc. Therefore I used
> >>definitions for ISAx which include the smaller ones as well.
> >
> >Unfortunately it is not that simple. We have the upward-compatible ISAs:
> 
> Mmph ;)  What do you think about this: Looking at the recent 
> MIPS code, we have a #define MIPS_USES_R4K_EXT and a #define 
> MIPS_USES_FPU, nothing else. No splitting of the different 
> ISAs and FPU instructions at all. And, looking at your 
> explanation, it seems to be really tricky to do this 
> splitting.

Somewhat complicated, but not really tricky. The OPCODE_IS_MEMBER
macro in binutils is IMHO roughly what we want to have in qemu at
some point (FPU instructions are already covered by the CP1 enabled
test). At least as long as that test doesn't slow down things too
much, for qemu this might be more relevant than for binutils.

> So I'll remove the  MIPS_FEATURE_ISAx definitions 
> from the patch (as mentioned, they were unused and only 
> example), this will change nothing compared to the existing 
> code.
>
> I'd prefer to make changes and improvements in small 
> readable pieces without changing too much. One small 
> improvement after the other, not a big 'perfect', but 
> unreadable, patch. So the splitting of the ISA levels is 
> something for an other patch.

I recommend to go for a sufficiently flexible interface first, and then
introduce it gradually in all appropriate places. A macro like:

MIPS_OPC(ISA, ASE, CPU)

which compares the arguments with the currently selected CPU emulation
and throws an RI exception if the feature doesn't exist:

    ...
    case OPC_LD:
        MIPS_OPC(MIPS-III, 0, 0);
	...
	break;
    ...

> improvement of this patch is to convert the compile time 
> switches MIPS_USES_R4K_EXT, MIPS_USES_FPU to compile *and* 
> runtime switches and to make machine runtime selectable (and 
> introduce MIPS_FEATURE_NEC_xxx).

My point is those MIPS_USES_*/MIPS_FEATURE_* should be abstracted
better, that is, they should move in the implementation of the
MIPS_OPC macro.

This is not meant as an objection, I'm just drawing from experience
with binutils, where it took some iterations (and wasted effort) to
get it sufficiently right.

[snip]
> >>+void cpu_mips_set_model(CPUMIPSState *env, uint32_t id)
> >>+{
> >>+    env->CP0_PRid = id;
> >>+    env->features = 0;
> >>+    switch (id) {
> >>+    case MIPS_R4Kc:
> >>+        set_feature(env, MIPS_FEATURE_ISA3);
> >>+        set_feature(env, MIPS_FEATURE_R4K_EXT);
> >>+        set_feature(env, MIPS_FEATURE_FPU);
> >>+        break;
> >
> >
> >What's the meaning of "R4Kc" here?
> >- R4000: We don't have 64bit (ISA III) support.
> >- 4kc: This one has neither ISA III nor FPU.
> 
> That's easy: I don't know ;) If you look at existing 
> mips-defs.h it is the name of the only hardcoded machine we 
> have at the  moment. It enables R4K extensions and FPU. It 
> was my goal to change no functionality, only to make first 
> step to runtime selection. lI'll remove the ISA option.

Well, there is no CPU named "R4Kc". What qemu emulates today resembles
mostly a 4kc, that is a MIPS32R1 CPU which has no FPU support.

I figure you are going for emulation of a vr5400, a MIPS-IV CPU with
FPU and some additional multiply-add instructions.

What I hope to get done is support for MIPS32R2 including FPU, this is
close to a 24kf.


Thiemo

Re: [Qemu-devel] [PATCH] MIPS instruction set configuration

[Stefan Weil]

Hi all,

just for information about current projects for QEMU MIPS:

my machine is AR7 which includes a MIPS 4KEc core.
This core supports the MIPS32R2 architecture and has no FPU.

As far as I know the MIPS architecture, most CPU features
can be read from well defined bits and bytes in the CP0 registers.

These registers should be set by every machine definition in QEMU.

So the emulation (translator) code could get all information needed from
the CP0 registers which are part of variable "env". There is no
need to introduce new defines or variables to get endianess,
instruction set, presence of FPU or internal timer, and other
features. And if generic property bits are not enough:
the processor identification is part of the CP0 registers, too.

Of course, one might mirror some features in extra variables for
performance reasons.

What do you think of my proposal?

Regards
Stefan

>
>Well, there is no CPU named "R4Kc". What qemu emulates today resembles
>mostly a 4kc, that is a MIPS32R1 CPU which has no FPU support.
>
>I figure you are going for emulation of a vr5400, a MIPS-IV CPU with
>FPU and some additional multiply-add instructions.
>
>What I hope to get done is support for MIPS32R2 including FPU, this is
>close to a 24kf.
>
>
>Thiemo
>
>

Re: [Qemu-devel] [PATCH] MIPS instruction set configuration

[Thiemo Seufer]

Stefan Weil wrote:
> Hi all,
> 
> just for information about current projects for QEMU MIPS:
> 
> my machine is AR7 which includes a MIPS 4KEc core.
> This core supports the MIPS32R2 architecture and has no FPU.
> 
> As far as I know the MIPS architecture, most CPU features
> can be read from well defined bits and bytes in the CP0 registers.
> 
> These registers should be set by every machine definition in QEMU.
> 
> So the emulation (translator) code could get all information needed from
> the CP0 registers which are part of variable "env". There is no
> need to introduce new defines or variables to get endianess,
> instruction set, presence of FPU or internal timer, and other
> features.

This is mostly true for MIPS32 (and later) conforming CPUs. Earlier ones
(like the vr5400 Dirk is interested in) didn't announce all their
capabilities via config registers, and coprocessor CP0, where the
config registers reside, wasn't standardised.

> And if generic property bits are not enough:
> the processor identification is part of the CP0 registers, too.
> 
> Of course, one might mirror some features in extra variables for
> performance reasons.
> 
> What do you think of my proposal?

It only works for the more modern CPUs. I think we have to invert the
scheme, that is, keep the "master copy" of the configuration in an
opaque place, and feed the config registers from there.


Thiemo

Re: [Qemu-devel] [PATCH] MIPS instruction set configuration

[Dirk Behme]

Thiemo Seufer wrote:
...
> I recommend to go for a sufficiently flexible interface first, and then
> introduce it gradually in all appropriate places. A macro like:
> 
> MIPS_OPC(ISA, ASE, CPU)
> 
> which compares the arguments with the currently selected CPU emulation
> and throws an RI exception if the feature doesn't exist:
> 
>     ...
>     case OPC_LD:
>         MIPS_OPC(MIPS-III, 0, 0);
> 	...
> 	break;
>     ...
> 
...
> My point is those MIPS_USES_*/MIPS_FEATURE_* should be abstracted
> better, that is, they should move in the implementation of the
> MIPS_OPC macro.

I really like your proposals of MIPS_OPC() and better
abstraction of MIPS_USES_*/MIPS_FEATURE_*!

Do you think you can post a patch which introduces the basic
parts of this functionality in the short term? Then we can
use it as a starter for more improvements.

> This is not meant as an objection, I'm just drawing from experience
> with binutils, where it took some iterations (and wasted effort) to
> get it sufficiently right.

You are right with your wasted effort argument above as well.

However, my patch does not only introduce
MIPS_USES_*/MIPS_FEATURE_*. It additionally introduces
cpu_mips_set_model() (yes, I know, this can be improved as
well, as Fabrice mentioned). So it shouldn't be to much
wasted effort to keep the cpu_mips_set_model() and convert
the three or four places of my MIPS_USES_*/MIPS_FEATURE_* to
your MIPS_OPC abstraction (in case my patch is accepted ;) ).

Btw, while talking about cpu_mips_set_model(): While
thinking about it I found that we should try to keep the
generic  parts of cpu and model selection logic in sync with
other architectures, I think at least with ARM (where
possible). There will be differences in details like in
instruction set splitting with your MIPS_OPC logic. But
would be nice if we make generic improvements to e.g.
cpu_mips_set_model() at MIPS, we make the same at ARM as well.

...
> Well, there is no CPU named "R4Kc". What qemu emulates today resembles
> mostly a 4kc, that is a MIPS32R1 CPU which has no FPU support.

Yes, I understand this. But two things:

First, in the current MIPS configuration FPU is enabled for
"R4Kc". I think we shouldn't disable anything what is there
at the moment, without giving an alternative for people
using this (e.g. introducing an additional machine). And
this only because there is no real world equivalent for this.

Second, while I agree that there is no real world equivalent
for this, I think this is one of the biggest advantages of a
simulator like qemu: Your are able to configure and test
machine and feature combinations which will never exist in
real world really quickly.

This second point is "the simulated machines should model
the real world 110% exactly" vs "I don't need 100%
simulation accuracy and wan't to be able to use
configurations which are not possible in real world".
Depends on how people will use qemu. I think while the first
argument may be valid for x86 where people want to run e.g.
Windows on Linux, for embedded processors like ARM or MIPS
the second may be valid and interesting as well.

Cheers

Dirk

Re: [Qemu-devel] [PATCH] MIPS instruction set configuration

[Thiemo Seufer]

Dirk Behme wrote:
> Thiemo Seufer wrote:
> ...
> >I recommend to go for a sufficiently flexible interface first, and then
> >introduce it gradually in all appropriate places. A macro like:
> >
> >MIPS_OPC(ISA, ASE, CPU)
> >
> >which compares the arguments with the currently selected CPU emulation
> >and throws an RI exception if the feature doesn't exist:
> >
> >    ...
> >    case OPC_LD:
> >        MIPS_OPC(MIPS-III, 0, 0);
> >	...
> >	break;
> >    ...
> >
> ...
> >My point is those MIPS_USES_*/MIPS_FEATURE_* should be abstracted
> >better, that is, they should move in the implementation of the
> >MIPS_OPC macro.
> 
> I really like your proposals of MIPS_OPC() and better
> abstraction of MIPS_USES_*/MIPS_FEATURE_*!
> 
> Do you think you can post a patch which introduces the basic
> parts of this functionality in the short term? Then we can
> use it as a starter for more improvements.

I'm busy this weekend.

[snip]
> >Well, there is no CPU named "R4Kc". What qemu emulates today resembles
> >mostly a 4kc, that is a MIPS32R1 CPU which has no FPU support.
> 
> Yes, I understand this. But two things:
> 
> First, in the current MIPS configuration FPU is enabled for
> "R4Kc". I think we shouldn't disable anything what is there
> at the moment, without giving an alternative for people
> using this (e.g. introducing an additional machine). And
> this only because there is no real world equivalent for this.

I agree we should have a generic "emulate everything" configuration
with MIPS64R2 + FPU + all ASEs + all non-conflicting CPU extensions.

> Second, while I agree that there is no real world equivalent
> for this, I think this is one of the biggest advantages of a
> simulator like qemu: Your are able to configure and test
> machine and feature combinations which will never exist in
> real world really quickly.

Probably, but the generic configuration will cover most of that
already.


Thiemo

Re: [Qemu-devel] Pending MIPS patches

[Raphaël Rigo]

Dirk Behme wrote:
> Hi Raphaël,
>
> maybe you find some time and like to answer to Fabrice's remark 
> regarding patch 8
> below?
>
> http://lists.gnu.org/archive/html/qemu-devel/2006-06/msg00419.html
>
> Would be nice to get as many of the pending MIPS patches applied as 
> possible.
>
> Many thanks
>
> Dirk
>
> Fabrice Bellard wrote:
>>> Pending MIPS patches (bugs & improvements):
>>> ===========================================
>
>>> 8. [PATCH] Add mips-user signal handling
>>> http://lists.gnu.org/archive/html/qemu-devel/2006-06/msg00319.html
>>
>>
>> OK. Need to check the handling of "env->PC += 4" in 
>> linux-user/main.c. I want to be sure there is not a better solution. 
>> Adding setup_rt_frame would be good too.
Hello,
there are 2 possibilities to handle this env->PC += 4:
    -    do a special case wrapper here to avoid increasing PC when it's 
a sigreturn.
    -    in do_sigreturn set PC with a -4 offset.
I chose the first way because I thought it would be cleaner, if anyone 
was to change this part of the code, then it would be clear that 
sigreturn directly sets PC. Using the 2nd possibility, one could change 
this += 4 without knowing do_sigreturn would be affected.

I also checked if we could get rid of this weird piece of code, but 
could not find a clear way to do so.

So i preferred maintainability over efficiency, but if you think
setting PC with an offset of -4 in signal.c is better, tell me and i 
will update the patch.

Thanks for reviewing,
Raphaël

Re: [Qemu-devel] Pending MIPS patches

[Fabrice Bellard]

Raphaël Rigo wrote:
> Dirk Behme wrote:
> 
>> Hi Raphaël,
>>
>> maybe you find some time and like to answer to Fabrice's remark 
>> regarding patch 8
>> below?
>>
>> http://lists.gnu.org/archive/html/qemu-devel/2006-06/msg00419.html
>>
>> Would be nice to get as many of the pending MIPS patches applied as 
>> possible.
>>
>> Many thanks
>>
>> Dirk
>>
>> Fabrice Bellard wrote:
>>
>>>> Pending MIPS patches (bugs & improvements):
>>>> ===========================================
>>
>>
>>>> 8. [PATCH] Add mips-user signal handling
>>>> http://lists.gnu.org/archive/html/qemu-devel/2006-06/msg00319.html
>>>
>>>
>>>
>>> OK. Need to check the handling of "env->PC += 4" in 
>>> linux-user/main.c. I want to be sure there is not a better solution. 
>>> Adding setup_rt_frame would be good too.
> 
> Hello,
> there are 2 possibilities to handle this env->PC += 4:
>    -    do a special case wrapper here to avoid increasing PC when it's 
> a sigreturn.
>    -    in do_sigreturn set PC with a -4 offset.
> I chose the first way because I thought it would be cleaner, if anyone 
> was to change this part of the code, then it would be clear that 
> sigreturn directly sets PC. Using the 2nd possibility, one could change 
> this += 4 without knowing do_sigreturn would be affected.
> 
> I also checked if we could get rid of this weird piece of code, but 
> could not find a clear way to do so.
> 
> So i preferred maintainability over efficiency, but if you think
> setting PC with an offset of -4 in signal.c is better, tell me and i 
> will update the patch.

There is another simpler solution and I applied it. BTW, the Linux 
kernel uses the same solution :-)

Fabrice.

Re: [Qemu-devel] Pending MIPS patches

[Fabrice Bellard]

Hi,

Another task to improve mips-user and the other QEMU user targets would 
be to properly define the errno values (use the TARGET_ prefix) and to 
use them at the appropriate places. A header "errno-generic.h" could be 
used for most targets and specific targets (such as mips) could add a 
errno.h in their directory.

Regards,

Fabrice.