From mboxrd@z Thu Jan 1 00:00:00 1970 From: Paolo Bonzini Subject: Re: [PATCH v4 2/3] target-i386: add migration support for Intel LMCE Date: Thu, 16 Jun 2016 12:41:32 +0200 Message-ID: References: <20160616060621.30422-1-haozhong.zhang@intel.com> <20160616060621.30422-3-haozhong.zhang@intel.com> <1d2312d2-4dd3-6a73-d0d7-84b4e8c749e2@redhat.com> <20160616102918.7geiaomeitldj7jy@hz-desktop> Mime-Version: 1.0 Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit To: qemu-devel@nongnu.org, Richard Henderson , Eduardo Habkost , "Michael S . Tsirkin" , Marcelo Tosatti , kvm@vger.kernel.org, Boris Petkov , Tony Luck , Andi Kleen , rkrcmar@redhat.com, Ashok Raj Return-path: Received: from mx1.redhat.com ([209.132.183.28]:55577 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751136AbcFPKlj (ORCPT ); Thu, 16 Jun 2016 06:41:39 -0400 In-Reply-To: <20160616102918.7geiaomeitldj7jy@hz-desktop> Sender: kvm-owner@vger.kernel.org List-ID: On 16/06/2016 12:29, Haozhong Zhang wrote: > On 06/16/16 11:51, Paolo Bonzini wrote: >> >> >> On 16/06/2016 08:06, Haozhong Zhang wrote: >>> Migration is only allowed between VCPUs with the same lmce option. >>> >>> Signed-off-by: Haozhong Zhang >>> --- >>> target-i386/machine.c | 25 +++++++++++++++++++++++++ >>> 1 file changed, 25 insertions(+) >>> >>> diff --git a/target-i386/machine.c b/target-i386/machine.c >>> index cb9adf2..00375a3 100644 >>> --- a/target-i386/machine.c >>> +++ b/target-i386/machine.c >>> @@ -347,6 +347,12 @@ static int cpu_post_load(void *opaque, int version_id) >>> return -EINVAL; >>> } >>> >>> + if (!cpu->enable_lmce && (env->mcg_cap & MCG_LMCE_P)) { >>> + error_report("Config mismatch: VCPU has LMCE enabled, " >>> + "but \"lmce\" option is disabled"); >>> + return -EINVAL; >>> + } >>> + >> >> I think this is unnecessary. Apart from this, the patch is good and can >> be squashed in patch 1 for v5. >> > > Without this check, the migration from LMCE enabled QEMU to LMCE > disabled QEMU will not fail. Is such configuration change considered > be error? If not, I will remove the error report and return, but add a > fix to remove MCG_LMCE_P from env->mcg_cap in this check. It's considered a user error. You can skip the "if" completely. Paolo > Haozhong > >> >>> /* >>> * Real mode guest segments register DPL should be zero. >>> * Older KVM version were setting it wrongly. >>> @@ -896,6 +902,24 @@ static const VMStateDescription vmstate_tsc_khz = { >>> } >>> }; >>> >>> +static bool mcg_ext_ctl_needed(void *opaque) >>> +{ >>> + X86CPU *cpu = opaque; >>> + CPUX86State *env = &cpu->env; >>> + return cpu->enable_lmce && env->mcg_ext_ctl; >>> +} >>> + >>> +static const VMStateDescription vmstate_mcg_ext_ctl = { >>> + .name = "cpu/mcg_ext_ctl", >>> + .version_id = 1, >>> + .minimum_version_id = 1, >>> + .needed = mcg_ext_ctl_needed, >>> + .fields = (VMStateField[]) { >>> + VMSTATE_UINT64(env.mcg_ext_ctl, X86CPU), >>> + VMSTATE_END_OF_LIST() >>> + } >>> +}; >>> + >>> VMStateDescription vmstate_x86_cpu = { >>> .name = "cpu", >>> .version_id = 12, >>> @@ -1022,6 +1046,7 @@ VMStateDescription vmstate_x86_cpu = { >>> #ifdef TARGET_X86_64 >>> &vmstate_pkru, >>> #endif >>> + &vmstate_mcg_ext_ctl, >>> NULL >>> } >>> }; >>> From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:46202) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bDUk4-0007gX-WE for qemu-devel@nongnu.org; Thu, 16 Jun 2016 06:41:45 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bDUk0-0003pO-KF for qemu-devel@nongnu.org; Thu, 16 Jun 2016 06:41:43 -0400 Received: from mx1.redhat.com ([209.132.183.28]:53201) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bDUk0-0003p9-CE for qemu-devel@nongnu.org; Thu, 16 Jun 2016 06:41:40 -0400 References: <20160616060621.30422-1-haozhong.zhang@intel.com> <20160616060621.30422-3-haozhong.zhang@intel.com> <1d2312d2-4dd3-6a73-d0d7-84b4e8c749e2@redhat.com> <20160616102918.7geiaomeitldj7jy@hz-desktop> From: Paolo Bonzini Message-ID: Date: Thu, 16 Jun 2016 12:41:32 +0200 MIME-Version: 1.0 In-Reply-To: <20160616102918.7geiaomeitldj7jy@hz-desktop> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v4 2/3] target-i386: add migration support for Intel LMCE List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: qemu-devel@nongnu.org, Richard Henderson , Eduardo Habkost , "Michael S . Tsirkin" , Marcelo Tosatti , kvm@vger.kernel.org, Boris Petkov , Tony Luck , Andi Kleen , rkrcmar@redhat.com, Ashok Raj On 16/06/2016 12:29, Haozhong Zhang wrote: > On 06/16/16 11:51, Paolo Bonzini wrote: >> >> >> On 16/06/2016 08:06, Haozhong Zhang wrote: >>> Migration is only allowed between VCPUs with the same lmce option. >>> >>> Signed-off-by: Haozhong Zhang >>> --- >>> target-i386/machine.c | 25 +++++++++++++++++++++++++ >>> 1 file changed, 25 insertions(+) >>> >>> diff --git a/target-i386/machine.c b/target-i386/machine.c >>> index cb9adf2..00375a3 100644 >>> --- a/target-i386/machine.c >>> +++ b/target-i386/machine.c >>> @@ -347,6 +347,12 @@ static int cpu_post_load(void *opaque, int version_id) >>> return -EINVAL; >>> } >>> >>> + if (!cpu->enable_lmce && (env->mcg_cap & MCG_LMCE_P)) { >>> + error_report("Config mismatch: VCPU has LMCE enabled, " >>> + "but \"lmce\" option is disabled"); >>> + return -EINVAL; >>> + } >>> + >> >> I think this is unnecessary. Apart from this, the patch is good and can >> be squashed in patch 1 for v5. >> > > Without this check, the migration from LMCE enabled QEMU to LMCE > disabled QEMU will not fail. Is such configuration change considered > be error? If not, I will remove the error report and return, but add a > fix to remove MCG_LMCE_P from env->mcg_cap in this check. It's considered a user error. You can skip the "if" completely. Paolo > Haozhong > >> >>> /* >>> * Real mode guest segments register DPL should be zero. >>> * Older KVM version were setting it wrongly. >>> @@ -896,6 +902,24 @@ static const VMStateDescription vmstate_tsc_khz = { >>> } >>> }; >>> >>> +static bool mcg_ext_ctl_needed(void *opaque) >>> +{ >>> + X86CPU *cpu = opaque; >>> + CPUX86State *env = &cpu->env; >>> + return cpu->enable_lmce && env->mcg_ext_ctl; >>> +} >>> + >>> +static const VMStateDescription vmstate_mcg_ext_ctl = { >>> + .name = "cpu/mcg_ext_ctl", >>> + .version_id = 1, >>> + .minimum_version_id = 1, >>> + .needed = mcg_ext_ctl_needed, >>> + .fields = (VMStateField[]) { >>> + VMSTATE_UINT64(env.mcg_ext_ctl, X86CPU), >>> + VMSTATE_END_OF_LIST() >>> + } >>> +}; >>> + >>> VMStateDescription vmstate_x86_cpu = { >>> .name = "cpu", >>> .version_id = 12, >>> @@ -1022,6 +1046,7 @@ VMStateDescription vmstate_x86_cpu = { >>> #ifdef TARGET_X86_64 >>> &vmstate_pkru, >>> #endif >>> + &vmstate_mcg_ext_ctl, >>> NULL >>> } >>> }; >>>