From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:33985) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Xgvqx-00010j-1N for qemu-devel@nongnu.org; Wed, 22 Oct 2014 09:21:33 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Xgvqp-0006HJ-Hi for qemu-devel@nongnu.org; Wed, 22 Oct 2014 09:21:26 -0400 Received: from mailapp01.imgtec.com ([195.59.15.196]:1413) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Xgvqp-0006HE-Bu for qemu-devel@nongnu.org; Wed, 22 Oct 2014 09:21:19 -0400 Message-ID: <5447AF4C.7070601@imgtec.com> Date: Wed, 22 Oct 2014 14:21:16 +0100 From: James Hogan MIME-Version: 1.0 References: <1405331763-57126-1-git-send-email-yongbok.kim@imgtec.com> <1405331763-57126-8-git-send-email-yongbok.kim@imgtec.com> In-Reply-To: <1405331763-57126-8-git-send-email-yongbok.kim@imgtec.com> Content-Type: text/plain; charset="ISO-8859-1" Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 07/20] target-mips: add msa_reset(), global msa register List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Yongbok Kim , qemu-devel@nongnu.org Cc: cristian.cuna@imgtec.com, leon.alrae@imgtec.com, aurelien@aurel32.net Hi, On 14/07/14 10:55, Yongbok Kim wrote: > +static const char * const msaregnames[] = { > + "w0.d0", "w0.d1", "w1.d0", "w1.d1", > + "w2.d0", "w2.d1", "w3.d0", "w3.d1", > + "w4.d0", "w4.d1", "w4.d0", "w4.d1", I think those last 2 should be w5.d0 and w5.d1 > +static inline int check_msa_access(CPUMIPSState *env, DisasContext *ctx, > + int wt, int ws, int wd) > +{ > + if (unlikely((ctx->hflags & MIPS_HFLAG_FPU) && > + !(ctx->hflags & MIPS_HFLAG_F64))) { > + generate_exception(ctx, EXCP_RI); > + return 0; > + } > + > + if (unlikely(!(ctx->hflags & MIPS_HFLAG_MSA))) { > + if (ctx->insn_flags & ASE_MSA) { > + generate_exception(ctx, EXCP_MSADIS); > + return 0; > + } else { > + generate_exception(ctx, EXCP_RI); > + return 0; > + } > + } > + > + if (env->active_msa.msair & MSAIR_WRP_BIT) { > + int curr_request = 0; > + if (wd != -1) { > + curr_request |= (1 << wd); > + } > + if (wt != -1) { > + curr_request |= (1 << wt); > + } > + if (ws != -1) { > + curr_request |= (1 << ws); > + } > + env->active_msa.msarequest = curr_request > + & (~env->active_msa.msaaccess | env->active_msa.msasave); > + if (unlikely(env->active_msa.msarequest != 0)) { Are you sure it's safe to access env here during code generation? How do you guarantee the values at translation time match the values at run time? > + generate_exception(ctx, EXCP_MSADIS); > + return 0; > + } > + } > + return 1; > +} newline between functions? > diff --git a/target-mips/translate_init.c b/target-mips/translate_init.c > index 29dc2ef..9e0f67b 100644 > --- a/target-mips/translate_init.c > +++ b/target-mips/translate_init.c > @@ -688,3 +688,48 @@ static void mvp_init (CPUMIPSState *env, const mips_def_t *def) > (0x0 << CP0MVPC1_PCX) | (0x0 << CP0MVPC1_PCP2) | > (0x1 << CP0MVPC1_PCP1); > } > + > +static void msa_reset(CPUMIPSState *env) > +{ > +#ifdef CONFIG_USER_ONLY > + /* MSA access enabled */ > + env->CP0_Config5 |= 1 << CP0C5_MSAEn; > + > + /* DSP and CP1 enabled, 64-bit FPRs */ > + env->CP0_Status |= (1 << CP0St_MX); > + env->hflags |= MIPS_HFLAG_DSP; why do you enable DSP? > + env->CP0_Status |= (1 << CP0St_CU1) | (1 << CP0St_FR); > + env->hflags |= MIPS_HFLAG_F64 | MIPS_HFLAG_COP1X; shouldn't that depend on the program being loaded, and whether it's built for fp32 or fp64? > +#endif > + > + /* Vector register partitioning not implemented */ > + env->active_msa.msair = 0; > + env->active_msa.msaaccess = 0xffffffff; the reset state is 0 according to the manual. Maybe this should depend on CONFIG_USER_ONLY. Cheers James