[RFC,v4] x86/kdump: terminate watchdog NMI interrupt to avoid kdump crashes

  If the cpu panics within the NMI interrupt context, there could be
unhandled NMI interrupts in the background which are blocked by processor
until next IRET instruction executes. Since that, it prevents nested
NMI handler execution.

In case of IRET execution during kdump reboot and no proper NMIs handler
registered at that point (such as during EFI loader), we need to ensure
watchdog no work any more, or kdump would crash later. So call
perf_event_exit_cpu() at the very last moment in the panic shutdown.

!! Here I know it's not allowed to call perf_event_exit_cpu() within nmi
context, because of mutex_lock, smp_call_function and so on.
Is there any experts know about the similar function which allowed to call
within atomic context (Neither x86_pmu_disable() nor x86_pmu_disable_all()
do work after my practice)?

Thank you in advance.

Here provide one of test case to reproduce the concerned issue:
  1. # cat uncorrected
     CPU 1 BANK 4
     STATUS uncorrected 0xc0
     MCGSTATUS  EIPV MCIP
     ADDR 0x1234
     RIP 0xdeadbabe
     RAISINGCPU 0
     MCGCAP SER CMCI TES 0x6
  2. # modprobe mce_inject
  3. # mce-inject uncorrected

Mce-inject would trigger kernel panic under NMI interrupt context. In
addition, we need another NMI interrupt raise (such as from watchdog)
during panic process. Set proper watchdog threshold value and/or add an
artificial delay to make sure watchdog interrupt raise during the panic
procedure and the involved issue would occur.

Fixes: ca0e22d4f011 ("x86/boot/compressed/64: Always switch to own page table")
Signed-off-by: Zeng Heng <zengheng4@huawei.com>
---
  v1: add dummy NMI interrupt handler in EFI loader
  v2: tidy up changelog, add comments (by Ingo Molnar)
  v3: add iret_to_self() to deal with blocked NMIs in advance
  v4: call perf_event_exit_cpu() to terminate watchdog in panic shutdown

 arch/x86/kernel/crash.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

--
2.25.1
  

On Fri, Feb 17, 2023 at 08:06:04PM +0800, Zeng Heng wrote:
> If the cpu panics within the NMI interrupt context, there could be
> unhandled NMI interrupts in the background which are blocked by processor
> until next IRET instruction executes. Since that, it prevents nested
> NMI handler execution.
> 
> In case of IRET execution during kdump reboot and no proper NMIs handler
> registered at that point (such as during EFI loader), we need to ensure
> watchdog no work any more, or kdump would crash later. So call
> perf_event_exit_cpu() at the very last moment in the panic shutdown.
> 
> !! Here I know it's not allowed to call perf_event_exit_cpu() within nmi
> context, because of mutex_lock, smp_call_function and so on.
> Is there any experts know about the similar function which allowed to call
> within atomic context (Neither x86_pmu_disable() nor x86_pmu_disable_all()
> do work after my practice)?
> 
> Thank you in advance.
> 
> Here provide one of test case to reproduce the concerned issue:
>   1. # cat uncorrected
>      CPU 1 BANK 4
>      STATUS uncorrected 0xc0
>      MCGSTATUS  EIPV MCIP
>      ADDR 0x1234
>      RIP 0xdeadbabe
>      RAISINGCPU 0
>      MCGCAP SER CMCI TES 0x6
>   2. # modprobe mce_inject
>   3. # mce-inject uncorrected
> 
> Mce-inject would trigger kernel panic under NMI interrupt context. In
> addition, we need another NMI interrupt raise (such as from watchdog)
> during panic process. Set proper watchdog threshold value and/or add an
> artificial delay to make sure watchdog interrupt raise during the panic
> procedure and the involved issue would occur.
> 
> Fixes: ca0e22d4f011 ("x86/boot/compressed/64: Always switch to own page table")
> Signed-off-by: Zeng Heng <zengheng4@huawei.com>
> ---
>   v1: add dummy NMI interrupt handler in EFI loader
>   v2: tidy up changelog, add comments (by Ingo Molnar)
>   v3: add iret_to_self() to deal with blocked NMIs in advance
>   v4: call perf_event_exit_cpu() to terminate watchdog in panic shutdown
> 
>  arch/x86/kernel/crash.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/arch/x86/kernel/crash.c b/arch/x86/kernel/crash.c
> index 305514431f26..f46df94bbdad 100644
> --- a/arch/x86/kernel/crash.c
> +++ b/arch/x86/kernel/crash.c
> @@ -25,6 +25,7 @@
>  #include <linux/slab.h>
>  #include <linux/vmalloc.h>
>  #include <linux/memblock.h>
> +#include <linux/perf_event.h>
> 
>  #include <asm/processor.h>
>  #include <asm/hardirq.h>
> @@ -170,6 +171,15 @@ void native_machine_crash_shutdown(struct pt_regs *regs)
>  #ifdef CONFIG_HPET_TIMER
>  	hpet_disable();
>  #endif
> +
> +	/*
> +	 * If the cpu panics within the NMI interrupt context,
> +	 * we need to ensure no more NMI interrupts blocked by
> +	 * processor. In case of IRET execution during kdump
> +	 * path and no proper NMIs handler registered at that
> +	 * point, here terminate watchdog in panic shutdown.
> +	 */
> +	perf_event_exit_cpu(smp_processor_id());

This kills all of perf, including but not limited to the hardware
watchdog. However, it does nothing to external NMI sources like the NMI
button found on some HP machines.

Still I suppose it is sufficient for the normal case.

>  	crash_save_cpu(regs, safe_smp_processor_id());
>  }
> 
> --
> 2.25.1
>
  

Peter Zijlstra <peterz@infradead.org> writes:

> On Fri, Feb 17, 2023 at 08:06:04PM +0800, Zeng Heng wrote:
>> If the cpu panics within the NMI interrupt context, there could be
>> unhandled NMI interrupts in the background which are blocked by processor
>> until next IRET instruction executes. Since that, it prevents nested
>> NMI handler execution.
>> 
>> In case of IRET execution during kdump reboot and no proper NMIs handler
>> registered at that point (such as during EFI loader)

EFI loader?  kexec on panic is supposed to be kernel to kernel.
If someone is getting EFI involved that is a bug.

>>, we need to ensure
>> watchdog no work any more, or kdump would crash later. So call
>> perf_event_exit_cpu() at the very last moment in the panic shutdown.

Why can't the crash recovery kernel handle this?

Sometimes we very much do have cases where the crash recovery kernel
can not handle it and we can in the dying kernel.  But every line
of code that is added to the code path the crashing kernel takes
increases the probability that something will go wrong and a crash
will not be captured.

>> !! Here I know it's not allowed to call perf_event_exit_cpu() within nmi
>> context, because of mutex_lock, smp_call_function and so on.
>> Is there any experts know about the similar function which allowed to call
>> within atomic context (Neither x86_pmu_disable() nor x86_pmu_disable_all()
>> do work after my practice)?
>> 
>> Thank you in advance.
>> 
>> Here provide one of test case to reproduce the concerned issue:
>>   1. # cat uncorrected
>>      CPU 1 BANK 4
>>      STATUS uncorrected 0xc0
>>      MCGSTATUS  EIPV MCIP
>>      ADDR 0x1234
>>      RIP 0xdeadbabe
>>      RAISINGCPU 0
>>      MCGCAP SER CMCI TES 0x6
>>   2. # modprobe mce_inject
>>   3. # mce-inject uncorrected
>> 
>> Mce-inject would trigger kernel panic under NMI interrupt context. In
>> addition, we need another NMI interrupt raise (such as from watchdog)
>> during panic process. Set proper watchdog threshold value and/or add an
>> artificial delay to make sure watchdog interrupt raise during the panic
>> procedure and the involved issue would occur.
>> 
>> Fixes: ca0e22d4f011 ("x86/boot/compressed/64: Always switch to own page table")
>> Signed-off-by: Zeng Heng <zengheng4@huawei.com>
>> ---
>>   v1: add dummy NMI interrupt handler in EFI loader
>>   v2: tidy up changelog, add comments (by Ingo Molnar)
>>   v3: add iret_to_self() to deal with blocked NMIs in advance
>>   v4: call perf_event_exit_cpu() to terminate watchdog in panic shutdown
>> 
>>  arch/x86/kernel/crash.c | 10 ++++++++++
>>  1 file changed, 10 insertions(+)
>> 
>> diff --git a/arch/x86/kernel/crash.c b/arch/x86/kernel/crash.c
>> index 305514431f26..f46df94bbdad 100644
>> --- a/arch/x86/kernel/crash.c
>> +++ b/arch/x86/kernel/crash.c
>> @@ -25,6 +25,7 @@
>>  #include <linux/slab.h>
>>  #include <linux/vmalloc.h>
>>  #include <linux/memblock.h>
>> +#include <linux/perf_event.h>
>> 
>>  #include <asm/processor.h>
>>  #include <asm/hardirq.h>
>> @@ -170,6 +171,15 @@ void native_machine_crash_shutdown(struct pt_regs *regs)
>>  #ifdef CONFIG_HPET_TIMER
>>  	hpet_disable();
>>  #endif
>> +
>> +	/*
>> +	 * If the cpu panics within the NMI interrupt context,
>> +	 * we need to ensure no more NMI interrupts blocked by
>> +	 * processor. In case of IRET execution during kdump
>> +	 * path and no proper NMIs handler registered at that
>> +	 * point, here terminate watchdog in panic shutdown.
>> +	 */
>> +	perf_event_exit_cpu(smp_processor_id());
>
> This kills all of perf, including but not limited to the hardware
> watchdog. However, it does nothing to external NMI sources like the NMI
> button found on some HP machines.
>
> Still I suppose it is sufficient for the normal case.

Except the architecture appears to be wrong.  I don't see any
explanation and I can't think of one why we don't just leave
NMIs deliberately disabled until the crash recover kernel
figured out how to enable them safely.

Eric


>>  	crash_save_cpu(regs, safe_smp_processor_id());
>>  }
>> 
>> --
>> 2.25.1
>>
  

在 2023/2/23 2:39, Eric W. Biederman 写道:
> Peter Zijlstra <peterz@infradead.org> writes:
>
>> On Fri, Feb 17, 2023 at 08:06:04PM +0800, Zeng Heng wrote:
>>> If the cpu panics within the NMI interrupt context, there could be
>>> unhandled NMI interrupts in the background which are blocked by processor
>>> until next IRET instruction executes. Since that, it prevents nested
>>> NMI handler execution.
>>>
>>> In case of IRET execution during kdump reboot and no proper NMIs handler
>>> registered at that point (such as during EFI loader)
> EFI loader?  kexec on panic is supposed to be kernel to kernel.
> If someone is getting EFI involved that is a bug.

In kdump path, kexec would start purgatory to verify the secondary kernel by

sha256. If verify passed, it would turn the control to EFI loader, and 
call the second

kernel to capture the environment as vmcore file.

As the mail said, if panic appears within NMI context, we never exit 
from that until

EFI loader handles page fault exception and executes IRET instruction 
when exit

from PF. At this moment, processor would allow the blocked NMI interrupt 
raise.


>> This kills all of perf, including but not limited to the hardware
>> watchdog. However, it does nothing to external NMI sources like the NMI
>> button found on some HP machines.
>>
>> Still I suppose it is sufficient for the normal case.
> I can't think of one why we don't just leave
> NMIs deliberately disabled

How to just leave NMIs disabled, could you explain it with more details ?

Zeng Heng

> until the crash recover kernel figured out how to enable them safely.
>
  

在 2023/2/23 10:29, Zeng Heng 写道:
>
> 在 2023/2/23 2:39, Eric W. Biederman 写道:
>> Peter Zijlstra <peterz@infradead.org> writes:
>>
>>> On Fri, Feb 17, 2023 at 08:06:04PM +0800, Zeng Heng wrote:
>>>> If the cpu panics within the NMI interrupt context, there could be
>>>> unhandled NMI interrupts in the background which are blocked by 
>>>> processor
>>>> until next IRET instruction executes. Since that, it prevents nested
>>>> NMI handler execution.
>>>>
>>>> In case of IRET execution during kdump reboot and no proper NMIs 
>>>> handler
>>>> registered at that point (such as during EFI loader)
>> EFI loader?  kexec on panic is supposed to be kernel to kernel.
>> If someone is getting EFI involved that is a bug.
>
> In kdump path, kexec would start purgatory to verify the secondary 
> kernel by
>
> sha256. If verify passed, it would turn the control to EFI loader, and 
> call the second
>
> kernel to capture the environment as vmcore file.
>
> As the mail said, if panic appears within NMI context, we never exit 
> from that until
>
> EFI loader handles page fault exception and executes IRET instruction 
> when exit
>
> from PF. At this moment, processor would allow the blocked NMI 
> interrupt raise.
>
>
>>> This kills all of perf, including but not limited to the hardware
>>> watchdog. However, it does nothing to external NMI sources like the NMI
>>> button found on some HP machines.
>>>
>>> Still I suppose it is sufficient for the normal case.
>> I can't think of one why we don't just leave
>> NMIs deliberately disabled
>
Inative_machine_crash_shutdown() has called lapic_shutdown() to disable 
any kind of

irq, but EFI loader assumes there is no any residual NMIs in the background.


Here is the first version for this issue:

https://lore.kernel.org/all/20230110102745.2514694-1-zengheng4@huawei.com/

Zeng Heng

>
>> until the crash recover kernel figured out how to enable them safely.
>>
>