Patchwork [V3] xl: HVM domain S3 bugfix

login
register
mail settings
Submitter Liu, Jinsong
Date Sept. 9, 2013, 3:29 a.m.
Message ID <DE8DF0795D48FD4CA783C40EC82923350136FF7D@SHSMSX101.ccr.corp.intel.com>
Download mbox | patch
Permalink /patch/273482/
State New
Headers show

Comments

Liu, Jinsong - Sept. 9, 2013, 3:29 a.m.
From 18344216b432648605726b137b348f28ef64a4ef Mon Sep 17 00:00:00 2001
From: Liu Jinsong <jinsong.liu@intel.com>
Date: Fri, 23 Aug 2013 23:30:23 +0800
Subject: [PATCH V3] xl: HVM domain S3 bugfix

Currently Xen hvm s3 has a bug coming from the difference between
qemu-traditioanl and qemu-xen. For qemu-traditional, the way to
resume from hvm s3 is via 'xl trigger' command. However, for
qemu-xen, the way to resume from hvm s3 inherited from standard
qemu, i.e. via QMP, and it doesn't work under Xen.

The root cause is, for qemu-xen, 'xl trigger' command didn't reset
devices, while QMP didn't unpause hvm domain though they did qemu
system reset.

We have two qemu patches one xl patch to fix the HVM S3 bug:
This patch is the xl patch. It invokes QMP system_wakeup so that
qemu logic for hvm s3 could be triggerred.

Signed-off-by: Liu Jinsong <jinsong.liu@intel.com>
---
 tools/libxl/libxl.c          |   31 +++++++++++++++++++++++++++++--
 tools/libxl/libxl_internal.h |    2 ++
 tools/libxl/libxl_qmp.c      |    5 +++++
 3 files changed, 36 insertions(+), 2 deletions(-)
Ian Campbell - Sept. 9, 2013, 1:54 p.m.
On Mon, 2013-09-09 at 03:29 +0000, Liu, Jinsong wrote:
> From 18344216b432648605726b137b348f28ef64a4ef Mon Sep 17 00:00:00 2001
> From: Liu Jinsong <jinsong.liu@intel.com>
> Date: Fri, 23 Aug 2013 23:30:23 +0800
> Subject: [PATCH V3] xl: HVM domain S3 bugfix
> 
> Currently Xen hvm s3 has a bug coming from the difference between
> qemu-traditioanl and qemu-xen. For qemu-traditional, the way to

"traditional"

> resume from hvm s3 is via 'xl trigger' command. However, for
> qemu-xen, the way to resume from hvm s3 inherited from standard
> qemu, i.e. via QMP, and it doesn't work under Xen.
> 
> The root cause is, for qemu-xen, 'xl trigger' command didn't reset
> devices, while QMP didn't unpause hvm domain though they did qemu
> system reset.
> 
> We have two qemu patches one xl patch to fix the HVM S3 bug:
> This patch is the xl patch. It invokes QMP system_wakeup so that
> qemu logic for hvm s3 could be triggerred.

"triggered"

> 
> Signed-off-by: Liu Jinsong <jinsong.liu@intel.com>

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

I'll hold off on applying until someone givers me a heads up that the
qemu side is in place. I'll try and remember to fix the commit message
typos as I commit, so no need to resend for those.

> ---
>  tools/libxl/libxl.c          |   31 +++++++++++++++++++++++++++++--
>  tools/libxl/libxl_internal.h |    2 ++
>  tools/libxl/libxl_qmp.c      |    5 +++++
>  3 files changed, 36 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
> index 81785df..267bc25 100644
> --- a/tools/libxl/libxl.c
> +++ b/tools/libxl/libxl.c
> @@ -4641,10 +4641,37 @@ int libxl_domain_sched_params_get(libxl_ctx *ctx, uint32_t domid,
>      return ret;
>  }
>  
> +static int libxl__domain_s3_resume(libxl__gc *gc, int domid)
> +{
> +    int rc = 0;
> +
> +    switch (libxl__domain_type(gc, domid)) {
> +    case LIBXL_DOMAIN_TYPE_HVM:
> +        switch (libxl__device_model_version_running(gc, domid)) {
> +        case LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN_TRADITIONAL:
> +            rc = xc_set_hvm_param(CTX->xch, domid, HVM_PARAM_ACPI_S_STATE, 0);
> +            break;
> +        case LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN:
> +            rc = libxl__qmp_system_wakeup(gc, domid);
> +            break;
> +        default:
> +            rc = ERROR_INVAL;
> +            break;
> +        }
> +        break;
> +    default:
> +        rc = ERROR_INVAL;
> +        break;
> +    }
> +
> +    return rc;
> +}
> +
>  int libxl_send_trigger(libxl_ctx *ctx, uint32_t domid,
>                         libxl_trigger trigger, uint32_t vcpuid)
>  {
>      int rc;
> +    GC_INIT(ctx);
>  
>      switch (trigger) {
>      case LIBXL_TRIGGER_POWER:
> @@ -4668,8 +4695,7 @@ int libxl_send_trigger(libxl_ctx *ctx, uint32_t domid,
>                                      XEN_DOMCTL_SENDTRIGGER_RESET, vcpuid);
>          break;
>      case LIBXL_TRIGGER_S3RESUME:
> -        xc_set_hvm_param(ctx->xch, domid, HVM_PARAM_ACPI_S_STATE, 0);
> -        rc = 0;
> +        rc = libxl__domain_s3_resume(gc, domid);
>          break;
>      default:
>          rc = -1;
> @@ -4684,6 +4710,7 @@ int libxl_send_trigger(libxl_ctx *ctx, uint32_t domid,
>          rc = ERROR_FAIL;
>      }
>  
> +    GC_FREE;
>      return rc;
>  }
>  
> diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
> index f051d91..0ef0a3a 100644
> --- a/tools/libxl/libxl_internal.h
> +++ b/tools/libxl/libxl_internal.h
> @@ -1408,6 +1408,8 @@ _hidden int libxl__qmp_query_serial(libxl__qmp_handler *qmp);
>  _hidden int libxl__qmp_pci_add(libxl__gc *gc, int d, libxl_device_pci *pcidev);
>  _hidden int libxl__qmp_pci_del(libxl__gc *gc, int domid,
>                                 libxl_device_pci *pcidev);
> +/* Resume hvm domain */
> +_hidden int libxl__qmp_system_wakeup(libxl__gc *gc, int domid);
>  /* Suspend QEMU. */
>  _hidden int libxl__qmp_stop(libxl__gc *gc, int domid);
>  /* Resume QEMU. */
> diff --git a/tools/libxl/libxl_qmp.c b/tools/libxl/libxl_qmp.c
> index f681f3a..3c3b301 100644
> --- a/tools/libxl/libxl_qmp.c
> +++ b/tools/libxl/libxl_qmp.c
> @@ -878,6 +878,11 @@ int libxl__qmp_pci_del(libxl__gc *gc, int domid, libxl_device_pci *pcidev)
>      return qmp_device_del(gc, domid, id);
>  }
>  
> +int libxl__qmp_system_wakeup(libxl__gc *gc, int domid)
> +{
> +    return qmp_run_command(gc, domid, "system_wakeup", NULL, NULL, NULL);
> +}
> +
>  int libxl__qmp_save(libxl__gc *gc, int domid, const char *filename)
>  {
>      libxl__json_object *args = NULL;
Liu, Jinsong - Oct. 22, 2013, 6:03 a.m.
Ian Campbell wrote:
> On Mon, 2013-09-09 at 03:29 +0000, Liu, Jinsong wrote:
>> From 18344216b432648605726b137b348f28ef64a4ef Mon Sep 17 00:00:00
>> 2001 
>> From: Liu Jinsong <jinsong.liu@intel.com>
>> Date: Fri, 23 Aug 2013 23:30:23 +0800
>> Subject: [PATCH V3] xl: HVM domain S3 bugfix
>> 
>> Currently Xen hvm s3 has a bug coming from the difference between
>> qemu-traditioanl and qemu-xen. For qemu-traditional, the way to
> 
> "traditional"
> 
>> resume from hvm s3 is via 'xl trigger' command. However, for
>> qemu-xen, the way to resume from hvm s3 inherited from standard
>> qemu, i.e. via QMP, and it doesn't work under Xen.
>> 
>> The root cause is, for qemu-xen, 'xl trigger' command didn't reset
>> devices, while QMP didn't unpause hvm domain though they did qemu
>> system reset.
>> 
>> We have two qemu patches one xl patch to fix the HVM S3 bug:
>> This patch is the xl patch. It invokes QMP system_wakeup so that
>> qemu logic for hvm s3 could be triggerred.
> 
> "triggered"
> 
>> 
>> Signed-off-by: Liu Jinsong <jinsong.liu@intel.com>
> 
> Acked-by: Ian Campbell <ian.campbell@citrix.com>
> 
> I'll hold off on applying until someone givers me a heads up that the
> qemu side is in place. I'll try and remember to fix the commit message
> typos as I commit, so no need to resend for those.
> 

Ian,

The 2 qemu patches have been checked in qemu tree w/ commit number
4bc78a877252d772b983810a7d2c0be00e9be70e
11addd0ab9371af2b6ec028c7fe4e4c4992252fc

So would you please check this xl patch in Xen?

Thanks,
Jinsong

>> ---
>>  tools/libxl/libxl.c          |   31 +++++++++++++++++++++++++++++--
>>  tools/libxl/libxl_internal.h |    2 ++
>>  tools/libxl/libxl_qmp.c      |    5 +++++
>>  3 files changed, 36 insertions(+), 2 deletions(-)
>> 
>> diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
>> index 81785df..267bc25 100644
>> --- a/tools/libxl/libxl.c
>> +++ b/tools/libxl/libxl.c
>> @@ -4641,10 +4641,37 @@ int libxl_domain_sched_params_get(libxl_ctx
>>  *ctx, uint32_t domid,      return ret; }
>> 
>> +static int libxl__domain_s3_resume(libxl__gc *gc, int domid) +{
>> +    int rc = 0;
>> +
>> +    switch (libxl__domain_type(gc, domid)) {
>> +    case LIBXL_DOMAIN_TYPE_HVM:
>> +        switch (libxl__device_model_version_running(gc, domid)) {
>> +        case LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN_TRADITIONAL:
>> +            rc = xc_set_hvm_param(CTX->xch, domid,
>> HVM_PARAM_ACPI_S_STATE, 0); +            break; +        case
>> LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN: +            rc =
>> libxl__qmp_system_wakeup(gc, domid); +            break; +       
>> default: +            rc = ERROR_INVAL;
>> +            break;
>> +        }
>> +        break;
>> +    default:
>> +        rc = ERROR_INVAL;
>> +        break;
>> +    }
>> +
>> +    return rc;
>> +}
>> +
>>  int libxl_send_trigger(libxl_ctx *ctx, uint32_t domid,
>>                         libxl_trigger trigger, uint32_t vcpuid)  {
>>      int rc;
>> +    GC_INIT(ctx);
>> 
>>      switch (trigger) {
>>      case LIBXL_TRIGGER_POWER:
>> @@ -4668,8 +4695,7 @@ int libxl_send_trigger(libxl_ctx *ctx,
>>                                      uint32_t domid,
>>      XEN_DOMCTL_SENDTRIGGER_RESET, vcpuid);          break; case
>> LIBXL_TRIGGER_S3RESUME: 
>> -        xc_set_hvm_param(ctx->xch, domid, HVM_PARAM_ACPI_S_STATE,
>> 0); 
>> -        rc = 0;
>> +        rc = libxl__domain_s3_resume(gc, domid);
>>          break;
>>      default:
>>          rc = -1;
>> @@ -4684,6 +4710,7 @@ int libxl_send_trigger(libxl_ctx *ctx,
>>      uint32_t domid,          rc = ERROR_FAIL; }
>> 
>> +    GC_FREE;
>>      return rc;
>>  }
>> 
>> diff --git a/tools/libxl/libxl_internal.h
>> b/tools/libxl/libxl_internal.h 
>> index f051d91..0ef0a3a 100644
>> --- a/tools/libxl/libxl_internal.h
>> +++ b/tools/libxl/libxl_internal.h
>> @@ -1408,6 +1408,8 @@ _hidden int
>>  libxl__qmp_query_serial(libxl__qmp_handler *qmp); _hidden int
>>  libxl__qmp_pci_add(libxl__gc *gc, int d, libxl_device_pci *pcidev);
>>                                 _hidden int
>> libxl__qmp_pci_del(libxl__gc *gc, int domid, libxl_device_pci
>>  *pcidev); +/* Resume hvm domain */ +_hidden int
>> libxl__qmp_system_wakeup(libxl__gc *gc, int domid);  /* Suspend
>> QEMU. */ _hidden int libxl__qmp_stop(libxl__gc *gc, int domid);  /*
>> Resume QEMU. */   
>> diff --git a/tools/libxl/libxl_qmp.c b/tools/libxl/libxl_qmp.c
>> index f681f3a..3c3b301 100644
>> --- a/tools/libxl/libxl_qmp.c
>> +++ b/tools/libxl/libxl_qmp.c
>> @@ -878,6 +878,11 @@ int libxl__qmp_pci_del(libxl__gc *gc, int
>>      domid, libxl_device_pci *pcidev) return qmp_device_del(gc,
>>  domid, id); }
>> 
>> +int libxl__qmp_system_wakeup(libxl__gc *gc, int domid) +{
>> +    return qmp_run_command(gc, domid, "system_wakeup", NULL, NULL,
>> NULL); +} +
>>  int libxl__qmp_save(libxl__gc *gc, int domid, const char *filename)
>>      { libxl__json_object *args = NULL;
Ian Campbell - Oct. 31, 2013, 10:04 p.m.
On Tue, 2013-10-22 at 06:03 +0000, Liu, Jinsong wrote:
> Ian Campbell wrote:
> > On Mon, 2013-09-09 at 03:29 +0000, Liu, Jinsong wrote:
> >> From 18344216b432648605726b137b348f28ef64a4ef Mon Sep 17 00:00:00
> >> 2001 
> >> From: Liu Jinsong <jinsong.liu@intel.com>
> >> Date: Fri, 23 Aug 2013 23:30:23 +0800
> >> Subject: [PATCH V3] xl: HVM domain S3 bugfix
> >> 
> >> Currently Xen hvm s3 has a bug coming from the difference between
> >> qemu-traditioanl and qemu-xen. For qemu-traditional, the way to
> > 
> > "traditional"
> > 
> >> resume from hvm s3 is via 'xl trigger' command. However, for
> >> qemu-xen, the way to resume from hvm s3 inherited from standard
> >> qemu, i.e. via QMP, and it doesn't work under Xen.
> >> 
> >> The root cause is, for qemu-xen, 'xl trigger' command didn't reset
> >> devices, while QMP didn't unpause hvm domain though they did qemu
> >> system reset.
> >> 
> >> We have two qemu patches one xl patch to fix the HVM S3 bug:
> >> This patch is the xl patch. It invokes QMP system_wakeup so that
> >> qemu logic for hvm s3 could be triggerred.
> > 
> > "triggered"
> > 
> >> 
> >> Signed-off-by: Liu Jinsong <jinsong.liu@intel.com>
> > 
> > Acked-by: Ian Campbell <ian.campbell@citrix.com>
> > 
> > I'll hold off on applying until someone givers me a heads up that the
> > qemu side is in place. I'll try and remember to fix the commit message
> > typos as I commit, so no need to resend for those.
> > 
> 
> Ian,
> 
> The 2 qemu patches have been checked in qemu tree w/ commit number
> 4bc78a877252d772b983810a7d2c0be00e9be70e
> 11addd0ab9371af2b6ec028c7fe4e4c4992252fc
> 
> So would you please check this xl patch in Xen?

Done. I suppose these changes need to make their way into
QEMU_UPSTREAM_REVISION in xen.git/Config.mk, but I assume that'll happen
in due course.

Patch

diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index 81785df..267bc25 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -4641,10 +4641,37 @@  int libxl_domain_sched_params_get(libxl_ctx *ctx, uint32_t domid,
     return ret;
 }
 
+static int libxl__domain_s3_resume(libxl__gc *gc, int domid)
+{
+    int rc = 0;
+
+    switch (libxl__domain_type(gc, domid)) {
+    case LIBXL_DOMAIN_TYPE_HVM:
+        switch (libxl__device_model_version_running(gc, domid)) {
+        case LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN_TRADITIONAL:
+            rc = xc_set_hvm_param(CTX->xch, domid, HVM_PARAM_ACPI_S_STATE, 0);
+            break;
+        case LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN:
+            rc = libxl__qmp_system_wakeup(gc, domid);
+            break;
+        default:
+            rc = ERROR_INVAL;
+            break;
+        }
+        break;
+    default:
+        rc = ERROR_INVAL;
+        break;
+    }
+
+    return rc;
+}
+
 int libxl_send_trigger(libxl_ctx *ctx, uint32_t domid,
                        libxl_trigger trigger, uint32_t vcpuid)
 {
     int rc;
+    GC_INIT(ctx);
 
     switch (trigger) {
     case LIBXL_TRIGGER_POWER:
@@ -4668,8 +4695,7 @@  int libxl_send_trigger(libxl_ctx *ctx, uint32_t domid,
                                     XEN_DOMCTL_SENDTRIGGER_RESET, vcpuid);
         break;
     case LIBXL_TRIGGER_S3RESUME:
-        xc_set_hvm_param(ctx->xch, domid, HVM_PARAM_ACPI_S_STATE, 0);
-        rc = 0;
+        rc = libxl__domain_s3_resume(gc, domid);
         break;
     default:
         rc = -1;
@@ -4684,6 +4710,7 @@  int libxl_send_trigger(libxl_ctx *ctx, uint32_t domid,
         rc = ERROR_FAIL;
     }
 
+    GC_FREE;
     return rc;
 }
 
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index f051d91..0ef0a3a 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -1408,6 +1408,8 @@  _hidden int libxl__qmp_query_serial(libxl__qmp_handler *qmp);
 _hidden int libxl__qmp_pci_add(libxl__gc *gc, int d, libxl_device_pci *pcidev);
 _hidden int libxl__qmp_pci_del(libxl__gc *gc, int domid,
                                libxl_device_pci *pcidev);
+/* Resume hvm domain */
+_hidden int libxl__qmp_system_wakeup(libxl__gc *gc, int domid);
 /* Suspend QEMU. */
 _hidden int libxl__qmp_stop(libxl__gc *gc, int domid);
 /* Resume QEMU. */
diff --git a/tools/libxl/libxl_qmp.c b/tools/libxl/libxl_qmp.c
index f681f3a..3c3b301 100644
--- a/tools/libxl/libxl_qmp.c
+++ b/tools/libxl/libxl_qmp.c
@@ -878,6 +878,11 @@  int libxl__qmp_pci_del(libxl__gc *gc, int domid, libxl_device_pci *pcidev)
     return qmp_device_del(gc, domid, id);
 }
 
+int libxl__qmp_system_wakeup(libxl__gc *gc, int domid)
+{
+    return qmp_run_command(gc, domid, "system_wakeup", NULL, NULL, NULL);
+}
+
 int libxl__qmp_save(libxl__gc *gc, int domid, const char *filename)
 {
     libxl__json_object *args = NULL;