[PATCH] x86: use enum instead of literals for trap values

[PATCH] x86: use enum instead of literals for trap values

[Kees Cook]

The traps are referred to by their numbers and it can be difficult to
understand them while reading the code without context. This patch adds
enumeration of the trap numbers and replaces the numbers with the correct
enum for x86.

Signed-off-by: Kees Cook <keescook@chromium.org>

---
I've updated Aditya Kali's earlier patch:
https://lkml.org/lkml/2011/4/22/328
---
 arch/x86/include/asm/traps.h |   25 +++++++++
 arch/x86/kernel/irqinit.c    |    2 +-
 arch/x86/kernel/traps.c      |  117 ++++++++++++++++++++++--------------------
 3 files changed, 88 insertions(+), 56 deletions(-)

diff --git a/arch/x86/include/asm/traps.h b/arch/x86/include/asm/traps.h
index 0012d09..768afb2 100644
--- a/arch/x86/include/asm/traps.h
+++ b/arch/x86/include/asm/traps.h
@@ -89,4 +89,29 @@ asmlinkage void smp_thermal_interrupt(void);
 asmlinkage void mce_threshold_interrupt(void);
 #endif
 
+/* Interrupts/Exceptions */
+enum {
+	INTR_DIV_BY_ZERO = 0,	/*  0 */
+	INTR_DEBUG,		/*  1 */
+	INTR_NMI,		/*  2 */
+	INTR_BREAKPOINT,	/*  3 */
+	INTR_OVERFLOW,		/*  4 */
+	INTR_BOUNDS_CHECK,	/*  5 */
+	INTR_INVALID_OP,	/*  6 */
+	INTR_NO_DEV,		/*  7 */
+	INTR_DBL_FAULT,		/*  8 */
+	INTR_SEG_OVERRUN,	/*  9 */
+	INTR_INVALID_TSS,	/* 10 */
+	INTR_NO_SEG,		/* 11 */
+	INTR_STACK_FAULT,	/* 12 */
+	INTR_GPF,		/* 13 */
+	INTR_PAGE_FAULT,	/* 14 */
+	INTR_SPURIOUS,		/* 15 */
+	INTR_COPROCESSOR,	/* 16 */
+	INTR_ALIGNMENT,		/* 17 */
+	INTR_MCE,		/* 18 */
+	INTR_SIMD_COPROCESSOR,	/* 19 */
+	INTR_IRET = 32,		/* 32 */
+};
+
 #endif /* _ASM_X86_TRAPS_H */
diff --git a/arch/x86/kernel/irqinit.c b/arch/x86/kernel/irqinit.c
index 313fb5c..67471bf 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(get_irq_regs(), 0, 16);
+	math_error(get_irq_regs(), 0, INTR_COPROCESSOR);
 	return IRQ_HANDLED;
 }
 
diff --git a/arch/x86/kernel/traps.c b/arch/x86/kernel/traps.c
index 4bbe04d..9f8d6a6 100644
--- a/arch/x86/kernel/traps.c
+++ b/arch/x86/kernel/traps.c
@@ -119,7 +119,7 @@ do_trap(int trapnr, int signr, char *str, struct pt_regs *regs,
 		 * traps 0, 1, 3, 4, and 5 should be forwarded to vm86.
 		 * On nmi (interrupt 2), do_trap should not be called.
 		 */
-		if (trapnr < 6)
+		if (trapnr < INTR_INVALID_OP)
 			goto vm86_trap;
 		goto trap_signal;
 	}
@@ -203,27 +203,32 @@ dotraplinkage void do_##name(struct pt_regs *regs, long error_code)	\
 	do_trap(trapnr, signr, str, regs, error_code, &info);		\
 }
 
-DO_ERROR_INFO(0, SIGFPE, "divide error", divide_error, FPE_INTDIV, regs->ip)
-DO_ERROR(4, SIGSEGV, "overflow", overflow)
-DO_ERROR(5, SIGSEGV, "bounds", bounds)
-DO_ERROR_INFO(6, SIGILL, "invalid opcode", invalid_op, ILL_ILLOPN, regs->ip)
-DO_ERROR(9, SIGFPE, "coprocessor segment overrun", coprocessor_segment_overrun)
-DO_ERROR(10, SIGSEGV, "invalid TSS", invalid_TSS)
-DO_ERROR(11, SIGBUS, "segment not present", segment_not_present)
+DO_ERROR_INFO(INTR_DIV_BY_ZERO, SIGFPE, "divide error", divide_error,
+		FPE_INTDIV, regs->ip)
+DO_ERROR(INTR_OVERFLOW, SIGSEGV, "overflow", overflow)
+DO_ERROR(INTR_BOUNDS_CHECK, SIGSEGV, "bounds", bounds)
+DO_ERROR_INFO(INTR_INVALID_OP, SIGILL, "invalid opcode", invalid_op,
+		ILL_ILLOPN, regs->ip)
+DO_ERROR(INTR_SEG_OVERRUN, SIGFPE, "coprocessor segment overrun",
+		coprocessor_segment_overrun)
+DO_ERROR(INTR_INVALID_TSS, SIGSEGV, "invalid TSS", invalid_TSS)
+DO_ERROR(INTR_NO_SEG, SIGBUS, "segment not present", segment_not_present)
 #ifdef CONFIG_X86_32
-DO_ERROR(12, SIGBUS, "stack segment", stack_segment)
+DO_ERROR(INTR_STACK_FAULT, SIGBUS, "stack segment", stack_segment)
 #endif
-DO_ERROR_INFO(17, SIGBUS, "alignment check", alignment_check, BUS_ADRALN, 0)
+DO_ERROR_INFO(INTR_ALIGNMENT, SIGBUS, "alignment check", alignment_check,
+		BUS_ADRALN, 0)
 
 #ifdef CONFIG_X86_64
 /* Runs on IST stack */
 dotraplinkage void do_stack_segment(struct pt_regs *regs, long error_code)
 {
 	if (notify_die(DIE_TRAP, "stack segment", regs, error_code,
-			12, SIGBUS) == NOTIFY_STOP)
+			INTR_STACK_FAULT, SIGBUS) == NOTIFY_STOP)
 		return;
 	preempt_conditional_sti(regs);
-	do_trap(12, SIGBUS, "stack segment", regs, error_code, NULL);
+	do_trap(INTR_STACK_FAULT, SIGBUS, "stack segment", regs, error_code,
+			NULL);
 	preempt_conditional_cli(regs);
 }
 
@@ -233,10 +238,10 @@ dotraplinkage void do_double_fault(struct pt_regs *regs, long error_code)
 	struct task_struct *tsk = current;
 
 	/* Return not checked because double check cannot be ignored */
-	notify_die(DIE_TRAP, str, regs, error_code, 8, SIGSEGV);
+	notify_die(DIE_TRAP, str, regs, error_code, INTR_DBL_FAULT, SIGSEGV);
 
 	tsk->thread.error_code = error_code;
-	tsk->thread.trap_no = 8;
+	tsk->thread.trap_no = INTR_DBL_FAULT;
 
 	/*
 	 * This is always a kernel trap and never fixable (and thus must
@@ -264,7 +269,7 @@ do_general_protection(struct pt_regs *regs, long error_code)
 		goto gp_in_kernel;
 
 	tsk->thread.error_code = error_code;
-	tsk->thread.trap_no = 13;
+	tsk->thread.trap_no = INTR_GPF;
 
 	if (show_unhandled_signals && unhandled_signal(tsk, SIGSEGV) &&
 			printk_ratelimit()) {
@@ -291,9 +296,9 @@ gp_in_kernel:
 		return;
 
 	tsk->thread.error_code = error_code;
-	tsk->thread.trap_no = 13;
+	tsk->thread.trap_no = INTR_GPF;
 	if (notify_die(DIE_GPF, "general protection fault", regs,
-				error_code, 13, SIGSEGV) == NOTIFY_STOP)
+				error_code, INTR_GPF, SIGSEGV) == NOTIFY_STOP)
 		return;
 	die("general protection fault", regs, error_code);
 }
@@ -302,13 +307,13 @@ gp_in_kernel:
 dotraplinkage void __kprobes do_int3(struct pt_regs *regs, long error_code)
 {
 #ifdef CONFIG_KGDB_LOW_LEVEL_TRAP
-	if (kgdb_ll_trap(DIE_INT3, "int3", regs, error_code, 3, SIGTRAP)
-			== NOTIFY_STOP)
+	if (kgdb_ll_trap(DIE_INT3, "int3", regs, error_code, INTR_BREAKPOINT,
+				SIGTRAP) == NOTIFY_STOP)
 		return;
 #endif /* CONFIG_KGDB_LOW_LEVEL_TRAP */
 
-	if (notify_die(DIE_INT3, "int3", regs, error_code, 3, SIGTRAP)
-			== NOTIFY_STOP)
+	if (notify_die(DIE_INT3, "int3", regs, error_code, INTR_BREAKPOINT,
+				SIGTRAP) == NOTIFY_STOP)
 		return;
 
 	/*
@@ -317,7 +322,7 @@ dotraplinkage void __kprobes do_int3(struct pt_regs *regs, long error_code)
 	 */
 	debug_stack_usage_inc();
 	preempt_conditional_sti(regs);
-	do_trap(3, SIGTRAP, "int3", regs, error_code, NULL);
+	do_trap(INTR_BREAKPOINT, SIGTRAP, "int3", regs, error_code, NULL);
 	preempt_conditional_cli(regs);
 	debug_stack_usage_dec();
 }
@@ -423,7 +428,7 @@ dotraplinkage void __kprobes do_debug(struct pt_regs *regs, long error_code)
 
 	if (regs->flags & X86_VM_MASK) {
 		handle_vm86_trap((struct kernel_vm86_regs *) regs,
-				error_code, 1);
+				error_code, INTR_DEBUG);
 		preempt_conditional_cli(regs);
 		debug_stack_usage_dec();
 		return;
@@ -453,14 +458,15 @@ dotraplinkage void __kprobes do_debug(struct pt_regs *regs, long error_code)
 /*
  * Note that we play around with the 'TS' bit in an attempt to get
  * the correct behaviour even in the presence of the asynchronous
- * IRQ13 behaviour
+ * INTR_GPF behaviour
  */
 void math_error(struct pt_regs *regs, int error_code, int trapnr)
 {
 	struct task_struct *task = current;
 	siginfo_t info;
 	unsigned short err;
-	char *str = (trapnr == 16) ? "fpu exception" : "simd exception";
+	char *str = (trapnr == INTR_COPROCESSOR) ? "fpu exception" :
+						"simd exception";
 
 	if (notify_die(DIE_TRAP, str, regs, error_code, trapnr, SIGFPE) == NOTIFY_STOP)
 		return;
@@ -485,7 +491,7 @@ void math_error(struct pt_regs *regs, int error_code, int trapnr)
 	info.si_signo = SIGFPE;
 	info.si_errno = 0;
 	info.si_addr = (void __user *)regs->ip;
-	if (trapnr == 16) {
+	if (trapnr == INTR_COPROCESSOR) {
 		unsigned short cwd, swd;
 		/*
 		 * (~cwd & swd) will mask out exceptions that are not set to unmasked
@@ -529,8 +535,9 @@ void math_error(struct pt_regs *regs, int error_code, int trapnr)
 		info.si_code = FPE_FLTRES;
 	} else {
 		/*
-		 * If we're using IRQ 13, or supposedly even some trap 16
-		 * implementations, it's possible we get a spurious trap...
+		 * If we're using INTR_GPF, or supposedly even some trap
+		 * INTR_COPROCESSOR implementations, it's possible we get a
+		 * spurious trap...
 		 */
 		return;		/* Spurious trap, no error */
 	}
@@ -543,13 +550,13 @@ dotraplinkage void do_coprocessor_error(struct pt_regs *regs, long error_code)
 	ignore_fpu_irq = 1;
 #endif
 
-	math_error(regs, error_code, 16);
+	math_error(regs, error_code, INTR_COPROCESSOR);
 }
 
 dotraplinkage void
 do_simd_coprocessor_error(struct pt_regs *regs, long error_code)
 {
-	math_error(regs, error_code, 19);
+	math_error(regs, error_code, INTR_SIMD_COPROCESSOR);
 }
 
 dotraplinkage void
@@ -574,7 +581,7 @@ asmlinkage void __attribute__((weak)) smp_threshold_interrupt(void)
  * 'math_state_restore()' saves the current math information in the
  * old math state array, and gets the new ones from the current task
  *
- * Careful.. There are problems with IBM-designed IRQ13 behaviour.
+ * Careful.. There are problems with IBM-designed INTR_GPF behaviour.
  * Don't touch unless you *really* know how it works.
  *
  * Must be called with kernel preemption disabled (eg with local
@@ -646,17 +653,17 @@ dotraplinkage void do_iret_error(struct pt_regs *regs, long error_code)
 	if (notify_die(DIE_TRAP, "iret exception",
 			regs, error_code, 32, SIGILL) == NOTIFY_STOP)
 		return;
-	do_trap(32, SIGILL, "iret exception", regs, error_code, &info);
+	do_trap(INTR_IRET, SIGILL, "iret exception", regs, error_code, &info);
 }
 #endif
 
 /* Set of traps needed for early debugging. */
 void __init early_trap_init(void)
 {
-	set_intr_gate_ist(1, &debug, DEBUG_STACK);
+	set_intr_gate_ist(INTR_DEBUG, &debug, DEBUG_STACK);
 	/* int3 can be called from all */
-	set_system_intr_gate_ist(3, &int3, DEBUG_STACK);
-	set_intr_gate(14, &page_fault);
+	set_system_intr_gate_ist(INTR_BREAKPOINT, &int3, DEBUG_STACK);
+	set_intr_gate(INTR_PAGE_FAULT, &page_fault);
 	load_idt(&idt_descr);
 }
 
@@ -672,30 +679,30 @@ void __init trap_init(void)
 	early_iounmap(p, 4);
 #endif
 
-	set_intr_gate(0, &divide_error);
-	set_intr_gate_ist(2, &nmi, NMI_STACK);
+	set_intr_gate(INTR_DIV_BY_ZERO, &divide_error);
+	set_intr_gate_ist(INTR_NMI, &nmi, NMI_STACK);
 	/* int4 can be called from all */
-	set_system_intr_gate(4, &overflow);
-	set_intr_gate(5, &bounds);
-	set_intr_gate(6, &invalid_op);
-	set_intr_gate(7, &device_not_available);
+	set_system_intr_gate(INTR_OVERFLOW, &overflow);
+	set_intr_gate(INTR_BOUNDS_CHECK, &bounds);
+	set_intr_gate(INTR_INVALID_OP, &invalid_op);
+	set_intr_gate(INTR_NO_DEV, &device_not_available);
 #ifdef CONFIG_X86_32
-	set_task_gate(8, GDT_ENTRY_DOUBLEFAULT_TSS);
+	set_task_gate(INTR_DBL_FAULT, GDT_ENTRY_DOUBLEFAULT_TSS);
 #else
-	set_intr_gate_ist(8, &double_fault, DOUBLEFAULT_STACK);
+	set_intr_gate_ist(INTR_DBL_FAULT, &double_fault, DOUBLEFAULT_STACK);
 #endif
-	set_intr_gate(9, &coprocessor_segment_overrun);
-	set_intr_gate(10, &invalid_TSS);
-	set_intr_gate(11, &segment_not_present);
-	set_intr_gate_ist(12, &stack_segment, STACKFAULT_STACK);
-	set_intr_gate(13, &general_protection);
-	set_intr_gate(15, &spurious_interrupt_bug);
-	set_intr_gate(16, &coprocessor_error);
-	set_intr_gate(17, &alignment_check);
+	set_intr_gate(INTR_SEG_OVERRUN, &coprocessor_segment_overrun);
+	set_intr_gate(INTR_INVALID_TSS, &invalid_TSS);
+	set_intr_gate(INTR_NO_SEG, &segment_not_present);
+	set_intr_gate_ist(INTR_STACK_FAULT, &stack_segment, STACKFAULT_STACK);
+	set_intr_gate(INTR_GPF, &general_protection);
+	set_intr_gate(INTR_SPURIOUS, &spurious_interrupt_bug);
+	set_intr_gate(INTR_COPROCESSOR, &coprocessor_error);
+	set_intr_gate(INTR_ALIGNMENT, &alignment_check);
 #ifdef CONFIG_X86_MCE
-	set_intr_gate_ist(18, &machine_check, MCE_STACK);
+	set_intr_gate_ist(INTR_MCE, &machine_check, MCE_STACK);
 #endif
-	set_intr_gate(19, &simd_coprocessor_error);
+	set_intr_gate(INTR_SIMD_COPROCESSOR, &simd_coprocessor_error);
 
 	/* Reserve all the builtin and the syscall vector: */
 	for (i = 0; i < FIRST_EXTERNAL_VECTOR; i++)
@@ -720,7 +727,7 @@ void __init trap_init(void)
 
 #ifdef CONFIG_X86_64
 	memcpy(&nmi_idt_table, &idt_table, IDT_ENTRIES * 16);
-	set_nmi_gate(1, &debug);
-	set_nmi_gate(3, &int3);
+	set_nmi_gate(INTR_DEBUG, &debug);
+	set_nmi_gate(INTR_BREAKPOINT, &int3);
 #endif
 }
-- 
1.7.0.4

-- 
Kees Cook
ChromeOS Security

Re: [PATCH] x86: use enum instead of literals for trap values

[Ingo Molnar]

* Kees Cook <keescook@chromium.org> wrote:

> The traps are referred to by their numbers and it can be difficult to
> understand them while reading the code without context. This patch adds
> enumeration of the trap numbers and replaces the numbers with the correct
> enum for x86.
> 
> Signed-off-by: Kees Cook <keescook@chromium.org>
> 
> ---
> I've updated Aditya Kali's earlier patch:
> https://lkml.org/lkml/2011/4/22/328
> ---
>  arch/x86/include/asm/traps.h |   25 +++++++++
>  arch/x86/kernel/irqinit.c    |    2 +-
>  arch/x86/kernel/traps.c      |  117 ++++++++++++++++++++++--------------------
>  3 files changed, 88 insertions(+), 56 deletions(-)
> 
> diff --git a/arch/x86/include/asm/traps.h b/arch/x86/include/asm/traps.h
> index 0012d09..768afb2 100644
> --- a/arch/x86/include/asm/traps.h
> +++ b/arch/x86/include/asm/traps.h
> @@ -89,4 +89,29 @@ asmlinkage void smp_thermal_interrupt(void);
>  asmlinkage void mce_threshold_interrupt(void);
>  #endif
>  
> +/* Interrupts/Exceptions */
> +enum {
> +	INTR_DIV_BY_ZERO = 0,	/*  0 */
> +	INTR_DEBUG,		/*  1 */
> +	INTR_NMI,		/*  2 */
> +	INTR_BREAKPOINT,	/*  3 */
> +	INTR_OVERFLOW,		/*  4 */
> +	INTR_BOUNDS_CHECK,	/*  5 */
> +	INTR_INVALID_OP,	/*  6 */
> +	INTR_NO_DEV,		/*  7 */
> +	INTR_DBL_FAULT,		/*  8 */
> +	INTR_SEG_OVERRUN,	/*  9 */
> +	INTR_INVALID_TSS,	/* 10 */
> +	INTR_NO_SEG,		/* 11 */
> +	INTR_STACK_FAULT,	/* 12 */
> +	INTR_GPF,		/* 13 */
> +	INTR_PAGE_FAULT,	/* 14 */
> +	INTR_SPURIOUS,		/* 15 */
> +	INTR_COPROCESSOR,	/* 16 */
> +	INTR_ALIGNMENT,		/* 17 */
> +	INTR_MCE,		/* 18 */
> +	INTR_SIMD_COPROCESSOR,	/* 19 */
> +	INTR_IRET = 32,		/* 32 */
> +};

> @@ -453,14 +458,15 @@ dotraplinkage void __kprobes do_debug(struct pt_regs *regs, long error_code)
>  /*
>   * Note that we play around with the 'TS' bit in an attempt to get
>   * the correct behaviour even in the presence of the asynchronous
> - * IRQ13 behaviour
> + * INTR_GPF behaviour
>   */

> @@ -529,8 +535,9 @@ void math_error(struct pt_regs *regs, int error_code, int trapnr)
>  		info.si_code = FPE_FLTRES;
>  	} else {
>  		/*
> -		 * If we're using IRQ 13, or supposedly even some trap 16
> -		 * implementations, it's possible we get a spurious trap...
> +		 * If we're using INTR_GPF, or supposedly even some trap
> +		 * INTR_COPROCESSOR implementations, it's possible we get a
> +		 * spurious trap...

There's confusion in this patch between legacy IRQ #13 [vector 
0x20 + 13 ] and #GPF general protection fault [vector 13] - they 
are not the same.

Furthermore, the INTR_ naming is not ideal either for (most of) 
these entries: for example we don't think of a page fault as an 
asynchronous interrupt entity - we think of it as a more or less 
synchronous fault/exception.

Thus a X86_*_FAULT_VEC naming pattern might be better:

	X86_PAGE_FAULT_VEC
	X86_DOUBLE_FAULT_VEC

(With X86_*_EXCEPTION_VEC applied where appropriate.)

I don't disagree with the general principle of the cleanup 
otherwise, the numeric literals are often ambiguous and 
confusing - as the trap 13 - irq 13 mixup above shows.

Thanks,

	Ingo

Re: [PATCH] x86: use enum instead of literals for trap values

[Kees Cook]

On Fri, Mar 9, 2012 at 1:28 AM, Ingo Molnar <mingo@elte.hu> wrote:
>
> * Kees Cook <keescook@chromium.org> wrote:
>
>> The traps are referred to by their numbers and it can be difficult to
>> understand them while reading the code without context. This patch adds
>> enumeration of the trap numbers and replaces the numbers with the correct
>> enum for x86.
>>
>> Signed-off-by: Kees Cook <keescook@chromium.org>
>>
>> ---
>> I've updated Aditya Kali's earlier patch:
>> https://lkml.org/lkml/2011/4/22/328
>> ---
>>  arch/x86/include/asm/traps.h |   25 +++++++++
>>  arch/x86/kernel/irqinit.c    |    2 +-
>>  arch/x86/kernel/traps.c      |  117 ++++++++++++++++++++++--------------------
>>  3 files changed, 88 insertions(+), 56 deletions(-)
>>
>> diff --git a/arch/x86/include/asm/traps.h b/arch/x86/include/asm/traps.h
>> index 0012d09..768afb2 100644
>> --- a/arch/x86/include/asm/traps.h
>> +++ b/arch/x86/include/asm/traps.h
>> @@ -89,4 +89,29 @@ asmlinkage void smp_thermal_interrupt(void);
>>  asmlinkage void mce_threshold_interrupt(void);
>>  #endif
>>
>> +/* Interrupts/Exceptions */
>> +enum {
>> +     INTR_DIV_BY_ZERO = 0,   /*  0 */
>> +     INTR_DEBUG,             /*  1 */
>> +     INTR_NMI,               /*  2 */
>> +     INTR_BREAKPOINT,        /*  3 */
>> +     INTR_OVERFLOW,          /*  4 */
>> +     INTR_BOUNDS_CHECK,      /*  5 */
>> +     INTR_INVALID_OP,        /*  6 */
>> +     INTR_NO_DEV,            /*  7 */
>> +     INTR_DBL_FAULT,         /*  8 */
>> +     INTR_SEG_OVERRUN,       /*  9 */
>> +     INTR_INVALID_TSS,       /* 10 */
>> +     INTR_NO_SEG,            /* 11 */
>> +     INTR_STACK_FAULT,       /* 12 */
>> +     INTR_GPF,               /* 13 */
>> +     INTR_PAGE_FAULT,        /* 14 */
>> +     INTR_SPURIOUS,          /* 15 */
>> +     INTR_COPROCESSOR,       /* 16 */
>> +     INTR_ALIGNMENT,         /* 17 */
>> +     INTR_MCE,               /* 18 */
>> +     INTR_SIMD_COPROCESSOR,  /* 19 */
>> +     INTR_IRET = 32,         /* 32 */
>> +};
>
>> @@ -453,14 +458,15 @@ dotraplinkage void __kprobes do_debug(struct pt_regs *regs, long error_code)
>>  /*
>>   * Note that we play around with the 'TS' bit in an attempt to get
>>   * the correct behaviour even in the presence of the asynchronous
>> - * IRQ13 behaviour
>> + * INTR_GPF behaviour
>>   */
>
>> @@ -529,8 +535,9 @@ void math_error(struct pt_regs *regs, int error_code, int trapnr)
>>               info.si_code = FPE_FLTRES;
>>       } else {
>>               /*
>> -              * If we're using IRQ 13, or supposedly even some trap 16
>> -              * implementations, it's possible we get a spurious trap...
>> +              * If we're using INTR_GPF, or supposedly even some trap
>> +              * INTR_COPROCESSOR implementations, it's possible we get a
>> +              * spurious trap...
>
> There's confusion in this patch between legacy IRQ #13 [vector
> 0x20 + 13 ] and #GPF general protection fault [vector 13] - they
> are not the same.
>
> Furthermore, the INTR_ naming is not ideal either for (most of)
> these entries: for example we don't think of a page fault as an
> asynchronous interrupt entity - we think of it as a more or less
> synchronous fault/exception.
>
> Thus a X86_*_FAULT_VEC naming pattern might be better:
>
>        X86_PAGE_FAULT_VEC
>        X86_DOUBLE_FAULT_VEC
>
> (With X86_*_EXCEPTION_VEC applied where appropriate.)

Oh, hrm, my v2 missed this bit about EXCEPTION. What should I use as
the canonical source for "FAULT" vs "EXCEPTION" for this enum?

> I don't disagree with the general principle of the cleanup
> otherwise, the numeric literals are often ambiguous and
> confusing - as the trap 13 - irq 13 mixup above shows.

Right, and leaves me a bit confused too. :)

-Kees

-- 
Kees Cook
ChromeOS Security

Re: [PATCH] x86: use enum instead of literals for trap values

[H. Peter Anvin]

On 03/09/2012 08:30 AM, Kees Cook wrote:
> On Fri, Mar 9, 2012 at 1:28 AM, Ingo Molnar <mingo@elte.hu> wrote:
>>
>> * Kees Cook <keescook@chromium.org> wrote:
>>
>>> The traps are referred to by their numbers and it can be difficult to
>>> understand them while reading the code without context. This patch adds
>>> enumeration of the trap numbers and replaces the numbers with the correct
>>> enum for x86.
>>>

I have to admit personally to prefer something like X86_XCP_XX where XX
is the two-letter code that the Intel documentation uses for that trap,
i.e. #GP, #BR, #MC and so on.

	-hpa

Re: [PATCH] x86: use enum instead of literals for trap values

[Kees Cook]

On Fri, Mar 9, 2012 at 9:57 AM, H. Peter Anvin <hpa@zytor.com> wrote:
> On 03/09/2012 08:30 AM, Kees Cook wrote:
>> On Fri, Mar 9, 2012 at 1:28 AM, Ingo Molnar <mingo@elte.hu> wrote:
>>>
>>> * Kees Cook <keescook@chromium.org> wrote:
>>>
>>>> The traps are referred to by their numbers and it can be difficult to
>>>> understand them while reading the code without context. This patch adds
>>>> enumeration of the trap numbers and replaces the numbers with the correct
>>>> enum for x86.
>>>>
>
> I have to admit personally to prefer something like X86_XCP_XX where XX
> is the two-letter code that the Intel documentation uses for that trap,
> i.e. #GP, #BR, #MC and so on.

We need a single person to decide on this bike shed color. :) If the
list of enum names can be agreed on, I'll be happy to do the
search/replace for it.

-Kees

-- 
Kees Cook
ChromeOS Security

Re: [PATCH] x86: use enum instead of literals for trap values

[Borislav Petkov]

On Fri, Mar 09, 2012 at 10:21:52AM -0800, Kees Cook wrote:
> > I have to admit personally to prefer something like X86_XCP_XX where XX
> > is the two-letter code that the Intel documentation uses for that trap,
> > i.e. #GP, #BR, #MC and so on.
> 
> We need a single person to decide on this bike shed color. :) If the
> list of enum names can be agreed on, I'll be happy to do the
> search/replace for it.

Well,

here are my 2¢: I agree with hpa because

a) it maps the CPU vendor documentation
b) it is nicely short

Thanks.

-- 
Regards/Gruss,
Boris.

Advanced Micro Devices GmbH
Einsteinring 24, 85609 Dornach
GM: Alberto Bozzo
Reg: Dornach, Landkreis Muenchen
HRB Nr. 43632 WEEE Registernr: 129 19551

Re: [PATCH] x86: use enum instead of literals for trap values

[Steven Rostedt]

On Fri, 2012-03-09 at 19:28 +0100, Borislav Petkov wrote:
> On Fri, Mar 09, 2012 at 10:21:52AM -0800, Kees Cook wrote:
> > > I have to admit personally to prefer something like X86_XCP_XX where XX
> > > is the two-letter code that the Intel documentation uses for that trap,
> > > i.e. #GP, #BR, #MC and so on.
> > 
> > We need a single person to decide on this bike shed color. :) If the
> > list of enum names can be agreed on, I'll be happy to do the
> > search/replace for it.
> 
> Well,
> 
> here are my 2¢: I agree with hpa because
> 
> a) it maps the CPU vendor documentation
> b) it is nicely short

I agree too, except where does "XCP" come from? Is it short for
"exception"? Doing a search on "XCP Intel" gives me hits on Armari XCP
an "XCP server". Perhaps to keep in line with the documentation, should
it be:

X86_VEC_XX?

-- Steve

Re: [PATCH] x86: use enum instead of literals for trap values

[H. Peter Anvin]

On 03/09/2012 10:52 AM, Steven Rostedt wrote:
> I agree too, except where does "XCP" come from? Is it short for
> "exception"?

Yes, short for "exception".  I hate to admit it, but it comes from the
constants we used in the Transmeta microcode ;)

	-hpa

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

Re: [PATCH] x86: use enum instead of literals for trap values

[Ingo Molnar]

* H. Peter Anvin <hpa@zytor.com> wrote:

> On 03/09/2012 10:52 AM, Steven Rostedt wrote:
> > I agree too, except where does "XCP" come from? Is it short for
> > "exception"?
> 
> Yes, short for "exception".  I hate to admit it, but it comes from the
> constants we used in the Transmeta microcode ;)

Hm, were I to see 'X86_XCP_MC' for the first time I'd have no 
idea what it means. Really, lets not use random new 
abbreviations, lets use well-known patterns.

Make it X86_TRAP_XX, it's both short and correct?

Thanks,

	Ingo

Re: [PATCH] x86: use enum instead of literals for trap values

[H. Peter Anvin]

On 03/09/2012 02:13 PM, Ingo Molnar wrote:
> 
> Make it X86_TRAP_XX, it's both short and correct?
> 

Works for me.

	-hpa

Re: [PATCH] x86: use enum instead of literals for trap values

[Pekka Enberg]

Hi,

On Sat, Mar 10, 2012 at 12:13 AM, Ingo Molnar <mingo@elte.hu> wrote:
>> On 03/09/2012 10:52 AM, Steven Rostedt wrote:
>> > I agree too, except where does "XCP" come from? Is it short for
>> > "exception"?
>>
>> Yes, short for "exception".  I hate to admit it, but it comes from the
>> constants we used in the Transmeta microcode ;)
>
> Hm, were I to see 'X86_XCP_MC' for the first time I'd have no
> idea what it means. Really, lets not use random new
> abbreviations, lets use well-known patterns.
>
> Make it X86_TRAP_XX, it's both short and correct?

Yeah, my thoughts exactly. I don't think I have ever seen "XCP" before
so it really feels like an obscure new TLA to me.

                        Pekka

Re: [PATCH] x86: use enum instead of literals for trap values

[Kees Cook]

On Fri, Mar 9, 2012 at 10:28 AM, Borislav Petkov <bp@amd64.org> wrote:
> On Fri, Mar 09, 2012 at 10:21:52AM -0800, Kees Cook wrote:
>> > I have to admit personally to prefer something like X86_XCP_XX where XX
>> > is the two-letter code that the Intel documentation uses for that trap,
>> > i.e. #GP, #BR, #MC and so on.
>>
>> We need a single person to decide on this bike shed color. :) If the
>> list of enum names can be agreed on, I'll be happy to do the
>> search/replace for it.
>
> Well,
>
> here are my 2¢: I agree with hpa because
>
> a) it maps the CPU vendor documentation
> b) it is nicely short

How about:

X86_XCP_DE = 0,         /* 0, Divide-by-zero */
X86_XCP_DB,             /* 1, Debug */
X86_XCP_NMI,            /* 2, Non-maskable Interrupt */
X86_XCP_BP,             /* 3, Breakpoint */
X86_XCP_OF,             /* 4, Overflow */
X86_XCP_BR,             /* 5, Bound Range Exceeded */
X86_XCP_UD,             /* 6, Invalid Opcode */
X86_XCP_NM,             /* 7, Device Not Available */
X86_XCP_DF,             /* 8, Double Fault */
X86_XCP_OLD_MF,         /* 9, Coprocessor Segment Overrun */
X86_XCP_TS,             /* 10, Invalid TSS */
X86_XCP_NP,             /* 11, Segment Not Present */
X86_XCP_SS,             /* 12, Stack-Segment Fault */
X86_XCP_GP,             /* 13, General Protection Fault  */
X86_XCP_PF,             /* 14, Page Fault */
X86_XCP_RES,            /* 15, Reserved */
X86_XCP_MF,             /* 16, x87 Floating-Point Exception */
X86_XCP_AC,             /* 17, Alignment Check */
X86_XCP_MC,             /* 18, Machine Check */
X86_XCP_XM,             /* 19, SIMD Floating-Point Exception */
X86_XCP_IRET = 32,      /* 32, IRET Exception */

There is a name collision for "MF", there's no mnemonic for NMI, IRET,
or the reserved "spurious" interrupt.

Can use "VEC" instead "XCP", as Steven suggests. FWIW, the table in
the Intel manual I had handy says "Exceptions/Interrupts".

-Kees

-- 
Kees Cook
ChromeOS Security

Re: [PATCH] x86: use enum instead of literals for trap values

[Borislav Petkov]

On Fri, Mar 09, 2012 at 10:54:19AM -0800, Kees Cook wrote:
> On Fri, Mar 9, 2012 at 10:28 AM, Borislav Petkov <bp@amd64.org> wrote:
> > On Fri, Mar 09, 2012 at 10:21:52AM -0800, Kees Cook wrote:
> >> > I have to admit personally to prefer something like X86_XCP_XX where XX
> >> > is the two-letter code that the Intel documentation uses for that trap,
> >> > i.e. #GP, #BR, #MC and so on.
> >>
> >> We need a single person to decide on this bike shed color. :) If the
> >> list of enum names can be agreed on, I'll be happy to do the
> >> search/replace for it.
> >
> > Well,
> >
> > here are my 2¢: I agree with hpa because
> >
> > a) it maps the CPU vendor documentation
> > b) it is nicely short
> 
> How about:
> 
> X86_XCP_DE = 0,         /* 0, Divide-by-zero */
> X86_XCP_DB,             /* 1, Debug */
> X86_XCP_NMI,            /* 2, Non-maskable Interrupt */
> X86_XCP_BP,             /* 3, Breakpoint */
> X86_XCP_OF,             /* 4, Overflow */
> X86_XCP_BR,             /* 5, Bound Range Exceeded */
> X86_XCP_UD,             /* 6, Invalid Opcode */
> X86_XCP_NM,             /* 7, Device Not Available */
> X86_XCP_DF,             /* 8, Double Fault */
> X86_XCP_OLD_MF,         /* 9, Coprocessor Segment Overrun */
> X86_XCP_TS,             /* 10, Invalid TSS */
> X86_XCP_NP,             /* 11, Segment Not Present */
> X86_XCP_SS,             /* 12, Stack-Segment Fault */
> X86_XCP_GP,             /* 13, General Protection Fault  */
> X86_XCP_PF,             /* 14, Page Fault */
> X86_XCP_RES,            /* 15, Reserved */

So is this reserved or are we using it for Spurious IRQs? If we use it,
then 'RES' is a bad name. Maybe we define our own like

X86_VEC_SP

and then do

X86_VEC_IR for IRET

in the manner we assumed for the rest?

> X86_XCP_MF,             /* 16, x87 Floating-Point Exception */
> X86_XCP_AC,             /* 17, Alignment Check */
> X86_XCP_MC,             /* 18, Machine Check */
> X86_XCP_XM,             /* 19, SIMD Floating-Point Exception */

Shouln't this be #XF actually? At least it is so in the AMD docs.

> X86_XCP_IRET = 32,      /* 32, IRET Exception */
> 
> There is a name collision for "MF", there's no mnemonic for NMI,

Well, in the AMD docs we actually do have the '#NMI' mnemonic in use.

> IRET, or the reserved "spurious" interrupt.
>
> Can use "VEC" instead "XCP", as Steven suggests.

Yeah, because those actually are fixed interrupt vectors, as they're
called in the AMD docs. Makes sense.

Thanks.

-- 
Regards/Gruss,
Boris.

Advanced Micro Devices GmbH
Einsteinring 24, 85609 Dornach
GM: Alberto Bozzo
Reg: Dornach, Landkreis Muenchen
HRB Nr. 43632 WEEE Registernr: 129 19551

Re: [PATCH] x86: use enum instead of literals for trap values

[Steven Rostedt]

On Fri, 2012-03-09 at 20:08 +0100, Borislav Petkov wrote:

> > Can use "VEC" instead "XCP", as Steven suggests.
> 
> Yeah, because those actually are fixed interrupt vectors, as they're
> called in the AMD docs. Makes sense.

I have to admit my bias may lean towards AMD's documentation as that's
the only x86_64 documentation I have/use. I don't know what Intel's
description is.

-- Steve

Re: [PATCH] x86: use enum instead of literals for trap values

[Kees Cook]

On Fri, Mar 9, 2012 at 11:08 AM, Borislav Petkov <bp@amd64.org> wrote:
> On Fri, Mar 09, 2012 at 10:54:19AM -0800, Kees Cook wrote:
>> On Fri, Mar 9, 2012 at 10:28 AM, Borislav Petkov <bp@amd64.org> wrote:
>> > On Fri, Mar 09, 2012 at 10:21:52AM -0800, Kees Cook wrote:
>> >> > I have to admit personally to prefer something like X86_XCP_XX where XX
>> >> > is the two-letter code that the Intel documentation uses for that trap,
>> >> > i.e. #GP, #BR, #MC and so on.
>> >>
>> >> We need a single person to decide on this bike shed color. :) If the
>> >> list of enum names can be agreed on, I'll be happy to do the
>> >> search/replace for it.
>> >
>> > Well,
>> >
>> > here are my 2¢: I agree with hpa because
>> >
>> > a) it maps the CPU vendor documentation
>> > b) it is nicely short
>>
>> How about:
>>
>> X86_XCP_DE = 0,         /* 0, Divide-by-zero */
>> X86_XCP_DB,             /* 1, Debug */
>> X86_XCP_NMI,            /* 2, Non-maskable Interrupt */
>> X86_XCP_BP,             /* 3, Breakpoint */
>> X86_XCP_OF,             /* 4, Overflow */
>> X86_XCP_BR,             /* 5, Bound Range Exceeded */
>> X86_XCP_UD,             /* 6, Invalid Opcode */
>> X86_XCP_NM,             /* 7, Device Not Available */
>> X86_XCP_DF,             /* 8, Double Fault */
>> X86_XCP_OLD_MF,         /* 9, Coprocessor Segment Overrun */
>> X86_XCP_TS,             /* 10, Invalid TSS */
>> X86_XCP_NP,             /* 11, Segment Not Present */
>> X86_XCP_SS,             /* 12, Stack-Segment Fault */
>> X86_XCP_GP,             /* 13, General Protection Fault  */
>> X86_XCP_PF,             /* 14, Page Fault */
>> X86_XCP_RES,            /* 15, Reserved */
>
> So is this reserved or are we using it for Spurious IRQs? If we use it,
> then 'RES' is a bad name. Maybe we define our own like
>
> X86_VEC_SP

Yeah, it's used for spurious.

> and then do
>
> X86_VEC_IR for IRET
>
> in the manner we assumed for the rest?

Sure. I wasn't sure if it was dangerous to take a two-letter combo. I
guess if there is a future collision, we can just solve it then.

>> X86_XCP_MF,             /* 16, x87 Floating-Point Exception */
>> X86_XCP_AC,             /* 17, Alignment Check */
>> X86_XCP_MC,             /* 18, Machine Check */
>> X86_XCP_XM,             /* 19, SIMD Floating-Point Exception */
>
> Shouln't this be #XF actually? At least it is so in the AMD docs.

XM in Intel, XF in AMD. I have no preference.

>> X86_XCP_IRET = 32,      /* 32, IRET Exception */
>>
>> There is a name collision for "MF", there's no mnemonic for NMI,
>
> Well, in the AMD docs we actually do have the '#NMI' mnemonic in use.

If AMD's list is more complete, I think that's a good enough reason to
use "XF" above. :)

>> IRET, or the reserved "spurious" interrupt.
>>
>> Can use "VEC" instead "XCP", as Steven suggests.
>
> Yeah, because those actually are fixed interrupt vectors, as they're
> called in the AMD docs. Makes sense.

That seems reasonable to me. Peter, how's that sound to you?

-Kees

-- 
Kees Cook
ChromeOS Security

Re: [PATCH] x86: use enum instead of literals for trap values

[H. Peter Anvin]

On 03/09/2012 11:08 AM, Borislav Petkov wrote:
> 
> So is this reserved or are we using it for Spurious IRQs? If we use it,
> then 'RES' is a bad name. Maybe we define our own like
> 
> X86_VEC_SP
> 
> and then do
> 
> X86_VEC_IR for IRET
> 
> in the manner we assumed for the rest?
> 

Actually I prefer if we didn't use two-letter versions for the ones that
aren't defined in the documentation; especially IRET which is a pure
Linux invention.

	-hpa

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

Re: [PATCH] x86: use enum instead of literals for trap values

[Steven Rostedt]

On Fri, 2012-03-09 at 12:13 -0800, H. Peter Anvin wrote:
> On 03/09/2012 11:08 AM, Borislav Petkov wrote:
> > 
> > So is this reserved or are we using it for Spurious IRQs? If we use it,
> > then 'RES' is a bad name. Maybe we define our own like
> > 
> > X86_VEC_SP
> > 
> > and then do
> > 
> > X86_VEC_IR for IRET
> > 
> > in the manner we assumed for the rest?
> > 
> 
> Actually I prefer if we didn't use two-letter versions for the ones that
> aren't defined in the documentation; especially IRET which is a pure
> Linux invention.
> 

I agree with this. The two-letter abbreviations are good if they come
from the official documentation. This makes it easy to reference.

For the Linux made up ones, a more descriptive name is better. Then
there wont be confusion when people look at "IR" and can't find it
online or in the document, and scratch their head thinking they are not
smart enough to do kernel development and we lose another kernel
developer ;-)

-- Steve

Re: [PATCH] x86: use enum instead of literals for trap values

[Borislav Petkov]

On Fri, Mar 09, 2012 at 03:21:58PM -0500, Steven Rostedt wrote:
> For the Linux made up ones, a more descriptive name is better. Then
> there wont be confusion when people look at "IR" and can't find it
> online or in the document, and scratch their head thinking they are not
> smart enough to do kernel development and we lose another kernel
> developer ;-)

Like that ever happens... Kernel developers have thick heads, in most
cases thicker than processor manuals.

:-)

-- 
Regards/Gruss,
    Boris.

Re: [PATCH] x86: use enum instead of literals for trap values

[Steven Rostedt]

On Sat, 2012-03-10 at 11:15 +0100, Borislav Petkov wrote:
> On Fri, Mar 09, 2012 at 03:21:58PM -0500, Steven Rostedt wrote:
> > For the Linux made up ones, a more descriptive name is better. Then
> > there wont be confusion when people look at "IR" and can't find it
> > online or in the document, and scratch their head thinking they are not
> > smart enough to do kernel development and we lose another kernel
> > developer ;-)
> 
> Like that ever happens... Kernel developers have thick heads, in most
> cases thicker than processor manuals.

I'm talking about new kernel developers, not the current ones ;-)

-- Steve

Re: [PATCH] x86: use enum instead of literals for trap values

[Borislav Petkov]

On Sat, Mar 10, 2012 at 08:54:36AM -0500, Steven Rostedt wrote:
> On Sat, 2012-03-10 at 11:15 +0100, Borislav Petkov wrote:
> > On Fri, Mar 09, 2012 at 03:21:58PM -0500, Steven Rostedt wrote:
> > > For the Linux made up ones, a more descriptive name is better. Then
> > > there wont be confusion when people look at "IR" and can't find it
> > > online or in the document, and scratch their head thinking they are not
> > > smart enough to do kernel development and we lose another kernel
> > > developer ;-)
> > 
> > Like that ever happens... Kernel developers have thick heads, in most
> > cases thicker than processor manuals.
> 
> I'm talking about new kernel developers, not the current ones ;-)

Yeah, this was more of an observation about what it takes to be a kernel
dude. Remember how off an on people appear on lkml asking what they
need to do in order to get involved in kernel development and how other
people go off in trying to be super helpful recite down all the newbie
docs and TODOs?

Well, my answer in such cases would rather be "grow a fat, thick head
and leave your emotions behind the corner - then you can stare on
hardware manuals and try to understand the code all you want!"

:-)

Oh well, enough bullshit...

-- 
Regards/Gruss,
    Boris.

Re: [PATCH] x86: use enum instead of literals for trap values

[Alexey Dobriyan]

On Fri, Mar 09, 2012 at 08:30:49AM -0800, Kees Cook wrote:
> >> +enum {
> >> +     INTR_DIV_BY_ZERO = 0,   /*  0 */
> >> +     INTR_DEBUG,             /*  1 */
> >> +     INTR_NMI,               /*  2 */
> >> +     INTR_BREAKPOINT,        /*  3 */
> >> +     INTR_OVERFLOW,          /*  4 */
> >> +     INTR_BOUNDS_CHECK,      /*  5 */
> >> +     INTR_INVALID_OP,        /*  6 */
> >> +     INTR_NO_DEV,            /*  7 */
> >> +     INTR_DBL_FAULT,         /*  8 */
> >> +     INTR_SEG_OVERRUN,       /*  9 */
> >> +     INTR_INVALID_TSS,       /* 10 */
> >> +     INTR_NO_SEG,            /* 11 */
> >> +     INTR_STACK_FAULT,       /* 12 */
> >> +     INTR_GPF,               /* 13 */
> >> +     INTR_PAGE_FAULT,        /* 14 */
> >> +     INTR_SPURIOUS,          /* 15 */
> >> +     INTR_COPROCESSOR,       /* 16 */
> >> +     INTR_ALIGNMENT,         /* 17 */
> >> +     INTR_MCE,               /* 18 */
> >> +     INTR_SIMD_COPROCESSOR,  /* 19 */
> >> +     INTR_IRET = 32,         /* 32 */
> >> +};

This should be written like INTR_DEBUG=1 etc

[PATCH] x86: Use enum instead of literals for trap values

[John Kacur]

From: Kees Cook <keescook@chromium.org>

commit c94082656dac74257f63e91f78d5d458ac781fa5 upstream

The traps are referred to by their numbers and it can be difficult to
understand them while reading the code without context. This patch adds
enumeration of the trap numbers and replaces the numbers with the correct
enum for x86.

Signed-off-by: Kees Cook <keescook@chromium.org>
Link: http://lkml.kernel.org/r/20120310000710.GA32667@www.outflux.net
Signed-off-by: H. Peter Anvin <hpa@zytor.com>
Cherry-picked-for: v2.3.37

This fixes the following build error
make O=/bld/v3.2.37
make[2]: Nothing to be done for `relocs'.
  Using /home/jkacur/linux-rt as source for kernel
  GEN     /bld/v3.2.37/Makefile
  CHK     include/linux/version.h
  CHK     include/generated/utsrelease.h
  CALL    /home/jkacur/linux-rt/scripts/checksyscalls.sh
  CHK     include/generated/compile.h
  CHK     kernel/config_data.h
  CC [M]  drivers/misc/sgi-xp/xpc_main.o
/home/jkacur/linux-rt/drivers/misc/sgi-xp/xpc_main.c: In function ‘xpc_system_die’:
/home/jkacur/linux-rt/drivers/misc/sgi-xp/xpc_main.c:1208: error: ‘X86_TRAP_DF’ undeclared (first use in this function)
/home/jkacur/linux-rt/drivers/misc/sgi-xp/xpc_main.c:1208: error: (Each undeclared identifier is reported only once
/home/jkacur/linux-rt/drivers/misc/sgi-xp/xpc_main.c:1208: error: for each function it appears in.)
/home/jkacur/linux-rt/drivers/misc/sgi-xp/xpc_main.c:1211: error: ‘X86_TRAP_MF’ undeclared (first use in this function)
/home/jkacur/linux-rt/drivers/misc/sgi-xp/xpc_main.c:1212: error: ‘X86_TRAP_XF’ undeclared (first use in this function)
make[4]: *** [drivers/misc/sgi-xp/xpc_main.o] Error 1
make[3]: *** [drivers/misc/sgi-xp] Error 2
make[2]: *** [drivers/misc] Error 2
make[1]: *** [drivers] Error 2
make: *** [sub-make] Error 2

Conflicts:
	arch/x86/kernel/traps.c
Signed-off-by: John Kacur <jkacur@redhat.com>
---
 arch/x86/include/asm/traps.h |   25 +++++++++
 arch/x86/kernel/irqinit.c    |    2 +-
 arch/x86/kernel/traps.c      |  120 ++++++++++++++++++++++-------------------
 3 files changed, 90 insertions(+), 57 deletions(-)

diff --git a/arch/x86/include/asm/traps.h b/arch/x86/include/asm/traps.h
index 0012d09..88eae2a 100644
--- a/arch/x86/include/asm/traps.h
+++ b/arch/x86/include/asm/traps.h
@@ -89,4 +89,29 @@ asmlinkage void smp_thermal_interrupt(void);
 asmlinkage void mce_threshold_interrupt(void);
 #endif
 
+/* Interrupts/Exceptions */
+enum {
+	X86_TRAP_DE = 0,	/*  0, Divide-by-zero */
+	X86_TRAP_DB,		/*  1, Debug */
+	X86_TRAP_NMI,		/*  2, Non-maskable Interrupt */
+	X86_TRAP_BP,		/*  3, Breakpoint */
+	X86_TRAP_OF,		/*  4, Overflow */
+	X86_TRAP_BR,		/*  5, Bound Range Exceeded */
+	X86_TRAP_UD,		/*  6, Invalid Opcode */
+	X86_TRAP_NM,		/*  7, Device Not Available */
+	X86_TRAP_DF,		/*  8, Double Fault */
+	X86_TRAP_OLD_MF,	/*  9, Coprocessor Segment Overrun */
+	X86_TRAP_TS,		/* 10, Invalid TSS */
+	X86_TRAP_NP,		/* 11, Segment Not Present */
+	X86_TRAP_SS,		/* 12, Stack Segment Fault */
+	X86_TRAP_GP,		/* 13, General Protection Fault */
+	X86_TRAP_PF,		/* 14, Page Fault */
+	X86_TRAP_SPURIOUS,	/* 15, Spurious Interrupt */
+	X86_TRAP_MF,		/* 16, x87 Floating-Point Exception */
+	X86_TRAP_AC,		/* 17, Alignment Check */
+	X86_TRAP_MC,		/* 18, Machine Check */
+	X86_TRAP_XF,		/* 19, SIMD Floating-Point Exception */
+	X86_TRAP_IRET = 32,	/* 32, IRET Exception */
+};
+
 #endif /* _ASM_X86_TRAPS_H */
diff --git a/arch/x86/kernel/irqinit.c b/arch/x86/kernel/irqinit.c
index b3300e6..e328f69 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(get_irq_regs(), 0, 16);
+	math_error(get_irq_regs(), 0, X86_TRAP_MF);
 	return IRQ_HANDLED;
 }
 
diff --git a/arch/x86/kernel/traps.c b/arch/x86/kernel/traps.c
index 31d9d0f..e6fbb94 100644
--- a/arch/x86/kernel/traps.c
+++ b/arch/x86/kernel/traps.c
@@ -119,7 +119,7 @@ do_trap(int trapnr, int signr, char *str, struct pt_regs *regs,
 		 * traps 0, 1, 3, 4, and 5 should be forwarded to vm86.
 		 * On nmi (interrupt 2), do_trap should not be called.
 		 */
-		if (trapnr < 6)
+		if (trapnr < X86_TRAP_UD)
 			goto vm86_trap;
 		goto trap_signal;
 	}
@@ -203,27 +203,31 @@ dotraplinkage void do_##name(struct pt_regs *regs, long error_code)	\
 	do_trap(trapnr, signr, str, regs, error_code, &info);		\
 }
 
-DO_ERROR_INFO(0, SIGFPE, "divide error", divide_error, FPE_INTDIV, regs->ip)
-DO_ERROR(4, SIGSEGV, "overflow", overflow)
-DO_ERROR(5, SIGSEGV, "bounds", bounds)
-DO_ERROR_INFO(6, SIGILL, "invalid opcode", invalid_op, ILL_ILLOPN, regs->ip)
-DO_ERROR(9, SIGFPE, "coprocessor segment overrun", coprocessor_segment_overrun)
-DO_ERROR(10, SIGSEGV, "invalid TSS", invalid_TSS)
-DO_ERROR(11, SIGBUS, "segment not present", segment_not_present)
+DO_ERROR_INFO(X86_TRAP_DE, SIGFPE, "divide error", divide_error, FPE_INTDIV,
+		regs->ip)
+DO_ERROR(X86_TRAP_OF, SIGSEGV, "overflow", overflow)
+DO_ERROR(X86_TRAP_BR, SIGSEGV, "bounds", bounds)
+DO_ERROR_INFO(X86_TRAP_UD, SIGILL, "invalid opcode", invalid_op, ILL_ILLOPN,
+		regs->ip)
+DO_ERROR(X86_TRAP_OLD_MF, SIGFPE, "coprocessor segment overrun",
+		coprocessor_segment_overrun)
+DO_ERROR(X86_TRAP_TS, SIGSEGV, "invalid TSS", invalid_TSS)
+DO_ERROR(X86_TRAP_NP, SIGBUS, "segment not present", segment_not_present)
 #ifdef CONFIG_X86_32
-DO_ERROR(12, SIGBUS, "stack segment", stack_segment)
+DO_ERROR(X86_TRAP_SS, SIGBUS, "stack segment", stack_segment)
 #endif
-DO_ERROR_INFO(17, SIGBUS, "alignment check", alignment_check, BUS_ADRALN, 0)
+DO_ERROR_INFO(X86_TRAP_AC, SIGBUS, "alignment check", alignment_check,
+		BUS_ADRALN, 0)
 
 #ifdef CONFIG_X86_64
 /* Runs on IST stack */
 dotraplinkage void do_stack_segment(struct pt_regs *regs, long error_code)
 {
 	if (notify_die(DIE_TRAP, "stack segment", regs, error_code,
-			12, SIGBUS) == NOTIFY_STOP)
+			X86_TRAP_SS, SIGBUS) == NOTIFY_STOP)
 		return;
 	preempt_conditional_sti(regs);
-	do_trap(12, SIGBUS, "stack segment", regs, error_code, NULL);
+	do_trap(X86_TRAP_SS, SIGBUS, "stack segment", regs, error_code, NULL);
 	preempt_conditional_cli(regs);
 }
 
@@ -233,10 +237,10 @@ dotraplinkage void do_double_fault(struct pt_regs *regs, long error_code)
 	struct task_struct *tsk = current;
 
 	/* Return not checked because double check cannot be ignored */
-	notify_die(DIE_TRAP, str, regs, error_code, 8, SIGSEGV);
+	notify_die(DIE_TRAP, str, regs, error_code, X86_TRAP_DF, SIGSEGV);
 
 	tsk->thread.error_code = error_code;
-	tsk->thread.trap_no = 8;
+	tsk->thread.trap_no = X86_TRAP_DF;
 
 	/*
 	 * This is always a kernel trap and never fixable (and thus must
@@ -264,7 +268,7 @@ do_general_protection(struct pt_regs *regs, long error_code)
 		goto gp_in_kernel;
 
 	tsk->thread.error_code = error_code;
-	tsk->thread.trap_no = 13;
+	tsk->thread.trap_no = X86_TRAP_GP;
 
 	if (show_unhandled_signals && unhandled_signal(tsk, SIGSEGV) &&
 			printk_ratelimit()) {
@@ -291,9 +295,9 @@ gp_in_kernel:
 		return;
 
 	tsk->thread.error_code = error_code;
-	tsk->thread.trap_no = 13;
-	if (notify_die(DIE_GPF, "general protection fault", regs,
-				error_code, 13, SIGSEGV) == NOTIFY_STOP)
+	tsk->thread.trap_no = X86_TRAP_GP;
+	if (notify_die(DIE_GPF, "general protection fault", regs, error_code,
+			X86_TRAP_GP, SIGSEGV) == NOTIFY_STOP)
 		return;
 	die("general protection fault", regs, error_code);
 }
@@ -302,13 +306,14 @@ gp_in_kernel:
 dotraplinkage void __kprobes do_int3(struct pt_regs *regs, long error_code)
 {
 #ifdef CONFIG_KGDB_LOW_LEVEL_TRAP
-	if (kgdb_ll_trap(DIE_INT3, "int3", regs, error_code, 3, SIGTRAP)
-			== NOTIFY_STOP)
+	if (kgdb_ll_trap(DIE_INT3, "int3", regs, error_code, X86_TRAP_BP,
+				SIGTRAP) == NOTIFY_STOP)
 		return;
 #endif /* CONFIG_KGDB_LOW_LEVEL_TRAP */
 #ifdef CONFIG_KPROBES
-	if (notify_die(DIE_INT3, "int3", regs, error_code, 3, SIGTRAP)
-			== NOTIFY_STOP)
+
+	if (notify_die(DIE_INT3, "int3", regs, error_code, X86_TRAP_BP,
+			SIGTRAP) == NOTIFY_STOP)
 		return;
 #else
 	if (notify_die(DIE_TRAP, "int3", regs, error_code, 3, SIGTRAP)
@@ -317,7 +322,7 @@ dotraplinkage void __kprobes do_int3(struct pt_regs *regs, long error_code)
 #endif
 
 	preempt_conditional_sti(regs);
-	do_trap(3, SIGTRAP, "int3", regs, error_code, NULL);
+	do_trap(X86_TRAP_BP, SIGTRAP, "int3", regs, error_code, NULL);
 	preempt_conditional_cli(regs);
 }
 
@@ -415,8 +420,8 @@ dotraplinkage void __kprobes do_debug(struct pt_regs *regs, long error_code)
 	preempt_conditional_sti(regs);
 
 	if (regs->flags & X86_VM_MASK) {
-		handle_vm86_trap((struct kernel_vm86_regs *) regs,
-				error_code, 1);
+		handle_vm86_trap((struct kernel_vm86_regs *) regs, error_code,
+					X86_TRAP_DB);
 		preempt_conditional_cli(regs);
 		return;
 	}
@@ -451,7 +456,8 @@ void math_error(struct pt_regs *regs, int error_code, int trapnr)
 	struct task_struct *task = current;
 	siginfo_t info;
 	unsigned short err;
-	char *str = (trapnr == 16) ? "fpu exception" : "simd exception";
+	char *str = (trapnr == X86_TRAP_MF) ? "fpu exception" :
+						"simd exception";
 
 	if (notify_die(DIE_TRAP, str, regs, error_code, trapnr, SIGFPE) == NOTIFY_STOP)
 		return;
@@ -476,7 +482,7 @@ void math_error(struct pt_regs *regs, int error_code, int trapnr)
 	info.si_signo = SIGFPE;
 	info.si_errno = 0;
 	info.si_addr = (void __user *)regs->ip;
-	if (trapnr == 16) {
+	if (trapnr == X86_TRAP_MF) {
 		unsigned short cwd, swd;
 		/*
 		 * (~cwd & swd) will mask out exceptions that are not set to unmasked
@@ -520,10 +526,11 @@ void math_error(struct pt_regs *regs, int error_code, int trapnr)
 		info.si_code = FPE_FLTRES;
 	} else {
 		/*
-		 * If we're using IRQ 13, or supposedly even some trap 16
-		 * implementations, it's possible we get a spurious trap...
+		 * If we're using IRQ 13, or supposedly even some trap
+		 * X86_TRAP_MF implementations, it's possible
+		 * we get a spurious trap, which is not an error.
 		 */
-		return;		/* Spurious trap, no error */
+		return;
 	}
 	force_sig_info(SIGFPE, &info, task);
 }
@@ -534,13 +541,13 @@ dotraplinkage void do_coprocessor_error(struct pt_regs *regs, long error_code)
 	ignore_fpu_irq = 1;
 #endif
 
-	math_error(regs, error_code, 16);
+	math_error(regs, error_code, X86_TRAP_MF);
 }
 
 dotraplinkage void
 do_simd_coprocessor_error(struct pt_regs *regs, long error_code)
 {
-	math_error(regs, error_code, 19);
+	math_error(regs, error_code, X86_TRAP_XF);
 }
 
 dotraplinkage void
@@ -658,20 +665,21 @@ dotraplinkage void do_iret_error(struct pt_regs *regs, long error_code)
 	info.si_errno = 0;
 	info.si_code = ILL_BADSTK;
 	info.si_addr = NULL;
-	if (notify_die(DIE_TRAP, "iret exception",
-			regs, error_code, 32, SIGILL) == NOTIFY_STOP)
+	if (notify_die(DIE_TRAP, "iret exception", regs, error_code,
+			X86_TRAP_IRET, SIGILL) == NOTIFY_STOP)
 		return;
-	do_trap(32, SIGILL, "iret exception", regs, error_code, &info);
+	do_trap(X86_TRAP_IRET, SIGILL, "iret exception", regs, error_code,
+		&info);
 }
 #endif
 
 /* Set of traps needed for early debugging. */
 void __init early_trap_init(void)
 {
-	set_intr_gate_ist(1, &debug, DEBUG_STACK);
+	set_intr_gate_ist(X86_TRAP_DB, &debug, DEBUG_STACK);
 	/* int3 can be called from all */
-	set_system_intr_gate_ist(3, &int3, DEBUG_STACK);
-	set_intr_gate(14, &page_fault);
+	set_system_intr_gate_ist(X86_TRAP_BP, &int3, DEBUG_STACK);
+	set_intr_gate(X86_TRAP_PF, &page_fault);
 	load_idt(&idt_descr);
 }
 
@@ -687,30 +695,30 @@ void __init trap_init(void)
 	early_iounmap(p, 4);
 #endif
 
-	set_intr_gate(0, &divide_error);
-	set_intr_gate_ist(2, &nmi, NMI_STACK);
+	set_intr_gate(X86_TRAP_DE, &divide_error);
+	set_intr_gate_ist(X86_TRAP_NMI, &nmi, NMI_STACK);
 	/* int4 can be called from all */
-	set_system_intr_gate(4, &overflow);
-	set_intr_gate(5, &bounds);
-	set_intr_gate(6, &invalid_op);
-	set_intr_gate(7, &device_not_available);
+	set_system_intr_gate(X86_TRAP_OF, &overflow);
+	set_intr_gate(X86_TRAP_BR, &bounds);
+	set_intr_gate(X86_TRAP_UD, &invalid_op);
+	set_intr_gate(X86_TRAP_NM, &device_not_available);
 #ifdef CONFIG_X86_32
-	set_task_gate(8, GDT_ENTRY_DOUBLEFAULT_TSS);
+	set_task_gate(X86_TRAP_DF, GDT_ENTRY_DOUBLEFAULT_TSS);
 #else
-	set_intr_gate_ist(8, &double_fault, DOUBLEFAULT_STACK);
+	set_intr_gate_ist(X86_TRAP_DF, &double_fault, DOUBLEFAULT_STACK);
 #endif
-	set_intr_gate(9, &coprocessor_segment_overrun);
-	set_intr_gate(10, &invalid_TSS);
-	set_intr_gate(11, &segment_not_present);
-	set_intr_gate_ist(12, &stack_segment, STACKFAULT_STACK);
-	set_intr_gate(13, &general_protection);
-	set_intr_gate(15, &spurious_interrupt_bug);
-	set_intr_gate(16, &coprocessor_error);
-	set_intr_gate(17, &alignment_check);
+	set_intr_gate(X86_TRAP_OLD_MF, &coprocessor_segment_overrun);
+	set_intr_gate(X86_TRAP_TS, &invalid_TSS);
+	set_intr_gate(X86_TRAP_NP, &segment_not_present);
+	set_intr_gate_ist(X86_TRAP_SS, &stack_segment, STACKFAULT_STACK);
+	set_intr_gate(X86_TRAP_GP, &general_protection);
+	set_intr_gate(X86_TRAP_SPURIOUS, &spurious_interrupt_bug);
+	set_intr_gate(X86_TRAP_MF, &coprocessor_error);
+	set_intr_gate(X86_TRAP_AC, &alignment_check);
 #ifdef CONFIG_X86_MCE
-	set_intr_gate_ist(18, &machine_check, MCE_STACK);
+	set_intr_gate_ist(X86_TRAP_MC, &machine_check, MCE_STACK);
 #endif
-	set_intr_gate(19, &simd_coprocessor_error);
+	set_intr_gate(X86_TRAP_XF, &simd_coprocessor_error);
 
 	/* Reserve all the builtin and the syscall vector: */
 	for (i = 0; i < FIRST_EXTERNAL_VECTOR; i++)
-- 
1.7.2.3

Re: [PATCH] x86: Use enum instead of literals for trap values

[Ben Hutchings]

[-- Attachment #1: Type: text/plain, Size: 2254 bytes --]

On Tue, 2013-01-22 at 23:44 +0100, John Kacur wrote:
> From: Kees Cook <keescook@chromium.org>
> 
> commit c94082656dac74257f63e91f78d5d458ac781fa5 upstream
> 
> The traps are referred to by their numbers and it can be difficult to
> understand them while reading the code without context. This patch adds
> enumeration of the trap numbers and replaces the numbers with the correct
> enum for x86.
> 
> Signed-off-by: Kees Cook <keescook@chromium.org>
> Link: http://lkml.kernel.org/r/20120310000710.GA32667@www.outflux.net
> Signed-off-by: H. Peter Anvin <hpa@zytor.com>
> Cherry-picked-for: v2.3.37
> 
> This fixes the following build error
> make O=/bld/v3.2.37
> make[2]: Nothing to be done for `relocs'.
>   Using /home/jkacur/linux-rt as source for kernel
>   GEN     /bld/v3.2.37/Makefile
>   CHK     include/linux/version.h
>   CHK     include/generated/utsrelease.h
>   CALL    /home/jkacur/linux-rt/scripts/checksyscalls.sh
>   CHK     include/generated/compile.h
>   CHK     kernel/config_data.h
>   CC [M]  drivers/misc/sgi-xp/xpc_main.o
> /home/jkacur/linux-rt/drivers/misc/sgi-xp/xpc_main.c: In function ‘xpc_system_die’:
> /home/jkacur/linux-rt/drivers/misc/sgi-xp/xpc_main.c:1208: error: ‘X86_TRAP_DF’ undeclared (first use in this function)
> /home/jkacur/linux-rt/drivers/misc/sgi-xp/xpc_main.c:1208: error: (Each undeclared identifier is reported only once
> /home/jkacur/linux-rt/drivers/misc/sgi-xp/xpc_main.c:1208: error: for each function it appears in.)
> /home/jkacur/linux-rt/drivers/misc/sgi-xp/xpc_main.c:1211: error: ‘X86_TRAP_MF’ undeclared (first use in this function)
> /home/jkacur/linux-rt/drivers/misc/sgi-xp/xpc_main.c:1212: error: ‘X86_TRAP_XF’ undeclared (first use in this function)
> make[4]: *** [drivers/misc/sgi-xp/xpc_main.o] Error 1
> make[3]: *** [drivers/misc/sgi-xp] Error 2
> make[2]: *** [drivers/misc] Error 2
> make[1]: *** [drivers] Error 2
> make: *** [sub-make] Error 2
[...]

Queued up for 3.2, thanks.  I've also added X86_UV and SGI_XP to the
x86 .config I build.

Ben.

-- 
Ben Hutchings
Everything should be made as simple as possible, but not simpler.
                                                           - Albert Einstein

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]