* [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, Oleg Nesterov, Li Zefan,
Steven Rostedt, wangnan0, Andy Lutomirski
Cc: linux-tip-commits
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, Oleg Nesterov,
Li Zefan, Steven Rostedt
Cc: linux-tip-commits
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, Oleg Nesterov, Li Zefan,
Steven Rostedt, linux-tip-commits
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
* [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] 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
* 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,
Steven Rostedt, linux-tip-commits, 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;
as well as URLs for NNTP newsgroup(s).