Skip to contentSkip to conversation

Pending MIPS patches

Qemu-devel In review Dirk Behme 6 participants 24 messages Jun 25, 2006 – Jul 8, 2006 1 message not in the archive

lore
Share & tools
lore

Dirk Behme

author
#0 lore raw

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

#1 lore raw
Quoted 6 lines · 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

Quoted 7 lines

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.

Quoted 2 lines

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.

Quoted 2 lines

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

Same remark.

Quoted 2 lines

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.

Quoted 1 line

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

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

Quoted 7 lines

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.

Quoted 8 lines

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.

Quoted 2 lines

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.

#2 lore raw
Quoted 4 lines · Fabrice Bellard
Quoted 2 lines

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.

Quoted 4 lines
Quoted 2 lines

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

Signature · 4 lines
Marius Groeger <[email protected]>
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
author
#3 lore raw
In reply to Fabrice Bellard
Quoted 11 lines · Fabrice Bellard
Quoted 2 lines

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.

Quoted 2 lines

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.

Quoted 5 lines
Quoted 2 lines

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:

Review

Quoted 2 lines
(quoted diff) +1 -1
-        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.

Quoted 1 line

+ 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!

#4 lore raw
Quoted 17 lines · Dirk Behme
Quoted 5 lines · Fabrice Bellard
Quoted 2 lines

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

Quoted 8 lines

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.

Quoted 2 lines

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

Same remark.

Quoted 7 lines

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.

author
#5 lore raw

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

Quoted 7 lines · Fabrice Bellard
Quoted 7 lines
Quoted 4 lines
Quoted 2 lines

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.

Quoted 7 lines
Quoted 5 lines

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

#6 lore raw

[Qemu-devel] Re: MIPS instruction set configuration

Quoted 13 lines · Dirk Behme
Quoted 11 lines · Fabrice Bellard
Quoted 11 lines
Quoted 4 lines
Quoted 2 lines

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

Quoted 7 lines
Quoted 3 lines
Quoted 1 line

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

Signature · 4 lines
Marius Groeger <[email protected]>
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
#7 lore raw

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

Quoted 23 lines · Marius Groeger
Quoted 21 lines · Dirk Behme
Quoted 17 lines · Fabrice Bellard
Quoted 7 lines
Quoted 5 lines
Quoted 2 lines

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

Same remark.

Quoted 7 lines

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 ?

Quoted 8 lines
Quoted 3 lines

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.

author
#8 lore raw

PATCH 1/1 MIPS instruction set configuration

Quoted 1 line · Fabrice Bellard

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

target-mips/cpu.h renamed from target-mips/cpu.h_orig +55 -0
@@ -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 renamed from target-mips/helper.c_orig +66 -0

69 lines not on this page · raw message

target-mips/translate.c renamed from target-mips/translate.c_orig +62 -80

189 lines not on this page · raw message

target-mips/mips-defs.h renamed from target-mips/mips-defs.h_orig +2 -21
@@ -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 renamed from vl.c_orig +1 -1
@@ -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 renamed from hw/mips_r4k.c_orig +9 -8
@@ -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 renamed from vl.h_orig +1 -1
@@ -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;
#9 lore raw

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

Quoted 6 lines · Dirk Behme
Quoted 5 lines · Fabrice Bellard

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.

Quoted 3 lines
Quoted 3 lines

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?

Quoted 12 lines

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.

Quoted 3 lines

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

target-mips/cpu.h enum { · line 287

Quoted 12 lines
target-mips/cpu.h renamed from target-mips/cpu.h_orig +8 -0
@@ -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.

target-mips/cpu.h enum { · line 288

Quoted 1 line
target-mips/cpu.h renamed from target-mips/cpu.h_orig +1 -0
+    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]

target-mips/helper.c void do_interrupt (CPUState *env) · line 485

Quoted 10 lines
target-mips/helper.c renamed from target-mips/helper.c_orig +10 -0
+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

#10 lore raw

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

Quoted 9 lines · Thiemo Seufer
Quoted 3 lines · Dirk Behme
Quoted 1 line · Fabrice Bellard

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.

Quoted 6 lines
Quoted 1 line
Quoted 1 line

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

Quoted 3 lines

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

#11 lore raw

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

Fabrice Bellard wrote:
[snip]

Quoted 5 lines
Quoted 5 lines
Quoted 3 lines
Quoted 3 lines

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

Quoted 7 lines
Quoted 3 lines

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

author
#12 lore raw

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

In reply to Fabrice Bellard
Quoted 1 line · Fabrice Bellard

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

...

Quoted 1 line

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

#13 lore raw

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

Quoted 11 lines · Dirk Behme
Quoted 1 line · Fabrice Bellard

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

...

Quoted 2 lines

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.

author
#14 lore raw

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

Quoted 4 lines · Fabrice Bellard
Quoted 2 lines · Dirk Behme

Fabrice Bellard wrote:

Quoted 8 lines
Quoted 6 lines
Quoted 4 lines

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

hw/mips_r4k.c renamed from hw/mips_r4k.c_orig +9 -8
@@ -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 renamed from vl.h_orig +1 -1
@@ -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 renamed from target-mips/cpu.h_orig +27 -0
@@ -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 renamed from target-mips/translate.c_orig +62 -80

189 lines not on this page · raw message

target-mips/helper.c renamed from target-mips/helper.c_orig +65 -0

68 lines not on this page · raw message

target-mips/mips-defs.h renamed from target-mips/mips-defs.h_orig +2 -21
@@ -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 renamed from vl.c_orig +1 -1
@@ -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);
author
#15 lore raw

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

In reply to Thiemo Seufer
Quoted 6 lines · Thiemo Seufer
Quoted 3 lines · Dirk Behme

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

Quoted 5 lines

target-mips/cpu.h enum { · line 286

Quoted 5 lines
target-mips/cpu.h renamed from target-mips/cpu.h_orig +5 -0
+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.

Quoted 5 lines
Quoted 1 line

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

Quoted 4 lines
Quoted 1 line

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

Quoted 7 lines
Quoted 7 lines
target-mips/helper.c renamed from target-mips/helper.c_orig +7 -0
+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);
Quoted 8 lines

target-mips/helper.c void do_interrupt (CPUState *env) · line 485

Quoted 3 lines
target-mips/helper.c renamed from target-mips/helper.c_orig +3 -0
+        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

#16 lore raw

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

Quoted 6 lines · Dirk Behme
Quoted 5 lines · Thiemo Seufer
Quoted 3 lines · Dirk Behme

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

Quoted 8 lines
Quoted 1 line

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.

Quoted 10 lines

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;
    ...
Quoted 4 lines

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]

Quoted 16 lines
Quoted 7 lines
Quoted 7 lines
target-mips/helper.c renamed from target-mips/helper.c_orig +7 -0
+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);
Quoted 8 lines

target-mips/helper.c void do_interrupt (CPUState *env) · line 485

Quoted 3 lines
target-mips/helper.c renamed from target-mips/helper.c_orig +3 -0
+        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.

Quoted 5 lines

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

#17 lore raw

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

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

Quoted 14 lines

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

#18 lore raw

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

Quoted 10 lines · 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.

Quoted 7 lines

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.

Quoted 7 lines

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

author
#19 lore raw

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

In reply to Thiemo Seufer

Thiemo Seufer wrote:
...

Quoted 15 lines

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

...

Quoted 3 lines

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.

Quoted 3 lines

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.

...

Quoted 2 lines

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

#20 lore raw

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

Quoted 22 lines · Dirk Behme

Thiemo Seufer wrote:
...

Quoted 15 lines

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

...

Quoted 3 lines

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.

Quoted 6 lines

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]

Quoted 10 lines
Quoted 2 lines

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.

Quoted 5 lines

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

A message that is not in the archive (sent off-list or not yet received): [email protected]

#22 lore raw
Quoted 7 lines · Dirk Behme

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

Quoted 15 lines

Would be nice to get as many of the pending MIPS patches applied as possible.

Many thanks

Dirk

Quoted 8 lines · Fabrice Bellard
Quoted 5 lines

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

#23 lore raw

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.

#24 lore raw
In reply to Raphaël Rigo
Quoted 35 lines · Raphaël Rigo
Quoted 7 lines · Dirk Behme

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

Quoted 18 lines

Would be nice to get as many of the pending MIPS patches applied as possible.

Many thanks

Dirk

Quoted 10 lines · Fabrice Bellard
Quoted 6 lines

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.

Quoted 4 lines

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.