[PATCH v7 00/10] toolstack-based approach to pvhvm guest kexec

[PATCH v7 00/10] toolstack-based approach to pvhvm guest kexec

[Vitaly Kuznetsov]

This patch series provides x86 PVHVM domains with an ability to perform
kexec/kdump-style operations.

The main change in v7 is the XENMEM_soft_reset->XEN_DOMCTL_soft_reset change of
the newly introduced hypercall.

Changes are mentioned in individual patches. Here is the list of them grouped
by the reviewer's name:

Jan Beulich:
- XENMEM_soft_reset->XEN_DOMCTL_soft_reset change
- print a warning on !mfn_valid case
- check PGC_allocated for pages
- don't re-read page->count_info
- minor structuring change
- pause both source and destination domain while processing the hypercall
- remove nr_transferred from public interface
- other minor fixes in PATCH 04/10
- add a comment on why we don't do two XSM_TARGET checks in dummy.h

Tim Deegan:
- use get_gfn_type_access() in PoD case

Wei Liu:
- replace LIBXL_HAVE_SHUTDWON_REASON_SOFT_RESET with LIBXL_HAVE_SOFT_RESET

Daniel De Graaf:
- add reset_transfer vector to FLASK (I dropped Acked-by: on the patch because
  of the XENMEM_soft_reset->XEN_DOMCTL_soft_reset change.)

Julien Grall:
- check that xc_domain_getinfo() return the domain we asked for

v6 of this series is available here:
http://lists.xenproject.org/archives/html/xen-devel/2015-05/msg01613.html

v5 and all the history of the series is available here:
http://lists.xenproject.org/archives/html/xen-devel/2014-12/msg01312.html

Vitaly Kuznetsov (10):
  xen: introduce SHUTDOWN_soft_reset shutdown reason
  libxl: support SHUTDOWN_soft_reset shutdown reason
  xen: introduce DOMDYING_locked state
  xen: Introduce XEN_DOMCTL_soft_reset
  xsm: add XEN_DOMCTL_soft_reset support
  libxc: support XEN_DOMCTL_soft_reset operation
  libxc: introduce soft reset for HVM domains
  xl: introduce enum domain_restart_type
  libxc: add XC_DEVICE_MODEL_SAVE_FILE
  (lib)xl: soft reset support

 docs/man/xl.cfg.pod.5                        |  12 ++
 tools/flask/policy/policy/modules/xen/xen.if |   3 +-
 tools/libxc/Makefile                         |   1 +
 tools/libxc/include/xenctrl.h                |   4 +
 tools/libxc/include/xenguest.h               |  22 +++
 tools/libxc/xc_domain.c                      |  12 ++
 tools/libxc/xc_soft_reset.c                  | 280 +++++++++++++++++++++++++++
 tools/libxl/libxl.c                          |   4 +
 tools/libxl/libxl.h                          |  14 ++
 tools/libxl/libxl_create.c                   | 119 +++++++++++-
 tools/libxl/libxl_dm.c                       |   2 +-
 tools/libxl/libxl_internal.h                 |  26 +++
 tools/libxl/libxl_types.idl                  |   4 +
 tools/libxl/xl.h                             |   7 +
 tools/libxl/xl_cmdimpl.c                     |  58 ++++--
 tools/python/xen/lowlevel/xl/xl.c            |   1 +
 xen/common/domain.c                          | 187 ++++++++++++++++++
 xen/common/domctl.c                          |  79 ++++++++
 xen/common/shutdown.c                        |   6 +
 xen/include/public/domctl.h                  |  28 +++
 xen/include/public/sched.h                   |   3 +-
 xen/include/xen/sched.h                      |   9 +-
 xen/include/xsm/dummy.h                      |  12 ++
 xen/include/xsm/xsm.h                        |   8 +
 xen/xsm/dummy.c                              |   2 +
 xen/xsm/flask/hooks.c                        |  18 ++
 xen/xsm/flask/policy/access_vectors          |   7 +
 27 files changed, 899 insertions(+), 29 deletions(-)
 create mode 100644 tools/libxc/xc_soft_reset.c

-- 
1.9.3

[PATCH v7 01/10] xen: introduce SHUTDOWN_soft_reset shutdown reason

[Vitaly Kuznetsov]

This special type of shutdown is supposed to be used by PVHVM guests when
they want to perform some sort of kexec/kdump. Toolstack will have to build
a new domain with the memory content of the original domain and start executing
the new one from the point where SHUTDOWN_soft_reset was called. This operation
is not supported for PV domains including the hardware domain.

Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
---
 xen/common/shutdown.c      | 6 ++++++
 xen/include/public/sched.h | 3 ++-
 2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/xen/common/shutdown.c b/xen/common/shutdown.c
index 9cfbf7a..03a8641 100644
--- a/xen/common/shutdown.c
+++ b/xen/common/shutdown.c
@@ -66,6 +66,12 @@ void hwdom_shutdown(u8 reason)
         machine_restart(0);
         break; /* not reached */
 
+    case SHUTDOWN_soft_reset:
+        printk("Hardware domain %d did unsupported soft reset, rebooting.\n",
+               hardware_domain->domain_id);
+        machine_restart(0);
+        break; /* not reached */
+
     default:
         printk("Hardware Dom%u shutdown (unknown reason %u): ",
                hardware_domain->domain_id, reason);
diff --git a/xen/include/public/sched.h b/xen/include/public/sched.h
index 4000ac9..800c808 100644
--- a/xen/include/public/sched.h
+++ b/xen/include/public/sched.h
@@ -159,7 +159,8 @@ DEFINE_XEN_GUEST_HANDLE(sched_watchdog_t);
 #define SHUTDOWN_suspend    2  /* Clean up, save suspend info, kill.         */
 #define SHUTDOWN_crash      3  /* Tell controller we've crashed.             */
 #define SHUTDOWN_watchdog   4  /* Restart because watchdog time expired.     */
-#define SHUTDOWN_MAX        4  /* Maximum valid shutdown reason.             */
+#define SHUTDOWN_soft_reset 5  /* Soft reset, rebuild keeping memory content */
+#define SHUTDOWN_MAX        5  /* Maximum valid shutdown reason.             */
 /* ` } */
 
 #endif /* __XEN_PUBLIC_SCHED_H__ */
-- 
1.9.3

[PATCH v7 02/10] libxl: support SHUTDOWN_soft_reset shutdown reason

[Vitaly Kuznetsov]

Use letter 'S' to indicate a domain in such state.

Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
---
Changes in v7:
- Remove LIBXL_HAVE_SHUTDWON_REASON_SOFT_RESET, the functionality will be
  advertised with LIBXL_HAVE_SOFT_RESET with the implementation [Wei Liu]
---
 tools/libxl/libxl_types.idl       | 1 +
 tools/libxl/xl_cmdimpl.c          | 2 +-
 tools/python/xen/lowlevel/xl/xl.c | 1 +
 3 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
index 65d479f..1aa90f5 100644
--- a/tools/libxl/libxl_types.idl
+++ b/tools/libxl/libxl_types.idl
@@ -177,6 +177,7 @@ libxl_shutdown_reason = Enumeration("shutdown_reason", [
     (2, "suspend"),
     (3, "crash"),
     (4, "watchdog"),
+    (5, "soft_reset"),
     ], init_val = "LIBXL_SHUTDOWN_REASON_UNKNOWN")
 
 libxl_vga_interface_type = Enumeration("vga_interface_type", [
diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index 373aa37..7abac5d 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -3704,7 +3704,7 @@ static void list_domains(bool verbose, bool context, bool claim, bool numa,
                          bool cpupool, const libxl_dominfo *info, int nb_domain)
 {
     int i;
-    static const char shutdown_reason_letters[]= "-rscw";
+    static const char shutdown_reason_letters[]= "-rscwS";
     libxl_bitmap nodemap;
     libxl_physinfo physinfo;
 
diff --git a/tools/python/xen/lowlevel/xl/xl.c b/tools/python/xen/lowlevel/xl/xl.c
index 32f982a..7c61160 100644
--- a/tools/python/xen/lowlevel/xl/xl.c
+++ b/tools/python/xen/lowlevel/xl/xl.c
@@ -784,6 +784,7 @@ PyMODINIT_FUNC initxl(void)
     _INT_CONST_LIBXL(m, SHUTDOWN_REASON_SUSPEND);
     _INT_CONST_LIBXL(m, SHUTDOWN_REASON_CRASH);
     _INT_CONST_LIBXL(m, SHUTDOWN_REASON_WATCHDOG);
+    _INT_CONST_LIBXL(m, SHUTDOWN_REASON_SOFT_RESET);
 
     genwrap__init(m);
 }
-- 
1.9.3

Re: [PATCH v7 02/10] libxl: support SHUTDOWN_soft_reset shutdown reason

[Ian Campbell]

On Wed, 2015-05-27 at 17:25 +0200, Vitaly Kuznetsov wrote:
> Use letter 'S' to indicate a domain in such state.
> 
> Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>

Assuming the hypervisor side is found acceptable:

Acked-by: Ian Campbell <ian.campbell@citrix.com>

[PATCH v7 03/10] xen: introduce DOMDYING_locked state

[Vitaly Kuznetsov]

The new state is required as a prerequisite to implementing soft reset
operation for domains. In this operation we need to make sure that source
domain's mappings don't change while we are reassigning all its memory to
the destination domain. This new state indicates that the particular domain is
dying (so its mappings are locked and do not change) but the cleanup procedure
has not being started yet. It will be set from outside of the domain_kill()
function.

Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
---
 xen/common/domain.c     | 1 +
 xen/include/xen/sched.h | 3 ++-
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/xen/common/domain.c b/xen/common/domain.c
index 6803c4d..7825c56 100644
--- a/xen/common/domain.c
+++ b/xen/common/domain.c
@@ -605,6 +605,7 @@ int domain_kill(struct domain *d)
     switch ( d->is_dying )
     {
     case DOMDYING_alive:
+    case DOMDYING_locked:
         domain_pause(d);
         d->is_dying = DOMDYING_dying;
         spin_barrier(&d->domain_lock);
diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h
index 80c6f62..f53b91d 100644
--- a/xen/include/xen/sched.h
+++ b/xen/include/xen/sched.h
@@ -354,7 +354,8 @@ struct domain
     enum guest_type guest_type;
 
     /* Is this guest dying (i.e., a zombie)? */
-    enum { DOMDYING_alive, DOMDYING_dying, DOMDYING_dead } is_dying;
+    enum { DOMDYING_alive, DOMDYING_locked, DOMDYING_dying, DOMDYING_dead }
+        is_dying;
 
     /* Domain is paused by controller software? */
     int              controller_pause_count;
-- 
1.9.3

[PATCH v7 04/10] xen: Introduce XEN_DOMCTL_soft_reset

[Vitaly Kuznetsov]

New operation reassigns all memory pages from source domain to the destination
domain mapping them at exactly the same GFNs. Pages mapped more than once (e.g.
grants) are being copied.

Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
---
Changes in v7:
- is_soft_reset flag added to struct domain to preserve destination domain's
  paused state across possible hypercall preemption.
[Jan Beulich]
- XENMEM_soft_reset -> XEN_DOMCTL_soft_reset
- domain_soft_reset() returns int
- no locking for is_dying as it is now proteced by domctl_lock
- print a warning on !mfn_valid case
- check PGC_allocated for pages
- no print on assign_pages failure (it prints error messages on both its failure paths)
- don't re-read page->count_info, use copy_page flag
- minor stucturing change
- pause both source and destination domain while processing the hypercall
- remove nr_transferred from public interface
[Tim Deegan]
- use get_gfn_type_access() in PoD case (x86-only)
---
 xen/common/domain.c         | 186 ++++++++++++++++++++++++++++++++++++++++++++
 xen/common/domctl.c         |  72 +++++++++++++++++
 xen/include/public/domctl.h |  28 +++++++
 xen/include/xen/sched.h     |   6 ++
 4 files changed, 292 insertions(+)

diff --git a/xen/common/domain.c b/xen/common/domain.c
index 7825c56..824f325 100644
--- a/xen/common/domain.c
+++ b/xen/common/domain.c
@@ -1006,6 +1006,192 @@ int domain_unpause_by_systemcontroller(struct domain *d)
     return 0;
 }
 
+int domain_soft_reset(struct domain *source_d, struct domain *dest_d,
+                      xen_pfn_t *gmfn_start)
+{
+    int rc = 0;
+    unsigned long mfn, mfn_new, gmfn, last_gmfn, count;
+    unsigned int order;
+    p2m_type_t p2mt;
+    struct page_info *page, *new_page, *next_page;
+    int drop_dom_ref, copy_page;
+
+    last_gmfn = domain_get_maximum_gpfn(source_d);
+    gmfn = *gmfn_start;
+    while ( gmfn <= last_gmfn )
+    {
+        page = get_page_from_gfn(source_d, gmfn, &p2mt, 0);
+        if ( unlikely(page == NULL) )
+        {
+#ifdef CONFIG_X86
+            struct p2m_domain *p2m = p2m_get_hostp2m(source_d);
+            p2m_access_t p2ma;
+            mfn_t mfn_p2m;
+
+            order = 0;
+            mfn_p2m = get_gfn_type_access(p2m, gmfn,
+                                          &p2mt, &p2ma, 0, &order);
+            if ( p2m_is_pod(p2mt) )
+            {
+                rc = guest_physmap_mark_populate_on_demand(dest_d, gmfn,
+                                                           order);
+                if ( unlikely(rc) )
+                {
+                    printk(XENLOG_G_ERR "Failed to mark Dom%d's GFN %lx"
+                           " (order: %d) as PoD\n", source_d->domain_id,
+                           gmfn, order);
+                    goto fail;
+                }
+            }
+            put_gfn(source_d, gmfn);
+            gmfn += 1ul << order;
+#else
+            gmfn++;
+#endif
+            goto preempt_check;
+        }
+
+        mfn = page_to_mfn(page);
+        if ( unlikely(!mfn_valid(mfn)) )
+        {
+            printk(XENLOG_G_WARNING "Dom%d's GFN %lx points to invalid MFN\n",
+                   source_d->domain_id, gmfn);
+            put_page(page);
+            gmfn++;
+            goto preempt_check;
+        }
+
+        next_page = page;
+        count = 0;
+        copy_page = 0;
+        while ( next_page && !is_xen_heap_page(next_page) &&
+                page_to_mfn(next_page) == mfn + count )
+        {
+            /*
+             * A normal page is supposed to be allocated and have count_info=2
+             * (1 from the domain and 1 from get_page_from_gfn() above).
+             * We should never see an unallocated page here and a page with
+             * count_info > 2 has to be copied.
+             */
+            if ( unlikely(!(next_page->count_info & PGC_allocated)) )
+            {
+                printk(XENLOG_G_ERR "Dom%d's page for GFN %lx is not"
+                       " allocated\n", source_d->domain_id, gmfn);
+                put_page(next_page);
+                rc = -EFAULT;
+                goto fail;
+            }
+            else if ( (next_page->count_info & PGC_count_mask) != 2 )
+            {
+                if ( !count )
+                    copy_page = 1;
+                break;
+            }
+
+            count++;
+            drop_dom_ref = 0;
+            spin_lock(&source_d->page_alloc_lock);
+            page_set_owner(next_page, NULL);
+            page_list_del(next_page, &source_d->page_list);
+            source_d->tot_pages--;
+            if ( unlikely(source_d->tot_pages == 0) )
+                drop_dom_ref = 1;
+            spin_unlock(&source_d->page_alloc_lock);
+            put_page(next_page);
+            if ( unlikely(drop_dom_ref) )
+                put_domain(source_d);
+
+            if ( unlikely(assign_pages(dest_d, next_page, 0, 0)) )
+            {
+                printk(XENLOG_G_ERR "Failed to assign Dom%d's page (GFN %lx)"
+                       " to Dom %d (tot_pages: %u, max_pages: %u)\n",
+                       source_d->domain_id, gmfn, dest_d->domain_id,
+                       dest_d->tot_pages, dest_d->max_pages);
+                rc = -EFAULT;
+                goto fail;
+            }
+
+            if ( unlikely(gmfn + count > last_gmfn) )
+            {
+                next_page = NULL;
+                break;
+            }
+
+            next_page = get_page_from_gfn(source_d, gmfn + count, &p2mt, 0);
+        }
+
+        if ( count )
+        {
+            if ( next_page )
+                put_page(next_page);
+        }
+        else if ( unlikely(copy_page) )
+        {
+            new_page = alloc_domheap_page(dest_d, 0);
+            if ( unlikely(new_page == NULL) )
+            {
+                printk(XENLOG_G_ERR "Failed to alloc a page to replace"
+                       " Dom%d's GFN %lx (MFN %lx) for Dom %d\n",
+                       source_d->domain_id, gmfn, mfn, dest_d->domain_id);
+                rc = -ENOMEM;
+                put_page(page);
+                goto fail;
+            }
+            mfn_new = page_to_mfn(new_page);
+            copy_domain_page(mfn_new, mfn);
+            mfn = mfn_new;
+            put_page(page);
+            if ( unlikely(guest_physmap_add_page(dest_d, gmfn, mfn, 0)) )
+            {
+                printk(XENLOG_G_ERR "Failed to add new GFN %lx"
+                       " (MFN %lx) to Dom%d\n",
+                       gmfn, mfn, dest_d->domain_id);
+                rc = -EFAULT;
+                goto fail;
+            }
+            gmfn++;
+        }
+        else
+        {
+            put_page(page);
+            gmfn++;
+            goto preempt_check;
+        }
+
+        while ( count )
+        {
+            order = get_order_from_pages(count);
+            if ( (1ul << order) > count )
+                order--;
+
+            guest_physmap_remove_page(source_d, gmfn, mfn, order);
+
+            if ( unlikely(guest_physmap_add_page(dest_d, gmfn, mfn, order)) )
+            {
+                printk(XENLOG_G_ERR "Failed to re-add Dom%d's GFN %lx"
+                       " (MFN %lx, order: %u) to Dom%d\n", source_d->domain_id,
+                       gmfn, mfn, order, dest_d->domain_id);
+                rc = -EFAULT;
+                goto fail;
+            }
+
+            count -= 1ul << order;
+            gmfn += 1ul << order;
+            mfn += 1ul << order;
+        }
+
+ preempt_check:
+        if ( hypercall_preempt_check() && gmfn <= last_gmfn )
+        {
+            *gmfn_start = gmfn;
+            return -ERESTART;
+        }
+    }
+
+ fail:
+    return rc;
+}
+
 int vcpu_reset(struct vcpu *v)
 {
     struct domain *d = v->domain;
diff --git a/xen/common/domctl.c b/xen/common/domctl.c
index e571e76..2fd21cb 100644
--- a/xen/common/domctl.c
+++ b/xen/common/domctl.c
@@ -703,6 +703,78 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xen_domctl_t) u_domctl)
         break;
     }
 
+    case XEN_DOMCTL_soft_reset:
+    {
+        struct domain *dest_d;
+
+        if ( d == current->domain )
+        {
+            ret = -EINVAL;
+            break;
+        }
+
+        dest_d = rcu_lock_domain_by_any_id(op->u.soft_reset.dest_domain);
+        if ( dest_d == NULL )
+        {
+            ret = -ESRCH;
+            break;
+        }
+
+        if ( d == dest_d || dest_d->is_dying || dest_d == current->domain )
+        {
+            ret = -EINVAL;
+            rcu_unlock_domain(dest_d);
+            break;
+        }
+
+        /*
+         * Mark the source domain as dying to prevent further changes of its
+         * mappings. is_dying flag is protected by domctl_lock.
+         */
+        if ( !d->is_dying )
+        {
+            domain_pause(d);
+            d->is_dying = DOMDYING_locked;
+        }
+        else if ( d->is_dying != DOMDYING_locked )
+        {
+            ret = -EINVAL;
+            rcu_unlock_domain(dest_d);
+            break;
+        }
+
+        /*
+         * Destination domain's paused state is preserved across possible
+         * hypercall preemption.
+         */
+        if ( !dest_d->is_soft_reset )
+        {
+            domain_pause(dest_d);
+            dest_d->is_soft_reset = 1;
+        }
+
+        copyback = 1;
+        ret = domain_soft_reset(d, dest_d, &op->u.soft_reset.gmfn_start);
+
+        if ( ret != -ERESTART )
+        {
+            dest_d->is_soft_reset = 0;
+            domain_unpause(dest_d);
+            rcu_unlock_domain(dest_d);
+        }
+        else
+        {
+            rcu_unlock_domain(dest_d);
+            ret = hypercall_create_continuation(
+                __HYPERVISOR_domctl, "h", u_domctl);
+            break;
+        }
+
+        /* Change cmd to support possible preemption of domain_kill() */
+        op->cmd = XEN_DOMCTL_destroydomain;
+        /* fall through */
+    }
+
     case XEN_DOMCTL_destroydomain:
         ret = domain_kill(d);
         if ( ret == -ERESTART )
diff --git a/xen/include/public/domctl.h b/xen/include/public/domctl.h
index 0c0ea4a..8a5db55 100644
--- a/xen/include/public/domctl.h
+++ b/xen/include/public/domctl.h
@@ -1065,6 +1065,32 @@ struct xen_domctl_monitor_op {
 typedef struct xen_domctl__op xen_domctl_monitor_op_t;
 DEFINE_XEN_GUEST_HANDLE(xen_domctl_monitor_op_t);
 
+/*
+ * Transfer all memory pages from one domain to the other. Pages are unmapped
+ * from the source domain and mapped at the same GFNs to the destination
+ * domain. The source domain is being destroyed.
+ *
+ * If a particular page is mapped more then once in the source domain a new
+ * empty page is being allocated for the destination domain and the content is
+ * being copied.
+ *
+ * The caller has to be priviliged and it is supposed to set gmfn_start to 0,
+ * this field is required for the hypercall continuation.
+ */
+struct xen_domctl_soft_reset {
+    /*
+     * [IN] Destination domain.
+     */
+    domid_t  dest_domain; /* destination domain */
+
+    /*
+     * [IN] Start from GMFN.
+     */
+    xen_pfn_t gmfn_start;
+};
+typedef struct xen_domctl_soft_reset xen_domctl_soft_reset_t;
+DEFINE_XEN_GUEST_HANDLE(xen_domctl_soft_reset_t);
+
 struct xen_domctl {
     uint32_t cmd;
 #define XEN_DOMCTL_createdomain                   1
@@ -1140,6 +1166,7 @@ struct xen_domctl {
 #define XEN_DOMCTL_setvnumainfo                  74
 #define XEN_DOMCTL_psr_cmt_op                    75
 #define XEN_DOMCTL_monitor_op                    77
+#define XEN_DOMCTL_soft_reset                    78
 #define XEN_DOMCTL_gdbsx_guestmemio            1000
 #define XEN_DOMCTL_gdbsx_pausevcpu             1001
 #define XEN_DOMCTL_gdbsx_unpausevcpu           1002
@@ -1203,6 +1230,7 @@ struct xen_domctl {
         struct xen_domctl_vnuma             vnuma;
         struct xen_domctl_psr_cmt_op        psr_cmt_op;
         struct xen_domctl_monitor_op        monitor_op;
+        struct xen_domctl_soft_reset        soft_reset;
         uint8_t                             pad[128];
     } u;
 };
diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h
index f53b91d..48b8272 100644
--- a/xen/include/xen/sched.h
+++ b/xen/include/xen/sched.h
@@ -396,6 +396,9 @@ struct domain
     bool_t           is_shut_down;     /* fully shut down? */
     int              shutdown_code;
 
+    /* Destination domain in the middle of soft reset operation */
+    bool_t           is_soft_reset;
+
     /* If this is not 0, send suspend notification here instead of
      * raising DOM_EXC */
     evtchn_port_t    suspend_evtchn;
@@ -605,6 +608,9 @@ void domain_shutdown(struct domain *d, u8 reason);
 void domain_resume(struct domain *d);
 void domain_pause_for_debugger(void);
 
+int domain_soft_reset(struct domain *source_d, struct domain *dest_d,
+                      xen_pfn_t *gmfn_start);
+
 int vcpu_start_shutdown_deferral(struct vcpu *v);
 void vcpu_end_shutdown_deferral(struct vcpu *v);
 
-- 
1.9.3

Re: [PATCH v7 04/10] xen: Introduce XEN_DOMCTL_soft_reset

[Tim Deegan]

Hi,

At 17:25 +0200 on 27 May (1432747540), Vitaly Kuznetsov wrote:
> New operation reassigns all memory pages from source domain to the destination
> domain mapping them at exactly the same GFNs. Pages mapped more than once (e.g.
> grants) are being copied.
> 
> Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
> ---
> Changes in v7:
> - is_soft_reset flag added to struct domain to preserve destination domain's
>   paused state across possible hypercall preemption.
> [Jan Beulich]
> - XENMEM_soft_reset -> XEN_DOMCTL_soft_reset
> - domain_soft_reset() returns int
> - no locking for is_dying as it is now proteced by domctl_lock
> - print a warning on !mfn_valid case
> - check PGC_allocated for pages
> - no print on assign_pages failure (it prints error messages on both its failure paths)
> - don't re-read page->count_info, use copy_page flag
> - minor stucturing change
> - pause both source and destination domain while processing the hypercall
> - remove nr_transferred from public interface
> [Tim Deegan]
> - use get_gfn_type_access() in PoD case (x86-only)
> ---
>  xen/common/domain.c         | 186 ++++++++++++++++++++++++++++++++++++++++++++
>  xen/common/domctl.c         |  72 +++++++++++++++++
>  xen/include/public/domctl.h |  28 +++++++
>  xen/include/xen/sched.h     |   6 ++
>  4 files changed, 292 insertions(+)
> 
> diff --git a/xen/common/domain.c b/xen/common/domain.c
> index 7825c56..824f325 100644
> --- a/xen/common/domain.c
> +++ b/xen/common/domain.c
> @@ -1006,6 +1006,192 @@ int domain_unpause_by_systemcontroller(struct domain *d)
>      return 0;
>  }
>  
> +int domain_soft_reset(struct domain *source_d, struct domain *dest_d,
> +                      xen_pfn_t *gmfn_start)
> +{
> +    int rc = 0;
> +    unsigned long mfn, mfn_new, gmfn, last_gmfn, count;
> +    unsigned int order;
> +    p2m_type_t p2mt;
> +    struct page_info *page, *new_page, *next_page;
> +    int drop_dom_ref, copy_page;
> +
> +    last_gmfn = domain_get_maximum_gpfn(source_d);
> +    gmfn = *gmfn_start;
> +    while ( gmfn <= last_gmfn )
> +    {
> +        page = get_page_from_gfn(source_d, gmfn, &p2mt, 0);

In order to be safe against p2m changes, you should use
get_gfn_type_access() _here_, and put_gfn() when you're finished with the
gfn.  You'll also need to get_page() the returned MFN, of course, to
make sure that it isn't freed before you reassign it.

> +        if ( unlikely(page == NULL) )
> +        {
> +#ifdef CONFIG_X86
> +            struct p2m_domain *p2m = p2m_get_hostp2m(source_d);
> +            p2m_access_t p2ma;
> +            mfn_t mfn_p2m;
> +
> +            order = 0;
> +            mfn_p2m = get_gfn_type_access(p2m, gmfn,
> +                                          &p2mt, &p2ma, 0, &order);
> +            if ( p2m_is_pod(p2mt) )
> +            {
> +                rc = guest_physmap_mark_populate_on_demand(dest_d, gmfn,
> +                                                           order);
> +                if ( unlikely(rc) )
> +                {
> +                    printk(XENLOG_G_ERR "Failed to mark Dom%d's GFN %lx"
> +                           " (order: %d) as PoD\n", source_d->domain_id,
> +                           gmfn, order);
> +                    goto fail;
> +                }
> +            }
> +            put_gfn(source_d, gmfn);
> +            gmfn += 1ul << order;
> +#else
> +            gmfn++;
> +#endif
> +            goto preempt_check;
> +        }
> +
> +        mfn = page_to_mfn(page);
> +        if ( unlikely(!mfn_valid(mfn)) )
> +        {
> +            printk(XENLOG_G_WARNING "Dom%d's GFN %lx points to invalid MFN\n",
> +                   source_d->domain_id, gmfn);
> +            put_page(page);
> +            gmfn++;
> +            goto preempt_check;
> +        }
> +
> +        next_page = page;
> +        count = 0;
> +        copy_page = 0;
> +        while ( next_page && !is_xen_heap_page(next_page) &&
> +                page_to_mfn(next_page) == mfn + count )

What's the purpose of this second loop?  It doesn't seem to be doing
anything that the outer loop couldn't.

Cheers,

Tim.

Re: [PATCH v7 04/10] xen: Introduce XEN_DOMCTL_soft_reset

[Vitaly Kuznetsov]

Tim Deegan <tim@xen.org> writes:

> Hi,
>
> At 17:25 +0200 on 27 May (1432747540), Vitaly Kuznetsov wrote:
>> New operation reassigns all memory pages from source domain to the destination
>> domain mapping them at exactly the same GFNs. Pages mapped more than once (e.g.
>> grants) are being copied.
>> 
>> Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
>> ---
>> Changes in v7:
>> - is_soft_reset flag added to struct domain to preserve destination domain's
>>   paused state across possible hypercall preemption.
>> [Jan Beulich]
>> - XENMEM_soft_reset -> XEN_DOMCTL_soft_reset
>> - domain_soft_reset() returns int
>> - no locking for is_dying as it is now proteced by domctl_lock
>> - print a warning on !mfn_valid case
>> - check PGC_allocated for pages
>> - no print on assign_pages failure (it prints error messages on both its failure paths)
>> - don't re-read page->count_info, use copy_page flag
>> - minor stucturing change
>> - pause both source and destination domain while processing the hypercall
>> - remove nr_transferred from public interface
>> [Tim Deegan]
>> - use get_gfn_type_access() in PoD case (x86-only)
>> ---
>>  xen/common/domain.c         | 186 ++++++++++++++++++++++++++++++++++++++++++++
>>  xen/common/domctl.c         |  72 +++++++++++++++++
>>  xen/include/public/domctl.h |  28 +++++++
>>  xen/include/xen/sched.h     |   6 ++
>>  4 files changed, 292 insertions(+)
>> 
>> diff --git a/xen/common/domain.c b/xen/common/domain.c
>> index 7825c56..824f325 100644
>> --- a/xen/common/domain.c
>> +++ b/xen/common/domain.c
>> @@ -1006,6 +1006,192 @@ int domain_unpause_by_systemcontroller(struct domain *d)
>>      return 0;
>>  }
>>  
>> +int domain_soft_reset(struct domain *source_d, struct domain *dest_d,
>> +                      xen_pfn_t *gmfn_start)
>> +{
>> +    int rc = 0;
>> +    unsigned long mfn, mfn_new, gmfn, last_gmfn, count;
>> +    unsigned int order;
>> +    p2m_type_t p2mt;
>> +    struct page_info *page, *new_page, *next_page;
>> +    int drop_dom_ref, copy_page;
>> +
>> +    last_gmfn = domain_get_maximum_gpfn(source_d);
>> +    gmfn = *gmfn_start;
>> +    while ( gmfn <= last_gmfn )
>> +    {
>> +        page = get_page_from_gfn(source_d, gmfn, &p2mt, 0);
>
> In order to be safe against p2m changes, you should use
> get_gfn_type_access() _here_, and put_gfn() when you're finished with the
> gfn.  You'll also need to get_page() the returned MFN, of course, to
> make sure that it isn't freed before you reassign it.

The only problem I see is the fact that get_gfn_type_access() is
x86-specific. Actually, I don't see why we can't have
get_gfn_type_access() for ARM: it locks the whole p2m with gfn_lock
(which is just p2m_lock() on x86) and then resolves the gfn. I'm not
sure what should be the right approach for this series: make this
hypercall x86-specific (temporary before we have all the required bits
in ARM) or try to make something up for ARM...

>> +        if ( unlikely(page == NULL) )
>> +        {
>> +#ifdef CONFIG_X86
>> +            struct p2m_domain *p2m = p2m_get_hostp2m(source_d);
>> +            p2m_access_t p2ma;
>> +            mfn_t mfn_p2m;
>> +
>> +            order = 0;
>> +            mfn_p2m = get_gfn_type_access(p2m, gmfn,
>> +                                          &p2mt, &p2ma, 0, &order);
>> +            if ( p2m_is_pod(p2mt) )
>> +            {
>> +                rc = guest_physmap_mark_populate_on_demand(dest_d, gmfn,
>> +                                                           order);
>> +                if ( unlikely(rc) )
>> +                {
>> +                    printk(XENLOG_G_ERR "Failed to mark Dom%d's GFN %lx"
>> +                           " (order: %d) as PoD\n", source_d->domain_id,
>> +                           gmfn, order);
>> +                    goto fail;
>> +                }
>> +            }
>> +            put_gfn(source_d, gmfn);
>> +            gmfn += 1ul << order;
>> +#else
>> +            gmfn++;
>> +#endif
>> +            goto preempt_check;
>> +        }
>> +
>> +        mfn = page_to_mfn(page);
>> +        if ( unlikely(!mfn_valid(mfn)) )
>> +        {
>> +            printk(XENLOG_G_WARNING "Dom%d's GFN %lx points to invalid MFN\n",
>> +                   source_d->domain_id, gmfn);
>> +            put_page(page);
>> +            gmfn++;
>> +            goto preempt_check;
>> +        }
>> +
>> +        next_page = page;
>> +        count = 0;
>> +        copy_page = 0;
>> +        while ( next_page && !is_xen_heap_page(next_page) &&
>> +                page_to_mfn(next_page) == mfn + count )
>
> What's the purpose of this second loop?  It doesn't seem to be doing
> anything that the outer loop couldn't.

True. This second loops searches for a continuous region to preserve the
order of mappings (when possible) but as it advances the gmfn it doesn't
bring any performance penalty. I was under an impression it makes this
code easier to read but if you think it doesn't I won't object.

Thanks,

-- 
  Vitaly

Re: [PATCH v7 04/10] xen: Introduce XEN_DOMCTL_soft_reset

[Tim Deegan]

At 13:56 +0200 on 28 May (1432821360), Vitaly Kuznetsov wrote:
> Tim Deegan <tim@xen.org> writes:
> >> +    last_gmfn = domain_get_maximum_gpfn(source_d);
> >> +    gmfn = *gmfn_start;
> >> +    while ( gmfn <= last_gmfn )
> >> +    {
> >> +        page = get_page_from_gfn(source_d, gmfn, &p2mt, 0);
> >
> > In order to be safe against p2m changes, you should use
> > get_gfn_type_access() _here_, and put_gfn() when you're finished with the
> > gfn.  You'll also need to get_page() the returned MFN, of course, to
> > make sure that it isn't freed before you reassign it.
> 
> The only problem I see is the fact that get_gfn_type_access() is
> x86-specific. Actually, I don't see why we can't have
> get_gfn_type_access() for ARM: it locks the whole p2m with gfn_lock
> (which is just p2m_lock() on x86) and then resolves the gfn. I'm not
> sure what should be the right approach for this series: make this
> hypercall x86-specific (temporary before we have all the required bits
> in ARM) or try to make something up for ARM...

I think it would be best to add this call to ARM.

> >> +        while ( next_page && !is_xen_heap_page(next_page) &&
> >> +                page_to_mfn(next_page) == mfn + count )
> >
> > What's the purpose of this second loop?  It doesn't seem to be doing
> > anything that the outer loop couldn't.
> 
> True. This second loops searches for a continuous region to preserve the
> order of mappings (when possible)

Ah; I think this, like the PoD case, should the more detailed p2m
lookup to get the actual order of the current mapping.  That should be
shorter and more readable, and probably more correct.

Cheers,

Tim.

Re: [PATCH v7 04/10] xen: Introduce XEN_DOMCTL_soft_reset

[Vitaly Kuznetsov]

Tim Deegan <tim@xen.org> writes:

> At 13:56 +0200 on 28 May (1432821360), Vitaly Kuznetsov wrote:
>> Tim Deegan <tim@xen.org> writes:
>> >> +    last_gmfn = domain_get_maximum_gpfn(source_d);
>> >> +    gmfn = *gmfn_start;
>> >> +    while ( gmfn <= last_gmfn )
>> >> +    {
>> >> +        page = get_page_from_gfn(source_d, gmfn, &p2mt, 0);
>> >
>> > In order to be safe against p2m changes, you should use
>> > get_gfn_type_access() _here_, and put_gfn() when you're finished with the
>> > gfn.  You'll also need to get_page() the returned MFN, of course, to
>> > make sure that it isn't freed before you reassign it.
>> 
>> The only problem I see is the fact that get_gfn_type_access() is
>> x86-specific. Actually, I don't see why we can't have
>> get_gfn_type_access() for ARM: it locks the whole p2m with gfn_lock
>> (which is just p2m_lock() on x86) and then resolves the gfn. I'm not
>> sure what should be the right approach for this series: make this
>> hypercall x86-specific (temporary before we have all the required bits
>> in ARM) or try to make something up for ARM...
>
> I think it would be best to add this call to ARM.
>
>> >> +        while ( next_page && !is_xen_heap_page(next_page) &&
>> >> +                page_to_mfn(next_page) == mfn + count )
>> >
>> > What's the purpose of this second loop?  It doesn't seem to be doing
>> > anything that the outer loop couldn't.
>> 
>> True. This second loops searches for a continuous region to preserve the
>> order of mappings (when possible)
>
> Ah; I think this, like the PoD case, should the more detailed p2m
> lookup to get the actual order of the current mapping.  That should be
> shorter and more readable, and probably more correct.

If we bring get_gfn_type_access() call to the top level it becomes
possible (and easy) but (if I'm not mistaken) we still need to walk
through all pages of the mapping checking that each of them has the
reqiuired count_info (so it is not mapped from other domain or xen
itself). In case we meet a 'bad' page we'll have to split the mapping
(and copy the page itself).

-- 
  Vitaly

Re: [PATCH v7 04/10] xen: Introduce XEN_DOMCTL_soft_reset

[Tim Deegan]

At 15:11 +0200 on 28 May (1432825919), Vitaly Kuznetsov wrote:
> Tim Deegan <tim@xen.org> writes:
> > At 13:56 +0200 on 28 May (1432821360), Vitaly Kuznetsov wrote:
> >> Tim Deegan <tim@xen.org> writes:
> >> >> +        while ( next_page && !is_xen_heap_page(next_page) &&
> >> >> +                page_to_mfn(next_page) == mfn + count )
> >> >
> >> > What's the purpose of this second loop?  It doesn't seem to be doing
> >> > anything that the outer loop couldn't.
> >> 
> >> True. This second loops searches for a continuous region to preserve the
> >> order of mappings (when possible)
> >
> > Ah; I think this, like the PoD case, should the more detailed p2m
> > lookup to get the actual order of the current mapping.  That should be
> > shorter and more readable, and probably more correct.
> 
> If we bring get_gfn_type_access() call to the top level it becomes
> possible (and easy) but (if I'm not mistaken) we still need to walk
> through all pages of the mapping checking that each of them has the
> reqiuired count_info (so it is not mapped from other domain or xen
> itself). In case we meet a 'bad' page we'll have to split the mapping
> (and copy the page itself).

Hmmm.  Yes, we can only reassign one page at a time.  I think that
will look cleaner if you split out the reassign-or-copy into a
separate function that takes a start + order and DTRT, and then having
the loop in this function handle one p2m entry (of whatever order) per
iteration.

BTW having looked at how messy this is ending up, and how it's still
incomplete, I'd agree with Jan that resetting the domain state might
be a better approach.

Tim.

Re: [PATCH v7 04/10] xen: Introduce XEN_DOMCTL_soft_reset

[Vitaly Kuznetsov]

Tim Deegan <tim@xen.org> writes:

> At 15:11 +0200 on 28 May (1432825919), Vitaly Kuznetsov wrote:
>> Tim Deegan <tim@xen.org> writes:
>> > At 13:56 +0200 on 28 May (1432821360), Vitaly Kuznetsov wrote:
>> >> Tim Deegan <tim@xen.org> writes:
>> >> >> +        while ( next_page && !is_xen_heap_page(next_page) &&
>> >> >> +                page_to_mfn(next_page) == mfn + count )
>> >> >
>> >> > What's the purpose of this second loop?  It doesn't seem to be doing
>> >> > anything that the outer loop couldn't.
>> >> 
>> >> True. This second loops searches for a continuous region to preserve the
>> >> order of mappings (when possible)
>> >
>> > Ah; I think this, like the PoD case, should the more detailed p2m
>> > lookup to get the actual order of the current mapping.  That should be
>> > shorter and more readable, and probably more correct.
>> 
>> If we bring get_gfn_type_access() call to the top level it becomes
>> possible (and easy) but (if I'm not mistaken) we still need to walk
>> through all pages of the mapping checking that each of them has the
>> reqiuired count_info (so it is not mapped from other domain or xen
>> itself). In case we meet a 'bad' page we'll have to split the mapping
>> (and copy the page itself).
>
> Hmmm.  Yes, we can only reassign one page at a time.  I think that
> will look cleaner if you split out the reassign-or-copy into a
> separate function that takes a start + order and DTRT, and then having
> the loop in this function handle one p2m entry (of whatever order) per
> iteration.
>
> BTW having looked at how messy this is ending up, and how it's still
> incomplete, I'd agree with Jan that resetting the domain state might
> be a better approach.

Even with the 'reset-everything' approach the function from this patch
will still be required in some form as we'll still have to walk the p2m
and each individual page checking it's count_info and replacing in case
of need. At the same time we'll have lots of other hypervisor-related
implications (tearing down everything) so I seriously doubt it's going
to end up less messy (toolstack-related changes might go away though).

-- 
  Vitaly

Re: [PATCH v7 04/10] xen: Introduce XEN_DOMCTL_soft_reset

[Jan Beulich]

>>> On 28.05.15 at 15:53, <vkuznets@redhat.com> wrote:
> Tim Deegan <tim@xen.org> writes:
>> BTW having looked at how messy this is ending up, and how it's still
>> incomplete, I'd agree with Jan that resetting the domain state might
>> be a better approach.
> 
> Even with the 'reset-everything' approach the function from this patch
> will still be required in some form as we'll still have to walk the p2m
> and each individual page checking it's count_info and replacing in case
> of need. At the same time we'll have lots of other hypervisor-related
> implications (tearing down everything) so I seriously doubt it's going
> to end up less messy (toolstack-related changes might go away though).

That would remain to be seen. I'm particularly unconvinced that
iterating over all pages an copying is going to be needed: Iirc it
was only grants mapped by another domain that we really need
to hunt down, and we should be able to do that without iterating
over the whole GFN space of the domain (or all of its assigned
pages).

Jan

[PATCH v7 05/10] xsm: add XEN_DOMCTL_soft_reset support

[Vitaly Kuznetsov]

Dummy policy just checks that the current domain is privileged. In the FLASK
policy two new vectors are added to the DOMAIN2 class: soft_reset and
reset_transfer. First one is being used to check that the domain making the
hypercall is allowed to do it, the second is being used that is's possible
to trasfer memory from source domain to destination domain. The default policy
requires their contexts to match.

Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
---
Changes in v7:
- Add reset_transfer vector to FLASK [Daniel De Graaf]
- XENMEM_soft_reset -> XEN_DOMCTL_soft_reset
- Add a comment on why we don't do two XSM_TARGET checks in dummy.h [Jan Beulich]
---
 tools/flask/policy/policy/modules/xen/xen.if |  3 ++-
 xen/common/domctl.c                          |  7 +++++++
 xen/include/xsm/dummy.h                      | 12 ++++++++++++
 xen/include/xsm/xsm.h                        |  8 ++++++++
 xen/xsm/dummy.c                              |  2 ++
 xen/xsm/flask/hooks.c                        | 18 ++++++++++++++++++
 xen/xsm/flask/policy/access_vectors          |  7 +++++++
 7 files changed, 56 insertions(+), 1 deletion(-)

diff --git a/tools/flask/policy/policy/modules/xen/xen.if b/tools/flask/policy/policy/modules/xen/xen.if
index 620d151..0f6ed8a 100644
--- a/tools/flask/policy/policy/modules/xen/xen.if
+++ b/tools/flask/policy/policy/modules/xen/xen.if
@@ -51,13 +51,14 @@ define(`create_domain_common', `
 			getaffinity setaffinity setvcpuextstate };
 	allow $1 $2:domain2 { set_cpuid settsc setscheduler setclaim
 			set_max_evtchn set_vnumainfo get_vnumainfo cacheflush
-			psr_cmt_op };
+			psr_cmt_op soft_reset };
 	allow $1 $2:security check_context;
 	allow $1 $2:shadow enable;
 	allow $1 $2:mmu { map_read map_write adjust memorymap physmap pinpage mmuext_op updatemp };
 	allow $1 $2:grant setup;
 	allow $1 $2:hvm { cacheattr getparam hvmctl irqlevel pciroute sethvmc
 			setparam pcilevel trackdirtyvram nested };
+	allow $2 $2:domain2 reset_transfer;
 ')
 
 # create_domain(priv, target)
diff --git a/xen/common/domctl.c b/xen/common/domctl.c
index 2fd21cb..ea7994a 100644
--- a/xen/common/domctl.c
+++ b/xen/common/domctl.c
@@ -727,6 +727,13 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xen_domctl_t) u_domctl)
             break;
         }
 
+        ret = xsm_soft_reset(XSM_PRIV, d, dest_d);
+        if ( ret )
+        {
+            rcu_unlock_domain(dest_d);
+            break;
+        }
+
         /*
          * Mark the source domain as dying to prevent further changes of its
          * mappings. is_dying flag is protected by domctl_lock.
diff --git a/xen/include/xsm/dummy.h b/xen/include/xsm/dummy.h
index f044c0f..ba44f87 100644
--- a/xen/include/xsm/dummy.h
+++ b/xen/include/xsm/dummy.h
@@ -193,6 +193,18 @@ static XSM_INLINE int xsm_memory_exchange(XSM_DEFAULT_ARG struct domain *d)
     return xsm_default_action(action, current->domain, d);
 }
 
+static XSM_INLINE int xsm_soft_reset(XSM_DEFAULT_ARG struct domain *d1,
+				     struct domain *d2)
+{
+    /*
+     * As it is not possible for a domain to have more than one target at a
+     * time the result of doing XSM_TARGET check for both domains would be
+     * equivalent to the XSM_PRIV check below.
+     */
+    XSM_ASSERT_ACTION(XSM_PRIV);
+    return xsm_default_action(action, current->domain, NULL);
+}
+
 static XSM_INLINE int xsm_memory_adjust_reservation(XSM_DEFAULT_ARG struct domain *d1,
                                                             struct domain *d2)
 {
diff --git a/xen/include/xsm/xsm.h b/xen/include/xsm/xsm.h
index c872d44..6edddfa 100644
--- a/xen/include/xsm/xsm.h
+++ b/xen/include/xsm/xsm.h
@@ -102,6 +102,8 @@ struct xsm_operations {
     int (*kexec) (void);
     int (*schedop_shutdown) (struct domain *d1, struct domain *d2);
 
+    int (*soft_reset) (struct domain *d1, struct domain *d2);
+
     char *(*show_irq_sid) (int irq);
     int (*map_domain_pirq) (struct domain *d);
     int (*map_domain_irq) (struct domain *d, int irq, void *data);
@@ -351,6 +353,12 @@ static inline int xsm_memory_exchange (xsm_default_t def, struct domain *d)
     return xsm_ops->memory_exchange(d);
 }
 
+static inline int xsm_soft_reset (xsm_default_t def, struct domain *d1,
+				  struct domain *d2)
+{
+    return xsm_ops->soft_reset(d1, d2);
+}
+
 static inline int xsm_memory_adjust_reservation (xsm_default_t def, struct domain *d1, struct
                                                                     domain *d2)
 {
diff --git a/xen/xsm/dummy.c b/xen/xsm/dummy.c
index e84b0e4..0f1158e 100644
--- a/xen/xsm/dummy.c
+++ b/xen/xsm/dummy.c
@@ -76,6 +76,8 @@ void xsm_fixup_ops (struct xsm_operations *ops)
     set_to_dummy_if_null(ops, kexec);
     set_to_dummy_if_null(ops, schedop_shutdown);
 
+    set_to_dummy_if_null(ops, soft_reset);
+
     set_to_dummy_if_null(ops, show_irq_sid);
     set_to_dummy_if_null(ops, map_domain_pirq);
     set_to_dummy_if_null(ops, map_domain_irq);
diff --git a/xen/xsm/flask/hooks.c b/xen/xsm/flask/hooks.c
index 11b7453..7ade3eb 100644
--- a/xen/xsm/flask/hooks.c
+++ b/xen/xsm/flask/hooks.c
@@ -565,6 +565,21 @@ static int flask_set_target(struct domain *d, struct domain *t)
     return rc;
 }
 
+static int flask_soft_reset(struct domain *d1, struct domain *d2)
+{
+    int rc;
+
+    rc = current_has_perm(d1, SECCLASS_DOMAIN2, DOMAIN2__SOFT_RESET);
+    if (rc)
+        return rc;
+
+    rc = current_has_perm(d2, SECCLASS_DOMAIN2, DOMAIN2__SOFT_RESET);
+    if (rc)
+        return rc;
+
+    return domain_has_perm(d1, d2, SECCLASS_DOMAIN2, DOMAIN2__RESET_TRANSFER);
+}
+
 static int flask_domctl(struct domain *d, int cmd)
 {
     switch ( cmd )
@@ -578,6 +593,7 @@ static int flask_domctl(struct domain *d, int cmd)
     case XEN_DOMCTL_memory_mapping:
     case XEN_DOMCTL_set_target:
     case XEN_DOMCTL_vm_event_op:
+    case XEN_DOMCTL_soft_reset:
 
     /* These have individual XSM hooks (arch/../domctl.c) */
     case XEN_DOMCTL_bind_pt_irq:
@@ -1629,6 +1645,8 @@ static struct xsm_operations flask_ops = {
     .kexec = flask_kexec,
     .schedop_shutdown = flask_schedop_shutdown,
 
+    .soft_reset = flask_soft_reset,
+
     .show_irq_sid = flask_show_irq_sid,
 
     .map_domain_pirq = flask_map_domain_pirq,
diff --git a/xen/xsm/flask/policy/access_vectors b/xen/xsm/flask/policy/access_vectors
index ea556df..5611760 100644
--- a/xen/xsm/flask/policy/access_vectors
+++ b/xen/xsm/flask/policy/access_vectors
@@ -224,6 +224,13 @@ class domain2
 # XEN_DOMCTL_monitor_op
 # XEN_DOMCTL_vm_event_op
     vm_event
+# XEN_DOMCTL_soft_reset:
+#  source = domain making the hypercall
+#  target = domain being reset (source or destination)
+    soft_reset
+#  source = source domain being reset
+#  target = destination domain being reset
+    reset_transfer
 # XENMEM_access_op
     mem_access
 # XENMEM_paging_op
-- 
1.9.3

Re: [PATCH v7 05/10] xsm: add XEN_DOMCTL_soft_reset support

[Daniel De Graaf]

On 05/27/2015 11:25 AM, Vitaly Kuznetsov wrote:
> Dummy policy just checks that the current domain is privileged. In the FLASK
> policy two new vectors are added to the DOMAIN2 class: soft_reset and
> reset_transfer. First one is being used to check that the domain making the
> hypercall is allowed to do it, the second is being used that is's possible
       vvvv      two typos in the commit message                ^^^^
> to trasfer memory from source domain to destination domain. The default policy
> requires their contexts to match.
>
> Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>

Acked-by: Daniel De Graaf <dgdegra@tycho.nsa.gov>

Re: [PATCH v7 05/10] xsm: add XEN_DOMCTL_soft_reset support

[Jan Beulich]

>>> On 27.05.15 at 17:25, <vkuznets@redhat.com> wrote:
> +static XSM_INLINE int xsm_soft_reset(XSM_DEFAULT_ARG struct domain *d1,
> +				     struct domain *d2)

Hard tabs used here ...

> +static inline int xsm_soft_reset (xsm_default_t def, struct domain *d1,
> +				  struct domain *d2)

... and here.

Jan

[PATCH v7 06/10] libxc: support XEN_DOMCTL_soft_reset operation

[Vitaly Kuznetsov]

Introduce xc_domain_soft_reset() function supporting XEN_DOMCTL_soft_reset.

Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
---
Changes in v7:
- XENMEM_soft_reset -> XEN_DOMCTL_soft_reset.
- xc_memory_soft_reset -> xc_domain_soft_reset
---
 tools/libxc/include/xenctrl.h |  4 ++++
 tools/libxc/xc_domain.c       | 12 ++++++++++++
 2 files changed, 16 insertions(+)

diff --git a/tools/libxc/include/xenctrl.h b/tools/libxc/include/xenctrl.h
index 09a7450..ceb64d5 100644
--- a/tools/libxc/include/xenctrl.h
+++ b/tools/libxc/include/xenctrl.h
@@ -1301,6 +1301,10 @@ int xc_domain_setvnuma(xc_interface *xch,
                         unsigned int *vcpu_to_vnode,
                         unsigned int *vnode_to_pnode);
 
+int xc_domain_soft_reset(xc_interface *xch,
+                         uint32_t source_domid,
+                         uint32_t dest_domid);
+
 #if defined(__i386__) || defined(__x86_64__)
 /*
  * PC BIOS standard E820 types and structure.
diff --git a/tools/libxc/xc_domain.c b/tools/libxc/xc_domain.c
index a7079a1..51f2833 100644
--- a/tools/libxc/xc_domain.c
+++ b/tools/libxc/xc_domain.c
@@ -2459,6 +2459,18 @@ int xc_domain_setvnuma(xc_interface *xch,
     return rc;
 }
 
+
+int xc_domain_soft_reset(xc_interface *xch,
+                         uint32_t source_domid,
+                         uint32_t dest_domid)
+{
+    DECLARE_DOMCTL;
+    domctl.cmd = XEN_DOMCTL_soft_reset;
+    domctl.domain = (domid_t)source_domid;
+    domctl.u.soft_reset.dest_domain = (domid_t)dest_domid;
+    domctl.u.soft_reset.gmfn_start = 0;
+    return do_domctl(xch, &domctl);
+}
 /*
  * Local variables:
  * mode: C
-- 
1.9.3

Re: [PATCH v7 06/10] libxc: support XEN_DOMCTL_soft_reset operation

[Ian Campbell]

On Wed, 2015-05-27 at 17:25 +0200, Vitaly Kuznetsov wrote:
> Introduce xc_domain_soft_reset() function supporting XEN_DOMCTL_soft_reset.
> 
> Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>

Assuming the hypervisor side is agreed this looks like a valid wrapping
of it:

Acked-by: Ian Campbell <ian.campbell@citrix.com>

[PATCH v7 07/10] libxc: introduce soft reset for HVM domains

[Vitaly Kuznetsov]

Add new xc_soft_reset() function which performs so-called 'soft reset'
for an HVM domain. It is being performed in the following way:
- Save HVM context and all HVM params of the source domain;
- Transfer all the source domain's memory to the destinatio domain with
  XEN_DOMCTL_soft_reset;
- Restore HVM context, HVM params, seed grant table for the new domain.
When the operation succeeds the source domain is being destroyed and the
destination domain is ready to proceed from where the source was stopped.

Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
---
Changes in v7:
- check that xc_domain_getinfo() return the domain we asked for [Julien Grall]
- xc_domain_soft_reset -> xc_soft_reset
- xc_soft_reset destroys the source domain, no need to do it manually
- nr_transfered was removed from hypercall interface, use
  xc_domain_get_pod_target for the source domain instead.
---
 tools/libxc/Makefile           |   1 +
 tools/libxc/include/xenguest.h |  21 ++++
 tools/libxc/xc_soft_reset.c    | 280 +++++++++++++++++++++++++++++++++++++++++
 3 files changed, 302 insertions(+)
 create mode 100644 tools/libxc/xc_soft_reset.c

diff --git a/tools/libxc/Makefile b/tools/libxc/Makefile
index 63878ec..62bc21b 100644
--- a/tools/libxc/Makefile
+++ b/tools/libxc/Makefile
@@ -69,6 +69,7 @@ $(patsubst %.c,%.opic,$(GUEST_SRCS-y)): CFLAGS += -DXG_LIBXL_HVM_COMPAT
 else
 GUEST_SRCS-y += xc_nomigrate.c
 endif
+GUEST_SRCS-y += xc_soft_reset.c
 
 vpath %.c ../../xen/common/libelf
 CFLAGS += -I../../xen/common/libelf
diff --git a/tools/libxc/include/xenguest.h b/tools/libxc/include/xenguest.h
index 7581263..c71b423 100644
--- a/tools/libxc/include/xenguest.h
+++ b/tools/libxc/include/xenguest.h
@@ -147,6 +147,27 @@ int xc_domain_restore2(xc_interface *xch, int io_fd, uint32_t dom,
  * of the new domain is automatically appended to the filename,
  * separated by a ".".
  */
+
+/**
+ * This function does soft reset for a domain. During soft reset all
+ * source domain's memory is being reassigned to the destination domain,
+ * HVM context and HVM params are being copied, source domain is being
+ * destroyed.
+ *
+ * @parm xch a handle to an open hypervisor interface
+ * @parm source_dom the id of the source domain
+ * @parm dest_dom the id of the destination domain
+ * @parm console_domid the id of the domain handling console
+ * @parm console_mfn returned with the mfn of the console page
+ * @parm store_domid the id of the domain handling store
+ * @parm store_mfn returned with the mfn of the store page
+ * @return 0 on success, -1 on failure
+ */
+int xc_soft_reset(xc_interface *xch, uint32_t source_dom,
+		  uint32_t dest_dom, domid_t console_domid,
+		  unsigned long *console_mfn, domid_t store_domid,
+		  unsigned long *store_mfn);
+
 #define XC_DEVICE_MODEL_RESTORE_FILE "/var/lib/xen/qemu-resume"
 
 /**
diff --git a/tools/libxc/xc_soft_reset.c b/tools/libxc/xc_soft_reset.c
new file mode 100644
index 0000000..ea07470
--- /dev/null
+++ b/tools/libxc/xc_soft_reset.c
@@ -0,0 +1,280 @@
+/******************************************************************************
+ * xc_soft_reset.c
+ *
+ * Do soft reset.
+ *
+ * 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;
+ * version 2.1 of the License.
+ *
+ * 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 <inttypes.h>
+#include <time.h>
+#include <stdlib.h>
+#include <unistd.h>
+#include <sys/time.h>
+
+#include "xc_private.h"
+#include "xc_core.h"
+#include "xc_bitops.h"
+#include "xc_dom.h"
+#include "xg_private.h"
+#include "xg_save_restore.h"
+
+#include <xen/hvm/params.h>
+
+int xc_soft_reset(xc_interface *xch, uint32_t source_dom,
+                  uint32_t dest_dom, domid_t console_domid,
+                  unsigned long *console_mfn, domid_t store_domid,
+                  unsigned long *store_mfn)
+{
+    xc_dominfo_t old_info, new_info;
+    int rc = 1;
+
+    uint32_t hvm_buf_size = 0;
+    uint8_t *hvm_buf = NULL;
+    unsigned long console_pfn, store_pfn, io_pfn, buffio_pfn;
+    uint64_t hvm_params[HVM_NR_PARAMS];
+    uint64_t pod_info[3];
+    xen_pfn_t sharedinfo_pfn;
+
+    DPRINTF("%s: soft reset domid %u -> %u", __func__, source_dom, dest_dom);
+
+    if ( xc_domain_getinfo(xch, source_dom, 1, &old_info) != 1 ||
+         old_info.domid != source_dom )
+    {
+        PERROR("Could not get old domain's info");
+        return 1;
+    }
+
+    if ( xc_domain_get_pod_target(xch, source_dom, &pod_info[0], &pod_info[1],
+                                  &pod_info[2]) )
+    {
+        PERROR("Could not get old domain's PoD info");
+        return 1;
+    }
+
+    if ( xc_domain_getinfo(xch, dest_dom, 1, &new_info) != 1 ||
+         new_info.domid != dest_dom )
+    {
+        PERROR("Could not get new domain's info");
+        return 1;
+    }
+
+    if ( !old_info.hvm || !new_info.hvm )
+    {
+        PERROR("Soft reset is supported for HVM only");
+        return 1;
+    }
+
+    sharedinfo_pfn = old_info.shared_info_frame;
+    if ( xc_get_pfn_type_batch(xch, source_dom, 1, &sharedinfo_pfn) )
+    {
+        PERROR("xc_get_pfn_type_batch failed");
+        goto out;
+    }
+
+    hvm_buf_size = xc_domain_hvm_getcontext(xch, source_dom, 0, 0);
+    if ( hvm_buf_size == -1 )
+    {
+        PERROR("Couldn't get HVM context size from Xen");
+        goto out;
+    }
+
+    hvm_buf = malloc(hvm_buf_size);
+    if ( !hvm_buf )
+    {
+        ERROR("Couldn't allocate memory");
+        goto out;
+    }
+
+    if ( xc_domain_hvm_getcontext(xch, source_dom, hvm_buf,
+                                  hvm_buf_size) == -1 )
+    {
+        PERROR("HVM:Could not get hvm buffer");
+        goto out;
+    }
+
+    xc_hvm_param_get(xch, source_dom, HVM_PARAM_STORE_PFN,
+                     &hvm_params[HVM_PARAM_STORE_PFN]);
+    store_pfn = hvm_params[HVM_PARAM_STORE_PFN];
+    *store_mfn = store_pfn;
+
+    xc_hvm_param_get(xch, source_dom,
+                     HVM_PARAM_CONSOLE_PFN,
+                     &hvm_params[HVM_PARAM_CONSOLE_PFN]);
+    console_pfn = hvm_params[HVM_PARAM_CONSOLE_PFN];
+    *console_mfn = console_pfn;
+
+    xc_hvm_param_get(xch, source_dom, HVM_PARAM_BUFIOREQ_PFN,
+                     &hvm_params[HVM_PARAM_BUFIOREQ_PFN]);
+    buffio_pfn = hvm_params[HVM_PARAM_BUFIOREQ_PFN];
+
+    xc_hvm_param_get(xch, source_dom, HVM_PARAM_IOREQ_PFN,
+                     &hvm_params[HVM_PARAM_IOREQ_PFN]);
+    io_pfn = hvm_params[HVM_PARAM_IOREQ_PFN];
+
+    xc_hvm_param_get(xch, source_dom, HVM_PARAM_IDENT_PT,
+                     &hvm_params[HVM_PARAM_IDENT_PT]);
+
+    xc_hvm_param_get(xch, source_dom, HVM_PARAM_PAGING_RING_PFN,
+                     &hvm_params[HVM_PARAM_PAGING_RING_PFN]);
+
+    xc_hvm_param_get(xch, source_dom, HVM_PARAM_VM86_TSS,
+                     &hvm_params[HVM_PARAM_VM86_TSS]);
+
+    xc_hvm_param_get(xch, source_dom, HVM_PARAM_ACPI_IOPORTS_LOCATION,
+                     &hvm_params[HVM_PARAM_ACPI_IOPORTS_LOCATION]);
+
+    xc_hvm_param_get(xch, source_dom, HVM_PARAM_VIRIDIAN,
+                     &hvm_params[HVM_PARAM_VIRIDIAN]);
+
+    xc_hvm_param_get(xch, source_dom, HVM_PARAM_PAE_ENABLED,
+                     &hvm_params[HVM_PARAM_PAE_ENABLED]);
+
+    xc_hvm_param_get(xch, source_dom, HVM_PARAM_STORE_EVTCHN,
+                     &hvm_params[HVM_PARAM_STORE_EVTCHN]);
+
+    xc_hvm_param_get(xch, source_dom, HVM_PARAM_IOREQ_SERVER_PFN,
+                     &hvm_params[HVM_PARAM_IOREQ_SERVER_PFN]);
+
+    xc_hvm_param_get(xch, source_dom, HVM_PARAM_NR_IOREQ_SERVER_PAGES,
+                     &hvm_params[HVM_PARAM_NR_IOREQ_SERVER_PAGES]);
+
+    xc_hvm_param_get(xch, source_dom, HVM_PARAM_VM_GENERATION_ID_ADDR,
+                     &hvm_params[HVM_PARAM_VM_GENERATION_ID_ADDR]);
+
+    rc = xc_domain_soft_reset(xch, source_dom, dest_dom);
+    if ( rc != 0 )
+    {
+        PERROR("Failed to devour original domain, rc=%d\n", rc);
+        goto out;
+    }
+
+    /*
+     * Shared info frame is being removed when guest maps shared info so this
+     * page is likely XEN_DOMCTL_PFINFO_XTAB but we need to replace it with an
+     * empty page in that case.
+     */
+    if ( sharedinfo_pfn == XEN_DOMCTL_PFINFO_XTAB &&
+         xc_domain_populate_physmap_exact(xch, dest_dom, 1, 0, 0,
+                                          &old_info.shared_info_frame) )
+    {
+        PERROR("Failed to populate pfn %lx (shared info)",
+               old_info.shared_info_frame);
+        goto out;
+    }
+
+    if ( xc_domain_hvm_setcontext(xch, dest_dom, hvm_buf,
+                                  hvm_buf_size) == -1 )
+    {
+        PERROR("HVM:Could not set hvm buffer");
+        goto out;
+    }
+
+    if ( store_pfn )
+        xc_clear_domain_page(xch, dest_dom, store_pfn);
+
+    if ( console_pfn )
+        xc_clear_domain_page(xch, dest_dom, console_pfn);
+
+    if ( buffio_pfn )
+        xc_clear_domain_page(xch, dest_dom, buffio_pfn);
+
+    if ( io_pfn )
+        xc_clear_domain_page(xch, dest_dom, io_pfn);
+
+    xc_hvm_param_set(xch, dest_dom, HVM_PARAM_STORE_PFN,
+                     hvm_params[HVM_PARAM_STORE_PFN]);
+
+    xc_hvm_param_set(xch, dest_dom,
+                     HVM_PARAM_CONSOLE_PFN,
+                     hvm_params[HVM_PARAM_CONSOLE_PFN]);
+
+    xc_hvm_param_set(xch, dest_dom, HVM_PARAM_BUFIOREQ_PFN,
+                     hvm_params[HVM_PARAM_BUFIOREQ_PFN]);
+
+    xc_hvm_param_set(xch, dest_dom, HVM_PARAM_IOREQ_PFN,
+                     hvm_params[HVM_PARAM_IOREQ_PFN]);
+
+    xc_hvm_param_set(xch, dest_dom, HVM_PARAM_IDENT_PT,
+                     hvm_params[HVM_PARAM_IDENT_PT]);
+
+    xc_hvm_param_set(xch, dest_dom, HVM_PARAM_PAGING_RING_PFN,
+                     hvm_params[HVM_PARAM_PAGING_RING_PFN]);
+
+    xc_hvm_param_set(xch, dest_dom, HVM_PARAM_VM86_TSS,
+                     hvm_params[HVM_PARAM_VM86_TSS]);
+
+    xc_hvm_param_set(xch, dest_dom, HVM_PARAM_ACPI_IOPORTS_LOCATION,
+                     hvm_params[HVM_PARAM_ACPI_IOPORTS_LOCATION]);
+
+    xc_hvm_param_set(xch, dest_dom, HVM_PARAM_VIRIDIAN,
+                     hvm_params[HVM_PARAM_VIRIDIAN]);
+
+    xc_hvm_param_set(xch, dest_dom, HVM_PARAM_PAE_ENABLED,
+                     hvm_params[HVM_PARAM_PAE_ENABLED]);
+
+    xc_hvm_param_set(xch, dest_dom, HVM_PARAM_STORE_EVTCHN,
+                     hvm_params[HVM_PARAM_STORE_EVTCHN]);
+
+    xc_hvm_param_set(xch, dest_dom, HVM_PARAM_IOREQ_SERVER_PFN,
+                     hvm_params[HVM_PARAM_IOREQ_SERVER_PFN]);
+
+    xc_hvm_param_set(xch, dest_dom, HVM_PARAM_NR_IOREQ_SERVER_PAGES,
+                     hvm_params[HVM_PARAM_NR_IOREQ_SERVER_PAGES]);
+
+    xc_hvm_param_set(xch, dest_dom, HVM_PARAM_VM_GENERATION_ID_ADDR,
+                     hvm_params[HVM_PARAM_VM_GENERATION_ID_ADDR]);
+
+    if ( xc_dom_gnttab_hvm_seed(xch, dest_dom, console_pfn, store_pfn,
+                               console_domid, store_domid) )
+    {
+        PERROR("Error seeding hvm grant table");
+        goto out;
+    }
+
+    /* Check if we need to add pages to the PoD cache of the new domain */
+    if ( pod_info[2] &&
+         xc_domain_set_pod_target(xch, dest_dom,
+                                  old_info.nr_pages +
+                                  (sharedinfo_pfn == XEN_DOMCTL_PFINFO_XTAB ?
+                                   1 : 0), NULL, NULL, NULL) )
+    {
+        PERROR("Failed to set POD target for new domain");
+        goto out;
+    }
+
+    rc = 0;
+ out:
+    if ( hvm_buf )
+        free(hvm_buf);
+
+    if ( (rc != 0) && (dest_dom != 0) ) {
+            PERROR("Failed to perform soft reset, destroying domain %d",
+                   dest_dom);
+	    xc_domain_destroy(xch, dest_dom);
+    }
+
+    return !!rc;
+}
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * tab-width: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
-- 
1.9.3

Re: [PATCH v7 07/10] libxc: introduce soft reset for HVM domains

[Ian Campbell]

On Wed, 2015-05-27 at 17:25 +0200, Vitaly Kuznetsov wrote:
> Add new xc_soft_reset() function which performs so-called 'soft reset'
> for an HVM domain. It is being performed in the following way:
> - Save HVM context and all HVM params of the source domain;
> - Transfer all the source domain's memory to the destinatio domain with

"destination"

>   XEN_DOMCTL_soft_reset;
> - Restore HVM context, HVM params, seed grant table for the new domain.
> When the operation succeeds the source domain is being destroyed and the

This is sounding a lot like a migration, with the refactoring done for
migration v2 is there any possibility we might be able to reuse any of
that?

(e.g. the list of hvmparams to be dealt with seems like something which
needs to be the same everywhere)

> +    if ( xc_domain_get_pod_target(xch, source_dom, &pod_info[0], &pod_info[1],
> +                                  &pod_info[2]) )

I don't like all the open coded [0], [1] and [2] in the following code
which this implies. You could define symbolic names TOT_PAGES=1, etc but
TBH I think you would be better off just having 3 appropriately named
variables instead of the array.

> +    {
> +        PERROR("Could not get old domain's PoD info");
> +        return 1;
> +    }
> +
> +    if ( xc_domain_getinfo(xch, dest_dom, 1, &new_info) != 1 ||
> +         new_info.domid != dest_dom )
> +    {
> +        PERROR("Could not get new domain's info");
> +        return 1;
> +    }
> +
> +    if ( !old_info.hvm || !new_info.hvm )
> +    {
> +        PERROR("Soft reset is supported for HVM only");
> +        return 1;
> +    }

Should do these sorts of checks before real work, like getting the PoD
info.

> +
> +    sharedinfo_pfn = old_info.shared_info_frame;
> +    if ( xc_get_pfn_type_batch(xch, source_dom, 1, &sharedinfo_pfn) )
> +    {
> +        PERROR("xc_get_pfn_type_batch failed");
> +        goto out;
> +    }
> +
> +    hvm_buf_size = xc_domain_hvm_getcontext(xch, source_dom, 0, 0);
> +    if ( hvm_buf_size == -1 )
> +    {
> +        PERROR("Couldn't get HVM context size from Xen");
> +        goto out;
> +    }
> +
> +    hvm_buf = malloc(hvm_buf_size);
> +    if ( !hvm_buf )
> +    {
> +        ERROR("Couldn't allocate memory");
> +        goto out;
> +    }
> +
> +    if ( xc_domain_hvm_getcontext(xch, source_dom, hvm_buf,
> +                                  hvm_buf_size) == -1 )
> +    {
> +        PERROR("HVM:Could not get hvm buffer");
> +        goto out;
> +    }
> +
> +    xc_hvm_param_get(xch, source_dom, HVM_PARAM_STORE_PFN,
> +                     &hvm_params[HVM_PARAM_STORE_PFN]);
> +    store_pfn = hvm_params[HVM_PARAM_STORE_PFN];
> +    *store_mfn = store_pfn;
> +
> +    xc_hvm_param_get(xch, source_dom,
> +                     HVM_PARAM_CONSOLE_PFN,
> +                     &hvm_params[HVM_PARAM_CONSOLE_PFN]);
> +    console_pfn = hvm_params[HVM_PARAM_CONSOLE_PFN];
> +    *console_mfn = console_pfn;
> +
> +    xc_hvm_param_get(xch, source_dom, HVM_PARAM_BUFIOREQ_PFN,
> +                     &hvm_params[HVM_PARAM_BUFIOREQ_PFN]);
> +    buffio_pfn = hvm_params[HVM_PARAM_BUFIOREQ_PFN];
> +
> +    xc_hvm_param_get(xch, source_dom, HVM_PARAM_IOREQ_PFN,
> +                     &hvm_params[HVM_PARAM_IOREQ_PFN]);
> +    io_pfn = hvm_params[HVM_PARAM_IOREQ_PFN];
> +
> +    xc_hvm_param_get(xch, source_dom, HVM_PARAM_IDENT_PT,
> +                     &hvm_params[HVM_PARAM_IDENT_PT]);
> +
> +    xc_hvm_param_get(xch, source_dom, HVM_PARAM_PAGING_RING_PFN,
> +                     &hvm_params[HVM_PARAM_PAGING_RING_PFN]);
> +
> +    xc_hvm_param_get(xch, source_dom, HVM_PARAM_VM86_TSS,
> +                     &hvm_params[HVM_PARAM_VM86_TSS]);
> +
> +    xc_hvm_param_get(xch, source_dom, HVM_PARAM_ACPI_IOPORTS_LOCATION,
> +                     &hvm_params[HVM_PARAM_ACPI_IOPORTS_LOCATION]);
> +
> +    xc_hvm_param_get(xch, source_dom, HVM_PARAM_VIRIDIAN,
> +                     &hvm_params[HVM_PARAM_VIRIDIAN]);
> +
> +    xc_hvm_param_get(xch, source_dom, HVM_PARAM_PAE_ENABLED,
> +                     &hvm_params[HVM_PARAM_PAE_ENABLED]);
> +
> +    xc_hvm_param_get(xch, source_dom, HVM_PARAM_STORE_EVTCHN,
> +                     &hvm_params[HVM_PARAM_STORE_EVTCHN]);
> +
> +    xc_hvm_param_get(xch, source_dom, HVM_PARAM_IOREQ_SERVER_PFN,
> +                     &hvm_params[HVM_PARAM_IOREQ_SERVER_PFN]);
> +
> +    xc_hvm_param_get(xch, source_dom, HVM_PARAM_NR_IOREQ_SERVER_PAGES,
> +                     &hvm_params[HVM_PARAM_NR_IOREQ_SERVER_PAGES]);
> +
> +    xc_hvm_param_get(xch, source_dom, HVM_PARAM_VM_GENERATION_ID_ADDR,
> +                     &hvm_params[HVM_PARAM_VM_GENERATION_ID_ADDR]);

Even if this can't be shared with the migration please at least make it
table driven (i.e. an array of HVM_PARAM_* to wander over) so that
get/set can use the same table and not get out of sync.

Does the set have to follow the xc_domain_soft_reset and/or the get
precede the xc_domain_soft_reset? Or could a simple helper implement get
+set in one "move" operation? That would also remove the possibility of
forgetting to do both halves of one hvm param.

Ian.

[PATCH v7 08/10] xl: introduce enum domain_restart_type

[Vitaly Kuznetsov]

As a preparation before adding new restart type (soft reset) put all
restart types into an enum.

Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
---
 tools/libxl/xl.h         |  6 ++++++
 tools/libxl/xl_cmdimpl.c | 23 ++++++++++-------------
 2 files changed, 16 insertions(+), 13 deletions(-)

diff --git a/tools/libxl/xl.h b/tools/libxl/xl.h
index 5bc138c..60045d4 100644
--- a/tools/libxl/xl.h
+++ b/tools/libxl/xl.h
@@ -186,6 +186,12 @@ enum output_format {
 };
 extern enum output_format default_output_format;
 
+typedef enum {
+    DOMAIN_RESTART_NONE = 0,     /* No domain restart */
+    DOMAIN_RESTART_NORMAL,       /* Domain should be restarter */
+    DOMAIN_RESTART_RENAME,       /* Domain should be renamed and restarted */
+} domain_restart_type;
+
 extern void printf_info_sexp(int domid, libxl_domain_config *d_config, FILE *fh);
 
 #define XL_GLOBAL_CONFIG XEN_CONFIG_DIR "/xl.conf"
diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index 7abac5d..01e72f8 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -2262,15 +2262,12 @@ static void reload_domain_config(uint32_t domid,
     }
 }
 
-/* Returns 1 if domain should be restarted,
- * 2 if domain should be renamed then restarted, or 0
- * Can update r_domid if domain is destroyed etc */
-static int handle_domain_death(uint32_t *r_domid,
-                               libxl_event *event,
-                               libxl_domain_config *d_config)
-
+/* Can update r_domid if domain is destroyed */
+static domain_restart_type handle_domain_death(uint32_t *r_domid,
+                                               libxl_event *event,
+                                               libxl_domain_config *d_config)
 {
-    int restart = 0;
+    domain_restart_type restart = DOMAIN_RESTART_NONE;
     libxl_action_on_shutdown action;
 
     switch (event->u.domain_shutdown.shutdown_reason) {
@@ -2325,12 +2322,12 @@ static int handle_domain_death(uint32_t *r_domid,
 
     case LIBXL_ACTION_ON_SHUTDOWN_RESTART_RENAME:
         reload_domain_config(*r_domid, d_config);
-        restart = 2;
+        restart = DOMAIN_RESTART_RENAME;
         break;
 
     case LIBXL_ACTION_ON_SHUTDOWN_RESTART:
         reload_domain_config(*r_domid, d_config);
-        restart = 1;
+        restart = DOMAIN_RESTART_NORMAL;
         /* fall-through */
     case LIBXL_ACTION_ON_SHUTDOWN_DESTROY:
         LOG("Domain %d needs to be cleaned up: destroying the domain",
@@ -2778,7 +2775,7 @@ start:
                 event->u.domain_shutdown.shutdown_reason,
                 event->u.domain_shutdown.shutdown_reason);
             switch (handle_domain_death(&domid, event, &d_config)) {
-            case 2:
+            case DOMAIN_RESTART_RENAME:
                 if (!preserve_domain(&domid, event, &d_config)) {
                     /* If we fail then exit leaving the old domain in place. */
                     ret = -1;
@@ -2786,7 +2783,7 @@ start:
                 }
 
                 /* Otherwise fall through and restart. */
-            case 1:
+            case DOMAIN_RESTART_NORMAL:
                 libxl_event_free(ctx, event);
                 libxl_evdisable_domain_death(ctx, deathw);
                 deathw = NULL;
@@ -2822,7 +2819,7 @@ start:
                 sleep(2);
                 goto start;
 
-            case 0:
+            case DOMAIN_RESTART_NONE:
                 LOG("Done. Exiting now");
                 libxl_event_free(ctx, event);
                 ret = 0;
-- 
1.9.3

Re: [PATCH v7 08/10] xl: introduce enum domain_restart_type

[Ian Campbell]

On Wed, 2015-05-27 at 17:25 +0200, Vitaly Kuznetsov wrote:
> As a preparation before adding new restart type (soft reset) put all
> restart types into an enum.
> 
> Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>

Acked-by: Ian Campbell <ian.campbell@citrix.com>

One typo:

> +    DOMAIN_RESTART_NORMAL,       /* Domain should be restarter */

"restarted".

This seems like a nice cleanup regardless of whether we do soft reset
into a new domain or on the existing domain.

Ian.

[PATCH v7 09/10] libxc: add XC_DEVICE_MODEL_SAVE_FILE

[Vitaly Kuznetsov]

Use this in libxl_dm instead of hard-coding.

Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
---
 tools/libxc/include/xenguest.h | 1 +
 tools/libxl/libxl_dm.c         | 2 +-
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/tools/libxc/include/xenguest.h b/tools/libxc/include/xenguest.h
index c71b423..98b3a37 100644
--- a/tools/libxc/include/xenguest.h
+++ b/tools/libxc/include/xenguest.h
@@ -168,6 +168,7 @@ int xc_soft_reset(xc_interface *xch, uint32_t source_dom,
 		  unsigned long *console_mfn, domid_t store_domid,
 		  unsigned long *store_mfn);
 
+#define XC_DEVICE_MODEL_SAVE_FILE "/var/lib/xen/qemu-save"
 #define XC_DEVICE_MODEL_RESTORE_FILE "/var/lib/xen/qemu-resume"
 
 /**
diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
index 0c6408d..869f4cd 100644
--- a/tools/libxl/libxl_dm.c
+++ b/tools/libxl/libxl_dm.c
@@ -31,7 +31,7 @@ static const char *libxl_tapif_script(libxl__gc *gc)
 
 const char *libxl__device_model_savefile(libxl__gc *gc, uint32_t domid)
 {
-    return libxl__sprintf(gc, "/var/lib/xen/qemu-save.%d", domid);
+    return libxl__sprintf(gc, XC_DEVICE_MODEL_SAVE_FILE".%d", domid);
 }
 
 static const char *qemu_xen_path(libxl__gc *gc)
-- 
1.9.3

Re: [PATCH v7 09/10] libxc: add XC_DEVICE_MODEL_SAVE_FILE

[Ian Campbell]

On Wed, 2015-05-27 at 17:25 +0200, Vitaly Kuznetsov wrote:
> Use this in libxl_dm instead of hard-coding.
> 
> Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>

This is something of an artefact of the layering weirdness regarding the
DM save/restore (saved by libxl, restored by libxc), but I think there
is no harm having this #define in libxc alongside the other one even if
it is actually (currently) handled by libxl.

Acked-by: Ian Campbell <ian.campbell@citrix.com>

> ---
>  tools/libxc/include/xenguest.h | 1 +
>  tools/libxl/libxl_dm.c         | 2 +-
>  2 files changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/libxc/include/xenguest.h b/tools/libxc/include/xenguest.h
> index c71b423..98b3a37 100644
> --- a/tools/libxc/include/xenguest.h
> +++ b/tools/libxc/include/xenguest.h
> @@ -168,6 +168,7 @@ int xc_soft_reset(xc_interface *xch, uint32_t source_dom,
>  		  unsigned long *console_mfn, domid_t store_domid,
>  		  unsigned long *store_mfn);
>  
> +#define XC_DEVICE_MODEL_SAVE_FILE "/var/lib/xen/qemu-save"
>  #define XC_DEVICE_MODEL_RESTORE_FILE "/var/lib/xen/qemu-resume"
>  
>  /**
> diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
> index 0c6408d..869f4cd 100644
> --- a/tools/libxl/libxl_dm.c
> +++ b/tools/libxl/libxl_dm.c
> @@ -31,7 +31,7 @@ static const char *libxl_tapif_script(libxl__gc *gc)
>  
>  const char *libxl__device_model_savefile(libxl__gc *gc, uint32_t domid)
>  {
> -    return libxl__sprintf(gc, "/var/lib/xen/qemu-save.%d", domid);
> +    return libxl__sprintf(gc, XC_DEVICE_MODEL_SAVE_FILE".%d", domid);
>  }
>  
>  static const char *qemu_xen_path(libxl__gc *gc)

[PATCH v7 10/10] (lib)xl: soft reset support

[Vitaly Kuznetsov]

Perform soft reset when a domain did SHUTDOWN_soft_reset. Migrate the
content with xc_domain_soft_reset(), reload dm and toolstack.

Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
---
Changes in v7:
- Save toolstack earlier.
- Introduce LIBXL_HAVE_SOFT_RESET [Wei Liu]
---
 docs/man/xl.cfg.pod.5        |  12 +++++
 tools/libxl/libxl.c          |   4 ++
 tools/libxl/libxl.h          |  14 +++++
 tools/libxl/libxl_create.c   | 119 +++++++++++++++++++++++++++++++++++++++----
 tools/libxl/libxl_internal.h |  26 ++++++++++
 tools/libxl/libxl_types.idl  |   3 ++
 tools/libxl/xl.h             |   1 +
 tools/libxl/xl_cmdimpl.c     |  33 +++++++++++-
 8 files changed, 201 insertions(+), 11 deletions(-)

diff --git a/docs/man/xl.cfg.pod.5 b/docs/man/xl.cfg.pod.5
index 8e4154f..ced5cc0 100644
--- a/docs/man/xl.cfg.pod.5
+++ b/docs/man/xl.cfg.pod.5
@@ -368,6 +368,13 @@ destroy the domain.
 write a "coredump" of the domain to F</var/xen/dump/NAME> and then
 restart the domain.
 
+=item B<soft-reset>
+
+create a new domain with the same configuration, reassign all the domain's
+memory to this new domain, kill the original domain, and continue execution
+of the new domain from where the action was triggered. Supported for HVM
+guests only.
+
 =back
 
 The default for C<on_poweroff> is C<destroy>.
@@ -386,6 +393,11 @@ Default is C<destroy>.
 
 Action to take if the domain crashes.  Default is C<destroy>.
 
+=item B<on_soft_reset="ACTION">
+
+Action to take if the domain performs 'soft reset' (e.g. does kexec).
+Default is C<soft-reset>.
+
 =back
 
 =head3 Direct Kernel Boot
diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index a6eb2df..1a01b4b 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -1492,6 +1492,7 @@ void libxl__domain_destroy(libxl__egc *egc, libxl__domain_destroy_state *dds)
         dds->stubdom.ao = ao;
         dds->stubdom.domid = stubdomid;
         dds->stubdom.callback = stubdom_destroy_callback;
+        dds->stubdom.soft_reset = false;
         libxl__destroy_domid(egc, &dds->stubdom);
     } else {
         dds->stubdom_finished = 1;
@@ -1500,6 +1501,7 @@ void libxl__domain_destroy(libxl__egc *egc, libxl__domain_destroy_state *dds)
     dds->domain.ao = ao;
     dds->domain.domid = dds->domid;
     dds->domain.callback = domain_destroy_callback;
+    dds->domain.soft_reset = dds->soft_reset;
     libxl__destroy_domid(egc, &dds->domain);
 }
 
@@ -1678,6 +1680,8 @@ static void devices_destroy_cb(libxl__egc *egc,
 
     libxl__unlock_domain_userdata(lock);
 
+    if (dis->soft_reset) goto out;
+
     rc = libxl__ev_child_fork(gc, &dis->destroyer, domain_destroy_domid_cb);
     if (rc < 0) goto out;
     if (!rc) { /* child */
diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
index 2ed7194..cc621dc 100644
--- a/tools/libxl/libxl.h
+++ b/tools/libxl/libxl.h
@@ -192,6 +192,14 @@
  * is not present, instead of ERROR_INVAL.
  */
 #define LIBXL_HAVE_ERROR_DOMAIN_NOTFOUND 1
+
+/*
+ * LIBXL_HAVE_SOFT_RESET indicates that libxl supports performing 'soft reset'
+ * for domains and there is 'soft_reset' shutdown reason in enum
+ * libxl_shutdown_reason.
+ */
+#define LIBXL_HAVE_SOFT_RESET 1
+
 /*
  * libxl ABI compatibility
  *
@@ -1005,6 +1013,12 @@ int static inline libxl_domain_create_restore_0x040200(
 
 #endif
 
+int libxl_domain_soft_reset(libxl_ctx *ctx, libxl_domain_config *d_config,
+                            uint32_t *domid, uint32_t domid_soft_reset,
+                            const libxl_asyncop_how *ao_how,
+                            const libxl_asyncprogress_how *aop_console_how)
+                            LIBXL_EXTERNAL_CALLERS_ONLY;
+
   /* A progress report will be made via ao_console_how, of type
    * domain_create_console_available, when the domain's primary
    * console is available and can be connected to.
diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
index f0da7dc..c82fee4 100644
--- a/tools/libxl/libxl_create.c
+++ b/tools/libxl/libxl_create.c
@@ -945,6 +945,9 @@ static void initiate_domain_create(libxl__egc *egc,
     if (restore_fd >= 0) {
         LOG(DEBUG, "restoring, not running bootloader");
         domcreate_bootloader_done(egc, &dcs->bl, 0);
+    } else if (dcs->domid_soft_reset != INVALID_DOMID) {
+        LOG(DEBUG, "soft reset, not running bootloader\n");
+        domcreate_bootloader_done(egc, &dcs->bl, 0);
     } else  {
         LOG(DEBUG, "running bootloader");
         dcs->bl.callback = domcreate_bootloader_done;
@@ -993,6 +996,7 @@ static void domcreate_bootloader_done(libxl__egc *egc,
     libxl_domain_config *const d_config = dcs->guest_config;
     libxl_domain_build_info *const info = &d_config->b_info;
     const int restore_fd = dcs->restore_fd;
+    const uint32_t domid_soft_reset = dcs->domid_soft_reset;
     libxl__domain_build_state *const state = &dcs->build_state;
     libxl__srm_restore_autogen_callbacks *const callbacks =
         &dcs->shs.callbacks.restore.a;
@@ -1016,7 +1020,7 @@ static void domcreate_bootloader_done(libxl__egc *egc,
     dcs->dmss.dm.callback = domcreate_devmodel_started;
     dcs->dmss.callback = domcreate_devmodel_started;
 
-    if ( restore_fd < 0 ) {
+    if ( (restore_fd < 0) && (domid_soft_reset == INVALID_DOMID) ) {
         rc = libxl__domain_build(gc, d_config, domid, state);
         domcreate_rebuild_done(egc, dcs, rc);
         return;
@@ -1046,8 +1050,13 @@ static void domcreate_bootloader_done(libxl__egc *egc,
         rc = ERROR_INVAL;
         goto out;
     }
-    libxl__xc_domain_restore(egc, dcs,
-                             hvm, pae, superpages);
+    if ( restore_fd >= 0 ) {
+        libxl__xc_domain_restore(egc, dcs,
+                                 hvm, pae, superpages);
+    } else {
+        libxl__xc_domain_soft_reset(egc, dcs);
+    }
+
     return;
 
  out:
@@ -1079,6 +1088,7 @@ void libxl__xc_domain_restore_done(libxl__egc *egc, void *dcs_void,
 
     /* convenience aliases */
     const uint32_t domid = dcs->guest_domid;
+    const uint32_t domid_soft_reset = dcs->domid_soft_reset;
     libxl_domain_config *const d_config = dcs->guest_config;
     libxl_domain_build_info *const info = &d_config->b_info;
     libxl__domain_build_state *const state = &dcs->build_state;
@@ -1131,9 +1141,12 @@ void libxl__xc_domain_restore_done(libxl__egc *egc, void *dcs_void,
     if (ret)
         goto out;
 
-    if (info->type == LIBXL_DOMAIN_TYPE_HVM) {
+    if (info->type == LIBXL_DOMAIN_TYPE_HVM && fd != -1) {
         state->saved_state = GCSPRINTF(
                        XC_DEVICE_MODEL_RESTORE_FILE".%d", domid);
+    } else if (domid_soft_reset != INVALID_DOMID) {
+        state->saved_state = GCSPRINTF(
+                       XC_DEVICE_MODEL_SAVE_FILE".%d", domid_soft_reset);
     }
 
 out:
@@ -1142,9 +1155,12 @@ out:
         libxl__file_reference_unmap(&state->pv_ramdisk);
     }
 
-    esave = errno;
-    libxl_fd_set_nonblock(ctx, fd, 0);
-    errno = esave;
+    if ( fd != -1 ) {
+        esave = errno;
+        libxl_fd_set_nonblock(ctx, fd, 0);
+        errno = esave;
+    }
+
     domcreate_rebuild_done(egc, dcs, ret);
 }
 
@@ -1529,20 +1545,67 @@ static void domcreate_destruction_cb(libxl__egc *egc,
 typedef struct {
     libxl__domain_create_state dcs;
     uint32_t *domid_out;
+    libxl__domain_destroy_state dds;
+    uint8_t *toolstack_buf;
+    uint32_t toolstack_len;
 } libxl__app_domain_create_state;
 
 static void domain_create_cb(libxl__egc *egc,
                              libxl__domain_create_state *dcs,
                              int rc, uint32_t domid);
 
+static void domain_soft_reset_cb(libxl__egc *egc,
+                                 libxl__domain_destroy_state *dds,
+                                 int rc)
+{
+    STATE_AO_GC(dds->ao);
+    libxl__app_domain_create_state *cdcs = CONTAINER_OF(dds, *cdcs, dds);
+
+    if (rc)
+        LOG(ERROR, "destruction of domain %u failed", dds->domid);
+
+    initiate_domain_create(egc, &cdcs->dcs);
+}
+
+void libxl__xc_domain_soft_reset(libxl__egc *egc,
+                                 libxl__domain_create_state *dcs)
+{
+    STATE_AO_GC(dcs->ao);
+    libxl_ctx *ctx = libxl__gc_owner(gc);
+    const uint32_t domid_soft_reset = dcs->domid_soft_reset;
+    const uint32_t domid = dcs->guest_domid;
+    libxl__app_domain_create_state *cdcs = CONTAINER_OF(dcs, *cdcs, dcs);
+    int rc;
+
+    rc = xc_soft_reset(ctx->xch, domid_soft_reset, domid,
+                       dcs->build_state.console_domid,
+                       &dcs->build_state.console_mfn,
+                       dcs->build_state.store_domid,
+                       &dcs->build_state.store_mfn);
+    if (rc) goto out;
+
+    rc = libxl__toolstack_restore(domid, cdcs->toolstack_buf,
+                                  cdcs->toolstack_len, &dcs->shs);
+out:
+    free(cdcs->toolstack_buf);
+    /*
+     * Now pretend we did normal restore and simply call
+     * libxl__xc_domain_restore_done().
+     */
+    libxl__xc_domain_restore_done(egc, dcs, rc, 0, 0);
+}
+
 static int do_domain_create(libxl_ctx *ctx, libxl_domain_config *d_config,
                             uint32_t *domid,
                             int restore_fd, int checkpointed_stream,
+                            uint32_t domid_soft_reset,
                             const libxl_asyncop_how *ao_how,
                             const libxl_asyncprogress_how *aop_console_how)
 {
     AO_CREATE(ctx, 0, ao_how);
     libxl__app_domain_create_state *cdcs;
+    libxl__domain_suspend_state *dss;
+    int rc;
 
     GCNEW(cdcs);
     cdcs->dcs.ao = ao;
@@ -1550,12 +1613,34 @@ static int do_domain_create(libxl_ctx *ctx, libxl_domain_config *d_config,
     libxl_domain_config_init(&cdcs->dcs.guest_config_saved);
     libxl_domain_config_copy(ctx, &cdcs->dcs.guest_config_saved, d_config);
     cdcs->dcs.restore_fd = restore_fd;
+    cdcs->dcs.domid_soft_reset = domid_soft_reset;
     cdcs->dcs.callback = domain_create_cb;
     cdcs->dcs.checkpointed_stream = checkpointed_stream;
     libxl__ao_progress_gethow(&cdcs->dcs.aop_console_how, aop_console_how);
     cdcs->domid_out = domid;
 
-    initiate_domain_create(egc, &cdcs->dcs);
+    if (domid_soft_reset != INVALID_DOMID) {
+        GCNEW(dss);
+        dss->ao = ao;
+        dss->domid = domid_soft_reset;
+        dss->dm_savefile = GCSPRINTF(XC_DEVICE_MODEL_SAVE_FILE".%d",
+                                     domid_soft_reset);
+
+        rc = libxl__domain_suspend_device_model(gc, dss);
+        if (rc) return AO_ABORT(rc);
+
+        rc = libxl__toolstack_save(domid_soft_reset, &cdcs->toolstack_buf,
+                                   &cdcs->toolstack_len, dss);
+        if (rc) return AO_ABORT(rc);
+
+        cdcs->dds.ao = ao;
+        cdcs->dds.domid = domid_soft_reset;
+        cdcs->dds.callback = domain_soft_reset_cb;
+        cdcs->dds.soft_reset = true;
+        libxl__domain_destroy(egc, &cdcs->dds);
+    }
+    else
+        initiate_domain_create(egc, &cdcs->dcs);
 
     return AO_INPROGRESS;
 }
@@ -1578,7 +1663,7 @@ int libxl_domain_create_new(libxl_ctx *ctx, libxl_domain_config *d_config,
                             const libxl_asyncop_how *ao_how,
                             const libxl_asyncprogress_how *aop_console_how)
 {
-    return do_domain_create(ctx, d_config, domid, -1, 0,
+    return do_domain_create(ctx, d_config, domid, -1, 0, INVALID_DOMID,
                             ao_how, aop_console_how);
 }
 
@@ -1589,7 +1674,21 @@ int libxl_domain_create_restore(libxl_ctx *ctx, libxl_domain_config *d_config,
                                 const libxl_asyncprogress_how *aop_console_how)
 {
     return do_domain_create(ctx, d_config, domid, restore_fd,
-                            params->checkpointed_stream, ao_how, aop_console_how);
+                            params->checkpointed_stream, INVALID_DOMID,
+                            ao_how, aop_console_how);
+}
+
+int libxl_domain_soft_reset(libxl_ctx *ctx, libxl_domain_config *d_config,
+                            uint32_t *domid, uint32_t domid_soft_reset,
+                            const libxl_asyncop_how *ao_how,
+                            const libxl_asyncprogress_how *aop_console_how)
+{
+    libxl_domain_build_info *const info = &d_config->b_info;
+
+    if (info->type != LIBXL_DOMAIN_TYPE_HVM) return ERROR_INVAL;
+
+    return do_domain_create(ctx, d_config, domid, -1, 0, domid_soft_reset,
+                            ao_how, aop_console_how);
 }
 
 /*
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 8aaa1ad..944603e 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -106,6 +106,7 @@
 #define TAP_DEVICE_SUFFIX "-emu"
 #define DISABLE_UDEV_PATH "libxl/disable_udev"
 #define DOMID_XS_PATH "domid"
+#define INVALID_DOMID ~0
 
 #define ARRAY_SIZE(a) (sizeof(a) / sizeof(a[0]))
 
@@ -3002,6 +3003,7 @@ struct libxl__destroy_domid_state {
     /* private to implementation */
     libxl__devices_remove_state drs;
     libxl__ev_child destroyer;
+    bool soft_reset;
 };
 
 struct libxl__domain_destroy_state {
@@ -3016,6 +3018,7 @@ struct libxl__domain_destroy_state {
     int stubdom_finished;
     libxl__destroy_domid_state domain;
     int domain_finished;
+    bool soft_reset;
 };
 
 /*
@@ -3116,6 +3119,7 @@ struct libxl__domain_create_state {
     libxl_domain_config *guest_config;
     libxl_domain_config guest_config_saved; /* vanilla config */
     int restore_fd;
+    uint32_t domid_soft_reset;
     libxl__domain_create_cb *callback;
     libxl_asyncprogress_how aop_console_how;
     /* private to domain_create */
@@ -3180,6 +3184,28 @@ _hidden void libxl__domain_save_device_model(libxl__egc *egc,
 
 _hidden const char *libxl__device_model_savefile(libxl__gc *gc, uint32_t domid);
 
+/*
+ * Soft reset is a special type of reset when the new domain is being built
+ * with the memory contents and vCPU contexts of the original domain thus
+ * allowing to continue execution from where the hypercall was done. This is
+ * currently supported for HVM domains only and is done as a modification for
+ * the domain create/restore path within libxl:
+ * do_domain_create()
+ *    libxl__domain_suspend_device_model()
+ *    libxl__toolstack_save()
+ *    libxl__domain_destroy()->domain_soft_reset_cb()->initiate_domain_create()
+ *    ... regular domain create path ...
+ *    domcreate_bootloader_done()
+ *       libxl__xc_domain_soft_reset()
+ *           xc_soft_reset() which saves HVM context and HVM params, calls
+ *             XEN_DOMCTL_soft_reset to transfer the domain's memory, restores
+ *             HVM context and HVM params for the new domain.
+ *       libxl__toolstack_restore()
+ *       libxl__xc_domain_restore_done() and follow the standard restore path.
+ */
+
+_hidden void libxl__xc_domain_soft_reset(libxl__egc *egc,
+                                         libxl__domain_create_state *dcs);
 
 /*
  * Convenience macros.
diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
index 1aa90f5..9d63de6 100644
--- a/tools/libxl/libxl_types.idl
+++ b/tools/libxl/libxl_types.idl
@@ -123,6 +123,8 @@ libxl_action_on_shutdown = Enumeration("action_on_shutdown", [
 
     (5, "COREDUMP_DESTROY"),
     (6, "COREDUMP_RESTART"),
+
+    (7, "SOFT_RESET"),
     ], init_val = "LIBXL_ACTION_ON_SHUTDOWN_DESTROY")
 
 libxl_trigger = Enumeration("trigger", [
@@ -573,6 +575,7 @@ libxl_domain_config = Struct("domain_config", [
     ("on_reboot", libxl_action_on_shutdown),
     ("on_watchdog", libxl_action_on_shutdown),
     ("on_crash", libxl_action_on_shutdown),
+    ("on_soft_reset", libxl_action_on_shutdown),
     ], dir=DIR_IN)
 
 libxl_diskinfo = Struct("diskinfo", [
diff --git a/tools/libxl/xl.h b/tools/libxl/xl.h
index 60045d4..3abb7d5 100644
--- a/tools/libxl/xl.h
+++ b/tools/libxl/xl.h
@@ -190,6 +190,7 @@ typedef enum {
     DOMAIN_RESTART_NONE = 0,     /* No domain restart */
     DOMAIN_RESTART_NORMAL,       /* Domain should be restarter */
     DOMAIN_RESTART_RENAME,       /* Domain should be renamed and restarted */
+    DOMAIN_RESTART_SOFT_RESET,   /* Soft reset should be performed */
 } domain_restart_type;
 
 extern void printf_info_sexp(int domid, libxl_domain_config *d_config, FILE *fh);
diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index 01e72f8..cee82f3 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -130,6 +130,8 @@ static const char *action_on_shutdown_names[] = {
 
     [LIBXL_ACTION_ON_SHUTDOWN_COREDUMP_DESTROY] = "coredump-destroy",
     [LIBXL_ACTION_ON_SHUTDOWN_COREDUMP_RESTART] = "coredump-restart",
+
+    [LIBXL_ACTION_ON_SHUTDOWN_SOFT_RESET] = "soft-reset",
 };
 
 /* Optional data, in order:
@@ -1329,6 +1331,13 @@ static void parse_config_data(const char *config_source,
         exit(1);
     }
 
+    if (xlu_cfg_get_string (config, "on_soft_reset", &buf, 0))
+        buf = "soft-reset";
+    if (!parse_action_on_shutdown(buf, &d_config->on_soft_reset)) {
+        fprintf(stderr, "Unknown on_soft_reset action \"%s\" specified\n", buf);
+        exit(1);
+    }
+
     /* libxl_get_required_shadow_memory() must be called after final values
      * (default or specified) for vcpus and memory are set, because the
      * calculation depends on those values. */
@@ -2286,6 +2295,9 @@ static domain_restart_type handle_domain_death(uint32_t *r_domid,
     case LIBXL_SHUTDOWN_REASON_WATCHDOG:
         action = d_config->on_watchdog;
         break;
+    case LIBXL_SHUTDOWN_REASON_SOFT_RESET:
+        action = d_config->on_soft_reset;
+        break;
     default:
         LOG("Unknown shutdown reason code %d. Destroying domain.",
             event->u.domain_shutdown.shutdown_reason);
@@ -2336,6 +2348,11 @@ static domain_restart_type handle_domain_death(uint32_t *r_domid,
         *r_domid = INVALID_DOMID;
         break;
 
+    case LIBXL_ACTION_ON_SHUTDOWN_SOFT_RESET:
+        reload_domain_config(*r_domid, d_config);
+        restart = DOMAIN_RESTART_SOFT_RESET;
+        break;
+
     case LIBXL_ACTION_ON_SHUTDOWN_COREDUMP_DESTROY:
     case LIBXL_ACTION_ON_SHUTDOWN_COREDUMP_RESTART:
         /* Already handled these above. */
@@ -2485,6 +2502,7 @@ static void evdisable_disk_ejects(libxl_evgen_disk_eject **diskws,
 static uint32_t create_domain(struct domain_create *dom_info)
 {
     uint32_t domid = INVALID_DOMID;
+    uint32_t domid_old = INVALID_DOMID;
 
     libxl_domain_config d_config;
 
@@ -2711,7 +2729,17 @@ start:
          * restore/migrate-receive it again.
          */
         restoring = 0;
-    }else{
+    } else if (domid_old != INVALID_DOMID) {
+        /* Do soft reset */
+        ret = libxl_domain_soft_reset(ctx, &d_config,
+                                      &domid, domid_old,
+                                      0, autoconnect_console_how);
+
+        if ( ret )
+            goto error_out;
+
+        domid_old = INVALID_DOMID;
+    } else {
         ret = libxl_domain_create_new(ctx, &d_config, &domid,
                                       0, autoconnect_console_how);
     }
@@ -2775,6 +2803,9 @@ start:
                 event->u.domain_shutdown.shutdown_reason,
                 event->u.domain_shutdown.shutdown_reason);
             switch (handle_domain_death(&domid, event, &d_config)) {
+            case DOMAIN_RESTART_SOFT_RESET:
+                domid_old = domid;
+                /* fall through */
             case DOMAIN_RESTART_RENAME:
                 if (!preserve_domain(&domid, event, &d_config)) {
                     /* If we fail then exit leaving the old domain in place. */
-- 
1.9.3

Re: [PATCH v7 10/10] (lib)xl: soft reset support

[Ian Campbell]

On Wed, 2015-05-27 at 17:25 +0200, Vitaly Kuznetsov wrote:
> Perform soft reset when a domain did SHUTDOWN_soft_reset. Migrate the
> content with xc_domain_soft_reset(), reload dm and toolstack.
> 
> Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
> ---
> Changes in v7:
> - Save toolstack earlier.
> - Introduce LIBXL_HAVE_SOFT_RESET [Wei Liu]
> ---
>  docs/man/xl.cfg.pod.5        |  12 +++++
>  tools/libxl/libxl.c          |   4 ++
>  tools/libxl/libxl.h          |  14 +++++
>  tools/libxl/libxl_create.c   | 119 +++++++++++++++++++++++++++++++++++++++----
>  tools/libxl/libxl_internal.h |  26 ++++++++++
>  tools/libxl/libxl_types.idl  |   3 ++
>  tools/libxl/xl.h             |   1 +
>  tools/libxl/xl_cmdimpl.c     |  33 +++++++++++-
>  8 files changed, 201 insertions(+), 11 deletions(-)
> 
> diff --git a/docs/man/xl.cfg.pod.5 b/docs/man/xl.cfg.pod.5
> index 8e4154f..ced5cc0 100644
> --- a/docs/man/xl.cfg.pod.5
> +++ b/docs/man/xl.cfg.pod.5
> @@ -368,6 +368,13 @@ destroy the domain.
>  write a "coredump" of the domain to F</var/xen/dump/NAME> and then
>  restart the domain.
>  
> +=item B<soft-reset>
> +
> +create a new domain with the same configuration, reassign all the domain's
> +memory to this new domain, kill the original domain, and continue execution

s/this/a/ I think?

> @@ -1529,20 +1545,67 @@ static void domcreate_destruction_cb(libxl__egc *egc,
>  typedef struct {
>      libxl__domain_create_state dcs;
>      uint32_t *domid_out;
> +    libxl__domain_destroy_state dds;
> +    uint8_t *toolstack_buf;
> +    uint32_t toolstack_len;

I think a libxl__domain_soft_reset_state would be appropriate here,
perhaps containing the app_domain_create_state rather than adding all
the fields next to it.

>  } libxl__app_domain_create_state;
> [...]
> @@ -1550,12 +1613,34 @@ static int do_domain_create(libxl_ctx *ctx, libxl_domain_config *d_config,
>      libxl_domain_config_init(&cdcs->dcs.guest_config_saved);
>      libxl_domain_config_copy(ctx, &cdcs->dcs.guest_config_saved, d_config);
>      cdcs->dcs.restore_fd = restore_fd;
> +    cdcs->dcs.domid_soft_reset = domid_soft_reset;
>      cdcs->dcs.callback = domain_create_cb;
>      cdcs->dcs.checkpointed_stream = checkpointed_stream;
>      libxl__ao_progress_gethow(&cdcs->dcs.aop_console_how, aop_console_how);
>      cdcs->domid_out = domid;

Since the backend of this function has very little in common between the
create and soft/reset cases, perhaps refactor the first half into a new
function to initialise a cdcs to be called by do_domain_create and the
soft reset function, the latter of which would go on to do the body of
the following if here.

Ian.

Re: [PATCH v7 00/10] toolstack-based approach to pvhvm guest kexec

[Jan Beulich]

>>> On 27.05.15 at 17:25, <vkuznets@redhat.com> wrote:
> This patch series provides x86 PVHVM domains with an ability to perform
> kexec/kdump-style operations.

Before I get to look at this latest version, may I go a step back and
ask for clarification whether all of these (seemingly fragile)
manipulations are actually needed (such a rationale for the series
would btw be quite good to have in the overview mail, rather than
having to wade through mailing list history). In particular, why is it
that we need a new domain in the first place? After all, native
kexec doesn't imply a new physical machine either. Perhaps that
was discussed a long while back, but I can't seem to find any of
that now that I would like to read through it again.

Jan

Re: [PATCH v7 00/10] toolstack-based approach to pvhvm guest kexec

[Vitaly Kuznetsov]

"Jan Beulich" <JBeulich@suse.com> writes:

>>>> On 27.05.15 at 17:25, <vkuznets@redhat.com> wrote:
>> This patch series provides x86 PVHVM domains with an ability to perform
>> kexec/kdump-style operations.
>
> Before I get to look at this latest version, may I go a step back and
> ask for clarification whether all of these (seemingly fragile)
> manipulations are actually needed (such a rationale for the series
> would btw be quite good to have in the overview mail, rather than
> having to wade through mailing list history). In particular, why is it
> that we need a new domain in the first place? After all, native
> kexec doesn't imply a new physical machine either. Perhaps that
> was discussed a long while back, but I can't seem to find any of
> that now that I would like to read through it again.

Yes, here are some highlights from last year's discussion:

http://lists.xen.org/archives/html/xen-devel/2014-08/msg01908.html

http://lists.xen.org/archives/html/xen-devel/2014-08/msg01979.html

http://lists.xen.org/archives/html/xen-devel/2014-08/msg01918.html

-- 
  Vitaly

Re: [PATCH v7 00/10] toolstack-based approach to pvhvm guest kexec

[Jan Beulich]

>>> On 28.05.15 at 14:27, <vkuznets@redhat.com> wrote:
> "Jan Beulich" <JBeulich@suse.com> writes:
> 
>>>>> On 27.05.15 at 17:25, <vkuznets@redhat.com> wrote:
>>> This patch series provides x86 PVHVM domains with an ability to perform
>>> kexec/kdump-style operations.
>>
>> Before I get to look at this latest version, may I go a step back and
>> ask for clarification whether all of these (seemingly fragile)
>> manipulations are actually needed (such a rationale for the series
>> would btw be quite good to have in the overview mail, rather than
>> having to wade through mailing list history). In particular, why is it
>> that we need a new domain in the first place? After all, native
>> kexec doesn't imply a new physical machine either. Perhaps that
>> was discussed a long while back, but I can't seem to find any of
>> that now that I would like to read through it again.
> 
> Yes, here are some highlights from last year's discussion:
> 
> http://lists.xen.org/archives/html/xen-devel/2014-08/msg01908.html 
> 
> http://lists.xen.org/archives/html/xen-devel/2014-08/msg01979.html 

I.e. what you currently implement is David's model without Konrad's
later alternative really having been explored? Iiuc David's main
reservation (which I share) was against a myriad of reset-this and
reset-that hypercalls, which Konrad's reset-everything would
address equally well.

Jan

Re: [PATCH v7 00/10] toolstack-based approach to pvhvm guest kexec

[Vitaly Kuznetsov]

"Jan Beulich" <JBeulich@suse.com> writes:

>>>> On 28.05.15 at 14:27, <vkuznets@redhat.com> wrote:
>> "Jan Beulich" <JBeulich@suse.com> writes:
>> 
>>>>>> On 27.05.15 at 17:25, <vkuznets@redhat.com> wrote:
>>>> This patch series provides x86 PVHVM domains with an ability to perform
>>>> kexec/kdump-style operations.
>>>
>>> Before I get to look at this latest version, may I go a step back and
>>> ask for clarification whether all of these (seemingly fragile)
>>> manipulations are actually needed (such a rationale for the series
>>> would btw be quite good to have in the overview mail, rather than
>>> having to wade through mailing list history). In particular, why is it
>>> that we need a new domain in the first place? After all, native
>>> kexec doesn't imply a new physical machine either. Perhaps that
>>> was discussed a long while back, but I can't seem to find any of
>>> that now that I would like to read through it again.
>> 
>> Yes, here are some highlights from last year's discussion:
>> 
>> http://lists.xen.org/archives/html/xen-devel/2014-08/msg01908.html 
>> 
>> http://lists.xen.org/archives/html/xen-devel/2014-08/msg01979.html 
>
> I.e. what you currently implement is David's model without Konrad's
> later alternative really having been explored? Iiuc David's main
> reservation (which I share) was against a myriad of reset-this and
> reset-that hypercalls, which Konrad's reset-everything would
> address equally well.

I was under an impression Konrad was also suggesting building a new
domain with "instead of chasing down different states (event channels,
grants, vcpu's, pagetables, etc) we would wipe everything to a nice
clean slate" (as we'll have to chase down all this bits with a
'reset-everything' hypercall) but now I'm not sure.

AFAICT If we follow down the 'reset-everything' road it's not going to
be any easier and less fragile. E.g. I don't see an easy way to deal
with grants: even if we can clean all the internal bits we still have
pages mapped to the backend domain (Qemu and other backends) and there
is no easy way to ask them to unmap everything. We could (in theory)
walk through all the pages of our domain replacing all pages which need
replacement with empty pages but we need to temporary assign old pages
somewhere, avoid over-allocation (as e.g. in tot_pages = max_pages case
we can't add a single page to the domain). The support burden of a
'reset-everything' hypercall is also going to be heavier as all newly
added bits will have to be added to it.

The approach used in this series is not significantly different from how
an HVM domain is doing normal reboot: we destroy the original domain and
create a new one instead of cleaning up the original one (as it looks
safer and much easier I suppose).

-- 
  Vitaly

Re: [PATCH v7 00/10] toolstack-based approach to pvhvm guest kexec

[Ian Campbell]

On Thu, 2015-05-28 at 15:41 +0200, Vitaly Kuznetsov wrote:
> > I.e. what you currently implement is David's model without Konrad's
> > later alternative really having been explored? Iiuc David's main
> > reservation (which I share) was against a myriad of reset-this and
> > reset-that hypercalls, which Konrad's reset-everything would
> > address equally well.

FWIW it seems to me that David's suggestion without Konrad's
modification is the simplest and least fragile approach. Is there some
impetus to prefer a reset-all hypercall?

[...]
> The approach used in this series is not significantly different from how
> an HVM domain is doing normal reboot: we destroy the original domain and
> create a new one instead of cleaning up the original one (as it looks
> safer and much easier I suppose).

Right, that was my first thought too.

Ian.

Re: [PATCH v7 00/10] toolstack-based approach to pvhvm guest kexec

[Vitaly Kuznetsov]

Ian Campbell <ian.campbell@citrix.com> writes:

> On Thu, 2015-05-28 at 15:41 +0200, Vitaly Kuznetsov wrote:
>> > I.e. what you currently implement is David's model without Konrad's
>> > later alternative really having been explored? Iiuc David's main
>> > reservation (which I share) was against a myriad of reset-this and
>> > reset-that hypercalls, which Konrad's reset-everything would
>> > address equally well.
>
> FWIW it seems to me that David's suggestion without Konrad's
> modification is the simplest and least fragile approach. Is there some
> impetus to prefer a reset-all hypercall?
>

I'm actually doing a 'proof-of-concept' for the 'reset-all' solution, I
hope to send it out this week. Personally, I think that the
'toolstack-based approach' would be less fragile and easier to support.

> [...]
>> The approach used in this series is not significantly different from how
>> an HVM domain is doing normal reboot: we destroy the original domain and
>> create a new one instead of cleaning up the original one (as it looks
>> safer and much easier I suppose).
>
> Right, that was my first thought too.
>
> Ian.

-- 
  Vitaly