traps: x86: correct copy/paste bug: a trap is a GATE_TRAP

PATCH In review Alexander van Heukelum 2 participants 3 messages Oct 4, 2008 1 message not in the archive x86

lore

Share & tools

lore

A message that is not in the archive (sent off-list or not yet received): [email protected]

Alexander van Heukelum

author

2008-10-04 10:14 UTC #1 lore raw

PATCH 1/1 traps: x86: correct copy/paste bug: a trap is a GATE_TRAP

Fix copy/paste/forgot-to-edit bug in desc.h.

Signed-off-by Alexander van Heukelum <[email protected]>

---

Quoted 40 lines · Ingo Molnar

Quoted 7 lines · Alexander van Heukelum
Quoted 3 lines
Hi Ingo,
This series unifies traps_32.c and traps_64.c.
wow, very nice! I've applied them to tip/x86/traps:
Quoted 9 lines
f58f3d5: traps: x86: finalize unification of traps.c
bf395d6: traps: x86: make traps_32.c and traps_64.c equal
f156f35: traps: x86: various noop-changes preparing for unification of traps_xx.c 70cfe30: traps: x86_64: use task_pid_nr(tsk) instead of tsk->pid in do_general_protection
dc89ce0: traps: i386: expand clear_mem_error and remove from mach_traps.h 6f8063f: traps: x86_64: make io_check_error equal to the one on i386
d025445: traps: i386: use preempt_conditional_sti/cli in do_int3
2180afa: traps: x86_64: make math_state_restore more like i386
686cc4a: traps: x86: converge trap_init functions
-tip testing found a spontaneus reboot bug on two 32-bit systems (one Intel and one AMD testbox), and i've bisected it down to:
| 686cc4a0c1ca92bffbc22a897c3b433dadbbf444 is first bad commit
| commit 686cc4a0c1ca92bffbc22a897c3b433dadbbf444
| Author: Alexander van Heukelum <[email protected]>
| Date:   Fri Oct 3 22:00:32 2008 +0200
|
|     traps: x86: converge trap_init functions
config attached. The bisection log:
# bad: [a229a9da] Merge branch 'timers/urgent' # good: [27de5e39] Merge branch 'out-of-tree' # good: [8e9cb9db] Merge branch 'tracing/ring-buffer'
 # bad:  [70cfe30f] traps: x86_64: use task_pid_nr(tsk) instead of tsk
 # bad:  [d0254456] traps: i386: use preempt_conditional_sti/cli in do
 # bad:  [2180afaf] traps: x86_64: make math_state_restore more like i
 # bad:  [686cc4a0] traps: x86: converge trap_init functions
so i've excluded these commits from tip/master for now.

Quoted 8 lines

there's no serial log entry visible - the spontaneous reboot happens at around when we hit user-space.
I suspect syscall entry setup might be borked - the stack frames of call gates versus interrupt gates are different and it's easy to make a small mistake there with such effects.
Ingo

*blush*

You were so right. I have no idea how this has slipped through testing. Could you see if this on top of the traps branch makes things go again? I'll get you some replacement patches to make the whole thing bisectable again.

Alexander

---

include/asm-x86/desc.h +1 -1

index 168c5cc..f06adac 100644
@@ -354,7 +354,7 @@ static inline void set_system_intr_gate(unsigned int n, void *addr)
 static inline void set_system_trap_gate(unsigned int n, void *addr)
 {
 	BUG_ON((unsigned)n > 0xFF);
-	_set_gate(n, GATE_INTERRUPT, addr, 0x3, 0, __KERNEL_CS);
+	_set_gate(n, GATE_TRAP, addr, 0x3, 0, __KERNEL_CS);
 }
 
 static inline void set_trap_gate(unsigned int n, void *addr)

Alexander van Heukelum

author

2008-10-04 10:33 UTC #2 lore raw

PATCH v2 1/9 traps: x86: converge trap_init functions

- set_system_gate on i386 is really set_system_trap_gate
- set_system_gate on x86_64 is really set_system_intr_gate
- ist=0 means no special stack switch is done:
- introduce STACKFAULT_STACK, DOUBLEFAULT_STACK, NMI_STACK, DEBUG_STACK and MCE_STACK as on x86_64.
- use the _ist variants with XXX_STACK set to zero
- remove set_system_gate

Signed-off-by Alexander van Heukelum <[email protected]>

---
 arch/x86/kernel/traps_32.c |   16 +++++++++-------
 arch/x86/kernel/traps_64.c |    6 +++---
 include/asm-x86/desc.h     |   14 +++++---------
 include/asm-x86/page_32.h  |    6 ++++++
 4 files changed, 23 insertions(+), 19 deletions(-)

Hi Ingo,

Here is the replacement patch for [PATCH 1/9] traps: x86: converge trap_init functions. The others did not touch desc.h.

Greetings,
Alexander

arch/x86/kernel/traps_32.c +9 -7

index da97cd2..c17fcc8 100644
@@ -852,10 +852,12 @@ void __init trap_init(void)
 #endif
 
 	set_intr_gate(0, &divide_error);
-	set_intr_gate(1, &debug);
-	set_intr_gate(2, &nmi);
-	set_system_intr_gate(3, &int3); /* int3 can be called from all */
-	set_system_intr_gate(4, &overflow); /* int4 can be called from all */
+	set_intr_gate_ist(1, &debug, DEBUG_STACK);
+	set_intr_gate_ist(2, &nmi, NMI_STACK);
+	/* int3 can be called from all */
+	set_system_intr_gate_ist(3, &int3, DEBUG_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);

@@ -863,14 +865,14 @@ void __init trap_init(void)
 	set_intr_gate(9, &coprocessor_segment_overrun);
 	set_intr_gate(10, &invalid_TSS);
 	set_intr_gate(11, &segment_not_present);
-	set_intr_gate(12, &stack_segment);
+	set_intr_gate_ist(12, &stack_segment, STACKFAULT_STACK);
 	set_intr_gate(13, &general_protection);
 	set_intr_gate(14, &page_fault);
 	set_intr_gate(15, &spurious_interrupt_bug);
 	set_intr_gate(16, &coprocessor_error);
 	set_intr_gate(17, &alignment_check);
 #ifdef CONFIG_X86_MCE
-	set_intr_gate(18, &machine_check);
+	set_intr_gate_ist(18, &machine_check, MCE_STACK);
 #endif
 	set_intr_gate(19, &simd_coprocessor_error);

@@ -886,7 +888,7 @@ void __init trap_init(void)
 		printk("done.\n");
 	}
 
-	set_system_gate(SYSCALL_VECTOR, &system_call);
+	set_system_trap_gate(SYSCALL_VECTOR, &system_call);
 
 	/* Reserve all the builtin and the syscall vector: */
 	for (i = 0; i < FIRST_EXTERNAL_VECTOR; i++)

arch/x86/kernel/traps_64.c +3 -3

index 80dbbc7..8cf590b 100644
@@ -652,9 +652,9 @@ void __init trap_init(void)
 	set_intr_gate_ist(1, &debug, DEBUG_STACK);
 	set_intr_gate_ist(2, &nmi, NMI_STACK);
 	/* int3 can be called from all */
-	set_system_gate_ist(3, &int3, DEBUG_STACK);
+	set_system_intr_gate_ist(3, &int3, DEBUG_STACK);
 	/* int4 can be called from all */
-	set_system_gate(4, &overflow);
+	set_system_intr_gate(4, &overflow);
 	set_intr_gate(5, &bounds);
 	set_intr_gate(6, &invalid_op);
 	set_intr_gate(7, &device_not_available);

@@ -674,7 +674,7 @@ void __init trap_init(void)
 	set_intr_gate(19, &simd_coprocessor_error);
 
 #ifdef CONFIG_IA32_EMULATION
-	set_system_gate(IA32_SYSCALL_VECTOR, ia32_syscall);
+	set_system_intr_gate(IA32_SYSCALL_VECTOR, ia32_syscall);
 #endif
 	/*
 	 * Should be a barrier for any external CPU state:

include/asm-x86/desc.h +5 -9

index ebc3078..f06adac 100644
@@ -351,20 +351,16 @@ static inline void set_system_intr_gate(unsigned int n, void *addr)
 	_set_gate(n, GATE_INTERRUPT, addr, 0x3, 0, __KERNEL_CS);
 }
 
-static inline void set_trap_gate(unsigned int n, void *addr)
+static inline void set_system_trap_gate(unsigned int n, void *addr)
 {
 	BUG_ON((unsigned)n > 0xFF);
-	_set_gate(n, GATE_TRAP, addr, 0, 0, __KERNEL_CS);
+	_set_gate(n, GATE_TRAP, addr, 0x3, 0, __KERNEL_CS);
 }
 
-static inline void set_system_gate(unsigned int n, void *addr)
+static inline void set_trap_gate(unsigned int n, void *addr)
 {
 	BUG_ON((unsigned)n > 0xFF);
-#ifdef CONFIG_X86_32
-	_set_gate(n, GATE_TRAP, addr, 0x3, 0, __KERNEL_CS);
-#else
-	_set_gate(n, GATE_INTERRUPT, addr, 0x3, 0, __KERNEL_CS);
-#endif
+	_set_gate(n, GATE_TRAP, addr, 0, 0, __KERNEL_CS);
 }
 
 static inline void set_task_gate(unsigned int n, unsigned int gdt_entry)

@@ -379,7 +375,7 @@ static inline void set_intr_gate_ist(int n, void *addr, unsigned ist)
 	_set_gate(n, GATE_INTERRUPT, addr, 0, ist, __KERNEL_CS);
 }
 
-static inline void set_system_gate_ist(int n, void *addr, unsigned ist)
+static inline void set_system_intr_gate_ist(int n, void *addr, unsigned ist)
 {
 	BUG_ON((unsigned)n > 0xFF);
 	_set_gate(n, GATE_INTERRUPT, addr, 0x3, ist, __KERNEL_CS);

include/asm-x86/page_32.h +6 -0

index fe86ee2..9eef433 100644
@@ -20,6 +20,12 @@
 #endif
 #define THREAD_SIZE 	(PAGE_SIZE << THREAD_ORDER)
 
+#define STACKFAULT_STACK 0
+#define DOUBLEFAULT_STACK 1
+#define NMI_STACK 0
+#define DEBUG_STACK 0
+#define MCE_STACK 0
+#define N_EXCEPTION_STACKS 1
 
 #ifdef CONFIG_X86_PAE
 /* 44=32+12, the limit we can fit into an unsigned long pfn */

Signature · 1 line

1.5.4.3

Ingo Molnar

2008-10-04 12:14 UTC #3 lore raw

In reply to Alexander van Heukelum

Quoted 55 lines · Alexander van Heukelum

Quoted 36 lines
Quoted 4 lines
Quoted 1 line
This series unifies traps_32.c and traps_64.c.
wow, very nice! I've applied them to tip/x86/traps:
Quoted 9 lines
f58f3d5: traps: x86: finalize unification of traps.c
bf395d6: traps: x86: make traps_32.c and traps_64.c equal
f156f35: traps: x86: various noop-changes preparing for unification of traps_xx.c 70cfe30: traps: x86_64: use task_pid_nr(tsk) instead of tsk->pid in do_general_protection
dc89ce0: traps: i386: expand clear_mem_error and remove from mach_traps.h 6f8063f: traps: x86_64: make io_check_error equal to the one on i386
d025445: traps: i386: use preempt_conditional_sti/cli in do_int3
2180afa: traps: x86_64: make math_state_restore more like i386
686cc4a: traps: x86: converge trap_init functions
-tip testing found a spontaneus reboot bug on two 32-bit systems (one Intel and one AMD testbox), and i've bisected it down to:
| 686cc4a0c1ca92bffbc22a897c3b433dadbbf444 is first bad commit
| commit 686cc4a0c1ca92bffbc22a897c3b433dadbbf444
| Author: Alexander van Heukelum <[email protected]>
| Date:   Fri Oct 3 22:00:32 2008 +0200
|
|     traps: x86: converge trap_init functions
config attached. The bisection log:
# bad: [a229a9da] Merge branch 'timers/urgent' # good: [27de5e39] Merge branch 'out-of-tree' # good: [8e9cb9db] Merge branch 'tracing/ring-buffer'
 # bad:  [70cfe30f] traps: x86_64: use task_pid_nr(tsk) instead of tsk
 # bad:  [d0254456] traps: i386: use preempt_conditional_sti/cli in do
 # bad:  [2180afaf] traps: x86_64: make math_state_restore more like i
 # bad:  [686cc4a0] traps: x86: converge trap_init functions
so i've excluded these commits from tip/master for now.
Quoted 8 lines
there's no serial log entry visible - the spontaneous reboot happens at around when we hit user-space.
I suspect syscall entry setup might be borked - the stack frames of call gates versus interrupt gates are different and it's easy to make a small mistake there with such effects.
Ingo
*blush*
You were so right. I have no idea how this has slipped through testing. Could you see if this on top of the traps branch makes things go again? I'll get you some replacement patches to make the whole thing bisectable again.
static inline void set_system_trap_gate(unsigned int n, void *addr) {

include/asm-x86/desc.h static inline void set_system_intr_gate(unsigned int n, void *addr) · line 358

Quoted 4 lines

include/asm-x86/desc.h +1 -1

 	BUG_ON((unsigned)n > 0xFF);
-	_set_gate(n, GATE_INTERRUPT, addr, 0x3, 0, __KERNEL_CS);
+	_set_gate(n, GATE_TRAP, addr, 0x3, 0, __KERNEL_CS);
 }

indeed, that would explain it :)

i've added your delta fix, and will propagate/squash it back to 1/9 if it tests out fine. (it's worth having this series fully bisectable)

Ingo