Re: [Qemu-devel] [PATCH 9/9] qemu/kvm: kvm hyper-v based guest crash event handling

[Paolo Bonzini <pbonzini@redhat.com>]

On 30/06/2015 13:33, Denis V. Lunev wrote:
> 
> +static int kvm_arch_handle_hv_crash(CPUState *cs)
> +{
> +    X86CPU *cpu = X86_CPU(cs);
> +    CPUX86State *env = &cpu->env;
> +
> +    /* Mark that Hyper-v guest crash occurred */
> +    env->hv_crash_occurred = 1;

This need not be a hv crash.  You can add crash_occurred to CPUState
directly, and set it in qemu_system_guest_panicked:

	if (current_cpu) {
	    current_cpu->crash_occurred = true;
        }

Then you would add two subsections: one for crash_occurred in exec.c
(attached to vmstate_cpu_common), one for hyperv crash params in
target-i386/machine.c.

This also gives an idea about splitting the patch: first the
introduction of qemu_system_guest_panicked and crash_occurred, second
the Hyper-V specific bits.

> +        if (cpu->hyperv_crash) {
> +            c->edx |= HV_X64_GUEST_CRASH_MSR_AVAILABLE;
> +            has_msr_hv_crash = true;

You can only set this to true if the kernel also supports the MSRs.

> +        }
> +
>          c = &cpuid_data.entries[cpuid_i++];
>          c->function = HYPERV_CPUID_ENLIGHTMENT_INFO;
>          if (cpu->hyperv_relaxed_timing) {
> @@ -761,6 +767,10 @@ void kvm_arch_reset_vcpu(X86CPU *cpu)
>      } else {
>          env->mp_state = KVM_MP_STATE_RUNNABLE;
>      }
> +    if (has_msr_hv_crash) {
> +            env->msr_hv_crash_ctl = HV_X64_MSR_CRASH_CTL_NOTIFY;

The value is always host-defined, so I think it doesn't need a field in
CPUX86State.  On the other hand, this:


+static bool hyperv_crash_enable_needed(void *opaque)
+{
+    X86CPU *cpu = opaque;
+    CPUX86State *env = &cpu->env;
+
+    return (env->msr_hv_crash_ctl & HV_X64_MSR_CRASH_CTL_CONTENTS) ?
+            true : false;
+}
+

can just check if any of the params fields is nonzero.

Thanks,

Paolo

> +            env->hv_crash_occurred = 0;
> +    }