[PATCH FIX] traps: x86: correct copy/paste bug: a trap is a GATE_TRAP

[PATCH FIX] traps: x86: correct copy/paste bug: a trap is a GATE_TRAP

[Alexander van Heukelum]

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

Signed-off-by: Alexander van Heukelum <heukelum@fastmail.fm>

---

> * Ingo Molnar <mingo@elte.hu> wrote:
> > * Alexander van Heukelum <heukelum@fastmail.fm> wrote:
> > > Hi Ingo,
> > > 
> > > This series unifies traps_32.c and traps_64.c.
> > 
> > wow, very nice! I've applied them to tip/x86/traps:
> > 
> > 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 <heukelum@fastmail.fm>
> | 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.
> 
> 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

---

diff --git a/include/asm-x86/desc.h b/include/asm-x86/desc.h
index 168c5cc..f06adac 100644
--- a/include/asm-x86/desc.h
+++ b/include/asm-x86/desc.h
@@ -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)

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

[Alexander van Heukelum]

 - 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 <heukelum@fastmail.fm>

---
 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

diff --git a/arch/x86/kernel/traps_32.c b/arch/x86/kernel/traps_32.c
index da97cd2..c17fcc8 100644
--- a/arch/x86/kernel/traps_32.c
+++ b/arch/x86/kernel/traps_32.c
@@ -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++)
diff --git a/arch/x86/kernel/traps_64.c b/arch/x86/kernel/traps_64.c
index 80dbbc7..8cf590b 100644
--- a/arch/x86/kernel/traps_64.c
+++ b/arch/x86/kernel/traps_64.c
@@ -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:
diff --git a/include/asm-x86/desc.h b/include/asm-x86/desc.h
index ebc3078..f06adac 100644
--- a/include/asm-x86/desc.h
+++ b/include/asm-x86/desc.h
@@ -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);
diff --git a/include/asm-x86/page_32.h b/include/asm-x86/page_32.h
index fe86ee2..9eef433 100644
--- a/include/asm-x86/page_32.h
+++ b/include/asm-x86/page_32.h
@@ -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 */
-- 
1.5.4.3

Re: [PATCH FIX] traps: x86: correct copy/paste bug: a trap is a GATE_TRAP

[Ingo Molnar]

* Alexander van Heukelum <heukelum@mailshack.com> wrote:

> > > > This series unifies traps_32.c and traps_64.c.
> > > 
> > > wow, very nice! I've applied them to tip/x86/traps:
> > > 
> > > 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 <heukelum@fastmail.fm>
> > | 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.
> > 
> > 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)
>  {
>  	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