From mboxrd@z Thu Jan 1 00:00:00 1970 From: Razvan Cojocaru Subject: Re: [PATCH RFC V2 5/6] xen, libxc: Request page fault injection via libxc Date: Thu, 17 Jul 2014 15:07:41 +0300 Message-ID: <53C7BC8D.5080407@bitdefender.com> References: <1405093418-23481-1-git-send-email-rcojocaru@bitdefender.com> <1405093418-23481-5-git-send-email-rcojocaru@bitdefender.com> <53C027BF.1090902@citrix.com> <1405598011.29996.23.camel@kazak.uk.xensource.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1405598011.29996.23.camel@kazak.uk.xensource.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: Ian Campbell , Andrew Cooper Cc: mdontu@bitdefender.com, tim@xen.org, JBeulich@suse.com, xen-devel@lists.xen.org List-Id: xen-devel@lists.xenproject.org On 07/17/2014 02:53 PM, Ian Campbell wrote: > On Fri, 2014-07-11 at 19:06 +0100, Andrew Cooper wrote: >> On 11/07/14 16:43, Razvan Cojocaru wrote: >>> Added new XEN_DOMCTL_set_pagefault_info hypercall, used by libxc's >>> new xc_domain_set_pagefault_info() function to set per-domain page >>> fault injection information. All a call does is set per-domain info, >>> and nothing actually happens until VMENTRY time, and then only if >>> all conditions are met (the guest is in user mode, the set value >>> matches CR3, and there are no other pending traps). >>> This mechanism allows bringing in swapped-out pages for inspection. >>> >>> Signed-off-by: Razvan Cojocaru >> >> For the record, I still think this is a bad idea to be working against a >> guest OS paging algorithm, and I am uneasy about whether it is sensible >> to introduce abilities like that into the Xen API. > > Indeed. > > From the tools side the new libxc function is a correct wrapping of the > underlying hypercall, so if/when the hypervisor maintainers are happy > with the interface it's fine by me, with one minor comment. Thanks for the review! >>> +int xc_domain_set_pagefault_info(xc_interface *xch, >>> + uint32_t domid, >>> + xen_domctl_set_pagefault_info_t *info) >>> +{ >>> + DECLARE_DOMCTL; >>> + >>> + if (info == NULL) >>> + return -1; >>> + >>> + domctl.cmd = XEN_DOMCTL_set_pagefault_info; >>> + domctl.domain = (domid_t)domid; >>> + domctl.u.set_pagefault_info.address_space = info->address_space; >>> + domctl.u.set_pagefault_info.virtual_address = info->virtual_address; >>> + domctl.u.set_pagefault_info.write_access = info->write_access; > > Aren't these three lines just "domctl.u.set_pagefault_info = *info;"? Yes, they are. I'll change that to the reader-friendlier one-liner. Thanks, Razvan Cojocaru