From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:46581) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZAePI-000414-Pz for qemu-devel@nongnu.org; Thu, 02 Jul 2015 09:20:04 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZAePC-0005ok-2b for qemu-devel@nongnu.org; Thu, 02 Jul 2015 09:20:00 -0400 Received: from mailhub.sw.ru ([195.214.232.25]:25222 helo=relay.sw.ru) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZAePB-0005jF-Jz for qemu-devel@nongnu.org; Thu, 02 Jul 2015 09:19:54 -0400 Message-ID: <1435843180.21037.13.camel@virtuozzo.com> From: Andrey Smetanin Date: Thu, 02 Jul 2015 16:19:40 +0300 In-Reply-To: <5594023F.70104@redhat.com> References: <1435664007-4965-1-git-send-email-den@openvz.org> <1435664007-4965-10-git-send-email-den@openvz.org> <5594023F.70104@redhat.com> Content-Type: text/plain; charset="UTF-8" Mime-Version: 1.0 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 9/9] qemu/kvm: kvm hyper-v based guest crash event handling Reply-To: asmetanin@virtuozzo.com List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paolo Bonzini Cc: "Denis V. Lunev" , qemu-devel@nongnu.org, kvm@vger.kernel.org, Andreas =?ISO-8859-1?Q?F=E4rber?= 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; > > + }