From mboxrd@z Thu Jan 1 00:00:00 1970 From: Kai Huang Subject: Re: [PATCH 07/10] VMX: handle PML enabling in vmx_vcpu_initialise Date: Mon, 30 Mar 2015 15:03:52 +0800 Message-ID: <5518F558.8030101@linux.intel.com> References: <1427423754-11841-1-git-send-email-kai.huang@linux.intel.com> <1427423754-11841-8-git-send-email-kai.huang@linux.intel.com> <5515C7CF.3070907@citrix.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <5515C7CF.3070907@citrix.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: Andrew Cooper , jbeulich@suse.com, tim@xen.org, kevin.tian@intel.com, yang.z.zhang@intel.com, xen-devel@lists.xen.org List-Id: xen-devel@lists.xenproject.org On 03/28/2015 05:12 AM, Andrew Cooper wrote: > On 27/03/15 02:35, Kai Huang wrote: >> It's possible domain has already been in log-dirty mode when creating vcpu, in >> which case we should enable PML for this vcpu if PML has been enabled for the >> domain. >> >> Signed-off-by: Kai Huang >> --- >> xen/arch/x86/hvm/vmx/vmx.c | 24 ++++++++++++++++++++++++ >> 1 file changed, 24 insertions(+) >> >> diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c >> index 453bcc5..fce3aa2 100644 >> --- a/xen/arch/x86/hvm/vmx/vmx.c >> +++ b/xen/arch/x86/hvm/vmx/vmx.c >> @@ -116,6 +116,30 @@ static int vmx_vcpu_initialise(struct vcpu *v) >> return rc; >> } >> >> + /* >> + * It's rare but still possible that domain has already been in log-dirty >> + * mode when vcpu is being created (commented by Tim), in which case we >> + * should enable PML for this vcpu if PML has been enabled for the domain, >> + * and failure to enable results in failure of creating this vcpu. >> + * >> + * Note even there's no vcpu created for the domain, vmx_domain_enable_pml >> + * will return successful in which case vmx_domain_pml_enabled will also >> + * return true. And even this is the first vcpu to be created with >> + * vmx_domain_pml_enabled being true, failure of enabling PML still results >> + * in failure of creating vcpu, to avoid complicated logic to revert PML >> + * style EPT table to non-PML style EPT table. >> + */ >> + if ( vmx_domain_pml_enabled(v->domain) ) >> + { >> + if ( (rc = vmx_vcpu_enable_pml(v)) != 0 ) > Given the comment here, is the assertion in the top of > vmx_vcpu_enable_pml() liable to trip? Do you mean below assertion at beginning of vmx_vcpu_enable_pml might not work here? ASSERT(!vmx_vcpu_pml_enabled(v)); To me it asserts for this particular vcpu, not the domain, so even in this case the assertion is reasonable and should work fine, shouldn't it? > >> + { >> + dprintk(XENLOG_ERR, "Failed to enable PML for vcpu %d\n", >> + v->vcpu_id); > Please use %pv to identify the domain as well as vcpu. Sure. Thanks, -Kai > > ~Andrew > >> + vmx_destroy_vmcs(v); >> + return rc; >> + } >> + } >> + >> vpmu_initialise(v); >> >> vmx_install_vlapic_mapping(v); > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xen.org > http://lists.xen.org/xen-devel