* [PATCH v2] x86, traps: Enable DEBUG_STACK after cpu_init() for TRAP_DB/BP. @ 2015-02-26 5:49 Wang Nan 2015-02-26 13:12 ` [tip:x86/asm] x86/traps: " tip-bot for Wang Nan 0 siblings, 1 reply; 11+ messages in thread From: Wang Nan @ 2015-02-26 5:49 UTC (permalink / raw) To: masami.hiramatsu.pt, rostedt Cc: mingo, hpa, tglx, x86, luto, oleg, dave.hansen, linux-kernel, lizefan 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. Signed-off-by: Wang Nan <wangnan0@huawei.com> Reviewed-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com> --- v1 -> v2: Correct grammar issues in comments. --- arch/x86/kernel/traps.c | 21 +++++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-) diff --git a/arch/x86/kernel/traps.c b/arch/x86/kernel/traps.c index 9d2073e..4281988 100644 --- a/arch/x86/kernel/traps.c +++ b/arch/x86/kernel/traps.c @@ -925,9 +925,17 @@ 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 it is impossible to hit an invalid stack. + * Using the original stack works well enough at this early + * stage. DEBUG_STACK will be 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 +1013,15 @@ void __init trap_init(void) */ cpu_init(); + /* + * X86_TRAP_DB and X86_TRAP_BP have been set + * in early_trap_init(). However, DEBUG_STACK works only after + * cpu_init() loads TSS. See comments in early_trap_init(). + */ + 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 -- 1.8.4 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* [tip:x86/asm] x86/traps: Enable DEBUG_STACK after cpu_init() for TRAP_DB/BP 2015-02-26 5:49 [PATCH v2] x86, traps: Enable DEBUG_STACK after cpu_init() for TRAP_DB/BP Wang Nan @ 2015-02-26 13:12 ` tip-bot for Wang Nan 2015-02-26 15:12 ` Andy Lutomirski 0 siblings, 1 reply; 11+ messages in thread From: tip-bot for Wang Nan @ 2015-02-26 13:12 UTC (permalink / raw) To: linux-tip-commits Cc: tglx, mingo, hpa, dave.hansen, masami.hiramatsu.pt, linux-kernel, oleg, rostedt, lizefan, wangnan0, luto Commit-ID: b4d8327024637cb2a1f7910dcb5d0ad7a096f473 Gitweb: http://git.kernel.org/tip/b4d8327024637cb2a1f7910dcb5d0ad7a096f473 Author: Wang Nan <wangnan0@huawei.com> AuthorDate: Thu, 26 Feb 2015 13:49:39 +0800 Committer: Ingo Molnar <mingo@kernel.org> CommitDate: Thu, 26 Feb 2015 12:29:20 +0100 x86/traps: Enable DEBUG_STACK after cpu_init() for TRAP_DB/BP 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. Signed-off-by: Wang Nan <wangnan0@huawei.com> Reviewed-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com> Acked-by: Steven Rostedt <rostedt@goodmis.org> Cc: <dave.hansen@linux.intel.com> Cc: <lizefan@huawei.com> Cc: <luto@amacapital.net> Cc: <oleg@redhat.com> Link: http://lkml.kernel.org/r/1424929779-13174-1-git-send-email-wangnan0@huawei.com Signed-off-by: Ingo Molnar <mingo@kernel.org> --- arch/x86/kernel/traps.c | 21 +++++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-) diff --git a/arch/x86/kernel/traps.c b/arch/x86/kernel/traps.c index 9d2073e..4281988 100644 --- a/arch/x86/kernel/traps.c +++ b/arch/x86/kernel/traps.c @@ -925,9 +925,17 @@ 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 it is impossible to hit an invalid stack. + * Using the original stack works well enough at this early + * stage. DEBUG_STACK will be 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 +1013,15 @@ void __init trap_init(void) */ cpu_init(); + /* + * X86_TRAP_DB and X86_TRAP_BP have been set + * in early_trap_init(). However, DEBUG_STACK works only after + * cpu_init() loads TSS. See comments in early_trap_init(). + */ + 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 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [tip:x86/asm] x86/traps: Enable DEBUG_STACK after cpu_init() for TRAP_DB/BP 2015-02-26 13:12 ` [tip:x86/asm] x86/traps: " tip-bot for Wang Nan @ 2015-02-26 15:12 ` Andy Lutomirski 2015-02-27 2:21 ` Wang Nan 0 siblings, 1 reply; 11+ messages in thread From: Andy Lutomirski @ 2015-02-26 15:12 UTC (permalink / raw) To: Thomas Gleixner, Ingo Molnar, H. Peter Anvin, Dave Hansen, Masami Hiramatsu, linux-kernel@vger.kernel.org, Oleg Nesterov, Li Zefan, Steven Rostedt, wangnan0, Andy Lutomirski Cc: linux-tip-commits@vger.kernel.org On Thu, Feb 26, 2015 at 5:12 AM, tip-bot for Wang Nan <tipbot@zytor.com> wrote: > Commit-ID: b4d8327024637cb2a1f7910dcb5d0ad7a096f473 > Gitweb: http://git.kernel.org/tip/b4d8327024637cb2a1f7910dcb5d0ad7a096f473 > Author: Wang Nan <wangnan0@huawei.com> > AuthorDate: Thu, 26 Feb 2015 13:49:39 +0800 > Committer: Ingo Molnar <mingo@kernel.org> > CommitDate: Thu, 26 Feb 2015 12:29:20 +0100 > > x86/traps: Enable DEBUG_STACK after cpu_init() for TRAP_DB/BP > > 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(). > Sorry, I'm late to the party. This patch, while technically correct AFAICT, is really ugly, because the whole point is that it *doesn't* use ist. In other words: > + set_intr_gate_ist(X86_TRAP_DB, &debug, 0); That should be set_intr_gate(X86_TRAP_DB, &debug); > /* 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); Similarly, this should be set_system_gate. > #ifdef CONFIG_X86_32 > set_intr_gate(X86_TRAP_PF, page_fault); > #endif > @@ -1005,6 +1013,15 @@ void __init trap_init(void) > */ > cpu_init(); > > + /* > + * X86_TRAP_DB and X86_TRAP_BP have been set > + * in early_trap_init(). However, DEBUG_STACK works only after > + * cpu_init() loads TSS. See comments in early_trap_init(). It's not that DEBUG_STACK only works after the TSS is loaded; it's that the IST mechanism only works after TSS is loaded. --Andy ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [tip:x86/asm] x86/traps: Enable DEBUG_STACK after cpu_init() for TRAP_DB/BP 2015-02-26 15:12 ` Andy Lutomirski @ 2015-02-27 2:21 ` Wang Nan 2015-02-27 2:33 ` Andy Lutomirski 0 siblings, 1 reply; 11+ messages in thread From: Wang Nan @ 2015-02-27 2:21 UTC (permalink / raw) To: Andy Lutomirski, Thomas Gleixner, Ingo Molnar, H. Peter Anvin, Dave Hansen, Masami Hiramatsu, linux-kernel@vger.kernel.org, Oleg Nesterov, Li Zefan, Steven Rostedt Cc: linux-tip-commits@vger.kernel.org On 2015/2/26 23:12, Andy Lutomirski wrote: > On Thu, Feb 26, 2015 at 5:12 AM, tip-bot for Wang Nan <tipbot@zytor.com> wrote: >> Commit-ID: b4d8327024637cb2a1f7910dcb5d0ad7a096f473 >> Gitweb: http://git.kernel.org/tip/b4d8327024637cb2a1f7910dcb5d0ad7a096f473 >> Author: Wang Nan <wangnan0@huawei.com> >> AuthorDate: Thu, 26 Feb 2015 13:49:39 +0800 >> Committer: Ingo Molnar <mingo@kernel.org> >> CommitDate: Thu, 26 Feb 2015 12:29:20 +0100 >> >> x86/traps: Enable DEBUG_STACK after cpu_init() for TRAP_DB/BP >> >> 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(). >> > > Sorry, I'm late to the party. This patch, while technically correct > AFAICT, is really ugly, because the whole point is that it *doesn't* > use ist. In other words: > >> + set_intr_gate_ist(X86_TRAP_DB, &debug, 0); > > That should be set_intr_gate(X86_TRAP_DB, &debug); > I considered this, but found that set_intr_gate() and set_intr_gate_ist(..., 0) behaviors differently: set_intr_gate() -> _trace_set_gate() requires 'trace_##addr' to be installed to trace_idt_table, while set_intr_gate_ist(..., 0) puts 'addr'. into trace_idt_table() through _set_gate(). Therefore, if we want to use set_intr_gate() we need to provide a trace_debug entry for it, which will be overwritten in trap_init() so it is in fact a useless symbol. Anyway, I'd like to post a v3 patch follow your advise. Please keep an eye on it. Thank you! >> /* 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); > > Similarly, this should be set_system_gate. > >> #ifdef CONFIG_X86_32 >> set_intr_gate(X86_TRAP_PF, page_fault); >> #endif >> @@ -1005,6 +1013,15 @@ void __init trap_init(void) >> */ >> cpu_init(); >> >> + /* >> + * X86_TRAP_DB and X86_TRAP_BP have been set >> + * in early_trap_init(). However, DEBUG_STACK works only after >> + * cpu_init() loads TSS. See comments in early_trap_init(). > > It's not that DEBUG_STACK only works after the TSS is loaded; it's > that the IST mechanism only works after TSS is loaded. > > --Andy > ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [tip:x86/asm] x86/traps: Enable DEBUG_STACK after cpu_init() for TRAP_DB/BP 2015-02-27 2:21 ` Wang Nan @ 2015-02-27 2:33 ` Andy Lutomirski 2015-02-27 3:28 ` [PATCH] x86, traps: early_trap_init() cleanup Wang Nan 2015-02-27 4:19 ` [PATCH v2] x86, traps: separate set_intr_gate() and cleanup early_trap_init() Wang Nan 0 siblings, 2 replies; 11+ messages in thread From: Andy Lutomirski @ 2015-02-27 2:33 UTC (permalink / raw) To: Wang Nan Cc: Thomas Gleixner, Ingo Molnar, H. Peter Anvin, Dave Hansen, Masami Hiramatsu, linux-kernel@vger.kernel.org, Oleg Nesterov, Li Zefan, Steven Rostedt, linux-tip-commits@vger.kernel.org On Thu, Feb 26, 2015 at 6:21 PM, Wang Nan <wangnan0@huawei.com> wrote: > On 2015/2/26 23:12, Andy Lutomirski wrote: >> On Thu, Feb 26, 2015 at 5:12 AM, tip-bot for Wang Nan <tipbot@zytor.com> wrote: >>> Commit-ID: b4d8327024637cb2a1f7910dcb5d0ad7a096f473 >>> Gitweb: http://git.kernel.org/tip/b4d8327024637cb2a1f7910dcb5d0ad7a096f473 >>> Author: Wang Nan <wangnan0@huawei.com> >>> AuthorDate: Thu, 26 Feb 2015 13:49:39 +0800 >>> Committer: Ingo Molnar <mingo@kernel.org> >>> CommitDate: Thu, 26 Feb 2015 12:29:20 +0100 >>> >>> x86/traps: Enable DEBUG_STACK after cpu_init() for TRAP_DB/BP >>> >>> 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(). >>> >> >> Sorry, I'm late to the party. This patch, while technically correct >> AFAICT, is really ugly, because the whole point is that it *doesn't* >> use ist. In other words: >> >>> + set_intr_gate_ist(X86_TRAP_DB, &debug, 0); >> >> That should be set_intr_gate(X86_TRAP_DB, &debug); >> > > I considered this, but found that set_intr_gate() and set_intr_gate_ist(..., 0) > behaviors differently: set_intr_gate() -> _trace_set_gate() requires 'trace_##addr' > to be installed to trace_idt_table, while set_intr_gate_ist(..., 0) puts 'addr'. > into trace_idt_table() through _set_gate(). > Therefore, if we want to use set_intr_gate() we need to provide a trace_debug entry > for it, which will be overwritten in trap_init() so it is in fact a useless symbol. Yuck. Then IMO we should have a separate helper for this. Better yet, we should rename set_intr_gate, because this distinction could easily be overlooked. > > Anyway, I'd like to post a v3 patch follow your advise. Please keep an eye on it. > > Thank you! > >>> /* 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); >> >> Similarly, this should be set_system_gate. >> >>> #ifdef CONFIG_X86_32 >>> set_intr_gate(X86_TRAP_PF, page_fault); >>> #endif >>> @@ -1005,6 +1013,15 @@ void __init trap_init(void) >>> */ >>> cpu_init(); >>> >>> + /* >>> + * X86_TRAP_DB and X86_TRAP_BP have been set >>> + * in early_trap_init(). However, DEBUG_STACK works only after >>> + * cpu_init() loads TSS. See comments in early_trap_init(). >> >> It's not that DEBUG_STACK only works after the TSS is loaded; it's >> that the IST mechanism only works after TSS is loaded. >> >> --Andy >> > > -- Andy Lutomirski AMA Capital Management, LLC ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH] x86, traps: early_trap_init() cleanup. 2015-02-27 2:33 ` Andy Lutomirski @ 2015-02-27 3:28 ` Wang Nan 2015-02-27 10:11 ` Borislav Petkov 2015-02-27 4:19 ` [PATCH v2] x86, traps: separate set_intr_gate() and cleanup early_trap_init() Wang Nan 1 sibling, 1 reply; 11+ messages in thread From: Wang Nan @ 2015-02-27 3:28 UTC (permalink / raw) To: luto, lizefan Cc: mingo, masami.hiramatsu.pt, linux-kernel, rostedt, linux-tip-commits, tglx, hpa, dave.hansen, oleg As early_trap_init() doesn't use IST, replace set_intr_gate_ist(..., 0) and set_system_intr_gate_ist(..., 0) with their standard counterparts. set_intr_gate() requires a trace_debug symbol which we don't have and won't use. Use a small macro trick as a workaround. Signed-off-by: Wang Nan <wangnan0@huawei.com> --- Hi Andy Lutomirski, This patch is a bit tricky, but I think we don't need to define another helper for such a small problem. What's your opinion? Thank you! --- arch/x86/kernel/traps.c | 23 ++++++++++++++--------- 1 file changed, 14 insertions(+), 9 deletions(-) diff --git a/arch/x86/kernel/traps.c b/arch/x86/kernel/traps.c index 4281988..c24434a 100644 --- a/arch/x86/kernel/traps.c +++ b/arch/x86/kernel/traps.c @@ -925,17 +925,22 @@ 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) { + /* - * 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 it is impossible to hit an invalid stack. - * Using the original stack works well enough at this early - * stage. DEBUG_STACK will be equipped after cpu_init() in - * trap_init(). + * Don't use IST to set 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 it is impossible to hit an invalid stack. Using the + * original stack works well enough at this early stage. DEBUG_STACK + * will be equipped after cpu_init() in trap_init(). + * + * Since set_intr_gate() needs a trace_debug but we don't have it, + * use the following #define as a workaround. */ - set_intr_gate_ist(X86_TRAP_DB, &debug, 0); +#define trace_debug debug + set_intr_gate(X86_TRAP_DB, debug); +#undef trace_debug /* int3 can be called from all */ - set_system_intr_gate_ist(X86_TRAP_BP, &int3, 0); + set_system_intr_gate(X86_TRAP_BP, &int3); #ifdef CONFIG_X86_32 set_intr_gate(X86_TRAP_PF, page_fault); #endif @@ -1015,7 +1020,7 @@ void __init trap_init(void) /* * X86_TRAP_DB and X86_TRAP_BP have been set - * in early_trap_init(). However, DEBUG_STACK works only after + * in early_trap_init(). However, ITS works only after * cpu_init() loads TSS. See comments in early_trap_init(). */ set_intr_gate_ist(X86_TRAP_DB, &debug, DEBUG_STACK); -- 1.8.4 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH] x86, traps: early_trap_init() cleanup. 2015-02-27 3:28 ` [PATCH] x86, traps: early_trap_init() cleanup Wang Nan @ 2015-02-27 10:11 ` Borislav Petkov 0 siblings, 0 replies; 11+ messages in thread From: Borislav Petkov @ 2015-02-27 10:11 UTC (permalink / raw) To: Wang Nan Cc: luto, lizefan, mingo, masami.hiramatsu.pt, linux-kernel, rostedt, linux-tip-commits, tglx, hpa, dave.hansen, oleg On Fri, Feb 27, 2015 at 11:28:50AM +0800, Wang Nan wrote: > As early_trap_init() doesn't use IST, replace set_intr_gate_ist(..., 0) > and set_system_intr_gate_ist(..., 0) with their standard counterparts. > > set_intr_gate() requires a trace_debug symbol which we don't have and > won't use. Use a small macro trick as a workaround. > > Signed-off-by: Wang Nan <wangnan0@huawei.com> > --- > > Hi Andy Lutomirski, > > This patch is a bit tricky, but I think we don't need to define another > helper for such a small problem. What's your opinion? > > Thank you! > > --- > arch/x86/kernel/traps.c | 23 ++++++++++++++--------- > 1 file changed, 14 insertions(+), 9 deletions(-) > > diff --git a/arch/x86/kernel/traps.c b/arch/x86/kernel/traps.c > index 4281988..c24434a 100644 > --- a/arch/x86/kernel/traps.c > +++ b/arch/x86/kernel/traps.c > @@ -925,17 +925,22 @@ 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) > { > + > /* > - * 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 it is impossible to hit an invalid stack. > - * Using the original stack works well enough at this early > - * stage. DEBUG_STACK will be equipped after cpu_init() in > - * trap_init(). > + * Don't use IST to set 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 it is impossible to hit an invalid stack. Using the > + * original stack works well enough at this early stage. DEBUG_STACK > + * will be equipped after cpu_init() in trap_init(). > + * > + * Since set_intr_gate() needs a trace_debug but we don't have it, > + * use the following #define as a workaround. > */ > - set_intr_gate_ist(X86_TRAP_DB, &debug, 0); > +#define trace_debug debug This goes to arch/x86/include/asm/traps.h like the rest of the trace_* defines. > + set_intr_gate(X86_TRAP_DB, debug); > +#undef trace_debug > /* int3 can be called from all */ > - set_system_intr_gate_ist(X86_TRAP_BP, &int3, 0); > + set_system_intr_gate(X86_TRAP_BP, &int3); > #ifdef CONFIG_X86_32 > set_intr_gate(X86_TRAP_PF, page_fault); > #endif -- Regards/Gruss, Boris. ECO tip #101: Trim your mails when you reply. -- ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v2] x86, traps: separate set_intr_gate() and cleanup early_trap_init(). 2015-02-27 2:33 ` Andy Lutomirski 2015-02-27 3:28 ` [PATCH] x86, traps: early_trap_init() cleanup Wang Nan @ 2015-02-27 4:19 ` Wang Nan 2015-03-02 9:55 ` Wang Nan ` (2 more replies) 1 sibling, 3 replies; 11+ messages in thread From: Wang Nan @ 2015-02-27 4:19 UTC (permalink / raw) To: luto, lizefan Cc: mingo, masami.hiramatsu.pt, linux-kernel, rostedt, linux-tip-commits, tglx, hpa, dave.hansen, oleg As early_trap_init() doesn't use IST, replace set_intr_gate_ist() and set_system_intr_gate_ist() with their standard counterparts. set_intr_gate() requires a trace_debug symbol which we don't have and won't use. This patch seprates set_intr_gate() into 2 parts, and uses base version in early_trap_init(). Signed-off-by: Wang Nan <wangnan0@huawei.com> --- Hi Andy Lutomirski, This patch should be less tricky than previous one [1]. I also tried to renaming all set_intr_gate(), but it causes too many code changes, so I think you will be satisfied with this one. Thank you! [1] https://lkml.org/lkml/2015/2/26/770 --- arch/x86/include/asm/desc.h | 7 ++++++- arch/x86/kernel/traps.c | 20 ++++++++++++-------- 2 files changed, 18 insertions(+), 9 deletions(-) diff --git a/arch/x86/include/asm/desc.h b/arch/x86/include/asm/desc.h index a94b82e..a0bf89f 100644 --- a/arch/x86/include/asm/desc.h +++ b/arch/x86/include/asm/desc.h @@ -376,11 +376,16 @@ static inline void _set_gate(int gate, unsigned type, void *addr, * Pentium F0 0F bugfix can have resulted in the mapped * IDT being write-protected. */ -#define set_intr_gate(n, addr) \ +#define set_intr_gate_notrace(n, addr) \ do { \ BUG_ON((unsigned)n > 0xFF); \ _set_gate(n, GATE_INTERRUPT, (void *)addr, 0, 0, \ __KERNEL_CS); \ + } while (0) + +#define set_intr_gate(n, addr) \ + do { \ + set_intr_gate_notrace(n, addr); \ _trace_set_gate(n, GATE_INTERRUPT, (void *)trace_##addr,\ 0, 0, __KERNEL_CS); \ } while (0) diff --git a/arch/x86/kernel/traps.c b/arch/x86/kernel/traps.c index 4281988..9965bd1 100644 --- a/arch/x86/kernel/traps.c +++ b/arch/x86/kernel/traps.c @@ -926,16 +926,20 @@ dotraplinkage void do_iret_error(struct pt_regs *regs, long error_code) void __init early_trap_init(void) { /* - * 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 it is impossible to hit an invalid stack. - * Using the original stack works well enough at this early - * stage. DEBUG_STACK will be equipped after cpu_init() in + * Don't use IST to set 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 it is impossible to hit an invalid + * stack. Using the original stack works well enough at this + * early stage. DEBUG_STACK will be equipped after cpu_init() in * trap_init(). + * + * We don't need to set trace_idt_table like set_intr_gate(), + * since we don't have trace_debug and it will be reset to + * 'debug' in trap_init() by set_intr_gate_ist(). */ - set_intr_gate_ist(X86_TRAP_DB, &debug, 0); + set_intr_gate_notrace(X86_TRAP_DB, debug); /* int3 can be called from all */ - set_system_intr_gate_ist(X86_TRAP_BP, &int3, 0); + set_system_intr_gate(X86_TRAP_BP, &int3); #ifdef CONFIG_X86_32 set_intr_gate(X86_TRAP_PF, page_fault); #endif @@ -1015,7 +1019,7 @@ void __init trap_init(void) /* * X86_TRAP_DB and X86_TRAP_BP have been set - * in early_trap_init(). However, DEBUG_STACK works only after + * in early_trap_init(). However, ITS works only after * cpu_init() loads TSS. See comments in early_trap_init(). */ set_intr_gate_ist(X86_TRAP_DB, &debug, DEBUG_STACK); -- 1.8.4 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH v2] x86, traps: separate set_intr_gate() and cleanup early_trap_init(). 2015-02-27 4:19 ` [PATCH v2] x86, traps: separate set_intr_gate() and cleanup early_trap_init() Wang Nan @ 2015-03-02 9:55 ` Wang Nan 2015-03-02 17:06 ` Andy Lutomirski 2015-03-04 23:51 ` [tip:x86/asm] x86/traps: Separate set_intr_gate() and clean up early_trap_init() tip-bot for Wang Nan 2 siblings, 0 replies; 11+ messages in thread From: Wang Nan @ 2015-03-02 9:55 UTC (permalink / raw) To: luto, lizefan Cc: mingo, masami.hiramatsu.pt, linux-kernel, rostedt, linux-tip-commits, tglx, hpa, dave.hansen, oleg Hi Andy Lutomirski, Do you have any comment on this patch? Thank you. On 2015/2/27 12:19, Wang Nan wrote: > As early_trap_init() doesn't use IST, replace set_intr_gate_ist() and > set_system_intr_gate_ist() with their standard counterparts. > > set_intr_gate() requires a trace_debug symbol which we don't have and > won't use. This patch seprates set_intr_gate() into 2 parts, and uses > base version in early_trap_init(). > > Signed-off-by: Wang Nan <wangnan0@huawei.com> > --- > > Hi Andy Lutomirski, > > This patch should be less tricky than previous one [1]. I also tried > to renaming all set_intr_gate(), but it causes too many code changes, > so I think you will be satisfied with this one. > > Thank you! > > [1] https://lkml.org/lkml/2015/2/26/770 > > --- > arch/x86/include/asm/desc.h | 7 ++++++- > arch/x86/kernel/traps.c | 20 ++++++++++++-------- > 2 files changed, 18 insertions(+), 9 deletions(-) > > diff --git a/arch/x86/include/asm/desc.h b/arch/x86/include/asm/desc.h > index a94b82e..a0bf89f 100644 > --- a/arch/x86/include/asm/desc.h > +++ b/arch/x86/include/asm/desc.h > @@ -376,11 +376,16 @@ static inline void _set_gate(int gate, unsigned type, void *addr, > * Pentium F0 0F bugfix can have resulted in the mapped > * IDT being write-protected. > */ > -#define set_intr_gate(n, addr) \ > +#define set_intr_gate_notrace(n, addr) \ > do { \ > BUG_ON((unsigned)n > 0xFF); \ > _set_gate(n, GATE_INTERRUPT, (void *)addr, 0, 0, \ > __KERNEL_CS); \ > + } while (0) > + > +#define set_intr_gate(n, addr) \ > + do { \ > + set_intr_gate_notrace(n, addr); \ > _trace_set_gate(n, GATE_INTERRUPT, (void *)trace_##addr,\ > 0, 0, __KERNEL_CS); \ > } while (0) > diff --git a/arch/x86/kernel/traps.c b/arch/x86/kernel/traps.c > index 4281988..9965bd1 100644 > --- a/arch/x86/kernel/traps.c > +++ b/arch/x86/kernel/traps.c > @@ -926,16 +926,20 @@ dotraplinkage void do_iret_error(struct pt_regs *regs, long error_code) > void __init early_trap_init(void) > { > /* > - * 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 it is impossible to hit an invalid stack. > - * Using the original stack works well enough at this early > - * stage. DEBUG_STACK will be equipped after cpu_init() in > + * Don't use IST to set 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 it is impossible to hit an invalid > + * stack. Using the original stack works well enough at this > + * early stage. DEBUG_STACK will be equipped after cpu_init() in > * trap_init(). > + * > + * We don't need to set trace_idt_table like set_intr_gate(), > + * since we don't have trace_debug and it will be reset to > + * 'debug' in trap_init() by set_intr_gate_ist(). > */ > - set_intr_gate_ist(X86_TRAP_DB, &debug, 0); > + set_intr_gate_notrace(X86_TRAP_DB, debug); > /* int3 can be called from all */ > - set_system_intr_gate_ist(X86_TRAP_BP, &int3, 0); > + set_system_intr_gate(X86_TRAP_BP, &int3); > #ifdef CONFIG_X86_32 > set_intr_gate(X86_TRAP_PF, page_fault); > #endif > @@ -1015,7 +1019,7 @@ void __init trap_init(void) > > /* > * X86_TRAP_DB and X86_TRAP_BP have been set > - * in early_trap_init(). However, DEBUG_STACK works only after > + * in early_trap_init(). However, ITS works only after > * cpu_init() loads TSS. See comments in early_trap_init(). > */ > set_intr_gate_ist(X86_TRAP_DB, &debug, DEBUG_STACK); > ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2] x86, traps: separate set_intr_gate() and cleanup early_trap_init(). 2015-02-27 4:19 ` [PATCH v2] x86, traps: separate set_intr_gate() and cleanup early_trap_init() Wang Nan 2015-03-02 9:55 ` Wang Nan @ 2015-03-02 17:06 ` Andy Lutomirski 2015-03-04 23:51 ` [tip:x86/asm] x86/traps: Separate set_intr_gate() and clean up early_trap_init() tip-bot for Wang Nan 2 siblings, 0 replies; 11+ messages in thread From: Andy Lutomirski @ 2015-03-02 17:06 UTC (permalink / raw) To: Wang Nan Cc: Li Zefan, Ingo Molnar, Masami Hiramatsu, linux-kernel@vger.kernel.org, Steven Rostedt, linux-tip-commits@vger.kernel.org, Thomas Gleixner, H. Peter Anvin, Dave Hansen, Oleg Nesterov On Thu, Feb 26, 2015 at 8:19 PM, Wang Nan <wangnan0@huawei.com> wrote: > As early_trap_init() doesn't use IST, replace set_intr_gate_ist() and > set_system_intr_gate_ist() with their standard counterparts. > > set_intr_gate() requires a trace_debug symbol which we don't have and > won't use. This patch seprates set_intr_gate() into 2 parts, and uses > base version in early_trap_init(). > > Signed-off-by: Wang Nan <wangnan0@huawei.com> Looks good to me. Ingo? --Andy > --- > > Hi Andy Lutomirski, > > This patch should be less tricky than previous one [1]. I also tried > to renaming all set_intr_gate(), but it causes too many code changes, > so I think you will be satisfied with this one. > > Thank you! > > [1] https://lkml.org/lkml/2015/2/26/770 > > --- > arch/x86/include/asm/desc.h | 7 ++++++- > arch/x86/kernel/traps.c | 20 ++++++++++++-------- > 2 files changed, 18 insertions(+), 9 deletions(-) > > diff --git a/arch/x86/include/asm/desc.h b/arch/x86/include/asm/desc.h > index a94b82e..a0bf89f 100644 > --- a/arch/x86/include/asm/desc.h > +++ b/arch/x86/include/asm/desc.h > @@ -376,11 +376,16 @@ static inline void _set_gate(int gate, unsigned type, void *addr, > * Pentium F0 0F bugfix can have resulted in the mapped > * IDT being write-protected. > */ > -#define set_intr_gate(n, addr) \ > +#define set_intr_gate_notrace(n, addr) \ > do { \ > BUG_ON((unsigned)n > 0xFF); \ > _set_gate(n, GATE_INTERRUPT, (void *)addr, 0, 0, \ > __KERNEL_CS); \ > + } while (0) > + > +#define set_intr_gate(n, addr) \ > + do { \ > + set_intr_gate_notrace(n, addr); \ > _trace_set_gate(n, GATE_INTERRUPT, (void *)trace_##addr,\ > 0, 0, __KERNEL_CS); \ > } while (0) > diff --git a/arch/x86/kernel/traps.c b/arch/x86/kernel/traps.c > index 4281988..9965bd1 100644 > --- a/arch/x86/kernel/traps.c > +++ b/arch/x86/kernel/traps.c > @@ -926,16 +926,20 @@ dotraplinkage void do_iret_error(struct pt_regs *regs, long error_code) > void __init early_trap_init(void) > { > /* > - * 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 it is impossible to hit an invalid stack. > - * Using the original stack works well enough at this early > - * stage. DEBUG_STACK will be equipped after cpu_init() in > + * Don't use IST to set 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 it is impossible to hit an invalid > + * stack. Using the original stack works well enough at this > + * early stage. DEBUG_STACK will be equipped after cpu_init() in > * trap_init(). > + * > + * We don't need to set trace_idt_table like set_intr_gate(), > + * since we don't have trace_debug and it will be reset to > + * 'debug' in trap_init() by set_intr_gate_ist(). > */ > - set_intr_gate_ist(X86_TRAP_DB, &debug, 0); > + set_intr_gate_notrace(X86_TRAP_DB, debug); > /* int3 can be called from all */ > - set_system_intr_gate_ist(X86_TRAP_BP, &int3, 0); > + set_system_intr_gate(X86_TRAP_BP, &int3); > #ifdef CONFIG_X86_32 > set_intr_gate(X86_TRAP_PF, page_fault); > #endif > @@ -1015,7 +1019,7 @@ void __init trap_init(void) > > /* > * X86_TRAP_DB and X86_TRAP_BP have been set > - * in early_trap_init(). However, DEBUG_STACK works only after > + * in early_trap_init(). However, ITS works only after > * cpu_init() loads TSS. See comments in early_trap_init(). > */ > set_intr_gate_ist(X86_TRAP_DB, &debug, DEBUG_STACK); > -- > 1.8.4 > -- Andy Lutomirski AMA Capital Management, LLC ^ permalink raw reply [flat|nested] 11+ messages in thread
* [tip:x86/asm] x86/traps: Separate set_intr_gate() and clean up early_trap_init() 2015-02-27 4:19 ` [PATCH v2] x86, traps: separate set_intr_gate() and cleanup early_trap_init() Wang Nan 2015-03-02 9:55 ` Wang Nan 2015-03-02 17:06 ` Andy Lutomirski @ 2015-03-04 23:51 ` tip-bot for Wang Nan 2 siblings, 0 replies; 11+ messages in thread From: tip-bot for Wang Nan @ 2015-03-04 23:51 UTC (permalink / raw) To: linux-tip-commits Cc: lizefan, masami.hiramatsu.pt, linux-kernel, oleg, luto, hpa, bp, mingo, rostedt, wangnan0, dave.hansen, tglx Commit-ID: 5eca7453d61003bf886992388f8cb407e6f0d051 Gitweb: http://git.kernel.org/tip/5eca7453d61003bf886992388f8cb407e6f0d051 Author: Wang Nan <wangnan0@huawei.com> AuthorDate: Fri, 27 Feb 2015 12:19:49 +0800 Committer: Ingo Molnar <mingo@kernel.org> CommitDate: Thu, 5 Mar 2015 00:47:29 +0100 x86/traps: Separate set_intr_gate() and clean up early_trap_init() As early_trap_init() doesn't use IST, replace set_intr_gate_ist() and set_system_intr_gate_ist() with their standard counterparts. set_intr_gate() requires a trace_debug symbol which we don't have and won't use. This patch separates set_intr_gate() into two parts, and uses base version in early_trap_init(). Reported-by: Andy Lutomirski <luto@amacapital.net> Signed-off-by: Wang Nan <wangnan0@huawei.com> Acked-by: Andy Lutomirski <luto@amacapital.net> Cc: <dave.hansen@linux.intel.com> Cc: <lizefan@huawei.com> Cc: <masami.hiramatsu.pt@hitachi.com> Cc: <oleg@redhat.com> Cc: <rostedt@goodmis.org> Cc: Borislav Petkov <bp@alien8.de> Cc: H. Peter Anvin <hpa@zytor.com> Cc: Thomas Gleixner <tglx@linutronix.de> Link: http://lkml.kernel.org/r/1425010789-13714-1-git-send-email-wangnan0@huawei.com Signed-off-by: Ingo Molnar <mingo@kernel.org> --- arch/x86/include/asm/desc.h | 7 ++++++- arch/x86/kernel/traps.c | 20 ++++++++++++-------- 2 files changed, 18 insertions(+), 9 deletions(-) diff --git a/arch/x86/include/asm/desc.h b/arch/x86/include/asm/desc.h index a94b82e..a0bf89f 100644 --- a/arch/x86/include/asm/desc.h +++ b/arch/x86/include/asm/desc.h @@ -376,11 +376,16 @@ static inline void _set_gate(int gate, unsigned type, void *addr, * Pentium F0 0F bugfix can have resulted in the mapped * IDT being write-protected. */ -#define set_intr_gate(n, addr) \ +#define set_intr_gate_notrace(n, addr) \ do { \ BUG_ON((unsigned)n > 0xFF); \ _set_gate(n, GATE_INTERRUPT, (void *)addr, 0, 0, \ __KERNEL_CS); \ + } while (0) + +#define set_intr_gate(n, addr) \ + do { \ + set_intr_gate_notrace(n, addr); \ _trace_set_gate(n, GATE_INTERRUPT, (void *)trace_##addr,\ 0, 0, __KERNEL_CS); \ } while (0) diff --git a/arch/x86/kernel/traps.c b/arch/x86/kernel/traps.c index 4281988..9965bd1 100644 --- a/arch/x86/kernel/traps.c +++ b/arch/x86/kernel/traps.c @@ -926,16 +926,20 @@ dotraplinkage void do_iret_error(struct pt_regs *regs, long error_code) void __init early_trap_init(void) { /* - * 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 it is impossible to hit an invalid stack. - * Using the original stack works well enough at this early - * stage. DEBUG_STACK will be equipped after cpu_init() in + * Don't use IST to set 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 it is impossible to hit an invalid + * stack. Using the original stack works well enough at this + * early stage. DEBUG_STACK will be equipped after cpu_init() in * trap_init(). + * + * We don't need to set trace_idt_table like set_intr_gate(), + * since we don't have trace_debug and it will be reset to + * 'debug' in trap_init() by set_intr_gate_ist(). */ - set_intr_gate_ist(X86_TRAP_DB, &debug, 0); + set_intr_gate_notrace(X86_TRAP_DB, debug); /* int3 can be called from all */ - set_system_intr_gate_ist(X86_TRAP_BP, &int3, 0); + set_system_intr_gate(X86_TRAP_BP, &int3); #ifdef CONFIG_X86_32 set_intr_gate(X86_TRAP_PF, page_fault); #endif @@ -1015,7 +1019,7 @@ void __init trap_init(void) /* * X86_TRAP_DB and X86_TRAP_BP have been set - * in early_trap_init(). However, DEBUG_STACK works only after + * in early_trap_init(). However, ITS works only after * cpu_init() loads TSS. See comments in early_trap_init(). */ set_intr_gate_ist(X86_TRAP_DB, &debug, DEBUG_STACK); ^ permalink raw reply related [flat|nested] 11+ messages in thread
end of thread, other threads:[~2015-03-04 23:52 UTC | newest] Thread overview: 11+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2015-02-26 5:49 [PATCH v2] x86, traps: Enable DEBUG_STACK after cpu_init() for TRAP_DB/BP Wang Nan 2015-02-26 13:12 ` [tip:x86/asm] x86/traps: " tip-bot for Wang Nan 2015-02-26 15:12 ` Andy Lutomirski 2015-02-27 2:21 ` Wang Nan 2015-02-27 2:33 ` Andy Lutomirski 2015-02-27 3:28 ` [PATCH] x86, traps: early_trap_init() cleanup Wang Nan 2015-02-27 10:11 ` Borislav Petkov 2015-02-27 4:19 ` [PATCH v2] x86, traps: separate set_intr_gate() and cleanup early_trap_init() Wang Nan 2015-03-02 9:55 ` Wang Nan 2015-03-02 17:06 ` Andy Lutomirski 2015-03-04 23:51 ` [tip:x86/asm] x86/traps: Separate set_intr_gate() and clean up early_trap_init() tip-bot for Wang Nan
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.