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

["Andres Lagar-Cavilla" <andres@xxxxxxxxxxxxxxxx>]

> 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.
- 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.

Cheers,
Andres
>
> Ian.
>
>
>



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