Skip to content

Commit 65c7b6c

Browse files
Waiman-Longopsiff
authored andcommitted
x86/nmi: Add an emergency handler in nmi_desc & use it in nmi_shootdown_cpus()
[ Upstream commit fe37c69 ] Depending on the type of panics, it was found that the __register_nmi_handler() function can be called in NMI context from nmi_shootdown_cpus() leading to a lockdep splat: WARNING: inconsistent lock state inconsistent {INITIAL USE} -> {IN-NMI} usage. lock(&nmi_desc[0].lock); <Interrupt> lock(&nmi_desc[0].lock); Call Trace: _raw_spin_lock_irqsave __register_nmi_handler nmi_shootdown_cpus kdump_nmi_shootdown_cpus native_machine_crash_shutdown __crash_kexec In this particular case, the following panic message was printed before: Kernel panic - not syncing: Fatal hardware error! This message seemed to be given out from __ghes_panic() running in NMI context. The __register_nmi_handler() function which takes the nmi_desc lock with irq disabled shouldn't be called from NMI context as this can lead to deadlock. The nmi_shootdown_cpus() function can only be invoked once. After the first invocation, all other CPUs should be stuck in the newly added crash_nmi_callback() and cannot respond to a second NMI. Fix it by adding a new emergency NMI handler to the nmi_desc structure and provide a new set_emergency_nmi_handler() helper to set crash_nmi_callback() in any context. The new emergency handler will preempt other handlers in the linked list. That will eliminate the need to take any lock and serve the panic in NMI use case. Signed-off-by: Waiman Long <longman@redhat.com> Signed-off-by: Ingo Molnar <mingo@kernel.org> Acked-by: Rik van Riel <riel@surriel.com> Cc: Thomas Gleixner <tglx@linutronix.de> Link: https://lore.kernel.org/r/20250206191844.131700-1-longman@redhat.com Signed-off-by: Sasha Levin <sashal@kernel.org> (cherry picked from commit de4469a90075b6c2028aabd7d8258d7c7ad20eb1)
1 parent 19d5bd3 commit 65c7b6c

3 files changed

Lines changed: 47 additions & 7 deletions

File tree

arch/x86/include/asm/nmi.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,8 @@ int __register_nmi_handler(unsigned int, struct nmiaction *);
5959

6060
void unregister_nmi_handler(unsigned int, const char *);
6161

62+
void set_emergency_nmi_handler(unsigned int type, nmi_handler_t handler);
63+
6264
void stop_nmi(void);
6365
void restart_nmi(void);
6466
void local_touch_nmi(void);

arch/x86/kernel/nmi.c

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,8 +39,12 @@
3939
#define CREATE_TRACE_POINTS
4040
#include <trace/events/nmi.h>
4141

42+
/*
43+
* An emergency handler can be set in any context including NMI
44+
*/
4245
struct nmi_desc {
4346
raw_spinlock_t lock;
47+
nmi_handler_t emerg_handler;
4448
struct list_head head;
4549
};
4650

@@ -131,9 +135,22 @@ static void nmi_check_duration(struct nmiaction *action, u64 duration)
131135
static int nmi_handle(unsigned int type, struct pt_regs *regs)
132136
{
133137
struct nmi_desc *desc = nmi_to_desc(type);
138+
nmi_handler_t ehandler;
134139
struct nmiaction *a;
135140
int handled=0;
136141

142+
/*
143+
* Call the emergency handler, if set
144+
*
145+
* In the case of crash_nmi_callback() emergency handler, it will
146+
* return in the case of the crashing CPU to enable it to complete
147+
* other necessary crashing actions ASAP. Other handlers in the
148+
* linked list won't need to be run.
149+
*/
150+
ehandler = desc->emerg_handler;
151+
if (ehandler)
152+
return ehandler(type, regs);
153+
137154
rcu_read_lock();
138155

139156
/*
@@ -223,6 +240,31 @@ void unregister_nmi_handler(unsigned int type, const char *name)
223240
}
224241
EXPORT_SYMBOL_GPL(unregister_nmi_handler);
225242

243+
/**
244+
* set_emergency_nmi_handler - Set emergency handler
245+
* @type: NMI type
246+
* @handler: the emergency handler to be stored
247+
*
248+
* Set an emergency NMI handler which, if set, will preempt all the other
249+
* handlers in the linked list. If a NULL handler is passed in, it will clear
250+
* it. It is expected that concurrent calls to this function will not happen
251+
* or the system is screwed beyond repair.
252+
*/
253+
void set_emergency_nmi_handler(unsigned int type, nmi_handler_t handler)
254+
{
255+
struct nmi_desc *desc = nmi_to_desc(type);
256+
257+
if (WARN_ON_ONCE(desc->emerg_handler == handler))
258+
return;
259+
desc->emerg_handler = handler;
260+
261+
/*
262+
* Ensure the emergency handler is visible to other CPUs before
263+
* function return
264+
*/
265+
smp_wmb();
266+
}
267+
226268
static void
227269
pci_serr_error(unsigned char reason, struct pt_regs *regs)
228270
{

arch/x86/kernel/reboot.c

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -908,15 +908,11 @@ void nmi_shootdown_cpus(nmi_shootdown_cb callback)
908908
shootdown_callback = callback;
909909

910910
atomic_set(&waiting_for_crash_ipi, num_online_cpus() - 1);
911-
/* Would it be better to replace the trap vector here? */
912-
if (register_nmi_handler(NMI_LOCAL, crash_nmi_callback,
913-
NMI_FLAG_FIRST, "crash"))
914-
return; /* Return what? */
911+
915912
/*
916-
* Ensure the new callback function is set before sending
917-
* out the NMI
913+
* Set emergency handler to preempt other handlers.
918914
*/
919-
wmb();
915+
set_emergency_nmi_handler(NMI_LOCAL, crash_nmi_callback);
920916

921917
apic_send_IPI_allbutself(NMI_VECTOR);
922918

0 commit comments

Comments
 (0)