On Thu, Jun 12, 2014 at 6:22 AM, Andrew Cooper <andrew.cooper3@citrix.com> wrote:

On 12/06/14 10:23, Dushyant Behl wrote:
> This patch is part of the work done under the gsoc project -
> Lazy Restore Using Memory Paging.
>
> This patch moves the code to initialize mempaging from xenpaging to libxc.

The source code is named libxc, but technically you are moving it into
libxenguest which is a separate library from libxenctrl.

> The code refractored from xenpaging is the code which sets up paging,
> initializes a shared ring and event channel to communicate with xen. This
> communication is done between the hypervisor and the dom0 tool which performs
> the activity of pager. The xenpaging code is changed to use the newly created
> routines and is tested to properly build and work with this code.
>
> The refractoring is done so that any tool which will act as pager in
> lazy restore or use memory paging can use a same routines to initialize mempaging.
> This refactoring will also allow any future (in-tree) tools to use mempaging.
>
> Signed-off-by: Dushyant Behl <myselfdushyantbehl@gmail.com>
> Reviewed-by: Andres Lagar-Cavilla <andres@lagarcavilla.org>

> ---
> tools/libxc/Makefile | 3 +
> tools/libxc/xc_mem_paging_setup.c | 132 ++++++++++++++++++++++++++++++++++++
> tools/libxc/xenctrl.h | 12 ++++
> tools/ocaml/libs/xc/xenctrl_stubs.c | 11 +--
> tools/xenpaging/Makefile | 4 +-
> tools/xenpaging/xenpaging.c | 93 +++----------------------
> 6 files changed, 167 insertions(+), 88 deletions(-)
> create mode 100644 tools/libxc/xc_mem_paging_setup.c
>
> diff --git a/tools/libxc/Makefile b/tools/libxc/Makefile
> index a74b19e..2439853 100644
> --- a/tools/libxc/Makefile
> +++ b/tools/libxc/Makefile
> @@ -69,6 +69,9 @@ GUEST_SRCS-$(CONFIG_ARM) += xc_dom_armzimageloader.c
> GUEST_SRCS-y += xc_dom_binloader.c
> GUEST_SRCS-y += xc_dom_compat_linux.c
>
> +# mem paging setup
> +GUEST_SRCS-y += xc_mem_paging_setup.c
> +

I don't think you need the comment here. It is obvious from the name of
the file, although I would suggest you name it "xc_dom_paging.c" or
something a tad less specific.

> GUEST_SRCS-$(CONFIG_X86) += xc_dom_x86.c
> GUEST_SRCS-$(CONFIG_X86) += xc_cpuid_x86.c
> GUEST_SRCS-$(CONFIG_X86) += xc_hvm_build_x86.c
> diff --git a/tools/libxc/xc_mem_paging_setup.c b/tools/libxc/xc_mem_paging_setup.c
> new file mode 100644
> index 0000000..125d203
> --- /dev/null
> +++ b/tools/libxc/xc_mem_paging_setup.c
> @@ -0,0 +1,132 @@
> +/*
> + * tools/libxc/xc_mem_paging_setup.c
> + *
> + * Routines to initialize memory paging. Create shared ring
> + * and event channels to communicate with the hypervisor.
> + *
> + * Copyright (c) 2014 Dushyant Behl
> + * Copyright (c) 2009 by Citrix Systems, Inc. (Patrick Colp)
> + *
> + * This library is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2.1 of the License, or (at your option) any later version.
> + *
> + * This library is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with this library; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
> + */
> +
> +#include "xc_private.h"
> +#include <xen/event_channel.h>
> +#include <xen/mem_event.h>
> +
> +/*
> + * Mem paging ring and event channel setup routine.
> + * Setup a shared ring and an event channel to communicate between
> + * hypervisor and the tool performing mem paging operations.

Please document here, or in the header file, what the error semantics of
the function are. Libxc is a complete mess so these things need
spelling out.

> + */
> +int xc_mem_paging_ring_setup(xc_interface *xch, domid_t domain_id,
> + void *ring_page, int *port,
> + uint32_t *evtchn_port,
> + xc_evtchn **xce_handle,
> + mem_event_back_ring_t *back_ring)

The parameters should be aligned with the xc_interface.

> +{
> + int rc;
> + unsigned long ring_pfn, mmap_pfn;

uint64_t's as these are hvmparams

David Vrabel's GenID series fixes the prototypes of
xc_[gs]et_hvm_param() to be sane.

Andrew, we were thinking this should be rebased onto your git branch for save restore rework, which I assume contains these GenID changes as well. Can you point to the right git coordinates? Thanks