[v4,0/3] xen: add support for automatic debug key actions in case of crash

Message

Jürgen Groß Dec. 14, 2020, 7:56 a.m. UTC

When the host crashes it would sometimes be nice to have additional
debug data available which could be produced via debug keys, but
halting the server for manual intervention might be impossible due to
the need to reboot/kexec rather sooner than later.

Add support for automatic debug key actions in case of crashes which
can be activated via boot- or runtime-parameter.

Changes in V4:
- addressed comments (now patch 3)
- added patches 1 and 2

Some further remarks to the new patches added:

Patch 1 adds Arm support for run_in_exception_handler(). Constructing
the related bug_frame is unfortunately not as easy as on x86.

I have verified that %c in inline assembly isn't supported by gcc 7 for
arm64, so this was the only way I've found to support this feature. In
theory it might be possible to add a variable referencing the called
function and to discard that variable again when linking, but I'd like
to add this more intrusive modification only if really wanted.

There is more potential for unifying struct bug_frame between x86 and
Arm, either by:
- using the Arm layout on x86, too (resulting in a grow of the bugframe
  data for the cases without messages)
- trying to construct the data in C instead of inline assembly, which
  will need to either keep the x86 assembler BUG_FRAME construction, or
  to add a few functions issuing the ASSERT/BUG/WARN which would then
  need to be called from *.S files.

Patch 2 opens up more potential for simplification: in theory there is
no need any more to call any key handler with the regs parameter,
allowing to use the same prototype for all handlers. The downside would
be to have an additional irq frame on the stack for the dump_registers()
and the do_debug_key() handlers. In case this is acceptable I'd be
happy to send a related cleanup patch.

Juergen Gross (3):
  xen/arm: add support for run_in_exception_handler()
  xen: enable keyhandlers to work without register set specified
  xen: add support for automatic debug key actions in case of crash

 docs/misc/xen-command-line.pandoc | 41 +++++++++++++++++
 xen/arch/arm/traps.c              | 10 ++++-
 xen/common/kexec.c                |  8 ++--
 xen/common/keyhandler.c           | 73 +++++++++++++++++++++++++++++--
 xen/common/shutdown.c             |  4 +-
 xen/drivers/char/console.c        |  2 +-
 xen/drivers/char/ns16550.c        |  3 +-
 xen/include/asm-arm/bug.h         | 32 +++++++++-----
 xen/include/xen/kexec.h           | 10 ++++-
 xen/include/xen/keyhandler.h      | 10 +++++
 10 files changed, 169 insertions(+), 24 deletions(-)

Comments

Jan Beulich Dec. 14, 2020, 9:09 a.m. UTC | #1

On 14.12.2020 08:56, Juergen Gross wrote:
> Patch 2 opens up more potential for simplification: in theory there is
> no need any more to call any key handler with the regs parameter,
> allowing to use the same prototype for all handlers. The downside would
> be to have an additional irq frame on the stack for the dump_registers()
> and the do_debug_key() handlers.

This isn't the only downside, is it? We'd then also need to be able
to (sufficiently cleanly) unwind through the new frame to reach the
prior one, in order to avoid logging less reliable information. Plus
decompose the prior frame as well to avoid logging less easy to
consume data.

Jan

Jürgen Groß Dec. 14, 2020, 9:21 a.m. UTC | #2

On 14.12.20 10:09, Jan Beulich wrote:
> On 14.12.2020 08:56, Juergen Gross wrote:
>> Patch 2 opens up more potential for simplification: in theory there is
>> no need any more to call any key handler with the regs parameter,
>> allowing to use the same prototype for all handlers. The downside would
>> be to have an additional irq frame on the stack for the dump_registers()
>> and the do_debug_key() handlers.
> 
> This isn't the only downside, is it? We'd then also need to be able
> to (sufficiently cleanly) unwind through the new frame to reach the
> prior one, in order to avoid logging less reliable information. Plus
> decompose the prior frame as well to avoid logging less easy to
> consume data.

Yes, this was implied by the "additional irq frame on the stack".


Juergen

Jan Beulich Dec. 14, 2020, 9:24 a.m. UTC | #3

On 14.12.2020 10:21, Jürgen Groß wrote:
> On 14.12.20 10:09, Jan Beulich wrote:
>> On 14.12.2020 08:56, Juergen Gross wrote:
>>> Patch 2 opens up more potential for simplification: in theory there is
>>> no need any more to call any key handler with the regs parameter,
>>> allowing to use the same prototype for all handlers. The downside would
>>> be to have an additional irq frame on the stack for the dump_registers()
>>> and the do_debug_key() handlers.
>>
>> This isn't the only downside, is it? We'd then also need to be able
>> to (sufficiently cleanly) unwind through the new frame to reach the
>> prior one, in order to avoid logging less reliable information. Plus
>> decompose the prior frame as well to avoid logging less easy to
>> consume data.
> 
> Yes, this was implied by the "additional irq frame on the stack".

Oh, okay - I read it as just referring to the possible concern of
more of the not overly large stack to get used.

Jan