* [Qemu-devel] [PATCH v1 1/1] mips: properly compute hflags and fcr0 on cpu reset
@ 2012-03-02 21:03 Meador Inge
2012-03-03 16:45 ` Andreas Färber
0 siblings, 1 reply; 3+ messages in thread
From: Meador Inge @ 2012-03-02 21:03 UTC (permalink / raw)
To: qemu-devel; +Cc: Maciej W. Rozycki, Nathan Froyd, aurelien
Currently 'cpu_reset' doesn't fully compute all of the needed
HFLAGs and fails to setup fcr0 after clearing the CPU state.
This can cause instruction exceptions. For example, using
'madd.d' on machines that should support it is kindly greeted
with:
qemu: uncaught target signal 4 (Illegal instruction) - core dumped
Illegal instruction (core dumped)
because fcr0 is bogus and MIPS_HFLAG_COP1X is not correcly set in hflags.
This is fixed by modifying 'cpu_reset' to use 'compute_hflags' and
initializing 'fcr0' from the current CPU model.
Signed-off-by: Maciej W. Rozycki <macro@codesourcery.com>
Signed-off-by: Nathan Froyd <froydnj@codesourcery.com>
Signed-off-by: Meador Inge <meadori@codesourcery.com>
---
target-mips/cpu.h | 49 +++++++++++++++++++++++++++++++++++++++++++++++
target-mips/op_helper.c | 49 -----------------------------------------------
target-mips/translate.c | 17 +++------------
3 files changed, 53 insertions(+), 62 deletions(-)
diff --git a/target-mips/cpu.h b/target-mips/cpu.h
index 71cb4e8..fc65348 100644
--- a/target-mips/cpu.h
+++ b/target-mips/cpu.h
@@ -737,4 +737,53 @@ static inline void cpu_pc_from_tb(CPUState *env, TranslationBlock *tb)
env->hflags |= tb->flags & MIPS_HFLAG_BMASK;
}
+static inline void compute_hflags(CPUState *env)
+{
+ env->hflags &= ~(MIPS_HFLAG_COP1X | MIPS_HFLAG_64 | MIPS_HFLAG_CP0 |
+ MIPS_HFLAG_F64 | MIPS_HFLAG_FPU | MIPS_HFLAG_KSU |
+ MIPS_HFLAG_UX);
+ if (!(env->CP0_Status & (1 << CP0St_EXL)) &&
+ !(env->CP0_Status & (1 << CP0St_ERL)) &&
+ !(env->hflags & MIPS_HFLAG_DM)) {
+ env->hflags |= (env->CP0_Status >> CP0St_KSU) & MIPS_HFLAG_KSU;
+ }
+#if defined(TARGET_MIPS64)
+ if (((env->hflags & MIPS_HFLAG_KSU) != MIPS_HFLAG_UM) ||
+ (env->CP0_Status & (1 << CP0St_PX)) ||
+ (env->CP0_Status & (1 << CP0St_UX))) {
+ env->hflags |= MIPS_HFLAG_64;
+ }
+ if (env->CP0_Status & (1 << CP0St_UX)) {
+ env->hflags |= MIPS_HFLAG_UX;
+ }
+#endif
+ if ((env->CP0_Status & (1 << CP0St_CU0)) ||
+ !(env->hflags & MIPS_HFLAG_KSU)) {
+ env->hflags |= MIPS_HFLAG_CP0;
+ }
+ if (env->CP0_Status & (1 << CP0St_CU1)) {
+ env->hflags |= MIPS_HFLAG_FPU;
+ }
+ if (env->CP0_Status & (1 << CP0St_FR)) {
+ env->hflags |= MIPS_HFLAG_F64;
+ }
+ if (env->insn_flags & ISA_MIPS32R2) {
+ if (env->active_fpu.fcr0 & (1 << FCR0_F64)) {
+ env->hflags |= MIPS_HFLAG_COP1X;
+ }
+ } else if (env->insn_flags & ISA_MIPS32) {
+ if (env->hflags & MIPS_HFLAG_64) {
+ env->hflags |= MIPS_HFLAG_COP1X;
+ }
+ } else if (env->insn_flags & ISA_MIPS4) {
+ /* All supported MIPS IV CPUs use the XX (CU3) to enable
+ and disable the MIPS IV extensions to the MIPS III ISA.
+ Some other MIPS IV CPUs ignore the bit, so the check here
+ would be too restrictive for them. */
+ if (env->CP0_Status & (1 << CP0St_CU3)) {
+ env->hflags |= MIPS_HFLAG_COP1X;
+ }
+ }
+}
+
#endif /* !defined (__MIPS_CPU_H__) */
diff --git a/target-mips/op_helper.c b/target-mips/op_helper.c
index c51b9cb..1f1c736 100644
--- a/target-mips/op_helper.c
+++ b/target-mips/op_helper.c
@@ -32,55 +32,6 @@
static inline void cpu_mips_tlb_flush (CPUState *env, int flush_global);
#endif
-static inline void compute_hflags(CPUState *env)
-{
- env->hflags &= ~(MIPS_HFLAG_COP1X | MIPS_HFLAG_64 | MIPS_HFLAG_CP0 |
- MIPS_HFLAG_F64 | MIPS_HFLAG_FPU | MIPS_HFLAG_KSU |
- MIPS_HFLAG_UX);
- if (!(env->CP0_Status & (1 << CP0St_EXL)) &&
- !(env->CP0_Status & (1 << CP0St_ERL)) &&
- !(env->hflags & MIPS_HFLAG_DM)) {
- env->hflags |= (env->CP0_Status >> CP0St_KSU) & MIPS_HFLAG_KSU;
- }
-#if defined(TARGET_MIPS64)
- if (((env->hflags & MIPS_HFLAG_KSU) != MIPS_HFLAG_UM) ||
- (env->CP0_Status & (1 << CP0St_PX)) ||
- (env->CP0_Status & (1 << CP0St_UX))) {
- env->hflags |= MIPS_HFLAG_64;
- }
- if (env->CP0_Status & (1 << CP0St_UX)) {
- env->hflags |= MIPS_HFLAG_UX;
- }
-#endif
- if ((env->CP0_Status & (1 << CP0St_CU0)) ||
- !(env->hflags & MIPS_HFLAG_KSU)) {
- env->hflags |= MIPS_HFLAG_CP0;
- }
- if (env->CP0_Status & (1 << CP0St_CU1)) {
- env->hflags |= MIPS_HFLAG_FPU;
- }
- if (env->CP0_Status & (1 << CP0St_FR)) {
- env->hflags |= MIPS_HFLAG_F64;
- }
- if (env->insn_flags & ISA_MIPS32R2) {
- if (env->active_fpu.fcr0 & (1 << FCR0_F64)) {
- env->hflags |= MIPS_HFLAG_COP1X;
- }
- } else if (env->insn_flags & ISA_MIPS32) {
- if (env->hflags & MIPS_HFLAG_64) {
- env->hflags |= MIPS_HFLAG_COP1X;
- }
- } else if (env->insn_flags & ISA_MIPS4) {
- /* All supported MIPS IV CPUs use the XX (CU3) to enable
- and disable the MIPS IV extensions to the MIPS III ISA.
- Some other MIPS IV CPUs ignore the bit, so the check here
- would be too restrictive for them. */
- if (env->CP0_Status & (1 << CP0St_CU3)) {
- env->hflags |= MIPS_HFLAG_COP1X;
- }
- }
-}
-
/*****************************************************************************/
/* Exceptions processing helpers */
diff --git a/target-mips/translate.c b/target-mips/translate.c
index d5b1c76..cace3a3 100644
--- a/target-mips/translate.c
+++ b/target-mips/translate.c
@@ -12769,20 +12769,16 @@ void cpu_reset (CPUMIPSState *env)
env->CP0_SRSConf3 = env->cpu_model->CP0_SRSConf3;
env->CP0_SRSConf4_rw_bitmask = env->cpu_model->CP0_SRSConf4_rw_bitmask;
env->CP0_SRSConf4 = env->cpu_model->CP0_SRSConf4;
+ env->active_fpu.fcr0 = env->cpu_model->CP1_fcr0;
env->insn_flags = env->cpu_model->insn_flags;
#if defined(CONFIG_USER_ONLY)
- env->hflags = MIPS_HFLAG_UM;
+ env->CP0_Status = (MIPS_HFLAG_UM << CP0St_KSU);
/* Enable access to the SYNCI_Step register. */
env->CP0_HWREna |= (1 << 1);
if (env->CP0_Config1 & (1 << CP0C1_FP)) {
- env->hflags |= MIPS_HFLAG_FPU;
+ env->CP0_Status |= (1 << CP0St_CU1);
}
-#ifdef TARGET_MIPS64
- if (env->active_fpu.fcr0 & (1 << FCR0_F64)) {
- env->hflags |= MIPS_HFLAG_F64;
- }
-#endif
#else
if (env->hflags & MIPS_HFLAG_BMASK) {
/* If the exception was raised from a delay slot,
@@ -12812,7 +12808,6 @@ void cpu_reset (CPUMIPSState *env)
}
/* Count register increments in debug mode, EJTAG version 1 */
env->CP0_Debug = (1 << CP0DB_CNT) | (0x1 << CP0DB_VER);
- env->hflags = MIPS_HFLAG_CP0;
if (env->CP0_Config3 & (1 << CP0C3_MT)) {
int i;
@@ -12840,11 +12835,7 @@ void cpu_reset (CPUMIPSState *env)
}
}
#endif
-#if defined(TARGET_MIPS64)
- if (env->cpu_model->insn_flags & ISA_MIPS3) {
- env->hflags |= MIPS_HFLAG_64;
- }
-#endif
+ compute_hflags(env);
env->exception_index = EXCP_NONE;
}
--
1.7.7.6
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [Qemu-devel] [PATCH v1 1/1] mips: properly compute hflags and fcr0 on cpu reset
2012-03-02 21:03 [Qemu-devel] [PATCH v1 1/1] mips: properly compute hflags and fcr0 on cpu reset Meador Inge
@ 2012-03-03 16:45 ` Andreas Färber
2012-03-03 17:26 ` Meador Inge
0 siblings, 1 reply; 3+ messages in thread
From: Andreas Färber @ 2012-03-03 16:45 UTC (permalink / raw)
To: Meador Inge; +Cc: Khansa Butt, Maciej W. Rozycki, qemu-devel, aurelien
Am 02.03.2012 22:03, schrieb Meador Inge:
> Currently 'cpu_reset' doesn't fully compute all of the needed
> HFLAGs and fails to setup fcr0 after clearing the CPU state.
> This can cause instruction exceptions. For example, using
> 'madd.d' on machines that should support it is kindly greeted
> with:
>
> qemu: uncaught target signal 4 (Illegal instruction) - core dumped
> Illegal instruction (core dumped)
>
> because fcr0 is bogus and MIPS_HFLAG_COP1X is not correcly set in hflags.
>
> This is fixed by modifying 'cpu_reset' to use 'compute_hflags' and
> initializing 'fcr0' from the current CPU model.
fcr0 issue has also been
Reported-by: Khansa Butt <khansa@kics.edu.pk>
e.g., http://patchwork.ozlabs.org/patch/133974/
Your use of compute_hflags() looks more future-proof.
>
> Signed-off-by: Maciej W. Rozycki <macro@codesourcery.com>
> Signed-off-by: Nathan Froyd <froydnj@codesourcery.com>
> Signed-off-by: Meador Inge <meadori@codesourcery.com>
> ---
> target-mips/cpu.h | 49 +++++++++++++++++++++++++++++++++++++++++++++++
> target-mips/op_helper.c | 49 -----------------------------------------------
> target-mips/translate.c | 17 +++------------
> 3 files changed, 53 insertions(+), 62 deletions(-)
>
> diff --git a/target-mips/cpu.h b/target-mips/cpu.h
> index 71cb4e8..fc65348 100644
> --- a/target-mips/cpu.h
> +++ b/target-mips/cpu.h
> @@ -737,4 +737,53 @@ static inline void cpu_pc_from_tb(CPUState *env, TranslationBlock *tb)
> env->hflags |= tb->flags & MIPS_HFLAG_BMASK;
> }
>
> +static inline void compute_hflags(CPUState *env)
> +{
Moving helper functions like these to cpu.h has proven troublesome for
QOM'ification (when they need access to MIPSCPU[Class] in addition to
CPUMIPSState) but it'll do for now.
Reviewed-by: Andreas Färber <afaerber@suse.de>
Andreas
--
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [Qemu-devel] [PATCH v1 1/1] mips: properly compute hflags and fcr0 on cpu reset
2012-03-03 16:45 ` Andreas Färber
@ 2012-03-03 17:26 ` Meador Inge
0 siblings, 0 replies; 3+ messages in thread
From: Meador Inge @ 2012-03-03 17:26 UTC (permalink / raw)
To: Andreas Färber; +Cc: Khansa Butt, Maciej W. Rozycki, qemu-devel, aurelien
On 03/03/2012 10:45 AM, Andreas Färber wrote:
> Am 02.03.2012 22:03, schrieb Meador Inge:
>> Currently 'cpu_reset' doesn't fully compute all of the needed
>> HFLAGs and fails to setup fcr0 after clearing the CPU state.
>> This can cause instruction exceptions. For example, using
>> 'madd.d' on machines that should support it is kindly greeted
>> with:
>>
>> qemu: uncaught target signal 4 (Illegal instruction) - core dumped
>> Illegal instruction (core dumped)
>>
>> because fcr0 is bogus and MIPS_HFLAG_COP1X is not correcly set in hflags.
>>
>> This is fixed by modifying 'cpu_reset' to use 'compute_hflags' and
>> initializing 'fcr0' from the current CPU model.
>
> fcr0 issue has also been
>
> Reported-by: Khansa Butt <khansa@kics.edu.pk>
>
> e.g., http://patchwork.ozlabs.org/patch/133974/
Ah, thanks. The fcr0 fix had been sitting in our local tree for a while and I
just forgot to check upstream patch submissions. I need to get in the habit of
looking at patchwork first.
> Your use of compute_hflags() looks more future-proof.
>
>>
>> Signed-off-by: Maciej W. Rozycki <macro@codesourcery.com>
>> Signed-off-by: Nathan Froyd <froydnj@codesourcery.com>
>> Signed-off-by: Meador Inge <meadori@codesourcery.com>
>> ---
>> target-mips/cpu.h | 49 +++++++++++++++++++++++++++++++++++++++++++++++
>> target-mips/op_helper.c | 49 -----------------------------------------------
>> target-mips/translate.c | 17 +++------------
>> 3 files changed, 53 insertions(+), 62 deletions(-)
>>
>> diff --git a/target-mips/cpu.h b/target-mips/cpu.h
>> index 71cb4e8..fc65348 100644
>> --- a/target-mips/cpu.h
>> +++ b/target-mips/cpu.h
>> @@ -737,4 +737,53 @@ static inline void cpu_pc_from_tb(CPUState *env, TranslationBlock *tb)
>> env->hflags |= tb->flags & MIPS_HFLAG_BMASK;
>> }
>>
>> +static inline void compute_hflags(CPUState *env)
>> +{
>
> Moving helper functions like these to cpu.h has proven troublesome for
> QOM'ification (when they need access to MIPSCPU[Class] in addition to
> CPUMIPSState) but it'll do for now.
Okay, I will keep that in mind for the future. Thanks for the review.
> Reviewed-by: Andreas Färber <afaerber@suse.de>
>
> Andreas
>
--
Meador Inge
CodeSourcery / Mentor Embedded
http://www.mentor.com/embedded-software
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2012-03-03 17:26 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-03-02 21:03 [Qemu-devel] [PATCH v1 1/1] mips: properly compute hflags and fcr0 on cpu reset Meador Inge
2012-03-03 16:45 ` Andreas Färber
2012-03-03 17:26 ` Meador Inge
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).