[Qemu-devel] Correction needed for R5900 instruction decoding

[Aleksandar Markovic]

Hi, Fridrik,

I did some closer code inspection of R5900 in last few days, and I noticed some 
sub-optimal implementation in the area where R5900-specific opcodes overlap 
with the rest-of-MIPS-CPUs opcodes.

The right implementation should be based on the principle that all such cases 
are covered with if statements involving INSN_R5900 flag, like this:

        if (ctx->insn_flags & INSN_R5900) {
            <R5900-specific handling>
        } else {
            <rest-of-MIPS-handling>
        }

You followed that principle for OPC_SPECIAL2 and OPC_SPECIAL3, but for some 
other opcodes not. For example, there are lines:

    if (reg == 0 && (opc == OPC_MFHI || opc == TX79_MMI_MFHI1 ||
                     opc == OPC_MFLO || opc == TX79_MMI_MFLO1)) {

or

     switch (opc) {
     case OPC_MFHI:
     case TX79_MMI_MFHI1:

Such implementation makes it difficult to discern R5900 and non-R5900 cases. 
Potentialy allows bugs to sneak in and affect non-R5900 support.

The correction is not that difficult, I gather. Worse comme to worst, you can 
remove R5900 MFLO1 and MFHI1 altogether, they are not that essential at this 
moment, but do try correcting the decoding stuff as I described. Can you please 
make these changes in next few days or so (given that 3.1 release is getting 
closer and closer), and send them to the list?

It is my bad that I didn't spot this during review, but in any case, I think 
this should be fixed in 3.1 to make sure that non-R5900 functionalities are 
intact.

Thanks,
Aleksandar