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

[Thiemo Seufer <ths@networkno.de>]

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