Re: [Qemu-devel] [PATCH v2 7/7] target/mips: Implement emulation of nanoMIPS EVA instructions

["Philippe Mathieu-Daudé" <philmd@redhat.com>]

Hi Aleksandar,

On 05/10/2018 17:19, Aleksandar Markovic wrote:
> From: Dimitrije Nikolic <dnikolic@wavecomp.com>
> 
> Implement emulation of nanoMIPS EVA instructions. They are all
> part of P.LS.E0 instruction pool, or one of its subpools.
> 

Shouldn't this be signed off by Dimitrije Nikolic too?

> Signed-off-by: Aleksandar Markovic <amarkovic@wavecomp.com>
> ---
>  target/mips/translate.c | 79 +++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 79 insertions(+)
> 
> diff --git a/target/mips/translate.c b/target/mips/translate.c
> index b0b2f40..3adf31f 100644
> --- a/target/mips/translate.c
> +++ b/target/mips/translate.c
> @@ -1979,6 +1979,17 @@ static inline void check_nms(DisasContext *ctx)
>      }
>  }
>  
> +/*
> + * This code generates a "reserved instruction" exception if the
> + * Config5 EVA bit is NOT set.
> + */
> +static inline void check_eva(DisasContext *ctx)
> +{
> +    if (!unlikely(ctx->CP0_Config5 & (1 << CP0C5_EVA))) {
> +        generate_exception_end(ctx, EXCP_RI);
> +    }
> +}
> +
>  
>  /* Define small wrappers for gen_load_fpr* so that we have a uniform
>     calling interface for 32 and 64-bit FPRs.  No sense in changing
> @@ -20011,6 +20022,74 @@ static int decode_nanomips_32_48_opc(CPUMIPSState *env, DisasContext *ctx)
>                      break;
>                  }
>                  break;
> +            case NM_P_LS_E0:
> +                check_eva(ctx);
> +                switch (extract32(ctx->opcode, 11, 4)) {
> +                case NM_LBE:
> +                    gen_ld(ctx, OPC_LBE, rt, rs, s);
> +                    break;
> +                case NM_SBE:
> +                    gen_st(ctx, OPC_SBE, rt, rs, s);
> +                    break;
> +                case NM_LBUE:
> +                    gen_ld(ctx, OPC_LBUE, rt, rs, s);
> +                    break;
> +                case NM_P_PREFE:
> +                    if (rt == 31) {
> +                        /* SYNCIE */
> +                        /* Break the TB to be able to sync copied instructions
> +                           immediately */
> +                        ctx->base.is_jmp = DISAS_STOP;
> +                    } else {
> +                        /* PREF */
> +                        /* Treat as NOP. */
> +                    }
> +                    break;
> +                case NM_LHE:
> +                    gen_ld(ctx, OPC_LHE, rt, rs, s);
> +                    break;
> +                case NM_SHE:
> +                    gen_st(ctx, OPC_SHE, rt, rs, s);
> +                    break;
> +                case NM_LHUE:
> +                    gen_ld(ctx, OPC_LHUE, rt, rs, s);
> +                    break;
> +                case NM_CACHEE:
> +                    /* Treat as no-op */

What about NMS core without caches? Shouldn't we use:

                       check_nms(ctx);

> +                    if (ctx->hflags & MIPS_HFLAG_ITC_CACHE) {
> +                        gen_cache_operation(ctx, rt, rs, s);
> +                    }
> +                    break;
> +                case NM_LWE:
> +                    gen_ld(ctx, OPC_LWE, rt, rs, s);
> +                    break;
> +                case NM_SWE:
> +                    gen_st(ctx, OPC_SWE, rt, rs, s);
> +                    break;
> +                case NM_P_LLE:
> +                    switch (extract32(ctx->opcode, 2, 2)) {
> +                    case NM_LL:
> +                        gen_ld(ctx, OPC_LLE, rt, rs, s);
> +                        break;
> +                    case NM_LLWP:
> +                    default:
> +                        generate_exception_end(ctx, EXCP_RI);
> +                        break;
> +                    }
> +                    break;
> +                case NM_P_SCE:
> +                    switch (extract32(ctx->opcode, 2, 2)) {
> +                    case NM_SC:
> +                        gen_st_cond(ctx, OPC_SCE, rt, rs, s);
> +                        break;
> +                    case NM_SCWP:
> +                    default:
> +                        generate_exception_end(ctx, EXCP_RI);
> +                        break;
> +                    }
> +                    break;
> +                }
> +                break;
>              case NM_P_LS_WM:
>              case NM_P_LS_UAWM:
>                  check_nms(ctx);
> 

The rest of this patch is OK.

Regards,

Phil.