Re: [Qemu-devel] [PATCH] target-mips: Enable access to required RDHWR hardware registers

[Meador Inge <meadori@codesourcery.com>]

On 08/21/2012 05:14 AM, Andreas Färber wrote:

> Am 21.08.2012 01:41, schrieb Meador Inge:
>> While running in the usermode emulator all of the MIPS32r2 *required*
>> RDHWR hardware registers should be accessible (the Linux kernel enables
>> access to these same registers).
>>
>> Signed-off-by: Meador Inge <meadori@codesourcery.com>
>> ---
>>  target-mips/translate.c |    7 +++++--
>>  1 files changed, 5 insertions(+), 2 deletions(-)
>>
>> diff --git a/target-mips/translate.c b/target-mips/translate.c
>> index 47daf85..849e75d 100644
>> --- a/target-mips/translate.c
>> +++ b/target-mips/translate.c
>> @@ -12768,8 +12768,11 @@ void cpu_state_reset(CPUMIPSState *env)
>>  
>>  #if defined(CONFIG_USER_ONLY)
>>      env->hflags = MIPS_HFLAG_UM;
>> -    /* Enable access to the SYNCI_Step register.  */
>> -    env->CP0_HWREna |= (1 << 1);
>> +    if (env->insn_flags & ISA_MIPS32R2) {
>> +        /* Enable access to the CPUNum, SYNCI_Step, CC, and CCRes RDHWR
>> +           hardware registers.  */
>> +        env->CP0_HWREna |= 0x0000000F;
>> +    }
> 
> So what about the non-MIPS32r2 case? IIUC then the SYNCI_Step register
> would no longer be accessible, which your commit message does not
> mention. Intentional?

Yes, that is intentional.  In Section 9.13 of [1] it is stated that these
registers are only required starting at release 2 of the architecture.  The
Linux kernel follows the same approach.

I guess I should just split this into two minor patches: (1) for enabling all
the registers and (2) for making them conditional on R2.

[1] MIPS® Architecture For Programmers,
    Volume III: The MIPS32 (R) and microMIPS32TM Privileged Resource
    Architecture
    Revision 3.12

> If this is a bugfix for v1.2.0 don't forget to mark "for-1.2".

Will do.

Thanks for the review.

-- 
Meador Inge
CodeSourcery / Mentor Embedded
http://www.mentor.com/embedded-software