On Thu, Feb 26, 2015 at 6:21 PM, Wang Nan <wangn...@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 <tip...@zytor.com> >> wrote: >>> Commit-ID: b4d8327024637cb2a1f7910dcb5d0ad7a096f473 >>> Gitweb: >>> http://git.kernel.org/tip/b4d8327024637cb2a1f7910dcb5d0ad7a096f473 >>> Author: Wang Nan <wangn...@huawei.com> >>> AuthorDate: Thu, 26 Feb 2015 13:49:39 +0800 >>> Committer: Ingo Molnar <mi...@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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
