Re: [PATCH] x86: Skip latched NMIs on early boot in kdump

["H. Peter Anvin" <hpa@zytor.com>]

[-- Attachment #1: Type: text/plain, Size: 1519 bytes --]

On 03/07/2014 11:39 AM, Don Zickus wrote:
> A customer generated an external NMI using their iLO to test kdump worked.
> Unfortunately, the machine hung.  Disabling the nmi_watchdog made things work.
> 
> I speculated the external NMI fired, caused the machine to panic (as expected)
> and the perf NMI from the watchdog came in and was latched.  My guess was this
> somehow caused the hang.
> 

... as any other unexpected exception would.

> 
> I also do not fully understand why the latched NMI is not happening immediately
> after the load idt call or why it comes after a page fault (the
> early_make_pgtable).  Further adding to my confusion is why the early printk
> magic didn't dump a stack as I believe I had that setup on my commandline.
> But I figured I would just report what I have observed.
> 

If the kdump is initiated from NMI context, I'm wondering if it might be
possible that we haven't actually executed an IRET until this one
happens, and the IRET re-enables NMI.

> My testing and debugging were based off a 3.10 kernel (RHEL-7) but has included
> Seiji's tracepoint cleanups to arch/x86/kernel/head_64.S|head64.c.  Not much
> has changed upstream here.  Also 3.14-rc4 still has the same hang.
> 
> Signed-off-by: Don Zickus <dzickus@redhat.com>

We really shouldn't be doing the fixup lookup for NMI, either.  Probably
it makes more sense to just IRET on NMI until we have the real interrupt
vectors set up, but it needs to be done a little earlier.

How does this patch work for you?

	-hpa


[-- Attachment #2: diff --]
[-- Type: text/plain, Size: 1343 bytes --]

diff --git a/arch/x86/kernel/head_32.S b/arch/x86/kernel/head_32.S
index 81ba276..d2a2159 100644
--- a/arch/x86/kernel/head_32.S
+++ b/arch/x86/kernel/head_32.S
@@ -544,6 +544,10 @@ ENDPROC(early_idt_handlers)
 	/* This is global to keep gas from relaxing the jumps */
 ENTRY(early_idt_handler)
 	cld
+
+	cmpl $X86_TRAP_NMI,(%esp)
+	je is_nmi		# Ignore NMI
+
 	cmpl $2,%ss:early_recursion_flag
 	je hlt_loop
 	incl %ss:early_recursion_flag
@@ -594,8 +598,9 @@ ex_entry:
 	pop %edx
 	pop %ecx
 	pop %eax
-	addl $8,%esp		/* drop vector number and error code */
 	decl %ss:early_recursion_flag
+is_nmi:
+	addl $8,%esp		/* drop vector number and error code */
 	iret
 ENDPROC(early_idt_handler)
 
diff --git a/arch/x86/kernel/head_64.S b/arch/x86/kernel/head_64.S
index e1aabdb..33f36c7 100644
--- a/arch/x86/kernel/head_64.S
+++ b/arch/x86/kernel/head_64.S
@@ -343,6 +343,9 @@ early_idt_handlers:
 ENTRY(early_idt_handler)
 	cld
 
+	cmpl $X86_TRAP_NMI,(%rsp)
+	je is_nmi		# Ignore NMI
+
 	cmpl $2,early_recursion_flag(%rip)
 	jz  1f
 	incl early_recursion_flag(%rip)
@@ -405,8 +408,9 @@ ENTRY(early_idt_handler)
 	popq %rdx
 	popq %rcx
 	popq %rax
-	addq $16,%rsp		# drop vector number and error code
 	decl early_recursion_flag(%rip)
+is_nmi:
+	addq $16,%rsp		# drop vector number and error code
 	INTERRUPT_RETURN
 ENDPROC(early_idt_handler)