Patchwork [RFC,1/5] suspend: add infrastructure

login
register
mail settings
Submitter Gerd Hoffmann
Date Jan. 11, 2012, 3:08 p.m.
Message ID <1326294541-15080-2-git-send-email-kraxel@redhat.com>
Download mbox | patch
Permalink /patch/135417/
State New
Headers show

Comments

Gerd Hoffmann - Jan. 11, 2012, 3:08 p.m.
This patch adds qemu_system_suspend_request() and void
qemu_system_wakeup_request() functions to qemu.

qemu_system_suspend_request is supposed to be called when the guest
asks for being be suspended, for example via ACPI.

qemu_system_wakeup_request is supposed to be called on events which
should wake up the guest.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 sysemu.h |    2 ++
 vl.c     |   21 +++++++++++++++++++++
 2 files changed, 23 insertions(+), 0 deletions(-)
Paolo Bonzini - Jan. 13, 2012, 3:51 p.m.
On 01/11/2012 04:08 PM, Gerd Hoffmann wrote:
> +void qemu_system_suspend_request(qemu_irq wake_irq)
> +{
> +    if (suspend_wake_irq != NULL) {
> +        return;
> +    }
> +    cpu_stop_current();
> +    qemu_notify_event();
> +    suspend_wake_irq = wake_irq;
> +}
> +
> +void qemu_system_wakeup_request(void)
> +{
> +    if (suspend_wake_irq == NULL) {
> +        return;
> +    }
> +    reset_requested = 1;
> +    qemu_irq_raise(suspend_wake_irq);
> +    suspend_wake_irq = NULL;
> +}

The code in acpi.c is confusing, but it seems to me that IRQ is raised 
when the system _enters_ S3.  See especially xen-all.c.  It would be 
lowered by acpi_pm1_cnt_reset when the reset actually happens on the 
next main loop iteration.  However, acpi_pm1_cnt_reset has never been 
called since its introduction in commit eaba51c (acpi, acpi_piix, 
vt82c686: factor out PM1_CNT logic, 2011-03-25).

Overall, it seems to me that with your new infrastructure a global 
Notifier would be a better fit than a qemu_irq that the board would have 
to pass all the way.  A notifier would also remove the ugly Xen special 
casing.  However, this can be done separately.

Paolo
Gerd Hoffmann - Jan. 16, 2012, 6:12 p.m.
On 01/13/12 16:51, Paolo Bonzini wrote:
> On 01/11/2012 04:08 PM, Gerd Hoffmann wrote:
>> +void qemu_system_suspend_request(qemu_irq wake_irq)
>> +{
>> +    if (suspend_wake_irq != NULL) {
>> +        return;
>> +    }
>> +    cpu_stop_current();
>> +    qemu_notify_event();
>> +    suspend_wake_irq = wake_irq;
>> +}
>> +
>> +void qemu_system_wakeup_request(void)
>> +{
>> +    if (suspend_wake_irq == NULL) {
>> +        return;
>> +    }
>> +    reset_requested = 1;
>> +    qemu_irq_raise(suspend_wake_irq);
>> +    suspend_wake_irq = NULL;
>> +}
> 
> The code in acpi.c is confusing, but it seems to me that IRQ is raised
> when the system _enters_ S3.  See especially xen-all.c.

> Overall, it seems to me that with your new infrastructure a global
> Notifier would be a better fit than a qemu_irq that the board would have
> to pass all the way.

Indeed.  Especially as it isn't actually a IRQ line but just a qemu_irq
struct which is abused as notifier ...

> A notifier would also remove the ugly Xen special
> casing.

Yes.

> However, this can be done separately.

It's easier to do it all in one go.  It also gives some bonus points
because it'll remove more code than it adds ;)

v2 goes out in a minute ...

thanks for the review,
  Gerd

Patch

diff --git a/sysemu.h b/sysemu.h
index 3806901..d1834a0 100644
--- a/sysemu.h
+++ b/sysemu.h
@@ -39,6 +39,8 @@  void vm_stop(RunState state);
 void vm_stop_force_state(RunState state);
 
 void qemu_system_reset_request(void);
+void qemu_system_suspend_request(qemu_irq wake_irq);
+void qemu_system_wakeup_request(void);
 void qemu_system_shutdown_request(void);
 void qemu_system_powerdown_request(void);
 void qemu_system_debug_request(void);
diff --git a/vl.c b/vl.c
index ba55b35..26413e3 100644
--- a/vl.c
+++ b/vl.c
@@ -1282,6 +1282,7 @@  static int shutdown_requested, shutdown_signal = -1;
 static pid_t shutdown_pid;
 static int powerdown_requested;
 static int debug_requested;
+static qemu_irq suspend_wake_irq;
 static RunState vmstop_requested = RUN_STATE_MAX;
 
 int qemu_shutdown_requested_get(void)
@@ -1397,6 +1398,26 @@  void qemu_system_reset_request(void)
     qemu_notify_event();
 }
 
+void qemu_system_suspend_request(qemu_irq wake_irq)
+{
+    if (suspend_wake_irq != NULL) {
+        return;
+    }
+    cpu_stop_current();
+    qemu_notify_event();
+    suspend_wake_irq = wake_irq;
+}
+
+void qemu_system_wakeup_request(void)
+{
+    if (suspend_wake_irq == NULL) {
+        return;
+    }
+    reset_requested = 1;
+    qemu_irq_raise(suspend_wake_irq);
+    suspend_wake_irq = NULL;
+}
+
 void qemu_system_killed(int signal, pid_t pid)
 {
     shutdown_signal = signal;