Skip to content

Commit f5869d6

Browse files
Waiman-Longjfvogel
authored andcommitted
x86/nmi: Add an emergency handler in nmi_desc & use it in nmi_shootdown_cpus()
[ Upstream commit fe37c699ae3eed6e02ee55fbf5cb9ceb7fcfd76c ] 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 <[email protected]> Signed-off-by: Ingo Molnar <[email protected]> Acked-by: Rik van Riel <[email protected]> Cc: Thomas Gleixner <[email protected]> Link: https://lore.kernel.org/r/[email protected] Signed-off-by: Sasha Levin <[email protected]> (cherry picked from commit 7eb29d704d2760454d39c2a6c63460d7ff79892d) Signed-off-by: Jack Vogel <[email protected]>
1 parent bfff524 commit f5869d6

File tree

3 files changed

+47
-7
lines changed

3 files changed

+47
-7
lines changed

arch/x86/include/asm/nmi.h

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

5757
void unregister_nmi_handler(unsigned int, const char *);
5858

59+
void set_emergency_nmi_handler(unsigned int type, nmi_handler_t handler);
60+
5961
void stop_nmi(void);
6062
void restart_nmi(void);
6163
void local_touch_nmi(void);

arch/x86/kernel/nmi.c

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

43+
/*
44+
* An emergency handler can be set in any context including NMI
45+
*/
4346
struct nmi_desc {
4447
raw_spinlock_t lock;
48+
nmi_handler_t emerg_handler;
4549
struct list_head head;
4650
};
4751

@@ -132,9 +136,22 @@ static void nmi_check_duration(struct nmiaction *action, u64 duration)
132136
static int nmi_handle(unsigned int type, struct pt_regs *regs)
133137
{
134138
struct nmi_desc *desc = nmi_to_desc(type);
139+
nmi_handler_t ehandler;
135140
struct nmiaction *a;
136141
int handled=0;
137142

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

140157
/*
@@ -224,6 +241,31 @@ void unregister_nmi_handler(unsigned int type, const char *name)
224241
}
225242
EXPORT_SYMBOL_GPL(unregister_nmi_handler);
226243

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

arch/x86/kernel/reboot.c

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

938938
atomic_set(&waiting_for_crash_ipi, num_online_cpus() - 1);
939-
/* Would it be better to replace the trap vector here? */
940-
if (register_nmi_handler(NMI_LOCAL, crash_nmi_callback,
941-
NMI_FLAG_FIRST, "crash"))
942-
return; /* Return what? */
939+
943940
/*
944-
* Ensure the new callback function is set before sending
945-
* out the NMI
941+
* Set emergency handler to preempt other handlers.
946942
*/
947-
wmb();
943+
set_emergency_nmi_handler(NMI_LOCAL, crash_nmi_callback);
948944

949945
apic_send_IPI_allbutself(NMI_VECTOR);
950946

0 commit comments

Comments
 (0)