Re: [Qemu-devel] [PATCH] target-mips: 64-bit FPU for user-mode emulation.

Re: [Qemu-devel] [PATCH] target-mips: 64-bit FPU for user-mode emulation.

[Petar Jovanovic]

Will there be an update to this patch?
It would be nice to see this in for 1.6.

As of fp64 mode for o32/Rev2, it does not have to be resolved now.

Regards,
Petar

On Sun, 9 Jun 2013, Thomas Schwinge wrote:
> In my reading of the relevant documents, the latter change is not correct
> for o32, and empirically has "interesting" effects on the glibc math
> testsuite, for example.  Keeping the FR register unset for o32 I'm
> proposing to fix with the following patch:

 Correct, unless (until?) the -mfp64 o32 ABI extension is implemented for 
Linux, CP0.Status.FR must remain 0 for o32 programs.

> diff --git target-mips/translate.c target-mips/translate.c
> index 0a53203..51837d4 100644
> --- target-mips/translate.c
> +++ target-mips/translate.c
> @@ -15962,10 +15962,12 @@ void cpu_state_reset(CPUMIPSState *env)
>      if (env->CP0_Config3 & (1 << CP0C3_DSPP)) {
>          env->CP0_Status |= (1 << CP0St_MX);
>      }
> -    /* Enable 64-bit FPU if the target cpu supports it.  */
> +# if defined(TARGET_ABI_MIPSN32) || defined(TARGET_ABI_MIPSN64)
> +    /* Enable 64-bit FPU if the target CPU supports it.  */
>      if (env->active_fpu.fcr0 & (1 << FCR0_F64)) {
>          env->CP0_Status |= (1 << CP0St_FR);

 This is not entirely correct, older 64-bit FPUs, i.e. any before the 
MIPS32/MIPS64 rev. 2 ISA (e.g. R4000, R10000, 5Kf, etc.) won't have the 
CP1.FIR.F64 bit set; it was only defined at that ISA level because for 
earlier architecture revisions the type of the FPU could have been 
inferred from the type of the CPU.  Therefore the condition has to be 
changed, perhaps the best way would simply be just checking in the 
CP0.Status mask if the FR bit is writable.

 Also I suppose there must be an else clause here:

    } else {
        fprintf(stderr, "A 64-bit FPU required for NewABI emulation\n");
        exit(1);

or suchlike because the NewABI mandates full 64-bit FPU operation (or the 
condtion might be changed to an assertion instead and the emulated 
environment checked elsewhere earlier on, because a 64-bit CPU is required 
for NewABI operation anyway and a 64-bit CPU can't ever have a 32-bit FPU 
-- I don't know QEMU well enough to be sure offhand, please check).

>      }
> +# endif
>  #else
>      if (env->hflags & MIPS_HFLAG_BMASK) {
>          /* If the exception was raised from a delay slot,

  Maciej

 

Re: [Qemu-devel] [PATCH] target-mips: 64-bit FPU for user-mode emulation.

[Thomas Schwinge]

[-- Attachment #1: Type: text/plain, Size: 603 bytes --]

Hi!

On Sun, 28 Jul 2013 02:39:23 +0000, Petar Jovanovic <Petar.Jovanovic@imgtec.com> wrote:
> Will there be an update to this patch?

| Switch to 64-bit FPU only for n32 and n64 ABIs, but not o32.  Fixup for
| commit 68473f15d4c9948986618f63828825beafcaf1cf.
| [...]

My patch has to be reworked per Maciej's comments,
<http://news.gmane.org/find-root.php?message_id=%3Calpine.DEB.1.10.1306091926470.16287%40tp.orcam.me.uk%3E>,
which I have not yet managed to allocate the time to do, and can't tell
when it's going to happen -- but I still have it on my TODO list.


Grüße,
 Thomas

[-- Attachment #2: Type: application/pgp-signature, Size: 489 bytes --]

Re: [Qemu-devel] [PATCH 07/10] mips64-linux-user: Use MIPS64R2-generic as default cpu.

[Aurelien Jarno]

On Sun, Feb 10, 2013 at 10:30:47AM -0800, Richard Henderson wrote:
> Signed-off-by: Richard Henderson <rth@twiddle.net>
> ---
>  linux-user/main.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> I don't actually recall why this patch was in my old tree.  It might
> just have been that I wanted a mips64r2 part for testing and didn't
> want to keep specifying -cpu on the command-line.  OTOH, I don't see
> any reason not to emulate as many ISA extensions as possible by default.
> 
> 
> r~
> 
> 
> 
> diff --git a/linux-user/main.c b/linux-user/main.c
> index 3a3be70..8c4dffd 100644
> --- a/linux-user/main.c
> +++ b/linux-user/main.c
> @@ -3525,7 +3525,9 @@ int main(int argc, char **argv, char **envp)
>          cpu_model = "Fujitsu MB86904";
>  #endif
>  #elif defined(TARGET_MIPS)
> -#if defined(TARGET_ABI_MIPSN32) || defined(TARGET_ABI_MIPSN64)
> +#if defined(TARGET_ABI_MIPSN64)
> +        cpu_model = "MIPS64R2-generic";
> +#elif defined(TARGET_ABI_MIPSN32)
>          cpu_model = "20Kc";
>  #else
>          cpu_model = "24Kf";

Why switching only N64 and not also for N32? The same way we should also
switch the O32 one to 74Kf which also has DSP R2 and MIPS16.

That looks fine only as a first step, otherwise the idea of having an
"any" CPU, as suggested by Peter, looks like a good idea. That said some
instruction set conflicts so we can't enable all of them in such a CPU.
The name "any" can be a bit misleading in that case. Is it also the case
on other architectures?

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

Re: [Qemu-devel] [PATCH 08/10] target-mips: Fix check_cp1_64bitmode

[Aurelien Jarno]

On Sun, Feb 10, 2013 at 10:30:48AM -0800, Richard Henderson wrote:
> COP1X refers to the availability of indexed memory operations,
> not whether the FPU has 64-bit registers.
> 
> Signed-off-by: Richard Henderson <rth@twiddle.net>
> ---
>  target-mips/translate.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/target-mips/translate.c b/target-mips/translate.c
> index b3b8dc6..a39e118 100644
> --- a/target-mips/translate.c
> +++ b/target-mips/translate.c
> @@ -1366,8 +1366,9 @@ static inline void check_cop1x(DisasContext *ctx)
>  
>  static inline void check_cp1_64bitmode(DisasContext *ctx)
>  {
> -    if (unlikely(~ctx->hflags & (MIPS_HFLAG_F64 | MIPS_HFLAG_COP1X)))
> +    if (unlikely(!(ctx->hflags & MIPS_HFLAG_F64))) {
>          generate_exception(ctx, EXCP_RI);
> +    }
>  }
>  

I think this is wrong. This check comes from the following patch:

http://lists.gnu.org/archive/html/qemu-devel/2007-12/msg00581.html

In short check_cp1_64bitmode() is called for COP1X instructions (nabla
ones) which need the FPU in 64-bit mode. COP1X instructions might not
always been able and might have to be activated (CU3 on MIPS IV, 64-bit
operations enabled on MIPS64R1), so checking the ISA is not enough.

Maybe the name is misleading, but IMHO the behavior is correct.

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

Re: [Qemu-devel] [PATCH 09/10] mipsn32-linux-user: Restrict address space to 31-bits.

[Aurelien Jarno]

On Sun, Feb 10, 2013 at 10:30:49AM -0800, Richard Henderson wrote:
> Signed-off-by: Richard Henderson <rth@twiddle.net>
> ---
>  linux-user/main.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/linux-user/main.c b/linux-user/main.c
> index 8c4dffd..25491ca 100644
> --- a/linux-user/main.c
> +++ b/linux-user/main.c
> @@ -46,10 +46,10 @@ int gdbstub_port;
>  envlist_t *envlist;
>  const char *cpu_model;
>  unsigned long mmap_min_addr;
> +
>  #if defined(CONFIG_USE_GUEST_BASE)
>  unsigned long guest_base;
>  int have_guest_base;
> -#if (TARGET_LONG_BITS == 32) && (HOST_LONG_BITS == 64)
>  /*
>   * When running 32-on-64 we should make sure we can fit all of the possible
>   * guest address space into a contiguous chunk of virtual host memory.
> @@ -57,16 +57,16 @@ int have_guest_base;
>   * This way we will never overlap with our own libraries or binaries or stack
>   * or anything else that QEMU maps.
>   */
> -# ifdef TARGET_MIPS
> +# if HOST_LONG_BITS == 64 \
> +     && (defined(TARGET_ABI_MIPSO32) || defined(TARGET_ABI_MIPSN32))
>  /* MIPS only supports 31 bits of virtual address space for user space */
> -unsigned long reserved_va = 0x77000000;
> -# else
> +unsigned long reserved_va = 0x7f000000;

Is it really wanted to change 0x77 into 0x7f? If yes, I think the
commit log should explain why.

> +# elif (TARGET_LONG_BITS == 32) && (HOST_LONG_BITS == 64)
>  unsigned long reserved_va = 0xf7000000;
> -# endif
>  #else
>  unsigned long reserved_va;
>  #endif
> -#endif
> +#endif /* CONFIG_USE_GUEST_BASE */
>  
>  static void usage(void);
>  
> -- 
> 1.8.1.2
> 
> 

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

Re: [Qemu-devel] [PATCH 09/10] mipsn32-linux-user: Restrict address space to 31-bits.

[Aurelien Jarno]

On Tue, Mar 05, 2013 at 03:03:44PM +0100, Aurelien Jarno wrote:
> On Sun, Feb 10, 2013 at 10:30:49AM -0800, Richard Henderson wrote:
> > Signed-off-by: Richard Henderson <rth@twiddle.net>
> > ---
> >  linux-user/main.c | 12 ++++++------
> >  1 file changed, 6 insertions(+), 6 deletions(-)
> > 
> > diff --git a/linux-user/main.c b/linux-user/main.c
> > index 8c4dffd..25491ca 100644
> > --- a/linux-user/main.c
> > +++ b/linux-user/main.c
> > @@ -46,10 +46,10 @@ int gdbstub_port;
> >  envlist_t *envlist;
> >  const char *cpu_model;
> >  unsigned long mmap_min_addr;
> > +
> >  #if defined(CONFIG_USE_GUEST_BASE)
> >  unsigned long guest_base;
> >  int have_guest_base;
> > -#if (TARGET_LONG_BITS == 32) && (HOST_LONG_BITS == 64)
> >  /*
> >   * When running 32-on-64 we should make sure we can fit all of the possible
> >   * guest address space into a contiguous chunk of virtual host memory.
> > @@ -57,16 +57,16 @@ int have_guest_base;
> >   * This way we will never overlap with our own libraries or binaries or stack
> >   * or anything else that QEMU maps.
> >   */
> > -# ifdef TARGET_MIPS
> > +# if HOST_LONG_BITS == 64 \
> > +     && (defined(TARGET_ABI_MIPSO32) || defined(TARGET_ABI_MIPSN32))
> >  /* MIPS only supports 31 bits of virtual address space for user space */
> > -unsigned long reserved_va = 0x77000000;
> > -# else
> > +unsigned long reserved_va = 0x7f000000;
> 
> Is it really wanted to change 0x77 into 0x7f? If yes, I think the
> commit log should explain why.
> 

Ping?

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

Re: [Qemu-devel] [PATCH 09/10] mipsn32-linux-user: Restrict address space to 31-bits.

[Richard Henderson]

On 03/26/2013 12:50 PM, Aurelien Jarno wrote:
>>> > > -# ifdef TARGET_MIPS
>>> > > +# if HOST_LONG_BITS == 64 \
>>> > > +     && (defined(TARGET_ABI_MIPSO32) || defined(TARGET_ABI_MIPSN32))
>>> > >  /* MIPS only supports 31 bits of virtual address space for user space */
>>> > > -unsigned long reserved_va = 0x77000000;
>>> > > -# else
>>> > > +unsigned long reserved_va = 0x7f000000;
>> > 
>> > Is it really wanted to change 0x77 into 0x7f? If yes, I think the
>> > commit log should explain why.
>> > 
> Ping?

As far as I could remember, that 0x77 was totally arbitrary too,
and so I thought I'd "fix" it.  I don't recall more, sorry.


r~

Re: [Qemu-devel] [PATCH 00/10] Support mips64/mipsn32-linux-user

[Aurelien Jarno]

Hi,

On Sun, Feb 10, 2013 at 10:30:40AM -0800, Richard Henderson wrote:
> Peter's recent userland syscall testing has encouraged me to clean
> up and re-submit some ages-old patches to enable userland testing
> of the other mips abis.
> 
> Please review.
> 
> 
> r~
> 
> 
> Richard Henderson (10):
>   mips-linux-user: Delete n32 and n64 signal stubs
>   mips-linux-user: Share o32 code for n32 and n64 signals
>   mips-linux-user: Enable mips64 and mipsn32 linux-user targets
>   mips-linux-user: Save and restore fpu and dsp from sigcontext
>   mips-linux-user: Fix n32 and n64 syscalls
>   mips64-linux-user: Enable 64-bit address mode and fpu
>   mips64-linux-user: Use MIPS64R2-generic as default cpu.
>   target-mips: Fix check_cp1_64bitmode
>   mipsn32-linux-user: Restrict address space to 31-bits.
>   mipsn32-linux-user: Configure the architecture properly
> 

I have applied patches 1, 2, 3, 4, 5, 6 and 10. I have taken the
freedom the fix the typo in patch 4 and also the tabs in patch 10 found
by Peter.

For patch 4, the remarks from Peter are valid, but they concerns already
existing code. It might be nice to follow-up with the corresponding
fixes.

For patches 7 to 9, please see my comments to the individual patch and
follow up if needed with a new patch series. I'll try to work on a "any"
cpu in the next days.

Thanks for the patches and to Peter for the review.

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

[Qemu-devel] [PATCH] target-mips: 64-bit FPU for user-mode emulation.

[Thomas Schwinge]

[-- Attachment #1: Type: text/plain, Size: 2651 bytes --]

Switch to 64-bit FPU only for n32 and n64 ABIs, but not o32.  Fixup for
commit 68473f15d4c9948986618f63828825beafcaf1cf.

Signed-off-by: Thomas Schwinge <thomas@codesourcery.com>

---

Hi!

On Sun, 10 Feb 2013 10:30:46 -0800, Richard Henderson <rth@twiddle.net> wrote:
> diff --git a/target-mips/translate.c b/target-mips/translate.c
> index 3b77b53..b3b8dc6 100644
> --- a/target-mips/translate.c
> +++ b/target-mips/translate.c
> @@ -15972,6 +15972,14 @@ void cpu_state_reset(CPUMIPSState *env)
>  
>  #if defined(CONFIG_USER_ONLY)
>      env->CP0_Status = (MIPS_HFLAG_UM << CP0St_KSU);
> +# ifdef TARGET_MIPS64
> +    /* Enable 64-bit register mode.  */
> +    env->CP0_Status |= (1 << CP0St_PX);
> +# endif
> +# ifdef TARGET_ABI_MIPSN64
> +    /* Enable 64-bit address mode.  */
> +    env->CP0_Status |= (1 << CP0St_UX);
> +# endif
>      /* Enable access to the CPUNum, SYNCI_Step, CC, and CCRes RDHWR
>         hardware registers.  */
>      env->CP0_HWREna |= 0x0000000F;
> @@ -15981,6 +15989,11 @@ void cpu_state_reset(CPUMIPSState *env)
>      if (env->CP0_Config3 & (1 << CP0C3_DSPP)) {
>          env->CP0_Status |= (1 << CP0St_MX);
>      }
> +    /* Enable 64-bit FPU if the target cpu supports it.  */
> +    env->active_fpu.fcr0 = env->cpu_model->CP1_fcr0;
> +    if (env->active_fpu.fcr0 & (1 << FCR0_F64)) {
> +        env->CP0_Status |= (1 << CP0St_FR);
> +    }
>  #else
>      if (env->hflags & MIPS_HFLAG_BMASK) {
>          /* If the exception was raised from a delay slot,

In my reading of the relevant documents, the latter change is not correct
for o32, and empirically has "interesting" effects on the glibc math
testsuite, for example.  Keeping the FR register unset for o32 I'm
proposing to fix with the following patch:

---
 target-mips/translate.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git target-mips/translate.c target-mips/translate.c
index 0a53203..51837d4 100644
--- target-mips/translate.c
+++ target-mips/translate.c
@@ -15962,10 +15962,12 @@ void cpu_state_reset(CPUMIPSState *env)
     if (env->CP0_Config3 & (1 << CP0C3_DSPP)) {
         env->CP0_Status |= (1 << CP0St_MX);
     }
-    /* Enable 64-bit FPU if the target cpu supports it.  */
+# if defined(TARGET_ABI_MIPSN32) || defined(TARGET_ABI_MIPSN64)
+    /* Enable 64-bit FPU if the target CPU supports it.  */
     if (env->active_fpu.fcr0 & (1 << FCR0_F64)) {
         env->CP0_Status |= (1 << CP0St_FR);
     }
+# endif
 #else
     if (env->hflags & MIPS_HFLAG_BMASK) {
         /* If the exception was raised from a delay slot,


Grüße,
 Thomas

[-- Attachment #2: Type: application/pgp-signature, Size: 489 bytes --]

Re: [Qemu-devel] [PATCH] target-mips: 64-bit FPU for user-mode emulation.

[Maciej W. Rozycki]

On Sun, 9 Jun 2013, Thomas Schwinge wrote:

> In my reading of the relevant documents, the latter change is not correct
> for o32, and empirically has "interesting" effects on the glibc math
> testsuite, for example.  Keeping the FR register unset for o32 I'm
> proposing to fix with the following patch:

 Correct, unless (until?) the -mfp64 o32 ABI extension is implemented for 
Linux, CP0.Status.FR must remain 0 for o32 programs.

> diff --git target-mips/translate.c target-mips/translate.c
> index 0a53203..51837d4 100644
> --- target-mips/translate.c
> +++ target-mips/translate.c
> @@ -15962,10 +15962,12 @@ void cpu_state_reset(CPUMIPSState *env)
>      if (env->CP0_Config3 & (1 << CP0C3_DSPP)) {
>          env->CP0_Status |= (1 << CP0St_MX);
>      }
> -    /* Enable 64-bit FPU if the target cpu supports it.  */
> +# if defined(TARGET_ABI_MIPSN32) || defined(TARGET_ABI_MIPSN64)
> +    /* Enable 64-bit FPU if the target CPU supports it.  */
>      if (env->active_fpu.fcr0 & (1 << FCR0_F64)) {
>          env->CP0_Status |= (1 << CP0St_FR);

 This is not entirely correct, older 64-bit FPUs, i.e. any before the 
MIPS32/MIPS64 rev. 2 ISA (e.g. R4000, R10000, 5Kf, etc.) won't have the 
CP1.FIR.F64 bit set; it was only defined at that ISA level because for 
earlier architecture revisions the type of the FPU could have been 
inferred from the type of the CPU.  Therefore the condition has to be 
changed, perhaps the best way would simply be just checking in the 
CP0.Status mask if the FR bit is writable.

 Also I suppose there must be an else clause here:

    } else {
        fprintf(stderr, "A 64-bit FPU required for NewABI emulation\n");
        exit(1);

or suchlike because the NewABI mandates full 64-bit FPU operation (or the 
condtion might be changed to an assertion instead and the emulated 
environment checked elsewhere earlier on, because a 64-bit CPU is required 
for NewABI operation anyway and a 64-bit CPU can't ever have a 32-bit FPU 
-- I don't know QEMU well enough to be sure offhand, please check).

>      }
> +# endif
>  #else
>      if (env->hflags & MIPS_HFLAG_BMASK) {
>          /* If the exception was raised from a delay slot,

  Maciej