From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:35860) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1a9DFf-0004uQ-Qq for qemu-devel@nongnu.org; Wed, 16 Dec 2015 09:40:27 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1a9DFb-0006zu-KD for qemu-devel@nongnu.org; Wed, 16 Dec 2015 09:40:23 -0500 Received: from smtp02.citrix.com ([66.165.176.63]:20544) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1a9DFb-0006yD-Cm for qemu-devel@nongnu.org; Wed, 16 Dec 2015 09:40:19 -0500 Date: Wed, 16 Dec 2015 14:39:35 +0000 From: Stefano Stabellini In-Reply-To: <1450269131-27735-6-git-send-email-ian.campbell@citrix.com> Message-ID: References: <1450269007.4053.48.camel@citrix.com> <1450269131-27735-1-git-send-email-ian.campbell@citrix.com> <1450269131-27735-6-git-send-email-ian.campbell@citrix.com> MIME-Version: 1.0 Content-Type: text/plain; charset="US-ASCII" Subject: Re: [Qemu-devel] [PATCH QEMU-XEN v7 5/8] xen: Switch uses of xc_map_foreign_{pages, bulk} to use libxenforeignmemory API. List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Ian Campbell Cc: wei.liu2@citrix.com, stefano.stabellini@eu.citrix.com, ian.jackson@eu.citrix.com, qemu-devel@nongnu.org, xen-devel@lists.xen.org On Wed, 16 Dec 2015, Ian Campbell wrote: > In Xen 4.7 we are refactoring parts libxenctrl into a number of > separate libraries which will provide backward and forward API and ABI > compatiblity. > > One such library will be libxenforeignmemory which provides access to > privileged foreign mappings and which will provide an interface > equivalent to xc_map_foreign_{pages,bulk}. > > The new xenforeignmemory_map() function behaves like > xc_map_foreign_pages() when the err argument is NULL and like > xc_map_foreign_bulk() when err is non-NULL, which maps into the shim > here onto checking err == NULL and calling the appropriate old > function. > > Note that xenforeignmemory_map() takes the number of pages before the > arrays themselves, in order to support potentially future use of > variable-length-arrays in the prototype (in the future, when Xen's > baseline toolchain requirements are new enough to ensure VLAs are > supported). > > In preparation for adding support for libxenforeignmemory add support > to the <=4.0 and <=4.6 compat code in xen_common.h to allow us to > switch to using the new API. These shims will disappear for versions > of Xen which include libxenforeignmemory. > > Since libxenforeignmemory will have its own handle type but for <= 4.6 > the functionality is provided by using a libxenctrl handle we > introduce a new global xen_fmem alongside the existing xen_xc. In fact > we make xen_fmem a pointer to the existing xen_xc, which then works > correctly with both <=4.0 (xc handle is an int) and <=4.6 (xc handle > is a pointer). In the latter case xen_fmem is actually a double > indirect pointer, but it all falls out in the wash. > > Unlike libxenctrl libxenforeignmemory has an explicit unmap function, > rather than just specifying that munmap should be used, so the unmap > paths are updated to use xenforeignmemory_unmap, which is a shim for > munmap on these versions of xen. The mappings in xen-hvm.c do not > appear to be unmapped (which makes sense for a qemu-dm process) > > In fb_disconnect this results in a change from simply mmap over the > existing mapping (with an implicit munmap) to expliclty unmapping with > xenforeignmemory_unmap and then mapping the required anonymous memory > in the same hole. I don't think this is a problem since any other > thread which was racily touching this region would already be running > the risk of hitting the mapping halfway through the call. If this is > thought to be a problem then we could consider adding an extra API to > the libxenforeignmemory interface to replace a foreign mapping with > anonymous shared memory, but I'd prefer not to. > > Signed-off-by: Ian Campbell > > diff --git a/include/hw/xen/xen_common.h b/include/hw/xen/xen_common.h > index 8f38310..55809b4 100644 > --- a/include/hw/xen/xen_common.h > +++ b/include/hw/xen/xen_common.h > @@ -41,6 +41,7 @@ static inline void *xc_map_foreign_bulk(int xc_handle, uint32_t dom, int prot, > typedef int XenXC; > typedef int xenevtchn_handle; > typedef int xengnttab_handle; > +typedef int xenforeignmemory_handle; > > # define XC_INTERFACE_FMT "%i" > # define XC_HANDLER_INITIAL_VALUE -1 > @@ -104,6 +105,10 @@ static inline XenXC xen_xc_interface_open(void *logger, void *dombuild_logger, > return xc_interface_open(); > } > > +#define xenforeignmemory_open(l, f) &xen_xc > +/* See below for xenforeignmemory_map */ > +#define xenforeignmemory_unmap(h, p, s) munmap(p, s * XC_PAGE_SIZE) > + > static inline int xc_fd(int xen_xc) > { > return xen_xc; > @@ -149,6 +154,7 @@ static inline void xs_close(struct xs_handle *xsh) > #else > > typedef xc_interface *XenXC; > +typedef xc_interface *xenforeignmemory_handle; > typedef xc_evtchn xenevtchn_handle; > typedef xc_gnttab xengnttab_handle; > > @@ -178,6 +184,10 @@ static inline XenXC xen_xc_interface_open(void *logger, void *dombuild_logger, > return xc_interface_open(logger, dombuild_logger, open_flags); > } > > +#define xenforeignmemory_open(l, f) &xen_xc > +/* See below for xenforeignmemory_map */ > +#define xenforeignmemory_unmap(h, p, s) munmap(p, s * XC_PAGE_SIZE) > + > /* FIXME There is now way to have the xen fd */ > static inline int xc_fd(xc_interface *xen_xc) > { @@ -501,4 +511,17 @@ static inline int xen_domain_create(XenXC xc, uint32_t ssidref, > } > #endif > > +#if CONFIG_XEN_CTRL_INTERFACE_VERSION < 470 > +static inline void *xenforeignmemory_map(XenXC *h, uint32_t dom, > + int prot, size_t pages, > + const xen_pfn_t arr[/*pages*/], > + int err[/*pages*/]) > +{ > + if (err) > + return xc_map_foreign_bulk(*h, dom, prot, arr, err, pages); > + else > + return xc_map_foreign_pages(*h, dom, prot, arr, pages); > +} > +#endif Why not move the definition of xenforeignmemory_open and xenforeignmemory_unmap here too?