[PATCH] Qemu-xen: HVM S3 bugfix

[PATCH] Qemu-xen: HVM S3 bugfix

[Liu, Jinsong]

[-- Attachment #1: Type: text/plain, Size: 2387 bytes --]

>From fc928436df17673bfea1d902859c682c99c1489d Mon Sep 17 00:00:00 2001
From: Liu Jinsong <jinsong.liu@intel.com>
Date: Wed, 14 Aug 2013 05:26:56 +0800
Subject: [PATCH] Qemu-xen: HVM S3 bugfix

This patch is used to fix HVM S3 bug caused from the difference
between qemu-traditioanl and standard qemu (which qemu-xen stems from).
For qemu-traditional, hvm s3 resumes via 'xl trigger' command interface.
For standard qemu, hvm s3 resumes via keyboard triggering.

So currently for qemu-xen, both 'xl' interface and keyboard triggering
hvm s3 cannot work. 'xl' interface didn't reset ioemu devices, while
keyboard triggering didn't unpause hvm domain.

This patch fixes the bug. It keeps compatible w/ current xen 'xl' interface,
by moving qemu system reset logic forward to qemu suspend.

Signed-off-by: Liu Jinsong <jinsong.liu@intel.com>
---
 vl.c |   25 ++++++++++++++++---------
 1 files changed, 16 insertions(+), 9 deletions(-)

diff --git a/vl.c b/vl.c
index 5314f55..deea92c 100644
--- a/vl.c
+++ b/vl.c
@@ -1619,10 +1619,15 @@ void qemu_system_reset_request(void)
 
 static void qemu_system_suspend(void)
 {
-    pause_all_vcpus();
-    notifier_list_notify(&suspend_notifiers, NULL);
-    runstate_set(RUN_STATE_SUSPENDED);
-    monitor_protocol_event(QEVENT_SUSPEND, NULL);
+    if (!xen_enabled()) {
+        pause_all_vcpus();
+        notifier_list_notify(&suspend_notifiers, NULL);
+        runstate_set(RUN_STATE_SUSPENDED);
+        monitor_protocol_event(QEVENT_SUSPEND, NULL);
+    } else {
+        notifier_list_notify(&suspend_notifiers, NULL);
+        qemu_system_reset(VMRESET_SILENT);
+    }
 }
 
 void qemu_system_suspend_request(void)
@@ -1740,11 +1745,13 @@ static bool main_loop_should_exit(void)
         }
     }
     if (qemu_wakeup_requested()) {
-        pause_all_vcpus();
-        cpu_synchronize_all_states();
-        qemu_system_reset(VMRESET_SILENT);
-        resume_all_vcpus();
-        monitor_protocol_event(QEVENT_WAKEUP, NULL);
+        if (!xen_enabled()) {
+            pause_all_vcpus();
+            cpu_synchronize_all_states();
+            qemu_system_reset(VMRESET_SILENT);
+            resume_all_vcpus();
+            monitor_protocol_event(QEVENT_WAKEUP, NULL);
+        }
     }
     if (qemu_powerdown_requested()) {
         qemu_system_powerdown();
-- 
1.7.1

[-- Attachment #2: 0001-Qemu-xen-HVM-S3-bugfix.patch --]
[-- Type: application/octet-stream, Size: 2388 bytes --]

From fc928436df17673bfea1d902859c682c99c1489d Mon Sep 17 00:00:00 2001
From: Liu Jinsong <jinsong.liu@intel.com>
Date: Wed, 14 Aug 2013 05:26:56 +0800
Subject: [PATCH] Qemu-xen: HVM S3 bugfix

This patch is used to fix HVM S3 bug caused from the difference
between qemu-traditioanl and standard qemu (which qemu-xen stems from).
For qemu-traditional, hvm s3 resumes via 'xl trigger' command interface.
For standard qemu, hvm s3 resumes via keyboard triggering.

So currently for qemu-xen, both 'xl' interface and keyboard triggering
hvm s3 cannot work. 'xl' interface didn't reset ioemu devices, while
keyboard triggering didn't unpause hvm domain.

This patch fixes the bug. It keeps compatible w/ current xen 'xl' interface,
by moving qemu system reset logic forward to qemu suspend.

Signed-off-by: Liu Jinsong <jinsong.liu@intel.com>
---
 vl.c |   25 ++++++++++++++++---------
 1 files changed, 16 insertions(+), 9 deletions(-)

diff --git a/vl.c b/vl.c
index 5314f55..deea92c 100644
--- a/vl.c
+++ b/vl.c
@@ -1619,10 +1619,15 @@ void qemu_system_reset_request(void)
 
 static void qemu_system_suspend(void)
 {
-    pause_all_vcpus();
-    notifier_list_notify(&suspend_notifiers, NULL);
-    runstate_set(RUN_STATE_SUSPENDED);
-    monitor_protocol_event(QEVENT_SUSPEND, NULL);
+    if (!xen_enabled()) {
+        pause_all_vcpus();
+        notifier_list_notify(&suspend_notifiers, NULL);
+        runstate_set(RUN_STATE_SUSPENDED);
+        monitor_protocol_event(QEVENT_SUSPEND, NULL);
+    } else {
+        notifier_list_notify(&suspend_notifiers, NULL);
+        qemu_system_reset(VMRESET_SILENT);
+    }
 }
 
 void qemu_system_suspend_request(void)
@@ -1740,11 +1745,13 @@ static bool main_loop_should_exit(void)
         }
     }
     if (qemu_wakeup_requested()) {
-        pause_all_vcpus();
-        cpu_synchronize_all_states();
-        qemu_system_reset(VMRESET_SILENT);
-        resume_all_vcpus();
-        monitor_protocol_event(QEVENT_WAKEUP, NULL);
+        if (!xen_enabled()) {
+            pause_all_vcpus();
+            cpu_synchronize_all_states();
+            qemu_system_reset(VMRESET_SILENT);
+            resume_all_vcpus();
+            monitor_protocol_event(QEVENT_WAKEUP, NULL);
+        }
     }
     if (qemu_powerdown_requested()) {
         qemu_system_powerdown();
-- 
1.7.1


[-- Attachment #3: Type: text/plain, Size: 126 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [PATCH] Qemu-xen: HVM S3 bugfix

[Liu, Jinsong]

Ping?

Thanks,
Jinsong

Liu, Jinsong wrote:
> From fc928436df17673bfea1d902859c682c99c1489d Mon Sep 17 00:00:00 2001
> From: Liu Jinsong <jinsong.liu@intel.com>
> Date: Wed, 14 Aug 2013 05:26:56 +0800
> Subject: [PATCH] Qemu-xen: HVM S3 bugfix
> 
> This patch is used to fix HVM S3 bug caused from the difference
> between qemu-traditioanl and standard qemu (which qemu-xen stems
> from). 
> For qemu-traditional, hvm s3 resumes via 'xl trigger' command
> interface. For standard qemu, hvm s3 resumes via keyboard triggering.
> 
> So currently for qemu-xen, both 'xl' interface and keyboard triggering
> hvm s3 cannot work. 'xl' interface didn't reset ioemu devices, while
> keyboard triggering didn't unpause hvm domain.
> 
> This patch fixes the bug. It keeps compatible w/ current xen 'xl'
> interface, by moving qemu system reset logic forward to qemu suspend.
> 
> Signed-off-by: Liu Jinsong <jinsong.liu@intel.com>
> ---
>  vl.c |   25 ++++++++++++++++---------
>  1 files changed, 16 insertions(+), 9 deletions(-)
> 
> diff --git a/vl.c b/vl.c
> index 5314f55..deea92c 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -1619,10 +1619,15 @@ void qemu_system_reset_request(void)
> 
>  static void qemu_system_suspend(void)
>  {
> -    pause_all_vcpus();
> -    notifier_list_notify(&suspend_notifiers, NULL);
> -    runstate_set(RUN_STATE_SUSPENDED);
> -    monitor_protocol_event(QEVENT_SUSPEND, NULL);
> +    if (!xen_enabled()) {
> +        pause_all_vcpus();
> +        notifier_list_notify(&suspend_notifiers, NULL);
> +        runstate_set(RUN_STATE_SUSPENDED);
> +        monitor_protocol_event(QEVENT_SUSPEND, NULL);
> +    } else {
> +        notifier_list_notify(&suspend_notifiers, NULL);
> +        qemu_system_reset(VMRESET_SILENT);
> +    }
>  }
> 
>  void qemu_system_suspend_request(void)
> @@ -1740,11 +1745,13 @@ static bool main_loop_should_exit(void)
>          }
>      }
>      if (qemu_wakeup_requested()) {
> -        pause_all_vcpus();
> -        cpu_synchronize_all_states();
> -        qemu_system_reset(VMRESET_SILENT);
> -        resume_all_vcpus();
> -        monitor_protocol_event(QEVENT_WAKEUP, NULL);
> +        if (!xen_enabled()) {
> +            pause_all_vcpus();
> +            cpu_synchronize_all_states();
> +            qemu_system_reset(VMRESET_SILENT);
> +            resume_all_vcpus();
> +            monitor_protocol_event(QEVENT_WAKEUP, NULL);
> +        }
>      }
>      if (qemu_powerdown_requested()) {
>          qemu_system_powerdown();

Re: [PATCH] Qemu-xen: HVM S3 bugfix

[George Dunlap]

On Mon, Aug 19, 2013 at 3:47 PM, Liu, Jinsong <jinsong.liu@intel.com> wrote:
> Ping?

I know Ian and Stefano are doing something at ARM this week -- Anthony
might be able to give a review, at least...
 -George

Re: [PATCH] Qemu-xen: HVM S3 bugfix

[Anthony PERARD]

On 13/08/13 15:15, Liu, Jinsong wrote:
> From fc928436df17673bfea1d902859c682c99c1489d Mon Sep 17 00:00:00 2001
> From: Liu Jinsong <jinsong.liu@intel.com>
> Date: Wed, 14 Aug 2013 05:26:56 +0800
> Subject: [PATCH] Qemu-xen: HVM S3 bugfix
> 
> This patch is used to fix HVM S3 bug caused from the difference
> between qemu-traditioanl and standard qemu (which qemu-xen stems from).
> For qemu-traditional, hvm s3 resumes via 'xl trigger' command interface.
> For standard qemu, hvm s3 resumes via keyboard triggering.
> 
> So currently for qemu-xen, both 'xl' interface and keyboard triggering
> hvm s3 cannot work. 'xl' interface didn't reset ioemu devices, while
> keyboard triggering didn't unpause hvm domain.
> 
> This patch fixes the bug. It keeps compatible w/ current xen 'xl' interface,
> by moving qemu system reset logic forward to qemu suspend.
> 
> Signed-off-by: Liu Jinsong <jinsong.liu@intel.com>
> ---
>  vl.c |   25 ++++++++++++++++---------
>  1 files changed, 16 insertions(+), 9 deletions(-)
> 
> diff --git a/vl.c b/vl.c
> index 5314f55..deea92c 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -1619,10 +1619,15 @@ void qemu_system_reset_request(void)
>  
>  static void qemu_system_suspend(void)
>  {
> -    pause_all_vcpus();
> -    notifier_list_notify(&suspend_notifiers, NULL);
> -    runstate_set(RUN_STATE_SUSPENDED);
> -    monitor_protocol_event(QEVENT_SUSPEND, NULL);
> +    if (!xen_enabled()) {
> +        pause_all_vcpus();
> +        notifier_list_notify(&suspend_notifiers, NULL);
> +        runstate_set(RUN_STATE_SUSPENDED);
> +        monitor_protocol_event(QEVENT_SUSPEND, NULL);
> +    } else {
> +        notifier_list_notify(&suspend_notifiers, NULL);
> +        qemu_system_reset(VMRESET_SILENT);
> +    }
>  }
>  
>  void qemu_system_suspend_request(void)
> @@ -1740,11 +1745,13 @@ static bool main_loop_should_exit(void)
>          }
>      }
>      if (qemu_wakeup_requested()) {
> -        pause_all_vcpus();
> -        cpu_synchronize_all_states();
> -        qemu_system_reset(VMRESET_SILENT);
> -        resume_all_vcpus();
> -        monitor_protocol_event(QEVENT_WAKEUP, NULL);
> +        if (!xen_enabled()) {
> +            pause_all_vcpus();
> +            cpu_synchronize_all_states();
> +            qemu_system_reset(VMRESET_SILENT);
> +            resume_all_vcpus();
> +            monitor_protocol_event(QEVENT_WAKEUP, NULL);
> +        }
>      }
>      if (qemu_powerdown_requested()) {
>          qemu_system_powerdown();

Hi,

I don't think changing vl.c is a good idee for this.

Could you try to use a QMP command instead ? Look for the QMP command
"system_wakeup", it's probably what's needed to be called on qemu-xen.

A simple way to test this would be to call the QMP command manually with
QMP/qmp present in the qemu tree.
$ ./QMP/qmp --path=/run/xen/qmp-libxl-$domid system_wakeup

-- 
Anthony PERARD

Re: [PATCH] Qemu-xen: HVM S3 bugfix

[Liu, Jinsong]

Anthony PERARD wrote:
> On 13/08/13 15:15, Liu, Jinsong wrote:
>> From fc928436df17673bfea1d902859c682c99c1489d Mon Sep 17 00:00:00
>> 2001 From: Liu Jinsong <jinsong.liu@intel.com>
>> Date: Wed, 14 Aug 2013 05:26:56 +0800
>> Subject: [PATCH] Qemu-xen: HVM S3 bugfix
>> 
>> This patch is used to fix HVM S3 bug caused from the difference
>> between qemu-traditioanl and standard qemu (which qemu-xen stems
>> from). 
>> For qemu-traditional, hvm s3 resumes via 'xl trigger' command
>> interface. For standard qemu, hvm s3 resumes via keyboard triggering.
>> 
>> So currently for qemu-xen, both 'xl' interface and keyboard
>> triggering 
>> hvm s3 cannot work. 'xl' interface didn't reset ioemu devices, while
>> keyboard triggering didn't unpause hvm domain.
>> 
>> This patch fixes the bug. It keeps compatible w/ current xen 'xl'
>> interface, by moving qemu system reset logic forward to qemu suspend.
>> 
>> Signed-off-by: Liu Jinsong <jinsong.liu@intel.com>
>> ---
>>  vl.c |   25 ++++++++++++++++---------
>>  1 files changed, 16 insertions(+), 9 deletions(-)
>> 
>> diff --git a/vl.c b/vl.c
>> index 5314f55..deea92c 100644
>> --- a/vl.c
>> +++ b/vl.c
>> @@ -1619,10 +1619,15 @@ void qemu_system_reset_request(void)
>> 
>>  static void qemu_system_suspend(void)
>>  {
>> -    pause_all_vcpus();
>> -    notifier_list_notify(&suspend_notifiers, NULL);
>> -    runstate_set(RUN_STATE_SUSPENDED);
>> -    monitor_protocol_event(QEVENT_SUSPEND, NULL);
>> +    if (!xen_enabled()) {
>> +        pause_all_vcpus();
>> +        notifier_list_notify(&suspend_notifiers, NULL);
>> +        runstate_set(RUN_STATE_SUSPENDED);
>> +        monitor_protocol_event(QEVENT_SUSPEND, NULL); +    } else {
>> +        notifier_list_notify(&suspend_notifiers, NULL);
>> +        qemu_system_reset(VMRESET_SILENT);
>> +    }
>>  }
>> 
>>  void qemu_system_suspend_request(void)
>> @@ -1740,11 +1745,13 @@ static bool main_loop_should_exit(void)     
>>      } }
>>      if (qemu_wakeup_requested()) {
>> -        pause_all_vcpus();
>> -        cpu_synchronize_all_states();
>> -        qemu_system_reset(VMRESET_SILENT);
>> -        resume_all_vcpus();
>> -        monitor_protocol_event(QEVENT_WAKEUP, NULL);
>> +        if (!xen_enabled()) {
>> +            pause_all_vcpus();
>> +            cpu_synchronize_all_states();
>> +            qemu_system_reset(VMRESET_SILENT);
>> +            resume_all_vcpus();
>> +            monitor_protocol_event(QEVENT_WAKEUP, NULL); +        }
>>      }
>>      if (qemu_powerdown_requested()) {
>>          qemu_system_powerdown();
> 
> Hi,
> 
> I don't think changing vl.c is a good idee for this.
> 
> Could you try to use a QMP command instead ? Look for the QMP command
> "system_wakeup", it's probably what's needed to be called on qemu-xen.
> 

Good idea! thanks!
2 patches will send out later.

Regards,
Jinsong