From a67971cd45ec201404f912b78a08e0325eb5b33f 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] xl: HVM domain S3 bugfix Currently 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 2 patches to fix the HVM S3 bug: qemu-xen patch and xl patch. This patch is the xl patch. It invokes QMP system_wakeup so that qemu-xen logic for hvm s3 could be triggerred. Signed-off-by: Liu Jinsong <jinsong.liu@intel.com> --- tools/libxl/libxl.c | 30 ++++++++++++++++++++++++++++-- tools/libxl/libxl_internal.h | 2 ++ tools/libxl/libxl_qmp.c | 5 +++++ 3 files changed, 35 insertions(+), 2 deletions(-) diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c index 81785df..6345335 100644 --- a/tools/libxl/libxl.c +++ b/tools/libxl/libxl.c @@ -4641,6 +4641,33 @@ int libxl_domain_sched_params_get(libxl_ctx *ctx, uint32_t domid, return ret; } +static int xc_domain_s3_resume(libxl_ctx *ctx, int domid) +{ + int rc = 0; + GC_INIT(ctx); + + 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; + } + default: + rc = ERROR_INVAL; + break; + } + + GC_FREE; + return rc; +} + int libxl_send_trigger(libxl_ctx *ctx, uint32_t domid, libxl_trigger trigger, uint32_t vcpuid) { @@ -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 = xc_domain_s3_resume(ctx, domid); break; default: rc = -1; 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; -- 1.7.1 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
On Fri, 2013-08-23 at 08:41 +0000, Liu, Jinsong wrote:> From a67971cd45ec201404f912b78a08e0325eb5b33f 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] xl: HVM domain S3 bugfix > > Currently 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 2 patches to fix the HVM S3 bug: qemu-xen patch and xl patch. > This patch is the xl patch. It invokes QMP system_wakeup so that > qemu-xen logic for hvm s3 could be triggerred. > > Signed-off-by: Liu Jinsong <jinsong.liu@intel.com> > --- > tools/libxl/libxl.c | 30 ++++++++++++++++++++++++++++-- > tools/libxl/libxl_internal.h | 2 ++ > tools/libxl/libxl_qmp.c | 5 +++++ > 3 files changed, 35 insertions(+), 2 deletions(-) > > diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c > index 81785df..6345335 100644 > --- a/tools/libxl/libxl.c > +++ b/tools/libxl/libxl.c > @@ -4641,6 +4641,33 @@ int libxl_domain_sched_params_get(libxl_ctx *ctx, uint32_t domid, > return ret; > } > > +static int xc_domain_s3_resume(libxl_ctx *ctx, int domid)Since this is an internal function it should be static int libxl__domain_s3_resume(libxl__gc *gc, int domid)> +{ > + int rc = 0; > + GC_INIT(ctx);This (and the corresponding GC_FREE) should be added to libxl_send_trigger, so that you have a gc to pass to this function.> + > + 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);With the above changes will become CTX->xch.> + break; > + case LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN: > + rc = libxl__qmp_system_wakeup(gc, domid); > + break; > + default: > + rc = ERROR_INVAL; > + break; > + } > + default: > + rc = ERROR_INVAL; > + break; > + } > + > + GC_FREE; > + return rc; > +} > + > int libxl_send_trigger(libxl_ctx *ctx, uint32_t domid, > libxl_trigger trigger, uint32_t vcpuid) > { > @@ -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 = xc_domain_s3_resume(ctx, domid); > break; > default: > rc = -1; > 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; > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xen.org > http://lists.xen.org/xen-devel
Ian Campbell wrote:>> --- a/tools/libxl/libxl.c >> +++ b/tools/libxl/libxl.c >> @@ -4641,6 +4641,33 @@ int libxl_domain_sched_params_get(libxl_ctx >> *ctx, uint32_t domid, return ret; } >> >> +static int xc_domain_s3_resume(libxl_ctx *ctx, int domid) > > Since this is an internal function it should be > static int libxl__domain_s3_resume(libxl__gc *gc, int domid) > >> +{ >> + int rc = 0; >> + GC_INIT(ctx); > > This (and the corresponding GC_FREE) should be added to > libxl_send_trigger, so that you have a gc to pass to this function. > >> + >> + 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); > > With the above changes will become CTX->xch. >Thanks! sorry for late reply (just return from a short vacation). Patch updated and already sent out. Regards, Jinsong
On Wed, 2013-08-28 at 16:35 +0000, Liu, Jinsong wrote:> Thanks! sorry for late reply (just return from a short vacation).No problem.> Patch updated and already sent out.I don''t seem to have it yet, lost in the tubes perhaps? Do you have a message-id handy (or perhaps a link to the archives) Ian.
Ian Campbell wrote:> On Wed, 2013-08-28 at 16:35 +0000, Liu, Jinsong wrote: >> Thanks! sorry for late reply (just return from a short vacation). > > No problem. > >> Patch updated and already sent out. > > I don''t seem to have it yet, lost in the tubes perhaps? Do you have a > message-id handy (or perhaps a link to the archives) > > Ian.Strange. I sent them by ''git send-email -1 --to xxx --cc yyy --annotate''. I just re-send patches via Outlook. Thanks, Jinsong
On Thu, Aug 29, 2013 at 9:55 AM, Liu, Jinsong <jinsong.liu@intel.com> wrote:> Ian Campbell wrote: >> On Wed, 2013-08-28 at 16:35 +0000, Liu, Jinsong wrote: >>> Thanks! sorry for late reply (just return from a short vacation). >> >> No problem. >> >>> Patch updated and already sent out. >> >> I don''t seem to have it yet, lost in the tubes perhaps? Do you have a >> message-id handy (or perhaps a link to the archives) >> >> Ian. > > Strange. I sent them by ''git send-email -1 --to xxx --cc yyy --annotate''. > I just re-send patches via Outlook.For some reason I have them in gmail (which is where I subscribe to xen-devel, but I didn''t receive them in my corporate mailbox, in spite of being cc''d. -George