| Message ID | DE8DF0795D48FD4CA783C40EC82923350136FF7D@SHSMSX101.ccr.corp.intel.com |
|---|---|
| State | New |
| Headers | show |
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;
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;
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.
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;