Re: [Qemu-devel] [Qemu-ppc] [PATCH 0/2] PPC handles mcrfs incorrectly

[David Gibson <david@gibson.dropbear.id.au>]

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

On Sun, Jan 24, 2016 at 03:41:24PM +0000, James Clarke wrote:
> Here is the description of the mcrfs instruction from the PowerPC Architecture
> Book, Version 2.02, Book I: PowerPC User Instruction Set Architecture
> (http://www.ibm.com/developerworks/systems/library/es-archguide-v2.html), found
> on page 120:

Thanks I've merged these fixes to ppc-for-2.6 which I'll send a pull
request for shortly.

> 
>     The contents of FPSCR field BFA are copied to Condition Register field BF.
>     All exception bits copied are set to 0 in the FPSCR. If the FX bit is
>     copied, it is set to 0 in the FPSCR.
> 
>     Special Registers Altered:
>         CR field BF
>         FX OX                        (if BFA=0)
>         UX ZX XX VXSNAN              (if BFA=1)
>         VXISI VXIDI VXZDZ VXIMZ      (if BFA=2)
>         VXVC                         (if BFA=3)
>         VXSOFT VXSQRT VXCVI          (if BFA=5)
> 
> However, currently every bit in FPSCR field BFA is set to 0, including ones not
> on that list.
> 
> I noticed this with the following simple C program:
> 
>     #include <fenv.h>
>     #include <stdio.h>
> 
>     int main(int argc, char **argv) {
>         int ret;
>         ret = fegetround();
>         printf("Current rounding: %d\n", ret);
>         ret = fesetround(FE_UPWARD);
>         printf("Setting to FE_UPWARD (%d): %d\n", FE_UPWARD, ret);
>         ret = fegetround();
>         printf("Current rounding: %d\n", ret);
>         ret = fegetround();
>         printf("Current rounding: %d\n", ret);
>         return 0;
>     }
> 
> which gave the output:
> 
>     Current rounding: 0
>     Setting to FE_UPWARD (2): 0
>     Current rounding: 2
>     Current rounding: 0
> 
> instead of (with these patches applied):
> 
>     Current rounding: 0
>     Setting to FE_UPWARD (2): 0
>     Current rounding: 2
>     Current rounding: 2
> 
> The relevant disassembly is in fegetround(), which, on my system, is:
> 
>     __GI___fegetround:
>     <+0>:   mcrfs  cr7, cr7
>     <+4>:   mfcr   r3
>     <+8>:   clrldi r3, r3, 62
>     <+12>:  blr
> 
> What happens is that, the first time fegetround() is called, FPSCR field 7 is
> retrieved. However, because of the bug in mcrfs, the entirety of field 7 is set
> to 0, which includes the rounding mode.
> 
> There are other issues this will fix, such as condition flags not persisting
> when they should if read, and if you were to read a specific field with some
> exception bits set, but no others were set in the entire register, then the
> bits would be cleared correctly, but FEX/VX would not be updated to 0 as they
> should be.
> 
> The first commit is because some FP_ macros needed to calculate
> FP_EX_CLEAR_BITS did not exist, and I reordered all the FP_ macros so that they
> are defined in the same order as the FPSCR_ macros.
> 
> James Clarke (2):
>   target-ppc: Make every FPSCR_ macro have a corresponding FP_ macro
>   target-ppc: mcrfs should always update FEX/VX and only clear exception
>     bits
> 
>  target-ppc/cpu.h       | 37 ++++++++++++++++++++++++++++---------
>  target-ppc/translate.c | 15 +++++++++++----
>  2 files changed, 39 insertions(+), 13 deletions(-)
> 

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]