[Qemu-devel] [PATCH 1/1] kvm: log available guest crash parameters

[Qemu-devel] [PATCH 1/1] kvm: log available guest crash parameters

[Denis V. Lunev]

From: Anton Nefedov <anton.nefedov@virtuozzo.com>

Windows reports BSOD parameters through Hyper-V crash MSRs. This
information is very useful for initial crash analysis and thus
it would be nice to see it in the QEMU log file. There is suitable
log mask for the purpose.

Linux guest does not provide this information, but still it would
be nice to log that we have crashed.

Signed-off-by: Anton Nefedov <anton.nefedov@virtuozzo.com>
Signed-off-by: Denis V. Lunev <den@openvz.org>
CC: Paolo Bonzini <pbonzini@redhat.com>
CC: Marcelo Tosatti <mtosatti@redhat.com>
CC: Richard Henderson <rth@twiddle.net>
CC: Eduardo Habkost <ehabkost@redhat.com>
---
 hw/misc/pvpanic.c    |  1 +
 include/sysemu/kvm.h |  2 ++
 kvm-all.c            |  1 +
 stubs/Makefile.objs  |  1 +
 stubs/kvm-crash.c    |  8 ++++++++
 target/i386/kvm.c    | 51 +++++++++++++++++++++++++++++++++++++++++++++++++++
 6 files changed, 64 insertions(+)
 create mode 100644 stubs/kvm-crash.c

diff --git a/hw/misc/pvpanic.c b/hw/misc/pvpanic.c
index 0ac1e6a..221aa4e 100644
--- a/hw/misc/pvpanic.c
+++ b/hw/misc/pvpanic.c
@@ -42,6 +42,7 @@ static void handle_event(int event)
     }
 
     if (event & PVPANIC_PANICKED) {
+        qemu_log_mask(LOG_GUEST_ERROR, "Guest panicked\n");
         qemu_system_guest_panicked();
         return;
     }
diff --git a/include/sysemu/kvm.h b/include/sysemu/kvm.h
index 3045ee7..70cab9d 100644
--- a/include/sysemu/kvm.h
+++ b/include/sysemu/kvm.h
@@ -527,4 +527,6 @@ int kvm_set_one_reg(CPUState *cs, uint64_t id, void *source);
  */
 int kvm_get_one_reg(CPUState *cs, uint64_t id, void *target);
 int kvm_get_max_memslots(void);
+
+void kvm_arch_log_crash(CPUState *cpu);
 #endif
diff --git a/kvm-all.c b/kvm-all.c
index 330219e..5f83389 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -2000,6 +2000,7 @@ int kvm_cpu_exec(CPUState *cpu)
                 ret = EXCP_INTERRUPT;
                 break;
             case KVM_SYSTEM_EVENT_CRASH:
+                kvm_arch_log_crash(cpu);
                 qemu_mutex_lock_iothread();
                 qemu_system_guest_panicked();
                 qemu_mutex_unlock_iothread();
diff --git a/stubs/Makefile.objs b/stubs/Makefile.objs
index a187295..3b1632a 100644
--- a/stubs/Makefile.objs
+++ b/stubs/Makefile.objs
@@ -35,3 +35,4 @@ stub-obj-y += qmp_pc_dimm_device_list.o
 stub-obj-y += target-monitor-defs.o
 stub-obj-y += target-get-monitor-def.o
 stub-obj-y += pc_madt_cpu_entry.o
+stub-obj-y += kvm-crash.o
diff --git a/stubs/kvm-crash.c b/stubs/kvm-crash.c
new file mode 100644
index 0000000..36ab7f0
--- /dev/null
+++ b/stubs/kvm-crash.c
@@ -0,0 +1,8 @@
+#include "qemu/osdep.h"
+#include "qemu-common.h"
+#include "sysemu/kvm.h"
+
+void kvm_arch_log_crash(CPUState *cs)
+{
+    return;
+}
diff --git a/target/i386/kvm.c b/target/i386/kvm.c
index 3b52821..899a83c 100644
--- a/target/i386/kvm.c
+++ b/target/i386/kvm.c
@@ -2020,6 +2020,36 @@ static int kvm_get_sregs(X86CPU *cpu)
     return 0;
 }
 
+static int kvm_read_msr_hv_crash(X86CPU *cpu, uint64_t *buf)
+{
+    int i, ret;
+    struct {
+        struct kvm_msrs info;
+        struct kvm_msr_entry entries[HV_X64_MSR_CRASH_PARAMS];
+    } msr_data;
+
+    if (!has_msr_hv_crash) {
+        return -ENOSYS;
+    }
+
+    for (i = 0; i < HV_X64_MSR_CRASH_PARAMS; i++) {
+        msr_data.entries[i].index = HV_X64_MSR_CRASH_P0 + i;
+    }
+    msr_data.info.nmsrs = HV_X64_MSR_CRASH_PARAMS;
+
+    ret = kvm_vcpu_ioctl(CPU(cpu), KVM_GET_MSRS, &msr_data);
+    if (ret < 0) {
+        return ret;
+    }
+
+    for (i = 0; i < ret; i++) {
+        const struct kvm_msr_entry *msr = msr_data.entries + i;
+        buf[msr->index - HV_X64_MSR_CRASH_P0] = msr->data;
+    }
+
+    return 0;
+}
+
 static int kvm_get_msrs(X86CPU *cpu)
 {
     CPUX86State *env = &cpu->env;
@@ -2767,6 +2797,27 @@ int kvm_arch_get_registers(CPUState *cs)
     return ret;
 }
 
+void kvm_arch_log_crash(CPUState *cs)
+{
+    uint64_t msrs[HV_X64_MSR_CRASH_PARAMS];
+    int ret;
+    X86CPU *cpu = X86_CPU(cs);
+
+    qemu_log_mask(LOG_GUEST_ERROR, "Guest crashed\n");
+    if (!has_msr_hv_crash) {
+        return;
+    }
+
+    ret = kvm_read_msr_hv_crash(cpu, msrs);
+    if (ret < 0) {
+        qemu_log_mask(LOG_GUEST_ERROR, "Failed to get HV crash parameters\n");
+        return;
+    }
+    qemu_log_mask(LOG_GUEST_ERROR, "HV crash parameters: (%#"PRIx64
+                  " %#"PRIx64" %#"PRIx64" %#"PRIx64" %#"PRIx64")\n",
+                  msrs[0], msrs[1], msrs[2], msrs[3], msrs[4]);
+}
+
 void kvm_arch_pre_run(CPUState *cpu, struct kvm_run *run)
 {
     X86CPU *x86_cpu = X86_CPU(cpu);
-- 
2.7.4

Re: [Qemu-devel] [PATCH 1/1] kvm: log available guest crash parameters

[Paolo Bonzini]

On 31/01/2017 05:07, Denis V. Lunev wrote:
> From: Anton Nefedov <anton.nefedov@virtuozzo.com>
> 
> Windows reports BSOD parameters through Hyper-V crash MSRs. This
> information is very useful for initial crash analysis and thus
> it would be nice to see it in the QEMU log file. There is suitable
> log mask for the purpose.
> 
> Linux guest does not provide this information, but still it would
> be nice to log that we have crashed.

We tell libvirt that the guest crashed through a QMP event, so I think
this should be passed as arguments of the QMP event.  You could add:

- a QAPI type GuestPanicInformation

{'union': 'GuestPanicInformation',
 'data': { 'hyper-v': 'GuestPanicInformationHyperV' } }

{'struct': 'GuestPanicInformationHyperV',
 'data': { 'arg1': 'uint64',
           'arg2': 'uint64',
           'arg3': 'uint64',
           'arg4': 'uint64' } }

- a QOM property on the CPU object, crash-information, whose getter
returns an error unless cpu->crash_occurred = true

- an optional GuestPanicInformation argument to the GUEST_PANICKED
event, which is passed if the QOM property returns a non-error value

Thanks,

Paolo

> Signed-off-by: Anton Nefedov <anton.nefedov@virtuozzo.com>
> Signed-off-by: Denis V. Lunev <den@openvz.org>
> CC: Paolo Bonzini <pbonzini@redhat.com>
> CC: Marcelo Tosatti <mtosatti@redhat.com>
> CC: Richard Henderson <rth@twiddle.net>
> CC: Eduardo Habkost <ehabkost@redhat.com>
> ---
>  hw/misc/pvpanic.c    |  1 +
>  include/sysemu/kvm.h |  2 ++
>  kvm-all.c            |  1 +
>  stubs/Makefile.objs  |  1 +
>  stubs/kvm-crash.c    |  8 ++++++++
>  target/i386/kvm.c    | 51 +++++++++++++++++++++++++++++++++++++++++++++++++++
>  6 files changed, 64 insertions(+)
>  create mode 100644 stubs/kvm-crash.c
> 
> diff --git a/hw/misc/pvpanic.c b/hw/misc/pvpanic.c
> index 0ac1e6a..221aa4e 100644
> --- a/hw/misc/pvpanic.c
> +++ b/hw/misc/pvpanic.c
> @@ -42,6 +42,7 @@ static void handle_event(int event)
>      }
>  
>      if (event & PVPANIC_PANICKED) {
> +        qemu_log_mask(LOG_GUEST_ERROR, "Guest panicked\n");
>          qemu_system_guest_panicked();
>          return;
>      }
> diff --git a/include/sysemu/kvm.h b/include/sysemu/kvm.h
> index 3045ee7..70cab9d 100644
> --- a/include/sysemu/kvm.h
> +++ b/include/sysemu/kvm.h
> @@ -527,4 +527,6 @@ int kvm_set_one_reg(CPUState *cs, uint64_t id, void *source);
>   */
>  int kvm_get_one_reg(CPUState *cs, uint64_t id, void *target);
>  int kvm_get_max_memslots(void);
> +
> +void kvm_arch_log_crash(CPUState *cpu);
>  #endif
> diff --git a/kvm-all.c b/kvm-all.c
> index 330219e..5f83389 100644
> --- a/kvm-all.c
> +++ b/kvm-all.c
> @@ -2000,6 +2000,7 @@ int kvm_cpu_exec(CPUState *cpu)
>                  ret = EXCP_INTERRUPT;
>                  break;
>              case KVM_SYSTEM_EVENT_CRASH:
> +                kvm_arch_log_crash(cpu);
>                  qemu_mutex_lock_iothread();
>                  qemu_system_guest_panicked();
>                  qemu_mutex_unlock_iothread();
> diff --git a/stubs/Makefile.objs b/stubs/Makefile.objs
> index a187295..3b1632a 100644
> --- a/stubs/Makefile.objs
> +++ b/stubs/Makefile.objs
> @@ -35,3 +35,4 @@ stub-obj-y += qmp_pc_dimm_device_list.o
>  stub-obj-y += target-monitor-defs.o
>  stub-obj-y += target-get-monitor-def.o
>  stub-obj-y += pc_madt_cpu_entry.o
> +stub-obj-y += kvm-crash.o
> diff --git a/stubs/kvm-crash.c b/stubs/kvm-crash.c
> new file mode 100644
> index 0000000..36ab7f0
> --- /dev/null
> +++ b/stubs/kvm-crash.c
> @@ -0,0 +1,8 @@
> +#include "qemu/osdep.h"
> +#include "qemu-common.h"
> +#include "sysemu/kvm.h"
> +
> +void kvm_arch_log_crash(CPUState *cs)
> +{
> +    return;
> +}
> diff --git a/target/i386/kvm.c b/target/i386/kvm.c
> index 3b52821..899a83c 100644
> --- a/target/i386/kvm.c
> +++ b/target/i386/kvm.c
> @@ -2020,6 +2020,36 @@ static int kvm_get_sregs(X86CPU *cpu)
>      return 0;
>  }
>  
> +static int kvm_read_msr_hv_crash(X86CPU *cpu, uint64_t *buf)
> +{
> +    int i, ret;
> +    struct {
> +        struct kvm_msrs info;
> +        struct kvm_msr_entry entries[HV_X64_MSR_CRASH_PARAMS];
> +    } msr_data;
> +
> +    if (!has_msr_hv_crash) {
> +        return -ENOSYS;
> +    }
> +
> +    for (i = 0; i < HV_X64_MSR_CRASH_PARAMS; i++) {
> +        msr_data.entries[i].index = HV_X64_MSR_CRASH_P0 + i;
> +    }
> +    msr_data.info.nmsrs = HV_X64_MSR_CRASH_PARAMS;
> +
> +    ret = kvm_vcpu_ioctl(CPU(cpu), KVM_GET_MSRS, &msr_data);
> +    if (ret < 0) {
> +        return ret;
> +    }
> +
> +    for (i = 0; i < ret; i++) {
> +        const struct kvm_msr_entry *msr = msr_data.entries + i;
> +        buf[msr->index - HV_X64_MSR_CRASH_P0] = msr->data;
> +    }
> +
> +    return 0;
> +}
> +
>  static int kvm_get_msrs(X86CPU *cpu)
>  {
>      CPUX86State *env = &cpu->env;
> @@ -2767,6 +2797,27 @@ int kvm_arch_get_registers(CPUState *cs)
>      return ret;
>  }
>  
> +void kvm_arch_log_crash(CPUState *cs)
> +{
> +    uint64_t msrs[HV_X64_MSR_CRASH_PARAMS];
> +    int ret;
> +    X86CPU *cpu = X86_CPU(cs);
> +
> +    qemu_log_mask(LOG_GUEST_ERROR, "Guest crashed\n");
> +    if (!has_msr_hv_crash) {
> +        return;
> +    }
> +
> +    ret = kvm_read_msr_hv_crash(cpu, msrs);
> +    if (ret < 0) {
> +        qemu_log_mask(LOG_GUEST_ERROR, "Failed to get HV crash parameters\n");
> +        return;
> +    }
> +    qemu_log_mask(LOG_GUEST_ERROR, "HV crash parameters: (%#"PRIx64
> +                  " %#"PRIx64" %#"PRIx64" %#"PRIx64" %#"PRIx64")\n",
> +                  msrs[0], msrs[1], msrs[2], msrs[3], msrs[4]);
> +}
> +
>  void kvm_arch_pre_run(CPUState *cpu, struct kvm_run *run)
>  {
>      X86CPU *x86_cpu = X86_CPU(cpu);
> 

Re: [Qemu-devel] [PATCH 1/1] kvm: log available guest crash parameters

[Denis V. Lunev]

On 01/31/2017 06:43 PM, Paolo Bonzini wrote:
>
> On 31/01/2017 05:07, Denis V. Lunev wrote:
>> From: Anton Nefedov <anton.nefedov@virtuozzo.com>
>>
>> Windows reports BSOD parameters through Hyper-V crash MSRs. This
>> information is very useful for initial crash analysis and thus
>> it would be nice to see it in the QEMU log file. There is suitable
>> log mask for the purpose.
>>
>> Linux guest does not provide this information, but still it would
>> be nice to log that we have crashed.
> We tell libvirt that the guest crashed through a QMP event, so I think
> this should be passed as arguments of the QMP event.  You could add:
>
> - a QAPI type GuestPanicInformation
>
> {'union': 'GuestPanicInformation',
>  'data': { 'hyper-v': 'GuestPanicInformationHyperV' } }
>
> {'struct': 'GuestPanicInformationHyperV',
>  'data': { 'arg1': 'uint64',
>            'arg2': 'uint64',
>            'arg3': 'uint64',
>            'arg4': 'uint64' } }
>
> - a QOM property on the CPU object, crash-information, whose getter
> returns an error unless cpu->crash_occurred = true
>
> - an optional GuestPanicInformation argument to the GUEST_PANICKED
> event, which is passed if the QOM property returns a non-error value
I like the idea of the event but also want to have the log message.
LOG_GUEST_ERROR level fits well to the idea at my opinion.

Thank you for the idea :)

Den

Re: [Qemu-devel] [PATCH 1/1] kvm: log available guest crash parameters

[Paolo Bonzini]

On 31/01/2017 10:46, Denis V. Lunev wrote:
> On 01/31/2017 06:43 PM, Paolo Bonzini wrote:
>>
>> On 31/01/2017 05:07, Denis V. Lunev wrote:
>>> From: Anton Nefedov <anton.nefedov@virtuozzo.com>
>>>
>>> Windows reports BSOD parameters through Hyper-V crash MSRs. This
>>> information is very useful for initial crash analysis and thus
>>> it would be nice to see it in the QEMU log file. There is suitable
>>> log mask for the purpose.
>>>
>>> Linux guest does not provide this information, but still it would
>>> be nice to log that we have crashed.
>> We tell libvirt that the guest crashed through a QMP event, so I think
>> this should be passed as arguments of the QMP event.  You could add:
>>
>> - a QAPI type GuestPanicInformation
>>
>> {'union': 'GuestPanicInformation',
>>  'data': { 'hyper-v': 'GuestPanicInformationHyperV' } }
>>
>> {'struct': 'GuestPanicInformationHyperV',
>>  'data': { 'arg1': 'uint64',
>>            'arg2': 'uint64',
>>            'arg3': 'uint64',
>>            'arg4': 'uint64' } }
>>
>> - a QOM property on the CPU object, crash-information, whose getter
>> returns an error unless cpu->crash_occurred = true
>>
>> - an optional GuestPanicInformation argument to the GUEST_PANICKED
>> event, which is passed if the QOM property returns a non-error value
> I like the idea of the event but also want to have the log message.
> LOG_GUEST_ERROR level fits well to the idea at my opinion.

Sure, you can do that anyway.  It's just that a log message would go
unnoticed, while QMP events can be passed up to management.  Seeing
bugcheck information in the GUI can be cool (e.g. to find immediately if
you have an INACCESSIBLE_BOOT_DEVICE bugcheck). :)

Paolo