[Qemu-devel] [PULL 16/19] qdev: switch reset to post-order

["Michael S. Tsirkin" <mst@redhat.com>]

From: Paolo Bonzini <pbonzini@redhat.com>

Post-order is the only sensible direction for the reset signals.
For example, suppose pre-order is used and the parent has some data
structures that cache children state (for example a list of active
requests).  When the reset method is invoked on the parent, these caches
could be in any state.

If post-order is used, on the other hand, these will be in a known state
when the reset method is invoked on the parent.

This change means that it is no longer possible to block the visit of
the devices, so the callback is changed to return void.  This is not
a problem, because PCI was returning 1 exactly in order to achieve the
same ordering that this patch implements.

PCI can then rely on the qdev core having sent a "reset signal" (whatever
that means) to the device, and only do the PCI-specific initialization
with pci_do_device_reset.

MST: fixed up virtio-ccw

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 include/hw/qdev-core.h |  2 +-
 hw/core/qdev.c         |  6 +++---
 hw/pci/pci.c           | 31 ++++++++++++++++---------------
 hw/s390x/virtio-ccw.c  |  5 +----
 4 files changed, 21 insertions(+), 23 deletions(-)

diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
index ecf5cb3..a9ce4a3 100644
--- a/include/hw/qdev-core.h
+++ b/include/hw/qdev-core.h
@@ -158,7 +158,7 @@ struct BusClass {
      * bindings can be found at http://playground.sun.com/1275/bindings/.
      */
     char *(*get_fw_dev_path)(DeviceState *dev);
-    int (*reset)(BusState *bus);
+    void (*reset)(BusState *bus);
     /* maximum devices allowed on the bus, 0: no limit. */
     int max_dev;
 };
diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index 5ddf1aa..d2ffe35 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -233,19 +233,19 @@ static int qbus_reset_one(BusState *bus, void *opaque)
 {
     BusClass *bc = BUS_GET_CLASS(bus);
     if (bc->reset) {
-        return bc->reset(bus);
+        bc->reset(bus);
     }
     return 0;
 }
 
 void qdev_reset_all(DeviceState *dev)
 {
-    qdev_walk_children(dev, qdev_reset_one, qbus_reset_one, NULL, NULL, NULL);
+    qdev_walk_children(dev, NULL, NULL, qdev_reset_one, qbus_reset_one, NULL);
 }
 
 void qbus_reset_all(BusState *bus)
 {
-    qbus_walk_children(bus, qdev_reset_one, qbus_reset_one, NULL, NULL, NULL);
+    qbus_walk_children(bus, NULL, NULL, qdev_reset_one, qbus_reset_one, NULL);
 }
 
 void qbus_reset_all_fn(void *opaque)
diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index 131e599..aa2a395 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -46,7 +46,7 @@
 static void pcibus_dev_print(Monitor *mon, DeviceState *dev, int indent);
 static char *pcibus_get_dev_path(DeviceState *dev);
 static char *pcibus_get_fw_dev_path(DeviceState *dev);
-static int pcibus_reset(BusState *qbus);
+static void pcibus_reset(BusState *qbus);
 static void pci_bus_finalize(Object *obj);
 
 static Property pci_props[] = {
@@ -167,16 +167,10 @@ void pci_device_deassert_intx(PCIDevice *dev)
     }
 }
 
-/*
- * This function is called on #RST and FLR.
- * FLR if PCI_EXP_DEVCTL_BCR_FLR is set
- */
-void pci_device_reset(PCIDevice *dev)
+static void pci_do_device_reset(PCIDevice *dev)
 {
     int r;
 
-    qdev_reset_all(&dev->qdev);
-
     dev->irq_state = 0;
     pci_update_irq_status(dev);
     pci_device_deassert_intx(dev);
@@ -209,27 +203,34 @@ void pci_device_reset(PCIDevice *dev)
 }
 
 /*
+ * This function is called on #RST and FLR.
+ * FLR if PCI_EXP_DEVCTL_BCR_FLR is set
+ */
+void pci_device_reset(PCIDevice *dev)
+{
+    qdev_reset_all(&dev->qdev);
+    pci_do_device_reset(dev);
+}
+
+/*
  * Trigger pci bus reset under a given bus.
- * To be called on RST# assert.
+ * Called via qbus_reset_all on RST# assert, after the devices
+ * have been reset qdev_reset_all-ed already.
  */
-static int pcibus_reset(BusState *qbus)
+static void pcibus_reset(BusState *qbus)
 {
     PCIBus *bus = DO_UPCAST(PCIBus, qbus, qbus);
     int i;
 
     for (i = 0; i < ARRAY_SIZE(bus->devices); ++i) {
         if (bus->devices[i]) {
-            pci_device_reset(bus->devices[i]);
+            pci_do_device_reset(bus->devices[i]);
         }
     }
 
     for (i = 0; i < bus->nirq; i++) {
         assert(bus->irq_count[i] == 0);
     }
-
-    /* topology traverse is done by pci_bus_reset().
-       Tell qbus/qdev walker not to traverse the tree */
-    return 1;
 }
 
 static void pci_host_bus_register(PCIBus *bus, DeviceState *parent)
diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c
index ecc80ec..b79f04e 100644
--- a/hw/s390x/virtio-ccw.c
+++ b/hw/s390x/virtio-ccw.c
@@ -30,13 +30,10 @@
 static void virtio_ccw_bus_new(VirtioBusState *bus, size_t bus_size,
                                VirtioCcwDevice *dev);
 
-static int virtual_css_bus_reset(BusState *qbus)
+static void virtual_css_bus_reset(BusState *qbus)
 {
     /* This should actually be modelled via the generic css */
     css_reset();
-
-    /* we dont traverse ourself, return 0 */
-    return 0;
 }
 
 
-- 
MST