* [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 a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox