From mboxrd@z Thu Jan 1 00:00:00 1970 From: Razvan Cojocaru Subject: Re: [PATCH 2/2] altp2m: Implement p2m_get_mem_access for altp2m views Date: Thu, 28 Jan 2016 17:20:38 +0200 Message-ID: <56AA31C6.4030202@bitdefender.com> References: <1453925201-15926-1-git-send-email-tlengyel@novetta.com> <1453925201-15926-2-git-send-email-tlengyel@novetta.com> <56AA27D402000078000CC080@prv-mh.provo.novell.com> <56AA2DC0.9060806@bitdefender.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail6.bemta3.messagelabs.com ([195.245.230.39]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1aOoMo-0007NF-3v for xen-devel@lists.xenproject.org; Thu, 28 Jan 2016 15:20:14 +0000 Received: from smtp02.buh.bitdefender.net (unknown [10.17.80.76]) by mx-sr.buh.bitdefender.com (Postfix) with ESMTP id 4CD17803B5 for ; Thu, 28 Jan 2016 17:20:11 +0200 (EET) In-Reply-To: List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: "Lengyel, Tamas" Cc: Wei Liu , Ian Campbell , Stefano Stabellini , George Dunlap , Andrew Cooper , Ian Jackson , Stefano Stabellini , Jan Beulich , xen-devel@lists.xenproject.org, Keir Fraser List-Id: xen-devel@lists.xenproject.org On 01/28/2016 05:12 PM, Lengyel, Tamas wrote: > > On Jan 28, 2016 8:02 AM, "Razvan Cojocaru" > wrote: >> >> On 01/28/2016 04:42 PM, Lengyel, Tamas wrote: >> > >> > On Jan 28, 2016 6:38 AM, "Jan Beulich" >> > >> wrote: >> >> >> >> >>> On 27.01.16 at 21:06, >> > >> wrote: >> >> > --- a/xen/arch/x86/mm/p2m.c >> >> > +++ b/xen/arch/x86/mm/p2m.c >> >> > @@ -1572,7 +1572,9 @@ void p2m_mem_access_emulate_check(struct > vcpu *v, >> >> > bool_t violation = 1; >> >> > const struct vm_event_mem_access *data = &rsp->u.mem_access; >> >> > >> >> > - if ( p2m_get_mem_access(v->domain, _gfn(data->gfn), >> > &access) == 0 ) >> >> > + if ( p2m_get_mem_access(v->domain, _gfn(data->gfn), >> >> > + altp2m_active(v->domain) ? >> > vcpu_altp2m(v).p2midx : 0, >> >> > + &access) == 0 ) >> >> >> >> This looks to be a behavioral change beyond what title and >> >> description say, and it's not clear whether that's actually the >> >> behavior everyone wants. >> > >> > I'm fairly comfident its exactly the expected behavior when one uses >> > mem_access in altp2m tables and emulation. Right now because the lack of >> > this AFAIK emulation would not work correctly with altp2m. But Razvan >> > probably can chime in as he uses this path actively. >> >> I've done an experiment to see how much slower using altp2m would be as >> compared to emulation - so I'm not a big user of the feature, but I did >> find it cumbersome to have to work with two sets of APIs (one for what >> could arguably be called the default altp2m view, i.e. the regular >> xc_set_mem_access(), and one for altp2m, i.e. >> xc_altp2m_set_mem_access()). Furthermore, the APIs do not currently >> offer the same features (most notably, xc_altp2m_get_mem_access() is >> completely missing). I've mentioned this to Tamas while initially trying >> to get it to work. >> >> Now, whether the behaviour I expect is what everyone expects is, of >> course, wide open to debate. But I think we can all agree that the >> altp2m interface can, and probably should, be improved. >> > > There is that, but also, what is the exact logic behind doing this check > before emulation? AFAIU emulation happens in response to a vm_event so > we should be fairly certain that this check succeeds as it just verifies > that indeed the permissions are restricted by mem_access in the p2m (and > with altp2m this should be the active one). But when is this check > normally expected to fail? That check is important, please do not remove it. A vm_event is sent into userspace to our monitoring application, but the monitoring application can actually remove the page restrictions before replying, so in that case emulation is pointless - there will be no more page faults for that instruction. Thanks, Razvan