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
Am 31.12.2011 08:42, schrieb Khansa Butt: On Fri, Dec 9, 2011 at 5:04 AM, Andreas Färber andreas.faer...@web.de wrote: Am 08.12.2011 06:25, schrieb kha...@kics.edu.pk: 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 qad...@kics.edu.pk Signed-off-by: (you) ---8--- and make it depend on TARGET_ABI_MIPSN64 then I will happily add my Acked-by. Why this is necessary to put env-hflags |= MIPS_HFLAG_UX; line under TARGET_ABI_MIPSN64? as this was already put under #if TARGET_MIPS64, is not it suffient? You're right. I was under the impression that both n32 and n64 were based off mips64, but mipsn32 is in fact based off mips. Adding NUBI64 support (as opposed to NUBI64W) would then probably be based off mips as well then. Quite confusing. So yes, no need to add #if defined(TARGET_ABI_MIPSN64) there, but do put it in its own patch with a description explaining why. Andreas
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
[cc'ing list] Am 30.12.2011 08:52, schrieb Khansa Butt: On Thu, Dec 29, 2011 at 4:17 PM, Andreas Färber andreas.faer...@web.de wrote: Also, given your observation, does it even make sense for cpu_mips_init() to call fpu_init() when all CPUState members it initializes get cleared in cpu_reset()? Maybe just move that call into cpu_reset() and rename it to fpu_reset()? mmu_init() and mvp_init() seem okay by contrast. why cpu_reset() calls memset? it does not reset all the members of CPUState only those which are in the range of offsetof(CPUMIPSState, breakpoints). what if I remove memset line? I agree that the memset() line is bad as-is. My suggestion on some other threads about having multiple CPUStates and/or ARM reset was to at least define a macro than to copy this knowledge everywhere. QOM may help to improve that in the future with better Object Orientation. Today, the convention is that all struct members before 'breakpoints' are zeroed on reset. Things that come after 'CPU_COMMON' are therefore not reset. Things before CPU_COMMON, like in your case, need to be saved before the memset() and restored afterwards (if their value is to be preserved over reset) or initialized to some value (if different from zero). I would strongly suggest to live with memset() for now as it's already quite complicated to get *anything* done on mips as you've noticed. :) Regards, Andreas
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
On Wed, Dec 14, 2011 at 10:05 PM, Richard Henderson r...@twiddle.net wrote:
On 12/08/2011 04:04 PM, Andreas Färber wrote:
+ /* 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]
That said, there's still something missing, e.g. MIPS_HFLAG_COP1X.
My first guess is simply
if (env-insn_flags (ISA_MIPS32 | ISA_MIPS4)) {
env-hflags |= MIPS_HFLAG_COP1X;
}
I don't understand why we add above lines. I think this issue is some
what related to cpu_model not with ISA
I've explained why I add env-active_fpu.fcr0 =
env-cpu_model-CP1_fcr0; line in reply to this patch to Andreas
Färber
and cc'ed you as well
immediately after this MIPS64 hunk.
r~
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
Am 29.12.2011 08:55, schrieb Khansa Butt:
On Fri, Dec 9, 2011 at 5:04 AM, Andreas Färber andreas.faer...@web.de wrote:
+/* 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.
Well, that sounds like a different and more generic problem that
shouldn't be fixed inside CONFIG_USER_ONLY TARGET_MIPS64.
And your reasoning should've definitely been in the commit message!
The problem here is not whether the patches observably work for you but
whether it's the correct way to fix it. We did this because it works
for us is never a convincing justification of a change.
If it doesn't work for you in linux-user it won't work in softmmu
either, so doing that before #if defined(CONFIG_USER_ONLY) where lots of
env-cpu_model stuff is being copyied (esp. before env-HABITS to honor
mips_def_t order) seems better.
Also, given your observation, does it even make sense for
cpu_mips_init() to call fpu_init() when all CPUState members it
initializes get cleared in cpu_reset()? Maybe just move that call into
cpu_reset() and rename it to fpu_reset()? mmu_init() and mvp_init() seem
okay by contrast.
When you've figured this out, please again put it into a separate patch
titled, e.g., target-mips: Fix FPU reset with appropriate explanation.
Andreas
[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 tre
On Thu, Dec 29, 2011 at 4:17 PM, Andreas Färber andreas.faer...@web.de wrote:
Am 29.12.2011 08:55, schrieb Khansa Butt:
On Fri, Dec 9, 2011 at 5:04 AM, Andreas Färber andreas.faer...@web.de
wrote:
+ /* 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.
Well, that sounds like a different and more generic problem that
shouldn't be fixed inside CONFIG_USER_ONLY TARGET_MIPS64.
And your reasoning should've definitely been in the commit message!
The problem here is not whether the patches observably work for you but
whether it's the correct way to fix it. We did this because it works
for us is never a convincing justification of a change.
If it doesn't work for you in linux-user it won't work in softmmu
either, so doing that before #if defined(CONFIG_USER_ONLY) where lots of
env-cpu_model stuff is being copyied (esp. before env-HABITS to honor
mips_def_t order) seems better.
Also, given your observation, does it even make sense for
cpu_mips_init() to call fpu_init() when all CPUState members it
initializes get cleared in cpu_reset()? Maybe just move that call into
cpu_reset() and rename it to fpu_reset()? mmu_init() and mvp_init() seem
okay by contrast.
why cpu_reset() calls memset? it does not reset all the members of CPUState only
those which are in the range of offsetof(CPUMIPSState, breakpoints).
what if I remove
memset line?
When you've figured this out, please again put it into a separate patch
titled, e.g., target-mips: Fix FPU reset with appropriate explanation.
Andreas
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
On Fri, Dec 9, 2011 at 5:04 AM, Andreas Färber andreas.faer...@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 kha...@kics.edu.pk:
From: Khansa Butt kha...@kics.edu.pk
Signed-off-by: Abdul Qadeer qad...@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 qad...@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
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
On 12/08/2011 04:04 PM, Andreas Färber wrote:
+/* 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]
That said, there's still something missing, e.g. MIPS_HFLAG_COP1X.
My first guess is simply
if (env-insn_flags (ISA_MIPS32 | ISA_MIPS4)) {
env-hflags |= MIPS_HFLAG_COP1X;
}
immediately after this MIPS64 hunk.
r~
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
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 kha...@kics.edu.pk:
From: Khansa Butt kha...@kics.edu.pk
Signed-off-by: Abdul Qadeer qad...@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 qad...@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]
Andreas
[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 tre
From: Khansa Butt kha...@kics.edu.pk
Signed-off-by: Abdul Qadeer qad...@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;
+/* 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;
}
--
1.7.3.4
[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 tre
From: Khansa Butt kha...@kics.edu.pk
Signed-off-by: Khansa Butt kha...@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;
+/* 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;
}
--
1.7.3.4
