[PATCH RFC/WIP 0/4] toolstack-based approach to pvhvm guest kexec

[PATCH RFC/WIP 0/4] toolstack-based approach to pvhvm guest kexec

[Vitaly Kuznetsov]

Here is an early prototype/proof-of-concept of an idea that instead of taking
care of all things (event channels, shared/vcpu info, grants,... ) we can
rebuild the whole domain performing kexec with toolstack.

This is a 'naive and simple' solution leveraging save/restore path. It was
tested with the changes to linux kernel listed below.

Now I'm going to take a look at how we can avoid copying memory and just
reassign it. Any ideas, suggestions and pointers are more than welcome!

diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c
index c0cb11f..b3b8ff0 100644
--- a/arch/x86/xen/enlighten.c
+++ b/arch/x86/xen/enlighten.c
@@ -33,6 +33,10 @@
 #include <linux/memblock.h>
 #include <linux/edd.h>
 
+#ifdef CONFIG_KEXEC
+#include <linux/kexec.h>
+#endif
+
 #include <xen/xen.h>
 #include <xen/events.h>
 #include <xen/interface/xen.h>
@@ -1810,6 +1814,22 @@ static struct notifier_block xen_hvm_cpu_notifier = {
        .notifier_call  = xen_hvm_cpu_notify,
 };
 
+#ifdef CONFIG_KEXEC
+static void xen_pvhvm_kexec_shutdown(void)
+{
+       native_machine_shutdown();
+       if (kexec_in_progress) {
+               xen_reboot(SHUTDOWN_kexec);
+       }
+}
+
+static void xen_pvhvm_crash_shutdown(struct pt_regs *regs)
+{
+       native_machine_crash_shutdown(regs);
+       xen_reboot(SHUTDOWN_kexec);
+}
+#endif
+
 static void __init xen_hvm_guest_init(void)
 {
        init_hvm_pv_info();
@@ -1826,6 +1846,10 @@ static void __init xen_hvm_guest_init(void)
        x86_init.irqs.intr_init = xen_init_IRQ;
        xen_hvm_init_time_ops();
        xen_hvm_init_mmu_ops();
+#ifdef CONFIG_KEXEC
+       machine_ops.shutdown = xen_pvhvm_kexec_shutdown;
+       machine_ops.crash_shutdown = xen_pvhvm_crash_shutdown;
+#endif
 }
 
 static bool xen_nopv = false;
diff --git a/include/xen/interface/sched.h b/include/xen/interface/sched.h
index 9ce0839..6ccf3c1 100644
--- a/include/xen/interface/sched.h
+++ b/include/xen/interface/sched.h
@@ -107,5 +107,6 @@ struct sched_watchdog {
 #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_kexec      5  /* Soft-reset for kexec.                      */
 
 #endif /* __XEN_PUBLIC_SCHED_H__ */

Vitaly Kuznetsov (4):
  Introduce SHUTDOWN_kexec shutdown reason
  libxc: treat domain in SHUTDOWN_kexec state as being already suspended
  libxl: add SHUTDOWN_kexec state support
  libxl: rebuild guest domains on kexec

 tools/libxc/xc_domain_save.c      |  3 +-
 tools/libxl/libxl.c               |  1 +
 tools/libxl/libxl.h               |  1 +
 tools/libxl/libxl_dom.c           |  6 ++++
 tools/libxl/libxl_internal.h      |  1 +
 tools/libxl/libxl_types.idl       |  1 +
 tools/libxl/xl_cmdimpl.c          | 62 +++++++++++++++++++++++++++++++++++++--
 tools/python/xen/lowlevel/xl/xl.c |  1 +
 xen/include/public/sched.h        |  3 +-
 9 files changed, 75 insertions(+), 4 deletions(-)

-- 
1.9.3

[PATCH RFC/WIP 1/4] Introduce SHUTDOWN_kexec shutdown reason

[Vitaly Kuznetsov]

Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
---
 tools/libxl/libxl_types.idl       | 1 +
 tools/python/xen/lowlevel/xl/xl.c | 1 +
 xen/include/public/sched.h        | 3 ++-
 3 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
index 649ce50..2405c13 100644
--- a/tools/libxl/libxl_types.idl
+++ b/tools/libxl/libxl_types.idl
@@ -148,6 +148,7 @@ libxl_shutdown_reason = Enumeration("shutdown_reason", [
     (2, "suspend"),
     (3, "crash"),
     (4, "watchdog"),
+    (5, "kexec"),
     ], init_val = "LIBXL_SHUTDOWN_REASON_UNKNOWN")
 
 libxl_vga_interface_type = Enumeration("vga_interface_type", [
diff --git a/tools/python/xen/lowlevel/xl/xl.c b/tools/python/xen/lowlevel/xl/xl.c
index 32f982a..04231f1 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_KEXEC);
 
     genwrap__init(m);
 }
diff --git a/xen/include/public/sched.h b/xen/include/public/sched.h
index a30b11d..6abf225 100644
--- a/xen/include/public/sched.h
+++ b/xen/include/public/sched.h
@@ -158,7 +158,8 @@ DEFINE_XEN_GUEST_HANDLE(sched_watchdog_t);
 #define SHUTDOWN_suspend    2  /* Clean up, save suspend info, kill.         */
 #define SHUTDOWN_crash      3  /* Tell controller we've crashed.             */
 #define SHUTDOWN_watchdog   4  /* Restart because watchdog time expired.     */
-#define SHUTDOWN_MAX        4  /* Maximum valid shutdown reason.             */
+#define SHUTDOWN_kexec      5  /* Soft-reboot for kexec.                     */
+#define SHUTDOWN_MAX        5  /* Maximum valid shutdown reason.             */
 /* ` } */
 
 #endif /* __XEN_PUBLIC_SCHED_H__ */
-- 
1.9.3

[PATCH RFC/WIP 2/4] libxc: treat domain in SHUTDOWN_kexec state as being already suspended

[Vitaly Kuznetsov]

Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
---
 tools/libxc/xc_domain_save.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/tools/libxc/xc_domain_save.c b/tools/libxc/xc_domain_save.c
index 42c4752..5423bfa 100644
--- a/tools/libxc/xc_domain_save.c
+++ b/tools/libxc/xc_domain_save.c
@@ -366,7 +366,8 @@ static int suspend_and_state(int (*suspend)(void*), void* data,
     }
 
     if ( (xc_domain_getinfo(xch, dom, 1, info) != 1) ||
-         !info->shutdown || (info->shutdown_reason != SHUTDOWN_suspend) )
+         !info->shutdown || ((info->shutdown_reason != SHUTDOWN_suspend) &&
+                             (info->shutdown_reason != SHUTDOWN_kexec)) )
     {
         ERROR("Domain not in suspended state");
         return -1;
-- 
1.9.3

[PATCH RFC/WIP 3/4] libxl: add SHUTDOWN_kexec state support

[Vitaly Kuznetsov]

Add kexec to domain suspend state and treat a domain as already suspended
and successfully responded to suspend request if the flag is set.

Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
---
 tools/libxl/libxl.c          | 1 +
 tools/libxl/libxl.h          | 1 +
 tools/libxl/libxl_dom.c      | 6 ++++++
 tools/libxl/libxl_internal.h | 1 +
 4 files changed, 9 insertions(+)

diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index 2d29ad2..0ba3285 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -786,6 +786,7 @@ int libxl_domain_suspend(libxl_ctx *ctx, uint32_t domid, int fd, int flags,
     dss->type = type;
     dss->live = flags & LIBXL_SUSPEND_LIVE;
     dss->debug = flags & LIBXL_SUSPEND_DEBUG;
+    dss->kexec = flags & LIBXL_SUSPEND_KEXEC;
 
     libxl__domain_suspend(egc, dss);
     return AO_INPROGRESS;
diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
index 06bbca6..c8185f9 100644
--- a/tools/libxl/libxl.h
+++ b/tools/libxl/libxl.h
@@ -689,6 +689,7 @@ int libxl_domain_suspend(libxl_ctx *ctx, uint32_t domid, int fd,
                          LIBXL_EXTERNAL_CALLERS_ONLY;
 #define LIBXL_SUSPEND_DEBUG 1
 #define LIBXL_SUSPEND_LIVE 2
+#define LIBXL_SUSPEND_KEXEC 4
 
 /* @param suspend_cancel [from xenctrl.h:xc_domain_resume( @param fast )]
  *   If this parameter is true, use co-operative resume. The guest
diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c
index 69e6088..b2f3a09 100644
--- a/tools/libxl/libxl_dom.c
+++ b/tools/libxl/libxl_dom.c
@@ -1038,6 +1038,12 @@ int libxl__domain_suspend_common_callback(void *user)
     /* Convenience aliases */
     const uint32_t domid = dss->domid;
 
+    if (dss->kexec) {
+        /* No suspend required if we're in SHUTDOWN_kexec */
+        dss->guest_responded = 1;
+        goto guest_suspended;
+    }
+
     if (dss->hvm) {
         xc_get_hvm_param(CTX->xch, domid, HVM_PARAM_CALLBACK_IRQ, &hvm_pvdrv);
         xc_get_hvm_param(CTX->xch, domid, HVM_PARAM_ACPI_S_STATE, &hvm_s_state);
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 9d17586..cdef241 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -2314,6 +2314,7 @@ struct libxl__domain_suspend_state {
     libxl_domain_type type;
     int live;
     int debug;
+    int kexec;
     const libxl_domain_remus_info *remus;
     /* private */
     xc_evtchn *xce; /* event channel handle */
-- 
1.9.3

[PATCH RFC/WIP 4/4] libxl: rebuild guest domains on kexec

[Vitaly Kuznetsov]

When a PVHVM linux guest performs kexec there are lots of things which
require taking care of:
- shared info, vcpu_info
- grants
- event channels
- ...
Instead of taking care of all these things we can rebuild the domain
performing kexec from scratch doing so-called soft-reboot. Here is a naive
and simple implementation which leverages save/restore (migrate) path. It
actually copies all the memory through a pipe so it is not very fast and
the host is required to have free memory to host two copies of the guest
doing kexec.

The idea was suggested by David Vrabel, Jan Beulich, and Konrad Rzeszutek Wilk.

Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
---
 tools/libxl/xl_cmdimpl.c | 62 ++++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 60 insertions(+), 2 deletions(-)

diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index 4fc46eb..a9bce17 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -1774,7 +1774,8 @@ 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
+ * 2 if domain should be renamed then restarted,
+ * 3 if domain performed kexec/kdump, or 0
  * Can update r_domid if domain is destroyed etc */
 static int handle_domain_death(uint32_t *r_domid,
                                libxl_event *event,
@@ -1801,6 +1802,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_KEXEC:
+        LOG("Domain kexec-ed.");
+        return 3;
     default:
         LOG("Unknown shutdown reason code %d. Destroying domain.",
             event->u.domain_shutdown.shutdown_reason);
@@ -2038,6 +2042,19 @@ static void evdisable_disk_ejects(libxl_evgen_disk_eject **diskws,
     }
 }
 
+static void domain_kexec_finish(libxl_ctx *ctx_cb, int rc, void *domid_arg)
+{
+    uint32_t domid = *(uint32_t *)domid_arg;
+    if (!rc) {
+        /* successful kexec, destroy the domain */
+        LOG("Domain kexec finished, destroying");
+        libxl_domain_destroy(ctx_cb, domid, 0);
+    }
+    else
+        LOG("Domain kexec failed, manual intervention required");
+    free(domid_arg);
+}
+
 static uint32_t create_domain(struct domain_create *dom_info)
 {
     uint32_t domid = INVALID_DOMID;
@@ -2066,6 +2083,9 @@ 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;
+    int pipefd[2];
+
+    pipefd[0] = pipefd[1] = -1;
 
     int restoring = (restore_file || (migrate_fd >= 0));
 
@@ -2250,6 +2270,12 @@ start:
          * restore/migrate-receive it again.
          */
         restoring = 0;
+        if (pipefd[0] != -1) {
+            /* we were doing kexec, close pipes */
+            close(pipefd[0]);
+            close(pipefd[1]);
+            pipefd[0] = pipefd[1] = -1;
+        }
     }else{
         ret = libxl_domain_create_new(ctx, &d_config, &domid,
                                       0, autoconnect_console_how);
@@ -2339,6 +2365,10 @@ start:
     }
     while (1) {
         libxl_event *event;
+        libxl_asyncop_how ao_how;
+        uint32_t domid_old;
+        void *domid_arg;
+
         ret = domain_wait_event(domid, &event);
         if (ret) goto out;
 
@@ -2351,6 +2381,33 @@ start:
             switch (handle_domain_death(&domid, event,
                                         (uint8_t **)&config_data, &config_len,
                                         &d_config)) {
+            case 3:
+                domid_old = domid;
+                if (!preserve_domain(&domid, event, &d_config)) {
+                    ret = -1;
+                    goto out;
+                }
+                if (libxl_pipe(ctx, pipefd)) {
+                    ret = -1;
+                    goto out;
+                }
+
+                domid_arg = (void *)malloc(sizeof(domid_old));
+                if (!domid_arg) {
+                    ret = -1;
+                    goto out;
+                }
+                *(uint32_t *)domid_arg = domid_old;
+
+                ao_how.callback = domain_kexec_finish;
+                ao_how.u.for_callback = domid_arg;
+
+                ret = libxl_domain_suspend(ctx, domid_old, pipefd[1], LIBXL_SUSPEND_KEXEC, &ao_how);
+
+                restoring = 1;
+                restore_fd = pipefd[0];
+
+                goto restart;
             case 2:
                 if (!preserve_domain(&domid, event, &d_config)) {
                     /* If we fail then exit leaving the old domain in place. */
@@ -2359,6 +2416,7 @@ start:
                 }
 
                 /* Otherwise fall through and restart. */
+restart:
             case 1:
                 libxl_event_free(ctx, event);
                 libxl_evdisable_domain_death(ctx, deathw);
@@ -3287,7 +3345,7 @@ static void list_domains(int verbose, int context, int claim, int numa,
                          const libxl_dominfo *info, int nb_domain)
 {
     int i;
-    static const char shutdown_reason_letters[]= "-rscw";
+    static const char shutdown_reason_letters[]= "-rscwk";
     libxl_bitmap nodemap;
     libxl_physinfo physinfo;
 
-- 
1.9.3

Re: [PATCH RFC/WIP 2/4] libxc: treat domain in SHUTDOWN_kexec state as being already suspended

[Andrew Cooper]

On 25/08/14 14:42, Vitaly Kuznetsov wrote:
> Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
> ---
>  tools/libxc/xc_domain_save.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/tools/libxc/xc_domain_save.c b/tools/libxc/xc_domain_save.c
> index 42c4752..5423bfa 100644
> --- a/tools/libxc/xc_domain_save.c
> +++ b/tools/libxc/xc_domain_save.c
> @@ -366,7 +366,8 @@ static int suspend_and_state(int (*suspend)(void*), void* data,
>      }
>  
>      if ( (xc_domain_getinfo(xch, dom, 1, info) != 1) ||
> -         !info->shutdown || (info->shutdown_reason != SHUTDOWN_suspend) )
> +         !info->shutdown || ((info->shutdown_reason != SHUTDOWN_suspend) &&
> +                             (info->shutdown_reason != SHUTDOWN_kexec)) )

This will break PV unless you mandate that SHUTDOWN_kexec follows the
same principle as SHUTDOWN_suspend, i.e. stashing the start info mfn in rdx.

Best alter the comment in public/sched.h when adding SHUTDOWN_kexec.

~Andrew

Re: [PATCH RFC/WIP 1/4] Introduce SHUTDOWN_kexec shutdown reason

[Jan Beulich]

>>> On 25.08.14 at 15:42, <vkuznets@redhat.com> wrote:
> --- a/xen/include/public/sched.h
> +++ b/xen/include/public/sched.h
> @@ -158,7 +158,8 @@ DEFINE_XEN_GUEST_HANDLE(sched_watchdog_t);
>  #define SHUTDOWN_suspend    2  /* Clean up, save suspend info, kill.         */
>  #define SHUTDOWN_crash      3  /* Tell controller we've crashed.             */
>  #define SHUTDOWN_watchdog   4  /* Restart because watchdog time expired.     */
> -#define SHUTDOWN_MAX        4  /* Maximum valid shutdown reason.             */
> +#define SHUTDOWN_kexec      5  /* Soft-reboot for kexec.                     */
> +#define SHUTDOWN_MAX        5  /* Maximum valid shutdown reason.             */

I have to admit that I dislike the term "kexec" in here as being Linux-
centric (I admit there being a kexec hypercall, but that's intentionally
Linux-centric I would say) - why not SHUTDOWN_soft_reset?

Jan

Re: [PATCH RFC/WIP 1/4] Introduce SHUTDOWN_kexec shutdown reason

[David Vrabel]

On 25/08/14 14:42, Vitaly Kuznetsov wrote:
> --- a/xen/include/public/sched.h
> +++ b/xen/include/public/sched.h
> @@ -158,7 +158,8 @@ DEFINE_XEN_GUEST_HANDLE(sched_watchdog_t);
>  #define SHUTDOWN_suspend    2  /* Clean up, save suspend info, kill.         */
>  #define SHUTDOWN_crash      3  /* Tell controller we've crashed.             */
>  #define SHUTDOWN_watchdog   4  /* Restart because watchdog time expired.     */
> -#define SHUTDOWN_MAX        4  /* Maximum valid shutdown reason.             */
> +#define SHUTDOWN_kexec      5  /* Soft-reboot for kexec.                     */

I agree with Jan on the name.

The documentation also needs to be expanded.  It should be clear that
this hypercall may return in a new domain.

David

Re: [PATCH RFC/WIP 1/4] Introduce SHUTDOWN_kexec shutdown reason

[Jan Beulich]

>>> On 26.08.14 at 12:16, <david.vrabel@citrix.com> wrote:
> On 25/08/14 14:42, Vitaly Kuznetsov wrote:
>> --- a/xen/include/public/sched.h
>> +++ b/xen/include/public/sched.h
>> @@ -158,7 +158,8 @@ DEFINE_XEN_GUEST_HANDLE(sched_watchdog_t);
>>  #define SHUTDOWN_suspend    2  /* Clean up, save suspend info, kill.        
>  */
>>  #define SHUTDOWN_crash      3  /* Tell controller we've crashed.            
>  */
>>  #define SHUTDOWN_watchdog   4  /* Restart because watchdog time expired.    
>  */
>> -#define SHUTDOWN_MAX        4  /* Maximum valid shutdown reason.             
> */
>> +#define SHUTDOWN_kexec      5  /* Soft-reboot for kexec.                     
> */
> 
> I agree with Jan on the name.
> 
> The documentation also needs to be expanded.  It should be clear that
> this hypercall may return in a new domain.

Perhaps s/may/will/ ?

Jan

Re: [PATCH RFC/WIP 1/4] Introduce SHUTDOWN_kexec shutdown reason

[David Vrabel]

On 26/08/14 13:40, Jan Beulich wrote:
>>>> On 26.08.14 at 12:16, <david.vrabel@citrix.com> wrote:
>> On 25/08/14 14:42, Vitaly Kuznetsov wrote:
>>> --- a/xen/include/public/sched.h
>>> +++ b/xen/include/public/sched.h
>>> @@ -158,7 +158,8 @@ DEFINE_XEN_GUEST_HANDLE(sched_watchdog_t);
>>>  #define SHUTDOWN_suspend    2  /* Clean up, save suspend info, kill.        
>>  */
>>>  #define SHUTDOWN_crash      3  /* Tell controller we've crashed.            
>>  */
>>>  #define SHUTDOWN_watchdog   4  /* Restart because watchdog time expired.    
>>  */
>>> -#define SHUTDOWN_MAX        4  /* Maximum valid shutdown reason.             
>> */
>>> +#define SHUTDOWN_kexec      5  /* Soft-reboot for kexec.                     
>> */
>>
>> I agree with Jan on the name.
>>
>> The documentation also needs to be expanded.  It should be clear that
>> this hypercall may return in a new domain.
> 
> Perhaps s/may/will/ ?

Yes, but only on success.

David