Re: [PATCH v1 4/4] x86/vmware: Run panic diagnostics before kdump by default

[Stephen Brennan <stephen.s.brennan@oracle.com>]

"Guilherme G. Piccoli" <gpiccoli@igalia.com> writes:
> This Message Is From an External Sender 
> This message came from outside your organization. 
> Report Suspicious 
>  
> Hi Petr, Zack - thanks for CCing me!
> Some comments below:
>
>
> On 18/09/2026 00:23, Zack Rusin wrote:
>>> [...]
>>> Maybe, we should start with something simple, and introduce
>>> one more panic notifier as a start. It might be called either:
>>>
>>>   + "panic_hypervisor_list" because "crash_kexec_post_notifiers = true"
>>>     seems to be primary set on hypervisors.
>>>
>>> But I would rather make it more generic and call it
>>>
>>>   + panic_pre_crash_kexec or panic_pre_kdump because there might be
>>>     more notifiers which are either 100% safe and useful or are worth
>>>     the risk before calling crash dump.
>>>
>>> We could put there x86/vmware notifiers as a start. And we could later
>>> move there other important notifiers.
>>>
>>> How does that sound, please?
>> 
>
> It's a good idea, IMO. We could start with this, Zach commented some
> implementation details below...and after it gets merged, we could move
> other hypervisors that currently set "crash_kexec_post_notifiers" to
> this list and eventually, unexport this symbol. We should avoid having
> code forcing this parameter, as Petr said, many notifiers are executed
> if that is set.
>
> (I'm CCing Stephen Brennan here, I recall he had problems with this
> being auto-set, we talked about that in the panic notifiers big
> discussions in the past heh)

Hi Guilherme, thanks for CCing me! You're right, I'm interested too :D
Your email arrived on the first day of my vacation, so I'm sorry for the
delayed response.

I like the above idea, starting simple with a second list for the
notifiers which are straightforward and have an urgent need to run.

My previous complaints were with code overriding
crash_kexec_post_notifiers, because that really interferes with your
mental model when debugging panic/kdump issues. Having two notifier
lists is a reasonable mental model, or at least more understandable than
silently and unconditionally modifying crash_kexec_post_notifiers.

I like the name "panic_pre_kdump" for my own tastes, though n==1 on
that.

I do think we're going to have trouble articulating exactly what are the
criteria for a "pre_kdump" notifier vs a "panic" notifier? Anyone
submitting a patch believes their notifier is important, so of course
they believe it belongs on the pre_kdump list! How will we draw the line
in the future?

- Is it safety? As in "the notifier doesn't take any locks"? I feel
  this should be part of the criteria. (Though I have never been
  successful in predicting what code would cause a kdump/panic failure)
- Is it urgency? As in "without this, the expected behavior of the
  system during a panic will not occur".
- Is it the fact that the notifier is communicating with the hypervisor?

From my perspective: informing the hypervisor of a panic is both safe
and urgent. If the hypervisor is configured to immediately halt and do
its own panic handling, great.

Providing the last 4KB of logs seems less safe and less urgent (though
obviously still immensely helpful). The kernel provides a huge variety
of consoles through which the hypervisor could have already received
these logs. If the system is configured not to use them... it seems the
tradeoff to not be debuggable has already been made :P
To me, that logic belongs in a standard panic notifier.

Plenty of people will likely disagree with me on that, but that's why it
would be helpful to have clarity on what separates pre_kdump from
standard panic notifiers.

> The only thing I'd like to suggest: I think we should have a parameter
> that disables running this list, which would be the opposite of
> "crash_kexec_post_notifiers".
>
> I would implement it as something like: "postpone_pre_kexec_notifiers"
> or something like that. The parameter would basically "move" this list
> execution to the same time as the current notifiers, gating them to
> "crash_kexec_post_notifiers". This way, we'd allow users to debug kexec
> failures maybe related to the "early" notifiers. WDYT?

Agreed for precisely the reason I said above: it's difficult to predict
which notifier is going to cause a problem. Having an escape hatch to
change this behavior without rebuilding a production kernel is critical,
IMO.

Thanks,
Stephen

>
>> [...]
>> I think that without that default though, x86 oops_end() can enter
>> crash_kexec(regs) before reaching panic(), for example with
>> panic_on_oops=1. To cover that path too, I'd call the chain from
>> __crash_kexec() after the image check and register capture, under the
>> existing kexec lock. A second call in vpanic(), immediately before
>> kmsg_dump_desc(), would cover the fallback path. And I think a
>> set-once guard would prevent duplicate or recursive dispatch.
>> 
>
> Regarding this, 2 things:
>
> a) I think you could change kexec_should_crash() to "return 0" also in
> case the new list is set to run, the same is done currently for
> "crash_kexec_post_notifiers". Makes sense?
>
> b) Well, does this whole panic diag thing you're implementing here aims
> only at x86 guests ? Or would it be possible to run, for example, arm64
> guests? Asking this because in x86 and some other architectures (but not
> arm64[0]), it's possible to override machine_crash_shutdown() handler,
> and run things prior to a kexec. Take a look on how Hyper-V does that on
> arch/x86 - this could be just what you need, except if you plan to have
> it for all architectures heh
>
> Finally, if possible please keep me looped in the following patches, I'm
> very interested on that =)
> Cheers,
>
>
> Guilherme
>
>
> [0]
> https://lore.kernel.org/r/427a8277-49f0-4317-d6c3-4a15d7070e55@igalia.com/