Re: [Qemu-devel] [PATCH] mips: Respect CP0.Status.CU1 for microMIPS FP branches

[Maciej W. Rozycki]

On Wed, 5 Nov 2014, Leon Alrae wrote:

> > qemu-umips-cu1-ex.diff
> > Index: qemu-git-trunk/target-mips/translate.c
> > ===================================================================
> > --- qemu-git-trunk.orig/target-mips/translate.c     2014-10-27 
> > 04:26:57.000000000 +0000
> > +++ qemu-git-trunk/target-mips/translate.c  2014-10-27 04:45:22.838923200 
> > +0000
> > @@ -13170,8 +13170,13 @@ static void decode_micromips32_opc (CPUM
> >              check_insn(ctx, ASE_MIPS3D);
> >              /* Fall through */
> >          do_cp1branch:
> > -            gen_compute_branch1(ctx, mips32_op,
> > -                                (ctx->opcode >> 18) & 0x7, imm << 1);
> > +            if (env->CP0_Config1 & (1 << CP0C1_FP)) {
> 
> I'm wondering if this test is needed at all, I would expect that
> check_cp1_enabled(ctx) is enough. Is it ever possible to have Status.CU1
> set to 1 if FPU isn't present? In translate_init.c the 5Kc CPU (and 5KEc
> you are introducing) is the only CPU without FPU that has Status.CU1
> writeable, which I'm not sure if it's correct. Probably the best way
> would be to check that on the real 5KEc which you seem to have handy :)

 Good point, CP0.Status.CU1 being writable on a non-FPU MIPS architecture 
(non-legacy) processor looks like a bug to me, I missed that detail.  
Here's what the original 5K manual[1] says:

"Coprocessor 1 and 2 can only be marked as usable if a
coprocessor is actually attached to the CPU.  For example,
if no coprocessor 2 is attached, software cannot set CU2.
Note that COP1X instructions are enabled by CU1.

"CU3 is unused by the processor but is implemented as a
read/write bit for backwards-compatibility.  This bit can
only be set to 1 if coprocessor 1 is attached to the CPU."

 I only have a bunch of 5KEf boards I'm afraid.  I could have a look at a 
5Kc sometime, but I don't have the system handy.  It may be easier for you 
to track down a 5K core card within Imagination somewhere.  I think the 
reference above should be enough though, so I'll cook up an update patch 
in the few next days, I think as an incremental one.

> Since this test is used also in other places in existing code and
> potential cleanup won't happen before 2.2 release due to incoming
> hard-freeze, this patch looks good to me.

 That sounds reasonable to me and thanks for the heads-up about the 
impending code freeze.  I think I'll post all the outstanding changes 
regardless to get the reviews rolling as the less obvious stuff may 
require further work and I'd like to get them integrated as soon as 2.3 
opens then.

 Now as to CP0.Status.CU1, while fixing the 5Kc and 5KEc processors is an 
obvious change, I think the removal of the extra check may not be such.  
The thing is in the original architecture -- and it still stands for CP2 
-- these bits used to control external coprocessors that may or may not 
have been present.  For example to have an R3000 processor with an FPU you 
wired an external R3010 unit to it.  Consequently all the CP0.Status.CUx 
bits were always writable and the relevant logic to intereact with the 
external chip enabled when requested.

 And there is software that relies on this property as prior to the 
introduction of the modern MIPS architecture there was no CP0.Config1 
register present to check the presence of an FPU with.  Instead what 
software was supposed to do was to enable CP1, execute a CFC instruction 
to read CP1.FIR that was supposed not to trap under these circumstances, 
and check the value obtained in the Imp field (as it was then called) aka 
ProcessorID.  If that was non-zero, an FPU was present, if it was zero 
(due to the floating bus being strapped IIUC), no FPU was available. [2] 
For example we have code in Linux doing this when run on the relevant 
hardware.

 Obviously COP1X instructions would trap on CP0.Status.CU3 instead and for 
example the FP emulator that we have in Linux is prepared for this 
situation (whether it should really emulate a full MIPS IV if not higher 
FP instruction set on a lower-ISA processor is another question).

 And I think all this draws the "right" implementation that we can make, 
across all the three coprocessors actually, e.g. for CP1, except stores 
and transfers from:

    check_cp1_enabled(ctx);
    if (ctx->CP0_Config1 & (1 << CP0C1_FP)) {
        handle_the_op();
    }

for stores and transfers from:

    check_cp1_enabled(ctx);
    if (ctx->CP0_Config1 & (1 << CP0C1_FP)) {
        write_destination(read(source));
    } else {
        write_destination(0);
    }

That is for an absent unit stores and transfers write 0 to their 
destination and all other instructions are treated as NOPs.

 For CP2 that we don't have support for our implementation would be dummy, 
omitting the conditional check along the following "then" part, and for 
CP3 the corresponding enable check would have to take ISA specifics into 
account.

 There's one further complication in that doubleword load/store major 
opcodes cause a reserved instruction exception instead on MIPS I 
processors.  Though actually I have a patch outstanding that handles this 
along overall tightening of instruction ISA level validity checks.

 Of course right now we don't have such legacy processors included in 
configuration, but the clean-up looks to me straightforward and little 
involving.  Actually including an R3000 processor with an FPU would be a 
bit trickier as they used a general interrupt line for exception delivery 
and we'd have to implement that; some that had the FPU coupled with the 
CPU on a single die, such as the R3081, even had it software-configurable, 
with a field in a CP0.Config register in the latter case (a different 
CP0.Config to what it is these days, at $3). [3]

 Then of course we'll have to make sure CP0.Status.CUx bits are hardwired 
to 0 where appropriate on modern MIPS architecture processors.

 Thanks for your review.

References:

[1] "MIPS64 5K Processor Core Family Software User's Manual",
     MIPS Technologies, Inc., Document Number: MD00012, Revision 02.08, 
     May 28, 2002

[2] "IDT MIPS Microprocessor Family Software Reference Manual",
    Integrated Device Technology, Inc., Version 2.0, October 1996

[3] "IDT R30xx Family Software Reference Manual",
    Integrated Device Technology, Inc., Revision 1.0, 1994

  Maciej