On Thu, Dec 29, 2011 at 4:17 PM, Andreas Färber <andreas.faer...@web.de> wrote: > Am 29.12.2011 08:55, schrieb Khansa Butt: >> On Fri, Dec 9, 2011 at 5:04 AM, Andreas Färber <andreas.faer...@web.de> >> wrote: >>>> + /* if cpu has FPU, MIPS_HFLAG_F64 must be included in env->hflags >>>> + so that floating point operations can be emulated */ >>>> + env->active_fpu.fcr0 = env->cpu_model->CP1_fcr0; >>>> if (env->active_fpu.fcr0 & (1 << FCR0_F64)) { >>>> env->hflags |= MIPS_HFLAG_F64; >>>> } >>> >>> Nack. env->active_fpu.fcr0 gets initialized in translate_init.c based on >>> 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 = >> 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 cpu_model->CR1_fcr0 was , the value of >> env->active_fpu.fcr0 will be zero now thats 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