On Wed, 2015-07-01 at 17:07 +0200, Paolo Bonzini wrote:
>
> 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:
Kernel just works with that value, kernel doesn't setup it.
The user space is allowed to setup this msr if qemu option "hv-crash" is
on. So the code "env->msr_hv_crash_ctl = HV_X64_MSR_CRASH_CTL_NOTIFY;"
setups msr in user space at cpu reset. When cpu setup it's registers
these msr's values are uploaded into kernel.
Anyway we need a code that initially set up crash ctl msr with value
HV_X64_MSR_CRASH_CTL_NOTIFY. And I think that code should be user space.
Any objections ?
>
>
> +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.
>
If we setup crash ctl msr by user space, we need it to migrate.
> Thanks,
>
> Paolo
>
> > + env->hv_crash_occurred = 0;
> > + }
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
