RE: [PATCH] Add MIPS UFR support

[Andrew Bennett <Andrew.Bennett@imgtec.com>]

> From: Mike Frysinger [mailto:vapier@gentoo.org] 
> Sent: 30 November 2013 08:57
> To: gdb-patches@sourceware.org
> Cc: Andrew Bennett
> Subject: Re: [PATCH] Add MIPS UFR support
> 
> On Friday 08 November 2013 12:36:45 Andrew Bennett wrote:
> > This patch adds support to the MIPS backend to deal with changing the FR
> > mode in user mode (which I will now refer to as UFR).  The technical
> > details are explained in the following paragraph.
> > 
> > The read only field UFR (at bit 28) in the floating point implementation
> > register (CP1 control register 0) represents if the CPU supports UFR.  The
> > UFR field (bit 2) in configuration register 5 (CP0 register 16, select 5)
> > allows user mode to enable or disable UFR support.  The current value of
> > the FR mode can be obtained if a read is made from the UFR register (CP1
> > control register 1), and UFR support is enabled. If register zero is
> > written to the UFR register, and UFR support is enabled, then the FR mode
> > is set to 0. If register zero is written to the UNFR register (CP1 control
> > register 4), and the UFR support is enabled, then the FR mode is set to 1.
> > 
> > To implement this I have firstly added the config 5 register to the
> > simulator model, and added support to read and write to it.  Secondly, I
> > have added support for the CTC1 and CFC1 instructions to write/read
> > to/from the UFR and UNFR registers.
> 
> is this standard functionality available to all CPUs ?  your new status_UFRP 
> bit overlaps with the existing status_CU0 bit, and you unconditionally enable 
> this feature.
> 
> > I have also added a testcase to validate the implementation.  To run the
> > testcase you will need to apply the following binutils patch:
> > 
> > https://sourceware.org/ml/binutils/2013-11/msg00065.html
> > 
> > 
> > The simulator patch is attached to this email and the ChangeLog is shown
> > below.
> 
> looks like your comments need tweaking to follow GNU style.  that means a 
> period at the end followed by two spaces and then the closing */.  i also see 
> "Unpredictable();" and that needs a space before the "(".

Many thanks for finding these I will amend my patch.

> in your mips.igen change, the code changes the else case from a NOP to 
> Unpredictable().  is that really what you want ?

Yes, I was following the spec from the MIPS 32 instruction set document at the
following URL: http://www.imgtec.com/powervr/insider/powervr-login.asp?doc=MD00082

> > This is my first patch to gdb, so I am unsure the protocol on committing. 
> > Would someone be able to clarify?
> 
> does your employer have copyright assignments in place ?
> 
> otherwise, we don't have a MIPS sim maintainer atm, so i'll do a crappy stand-
> in job.

No, but we are working on getting this sorted out.


Regards,


Andrew