Re: [Qemu-devel] [PATCH 0/2] target/mips: Fix decoding mechanisms of R5900 M{F, T}{HI, LO}1 and DIV[U]1

[Aleksandar Markovic]

> From: Fredrik Noring <address@hidden>
>
> Subject: [PATCH 0/2] target/mips: Fix decoding mechanisms of R5900 
> M{F,T}{HI,LO}1 and DIV[U]1
> 
> This series amends the R5900 support with the following changes:
>
> ...

Hi, Fridrek,

The two patches look very good. Yes, there will be some code duplication now, 
but the overall MIPS code is safer, R5900 code better encapsulated - and you 
have more flexibility to tweak R5900 functionality.

For LL, SC, LLD and SCD instructions, there is a need to properly insulate 
their R5900 versions too, similar to this:

    case OPC_SC:
        if(ctx->insn_flags & INSN_R5900) {
             check_insn_opc_user_only(ctx, INSN_R5900);
        } else {
            check_insn(ctx, ISA_MIPS2);
        }
        gen_st_cond(ctx, op, rt, rs, imm);
        break;

(the code above is just a form of pseudocode illustrating the idea; I don't 
guarantee the correctness for build purposes, or if this is the best code 
organization)

Non-R5900 code (for the time being) should never invoke 
check_insn_opc_user_only(). *The only way* of distinguishing R5900 code paths 
from the other CPUs code paths should be by using  "if(ctx->insn_flags & 
INSN_R5900)"!

For changes in decode_opc_special_legacy(), there shouldn't be there, but there 
should be a separate function decode_opc_special_tx59() or so.

Thanks,
Aleksandar