Re: [Xen-devel] [PATCH QEMU-XEN v7 5/8] xen: Switch uses of xc_map_foreign_{pages, bulk} to use libxenforeignmemory API.

[Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>]

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 <ian.campbell@xxxxxxxxxx>
>  
> 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?

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel