Re: [Xen-devel] [PATCH 1 of 2] Use memops for mem paging, sharing, and access, instead of domctls

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

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" ;-)

Ian.



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel