From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrey Smetanin Subject: Re: [PATCH v2 2/5] target-i386/kvm: Hyper-V SynIC MSR's support Date: Wed, 11 Nov 2015 12:25:12 +0300 Message-ID: <56430978.4070506@virtuozzo.com> References: <1447159964-22987-1-git-send-email-asmetanin@virtuozzo.com> <1447159964-22987-3-git-send-email-asmetanin@virtuozzo.com> <5641EDA2.6020707@redhat.com> <564305B4.9060907@virtuozzo.com> <5643079E.8010908@redhat.com> Reply-To: Mime-Version: 1.0 Content-Type: text/plain; charset="windows-1252"; format=flowed Content-Transfer-Encoding: 7bit Cc: "Denis V. Lunev" , Richard Henderson , Eduardo Habkost , =?UTF-8?Q?Andreas_F=c3=a4rber?= , Marcelo Tosatti , Roman Kagan , To: Paolo Bonzini , Return-path: Received: from mx2.parallels.com ([199.115.105.18]:60758 "EHLO mx2.parallels.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751265AbbKKJZ2 (ORCPT ); Wed, 11 Nov 2015 04:25:28 -0500 In-Reply-To: <5643079E.8010908@redhat.com> Sender: kvm-owner@vger.kernel.org List-ID: On 11/11/2015 12:17 PM, Paolo Bonzini wrote: > > > On 11/11/2015 10:09, Andrey Smetanin wrote: >>> >>> I would prefer to put this in kvm_arch_init_vcpu, if possible. >>> >> Ok. I think the kvm_arch_init_vcpu() is called after migration restores >> cpu->env->msr_hv_synic_* values, so unconditional initialization of >> cpu->env->msr_hv_synic_* values can overwrite migrated values. The check >> "if (!env->msr_hv_synic_version) {" is neccessary for first time >> initialization to protect against such overwriting. This is why this >> code migrates 'msr_hv_synic_version' value. > > No, kvm_arch_init_vcpu is called at the very beginning, when the VCPU > thread is created. > > main > -> machine_class->init > -> pc_init1 > -> pc_cpus_init > -> pc_new_cpu > -> cpu_x86_create > -> object_property_set_bool > -> x86_cpu_realizefn > -> qemu_init_vcpu > -> qemu_kvm_start_vcpu > -> qemu_kvm_cpu_thread_fn (in new thread) > -> kvm_init_vcpu > -> kvm_arch_init_vcpu > > This is long before qemu_start_incoming_migration, which is among the > last things done before calling main_loop In this case I'll remove migration of msr_hv_synic_version and make first time initialization inside kvm_arch_init_vcpu() - inside section where SynIC availability cpuid bit is set. Thank you for clarification. > > Paolo > From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:57940) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZwRnq-0000sK-T6 for qemu-devel@nongnu.org; Wed, 11 Nov 2015 04:34:55 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZwRnn-0003bu-U2 for qemu-devel@nongnu.org; Wed, 11 Nov 2015 04:34:54 -0500 Received: from mx2.parallels.com ([199.115.105.18]:53114) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZwRnn-0003bq-Or for qemu-devel@nongnu.org; Wed, 11 Nov 2015 04:34:51 -0500 References: <1447159964-22987-1-git-send-email-asmetanin@virtuozzo.com> <1447159964-22987-3-git-send-email-asmetanin@virtuozzo.com> <5641EDA2.6020707@redhat.com> <564305B4.9060907@virtuozzo.com> <5643079E.8010908@redhat.com> From: Andrey Smetanin Message-ID: <56430978.4070506@virtuozzo.com> Date: Wed, 11 Nov 2015 12:25:12 +0300 MIME-Version: 1.0 In-Reply-To: <5643079E.8010908@redhat.com> Content-Type: text/plain; charset="windows-1252"; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v2 2/5] target-i386/kvm: Hyper-V SynIC MSR's support Reply-To: asmetanin@virtuozzo.com List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paolo Bonzini , qemu-devel@nongnu.org Cc: Eduardo Habkost , kvm@vger.kernel.org, Marcelo Tosatti , Roman Kagan , "Denis V. Lunev" , =?UTF-8?Q?Andreas_F=c3=a4rber?= , Richard Henderson On 11/11/2015 12:17 PM, Paolo Bonzini wrote: > > > On 11/11/2015 10:09, Andrey Smetanin wrote: >>> >>> I would prefer to put this in kvm_arch_init_vcpu, if possible. >>> >> Ok. I think the kvm_arch_init_vcpu() is called after migration restores >> cpu->env->msr_hv_synic_* values, so unconditional initialization of >> cpu->env->msr_hv_synic_* values can overwrite migrated values. The check >> "if (!env->msr_hv_synic_version) {" is neccessary for first time >> initialization to protect against such overwriting. This is why this >> code migrates 'msr_hv_synic_version' value. > > No, kvm_arch_init_vcpu is called at the very beginning, when the VCPU > thread is created. > > main > -> machine_class->init > -> pc_init1 > -> pc_cpus_init > -> pc_new_cpu > -> cpu_x86_create > -> object_property_set_bool > -> x86_cpu_realizefn > -> qemu_init_vcpu > -> qemu_kvm_start_vcpu > -> qemu_kvm_cpu_thread_fn (in new thread) > -> kvm_init_vcpu > -> kvm_arch_init_vcpu > > This is long before qemu_start_incoming_migration, which is among the > last things done before calling main_loop In this case I'll remove migration of msr_hv_synic_version and make first time initialization inside kvm_arch_init_vcpu() - inside section where SynIC availability cpuid bit is set. Thank you for clarification. > > Paolo >