From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Jan Beulich" Subject: Re: [PATCH RFC V9 4/5] xen, libxc: Request page fault injection via libxc Date: Mon, 01 Sep 2014 13:05:17 +0100 Message-ID: <54047D1D020000780002F73A@mail.emea.novell.com> References: <1409226482-12657-1-git-send-email-rcojocaru@bitdefender.com> <1409226482-12657-4-git-send-email-rcojocaru@bitdefender.com> <53FF36A1020000780002EAED@mail.emea.novell.com> <53FF1BD8.5010401@bitdefender.com> <53FF38A6020000780002EB2B@mail.emea.novell.com> <54002F43.4070802@bitdefender.com> <5400638A020000780002EFD6@mail.emea.novell.com> <540421E1.9020505@bitdefender.com> <540453C8020000780002F59C@mail.emea.novell.com> <54045E7C.50604@bitdefender.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail6.bemta5.messagelabs.com ([195.245.231.135]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1XOQMM-0003vu-3f for xen-devel@lists.xenproject.org; Mon, 01 Sep 2014 12:05:22 +0000 In-Reply-To: <54045E7C.50604@bitdefender.com> Content-Disposition: inline List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Razvan Cojocaru Cc: kevin.tian@intel.com, ian.campbell@citrix.com, stefano.stabellini@eu.citrix.com, andrew.cooper3@citrix.com, eddie.dong@intel.com, tim@xen.org, jun.nakajima@intel.com, xen-devel@lists.xenproject.org, ian.jackson@eu.citrix.com List-Id: xen-devel@lists.xenproject.org >>> On 01.09.14 at 13:54, wrote: > On 09/01/2014 12:08 PM, Jan Beulich wrote: >>>>> On 01.09.14 at 09:36, wrote: >>> On 08/29/2014 12:27 PM, Jan Beulich wrote: >>>>>>> On 29.08.14 at 09:44, wrote: >>>>> I do understand the preference for a VCPU-based mechanism from a >>>>> concurrency point of view, but that would simply potentially fail for >>>>> us, hence defeating the purpose of the patch. I'm also not sure how that >>>>> would be useful in the general case either, since the same problem that >>>>> applies to us would seem to apply to the general case as well. >>>> >>>> Yeah, the whole thing probably needs a bit more thinking so that the >>>> interface doesn't end up being a BitDefender-special. Indeed together >>>> with the address space qualification, the interface might not be very >>>> useful when made vCPU-bound. And taking it a little further into the >>>> "generic" direction, allowing this to only inject #PF doesn't make a >>>> very nice interface either. Plus we already have HVMOP_inject_trap, >>>> i.e. your first line of thinking (and eventual explaining as the >>>> motivation for a patch) should be why that can't be used. >>> >>> I'd say that it's memory-introspection specific rather than 3rd-party >>> vendor specific. Without this this patch, memory-introspection support >>> in general is impacted / less flexible, since there's no other way to >>> bring swapped out pages back in. >>> >>> For all the reasons you've explained (at least as far as I understand >>> it) there's not much room to go more generic - so maybe just renaming >>> the libxc wrapper to something more specific ( >>> xc_domain_request_usermode_pagefault?) is the solution here? >> >> Maybe, but only after you explained why the existing interface can >> neither be used nor suitably extended. > > As far as I understand the HVMOP_inject_trap interface, it is simply (in > this case) a way to trigger the equivalent of hvm_inject_page_fault() > from userspace (via libxc). > > We need two additional checks: > > 1. That CR3 matches, because the way the application works, we need to > inject a page fault related to the address space of whatever process is > interesting at the time, and > > 2. That we're in usermode (the CPL check), because we know that it's > safe to inject a page fault when we're in user mode, but it's not always > safe to do so in kernel mode. > > The current interface seems to be a low-level, basic wrapper around > hvm_inject_trap(). > > What we're trying to do is ask for a page fault when we're both A) in > usermode, and B) when a target process matches - and are only interested > in page faults, no other trap vector. > > Technically, the checks could, probably, be moved into (and new > parameters added to) hvm_inject_trap() & friends, but it seems unlikely > that users other than memory introspection applications would be > interested in them, while they would possibly add to the complexity of > the interface. The rest of the clients would have to carry dummy > parameters around to use it. I'm not sure: Injecting faults with certain restrictions (like in your case CPL and CR3) does seem to make quite some sense in general when the fault isn't intended to be terminal (of a process or the entire VM). It's just that so far we used fault injection only to indicate error conditions. But as always - let's see if others are of differing opinion before settling on either route. Jan