Re: [PATCH v8] x86: mce: kexec: switch MCE handler for kexec/kdump

Ingo Molnar
2015-04-09T08:00:37Z

* Borislav Petkov <bp@alien8.de> wrote:

> Btw, Ingo had some reservations about this. Ingo?

Yeah, so my concerns are the following:

> kexec disables (or "shoots down") all CPUs other than the crashing
> CPU before entering the 2nd kernel. However, MCA is still enabled so
> if an MCE happens and broadcasts to the CPUs after the main thread
> starts the 2nd kernel (which might not initialize its MCE handler
> yet, or might decide not to enable it) the MCE handler runs only on
> the other CPUs (not on the main thread) leading to kernel panic
> during MCE synchronization. The user-visible effect of this bug is a
> kdump failure.

So the thing is, when we boot up the second kernel there will be a
window where the old handler isn't valid (because the new kernel has
its own pagetables, etc.) and the new handler is not installed yet.

If an MCE hits that window, it's bad luck. (unless the bootup sequence
is rearchitected significantly to allow cross-kernel inheritance of
MCE handlers.)

So I think we can ignore _that_ race.

The more significant question is: what happens when an MCE arrives
whiel the kdump is proceeding - as kdumps can take a long time to
finish when there's a lot of RAM.

But ... since the 'shootdown' is analogous to a CPU hotplug CPU-down
sequence, I suppose that the existing MCE code should already properly
handle the case where an MCE arrives on a (supposedly) dead CPU,
right? In that case installing a separate MCE handler looks like the
wrong thing.

> Our standard MCE handler do_machine_check() assumes a bunch of
> things about system's status and it's hard to alter it to cover
> kexec/kdump context, so add another, kdump-specific one and switch
> to it.

So I don't like this principle either: 'our current code is a mess
that might not work, add new one'.

> Note that this problem exists since current MCE handler was
> implemented in 2.6.32, and recently commit 716079f66eac ("mce: Panic
> when a core has reached a timeout") made it more visible by changing
> the default behavior of the synchronization timeout from "ignore" to
> "panic".

Looks like that's the real problem. How about the kdump crash dumper
sets it back to 'ignore' again when we crash, and also double check
how we handle various corner cases?

Thanks,

Ingo