Re: [PATCH-for-6.1?] target/mips: Remove MOVZ/MOVN opcodes from Loongson 2E

[Maciej W. Rozycki]

On Mon, 2 Aug 2021, Philippe Mathieu-Daudé wrote:

> >  At least this trivial program:
> > 
> > int main(void)
> > {
> > asm volatile(".set push; .set mips4; movn $0,$0,$0; .set pop");
> > return 0;
> > }
> > 
> > does not trap on actual hardware.
> 
> Thank you very much for your time and testing!

 You're welcome!  I'm glad to be of help.

  Maciej

Re: [PATCH-for-6.1?] target/mips: Remove MOVZ/MOVN opcodes from Loongson 2E

[Maciej W. Rozycki]

On Mon, 2 Aug 2021, Philippe Mathieu-Daudé wrote:

> > Per the "Godson-2E User Manual v0.6", the Loongson 2E processor
> > does not implement the MOVZ/MOVN instructions
> 
> I'm confused because I can't find MOVZ/MOVN in the 2E manual and
> the 2F explicits the difference. However looking at binutils,
> these opcodes are also emited on the 2E:
> https://sourceware.org/git/?p=binutils-gdb.git;a=commitdiff;h=350cc38db21f1cd651a6d46687542a0fce5e0303;hp=569502941afa825c5278b320ccedeefc82e8ed0e

 I find the manual a bit messy.  It does say however:

"3.2.3 Instruction set mode

"Godson-2E processor implements a full feature MIPS III Instruction Set 
Architecture (ISA) plus some MIPS IV ISA instructions, like paired single, 
move condition and multiply add."

> Cc'ing Mark & Maciej in case they can enlighten me, and few
> Loongson develeper in case they could check, because I don't
> have 2E hardware to test.

 At least this trivial program:

int main(void)
{
asm volatile(".set push; .set mips4; movn $0,$0,$0; .set pop");
return 0;
}

does not trap on actual hardware.  I may not be able to find time right 
now for a more exhaustive test.

  Maciej

Re: [RFC PATCH 32/42] docker: Add gentoo-mipsr5900el-cross image

[Maciej W. Rozycki]

On Wed, 17 Mar 2021, Philippe Mathieu-Daudé wrote:

> Maciej, I tried your suggestion as follow:
> 
> $ mipsel-linux-gnu-gcc --version
> mipsel-linux-gnu-gcc (Debian 8.3.0-2) 8.3.0
> Copyright (C) 2018 Free Software Foundation, Inc.
> This is free software; see the source for copying conditions.  There is NO
> warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
> 
> $ mipsel-linux-gnu-gcc -print-multi-lib
> .;
> n32;@mabi=n32
> 64;@mabi=64

 OK, so these are for the three ABIs Linux support, plain.  You'd have to 
bootstrap your own compiler and either reconfigure it at least for n32 
with a `-msoft-float' multilib, or build a non-multilib compiler for this 
ABI variant.  Either way including glibc which to the best of my knowledge 
does support soft-float MIPS configurations.

> soft-float is not listed, but:
> 
> $ touch a.c
> $ mipsel-linux-gnu-gcc -mabi=n32 -march=r5900 -msoft-float -c a.c
> $ file a.o
> a.o: ELF 32-bit LSB relocatable, MIPS, N32 MIPS-III version 1 (SYSV),
> not stripped
> $ readelf -h a.o
> ELF Header:
>   Magic:   7f 45 4c 46 01 01 01 00 00 00 00 00 00 00 00 00
>   Class: ELF32
>   Data:  2's complement, little endian
>   Version:   1 (current)
>   OS/ABI:UNIX - System V
>   ABI Version:   0
>   Type:  REL (Relocatable file)
>   Machine:   MIPS R3000
>   Version:   0x1
>   Entry point address:   0x0
>   Start of program headers:  0 (bytes into file)
>   Start of section headers:  456 (bytes into file)
>   Flags: 0x20920026, pic, cpic, abi2, 5900,
> mips3
>   Size of this header:   52 (bytes)
>   Size of program headers:   0 (bytes)
>   Number of program headers: 0
>   Size of section headers:   40 (bytes)
>   Number of section headers: 13
>   Section header string table index: 12
> 
> So we can compile, but:

 Yes, all code models are always available for compilation (you can choose 
the default though).  We (ab)use this feature for instance to build the 
Linux kernel, which obviously uses the bare metal non-PIC model.

> $ mipsel-linux-gnu-gcc -mabi=n32 -march=r5900 -msoft-float -L
> /usr/lib/mipsel-linux-gnu tests/tcg/mips/test-r5900-dmult.c
> /usr/lib/gcc-cross/mipsel-linux-gnu/8/../../../../mipsel-linux-gnu/bin/ld:
> /usr/lib/../lib32/Scrt1.o: relocations in generic ELF (EM: 3)
> /usr/lib/gcc-cross/mipsel-linux-gnu/8/../../../../mipsel-linux-gnu/bin/ld:
> /usr/lib/../lib32/Scrt1.o: error adding symbols: file in wrong format
> collect2: error: ld returned 1 exit status

 Yes, the problem is in the missing runtime, and whatever GCC defaults to 
is not link-compatible.  The message from the linker indicates it couldn't 
have chosen a MIPS emulation at all even.

> $ mipsel-linux-gnu-readelf -h /usr/lib/mipsel-linux-gnu/Scrt1.o
> ELF Header:
>   Magic:   7f 45 4c 46 01 01 01 00 00 00 00 00 00 00 00 00
>   Class: ELF32
>   Data:  2's complement, little endian
>   Version:   1 (current)
>   OS/ABI:UNIX - System V
>   ABI Version:   0
>   Type:  REL (Relocatable file)
>   Machine:   MIPS R3000
>   Version:   0x1
>   Entry point address:   0x0
>   Start of program headers:  0 (bytes into file)
>   Start of section headers:  908 (bytes into file)
>   Flags: 0x70001007, noreorder, pic, cpic,
> o32, mips32r2
>   Size of this header:   52 (bytes)
>   Size of program headers:   0 (bytes)
>   Number of program headers: 0
>   Size of section headers:   40 (bytes)
>   Number of section headers: 15
>   Section header string table index: 14

 That is a plain o32 ABI object.

> The multilib documentation is scarce :)

 Ask questions. :)

> Also I wondered if I could abuse the linker a bit, knowing the resulting
> binary would be crap due to ABI mismatch, and got:
> 
> $ mipsel-linux-gnu-gcc -march=r5900 -msoft-float -o test-r5900-dmult
> test-r5900-dmult.o
> collect2: fatal error: ld terminated with signal 11 [Segmentation
> fault], core dumped
> compilation terminated.
> /usr/lib/gcc-cross/mipsel-linux-gnu/8/../../../../mipsel-linux-gnu/bin/ld:
> test-r5900-dmult.o: ABI is incompatible with that of the selected emulation
> /usr/lib/gcc-cross/mipsel-linux-gnu/8/../../../../mipsel-linux-gnu/bin/ld:
> failed to merge target specific data of file test-r5900-dmult.o
> /usr/lib/gcc-cross/mipsel-linux-gnu/8/../../../../mipsel-linux-gnu/bin/ld:
> BFD (GNU Binutils for Debian) 2.31.1 assertion fail
> ../../bfd/elfxx-mips.c:3566

 This looks like a linker bug, it's not supposed to 

Re: [RFC PATCH 32/42] docker: Add gentoo-mipsr5900el-cross image

[Maciej W. Rozycki]

On Fri, 12 Mar 2021, Philippe Mathieu-Daudé wrote:

> > but your two options to 
> > choose from are:
> > 
> > 1. Build for the soft-float ABI (`-msoft-float') where any FP calculations 
> >are compiled such as to be made by the CPU using integer arithmetic.
> 
> With the Debian toolchain I get:
> 
> /usr/include/stdio.h:27:10: fatal error: bits/libc-header-start.h: No
> such file or directory
>  #include 
>   ^~

 You need to have a soft-float multilib available.  You can use:

$ gcc -print-multi-lib

to see what multilibs are available.  You'd have to have one that includes 
`-msoft-float' and `-mabi=n32' both at a time.  If there's no such one, 
then you'll have to build your own cross-compiler (including soft-float 
libc).

> > 2. Build for a generic MIPS ISA, for the R5900/n32 that would be MIPS III
> >(`-march=mips3'), and rely on the kernel FPU emulation.
> 
> Shouldn't -march=r5900 imply -march=mips3?

 It does:

MIPS_CPU ("r5900", PROCESSOR_R5900, 3, 0)

however it is a distinct architecture owing to the differences from plain 
MIPS III ISA, and obviously the compiler and the assembler both know that.

  Maciej

Re: [RFC PATCH 32/42] docker: Add gentoo-mipsr5900el-cross image

[Maciej W. Rozycki]

On Fri, 12 Mar 2021, Philippe Mathieu-Daudé wrote:

> >>> Is there any way we can do this with a distro that isn't Gentoo
> >>> so that we can get a container build that is fast enough to be
> >>> useful for CI ?
> 
> Using the Debian cross image I get:
> 
> /home/phil/source/qemu/tests/docker/docker.py --engine auto cc --cc
> mips64el-linux-gnuabi64-gcc -i qemu/debian-mips64el-cross -s
> /home/phil/source/qemu --   -Wall -Werror -O0 -g -fno-strict-aliasing
> -mabi=n32 -march=r5900
> /home/phil/source/qemu/tests/tcg/mips/test-r5900-dmult.c -o
> test-r5900-dmult  -static
> cc1: error: unsupported combination: -march=r5900 -mhard-float
> -mdouble-float
> 
> No clue what is setting '-mhard-float -mdouble-float' yet.

 The R5900 has an FPU that only supports the single floating-point format.  
It's also not an IEEE 754 format.  The Linux kernel ABI does support the 
double and also the single floating-point format, both compliant with IEEE 
754.

 In the absence of a suitable FPU emulation code included with the kernel 
will handle the missing instructions (you can use the `nofpu' kernel 
parameter to force that in the presence of an FPU too).  Beware however 
that a recent change to the Linux kernel made FPU emulation code optional 
to suit some deeply embedded applications known never to use FPU machine 
instructions.

 NB the presence of emulation is always required for MIPS ISA compliance 
if FPU machine instructions are ever to be used in a given application, 
because operations are allowed to trap regardless and rely on emulation.

 I don't know what you are trying to achieve, but your two options to 
choose from are:

1. Build for the soft-float ABI (`-msoft-float') where any FP calculations 
   are compiled such as to be made by the CPU using integer arithmetic.

2. Build for a generic MIPS ISA, for the R5900/n32 that would be MIPS III
   (`-march=mips3'), and rely on the kernel FPU emulation.  Note that some 
   integer MIPS III operations are missing too from the R5900 and have to 
   be emulated by the kernel for MIPS/Linux n32 psABI compliance (an 
   implementation can be pinched from an old libgcc version that was still 
   under GNU GPLv2 or another algorithm reused, e.g. my `__div64_32' piece
   easily adapted).

 HTH,

  Maciej

Re: [RFC PATCH 28/42] target/mips/tx79: Move RDHWR usermode kludge to trans_SQ()

[Maciej W. Rozycki]

On Tue, 16 Feb 2021, Fredrik Noring wrote:

> > Not that it's odd (the final address is masked, remember), but that it a 
> > store
> > to an address in the zero page.
> 
> The address always resolves to 0xe83b (then masked) in 32-bit KSEG2,
> because rt is always $3 and rd is always $29 so -6085(zero), hence the
> last page (which is much better) rather than the first, as Maciej
> discovered:
> 
> https://patchwork.kernel.org/comment/23824173/
> 
> Other possible RDHWR encodings are no longer used, and can therefore be
> ignored and revert to SQ:
> 
> https://patchwork.kernel.org/comment/23842167/

 Or rather were never used in the general case (I can't rule out someone 
using that stuff for something, but I wouldn't call it supported; I used 
some of it internally while evaluating the speed of RDHWR emulation before 
the use of $3 or indeed RDHWR was settled in the TLS psABI, though the 
actual code that ultimately went into Linux was developed independently).

> > I would do this as
> > 
> > {
> >   RDHWR_user  01 0 . . 0 111011   @rd_rt
> >   SQ  01 . .  @ldst
> > }
> 
> Both rd and rt have fixed values, as mentioned.

 I would suggest actually supporting variable `rt', see below.  Would it 
be a problem?

> For reference, RDHWR is currently done like this in the Linux kernel:
> 
>   if (IS_ENABLED(CONFIG_CPU_R5900)) {
>   /*
>* On the R5900, a valid RDHWR instruction
>*
>* ++---+++---++
>* | 01 | 0 | rt | rd | 0 | 111011 |
>* ++---+++---++
>*  6   5  55 56
>*
>* having rt $3 (v1) and rd $29 (MIPS_HWR_ULR) is
>* interpreted as the R5900 specific SQ instruction
>*
>* ++---++-+
>* | 01 |  base | rt |offset   |
>* ++---++-+
>*  6   5  516
>*
>* with
>*
>* sq v1,-6085(zero)
>*
>* that asserts an address exception since -6085(zero)
>* always resolves to 0xe83b in 32-bit KSEG2.
>*
>* Other legacy values of rd, such as MIPS_HWR_CPUNUM,
>* are ignored.
>*/
>   if (insn.r_format.func == rdhwr_op &&
>   insn.r_format.rd == MIPS_HWR_ULR &&
>   insn.r_format.rt == 3 &&

 I suggest leaving the `rt' check out for consistency, as changing the 
register to read the value of UserLocal into from psABI-mandated $3 does 
not cause any issue with the R5900 (the `rt' field overlaps between both 
machine instructions, so the encoding placed there does not affect the 
KSEG2 access trap caused) and those encodings are also emulated in the 
slow path for other legacy ISA CPUs:

case MIPS_HWR_ULR:  /* Read UserLocal register */
regs->regs[rt] = ti->tp_value;
return 0;


 So e.g. `rdhwr $25, $29' is interpreted as `sq $25,-6085($0)' by the 
R5900 => no issue, it still traps.

 I know I have previously written that we can ignore `rt' encodings other 
than $3, but they are harmless and handling them saves a couple of machine 
instructions needed to make the check, so I think while we can, we do not 
actually have to ignore them.

>   insn.r_format.rs == 0 &&
>   insn.r_format.re == 0) {
>   if (compute_return_epc(regs) < 0 ||
>   simulate_rdhwr(regs, insn.r_format.rd,
>  insn.r_format.rt) < 0)
>   goto sigill;
>   return;
>   }
>   goto sigbus;
>   } else ...

 Code continuation quoted left for reference.

  Maciej

Re: [RFC PATCH 41/42] tests/acceptance: Test R5900 CPU with BusyBox from Sony PS2

[Maciej W. Rozycki]

On Mon, 15 Feb 2021, Fredrik Noring wrote:

> For n32, there's a provisional patch that needs reworking to get merged
> (to have libc emulate DMULT etc. rather than the kernel/QEMU).[1]

 FWIW I think standard `-march=mips3' n32 Linux binaries need to work with 
the R5900, and therefore the kernel will have to emulate the missing part 
of the ABI (since years ago we chose to have it in the kernel rather than 
in a SIGILL handler).  The patch proposed only allows building GNU libc 
with `-march=r5900' which I find an optional feature (I don't know offhand
what advantages over `-march=mips3' would be there).

  Maciej

Re: [PATCH-for-5.2?] target/mips/translate: Check R6 reserved encoding for Load Linked Word

[Maciej W. Rozycki]

On Tue, 8 Dec 2020, Philippe Mathieu-Daudé wrote:

> Duh I hit that again, read the patch again, looks correct. I guess
> I got confused myself reviewing the offending patch...
> So I'm applying this patch to mips-next queue, using
> Fixes: d9224450208 ("target-mips: Tighten ISA level checks")

 What's wrong with current code?  What I can see is:

case OPC_LL: /* Load and stores */
check_insn(ctx, ISA_MIPS2);
if (ctx->insn_flags & INSN_R5900) {
check_insn_opc_user_only(ctx, INSN_R5900);
}
/* Fallthrough */
case OPC_LWL:
case OPC_LWR:
check_insn_opc_removed(ctx, ISA_MIPS32R6);
 /* Fallthrough */
case OPC_LB:
case OPC_LH:
case OPC_LW:
case OPC_LWPC:
case OPC_LBU:
case OPC_LHU:
 gen_ld(ctx, op, rt, rs, imm);
 break;

which looks absolutely right to me: LL is accepted with MIPS2--MIPS32R5 
(including R5900 in user emulation only), LWL/LWR are accepted with 
MIPS1--MIPS32R5 and the remaining loads are accepted everywhere.  What 
else do you need?

  Maciej

Re: [PATCH 3/4] default-configs: Support o32 ABI with 64-bit MIPS CPUs

[Maciej W. Rozycki]

On Thu, 19 Nov 2020, Philippe Mathieu-Daudé wrote:

> MIPS o32 ABI on 64-bit CPUs looks like a ILP32-on-64bit data
> model, allowing 64-bit arithmetic and data movement instructions.
> 
> This is the default ABI used by the "Sony Linux Toolkit for
> Playstation 2".

 Please don't, not at least with a generic configuration (i.e. make it 
unambiguous that this is R5900-specific).  This only works with R5900 
because it does not implement the MIPS ISA correctly (e.g. see what $ra is 
set to with JAL/JALR/etc. in the kernel mode), and it is not supported by 
the standard Linux ABI.  Use n32 instead, which has the same functionality 
and is standard (and is also a better ABI in terms of performance).

 You'd probably need to implement all the R5900 addressing quirks for your 
proposed hack to match hardware, or otherwise you'll end up with emulation 
that creates its own reality.

  Maciej

Re: [Qemu-devel] [RFC PATCH 0/2] docker: Add gentoo-mipsr5900el-cross image

[Maciej W. Rozycki]

On Mon, 19 Nov 2018, Fredrik Noring wrote:

> > The first patch adds a cross toolchain for the R5900 MIPS.
> > It is working correctly but the patches provided by Fredrik in [1] don't
> > have proper S-o-b, thus it is tagged RFC.
> > Fredrik: any update on the status of those patches upstream?
> 
> With GCC 8.2.0 and a current Gentoo this is even simpler. You should
> only need commit d728eb9085d8 ("MIPS: Default to --with-llsc for the
> R5900 Linux target as well"):
> 
> https://gcc.gnu.org/git/?p=gcc.git;a=commit;h=d728eb9085d8

 Of course you can instead just set the default manually by using 
`--with-llsc' when configuring GCC or specify `-mllsc' explicitly in 
CFLAGS with a compiler that has been already built without that set by 
default.

> For the o32 ABI with Glibc, only commit 8e3c00db16fc ("MIPS: Use `.set
> mips2' to emulate LL/SC for the R5900 too") is needed:
> 
> https://sourceware.org/git/?p=glibc.git;a=commit;h=8e3c00db16fc
> 
> Eventually, with a future GCC/Glibc release, no patches will be needed.

 For the record the glibc change will be available with upcoming glibc 
2.29, currently scheduled Feb 1st, 2019, and the GCC change will appear in 
GCC 9.1.0, currently scheduled Mar-Apr 2019.

 FWIW,

  Maciej

Re: [Qemu-devel] [PATCH 0/2] linux-user/mips: Support the n32 ABI for the R5900

[Maciej W. Rozycki]

On Fri, 9 Nov 2018, Maciej W. Rozycki wrote:

> > Some readelf results:
> > 
> > mips64el/stretch
> > 
> >   Magic:   7f 45 4c 46 02 01 01 00 00 00 00 00 00 00 00 00
> >   Class:   ELF64
> >   Flags:   0x8007, noreorder, pic, cpic, mips64r2
> 
>  Hmm, that's weird -- what executable did you check?  There may be some 
> that are n64, or maybe they've switched (which I would applaud, FWIW).  I 
> remember seeing mostly n32, with minimal support for n64, but that was a 
> while ago -- jessie or suchlike, I believe.  Using MIPS64r2 as the base 
> ISA also looks new to me, that used to be plain MIPS III, and some of 
> Debian's MIPS build systems used to be either MIPS III (Lemote Loongson) 
> or MIPS64r1 (Broadcom SiByte).

 OK, I definitely got this confused.  I did some checking and jessie 
didn't even have a 64-bit MIPS port.  I got their build systems right 
though, and the kernel is 64-bit for systems that support it.

> > Any binaries that need qemu-mipsn32 or qemu-mipsn32el?
> 
>  I'd expect at least the n32 dynamic loader (along with libc and some 
> other essential DSOs) to be present with MIPS64 Debian.  Traditionally, 
> under the FHS rules, it would be installed as /lib32/ld.so.1 (with the o32 
> one as /lib/ld.so.1 and the n64 as /lib64/ld.so.1), but Debian uses their 
> own multiarch filesystem layout standard, and offhand I don't remember 
> what the paths are defined to there.

 So with jessie you can install the `libc6-dev-mipsn32' package, which 
will get you n32 glibc development libraries and will pull the 
complementing n32 dynamic loader (at /lib32/ld.so.1 actually) and n32 
glibc shared libraries as well.

 Unfortunately multilib support files, such as the CRT files, seem to be 
missing from GCC for n32 or I cannot find them.  Otherwise you would be 
able to compile and link n32 binaries just by calling `gcc -mabi=n32'.  
Still the dynamic loader is directly runnable, as I noted above.

 HTH,

  Maciej

Re: [Qemu-devel] [PATCH v2 4/6] target/mips: Fix decoding mechanism of special R5900 opcodes

[Maciej W. Rozycki]

On Fri, 9 Nov 2018, Aleksandar Markovic wrote:

> > > I think that the best solution that you exclude DDIV, DDIVU, DMULT, DMULTU
> > > in a separate patch - there is no document to support their inclusion.
> >
> > As Maciej noted, the 64-bit MIPS Linux psABI is indivisible, so how could
> > your alternative possibly work?
> 
> Since we are rapidly approaching 3.1 release, we don't have time for 
> prolonged discussions - so please provide the patch that removes 
> emulation of these instructions that don't belong to R5900 set, and, if 
> you find a justification document later on, they can be reintroduced in 
> 3.1+ timeframe.

 You know well enough that nobody was bothered over the years to actually 
document the 64-bit MIPS Linux psABI (there is the 64-bit ELF document 
from SGI, relevant for IRIX, which has been partially implemented by 
Linux) and even when it comes to the 32-bit psABI the only document is 
from SGI from mid 1990s, that has several errors and surely was not 
written with Linux in mind (and FWIW not entirely with IRIX either).

 The psABI has been set by the architecture back in 1991 and what Linux 
has implemented on top of that around 2001, along with common sense.  You 
can't question what was done 17 years ago asking for a backing piece of 
paper (possibly virtual).  You can put anything on paper and if it does 
not match reality, then it is irrelevant.

 If you question what I state, then ask the MIPS/Linux kernel developers 
at the relevant mailing list, i.e. .

  Maciej

Re: [Qemu-devel] [PATCH 0/2] linux-user/mips: Support the n32 ABI for the R5900

[Maciej W. Rozycki]

On Fri, 9 Nov 2018, Laurent Vivier wrote:

> >  I believe MIPS64 Debian is all n32.
> 
> Some readelf results:
> 
> mips64el/stretch
> 
>   Magic:   7f 45 4c 46 02 01 01 00 00 00 00 00 00 00 00 00
>   Class:   ELF64
>   Flags:   0x8007, noreorder, pic, cpic, mips64r2

 Hmm, that's weird -- what executable did you check?  There may be some 
that are n64, or maybe they've switched (which I would applaud, FWIW).  I 
remember seeing mostly n32, with minimal support for n64, but that was a 
while ago -- jessie or suchlike, I believe.  Using MIPS64r2 as the base 
ISA also looks new to me, that used to be plain MIPS III, and some of 
Debian's MIPS build systems used to be either MIPS III (Lemote Loongson) 
or MIPS64r1 (Broadcom SiByte).

> Are there any distros that are 32bit but n32?

 You need 64-bit hardware for n32, so you can't do a 32-bit distribution 
with n32.  FAOD n32 is ILP32 with 64-bit `long long' and 64-bit stack 
slots both using the native 64-bit register width.

> Any binaries that need qemu-mipsn32 or qemu-mipsn32el?

 I'd expect at least the n32 dynamic loader (along with libc and some 
other essential DSOs) to be present with MIPS64 Debian.  Traditionally, 
under the FHS rules, it would be installed as /lib32/ld.so.1 (with the o32 
one as /lib/ld.so.1 and the n64 as /lib64/ld.so.1), but Debian uses their 
own multiarch filesystem layout standard, and offhand I don't remember 
what the paths are defined to there.

 Anyway, once you have tracked down the `ld.so.1' binary you can invoke it 
directly for some output, and it should be good enough for your smoke test 
purposes IIUC.

 I am away travelling through the end of this week and I have limited 
access to resources.  I can help more once I am back next week.

  Maciej

Re: [Qemu-devel] [PATCH 0/2] linux-user/mips: Support the n32 ABI for the R5900

[Maciej W. Rozycki]

On Fri, 9 Nov 2018, Laurent Vivier wrote:

> if you have time, o32 & n32 support needs to be reworked.
> 
> We have two binaries qemu-mips and qemu-mipsn32 sharing the same ELF
> mask/magic.
> 
> As n32 identifies a kernel ABI version, we should have only one binary
> for qemu-mips and qemu-mipsn32 and the ABI version should be identified
> at runtime as it is done for ARM:
> 
>   ce4defa062 Arm Linux EABI syscall support.

 Are you sure?  So is the ARM ABI handled with ce4defa062 64-bit?

 The o32 ABI is 32-bit (32-bit GPR width) while n32 is 64-bit (64-bit GPR 
width).  The correspondence between the i386 ABI and the x86-64 x32 ABI is 
analogous to these two ABIs.

 How are these x86 ABIs handled WRT the user emulation mode; are they 
analogous to the ARM ABIs you've mentioned?

> [I think we can use e_flags for that]

 The EF_MIPS_ABI2 `e_flags' bit denotes the n32 ABI.  The container format 
is ELF32 obviously, because addressing is 32-bit with n32.

> I never did the change because I don't know where to find mipsn32
> binaries to test my changes.

 I believe MIPS64 Debian is all n32.

  Maciej

Re: [Qemu-devel] [PATCH v2 4/6] target/mips: Fix decoding mechanism of special R5900 opcodes

[Maciej W. Rozycki]

On Fri, 9 Nov 2018, Aleksandar Markovic wrote:

> > ... and DMULT, DMULTU, DDIV and DDIVU
> > are part of the MIPS III ISA. They are emulated in user mode to support
> > generic MIPS III programs.
> 
> Pure MIPS III executables should not be a concern of the R5900 
> emulation, but R5900 executables.

 I repeat: MIPS III is the available instruction set defined with the base 
64-bit MIPS Linux psABI and must not be subsetted.  You have to support it 
in QEMU Linux user emulation mode for any 64-bit MIPS processor and this 
is not debatable, period.  QEMU has no control over the Linux ABI, it has 
to accept it as it is.

  Maciej

Re: [Qemu-devel] [PATCH v2 4/6] target/mips: Fix decoding mechanism of special R5900 opcodes

[Maciej W. Rozycki]

On Thu, 8 Nov 2018, Fredrik Noring wrote:

> > Fredrik, do you know by any chance if a document exists that would justify
> > inclusion of non-R5900 DMULT, DMULTU, DDIV, DDIVU in R5900 executables by
> > gcc for R5900? Is it included by cross-gcc or by native gcc, or by both?
> > 
> > I think gcc folks must have had a good reason for that, some kind of
> > design - it can't be 'I really like/miss this instruction, let's include
> > it...'
> 
> The R5900 reports itself as MIPS III and DMULT, DMULTU, DDIV and DDIVU
> are part of the MIPS III ISA. They are emulated in user mode to support
> generic MIPS III programs.

 FAOD, GCC does not emit these instructions if the R5900 architecture has 
been selected for compilation, e.g.:

/* ISA supports instructions DMULT and DMULTU. */
#define ISA_HAS_DMULT   (TARGET_64BIT   \
 && !TARGET_MIPS5900\
 && mips_isa_rev <= 5)

however they are a part of the base 64-bit MIPS Linux user psABI, which is 
the whole of the MIPS III ISA, so the runtime has to support them one way 
or another (just like LL, SC and SYNC are a part of the 32-bit MIPS Linux 
user psABI even though they are not supported by MIPS I hardware).

  Maciej

Re: [Qemu-devel] [PATCH 1/2] target/mips: Fix decoding mechanism of R5900 MFLO1, MFHI1, MTLO1 and MTHI1

[Maciej W. Rozycki]

On Mon, 5 Nov 2018, Fredrik Noring wrote:

> where ac can range from 0 to 3. Do you have a link to a better reference,
> by chance, that isn't tied to microMIPS?

 Here:  you'll find everything you need I 
believe.

 HTH,

  Maciej

Re: [Qemu-devel] [PATCH 1/2] target/mips: Fix decoding mechanism of R5900 MFLO1, MFHI1, MTLO1 and MTHI1

[Maciej W. Rozycki]

On Sun, 4 Nov 2018, Fredrik Noring wrote:

> It appears the correct function is tcg_gen_mov_tl because the TX79 manual
> says
> 
>   MFHI:  GPR[rd]63..0 <- HI63..0
>   MFLO:  GPR[rd]63..0 <- LO63..0
>   MTHI:  HI63..0 <- GPR[rs]63..0
>   MTLO:  LO63..0 <- GPR[rs]63..0
>   MFHI1: GPR[rd]63..0 <- HI127..64
>   MFLO1: GPR[rd]63..0 <- LO127..64
>   MTHI1: HI127..64 <- GPR[rs]63..0
>   MTLO1: LO127..64 <- GPR[rs]63..0
> 
> so the GPR is copied to/from in full in all cases. This is slightly
> different to how acc = 1 is handled in gen_HILO.

 However `gen_HILO' looks wrong to me as it'll truncate the values of 
$acc3-$acc1 with the 64-bit DSP ASE.

  Maciej

Re: [Qemu-devel] [PATCH] target/mips: Support Toshiba specific three-operand MADD and MADDU

[Maciej W. Rozycki]

On Mon, 29 Oct 2018, Aleksandar Markovic wrote:

> > > Without TARGET_MIPS64, we can't say we emulate R5900 - we are emulating
> > > some other CPU that never existed.
> > >
> > > Convince me that I am wrong.
> > 
> > R5900 O32 is usable.
> 
> Absolutely not. This kind of emulation infidelity can't be in QEMU 
> upstream. Even one step further, this should be forbidden, impossible to 
> build.

 What do you mean by "R5900 o32 is not usable unless TARGET_MIPS64"?

 The user emulation mode does not emulate a CPU, it emulates an OS's user 
ABI.  None of the MIPS/Linux user ABIs exposes the privileged context and 
the user instruction set is also defined by the ABI, as far as both 
limitations (e.g. in o32 no MIPS III instructions are allowed, though 
32-bit MIPS IV ones are) and extensions (e.g. many FPU instructions, LL/SC 
and RDHWR implemented even if missing from hardware) are concerned.

 So user emulation is never going to be an accurate CPU emulation anyway.

 The R5900 specifically indeed has no way to enforce the o32 instruction 
set, unlike all the other MIPS CPUs, but that has to be considered a 
quirk, because the operation of the disallowed instructions is 
unpredictable in o32 anyway and the first context switch when actually 
executing real Linux rather than QEMU user emulation would clobber user 
registers.  And I don't think we need to define unpredictable operation in 
hardware-specific way, it may well send SIGILL telling the user they did 
something wrong (as opposed to silent context corruption observed with 
real hw).

 If you spoke about system emulation, then that would be a different 
matter and I would tend to agree, noting however that even there QEMU does 
not emulate all aspects of the architecture anyway, so it's always an 
approximation defined such as to fit the people's needs.

 What is this discussion about anyway?  Do you require now for new CPU 
subarchitecture acceptance that the whole instruction set has been 
implemented with the first submission?  I don't think that was the case in 
the past and even artificial inexisting CPUs have been created (which 
still are there AFAICT), but maybe I'm missing something, and of course 
the rules may change.

  Maciej

Re: [Qemu-devel] [PATCH] target/mips: Support Toshiba specific three-operand MADD and MADDU

[Maciej W. Rozycki]

On Sun, 28 Oct 2018, Aleksandar Markovic wrote:

> I truly need your help here. As you can conclude from the discussion, 
> R5900 folks (anybody correct me if I am wrong) have some problems using 
> any ABI other than O32.

 The maximum the R5900 can support is the n32 ABI, owing to 32-bit virtual 
addressing.  And that ABI is even more troublesome due to this processor's 
peculiarities, requiring extra effort in addition to what has to be done 
to support o32 only.  The lack of sign-extension of the link address that 
becomes visible with 64-bit register accesses is just one issue to name.  
Hence the staged approach chosen.  We'll get n32 support eventually as 
well, once o32 has been sorted, but there'll be no n64 support ever.

  Maciej

Re: [Qemu-devel] [PATCH v8 00/38] target/mips: Limited support for the R5900

[Maciej W. Rozycki]

On Fri, 26 Oct 2018, Richard Henderson wrote:

> > Overall this source file is clearly a modified copy of an ancient version 
> > of the opcode table included with the opcodes library from binutils and I 
> > think it would benefit from a refresh.
> 
> You can't do that because of GPL v3, sadly.

 I've been aware of that, however the changes I mentioned are pretty 
mechanical and can be easily made from scratch by someone them who hasn't 
looked at binutils, even based on the description I already made.  You 
don't copyright an idea, only actual written code.

  Maciej

Re: [Qemu-devel] [PATCH v8 00/38] target/mips: Limited support for the R5900

[Maciej W. Rozycki]

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

Re: [Qemu-devel] [PATCH v8 00/38] target/mips: Limited support for the R5900

[Maciej W. Rozycki]

Hi Fredrik,

> > > Option 3: Extend the mips_opcode::membership field.
> > 
> > It's trivial to extend the field to uint64_t.
> 
> Is the membership field intended to be used? The opcodes for CLZ and CLO
> clash with the R5900 opcodes for MADD1 and MADDU1, resulting in incorrect
> disassembly of MADD1 and MADDU1. For example:
> 
>   0x70853020 madd1  a2,a0,a1  disassembles into  clz a2 or a1,a0
>   0x70853021 maddu1 a2,a0,a1  disassembles into  clo a2 or a1,a0
> 
> (CLZ and CLO are members of I32|N55, whereas MADD1 and MADDU1 are EE.)

 It looks like a disassembler bug somewhere then (maybe in your patched 
version only), because the R5900 is not supposed to match I32 (because it 
does not implement the MIPS32 ISA; it's only MIPS I aka I1 with additions 
or MIPS IV aka I4 with exclusions, or anything between with both additions 
and exclusions, with I believe MIPS III aka I3 being the closest match), 
and it is not supposed to match N55 either (because it is obviously not a 
Vr5500 processor).

 Overall this source file is clearly a modified copy of an ancient version 
of the opcode table included with the opcodes library from binutils and I 
think it would benefit from a refresh.  In particular separating an ASE 
field and adding an exclusions field, as it has been done with opcodes, 
would make it much easier to maintain this table.  The table in opcodes is 
already messy due to several exceptions to the alphabetical order (and it 
could be improved a bit I believe), but I find its QEMU version even 
messier.

 HTH,

  Maciej

Re: [Qemu-devel] [PATCH v8 00/38] target/mips: Limited support for the R5900

[Maciej W. Rozycki]

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

Re: [Qemu-devel] [PATCH v8 00/38] target/mips: Limited support for the R5900

[Maciej W. Rozycki]

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

Re: [Qemu-devel] [PATCH v8 00/38] target/mips: Limited support for the R5900

[Maciej W. Rozycki]

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  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

Re: [Qemu-devel] [PATCH v5 6/8] target/mips: Define the R5900 CPU

[Maciej W. Rozycki]

Hi Fredrik,

> >  The C790 is a follow-up to the R5900.  The R5900 has an FPU that is not 
> > compliant to the IEEE 754 standard for floating-point arithmetic.  It 
> > doesn't implement exceptions, infinities, NaNs or denormals.  It doesn't 
> > implement the the double format either, but that is really tangential, 
> > because the same was the case with the IDT R4650, which had a standard 
> > IEEE 754 FPU, but no double format either.  Finally it has some anomalies 
> > in the FP instruction set; not all opcode encodings are standard.
> 
> Do you know where one might find a list of the nonstandard FP instructions?
> 
> The command
> 
> % grep 'FP_.*EE' binutils/opcodes/mips-opc.c
> 
> gives a list of candidates, I suppose?

 Yeah, those that have EE in `membership' are extra and those that have EE 
in `exclusions' are missing.

 See also: .

  Maciej

Re: [Qemu-devel] [PATCH] target/mips: Support Toshiba specific three-operand MADD and MADDU

[Maciej W. Rozycki]

On Tue, 16 Oct 2018, Fredrik Noring wrote:

> > I would not implement r5900 for mips32 in that case,
> > I would implement it only for TARGET_MIPS64.
> 
> R5900 Linux implements the O32 ABI, which is why 32-bit QEMU user-mode is
> very useful. Perhaps a better alternative is to define the MMI registers
> as 128-bit, similar to
> 
> static TCGv_u128 mmi_gpr[32];
> 
> and then copy cpu_gpr to/from mmi_gpr as needed when running the MMIs?

 FWIW, I agree as far as the user emulation mode is concerned.  All 64-bit 
MIPS hardware is currently set up by the Linux kernel for 64-bit execution 
by keeping CP0.Status.UX set when running o32 user processes anyway.

 A change to this policy (and also the use of CP0.Status.PX for n32) has 
been discussed, in partictular in the course of investigating address 
space overflows caused by GCC using the indexed addressing modes under the 
assumption that the address space wraps at 32 bits for o32 and n32 
software, where indeed it does not.  No change has been implemented 
though.

  Maciej

Re: [Qemu-devel] [PATCH] target/mips: Support Toshiba specific three-operand MADD and MADDU

[Maciej W. Rozycki]

On Tue, 16 Oct 2018, Fredrik Noring wrote:

> One option is to create a new array such as
> 
> static TCGv_i64 mmi_gpr[32];
> 
> that represents the upper 64 bits of each GPR. Then cpu_gpr must be of
> a 64-bit type too, even when QEMU runs in 32-bit user mode. The R5900
> does not implement CP0.Status.UX in hardware, though, so system mode is
> 64 bits, regardless.

 It's more like modern CP0.Status.PX however, as the hardware does not 
implement 64-bit memory segments and only has legacy 32-bit segments 
implemented.

 Due to a hardware quirk however the value recorded in the target register 
(usually $ra) does not get sign-extended with linked jump or branch 
instructions, contrary to what is expected with processors implementing 
32-bit segments only.  This has implications for kernel code running from 
KSEG0/KSEG1/KSEG2 and has to be worked around in software, as experience 
has shown.

  Maciej

Re: [Qemu-devel] [PATCH] target/mips: Support Toshiba specific three-operand MADD and MADDU

[Maciej W. Rozycki]

On Sun, 14 Oct 2018, Philippe Mathieu-Daudé wrote:

> > > +gen_move_low32(cpu_LO[acc], t2);
> > > +gen_move_high32(cpu_HI[acc], t2);
> > > +if (rd) {
> > > +gen_move_low32(cpu_gpr[rd], t2);
> >
> > As above, are LO, HI and GPR[rd] sign-extended to 64 bits when required?
> 
>   MADDU rd, rs, rt
>   MADDU rs, rt
>   Multiply the contents of registers rs and rt as unsigned integers,
> and add the doubleword (64-bit)
>   result to multiply/divide registers HI and LO. Also, store the lower
> 32 bits of the add result in register
>   rd. In the MADDU rs, rt format, the store operation to a general
> register is omitted.
> 
> This one looks correct.

 Obviously with processors such as the TX49 or the TX79 where GPRs are 
64-bit for the purpose of integer arithmetic (i.e. any multimedia 
extension aside) any 32-bit results written to an integer register are 
implicitly sign-extended to the full 64-bit width of the destination 
register, as per the requirements of the MIPS architecture.  Otherwise 
operation of any subsequent 32-bit instruction (other than SLL or SLLV) 
using that that result as an input operand would be unpredictable.  You 
need to respect that in QEMU too.

 So results of individual operations are as in the comments with this 
code:

mthi$0  # HI <- 0
mtlo$0  # LO <- 0
addiu   $2, $0, 1   # $2 <- 1
lui $3, 0x4000  # $3 <- 0x4000
maddu   $4, $3, $2  # HI <- 0
# LO <- 0x4000
# $4 <- 0x4000
maddu   $5, $4, $2  # HI <- 0
# LO <- 0x8000
# $5 <- 0x8000
maddu   $6, $4, $2  # HI <- 1
# LO <- 0
# $6 <- 0

Similarly:

addiu   $2, $0, 1   # $2 <- 1
sll $3, $2, 31  # $3 <- 0x8000
dsll$4, $2, 31  # $4 <- 0x8000
dsll$5, $3, 0   # $5 <- 0x8000
sll $6, $3, 0   # $6 <- 0x8000
dsll$7, $4, 0   # $7 <- 0x8000
sll $8, $4, 0   # $8 <- 0x8000
daddu   $9, $3, $3  # $9 <- 0x
addu$10, $3, $3 # $10 <- 0
daddu   $11, $4, $4 # $11 <- 0x1 
addu$12, $4, $4 # unpredictable!

 HTH,

  Maciej

Re: [Qemu-devel] [PATCH v6 2/7] target/mips: Support R5900 specific three-operand MULT and MULTU

[Maciej W. Rozycki]

On Sun, 30 Sep 2018, Philippe Mathieu-Daudé wrote:

> > TX79 do not implement DMULT or DMULTU, the Tx49 does and they do support 
> > the extra `rd' operand there[1].  Still no DMADD or DMADDU though.
> 
> As does the TX39.

 Umm, the TX39 is 32-bit and does not have 64-bit instructions, so it 
can't have DMULT or DMULTU.

> There is also the TX99 series (based on 25Kf):
> https://www.toshiba.co.jp/about/press/2002_02/pr1901.htm

 Hmm, did the TX99 actually tape out?  I thought the 25Kf was withdrawn 
and never made it to market.  I only saw a couple of 25Kf LVs on Malta CPU 
daughtercards back in my MIPS UK days in mid 2000s.

  Maciej

Re: [Qemu-devel] [PATCH v6 2/7] target/mips: Support R5900 specific three-operand MULT and MULTU

[Maciej W. Rozycki]

On Sun, 30 Sep 2018, Philippe Mathieu-Daudé wrote:

> >> I'd use:
> >>
> >>MIPS_INVAL("mul/div Toshiba");
> > 
> >  But just like `gen_mul_vr54xx' this function doesn't handle division!
> 
> Per the commit message, I understood this function would eventually
> handle "the R5900 specific pipeline 1 instruction variants MULT1,
> MULTU1, DIV1, DIVU1, ..."

 Toshiba division instructions do not support the extra operand and always 
return their result in the MD accumulator only.  The same applies to the 
pipeline 1 instruction variants, so I've been thinking that a different 
handler would better be used for DIV1 and DIVU1.  But maybe we can fold it 
together after all and just force `rd' to be 0 at the call site for 
DIV1/DIVU1.

 I'm not sure at this point which approach would be most beneficial, but 
as it stands the function does not handle division operations.  If it 
starts in the future, then we can rename/update it accordingly.

 Note that while the R5900 (shouldn't that be called TX59 actually?) or 
TX79 do not implement DMULT or DMULTU, the Tx49 does and they do support 
the extra `rd' operand there[1].  Still no DMADD or DMADDU though.

References:

[1] "64-Bit TX System RISC TX49/H2, TX49/H3, TX49/H4 Core Architecture", 
Rev 1.0, Toshiba Corporation, September 2004, Table 5-8 "Extensions to 
the ISA: Multiply and Divide Instructions", p. 5-7

  Maciej

Re: [Qemu-devel] [PATCH v6 2/7] target/mips: Support R5900 specific three-operand MULT and MULTU

[Maciej W. Rozycki]

On Sun, 30 Sep 2018, Philippe Mathieu-Daudé wrote:

> > +MIPS_INVAL("mul R5900");
> 
> I'd use:
> 
>MIPS_INVAL("mul/div Toshiba");

 But just like `gen_mul_vr54xx' this function doesn't handle division!

> > @@ -22378,6 +22449,8 @@ static void decode_opc_special_legacy(CPUMIPSState 
> > *env, DisasContext *ctx)
> >  check_insn(ctx, INSN_VR54XX);
> >  op1 = MASK_MUL_VR54XX(ctx->opcode);
> >  gen_mul_vr54xx(ctx, op1, rd, rs, rt);
> > +} else if (ctx->insn_flags & INSN_R5900) {
> > +gen_mul_txxx(ctx, op1, 0, rd, rs, rt);
> 
> Similarly, I'd name this gen_muldiv_txx9().

 Likewise.  I agree with the `_txx9' suffix update, it makes sense to me.

  Maciej

Re: [Qemu-devel] [PATCH v5 2/8] target/mips: Support R5900 specific three-operand MULT and MULTU

[Maciej W. Rozycki]

On Fri, 28 Sep 2018, Philippe Mathieu-Daudé wrote:

> >  What's wrong with `gen_mul_r5900' anyway?
> 
> I plan to use this function (adding MADD/MADDU) for R3900 based cores
> (which don't seemt related to Emotion Engine).

 Fair enough.  I reached for documentation and these instructions seem to 
be shared by all members of Toshiba's TX System RISC family, i.e. TX19, 
TX39, etc.  So I think `gen_mul_toshiba' or `gen_mul_tx' or `gen_mul_txxx' 
will do.

  Maciej

Re: [Qemu-devel] [PATCH v5 2/8] target/mips: Support R5900 specific three-operand MULT and MULTU

[Maciej W. Rozycki]

On Fri, 28 Sep 2018, Philippe Mathieu-Daudé wrote:

> > > Note, these instructions are also valid on the R3900 (which also has
> > > MADD/MADDU).
> > >
> > > Would gen_mul_toshiba() be a better common name? I don't like it but
> > > can't think of another.
> >
> > I propose gen_mul_3op, since its distinctive feature is the three operands.
> 
> Fine by me.

 Bikeshedding, but...  I haven't looked at how we implement it, however 
generic MIPS architecture also has a 3-operand MUL instruction defined, 
reusing, for R1-R5, the encoding used earlier on by IDT R4650 and NEC 
Vr5500, and using a different one for R6.  So I'd rather we avoided 
confusion here.

 Perhaps `gen_mul_ee' then for Emotion Engine? -- EE is what libopcodes 
uses for R5900 instructions where brevity matters.  Or `gen_mul_4op', 
because we have 4 operands really, with the HI/LO accumulator being an 
implicit one.

 What's wrong with `gen_mul_r5900' anyway?

  Maciej

Re: [Qemu-devel] [PATCH v5 2/8] target/mips: Support R5900 specific three-operand MULT and MULTU

[Maciej W. Rozycki]

On Thu, 27 Sep 2018, Philippe Mathieu-Daudé wrote:

> > +static void gen_mul_r5900(DisasContext *ctx, uint32_t opc,
> > +  int acc, int rd, int rs, int rt)
> 
> Since we have acc = 0 we can directly use cpu_LO[0] and cpu_HI[0],
> removing needs for an 'acc' argument.

 Corresponding MULT1 and MULTU1 instructions use `acc = 1' though, as do 
similar variations of all the other MDU instructions.  Maybe the addition 
of the required `acc' argument could be staged for a separate patch with 
all the pipeline 1 MDU instructions though.

  Maciej

Re: [Qemu-devel] [PATCH v5 6/8] target/mips: Define the R5900 CPU

[Maciej W. Rozycki]

On Tue, 25 Sep 2018, Philippe Mathieu-Daudé wrote:

>  >From the DS:
> 
>    The C790 core has the following features:
> - Large on-chip caches
>  • Instruction cache: 32KB, 2-way set associative
>  • Data cache: 32KB, 2-way set-associative (with write-back protocol)
> 
>  0x2 << CP0C0_K0 is 'Uncached', why you selected this and not 0x3 for
>  'Cacheable, write-back, write allocate'?
> >>>
> >>>  No other MIPS emulation does what you suggest, which is why I refrained 
> >>> from commenting on the K0 field and commented on the ICE/DCE bits only.  
> >>> See:
> >>>
> >>> /* Have config1, uncached coherency */
> >>> #define MIPS_CONFIG0  \
> >>>   ((1U << CP0C0_M) | (0x2 << CP0C0_K0))
> >>>
> >>> elsewhere in this file.
> >>
> >> Yes, this was just out of curiosity.
> 
> Maciej, FYI I now remembered the thread that triggered this curiosity:
> 
> http://lists.nongnu.org/archive/html/qemu-devel/2018-08/msg03796.html

 Thanks for the pointer; I'm not subscribed to the QEMU mailing list these 
days anymore.

 For the record, you can achieve the same effect by using the TLB and the 
cache mode of the page(s) mapped, possibly with a wired entry, rather than 
with the physical kernel segments (KSEG0/KSEG1/XKPHYS).

 There's also SPRAM that uses the cache interface in some implementations 
(MTI 4K and M4K cores if memory serves; possibly other ones as well), that 
you may want to model in QEMU one day too.

  Maciej

Re: [Qemu-devel] [PATCH v5 6/8] target/mips: Define the R5900 CPU

[Maciej W. Rozycki]

On Mon, 24 Sep 2018, Philippe Mathieu-Daudé wrote:

> >> >From the DS:
> >>
> >>   The C790 core has the following features:
> >>- Large on-chip caches
> >> • Instruction cache: 32KB, 2-way set associative
> >> • Data cache: 32KB, 2-way set-associative (with write-back protocol)
> >>
> >> 0x2 << CP0C0_K0 is 'Uncached', why you selected this and not 0x3 for
> >> 'Cacheable, write-back, write allocate'?
> > 
> >  No other MIPS emulation does what you suggest, which is why I refrained 
> > from commenting on the K0 field and commented on the ICE/DCE bits only.  
> > See:
> > 
> > /* Have config1, uncached coherency */
> > #define MIPS_CONFIG0  \
> >   ((1U << CP0C0_M) | (0x2 << CP0C0_K0))
> > 
> > elsewhere in this file.
> 
> Yes, this was just out of curiosity.

 Also the value of this field at reset is undefined for this CPU (as is 
architecturally, though some implementations might work otherwise), so 0x2 
is as good as any.  For the user emulation mode we could consider choosing 
what the kernel would use, however that would only matter if we had the 
cache properly emulated.

  Maciej

Re: [Qemu-devel] [PATCH v5 0/8] target/mips: Support R5900 GCC programs in user mode

[Maciej W. Rozycki]

On Sun, 23 Sep 2018, Fredrik Noring wrote:

> > Patch 4 will break bisect on clang builds. The reason for this is that
> > clang treats unused functions as errors. Therefore, patch 4 must be merged
> > with some of subsequent patches that contain first invocation of the
> > function currently defined in patch 4. I know this is in some way
> > illogical, but not breaking the bisect takes precedence.
> 
> Right. GCC accepts it since static inline functions are exempted.

 How does clang handle static inline function definitions in header files 
then?  Such cases are pretty common, also in system headers, and all those 
functions are not necessarily used by every program including the relevant 
header.

  Maciej

Re: [Qemu-devel] [PATCH v5 6/8] target/mips: Define the R5900 CPU

[Maciej W. Rozycki]

Hi Philippe,

> > diff --git a/target/mips/translate_init.inc.c 
> > b/target/mips/translate_init.inc.c
> > index b3320b9dc7..71fd83de06 100644
> > --- a/target/mips/translate_init.inc.c
> > +++ b/target/mips/translate_init.inc.c
> > @@ -410,6 +410,53 @@ const mips_def_t mips_defs[] =
> >  .insn_flags = CPU_MIPS32R5 | ASE_MSA,
> >  .mmu_type = MMU_TYPE_R4000,
> >  },
> > +{
> > +.name = "R5900",
> 
> What bothers me here is you are not modeling "The" unique R5900, but a
> cpu which implements the R5900 architecture.

 There was only one R5900 implementation.

> >From the "TX7901 User’s Manual (Rev. 6.30T – Nov, 2001)":
> 
>   The TX7901 MIPS RISC microcontroller is a highly integrated solution
>   based on Toshiba’s dual-issue super-scalar pipeline Processor Core,
>   the C790 (henceforth referred to as “the C790”).
> 
>   C790 High-performance MIPS CPU Core on which the TX7901 is based.
> 
> So the correct core name is "C790".

 The C790 is a follow-up to the R5900.  The R5900 has an FPU that is not 
compliant to the IEEE 754 standard for floating-point arithmetic.  It 
doesn't implement exceptions, infinities, NaNs or denormals.  It doesn't 
implement the the double format either, but that is really tangential, 
because the same was the case with the IDT R4650, which had a standard 
IEEE 754 FPU, but no double format either.  Finally it has some anomalies 
in the FP instruction set; not all opcode encodings are standard.

 No documentation has been made publicly available for the R5900; my 
understanding is it was only released under NDA as a part of the PS2 SDK.  
However I have been told that the only software-visible difference (except 
for graphic coprocessors) between the R5900 and the C790 is the FPU, which 
in the latter processor is a standard IEEE 754 implementation.

 So the document named is the best public reference for the R5900.  Also 
any emulation of the C790 will have to use a separate template.

> > +.CP0_PRid = 0x3800,
> 
> "The implementation number of the C790 processor is 0x38".

 I'll leave the answer to what the values of PRId are for the R5900 vs the 
C790 to Fredrik or Jürgen.  I suspect that the difference is in the Rev 
field only.

> >From the DS:
> 
>   The C790 core has the following features:
>- Large on-chip caches
> • Instruction cache: 32KB, 2-way set associative
> • Data cache: 32KB, 2-way set-associative (with write-back protocol)
> 
> 0x2 << CP0C0_K0 is 'Uncached', why you selected this and not 0x3 for
> 'Cacheable, write-back, write allocate'?

 No other MIPS emulation does what you suggest, which is why I refrained 
from commenting on the K0 field and commented on the ICE/DCE bits only.  
See:

/* Have config1, uncached coherency */
#define MIPS_CONFIG0  \
  ((1U << CP0C0_M) | (0x2 << CP0C0_K0))

elsewhere in this file.

> > +.PABITS = 32,
> 
> 20 bits PFN + 12 bits offset = 32 (Maciej, can you verify please?).

 That's what I wrote previously.

 NB the offset being 12-bit is only the minimum, depending on the PageMask 
register and used for 4kiB pages only.  But it is a useful simplification.

  Maciej

Re: [Qemu-devel] [PATCH v4 5/8] target/mips: R5900 DMULT[U], DDIV[U], LL, SC, LLD and SCD are user only

[Maciej W. Rozycki]

Hi Fredrik,

> I agree, that is important too. I will post an updated v5 soon. Another
> alternative change is to define check_insn_opc_user_only as
> 
> static inline void check_insn_opc_user_only(DisasContext *ctx, int flags)
> {
> #ifndef CONFIG_USER_ONLY
> check_insn_opc_removed(ctx, flags);
> #endif
> }
> 
> by referring to check_insn_opc_removed (instead of copying its definition).
> Would you consider this an improvement for v5 too?

 Yes, it does look like an improvement to me, reducing code duplication.  
Thanks for looking into it further.

> > here (and swapping the two former calls ought to be fixed separately; I 
> > haven't checked if there are more cases like this, but if so, then they 
> > would best be amended with a single change).
> 
> I'll defer other ordering and indentation fixes since I'm not sure whether
> such changes would be accepted.

 Sure, no need for you to rush doing that.  In the absence of someone 
willing to do such clean-ups voluntarily I would consider it a 
maintainer's duty really.

  Maciej

Re: [Qemu-devel] [PATCH v4 5/8] target/mips: R5900 DMULT[U], DDIV[U], LL, SC, LLD and SCD are user only

[Maciej W. Rozycki]

Hi Fredrik,

 Nitpicking here, but I think it's what makes code clean and pleasant to 
read.

On Sun, 16 Sep 2018, Fredrik Noring wrote:

> diff --git a/target/mips/translate.c b/target/mips/translate.c
> index 77d678353e..327e96307b 100644
> --- a/target/mips/translate.c
> +++ b/target/mips/translate.c
> @@ -22458,6 +22458,7 @@ static void decode_opc_special_legacy(CPUMIPSState 
> *env, DisasContext *ctx)
>  case OPC_DMULTU:
>  case OPC_DDIV:
>  case OPC_DDIVU:
> +check_insn_opc_user_only(ctx, INSN_R5900);
>  check_insn(ctx, ISA_MIPS3);
>  check_mips_64(ctx);
>  gen_muldiv(ctx, op1, 0, rs, rt);

 I think it would make sense to keep the order of the checks consistent, 
or otherwise code starts looking messy.

 The predominant order appears to be (as applicable) starting with a 
check for the lowest ISA membership, followed by a check for the highest 
ISA membership (R6 instruction cuts) and ending with processor-specific 
special cases.  I think this order makes sense in that it starts with 
checks that have a wider scope and then moves on to ones of a narrower 
scope, and I think keeping it would be good (as would fixing where the 
addition of R6 broke it).  Mode checks for otherwise existing 
instructions then follow, which complements the pattern.

 So please make it:

check_insn(ctx, ISA_MIPS3);
check_insn_opc_user_only(ctx, INSN_R5900);
check_mips_64(ctx);

> @@ -24962,6 +24963,7 @@ static void decode_opc(CPUMIPSState *env, 
> DisasContext *ctx)
>   break;
>  case OPC_LL: /* Load and stores */
>  check_insn(ctx, ISA_MIPS2);
> +check_insn_opc_user_only(ctx, INSN_R5900);
>  /* Fallthrough */
>  case OPC_LWL:
>  case OPC_LWR:
> @@ -24987,6 +24989,7 @@ static void decode_opc(CPUMIPSState *env, 
> DisasContext *ctx)
>  case OPC_SC:
>  check_insn(ctx, ISA_MIPS2);
>   check_insn_opc_removed(ctx, ISA_MIPS32R6);
> +check_insn_opc_user_only(ctx, INSN_R5900);
>   gen_st_cond(ctx, op, rt, rs, imm);
>   break;
>  case OPC_CACHE:

 OK in these two cases (noting a preexisting formatting issue to fix 
separately).

> @@ -25253,9 +25256,11 @@ static void decode_opc(CPUMIPSState *env, 
> DisasContext *ctx)
>  
>  #if defined(TARGET_MIPS64)
>  /* MIPS64 opcodes */
> +case OPC_LLD:
> +check_insn_opc_user_only(ctx, INSN_R5900);
> +/* fall through */
>  case OPC_LDL:
>  case OPC_LDR:
> -case OPC_LLD:
>  check_insn_opc_removed(ctx, ISA_MIPS32R6);
>  /* fall through */
>  case OPC_LWU:

 And here, because of the fall-through.

> @@ -25275,6 +25280,7 @@ static void decode_opc(CPUMIPSState *env, 
> DisasContext *ctx)
>  break;
>  case OPC_SCD:
>  check_insn_opc_removed(ctx, ISA_MIPS32R6);
> +check_insn_opc_user_only(ctx, INSN_R5900);
>  check_insn(ctx, ISA_MIPS3);
>  check_mips_64(ctx);
>  gen_st_cond(ctx, op, rt, rs, imm);

 And please make it:

check_insn_opc_removed(ctx, ISA_MIPS32R6);
check_insn(ctx, ISA_MIPS3);
check_insn_opc_user_only(ctx, INSN_R5900);
check_mips_64(ctx);

here (and swapping the two former calls ought to be fixed separately; I 
haven't checked if there are more cases like this, but if so, then they 
would best be amended with a single change).

  Maciej

Re: [Qemu-devel] [PATCH v3] target/mips: Support R5900 GCC programs in user mode

[Maciej W. Rozycki]

Hi Fredrik,

> Many thanks for your review, Maciej,

 You are welcome!

> >  Eventually you'll have to remove all these instructions (plus LL and SC) 
> > from the system emulation mode.  In fact I think it would make sense to do 
> > that right away, because I believe it will be a reasonably simple update. 
> > However if it turns out to be a can of worms after all, then I think we 
> > can defer it, because we do not have a bare-metal environment ready for 
> > this CPU anyway (or do we?).
> 
> Indeed, very simple. I added a new check function check_insn_opc_user_only,
> similar to check_insn_opc_removed, conditional on USER_ONLY and so a simple
> 
>   check_insn_opc_user_only(ctx, INSN_R5900);
> 
> has to be added for these OPC cases. To test this I tried to build GCC with
> the target mips64r5900el but it unfortunately didn't work as GCC failed to
> compile itself:
> 
>   ../sysdeps/mips/mips64/mul_1.S:49: Error: opcode not supported on
>   this processor: r5900 (mips3) `dmultu $8,$7'

 That file is a part of GMP rather that GCC I believe.  A copy is used 
by glibc too.

 I think you'll have to arrange for GMP and glibc builds to avoid this 
file for R5900 targets, either by supplying a handcoded R5900 assembly 
alternative that avoids the missing instruction (possibly by using a 
sequence of MULTU operations), or by using the generic C-language 
version from mul_1.c.

 I wouldn't recommend placing an architecture override within the 
generic 64-bit MIPS assembly so as to force it to build, as any kernel 
DMULTU emulation is bound to be slower, because it'll have to do all 
that we need to do here anyway, and then it'll add the overhead of 
calling an exception handler.

 I suspect there will be more such cases to handle in GMP and glibc.  
As GMP holds the master copies of code you'll need to submit any updates 
to that project first, and once accepted they can be carried over to 
glibc.

 As I suggested before you may wish to focus on making o32 work first 
rather than keeping being distracted by 64-bit issues, so that we have 
at least some functional code upstream before diving into enhancements 
and corner cases.

 HTH,

  Maciej

Re: [Qemu-devel] [PATCH v2] target/mips: Initial support for MIPS R5900

[Maciej W. Rozycki]

Hi Fredrik,

> Aleksandar, Aurelien, Maciej -- are you happy with this initial v2 patch?

 I have been more thorough on this occasion, and I do hope I have caught 
everything.  See the notes below, in addition to what the others wrote.  

 Please apply to v3 accordingly; I started writing this before you sent 
that version.

> --- a/target/mips/mips-defs.h
> +++ b/target/mips/mips-defs.h
> @@ -62,6 +63,7 @@
>  #define  CPU_MIPS3   (CPU_MIPS2 | ISA_MIPS3)
>  #define  CPU_MIPS4   (CPU_MIPS3 | ISA_MIPS4)
>  #define  CPU_VR54XX  (CPU_MIPS4 | INSN_VR54XX)
> +#define  CPU_R5900   (CPU_MIPS4 | INSN_R5900)

 I think this has to be (CPU_MIPS3 | INSN_R5900) really.

 While the CPU does support MIPS IV MOVN, MOVZ and PREF instructions, 
that's all it has from that ISA.  Even the Tx79, which was supposed to 
have a regular IEEE-754 FPU, did not have the extra FP condition bits, 
MOVN.fmt, MOVZ.fmt or any of the MOVF* or MOVT* instructions, indexed 
load/store instructions, multiply-accumulate instructions, etc. defined.

 So I think the R5900 has to be treated as a MIPS III implementation, with 
some aberrations.  I don't expect it to be a big deal to add INSN_R5900 
qualification to MOVN, MOVZ and PREF instruction emulation code right 
away.

 Then presumably you are going to add emulation for all the missing MIPS 
III instructions to the Linux kernel eventually, to match the MIPS III 
Linux user ABI?  Apart from LLD and SCD discussed previously this will 
have to include DDIV, DDIVU, DMULT and DMULTU.

 Eventually you'll have to remove all these instructions (plus LL and SC) 
from the system emulation mode.  In fact I think it would make sense to do 
that right away, because I believe it will be a reasonably simple update. 
However if it turns out to be a can of worms after all, then I think we 
can defer it, because we do not have a bare-metal environment ready for 
this CPU anyway (or do we?).

> --- a/target/mips/translate.c
> +++ b/target/mips/translate.c
> @@ -17357,7 +17382,11 @@ static void decode_opc_special_legacy(CPUMIPSState 
> *env, DisasContext *ctx)
>  break;
>  case OPC_MULT:
>  case OPC_MULTU:
> -if (sa) {
> +if (ctx->insn_flags & INSN_R5900) {
> +gen_muldiv(ctx, op1, 0, rs, rt);
> +if (rd != 0)
> +gen_mul_r5900(ctx, op1, rd, rs, rt);
> +} else if (sa) {
>  check_insn(ctx, INSN_VR54XX);
>  op1 = MASK_MUL_VR54XX(ctx->opcode);
>  gen_mul_vr54xx(ctx, op1, rd, rs, rt);

 I think there is no point in executing the multiplication twice if `rd != 
0' and also we want to continue trapping on `sa != 0' for the non-Vr54xx 
case, so how about:

if (sa) {
check_insn(ctx, INSN_VR54XX);
op1 = MASK_MUL_VR54XX(ctx->opcode);
gen_mul_vr54xx(ctx, op1, rd, rs, rt);
} else if (rd && (ctx->insn_flags & INSN_R5900)) {
gen_mul_r5900(ctx, op1, rd, rs, rt);
} else {
gen_muldiv(ctx, op1, rd & 3, rs, rt);
}

?

> --- a/target/mips/translate_init.inc.c
> +++ b/target/mips/translate_init.inc.c
> @@ -410,6 +410,50 @@ const mips_def_t mips_defs[] =
>  .insn_flags = CPU_MIPS32R5 | ASE_MSA,
>  .mmu_type = MMU_TYPE_R4000,
>  },
> +{
> +.name = "R5900",
> +.CP0_PRid = 0x3800,
> +/* No L2 cache, icache size 32k, dcache size 32k, uncached 
> coherency. */
> +.CP0_Config0 = (1 << 17) | (0x3 << 9) | (0x3 << 6) | (0x2 << 
> CP0C0_K0),

 Why ICE set but DCE clear?  I guess this corresponds to the incorrect L2 
cache reference as bit #17 is SC on the original R4000.  That reference 
has to go, obviously, as there's no L2 cache indication in the R5900's 
Config register.

 As to ICE and DCE their reset defaults are both 0, so we might as well 
use that, as we don't emulate caches anyway.  If it turns out to matter to 
any software, then we'll have to provide a more correct Config register 
emulation as its writable bits are processor-specific.

> +/* Note: Config1 is only used internally, the R5900 has only 
> Config0. */
> +.CP0_Status_rw_bitmask = 0xF4C79C1F,

 Maybe swap the two lines so that the Config1 register reference does not 
confusingly seem to describe the Status r/w bitmask?

> +#ifdef CONFIG_USER_ONLY
> + /*
> +  * R5900 hardware traps to the Linux kernel for IEEE 754-1985 and LL/SC
> +  * emulation. For user-only, qemu is the kernel, so we emulate the traps
> +  * by simply emulating the instructions directly.
> +  */

 Please use spaces rather than tabs for indentation, as per QEMU's coding 
standard.

> +.CP0_Config1 = (1 << CP0C1_FP) | (47 << CP0C1_MMU),
> +.CP0_LLAddr_rw_bitmask = 0x,
> +.CP0_LLAddr_shift = 4,
> +.CP1_fcr0 = (0x38 << FCR0_PRID) | (0x0 << FCR0_REV),
> +.CP1_fcr31 = 0,
> +.CP1_fcr31_rw_bitmask 

Re: [Qemu-devel] [PATCH v2] target/mips: Initial support for MIPS R5900

[Maciej W. Rozycki]

Hi Fredrik,

> Aleksandar, Aurelien, Maciej -- are you happy with this initial v2 patch?

 I have skimmed over and I have a couple of comments.  I'll try to 
finalise them ASAP, however I'm currently at the GNU Tools Cauldron and 
much of my time is taken by the event.

  Maciej

Re: [Qemu-devel] [RFC] target/mips: Initial support for MIPS R5900

[Maciej W. Rozycki]

On Sat, 7 Jul 2018, Fredrik Noring wrote:

> The MIPS R5900 is normally taken to be MIPS3, but it has MOVN, MOVZ and PREF
> defined in MIPS4 which is why ISA_MIPS4 is chosen for this patch.

 It also has several instructions removed, so I don't think you can really 
just mark it MIPS IV without special-casing those instructions, or the 
emulation won't be accurate (and consequently programs that use them won't 
trigger exceptions that they are supposed to).

> Some flags in the mips_defs array are marked FIXME as I don't know the
> proper values.

 Well, the FPU is non-standard so until you implement it I'd rather kept 
it disabled and then all the FPU-related settings can go for now.  For the 
rest see below.

> --- a/target/mips/translate_init.inc.c
> +++ b/target/mips/translate_init.inc.c
> @@ -411,6 +411,26 @@ const mips_def_t mips_defs[] =
>  .mmu_type = MMU_TYPE_R4000,
>  },
>  {
> +.name = "R5900",
> +.CP0_PRid = 0x3800,
> +/* No L2 cache, icache size 32k, dcache size 32k, uncached 
> coherency. */
> +.CP0_Config0 = (1 << 17) | (0x3 << 9) | (0x3 << 6) | (0x2 << 
> CP0C0_K0),
> +/* Note: Config1 is only used internally, the R5900 has only 
> Config0. */
> +.CP0_Config1 = (1 << CP0C1_FP) | (47 << CP0C1_MMU),

 So I'd clear CP0C1_FP then; also make sure accessing CP0.Config1 from 
emulated code does what it does on actual hardware.

> +.CP0_LLAddr_rw_bitmask = 0x, /* FIXME */
> +.CP0_LLAddr_shift = 4,   /* FIXME */

 No LL/SC in the R5900, so the LLAddr settings can go.

> +.SYNCI_Step = 16,/* FIXME */

 SYNCI is MIPS32r2+, so this can go.

> +.CCRes = 2,  /* FIXME */

 Likewise, CCRes is MIPS32r2+, so this can go.

> +.CP0_Status_rw_bitmask = 0x3678, /* FIXME */

 This has to indicate which bits in CP0.Status are writable.  Check with 
the manual and/or actual hardware.

> +.CP1_fcr0 = (0x38 << FCR0_PRID) | (0x0 << FCR0_REV),
> +.CP1_fcr31 = 0,
> +.CP1_fcr31_rw_bitmask = 0x0183,  /* FIXME */

 This is all FPU stuff and it can go.

> +.SEGBITS = 40,   /* FIXME */

 This is the number of virtual address bits.  Determined by the highest 
writable CP0.EntryHi.VPN2 bit.

> +.PABITS = 36,/* FIXME */

 Likewise physical address bits.  Determined by the highest writable 
CP0.EntryLo0.PFN and CP0.EntryLo1.PFN bit.

> +.insn_flags = CPU_R5900,
> +.mmu_type = MMU_TYPE_R4000,  /* FIXME */

 This looks right to me.

 FWIW; I don't have any authority for QEMU maintenance.

  Maciej

Re: [Qemu-devel] [PATCH v6 8/9] target-mips: Add nan2008 flavor of <CEIL|CVT|FLOOR|ROUND|TRUNC>.<L|W>.<S|D>

[Maciej W. Rozycki]

On Mon, 20 Jun 2016, Aleksandar Markovic wrote:

> This patch is about NaN-2008 flavor of Mips instructions CEIL, CVT, 
> FLOOR, ROUND, TRUNC only (its title is "Add nan2008 flavor...").
> 
> Legacy-NaN flavors of the same Mips instructions already operate 
> correctly, and there is nothing to be fixed.
> 
> The commit message is not description of another bug, it explains the 
> context of the change, and the content of added code segments.

 Well, if the matter does not affect MIPS emulation, then why do you think 
there's a need to mention it in the first place?

 I don't think stating what other emulation targets do in a commit 
description for a platform feature adds any value, but I think it bears 
the risk to confuse people examining code history.  Your description has 
to be meaningful for someone reading it say 10 years on from now who does 
not necessarily know the context nor has any of us available to ask.  
Please consider this.

 If you do think this mention adds value, then please make it unambiguous 
in the commit description that the third conversion variant applies to 
non-MIPS targets only.  It wasn't to me at all or I wouldn't have spoken 
out.

 Thank you.

  Maciej

Re: [Qemu-devel] [PATCH v6 8/9] target-mips: Add nan2008 flavor of <CEIL|CVT|FLOOR|ROUND|TRUNC>.<L|W>.<S|D>

[Maciej W. Rozycki]

On Tue, 14 Jun 2016, Leon Alrae wrote:

> > 1. A bug fix for SoftFloat legacy-NaN (original) MIPS support, which has 
> >been there probably since forever (i.e. since the MIPS target was added 
> >to QEMU).
> 
> I've just done another round of review and as far as I can tell these
> patches don't modify the legacy-NaN MIPS behaviour. I believe Aleksandar
> was referring to new functionality (i.e. 2008 NaN) only.

 Well, if there's a bug, then it needs to be fixed rather than documented.

  Maciej

Re: [Qemu-devel] [PATCH v6 8/9] target-mips: Add nan2008 flavor of <CEIL|CVT|FLOOR|ROUND|TRUNC>.<L|W>.<S|D>

[Maciej W. Rozycki]

On Fri, 10 Jun 2016, Aleksandar Markovic wrote:

> The changes that make QEMU behavior the same as hardware behavior (in 
> relation to CEIL, CVT, FLOOR, ROUND, TRUNC Mips instructions) are 
> already contained in this patch.

 Good, however that means that you've really combined two logically 
separate changes into a single patch:

1. A bug fix for SoftFloat legacy-NaN (original) MIPS support, which has 
   been there probably since forever (i.e. since the MIPS target was added 
   to QEMU).

2. A new feature for 2008-NaN MIPS support.

To me it really looks like the two need to be separate patches, with the 
bug fix applied first (or among any other bug fixes at the beginning) in 
the patch set, or even as a separate change marked as a prerequisite for 
the rest of the changes.

 The bug fix will then be self-contained and more prominently exposed, 
rather than being buried among feature additions.  It can then be 
independently reviewed and likely more easily accepted as long as it is 
technically correct.  It can also be cherry-picked and backported easily 
if necessary, perhaps outside the upstream tree.

 Review of the new feature set can then follow, once the bug(s) have been 
fixed.

> I just mentioned Mips-A / Mips-B / SoftFloat differences as an 
> explanation/observation related to the change in this patch.

 Maybe it's just myself, but from your description I got the impression 
that your change preserves the status quo and the explanation merely 
serves the purpose of documenting it.  Please consider rewriting it such 
that it is unambiguous that the SoftFloat bug is being fixed with your 
change.

 Obviously once you've made the bug fix a separate change, it'll become 
unambiguous naturally, as then you won't have the 2008-NaN feature along 
it obfuscating the picture.

  Maciej

Re: [Qemu-devel] [PATCH v6 8/9] target-mips: Add nan2008 flavor of <CEIL|CVT|FLOOR|ROUND|TRUNC>.<L|W>.<S|D>

[Maciej W. Rozycki]

On Fri, 10 Jun 2016, Aleksandar Markovic wrote:

> I referred to SoftFloat library in QEMU code, and the case "Operand is 
> smaller than INT_MIN" is different between SoftFloat and Mips-B, while 
> the case "Operand is a NaN" is different between SoftFloat and Mips-A.

 Ah, but then you just can and indeed need to fix it to match hardware.  
I had it done properly with the original patches I believe.

  Maciej

Re: [Qemu-devel] [PATCH] configure: Use $(..) instead of deprecated `..`

[Maciej W. Rozycki]

On Wed, 8 Jun 2016, Eric Blake wrote:

> >  Unlike `..` the $(..) Bourne shell construct is not fully portable, some 
> > implementations do not recognise it.
> 
> All POSIX implementations support it. The only shell that doesn't is
> from Solaris /bin/sh, and that pre-dates modern OpenSolaris which has
> (finally) modernized their shell.  And I don't think we have anyone
> actively trying to build qemu for Solaris (we have bug reports for
> mingw, BSD, and Mac OS, but I haven't seen anyone complaining about a
> failed Solaris build).

 The IRIX shell doesn't support it either I believe, as I suppose some 
other older systems.  We may or may not care about them (we might as well 
kindly ask people to install bash or suchlike), but I think it makes sense 
to make it explicit.

> >  NB given the above, and especially because of the introduced functional 
> > regression mentioned, I don't think this change qualifies as trivial.
> 
> I agree that calling this trivial may be a stretch, but I see no problem
> with the patch itself if the commit message is beefed up to provide more
> justification than just "silence a lint-like tool".

 Agreed, I think it's worth the little extra time needed to write it and 
have a commit description explanatory enough for anyone to understand the 
motivation.  Having one often turns a questionable change into an obvious 
one, not to mention that someone might scratch their head staring at a 
commit years on from now as they try to understand what the author 
actually meant to do.

 I find myself doing this frequently when discovering bugs introduced to 
software 10-15 years ago with commits having little if any description (as 
per the standards of that time), trying to figure out how to fix the bug
while not breaking the original intent of the change.

  Maciej

Re: [Qemu-devel] [PATCH] configure: Use $(..) instead of deprecated `..`

[Maciej W. Rozycki]

On Wed, 8 Jun 2016, Stefan Weil wrote:

> >  Unlike `..` the $(..) Bourne shell construct is not fully portable, some 
> > implementations do not recognise it.  Consequently this change potentially 
> > breaks building QEMU on some systems, possibly in a non-obvious way, as 
> > there's no explicit check for the presence this feature and a graceful 
> > failure path included with this patch or the other one AFAICT.  We may or 
> > may not care about those systems, but still this is a functional 
> > regression and therefore I think there has to be a good reason for 
> > introducing it.
> 
> 
> As the $() construct was already used in QEMU code before this commit
> (we had a mixture of both styles), I don't expect anything to break
> which was not broken before.

 Fair enough then.  Thanks for the explanation.  It would have made sense 
to mention it in the commit message though.

  Maciej

Re: [Qemu-devel] [PATCH] configure: Use $(..) instead of deprecated `..`

[Maciej W. Rozycki]

On Mon, 16 May 2016, Stefan Weil wrote:

> This fixes these warnings from shellcheck:
> 
> ^-- SC2006: Use $(..) instead of deprecated `..`
> 
> Signed-off-by: Stefan Weil 
> ---
> 
> More warnings from shellcheck for configure and other files
> will be handled by later patches.

 Unlike `..` the $(..) Bourne shell construct is not fully portable, some 
implementations do not recognise it.  Consequently this change potentially 
breaks building QEMU on some systems, possibly in a non-obvious way, as 
there's no explicit check for the presence this feature and a graceful 
failure path included with this patch or the other one AFAICT.  We may or 
may not care about those systems, but still this is a functional 
regression and therefore I think there has to be a good reason for 
introducing it.

 So what's the technical justification -- beyond shutting up some random 
checker tool -- for making this change?  Does the benefit recognised in 
making this change outweigh the limitation introduced?  How about handling 
failures properly?

 NB given the above, and especially because of the introduced functional 
regression mentioned, I don't think this change qualifies as trivial.

  Maciej

Re: [Qemu-devel] [PATCH v6 8/9] target-mips: Add nan2008 flavor of <CEIL|CVT|FLOOR|ROUND|TRUNC>.<L|W>.<S|D>

[Maciej W. Rozycki]

On Mon, 16 May 2016, Aleksandar Markovic wrote:

> Here one can distinguish three cases:
> 
> CASE MIPS-A: (FCR31.NAN2008 == 1)
> 
>1. Operand is a NaN, result should be 0;
>2. Operand is larger than INT_MAX, result should be INT_MAX;
>2. Operand is smaller than INT_MIN, result should be INT_MIN.
> 
> CASE MIPS-B: (FCR31.NAN2008 == 0)
> 
>1. Operand is a NaN, result should be INT_MAX;
>2. Operand is larger than INT_MAX, result should be INT_MAX;
>2. Operand is smaller than INT_MIN, result should be INT_MAX.
> 
> CASE SOFTFLOAT:
> 
>1. Operand is a NaN, result is INT_MAX;
>2. Operand is larger than INT_MAX, result is INT_MAX;
>2. Operand is smaller than INT_MIN, result is INT_MIN.
> 
> It is interesting that neither MIPS-A nor MIPS-B desired behaviors
> are in this sense identical to correspondent SoftFloat behavior.

 A bug in SoftFloat probably -- it has only been fixed quite recently to 
handle all MIPS architecture's peculiarites correctly and this case must 
have been missed then, making data produced not necessarily identical 
between the legacy-NaN hard-float and the soft-float ABIs, even though 
intent has been for it to be.

 FAOD, you refer to SoftFloat in the most recent version of GCC, do you?

  Maciej

Re: [Qemu-devel] [PATCH v5 7/9] target-mips: Add nan2008 flavor of <CEIL|CVT|FLOOR|ROUND|TRUNC>.<L|W>.<S|D>

[Maciej W. Rozycki]

On Wed, 4 May 2016, Aleksandar Markovic wrote:

> > > @@ -8919,7 +8920,11 @@ static void gen_farith (DisasContext *ctx, enum 
> > > fopcode op1,
> > >  TCGv_i64 fp64 = tcg_temp_new_i64();
> > >
> > >  gen_load_fpr32(ctx, fp32, fs);
> > > -gen_helper_float_roundl_s(fp64, cpu_env, fp32);
> > > +if ((ctx->insn_flags & ISA_MIPS32R6) && (ctx->nan2008)) {
> 
> > Why testing the version of the architecture? This will generate wrong
> > helper for P5600 which is R5 and IEEE 754-2008 compliant.
> 
> The code reflects the documentation.
> 
> I just want to bring to your attention following two versions of the 
> paragraph detailing return value of ROUND.L.S for some special cases - one 
> version from R6 documention, and the other from R5 documentation:
> 
> R6: ("MIPS® Architecture For Programmers Volume II-A: The MIPS64® Instruction 
> Set Reference Manual", Revision 6.04, page 393)
> 
> "When the source value is Infinity, NaN, or rounds to an integer outside the 
> range -2 63 to 2 63 -1, the result cannot be
> represented correctly and an IEEE Invalid Operation condition exists. The 
> Invalid Operation flag is set in the FCSR.
> If the Invalid Operation Enable bit is set in the FCSR, no result is written 
> to fd and an Invalid Operation exception is
> taken immediately. Otherwise, a default result is written to fd. On cores 
> with FCSR[NAN2008]=0, the default result is
> 2^63–1. On cores with FCSR[NAN2008]=1, the default result is:
>   • 0 when the input value is NaN
>   • 2^63 –1 when the input value is +infinity or rounds to a number larger 
> than 2^63 –1
>   • -2^63 –1 when the input value is –infinity or rounds to a number smaller 
> than -2^63 –1"

 This is clearly wrong given that FCSR[NAN2008] is hardwired to 1 in R6; 
see Table 6.8 in MIPS64 vol. I rev. 6.01.  Please note that individual 
instruction descriptions should not be used as a (primary) reference, 
especially in the case of documentation discrepancies -- refer to Table 
6.3 in vol. I for architectural definitions.

> R5: ("MIPS® Architecture For Programmers Volume II-A: The MIPS64® Instruction 
> Set Reference Manual", Revision 5.04, page 314)
> 
> "When the source value is Infinity, NaN, or rounds to an integer outside the 
> range -2 63 to 2 63 -1, the result cannot be
> represented correctly and an IEEE Invalid Operation condition exists. In this 
> case the Invalid Operation flag is set in
> the FCSR. If the Invalid Operation Enable bit is set in the FCSR, no result 
> is written to fd and an Invalid Operation
> exception is taken immediately. Otherwise, the default result, 2 63 – 1, is 
> written to fd."

 This is likewise wrong, and refer to Table 5.7 and 5.3 respectively in 
MIPS64 vol. I rev. 5.04.

 In any case FCSR[NAN2008] is the only condition to be referred.

  Maciej

Re: [Qemu-devel] [PATCH v5 5/9] target-mips: Activate IEEE 274-2008 signaling NaN bit meaning

[Maciej W. Rozycki]

On Mon, 25 Apr 2016, Aleksandar Markovic wrote:

> No, nothing is lost. The plan is to add this functionality at a later time.

 OK then, as you prefer.  Although I find the order somewhat odd as r5+ is 
a special case of r3.

  Maciej

Re: [Qemu-devel] [PATCH v5 5/9] target-mips: Activate IEEE 274-2008 signaling NaN bit meaning

[Maciej W. Rozycki]

On Mon, 18 Apr 2016, Aleksandar Markovic wrote:

> Functions mips_cpu_reset() and msa_reset() are updated so that flag
> snan_bit_is_one is properly set for any Mips FPU/MSA configuration.
> For main FPUs, CPUs with FCR31's FCR31_NAN2008 bit set will invoke
> set_snan_bit_is_one(0). For MSA, as it is IEEE 274-2008 compliant
> from it inception, set_snan_bit_is_one(0) will always be invoked.

 I have skimmed over the series -- have you lost MIPSr3 support (writable 
FCSR ABS2008 and NAN2008 bits) in porting?

  Maciej

Re: [Qemu-devel] [PATCH 0/2] target-mips: Initiate IEEE 754-2008 support for Mips

[Maciej W. Rozycki]

On Thu, 7 Apr 2016, Aleksandar Markovic wrote:

> This patch series is based on the original set of patches proposed by Maciej
> Rozycki: http://lists.nongnu.org/archive/html/qemu-devel/2014-12/msg00968.html

 Please try and spell out my name in full, thank you.

  Maciej

Re: [Qemu-devel] Memory mapping on MIPS

[Maciej W. Rozycki]

On Mon, 22 Feb 2016, Igor R wrote:

> What should happen if I map KSEG2 directly as a continuation of KSEG1,
> i.e. substitute TLB lookup with "address - (int32_t)KSEG1_BASE"? Guest
> Linux seems to work correctly (but maybe it's just a matter of luck?).

 The 32-bit MIPS port of Linux uses KSEG2 for kernel virtual mappings, 
that is memory allocated via `vmalloc' and loadable modules.  Your luck is 
the former is seldom used and you may not have loadable modules in your 
configuration either.

  Maciej

Re: [Qemu-devel] [PATCH 14/15] tcg-mips: Use mipsr6 instructions in branches

[Maciej W. Rozycki]

On Tue, 9 Feb 2016, Richard Henderson wrote:

> > So to be correct + efficient, it should only put the nop in if the next
> > generated instruction is a CTI. I imagine that would be a bit messy /
> > fragile, but maybe doable? I haven't looked too deeply.
> 
> Ouch, I didn't notice this about these insns.
> 
> I suppose this might be rare enough that it's still worth thinking about.  Off
> the top of my head I can't think of any way to save extra state, but perhaps
> just looking back at the previous insn's major opcode is enough when emitting
> any forbidden insn.

 FWIW I think this is a reasonable approach, applying to the regular MIPS 
ISA where the size of the instruction word is fixed so you can look at the 
preceding instruction in a reproducible manner.  And in the microMIPSr6 
ISA there are no forbidden slots, so no issue there in the first place.

  Maciej

Re: [Qemu-devel] [PATCH] target-mips: silence NaNs for cvt.s.d and cvt.d.s

[Maciej W. Rozycki]

On Sun, 6 Dec 2015, Aurelien Jarno wrote:

> cvt.s.d and cvt.d.s are FP operations and thus need to convert input
> sNaN into corresponding qNaN. Explicitely use the floatXX_maybe_silence_nan
> functions for that as the floatXX_to_floatXX functions do not do that.
> 
> Cc: Leon Alrae 
> Signed-off-by: Aurelien Jarno 
> ---
>  target-mips/op_helper.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/target-mips/op_helper.c b/target-mips/op_helper.c
> index d2c98c9..20e79be 100644
> --- a/target-mips/op_helper.c
> +++ b/target-mips/op_helper.c
> @@ -2545,6 +2545,7 @@ uint64_t helper_float_cvtd_s(CPUMIPSState *env, 
> uint32_t fst0)
>  uint64_t fdt2;
>  
>  fdt2 = float32_to_float64(fst0, >active_fpu.fp_status);
> +fdt2 = float64_maybe_silence_nan(fdt2);
>  update_fcr31(env, GETPC());
>  return fdt2;
>  }
> @@ -2634,6 +2635,7 @@ uint32_t helper_float_cvts_d(CPUMIPSState *env, 
> uint64_t fdt0)
>  uint32_t fst2;
>  
>  fst2 = float64_to_float32(fdt0, >active_fpu.fp_status);
> +fst2 = float32_maybe_silence_nan(fst2);
>  update_fcr31(env, GETPC());
>  return fst2;
>  }

 FYI, I posted a more general fix to this a while ago, however the review 
regrettably went nowhere.  See the archive of discussion starting at: 
 
for details, including the justification and further design consideration.

  Maciej

Re: [Qemu-devel] [PATCH v2] target-mips: remove wrong checks for recip.fmt and rsqrt.fmt

[Maciej W. Rozycki]

On Mon, 19 Oct 2015, Leon Alrae wrote:

> >> Instructions recip.{s|d} and rsqrt.{s|d} do not require 64-bit FPU neither
> >> they require any particular mode for its FPU. This patch removes the checks
> >> that may break a program that uses these instructions.
> > 
> > That is correct. That said these instructions do require at least a
> > MIPS32R2 or a MIPS64R1 CPU. I guess we should add these checks now that
> > check_cop1x do not guard them anymore.
> 
> I suspect that this might too restrictive if we take into account also
> legacy processors. As far as I know these instructions were already
> present in MIPS IV implementations. I think it'll be safer if for the
> pre-MIPS32R2 and pre-MIPS64R1 CPUs we keep the check_cop1x. (actually
> I'm not certain if check_cop1x is correct, but at least we aren't
> relaxing the previous restrictions too much).

 These instructions are unrestricted in the MIPS IV ISA[1].  In the 
MIPS64r1 ISA the double-precision variants do require CP0.Status.FR=1[2].  
See also .

References:

[1] "MIPS IV Instruction Set", MIPS Technologies, Inc., Revision 3.2, By 
Charles Price, September, 1995, pp. B-83, B-86



[2] "MIPS Architecture For Programmers, Volume II: The MIPS64 Instruction 
Set", MIPS Technologies, Inc., Document Number: MD00087, Revision 
1.00, August 29, 2002, pp. 244, 258

  Maciej

Re: [Qemu-devel] [PATCH pic32 v2 4/5] Two new processor variants: M4K and microAptivP.

[Maciej W. Rozycki]

On Wed, 1 Jul 2015, Aurelien Jarno wrote:

  diff --git a/target-mips/translate_init.c b/target-mips/translate_init.c
  index ddfaff8..430a547 100644
  --- a/target-mips/translate_init.c
  +++ b/target-mips/translate_init.c
  @@ -232,6 +232,52 @@ static const mips_def_t mips_defs[] =
   .mmu_type = MMU_TYPE_FMT,
   },
   {
  +/* Configuration for Microchip PIC32MX microcontroller. */
  +.name = M4K,
  +.CP0_PRid = 0x00018765,

 Hmm, does it make sense to set the Revision field here?  We keep it at 0 
for other templates, so why not 0x00018700?

 Also I suggest to move the template earlier on so that entries remain 
sorted by PRId, at least within the same vendor.  So this would go between 
4KEmR1 and 4KEc (the M4K is an MTI RTL, quite an old one actually).

  +{
  +/* Configuration for Microchip PIC32MZ microcontroller. */
  +.name = microAptivP,
  +.CP0_PRid = 0x00019e28,

 Same question here, why not 0x00019e00?  Also why not microAptivUP as 
documentation calls it (vs microAptivUC)?

 And again, it looks to me like the entry better followed M14Kc.

 Otherwise it looks ok, though I haven't look at the PIC32 manual to
 check the values.

 I haven't checked if the bit patterns for configuration registers are 
sane either.  These RTLs are configurable, so (within some limits) real 
hardware will have different values anyway.

  Maciej

Re: [Qemu-devel] [PATCH pic32 v2 2/5] Fixed random index generation for TLBWR instruction. It was not quite random and did not skip Wired entries.

[Maciej W. Rozycki]

On Wed, 1 Jul 2015, Aurelien Jarno wrote:

 Secondly, I don't think calling random() is the correct thing to do.
 It's an expensive function that is not thread safe. Quoting the
 specification:
 
   Within the required constraints of the upper and lower bounds, the
   manner in which the processor selects values for the Random register
   is implementation-dependent.
 
 So it's fine if we use a PRNG like the current code, but I agree we
 might want to improve it if it has some issues. We want to keep its
 value reproducible though so that the icount mode works as expected.

 Implementations often implement CP0.Random as a free-running counter that 
decrements between the bounds set as each instruction executes.

  Maciej

Re: [Qemu-devel] [PATCH] target-mips: add ERETNC instruction and Config5.LLB bit

[Maciej W. Rozycki]

On Fri, 5 Jun 2015, Leon Alrae wrote:

  As a side note, I have seen that you have added a check for MIPS2 to the
  ERET instruction. This is correct, but given in practice we don't
  emulate any MIPS1 CPU, I do wonder if it's not the time to make MIPS2
  the basic instruction set and remove all MIPS2 checks.
 
 Yes, in current codebase the MIPS2 checks don't seem to have much value
 and the removal makes sense. On the other hand I'm wondering if there
 are QEMU users who maintain artificial MIPS1 CPU templates locally to
 test if their compiler doesn't emit any non-MIPS1 instructions.

 Actually ERET is an instruction that has been added with the MIPS III 
ISA.  Sort of, that is, as it was not treated a part of the ISA, but an 
implementation detail specific to the particular exception model, 
introduced with the R4000.  It was only accepted as a part of the 
instruction set with the MIPS32/MIPS64 (r1) architecture that made the 
privilege model a part of the ISA.  Either way real MIPS II hardware (the 
R6000 and some LSI ISA approximates) used RFE instead of ERET, just like 
the R3000.

 As to the removal of MIPS I checks, I think there is some value in 
keeping them for documentation purposes if nothing else.  There's the TLB 
and cache model missing for R3000 class processors, but adding it 
shouldn't be that difficult.  As I keep maintaining support for real MIPS 
I hardware still around I'd be happy to add the missing bits and the right 
templates for some MIPS I implementations sometime, however regrettably I 
can't promise any deadline.  Adding an actual system to emulate along the 
CPU would be another matter.

 It would make sense IMO however to define the minimum ISA level supported 
somehow at the `configure' stage, so that parts of the emulator to support 
older processor models can be optimised away at the compilation stage.  
This would be conceptually similar to what we do in the Linux kernel with 
some optional features, including the ISA level, that are hardcoded to 
specific settings for individual systems where we know from how the system 
has been built that are always present (or absent).

 NB MIPS I user-mode Linux emulation would not be a perfect model as we 
have some additions to the kernel extending the bare MIPS I ISA in the 
Reserved Instruction exception handler to make software's life easier.

  Maciej

Re: [Qemu-devel] [PATCH v3 2/2] semihosting: add --semihosting-config arg sub-argument

[Maciej W. Rozycki]

On Thu, 21 May 2015, Liviu Ionescu wrote:

 p.s. and I think there are major differences between using -append to 
 pass arguments to the kernel and passing arguments to semihosting, this 
 being one of the reasons for suggesting a new command.

 Out of curiosity, why do you think there are differences between passing 
arguments to `vmlinux' and any other bare-metal executable, or maybe more 
specifically what differences do you have in mind?

  Maciej

Re: [Qemu-devel] [PATCH v3 2/2] target-mips: Misaligned memory accesses for MSA

[Maciej W. Rozycki]

On Thu, 14 May 2015, Leon Alrae wrote:

   I don't think we have.  The specification is a bit unclear I must admit 
  and it also defines the details of vector load and store operations as 
  implementation dependent, so there's no further clarification.
 
 This is specified in MIPS Architecture For Programmers Volume I-A:
 Introduction to the MIPS64 Architecture, Revision: 6.01, Imagination
 Technologies, Document Number: MD00083, August 20, 2014, p.142:
 
 For example, a misaligned vector load instruction will never leave its
 vector destination register half written, if part of a page split
 succeeds and the other part takes an exception. It is either all done,
 or not at all.

 Thanks.  It's good to see r6 has clarified the matters around here and 
I think we can (and for simplicity ought to) apply them to r5 too.

  Maciej

Re: [Qemu-devel] [PATCH v3 2/2] target-mips: Misaligned memory accesses for MSA

[Maciej W. Rozycki]

On Wed, 13 May 2015, Leon Alrae wrote:

  Certainly we do.  It's all in softmmu_template.h.
 
 I believe the problem is that MSA vector register's size is 16-bytes
 (this DATA_SIZE isn't supported in softmmu_template) and MSA load/store
 is supposed to be atomic.

 Not really AFAICT.  Here's what the specification says[1]:

The vector load instruction is atomic at the element level with no 
guaranteed ordering among elements, i.e. each element load is an atomic 
operation issued in no particular order with respect to the element's 
vector position.

and[2]:

The vector store instruction is atomic at the element level with no 
guaranteed ordering among elements, i.e. each element store is an atomic 
operation issued in no particular order with respect to the element's 
vector position.

so you only need to get atomic up to 8 bytes (with LD.D and ST.D, less 
with the narrower vector elements), and that looks supported to me.

References:

[1] MIPS Architecture for Programmers, Volume IV-j: The MIPS32 SIMD 
Architecture Module, Revision 1.07, MIPS Technologies, Inc., 
Document Number: MD00866, October 2, 2013, p. 314

[2] same, p. 414

  Maciej

Re: [Qemu-devel] [PATCH v3 2/2] target-mips: Misaligned memory accesses for MSA

[Maciej W. Rozycki]

On Wed, 13 May 2015, Richard Henderson wrote:

  +static inline void ensure_atomic_msa_block_access(CPUMIPSState *env,
  +  target_ulong addr,
  +  int rw,
  +  int mmu_idx)
   {
  +#if !defined(CONFIG_USER_ONLY)
  +#define MSA_PAGESPAN(x) (unlikelyx)  ~TARGET_PAGE_MASK)   
   \
  +   + MSA_WRLEN/8 - 1) = TARGET_PAGE_SIZE))
  +CPUState *cs = CPU(mips_env_get_cpu(env));
  +target_ulong page_addr;
   
  +if (MSA_PAGESPAN(addr)) {
  +/* first page */
  +tlb_fill(cs, addr, rw, mmu_idx, 0);
  +/* second page */
  +page_addr = (addr  TARGET_PAGE_MASK) + TARGET_PAGE_SIZE;
  +tlb_fill(cs, page_addr, rw, mmu_idx, 0);
   }
  +#endif
   }
 
 This doesn't do quite what you think it does.  It does trap if the page isn't
 mapped at all, but it doesn't trap if e.g. rw is set and the page is 
 read-only.
 That requires a subsequent check for what permissions were installed by
 tlb_set_page.
 
 I had thought there was a way to look this up besides duplicating the code in
 softmmu_template.h, but I suppose that's in a patch set that never made it in.

 We must have a way to deal with memory access operations issued by a 
single machine instruction crossing a page boundary already as this is 
what MIPS16 SAVE and RESTORE instructions as well as microMIPS SWP, SDP, 
SWM, SDM, LWP, LDP, LWM and LDM ones do.  Perhaps these are worth 
looking into and their approach copying (or reusing) here?

  Maciej

Re: [Qemu-devel] [PATCH v3 2/2] target-mips: Misaligned memory accesses for MSA

[Maciej W. Rozycki]

On Wed, 13 May 2015, Richard Henderson wrote:

  I believe the problem is that MSA vector register's size is 16-bytes
  (this DATA_SIZE isn't supported in softmmu_template) and MSA load/store
  is supposed to be atomic.
  
   Not really AFAICT.  Here's what the specification says[1]:
  
  The vector load instruction is atomic at the element level with no 
  guaranteed ordering among elements, i.e. each element load is an atomic 
  operation issued in no particular order with respect to the element's 
  vector position.
  
  and[2]:
  
  The vector store instruction is atomic at the element level with no 
  guaranteed ordering among elements, i.e. each element store is an atomic 
  operation issued in no particular order with respect to the element's 
  vector position.
  
  so you only need to get atomic up to 8 bytes (with LD.D and ST.D, less 
  with the narrower vector elements), and that looks supported to me.
 
 There's atomic in the transactional sense, and then there's atomic in the
 visibility to other actors on the bus sense.
 
 Presumably Leon is talking about the first, wherein we must ensure all writes
 to both pages must succeed.  Which just means making sure that both pages are
 present and writable before modifying any memory.

 I don't think we have.  The specification is a bit unclear I must admit 
and it also defines the details of vector load and store operations as 
implementation dependent, so there's no further clarification.

 However any unaligned loads or stores that cross a data-bus-width 
boundary require two bus cycles to complete and therefore by definition 
are not atomic in the visibility to other actors on the bus sense.  
Therefore the only atomicity sense that can be considered here is I 
believe transactional, on the per-element basis as this is what the 
specification refers to.

 Then the exact semantics of loads and stores is left up to the 
implementer, so for example ST.H can be implemented as 2 
doubleword-store transactions, or 4 word-store transactions (that 
wouldn't be allowed with ST.D), or 8 halfword-store transactions (that 
wouldn't be allowed with ST.W), but not 16 byte-store transactions (that 
would be allowed with ST.B).

 Consequently I believe only individual vector element writes (or reads, 
for that matter) are required to either successfully complete or 
completely back out, and a TLB, an address error or a bus error 
exception (or perhaps a hardware interrupt exception even) happening in 
the middle of a vector load or store instruction may observe the 
destination vector register or memory respectively partially updated 
with elements already transferred (but not an individual element 
partially transferred).

 That would be consistent with what happens with the other multi-word 
transfer instructions I mentioned when they get interrupted on the way 
(yes, they do allow hardware interrupts to break them too) and likely 
easier to implement as well.

 That's just my intepretation though.  Perhaps the specification needs a 
further clarification.

  Maciej

Re: [Qemu-devel] [PATCH 2/3] ppc64-softmmu: Remove unsupported FDC from config

[Maciej W. Rozycki]

On Wed, 11 Mar 2015, Alexander Graf wrote:

  So if you know how to get working floppy disk with qemu-system-ppc64,
  that would help me a lot in rejecting requests from libvirt folks :)
  Thanks :)
 
 I don't think you want floppy disk emulation on -M pseries at all. In
 fact, you only ever want floppy disk emulation on x86. So I'd recommend
 to change the logic in libvirt accordingly and just hard code floppy
 emulation to x86 (and if alpha or some other weird architecture needs it
 later as well, have them extend the list).

 For the record: the MIPS Malta board that we support also has direct PC 
FDD support.  The real board has a SMSC FDC37M817 super I/O controller 
chip wired to Intel 82371EB (PIIX4E) south bridge's LPC interface and a 
suitable IDC header for a FDD data cable soldered onto the PCB as well.  
So it's not only x86 that can make use of floppy disk emulation.  Note 
that there's no real ISA (no slots) on the Malta board, only the LPC and 
PIIX4E on-chip stuff.

  Maciej

Re: [Qemu-devel] mfocrf missing on e500v2

[Maciej W. Rozycki]

On Fri, 6 Mar 2015, Alexander Graf wrote:

 I've finally managed to check out why my e500v2 automated tests fail to
 run. Apparently they break because autotest wants to execute target code
 and runs into an illegal instruction while doing that:
 
 Program received signal SIGILL, Illegal instruction.
 0xb57c84cc in ?? ()
 (gdb) x /i $pc
 = 0xb57c84cc:mfocrf  r0,1
 
 And lo and behold, I really couldn't find any reference to mfocrf in the
 e500v2 spec.

 Do real e500 processors trap on this operation as well?  Here's what 
the Power ISA [1] says on the mtocrf and mfocrf instructions:

These forms of the mtcrf and mfcr instructions are intended to replace 
the old forms of the instructions (the forms shown in page 102), which 
will eventually be phased out of the architecture. The new forms are 
backward compatible with most processors that comply with versions of 
the architecture that precede Version 2.00.  On those processors, the 
new forms are treated as the old forms.

However, on some processors that comply with versions of the 
architecture that precede Version 2.00 the new forms may be treated as 
follows:

mtocrf: may cause the system illegal instruction error handler to be 
invoked

mfocrf: may place an undefined value into register RT

So it looks to me like the ISA, for its extension, relies on partial 
instruction decoding here.

 Furthermore both the opcode and the extended opcode are the same for 
each of mtcrf/mtocrf and mfcr/mfocrf, and the instructions within these 
pairs only differ by the value of a single bit within a field used for 
RA by operations that use it.  That bit is specified as reserved (/) for 
mtcrf/mfcr by e500 processor documentation [2][3][4] and likely a don't 
care in real implementations.  Offhand I suspect this is what pre-2.00 
ISA revisions did too.

 So perhaps the partial decoding of mtcrf and mfcr has to be implemented 
in QEMU for the processors concerned instead then?  The answer will 
depend on what actual hardware does.

References:

[1] Power ISA, IBM Corporation, Version 2.06 Revision B, July 23, 
2010, Section 3.3.15.1 Move To/From One Condition Register Field 
Instructions, p. 103

[2] PowerPC e500 Core Family Reference Manual, Freescale 
Semiconductor, Document ID: E500CORERM, Rev. 1, 4/2005, Table D-1 
Instructions (Binary) by Mnemonic, p. D-17

[3] same, Table D-2 Instructions (Decimal and Hexadecimal) by Opcode,
p. D-32

[4] same, Table D-3 Instructions (Binary) by Form, pp. D-37, D-48

  Maciej

Re: [Qemu-devel] mfocrf missing on e500v2

[Maciej W. Rozycki]

On Fri, 6 Mar 2015, Alexander Graf wrote:

  And lo and behold, I really couldn't find any reference to mfocrf in the
  e500v2 spec.
  
  Do real e500 processors trap on this operation as well?  
 
 The illegal instruction I posted about above was when running qemu 
 *on* e500v2, because TCG emits mfocrf ;).

 Ah OK, fair enough!  I guess that answers the question.

  Maciej

Re: [Qemu-devel] [PATCH 2/2] target-mips: add missing MSA and correct FP in VMState

[Maciej W. Rozycki]

On Thu, 19 Feb 2015, Leon Alrae wrote:

  Surely these fp_status fields are simply implementation of the architectural
  CSR registers?
  
  IMO you shouldn't store things related to TCG state, but always how the
  architecture represents it.  That way you're free to change the TCG
  implementation without breaking save/restore.
 
 Good point. Saving fp_status and msa_fp_status doesn't seem to be needed
 at all as they can be restored from FCSR and MSACSR respectively.

 Agreed.  Richard, thanks for the suggestion!

  Maciej

Re: [Qemu-devel] [PATCH] target-mips: fix CP0.BadVAddr by stopping translation on Address error

[Maciej W. Rozycki]

On Thu, 19 Feb 2015, Leon Alrae wrote:

   I think this deserves a better description as it is about the specific 
  case of an unaligned standard MIPS instruction fetch.  Address Error 
  exceptions can also happen for other reasons: unaligned data accesses or 
  any accesses outside memory segments the current execution mode is allowed 
  to reach.
 
 I believe that the one line change in the patch makes that clear. I
 agree however that the description itself could be more precise.

 I agree the patch itself is clear, but someone reading the commit might 
infer this is the only place the Address Error exception happens.  Also 
the patch itself does not show up in some places, e.g. `git log' where -p 
hasn't been also supplied, to complement information provided in the log.

   While at it I think it may be worth double-checking if the other places 
  that trigger this exception get it right.
 
 Other places seem to look fine. Even decode_micromips_opc handles it
 correctly whereas decode_opc -– which obviously was implemented before
 microMIPS -- never got fixed.

 Great that you checked, thanks!

  Maciej

Re: [Qemu-devel] [PATCH 7/7] target-mips: Add IEEE 754-2008 features support

[Maciej W. Rozycki]

On Tue, 9 Dec 2014, Maciej W. Rozycki wrote:

 Index: qemu-git-trunk/target-mips/op_helper.c
 ===
 --- qemu-git-trunk.orig/target-mips/op_helper.c   2014-12-08 
 23:22:12.0 +
 +++ qemu-git-trunk/target-mips/op_helper.c2014-12-08 23:25:02.558929097 
 +
 @@ -2274,8 +2274,12 @@ void mips_cpu_unassigned_access(CPUState
  
  #define FLOAT_TWO32 make_float32(1  30)
  #define FLOAT_TWO64 make_float64(1ULL  62)
 -#define FP_TO_INT32_OVERFLOW 0x7fff
 -#define FP_TO_INT64_OVERFLOW 0x7fffULL
 +#define FLOAT_INAN32(env)   \
 +(((env)-active_fpu.fcr31  (1  FCR31_NAN2008))   \
 + ? 0x : 0x7fff)
 +#define FLOAT_INAN64(env)   \
 +(((env)-active_fpu.fcr31  (1  FCR31_NAN2008))   \
 + ? 0xULL : 0x7fffULL)
  
  /* convert MIPS rounding mode in FCR31 to IEEE library */
  unsigned int ieee_rm[] = {
[...]
 @@ -2481,7 +2482,7 @@ uint64_t helper_float_cvtl_d(CPUMIPSStat
  dt2 = float64_to_int64(fdt0, env-active_fpu.fp_status);
  if (get_float_exception_flags(env-active_fpu.fp_status)
   (float_flag_invalid | float_flag_overflow)) {
 -dt2 = FP_TO_INT64_OVERFLOW;
 +dt2 = FLOAT_INAN64(env);
  }
  update_fcr31(env, GETPC());
  return dt2;
 @@ -2494,7 +2495,7 @@ uint64_t helper_float_cvtl_s(CPUMIPSStat
  dt2 = float32_to_int64(fst0, env-active_fpu.fp_status);
  if (get_float_exception_flags(env-active_fpu.fp_status)
   (float_flag_invalid | float_flag_overflow)) {
 -dt2 = FP_TO_INT64_OVERFLOW;
 +dt2 = FLOAT_INAN64(env);
  }
  update_fcr31(env, GETPC());
  return dt2;
 @@ -2520,14 +2521,14 @@ uint64_t helper_float_cvtpw_ps(CPUMIPSSt
  wt2 = float32_to_int32(fdt0  0X, env-active_fpu.fp_status);
  excp = get_float_exception_flags(env-active_fpu.fp_status);
  if (excp  (float_flag_overflow | float_flag_invalid)) {
 -wt2 = FP_TO_INT32_OVERFLOW;
 +wt2 = FLOAT_INAN32(env);
  }
  
  set_float_exception_flags(0, env-active_fpu.fp_status);
  wth2 = float32_to_int32(fdt0  32, env-active_fpu.fp_status);
  excph = get_float_exception_flags(env-active_fpu.fp_status);
  if (excph  (float_flag_overflow | float_flag_invalid)) {
 -wth2 = FP_TO_INT32_OVERFLOW;
 +wth2 = FLOAT_INAN32(env);
  }
  
  set_float_exception_flags(excp | excph, env-active_fpu.fp_status);
 @@ -2588,7 +2589,7 @@ uint32_t helper_float_cvtw_s(CPUMIPSStat
  wt2 = float32_to_int32(fst0, env-active_fpu.fp_status);
  if (get_float_exception_flags(env-active_fpu.fp_status)
   (float_flag_invalid | float_flag_overflow)) {
 -wt2 = FP_TO_INT32_OVERFLOW;
 +wt2 = FLOAT_INAN32(env);
  }
  update_fcr31(env, GETPC());
  return wt2;
 @@ -2601,7 +2602,7 @@ uint32_t helper_float_cvtw_d(CPUMIPSStat
  wt2 = float64_to_int32(fdt0, env-active_fpu.fp_status);
  if (get_float_exception_flags(env-active_fpu.fp_status)
   (float_flag_invalid | float_flag_overflow)) {
 -wt2 = FP_TO_INT32_OVERFLOW;
 +wt2 = FLOAT_INAN32(env);
  }
  update_fcr31(env, GETPC());
  return wt2;
 @@ -2616,7 +2617,7 @@ uint64_t helper_float_roundl_d(CPUMIPSSt
  restore_rounding_mode(env);
  if (get_float_exception_flags(env-active_fpu.fp_status)
   (float_flag_invalid | float_flag_overflow)) {
 -dt2 = FP_TO_INT64_OVERFLOW;
 +dt2 = FLOAT_INAN64(env);
  }
  update_fcr31(env, GETPC());
  return dt2;
 @@ -2631,7 +2632,7 @@ uint64_t helper_float_roundl_s(CPUMIPSSt
  restore_rounding_mode(env);
  if (get_float_exception_flags(env-active_fpu.fp_status)
   (float_flag_invalid | float_flag_overflow)) {
 -dt2 = FP_TO_INT64_OVERFLOW;
 +dt2 = FLOAT_INAN64(env);
  }
  update_fcr31(env, GETPC());
  return dt2;
 @@ -2646,7 +2647,7 @@ uint32_t helper_float_roundw_d(CPUMIPSSt
  restore_rounding_mode(env);
  if (get_float_exception_flags(env-active_fpu.fp_status)
   (float_flag_invalid | float_flag_overflow)) {
 -wt2 = FP_TO_INT32_OVERFLOW;
 +wt2 = FLOAT_INAN32(env);
  }
  update_fcr31(env, GETPC());
  return wt2;
 @@ -2661,7 +2662,7 @@ uint32_t helper_float_roundw_s(CPUMIPSSt
  restore_rounding_mode(env);
  if (get_float_exception_flags(env-active_fpu.fp_status)
   (float_flag_invalid | float_flag_overflow)) {
 -wt2 = FP_TO_INT32_OVERFLOW;
 +wt2 = FLOAT_INAN32(env);
  }
  update_fcr31(env, GETPC());
  return wt2;
 @@ -2674,7 +2675,7 @@ uint64_t helper_float_truncl_d(CPUMIPSSt
  dt2 = float64_to_int64_round_to_zero(fdt0, env-active_fpu.fp_status);
  if (get_float_exception_flags(env-active_fpu.fp_status)
   (float_flag_invalid

Re: [Qemu-devel] [PULL 06/37] target-ppc: VXSQRT Should Not Be Set for NaNs

[Maciej W. Rozycki]

On Wed, 7 Jan 2015, Alexander Graf wrote:

 diff --git a/target-ppc/fpu_helper.c b/target-ppc/fpu_helper.c
 index 7f74466..81db60f 100644
 --- a/target-ppc/fpu_helper.c
 +++ b/target-ppc/fpu_helper.c
 @@ -920,14 +923,16 @@ uint64_t helper_fsqrt(CPUPPCState *env, uint64_t arg)
  
  farg.ll = arg;
  
 -if (unlikely(float64_is_neg(farg.d)  !float64_is_zero(farg.d))) {
 -/* Square root of a negative nonzero number */
 -farg.ll = fload_invalid_op_excp(env, POWERPC_EXCP_FP_VXSQRT, 1);
 -} else {
 +if (unlikely(float64_is_any_nan(farg.d))) {
  if (unlikely(float64_is_signaling_nan(farg.d))) {
 -/* sNaN square root */
 +/* sNaN reciprocal square root */

 This change to the comment looks accidental, compare the changes below.  
Should it be reverted?  [Found this while resolving merge conflicts.]

  fload_invalid_op_excp(env, POWERPC_EXCP_FP_VXSNAN, 1);
 +farg.ll = float64_snan_to_qnan(farg.ll);
  }
 +} else if (unlikely(float64_is_neg(farg.d)  !float64_is_zero(farg.d))) 
 {
 +/* Square root of a negative nonzero number */
 +farg.ll = fload_invalid_op_excp(env, POWERPC_EXCP_FP_VXSQRT, 1);
 +} else {
  farg.d = float64_sqrt(farg.d, env-fp_status);
  }
  return farg.ll;
 @@ -974,17 +979,20 @@ uint64_t helper_frsqrte(CPUPPCState *env, uint64_t arg)
  
  farg.ll = arg;
  
 -if (unlikely(float64_is_neg(farg.d)  !float64_is_zero(farg.d))) {
 -/* Reciprocal square root of a negative nonzero number */
 -farg.ll = fload_invalid_op_excp(env, POWERPC_EXCP_FP_VXSQRT, 1);
 -} else {
 +if (unlikely(float64_is_any_nan(farg.d))) {
  if (unlikely(float64_is_signaling_nan(farg.d))) {
  /* sNaN reciprocal square root */
  fload_invalid_op_excp(env, POWERPC_EXCP_FP_VXSNAN, 1);
 +farg.ll = float64_snan_to_qnan(farg.ll);
  }
 +} else if (unlikely(float64_is_neg(farg.d)  !float64_is_zero(farg.d))) 
 {
 +/* Reciprocal square root of a negative nonzero number */
 +farg.ll = fload_invalid_op_excp(env, POWERPC_EXCP_FP_VXSQRT, 1);
 +} else {
  farg.d = float64_sqrt(farg.d, env-fp_status);
  farg.d = float64_div(float64_one, farg.d, env-fp_status);
  }
 +
  return farg.ll;
  }
  

  Maciej

Re: [Qemu-devel] [PATCH v2 1/2] target-mips: Rework ABIs to allow all required configurations

[Maciej W. Rozycki]

On Mon, 9 Feb 2015, Peter Maydell wrote:

  I'm not sure if it's a good idea to change the meaning of linux-user
  qemu-mips64 and qemu-mips64el, this will cause unnecessary confusion in
  my opinion. I think we’d be better off leaving it consistent across QEMU
  versions.
 
   Well, this is an example how the names could have been consistent from
  the beginning, and I actually agree we need to take a notion of what's
  already there.
 
 I think don't break executable names that are already present
 is a hard requirement. These get baked into binfmt-misc configurations
 and effectively become part of QEMU's ABI to users.

 Acknowledged, I do recognise and accept the requirement.

  So alternatively these could be called `mips64o32' and
  `mips64o32el' though I find these names somewhat ugly.  Although perhaps
  not anymore if we kept what we have now for backwards compatibility and
  added a set of uniform target names like this:
 
  - mips32o32|mips32o32el (or maybe just mipso32|mipso32el),
 
  - mips64o32|mips64o32el,
 
  - mips64n64|mips64n64el,
 
  - mips64n32|mips64n32el.
 
  Or maybe just the three latters, leaving mips|mipsel as it is.  WDYT?
 
 So which of these four does the current mips64/mips64el correspond
 to? That's the third in the list, right? And the current mipsn32/mipsn32el
 is the fourth?

 Correct.

 So this is adding alias names for our existing targets
 and creating a new one? I think I'd just leave the current names and
 define new target names where we need to.

 What I have been a bit concerned of is how the way these names were added 
(i.e. in an incremental manner and not really architected) results in what 
looks like a random selection of arbitrary names that are all but obvious 
to the user.

 Only mipsn32/mipsn32el have any notion of the selected ABI in the name, 
but then they lack any emphasis that this is a 64-bit ABI.  The remaining 
two pairs, mips/mipsel and mips64/mips64el, respectively refer to 
architecture names rather than a particular ABI, in the traditional sense 
(such as used by the GNU config.guess/config.sub scripts or reported by 
uname(2) on Linux) merely meaning 32-bit or 64-bit MIPS processors without 
any further implication as to the ABI (of course in the context of Linux a 
32-bit CPU cannot support anything but o32, but that's an indirect 
implication only).

 Of course ideally we'd have a single executable supporting all the ABIs 
and selecting the right one on the fly.  Or maybe two, to account for the 
two endiannesses, although being able to switch that at the run time so 
that a single interpreter handles all MIPS executables would be a great 
feature too.  But that's a differenr matter, and if such a unified 
configuration is ever implemented, then we can simply alias all the 
individual configuration names invented until then to one another.

 If we could figure out and write down what the design principle
 is for which targets to create to handle different ABIs in linux-user
 that would be handy, because at some point I need to think about
 this to handle the equivalent situation for ARM (a probable
 upcoming ILP32 ABI, and what to do about running 32-bit code on
 64-bit CPU definitions)...

 I think this is as simple as following what hardware offers, under the 
assumption that we want to be able to reproduce a simulated user-mode 
environment just as it would look like on actual hardware.  This could be 
for any reason, I think performance in the first place as QEMU run on a 
decent host machine can often be much, much faster than actual target 
hardware, and then the inability to reproduce target hardware beyond the 
CPU or to run a real Linux kernel on such hardware in the system emulation 
mode.

 So referring to the MIPS target -- as one I'm the most familiar with -- 
we have a choice of 3 Linux user ABIs in both endiannesses each.  And then 
we have a bunch of processors we already simulate or the simulation of 
which can be added, often in a straightforward manner as lots of 
documentation is available.  And now I expect to be able to pick any 
compatible CPU-ABI pair and simulate it.  It should correctly simulate all 
the instructions CPU implements and correctly trap with SIGILL on all the 
instructions CPU does not implement.  It can then be used for example for 
toolchain correctness validation.

 I don't know exactly how ARM architecture subsetting has been defined, 
but with the MIPS architecture it has been the case that 32-bit additions 
were added to a 64-bit architecture.  Specifically the MIPS IV 
architecture, a 64-bit one, added several features to the MIPS III 
architecture, also a 64-bit one.  These were conditional move operations, 
extra floating-point condition bits, the indexed addressing mode for 
floating-point load/store instructions and cache prefetch instructions. 
These were naturally available for 32-bit aka o32 programs run in the 
32-bit compatibility user mode.  This is unlike any 64-bit instructions as 

Re: [Qemu-devel] [PATCH 7/7] target-mips: Add IEEE 754-2008 features support

[Maciej W. Rozycki]

On Tue, 10 Feb 2015, Leon Alrae wrote:

   These cases could be addressed by either replacing subtraction from 0.0 
  with multiplication by -1.0, or by tweaking the rounding mode as needed 
  temporarily.  Given that the computational cost of multiplication is 
  uncertain and likely higher or at best the same as the cost of addition or 
  subtraction, I'd be leaning towards the latter solution.
 
 My first thought was to treat zero in NEG.fmt as a special case and use
 float32_chs() for it. But tweaking the rounding mode temporarily
 probably is better as we will get consistent behaviour for zero as well
 as input denormals which are squashed in float32_sub() when
 flush_inputs_to_zero flag is set (actually I'm not sure if legacy fp
 instructions should flush input denormals, but according to the spec
 this is implementation dependent so I won't worry about this).

 As expected setting CP1.FCSR.FS on a randomly picked R4400 processor:

CPU0 revision is: 0440 (R4400SC)
FPU revision is: 0500

does flush a NEG.fmt's input denormal to 0.  Given this program:

#include stdint.h
#include stdio.h

int main(void)
{
union {
double d;
uint64_t i;
} x = { .i = 0x000123456789abcdULL }, y, z;
unsigned long tmp, fcsr;

printf(x: %016lx\n, x.i);
asm volatile(
   cfc1%1, $31\n
   or  %2, %1, %4\n
   ctc1%2, $31\n
   neg.d   %0, %3\n
   ctc1%1, $31
: =f (y.d), =r (fcsr), =r (tmp)
: f (x.d), r (1  24));
printf(y: %016lx\n, y.i);
asm volatile(
   neg.d   %0, %1
: =f (z.d) : f (x.d));
printf(z: %016lx\n, z.i);
x.i = 0;
printf(+: %016lx\n, x.i);
asm volatile(
   neg.d   %0, %1
: =f (y.d) : f (x.d));
printf(-: %016lx\n, y.i);
return 0;
}

I get this output:

x: 000123456789abcd
y: 8000
z: 800123456789abcd
+: 
-: 8000

under Linux.  According to R4400 documentation the value of `z' must have 
been calculated by the in-kernel emulator in the Unimplemented Operation 
handler as for this processor implementation any denormalised operands 
cause this exception except for compare instructions.  But in any case all 
the results are consistent.  So we don't actually have to do anything for 
the flush-to-zero mode, our calculation should work out as expected (as 
long as the `float_round_down' rounding mode is respected that is).

 While at it I included the result of the negation of 0 for completeness.

  Maciej

Re: [Qemu-devel] [PATCH 7/7] target-mips: Add IEEE 754-2008 features support

[Maciej W. Rozycki]

On Mon, 9 Feb 2015, Leon Alrae wrote:

  +if (info-elf_flags  EF_MIPS_NAN2008)
  +env-active_fpu.fcr31 |=
  +(1  FCR31_NAN2008)  env-active_fpu.fcr31_rw_bitmask;
  +else
  +env-active_fpu.fcr31 =
  +~((1  FCR31_NAN2008)  env-active_fpu.fcr31_rw_bitmask);
 
 braces are needed here

 Umm...

  +uint32_t helper_float_chs_s(CPUMIPSState *env, uint32_t fst0)
  +{
  +uint32_t fst1;
  +
  +fst1 = float32_sub(0, fst0, env-active_fpu.fp_status);
  +update_fcr31(env, GETPC());
  +return fst1;
  +}
 
 I think there is one case where helper_float_chs_{d,s,ps} are not
 correct -- when we have zero. In this case in subFloat32Sigs() we call:
 
 return packFloat32(status-float_rounding_mode == float_round_down, 0, 0);
 
 and the packFloat32() definition:
 
 static inline float32 packFloat32(flag zSign, int_fast16_t zExp,
 uint32_t zSig)
 {
 
 return make_float32(
   ( ( (uint32_t) zSign )31 ) + ( ( (uint32_t) zExp )23 ) +
 zSig);
 
 }
 
 Which means that the sign may not get changed, whereas I believe NEG.fmt
 is supposed to reverse the sign bit of positive/negative zero regardless
 of rounding mode.

 Good catch, I missed this corner case, thanks!  Another corner case is 
then helper_float_abs_{d,s,ps} with -0 as input and `float_round_down' 
being the current rounding mode.

 These cases could be addressed by either replacing subtraction from 0.0 
with multiplication by -1.0, or by tweaking the rounding mode as needed 
temporarily.  Given that the computational cost of multiplication is 
uncertain and likely higher or at best the same as the cost of addition or 
subtraction, I'd be leaning towards the latter solution.

 Thoughts?

  Maciej

Re: [Qemu-devel] [PATCH v2 1/2] target-mips: Rework ABIs to allow all required configurations

[Maciej W. Rozycki]

On Mon, 9 Feb 2015, Leon Alrae wrote:

  Rework the MIPS ABIs and CPU emulations available according to the 
  following target list:
  
  - mips|mipsel   -- 32-bit CPUs only, system and user emulation mode, 
 o32 user ABI,
  
  - mips64|mips64el   -- 32-bit and 64-bit CPUs, system and user emulation 
 mode, o32 user ABI,
 
 I'm not sure if it's a good idea to change the meaning of linux-user
 qemu-mips64 and qemu-mips64el, this will cause unnecessary confusion in
 my opinion. I think we’d be better off leaving it consistent across QEMU
 versions.

 Well, this is an example how the names could have been consistent from 
the beginning, and I actually agree we need to take a notion of what's 
already there.  So alternatively these could be called `mips64o32' and 
`mips64o32el' though I find these names somewhat ugly.  Although perhaps 
not anymore if we kept what we have now for backwards compatibility and 
added a set of uniform target names like this:

- mips32o32|mips32o32el (or maybe just mipso32|mipso32el),

- mips64o32|mips64o32el,

- mips64n64|mips64n64el,

- mips64n32|mips64n32el.

Or maybe just the three latters, leaving mips|mipsel as it is.  WDYT?

 Do we really need MIPS64 executables for o32 ABI for linux-user? They
 would merely enable MIPS64 CPUs to run o32 programs. So far we've been
 handling this by using 32-bit CPUs (artificial if the real CPU don't
 exist), therefore I don't see an issue here. Also I'm concerned that
 once we add new executables, it will be difficult to revert that change
 later, thus we must be certain that this is the right way to go.

 There is a slight difference for some processors that do not have 32-bit 
counterparts.  Think of an o32 program run on an R1 processor, or, to 
pick a more modern example, a Loongson-2E CPU.  I think NetLogic or Cavium 
implementations qualify here as well.  I don't think hacking QEMU sources 
to add even more artificial silicon is a good way to address these cases.

  Maciej

Re: [Qemu-devel] [PATCH 1/7] softfloat: Fix sNaN handling in FP conversion operations

[Maciej W. Rozycki]

On Fri, 6 Feb 2015, Maciej W. Rozycki wrote:

   I think this means that:
(1) we want to put handling of silencing the signaling NaNs
into the NaN conversion functions themselves (since it's
too late to do it correctly once the commonNaNtoFloat16
function has thrown away data)
(2) that handling needs to be target specific, as most details
of quiet vs signaling NaNs are
  
   I note in passing that the float*ToCommonNaN and commonNaNToFloat*
   functions are used only in the back-to-back call sequence of
  return commonNaNToFloatX(floatYToCommonNaN(...))
   for handling the NaN inputs on FP-to-FP conversion.
  
I believe my proposal addresses your concerns in a burden-free way for
   target maintainers who look after processors that implement the IEEE 754
   standard as it stands.
  
  I don't, which is why I made the comment above. If you're going to
  fix up NaN handling in the float-to-float conversion routines we
  should do it in a way that lets us handle the behaviour of
  target CPUs we care about.
 
  No argument about maintaining correct emulation where it already is such, 
 sure.  Please note that `float64_maybe_silence_nan' and friends are 
 already target-specific, which should generally let us deal with things, 
 and I am sort of surprised ARM sets certain rules for sNaN silencing for 
 conversions and different ones for other arithmetic operations.  Or is it 
 really that an input sNaN's sign bit is propagated in the single weird 
 corner case only?

 OK, I see where the issue is -- this is specifically about the case where 
dropping trailing significand bits of an sNaN in the process of format 
conversion would result in an infinity being produced.  This is handled by 
`commonNaNToFloat*' functions, but not in a way the ARM architecture 
requires.  As a preexisting problem this has nothing to do with my 
proposal and neither my proposal makes things worse, though I indeed 
missed the opportunity to correct it.

 I've thought of two general solutions:

1. Given that `*_default_nan' are now functions make them accept the sign 
   bit as input and propagate it to output if required by a given 
   implementation.  Conversions would pass the input sign bit while
   arithmetic operations would hardcode it to 0.

2. Quieten an sNaN being converted in the wider format operated on.  For a 
   widening conversion it would be the output format just as in the 
   original patch proposed.  For a narrowing conversion it would be the 
   input format.

I'm leaning towards the second option as being more elegant and robust.

 So to give a specific example, `float32_to_float64' would have:

if (aSig) {
float64 nan;
nan = commonNaNToFloat64(float32ToCommonNaN(a, status), status);
return float64_maybe_silence_nan(nan);
}

but `float64_to_float32' would have:

if (aSig) {
float64 nan;
nan = float64_maybe_silence_nan(a);
return commonNaNToFloat32(float64ToCommonNaN(nan, status), status);
}

instead.  This does the right thing as far as NaN quietening is concerned 
and avoids the `!mantissa' case triggering in `commonNaNToFloat32'.

 Based on the lone contents of fpu/softfloat-specialize.h this solution 
looks correct to me, in particular for the SNAN_BIT_IS_ONE case -- are you 
aware of any processor architectures for which this approach would not 
produce the result required?

 I've had a look at target-arm/helper.c and target-arm/helper-a64.c and 
AFAICT the hacks they do for NaN quietening do not follow the architecture 
specification in narrowing conversions (they merely reproduce what I 
proposed with the original patch and therefore do not preserve the sign 
bit in the case concerned), so the approach I outlined above will actually 
progress these targets as well, and these hacks can then be removed.  
This solution also meets the IEEE 754-2008 requirement for the way NaN 
quietening is done where the preferred NaN encoding is implemented.

  Maciej

Re: [Qemu-devel] [PATCH 1/7] softfloat: Fix sNaN handling in FP conversion operations

[Maciej W. Rozycki]

On Fri, 6 Feb 2015, Peter Maydell wrote:

   What I think would make sense here is instead of say `float32_to_float64'
  making a call to `float64_maybe_silence_nan' directly, we'd have a static
  inline function or a macro called say `float64_convert_silence_nan'
  invoked where the former is in my patch proposed.  That function in turn
  would call the former function by default and which could be overridden by
  something else for individual targets that require it.
 
 This still doesn't work for ARM, because you can't do the
 silencing after conversion. You have to have a way for
 targets to put in target-specific behaviour directly in the
 64-to-32 conversion process, before data is lost in the narrowing.

 Somehow I missed from your description that ARM actually requires actions 
at an earlier stage, sorry.  What I understood is that it is only the sign 
bit that is lost, and that could be easily handled with a bespoke version 
of `float64_convert_silence_nan', etc.  Is that the sign bit is supposed 
to be preserved in quietening an sNaN iff truncation of a non-zero wider 
trailing significand field produced all-zeros in the narrower result, and 
otherwise set to zero then?

   And I think we shouldn't be making our decisions based on x86 despite its
  ubiquity -- it is, especially as the x87 implementation is concerned, the
  earliest attempt to implement IEEE 754 with all the oddities resulting.
  Or more specifically not even that but more of an FP implementation that
  (as of the 8087 FPU) Intel wanted to become the basis of the upcoming IEEE
  754 standard, and mostly succeeded in principle, but not in some details,
  some of which were later corrected with the 80287 and 80387
  implementations, and some of which had to stay, for compatibility reasons.
 
 My point is that you mustn't regress other targets. So either you
 need to retain the same behaviour for targets which don't specifically
 add new code/flags/whatever, or you need to go through and look up
 all the other architecture specifications to check that you haven't
 broken them (ie that any changes you make are actually improving their
 accuracy). This patch isn't doing the first of those, so you need
 to do the second...

 I agree making sure no target has regressed is a valuable goal, but TBH 
I'd welcome some cooperation from target maintainers here, especially as 
the current situation looks like core code is non-compliant and I find it
hard to believe all the processors out there (except MIPS) got their IEEE 
754 implementation wrong.  So what this change does I'd expect actually to 
*progress* most of the targets, except from maybe a couple of oddballs.

 And I would very much appreciate feedback from target maintainers of 
these oddball architectures, after all they know the internals of the 
processors they care of the best!

   Besides are you sure?
 
 I was asking if you'd checked all the targets whose behaviour you're
 changing, not making a definite you have broken x86 statement.
 x86 is just the most likely because as you say it is the weirdest
 and least compliant.

 We can surely sort out the level of support x87 requires in further 
updates to the proposed change.  I'd expect x86's more modern SSE 
implementation of IEEE 754, that I'm less familiar with (which I guess 
just shows my age), got the details just as the standard expects them, 
which in turn implies more complication on our side.  But if we set the 
minimum at 80387, then as it stands it does not appear to me that this 
change will regress anything.

 But as I say, I think it would be most productive if target maintainers 
of at least ARM and x86, and maybe other architectures as well if they 
have quirks in this area, chimed in and identified issues.  I think it is 
impractical to require submitters of a generic change like this to chase 
architecture manuals of all the processors affected and study and 
understand their instruction sets to infer whether the change is going to 
have a negative effect.  Especially given that from the current code and 
the standard it is supposed to implement it is clear to me that the code 
is buggy.

 I also think that papering the issue over by fixing the thing up somehow 
in the MIPS TCG and letting other people worry about their architecture of 
interest is not the way to go, although it clearly looks like the path of 
least resistance to me.

  I think this means that:
   (1) we want to put handling of silencing the signaling NaNs
   into the NaN conversion functions themselves (since it's
   too late to do it correctly once the commonNaNtoFloat16
   function has thrown away data)
   (2) that handling needs to be target specific, as most details
   of quiet vs signaling NaNs are
 
  I note in passing that the float*ToCommonNaN and commonNaNToFloat*
  functions are used only in the back-to-back call sequence of
 return commonNaNToFloatX(floatYToCommonNaN(...))
  for handling the NaN inputs on FP-to-FP conversion.
 
 

Re: [Qemu-devel] [PATCH 1/7] softfloat: Fix sNaN handling in FP conversion operations

[Maciej W. Rozycki]

On Thu, 5 Feb 2015, Peter Maydell wrote:

  Fix sNaN handling in floating-point format conversion operations, that
  are classified by the IEEE 754-2008 standard as general-computational
  operations [1]:
 
  5.4 formatOf general-computational operations
 
  5.4.2 Conversion operations for floating-point formats and decimal
  character sequences
 
  Implementations shall provide the following formatOf conversion
  operations from all supported floating-point formats to all supported
  floating-point formats, as well as conversions to and from decimal
  character sequences.  These operations shall not propagate non-canonical
  results.  Some format conversion operations produce results in a
  different radix than the operands.
 
  according to the quietening requirement [2] set by the same standard:
 
  7.2 Invalid operation
 
  For operations producing results in floating-point format, the default
  result of an operation that signals the invalid operation exception
  shall be a quiet NaN that should provide some diagnostic information
  (see 6.2).
 
  These operations are:
  a)   any general-computational or signaling-computational operation
   on a signaling NaN (see 6.2), except for some conversions (see
   5.12)
 
  and the reference above is [3]:
 
  5.12 Details of conversion between floating-point data and external
  character sequences
 
  so does not apply to conversions a pair of floating-point formats.
 
  Therefore quieten any sNaN encountered in floating-point format
  conversions, in the usual manner.
 
  References:
 
  [1] IEEE Standard for Floating-Point Arithmetic, IEEE Computer
  Society, IEEE Std 754-2008, 29 August 2008, pp. 21-22
 
  [2] same, p. 37
 
  [3] same, p. 30
 
  Signed-off-by: Maciej W. Rozycki ma...@codesourcery.com
  ---
   This is in particular how MIPS hardware operates, other processors
  supposedly do the same if they claim compliance to IEEE 754.
 
 ARM currently handles this issue by calling the _maybe_silence_nan()
 function in all target-specific helper functions that call
 float-to-float conversion functions (but that has its own
 issues, see below).
 
 Have you looked at the other architectures that use these
 functions to convert float values to see what their NaN
 handling semantics are? I agree about what the spec says,
 but we may well have targets which are not fully IEEE
 compliant and rely on the current semantics (x86 springs to
 mind).

 I think the IEEE 754 standard-compliant behaviour shouldn't be left to 
target helpers, it would bound to cause discrepancies and chaos.  I 
wouldn't expect every target maintainer to be fluent in IEEE 754 arcana, 
or even have access to the standard's normative document.

 What I think would make sense here is instead of say `float32_to_float64' 
making a call to `float64_maybe_silence_nan' directly, we'd have a static 
inline function or a macro called say `float64_convert_silence_nan' 
invoked where the former is in my patch proposed.  That function in turn 
would call the former function by default and which could be overridden by 
something else for individual targets that require it.

 And I think we shouldn't be making our decisions based on x86 despite its 
ubiquity -- it is, especially as the x87 implementation is concerned, the 
earliest attempt to implement IEEE 754 with all the oddities resulting.  
Or more specifically not even that but more of an FP implementation that 
(as of the 8087 FPU) Intel wanted to become the basis of the upcoming IEEE 
754 standard, and mostly succeeded in principle, but not in some details, 
some of which were later corrected with the 80287 and 80387 
implementations, and some of which had to stay, for compatibility reasons.

 For one the x87 doesn't really implement proper single or double 
arithmetic.  While there are two precision control bits in its control 
register, not all arithmetic operations respect them, making their 
calculation unconditionally in the extended precision instead.  And even 
these that do only respect them for the significand and not the 
exponent[1]:


* The precision control (PC) bits (bits 9-8) can be used to set the MCP 
  internal operating precision of the significand at less than the default 
  of 64 bits (extended precision).  This can be useful in providing 
  compatibility with early generation arithmetic processors of smaller 
  precision.  PC affects only the instructions ADD, SUB, DIV, MUL, and 
  SQRT. For all other instructions, either the precision is determined by 
  the opcode or extended precision is used.


 So I think that any x87 intricacies are best handled either by hooks 
similar to `float64_convert_silence_nan' I propose or by individual 
implementation of whole operations where the actions required diverge from 
the standard so much as to make the use of such hooks infeasible.

 Besides are you sure?  I see x87 documentation say this[2

Re: [Qemu-devel] [PATCH v2 6/7] softfloat: Add SoftFloat status `nan2008_mode' flag

[Maciej W. Rozycki]

On Thu, 5 Feb 2015, Peter Maydell wrote:

  Index: qemu-git-trunk/fpu/softfloat-specialize.h
  ===
  --- qemu-git-trunk.orig/fpu/softfloat-specialize.h  2014-12-11 
  22:42:41.128934304 +
  +++ qemu-git-trunk/fpu/softfloat-specialize.h   2014-12-11 
  22:43:02.128938514 +
  @@ -103,6 +109,10 @@ inline float16 float16_default_nan(STATU
   {
   #if defined(TARGET_ARM)
   return const_float16(0x7E00);
  +#elif SNAN_BIT_IS_VARIABLE
  +return STATUS(nan2008_mode)
  +   ? const_float16(0x7E00)
  +   : const_float16(0x7DFF);
   #elif SNAN_BIT_IS_ONE
   return const_float16(0x7DFF);
   #else
 
 Ah, I see now what the previous patch was in aid of.
 
 (I hadn't realised that the 2008 rev of IEEE754 nailed down
 the SNaN/QNaN bit sense. That was always a dumb thing to have
 left impdef, so good news I guess.)

 For MIPS the good side (or more likely intended) effect of the quiet bit 
being set for sNaNs was you could preinitialise hardware registers in any 
new FP context created to all-ones to have them trap uninitialised read 
accesses for both single and double (and for that matter paired-single) 
arithmetic.  With the quiet bit being clear for sNaNs, there is no such a 
single encoding available, you have to choose if you want to trap for 
single (and paired-single) arithmetic only, double arithmetic only, or not 
at all.

  Maciej

Re: [Qemu-devel] [PATCH 0/7] MIPS: IEEE 754-2008 features support

[Maciej W. Rozycki]

On Tue, 3 Feb 2015, Thomas Schwinge wrote:

   I think Thomas, being the writer of the majority of code comprising these 
  patches
 
 Too bad that Git doesn't allow for listing several authors.  ;-)

 I believe `Signed-off-by' serves this purpose:

The Signed-off-by: tag indicates that the signer was involved in the
development of the patch, or that he/she was in the patch's delivery path.

-- as originally specified by linux/Documentation/SubmittingPatches that 
we refer to from http://wiki.qemu.org/Contribute/SubmitAPatch.  See also 
Section 1, Point 12 Sign your work in the same document referred.

 The way the tag has been defined may make it a little bit ambiguous who 
the actual authors are, but I think it is safe to interpret it as meaning 
that all people named by `Signed-off-by' tags of a given patch being the 
authors of the change.

  is now in the best position to make such a statement happen.  
  Thomas -- will you be able to take it from here?  Thanks!
 
 It is fine to license these changes under the SoftFloat-2a license.

 Great!  Thanks for confirming.

  Maciej

Re: [Qemu-devel] [PATCH v2 3/7] softfloat: Convert `*_default_nan' variables into inline functions

[Maciej W. Rozycki]

On Sat, 31 Jan 2015, Peter Maydell wrote:

Hmm, so perhaps my idea for a later improvement:
  
Eventually we might want to move the new inline functions into a
   separate header to be included from softfloat.h instead of softfloat.c,
   but let's make changes one step at a time.
  
   will actually have to be made right away.  I suspect GCC is more liberal
   here due to its convoluted extern/static/inline semantics history.
   Sigh...
 
  I would suggest just using static inline, as we do elsewhere
  for little utility functions.
 
   Yes, that's exactly what they'd have to be moved into a separate header
  for.
 
 Why do they need to be moved into a different header to do this?
 I must be missing something...

 This is because fpu/softfloat-specialize.h is an implementation header 
private to SoftFloat and therefore such inline definitions won't be seen 
by users outside SoftFloat, such as target-mips/msa_helper.c.  And IMO 
they shouldn't be moved into include/fpu/softfloat.h itself as this header 
contains generic stuff and is supposed to have no TARGET_foo stuff, as 
observed by current usage and inferred from comments in fpu/softfloat.c.

 So ultimately I think the newly converted `*_default_nan' inline 
functions will need to go into include/fpu/softfloat-public-specialize.h 
or suchlike, which will be pulled from include/fpu/softfloat.h for general 
availability.

 Overall I think the whole arrangement in fpu/softfloat-specialize.h has a 
potential to being cleaned up by removing all the chains of #ifdef's and 
splitting the conditional bits into separate headers matching the current 
variants.  E.g. the presence of this construct:

#if defined(TARGET_SPARC)
const float64 float64_default_nan = const_float64(LIT64( 0x7FFF ));
#elif defined(TARGET_PPC) || defined(TARGET_ARM) || defined(TARGET_ALPHA)
const float64 float64_default_nan = const_float64(LIT64( 0x7FF8 ));
#elif SNAN_BIT_IS_ONE
const float64 float64_default_nan = const_float64(LIT64( 0x7FF7 ));
#else
const float64 float64_default_nan = const_float64(LIT64( 0xFFF8 ));
#endif

asks for 4 independent headers defining the 4 bit pattern styles used by 
targets for symbolic FP data, one each -- perhaps even pulled indirectly 
via target headers rather than copying the chain of #ifdef's above around 
#include's from wherever they'll be pulled.

 Please correct me if I am wrong, but it appears to me possible right away 
by removing fpu/softfloat-specialize.h and instead creating individual 
target-*/softfloat-specialize.h headers that'll pull the right variant 
each target requires from a pool of templates made available in fpu/.  
Then include/fpu/softfloat.h could do the same for the public stuff (i.e. 
these NaN bits concerned here) placed similarly in include/fpu/.

 FWIW,

  Maciej

Re: [Qemu-devel] [PATCH v2 3/7] softfloat: Convert `*_default_nan' variables into inline functions

[Maciej W. Rozycki]

On Fri, 30 Jan 2015, Peter Maydell wrote:

   Hmm, so perhaps my idea for a later improvement:
 
   Eventually we might want to move the new inline functions into a
  separate header to be included from softfloat.h instead of softfloat.c,
  but let's make changes one step at a time.
 
  will actually have to be made right away.  I suspect GCC is more liberal
  here due to its convoluted extern/static/inline semantics history.
  Sigh...
 
 I would suggest just using static inline, as we do elsewhere
 for little utility functions.

 Yes, that's exactly what they'd have to be moved into a separate header 
for.

  Maciej

Re: [Qemu-devel] [PATCH v2 3/7] softfloat: Convert `*_default_nan' variables into inline functions

[Maciej W. Rozycki]

On Fri, 30 Jan 2015, Leon Alrae wrote:

  @@ -760,6 +760,6 @@ static inline int float128_is_any_nan(fl
   
  /*
   | The pattern for a default generated quadruple-precision NaN.
   
  **/
  -extern const float128 float128_default_nan;
  +__inline__ float128 float128_default_nan(void);
   
 
 Unfortunately clang complains about it and eventually it won't link:
 
   CCaarch64-softmmu/target-arm/helper-a64.o
 In file included from /qemu/target-arm/helper-a64.c:20:
 In file included from /qemu/target-arm/cpu.h:37:
 In file included from /qemu/include/qemu-common.h:120:
 In file included from /qemu/include/qemu/bswap.h:8:
 /qemu/include/fpu/softfloat.h:485:20: warning: inline function
 'float32_default_nan' is not defined [-Wundefined-inline]
 __inline__ float32 float32_default_nan(void);
^
 /qemu/target-arm/helper-a64.c:345:19: note: used here
 nan = float32_default_nan();
   ^
 In file included from /qemu/target-arm/helper-a64.c:20:
 In file included from /qemu/target-arm/cpu.h:37:
 In file included from /qemu/include/qemu-common.h:120:
 In file included from /qemu/include/qemu/bswap.h:8:
 /qemu/include/fpu/softfloat.h:597:20: warning: inline function
 'float64_default_nan' is not defined [-Wundefined-inline]
 __inline__ float64 float64_default_nan(void);
^
 /qemu/target-arm/helper-a64.c:374:19: note: used here
 nan = float64_default_nan();
   ^
 2 warnings generated.
   CCaarch64-softmmu/target-arm/gdbstub64.o
   CCaarch64-softmmu/target-arm/crypto_helper.o
   GEN   trace/generated-helpers.c
   CCaarch64-softmmu/trace/generated-helpers.o
   LINK  aarch64-softmmu/qemu-system-aarch64
 fpu/softfloat.o: In function `commonNaNToFloat64':
 /qemu/fpu/softfloat-specialize.h:796: undefined reference to
 `float64_default_nan'
 /qemu/fpu/softfloat-specialize.h:805: undefined reference to
 `float64_default_nan'
 fpu/softfloat.o: In function `commonNaNToFloatx80':
 /qemu/fpu/softfloat-specialize.h:1007: undefined reference to
 `floatx80_default_nan'
 /qemu/fpu/softfloat-specialize.h:1014: undefined reference to
 `floatx80_default_nan'

 Hmm, so perhaps my idea for a later improvement:

  Eventually we might want to move the new inline functions into a
 separate header to be included from softfloat.h instead of softfloat.c,
 but let's make changes one step at a time.

will actually have to be made right away.  I suspect GCC is more liberal 
here due to its convoluted extern/static/inline semantics history.  
Sigh...

  Maciej

Re: [Qemu-devel] [PATCH] target-mips: use CP0EnLo_XI instead of magic number

[Maciej W. Rozycki]

On Thu, 29 Jan 2015, Leon Alrae wrote:

   And do we want to have CP0C3_LPA set in the few templates that do in the 
  first place?  AFAICT we don't really implement LPA so this bit will 
  confuse software.  Of course implementing it would be another option, not 
  very complicated AFAICS, and if we can track the requirement to update 
  MFC0 at that time, then the clean-up I mentioned above can be deferred 
  until then.
 
 In general I don't think it's a good idea to indicate presence of a
 feature in a CPU config if it isn't implemented at all in QEMU -- as you
 said, it will confuse software. As far as LPA goes I've got already an
 implementation of it (and XPA as well) which I haven't submitted
 upstream yet. I'll make sure it contains the change you suggested.

 Great, I'll leave it to you to sort out then.  Thanks!

  Maciej

Re: [Qemu-devel] [PATCH] target-mips: use CP0EnLo_XI instead of magic number

[Maciej W. Rozycki]

On Mon, 26 Jan 2015, Leon Alrae wrote:

 Signed-off-by: Leon Alrae leon.al...@imgtec.com
 ---

 Enthusiastically:

Reviewed-by: Maciej W. Rozycki ma...@linux-mips.org

 However...

 diff --git a/target-mips/translate.c b/target-mips/translate.c
 index 635192c..77d89be 100644
 --- a/target-mips/translate.c
 +++ b/target-mips/translate.c
 @@ -4947,7 +4947,7 @@ static void gen_mfc0(DisasContext *ctx, TCGv arg, int 
 reg, int sel)
  #if defined(TARGET_MIPS64)
  if (ctx-rxi) {
  TCGv tmp = tcg_temp_new();
 -tcg_gen_andi_tl(tmp, arg, (3ull  62));
 +tcg_gen_andi_tl(tmp, arg, (3ull  CP0EnLo_XI));
  tcg_gen_shri_tl(tmp, tmp, 32);

... don't we need to do:

tcg_gen_andi_tl(arg, arg, ~(3ull  CP0EnLo_XI));

here and for EntryLo1 as well (for LPA-enabled processors)?

  tcg_gen_or_tl(arg, arg, tmp);
  tcg_temp_free(tmp);

 And do we want to have CP0C3_LPA set in the few templates that do in the 
first place?  AFAICT we don't really implement LPA so this bit will 
confuse software.  Of course implementing it would be another option, not 
very complicated AFAICS, and if we can track the requirement to update 
MFC0 at that time, then the clean-up I mentioned above can be deferred 
until then.

  Maciej

Re: [Qemu-devel] Qemu with GDB - Query

[Maciej W. Rozycki]

On Sat, 24 Jan 2015, manish tiwari wrote:

 I am new to QEMU and trying to attach gdb with qemu on powepc host.
 
 I have tried below options
 
 qemu-system-ppc -enable-kvm -nographic -m 512 -M ppce500 -cpu e500mc -gdb
 tcp::1234 -s -S -kernel uImage -initrd rootfs.ext2.gz -append
 root=/dev/ram rw console=ttyS0,115200 -serial tcp::,server,
 
 
 With the above command I have attached gdb with qemu.
 Now I am not sure what needs to be done on host side(I have cross compiled
 GDB in the Host file system).
 
 Please help me what i need to do in host side ?

 So far you only enabled QEMU's RSP stub so that you can attach with GDB 
later on (RSP stands for GDB's Remote Serial Protocol).  So to attach from 
GDB you now need to issue a command like:

(gdb) target remote system_running_qemu:1234

from the machine you have GDB on.  Substitute `system_running_qemu' above 
with the name of the machine you have started QEMU on; if it is the same 
machine as one that runs GDB, then you can omit this part altogether.

 For the record, this is a QEMU developers' list, such questions are best 
directed to a place dedicated to QEMU users.  I am sure your favourite 
Internet search engine will be able to locate such a place for you.  
Please post your any further questions there rather than here.  Thank you.

  Maciej

Re: [Qemu-devel] [PATCH] target-mips: ll and lld cause AdEL exception for unaligned address

[Maciej W. Rozycki]

On Mon, 26 Jan 2015, Leon Alrae wrote:

 Signed-off-by: Leon Alrae leon.al...@imgtec.com
 ---

Reviewed-by: Maciej W. Rozycki ma...@linux-mips.org

  Maciej

Re: [Qemu-devel] [PATCH] target-mips: fix CP0.BadVAddr by stopping translation on Address error

[Maciej W. Rozycki]

On Mon, 26 Jan 2015, Leon Alrae wrote:

 BadVAddr is supposed to capture the most recent address that caused
 the exception. Currently this is not happening as translation is not stopped
 and BadVAddr is updated with subsequent addresses.
 
 Signed-off-by: Leon Alrae leon.al...@imgtec.com
 ---

 I think this deserves a better description as it is about the specific 
case of an unaligned standard MIPS instruction fetch.  Address Error 
exceptions can also happen for other reasons: unaligned data accesses or 
any accesses outside memory segments the current execution mode is allowed 
to reach.

 While at it I think it may be worth double-checking if the other places 
that trigger this exception get it right.

  Maciej

Re: [Qemu-devel] [PATCH] target-mips: fix detection of the end of the page during translation

[Maciej W. Rozycki]

On Mon, 26 Jan 2015, Leon Alrae wrote:

 The test is supposed to terminate TB if the end of the page is reached.
 However, with current implementation it may never succeed for microMIPS or
 mips16.
 
 Reported-by: Richard Henderson r...@twiddle.net
 Signed-off-by: Leon Alrae leon.al...@imgtec.com
 ---

 I'm not sure if you need this, but just in case it helps anyhow.

Reviewed-by: Maciej W. Rozycki ma...@linux-mips.org

 diff --git a/target-mips/translate.c b/target-mips/translate.c
 index e9d86b2..f33c10c 100644
 --- a/target-mips/translate.c
 +++ b/target-mips/translate.c
 @@ -19103,6 +19104,7 @@ gen_intermediate_code_internal(MIPSCPU *cpu, 
 TranslationBlock *tb,
  qemu_log(search pc %d\n, search_pc);
  
  pc_start = tb-pc;
 +next_page_start = (pc_start  TARGET_PAGE_MASK) + TARGET_PAGE_SIZE;

 As a related issue -- I don't know offhand how far we are with small page 
support, but we may have to revise these macros -- or specifically how 
TARGET_PAGE_BITS these build on has been defined -- once we get there, to 
avoid surprises.  Just a heads-up!

  Maciej

Re: [Qemu-devel] [PATCH] target-mips: Clean up switch fall through after commit fecd264

[Maciej W. Rozycki]

On Tue, 20 Jan 2015, Peter Maydell wrote:

 In this particular case, this part of the file is fine and the
 problem is simply that this patch as it stands introduces a single
 line (the one above) that's not indented correctly. The only fix
 required is to delete one space in the line added by the patch
 (thus avoiding introducing any new misindented lines).

 Right!  That's what I've been following too -- not to introduce 
misindentation despite any surrounding lines suffering from this problem.

 In the general case, we tend to not do whole-file reindentation,
 because it breaks 'git blame' and similar tools. It's not an
 outright ban, though -- I guess it comes down to a maintainer
 judgement call whether they think the benefit outweighs the cost
 for a particular bit of code.

 Fair enough.  In some cases misindentation causes confusion, but here it 
does seem to be the case.

  Maciej

Re: [Qemu-devel] [PATCH] target-mips: Clean up switch fall through after commit fecd264

[Maciej W. Rozycki]

On Tue, 20 Jan 2015, Markus Armbruster wrote:

  diff --git a/target-mips/translate.c b/target-mips/translate.c
  index e9d86b2..8abc12b 100644
  --- a/target-mips/translate.c
  +++ b/target-mips/translate.c
  @@ -18729,6 +18729,7 @@ static void decode_opc(CPUMIPSState *env, 
  DisasContext *ctx)
   case OPC_SWL:
   case OPC_SWR:
   check_insn_opc_removed(ctx, ISA_MIPS32R6);
  + /* fall through */
 
  Indent here seems to be out by one? The others look OK.
 
 Sorry about that.  Fix up on commit, or would you like a respin?

 It looks to me like this whole file requires reindentation, perhaps a 
mechanical update like that would be better.  There are many lines with 9 
leading spaces (`wc -l' tells me 20), that I deliberately left as they are 
with my recent patches in this area so as not to obfuscate semantic 
changes.  There may be some more issues.

  Maciej

Re: [Qemu-devel] [PATCH 2/2] m48t59: add mem_base value to m48t59_init_isa()

[Maciej W. Rozycki]

On Mon, 19 Jan 2015, Paolo Bonzini wrote:

  The reason I asked is simply because ISA devices never do MMIO (apart
  for the VGA window).
  
  You mean in the QEMU world? At least physical SCSI and Ethernet
  adapters had a MMIO space for the onboard ROM.
 
 Uh right, ROMs count as MMIO too.

 Some ISA Ethernet cards also used MMIO for r/w access, probably to get at 
packet memory more efficiently (I don't remember the details offhand) as 
port I/O transactions were notoriously slow; in any case this is where the 
Memory field printed by `ifconfig' under Linux comes from.  I'm sure 
there was other ISA equipment too using MMIO for one purpose or another.

 On a PC/AT class x86 computer these resources would normally be allocated 
somehow to the memory space in the 0xd-0xe range, to work with 
real-mode software.  With somehow usually meaning jumpers, though newer 
cards may have had DOS configuration software available to set it up, in a 
similar manner to how ECU configured port I/O and MMIO resources for EISA 
equipment.

 BTW there were ISA DRAM expansion cards in existence too.

  Maciej

Re: [Qemu-devel] [PATCH] target-mips: add CPU definition for MIPS-II

[Maciej W. Rozycki]

On Tue, 25 Nov 2014, Vasileios Kalintiris wrote:

 Add mips2-generic among CPU definitions for MIPS.
 
 Signed-off-by: Vasileios Kalintiris vasileios.kalinti...@imgtec.com
 ---
  target-mips/translate_init.c | 23 +++
  1 file changed, 23 insertions(+)
 
 diff --git a/target-mips/translate_init.c b/target-mips/translate_init.c
 index 148b394..d4b1cd8 100644
 --- a/target-mips/translate_init.c
 +++ b/target-mips/translate_init.c
 @@ -108,6 +108,29 @@ struct mips_def_t {
  static const mips_def_t mips_defs[] =
  {
  {
 +/* A generic CPU providing MIPS-II features.
 +   FIXME: Eventually this should be replaced by a real CPU model. */

 Umm, this comment is wishful thinking I am afraid, getting a real MIPS II 
processor emulated, i.e. the R6000, will be quite a challenge.

 For one COP0 is completely different to anything else, in particular as 
far as the MMU and the cache are concerned.  Plus we don't know the 
opcodes for some instructions, e.g. the R6000-specific LD and SD 
operations, implemented analogously to LDC1 and SDC1; their opcodes might 
be just the same as MIPS III+ LD/SD (which should be safe for user 
software as in 32-bit user mode they'd trap on MIPS III+ processors such 
as the R4000, etc.), but I wouldn't bet on it.  Likewise all the cache 
maintenance instructions.

 To the best of my knowledge there wasn't any other plain MIPS II 
implementation that might be easier to emulate.  LSI's TinyRISC processors 
were close, but lacked the LL and SC instructions (TR4101 didn't have SYNC 
either).  OTOH they had the MIPS16 ASE implemented; the original version 
that is of course, i.e. no compact jumps, SAVE/RESTORE, etc.  They had no 
FPU either -- a COP1 interface was available and an FPU chip apparently 
planned, but never implemented.

 All the other 32-bit pieces of the era were AFAIK more of MIPS I than 
MIPS II implementations.  That in particular includes IDT and Toshiba 
gear.  Maybe I have missed something and someone can speak out who knows 
better what was available around mid 1990s though.

 +.name = mips2-generic,
 +.CP0_PRid = 0x00018000,

 You need to set CP0.PRId such that CompanyID is 0, meaning a legacy 
processor, or otherwise software will become confused.  In the R6000 the 
actual value was, according to Linux sources, 0x0300, the same as in 
the R3000A.  I am not actually sure how the two quite different processors 
could be told apart, perhaps poking at another CP0 register would do.

 +.CP0_Config0 = MIPS_CONFIG0 | (MMU_TYPE_R4000  CP0C0_MT),
 +.CP0_Config1 = MIPS_CONFIG1 | (1  CP0C1_FP) | (15  CP0C1_MMU) |
 +  (0  CP0C1_IS) | (3  CP0C1_IL) | (1  CP0C1_IA) |
 +  (0  CP0C1_DS) | (3  CP0C1_DL) | (1  CP0C1_DA) |
 +  (0  CP0C1_CA),
 +.CP0_Config2 = MIPS_CONFIG2,
 +.CP0_Config3 = MIPS_CONFIG3,

 Likewise, these aren't right, there were no CP0 Config registers on the 
R6000, the cache subsystem was completely different.  I realise some of 
these bits are fake for internal use, but the rest should best be cleared, 
they can only confuse the reader.

 +.CP0_LLAddr_rw_bitmask = 0,
 +.CP0_LLAddr_shift = 4,
 +.SYNCI_Step = 32,
 +.CCRes = 2,
 +.CP0_Status_rw_bitmask = 0x3011,
 +.CP1_fcr0 = (1  FCR0_W) | (1  FCR0_D) | (1  FCR0_S),

 This isn't right, for a MIPS II processor CP1.FIR will have these bits 
clear and the Implementation/Revision fields set to a non-zero value to 
let software probe for the presence of the FPU.

 +.SEGBITS = 32,
 +.PABITS = 32,
 +.insn_flags = CPU_MIPS2,
 +.mmu_type = MMU_TYPE_R4000,
 +},

 Yeah, as I say the MMU type isn't really right.  Mind that these 
processors used RFE rather than ERET for exception return too, to reflect 
a different exception model (the same as with MIPS I) and consequently 
layout of the CP0.Status register; all of which to be taken into account 
for a MIPS II processor as well.

 What do you really need this processor template for and how do you 
propose to use it?  Is it worth including one upstream that diverges so 
much from what the actual processor was like?  Is it for the user 
emulation mode only?

  Maciej

[Qemu-devel] [PATCH] target-mips: Make CP0.Status.CU1 read-only for the 5Kc and 5KEc processors

[Maciej W. Rozycki]

Signed-off-by: Maciej W. Rozycki ma...@codesourcery.com
---
Hi,

 As we discussed previously, please apply.

 [This got stuck in postponed e-mail, I thought I had sent it already.]

  Maciej

qemu-mips-5kc-cu1.diff
Index: qemu-git-trunk/target-mips/translate_init.c
===
--- qemu-git-trunk.orig/target-mips/translate_init.c2014-11-20 
10:47:31.578938672 +
+++ qemu-git-trunk/target-mips/translate_init.c 2014-11-20 10:47:39.078939989 
+
@@ -474,7 +474,7 @@ static const mips_def_t mips_defs[] =
 .CP0_LLAddr_shift = 4,
 .SYNCI_Step = 32,
 .CCRes = 2,
-.CP0_Status_rw_bitmask = 0x32F8,
+.CP0_Status_rw_bitmask = 0x12F8,
 .SEGBITS = 42,
 .PABITS = 36,
 .insn_flags = CPU_MIPS64,
@@ -575,7 +575,7 @@ static const mips_def_t mips_defs[] =
 .CP0_LLAddr_shift = 4,
 .SYNCI_Step = 32,
 .CCRes = 2,
-.CP0_Status_rw_bitmask = 0x32F8,
+.CP0_Status_rw_bitmask = 0x12F8,
 .SEGBITS = 42,
 .PABITS = 36,
 .insn_flags = CPU_MIPS64R2,