From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:47178) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bDbDU-0001WD-0g for qemu-devel@nongnu.org; Thu, 16 Jun 2016 13:36:33 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bDbDO-0003lJ-Um for qemu-devel@nongnu.org; Thu, 16 Jun 2016 13:36:31 -0400 Received: from mx1.redhat.com ([209.132.183.28]:36851) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bDbDO-0003lE-NY for qemu-devel@nongnu.org; Thu, 16 Jun 2016 13:36:26 -0400 Date: Thu, 16 Jun 2016 14:36:24 -0300 From: Eduardo Habkost Message-ID: <20160616173624.GO18662@thinpad.lan.raisama.net> 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> <20160616105529.dpmjjeqsdnf5cdnm@hz-desktop> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20160616105529.dpmjjeqsdnf5cdnm@hz-desktop> 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: Paolo Bonzini , qemu-devel@nongnu.org, Richard Henderson , "Michael S . Tsirkin" , Marcelo Tosatti , kvm@vger.kernel.org, Boris Petkov , Tony Luck , Andi Kleen , rkrcmar@redhat.com, Ashok Raj On Thu, Jun 16, 2016 at 06:55:29PM +0800, Haozhong Zhang wrote: > On 06/16/16 12:41, Paolo Bonzini wrote: > > > > > > 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. > > > > Eduardo said nice for this part in previous version [1], so we may wait > for his comments? > > [1] http://lists.nongnu.org/archive/html/qemu-devel/2016-06/msg01992.html I agree we don't need this check, but I still believe it is a nice thing to have. In addition to detecting user errors, they don't hurt and are useful for things like "-cpu host", that don't guarantee live-migration compatibility but still allow migration if you ensure host capabilities are the same on both sides. (I was going to suggest enabling lmce automatically on "-cpu host" as a follow-up patch, BTW.) -- Eduardo