From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrew Cooper Subject: Re: [PATCH Remus v1 5/8] tools/libxc: defer the setting of HVM_PARAM_IDENT_PT Date: Thu, 7 May 2015 16:24:24 +0100 Message-ID: <554B83A8.9060605@citrix.com> References: <1430980646-316-1-git-send-email-yanghy@cn.fujitsu.com> <1430980646-316-6-git-send-email-yanghy@cn.fujitsu.com> <554B3FE5.2060300@citrix.com> <554B6FCD.3030804@cn.fujitsu.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <554B6FCD.3030804@cn.fujitsu.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Hongyang Yang , xen-devel@lists.xen.org Cc: wei.liu2@citrix.com, ian.campbell@citrix.com, wency@cn.fujitsu.com, eddie.dong@intel.com, yunhong.jiang@intel.com, ian.jackson@eu.citrix.com, rshriram@cs.ubc.ca List-Id: xen-devel@lists.xenproject.org On 07/05/15 14:59, Hongyang Yang wrote: > > > On 05/07/2015 06:35 PM, Andrew Cooper wrote: >> On 07/05/15 07:37, Yang Hongyang wrote: >>> Set the hvm param HVM_PARAM_IDENT_PT second time will fail. >>> So defer the setting of HVM_PARAM_IDENT_PT. >>> >>> Signed-off-by: Yang Hongyang >> >> I would argue that this is a Xen bug, not a migration bug. There is >> nothing conceptually wrong with setting this param multiple times. > > I noticed that in legacy migration, this was set multiple times, so > this patch is only a workaround that will avoid the fail in my test. > will try your hypervisor patch tomorrow. Try this version, which has been rebased over Pauls' cleanup in this area which has just been committed to staging. ~Andrew diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c index 371fd33..941e9ea 100644 --- a/xen/arch/x86/hvm/hvm.c +++ b/xen/arch/x86/hvm/hvm.c @@ -5710,12 +5710,13 @@ static int hvmop_set_param( rc = -EINVAL; break; case HVM_PARAM_IDENT_PT: - rc = -EINVAL; - if ( d->arch.hvm_domain.params[a.index] != 0 ) - break; - rc = 0; - if ( !paging_mode_hap(d) ) + d->arch.hvm_domain.params[a.index] = a.value; + /* + * Only actually required for VT-x lacking unrestricted_guest + * capabilities. Short circuit the pause if possible. + */ + if ( !paging_mode_hap(d) || !cpu_has_vmx ) break; /* @@ -5729,7 +5730,6 @@ static int hvmop_set_param( rc = 0; domain_pause(d); - d->arch.hvm_domain.params[a.index] = a.value; for_each_vcpu ( d, v ) paging_update_cr3(v); domain_unpause(d);