qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH 3/9] target-mips: Decode EVA load & store instru


From: James Hogan
Subject: Re: [Qemu-devel] [PATCH 3/9] target-mips: Decode EVA load & store instructions
Date: Fri, 7 Oct 2016 16:48:34 +0100
User-agent: Mutt/1.5.24 (2015-08-30)

On Fri, Oct 07, 2016 at 04:34:27PM +0100, Yongbok Kim wrote:
> 
> 
> On 06/09/2016 12:03, James Hogan wrote:
> > Implement decoding of EVA loads and stores. These access the user
> > address space from kernel mode when implemented, so for each instruction
> > we need to check that EVA is available from Config5.EVA & check for
> > sufficient COP0 privelege (with the new check_eva()), and then override
> 
> privilege

Good spot, thanks

> 
> > the mem_idx used for the operation.
> > 
> > Unfortunately some Loongson 2E instructions use overlapping encodings,
> > so we must be careful not to prevent those from being decoded when EVA
> > is absent.
> > 
> > Signed-off-by: James Hogan <address@hidden>
> > Cc: Leon Alrae <address@hidden>
> > Cc: Aurelien Jarno <address@hidden>
> > ---
> >  target-mips/translate.c | 106 +++++++++++++++++++++++++++++++++++++++++-
> >  1 file changed, 106 insertions(+), 0 deletions(-)
> > 
> > diff --git a/target-mips/translate.c b/target-mips/translate.c
> > index df2befbd5294..8506c39a359c 100644
> > --- a/target-mips/translate.c
> > +++ b/target-mips/translate.c
> > @@ -426,6 +426,24 @@ enum {
> >      OPC_EXTR_W_DSP     = 0x38 | OPC_SPECIAL3,
> >      OPC_DEXTR_W_DSP    = 0x3C | OPC_SPECIAL3,
> >  
> > +    /* EVA */
> > +    OPC_LWLE           = 0x19 | OPC_SPECIAL3,
> > +    OPC_LWRE           = 0x1A | OPC_SPECIAL3,
> > +    OPC_CACHEE         = 0x1B | OPC_SPECIAL3,
> > +    OPC_SBE            = 0x1C | OPC_SPECIAL3,
> > +    OPC_SHE            = 0x1D | OPC_SPECIAL3,
> > +    OPC_SCE            = 0x1E | OPC_SPECIAL3,
> > +    OPC_SWE            = 0x1F | OPC_SPECIAL3,
> > +    OPC_SWLE           = 0x21 | OPC_SPECIAL3,
> > +    OPC_SWRE           = 0x22 | OPC_SPECIAL3,
> > +    OPC_PREFE          = 0x23 | OPC_SPECIAL3,
> > +    OPC_LBUE           = 0x28 | OPC_SPECIAL3,
> > +    OPC_LHUE           = 0x29 | OPC_SPECIAL3,
> > +    OPC_LBE            = 0x2C | OPC_SPECIAL3,
> > +    OPC_LHE            = 0x2D | OPC_SPECIAL3,
> > +    OPC_LLE            = 0x2E | OPC_SPECIAL3,
> > +    OPC_LWE            = 0x2F | OPC_SPECIAL3,
> > +
> 
> EVA for MIPS32 only. It is OK but it's worth mentioning it in the commit
> messages.

Are you referring to the lack of sde/lde instructions? My understanding
is that these were never defined since EVA is aimed at 32-bit code
(although segmentation control can still be implemented on a MIPS64
core, e.g. P6600).

> 
> >      /* R6 */
> >      R6_OPC_PREF        = 0x35 | OPC_SPECIAL3,
> >      R6_OPC_CACHE       = 0x25 | OPC_SPECIAL3,
> > @@ -1430,6 +1448,7 @@ typedef struct DisasContext {
> >      bool bp;
> >      uint64_t PAMask;
> >      bool mvh;
> > +    bool eva;
> >      int CP0_LLAddr_shift;
> >      bool ps;
> >      bool vp;
> > @@ -2215,29 +2234,47 @@ static void gen_ld(DisasContext *ctx, uint32_t opc,
> >          tcg_gen_qemu_ld_tl(t0, t0, mem_idx, MO_TESL);
> >          gen_store_gpr(t0, rt);
> >          break;
> > +    case OPC_LWE:
> > +        mem_idx = MIPS_HFLAG_UM;
> > +        /* fall through */
> >      case OPC_LW:
> >          tcg_gen_qemu_ld_tl(t0, t0, mem_idx, MO_TESL |
> >                             ctx->default_tcg_memop_mask);
> >          gen_store_gpr(t0, rt);
> >          break;
> > +    case OPC_LHE:
> > +        mem_idx = MIPS_HFLAG_UM;
> > +        /* fall through */
> >      case OPC_LH:
> >          tcg_gen_qemu_ld_tl(t0, t0, mem_idx, MO_TESW |
> >                             ctx->default_tcg_memop_mask);
> >          gen_store_gpr(t0, rt);
> >          break;
> > +    case OPC_LHUE:
> > +        mem_idx = MIPS_HFLAG_UM;
> > +        /* fall through */
> >      case OPC_LHU:
> >          tcg_gen_qemu_ld_tl(t0, t0, mem_idx, MO_TEUW |
> >                             ctx->default_tcg_memop_mask);
> >          gen_store_gpr(t0, rt);
> >          break;
> > +    case OPC_LBE:
> > +        mem_idx = MIPS_HFLAG_UM;
> > +        /* fall through */
> >      case OPC_LB:
> >          tcg_gen_qemu_ld_tl(t0, t0, mem_idx, MO_SB);
> >          gen_store_gpr(t0, rt);
> >          break;
> > +    case OPC_LBUE:
> > +        mem_idx = MIPS_HFLAG_UM;
> > +        /* fall through */
> >      case OPC_LBU:
> >          tcg_gen_qemu_ld_tl(t0, t0, mem_idx, MO_UB);
> >          gen_store_gpr(t0, rt);
> >          break;
> > +    case OPC_LWLE:
> > +        mem_idx = MIPS_HFLAG_UM;
> > +        /* fall through */
> >      case OPC_LWL:
> >          t1 = tcg_temp_new();
> >          /* Do a byte access to possibly trigger a page
> > @@ -2261,6 +2298,9 @@ static void gen_ld(DisasContext *ctx, uint32_t opc,
> >          tcg_gen_ext32s_tl(t0, t0);
> >          gen_store_gpr(t0, rt);
> >          break;
> > +    case OPC_LWRE:
> > +        mem_idx = MIPS_HFLAG_UM;
> > +        /* fall through */
> >      case OPC_LWR:
> >          t1 = tcg_temp_new();
> >          /* Do a byte access to possibly trigger a page
> > @@ -2285,6 +2325,9 @@ static void gen_ld(DisasContext *ctx, uint32_t opc,
> >          tcg_gen_ext32s_tl(t0, t0);
> >          gen_store_gpr(t0, rt);
> >          break;
> > +    case OPC_LLE:
> > +        mem_idx = MIPS_HFLAG_UM;
> > +        /* fall through */
> >      case OPC_LL:
> >      case R6_OPC_LL:
> >          op_ld_ll(t0, t0, mem_idx, ctx);
> > @@ -2317,20 +2360,35 @@ static void gen_st (DisasContext *ctx, uint32_t 
> > opc, int rt,
> >          gen_helper_0e2i(sdr, t1, t0, mem_idx);
> >          break;
> >  #endif
> > +    case OPC_SWE:
> > +        mem_idx = MIPS_HFLAG_UM;
> > +        /* fall through */
> >      case OPC_SW:
> >          tcg_gen_qemu_st_tl(t1, t0, mem_idx, MO_TEUL |
> >                             ctx->default_tcg_memop_mask);
> >          break;
> > +    case OPC_SHE:
> > +        mem_idx = MIPS_HFLAG_UM;
> > +        /* fall through */
> >      case OPC_SH:
> >          tcg_gen_qemu_st_tl(t1, t0, mem_idx, MO_TEUW |
> >                             ctx->default_tcg_memop_mask);
> >          break;
> > +    case OPC_SBE:
> > +        mem_idx = MIPS_HFLAG_UM;
> > +        /* fall through */
> >      case OPC_SB:
> >          tcg_gen_qemu_st_tl(t1, t0, mem_idx, MO_8);
> >          break;
> > +    case OPC_SWLE:
> > +        mem_idx = MIPS_HFLAG_UM;
> > +        /* fall through */
> >      case OPC_SWL:
> >          gen_helper_0e2i(swl, t1, t0, mem_idx);
> >          break;
> > +    case OPC_SWRE:
> > +        mem_idx = MIPS_HFLAG_UM;
> > +        /* fall through */
> >      case OPC_SWR:
> >          gen_helper_0e2i(swr, t1, t0, mem_idx);
> >          break;
> > @@ -2363,6 +2421,9 @@ static void gen_st_cond (DisasContext *ctx, uint32_t 
> > opc, int rt,
> >          op_st_scd(t1, t0, rt, mem_idx, ctx);
> >          break;
> >  #endif
> > +    case OPC_SCE:
> > +        mem_idx = MIPS_HFLAG_UM;
> > +        /* fall through */
> >      case OPC_SC:
> >      case R6_OPC_SC:
> >          op_st_sc(t1, t0, rt, mem_idx, ctx);
> > @@ -17981,13 +18042,57 @@ static void decode_opc_special3(CPUMIPSState 
> > *env, DisasContext *ctx)
> >  {
> >      int rs, rt, rd, sa;
> >      uint32_t op1, op2;
> > +    int16_t imm;
> >  
> >      rs = (ctx->opcode >> 21) & 0x1f;
> >      rt = (ctx->opcode >> 16) & 0x1f;
> >      rd = (ctx->opcode >> 11) & 0x1f;
> >      sa = (ctx->opcode >> 6) & 0x1f;
> > +    imm = (int16_t)ctx->opcode >> 7;
> 
> imm = (int16_t) (ctx->opcode >> 7) & 0x1ff;

That won't sign extend it correctly.

> 
> >  
> >      op1 = MASK_SPECIAL3(ctx->opcode);
> > +
> > +    /*
> > +     * EVA loads and stores overlap Loongson 2E instructions decoded by
> > +     * decode_opc_special3_legacy(), so be careful to allow their decoding 
> > when
> > +     * EVA is absent.
> > +     */
> > +    if (ctx->eva) {
> > +        switch (op1) {
> > +        case OPC_LWLE ... OPC_LWRE:
> > +            check_insn_opc_removed(ctx, ISA_MIPS32R6);
> > +            /* fall through */
> > +        case OPC_LBUE ... OPC_LHUE:
> > +        case OPC_LBE ... OPC_LWE:
> > +            check_cp0_enabled(ctx);
> > +            gen_ld(ctx, op1, rt, rs, imm);
> > +            return;
> > +        case OPC_SWLE ... OPC_SWRE:
> > +            check_insn_opc_removed(ctx, ISA_MIPS32R6);
> > +            /* fall through */
> > +        case OPC_SBE ... OPC_SHE:
> > +        case OPC_SWE:
> > +            check_cp0_enabled(ctx);
> > +            gen_st(ctx, op1, rt, rs, imm);
> > +            return;
> > +        case OPC_SCE:
> > +            check_cp0_enabled(ctx);
> > +            gen_st_cond(ctx, op1, rt, rs, imm);
> > +            return;
> > +        case OPC_CACHEE:
> > +            check_cp0_enabled(ctx);
> > +            if (ctx->hflags & MIPS_HFLAG_ITC_CACHE) {
> > +                gen_cache_operation(ctx, rt, rs, imm);
> > +            }
> > +            /* Treat as NOP. */
> 
> this comment is not relevant any more.

Well its relevant in the sense that the CACHEE operation doesn't
actually do anything of any significance. It won't even trigger a TLB
exception (which technically it should).

I can drop it though if you prefer.

> 
> > +            return;
> > +        case OPC_PREFE:
> > +            check_cp0_enabled(ctx);
> > +            /* Treat as NOP. */
> > +            return;
> > +        }
> > +    }
> > +
> >      switch (op1) {
> >      case OPC_EXT:
> >      case OPC_INS:
> > @@ -19883,6 +19988,7 @@ void gen_intermediate_code(CPUMIPSState *env, 
> > struct TranslationBlock *tb)
> >      ctx.bp = (env->CP0_Config3 >> CP0C3_BP) & 1;
> >      ctx.PAMask = env->PAMask;
> >      ctx.mvh = (env->CP0_Config5 >> CP0C5_MVH) & 1;
> > +    ctx.eva = (env->CP0_Config5 >> CP0C5_EVA) & 1;
> >      ctx.CP0_LLAddr_shift = env->CP0_LLAddr_shift;
> >      ctx.cmgcr = (env->CP0_Config3 >> CP0C3_CMGCR) & 1;
> >      /* Restore delay slot state from the tb context.  */
> > 
> 
> Regards,
> Yongbok

Thanks for reviewing,

Cheers
James

Attachment: signature.asc
Description: Digital signature


reply via email to

[Prev in Thread] Current Thread [Next in Thread]