lkml.org 
[lkml]   [2012]   [Mar]   [9]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    SubjectRe: [PATCH] x86: use enum instead of literals for trap values
    From
    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
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

    \
     
     \ /
      Last update: 2012-03-09 17:33    [W:0.031 / U:1.056 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site