From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:43501) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TOGSJ-0003wf-Dm for qemu-devel@nongnu.org; Tue, 16 Oct 2012 19:21:48 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1TOGSH-0004Kz-Ud for qemu-devel@nongnu.org; Tue, 16 Oct 2012 19:21:47 -0400 Received: from hall.aurel32.net ([88.191.126.93]:52850) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TOGSH-0004K9-O6 for qemu-devel@nongnu.org; Tue, 16 Oct 2012 19:21:45 -0400 Date: Wed, 17 Oct 2012 01:21:43 +0200 From: Aurelien Jarno Message-ID: <20121016232143.GA18454@ohm.aurel32.net> References: <1350319158-7263-1-git-send-email-proljc@gmail.com> <1350319158-7263-6-git-send-email-proljc@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-15 Content-Disposition: inline In-Reply-To: <1350319158-7263-6-git-send-email-proljc@gmail.com> Subject: Re: [Qemu-devel] [PATCH v11 05/14] target-mips: Add ASE DSP load instructions List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Jia Liu Cc: qemu-devel@nongnu.org On Tue, Oct 16, 2012 at 12:39:09AM +0800, Jia Liu wrote: > Add MIPS ASE DSP Load instructions. > > Signed-off-by: Jia Liu > --- > target-mips/translate.c | 89 +++++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 89 insertions(+) > > diff --git a/target-mips/translate.c b/target-mips/translate.c > index f1e5bb0..7f08700 100644 > --- a/target-mips/translate.c > +++ b/target-mips/translate.c > @@ -313,6 +313,9 @@ enum { > OPC_MODU_G_2E = 0x23 | OPC_SPECIAL3, > OPC_DMOD_G_2E = 0x26 | OPC_SPECIAL3, > OPC_DMODU_G_2E = 0x27 | OPC_SPECIAL3, > + > + /* MIPS DSP Load */ > + OPC_LX_DSP = 0x0A | OPC_SPECIAL3, > }; > > /* BSHFL opcodes */ > @@ -340,6 +343,17 @@ enum { > #endif > }; > > +#define MASK_LX(op) (MASK_SPECIAL3(op) | (op & (0x1F << 6))) > +/* MIPS DSP Load */ > +enum { > + OPC_LBUX = (0x06 << 6) | OPC_LX_DSP, > + OPC_LHX = (0x04 << 6) | OPC_LX_DSP, > + OPC_LWX = (0x00 << 6) | OPC_LX_DSP, > +#if defined(TARGET_MIPS64) > + OPC_LDX = (0x08 << 6) | OPC_LX_DSP, > +#endif > +}; > + > /* Coprocessor 0 (rs field) */ > #define MASK_CP0(op) MASK_OP_MAJOR(op) | (op & (0x1F << 21)) > > @@ -12213,6 +12227,64 @@ static int decode_micromips_opc (CPUMIPSState *env, DisasContext *ctx, int *is_b > > #endif > > +/* MIPSDSP functions. */ > +static void gen_mipsdsp_ld(CPUMIPSState *env, DisasContext *ctx, uint32_t opc, > + int rd, int base, int offset) > +{ > + const char *opn = "ldx"; > + TCGv t0 = tcg_temp_new(); As the function can exit if rd == 0, this will create a temp leak, which can be used by an attacker to crash QEMU. The tcg_temp_new() part should be moved after the if > + if (rd == 0 && env->insn_flags & (ASE_DSP | ASE_DSPR2)) { > + MIPS_DEBUG("NOP"); > + return; I still don't get the second part of the if testing the insn_flags. It should be dropped. > + } else if (base == 0) { > + if (offset == 0) { > + /* Address error. */ > + generate_exception(ctx, EXCP_AdEL); I don't think this is correct. > + } else { > + gen_load_gpr(t0, offset); Also gen_load_gpr() already handle the case offset == 0 > + } > + } else if (offset == 0) { > + gen_load_gpr(t0, base); > + } else { > + gen_op_addr_add(ctx, t0, cpu_gpr[base], cpu_gpr[offset]); > + save_cpu_state(ctx, 0); > + } save_cpu_state() should not be conditionnal. > + check_dsp(ctx); Please move that higher in the function. > + switch (opc) { > + case OPC_LBUX: > + op_ld_lbu(t0, t0, ctx); > + gen_store_gpr(t0, rd); > + opn = "lbux"; > + break; > + case OPC_LHX: > + op_ld_lh(t0, t0, ctx); > + gen_store_gpr(t0, rd); > + opn = "lhx"; > + break; > + case OPC_LWX: > + op_ld_lw(t0, t0, ctx); > + gen_store_gpr(t0, rd); > + opn = "lwx"; > + break; > +#if defined(TARGET_MIPS64) > + case OPC_LDX: > + op_ld_ld(t0, t0, ctx); > + gen_store_gpr(t0, rd); > + opn = "ldx"; > + break; > +#endif > + } > + (void)opn; /* avoid a compiler warning */ > + MIPS_DEBUG("%s %s, %s(%s)", opn, > + regnames[rd], regnames[offset], regnames[base]); > + tcg_temp_free(t0); > +} > + > + > +/* End MIPSDSP functions. */ > + > static void decode_opc (CPUMIPSState *env, DisasContext *ctx, int *is_branch) > { > int32_t offset; > @@ -12569,6 +12641,23 @@ static void decode_opc (CPUMIPSState *env, DisasContext *ctx, int *is_branch) > check_insn(env, ctx, INSN_LOONGSON2E); > gen_loongson_integer(ctx, op1, rd, rs, rt); > break; > + case OPC_LX_DSP: > + op2 = MASK_LX(ctx->opcode); > + switch (op2) { > +#if defined(TARGET_MIPS64) > + case OPC_LDX: > +#endif > + case OPC_LBUX: > + case OPC_LHX: > + case OPC_LWX: > + gen_mipsdsp_ld(env, ctx, op2, rd, rs, rt); > + break; > + default: /* Invalid */ > + MIPS_INVAL("MASK LX"); > + generate_exception(ctx, EXCP_RI); > + break; > + } > + break; > #if defined(TARGET_MIPS64) > case OPC_DEXTM ... OPC_DEXT: > case OPC_DINSM ... OPC_DINS: > -- > 1.7.10.2 (Apple Git-33) > > -- Aurelien Jarno GPG: 1024D/F1BCDB73 aurelien@aurel32.net http://www.aurel32.net