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

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

[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;
+    }
     if (env->CP0_Config1 & (1 << CP0C1_FP)) {
         env->hflags |= MIPS_HFLAG_FPU;
     }
-- 
1.7.7.6

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

[Andreas Färber]

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?

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

Andreas

>      if (env->CP0_Config1 & (1 << CP0C1_FP)) {
>          env->hflags |= MIPS_HFLAG_FPU;
>      }
> 


-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

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

[Meador Inge]

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

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

[Aurelien Jarno]

Le 21/08/2012 17:41, Meador Inge a écrit :
> 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.

Remember this is linux user mode, as such you should not look at the
MIPS architecture, but rather at the behavour of MIPS architecture +
Linux kernel. SYNCI_Step is emulated by the Linux kernel for non R2
CPUs, as such it should be  available even for non R2 CPUs *in user mode
only*. At a first glance, it seems to be the same for CPUNum, CC and
CCRes, but someone has to check that more in details.

-- 
Aurelien Jarno                          GPG: 1024D/F1BCDB73
aurelien@aurel32.net                 http://www.aurel32.net

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

[Meador Inge]

On 08/21/2012 10:52 AM, Aurelien Jarno wrote:

> Le 21/08/2012 17:41, Meador Inge a écrit :
>> On 08/21/2012 05:14 AM, Andreas Färber wrote:
>>> 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.
> 
> Remember this is linux user mode, as such you should not look at the
> MIPS architecture, but rather at the behavour of MIPS architecture +
> Linux kernel. SYNCI_Step is emulated by the Linux kernel for non R2
> CPUs, as such it should be  available even for non R2 CPUs *in user mode
> only*. At a first glance, it seems to be the same for CPUNum, CC and
> CCRes, but someone has to check that more in details.

Ah, thanks Aurelien.  I see that in the kernel sources now.  Specifically,
it seems that 'simulate_rdhwr' handles the emulation [1] and that it includes
support for CPUNum, SYNCI_Step, CC, CCRes, and ULR (User Local register).  So,
I will remove the conditional.

I am not quite sure how to handle ULR at the moment.

[1]
http://git.kernel.org/?p=linux/kernel/git/torvalds/linux.git;a=blob;f=arch/mips/kernel/traps.c;h=9be3df1fa8a461dc4e1e5563582f483e4ed7c103;hb=HEAD

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