[PATCH v9 00/11] toolstack-assisted approach to PVHVM guest kexec

[PATCH v9 00/11] toolstack-assisted approach to PVHVM guest kexec

[Vitaly Kuznetsov]

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

Changes since v8 (also mentioned in individual patches) grouped by reviewer's
name:
[Ian Jackson]
- PATCH 01:
  - describe the expected behavior for 'soft reset'.
- PATCH 02:
  - Introduce on_soft_reset here (moved from PATCH 11).
- PATCH 09:
  - Add 'Acked-by:'.
- PATCH 11:
  - Redistribution of hunks between this patch and PATCH 3.
  - Shorten lines to 70-75 chars.
  - Rephrase 'soft-reset' action in xl.cfg.pod.5.

[Konrad Rzeszutek Wilk]
- PATCH 03:
  - Add 'Reviewed-by:'.
- PATCH 04:
  - Check !d->controller_pause_count instead of pausing/unpausing the domain
    in evtchn_reset().
- PATCH 05:
  - Use WARN_GRANT_MAX define instead of hardcoded 10.
  - Update commit message explaining why 'g' keyhandler is not enough.
- PATCH 06:
  - Introduce vcpu_info_reset() helper.
  - Take shutdown_lock while checking v->paused_for_shutdown in
    domain_soft_reset().
- PATCH 08:
  - Comments fixes.
  - pirq 0 is a valid pirq.
  - s/0/PAGE_ORDER_4K/ for guest_physmap_{add,remove}_page.
  - Free new page in case of guest_physmap_add_page() failure.
- PATCH 09:
  - Add 'Reviewed-by:'.
- PATCH 11:
  - Pause/unpase the domain when doing xc_domain_soft_reset.

[Daniel De Graaf]
- PATCH 07:
  - Add 'Acked-by:'.

[Julien Grall]
- PATCH 08:
  - Make ARM-specific hook return -ENOSYS.

[Wei Liu]
- PATCH 09:
  - Add 'Acked-by:'.

Original description:

The list of currently known issues blocking kexec (and why the series is
required) is:
- Bound event channels.
- Registered vcpu_info.
- PIRQ/emuirq mappings.
- shared_info frame after XENMAPSPACE_shared_info operation.
- Active grant mappings.

Previously there were several attempts to solve these issues in different ways:
- Individual 'shutdown' hypercalls (e.g. VCPUOP_reset_vcpu_info) (we agreed
  that one 'reset everything' hypercall is better).
- Try building new domain reassigning old domain's memory (memory reassignment
  turned out being too cumbersome).
- Toolstack-less 'reset everything' (turned out being impossible because there
  is not enough knowledge in the hypervisor, e.g. interdomain channels are
  being set up by the toolstack).

This series is a mix of the previously sent 'toolstack-based' and
'reset everything' series. Here are some key points:
- No new domain is created.
- Domain is asking for soft reset with SCHEDOP_shutdown with
  SHUTDOWN_soft_reset shutdown reason.
- XEN_DOMCTL_soft_reset is being called by the toolstack.
- Device model is being restarted.

With regards to active grants. In this series we restart domain's device model,
remove it from xenstore and introduce it back. The only 'misbehaving' backend
keeping active mapping I currently observe is xenconsoled: currently it has no
interface to disconnect from a domain (it just periodically scans all domains
to determine if any of them are dead). This is not an issue for us because:
- This matches standard domain startup as this grant mapping is being set up by
  the toolstack.
- Guest domain is aware of this special page.
grant_table_warn_active_grants() is required to find possible misbehaving
backends in future.

v8 of the 'toolstack-assisted approach to pvhvm guest kexec' is available here:
http://lists.xen.org/archives/html/xen-devel/2015-06/msg03632.html

Vitaly Kuznetsov (11):
  xen: introduce SHUTDOWN_soft_reset shutdown reason
  libxl: support SHUTDOWN_soft_reset shutdown reason
  xl: introduce enum domain_restart_type
  xen: evtchn: make evtchn_reset() ready for soft reset
  xen: grant_table: implement grant_table_warn_active_grants()
  xen: Introduce XEN_DOMCTL_soft_reset
  flask: DOMCTL_soft_reset support
  xen: arch-specific hooks for domain_soft_reset()
  libxc: support XEN_DOMCTL_soft_reset operation
  libxc: add XC_DEVICE_MODEL_SAVE_FILE
  (lib)xl: soft reset support

 docs/man/xl.cfg.pod.5                        |  10 ++
 tools/flask/policy/policy/modules/xen/xen.if |   2 +-
 tools/libxc/include/xenctrl.h                |   3 +
 tools/libxc/include/xenguest.h               |   2 +
 tools/libxc/xc_domain.c                      |   9 ++
 tools/libxl/libxl.c                          |  23 +++-
 tools/libxl/libxl.h                          |  15 +++
 tools/libxl/libxl_create.c                   | 193 ++++++++++++++++++++++++---
 tools/libxl/libxl_dm.c                       |   2 +-
 tools/libxl/libxl_internal.h                 |   4 +
 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/arch/arm/domain.c                        |   5 +
 xen/arch/x86/domain.c                        |  84 ++++++++++++
 xen/arch/x86/hvm/hvm.c                       |   2 +-
 xen/common/domain.c                          |  48 +++++--
 xen/common/domctl.c                          |   9 ++
 xen/common/event_channel.c                   |  43 +++---
 xen/common/grant_table.c                     |  35 +++++
 xen/common/shutdown.c                        |   6 +
 xen/include/asm-x86/hvm/hvm.h                |   2 +
 xen/include/public/domctl.h                  |   1 +
 xen/include/public/sched.h                   |  11 +-
 xen/include/xen/domain.h                     |   2 +
 xen/include/xen/event.h                      |   3 +
 xen/include/xen/grant_table.h                |   5 +
 xen/include/xen/sched.h                      |   2 +
 xen/xsm/flask/hooks.c                        |   3 +
 xen/xsm/flask/policy/access_vectors          |   2 +
 31 files changed, 518 insertions(+), 78 deletions(-)

-- 
2.4.3

[PATCH v9 01/11] 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.

Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
---
Changes since v8:
- describe the expected behavior for 'soft reset' [Ian Jackson]
---
 xen/common/shutdown.c      |  6 ++++++
 xen/include/public/sched.h | 11 ++++++++++-
 2 files changed, 16 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..b91bb9b 100644
--- a/xen/include/public/sched.h
+++ b/xen/include/public/sched.h
@@ -159,7 +159,16 @@ 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.             */
+
+/*
+ * Domain asked to perform 'soft reset' for it. The expected behavior is to
+ * reset internal Xen state for the domain returning it to the point where it
+ * was created but leaving the domain's memory and vCPU contexts intact. This
+ * will allow the domain to start over and set up all Xen specific interfaces
+ * again.
+ */
+#define SHUTDOWN_soft_reset 5
+#define SHUTDOWN_MAX        5  /* Maximum valid shutdown reason.             */
 /* ` } */
 
 #endif /* __XEN_PUBLIC_SCHED_H__ */
-- 
2.4.3

Re: [PATCH v9 01/11] xen: introduce SHUTDOWN_soft_reset shutdown reason

[Konrad Rzeszutek Wilk]

On Thu, Jul 16, 2015 at 06:27:16PM +0200, Vitaly Kuznetsov wrote:
> This special type of shutdown is supposed to be used by PVHVM guests when
> they want to perform some sort of kexec/kdump.
> 
> Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>

Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> ---
> Changes since v8:
> - describe the expected behavior for 'soft reset' [Ian Jackson]
> ---
>  xen/common/shutdown.c      |  6 ++++++
>  xen/include/public/sched.h | 11 ++++++++++-
>  2 files changed, 16 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..b91bb9b 100644
> --- a/xen/include/public/sched.h
> +++ b/xen/include/public/sched.h
> @@ -159,7 +159,16 @@ 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.             */
> +
> +/*
> + * Domain asked to perform 'soft reset' for it. The expected behavior is to
> + * reset internal Xen state for the domain returning it to the point where it
> + * was created but leaving the domain's memory and vCPU contexts intact. This
> + * will allow the domain to start over and set up all Xen specific interfaces
> + * again.
> + */
> +#define SHUTDOWN_soft_reset 5
> +#define SHUTDOWN_MAX        5  /* Maximum valid shutdown reason.             */
>  /* ` } */
>  
>  #endif /* __XEN_PUBLIC_SCHED_H__ */
> -- 
> 2.4.3
> 

Re: [PATCH v9 01/11] xen: introduce SHUTDOWN_soft_reset shutdown reason

[Jan Beulich]

>>> On 16.07.15 at 18:27, <vkuznets@redhat.com> wrote:
> --- a/xen/include/public/sched.h
> +++ b/xen/include/public/sched.h
> @@ -159,7 +159,16 @@ 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.             */
> +
> +/*
> + * Domain asked to perform 'soft reset' for it. The expected behavior is to
> + * reset internal Xen state for the domain returning it to the point where it
> + * was created but leaving the domain's memory and vCPU contexts intact. This

... memory contents and vCPU contexts ...

Other than that
Acked-by: Jan Beulich <jbeulich@suse.com>

> + * will allow the domain to start over and set up all Xen specific interfaces
> + * again.
> + */
> +#define SHUTDOWN_soft_reset 5
> +#define SHUTDOWN_MAX        5  /* Maximum valid shutdown reason.            
>  */
>  /* ` } */
>  
>  #endif /* __XEN_PUBLIC_SCHED_H__ */
> -- 
> 2.4.3

[PATCH v9 02/11] libxl: support SHUTDOWN_soft_reset shutdown reason

[Vitaly Kuznetsov]

Use letter 'S' to indicate a domain in such state. Introduce new
'on_soft_reset' action and default it to 'restart' for now.

Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
---
Changes since v8:
- Introduce on_soft_reset here (moved from PATCH 11) [Ian Jackson]
---
 docs/man/xl.cfg.pod.5             |  5 +++++
 tools/libxl/libxl_types.idl       |  2 ++
 tools/libxl/xl_cmdimpl.c          | 12 +++++++++++-
 tools/python/xen/lowlevel/xl/xl.c |  1 +
 4 files changed, 19 insertions(+), 1 deletion(-)

diff --git a/docs/man/xl.cfg.pod.5 b/docs/man/xl.cfg.pod.5
index 382f30b..3489b27 100644
--- a/docs/man/xl.cfg.pod.5
+++ b/docs/man/xl.cfg.pod.5
@@ -364,6 +364,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<restart>.
+
 =back
 
 =head3 Direct Kernel Boot
diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
index 8dacf8d..337257a 100644
--- a/tools/libxl/libxl_types.idl
+++ b/tools/libxl/libxl_types.idl
@@ -181,6 +181,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", [
@@ -609,6 +610,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_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index 971209c..c7aaa19 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -1337,6 +1337,13 @@ static void parse_config_data(const char *config_source,
         exit(1);
     }
 
+    if (xlu_cfg_get_string (config, "on_soft_reset", &buf, 0))
+        buf = "restart";
+    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. */
@@ -2334,6 +2341,9 @@ static int 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);
@@ -3758,7 +3768,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);
 }
-- 
2.4.3

[PATCH v9 03/11] 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>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
---
Changes since v8:
- add Konrad's R-b

Changes since v7:
- s,restarter,restarted, [Ian Campbell]
---
 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 13bccba..6c19c0d 100644
--- a/tools/libxl/xl.h
+++ b/tools/libxl/xl.h
@@ -190,6 +190,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 restarted */
+    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 c7aaa19..f1604b1 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -2314,15 +2314,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) {
@@ -2380,12 +2377,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",
@@ -2842,7 +2839,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;
@@ -2850,7 +2847,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;
@@ -2886,7 +2883,7 @@ start:
                 sleep(2);
                 goto start;
 
-            case 0:
+            case DOMAIN_RESTART_NONE:
                 LOG("Done. Exiting now");
                 libxl_event_free(ctx, event);
                 ret = 0;
-- 
2.4.3

[PATCH v9 04/11] xen: evtchn: make evtchn_reset() ready for soft reset

[Vitaly Kuznetsov]

We need to close all event channel so the domain performing soft reset
will be able to open them back.

Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
---
Changes since v8:
- Check !d->controller_pause_count instead of pausing/unpausing the domain
  in evtchn_reset(). [Konrad Rzeszutek Wilk]
---
 xen/common/event_channel.c | 43 +++++++++++++++++++------------------------
 xen/include/xen/event.h    |  3 +++
 2 files changed, 22 insertions(+), 24 deletions(-)

diff --git a/xen/common/event_channel.c b/xen/common/event_channel.c
index 7640e30..4fa1e4a 100644
--- a/xen/common/event_channel.c
+++ b/xen/common/event_channel.c
@@ -981,44 +981,28 @@ int evtchn_unmask(unsigned int port)
 }
 
 
-static long evtchn_reset(evtchn_reset_t *r)
+int evtchn_reset(struct domain *d)
 {
-    domid_t dom = r->dom;
-    struct domain *d;
-    int i, rc;
-
-    d = rcu_lock_domain_by_any_id(dom);
-    if ( d == NULL )
-        return -ESRCH;
+    unsigned int i;
 
-    rc = xsm_evtchn_reset(XSM_TARGET, current->domain, d);
-    if ( rc )
-        goto out;
+    if ( d != current->domain && !d->controller_pause_count )
+        return -EINVAL;
 
     for ( i = 0; port_is_valid(d, i); i++ )
         evtchn_close(d, i, 1);
 
     spin_lock(&d->event_lock);
 
-    if ( (dom == DOMID_SELF) && d->evtchn_fifo )
+    if ( d->evtchn_fifo )
     {
-        /*
-         * Guest domain called EVTCHNOP_reset with DOMID_SELF, destroying
-         * FIFO event array and control blocks, resetting evtchn_port_ops to
-         * evtchn_port_ops_2l.
-         */
+        /* Switching back to 2-level ABI. */
         evtchn_fifo_destroy(d);
         evtchn_2l_init(d);
     }
 
     spin_unlock(&d->event_lock);
 
-    rc = 0;
-
-out:
-    rcu_unlock_domain(d);
-
-    return rc;
+    return 0;
 }
 
 static long evtchn_set_priority(const struct evtchn_set_priority *set_priority)
@@ -1143,9 +1127,20 @@ long do_event_channel_op(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
 
     case EVTCHNOP_reset: {
         struct evtchn_reset reset;
+        struct domain *d;
+
         if ( copy_from_guest(&reset, arg, 1) != 0 )
             return -EFAULT;
-        rc = evtchn_reset(&reset);
+
+        d = rcu_lock_domain_by_any_id(reset.dom);
+        if ( d == NULL )
+            return -ESRCH;
+
+        rc = xsm_evtchn_reset(XSM_TARGET, current->domain, d);
+        if ( !rc )
+            rc = evtchn_reset(d);
+
+        rcu_unlock_domain(d);
         break;
     }
 
diff --git a/xen/include/xen/event.h b/xen/include/xen/event.h
index af923d1..b87924a 100644
--- a/xen/include/xen/event.h
+++ b/xen/include/xen/event.h
@@ -126,6 +126,9 @@ void evtchn_check_pollers(struct domain *d, unsigned int port);
 
 void evtchn_2l_init(struct domain *d);
 
+/* Close all event channels and reset to 2-level ABI. */
+int evtchn_reset(struct domain *d);
+
 /*
  * Low-level event channel port ops.
  */
-- 
2.4.3

Re: [PATCH v9 04/11] xen: evtchn: make evtchn_reset() ready for soft reset

[Konrad Rzeszutek Wilk]

On Thu, Jul 16, 2015 at 06:27:19PM +0200, Vitaly Kuznetsov wrote:
> We need to close all event channel so the domain performing soft reset
> will be able to open them back.
> 
> Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>

Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> ---
> Changes since v8:
> - Check !d->controller_pause_count instead of pausing/unpausing the domain
>   in evtchn_reset(). [Konrad Rzeszutek Wilk]
> ---
>  xen/common/event_channel.c | 43 +++++++++++++++++++------------------------
>  xen/include/xen/event.h    |  3 +++
>  2 files changed, 22 insertions(+), 24 deletions(-)
> 
> diff --git a/xen/common/event_channel.c b/xen/common/event_channel.c
> index 7640e30..4fa1e4a 100644
> --- a/xen/common/event_channel.c
> +++ b/xen/common/event_channel.c
> @@ -981,44 +981,28 @@ int evtchn_unmask(unsigned int port)
>  }
>  
>  
> -static long evtchn_reset(evtchn_reset_t *r)
> +int evtchn_reset(struct domain *d)
>  {
> -    domid_t dom = r->dom;
> -    struct domain *d;
> -    int i, rc;
> -
> -    d = rcu_lock_domain_by_any_id(dom);
> -    if ( d == NULL )
> -        return -ESRCH;
> +    unsigned int i;
>  
> -    rc = xsm_evtchn_reset(XSM_TARGET, current->domain, d);
> -    if ( rc )
> -        goto out;
> +    if ( d != current->domain && !d->controller_pause_count )
> +        return -EINVAL;
>  
>      for ( i = 0; port_is_valid(d, i); i++ )
>          evtchn_close(d, i, 1);
>  
>      spin_lock(&d->event_lock);
>  
> -    if ( (dom == DOMID_SELF) && d->evtchn_fifo )
> +    if ( d->evtchn_fifo )
>      {
> -        /*
> -         * Guest domain called EVTCHNOP_reset with DOMID_SELF, destroying
> -         * FIFO event array and control blocks, resetting evtchn_port_ops to
> -         * evtchn_port_ops_2l.
> -         */
> +        /* Switching back to 2-level ABI. */
>          evtchn_fifo_destroy(d);
>          evtchn_2l_init(d);
>      }
>  
>      spin_unlock(&d->event_lock);
>  
> -    rc = 0;
> -
> -out:
> -    rcu_unlock_domain(d);
> -
> -    return rc;
> +    return 0;
>  }
>  
>  static long evtchn_set_priority(const struct evtchn_set_priority *set_priority)
> @@ -1143,9 +1127,20 @@ long do_event_channel_op(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
>  
>      case EVTCHNOP_reset: {
>          struct evtchn_reset reset;
> +        struct domain *d;
> +
>          if ( copy_from_guest(&reset, arg, 1) != 0 )
>              return -EFAULT;
> -        rc = evtchn_reset(&reset);
> +
> +        d = rcu_lock_domain_by_any_id(reset.dom);
> +        if ( d == NULL )
> +            return -ESRCH;
> +
> +        rc = xsm_evtchn_reset(XSM_TARGET, current->domain, d);
> +        if ( !rc )
> +            rc = evtchn_reset(d);
> +
> +        rcu_unlock_domain(d);
>          break;
>      }
>  
> diff --git a/xen/include/xen/event.h b/xen/include/xen/event.h
> index af923d1..b87924a 100644
> --- a/xen/include/xen/event.h
> +++ b/xen/include/xen/event.h
> @@ -126,6 +126,9 @@ void evtchn_check_pollers(struct domain *d, unsigned int port);
>  
>  void evtchn_2l_init(struct domain *d);
>  
> +/* Close all event channels and reset to 2-level ABI. */
> +int evtchn_reset(struct domain *d);
> +
>  /*
>   * Low-level event channel port ops.
>   */
> -- 
> 2.4.3
> 

Re: [PATCH v9 04/11] xen: evtchn: make evtchn_reset() ready for soft reset

[Jan Beulich]

>>> On 16.07.15 at 18:27, <vkuznets@redhat.com> wrote:
> We need to close all event channel so the domain performing soft reset
> will be able to open them back.
> 
> Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>

Acked-by: Jan Beulich <jbeulich@suse.com>

[PATCH v9 05/11] xen: grant_table: implement grant_table_warn_active_grants()

[Vitaly Kuznetsov]

Log first 10 active grants for a domain. This function is going to be used
for soft reset, active grants on this path usually mean misbehaving backends
refusing to release their mappings on shutdown. We need that in addition to
the already existent 'g' keyhandler as such misbehaving backends can cause a
domain to crash right after the soft reset operation and 'g' option won't be
available in this case.

Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
---
Changes since v8:
- Use WARN_GRANT_MAX define instead of hardcoded 10 [Konrad Rzeszutek Wilk]
- Update commit message explaining why 'g' keyhandler is not enough
  [Konrad Rzeszutek Wilk]
---
 xen/common/grant_table.c      | 35 +++++++++++++++++++++++++++++++++++
 xen/include/xen/grant_table.h |  5 +++++
 2 files changed, 40 insertions(+)

diff --git a/xen/common/grant_table.c b/xen/common/grant_table.c
index 92f078e..c2e0210 100644
--- a/xen/common/grant_table.c
+++ b/xen/common/grant_table.c
@@ -3348,6 +3348,41 @@ gnttab_release_mappings(
     }
 }
 
+void grant_table_warn_active_grants(struct domain *d)
+{
+    struct grant_table *gt = d->grant_table;
+    struct active_grant_entry *act;
+    grant_ref_t ref;
+    unsigned int nr_active = 0;
+
+#define WARN_GRANT_MAX 10
+
+    read_lock(&gt->lock);
+
+    for ( ref = 0; ref != nr_grant_entries(gt); ref++ )
+    {
+        act = active_entry_acquire(gt, ref);
+        if ( !act->pin )
+        {
+            active_entry_release(act);
+            continue;
+        }
+
+        nr_active++;
+        if ( nr_active <= WARN_GRANT_MAX )
+            printk(XENLOG_G_DEBUG "Dom%d has an active grant: GFN: %lx"
+                   " (MFN: %lx)\n", d->domain_id, act->gfn, act->frame);
+        active_entry_release(act);
+    }
+
+    if ( nr_active > WARN_GRANT_MAX )
+        printk(XENLOG_G_DEBUG "Dom%d has too many (%d) active grants"
+               " to report\n", d->domain_id, nr_active);
+
+    read_unlock(&gt->lock);
+
+#undef WARN_GRANT_MAX
+}
 
 void
 grant_table_destroy(
diff --git a/xen/include/xen/grant_table.h b/xen/include/xen/grant_table.h
index 9c7b5a3..54005cc 100644
--- a/xen/include/xen/grant_table.h
+++ b/xen/include/xen/grant_table.h
@@ -90,6 +90,11 @@ void grant_table_destroy(
     struct domain *d);
 void grant_table_init_vcpu(struct vcpu *v);
 
+/*
+ * Check if domain has active grants and log first 10 of them.
+ */
+void grant_table_warn_active_grants(struct domain *d);
+
 /* Domain death release of granted mappings of other domains' memory. */
 void
 gnttab_release_mappings(
-- 
2.4.3

Re: [PATCH v9 05/11] xen: grant_table: implement grant_table_warn_active_grants()

[Konrad Rzeszutek Wilk]

On Thu, Jul 16, 2015 at 06:27:20PM +0200, Vitaly Kuznetsov wrote:
> Log first 10 active grants for a domain. This function is going to be used
> for soft reset, active grants on this path usually mean misbehaving backends
> refusing to release their mappings on shutdown. We need that in addition to
> the already existent 'g' keyhandler as such misbehaving backends can cause a
> domain to crash right after the soft reset operation and 'g' option won't be
> available in this case.
> 
> Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>

Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> ---
> Changes since v8:
> - Use WARN_GRANT_MAX define instead of hardcoded 10 [Konrad Rzeszutek Wilk]
> - Update commit message explaining why 'g' keyhandler is not enough
>   [Konrad Rzeszutek Wilk]
> ---
>  xen/common/grant_table.c      | 35 +++++++++++++++++++++++++++++++++++
>  xen/include/xen/grant_table.h |  5 +++++
>  2 files changed, 40 insertions(+)
> 
> diff --git a/xen/common/grant_table.c b/xen/common/grant_table.c
> index 92f078e..c2e0210 100644
> --- a/xen/common/grant_table.c
> +++ b/xen/common/grant_table.c
> @@ -3348,6 +3348,41 @@ gnttab_release_mappings(
>      }
>  }
>  
> +void grant_table_warn_active_grants(struct domain *d)
> +{
> +    struct grant_table *gt = d->grant_table;
> +    struct active_grant_entry *act;
> +    grant_ref_t ref;
> +    unsigned int nr_active = 0;
> +
> +#define WARN_GRANT_MAX 10
> +
> +    read_lock(&gt->lock);
> +
> +    for ( ref = 0; ref != nr_grant_entries(gt); ref++ )
> +    {
> +        act = active_entry_acquire(gt, ref);
> +        if ( !act->pin )
> +        {
> +            active_entry_release(act);
> +            continue;
> +        }
> +
> +        nr_active++;
> +        if ( nr_active <= WARN_GRANT_MAX )
> +            printk(XENLOG_G_DEBUG "Dom%d has an active grant: GFN: %lx"
> +                   " (MFN: %lx)\n", d->domain_id, act->gfn, act->frame);
> +        active_entry_release(act);
> +    }
> +
> +    if ( nr_active > WARN_GRANT_MAX )
> +        printk(XENLOG_G_DEBUG "Dom%d has too many (%d) active grants"
> +               " to report\n", d->domain_id, nr_active);
> +
> +    read_unlock(&gt->lock);
> +
> +#undef WARN_GRANT_MAX
> +}
>  
>  void
>  grant_table_destroy(
> diff --git a/xen/include/xen/grant_table.h b/xen/include/xen/grant_table.h
> index 9c7b5a3..54005cc 100644
> --- a/xen/include/xen/grant_table.h
> +++ b/xen/include/xen/grant_table.h
> @@ -90,6 +90,11 @@ void grant_table_destroy(
>      struct domain *d);
>  void grant_table_init_vcpu(struct vcpu *v);
>  
> +/*
> + * Check if domain has active grants and log first 10 of them.
> + */
> +void grant_table_warn_active_grants(struct domain *d);
> +
>  /* Domain death release of granted mappings of other domains' memory. */
>  void
>  gnttab_release_mappings(
> -- 
> 2.4.3
> 

Re: [PATCH v9 05/11] xen: grant_table: implement grant_table_warn_active_grants()

[Jan Beulich]

>>> On 16.07.15 at 18:27, <vkuznets@redhat.com> wrote:
> --- a/xen/common/grant_table.c
> +++ b/xen/common/grant_table.c
> @@ -3348,6 +3348,41 @@ gnttab_release_mappings(
>      }
>  }
>  
> +void grant_table_warn_active_grants(struct domain *d)
> +{
> +    struct grant_table *gt = d->grant_table;
> +    struct active_grant_entry *act;
> +    grant_ref_t ref;
> +    unsigned int nr_active = 0;
> +
> +#define WARN_GRANT_MAX 10
> +
> +    read_lock(&gt->lock);
> +
> +    for ( ref = 0; ref != nr_grant_entries(gt); ref++ )
> +    {
> +        act = active_entry_acquire(gt, ref);
> +        if ( !act->pin )
> +        {
> +            active_entry_release(act);
> +            continue;
> +        }
> +
> +        nr_active++;
> +        if ( nr_active <= WARN_GRANT_MAX )
> +            printk(XENLOG_G_DEBUG "Dom%d has an active grant: GFN: %lx"
> +                   " (MFN: %lx)\n", d->domain_id, act->gfn, act->frame);
> +        active_entry_release(act);
> +    }
> +
> +    if ( nr_active > WARN_GRANT_MAX )
> +        printk(XENLOG_G_DEBUG "Dom%d has too many (%d) active grants"
> +               " to report\n", d->domain_id, nr_active);

Please don't break format strings across lines, even if that (with
nothing else on the same line) makes the line longer than 80
characters. With that adjusted
Acked-by: Jan Beulich <jbeulich@suse.com>

[PATCH v9 06/11] xen: Introduce XEN_DOMCTL_soft_reset

[Vitaly Kuznetsov]

New domctl resets state for a domain allowing it to 'start over': register
vcpu_info, switch to FIFO ABI for event channels. Still active grants are
being logged to help debugging misbehaving backends.

Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
---
Changes since v8:
- Introduce vcpu_info_reset() helper. [Konrad Rzeszutek Wilk]
- Take shutdown_lock while checking v->paused_for_shutdown in
  domain_soft_reset() [Konrad Rzeszutek Wilk]
---
 xen/common/domain.c         | 44 ++++++++++++++++++++++++++++++++++++--------
 xen/common/domctl.c         |  9 +++++++++
 xen/include/public/domctl.h |  1 +
 xen/include/xen/sched.h     |  2 ++
 4 files changed, 48 insertions(+), 8 deletions(-)

diff --git a/xen/common/domain.c b/xen/common/domain.c
index 8efef5c..4f805d5 100644
--- a/xen/common/domain.c
+++ b/xen/common/domain.c
@@ -110,6 +110,16 @@ static void vcpu_check_shutdown(struct vcpu *v)
     spin_unlock(&d->shutdown_lock);
 }
 
+static void vcpu_info_reset(struct vcpu *v)
+{
+    struct domain *d = v->domain;
+
+    v->vcpu_info = ((v->vcpu_id < XEN_LEGACY_MAX_VCPUS)
+                    ? (vcpu_info_t *)&shared_info(d, vcpu_info[v->vcpu_id])
+                    : &dummy_vcpu_info);
+    v->vcpu_info_mfn = INVALID_MFN;
+}
+
 struct vcpu *alloc_vcpu(
     struct domain *d, unsigned int vcpu_id, unsigned int cpu_id)
 {
@@ -145,10 +155,7 @@ struct vcpu *alloc_vcpu(
         v->runstate.state = RUNSTATE_offline;        
         v->runstate.state_entry_time = NOW();
         set_bit(_VPF_down, &v->pause_flags);
-        v->vcpu_info = ((vcpu_id < XEN_LEGACY_MAX_VCPUS)
-                        ? (vcpu_info_t *)&shared_info(d, vcpu_info[vcpu_id])
-                        : &dummy_vcpu_info);
-        v->vcpu_info_mfn = INVALID_MFN;
+        vcpu_info_reset(v);
         init_waitqueue_vcpu(v);
     }
 
@@ -1010,6 +1017,29 @@ int domain_unpause_by_systemcontroller(struct domain *d)
     return 0;
 }
 
+int domain_soft_reset(struct domain *d)
+{
+    struct vcpu *v;
+    int rc;
+
+    spin_lock(&d->shutdown_lock);
+    for_each_vcpu ( d, v )
+        if ( !v->paused_for_shutdown )
+            return -EINVAL;
+    spin_unlock(&d->shutdown_lock);
+
+    rc = evtchn_reset(d);
+
+    grant_table_warn_active_grants(d);
+
+    for_each_vcpu ( d, v )
+        unmap_vcpu_info(v);
+
+    domain_resume(d);
+
+    return 0;
+}
+
 int vcpu_reset(struct vcpu *v)
 {
     struct domain *d = v->domain;
@@ -1121,8 +1151,7 @@ int map_vcpu_info(struct vcpu *v, unsigned long gfn, unsigned offset)
 
 /*
  * Unmap the vcpu info page if the guest decided to place it somewhere
- * else.  This is only used from arch_domain_destroy, so there's no
- * need to do anything clever.
+ * else. This is used from arch_domain_destroy and domain_soft_reset.
  */
 void unmap_vcpu_info(struct vcpu *v)
 {
@@ -1135,8 +1164,7 @@ void unmap_vcpu_info(struct vcpu *v)
     unmap_domain_page_global((void *)
                              ((unsigned long)v->vcpu_info & PAGE_MASK));
 
-    v->vcpu_info = &dummy_vcpu_info;
-    v->vcpu_info_mfn = INVALID_MFN;
+    vcpu_info_reset(v);
 
     put_page_and_type(mfn_to_page(mfn));
 }
diff --git a/xen/common/domctl.c b/xen/common/domctl.c
index 7f959f3..41891b1 100644
--- a/xen/common/domctl.c
+++ b/xen/common/domctl.c
@@ -704,6 +704,15 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xen_domctl_t) u_domctl)
         break;
     }
 
+    case XEN_DOMCTL_soft_reset:
+        if ( d == current->domain )
+        {
+            ret = -EINVAL;
+            break;
+        }
+        ret = domain_soft_reset(d);
+        break;
+
     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 8b1d6b4..93e85cf 100644
--- a/xen/include/public/domctl.h
+++ b/xen/include/public/domctl.h
@@ -1130,6 +1130,7 @@ struct xen_domctl {
 #define XEN_DOMCTL_psr_cmt_op                    75
 #define XEN_DOMCTL_monitor_op                    77
 #define XEN_DOMCTL_psr_cat_op                    78
+#define XEN_DOMCTL_soft_reset                    79
 #define XEN_DOMCTL_gdbsx_guestmemio            1000
 #define XEN_DOMCTL_gdbsx_pausevcpu             1001
 #define XEN_DOMCTL_gdbsx_unpausevcpu           1002
diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h
index b29d9e7..18d8b57 100644
--- a/xen/include/xen/sched.h
+++ b/xen/include/xen/sched.h
@@ -610,6 +610,8 @@ 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 *d);
+
 int vcpu_start_shutdown_deferral(struct vcpu *v);
 void vcpu_end_shutdown_deferral(struct vcpu *v);
 
-- 
2.4.3

Re: [PATCH v9 06/11] xen: Introduce XEN_DOMCTL_soft_reset

[Konrad Rzeszutek Wilk]

On Thu, Jul 16, 2015 at 06:27:21PM +0200, Vitaly Kuznetsov wrote:
> New domctl resets state for a domain allowing it to 'start over': register
> vcpu_info, switch to FIFO ABI for event channels. Still active grants are
> being logged to help debugging misbehaving backends.
> 
> Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
> ---
> Changes since v8:
> - Introduce vcpu_info_reset() helper. [Konrad Rzeszutek Wilk]
> - Take shutdown_lock while checking v->paused_for_shutdown in
>   domain_soft_reset() [Konrad Rzeszutek Wilk]
> ---
>  xen/common/domain.c         | 44 ++++++++++++++++++++++++++++++++++++--------
>  xen/common/domctl.c         |  9 +++++++++
>  xen/include/public/domctl.h |  1 +
>  xen/include/xen/sched.h     |  2 ++
>  4 files changed, 48 insertions(+), 8 deletions(-)
> 
> diff --git a/xen/common/domain.c b/xen/common/domain.c
> index 8efef5c..4f805d5 100644
> --- a/xen/common/domain.c
> +++ b/xen/common/domain.c
> @@ -110,6 +110,16 @@ static void vcpu_check_shutdown(struct vcpu *v)
>      spin_unlock(&d->shutdown_lock);
>  }
>  
> +static void vcpu_info_reset(struct vcpu *v)
> +{
> +    struct domain *d = v->domain;
> +
> +    v->vcpu_info = ((v->vcpu_id < XEN_LEGACY_MAX_VCPUS)
> +                    ? (vcpu_info_t *)&shared_info(d, vcpu_info[v->vcpu_id])
> +                    : &dummy_vcpu_info);
> +    v->vcpu_info_mfn = INVALID_MFN;
> +}

Great!
> +
>  struct vcpu *alloc_vcpu(
>      struct domain *d, unsigned int vcpu_id, unsigned int cpu_id)
>  {
> @@ -145,10 +155,7 @@ struct vcpu *alloc_vcpu(
>          v->runstate.state = RUNSTATE_offline;        
>          v->runstate.state_entry_time = NOW();
>          set_bit(_VPF_down, &v->pause_flags);
> -        v->vcpu_info = ((vcpu_id < XEN_LEGACY_MAX_VCPUS)
> -                        ? (vcpu_info_t *)&shared_info(d, vcpu_info[vcpu_id])
> -                        : &dummy_vcpu_info);
> -        v->vcpu_info_mfn = INVALID_MFN;
> +        vcpu_info_reset(v);
>          init_waitqueue_vcpu(v);
>      }
>  
> @@ -1010,6 +1017,29 @@ int domain_unpause_by_systemcontroller(struct domain *d)
>      return 0;
>  }
>  
> +int domain_soft_reset(struct domain *d)
> +{
> +    struct vcpu *v;
> +    int rc;
> +
> +    spin_lock(&d->shutdown_lock);
> +    for_each_vcpu ( d, v )
> +        if ( !v->paused_for_shutdown )
> +            return -EINVAL;

Ahem. You leave the spinlock still locked :-(

> +    spin_unlock(&d->shutdown_lock);
> +
> +    rc = evtchn_reset(d);
> +
> +    grant_table_warn_active_grants(d);
> +
> +    for_each_vcpu ( d, v )
> +        unmap_vcpu_info(v);
> +
> +    domain_resume(d);
> +
> +    return 0;
> +}

Re: [PATCH v9 06/11] xen: Introduce XEN_DOMCTL_soft_reset

[Vitaly Kuznetsov]

Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> writes:

>>  
>> @@ -1010,6 +1017,29 @@ int domain_unpause_by_systemcontroller(struct domain *d)
>>      return 0;
>>  }
>>  
>> +int domain_soft_reset(struct domain *d)
>> +{
>> +    struct vcpu *v;
>> +    int rc;
>> +
>> +    spin_lock(&d->shutdown_lock);
>> +    for_each_vcpu ( d, v )
>> +        if ( !v->paused_for_shutdown )
>> +            return -EINVAL;
>
> Ahem. You leave the spinlock still locked :-(
>

Ouch... :-(

-- 
  Vitaly

Re: [PATCH v9 06/11] xen: Introduce XEN_DOMCTL_soft_reset

[Jan Beulich]

>>> On 16.07.15 at 18:27, <vkuznets@redhat.com> wrote:
> New domctl resets state for a domain allowing it to 'start over': register
> vcpu_info, switch to FIFO ABI for event channels. Still active grants are
> being logged to help debugging misbehaving backends.
> 
> Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>

With the lock release issue fixed
Acked-by: Jan Beulich <jbeulich@suse.com>

[PATCH v9 07/11] flask: DOMCTL_soft_reset support

[Vitaly Kuznetsov]

Add new soft_reset vector to domain2 class, add it to create_domain
in the default policy.

Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
Acked-by: Daniel De Graaf <dgdegra@tycho.nsa.gov>
---
Changes since v8:
- Add Daniel's A-b.
---
 tools/flask/policy/policy/modules/xen/xen.if | 2 +-
 xen/xsm/flask/hooks.c                        | 3 +++
 xen/xsm/flask/policy/access_vectors          | 2 ++
 3 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/tools/flask/policy/policy/modules/xen/xen.if b/tools/flask/policy/policy/modules/xen/xen.if
index da4c95b..370e711 100644
--- a/tools/flask/policy/policy/modules/xen/xen.if
+++ b/tools/flask/policy/policy/modules/xen/xen.if
@@ -52,7 +52,7 @@ 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_cat_op };
+			psr_cmt_op psr_cat_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 };
diff --git a/xen/xsm/flask/hooks.c b/xen/xsm/flask/hooks.c
index 882681f..da3e1b2 100644
--- a/xen/xsm/flask/hooks.c
+++ b/xen/xsm/flask/hooks.c
@@ -738,6 +738,9 @@ static int flask_domctl(struct domain *d, int cmd)
     case XEN_DOMCTL_psr_cat_op:
         return current_has_perm(d, SECCLASS_DOMAIN2, DOMAIN2__PSR_CAT_OP);
 
+    case XEN_DOMCTL_soft_reset:
+        return current_has_perm(d, SECCLASS_DOMAIN2, DOMAIN2__SOFT_RESET);
+
     default:
         printk("flask_domctl: Unknown op %d\n", cmd);
         return -EPERM;
diff --git a/xen/xsm/flask/policy/access_vectors b/xen/xsm/flask/policy/access_vectors
index b2a20c3..66bed41 100644
--- a/xen/xsm/flask/policy/access_vectors
+++ b/xen/xsm/flask/policy/access_vectors
@@ -232,6 +232,8 @@ class domain2
 # XEN_DOMCTL_monitor_op
 # XEN_DOMCTL_vm_event_op
     vm_event
+# XEN_DOMCTL_soft_reset
+    soft_reset
 # XENMEM_access_op
     mem_access
 # XENMEM_paging_op
-- 
2.4.3

[PATCH v9 08/11] xen: arch-specific hooks for domain_soft_reset()

[Vitaly Kuznetsov]

x86-specific hook cleans up the pirq-emuirq mappings, destroys all ioreq
servers and and replaces the shared_info frame with an empty page to support
subsequent XENMAPSPACE_shared_info call.

ARM-specific hook is -ENOSYS for now.

Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
---
Changes since v8:
- Comments fixes [Konrad Rzeszutek Wilk]
- pirq 0 is a valid pirq [Konrad Rzeszutek Wilk]
- s/0/PAGE_ORDER_4K/ for guest_physmap_{add,remove}_page
  [Konrad Rzeszutek Wilk]
- Free new page in case of guest_physmap_add_page() failure
  [Konrad Rzeszutek Wilk]
- Make ARM-specific hook return -ENOSYS [Julien Grall]

Changes since 'PATCH RFC' of the 'reset everything' approach to PVHVM guest kexec:
- Coding style, check get_gfn_query() return value, various minor fixes [Jan Beulich]
- Do not unpause VCPUs on arch hook failure [Jan Beulich]
---
 xen/arch/arm/domain.c         |  5 +++
 xen/arch/x86/domain.c         | 84 +++++++++++++++++++++++++++++++++++++++++++
 xen/arch/x86/hvm/hvm.c        |  2 +-
 xen/common/domain.c           |  4 +++
 xen/include/asm-x86/hvm/hvm.h |  2 ++
 xen/include/xen/domain.h      |  2 ++
 6 files changed, 98 insertions(+), 1 deletion(-)

diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
index b2bfc7d..5bdc2e9 100644
--- a/xen/arch/arm/domain.c
+++ b/xen/arch/arm/domain.c
@@ -655,6 +655,11 @@ void arch_domain_unpause(struct domain *d)
 {
 }
 
+int arch_domain_soft_reset(struct domain *d)
+{
+    return -ENOSYS;
+}
+
 static int is_guest_pv32_psr(uint32_t psr)
 {
     switch (psr & PSR_MODE_MASK)
diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
index 34ecd7c..1829535 100644
--- a/xen/arch/x86/domain.c
+++ b/xen/arch/x86/domain.c
@@ -704,6 +704,90 @@ void arch_domain_unpause(struct domain *d)
         viridian_time_ref_count_thaw(d);
 }
 
+int arch_domain_soft_reset(struct domain *d)
+{
+    struct page_info *page = virt_to_page(d->shared_info), *new_page;
+    int ret = 0;
+    struct domain *owner;
+    unsigned long mfn, mfn_new, gfn;
+    p2m_type_t p2mt;
+    unsigned int i;
+
+    /* Soft reset is supported for HVM domains only. */
+    if ( !is_hvm_domain(d) )
+        return -EINVAL;
+
+    hvm_destroy_all_ioreq_servers(d);
+
+    spin_lock(&d->event_lock);
+    for ( i = 0; i < d->nr_pirqs ; i++ )
+    {
+        if ( domain_pirq_to_emuirq(d, i) != IRQ_UNBOUND )
+        {
+            ret = unmap_domain_pirq_emuirq(d, i);
+            if ( ret )
+                break;
+        }
+    }
+    spin_unlock(&d->event_lock);
+
+    if ( ret )
+        return ret;
+
+    /*
+     * shared_info page needs to be replaced with a new page, otherwise we
+     * will get a hole if the domain does XENMAPSPACE_shared_info.
+     */
+
+    owner = page_get_owner_and_reference(page);
+    /* If shared_info page wasn't used, we do not need to replace it. */
+    if ( owner != d )
+        goto exit_put_page;
+
+    mfn = page_to_mfn(page);
+    if ( !mfn_valid(mfn) )
+    {
+        printk(XENLOG_G_ERR "Dom%d's shared_info page points to invalid MFN\n",
+               d->domain_id);
+        ret = -EINVAL;
+        goto exit_put_page;
+    }
+
+    gfn = mfn_to_gmfn(d, mfn);
+    if ( mfn_x(get_gfn_query(d, gfn, &p2mt)) == INVALID_MFN )
+    {
+        printk(XENLOG_G_ERR "Failed to get Dom%d's shared_info GFN (%lx)\n",
+               d->domain_id, gfn);
+        ret = -EINVAL;
+        goto exit_put_page;
+    }
+
+    new_page = alloc_domheap_page(d, 0);
+    if ( !new_page )
+    {
+        printk(XENLOG_G_ERR "Failed to alloc a page to replace"
+               " Dom%d's shared_info frame %lx\n", d->domain_id, gfn);
+        ret = -ENOMEM;
+        goto exit_put_gfn;
+    }
+    mfn_new = page_to_mfn(new_page);
+    guest_physmap_remove_page(d, gfn, mfn, PAGE_ORDER_4K);
+
+    ret = guest_physmap_add_page(d, gfn, mfn_new, PAGE_ORDER_4K);
+    if ( ret )
+    {
+        printk(XENLOG_G_ERR "Failed to add a page to replace"
+               " Dom%d's shared_info frame %lx\n", d->domain_id, gfn);
+        free_domheap_page(new_page);
+    }
+ exit_put_gfn:
+    put_gfn(d, gfn);
+ exit_put_page:
+    put_page(page);
+
+    return ret;
+}
+
 /*
  * These are the masks of CR4 bits (subject to hardware availability) which a
  * PV guest may not legitimiately attempt to modify.
diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
index 545aa91..08a7a10 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -1370,7 +1370,7 @@ static void hvm_all_ioreq_servers_remove_vcpu(struct domain *d, struct vcpu *v)
     spin_unlock(&d->arch.hvm_domain.ioreq_server.lock);
 }
 
-static void hvm_destroy_all_ioreq_servers(struct domain *d)
+void hvm_destroy_all_ioreq_servers(struct domain *d)
 {
     struct hvm_ioreq_server *s, *next;
 
diff --git a/xen/common/domain.c b/xen/common/domain.c
index 4f805d5..8401b42 100644
--- a/xen/common/domain.c
+++ b/xen/common/domain.c
@@ -1035,6 +1035,10 @@ int domain_soft_reset(struct domain *d)
     for_each_vcpu ( d, v )
         unmap_vcpu_info(v);
 
+    rc = arch_domain_soft_reset(d);
+    if (rc)
+        return rc;
+
     domain_resume(d);
 
     return 0;
diff --git a/xen/include/asm-x86/hvm/hvm.h b/xen/include/asm-x86/hvm/hvm.h
index 82f1b32..450ca0c 100644
--- a/xen/include/asm-x86/hvm/hvm.h
+++ b/xen/include/asm-x86/hvm/hvm.h
@@ -229,6 +229,8 @@ struct hvm_ioreq_server *hvm_select_ioreq_server(struct domain *d,
 int hvm_send_ioreq(struct hvm_ioreq_server *s, ioreq_t *p, bool_t buffered);
 unsigned int hvm_broadcast_ioreq(ioreq_t *p, bool_t buffered);
 
+void hvm_destroy_all_ioreq_servers(struct domain *d);
+
 void hvm_get_guest_pat(struct vcpu *v, u64 *guest_pat);
 int hvm_set_guest_pat(struct vcpu *v, u64 guest_pat);
 
diff --git a/xen/include/xen/domain.h b/xen/include/xen/domain.h
index 848db8a..a469fe0 100644
--- a/xen/include/xen/domain.h
+++ b/xen/include/xen/domain.h
@@ -65,6 +65,8 @@ void arch_domain_shutdown(struct domain *d);
 void arch_domain_pause(struct domain *d);
 void arch_domain_unpause(struct domain *d);
 
+int arch_domain_soft_reset(struct domain *d);
+
 int arch_set_info_guest(struct vcpu *, vcpu_guest_context_u);
 void arch_get_info_guest(struct vcpu *, vcpu_guest_context_u);
 
-- 
2.4.3

Re: [PATCH v9 08/11] xen: arch-specific hooks for domain_soft_reset()

[Konrad Rzeszutek Wilk]

On Thu, Jul 16, 2015 at 06:27:23PM +0200, Vitaly Kuznetsov wrote:
> x86-specific hook cleans up the pirq-emuirq mappings, destroys all ioreq
> servers and and replaces the shared_info frame with an empty page to support
> subsequent XENMAPSPACE_shared_info call.
> 
> ARM-specific hook is -ENOSYS for now.
> 
> Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>

Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> ---
> Changes since v8:
> - Comments fixes [Konrad Rzeszutek Wilk]
> - pirq 0 is a valid pirq [Konrad Rzeszutek Wilk]
> - s/0/PAGE_ORDER_4K/ for guest_physmap_{add,remove}_page
>   [Konrad Rzeszutek Wilk]
> - Free new page in case of guest_physmap_add_page() failure
>   [Konrad Rzeszutek Wilk]
> - Make ARM-specific hook return -ENOSYS [Julien Grall]
> 
> Changes since 'PATCH RFC' of the 'reset everything' approach to PVHVM guest kexec:
> - Coding style, check get_gfn_query() return value, various minor fixes [Jan Beulich]
> - Do not unpause VCPUs on arch hook failure [Jan Beulich]
> ---
>  xen/arch/arm/domain.c         |  5 +++
>  xen/arch/x86/domain.c         | 84 +++++++++++++++++++++++++++++++++++++++++++
>  xen/arch/x86/hvm/hvm.c        |  2 +-
>  xen/common/domain.c           |  4 +++
>  xen/include/asm-x86/hvm/hvm.h |  2 ++
>  xen/include/xen/domain.h      |  2 ++
>  6 files changed, 98 insertions(+), 1 deletion(-)
> 
> diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
> index b2bfc7d..5bdc2e9 100644
> --- a/xen/arch/arm/domain.c
> +++ b/xen/arch/arm/domain.c
> @@ -655,6 +655,11 @@ void arch_domain_unpause(struct domain *d)
>  {
>  }
>  
> +int arch_domain_soft_reset(struct domain *d)
> +{
> +    return -ENOSYS;
> +}
> +
>  static int is_guest_pv32_psr(uint32_t psr)
>  {
>      switch (psr & PSR_MODE_MASK)
> diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
> index 34ecd7c..1829535 100644
> --- a/xen/arch/x86/domain.c
> +++ b/xen/arch/x86/domain.c
> @@ -704,6 +704,90 @@ void arch_domain_unpause(struct domain *d)
>          viridian_time_ref_count_thaw(d);
>  }
>  
> +int arch_domain_soft_reset(struct domain *d)
> +{
> +    struct page_info *page = virt_to_page(d->shared_info), *new_page;
> +    int ret = 0;
> +    struct domain *owner;
> +    unsigned long mfn, mfn_new, gfn;
> +    p2m_type_t p2mt;
> +    unsigned int i;
> +
> +    /* Soft reset is supported for HVM domains only. */
> +    if ( !is_hvm_domain(d) )
> +        return -EINVAL;
> +
> +    hvm_destroy_all_ioreq_servers(d);
> +
> +    spin_lock(&d->event_lock);
> +    for ( i = 0; i < d->nr_pirqs ; i++ )
> +    {
> +        if ( domain_pirq_to_emuirq(d, i) != IRQ_UNBOUND )
> +        {
> +            ret = unmap_domain_pirq_emuirq(d, i);
> +            if ( ret )
> +                break;
> +        }
> +    }
> +    spin_unlock(&d->event_lock);
> +
> +    if ( ret )
> +        return ret;
> +
> +    /*
> +     * shared_info page needs to be replaced with a new page, otherwise we
> +     * will get a hole if the domain does XENMAPSPACE_shared_info.
> +     */
> +
> +    owner = page_get_owner_and_reference(page);
> +    /* If shared_info page wasn't used, we do not need to replace it. */
> +    if ( owner != d )
> +        goto exit_put_page;
> +
> +    mfn = page_to_mfn(page);
> +    if ( !mfn_valid(mfn) )
> +    {
> +        printk(XENLOG_G_ERR "Dom%d's shared_info page points to invalid MFN\n",
> +               d->domain_id);
> +        ret = -EINVAL;
> +        goto exit_put_page;
> +    }
> +
> +    gfn = mfn_to_gmfn(d, mfn);
> +    if ( mfn_x(get_gfn_query(d, gfn, &p2mt)) == INVALID_MFN )
> +    {
> +        printk(XENLOG_G_ERR "Failed to get Dom%d's shared_info GFN (%lx)\n",
> +               d->domain_id, gfn);
> +        ret = -EINVAL;
> +        goto exit_put_page;
> +    }
> +
> +    new_page = alloc_domheap_page(d, 0);
> +    if ( !new_page )
> +    {
> +        printk(XENLOG_G_ERR "Failed to alloc a page to replace"
> +               " Dom%d's shared_info frame %lx\n", d->domain_id, gfn);
> +        ret = -ENOMEM;
> +        goto exit_put_gfn;
> +    }
> +    mfn_new = page_to_mfn(new_page);
> +    guest_physmap_remove_page(d, gfn, mfn, PAGE_ORDER_4K);
> +
> +    ret = guest_physmap_add_page(d, gfn, mfn_new, PAGE_ORDER_4K);
> +    if ( ret )
> +    {
> +        printk(XENLOG_G_ERR "Failed to add a page to replace"
> +               " Dom%d's shared_info frame %lx\n", d->domain_id, gfn);
> +        free_domheap_page(new_page);
> +    }
> + exit_put_gfn:
> +    put_gfn(d, gfn);
> + exit_put_page:
> +    put_page(page);
> +
> +    return ret;
> +}
> +
>  /*
>   * These are the masks of CR4 bits (subject to hardware availability) which a
>   * PV guest may not legitimiately attempt to modify.
> diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
> index 545aa91..08a7a10 100644
> --- a/xen/arch/x86/hvm/hvm.c
> +++ b/xen/arch/x86/hvm/hvm.c
> @@ -1370,7 +1370,7 @@ static void hvm_all_ioreq_servers_remove_vcpu(struct domain *d, struct vcpu *v)
>      spin_unlock(&d->arch.hvm_domain.ioreq_server.lock);
>  }
>  
> -static void hvm_destroy_all_ioreq_servers(struct domain *d)
> +void hvm_destroy_all_ioreq_servers(struct domain *d)
>  {
>      struct hvm_ioreq_server *s, *next;
>  
> diff --git a/xen/common/domain.c b/xen/common/domain.c
> index 4f805d5..8401b42 100644
> --- a/xen/common/domain.c
> +++ b/xen/common/domain.c
> @@ -1035,6 +1035,10 @@ int domain_soft_reset(struct domain *d)
>      for_each_vcpu ( d, v )
>          unmap_vcpu_info(v);
>  
> +    rc = arch_domain_soft_reset(d);
> +    if (rc)
> +        return rc;
> +
>      domain_resume(d);
>  
>      return 0;
> diff --git a/xen/include/asm-x86/hvm/hvm.h b/xen/include/asm-x86/hvm/hvm.h
> index 82f1b32..450ca0c 100644
> --- a/xen/include/asm-x86/hvm/hvm.h
> +++ b/xen/include/asm-x86/hvm/hvm.h
> @@ -229,6 +229,8 @@ struct hvm_ioreq_server *hvm_select_ioreq_server(struct domain *d,
>  int hvm_send_ioreq(struct hvm_ioreq_server *s, ioreq_t *p, bool_t buffered);
>  unsigned int hvm_broadcast_ioreq(ioreq_t *p, bool_t buffered);
>  
> +void hvm_destroy_all_ioreq_servers(struct domain *d);
> +
>  void hvm_get_guest_pat(struct vcpu *v, u64 *guest_pat);
>  int hvm_set_guest_pat(struct vcpu *v, u64 guest_pat);
>  
> diff --git a/xen/include/xen/domain.h b/xen/include/xen/domain.h
> index 848db8a..a469fe0 100644
> --- a/xen/include/xen/domain.h
> +++ b/xen/include/xen/domain.h
> @@ -65,6 +65,8 @@ void arch_domain_shutdown(struct domain *d);
>  void arch_domain_pause(struct domain *d);
>  void arch_domain_unpause(struct domain *d);
>  
> +int arch_domain_soft_reset(struct domain *d);
> +
>  int arch_set_info_guest(struct vcpu *, vcpu_guest_context_u);
>  void arch_get_info_guest(struct vcpu *, vcpu_guest_context_u);
>  
> -- 
> 2.4.3
> 

Re: [PATCH v9 08/11] xen: arch-specific hooks for domain_soft_reset()

[Jan Beulich]

>>> On 16.07.15 at 18:27, <vkuznets@redhat.com> wrote:
> --- a/xen/arch/x86/domain.c
> +++ b/xen/arch/x86/domain.c
> @@ -704,6 +704,90 @@ void arch_domain_unpause(struct domain *d)
>          viridian_time_ref_count_thaw(d);
>  }
>  
> +int arch_domain_soft_reset(struct domain *d)
> +{
> +    struct page_info *page = virt_to_page(d->shared_info), *new_page;
> +    int ret = 0;
> +    struct domain *owner;
> +    unsigned long mfn, mfn_new, gfn;
> +    p2m_type_t p2mt;
> +    unsigned int i;
> +
> +    /* Soft reset is supported for HVM domains only. */
> +    if ( !is_hvm_domain(d) )
> +        return -EINVAL;

This suggests that the whole function may better go somewhere in
hvm/.

And is this intentionally excluding PVH?

> +    hvm_destroy_all_ioreq_servers(d);

Or if the function was to stay here, you'd probably better add a
hvm_domain_soft_reset() instead of making this function non-
static.

> +    spin_lock(&d->event_lock);
> +    for ( i = 0; i < d->nr_pirqs ; i++ )
> +    {
> +        if ( domain_pirq_to_emuirq(d, i) != IRQ_UNBOUND )
> +        {
> +            ret = unmap_domain_pirq_emuirq(d, i);
> +            if ( ret )
> +                break;
> +        }
> +    }
> +    spin_unlock(&d->event_lock);
> +
> +    if ( ret )
> +        return ret;
> +
> +    /*
> +     * shared_info page needs to be replaced with a new page, otherwise we

"The shared info page ..."

> +     * will get a hole if the domain does XENMAPSPACE_shared_info.
> +     */
> +
> +    owner = page_get_owner_and_reference(page);
> +    /* If shared_info page wasn't used, we do not need to replace it. */
> +    if ( owner != d )

owner may be NULL, in which case you mustn't put_page(). Or if
you're sure owner can't be NULL here, then please ASSERT() so.
(I'm about to post a patch adjusting a remotely similar case in the
grant table code).

> +        goto exit_put_page;
> +
> +    mfn = page_to_mfn(page);
> +    if ( !mfn_valid(mfn) )

This check makes sense at best ahead of calling
page_get_owner_and_reference(), but I can't see how
virt_to_page() could produce an invalid page unless there was a
bug elsewhere, or d->shared_info was NULL.

> +    {
> +        printk(XENLOG_G_ERR "Dom%d's shared_info page points to invalid MFN\n",
> +               d->domain_id);
> +        ret = -EINVAL;
> +        goto exit_put_page;
> +    }
> +
> +    gfn = mfn_to_gmfn(d, mfn);
> +    if ( mfn_x(get_gfn_query(d, gfn, &p2mt)) == INVALID_MFN )

Perhaps even != mfn?

> +    {
> +        printk(XENLOG_G_ERR "Failed to get Dom%d's shared_info GFN (%lx)\n",
> +               d->domain_id, gfn);
> +        ret = -EINVAL;
> +        goto exit_put_page;
> +    }
> +
> +    new_page = alloc_domheap_page(d, 0);
> +    if ( !new_page )
> +    {
> +        printk(XENLOG_G_ERR "Failed to alloc a page to replace"
> +               " Dom%d's shared_info frame %lx\n", d->domain_id, gfn);
> +        ret = -ENOMEM;
> +        goto exit_put_gfn;
> +    }
> +    mfn_new = page_to_mfn(new_page);

mfn_new is used just once, and hence this variable can be easily
eliminated (either be re-using mfn after switching this and the
following lines, or by just not using an intermediate variable here
at all.

> --- a/xen/common/domain.c
> +++ b/xen/common/domain.c
> @@ -1035,6 +1035,10 @@ int domain_soft_reset(struct domain *d)
>      for_each_vcpu ( d, v )
>          unmap_vcpu_info(v);
>  
> +    rc = arch_domain_soft_reset(d);
> +    if (rc)
> +        return rc;

I don't think this can be done this late, or else you'd need to have
a way to undo everything you've done earlier in the function. This
of course also applies to what's being done inside the function. Or
if undoing is - as it looks like - rather hard, perhaps the domain
would better be crashed than left in an inconsistent state?

Also - coding style.

Jan

Re: [PATCH v9 08/11] xen: arch-specific hooks for domain_soft_reset()

[Vitaly Kuznetsov]

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

>>>> On 16.07.15 at 18:27, <vkuznets@redhat.com> wrote:
>> --- a/xen/arch/x86/domain.c
>> +++ b/xen/arch/x86/domain.c
>> @@ -704,6 +704,90 @@ void arch_domain_unpause(struct domain *d)
>>          viridian_time_ref_count_thaw(d);
>>  }
>>  
>> +int arch_domain_soft_reset(struct domain *d)
>> +{
>> +    struct page_info *page = virt_to_page(d->shared_info), *new_page;
>> +    int ret = 0;
>> +    struct domain *owner;
>> +    unsigned long mfn, mfn_new, gfn;
>> +    p2m_type_t p2mt;
>> +    unsigned int i;
>> +
>> +    /* Soft reset is supported for HVM domains only. */
>> +    if ( !is_hvm_domain(d) )
>> +        return -EINVAL;
>
> This suggests that the whole function may better go somewhere in
> hvm/.
>
> And is this intentionally excluding PVH?
>

Not really.

As far as I understand the descision is to go for "HVM without dm" way
for PVH in future? In that case, isn't is_hvm_domain() supposed to
return true for it? I hope the selected approach to kexec should "just
work" for such PVH guests.

>...
>> --- a/xen/common/domain.c
>> +++ b/xen/common/domain.c
>> @@ -1035,6 +1035,10 @@ int domain_soft_reset(struct domain *d)
>>      for_each_vcpu ( d, v )
>>          unmap_vcpu_info(v);
>>  
>> +    rc = arch_domain_soft_reset(d);
>> +    if (rc)
>> +        return rc;
>
> I don't think this can be done this late, or else you'd need to have
> a way to undo everything you've done earlier in the function. This
> of course also applies to what's being done inside the function. Or
> if undoing is - as it looks like - rather hard, perhaps the domain
> would better be crashed than left in an inconsistent state?

Crashing the domain seems reasonable.

-- 
  Vitaly

Re: [PATCH v9 08/11] xen: arch-specific hooks for domain_soft_reset()

[Jan Beulich]

>>> On 24.07.15 at 14:41, <vkuznets@redhat.com> wrote:
> "Jan Beulich" <JBeulich@suse.com> writes:
> 
>>>>> On 16.07.15 at 18:27, <vkuznets@redhat.com> wrote:
>>> --- a/xen/arch/x86/domain.c
>>> +++ b/xen/arch/x86/domain.c
>>> @@ -704,6 +704,90 @@ void arch_domain_unpause(struct domain *d)
>>>          viridian_time_ref_count_thaw(d);
>>>  }
>>>  
>>> +int arch_domain_soft_reset(struct domain *d)
>>> +{
>>> +    struct page_info *page = virt_to_page(d->shared_info), *new_page;
>>> +    int ret = 0;
>>> +    struct domain *owner;
>>> +    unsigned long mfn, mfn_new, gfn;
>>> +    p2m_type_t p2mt;
>>> +    unsigned int i;
>>> +
>>> +    /* Soft reset is supported for HVM domains only. */
>>> +    if ( !is_hvm_domain(d) )
>>> +        return -EINVAL;
>>
>> This suggests that the whole function may better go somewhere in
>> hvm/.
>>
>> And is this intentionally excluding PVH?
>>
> 
> Not really.
> 
> As far as I understand the descision is to go for "HVM without dm" way
> for PVH in future? In that case, isn't is_hvm_domain() supposed to
> return true for it?

We'd have to see how these is_...() functions end up getting
structured. For now, to cover both HVM and PVH we have
has_hvm_container_...().

Jan

[PATCH v9 09/11] 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>
Acked-by: Wei Liu <wei.liu2@citrix.com>
Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
---
Changes since v8:
- Add Wei's A-b.
- Add Konrad's R-b.
- Add Ian J A-b.
---
 tools/libxc/include/xenctrl.h | 3 +++
 tools/libxc/xc_domain.c       | 9 +++++++++
 2 files changed, 12 insertions(+)

diff --git a/tools/libxc/include/xenctrl.h b/tools/libxc/include/xenctrl.h
index 0bbae2a..a4a8843 100644
--- a/tools/libxc/include/xenctrl.h
+++ b/tools/libxc/include/xenctrl.h
@@ -1289,6 +1289,9 @@ 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 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 6db8d13..ddd04f3 100644
--- a/tools/libxc/xc_domain.c
+++ b/tools/libxc/xc_domain.c
@@ -2452,6 +2452,15 @@ int xc_domain_setvnuma(xc_interface *xch,
     return rc;
 }
 
+
+int xc_domain_soft_reset(xc_interface *xch,
+                         uint32_t domid)
+{
+    DECLARE_DOMCTL;
+    domctl.cmd = XEN_DOMCTL_soft_reset;
+    domctl.domain = (domid_t)domid;
+    return do_domctl(xch, &domctl);
+}
 /*
  * Local variables:
  * mode: C
-- 
2.4.3

[PATCH v9 10/11] 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>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
---
Changes since v8:
- None
---
 tools/libxc/include/xenguest.h | 2 ++
 tools/libxl/libxl_dm.c         | 2 +-
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/tools/libxc/include/xenguest.h b/tools/libxc/include/xenguest.h
index 7581263..c95e93c 100644
--- a/tools/libxc/include/xenguest.h
+++ b/tools/libxc/include/xenguest.h
@@ -147,6 +147,8 @@ 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 ".".
  */
+
+#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 ad434f0..613ab8b 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)
-- 
2.4.3

[PATCH v9 11/11] (lib)xl: soft reset support

[Vitaly Kuznetsov]

Use existing create/restore path to perform 'soft reset' for HVM
domains. Tear everything down, e.g. destroy domain's device model,
remove the domain from xenstore, save toolstack record and start
over.

Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
---
Changes since v8:
- Pause/unpase the domain when doing xc_domain_soft_reset
  [Konrad Rzeszutek Wilk]
- Redistribution of hunks between this patch and PATCH 3 [Ian Jackson]
- Shorten lines to 70-75 chars [Ian Jackson]
- Rephrase 'soft-reset' action in xl.cfg.pod.5 [Ian Jackson]

Changes since v7
- 'Reset everything approach': XEN_DOMCTL_soft_reset doesn't destroy
  the original domain any more.
- libxl__domain_soft_reset_state introduced [Ian Campbell]
- Separate do_domain_soft_reset to not intertwine with do_domain_create
  [Ian Campbell]

Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
---
 docs/man/xl.cfg.pod.5        |   7 +-
 tools/libxl/libxl.c          |  23 +++++-
 tools/libxl/libxl.h          |  15 ++++
 tools/libxl/libxl_create.c   | 193 ++++++++++++++++++++++++++++++++++++++-----
 tools/libxl/libxl_internal.h |   4 +
 tools/libxl/libxl_types.idl  |   2 +
 tools/libxl/xl.h             |   1 +
 tools/libxl/xl_cmdimpl.c     |  25 +++++-
 8 files changed, 240 insertions(+), 30 deletions(-)

diff --git a/docs/man/xl.cfg.pod.5 b/docs/man/xl.cfg.pod.5
index 3489b27..caed758 100644
--- a/docs/man/xl.cfg.pod.5
+++ b/docs/man/xl.cfg.pod.5
@@ -346,6 +346,11 @@ destroy the domain.
 write a "coredump" of the domain to F</var/lib/xen/dump/NAME> and then
 restart the domain.
 
+=item B<soft-reset>
+
+reset all Xen specific interfaces for the Xen-aware HVM domain allowing
+it to reastablish these interfaces and continue executing the domain.
+
 =back
 
 The default for C<on_poweroff> is C<destroy>.
@@ -367,7 +372,7 @@ 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<restart>.
+Default is C<soft-reset>.
 
 =back
 
diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index 38aff8d..2aa66a3 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -1495,6 +1495,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;
@@ -1503,6 +1504,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);
 }
 
@@ -1683,10 +1685,14 @@ static void devices_destroy_cb(libxl__egc *egc,
 
     /* Clean up qemu-save and qemu-resume files. They are
      * intermediate files created by libxc. Unfortunately they
-     * don't fit in existing userdata scheme very well.
+     * don't fit in existing userdata scheme very well. In soft reset
+     * case we need to keep the file.
      */
-    rc = libxl__remove_file(gc, libxl__device_model_savefile(gc, domid));
-    if (rc < 0) goto out;
+    if (!dis->soft_reset) {
+        rc = libxl__remove_file(gc,
+                                libxl__device_model_savefile(gc, domid));
+        if (rc < 0) goto out;
+    }
     rc = libxl__remove_file(gc,
              GCSPRINTF(XC_DEVICE_MODEL_RESTORE_FILE".%u", domid));
     if (rc < 0) goto out;
@@ -1697,7 +1703,16 @@ static void devices_destroy_cb(libxl__egc *egc,
         ctx->xch = xc_interface_open(ctx->lg,0,0);
         if (!ctx->xch) goto badchild;
 
-        rc = xc_domain_destroy(ctx->xch, domid);
+        if (!dis->soft_reset) {
+            rc = xc_domain_destroy(ctx->xch, domid);
+        }
+        else {
+            rc = xc_domain_pause(ctx->xch, domid);
+            if (rc < 0) goto badchild;
+            rc = xc_domain_soft_reset(ctx->xch, domid);
+            if (rc < 0) goto badchild;
+            rc = xc_domain_unpause(ctx->xch, domid);
+        }
         if (rc < 0) goto badchild;
         _exit(0);
 
diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
index e9d63c9..a579383 100644
--- a/tools/libxl/libxl.h
+++ b/tools/libxl/libxl.h
@@ -205,6 +205,13 @@
 #define LIBXL_HAVE_BUILDINFO_ARM_GIC_VERSION 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
  *
  * The only guarantee which libxl makes regarding ABI compatibility
@@ -1101,6 +1108,14 @@ 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,
+                            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 b785ddd..2e66e00 100644
--- a/tools/libxl/libxl_create.c
+++ b/tools/libxl/libxl_create.c
@@ -484,8 +484,6 @@ int libxl__domain_make(libxl__gc *gc, libxl_domain_config *d_config,
     /* convenience aliases */
     libxl_domain_create_info *info = &d_config->c_info;
 
-    assert(!libxl_domid_valid_guest(*domid));
-
     uuid_string = libxl__uuid2string(gc, info->uuid);
     if (!uuid_string) {
         rc = ERROR_NOMEM;
@@ -506,7 +504,6 @@ int libxl__domain_make(libxl__gc *gc, libxl_domain_config *d_config,
         }
         flags |= XEN_DOMCTL_CDF_hap;
     }
-    *domid = -1;
 
     /* Ultimately, handle is an array of 16 uint8_t, same as uuid */
     libxl_uuid_copy(ctx, (libxl_uuid *)handle, &info->uuid);
@@ -518,13 +515,17 @@ int libxl__domain_make(libxl__gc *gc, libxl_domain_config *d_config,
         goto out;
     }
 
-    ret = xc_domain_create_config(ctx->xch, info->ssidref,
-                                  handle, flags, domid,
-                                  xc_config);
-    if (ret < 0) {
-        LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "domain creation fail");
-        rc = ERROR_FAIL;
-        goto out;
+    /* Valid domid here means we're soft resetting */
+    if (!libxl_domid_valid_guest(*domid)) {
+        ret = xc_domain_create_config(ctx->xch, info->ssidref,
+                                      handle, flags, domid,
+                                      xc_config);
+        if (ret < 0) {
+            LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR,
+                             "domain creation fail");
+            rc = ERROR_FAIL;
+            goto out;
+        }
     }
 
     rc = libxl__arch_domain_save_config(gc, d_config, xc_config);
@@ -734,9 +735,8 @@ static void initiate_domain_create(libxl__egc *egc,
     libxl_domain_config *const d_config = dcs->guest_config;
     libxl__domain_build_state *const state = &dcs->build_state;
     const int restore_fd = dcs->restore_fd;
-    memset(&dcs->build_state, 0, sizeof(dcs->build_state));
 
-    domid = 0;
+    domid = dcs->domid_soft_reset;
 
     if (d_config->c_info.ssid_label) {
         char *s = d_config->c_info.ssid_label;
@@ -884,7 +884,7 @@ static void initiate_domain_create(libxl__egc *egc,
             d_config->nics[i].devid = ++last_devid;
     }
 
-    if (restore_fd >= 0) {
+    if (restore_fd >= 0 || dcs->domid_soft_reset != INVALID_DOMID) {
         LOG(DEBUG, "restoring, not running bootloader");
         domcreate_bootloader_done(egc, &dcs->bl, 0);
     } else  {
@@ -958,7 +958,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 && dcs->domid_soft_reset == INVALID_DOMID) {
         rc = libxl__domain_build(gc, d_config, domid, state);
         domcreate_rebuild_done(egc, dcs, rc);
         return;
@@ -988,9 +988,11 @@ static void domcreate_bootloader_done(libxl__egc *egc,
         rc = ERROR_INVAL;
         goto out;
     }
-    libxl__xc_domain_restore(egc, dcs,
-                             hvm, pae, superpages);
-    return;
+    if ( restore_fd >= 0 ) {
+        libxl__xc_domain_restore(egc, dcs,
+                                 hvm, pae, superpages);
+        return;
+    }
 
  out:
     libxl__xc_domain_restore_done(egc, dcs, rc, 0, 0);
@@ -1074,8 +1076,12 @@ void libxl__xc_domain_restore_done(libxl__egc *egc, void *dcs_void,
         goto out;
 
     if (info->type == LIBXL_DOMAIN_TYPE_HVM) {
-        state->saved_state = GCSPRINTF(
-                       XC_DEVICE_MODEL_RESTORE_FILE".%d", domid);
+        if (fd != -1)
+            state->saved_state = GCSPRINTF(
+                XC_DEVICE_MODEL_RESTORE_FILE".%d", domid);
+        else
+            state->saved_state = GCSPRINTF(
+                XC_DEVICE_MODEL_SAVE_FILE".%d", domid);
     }
 
 out:
@@ -1084,9 +1090,12 @@ out:
         libxl__file_reference_unmap(&state->pv_ramdisk);
     }
 
-    esave = errno;
-    libxl_fd_set_nonblock(ctx, fd, 0);
-    errno = esave;
+    /* fd == -1 here means we're doing soft reset */
+    if (fd != -1) {
+        esave = errno;
+        libxl_fd_set_nonblock(ctx, fd, 0);
+        errno = esave;
+    }
     domcreate_rebuild_done(egc, dcs, ret);
 }
 
@@ -1507,6 +1516,14 @@ typedef struct {
     uint32_t *domid_out;
 } libxl__app_domain_create_state;
 
+typedef struct {
+    libxl__app_domain_create_state cdcs;
+    libxl__domain_destroy_state dds;
+    libxl__domain_suspend_state dss;
+    uint8_t *toolstack_buf;
+    uint32_t toolstack_len;
+} libxl__domain_soft_reset_state;
+
 static void domain_create_cb(libxl__egc *egc,
                              libxl__domain_create_state *dcs,
                              int rc, uint32_t domid);
@@ -1528,6 +1545,7 @@ static int do_domain_create(libxl_ctx *ctx, libxl_domain_config *d_config,
     cdcs->dcs.restore_fd = restore_fd;
     cdcs->dcs.callback = domain_create_cb;
     cdcs->dcs.checkpointed_stream = checkpointed_stream;
+    cdcs->dcs.domid_soft_reset = INVALID_DOMID;
     libxl__ao_progress_gethow(&cdcs->dcs.aop_console_how, aop_console_how);
     cdcs->domid_out = domid;
 
@@ -1536,6 +1554,122 @@ static int do_domain_create(libxl_ctx *ctx, libxl_domain_config *d_config,
     return AO_INPROGRESS;
 }
 
+static void domain_soft_reset_cb(libxl__egc *egc,
+                                 libxl__domain_destroy_state *dds,
+                                 int rc)
+{
+    STATE_AO_GC(dds->ao);
+    libxl__domain_soft_reset_state *srs = CONTAINER_OF(dds, *srs, dds);
+    libxl__app_domain_create_state *cdcs = &srs->cdcs;
+
+    if (rc) {
+        LOG(ERROR, "destruction of domain %u failed", dds->domid);
+        goto error;
+    }
+
+    rc = libxl__toolstack_restore(cdcs->dcs.domid_soft_reset,
+                                  srs->toolstack_buf,
+                                  srs->toolstack_len,
+                                  &cdcs->dcs.shs);
+    if (rc) {
+        LOG(ERROR, "failed to restore toolstack record");
+        goto error;
+    }
+
+    initiate_domain_create(egc, &cdcs->dcs);
+    return;
+
+error:
+    domcreate_complete(egc, &cdcs->dcs, rc);
+}
+
+static int do_domain_soft_reset(libxl_ctx *ctx,
+                                libxl_domain_config *d_config,
+                                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__domain_soft_reset_state *srs;
+    libxl__app_domain_create_state *cdcs;
+    libxl__domain_create_state *dcs;
+    libxl__domain_build_state *state;
+    libxl__domain_suspend_state *dss;
+    char *dom_path, *xs_store_mfn, *xs_console_mfn;
+    uint32_t domid_out;
+    int rc;
+
+    GCNEW(srs);
+    cdcs = &srs->cdcs;
+    dcs = &cdcs->dcs;
+    state = &dcs->build_state;
+    dss = &srs->dss;
+
+    srs->cdcs.dcs.ao = ao;
+    srs->cdcs.dcs.guest_config = d_config;
+    libxl_domain_config_init(&srs->cdcs.dcs.guest_config_saved);
+    libxl_domain_config_copy(ctx, &srs->cdcs.dcs.guest_config_saved,
+                             d_config);
+    cdcs->dcs.restore_fd = -1;
+    cdcs->dcs.domid_soft_reset = domid_soft_reset;
+    cdcs->dcs.callback = domain_create_cb;
+    libxl__ao_progress_gethow(&srs->cdcs.dcs.aop_console_how,
+                              aop_console_how);
+    cdcs->domid_out = &domid_out;
+
+    dom_path = libxl__xs_get_dompath(gc, domid_soft_reset);
+    if (!dom_path) {
+        LOG(ERROR, "failed to read domain path");
+        return AO_CREATE_FAIL(ERROR_FAIL);
+    }
+
+    xs_store_mfn = xs_read(ctx->xsh, XBT_NULL,
+                           GCSPRINTF("%s/store/ring-ref", dom_path),
+                           NULL);
+    state->store_mfn = xs_store_mfn ? atol(xs_store_mfn): 0;
+    free(xs_store_mfn);
+
+    xs_console_mfn = xs_read(ctx->xsh, XBT_NULL,
+                             GCSPRINTF("%s/console/ring-ref", dom_path),
+                             NULL);
+    state->console_mfn = xs_console_mfn ? atol(xs_console_mfn): 0;
+    free(xs_console_mfn);
+
+    dss->ao = ao;
+    dss->domid = domid_soft_reset;
+    dss->dm_savefile = GCSPRINTF(XC_DEVICE_MODEL_SAVE_FILE".%d",
+                                 domid_soft_reset);
+
+    rc = libxl__toolstack_save(domid_soft_reset, &srs->toolstack_buf,
+                               &srs->toolstack_len, dss);
+    if (rc) {
+        LOG(ERROR, "failed to save toolstack record");
+        return AO_CREATE_FAIL(ERROR_FAIL);
+    }
+
+    rc = libxl__domain_suspend_device_model(gc, dss);
+    if (rc) {
+        LOG(ERROR, "failed to suspend device model");
+        return AO_CREATE_FAIL(ERROR_FAIL);
+    }
+
+    /*
+     * On the domain creation path it will be introduced to xenstore
+     * with (probably) different store/console channels so we need to
+     * release it here.
+     */
+    xs_release_domain(ctx->xsh, cdcs->dcs.domid_soft_reset);
+
+    srs->dds.ao = ao;
+    srs->dds.domid = domid_soft_reset;
+    srs->dds.callback = domain_soft_reset_cb;
+    srs->dds.soft_reset = true;
+    libxl__domain_destroy(egc, &srs->dds);
+
+    return AO_INPROGRESS;
+}
+
 static void domain_create_cb(libxl__egc *egc,
                              libxl__domain_create_state *dcs,
                              int rc, uint32_t domid)
@@ -1567,6 +1701,21 @@ int libxl_domain_create_restore(libxl_ctx *ctx, libxl_domain_config *d_config,
                             params->checkpointed_stream, ao_how, aop_console_how);
 }
 
+int libxl_domain_soft_reset(libxl_ctx *ctx,
+                            libxl_domain_config *d_config,
+                            uint32_t domid,
+                            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_soft_reset(ctx, d_config, domid, ao_how,
+                                aop_console_how);
+}
+
 /*
  * Local variables:
  * mode: C
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 5235d25..a648734 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -107,6 +107,7 @@
 #define STUBDOM_SPECIAL_CONSOLES 3
 #define TAP_DEVICE_SUFFIX "-emu"
 #define DOMID_XS_PATH "domid"
+#define INVALID_DOMID ~0
 
 #define ARRAY_SIZE(a) (sizeof(a) / sizeof(a[0]))
 
@@ -3098,6 +3099,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 {
@@ -3112,6 +3114,7 @@ struct libxl__domain_destroy_state {
     int stubdom_finished;
     libxl__destroy_domid_state domain;
     int domain_finished;
+    bool soft_reset;
 };
 
 /*
@@ -3212,6 +3215,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 */
diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
index 337257a..f887b28 100644
--- a/tools/libxl/libxl_types.idl
+++ b/tools/libxl/libxl_types.idl
@@ -127,6 +127,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", [
diff --git a/tools/libxl/xl.h b/tools/libxl/xl.h
index 6c19c0d..0021112 100644
--- a/tools/libxl/xl.h
+++ b/tools/libxl/xl.h
@@ -194,6 +194,7 @@ typedef enum {
     DOMAIN_RESTART_NONE = 0,     /* No domain restart */
     DOMAIN_RESTART_NORMAL,       /* Domain should be restarted */
     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 f1604b1..97a3aac 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:
@@ -1338,7 +1340,7 @@ static void parse_config_data(const char *config_source,
     }
 
     if (xlu_cfg_get_string (config, "on_soft_reset", &buf, 0))
-        buf = "restart";
+        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);
@@ -2391,6 +2393,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. */
@@ -2566,6 +2573,7 @@ static uint32_t create_domain(struct domain_create *dom_info)
     int restore_fd = -1;
     const libxl_asyncprogress_how *autoconnect_console_how;
     struct save_file_header hdr;
+    uint32_t domid_soft_reset = INVALID_DOMID;
 
     int restoring = (restore_file || (migrate_fd >= 0));
 
@@ -2775,7 +2783,13 @@ start:
          * restore/migrate-receive it again.
          */
         restoring = 0;
-    }else{
+    } else if ( domid_soft_reset != INVALID_DOMID ) {
+        /* Do soft reset */
+        ret = libxl_domain_soft_reset(ctx, &d_config, domid_soft_reset,
+                                      0, autoconnect_console_how);
+        domid = domid_soft_reset;
+        domid_soft_reset = INVALID_DOMID;
+    } else {
         ret = libxl_domain_create_new(ctx, &d_config, &domid,
                                       0, autoconnect_console_how);
     }
@@ -2839,8 +2853,13 @@ 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_soft_reset = domid;
+                domid = INVALID_DOMID;
+                /* fall through */
             case DOMAIN_RESTART_RENAME:
-                if (!preserve_domain(&domid, event, &d_config)) {
+                if (domid_soft_reset == INVALID_DOMID &&
+                    !preserve_domain(&domid, event, &d_config)) {
                     /* If we fail then exit leaving the old domain in place. */
                     ret = -1;
                     goto out;
-- 
2.4.3

Re: [PATCH v9 11/11] (lib)xl: soft reset support

[Konrad Rzeszutek Wilk]

> diff --git a/docs/man/xl.cfg.pod.5 b/docs/man/xl.cfg.pod.5
> index 3489b27..caed758 100644
> --- a/docs/man/xl.cfg.pod.5
> +++ b/docs/man/xl.cfg.pod.5
> @@ -346,6 +346,11 @@ destroy the domain.
>  write a "coredump" of the domain to F</var/lib/xen/dump/NAME> and then
>  restart the domain.
>  
> +=item B<soft-reset>
> +
> +reset all Xen specific interfaces for the Xen-aware HVM domain allowing

s/reset/Reset/
> +it to reastablish these interfaces and continue executing the domain.
> +
>  =back
>  
>  The default for C<on_poweroff> is C<destroy>.
> @@ -367,7 +372,7 @@ 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<restart>.
> +Default is C<soft-reset>.
>  
>  =back
>  

.. snip..
> @@ -518,13 +515,17 @@ int libxl__domain_make(libxl__gc *gc, libxl_domain_config *d_config,
>          goto out;
>      }
>  
> -    ret = xc_domain_create_config(ctx->xch, info->ssidref,
> -                                  handle, flags, domid,
> -                                  xc_config);
> -    if (ret < 0) {
> -        LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "domain creation fail");
> -        rc = ERROR_FAIL;
> -        goto out;
> +    /* Valid domid here means we're soft resetting */

Missing full stop.
> +    if (!libxl_domid_valid_guest(*domid)) {
> +        ret = xc_domain_create_config(ctx->xch, info->ssidref,
> +                                      handle, flags, domid,
> +                                      xc_config);
> +        if (ret < 0) {
> +            LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR,
> +                             "domain creation fail");
> +            rc = ERROR_FAIL;
> +            goto out;
> +        }
>      }
>  
>      rc = libxl__arch_domain_save_config(gc, d_config, xc_config);
> @@ -734,9 +735,8 @@ static void initiate_domain_create(libxl__egc *egc,
>      libxl_domain_config *const d_config = dcs->guest_config;
>      libxl__domain_build_state *const state = &dcs->build_state;
>      const int restore_fd = dcs->restore_fd;
> -    memset(&dcs->build_state, 0, sizeof(dcs->build_state));
>  
> -    domid = 0;
> +    domid = dcs->domid_soft_reset;
>  
>      if (d_config->c_info.ssid_label) {
>          char *s = d_config->c_info.ssid_label;
> @@ -884,7 +884,7 @@ static void initiate_domain_create(libxl__egc *egc,
>              d_config->nics[i].devid = ++last_devid;
>      }
>  
> -    if (restore_fd >= 0) {
> +    if (restore_fd >= 0 || dcs->domid_soft_reset != INVALID_DOMID) {
>          LOG(DEBUG, "restoring, not running bootloader");
>          domcreate_bootloader_done(egc, &dcs->bl, 0);
>      } else  {
> @@ -958,7 +958,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 && dcs->domid_soft_reset == INVALID_DOMID) {
>          rc = libxl__domain_build(gc, d_config, domid, state);
>          domcreate_rebuild_done(egc, dcs, rc);
>          return;
> @@ -988,9 +988,11 @@ static void domcreate_bootloader_done(libxl__egc *egc,
>          rc = ERROR_INVAL;
>          goto out;
>      }
> -    libxl__xc_domain_restore(egc, dcs,
> -                             hvm, pae, superpages);
> -    return;
> +    if ( restore_fd >= 0 ) {
> +        libxl__xc_domain_restore(egc, dcs,
> +                                 hvm, pae, superpages);
> +        return;
> +    }
>  
>   out:
>      libxl__xc_domain_restore_done(egc, dcs, rc, 0, 0);
> @@ -1074,8 +1076,12 @@ void libxl__xc_domain_restore_done(libxl__egc *egc, void *dcs_void,
>          goto out;
>  
>      if (info->type == LIBXL_DOMAIN_TYPE_HVM) {
> -        state->saved_state = GCSPRINTF(
> -                       XC_DEVICE_MODEL_RESTORE_FILE".%d", domid);
> +        if (fd != -1)
> +            state->saved_state = GCSPRINTF(
> +                XC_DEVICE_MODEL_RESTORE_FILE".%d", domid);
> +        else
> +            state->saved_state = GCSPRINTF(
> +                XC_DEVICE_MODEL_SAVE_FILE".%d", domid);
>      }
>  
>  out:
> @@ -1084,9 +1090,12 @@ out:
>          libxl__file_reference_unmap(&state->pv_ramdisk);
>      }
>  
> -    esave = errno;
> -    libxl_fd_set_nonblock(ctx, fd, 0);
> -    errno = esave;
> +    /* fd == -1 here means we're doing soft reset */

Missing full stop.

> +    if (fd != -1) {
> +        esave = errno;
> +        libxl_fd_set_nonblock(ctx, fd, 0);
> +        errno = esave;
> +    }
>      domcreate_rebuild_done(egc, dcs, ret);
>  }
>  
> @@ -1507,6 +1516,14 @@ typedef struct {
>      uint32_t *domid_out;
>  } libxl__app_domain_create_state;
>  
> +typedef struct {
> +    libxl__app_domain_create_state cdcs;
> +    libxl__domain_destroy_state dds;
> +    libxl__domain_suspend_state dss;
> +    uint8_t *toolstack_buf;
> +    uint32_t toolstack_len;
> +} libxl__domain_soft_reset_state;
> +
>  static void domain_create_cb(libxl__egc *egc,
>                               libxl__domain_create_state *dcs,
>                               int rc, uint32_t domid);
> @@ -1528,6 +1545,7 @@ static int do_domain_create(libxl_ctx *ctx, libxl_domain_config *d_config,
>      cdcs->dcs.restore_fd = restore_fd;
>      cdcs->dcs.callback = domain_create_cb;
>      cdcs->dcs.checkpointed_stream = checkpointed_stream;
> +    cdcs->dcs.domid_soft_reset = INVALID_DOMID;
>      libxl__ao_progress_gethow(&cdcs->dcs.aop_console_how, aop_console_how);
>      cdcs->domid_out = domid;
>  
> @@ -1536,6 +1554,122 @@ static int do_domain_create(libxl_ctx *ctx, libxl_domain_config *d_config,
>      return AO_INPROGRESS;
>  }
>  
> +static void domain_soft_reset_cb(libxl__egc *egc,
> +                                 libxl__domain_destroy_state *dds,
> +                                 int rc)
> +{
> +    STATE_AO_GC(dds->ao);
> +    libxl__domain_soft_reset_state *srs = CONTAINER_OF(dds, *srs, dds);
> +    libxl__app_domain_create_state *cdcs = &srs->cdcs;
> +
> +    if (rc) {
> +        LOG(ERROR, "destruction of domain %u failed", dds->domid);

s/failed/failed!/ or s/failed/failed./

> +        goto error;
> +    }
> +
> +    rc = libxl__toolstack_restore(cdcs->dcs.domid_soft_reset,
> +                                  srs->toolstack_buf,
> +                                  srs->toolstack_len,
> +                                  &cdcs->dcs.shs);
> +    if (rc) {
> +        LOG(ERROR, "failed to restore toolstack record");

I bet you know what I am going to say here :-)

> +        goto error;
> +    }
> +
> +    initiate_domain_create(egc, &cdcs->dcs);
> +    return;
> +
> +error:
> +    domcreate_complete(egc, &cdcs->dcs, rc);
> +}
> +
> +static int do_domain_soft_reset(libxl_ctx *ctx,
> +                                libxl_domain_config *d_config,
> +                                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__domain_soft_reset_state *srs;
> +    libxl__app_domain_create_state *cdcs;
> +    libxl__domain_create_state *dcs;
> +    libxl__domain_build_state *state;
> +    libxl__domain_suspend_state *dss;
> +    char *dom_path, *xs_store_mfn, *xs_console_mfn;
> +    uint32_t domid_out;
> +    int rc;
> +
> +    GCNEW(srs);
> +    cdcs = &srs->cdcs;
> +    dcs = &cdcs->dcs;
> +    state = &dcs->build_state;
> +    dss = &srs->dss;
> +
> +    srs->cdcs.dcs.ao = ao;
> +    srs->cdcs.dcs.guest_config = d_config;
> +    libxl_domain_config_init(&srs->cdcs.dcs.guest_config_saved);
> +    libxl_domain_config_copy(ctx, &srs->cdcs.dcs.guest_config_saved,
> +                             d_config);
> +    cdcs->dcs.restore_fd = -1;
> +    cdcs->dcs.domid_soft_reset = domid_soft_reset;
> +    cdcs->dcs.callback = domain_create_cb;
> +    libxl__ao_progress_gethow(&srs->cdcs.dcs.aop_console_how,
> +                              aop_console_how);
> +    cdcs->domid_out = &domid_out;
> +
> +    dom_path = libxl__xs_get_dompath(gc, domid_soft_reset);
> +    if (!dom_path) {
> +        LOG(ERROR, "failed to read domain path");
> +        return AO_CREATE_FAIL(ERROR_FAIL);
> +    }
> +
> +    xs_store_mfn = xs_read(ctx->xsh, XBT_NULL,
> +                           GCSPRINTF("%s/store/ring-ref", dom_path),
> +                           NULL);
> +    state->store_mfn = xs_store_mfn ? atol(xs_store_mfn): 0;
> +    free(xs_store_mfn);
> +
> +    xs_console_mfn = xs_read(ctx->xsh, XBT_NULL,
> +                             GCSPRINTF("%s/console/ring-ref", dom_path),
> +                             NULL);
> +    state->console_mfn = xs_console_mfn ? atol(xs_console_mfn): 0;
> +    free(xs_console_mfn);
> +
> +    dss->ao = ao;
> +    dss->domid = domid_soft_reset;
> +    dss->dm_savefile = GCSPRINTF(XC_DEVICE_MODEL_SAVE_FILE".%d",
> +                                 domid_soft_reset);
> +
> +    rc = libxl__toolstack_save(domid_soft_reset, &srs->toolstack_buf,
> +                               &srs->toolstack_len, dss);
> +    if (rc) {
> +        LOG(ERROR, "failed to save toolstack record");

Here.
> +        return AO_CREATE_FAIL(ERROR_FAIL);
> +    }
> +
> +    rc = libxl__domain_suspend_device_model(gc, dss);
> +    if (rc) {
> +        LOG(ERROR, "failed to suspend device model");

Ditto
> +        return AO_CREATE_FAIL(ERROR_FAIL);
> +    }
> +
> +    /*
> +     * On the domain creation path it will be introduced to xenstore

What is 'it'?

> +     * with (probably) different store/console channels so we need to
> +     * release it here.

I think what you mean is that:
On the domain creation path the store/console channels will be
introduced which will probably different so we need to release
the old ones here.
?

> +     */
> +    xs_release_domain(ctx->xsh, cdcs->dcs.domid_soft_reset);
> +
> +    srs->dds.ao = ao;
> +    srs->dds.domid = domid_soft_reset;
> +    srs->dds.callback = domain_soft_reset_cb;
> +    srs->dds.soft_reset = true;
> +    libxl__domain_destroy(egc, &srs->dds);
> +
> +    return AO_INPROGRESS;
> +}
> +

.. snip..
> @@ -2775,7 +2783,13 @@ start:
>           * restore/migrate-receive it again.
>           */
>          restoring = 0;
> -    }else{
> +    } else if ( domid_soft_reset != INVALID_DOMID ) {
> +        /* Do soft reset */

Full stop missing.
> +        ret = libxl_domain_soft_reset(ctx, &d_config, domid_soft_reset,
> +                                      0, autoconnect_console_how);
> +        domid = domid_soft_reset;
> +        domid_soft_reset = INVALID_DOMID;
> +    } else {
>          ret = libxl_domain_create_new(ctx, &d_config, &domid,
>                                        0, autoconnect_console_how);
>      }
> @@ -2839,8 +2853,13 @@ 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_soft_reset = domid;
> +                domid = INVALID_DOMID;
> +                /* fall through */
>              case DOMAIN_RESTART_RENAME:
> -                if (!preserve_domain(&domid, event, &d_config)) {
> +                if (domid_soft_reset == INVALID_DOMID &&
> +                    !preserve_domain(&domid, event, &d_config)) {
>                      /* If we fail then exit leaving the old domain in place. */
>                      ret = -1;
>                      goto out;
> -- 
> 2.4.3
> 

Re: [PATCH v9 11/11] (lib)xl: soft reset support

[Vitaly Kuznetsov]

Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> writes:

...

>> +        return AO_CREATE_FAIL(ERROR_FAIL);
>> +    }
>> +
>> +    /*
>> +     * On the domain creation path it will be introduced to xenstore
>
> What is 'it'?
>

The domain :-)

>> +     * with (probably) different store/console channels so we need to
>> +     * release it here.
>
> I think what you mean is that:
> On the domain creation path the store/console channels will be
> introduced which will probably different so we need to release
> the old ones here.
> ?

I meant to say is we introduce the newly created domain to xenstored
(and all appropriate records are being created). But your desctiption is
better.

...

-- 
  Vitaly