From: ebiederm@xmission.com (Eric W. Biederman)
To: "Fernando Luis Vázquez Cao" <fernando@oss.ntt.co.jp>
Cc: Don Zickus <dzickus@redhat.com>,
linux-tip-commits@vger.kernel.org, torvalds@linux-foundation.org,
kexec@lists.infradead.org, linux-kernel@vger.kernel.org,
mingo@redhat.com, tglx@linutronix.de, hpa@zytor.com,
mingo@elte.hu, Yinghai Lu <yinghai@kernel.org>,
akpm@linux-foundation.org, vgoyal@redhat.com
Subject: Re: [PATCH 1/2] boot: ignore early NMIs
Date: Wed, 07 Mar 2012 20:41:05 -0800 [thread overview]
Message-ID: <m1eht31z8e.fsf@fess.ebiederm.org> (raw)
In-Reply-To: <4F573E74.5040504@oss.ntt.co.jp> ("Fernando Luis Vázquez Cao"'s message of "Wed, 07 Mar 2012 19:54:44 +0900")
Fernando Luis Vázquez Cao <fernando@oss.ntt.co.jp> writes:
> Subject: [PATCH] boot: ignore early NMIs
>
> From: Fernando Luis Vazquez Cao <fernando@oss.ntt.co.jp>
>
> NMIs very early in the boot process are rarely critical (usually
> it just means that there was a spurious bit flip somewhere in the
> hardware, or that this is a kdump kernel and we received an NMI
> generated in the previous context), so the current behavior of
> halting the system when one occurs is probably a bit over the top.
>
> This patch changes the early IDT so that NMIs are ignored and the
> kernel can, hopefully, continue executing other code. Harsher
> measures (panic, etc) are defered to the final NMI handler, which
> can actually make an informed decision.
>
> This issue presented itself in our environment as seemingly
> random hangs in kdump.
>
> Signed-off-by: Fernando Luis Vazquez Cao <fernando@oss.ntt.co.jp>
> ---
>
> diff -urNp linux-3.3-rc6-orig/arch/x86/kernel/head64.c linux-3.3-rc6/arch/x86/kernel/head64.c
> --- linux-3.3-rc6-orig/arch/x86/kernel/head64.c 2012-03-07 15:49:01.834241787 +0900
> +++ linux-3.3-rc6/arch/x86/kernel/head64.c 2012-03-07 18:39:03.173732875 +0900
> @@ -71,7 +71,7 @@ void __init x86_64_start_kernel(char * r
> (__START_KERNEL & PGDIR_MASK)));
> BUILD_BUG_ON(__fix_to_virt(__end_of_fixed_addresses) <= MODULES_END);
>
> - /* clear bss before set_intr_gate with early_idt_handler */
> + /* clear bss before set_intr_gate with early_idt_handlers */
> clear_bss();
>
> /* Make NULL pointers segfault */
> @@ -79,13 +79,8 @@ void __init x86_64_start_kernel(char * r
>
> max_pfn_mapped = KERNEL_IMAGE_SIZE >> PAGE_SHIFT;
>
> - for (i = 0; i < NUM_EXCEPTION_VECTORS; i++) {
> -#ifdef CONFIG_EARLY_PRINTK
> + for (i = 0; i < NUM_EXCEPTION_VECTORS; i++)
> set_intr_gate(i, &early_idt_handlers[i]);
> -#else
> - set_intr_gate(i, early_idt_handler);
> -#endif
> - }
> load_idt((const struct desc_ptr *)&idt_descr);
>
> if (console_loglevel == 10)
> diff -urNp linux-3.3-rc6-orig/arch/x86/kernel/head_64.S linux-3.3-rc6/arch/x86/kernel/head_64.S
> --- linux-3.3-rc6-orig/arch/x86/kernel/head_64.S 2012-03-07 15:49:01.838241839 +0900
> +++ linux-3.3-rc6/arch/x86/kernel/head_64.S 2012-03-07 18:41:21.811516876 +0900
> @@ -270,18 +270,29 @@ bad_address:
> jmp bad_address
>
> .section ".init.text","ax"
> -#ifdef CONFIG_EARLY_PRINTK
> .globl early_idt_handlers
> early_idt_handlers:
> - i = 0
> + vector = 0
> .rept NUM_EXCEPTION_VECTORS
> - movl $i, %esi
> - jmp early_idt_handler
> - i = i + 1
> + /*
> + * NMIs (vector 2) this early in the boot process are rarely critical
> + * (usually it just means that there was a spurious bit flip somewhere
> + * in the hardware, or that this is a kdump kernel and we received an
> + * NMI generated in the previous context), so we ignore them here and
> + * try to continue (see early_nmi_handler implementation below).
> + * Harsher measures (panic, etc) are defered to the final NMI handler,
> + * which can actually make an informed decision.
> + */
> + .if vector == 2
> + jmp early_nmi_handler
Is just a jump and not a move followed by a jump still 10 bytes?
I hate to say it but I think this fails miserably for any exception
after a nmi.
I expect the simplest solution is to modify early_idt_handler to test
for vector == 2.
Doing something less brittle than:
> extern const char early_idt_handlers[NUM_EXCEPTION_VECTORS][10];
in segment.h might be a good idea as well.
Eric
> + .else
> + movl $vector, %esi
> + jmp early_exception_handler
> + .endif
> + vector = vector + 1
> .endr
> -#endif
>
> -ENTRY(early_idt_handler)
> +early_exception_handler:
> #ifdef CONFIG_EARLY_PRINTK
> cmpl $2,early_recursion_flag(%rip)
> jz 1f
> @@ -315,6 +326,9 @@ ENTRY(early_idt_handler)
> 1: hlt
> jmp 1b
>
> +early_nmi_handler:
> + iretq
> +
> #ifdef CONFIG_EARLY_PRINTK
> early_recursion_flag:
> .long 0
_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
WARNING: multiple messages have this Message-ID (diff)
From: ebiederm@xmission.com (Eric W. Biederman)
To: "Fernando Luis Vázquez Cao" <fernando@oss.ntt.co.jp>
Cc: Don Zickus <dzickus@redhat.com>,
linux-tip-commits@vger.kernel.org,
Yinghai Lu <yinghai@kernel.org>,
mingo@elte.hu, kexec@lists.infradead.org,
linux-kernel@vger.kernel.org, mingo@redhat.com, hpa@zytor.com,
akpm@linux-foundation.org, torvalds@linux-foundation.org,
tglx@linutronix.de, vgoyal@redhat.com
Subject: Re: [PATCH 1/2] boot: ignore early NMIs
Date: Wed, 07 Mar 2012 20:41:05 -0800 [thread overview]
Message-ID: <m1eht31z8e.fsf@fess.ebiederm.org> (raw)
In-Reply-To: <4F573E74.5040504@oss.ntt.co.jp> ("Fernando Luis Vázquez Cao"'s message of "Wed, 07 Mar 2012 19:54:44 +0900")
Fernando Luis Vázquez Cao <fernando@oss.ntt.co.jp> writes:
> Subject: [PATCH] boot: ignore early NMIs
>
> From: Fernando Luis Vazquez Cao <fernando@oss.ntt.co.jp>
>
> NMIs very early in the boot process are rarely critical (usually
> it just means that there was a spurious bit flip somewhere in the
> hardware, or that this is a kdump kernel and we received an NMI
> generated in the previous context), so the current behavior of
> halting the system when one occurs is probably a bit over the top.
>
> This patch changes the early IDT so that NMIs are ignored and the
> kernel can, hopefully, continue executing other code. Harsher
> measures (panic, etc) are defered to the final NMI handler, which
> can actually make an informed decision.
>
> This issue presented itself in our environment as seemingly
> random hangs in kdump.
>
> Signed-off-by: Fernando Luis Vazquez Cao <fernando@oss.ntt.co.jp>
> ---
>
> diff -urNp linux-3.3-rc6-orig/arch/x86/kernel/head64.c linux-3.3-rc6/arch/x86/kernel/head64.c
> --- linux-3.3-rc6-orig/arch/x86/kernel/head64.c 2012-03-07 15:49:01.834241787 +0900
> +++ linux-3.3-rc6/arch/x86/kernel/head64.c 2012-03-07 18:39:03.173732875 +0900
> @@ -71,7 +71,7 @@ void __init x86_64_start_kernel(char * r
> (__START_KERNEL & PGDIR_MASK)));
> BUILD_BUG_ON(__fix_to_virt(__end_of_fixed_addresses) <= MODULES_END);
>
> - /* clear bss before set_intr_gate with early_idt_handler */
> + /* clear bss before set_intr_gate with early_idt_handlers */
> clear_bss();
>
> /* Make NULL pointers segfault */
> @@ -79,13 +79,8 @@ void __init x86_64_start_kernel(char * r
>
> max_pfn_mapped = KERNEL_IMAGE_SIZE >> PAGE_SHIFT;
>
> - for (i = 0; i < NUM_EXCEPTION_VECTORS; i++) {
> -#ifdef CONFIG_EARLY_PRINTK
> + for (i = 0; i < NUM_EXCEPTION_VECTORS; i++)
> set_intr_gate(i, &early_idt_handlers[i]);
> -#else
> - set_intr_gate(i, early_idt_handler);
> -#endif
> - }
> load_idt((const struct desc_ptr *)&idt_descr);
>
> if (console_loglevel == 10)
> diff -urNp linux-3.3-rc6-orig/arch/x86/kernel/head_64.S linux-3.3-rc6/arch/x86/kernel/head_64.S
> --- linux-3.3-rc6-orig/arch/x86/kernel/head_64.S 2012-03-07 15:49:01.838241839 +0900
> +++ linux-3.3-rc6/arch/x86/kernel/head_64.S 2012-03-07 18:41:21.811516876 +0900
> @@ -270,18 +270,29 @@ bad_address:
> jmp bad_address
>
> .section ".init.text","ax"
> -#ifdef CONFIG_EARLY_PRINTK
> .globl early_idt_handlers
> early_idt_handlers:
> - i = 0
> + vector = 0
> .rept NUM_EXCEPTION_VECTORS
> - movl $i, %esi
> - jmp early_idt_handler
> - i = i + 1
> + /*
> + * NMIs (vector 2) this early in the boot process are rarely critical
> + * (usually it just means that there was a spurious bit flip somewhere
> + * in the hardware, or that this is a kdump kernel and we received an
> + * NMI generated in the previous context), so we ignore them here and
> + * try to continue (see early_nmi_handler implementation below).
> + * Harsher measures (panic, etc) are defered to the final NMI handler,
> + * which can actually make an informed decision.
> + */
> + .if vector == 2
> + jmp early_nmi_handler
Is just a jump and not a move followed by a jump still 10 bytes?
I hate to say it but I think this fails miserably for any exception
after a nmi.
I expect the simplest solution is to modify early_idt_handler to test
for vector == 2.
Doing something less brittle than:
> extern const char early_idt_handlers[NUM_EXCEPTION_VECTORS][10];
in segment.h might be a good idea as well.
Eric
> + .else
> + movl $vector, %esi
> + jmp early_exception_handler
> + .endif
> + vector = vector + 1
> .endr
> -#endif
>
> -ENTRY(early_idt_handler)
> +early_exception_handler:
> #ifdef CONFIG_EARLY_PRINTK
> cmpl $2,early_recursion_flag(%rip)
> jz 1f
> @@ -315,6 +326,9 @@ ENTRY(early_idt_handler)
> 1: hlt
> jmp 1b
>
> +early_nmi_handler:
> + iretq
> +
> #ifdef CONFIG_EARLY_PRINTK
> early_recursion_flag:
> .long 0
next prev parent reply other threads:[~2012-03-08 4:38 UTC|newest]
Thread overview: 127+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-02-11 23:09 [tip:x86/debug] x86/kdump: No need to disable ioapic/ lapic in crash path tip-bot for Don Zickus
2012-02-12 1:04 ` Yinghai Lu
2012-02-12 1:04 ` Yinghai Lu
2012-02-12 3:13 ` Eric W. Biederman
2012-02-12 3:13 ` Eric W. Biederman
2012-02-12 4:17 ` Yinghai Lu
2012-02-12 4:17 ` Yinghai Lu
2012-02-13 12:52 ` Eric W. Biederman
2012-02-13 12:52 ` Eric W. Biederman
2012-02-13 16:51 ` Yinghai Lu
2012-02-13 16:51 ` Yinghai Lu
2012-02-13 18:16 ` Yinghai Lu
2012-02-13 18:16 ` Yinghai Lu
2012-02-16 17:27 ` Don Zickus
2012-02-16 17:27 ` Don Zickus
2012-02-16 21:53 ` Yinghai Lu
2012-02-16 21:53 ` Yinghai Lu
2012-02-16 21:56 ` Don Zickus
2012-02-16 21:56 ` Don Zickus
2012-02-17 3:38 ` Eric W. Biederman
2012-02-17 3:38 ` Eric W. Biederman
2012-02-17 12:41 ` Eric W. Biederman
2012-02-17 12:41 ` Eric W. Biederman
2012-02-17 15:49 ` HATAYAMA Daisuke
2012-02-17 15:49 ` HATAYAMA Daisuke
2012-02-17 20:18 ` Don Zickus
2012-02-17 20:18 ` Don Zickus
2012-02-20 5:17 ` HATAYAMA Daisuke
2012-02-20 5:17 ` HATAYAMA Daisuke
2012-02-20 15:24 ` Don Zickus
2012-02-20 15:24 ` Don Zickus
2012-02-17 19:54 ` Don Zickus
2012-02-17 19:54 ` Don Zickus
2012-02-18 3:21 ` Eric W. Biederman
2012-02-18 3:21 ` Eric W. Biederman
2012-02-20 15:14 ` Don Zickus
2012-02-20 15:14 ` Don Zickus
2012-02-21 8:01 ` Eric W. Biederman
2012-02-21 8:01 ` Eric W. Biederman
2012-02-21 13:59 ` Don Zickus
2012-02-21 13:59 ` Don Zickus
2012-02-29 23:19 ` Eric W. Biederman
2012-02-29 23:19 ` Eric W. Biederman
2012-03-07 10:53 ` Fernando Luis Vázquez Cao
2012-03-07 10:53 ` Fernando Luis Vázquez Cao
2012-03-07 10:54 ` [PATCH 1/2] boot: ignore early NMIs Fernando Luis Vázquez Cao
2012-03-07 10:54 ` Fernando Luis Vázquez Cao
2012-03-07 10:56 ` [PATCH 2/2] boot: add early NMI counter Fernando Luis Vázquez Cao
2012-03-07 10:56 ` Fernando Luis Vázquez Cao
2012-03-08 4:50 ` Eric W. Biederman
2012-03-08 4:50 ` Eric W. Biederman
2012-03-08 6:00 ` Fernando Luis Vázquez Cao
2012-03-08 6:00 ` Fernando Luis Vázquez Cao
2012-03-08 4:41 ` Eric W. Biederman [this message]
2012-03-08 4:41 ` [PATCH 1/2] boot: ignore early NMIs Eric W. Biederman
2012-03-08 5:53 ` Fernando Luis Vázquez Cao
2012-03-08 5:53 ` Fernando Luis Vázquez Cao
2012-03-08 16:35 ` Eric W. Biederman
2012-03-08 16:35 ` Eric W. Biederman
2012-03-09 9:31 ` Fernando Luis Vázquez Cao
2012-03-09 9:31 ` Fernando Luis Vázquez Cao
2012-03-09 9:51 ` [PATCH 1/3] boot: fortify early_idt_handlers definition Fernando Luis Vázquez Cao
2012-03-09 9:51 ` Fernando Luis Vázquez Cao
2012-03-09 9:55 ` [PATCH 2/3] boot: ignore early NMIs Fernando Luis Vázquez Cao
2012-03-09 9:55 ` Fernando Luis Vázquez Cao
2012-03-09 10:01 ` [PATCH 3/3] boot: add early NMI counter Fernando Luis Vázquez Cao
2012-03-09 10:01 ` Fernando Luis Vázquez Cao
2012-03-09 20:52 ` [PATCH 1/2] boot: ignore early NMIs H. Peter Anvin
2012-03-09 20:52 ` H. Peter Anvin
2012-03-12 5:43 ` Fernando Luis Vázquez Cao
2012-03-12 5:43 ` Fernando Luis Vázquez Cao
2012-03-12 5:49 ` H. Peter Anvin
2012-03-12 5:49 ` H. Peter Anvin
2012-03-12 6:14 ` Fernando Luis Vázquez Cao
2012-03-12 6:14 ` Fernando Luis Vázquez Cao
2012-03-12 13:36 ` Vivek Goyal
2012-03-12 13:36 ` Vivek Goyal
2012-03-12 19:02 ` Eric W. Biederman
2012-03-12 19:02 ` Eric W. Biederman
2012-03-12 19:58 ` Vivek Goyal
2012-03-12 19:58 ` Vivek Goyal
2012-03-12 20:02 ` H. Peter Anvin
2012-03-12 20:02 ` H. Peter Anvin
2012-03-12 18:40 ` H. Peter Anvin
2012-03-12 18:40 ` H. Peter Anvin
2012-03-12 20:01 ` Eric W. Biederman
2012-03-12 20:01 ` Eric W. Biederman
2012-03-12 20:04 ` H. Peter Anvin
2012-03-12 20:04 ` H. Peter Anvin
2012-03-12 20:16 ` H. Peter Anvin
2012-03-12 20:16 ` H. Peter Anvin
2012-03-13 2:11 ` Fernando Luis Vázquez Cao
2012-03-13 2:11 ` Fernando Luis Vázquez Cao
2012-03-13 13:33 ` Don Zickus
2012-03-13 13:33 ` Don Zickus
2012-03-15 0:43 ` Simon Horman
2012-03-15 0:43 ` Simon Horman
2012-03-13 1:43 ` Fernando Luis Vázquez Cao
2012-03-13 1:43 ` Fernando Luis Vázquez Cao
2012-03-12 14:41 ` Don Zickus
2012-03-12 14:41 ` Don Zickus
2012-03-07 15:50 ` [tip:x86/debug] x86/kdump: No need to disable ioapic/ lapic in crash path Vivek Goyal
2012-03-07 15:50 ` Vivek Goyal
2012-03-07 18:27 ` Yinghai Lu
2012-03-07 18:27 ` Yinghai Lu
2012-03-08 1:29 ` Fernando Luis Vázquez Cao
2012-03-08 1:29 ` Fernando Luis Vázquez Cao
2012-03-09 0:59 ` HATAYAMA Daisuke
2012-03-09 0:59 ` HATAYAMA Daisuke
2012-03-09 2:48 ` Eric W. Biederman
2012-03-09 2:48 ` Eric W. Biederman
2012-02-12 11:12 ` Ingo Molnar
2012-02-12 11:12 ` Ingo Molnar
2012-02-13 15:28 ` Don Zickus
2012-02-13 15:28 ` Don Zickus
2012-02-13 16:52 ` Yinghai Lu
2012-02-13 16:52 ` Yinghai Lu
2012-02-13 22:12 ` Don Zickus
2012-02-13 22:12 ` Don Zickus
2012-02-13 22:51 ` Don Zickus
2012-02-13 22:51 ` Don Zickus
2012-02-16 2:53 ` Don Zickus
2012-02-16 2:53 ` Don Zickus
2012-02-16 18:43 ` Yinghai Lu
2012-02-16 18:43 ` Yinghai Lu
2012-02-16 21:41 ` Don Zickus
2012-02-16 21:41 ` Don Zickus
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=m1eht31z8e.fsf@fess.ebiederm.org \
--to=ebiederm@xmission.com \
--cc=akpm@linux-foundation.org \
--cc=dzickus@redhat.com \
--cc=fernando@oss.ntt.co.jp \
--cc=hpa@zytor.com \
--cc=kexec@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-tip-commits@vger.kernel.org \
--cc=mingo@elte.hu \
--cc=mingo@redhat.com \
--cc=tglx@linutronix.de \
--cc=torvalds@linux-foundation.org \
--cc=vgoyal@redhat.com \
--cc=yinghai@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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.