On Feb 8, 2007, at 3:06 AM, Zhu Ebony-r57400 wrote:

quoted

-----Original Message-----
From: Kumar Gala [mailto:galak@kernel.crashing.org]
Sent: Thursday, February 08, 2007 3:33 PM
To: Zhu Ebony-r57400
Cc: paulus@samba.org; linuxppc-dev@ozlabs.org
Subject: Re: [patch][5/5] powerpc V2: Add the general support
for Embedded Floating-Point instructions


On Feb 7, 2007, at 9:55 PM, ebony.zhu@freescale.com wrote:

quoted

Add the general support for Embedded Floating-Point instructions to
fully comply with IEEE-754.

Signed-off-by:Ebony Zhu [off-list ref]
---
 arch/powerpc/kernel/head_fsl_booke.S |    4
 arch/powerpc/kernel/traps.c          |   57 +++++
 arch/powerpc/math-emu/Makefile       |   25 ++
 arch/powerpc/math-emu/sfp-machine.h  |    2
 arch/powerpc/math-emu/spe.h          |    1
 arch/powerpc/sysdev/Makefile         |    1
 arch/powerpc/sysdev/sigfpe_handler.c |  361 +++++++++++++++++++++++
+++++++++++
 7 files changed, 442 insertions(+), 9 deletions(-)

I thought we were going to have some general Kconfig option
to enable all this?  EMDEDDED_FP_IEEE or something like that

So that users have chance to enable/disable fully IEEE compliance?
Segher
had mentioned this, and it sounds reasonable.

Its alot of code to bring in if you don't care about IEEE compliance.

quoted

quoted

diff --git a/arch/powerpc/kernel/head_fsl_booke.S b/arch/powerpc/

kernel/head_fsl_booke.S
index 66877bd..0d05db0 100644

--- a/arch/powerpc/kernel/head_fsl_booke.S
+++ b/arch/powerpc/kernel/head_fsl_booke.S

@@ -705,7 +705,7 @@ #else
 #endif /* CONFIG_SPE */

 	/* SPE Floating Point Round */
-	EXCEPTION(0x2050, SPEFloatingPointRound, unknown_exception,

EXC_XFER_EE)
+	EXCEPTION(0x2050, SPEFloatingPointRound,
SPEFloatingPointException_Round, EXC_XFER_EE)

 	/* Performance Monitor */
 	EXCEPTION(0x2060, PerformanceMonitor,
performance_monitor_exception, EXC_XFER_STD)

@@ -840,6 +840,8 @@ load_up_spe:
 	oris	r5,r5,MSR_SPE@h
 	mtmsr	r5			/* enable use of SPE now */
 	isync
+	li	r5,(SPEFSCR_FINVE | SPEFSCR_FDBZE | SPEFSCR_FUNFE |

SPEFSCR_FOVFE)
+	mtspr   SPRN_SPEFSCR,r5

We should do this via INIT_THREAD, is there a reason that you
want to
set these always?

I just thought it's the first time that an SPE instruction is
encountered, so I
enable the exceptions here.

Lets do this via INIT_THREAD instead, its cleaner. (just remember to  
add the proper ifdef protection for SPE_IEEE

quoted

quoted

 /*
  * For SMP, we don't do lazy SPE switching because it just gets too
  * horrendously complex, especially when a task switches

from one CPU

quoted

diff --git a/arch/powerpc/kernel/traps.c

b/arch/powerpc/kernel/traps.c

quoted

index 6915b91..30ab0f7 100644

--- a/arch/powerpc/kernel/traps.c
+++ b/arch/powerpc/kernel/traps.c

@@ -986,9 +986,16 @@ #endif /* CONFIG_FSL_BOOKE */
 #ifdef CONFIG_SPE
 void SPEFloatingPointException(struct pt_regs *regs)
 {
+	extern int spedata_handler(struct pt_regs *regs);
 	unsigned long spefscr;
 	int fpexc_mode;
 	int code = 0;
+	int err;
+	
+	preempt_disable();
+	if (regs->msr & MSR_SPE)
+		giveup_spe(current);
+	preempt_enable();

use flush_spe_to_thread(current);

OK. It works, and I'll change it.

quoted

quoted

 	spefscr = current->thread.spefscr;
 	fpexc_mode = current->thread.fpexc_mode;

@@ -1013,9 +1020,55 @@ void SPEFloatingPointException(struct pt
 		code = FPE_FLTRES;

 	current->thread.spefscr = spefscr;
+	err = spedata_handler(regs);
+	if (err == 0) {
+		regs->nip += 4;		/* skip emulated instruction */
+		emulate_single_step(regs);
+		return;
+	}

Take a look at the path I put up that reworks the error
handling from
do_mathemu() we need to be doing something similar (in parsing
spefscr/fpexc_mode to setup code properly)

It's new in trap.c? I'll look into it.

Pretty recent commit, in Paul's tree, not yet in linus's.

(or look at my last pull request email to paul)

quoted

quoted

-	_exception(SIGFPE, regs, code, regs->nip);
-	return;
+	if (err == -EFAULT) {
+		/* got an error reading the instruction */
+		_exception(SIGSEGV, regs, SEGV_ACCERR, regs->nip);
+	} else if (err == -EINVAL) {
+		/* didn't recognize the instruction */
+		printk(KERN_ERR "unrecognized spe instruction "
+		       "in %s at %lx\n", current->comm, regs->nip);

We should probably just SIGSEGV in this case since will never make
forward progress once we hit this case.

Won't it hit the case that the instruction is not an spe instruction?

No, since we will not get this exception on something that's not an  
SPE instruction.

quoted

quoted

diff --git a/arch/powerpc/math-emu/Makefile

b/arch/powerpc/math-emu/

quoted

Makefile
index 29bc912..2da11ba 100644

--- a/arch/powerpc/math-emu/Makefile
+++ b/arch/powerpc/math-emu/Makefile

@@ -1,16 +1,29 @@

-obj-y				:= math.o fmr.o lfd.o stfd.o
-
-obj-$(CONFIG_MATH_EMULATION)	+= fabs.o fadd.o

fadds.o fcmpo.o

quoted

fcmpu.o \
+obj-y				:= fabs.o fneg.o

types.o udivmodti4.o

This isn't right, we don't want to always build these files.

fabs.o/fneg.o should be built in the case
that CONFIG_MATH_EMULATION or CONFIG_SPE or both are
enabled. Because I reused fabs.c and fneg.c to implement instruction
efdabs and
efdneg. Therefore, these two files should be build once we have chance
to build the files in directory math-emu. Types.c and udivmodti4.c
contain the function
that both math emulation instructions and spe instructions may  
call, so
they always need
to be built.

Right, but doesn't the new rule say to always build, regardless if  
CONFIG_MATH_EMULATION or CONFIG_SPE are set?

Why not just duplicate fabs.o fneg.o ... in both lists.  I don't  
think that will cause any harm.

- k