From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:50967) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RgXGv-00021c-B8 for qemu-devel@nongnu.org; Fri, 30 Dec 2011 02:53:02 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1RgXGs-0007ZO-HV for qemu-devel@nongnu.org; Fri, 30 Dec 2011 02:53:01 -0500 Received: from mail-wi0-f173.google.com ([209.85.212.173]:35921) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RgXGs-0007Yz-DI for qemu-devel@nongnu.org; Fri, 30 Dec 2011 02:52:58 -0500 Received: by wibhm2 with SMTP id hm2so7746277wib.4 for ; Thu, 29 Dec 2011 23:52:56 -0800 (PST) MIME-Version: 1.0 In-Reply-To: References: <4EFC4C65.5090503@web.de> Date: Fri, 30 Dec 2011 12:52:56 +0500 Message-ID: From: Khansa Butt Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable Subject: [Qemu-devel] [PATCH 2/3] target-mips:enabling of 64 bit user mode and floating point operations MIPS_HFLAG_UX is included in env->hflags so that the address computation for LD instruction does not treated as 32 bit code see gen_op_addr_add() in t List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: qemu-devel@nongnu.org On Thu, Dec 29, 2011 at 4:17 PM, Andreas F=E4rber = wrote: > Am 29.12.2011 08:55, schrieb Khansa Butt: >> On Fri, Dec 9, 2011 at 5:04 AM, Andreas F=E4rber wrote: >>>> + =A0 =A0/* if cpu has FPU, MIPS_HFLAG_F64 must be included in env->hf= lags >>>> + =A0 =A0 =A0 so that floating point operations can be emulated */ >>>> + =A0 =A0env->active_fpu.fcr0 =3D env->cpu_model->CP1_fcr0; >>>> =A0 =A0 =A0if (env->active_fpu.fcr0 & (1 << FCR0_F64)) { >>>> =A0 =A0 =A0 =A0 =A0env->hflags |=3D MIPS_HFLAG_F64; >>>> =A0 =A0 =A0} >>> >>> Nack. env->active_fpu.fcr0 gets initialized in translate_init.c based o= n >>> cpu_model->CR1_fcr0, where FCR0_F64 is set only for 24Kf, 34Kf, >>> MIPS64R2-generic. TARGET_ABI_MIPSN64 linux-user defaults to 20Kc. So it >>> seems to rather be an issue of using the right -cpu parameter or >>> changing the default for n64. [cc'ing Nathan, who introduced the if] >> >> The reason why I add this line " env->active_fpu.fcr0 =3D >> env->cpu_model->CP1_fcr0" is as follows >> in translate_init.c fpu_init() initializes active_fpu for given cpu >> model afterwards cpu_reset() reset the values >> to zero using this >> memset(env, 0, offsetof(CPUMIPSState, breakpoints)); >> so whatever the value of =A0cpu_model->CR1_fcr0 was , the value of >> env->active_fpu.fcr0 will be zero now =A0thats why I add above >> line to retrieve the correct env->active_fpu.fcr0 value according to >> CPU model( whether it is 24Kf or 20Kc or something else) >> During the development of mips64-linux-user I observed this issue. I >> gave qemu-mips64 command with -cpu option equal to MIPS64R2-generic >> and an illegal instruction error occurred, so I used above hunk. > > Well, that sounds like a different and more generic problem that > shouldn't be fixed inside CONFIG_USER_ONLY && TARGET_MIPS64. > And your reasoning should've definitely been in the commit message! > > The problem here is not whether the patches observably work for you but > whether it's the correct way to fix it. "We did this because it works > for us" is never a convincing justification of a change. > If it doesn't work for you in linux-user it won't work in softmmu > either, so doing that before #if defined(CONFIG_USER_ONLY) where lots of > env->cpu_model stuff is being copyied (esp. before env->HABITS to honor > mips_def_t order) seems better. > > Also, given your observation, does it even make sense for > cpu_mips_init() to call fpu_init() when all CPUState members it > initializes get cleared in cpu_reset()? Maybe just move that call into > cpu_reset() and rename it to fpu_reset()? mmu_init() and mvp_init() seem > okay by contrast. why cpu_reset() calls memset? it does not reset all the members of CPUState= only those which are in the range of offsetof(CPUMIPSState, breakpoints). what if I remove memset line? > > When you've figured this out, please again put it into a separate patch > titled, e.g., "target-mips: Fix FPU reset" with appropriate explanation. > > Andreas