Re: [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

[Khansa Butt <khansa@kics.edu.pk>]

On Fri, Dec 9, 2011 at 5:04 AM, Andreas Färber <andreas.faerber@web.de> wrote:
> Thanks for extending the commit description. Please see this for a
> template though:
>
> http://live.gnome.org/Git/CommitMessages
>
> Looks like there's an empty line missing between subject and description
> (and the space after "target-mips:").
>
> Am 08.12.2011 06:25, schrieb khansa@kics.edu.pk:
>> From: Khansa Butt <khansa@kics.edu.pk>
>>
>>
>> Signed-off-by: Abdul Qadeer <qadeer@kics.edu.pk>
>> ---
>>  target-mips/translate.c |    4 ++++
>>  1 files changed, 4 insertions(+), 0 deletions(-)
>>
>> diff --git a/target-mips/translate.c b/target-mips/translate.c
>> index d5b1c76..452a63b 100644
>> --- a/target-mips/translate.c
>> +++ b/target-mips/translate.c
>> @@ -12779,6 +12779,10 @@ void cpu_reset (CPUMIPSState *env)
>>          env->hflags |= MIPS_HFLAG_FPU;
>>      }
>>  #ifdef TARGET_MIPS64
>> +    env->hflags |=  MIPS_HFLAG_UX;
>
> So for those of us not knowing mips, it's defined as:
>
> #define MIPS_HFLAG_UX     0x00200 /* 64-bit user mode                 */
>
> The code above is inside CONFIG_USER_ONLY, so this looks right for n64
> but not for n32 ABI.
>
> If you put this into its own patch with a description of
>
> ---8<---
> target-mips: Enable 64 bit user mode for n64
>
> For user mode n64 ABI emulation, MIPS_HFLAG_UX is included in
> env->hflags so that the address computation for LD instruction does not
> get treated as 32 bit code, see gen_op_addr_add() in translate.c.
>
> Signed-off-by: Abdul Qadeer <qadeer@kics.edu.pk>
> Signed-off-by: (you)
> ---8<---
>
> and make it depend on TARGET_ABI_MIPSN64 then I will happily add my
> Acked-by.
>
>
>> +    /* 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.

>
> Andreas