From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ian Campbell Subject: Re: [PATCH 1 of 2] Use memops for mem paging, sharing, and access, instead of domctls Date: Fri, 3 Feb 2012 08:40:46 +0000 Message-ID: <1328258446.13189.48.camel@dagon.hellion.org.uk> References: <8e64acc49aa80c1860b2.1328126979@xdev.gridcentric.ca> <1328187397.5359.9.camel@zakaz.uk.xensource.com> <148b4d34646effc399f9eab9c07c30c2.squirrel@webmail.lagarcavilla.org> <1328194692.2924.29.camel@zakaz.uk.xensource.com> <69c1675a2a6774a3e5172b02762b0e3d.squirrel@webmail.lagarcavilla.org> <1328214343.13189.4.camel@dagon.hellion.org.uk> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xensource.com Errors-To: xen-devel-bounces@lists.xensource.com To: "andres@lagarcavilla.org" Cc: "xen-devel@lists.xensource.com" , "Tim (Xen.org)" , "andres@gridcentric.ca" , Ian Jackson , "keir.xen@gmail.com" , "JBeulich@suse.com" , "adin@gridcentric.ca" List-Id: xen-devel@lists.xenproject.org On Thu, 2012-02-02 at 23:09 +0000, Andres Lagar-Cavilla wrote: > > On Thu, 2012-02-02 at 20:23 +0000, Andres Lagar-Cavilla wrote: > >> > On Thu, 2012-02-02 at 14:25 +0000, Andres Lagar-Cavilla wrote: > >> >> > On Wed, 2012-02-01 at 20:09 +0000, Andres Lagar-Cavilla wrote: > >> >> >> > >> >> >> > >> >> >> +int xc_mem_event_memop(xc_interface *xch, domid_t domain_id, > >> >> >> + unsigned int op, unsigned int mode, > >> >> >> + uint64_t gfn, void *buffer) > >> >> >> +{ > >> >> >> + xen_mem_event_op_t meo; > >> >> >> + > >> >> >> + memset(&meo, 0, sizeof(meo)); > >> >> >> + > >> >> >> + meo.op = op; > >> >> >> + meo.domain = domain_id; > >> >> >> + meo.gfn = gfn; > >> >> >> + meo.buffer = (unsigned long) buffer; > >> >> > > >> >> > Hypercall arguments need to use the xc_hypercall_buffer stuff in > >> order > >> >> > to ensure that the memory is safe to access from a hypercall (in > >> >> > particular that it is mlocked or similar)-- I don't see where that > >> >> > happens for this buffer. > >> >> > > >> >> > meo itself is bounced by do_memory_op so that is OK. > >> >> > > >> >> > I was also a little suspicious of ring_addr and shared_addr in > >> >> > xc_mem_event_control in this regard. > >> >> > > >> >> > Or does the mem sharing code make it's own arrangements for these > >> >> sorts > >> >> > of things somewhere? > >> >> > >> >> It's rather unclean. They do not get mlock'ed (and there is no sanity > >> >> check to ensure they're page-aligned). They should follow the pattern > >> >> established in xc_mem_paging_load. I'll get on it, probably a > >> separate > >> >> patch. > >> > > >> > If you are reworking this It Would Be Nice If (tm) you could make it > >> use > >> > xc_hypercall_buffer_alloc and friends to allocate and manage the > >> buffer > >> > while you are there e.g. the void *buffer above would become struct > >> > xc_hypercall_buffer *buffer -- see xc_perfc_query for example. > >> > >> Took me a while to grok those hypercall buffer macros. Conclusion: that > >> would need to change callers of paging_load (xenpaging in tree). > > > > Yes, in this case you probably want to bubble their use right up to the > > top level rather than bouncing in the internals like we do for smaller > > and.or more transient data. > > > >> Can we keep this separate? domctl interface also suffers from all these > >> problems. domctl->memops switch is orthogonal to the buffer badness. I > >> want to tackle one problem at a time. > > > > Sure, read "IWBNI" as "in the fullness of time" ;-) > > Ok, trying to mb() what's needed now and what will take place in the > fullness of time: > - ring creation when initializing mem event is ugly, but cannot be fixed > short of a kernel driver. I will however produce a patch now that at least > mlock's these guys. Sounds good. mb() goes here ;-) > - All libxc consumers of these interfaces should be updated to init these > magic pages (and the paging load buffer) using xc_hypercall_buffer_alloc > in their own code. Correct, this can be done whenever though. Ian.