Re: [PATCH 1/4] x86/kexec: Stop hooking NMIs with trap_nop()

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 24/03/2026 11:52 am, Jan Beulich wrote:
> On 19.03.2026 13:25, Andrew Cooper wrote:
>> When FRED is active, it is not possible to hook NMIs like this.
>>
>> NMI hooking in the crash path has undergone several revisions since its
>> introduction.  Notably since commit e7f147bf4ac7 ("x86/crash: Drop manual
>> hooking of exception_table[]") we use the regular nmi_callback()
>> infrastructure.
>>
>> Instead of asserting that we don't enter do_nmi_crash() on the crashing CPU,
>> tolerate it and return early.  It's a marginally longer codepath but behaves
>> the same and is compatible with FRED.
>>
>> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>
>
>> The other use of hooking the NMI handler like this is in play_dead() and
>> introduced by commit 73cb1383bf8d ("x86/idle: re-arrange dead-idle
>> handling").  It's unsafe, and the commit even mentions so for #MC.
> Why is wiring to trap_nop() unsafe?

I meant unsafe in FRED for the same reason; trap_nop() doesn't make the
NMI path safe.

> For FRED, shouldn't do_nmi() then gain a similar early exit for offlined
> CPUs, replacing the IDT editing?

That's not really good enough in FRED.  For starters it doesn't cover
entry_from_{pv,xen}().

e.g. I've discovered (the hard way) the problems of putting printk()
ahead of NMI dispatch when testing NMIs.

And there's still an open question about #MC in both modes.

>
>> On x86, we simply cannot free the per-cpu block for any CPU that hasn't been
>> put back into the wait-for-SIPI state.
> Please remind me, is there a reason we can't put CPUs we have offlined (not
> parked) into that state?

INIT clears CR4.MCE.  Any multi-target #MC (even non-fully-broadcast
ones) which includes this CPU escalates to SHUTDOWN.

Also if you INIT any CPU the system doesn't like, you get SHUTDOWN too. 
Doing this to CPU0 is guaranteed to SHUTDOWN.

One fun issue I found doing the AMD Entrysign work was that firmware no
longer hands over to the OS with the APs in the Wait-for-SIPI state;
they're typically handed over in Mwait.

~Andrew