From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrew Cooper Subject: Re: [PATCH v3 1/2] x86/mem_event: Deliver gla fault EPT violation information Date: Fri, 08 Aug 2014 09:04:38 +0100 Message-ID: <53E48496.503@citrix.com> References: <1407453172-10785-1-git-send-email-tamas.lengyel@zentific.com> <53E4919B020000780002A504@mail.emea.novell.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" 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 1XFfAU-0006QG-Uw for xen-devel@lists.xenproject.org; Fri, 08 Aug 2014 08:04:55 +0000 In-Reply-To: <53E4919B020000780002A504@mail.emea.novell.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 , Tamas K Lengyel Cc: kevin.tian@intel.com, ian.campbell@citrix.com, stefano.stabellini@eu.citrix.com, jun.nakajima@intel.com, eddie.dong@intel.com, ian.jackson@eu.citrix.com, Aravind.Gopalakrishnan@amd.com, suravee.suthikulpanit@amd.com, xen-devel@lists.xenproject.org, boris.ostrovsky@oracle.com List-Id: xen-devel@lists.xenproject.org On 08/08/2014 08:00, Jan Beulich wrote: >>>> On 08.08.14 at 01:12, wrote: >> --- a/xen/arch/x86/hvm/hvm.c >> +++ b/xen/arch/x86/hvm/hvm.c >> @@ -2725,6 +2725,8 @@ void hvm_inject_page_fault(int errcode, unsigned long cr2) >> int hvm_hap_nested_page_fault(paddr_t gpa, >> bool_t gla_valid, >> unsigned long gla, >> + bool_t fault_in_gpt, >> + bool_t fault_gla, >> bool_t access_r, >> bool_t access_w, >> bool_t access_x) > Afaic it is out of question to have a function with _six_ boolean > parameters. This needs to be consolidated into a single flags field. I > have actually done that already, in a patch serving a different > purpose (see attached), as discussed recently on this list. I would > very much appreciate if you either re-based yours on top of that or > modified it along those lines. > >> @@ -2371,11 +2372,19 @@ static void ept_handle_violation(unsigned long qualification, paddr_t gpa) >> } >> >> if ( qualification & EPT_GLA_VALID ) >> + { >> __vmread(GUEST_LINEAR_ADDRESS, &gla); >> + fault_gla = !!(qualification & EPT_GLA_FAULT); >> + fault_in_gpt = !fault_gla; > I am actually not agreeing with Andrew regarding the need for two > flags here, if we already know that SVM also properly expresses the > distinction between faults on page table accesses and faults on the > actual translation. The attached patch is also coded in this way, and > I agree with your earlier arguing for just a single flag. I also agree. My suggestion for two flags was on the (wrong) assumption that AMD didn't currently support providing this information (although I should have picked up on this and retracted my suggestion). ~Andrew