From mboxrd@z Thu Jan 1 00:00:00 1970 From: Kai Huang Subject: Re: [PATCH] x86/EPT: defer enabling of A/D maintenance until PML get enabled Date: Thu, 15 Oct 2015 15:41:14 +0800 Message-ID: <561F589A.5010902@linux.intel.com> References: <56096DE002000078000A63EE@prv-mh.provo.novell.com> <560BCD6B02000078000A6FF0@prv-mh.provo.novell.com> <561DADA1.3050303@linux.intel.com> <561E1B7B.3000900@linux.intel.com> <561E3C0102000078000AACEB@prv-mh.provo.novell.com> <561F4AE5.2080102@linux.intel.com> <561F6DCB02000078000AB338@prv-mh.provo.novell.com> <561F5727.5060802@linux.intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: Received: from mail6.bemta14.messagelabs.com ([193.109.254.103]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1ZmdDb-0003n1-S5 for xen-devel@lists.xenproject.org; Thu, 15 Oct 2015 07:44:55 +0000 In-Reply-To: <561F5727.5060802@linux.intel.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: Jan Beulich , Kai Huang Cc: Andrew Cooper , Kevin Tian , Jun Nakajima , xen-devel List-Id: xen-devel@lists.xenproject.org On 10/15/2015 03:35 PM, Kai Huang wrote: > > > On 10/15/2015 03:11 PM, Jan Beulich wrote: >>>>> On 15.10.15 at 08:42, wrote: >>> Thanks for your comments Jan. Actually I am not happy with combining >>> with EPT A/D bit update with PML enabling to single function. After >>> thinking again, how about adding a separate vmx function (ex, >>> vmx_domain_update_eptp) to update EPTP of VMCS of all vcpus of domain >>> after p2m->ept.ept_ad is updated. Another good is this function can >>> also >>> be used in the future for other runtime updates to p2m->ept. >>> >>> What's your idea? >> I don't mind, but that's really more of a question to the VMX >> maintainers. > Then I would prefer this way. > > Kevin, > > Do you have any comments on this thread? >> >>> --- a/xen/arch/x86/mm/p2m-ept.c >>> +++ b/xen/arch/x86/mm/p2m-ept.c >>> @@ -1129,17 +1129,26 @@ void ept_sync_domain(struct p2m_domain *p2m) >>> >>> static void ept_enable_pml(struct p2m_domain *p2m) >>> { >>> /* >>> - * No need to check if vmx_domain_enable_pml has succeeded or >>> not, as >>> + * No need to return if vmx_domain_enable_pml has succeeded or >>> not, as >> It seems to me that you'd better use "whether" instead of "if" now >> (and then perhaps also drop the "or not"). > OK. Thanks. >> >>> * ept_p2m_type_to_flags will do the check, and write >>> protection will be >>> * used if PML is not enabled. >>> */ >>> - vmx_domain_enable_pml(p2m->domain); >>> + if ( vmx_domain_enable_pml(p2m->domain) ) >>> + return; >>> + >>> + p2m->ept.ept_ad = 1; >>> + vmx_domain_update_eptp(p2m->domain); >> Shouldn't you enable A/D _before_ enabling PML, at least without >> having a domain-is-paused check here? > Looks we don't have such function. How about just add > ASSERT(atomic_read(&d->pause_count)), just the same as in > vmx_domain_enable_pml ? I mean we can enable A/D before enabling PML, but if so we need additional code to clear A/D bit if vmx_domain_enable_pml failed. My thinking is considering the function is called when domain is paused, so there's no difference to enable A/D before or after enabling PML. Thanks, -Kai > > Thanks, > -Kai >> >> Jan >> >> > > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xen.org > http://lists.xen.org/xen-devel >