Re: [PATCH v2] MIPS: Fix type and FCSR mask.

From: Ralf Baechle <ralf@linux-mips.org>
To: "Maciej W. Rozycki" <macro@imgtec.com>
Cc: "Steven J. Hill" <Steven.Hill@caviumnetworks.com>,
	LMO <linux-mips@linux-mips.org>
Subject: Re: [PATCH v2] MIPS: Fix type and FCSR mask.
Date: Wed, 11 May 2016 13:41:17 +0200	[thread overview]
Message-ID: <20160511114117.GK16402@linux-mips.org> (raw)
In-Reply-To: <alpine.DEB.2.00.1605111200200.6794@tp.orcam.me.uk>

On Wed, May 11, 2016 at 12:16:13PM +0100, Maciej W. Rozycki wrote:

> On Tue, 10 May 2016, Steven J. Hill wrote:
> 
> > The FCSR register is always 32-bits regardless if the platform is
> > 32 or 64-bits. Change the type from 'long' to 'int' to reflect this.
> > The entire upper half-word of the FCSR register orginally set all
> > the bits to 1. Some platforms like the Octeon III simulator will
> > actually fault if ones are written to the reserved and/or the FPU
> > bits. Correct the mask to avoid this.
> 
>  This change is broken, see the description of commit 9b26616c8d9d ("MIPS: 
> Respect the ISA level in FCSR handling") where this code comes from.  The 
> very purpose is to probe for the writability of bits 31:18, in particular 
> NAN2008 and ABS2008 stuff, but it applies to vendor bits too.  An accurate 
> identification of writable bits is required for the correct presentation 
> of FCSR via ptrace(2) for programs like GDB.
> 
>  You need to fix your simulator instead, the architecture does not permit 
> trapping on optional FCSR bits (there are no reserved bits there anymore 
> with the current architecture revision) especially as access to this 
> register is unprivileged.  I don't think we can support arbitrary 
> non-compliant architecture implementations -- if you need to handle an 
> erratum, then please do it on a PRId by PRId basis.
> 
>  As to changing the data type, I'm fine in principle, but then please do 
> so across all our source base where CP1 control registers are handled.  
> Here the `long' type is used for consistency with the rest of code, so 
> changing just this single place seems gratuitous to me.
> 
>  Ralf, please discard this change until it has been corrected.

Using a 32 bit variable made sufficient sense to me to apply the patch
to me.  However I agree, that the simulator's behaviour is overzealous.
While in violation of the architecture specification this is probably
similar in spirit as the mode of MIPSsim that was keeping every bit of
the system as a tristate (0, 1 and uninitialized) which indeed cought a
number of issues.

  Ralf