Re: [PATCH 2/2] qemu/kvm: kvm guest crash event handling

[Paolo Bonzini <pbonzini@redhat.com>]

On 11/06/2015 15:18, Denis V. Lunev wrote:
> From: Andrey Smetanin <asmetanin@virtuozzo.com>
> 
> KVM Hyper-V based guests can notify hypervisor about
> occurred guest crash. This patch does handling of KVM crash event
> by sending to libvirt guest panic event that allows to gather
> guest crash dump by QEMU/LIBVIRT.
> 
> The idea is to provide functionality equal to pvpanic device without
> QEMU guest agent for Windows.
> 
> The idea is borrowed from Linux HyperV bus driver and validated against
> Windows 2k12.
> 
> Signed-off-by: Andrey Smetanin <asmetanin@virtuozzo.com>
> Signed-off-by: Denis V. Lunev <den@openvz.org>
> CC: Gleb Natapov <gleb@kernel.org>
> CC: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  include/sysemu/sysemu.h        |  2 ++
>  kvm-all.c                      |  8 ++++++++
>  linux-headers/asm-x86/hyperv.h |  2 ++
>  linux-headers/linux/kvm.h      | 11 +++++++++++
>  target-i386/cpu-qom.h          |  1 +
>  target-i386/cpu.c              |  1 +
>  target-i386/kvm.c              |  4 ++++
>  vl.c                           | 31 +++++++++++++++++++++++++++++++
>  8 files changed, 60 insertions(+)
> 
> diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
> index 853d90a..82d3213 100644
> --- a/include/sysemu/sysemu.h
> +++ b/include/sysemu/sysemu.h
> @@ -61,6 +61,8 @@ void qemu_system_shutdown_request(void);
>  void qemu_system_powerdown_request(void);
>  void qemu_register_powerdown_notifier(Notifier *notifier);
>  void qemu_system_debug_request(void);
> +void qemu_system_crash_request(uint64_t p0, uint64_t p1, uint64_t p2,
> +                                uint64_t p3, uint64_t p4);
>  void qemu_system_vmstop_request(RunState reason);
>  void qemu_system_vmstop_request_prepare(void);
>  int qemu_shutdown_requested_get(void);
> diff --git a/kvm-all.c b/kvm-all.c
> index 53e01d4..cee23bc 100644
> --- a/kvm-all.c
> +++ b/kvm-all.c
> @@ -1844,6 +1844,14 @@ int kvm_cpu_exec(CPUState *cpu)
>                  qemu_system_reset_request();
>                  ret = EXCP_INTERRUPT;
>                  break;
> +            case KVM_SYSTEM_EVENT_CRASH:
> +                qemu_system_crash_request(run->system_event.crash.p0,
> +                                            run->system_event.crash.p1,
> +                                            run->system_event.crash.p2,
> +                                            run->system_event.crash.p3,
> +                                            run->system_event.crash.p4);

This needs to be synchronous, so you can do it here:

        qapi_event_send_guest_panicked(GUEST_PANIC_ACTION_PAUSE, &error_abort);
        vm_stop(RUN_STATE_GUEST_PANICKED);

The five values are never read back.  Please include them in the CPU
state and in the migration stream.  Migration to file is commonly used
to gather post mortem dumps, and there are even tools to convert the
migration file format to Windows crash dump format.  The tools could be
improved to find the crash data and populate the appropriate fields in
the dump file's header.

Paolo

> +                ret = 0;
> +                break;
>              default:
>                  DPRINTF("kvm_arch_handle_exit\n");
>                  ret = kvm_arch_handle_exit(cpu, run);
> diff --git a/linux-headers/asm-x86/hyperv.h b/linux-headers/asm-x86/hyperv.h
> index ce6068d..a5df1ab 100644
> --- a/linux-headers/asm-x86/hyperv.h
> +++ b/linux-headers/asm-x86/hyperv.h
> @@ -108,6 +108,8 @@
>  #define HV_X64_HYPERCALL_PARAMS_XMM_AVAILABLE		(1 << 4)
>  /* Support for a virtual guest idle state is available */
>  #define HV_X64_GUEST_IDLE_STATE_AVAILABLE		(1 << 5)
> +/* Guest crash data handler available */
> +#define HV_X64_GUEST_CRASH_MSR_AVAILABLE                (1 << 10)
>  
>  /*
>   * Implementation recommendations. Indicates which behaviors the hypervisor
> diff --git a/linux-headers/linux/kvm.h b/linux-headers/linux/kvm.h
> index fad9e5c..e169602 100644
> --- a/linux-headers/linux/kvm.h
> +++ b/linux-headers/linux/kvm.h
> @@ -317,8 +317,19 @@ struct kvm_run {
>  		struct {
>  #define KVM_SYSTEM_EVENT_SHUTDOWN       1
>  #define KVM_SYSTEM_EVENT_RESET          2
> +#define KVM_SYSTEM_EVENT_CRASH          3
>  			__u32 type;
>  			__u64 flags;
> +                        union {
> +                                struct {
> +                                        /* Guest crash related parameters */
> +                                        __u64 p0;
> +                                        __u64 p1;
> +                                        __u64 p2;
> +                                        __u64 p3;
> +                                        __u64 p4;
> +                                } crash;
> +                        };
>  		} system_event;
>  		/* KVM_EXIT_S390_STSI */
>  		struct {
> diff --git a/target-i386/cpu-qom.h b/target-i386/cpu-qom.h
> index 7a4fddd..c35b624 100644
> --- a/target-i386/cpu-qom.h
> +++ b/target-i386/cpu-qom.h
> @@ -89,6 +89,7 @@ typedef struct X86CPU {
>      bool hyperv_relaxed_timing;
>      int hyperv_spinlock_attempts;
>      bool hyperv_time;
> +    bool hyperv_crash;
>      bool check_cpuid;
>      bool enforce_cpuid;
>      bool expose_kvm;
> diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> index 4e7cdaa..af0552a 100644
> --- a/target-i386/cpu.c
> +++ b/target-i386/cpu.c
> @@ -3117,6 +3117,7 @@ static Property x86_cpu_properties[] = {
>      DEFINE_PROP_BOOL("hv-relaxed", X86CPU, hyperv_relaxed_timing, false),
>      DEFINE_PROP_BOOL("hv-vapic", X86CPU, hyperv_vapic, false),
>      DEFINE_PROP_BOOL("hv-time", X86CPU, hyperv_time, false),
> +    DEFINE_PROP_BOOL("hv-crash", X86CPU, hyperv_crash, false),
>      DEFINE_PROP_BOOL("check", X86CPU, check_cpuid, false),
>      DEFINE_PROP_BOOL("enforce", X86CPU, enforce_cpuid, false),
>      DEFINE_PROP_BOOL("kvm", X86CPU, expose_kvm, true),
> diff --git a/target-i386/kvm.c b/target-i386/kvm.c
> index 5a236e3..b63e4bb 100644
> --- a/target-i386/kvm.c
> +++ b/target-i386/kvm.c
> @@ -516,6 +516,10 @@ int kvm_arch_init_vcpu(CPUState *cs)
>              c->eax |= 0x200;
>              has_msr_hv_tsc = true;
>          }
> +        if (cpu->hyperv_crash) {
> +            c->edx |= HV_X64_GUEST_CRASH_MSR_AVAILABLE;
> +        }
> +
>          c = &cpuid_data.entries[cpuid_i++];
>          c->function = HYPERV_CPUID_ENLIGHTMENT_INFO;
>          if (cpu->hyperv_relaxed_timing) {
> diff --git a/vl.c b/vl.c
> index 66ccd06..76b5484 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -1505,6 +1505,14 @@ static pid_t shutdown_pid;
>  static int powerdown_requested;
>  static int debug_requested;
>  static int suspend_requested;
> +
> +static int crash_requested;
> +static uint64_t crash_p0;
> +static uint64_t crash_p1;
> +static uint64_t crash_p2;
> +static uint64_t crash_p3;
> +static uint64_t crash_p4;
> +
>  static WakeupReason wakeup_reason;
>  static NotifierList powerdown_notifiers =
>      NOTIFIER_LIST_INITIALIZER(powerdown_notifiers);
> @@ -1578,6 +1586,13 @@ static int qemu_debug_requested(void)
>      return r;
>  }
>  
> +static int qemu_crash_requested(void)
> +{
> +    int r = crash_requested;
> +    crash_requested = 0;
> +    return r;
> +}
> +
>  void qemu_register_reset(QEMUResetHandler *func, void *opaque)
>  {
>      QEMUResetEntry *re = g_malloc0(sizeof(QEMUResetEntry));
> @@ -1729,9 +1744,25 @@ void qemu_system_debug_request(void)
>      qemu_notify_event();
>  }
>  
> +void qemu_system_crash_request(uint64_t p0, uint64_t  p1, uint64_t p2,
> +                                uint64_t p3, uint64_t p4)
> +{
> +    crash_p0 = p0;
> +    crash_p1 = p1;
> +    crash_p2 = p2;
> +    crash_p3 = p3;
> +    crash_p4 = p4;
> +    crash_requested = 1;
> +    qemu_notify_event();
> +}
> +
>  static bool main_loop_should_exit(void)
>  {
>      RunState r;
> +    if (qemu_crash_requested()) {
> +        qapi_event_send_guest_panicked(GUEST_PANIC_ACTION_PAUSE, &error_abort);
> +        vm_stop(RUN_STATE_GUEST_PANICKED);
> +    }
>      if (qemu_debug_requested()) {
>          vm_stop(RUN_STATE_DEBUG);
>      }
>