| Message ID | cover.1540134918.git.noring@nocrew.org |
|---|---|
| Headers | show |
| Series | target/mips: Limited support for the R5900 | expand |
> From: Fredrik Noring <noring@nocrew.org> > Subject: [PATCH v8 00/38] target/mips: Limited support for the R5900 > Hi, Fredrik. The series looks good. I added ASE_MMI flag along with INSN_R5900, I think this fits better in the overall MIPS for QEMU design. I made a couple of other minor changes. I experienced some build errors (see the end of this mail), so I had to exclude some patches, but all others are fine, and had my "Reviewed-by". 32 patches will be included in the next MIPS queue. Thanks for all efforts! Aleksandar /home/rtrk/Build/qemu-pull-request-oct-22/target/mips/translate.c: In function ‘gen_mul_txx9’: /home/rtrk/Build/qemu-pull-request-oct-22/target/mips/translate.c:4888:38: error: passing argument 3 of ‘tcg_gen_add2_i32’ from incompatible pointer type [-Werror=incompatible-pointer-types] tcg_gen_add2_i32(t2, t3, cpu_LO[acc], cpu_HI[acc], t2, t3); ^~~~~~ In file included from /home/rtrk/Build/qemu-pull-request-oct-22/target/mips/translate.c:29:0: /home/rtrk/Build/qemu-pull-request-oct-22/tcg/tcg-op.h:314:6: note: expected ‘TCGv_i32 {aka struct TCGv_i32_d *}’ but argument is of type ‘TCGv_i64 {aka struct TCGv_i64_d *}’ void tcg_gen_add2_i32(TCGv_i32 rl, TCGv_i32 rh, TCGv_i32 al, ^~~~~~~~~~~~~~~~ /home/rtrk/Build/qemu-pull-request-oct-22/target/mips/translate.c:4888:51: error: passing argument 4 of ‘tcg_gen_add2_i32’ from incompatible pointer type [-Werror=incompatible-pointer-types] tcg_gen_add2_i32(t2, t3, cpu_LO[acc], cpu_HI[acc], t2, t3); ^~~~~~ In file included from /home/rtrk/Build/qemu-pull-request-oct-22/target/mips/translate.c:29:0: /home/rtrk/Build/qemu-pull-request-oct-22/tcg/tcg-op.h:314:6: note: expected ‘TCGv_i32 {aka struct TCGv_i32_d *}’ but argument is of type ‘TCGv_i64 {aka struct TCGv_i64_d *}’ void tcg_gen_add2_i32(TCGv_i32 rl, TCGv_i32 rh, TCGv_i32 al, ^~~~~~~~~~~~~~~~ /home/rtrk/Build/qemu-pull-request-oct-22/target/mips/translate.c:4908:38: error: passing argument 3 of ‘tcg_gen_add2_i32’ from incompatible pointer type [-Werror=incompatible-pointer-types] tcg_gen_add2_i32(t2, t3, cpu_LO[acc], cpu_HI[acc], t2, t3); ^~~~~~ In file included from /home/rtrk/Build/qemu-pull-request-oct-22/target/mips/translate.c:29:0: /home/rtrk/Build/qemu-pull-request-oct-22/tcg/tcg-op.h:314:6: note: expected ‘TCGv_i32 {aka struct TCGv_i32_d *}’ but argument is of type ‘TCGv_i64 {aka struct TCGv_i64_d *}’ void tcg_gen_add2_i32(TCGv_i32 rl, TCGv_i32 rh, TCGv_i32 al, ^~~~~~~~~~~~~~~~ /home/rtrk/Build/qemu-pull-request-oct-22/target/mips/translate.c:4908:51: error: passing argument 4 of ‘tcg_gen_add2_i32’ from incompatible pointer type [-Werror=incompatible-pointer-types] tcg_gen_add2_i32(t2, t3, cpu_LO[acc], cpu_HI[acc], t2, t3); ^~~~~~ In file included from /home/rtrk/Build/qemu-pull-request-oct-22/target/mips/translate.c:29:0: /home/rtrk/Build/qemu-pull-request-oct-22/tcg/tcg-op.h:314:6: note: expected ‘TCGv_i32 {aka struct TCGv_i32_d *}’ but argument is of type ‘TCGv_i64 {aka struct TCGv_i64_d *}’ void tcg_gen_add2_i32(TCGv_i32 rl, TCGv_i32 rh, TCGv_i32 al, ^~~~~~~~~~~~~~~~ cc1: all warnings being treated as errors /home/rtrk/Build/qemu-pull-request-oct-22/rules.mak:69: recipe for target 'target/mips/translate.o' failed make[1]: *** [target/mips/translate.o] Error 1 Makefile:483: recipe for target 'subdir-mips64-softmmu' failed make: *** [subdir-mips64-softmmu] Error 2
Many thanks, Aleksandar, > I added ASE_MMI flag along with INSN_R5900, I think this fits better in > the overall MIPS for QEMU design. Maciej -- can we add "MMI" under "ASEs implemented" in the kernel too, even if it is a vendor-specific architecture extension that normally isn't counted as an ASE? QEMU simply calls these "vendor specific ASEs". Aleksandar -- please or ASE_MMI to insn_flags here: --- a/target/mips/translate_init.inc.c +++ b/target/mips/translate_init.inc.c @@ -466,7 +466,7 @@ const mips_def_t mips_defs[] = #endif /* !CONFIG_USER_ONLY */ .SEGBITS = 32, .PABITS = 32, - .insn_flags = CPU_R5900, + .insn_flags = CPU_R5900 | ASE_MMI, .mmu_type = MMU_TYPE_R4000, }, { Strictly speaking, MADD, MADDU, MULT, MULTU, MULT1, MULTU1, DIV1, DIVU1, MADD1, MADDU1, MFHI1, MFLO1, MTHI1 and MTLO1 are not part of what the Toshiba TX System RISC TX79 Core Architecture manual specifies as "Multimedia Instructions", section B.3.2, on page B-3, even though their opcodes are covered by TX79_CLASS_MMI and the decode_tx79_mmi function. Can we adjust ASE_MMI for QEMU accordingly? Also, doesn't it make sense to cover LQ and SQ with ASE_MMI as well, as those two really are MMIs? Finally, as far as I know, the MMIs cannot be disabled on R5900 hardware. --- a/target/mips/translate.c +++ b/target/mips/translate.c @@ -26099,7 +26099,7 @@ static void decode_opc(CPUMIPSState *env, DisasContext *ctx) } break; case OPC_SPECIAL3: - if (ctx->insn_flags & INSN_R5900) { + if ((ctx->insn_flags & INSN_R5900) && (ctx->insn_flags & ASE_MMI)) { decode_tx79_sq(env, ctx); /* TX79_SQ */ } else { decode_opc_special3(env, ctx); @@ -26763,7 +26763,7 @@ static void decode_opc(CPUMIPSState *env, DisasContext *ctx) } break; case OPC_MSA: /* OPC_MDMX */ - if (ctx->insn_flags & INSN_R5900) { + if ((ctx->insn_flags & INSN_R5900) && (ctx->insn_flags & ASE_MMI)) { decode_tx79_lq(env, ctx); /* TX79_LQ */ } else { /* MDMX: Not implemented. */ > I experienced some build errors (see the end of this mail), so I had to > exclude some patches, but all others are fine, and had my "Reviewed-by". > 32 patches will be included in the next MIPS queue. Ah, I didn't test the 64-bit build on the MADD[U][1] instructions. I will look into them and post updated patches. Regarding the R5900 FPU: It appears reasonable to introduce an ELF ABI variant for the nonstandard R5900 FPU. A testsuite covering the anomalies seems to be needed as well. Careful verification on hardware is needed. I think it's probably best to keep the R5900 FPU disabled in QEMU until these things have been sorted out. I discovered that I lost the disassembly of MULT1 and MULTU1 in v8, as shown in the attached patch below. This small change belongs to commit bebf09ef3977 ("target/mips: Support R5900 three-operand MULT1 and MULTU1 instructions") in your tags/mips-queue-oct-2018-part-2. Please apply: --- a/disas/mips.c +++ b/disas/mips.c @@ -2736,10 +2736,14 @@ const struct mips_opcode mips_builtin_opcodes[] = {"mult", "s,t", 0x00000018, 0xfc00ffff, RD_s|RD_t|WR_HILO|IS_M, 0, I1 }, {"mult", "7,s,t", 0x00000018, 0xfc00e7ff, WR_a|RD_s|RD_t, 0, D33 }, {"mult", "d,s,t", 0x00000018, 0xfc0007ff, RD_s|RD_t|WR_HILO|WR_d|IS_M, 0, G1 }, +{"mult1", "s,t", 0x70000018, 0xfc00ffff, RD_s | RD_t | WR_HILO | IS_M, 0, EE }, +{"mult1", "d,s,t", 0x70000018, 0xfc0007ff, WR_d | RD_s | RD_t | WR_HILO | IS_M, 0, EE }, {"multp", "s,t", 0x00000459, 0xfc00ffff, RD_s|RD_t|MOD_HILO, 0, SMT }, {"multu", "s,t", 0x00000019, 0xfc00ffff, RD_s|RD_t|WR_HILO|IS_M, 0, I1 }, {"multu", "7,s,t", 0x00000019, 0xfc00e7ff, WR_a|RD_s|RD_t, 0, D33 }, {"multu", "d,s,t", 0x00000019, 0xfc0007ff, RD_s|RD_t|WR_HILO|WR_d|IS_M, 0, G1 }, +{"multu1", "s,t", 0x70000019, 0xfc00ffff, RD_s | RD_t | WR_HILO | IS_M, 0, EE }, +{"multu1", "d,s,t", 0x70000019, 0xfc0007ff, WR_d | RD_s | RD_t | WR_HILO | IS_M, 0, EE }, {"mulu", "d,s,t", 0x00000059, 0xfc0007ff, RD_s|RD_t|WR_HILO|WR_d, 0, N5 }, {"neg", "d,w", 0x00000022, 0xffe007ff, WR_d|RD_t, 0, I1 }, /* sub 0 */ {"negu", "d,w", 0x00000023, 0xffe007ff, WR_d|RD_t, 0, I1 }, /* subu 0 */ Fredrik
> From: Fredrik Noring <noring@nocrew.org> > > Subject: Re: [PATCH v8 00/38] target/mips: Limited support for the R5900 > > Many thanks, Aleksandar, > > > I added ASE_MMI flag along with INSN_R5900, I think this fits better in > > the overall MIPS for QEMU design. > > Maciej -- can we add "MMI" under "ASEs implemented" in the kernel too, > even if it is a vendor-specific architecture extension that normally > isn't counted as an ASE? QEMU simply calls these "vendor specific ASEs". > > Aleksandar -- please or ASE_MMI to insn_flags here: > > --- a/target/mips/translate_init.inc.c > +++ b/target/mips/translate_init.inc.c > @@ -466,7 +466,7 @@ const mips_def_t mips_defs[] = > #endif /* !CONFIG_USER_ONLY */ > .SEGBITS = 32, > .PABITS = 32, > - .insn_flags = CPU_R5900, > + .insn_flags = CPU_R5900 | ASE_MMI, > .mmu_type = MMU_TYPE_R4000, > }, > { > Hi, Fredrik. I understood what you said about ASE_MMI and other changes you want to be included. Pull request with 32 patches from this series is already sent, and I would like to avoid sending v2 of that request. Let's wait for some time until the pull request is hopefully accepted. There will be most likely another one at the beginning of the next week. We are appoaching QEMU 3.1 soft freeze (Oct 30), and at this point we would like to stabilize the code, and to integrate only crucial patches. I suggest that you create a new series "target/mips: Amend R5900 support". It should be based on the code submitted in the pull request. Place the most crucial patches as the first ones, at the beginning of the series. Less important at the end. FPU changes are too risky at this stage od 3.1 development cycle, and I would leave them for QEMU 3.2+. Regards and thanks again, Aleksandar > Strictly speaking, MADD, MADDU, MULT, MULTU, MULT1, MULTU1, DIV1, DIVU1, > MADD1, MADDU1, MFHI1, MFLO1, MTHI1 and MTLO1 are not part of what the > Toshiba TX System RISC TX79 Core Architecture manual specifies as > "Multimedia Instructions", section B.3.2, on page B-3, even though > their opcodes are covered by TX79_CLASS_MMI and the decode_tx79_mmi > function. Can we adjust ASE_MMI for QEMU accordingly? > > Also, doesn't it make sense to cover LQ and SQ with ASE_MMI as well, as > those two really are MMIs? > > Finally, as far as I know, the MMIs cannot be disabled on R5900 hardware. > > --- a/target/mips/translate.c > +++ b/target/mips/translate.c > @@ -26099,7 +26099,7 @@ static void decode_opc(CPUMIPSState *env, DisasContext *ctx) > } > break; > case OPC_SPECIAL3: > - if (ctx->insn_flags & INSN_R5900) { > + if ((ctx->insn_flags & INSN_R5900) && (ctx->insn_flags & ASE_MMI)) { > decode_tx79_sq(env, ctx); /* TX79_SQ */ > } else { > decode_opc_special3(env, ctx); > @@ -26763,7 +26763,7 @@ static void decode_opc(CPUMIPSState *env, DisasContext *ctx) > } > break; > case OPC_MSA: /* OPC_MDMX */ > - if (ctx->insn_flags & INSN_R5900) { > + if ((ctx->insn_flags & INSN_R5900) && (ctx->insn_flags & ASE_MMI)) { > decode_tx79_lq(env, ctx); /* TX79_LQ */ > } else { > /* MDMX: Not implemented. */ > > > I experienced some build errors (see the end of this mail), so I had to > > exclude some patches, but all others are fine, and had my "Reviewed-by". > > 32 patches will be included in the next MIPS queue. > > Ah, I didn't test the 64-bit build on the MADD[U][1] instructions. I will > look into them and post updated patches. > > Regarding the R5900 FPU: It appears reasonable to introduce an ELF ABI > variant for the nonstandard R5900 FPU. A testsuite covering the anomalies > seems to be needed as well. Careful verification on hardware is needed. > I think it's probably best to keep the R5900 FPU disabled in QEMU until > these things have been sorted out. > > I discovered that I lost the disassembly of MULT1 and MULTU1 in v8, as > shown in the attached patch below. This small change belongs to commit > bebf09ef3977 ("target/mips: Support R5900 three-operand MULT1 and MULTU1 > instructions") in your tags/mips-queue-oct-2018-part-2. Please apply: > > --- a/disas/mips.c > +++ b/disas/mips.c > @@ -2736,10 +2736,14 @@ const struct mips_opcode mips_builtin_opcodes[] = > {"mult", "s,t", 0x00000018, 0xfc00ffff, RD_s|RD_t|WR_HILO|IS_M, 0, > I1 }, > {"mult", "7,s,t", 0x00000018, 0xfc00e7ff, WR_a|RD_s|RD_t, 0, > D33 }, > {"mult", "d,s,t", 0x00000018, 0xfc0007ff, RD_s|RD_t|WR_HILO|WR_d|IS_M, > 0, G1 }, > +{"mult1", "s,t", 0x70000018, 0xfc00ffff, RD_s | RD_t | WR_HILO | IS_M, 0, EE }, > +{"mult1", "d,s,t", 0x70000018, 0xfc0007ff, WR_d | RD_s | RD_t | WR_HILO | IS_M, 0, > EE }, > {"multp", "s,t", 0x00000459, 0xfc00ffff, RD_s|RD_t|MOD_HILO, 0, > SMT }, > {"multu", "s,t", 0x00000019, 0xfc00ffff, RD_s|RD_t|WR_HILO|IS_M, 0, > I1 }, > {"multu", "7,s,t", 0x00000019, 0xfc00e7ff, WR_a|RD_s|RD_t, 0, > D33 }, > {"multu", "d,s,t", 0x00000019, 0xfc0007ff, RD_s|RD_t|WR_HILO|WR_d|IS_M, > 0, G1 }, > +{"multu1", "s,t", 0x70000019, 0xfc00ffff, RD_s | RD_t | WR_HILO | IS_M, 0, EE }, > +{"multu1", "d,s,t", 0x70000019, 0xfc0007ff, WR_d | RD_s | RD_t | WR_HILO | IS_M, 0, > EE }, > {"mulu", "d,s,t", 0x00000059, 0xfc0007ff, RD_s|RD_t|WR_HILO|WR_d, 0, > N5 }, > {"neg", "d,w", 0x00000022, 0xffe007ff, WR_d|RD_t, 0, > I1 }, /* sub 0 */ > {"negu", "d,w", 0x00000023, 0xffe007ff, WR_d|RD_t, 0, > I1 }, /* subu 0 */ > > Fredrik >
Hi Maciej, > > I added ASE_MMI flag along with INSN_R5900, I think this fits better in > > the overall MIPS for QEMU design. > > Maciej -- can we add "MMI" under "ASEs implemented" in the kernel too, > even if it is a vendor-specific architecture extension that normally > isn't counted as an ASE? QEMU simply calls these "vendor specific ASEs". I have no authority to approve such a change for the kernel, but it looks reasonable to me and I will support you with it, with one reservation however. As this is an ISA extension in the vendor-specific space, I think it belongs to a vendor-specific namespace, so as to make it clear it is not a generic architectural feature and also to avoid name clashes. So it has to be called Toshiba MMI or suchlike, similarly to how I requested that for the Longsoon MMI feature in a recent binutils review (cf <https://sourceware.org/ml/binutils/2018-07/msg00201.html> and binutils commit 8095d2f70e1a ("MIPS/GAS: Split Loongson MMI Instructions from loongson2f/3a")), with all the consequences throughout. > Aleksandar -- please or ASE_MMI to insn_flags here: > > --- a/target/mips/translate_init.inc.c > +++ b/target/mips/translate_init.inc.c > @@ -466,7 +466,7 @@ const mips_def_t mips_defs[] = > #endif /* !CONFIG_USER_ONLY */ > .SEGBITS = 32, > .PABITS = 32, > - .insn_flags = CPU_R5900, > + .insn_flags = CPU_R5900 | ASE_MMI, > .mmu_type = MMU_TYPE_R4000, > }, > { So I think it better be called ASE_TOSHIBA_MMI here. > Strictly speaking, MADD, MADDU, MULT, MULTU, MULT1, MULTU1, DIV1, DIVU1, > MADD1, MADDU1, MFHI1, MFLO1, MTHI1 and MTLO1 are not part of what the > Toshiba TX System RISC TX79 Core Architecture manual specifies as > "Multimedia Instructions", section B.3.2, on page B-3, even though > their opcodes are covered by TX79_CLASS_MMI and the decode_tx79_mmi > function. Can we adjust ASE_MMI for QEMU accordingly? NB all but pipeline 1 instructions of these are also implemented by other members of the TXx9 family. They seem to be referred to as just "multiply and multiply-add instructions" in the TX79 manual (cf Section B.3.1). > Also, doesn't it make sense to cover LQ and SQ with ASE_MMI as well, as > those two really are MMIs? And they're certainly listed as such in the TX79 manual (cf Section B.3.2). > Regarding the R5900 FPU: It appears reasonable to introduce an ELF ABI > variant for the nonstandard R5900 FPU. Indeed and in particular given that the R5900 does not produce any FPU exceptions it should be quite straightforward for the Linux kernel to recognise this specific ABI annotation with ELF binaries and switch its FP environment between R5900 native float and IEEE 754 emulated float accordingly. We could then make QEMU run in the user emulation mode do the same. Of course all the pieces of the toolchain as well as the dynamic loader in use would have to taught to prevent incompatible pieces of hard float code from being used together. Maciej
On Mon, 22 Oct 2018, Maciej W. Rozycki wrote:
> Hi Maciej,
What an odd copy & paste thinko! I can't believe I addressed myself in
the opening of my e-mail. :)
Maciej
Hi Aleksandar, > Pull request with 32 patches from this series is already sent, and I would > like to avoid sending v2 of that request. Let's wait for some time until > the pull request is hopefully accepted. There will be most likely another > one at the beginning of the next week. > > We are appoaching QEMU 3.1 soft freeze (Oct 30), and at this point we > would like to stabilize the code, and to integrate only crucial patches. > I suggest that you create a new series "target/mips: Amend R5900 support". > It should be based on the code submitted in the pull request. Place the > most crucial patches as the first ones, at the beginning of the series. The R5900 testsuite in tests/tcg/mips/mipsr5900 fails unless ASE_MMI is part of insn_flags for the R5900: --- a/target/mips/translate_init.inc.c +++ b/target/mips/translate_init.inc.c @@ -466,7 +466,7 @@ const mips_def_t mips_defs[] = #endif /* !CONFIG_USER_ONLY */ .SEGBITS = 32, .PABITS = 32, - .insn_flags = CPU_R5900, + .insn_flags = CPU_R5900 | ASE_MMI, .mmu_type = MMU_TYPE_R4000, }, { Perhaps that is the only (somewhat) crucial problem, depending on how the testsuites are used, of course. The other ASE_MMI changes and the disassembly of MULT1 and MULTU1 can wait, as can R5900 support for MADD, MADDU, MADD1 and MADDU1, in my opinion. > FPU changes are too risky at this stage od 3.1 development cycle, and I > would leave them for QEMU 3.2+. Agreed! As Maciej just noted, there are also toolchain issues that need to be addressed for the R5900 FPU. Fredrik
On 22/10/18 20:40, Maciej W. Rozycki wrote: > On Mon, 22 Oct 2018, Maciej W. Rozycki wrote: > >> Hi Maciej, > > What an odd copy & paste thinko! I can't believe I addressed myself in > the opening of my e-mail. :) Haha when I saw your mail I thought "weird, there is another Maciej involved in this MIPS specific thread!?"
On Mon, Oct 22, 2018 at 3:34 PM Aleksandar Markovic <amarkovic@wavecomp.com> wrote: > > From: Fredrik Noring <noring@nocrew.org> > > Subject: [PATCH v8 00/38] target/mips: Limited support for the R5900 > > > I experienced some build errors (see the end of this mail), so I had to exclude some patches, but all others are fine, and had my "Reviewed-by". 32 patches will be included in the next MIPS queue. Thank you a lot Aleksandar for taking this series! I appreciate a lot, being backed by a company, you also care about hobbyist contributions. This is not always an easy task for maintainers, and from the hobbyist point of view, this means a lot to me. Regards, Phil.
Hi Maciej, > I have no authority to approve such a change for the kernel, but it looks > reasonable to me and I will support you with it, with one reservation > however. As this is an ISA extension in the vendor-specific space, I > think it belongs to a vendor-specific namespace, so as to make it clear it > is not a generic architectural feature and also to avoid name clashes. > > So it has to be called Toshiba MMI or suchlike, similarly to how I > requested that for the Longsoon MMI feature in a recent binutils review > (cf <https://sourceware.org/ml/binutils/2018-07/msg00201.html> and > binutils commit 8095d2f70e1a ("MIPS/GAS: Split Loongson MMI Instructions > from loongson2f/3a")), with all the consequences throughout. Vendor ASE namespaces makes sense to me. I can prepare a patch for it. > NB all but pipeline 1 instructions of these are also implemented by other > members of the TXx9 family. They seem to be referred to as just "multiply > and multiply-add instructions" in the TX79 manual (cf Section B.3.1). Would ASE_TOSHIBA_MMI -- TX79 128-bit multimedia instructions ASE_TOSHIBA_MAC -- TXx9 multiply and multiply-add instructions (MADD etc.) ASE_TOSHIBA_MAC1 -- TX79 pipeline 1 variant of ASE_TOSHIBA_MAC ASE_TOSHIBA_FMA -- R5900 FPU extensions (MADD.s etc.) be acceptable for the currently known Toshiba extensions? (Please propose better names.) One complication is that it seems only 8 bits are available for all vendor ASEs, and Toshiba would then scoop up half of those. Fredrik
Hi Maciej, > target/mips/translate.c:4888:38: error: passing argument 3 of > ‘tcg_gen_add2_i32’ from incompatible pointer type > [-Werror=incompatible-pointer-types] > tcg_gen_add2_i32(t2, t3, cpu_LO[acc], cpu_HI[acc], t2, t3); > ^~~~~~ Would you know if any MIPS ISA have LO and HI registers that are not 32-bit? In QEMU they can obviously be either 32-bit or 64-bit, which causes the compilation error here. Fredrik
Hi Fredrik, > > target/mips/translate.c:4888:38: error: passing argument 3 of > > ‘tcg_gen_add2_i32’ from incompatible pointer type > > [-Werror=incompatible-pointer-types] > > tcg_gen_add2_i32(t2, t3, cpu_LO[acc], cpu_HI[acc], t2, t3); > > ^~~~~~ > > Would you know if any MIPS ISA have LO and HI registers that are not > 32-bit? In QEMU they can obviously be either 32-bit or 64-bit, which > causes the compilation error here. Actually with all 64-bit MIPS ISAs HI/LO are a pair of 64-bit registers, that is with MIPS III, MIPS IV, and then MIPS64 R1 to R5 ISAs (base R6 ISA removed the MD accumulator, although it has been retained along with the 3 other ones in the DSP ASE). The R5900 CPU is an oddball here, having no 64-bit multiply or divide instructions, however documentation indicates these registers are still 64-bit as far as the base instruction set is concerned, i.e. it says you can actually write the upper halves with any bit patterns explicitly with the MTHI and MTLO instructions. And then they're really 128-bit as far as the full instruction set of the R5900 is concerned, for all the pipeline 1 MD instructions operate on bits 95:64 and some MMI instructions operate on the full 128-bit width of the accumulator. Interestingly enough architecturally trying to use HI/LO values that are not properly sign-extended 32-bit numbers does not make the operation of 32-bit multiply-accumulate instructions unpredictable, as they are specified to simply ignore the upper 32 bits of a 64-bit value contained there, and the the TX79 manual follows. This is unlike with the GPR inputs to all MD instructions, which architecturally have to be sign-extended. Contrariwise, the TX79 manual says that GPR inputs to the unsigned variants of MD instructions have to be zero-extended, and I do hope this is just an editorial mistake and hardware does not follow (especially as the description of MULTU on page A-87 disagress in this regard with one on page B-25, and all the relevant pseudocode operation specifications consistently use NotWordValue as the input validation condition, although that has been nowhere actually formally defined). Otherwise lots of software would break and you'd have to use a DSLL32/DSRL32 instruction pair every time before feeding the result of other 32-bit calculations to those instructions. BTW, notice that the pseudocode operation specification of the TX79 MD instructions does clearly indicate the sign-extension of output HI/LO contents, e.g. for MULTU we have: prod <- (0 || GPR[rs]31..0) * (0 || GPR[rt]31..0) LO63..0 <- (prod 31)32 || prod31..0 HI63..0 <- (prod 63)32 || prod63..32 GPR[rd]63..0 <- (prod 31)32 || prod31..0 HTH, Maciej
Hi Fredrik, > > NB all but pipeline 1 instructions of these are also implemented by other > > members of the TXx9 family. They seem to be referred to as just "multiply > > and multiply-add instructions" in the TX79 manual (cf Section B.3.1). > > Would > > ASE_TOSHIBA_MMI -- TX79 128-bit multimedia instructions > ASE_TOSHIBA_MAC -- TXx9 multiply and multiply-add instructions (MADD etc.) > ASE_TOSHIBA_MAC1 -- TX79 pipeline 1 variant of ASE_TOSHIBA_MAC > ASE_TOSHIBA_FMA -- R5900 FPU extensions (MADD.s etc.) > > be acceptable for the currently known Toshiba extensions? (Please propose > better names.) One complication is that it seems only 8 bits are available > for all vendor ASEs, and Toshiba would then scoop up half of those. I'm not sure if every single random vendor-specific instruction (or a bunch of) deserves its own ASE designation, be it internal or externally exposed. I think the MMI set being a substantial architectural feature makes sense to be shown in /proc/cpuinfo (in Linux), but I don't think there's much more about it. It's limited to 2 implementations only, so internally I think it can well be handled with a macro or static inline function (as appropriate) which boil down to (CPU_R5900 || CPU_TX79). And if you run out of bits for ASEs regardless, then I suggest just to expand the field in question. In QEMU you can rely on the presence of the `uint64_t' data type, so with only 8 bits exhausted you're far from getting into trouble. Maciej
Hi Maciej, > I'm not sure if every single random vendor-specific instruction (or a > bunch of) deserves its own ASE designation, be it internal or externally > exposed. I think the MMI set being a substantial architectural feature > makes sense to be shown in /proc/cpuinfo (in Linux), but I don't think > there's much more about it. It's limited to 2 implementations only, so > internally I think it can well be handled with a macro or static inline > function (as appropriate) which boil down to (CPU_R5900 || CPU_TX79). Are there benefits in leaving out features? Their utility, such as in choosing compiler options, may not correlate with their (lack of) architectural weight. A random pc, for instance, comes fully dressed flying the flags of fpu vme de pse tsc msr pae mce cx8 apic sep mtrr pge mca cmov pat pse36 clflush dts acpi mmx fxsr sse sse2 ss ht tm pbe syscall nx rdtscp lm constant_tsc arch_perfmon pebs bts rep_good nopl xtopology nonstop_tsc aperfmperf eagerfpu pni pclmulqdq dtes64 monitor ds_cpl vmx smx est tm2 ssse3 cx16 xtpr pdcm pcid sse4_1 sse4_2 x2apic popcnt tsc_deadline_timer aes xsave avx lahf_lm epb kaiser tpr_shadow vnmi flexpriority ept vpid xsaveopt dtherm ida arat pln pts in its /proc/cpuinfo. It also has a bugs field with cpu_meltdown spectre_v1 spectre_v2 where the R5900 could have an entry for its short loop bug. > And if you run out of bits for ASEs regardless, then I suggest just to > expand the field in question. In QEMU you can rely on the presence of the > `uint64_t' data type, so with only 8 bits exhausted you're far from > getting into trouble. DisasContext::insn_flags is already uint64_t, where bits 63..56 are reserved for vendor-specific ASEs. Of course, one could organise them differently, especially since they may be mutually exclusive, or one could use a new ASE-specific field for them. Fredrik
The primary purpose of these changes is to support programs compiled by GCC for the R5900 target and thereby run R5900 Linux distributions, for example Gentoo. GCC in version 7.3, by itself, by inspection of the GCC source code and inspection of the generated machine code, for the R5900 target, only emits two instructions that are specific to the R5900: the three- operand MULT and MULTU. GCC and libc also emit certain MIPS III instructions that are not part of the R5900 ISA. They are normally trapped and emulated by the Linux kernel, and therefore need to be treated accordingly by QEMU. This is addressed, in turn, by the patch series. A program compiled by GCC is taken to mean source code compiled by GCC under the restrictions above. One can, with the apparent limitations, with a bit of effort obtain a fully functioning operating system such as R5900 Gentoo. Strictly speaking, programs need not be compiled by GCC to make use of this change. Instructions and other facilities of the R5900 not implemented by these changes are intended to signal provisional exceptions. One such example is the FPU that is not compliant with IEEE 754-1985 in system mode. It is therefore provisionally disabled. In user space the FPU is trapped and emulated by IEEE 754-1985 compliant software in the kernel, and this is handled accordingly by QEMU. Another example is the 93 multimedia instructions specific to the R5900 that generate provisional reserved instruction exception signals. One of the benefits of running a Linux distribution under QEMU is that programs can be compiled with a native compiler, where the host and target are the same, as opposed to a cross-compiler, where they are not the same. This is especially important in cases where the target hardware does not have the resources to run a native compiler. Problems with cross-compilation are often related to host and target differences in integer sizes, pointer sizes, endianness, machine code, ABI, etc. Sometimes cross-compilation is not even supported by the build script for a given package. One effective way to avoid those problems is to replace the cross-compiler with a native compiler. This change of compilation methods does not resolve the inherent problems with cross-compilation. The native compiler naturally replaces the cross-compiler, because one typically uses one or the other, and preferably the native compiler when the circumstances admit this. The native compiler is also a good test case for the R5900 QEMU user mode. Additionally, Gentoo is well- known for compiling and installing its packages from sources. This change has been tested with Gentoo compiled for R5900, including native compilation of several packages under QEMU. I used the Gentoo sys-devel/crossdev package https://wiki.gentoo.org/wiki/Crossdev with patches mainly to simplify the handling of LL/SC and floating point support, to avoid complications with additional configure and compiler flags. Busybox https://busybox.net/ can also be used to build a simple functional R5900 program. It can be used to test the R5900 CPU in QEMU user mode. The R5900 implements the 64-bit MIPS III instruction set except DMULT, DMULTU, DDIV, DDIVU, LL, SC, LLD and SCD. The MIPS IV instructions MOVN, MOVZ and PREF are implemented. It has the R5900 specific three-operand instructions MADD, MADDU, MULT and MULTU as well as pipeline 1 versions MULT1, MULTU1, DIV1, DIVU1, MADD1, MADDU1, MFHI1, MFLO1, MTHI1 and MTLO1. A set of 93 128-bit multimedia instructions specific to the R5900 is also implemented. The Toshiba TX System RISC TX79 Core Architecture manual https://wiki.qemu.org/File:C790.pdf describes the C790 processor that is a follow-up to the R5900. There are a few notable differences in that the R5900 FPU - is not IEEE 754-1985 compliant, - does not implement double format, and - its machine code is nonstandard. Changes in v8: - Support, disassembly and tests for MADD, MADD1, MADDU and MADDU1 - Support, disassembly and tests for MTLO1, MTHI1, MFLO1 and MFHI1 - Support, disassembly and tests for MULT1, MULTU1, DIV1 and DIVU1 - Opcode definitions and placeholder code for all unsupported MMIs - check_insn_opc_user_only flags parameter type is uint64_t - Toshiba TX System RISC TX79 manual PDF QEMU wiki link - Merge of [PATCH v7 7/7] with Toshiba/Sony rename Changes in v7: - Rename gen_mul_txxx to gen_mul_txx9 - Use MIPS_INVAL("mul TXx9") - Reviewed-by: Philippe Mathieu-Daudé Changes in v6: - Set the CP0 PRId implementation number to 0x2E for the R5900 - Refer to the C790 follow-up in the definition of the R5900 - Define and use check_insn_opc_user_only in the same change - Rename gen_mul_r5900 to gen_mul_txxx - Enclose single statements in braces - Expand and reword commit messages and notes - Reword the cover letter subject line - All changes build with GCC and Clang - Approval from checkpatch.pl Changes in v5: - Reorder check_insn_opc_user_only calls - Call check_insn_opc_removed in check_insn_opc_user_only Changes in v4: - Split into a patch series consisting of eight changes - Expand commit messages and notes - Introduce check_insn_opc_user_only - Base R5900 on MIPS III, with MOVN, MOVZ and PREF from MIPS IV - DMULT, DMULTU, DDIV, DDIVU, LL, SC, LLD and SCD are user only - Note Toshiba/Sony R5900 for EF_MIPS_MACH_R5900 definition - Rework gen_mul_r5900 - Fix ICE and DCE - Fix SEGBITS and PABITS - Fix indentation Changes in v3: - Apply to HEAD - Remove the word "initial" from subject line Changes in v2: - Update mips_defs array with R5900 values - LL/SC and FPU are user only Fredrik Noring (38): target/mips: Define R5900 instructions and CPU preprocessor constants disas/mips: Define R5900 disassembly constants target/mips: R5900 Multimedia Instruction overview note target/mips: Define R5900 MMI class, and LQ and SQ opcode constants target/mips: Define R5900 MMI{0,1,2,3} subclasses and MMI opcode constants target/mips: Define R5900 MMI0 opcode constants target/mips: Define R5900 MMI1 opcode constants target/mips: Define R5900 MMI2 opcode constants target/mips: Define R5900 MMI3 opcode constants target/mips: Placeholder for R5900 MMI SQ, handle user mode RDHWR target/mips: Placeholder for R5900 MMI LQ target/mips: Placeholder for R5900 MMI instruction class target/mips: Placeholder for R5900 MMI0 instruction subclass target/mips: Placeholder for R5900 MMI1 instruction subclass target/mips: Placeholder for R5900 MMI2 instruction subclass target/mips: Placeholder for R5900 MMI3 instruction subclass target/mips: Support R5900 three-operand MULT and MULTU target/mips: Support R5900 three-operand MULT1 and MULTU1 target/mips: Support R5900 MFLO1, MTLO1, MFHI1 and MTHI1 target/mips: Support R5900 DIV1 and DIVU1 target/mips: Support R5900 MOVN, MOVZ and PREF from MIPS IV target/mips: Support R5900 three-operand MADD and MADD1 target/mips: Support R5900 three-operand MADDU and MADDU1 target/mips: R5900 DMULT[U], DDIV[U], LL[D] and SC[D] are user only tests/tcg/mips: Test R5900 three-operand MULT tests/tcg/mips: Test R5900 three-operand MULTU tests/tcg/mips: Test R5900 three-operand MULT1 tests/tcg/mips: Test R5900 three-operand MULTU1 tests/tcg/mips: Test R5900 MFLO1 and MFHI1 tests/tcg/mips: Test R5900 MTLO1 and MTHI1 tests/tcg/mips: Test R5900 DIV1 tests/tcg/mips: Test R5900 DIVU1 tests/tcg/mips: Test R5900 three-operand MADD tests/tcg/mips: Test R5900 three-operand MADD1 tests/tcg/mips: Test R5900 three-operand MADDU tests/tcg/mips: Test R5900 three-operand MADDU1 target/mips: Define the R5900 CPU linux-user/mips: Recognise the R5900 CPU model disas/mips.c | 20 + linux-user/mips/target_elf.h | 3 + target/mips/mips-defs.h | 2 + target/mips/translate.c | 871 ++++++++++++++++++++++++++++- target/mips/translate_init.inc.c | 59 ++ tests/tcg/mips/mipsr5900/Makefile | 32 ++ tests/tcg/mips/mipsr5900/div1.c | 73 +++ tests/tcg/mips/mipsr5900/divu1.c | 48 ++ tests/tcg/mips/mipsr5900/madd.c | 78 +++ tests/tcg/mips/mipsr5900/maddu.c | 70 +++ tests/tcg/mips/mipsr5900/mflohi1.c | 35 ++ tests/tcg/mips/mipsr5900/mtlohi1.c | 40 ++ tests/tcg/mips/mipsr5900/mult.c | 76 +++ tests/tcg/mips/mipsr5900/multu.c | 68 +++ 14 files changed, 1465 insertions(+), 10 deletions(-) create mode 100644 tests/tcg/mips/mipsr5900/Makefile create mode 100644 tests/tcg/mips/mipsr5900/div1.c create mode 100644 tests/tcg/mips/mipsr5900/divu1.c create mode 100644 tests/tcg/mips/mipsr5900/madd.c create mode 100644 tests/tcg/mips/mipsr5900/maddu.c create mode 100644 tests/tcg/mips/mipsr5900/mflohi1.c create mode 100644 tests/tcg/mips/mipsr5900/mtlohi1.c create mode 100644 tests/tcg/mips/mipsr5900/mult.c create mode 100644 tests/tcg/mips/mipsr5900/multu.c