[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH XEN v5 09/23] tools: Refactor hypercall calling wrappers into libxencall.
On Wed, 2015-11-11 at 15:08 +0000, Ian Jackson wrote: > Ian Campbell writes ("[PATCH XEN v5 09/23] tools: Refactor hypercall > calling wrappers into libxencall."): > > libxencall will provide a stable API and ABI for calling hypercalls > > (although those hypercalls themselves may not have a stable API). As > > well as the hypercall buffer infrastructure needed in order to safely > > provide pointer arguments to hypercalls. > ... > > +/* > > + * This library allows you to make arbitrary hypercalls (subject to > > + * sufficient permission for the process and the domain itself). Note > > + * that while the library interface is stable the hypercalls are > > + * subject to their own rules. > > Something needs to say what the error handling is like. > > Do these functions set errno ? Yes, they should have the customary "return -1 and set errno" pattern. I shall make this explicit. > > +/* > > + * Call hypercalls with varying numbers of arguments. > > + */ > > +int xencall0(xencall_handle *xcall, unsigned int op); > > Is the return value the raw hypercall return value, or is hypervisor > do_foo returning -EFOOBAR turned into to -1/errno=EFOOBAR ? > (Hopefully the answer to this doesn't depend on the hypercall ABI...) Oh god, this looks to vary by platform :-( Linux returns the hypercall return value directly from the ioctl as its return value, which the usual libc stuff turns into -1/errno=EFOO on failure. FreeBSD has a separate hypervisor retval field in the ioctl argument struct and does: return (ret == 0) ? hypercall->retval : ret; MiniOS goes for -1/errno=EFOO. What a mess. Either Linux+MiniOS or FreeBSD are going to have to change (i.e. in the wrappers in this library, not the underlying mechanisms) to match the other for consistency. On FreeBSD hypercall->retval is in the XEN_ERRNO_* space. But I think we probably want to map onto the FreeBSD ERRNO space, IIRC we decided on this approach, rather than converting all our callers to the XEN_ERRNO_* space, in a previous discussion. If we are going to standardise on one of those then given we've gone for -1/errno in most other places and it's the "normal" way to do things I think we should do the same here and the freebsd driver in libxencall should set errno. There are other alternatives, such as having this library return the "privcmd ioctl" result in its return value + errno and the "hypercall result" in a separate output pointer argument. This would require, possibly substantial, changes in the callers, but right now they are all in libxc, which could munge that back into the current somewhat braindead scheme allowing us to update those call paths piecemeal and allowing new users to use the sane interface. Thoughts? > > +/* > > + * Allocate and free memory which is suitable for use as a pointer > > + * argument to a hypercall. > > + */ > > +void *xencall_alloc_buffer_pages(xencall_handle *xcall, int nr_pages); > > +void xencall_free_buffer_pages(xencall_handle *xcall, void *p, int > > nr_pages); > > + > > +void *xencall_alloc_buffer(xencall_handle *xcall, size_t size); > > +void xencall_free_buffer(xencall_handle *xcall, void *p); > > See above re error handling. > > Can these functions be used without (a) knowing the page size > (b) a rounding macro ? > > It would be best to save callers the trouble of providing those > themselves. The library deals with this internally, size can be anything the caller likes. Ian. _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx http://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |