diff mbox series

[1/9] target/mips: Require TARGET_MIPS64 for R5900 multimedia instructions

Message ID 781155aaf175ea20903c12e26316ea40085457bb.1547403692.git.noring@nocrew.org
State New
Headers show
Series target/mips: Limited support for R5900 multimedia instructions | expand

Commit Message

Fredrik Noring Jan. 13, 2019, 7:02 p.m. UTC
The R5900 MMIs operate on 128-bit registers that will be split into
two halves: lower 64-bit GPRs and upper 64-bit MMRs. The MMIs will
therefore be left unimplemented in 32-bit mode with the o32 ABI.

Signed-off-by: Fredrik Noring <noring@nocrew.org>
---
 target/mips/translate.c | 28 ++++++++++++++++------------
 1 file changed, 16 insertions(+), 12 deletions(-)

Comments

Aleksandar Markovic Jan. 15, 2019, 9:03 p.m. UTC | #1
> From: Fredrik Noring <noring@nocrew.org>
> Sent: Sunday, January 13, 2019 8:02 PM
> To: Aleksandar Markovic; Aurelien Jarno; Philippe Mathieu-Daudé
> Cc: Jürgen Urban; Maciej W. Rozycki; qemu-devel@nongnu.org
> Subject: [PATCH 1/9] target/mips: Require TARGET_MIPS64 for R5900 multimedia instructions
> 
> The R5900 MMIs operate on 128-bit registers that will be split into
> two halves: lower 64-bit GPRs and upper 64-bit MMRs. The MMIs will
> therefore be left unimplemented in 32-bit mode with the o32 ABI.
> 
> Signed-off-by: Fredrik Noring <noring@nocrew.org>
> ---
>  target/mips/translate.c | 28 ++++++++++++++++------------
>  1 file changed, 16 insertions(+), 12 deletions(-)
>

Sorry, I have to disagree with this. Processor model must stay the same, and
Linux ABI should not affect, for example, the number and size of processor
registers - just like it is the case in reality.

I recommend to you to accept the mindset that QEMU is not a part of some
toolchain in a wide sense of this word.

QEMU is an independent software tool - it is for example, a compiler-agnostic
tool, and the only connection between a compiler and QEMU is the processor
documentation - and this is the reason they work together so well. They shouldn't
be "tweaked" and "integrated" to work together - both QEMU and compiler should
rely only on the processor specification, and should not know anything about the
other side.

Similar reason can be applied to ABI, QEMU Linux user mode ABI obviously
deals with specifics of ABIs, but should not touch processor model. Processor
model should not depend on conventions of any ABI. The fact that, for example,
some ABI "promises" that it will not use, let's say, some registers or instructions
is irrelevant to QEMU.

My advice for you is to focus on n32 at the time being.

o32 can be implemented with the same 64-bit processor model, but in a much
different way that you attempted some time ago. To avoid waste of our energy
and time, I am suggesting that we finish n32, and think of o32 in future.

Thanks, and hope you understand my points, and please accept my advices.

Aleksandar
 
> 
> 
> diff --git a/target/mips/translate.c b/target/mips/translate.c
> index 057aaf9a44..a538351032 100644
> --- a/target/mips/translate.c
> +++ b/target/mips/translate.c
> @@ -27218,6 +27218,7 @@ static void decode_opc_special3_legacy(CPUMIPSState *env, > DisasContext *ctx)
>      }
>  }
> 
> +#if defined(TARGET_MIPS64)
>  static void decode_mmi0(CPUMIPSState *env, DisasContext *ctx)
>  {
>      uint32_t opc = MASK_MMI0(ctx->opcode);
> @@ -27351,6 +27352,7 @@ static void decode_mmi3(CPUMIPSState *env, DisasContext *ctx)
>          break;
>      }
>  }
> +#endif /* defined(TARGET_MIPS64) */
> 
>  static void decode_mmi(CPUMIPSState *env, DisasContext *ctx)
>  {
> @@ -27360,18 +27362,6 @@ static void decode_mmi(CPUMIPSState *env, DisasContext *ctx)
>      int rd = extract32(ctx->opcode, 11, 5);
> 
>      switch (opc) {
> -    case MMI_OPC_CLASS_MMI0:
> -        decode_mmi0(env, ctx);
> -        break;
> -    case MMI_OPC_CLASS_MMI1:
> -        decode_mmi1(env, ctx);
> -        break;
> -    case MMI_OPC_CLASS_MMI2:
> -        decode_mmi2(env, ctx);
> -        break;
> -    case MMI_OPC_CLASS_MMI3:
> -        decode_mmi3(env, ctx);
> -        break;
>      case MMI_OPC_MULT1:
>      case MMI_OPC_MULTU1:
>      case MMI_OPC_MADD:
> @@ -27392,6 +27382,7 @@ static void decode_mmi(CPUMIPSState *env, DisasContext *ctx)
>      case MMI_OPC_MFHI1:
>          gen_HILO1_tx79(ctx, opc, rd);
>          break;
> +#if defined(TARGET_MIPS64)
>      case MMI_OPC_PLZCW:         /* TODO: MMI_OPC_PLZCW */
>      case MMI_OPC_PMFHL:         /* TODO: MMI_OPC_PMFHL */
>      case MMI_OPC_PMTHL:         /* TODO: MMI_OPC_PMTHL */
> @@ -27403,6 +27394,19 @@ static void decode_mmi(CPUMIPSState *env, DisasContext *ctx)
>      case MMI_OPC_PSRAW:         /* TODO: MMI_OPC_PSRAW */
>          generate_exception_end(ctx, EXCP_RI);    /* TODO: MMI_OPC_CLASS_MMI */
>          break;
> +    case MMI_OPC_CLASS_MMI0:
> +        decode_mmi0(env, ctx);
> +        break;
> +    case MMI_OPC_CLASS_MMI1:
> +        decode_mmi1(env, ctx);
> +        break;
> +    case MMI_OPC_CLASS_MMI2:
> +        decode_mmi2(env, ctx);
> +        break;
> +    case MMI_OPC_CLASS_MMI3:
> +        decode_mmi3(env, ctx);
> +        break;
> +#endif
>      default:
>          MIPS_INVAL("TX79 MMI class");
>          generate_exception_end(ctx, EXCP_RI);
> --
> 2.19.2
Fredrik Noring Jan. 16, 2019, 3:36 p.m. UTC | #2
Hi Aleksandar,

> Sorry, I have to disagree with this.

It was, without a doubt, entirely clear that the o32 ABI was going to stay
in after this MMI patch series. In other words, this series does not imply
the removal of o32. This is obvious, as discussed previously, since the o32
ABI is currently the main use case for R5900 QEMU and the reason the R5900
was implemented for QEMU to begin with.

> Processor model must stay the same, and
> Linux ABI should not affect, for example, the number and size of processor
> registers - just like it is the case in reality.

I thought Maciej's reply to you was quite clear on this topic?

> QEMU is an independent software tool - it is for example, a compiler-agnostic
> tool, and the only connection between a compiler and QEMU is the processor
> documentation - and this is the reason they work together so well. They shouldn't
> be "tweaked" and "integrated" to work together - both QEMU and compiler should
> rely only on the processor specification, and should not know anything about the
> other side.

The approach was to implement ABIs and instructions that are actually used,
and leave unused or optional instructions for later. The reverse, removing
ABIs in-use (o32) and focusing on unused instructions (MMIs) does not make
sense, so I will obviously not do that.

> My advice for you is to focus on n32 at the time being.
> 
> o32 can be implemented with the same 64-bit processor model, but in a much
> different way that you attempted some time ago. To avoid waste of our energy
> and time, I am suggesting that we finish n32, and think of o32 in future.

The o32 ABI is more important than n32 now, because o32 is in-use and
ready with Glibc, GCC, GAS and the Linux kernel. n32 is easy to enable
with a three-line patch, so we can actually use both now.

I recommend that we implement limited support for MMIs for n32 and
eventually system mode, and leave it as unsupported for o32 for the time
being. [ Perhaps MMIs for o32 could be implemented with 96-bit registers,
in contrast to the 64-bit registers for n32, but having LW and LQ without
LD seems strange to me. ]

Fredrik
Aleksandar Markovic Jan. 16, 2019, 7:20 p.m. UTC | #3
> From: Fredrik Noring <noring@nocrew.org>
> Sent: Wednesday, January 16, 2019 4:36 PM
> To: Aleksandar Markovic
> Cc: Aurelien Jarno; Philippe Mathieu-Daudé; Jürgen Urban; Maciej W. Rozycki; > qemu-devel@nongnu.org
> Subject: Re: [PATCH 1/9] target/mips: Require TARGET_MIPS64 for R5900 multimedia instructions
> 
> Hi Aleksandar,
> 
> > Sorry, I have to disagree with this.
> 
> It was, without a doubt, entirely clear that the o32 ABI was going to stay
> in after this MMI patch series. In other words, this series does not imply
> the removal of o32. This is obvious, as discussed previously, since the o32
> ABI is currently the main use case for R5900 QEMU and the reason the R5900
> was implemented for QEMU to begin with.
> 

Fredrik,

Modeling a 64-bit processor as a 32-bit one should not be a part of QEMU
upstream.

Thanks for your efforts so far,
Aleksandar

> > Processor model must stay the same, and
> > Linux ABI should not affect, for example, the number and size of processor
> > registers - just like it is the case in reality.
> 
> I thought Maciej's reply to you was quite clear on this topic?
> 
> > QEMU is an independent software tool - it is for example, a compiler-agnostic
> > tool, and the only connection between a compiler and QEMU is the processor
> > documentation - and this is the reason they work together so well. They shouldn't
> > be "tweaked" and "integrated" to work together - both QEMU and compiler should
> > rely only on the processor specification, and should not know anything about the
> > other side.
> 
> The approach was to implement ABIs and instructions that are actually used,
> and leave unused or optional instructions for later. The reverse, removing
> ABIs in-use (o32) and focusing on unused instructions (MMIs) does not make
> sense, so I will obviously not do that.
> 
> > My advice for you is to focus on n32 at the time being.
> >
> > o32 can be implemented with the same 64-bit processor model, but in a much
> > different way that you attempted some time ago. To avoid waste of our energy
> > and time, I am suggesting that we finish n32, and think of o32 in future.
> 
> The o32 ABI is more important than n32 now, because o32 is in-use and
> ready with Glibc, GCC, GAS and the Linux kernel. n32 is easy to enable
> with a three-line patch, so we can actually use both now.
> 
> I recommend that we implement limited support for MMIs for n32 and
> eventually system mode, and leave it as unsupported for o32 for the time
> being. [ Perhaps MMIs for o32 could be implemented with 96-bit registers,
> in contrast to the 64-bit registers for n32, but having LW and LQ without
> LD seems strange to me. ]
> 
> Fredrik
Jürgen Urban Jan. 20, 2019, 6:18 p.m. UTC | #4
Hello Aleksandar and Fredrik,

> Gesendet: Mittwoch, 16. Januar 2019 um 20:20 Uhr
> Von: "Aleksandar Markovic" <amarkovic@wavecomp.com>
> An: "Fredrik Noring" <noring@nocrew.org>
> Cc: "Aurelien Jarno" <aurelien@aurel32.net>, "Philippe Mathieu-Daudé" <f4bug@amsat.org>, "Jürgen Urban" <JuergenUrban@gmx.de>, "Maciej W. Rozycki" <macro@linux-mips.org>, "qemu-devel@nongnu.org" <qemu-devel@nongnu.org>
> Betreff: Re: [PATCH 1/9] target/mips: Require TARGET_MIPS64 for R5900 multimedia instructions
>
> > From: Fredrik Noring <noring@nocrew.org>
> > Sent: Wednesday, January 16, 2019 4:36 PM
> > To: Aleksandar Markovic
> > Cc: Aurelien Jarno; Philippe Mathieu-Daudé; Jürgen Urban; Maciej W. Rozycki; > qemu-devel@nongnu.org
> > Subject: Re: [PATCH 1/9] target/mips: Require TARGET_MIPS64 for R5900 multimedia instructions
> > 
> > Hi Aleksandar,
> > 
> > > Sorry, I have to disagree with this.
> > 
> > It was, without a doubt, entirely clear that the o32 ABI was going to stay
> > in after this MMI patch series. In other words, this series does not imply
> > the removal of o32. This is obvious, as discussed previously, since the o32
> > ABI is currently the main use case for R5900 QEMU and the reason the R5900
> > was implemented for QEMU to begin with.
> > 
> 
> Fredrik,
> 
> Modeling a 64-bit processor as a 32-bit one should not be a part of QEMU
> upstream.

I tried to find the patch where you have a problem with, but I wasn't able. I don't see a problem in [PATCH 1/9]. So I don't know where the actual problem is coming from.

Let me explain how what the actual system was able to do:
PS2 Linux was using 64-Bit and 128-Bit instructions with ABI o32 since the first day it was released. The toolchains which I released later was also using the feature that the CPU has 128/64-Bit registers which can be used when ABI o32 was used. It is even possible to call Linux ABI n32 syscalls from ABI o32 ELF and vice versa. I used this feature to get around some problems.
The Linux kernel which I released was able to execute MIPS III code (complete instruction set) on R5900 without any problems as long as the addresses were within 32-Bit range, e.g. in ABI n32. All missing instructions including IEEE794 compliance were handled by the Linux exception handlers. It was even able to differ between LL/SC and SQ/LQ, although the same instruction encoding is used.
It was possible to execute unmodified MIPS III ELF files without any problem.
In order to correctly emulate the real PS2 Linux which I released, QEMU has to support 64-Bit registers when using ABI o32 and it would need a way to configure whether all MIPS III instructions should be supported or not.
I.e. QEMU should be able to support ABI o32 with a 64-Bit processor. When this is not working there is a problem with the emulation and the problem is not r5900 related, as ABI o32 code is written in a way that it doesn't matter whether the CPU is 128-Bit (like R5900), 64-Bit or 32-Bit.

Best regards
Jürgen Urban

> > > Processor model must stay the same, and
> > > Linux ABI should not affect, for example, the number and size of processor
> > > registers - just like it is the case in reality.
> > 
> > I thought Maciej's reply to you was quite clear on this topic?
> > 
> > > QEMU is an independent software tool - it is for example, a compiler-agnostic
> > > tool, and the only connection between a compiler and QEMU is the processor
> > > documentation - and this is the reason they work together so well. They shouldn't
> > > be "tweaked" and "integrated" to work together - both QEMU and compiler should
> > > rely only on the processor specification, and should not know anything about the
> > > other side.
> > 
> > The approach was to implement ABIs and instructions that are actually used,
> > and leave unused or optional instructions for later. The reverse, removing
> > ABIs in-use (o32) and focusing on unused instructions (MMIs) does not make
> > sense, so I will obviously not do that.
> > 
> > > My advice for you is to focus on n32 at the time being.
> > >
> > > o32 can be implemented with the same 64-bit processor model, but in a much
> > > different way that you attempted some time ago. To avoid waste of our energy
> > > and time, I am suggesting that we finish n32, and think of o32 in future.
> > 
> > The o32 ABI is more important than n32 now, because o32 is in-use and
> > ready with Glibc, GCC, GAS and the Linux kernel. n32 is easy to enable
> > with a three-line patch, so we can actually use both now.
> > 
> > I recommend that we implement limited support for MMIs for n32 and
> > eventually system mode, and leave it as unsupported for o32 for the time
> > being. [ Perhaps MMIs for o32 could be implemented with 96-bit registers,
> > in contrast to the 64-bit registers for n32, but having LW and LQ without
> > LD seems strange to me. ]
> > 
> > Fredrik
>
diff mbox series

Patch

diff --git a/target/mips/translate.c b/target/mips/translate.c
index 057aaf9a44..a538351032 100644
--- a/target/mips/translate.c
+++ b/target/mips/translate.c
@@ -27218,6 +27218,7 @@  static void decode_opc_special3_legacy(CPUMIPSState *env, DisasContext *ctx)
     }
 }
 
+#if defined(TARGET_MIPS64)
 static void decode_mmi0(CPUMIPSState *env, DisasContext *ctx)
 {
     uint32_t opc = MASK_MMI0(ctx->opcode);
@@ -27351,6 +27352,7 @@  static void decode_mmi3(CPUMIPSState *env, DisasContext *ctx)
         break;
     }
 }
+#endif /* defined(TARGET_MIPS64) */
 
 static void decode_mmi(CPUMIPSState *env, DisasContext *ctx)
 {
@@ -27360,18 +27362,6 @@  static void decode_mmi(CPUMIPSState *env, DisasContext *ctx)
     int rd = extract32(ctx->opcode, 11, 5);
 
     switch (opc) {
-    case MMI_OPC_CLASS_MMI0:
-        decode_mmi0(env, ctx);
-        break;
-    case MMI_OPC_CLASS_MMI1:
-        decode_mmi1(env, ctx);
-        break;
-    case MMI_OPC_CLASS_MMI2:
-        decode_mmi2(env, ctx);
-        break;
-    case MMI_OPC_CLASS_MMI3:
-        decode_mmi3(env, ctx);
-        break;
     case MMI_OPC_MULT1:
     case MMI_OPC_MULTU1:
     case MMI_OPC_MADD:
@@ -27392,6 +27382,7 @@  static void decode_mmi(CPUMIPSState *env, DisasContext *ctx)
     case MMI_OPC_MFHI1:
         gen_HILO1_tx79(ctx, opc, rd);
         break;
+#if defined(TARGET_MIPS64)
     case MMI_OPC_PLZCW:         /* TODO: MMI_OPC_PLZCW */
     case MMI_OPC_PMFHL:         /* TODO: MMI_OPC_PMFHL */
     case MMI_OPC_PMTHL:         /* TODO: MMI_OPC_PMTHL */
@@ -27403,6 +27394,19 @@  static void decode_mmi(CPUMIPSState *env, DisasContext *ctx)
     case MMI_OPC_PSRAW:         /* TODO: MMI_OPC_PSRAW */
         generate_exception_end(ctx, EXCP_RI);    /* TODO: MMI_OPC_CLASS_MMI */
         break;
+    case MMI_OPC_CLASS_MMI0:
+        decode_mmi0(env, ctx);
+        break;
+    case MMI_OPC_CLASS_MMI1:
+        decode_mmi1(env, ctx);
+        break;
+    case MMI_OPC_CLASS_MMI2:
+        decode_mmi2(env, ctx);
+        break;
+    case MMI_OPC_CLASS_MMI3:
+        decode_mmi3(env, ctx);
+        break;
+#endif
     default:
         MIPS_INVAL("TX79 MMI class");
         generate_exception_end(ctx, EXCP_RI);