Re: [MIPS] FPU emulator: allow Cause bits of FCSR to be writeable by ctc1

["Kevin D. Kissell" <kevink@paralogos.com>]

Atsushi Nemoto wrote:
> On Wed, 05 May 2010 08:43:16 -0700, "Kevin D. Kissell" <kevink@paralogos.com> wrote:
>   
>>>> I'm glad to hear that the patch is functional.  It was pretty clear that
>>>> it would solve your problem, but I was concerned that the inability to
>>>> write the Cause bits was done as a way around some other problem. 
>>>> Someone went to a certain amount of trouble to not accept them as
>>>> inputs, in violation of spec.  I just looked back, and the bug was
>>>> introduced as part of a patch of Ralf's from April 2005 entitled "Fix
>>>> Preemption and SMP problems in the FP emulator".  It's unlikely that
>>>> overriding CTC semantics actually fixed any underlying problems, but it
>>>> may have masked symptoms of problems that we can only hope have been
>>>> fixed in the mean time. Anyone run this patch on an FPUless SMP?   Oh,
>>>> for a 34Kf with a bunch of TCs! ;o)
>>>>         
>>> That's commit ID 72402ec9efdb2ea7e0ec6223cf20e7301a57da02 and the patch
>>> was comitted during the CVS days which only records the comitter but not
>>> the author.  The actual author is Atsushi Nemoto.  
>>>       
>> Well,. I certainly understood that you were simply the guy who did the
>> commit.  Didn't mean to make it sound like an accusation, but it was
>> marginally easier to type your name and date than to find, cut, and
>> paste the commit ID.  Sorry if it came off wrong.  It would be cool if
>> Atsushi remembered the reasoning behind the change, but empirical proof
>> that undoing it doesn't create a subtle problem for current SMP kernels
>> would be better still.
>>     
>
> Yes, that's my patch.  Though I cannot remember precisely, maybe I
> just had (mis)thought Cause bits in FCSR are read-only at that time.
> I have never used real SMP MIPS platforms, so there must be no
> SMP-related issues.
>
> I'm OK with Kevin's fix.  Thank you very much for reporting and
> investigating.
>   
The patch was labeled for "preemption and SMP problems", and if you 
weren't working on an SMP system, I'd guess that you were working with 
full preemption and seeing a problem that you might have assumed was 
also relevant to SMP.  The FPU emulator *shouldn't* have pre-emption 
issues, since it works off of data structures that are instantiated per 
thread context.  The FCSR seen by thread A is logically independent of 
that seen by thread B, so that even if one emulation was preempted in 
the middle by another, they shouldn't be able to interfere with one 
another.  That was the concept, anyway.

I'm cool with the patch as is, but in the general spirit of regarding 
numeric constants other than 0 and 1 as instruments of Satan, it would 
probably be even better if those reserved bits were defined 
(FPU_CSR_RSVD, or whatever is compatible with existing convention for 
such bits) along with the other FCSR bit masks in mipsregs.h, so that 
the assigment looks like:

          ctx->fcr31 = (value & ~(FPU_CSR_RSVD | 0x3)) |
                   ieee_rm[value & 0x3];

That way, in the unlikely event that those reserved bits get 
architecturally assigned, the definition of the field can be changed, 
and that absolute hex mask won't be hardwired and burried in the code to 
screw things up.  If you don't want to touch mipsregs.h, then you should 
consider just using 0x3 as the value mask, and not 0x1c0003 as I 
proposed.  I honestly can't think of any problems that would arise from 
implementing those bits as R/W, even though they aren't in hardware, 
apart from maybe not successfully running AVPs as user-mode Linux binaries.

          Regards,

          Kevin K.