[PATCH 0/5] x86: Merge fpu and simd exception handlers

[PATCH 0/5] x86: Merge fpu and simd exception handlers

[Brian Gerst]

This patch series merges the fpu and simd exception handlers into one
function.  The only difference is where the status bits are read from.
This also applies a fix for multiple simd exceptions that was already 
done for fpu.

[PATCH 1/5] x86-32: Split cache flush handler from simd handler
[PATCH 2/5] x86-32: Remove die_if_kernel()
[PATCH 3/5] x86: Merge simd_math_error() into math_error()
[PATCH 4/5] x86: Merge kernel_math_error() into math_error()
[PATCH 5/5] x86-32: Don't set ignore_fpu_irq in simd exception

 arch/x86/include/asm/traps.h |    2 +-
 arch/x86/kernel/entry_32.S   |   16 +++-
 arch/x86/kernel/irqinit.c    |    2 +-
 arch/x86/kernel/traps.c      |  195 +++++++++++++++---------------------------
 4 files changed, 87 insertions(+), 128 deletions(-)

[PATCH 1/5] x86-32: Split cache flush handler from simd handler

[Brian Gerst]

Make the cache flush handler a seperate function, and use
an alternative to call the appropriate handler.

Signed-off-by: Brian Gerst <brgerst@gmail.com>
---
 arch/x86/kernel/entry_32.S |   16 +++++++++++++++-
 arch/x86/kernel/traps.c    |   43 ++++++++++++++++++++++++-------------------
 2 files changed, 39 insertions(+), 20 deletions(-)

diff --git a/arch/x86/kernel/entry_32.S b/arch/x86/kernel/entry_32.S
index 44a8e0d..c3769fa 100644
--- a/arch/x86/kernel/entry_32.S
+++ b/arch/x86/kernel/entry_32.S
@@ -53,6 +53,7 @@
 #include <asm/processor-flags.h>
 #include <asm/ftrace.h>
 #include <asm/irq_vectors.h>
+#include <asm/cpufeature.h>
 
 /* Avoid __ASSEMBLER__'ifying <linux/audit.h> just for this.  */
 #include <linux/elf-em.h>
@@ -905,7 +906,20 @@ ENTRY(simd_coprocessor_error)
 	RING0_INT_FRAME
 	pushl $0
 	CFI_ADJUST_CFA_OFFSET 4
-	pushl $do_simd_coprocessor_error
+661:	pushl $do_cache_flush_error
+662:
+.section .altinstructions,"a"
+	ALIGN
+	.long 661b
+	.long 663f
+	.byte X86_FEATURE_XMM
+	.byte 662b-661b
+	.byte 664f-663f
+.previous
+.section .altinstr_replacement,"ax"	
+663:	pushl $do_simd_coprocessor_error
+664:
+.previous
 	CFI_ADJUST_CFA_OFFSET 4
 	jmp error_code
 	CFI_ENDPROC
diff --git a/arch/x86/kernel/traps.c b/arch/x86/kernel/traps.c
index 1168e44..45b9cb0 100644
--- a/arch/x86/kernel/traps.c
+++ b/arch/x86/kernel/traps.c
@@ -257,6 +257,27 @@ dotraplinkage void do_double_fault(struct pt_regs *regs, long error_code)
 }
 #endif
 
+#ifdef CONFIG_X86_32
+dotraplinkage void
+do_cache_flush_error(struct pt_regs *regs, long error_code)
+{
+	conditional_sti(regs);
+
+	/*
+	 * Handle strange cache flush from user space exception.
+	 * This is undocumented behaviour.
+	 */
+	if (regs->flags & X86_VM_MASK) {
+		handle_vm86_fault((struct kernel_vm86_regs *)regs, error_code);
+		return;
+	}
+	current->thread.trap_no = 19;
+	current->thread.error_code = error_code;
+	die_if_kernel("cache flush denied", regs, error_code);
+	force_sig(SIGSEGV, current);
+}
+#endif
+
 dotraplinkage void __kprobes
 do_general_protection(struct pt_regs *regs, long error_code)
 {
@@ -729,30 +750,14 @@ do_simd_coprocessor_error(struct pt_regs *regs, long error_code)
 	conditional_sti(regs);
 
 #ifdef CONFIG_X86_32
-	if (cpu_has_xmm) {
-		/* Handle SIMD FPU exceptions on PIII+ processors. */
-		ignore_fpu_irq = 1;
-		simd_math_error((void __user *)regs->ip);
-		return;
-	}
-	/*
-	 * Handle strange cache flush from user space exception
-	 * in all other cases.  This is undocumented behaviour.
-	 */
-	if (regs->flags & X86_VM_MASK) {
-		handle_vm86_fault((struct kernel_vm86_regs *)regs, error_code);
-		return;
-	}
-	current->thread.trap_no = 19;
-	current->thread.error_code = error_code;
-	die_if_kernel("cache flush denied", regs, error_code);
-	force_sig(SIGSEGV, current);
+	ignore_fpu_irq = 1;
 #else
 	if (!user_mode(regs) &&
 			kernel_math_error(regs, "kernel simd math error", 19))
 		return;
-	simd_math_error((void __user *)regs->ip);
 #endif
+
+	simd_math_error((void __user *)regs->ip);
 }
 
 dotraplinkage void
-- 
1.6.6.1

Re: [PATCH 1/5] x86-32: Split cache flush handler from simd handler

[H. Peter Anvin]

On 03/18/2010 11:20 AM, Brian Gerst wrote:
> Make the cache flush handler a seperate function, and use
> an alternative to call the appropriate handler.
>  
> +#ifdef CONFIG_X86_32
> +dotraplinkage void
> +do_cache_flush_error(struct pt_regs *regs, long error_code)
> +{
> +	conditional_sti(regs);
> +
> +	/*
> +	 * Handle strange cache flush from user space exception.
> +	 * This is undocumented behaviour.
> +	 */
> +	if (regs->flags & X86_VM_MASK) {
> +		handle_vm86_fault((struct kernel_vm86_regs *)regs, error_code);
> +		return;
> +	}
> +	current->thread.trap_no = 19;
> +	current->thread.error_code = error_code;
> +	die_if_kernel("cache flush denied", regs, error_code);
> +	force_sig(SIGSEGV, current);
> +}
> +#endif

Does anyone have *any idea* what processor this applies to?  I've
tracked the code back all the way to the original inclusion in the
kernel, and there isn't even the slightest hint.

The comment, of course, is a great example on how *not* to write
comments... it should have mentioned the CPU in question.

	-hpa

Re: [PATCH 1/5] x86-32: Split cache flush handler from simd handler

[Brian Gerst]

On Fri, Mar 19, 2010 at 6:33 PM, H. Peter Anvin <hpa@zytor.com> wrote:
> On 03/18/2010 11:20 AM, Brian Gerst wrote:
>> Make the cache flush handler a seperate function, and use
>> an alternative to call the appropriate handler.
>>
>> +#ifdef CONFIG_X86_32
>> +dotraplinkage void
>> +do_cache_flush_error(struct pt_regs *regs, long error_code)
>> +{
>> +     conditional_sti(regs);
>> +
>> +     /*
>> +      * Handle strange cache flush from user space exception.
>> +      * This is undocumented behaviour.
>> +      */
>> +     if (regs->flags & X86_VM_MASK) {
>> +             handle_vm86_fault((struct kernel_vm86_regs *)regs, error_code);
>> +             return;
>> +     }
>> +     current->thread.trap_no = 19;
>> +     current->thread.error_code = error_code;
>> +     die_if_kernel("cache flush denied", regs, error_code);
>> +     force_sig(SIGSEGV, current);
>> +}
>> +#endif
>
> Does anyone have *any idea* what processor this applies to?  I've
> tracked the code back all the way to the original inclusion in the
> kernel, and there isn't even the slightest hint.
>
> The comment, of course, is a great example on how *not* to write
> comments... it should have mentioned the CPU in question.

This thread appears to describe the problem:
http://marc.info/?t=104960872800014&r=1&w=2

And the initial patch:
http://marc.info/?l=linux-kernel&m=104960870106838&w=2

It looks like to me, that an AMD 486 clone has an erratum where the
invd instruction from userspace generates exception 19 (13 hex)
instead of #GP (13 dec).

--
Brian Gerst

Re: [PATCH 1/5] x86-32: Split cache flush handler from simd handler

[H. Peter Anvin]

On 03/19/2010 09:08 PM, Brian Gerst wrote:
>>
>> Does anyone have *any idea* what processor this applies to?  I've
>> tracked the code back all the way to the original inclusion in the
>> kernel, and there isn't even the slightest hint.
>>
>> The comment, of course, is a great example on how *not* to write
>> comments... it should have mentioned the CPU in question.
> 
> This thread appears to describe the problem:
> http://marc.info/?t=104960872800014&r=1&w=2
> 
> And the initial patch:
> http://marc.info/?l=linux-kernel&m=104960870106838&w=2
> 
> It looks like to me, that an AMD 486 clone has an erratum where the
> invd instruction from userspace generates exception 19 (13 hex)
> instead of #GP (13 dec).
> 

Ah, guess it was even older than I first realized ... I should have
searched for the string "cache flush denied" instead.

Sounds like we should do three things:

a) update the comment to actually reflect what is going on;
b) compile it out for > 486;
c) report the error as trap 13 rather than 19.

	-hpa

-- 
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel.  I don't speak on their behalf.

Re: [PATCH 1/5] x86-32: Split cache flush handler from simd handler

[H. Peter Anvin]

On 03/19/2010 09:55 PM, H. Peter Anvin wrote:
> 
> Ah, guess it was even older than I first realized ... I should have
> searched for the string "cache flush denied" instead.
> 
> Sounds like we should do three things:
> 
> a) update the comment to actually reflect what is going on;
> b) compile it out for > 486;
> c) report the error as trap 13 rather than 19.
> 

Actually what we should do is recognize that this is nothing other than
a misdirected #GP, and as a result send the fault down the #GP handler
(do_general_protection)... there really isn't any need for a special
routine at all.

	-hpa

[PATCH 2/5] x86-32: Remove die_if_kernel()

[Brian Gerst]

Merge die_if_kernel() into do_cache_flush_error().

Signed-off-by: Brian Gerst <brgerst@gmail.com>
---
 arch/x86/kernel/traps.c |   12 ++----------
 1 files changed, 2 insertions(+), 10 deletions(-)

diff --git a/arch/x86/kernel/traps.c b/arch/x86/kernel/traps.c
index 45b9cb0..cea343e 100644
--- a/arch/x86/kernel/traps.c
+++ b/arch/x86/kernel/traps.c
@@ -108,15 +108,6 @@ static inline void preempt_conditional_cli(struct pt_regs *regs)
 	dec_preempt_count();
 }
 
-#ifdef CONFIG_X86_32
-static inline void
-die_if_kernel(const char *str, struct pt_regs *regs, long err)
-{
-	if (!user_mode_vm(regs))
-		die(str, regs, err);
-}
-#endif
-
 static void __kprobes
 do_trap(int trapnr, int signr, char *str, struct pt_regs *regs,
 	long error_code, siginfo_t *info)
@@ -273,7 +264,8 @@ do_cache_flush_error(struct pt_regs *regs, long error_code)
 	}
 	current->thread.trap_no = 19;
 	current->thread.error_code = error_code;
-	die_if_kernel("cache flush denied", regs, error_code);
+	if (!user_mode(regs))
+		die("cache flush denied", regs, error_code);
 	force_sig(SIGSEGV, current);
 }
 #endif
-- 
1.6.6.1

[PATCH 3/5] x86: Merge simd_math_error() into math_error()

[Brian Gerst]

The only difference between FPU and SIMD exceptions is where the
status bits are read from (cwd/swd vs. mxcsr).  This also fixes
the discrepency introduced by commit adf77bac, which fixed FPU
but not SIMD.

Signed-off-by: Brian Gerst <brgerst@gmail.com>
---
 arch/x86/include/asm/traps.h |    2 +-
 arch/x86/kernel/irqinit.c    |    2 +-
 arch/x86/kernel/traps.c      |  100 +++++++++++++----------------------------
 3 files changed, 34 insertions(+), 70 deletions(-)

diff --git a/arch/x86/include/asm/traps.h b/arch/x86/include/asm/traps.h
index 4da91ad..f66cda5 100644
--- a/arch/x86/include/asm/traps.h
+++ b/arch/x86/include/asm/traps.h
@@ -79,7 +79,7 @@ static inline int get_si_code(unsigned long condition)
 
 extern int panic_on_unrecovered_nmi;
 
-void math_error(void __user *);
+void math_error(struct pt_regs *, int, int);
 void math_emulate(struct math_emu_info *);
 #ifndef CONFIG_X86_32
 asmlinkage void smp_thermal_interrupt(void);
diff --git a/arch/x86/kernel/irqinit.c b/arch/x86/kernel/irqinit.c
index ef257fc..15bd039 100644
--- a/arch/x86/kernel/irqinit.c
+++ b/arch/x86/kernel/irqinit.c
@@ -61,7 +61,7 @@ static irqreturn_t math_error_irq(int cpl, void *dev_id)
 	outb(0, 0xF0);
 	if (ignore_fpu_irq || !boot_cpu_data.hard_math)
 		return IRQ_NONE;
-	math_error((void __user *)get_irq_regs()->ip);
+	math_error(get_irq_regs(), 0, 16);
 	return IRQ_HANDLED;
 }
 
diff --git a/arch/x86/kernel/traps.c b/arch/x86/kernel/traps.c
index cea343e..5628307 100644
--- a/arch/x86/kernel/traps.c
+++ b/arch/x86/kernel/traps.c
@@ -617,36 +617,48 @@ static int kernel_math_error(struct pt_regs *regs, const char *str, int trapnr)
  * the correct behaviour even in the presence of the asynchronous
  * IRQ13 behaviour
  */
-void math_error(void __user *ip)
+void math_error(struct pt_regs *regs, int error_code, int trapnr)
 {
 	struct task_struct *task;
 	siginfo_t info;
-	unsigned short cwd, swd, err;
+	unsigned short err;
 
 	/*
 	 * Save the info for the exception handler and clear the error.
 	 */
 	task = current;
 	save_init_fpu(task);
-	task->thread.trap_no = 16;
-	task->thread.error_code = 0;
+	task->thread.trap_no = trapnr;
+	task->thread.error_code = error_code;
 	info.si_signo = SIGFPE;
 	info.si_errno = 0;
-	info.si_addr = ip;
-	/*
-	 * (~cwd & swd) will mask out exceptions that are not set to unmasked
-	 * status.  0x3f is the exception bits in these regs, 0x200 is the
-	 * C1 reg you need in case of a stack fault, 0x040 is the stack
-	 * fault bit.  We should only be taking one exception at a time,
-	 * so if this combination doesn't produce any single exception,
-	 * then we have a bad program that isn't synchronizing its FPU usage
-	 * and it will suffer the consequences since we won't be able to
-	 * fully reproduce the context of the exception
-	 */
-	cwd = get_fpu_cwd(task);
-	swd = get_fpu_swd(task);
+	info.si_addr = (void __user *)regs->ip;
+	if (trapnr == 16) {
+		unsigned short cwd, swd;
+		/*
+		 * (~cwd & swd) will mask out exceptions that are not set to unmasked
+		 * status.  0x3f is the exception bits in these regs, 0x200 is the
+		 * C1 reg you need in case of a stack fault, 0x040 is the stack
+		 * fault bit.  We should only be taking one exception at a time,
+		 * so if this combination doesn't produce any single exception,
+		 * then we have a bad program that isn't synchronizing its FPU usage
+		 * and it will suffer the consequences since we won't be able to
+		 * fully reproduce the context of the exception
+		 */
+		cwd = get_fpu_cwd(task);
+		swd = get_fpu_swd(task);
 
-	err = swd & ~cwd;
+		err = swd & ~cwd;
+	} else {
+		/*
+		 * The SIMD FPU exceptions are handled a little differently, as there
+		 * is only a single status/control register.  Thus, to determine which
+		 * unmasked exception was caught we must mask the exception mask bits
+		 * at 0x1f80, and then use these to mask the exception bits at 0x3f.
+		 */
+		unsigned short mxcsr = get_fpu_mxcsr(task);
+		err = ~(mxcsr >> 7) & mxcsr;
+	}
 
 	if (err & 0x001) {	/* Invalid op */
 		/*
@@ -685,55 +697,7 @@ dotraplinkage void do_coprocessor_error(struct pt_regs *regs, long error_code)
 		return;
 #endif
 
-	math_error((void __user *)regs->ip);
-}
-
-static void simd_math_error(void __user *ip)
-{
-	struct task_struct *task;
-	siginfo_t info;
-	unsigned short mxcsr;
-
-	/*
-	 * Save the info for the exception handler and clear the error.
-	 */
-	task = current;
-	save_init_fpu(task);
-	task->thread.trap_no = 19;
-	task->thread.error_code = 0;
-	info.si_signo = SIGFPE;
-	info.si_errno = 0;
-	info.si_code = __SI_FAULT;
-	info.si_addr = ip;
-	/*
-	 * The SIMD FPU exceptions are handled a little differently, as there
-	 * is only a single status/control register.  Thus, to determine which
-	 * unmasked exception was caught we must mask the exception mask bits
-	 * at 0x1f80, and then use these to mask the exception bits at 0x3f.
-	 */
-	mxcsr = get_fpu_mxcsr(task);
-	switch (~((mxcsr & 0x1f80) >> 7) & (mxcsr & 0x3f)) {
-	case 0x000:
-	default:
-		break;
-	case 0x001: /* Invalid Op */
-		info.si_code = FPE_FLTINV;
-		break;
-	case 0x002: /* Denormalize */
-	case 0x010: /* Underflow */
-		info.si_code = FPE_FLTUND;
-		break;
-	case 0x004: /* Zero Divide */
-		info.si_code = FPE_FLTDIV;
-		break;
-	case 0x008: /* Overflow */
-		info.si_code = FPE_FLTOVF;
-		break;
-	case 0x020: /* Precision */
-		info.si_code = FPE_FLTRES;
-		break;
-	}
-	force_sig_info(SIGFPE, &info, task);
+	math_error(regs, error_code, 16);
 }
 
 dotraplinkage void
@@ -749,7 +713,7 @@ do_simd_coprocessor_error(struct pt_regs *regs, long error_code)
 		return;
 #endif
 
-	simd_math_error((void __user *)regs->ip);
+	math_error(regs, error_code, 19);
 }
 
 dotraplinkage void
-- 
1.6.6.1

[PATCH 4/5] x86: Merge kernel_math_error() into math_error()

[Brian Gerst]

Clean up the kernel exception handling and make it more similar to
the other traps.

Signed-off-by: Brian Gerst <brgerst@gmail.com>
---
 arch/x86/kernel/traps.c |   44 ++++++++++++++++----------------------------
 1 files changed, 16 insertions(+), 28 deletions(-)

diff --git a/arch/x86/kernel/traps.c b/arch/x86/kernel/traps.c
index 5628307..6384066 100644
--- a/arch/x86/kernel/traps.c
+++ b/arch/x86/kernel/traps.c
@@ -598,20 +598,6 @@ dotraplinkage void __kprobes do_debug(struct pt_regs *regs, long error_code)
 	return;
 }
 
-#ifdef CONFIG_X86_64
-static int kernel_math_error(struct pt_regs *regs, const char *str, int trapnr)
-{
-	if (fixup_exception(regs))
-		return 1;
-
-	notify_die(DIE_GPF, str, regs, 0, trapnr, SIGFPE);
-	/* Illegal floating point operation in the kernel */
-	current->thread.trap_no = trapnr;
-	die(str, regs, 0);
-	return 0;
-}
-#endif
-
 /*
  * Note that we play around with the 'TS' bit in an attempt to get
  * the correct behaviour even in the presence of the asynchronous
@@ -619,14 +605,28 @@ static int kernel_math_error(struct pt_regs *regs, const char *str, int trapnr)
  */
 void math_error(struct pt_regs *regs, int error_code, int trapnr)
 {
-	struct task_struct *task;
+	struct task_struct *task = current;
 	siginfo_t info;
 	unsigned short err;
+	char *str = (trapnr == 16) ? "fpu exception" : "simd exception";
+
+	if (notify_die(DIE_TRAP, str, regs, error_code, trapnr, SIGFPE) == NOTIFY_STOP)
+		return;
+	conditional_sti(regs);
+
+	if (!user_mode_vm(regs))
+	{
+		if (!fixup_exception(regs)) {
+			task->thread.error_code = error_code;
+			task->thread.trap_no = trapnr;
+			die(str, regs, error_code);
+		}
+		return;
+	}
 
 	/*
 	 * Save the info for the exception handler and clear the error.
 	 */
-	task = current;
 	save_init_fpu(task);
 	task->thread.trap_no = trapnr;
 	task->thread.error_code = error_code;
@@ -687,14 +687,8 @@ void math_error(struct pt_regs *regs, int error_code, int trapnr)
 
 dotraplinkage void do_coprocessor_error(struct pt_regs *regs, long error_code)
 {
-	conditional_sti(regs);
-
 #ifdef CONFIG_X86_32
 	ignore_fpu_irq = 1;
-#else
-	if (!user_mode(regs) &&
-	    kernel_math_error(regs, "kernel x87 math error", 16))
-		return;
 #endif
 
 	math_error(regs, error_code, 16);
@@ -703,14 +697,8 @@ dotraplinkage void do_coprocessor_error(struct pt_regs *regs, long error_code)
 dotraplinkage void
 do_simd_coprocessor_error(struct pt_regs *regs, long error_code)
 {
-	conditional_sti(regs);
-
 #ifdef CONFIG_X86_32
 	ignore_fpu_irq = 1;
-#else
-	if (!user_mode(regs) &&
-			kernel_math_error(regs, "kernel simd math error", 19))
-		return;
 #endif
 
 	math_error(regs, error_code, 19);
-- 
1.6.6.1

[PATCH 5/5] x86-32: Don't set ignore_fpu_irq in simd exception

[Brian Gerst]

Any processor that supports simd will have an internal fpu, and the
irq13 handler will not be enabled.

Signed-off-by: Brian Gerst <brgerst@gmail.com>
---
 arch/x86/kernel/traps.c |    4 ----
 1 files changed, 0 insertions(+), 4 deletions(-)

diff --git a/arch/x86/kernel/traps.c b/arch/x86/kernel/traps.c
index 6384066..91da091 100644
--- a/arch/x86/kernel/traps.c
+++ b/arch/x86/kernel/traps.c
@@ -697,10 +697,6 @@ dotraplinkage void do_coprocessor_error(struct pt_regs *regs, long error_code)
 dotraplinkage void
 do_simd_coprocessor_error(struct pt_regs *regs, long error_code)
 {
-#ifdef CONFIG_X86_32
-	ignore_fpu_irq = 1;
-#endif
-
 	math_error(regs, error_code, 19);
 }
 
-- 
1.6.6.1