diff mbox

[1/2] qem-xen: add later wakeup logic when qemu wakeup

Message ID DE8DF0795D48FD4CA783C40EC829233501348C60@SHSMSX101.ccr.corp.intel.com
State New
Headers show

Commit Message

Liu, Jinsong Sept. 1, 2013, 9:51 a.m. UTC 
From 86ad3bb83a984ad7bbc00b81d6a0bfc1abc543ca Mon Sep 17 00:00:00 2001
From: Liu Jinsong <jinsong.liu@intel.com>
Date: Sun, 1 Sep 2013 23:39:14 +0800
Subject: [PATCH 1/2] qemu-xen: add later wakeup logic when qemu wakeup

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 two qemu-xen patches and one xl patch to fix the HVM S3 bug.
This patch is the qemu-xen patch 1. It provides a later wakeup notifier
and a register function, and notifies the later wakeup list when
qemu wakup by 'xl trigger' command.

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

Comments

Anthony PERARD Sept. 3, 2013, 10:55 a.m. UTC | #1 
On 01/09/13 10:51, Liu, Jinsong wrote:
> From 86ad3bb83a984ad7bbc00b81d6a0bfc1abc543ca Mon Sep 17 00:00:00 2001
> From: Liu Jinsong <jinsong.liu@intel.com>
> Date: Sun, 1 Sep 2013 23:39:14 +0800
> Subject: [PATCH 1/2] qemu-xen: add later wakeup logic when qemu wakeup
> 
> 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 two qemu-xen patches and one xl patch to fix the HVM S3 bug.
> This patch is the qemu-xen patch 1. It provides a later wakeup notifier
> and a register function, and notifies the later wakeup list when
> qemu wakup by 'xl trigger' command.
> 
> Signed-off-by: Liu Jinsong <jinsong.liu@intel.com>
> ---
>  sysemu.h |    1 +
>  vl.c     |    8 ++++++++
>  2 files changed, 9 insertions(+), 0 deletions(-)
> 
> diff --git a/sysemu.h b/sysemu.h
> index b71f244..4dbcab7 100644
> --- a/sysemu.h
> +++ b/sysemu.h
> @@ -49,6 +49,7 @@ void qemu_register_suspend_notifier(Notifier *notifier);
>  void qemu_system_wakeup_request(WakeupReason reason);
>  void qemu_system_wakeup_enable(WakeupReason reason, bool enabled);
>  void qemu_register_wakeup_notifier(Notifier *notifier);
> +void qemu_register_later_wakeup_notifier(Notifier *notifier);
>  void qemu_system_shutdown_request(void);
>  void qemu_system_powerdown_request(void);
>  void qemu_register_powerdown_notifier(Notifier *notifier);
> diff --git a/vl.c b/vl.c
> index 5314f55..1c4842d 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -1478,6 +1478,8 @@ static NotifierList suspend_notifiers =
>      NOTIFIER_LIST_INITIALIZER(suspend_notifiers);
>  static NotifierList wakeup_notifiers =
>      NOTIFIER_LIST_INITIALIZER(wakeup_notifiers);
> +static NotifierList later_wakeup_notifiers =
> +    NOTIFIER_LIST_INITIALIZER(later_wakeup_notifiers);
>  static uint32_t wakeup_reason_mask = ~0;
>  static RunState vmstop_requested = RUN_STATE_MAX;
>  
> @@ -1668,6 +1670,11 @@ void qemu_register_wakeup_notifier(Notifier *notifier)
>      notifier_list_add(&wakeup_notifiers, notifier);
>  }
>  
> +void qemu_register_later_wakeup_notifier(Notifier *notifier)
> +{
> +    notifier_list_add(&later_wakeup_notifiers, notifier);
> +}
> +
>  void qemu_system_killed(int signal, pid_t pid)
>  {
>      shutdown_signal = signal;
> @@ -1744,6 +1751,7 @@ static bool main_loop_should_exit(void)
>          cpu_synchronize_all_states();
>          qemu_system_reset(VMRESET_SILENT);
>          resume_all_vcpus();
> +        notifier_list_notify(&later_wakeup_notifiers, NULL);
>          monitor_protocol_event(QEVENT_WAKEUP, NULL);
>      }
>      if (qemu_powerdown_requested()) {
> 

The patch those not apply properly to QEMU (upstream) but it just
because the file sysemu.h have been moved to include/sysemu/sysemu.h

Once this is fix:
Acked-by: Anthony PERARD <anthony.perard@citrix.com>
Liu, Jinsong Sept. 3, 2013, 11:04 a.m. UTC | #2 
Anthony PERARD wrote:
> On 01/09/13 10:51, Liu, Jinsong wrote:
>> From 86ad3bb83a984ad7bbc00b81d6a0bfc1abc543ca Mon Sep 17 00:00:00
>> 2001 
>> From: Liu Jinsong <jinsong.liu@intel.com>
>> Date: Sun, 1 Sep 2013 23:39:14 +0800
>> Subject: [PATCH 1/2] qemu-xen: add later wakeup logic when qemu
>> wakeup 
>> 
>> 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 two qemu-xen patches and one xl patch to fix the HVM S3 bug.
>> This patch is the qemu-xen patch 1. It provides a later wakeup
>> notifier 
>> and a register function, and notifies the later wakeup list when
>> qemu wakup by 'xl trigger' command.
>> 
>> Signed-off-by: Liu Jinsong <jinsong.liu@intel.com>
>> ---
>>  sysemu.h |    1 +
>>  vl.c     |    8 ++++++++
>>  2 files changed, 9 insertions(+), 0 deletions(-)
>> 
>> diff --git a/sysemu.h b/sysemu.h
>> index b71f244..4dbcab7 100644
>> --- a/sysemu.h
>> +++ b/sysemu.h
>> @@ -49,6 +49,7 @@ void qemu_register_suspend_notifier(Notifier
>>  *notifier); void qemu_system_wakeup_request(WakeupReason reason);
>>  void qemu_system_wakeup_enable(WakeupReason reason, bool enabled);
>>  void qemu_register_wakeup_notifier(Notifier *notifier);
>> +void qemu_register_later_wakeup_notifier(Notifier *notifier);
>>  void qemu_system_shutdown_request(void);
>>  void qemu_system_powerdown_request(void);
>>  void qemu_register_powerdown_notifier(Notifier *notifier);
>> diff --git a/vl.c b/vl.c
>> index 5314f55..1c4842d 100644
>> --- a/vl.c
>> +++ b/vl.c
>> @@ -1478,6 +1478,8 @@ static NotifierList suspend_notifiers =
>>      NOTIFIER_LIST_INITIALIZER(suspend_notifiers);
>>  static NotifierList wakeup_notifiers =
>>      NOTIFIER_LIST_INITIALIZER(wakeup_notifiers);
>> +static NotifierList later_wakeup_notifiers =
>> +    NOTIFIER_LIST_INITIALIZER(later_wakeup_notifiers);
>>  static uint32_t wakeup_reason_mask = ~0;
>>  static RunState vmstop_requested = RUN_STATE_MAX;
>> 
>> @@ -1668,6 +1670,11 @@ void qemu_register_wakeup_notifier(Notifier
>>      *notifier) notifier_list_add(&wakeup_notifiers, notifier);  }
>> 
>> +void qemu_register_later_wakeup_notifier(Notifier *notifier) +{
>> +    notifier_list_add(&later_wakeup_notifiers, notifier); +}
>> +
>>  void qemu_system_killed(int signal, pid_t pid)
>>  {
>>      shutdown_signal = signal;
>> @@ -1744,6 +1751,7 @@ static bool main_loop_should_exit(void)
>>          cpu_synchronize_all_states();
>>          qemu_system_reset(VMRESET_SILENT);
>>          resume_all_vcpus();
>> +        notifier_list_notify(&later_wakeup_notifiers, NULL);
>>          monitor_protocol_event(QEVENT_WAKEUP, NULL);      }
>>      if (qemu_powerdown_requested()) {
>> 
> 
> The patch those not apply properly to QEMU (upstream) but it just
> because the file sysemu.h have been moved to include/sysemu/sysemu.h
> 
> Once this is fix:
> Acked-by: Anthony PERARD <anthony.perard@citrix.com>

Yes. The patches are for qemu-xen tree, to fix xen hvm s3 issue.

Where should the 2 patches be checked in? qemu upstream (then backport to qemu-xen tree), or, qemu-xen tree?

Thanks,
Jinsong
Stefano Stabellini Sept. 5, 2013, 5:27 p.m. UTC | #3 
On Tue, 3 Sep 2013, Liu, Jinsong wrote:
> Anthony PERARD wrote:
> > On 01/09/13 10:51, Liu, Jinsong wrote:
> >> From 86ad3bb83a984ad7bbc00b81d6a0bfc1abc543ca Mon Sep 17 00:00:00
> >> 2001 
> >> From: Liu Jinsong <jinsong.liu@intel.com>
> >> Date: Sun, 1 Sep 2013 23:39:14 +0800
> >> Subject: [PATCH 1/2] qemu-xen: add later wakeup logic when qemu
> >> wakeup 
> >> 
> >> 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 two qemu-xen patches and one xl patch to fix the HVM S3 bug.
> >> This patch is the qemu-xen patch 1. It provides a later wakeup
> >> notifier 
> >> and a register function, and notifies the later wakeup list when
> >> qemu wakup by 'xl trigger' command.
> >> 
> >> Signed-off-by: Liu Jinsong <jinsong.liu@intel.com>
> >> ---
> >>  sysemu.h |    1 +
> >>  vl.c     |    8 ++++++++
> >>  2 files changed, 9 insertions(+), 0 deletions(-)
> >> 
> >> diff --git a/sysemu.h b/sysemu.h
> >> index b71f244..4dbcab7 100644
> >> --- a/sysemu.h
> >> +++ b/sysemu.h
> >> @@ -49,6 +49,7 @@ void qemu_register_suspend_notifier(Notifier
> >>  *notifier); void qemu_system_wakeup_request(WakeupReason reason);
> >>  void qemu_system_wakeup_enable(WakeupReason reason, bool enabled);
> >>  void qemu_register_wakeup_notifier(Notifier *notifier);
> >> +void qemu_register_later_wakeup_notifier(Notifier *notifier);
> >>  void qemu_system_shutdown_request(void);
> >>  void qemu_system_powerdown_request(void);
> >>  void qemu_register_powerdown_notifier(Notifier *notifier);
> >> diff --git a/vl.c b/vl.c
> >> index 5314f55..1c4842d 100644
> >> --- a/vl.c
> >> +++ b/vl.c
> >> @@ -1478,6 +1478,8 @@ static NotifierList suspend_notifiers =
> >>      NOTIFIER_LIST_INITIALIZER(suspend_notifiers);
> >>  static NotifierList wakeup_notifiers =
> >>      NOTIFIER_LIST_INITIALIZER(wakeup_notifiers);
> >> +static NotifierList later_wakeup_notifiers =
> >> +    NOTIFIER_LIST_INITIALIZER(later_wakeup_notifiers);
> >>  static uint32_t wakeup_reason_mask = ~0;
> >>  static RunState vmstop_requested = RUN_STATE_MAX;
> >> 
> >> @@ -1668,6 +1670,11 @@ void qemu_register_wakeup_notifier(Notifier
> >>      *notifier) notifier_list_add(&wakeup_notifiers, notifier);  }
> >> 
> >> +void qemu_register_later_wakeup_notifier(Notifier *notifier) +{
> >> +    notifier_list_add(&later_wakeup_notifiers, notifier); +}
> >> +
> >>  void qemu_system_killed(int signal, pid_t pid)
> >>  {
> >>      shutdown_signal = signal;
> >> @@ -1744,6 +1751,7 @@ static bool main_loop_should_exit(void)
> >>          cpu_synchronize_all_states();
> >>          qemu_system_reset(VMRESET_SILENT);
> >>          resume_all_vcpus();
> >> +        notifier_list_notify(&later_wakeup_notifiers, NULL);
> >>          monitor_protocol_event(QEVENT_WAKEUP, NULL);      }
> >>      if (qemu_powerdown_requested()) {
> >> 
> > 
> > The patch those not apply properly to QEMU (upstream) but it just
> > because the file sysemu.h have been moved to include/sysemu/sysemu.h
> > 
> > Once this is fix:
> > Acked-by: Anthony PERARD <anthony.perard@citrix.com>
> 
> Yes. The patches are for qemu-xen tree, to fix xen hvm s3 issue.
> 
> Where should the 2 patches be checked in? qemu upstream (then backport to qemu-xen tree), or, qemu-xen tree?

In general patches should go to qemu upstream first and then
be backported to qemu-xen.
Liu, Jinsong Sept. 6, 2013, 9:03 a.m. UTC | #4 
Stefano Stabellini wrote:
> On Tue, 3 Sep 2013, Liu, Jinsong wrote:
>> Anthony PERARD wrote:
>>> On 01/09/13 10:51, Liu, Jinsong wrote:
>>>> From 86ad3bb83a984ad7bbc00b81d6a0bfc1abc543ca Mon Sep 17 00:00:00
>>>> 2001 From: Liu Jinsong <jinsong.liu@intel.com>
>>>> Date: Sun, 1 Sep 2013 23:39:14 +0800
>>>> Subject: [PATCH 1/2] qemu-xen: add later wakeup logic when qemu
>>>> wakeup 
>>>> 
>>>> 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 two qemu-xen patches and one xl patch to fix the HVM S3
>>>> bug. This patch is the qemu-xen patch 1. It provides a later
>>>> wakeup notifier and a register function, and notifies the later
>>>> wakeup list when qemu wakup by 'xl trigger' command.
>>>> 
>>>> Signed-off-by: Liu Jinsong <jinsong.liu@intel.com>
>>>> ---
>>>>  sysemu.h |    1 +
>>>>  vl.c     |    8 ++++++++
>>>>  2 files changed, 9 insertions(+), 0 deletions(-)
>>>> 
>>>> diff --git a/sysemu.h b/sysemu.h
>>>> index b71f244..4dbcab7 100644
>>>> --- a/sysemu.h
>>>> +++ b/sysemu.h
>>>> @@ -49,6 +49,7 @@ void qemu_register_suspend_notifier(Notifier
>>>>  *notifier); void qemu_system_wakeup_request(WakeupReason reason);
>>>>  void qemu_system_wakeup_enable(WakeupReason reason, bool enabled);
>>>>  void qemu_register_wakeup_notifier(Notifier *notifier);
>>>> +void qemu_register_later_wakeup_notifier(Notifier *notifier);
>>>>  void qemu_system_shutdown_request(void);
>>>>  void qemu_system_powerdown_request(void);
>>>>  void qemu_register_powerdown_notifier(Notifier *notifier);
>>>> diff --git a/vl.c b/vl.c
>>>> index 5314f55..1c4842d 100644
>>>> --- a/vl.c
>>>> +++ b/vl.c
>>>> @@ -1478,6 +1478,8 @@ static NotifierList suspend_notifiers =
>>>>      NOTIFIER_LIST_INITIALIZER(suspend_notifiers);
>>>>  static NotifierList wakeup_notifiers =
>>>>      NOTIFIER_LIST_INITIALIZER(wakeup_notifiers);
>>>> +static NotifierList later_wakeup_notifiers =
>>>> +    NOTIFIER_LIST_INITIALIZER(later_wakeup_notifiers);
>>>>  static uint32_t wakeup_reason_mask = ~0;
>>>>  static RunState vmstop_requested = RUN_STATE_MAX;
>>>> 
>>>> @@ -1668,6 +1670,11 @@ void qemu_register_wakeup_notifier(Notifier
>>>>      *notifier) notifier_list_add(&wakeup_notifiers, notifier);  }
>>>> 
>>>> +void qemu_register_later_wakeup_notifier(Notifier *notifier) +{
>>>> +    notifier_list_add(&later_wakeup_notifiers, notifier); +} +
>>>>  void qemu_system_killed(int signal, pid_t pid)
>>>>  {
>>>>      shutdown_signal = signal;
>>>> @@ -1744,6 +1751,7 @@ static bool main_loop_should_exit(void)
>>>>          cpu_synchronize_all_states();
>>>>          qemu_system_reset(VMRESET_SILENT);
>>>>          resume_all_vcpus();
>>>> +        notifier_list_notify(&later_wakeup_notifiers, NULL);
>>>>          monitor_protocol_event(QEVENT_WAKEUP, NULL);      }
>>>>      if (qemu_powerdown_requested()) {
>>>> 
>>> 
>>> The patch those not apply properly to QEMU (upstream) but it just
>>> because the file sysemu.h have been moved to include/sysemu/sysemu.h
>>> 
>>> Once this is fix:
>>> Acked-by: Anthony PERARD <anthony.perard@citrix.com>
>> 
>> Yes. The patches are for qemu-xen tree, to fix xen hvm s3 issue.
>> 
>> Where should the 2 patches be checked in? qemu upstream (then
>> backport to qemu-xen tree), or, qemu-xen tree? 
> 
> In general patches should go to qemu upstream first and then
> be backported to qemu-xen.

OK, I will adjust system.h and then send to qemu upstream first.

Thanks,
Jinsong
diff mbox

Patch

diff --git a/sysemu.h b/sysemu.h
index b71f244..4dbcab7 100644
--- a/sysemu.h
+++ b/sysemu.h
@@ -49,6 +49,7 @@  void qemu_register_suspend_notifier(Notifier *notifier);
 void qemu_system_wakeup_request(WakeupReason reason);
 void qemu_system_wakeup_enable(WakeupReason reason, bool enabled);
 void qemu_register_wakeup_notifier(Notifier *notifier);
+void qemu_register_later_wakeup_notifier(Notifier *notifier);
 void qemu_system_shutdown_request(void);
 void qemu_system_powerdown_request(void);
 void qemu_register_powerdown_notifier(Notifier *notifier);
diff --git a/vl.c b/vl.c
index 5314f55..1c4842d 100644
--- a/vl.c
+++ b/vl.c
@@ -1478,6 +1478,8 @@  static NotifierList suspend_notifiers =
     NOTIFIER_LIST_INITIALIZER(suspend_notifiers);
 static NotifierList wakeup_notifiers =
     NOTIFIER_LIST_INITIALIZER(wakeup_notifiers);
+static NotifierList later_wakeup_notifiers =
+    NOTIFIER_LIST_INITIALIZER(later_wakeup_notifiers);
 static uint32_t wakeup_reason_mask = ~0;
 static RunState vmstop_requested = RUN_STATE_MAX;
 
@@ -1668,6 +1670,11 @@  void qemu_register_wakeup_notifier(Notifier *notifier)
     notifier_list_add(&wakeup_notifiers, notifier);
 }
 
+void qemu_register_later_wakeup_notifier(Notifier *notifier)
+{
+    notifier_list_add(&later_wakeup_notifiers, notifier);
+}
+
 void qemu_system_killed(int signal, pid_t pid)
 {
     shutdown_signal = signal;
@@ -1744,6 +1751,7 @@  static bool main_loop_should_exit(void)
         cpu_synchronize_all_states();
         qemu_system_reset(VMRESET_SILENT);
         resume_all_vcpus();
+        notifier_list_notify(&later_wakeup_notifiers, NULL);
         monitor_protocol_event(QEVENT_WAKEUP, NULL);
     }
     if (qemu_powerdown_requested()) {