Patchwork Allow QEMUMachine to override reset sequencing

login
register
mail settings
Submitter David Gibson
Date Aug. 7, 2012, 6:41 a.m.
Message ID <1344321711-15782-1-git-send-email-david@gibson.dropbear.id.au>
Download mbox | patch
Permalink /patch/175508/
State New
Headers show

Comments

David Gibson - Aug. 7, 2012, 6:41 a.m.
qemu_system_reset() function always performs the same basic actions on
all machines.  This includes running all the reset handler hooks,
however the order in which these will run is not always easily predictable.

This patch splits the core of qemu_system_reset() - the invocation of
the reset handlers - out into a new qemu_devices_reset() function.
qemu_system_reset() will usually call qemu_devices_reset(), but that
can be now overriden by a new reset method in the QEMUMachine
structure.

Individual machines can use this reset method, if necessary, to
perform any extra, machine specific initializations which have to
occur before or after the bulk of the reset handlers.  It's expected
that the method will call qemu_devices_reset() at some point, but if
the machine has really strange ordering requirements between devices
resets it could even override that with it's own reset sequence (with
great care, obviously).

For a specific example of when this might be needed: a number of
machines (but not PC) load images specified with -kernel or -initrd
directly into the machine RAM before booting the guest.  This mostly
works at the moment, but to make this actually safe requires that this
load occurs after peripheral devices are reset - otherwise they could
have active DMAs in progress which would clobber the in memory images.
Some machines (notably pseries) also have other entry conditions which
need to be set up as the last thing before executing in guest space -
some of this could be considered "emulated firmware" in the sense that
the actions of the firmware are emulated directly by qemu rather than
by executing a firmware image within the guest.  When the platform's
firmware to OS interface is sufficiently well specified, this saves
time both in implementing the "firmware" and executing it.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 hw/boards.h |    3 +++
 sysemu.h    |    1 +
 vl.c        |   11 ++++++++++-
 3 files changed, 14 insertions(+), 1 deletion(-)
Benjamin Herrenschmidt - Aug. 7, 2012, 9:53 p.m.
On Tue, 2012-08-07 at 16:41 +1000, David Gibson wrote:
> qemu_system_reset() function always performs the same basic actions on
> all machines.  This includes running all the reset handler hooks,
> however the order in which these will run is not always easily predictable.
> 
> This patch splits the core of qemu_system_reset() - the invocation of
> the reset handlers - out into a new qemu_devices_reset() function.
> qemu_system_reset() will usually call qemu_devices_reset(), but that
> can be now overriden by a new reset method in the QEMUMachine
> structure.

 .../...

Hi Anthony !

What's your take on this one ? It's a pre-req for a subsequent patch
that finally fixes reset/reboot for us, and we'd like to have that
latter patch in before the hard freeze :-) But Alex can't really take it
until the hook is in.

Will you take this hook directly ?

Cheers,
Ben.
Andreas Färber - Aug. 9, 2012, 3:47 p.m.
Am 07.08.2012 08:41, schrieb David Gibson:
> qemu_system_reset() function always performs the same basic actions on
> all machines.  This includes running all the reset handler hooks,
> however the order in which these will run is not always easily predictable.
> 
> This patch splits the core of qemu_system_reset() - the invocation of
> the reset handlers - out into a new qemu_devices_reset() function.
> qemu_system_reset() will usually call qemu_devices_reset(), but that
> can be now overriden by a new reset method in the QEMUMachine
> structure.
> 
> Individual machines can use this reset method, if necessary, to
> perform any extra, machine specific initializations which have to
> occur before or after the bulk of the reset handlers.  It's expected
> that the method will call qemu_devices_reset() at some point, but if
> the machine has really strange ordering requirements between devices
> resets it could even override that with it's own reset sequence (with
> great care, obviously).
> 
> For a specific example of when this might be needed: a number of
> machines (but not PC) load images specified with -kernel or -initrd
> directly into the machine RAM before booting the guest.  This mostly
> works at the moment, but to make this actually safe requires that this
> load occurs after peripheral devices are reset - otherwise they could
> have active DMAs in progress which would clobber the in memory images.
> Some machines (notably pseries) also have other entry conditions which
> need to be set up as the last thing before executing in guest space -
> some of this could be considered "emulated firmware" in the sense that
> the actions of the firmware are emulated directly by qemu rather than
> by executing a firmware image within the guest.  When the platform's
> firmware to OS interface is sufficiently well specified, this saves
> time both in implementing the "firmware" and executing it.
> 
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>

Reviewed-by: Andreas Färber <afaerber@suse.de>

I'll put together a follow-up to show what I meant in the v1 thread.

Andreas

Patch

diff --git a/hw/boards.h b/hw/boards.h
index 59c01d0..a2e0a54 100644
--- a/hw/boards.h
+++ b/hw/boards.h
@@ -12,11 +12,14 @@  typedef void QEMUMachineInitFunc(ram_addr_t ram_size,
                                  const char *initrd_filename,
                                  const char *cpu_model);
 
+typedef void QEMUMachineResetFunc(void);
+
 typedef struct QEMUMachine {
     const char *name;
     const char *alias;
     const char *desc;
     QEMUMachineInitFunc *init;
+    QEMUMachineResetFunc *reset;
     int use_scsi;
     int max_cpus;
     unsigned int no_serial:1,
diff --git a/sysemu.h b/sysemu.h
index 7fbf8f4..a2383d9 100644
--- a/sysemu.h
+++ b/sysemu.h
@@ -62,6 +62,7 @@  int qemu_powerdown_requested(void);
 void qemu_system_killed(int signal, pid_t pid);
 void qemu_kill_report(void);
 extern qemu_irq qemu_system_powerdown;
+void qemu_devices_reset(void);
 void qemu_system_reset(bool report);
 
 void qemu_add_exit_notifier(Notifier *notify);
diff --git a/vl.c b/vl.c
index 77e8962..12d5775 100644
--- a/vl.c
+++ b/vl.c
@@ -1406,7 +1406,7 @@  void qemu_unregister_reset(QEMUResetHandler *func, void *opaque)
     }
 }
 
-void qemu_system_reset(bool report)
+void qemu_devices_reset(void)
 {
     QEMUResetEntry *re, *nre;
 
@@ -1414,6 +1414,15 @@  void qemu_system_reset(bool report)
     QTAILQ_FOREACH_SAFE(re, &reset_handlers, entry, nre) {
         re->func(re->opaque);
     }
+}
+
+void qemu_system_reset(bool report)
+{
+    if (current_machine->reset) {
+        current_machine->reset();
+    } else {
+        qemu_devices_reset();
+    }
     if (report) {
         monitor_protocol_event(QEVENT_RESET, NULL);
     }