Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752730AbbBZEJs (ORCPT ); Wed, 25 Feb 2015 23:09:48 -0500 Received: from cdptpa-outbound-snat.email.rr.com ([107.14.166.230]:10246 "EHLO cdptpa-oedge-vip.email.rr.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1751852AbbBZEJr (ORCPT ); Wed, 25 Feb 2015 23:09:47 -0500 Date: Wed, 25 Feb 2015 23:10:36 -0500 From: Steven Rostedt To: Wang Nan Cc: , , , , , , , , , Subject: Re: [PATCH] x86, traps: Enable DEBUG_STACK after cpu_init() for TRAP_DB/BP. Message-ID: <20150225231036.7c0de2b8@grimm.local.home> In-Reply-To: <1424923076-6106-1-git-send-email-wangnan0@huawei.com> References: <1424923076-6106-1-git-send-email-wangnan0@huawei.com> X-Mailer: Claws Mail 3.11.1 (GTK+ 2.24.25; x86_64-pc-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit X-RR-Connecting-IP: 107.14.168.130:25 X-Cloudmark-Score: 0 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2942 Lines: 85 On Thu, 26 Feb 2015 11:57:56 +0800 Wang Nan wrote: > Before this patch early_trap_init() installs DEBUG_STACK for X86_TRAP_BP > and X86_TRAP_DB. However, DEBUG_STACK doesn't work correctly until > cpu_init() <-- trap_init(). > > This patch passes 0 to set_intr_gate_ist() and > set_system_intr_gate_ist() instead of DEBUG_STACK to let it use same > stack as kernel, and installs DEBUG_STACK for them in trap_init(). > > As core runs at ring 0 between early_trap_init() and trap_init(), there > is no chance to get a bad stack before trap_init(). > > As NMI is also enabled in trap_init(), we don't need to care about > is_debug_stack() and related things used in arch/x86/kernel/nmi.c. Looks fine to me, except for some grammar issues in the comments. > > Signed-off-by: Wang Nan > --- > arch/x86/kernel/traps.c | 20 ++++++++++++++++++-- > 1 file changed, 18 insertions(+), 2 deletions(-) > > diff --git a/arch/x86/kernel/traps.c b/arch/x86/kernel/traps.c > index 9d2073e..a9b8640 100644 > --- a/arch/x86/kernel/traps.c > +++ b/arch/x86/kernel/traps.c > @@ -925,9 +925,16 @@ 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(X86_TRAP_DB, &debug, DEBUG_STACK); > + /* > + * Don't set ist to DEBUG_STACK as it doesn't work until TSS is > + * ready in cpu_init() <-- trap_init(). Before trap_init(), CPU > + * runs at ring 0 so there should be impossible to hit a invalid "so it should be impossible", although I might prefer, "it is impossible to", but nothing is ever guaranteed. "an invalid" > + * stack. Use original stack is enough. DEBUG_STACK will be "Using the original stack works well enough at this early stage." > + * equipped after cpu_init() in trap_init(). > + */ > + set_intr_gate_ist(X86_TRAP_DB, &debug, 0); > /* int3 can be called from all */ > - set_system_intr_gate_ist(X86_TRAP_BP, &int3, DEBUG_STACK); > + set_system_intr_gate_ist(X86_TRAP_BP, &int3, 0); > #ifdef CONFIG_X86_32 > set_intr_gate(X86_TRAP_PF, page_fault); > #endif > @@ -1005,6 +1012,15 @@ void __init trap_init(void) > */ > cpu_init(); > > + /* > + * X86_TRAP_DB and X86_TRAP_BP have been setup > + * in early_trap_init(). However, DEBUG_STACK works only after > + * cpu_init() load TSS. See comments in early_trap_init(). "cpu_init() loads TSS" -- Steve > + */ > + set_intr_gate_ist(X86_TRAP_DB, &debug, DEBUG_STACK); > + /* int3 can be called from all */ > + set_system_intr_gate_ist(X86_TRAP_BP, &int3, DEBUG_STACK); > + > x86_init.irqs.trap_init(); > > #ifdef CONFIG_X86_64 -- 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/