lkml.org 
[lkml]   [2012]   [Mar]   [9]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH] x86: use enum instead of literals for trap values

    * 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


    \
     
     \ /
      Last update: 2012-03-09 10:31    [W:0.025 / U:17.964 seconds]
    ©2003-2017 Jasper Spaans. hosted at Digital OceanAdvertise on this site