[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20110503131820.GB23498@redhat.com>
Date: Tue, 3 May 2011 09:18:20 -0400
From: Don Zickus <dzickus@...hat.com>
To: Vaibhav Nagarnaik <vnagarnaik@...gle.com>
Cc: Steven Rostedt <rostedt@...dmis.org>,
Thomas Gleixner <tglx@...utronix.de>,
Ingo Molnar <mingo@...hat.com>,
Michael Rubin <mrubin@...gle.com>,
David Sharp <dhsharp@...gle.com>, linux-kernel@...r.kernel.org,
x86@...nel.org, Aditya Kali <adityakali@...gle.com>
Subject: Re: [PATCH 1/2] x86: Change trap definitions to enumerated values
On Fri, Apr 29, 2011 at 05:52:52PM -0700, Vaibhav Nagarnaik wrote:
> From: Aditya Kali <adityakali@...gle.com>
>
> 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 replaced the numbers to the correct
> enum for x86.
My only problem with this patch is you took a straight forward conversion
which didn't really change the functionality and sprinkled some trace
points in there.
I am ok with the conversion to something readable, but I don't know enough
about the trace point to know if those are correct spots.
Cheers,
Don
>
> Signed-off-by: Vaibhav Nagarnaik <vnagarnaik@...gle.com>
> ---
> arch/x86/include/asm/traps.h | 24 ++++++++
> arch/x86/kernel/traps.c | 121 +++++++++++++++++++++++-------------------
> 2 files changed, 91 insertions(+), 54 deletions(-)
>
> diff --git a/arch/x86/include/asm/traps.h b/arch/x86/include/asm/traps.h
> index 0310da6..c561caf 100644
> --- a/arch/x86/include/asm/traps.h
> +++ b/arch/x86/include/asm/traps.h
> @@ -87,4 +87,28 @@ 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 */
> +};
> +
> #endif /* _ASM_X86_TRAPS_H */
> diff --git a/arch/x86/kernel/traps.c b/arch/x86/kernel/traps.c
> index fbfc162..e058e8b 100644
> --- a/arch/x86/kernel/traps.c
> +++ b/arch/x86/kernel/traps.c
> @@ -129,7 +129,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;
> }
> @@ -213,27 +213,29 @@ 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_INFO(INTR_ALIGNMENT, SIGBUS, "alignment check", alignment_check, BUS_ADRALN, 0)
> +DO_ERROR_INFO(INTR_INVALID_OP, SIGILL, "invalid opcode", invalid_op, ILL_ILLOPN, regs->ip)
> +
> +DO_ERROR(INTR_OVERFLOW, SIGSEGV, "overflow", overflow)
> +DO_ERROR(INTR_BOUNDS_CHECK, SIGSEGV, "bounds", bounds)
> +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)
>
> #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);
> }
>
> @@ -243,10 +245,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
> @@ -274,7 +276,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()) {
> @@ -301,9 +303,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);
> }
> @@ -372,7 +374,8 @@ io_check_error(unsigned char reason, struct pt_regs *regs)
> static notrace __kprobes void
> unknown_nmi_error(unsigned char reason, struct pt_regs *regs)
> {
> - if (notify_die(DIE_NMIUNKNOWN, "nmi", regs, reason, 2, SIGINT) ==
> + if (notify_die(DIE_NMIUNKNOWN, "nmi", regs, reason, INTR_NMI,
> + SIGINT) ==
> NOTIFY_STOP)
> return;
> #ifdef CONFIG_MCA
> @@ -404,7 +407,8 @@ static notrace __kprobes void default_do_nmi(struct pt_regs *regs)
> * NMI, otherwise we may lose it, because the CPU-specific
> * NMI can not be detected/processed on other CPUs.
> */
> - if (notify_die(DIE_NMI, "nmi", regs, 0, 2, SIGINT) == NOTIFY_STOP)
> + if (notify_die(DIE_NMI, "nmi", regs, 0, INTR_NMI, SIGINT) ==
> + NOTIFY_STOP)
> return;
>
> /* Non-CPU-specific NMI: NMI sources can be processed on any CPU */
> @@ -460,22 +464,24 @@ void restart_nmi(void)
> 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 */
> #ifdef CONFIG_KPROBES
> - if (notify_die(DIE_INT3, "int3", regs, error_code, 3, SIGTRAP)
> + if (notify_die(DIE_INT3, "int3", regs, error_code, INTR_BREAKPOINT,
> + SIGTRAP)
> == NOTIFY_STOP)
> return;
> #else
> - if (notify_die(DIE_TRAP, "int3", regs, error_code, 3, SIGTRAP)
> + if (notify_die(DIE_TRAP, "int3", regs, error_code, INTR_BREAKPOINT,
> + SIGTRAP)
> == NOTIFY_STOP)
> return;
> #endif
>
> 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);
> }
>
> @@ -574,7 +580,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);
> return;
> }
> @@ -602,14 +608,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;
> @@ -634,7 +641,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
> @@ -678,8 +685,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 */
> }
> @@ -692,23 +700,25 @@ 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
> do_spurious_interrupt_bug(struct pt_regs *regs, long error_code)
> {
> + trace_trap_entry(INTR_SPURIOUS);
> conditional_sti(regs);
> #if 0
> /* No need to warn about this any longer. */
> printk(KERN_INFO "Ignoring P6 Local APIC Spurious Interrupt Bug...\n");
> #endif
> + trace_trap_exit(INTR_SPURIOUS);
> }
>
> asmlinkage void __attribute__((weak)) smp_thermal_interrupt(void)
> @@ -745,7 +755,7 @@ void __math_state_restore(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 (in this case,
> @@ -780,6 +790,7 @@ EXPORT_SYMBOL_GPL(math_state_restore);
> dotraplinkage void __kprobes
> do_device_not_available(struct pt_regs *regs, long error_code)
> {
> + trace_trap_entry(INTR_NO_DEV);
> #ifdef CONFIG_MATH_EMULATION
> if (read_cr0() & X86_CR0_EM) {
> struct math_emu_info info = { };
> @@ -788,6 +799,7 @@ do_device_not_available(struct pt_regs *regs, long error_code)
>
> info.regs = regs;
> math_emulate(&info);
> + trace_trap_exit(INTR_NO_DEV);
> return;
> }
> #endif
> @@ -795,6 +807,7 @@ do_device_not_available(struct pt_regs *regs, long error_code)
> #ifdef CONFIG_X86_32
> conditional_sti(regs);
> #endif
> + trace_trap_exit(INTR_NO_DEV);
> }
>
> #ifdef CONFIG_X86_32
> @@ -817,10 +830,10 @@ dotraplinkage void do_iret_error(struct pt_regs *regs, long error_code)
> /* 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);
> }
>
> @@ -836,30 +849,30 @@ void __init trap_init(void)
> early_iounmap(p, 4);
> #endif
>
> - set_intr_gate(0, ÷_error);
> - set_intr_gate_ist(2, &nmi, NMI_STACK);
> + set_intr_gate(INTR_DIV_BY_ZERO, ÷_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++)
> --
> 1.7.3.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@...r.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists