| Message ID | 1345506102-8444-1-git-send-email-meadori@codesourcery.com |
|---|---|
| State | New |
| Headers | show |
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; > } >
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.
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.
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
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; }
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(-)