All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] x86/idt: load idt early in start_secondary
@ 2017-11-27 14:21 Chunyu Hu
  2017-11-27 23:35 ` [tip:x86/urgent] x86/idt: Load " tip-bot for Chunyu Hu
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Chunyu Hu @ 2017-11-27 14:21 UTC (permalink / raw)
  To: tglx; +Cc: mingo, hpa, peterz, luto, bp, rostedt, linux-kernel

For ap, idt is first loaded in cpu_init() with load_current_idt(),
that is to say, no exception can be handled before there. And then
the idt_table has been completed by the bp.

While there are some WARNs which needs the UD exception handling
in the early boot code might be triggered when something uexpected
happens during boot. In that case, cpu would fail to boot as the
exception can't be handled. A WARNing during boot is not usually
meaning the system could not boot.

One use case is when ftrace=function is setup in kernel cmdline, the
ftrace callback function will be called for every traced function.
And in my case, the first traced function is load_ucode_ap. And there
are WARN()s in function trace callback handling, it failed to reboot
as one of the WARN()s is triggered before load_current_idt() executed.

To make WARN()s can work earlier to ap, we load the idt_table early
in start_secondary, and keep the second time idt load in cpu_init,
as there is a load_ucode_ap() there.

Signed-off-by: Chunyu Hu <chuhu@redhat.com>
---
 arch/x86/kernel/smpboot.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/kernel/smpboot.c b/arch/x86/kernel/smpboot.c
index 3d01df7..05a97d5 100644
--- a/arch/x86/kernel/smpboot.c
+++ b/arch/x86/kernel/smpboot.c
@@ -237,7 +237,7 @@ static void notrace start_secondary(void *unused)
 	load_cr3(swapper_pg_dir);
 	__flush_tlb_all();
 #endif
-
+	load_current_idt();
 	cpu_init();
 	x86_cpuinit.early_percpu_clock_init();
 	preempt_disable();
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [tip:x86/urgent] x86/idt: Load idt early in start_secondary
  2017-11-27 14:21 [PATCH] x86/idt: load idt early in start_secondary Chunyu Hu
@ 2017-11-27 23:35 ` tip-bot for Chunyu Hu
  2017-11-28  3:41 ` [PATCH] x86/idt: load " Andy Lutomirski
  2017-11-28  7:22 ` [tip:x86/urgent] x86/idt: Load " tip-bot for Chunyu Hu
  2 siblings, 0 replies; 8+ messages in thread
From: tip-bot for Chunyu Hu @ 2017-11-27 23:35 UTC (permalink / raw)
  To: linux-tip-commits; +Cc: linux-kernel, tglx, hpa, chuhu, mingo

Commit-ID:  d632044a53571267034f430a5433f048ff8168da
Gitweb:     https://git.kernel.org/tip/d632044a53571267034f430a5433f048ff8168da
Author:     Chunyu Hu <chuhu@redhat.com>
AuthorDate: Mon, 27 Nov 2017 22:21:39 +0800
Committer:  Thomas Gleixner <tglx@linutronix.de>
CommitDate: Tue, 28 Nov 2017 00:28:56 +0100

x86/idt: Load idt early in start_secondary

On a secondary, idt is first loaded in cpu_init() with load_current_idt(),
i.e. no exceptions can be handled before that point.

The conversion of WARN() to use UD requires the IDT being loaded earlier as
any warning between start_secondary() and load_curren_idt() in cpu_init()
will result in an unhandled @UD exception and therefore fail the bringup of
the CPU.

Install the IDT handlers right in start_secondary() before calling cpu_init().

[ tglx: Massaged changelog ]

Signed-off-by: Chunyu Hu <chuhu@redhat.com>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Cc: peterz@infradead.org
Cc: bp@alien8.de
Cc: rostedt@goodmis.org
Cc: luto@kernel.org
Link: https://lkml.kernel.org/r/1511792499-4073-1-git-send-email-chuhu@redhat.com

---
 arch/x86/kernel/smpboot.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/kernel/smpboot.c b/arch/x86/kernel/smpboot.c
index 3d01df7..05a97d5 100644
--- a/arch/x86/kernel/smpboot.c
+++ b/arch/x86/kernel/smpboot.c
@@ -237,7 +237,7 @@ static void notrace start_secondary(void *unused)
 	load_cr3(swapper_pg_dir);
 	__flush_tlb_all();
 #endif
-
+	load_current_idt();
 	cpu_init();
 	x86_cpuinit.early_percpu_clock_init();
 	preempt_disable();

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH] x86/idt: load idt early in start_secondary
  2017-11-27 14:21 [PATCH] x86/idt: load idt early in start_secondary Chunyu Hu
  2017-11-27 23:35 ` [tip:x86/urgent] x86/idt: Load " tip-bot for Chunyu Hu
@ 2017-11-28  3:41 ` Andy Lutomirski
  2017-11-28  4:57   ` Chunyu Hu
  2017-11-28  7:22 ` [tip:x86/urgent] x86/idt: Load " tip-bot for Chunyu Hu
  2 siblings, 1 reply; 8+ messages in thread
From: Andy Lutomirski @ 2017-11-28  3:41 UTC (permalink / raw)
  To: Chunyu Hu
  Cc: Thomas Gleixner, Ingo Molnar, H. Peter Anvin, Peter Zijlstra,
	Andrew Lutomirski, Borislav Petkov, Steven Rostedt, linux-kernel

On Mon, Nov 27, 2017 at 6:21 AM, Chunyu Hu <chuhu@redhat.com> wrote:
> For ap, idt is first loaded in cpu_init() with load_current_idt(),
> that is to say, no exception can be handled before there. And then
> the idt_table has been completed by the bp.
>
> While there are some WARNs which needs the UD exception handling
> in the early boot code might be triggered when something uexpected
> happens during boot. In that case, cpu would fail to boot as the
> exception can't be handled. A WARNing during boot is not usually
> meaning the system could not boot.
>
> One use case is when ftrace=function is setup in kernel cmdline, the
> ftrace callback function will be called for every traced function.
> And in my case, the first traced function is load_ucode_ap. And there
> are WARN()s in function trace callback handling, it failed to reboot
> as one of the WARN()s is triggered before load_current_idt() executed.
>
> To make WARN()s can work earlier to ap, we load the idt_table early
> in start_secondary, and keep the second time idt load in cpu_init,
> as there is a load_ucode_ap() there.

Reviewed-by: Andy Lutomirski <luto@kernel.org>

Except that this is presumably missing a Cc: stable.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] x86/idt: load idt early in start_secondary
  2017-11-28  3:41 ` [PATCH] x86/idt: load " Andy Lutomirski
@ 2017-11-28  4:57   ` Chunyu Hu
  2017-11-28  5:28     ` Andy Lutomirski
  2017-11-28 14:56     ` Steven Rostedt
  0 siblings, 2 replies; 8+ messages in thread
From: Chunyu Hu @ 2017-11-28  4:57 UTC (permalink / raw)
  To: Andy Lutomirski, Thomas Gleixner
  Cc: Ingo Molnar, H. Peter Anvin, Peter Zijlstra, Borislav Petkov,
	Steven Rostedt, linux-kernel



----- Original Message -----
> From: "Andy Lutomirski" <luto@kernel.org>
> To: "Chunyu Hu" <chuhu@redhat.com>
> Cc: "Thomas Gleixner" <tglx@linutronix.de>, "Ingo Molnar" <mingo@kernel.org>, "H. Peter Anvin" <hpa@zytor.com>,
> "Peter Zijlstra" <peterz@infradead.org>, "Andrew Lutomirski" <luto@kernel.org>, "Borislav Petkov" <bp@alien8.de>,
> "Steven Rostedt" <rostedt@goodmis.org>, linux-kernel@vger.kernel.org
> Sent: Tuesday, November 28, 2017 11:41:20 AM
> Subject: Re: [PATCH] x86/idt: load idt early in start_secondary
> 
> On Mon, Nov 27, 2017 at 6:21 AM, Chunyu Hu <chuhu@redhat.com> wrote:
> > For ap, idt is first loaded in cpu_init() with load_current_idt(),
> > that is to say, no exception can be handled before there. And then
> > the idt_table has been completed by the bp.
> >
> > While there are some WARNs which needs the UD exception handling
> > in the early boot code might be triggered when something uexpected
> > happens during boot. In that case, cpu would fail to boot as the
> > exception can't be handled. A WARNing during boot is not usually
> > meaning the system could not boot.
> >
> > One use case is when ftrace=function is setup in kernel cmdline, the
> > ftrace callback function will be called for every traced function.
> > And in my case, the first traced function is load_ucode_ap. And there
> > are WARN()s in function trace callback handling, it failed to reboot
> > as one of the WARN()s is triggered before load_current_idt() executed.
> >
> > To make WARN()s can work earlier to ap, we load the idt_table early
> > in start_secondary, and keep the second time idt load in cpu_init,
> > as there is a load_ucode_ap() there.
> 
> Reviewed-by: Andy Lutomirski <luto@kernel.org>
> 
> Except that this is presumably missing a Cc: stable.

Do I need to resend this origin version and Cc stable?

> 

-- 
Regards,
Chunyu Hu

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] x86/idt: load idt early in start_secondary
  2017-11-28  4:57   ` Chunyu Hu
@ 2017-11-28  5:28     ` Andy Lutomirski
  2017-11-28  7:17       ` Thomas Gleixner
  2017-11-28 14:56     ` Steven Rostedt
  1 sibling, 1 reply; 8+ messages in thread
From: Andy Lutomirski @ 2017-11-28  5:28 UTC (permalink / raw)
  To: Chunyu Hu
  Cc: Andy Lutomirski, Thomas Gleixner, Ingo Molnar, H. Peter Anvin,
	Peter Zijlstra, Borislav Petkov, Steven Rostedt, linux-kernel

On Mon, Nov 27, 2017 at 8:57 PM, Chunyu Hu <chuhu@redhat.com> wrote:
>
>
> ----- Original Message -----
>> From: "Andy Lutomirski" <luto@kernel.org>
>> To: "Chunyu Hu" <chuhu@redhat.com>
>> Cc: "Thomas Gleixner" <tglx@linutronix.de>, "Ingo Molnar" <mingo@kernel.org>, "H. Peter Anvin" <hpa@zytor.com>,
>> "Peter Zijlstra" <peterz@infradead.org>, "Andrew Lutomirski" <luto@kernel.org>, "Borislav Petkov" <bp@alien8.de>,
>> "Steven Rostedt" <rostedt@goodmis.org>, linux-kernel@vger.kernel.org
>> Sent: Tuesday, November 28, 2017 11:41:20 AM
>> Subject: Re: [PATCH] x86/idt: load idt early in start_secondary
>>
>> On Mon, Nov 27, 2017 at 6:21 AM, Chunyu Hu <chuhu@redhat.com> wrote:
>> > For ap, idt is first loaded in cpu_init() with load_current_idt(),
>> > that is to say, no exception can be handled before there. And then
>> > the idt_table has been completed by the bp.
>> >
>> > While there are some WARNs which needs the UD exception handling
>> > in the early boot code might be triggered when something uexpected
>> > happens during boot. In that case, cpu would fail to boot as the
>> > exception can't be handled. A WARNing during boot is not usually
>> > meaning the system could not boot.
>> >
>> > One use case is when ftrace=function is setup in kernel cmdline, the
>> > ftrace callback function will be called for every traced function.
>> > And in my case, the first traced function is load_ucode_ap. And there
>> > are WARN()s in function trace callback handling, it failed to reboot
>> > as one of the WARN()s is triggered before load_current_idt() executed.
>> >
>> > To make WARN()s can work earlier to ap, we load the idt_table early
>> > in start_secondary, and keep the second time idt load in cpu_init,
>> > as there is a load_ucode_ap() there.
>>
>> Reviewed-by: Andy Lutomirski <luto@kernel.org>
>>
>> Except that this is presumably missing a Cc: stable.
>
> Do I need to resend this origin version and Cc stable?

I imagine that Ingo can add it.

>
>>
>
> --
> Regards,
> Chunyu Hu
>

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] x86/idt: load idt early in start_secondary
  2017-11-28  5:28     ` Andy Lutomirski
@ 2017-11-28  7:17       ` Thomas Gleixner
  0 siblings, 0 replies; 8+ messages in thread
From: Thomas Gleixner @ 2017-11-28  7:17 UTC (permalink / raw)
  To: Andy Lutomirski
  Cc: Chunyu Hu, Ingo Molnar, H. Peter Anvin, Peter Zijlstra,
	Borislav Petkov, Steven Rostedt, linux-kernel

On Mon, 27 Nov 2017, Andy Lutomirski wrote:
> On Mon, Nov 27, 2017 at 8:57 PM, Chunyu Hu <chuhu@redhat.com> wrote:
> >> > To make WARN()s can work earlier to ap, we load the idt_table early
> >> > in start_secondary, and keep the second time idt load in cpu_init,
> >> > as there is a load_ucode_ap() there.
> >>
> >> Reviewed-by: Andy Lutomirski <luto@kernel.org>
> >>
> >> Except that this is presumably missing a Cc: stable.
> >
> > Do I need to resend this origin version and Cc stable?
> 
> I imagine that Ingo can add it.

Done.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [tip:x86/urgent] x86/idt: Load idt early in start_secondary
  2017-11-27 14:21 [PATCH] x86/idt: load idt early in start_secondary Chunyu Hu
  2017-11-27 23:35 ` [tip:x86/urgent] x86/idt: Load " tip-bot for Chunyu Hu
  2017-11-28  3:41 ` [PATCH] x86/idt: load " Andy Lutomirski
@ 2017-11-28  7:22 ` tip-bot for Chunyu Hu
  2 siblings, 0 replies; 8+ messages in thread
From: tip-bot for Chunyu Hu @ 2017-11-28  7:22 UTC (permalink / raw)
  To: linux-tip-commits; +Cc: chuhu, tglx, linux-kernel, mingo, hpa

Commit-ID:  55d2d0ad2fb4325f615d1950486fbc5e6fba1769
Gitweb:     https://git.kernel.org/tip/55d2d0ad2fb4325f615d1950486fbc5e6fba1769
Author:     Chunyu Hu <chuhu@redhat.com>
AuthorDate: Mon, 27 Nov 2017 22:21:39 +0800
Committer:  Thomas Gleixner <tglx@linutronix.de>
CommitDate: Tue, 28 Nov 2017 08:15:40 +0100

x86/idt: Load idt early in start_secondary

On a secondary, idt is first loaded in cpu_init() with load_current_idt(),
i.e. no exceptions can be handled before that point.

The conversion of WARN() to use UD requires the IDT being loaded earlier as
any warning between start_secondary() and load_curren_idt() in cpu_init()
will result in an unhandled @UD exception and therefore fail the bringup of
the CPU.

Install the IDT handlers right in start_secondary() before calling cpu_init().

[ tglx: Massaged changelog ]

Fixes: 9a93848fe787 ("x86/debug: Implement __WARN() using UD0")
Signed-off-by: Chunyu Hu <chuhu@redhat.com>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Cc: stable@vger.kernel.org
Cc: peterz@infradead.org
Cc: bp@alien8.de
Cc: rostedt@goodmis.org
Cc: luto@kernel.org
Link: https://lkml.kernel.org/r/1511792499-4073-1-git-send-email-chuhu@redhat.com
---
 arch/x86/kernel/smpboot.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/kernel/smpboot.c b/arch/x86/kernel/smpboot.c
index 3d01df7..05a97d5 100644
--- a/arch/x86/kernel/smpboot.c
+++ b/arch/x86/kernel/smpboot.c
@@ -237,7 +237,7 @@ static void notrace start_secondary(void *unused)
 	load_cr3(swapper_pg_dir);
 	__flush_tlb_all();
 #endif
-
+	load_current_idt();
 	cpu_init();
 	x86_cpuinit.early_percpu_clock_init();
 	preempt_disable();

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH] x86/idt: load idt early in start_secondary
  2017-11-28  4:57   ` Chunyu Hu
  2017-11-28  5:28     ` Andy Lutomirski
@ 2017-11-28 14:56     ` Steven Rostedt
  1 sibling, 0 replies; 8+ messages in thread
From: Steven Rostedt @ 2017-11-28 14:56 UTC (permalink / raw)
  To: Chunyu Hu
  Cc: Andy Lutomirski, Thomas Gleixner, Ingo Molnar, H. Peter Anvin,
	Peter Zijlstra, Borislav Petkov, linux-kernel

On Mon, 27 Nov 2017 23:57:36 -0500 (EST)
Chunyu Hu <chuhu@redhat.com> wrote:

> > Except that this is presumably missing a Cc: stable.  
> 
> Do I need to resend this origin version and Cc stable?

No. Adding a "Cc: stable@vger.kernel.org" to the patch (and really to
the change log of the commit) will flag the stable maintainers to
backport the patch to the stable trees when it gets accepted by Linus.

-- Steve

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2017-11-28 14:56 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-11-27 14:21 [PATCH] x86/idt: load idt early in start_secondary Chunyu Hu
2017-11-27 23:35 ` [tip:x86/urgent] x86/idt: Load " tip-bot for Chunyu Hu
2017-11-28  3:41 ` [PATCH] x86/idt: load " Andy Lutomirski
2017-11-28  4:57   ` Chunyu Hu
2017-11-28  5:28     ` Andy Lutomirski
2017-11-28  7:17       ` Thomas Gleixner
2017-11-28 14:56     ` Steven Rostedt
2017-11-28  7:22 ` [tip:x86/urgent] x86/idt: Load " tip-bot for Chunyu Hu

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.