RE: [PATCH] Add MIPS UFR support

[Andrew Bennett <Andrew dot Bennett at imgtec dot 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.
>

The following patch and ChangeLog addresses the issues raised with coding style, and status_UFR.
I have also removed the ufr macro instruction from the testcase, and replaced it with the 
actual ctc1 instruction as my ufr patch to binutils does not support this anymore.  Finally, 
I have added extra assembler mnemonics to the CFC1 and CTC1 instructions to print out the 
symbolic names for the CP1 ufr and unfr registers in the same manner as binutils does.

Steve Ellcey (cc'd on this email) does have copyright assignment for gdb.  Would you be happy
if we used his copyright assignment?  Steve is also happy to submit the patch if you don't want to.

Regards,


Andrew


2013-12-18  Andrew Bennett  <andrew.bennett@imgtec.com>
		Steve Ellcey  <steve.ellcey@imgtec.com>
	sim/mips/
	* interp.c (ColdReset): Reset the config 5 register and
	set FCR0 to allow user mode FR switching instructions.
	(decode_coproc): Add support for reading and setting the config 5
	register.
	* mips.igen (CTC1c): Add UFR support.
	(CFC1c): Likewise.
	* sim-main.h (_sim_cpu): Add config 5 register and support macros.

	sim/testsuite/sim/mips/
	* basic.exp: Added ufr testcase.
	* ufr.s: New test.


 sim/mips/interp.c                |   17 ++++++++++++++++
 sim/mips/mips.igen               |   30 ++++++++++++++++++++++++++--
 sim/mips/sim-main.h              |    6 ++++++
 sim/testsuite/sim/mips/basic.exp |    2 ++
 sim/testsuite/sim/mips/ufr.s     |   40 ++++++++++++++++++++++++++++++++++++++
 7 files changed, 108 insertions(+), 2 deletions(-)
 create mode 100644 sim/testsuite/sim/mips/ufr.s

diff --git a/sim/mips/interp.c b/sim/mips/interp.c
index 032570a..44e9b93 100644
--- a/sim/mips/interp.c
+++ b/sim/mips/interp.c
@@ -1810,6 +1810,12 @@ ColdReset (SIM_DESC sd)
 	}
       if (BigEndianMem)
 	C0_CONFIG |= 0x00008000;	/* Big Endian */
+
+      /* Initialise the Config5 register.  */
+      C5_CONFIG = 0;
+
+      /* User mode FR switching instructions supported.  */
+      FCR0 |= fir_UFRP;
     }
 }
 
@@ -2349,6 +2355,17 @@ decode_coproc (SIM_DESC sd,
 #endif
 	      }
 	  }
+	/* This covers the case of reading and setting the Config5 register.  */
+	else if (((code == 0x00) || (code == 0x04))  /* MFC0 or MTC0 register 16 select 5 */
+	         && rd == 16 && ((tail & 0x07) == 5))
+	  {
+	    if (code == 0x00)
+	      GPR[rt] = C5_CONFIG;
+	    else if (FCR0 & fir_UFRP)
+	      C5_CONFIG = GPR[rt];
+	    else
+	      C5_CONFIG = (GPR[rt] & ~config5_UFR);
+	  }
 	else if ((code == 0x00 || code == 0x01)
 		 && rd == 16)
 	  {
diff --git a/sim/mips/mips.igen b/sim/mips/mips.igen
index 5a6326f..f2b44fd 100644
--- a/sim/mips/mips.igen
+++ b/sim/mips/mips.igen
@@ -4506,6 +4506,7 @@
 }
 
 010001,00010,5.RT,5.FS,00000000000:COP1:32,f::CFC1c
+"cfc1 r<RT>, c1_ufr": FS == 1
 "cfc1 r<RT>, f<FS>"
 *mipsV:
 *mips32:
@@ -4520,7 +4521,15 @@
       TRACE_ALU_INPUT1 (fcr);
       GPR[RT] = fcr;
     }
-  /* else NOP */
+  else if ((FS == 1) && (FCR0 & fir_UFRP))
+    {
+      if (C5_CONFIG & config5_UFR)
+        GPR[RT] = (unsigned32) ((SR & status_FR) >> 26);
+      else
+        SignalException (ReservedInstruction, instruction_0);
+    }
+  else
+    Unpredictable ();
   TRACE_ALU_RESULT (GPR[RT]);
 }
 
@@ -4551,6 +4560,8 @@
 }
 
 010001,00110,5.RT,5.FS,00000000000:COP1:32,f::CTC1c
+"ctc1 r<RT>, c1_ufr": RT == 0 && FS == 1
+"ctc1 r<RT>, c1_unfr": RT == 0 && FS == 4
 "ctc1 r<RT>, f<FS>"
 *mipsV:
 *mips32:
@@ -4562,7 +4573,22 @@
   TRACE_ALU_INPUT1 (GPR[RT]);
   if (FS == 25 || FS == 26 || FS == 28 || FS == 31)
       StoreFCR (FS, GPR[RT]);
-  /* else NOP */
+  else if ((FS == 1) && (RT == 0) && (FCR0 & fir_UFRP))
+    {
+      if (C5_CONFIG & config5_UFR)
+        SR &= ~status_FR;
+      else
+        SignalException (ReservedInstruction, instruction_0);
+    }
+  else if ((FS == 4) && (RT == 0) && (FCR0 & fir_UFRP))
+    {
+      if (C5_CONFIG & config5_UFR)
+        SR |= status_FR;
+      else
+        SignalException (ReservedInstruction, instruction_0);
+    }
+  else
+    Unpredictable ();
 }
 
 
diff --git a/sim/mips/sim-main.h b/sim/mips/sim-main.h
index b8c75c9..2742c8e 100644
--- a/sim/mips/sim-main.h
+++ b/sim/mips/sim-main.h
@@ -407,6 +407,10 @@ struct _sim_cpu {
   unsigned_word c0_config_reg;
 #define C0_CONFIG ((CPU)->c0_config_reg)
 
+  unsigned_word c5_config_reg;
+#define C5_CONFIG ((CPU)->c5_config_reg)
+#define config5_UFR	(1 << 2) 	/* Allow user mode access to StatusFR */
+
 /* The following are pseudonyms for standard registers */
 #define ZERO    (REGISTERS[0])
 #define V0      (REGISTERS[2])
@@ -576,6 +580,8 @@ struct sim_state {
 #define cause_set_EXC(x)  CAUSE = (CAUSE & ~cause_EXC_mask)  | ((x << cause_EXC_shift)  & cause_EXC_mask)
 #define cause_set_EXC2(x) CAUSE = (CAUSE & ~cause_EXC2_mask) | ((x << cause_EXC2_shift) & cause_EXC2_mask)
 
+/* FIR bits used by MIPS32/MIPS64.  */
+#define fir_UFRP         (1 << 28)      /* User mode FR switching instructions: 0 = not supported, 1 = supported */
 
 /* NOTE: We keep the following status flags as bit values (1 for true,
    0 for false). This allows them to be used in binary boolean
diff --git a/sim/testsuite/sim/mips/basic.exp b/sim/testsuite/sim/mips/basic.exp
index 1c78c87..be7284a 100644
--- a/sim/testsuite/sim/mips/basic.exp
+++ b/sim/testsuite/sim/mips/basic.exp
@@ -86,4 +86,6 @@ if {[istarget mips*-*-elf]} {
 
     run_sim_test mips32-dsp.s $dspmodels
     run_sim_test mips32-dsp2.s $dspmodels
+
+    run_sim_test ufr.s $submodels
 }
diff --git a/sim/testsuite/sim/mips/ufr.s b/sim/testsuite/sim/mips/ufr.s
new file mode 100644
index 0000000..785fefc
--- /dev/null
+++ b/sim/testsuite/sim/mips/ufr.s
@@ -0,0 +1,40 @@
+# mips test ufr, expected to pass.
+# mach:	 mips32r2 mips64r2
+# as:		-mabi=eabi
+# ld:		-N -Ttext=0x80010000
+#output:	*\\npass\\n
+
+	.include "testutils.inc"
+
+	setup
+
+	.set noreorder
+
+	.ent DIAG
+DIAG:
+	writemsg "[1] Enable writing to FR"
+	li $2, 0x4
+	mfc0 $3, $16, 5
+	or $3, $3, $2
+	mtc0 $3, $16, 5
+	mfc0 $3, $16, 5
+	and $3, $3, $2
+	beq $3, $0, _fail
+	nop
+
+	writemsg "[2] Change to FR 0 mode"
+	ctc1 $0, $1
+	cfc1 $2, $1
+	bne $2, $0, _fail
+	nop
+
+	writemsg "[3] Change to FR 1 mode"
+	ctc1 $0, $4
+	cfc1 $2, $1
+	li $3, 1
+	bne $2, $3, _fail
+	nop
+
+	pass
+
+	.end DIAG