From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:39342) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VtiC1-00037i-CG for qemu-devel@nongnu.org; Thu, 19 Dec 2013 13:19:35 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1VtiBv-0002Xr-Bl for qemu-devel@nongnu.org; Thu, 19 Dec 2013 13:19:29 -0500 Received: from mx1.redhat.com ([209.132.183.28]:61604) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VtiBv-0002XZ-37 for qemu-devel@nongnu.org; Thu, 19 Dec 2013 13:19:23 -0500 Received: from int-mx09.intmail.prod.int.phx2.redhat.com (int-mx09.intmail.prod.int.phx2.redhat.com [10.5.11.22]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id rBJIJMc0002418 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Thu, 19 Dec 2013 13:19:22 -0500 Date: Thu, 19 Dec 2013 20:23:14 +0200 From: "Michael S. Tsirkin" Message-ID: <20131219182314.GA15108@redhat.com> References: <1386348867-25038-1-git-send-email-pbonzini@redhat.com> <1386348867-25038-5-git-send-email-pbonzini@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <1386348867-25038-5-git-send-email-pbonzini@redhat.com> Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH 4/4] qdev: switch reset to post-order List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paolo Bonzini Cc: qemu-devel@nongnu.org On Fri, Dec 06, 2013 at 05:54:27PM +0100, Paolo Bonzini wrote: > 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 cache= s > could be in any state. >=20 > If post-order is used, on the other hand, these will be in a known stat= e > when the reset method is invoked on the parent. >=20 > 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. >=20 > PCI can then rely on the qdev core having sent a "reset signal" (whatev= er > that means) to the device, and only do the PCI-specific initialization > with pci_do_device_reset. >=20 > Signed-off-by: Paolo Bonzini > --- > hw/core/qdev.c | 6 +++--- > hw/pci/pci.c | 31 ++++++++++++++++--------------- > include/hw/qdev-core.h | 2 +- > 3 files changed, 20 insertions(+), 19 deletions(-) >=20 > diff --git a/hw/core/qdev.c b/hw/core/qdev.c > index 1c114b7..9ba8ab1 100644 > --- a/hw/core/qdev.c > +++ b/hw/core/qdev.c > @@ -233,19 +233,19 @@ static int qbus_reset_one(BusState *bus, void *op= aque) > { > BusClass *bc =3D BUS_GET_CLASS(bus); > if (bc->reset) { > - return bc->reset(bus); > + bc->reset(bus); > } > return 0; > } > =20 > 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); > } > =20 > 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); > } > =20 > void qbus_reset_all_fn(void *opaque) > diff --git a/hw/pci/pci.c b/hw/pci/pci.c > index 0efc544..e10d74b 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 inden= t); > 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); > =20 > static Property pci_props[] =3D { > DEFINE_PROP_PCI_DEVFN("addr", PCIDevice, devfn, -1), > @@ -165,16 +165,10 @@ void pci_device_deassert_intx(PCIDevice *dev) > } > } > =20 > -/* > - * 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; > =20 > - qdev_reset_all(&dev->qdev); > - > dev->irq_state =3D 0; > pci_update_irq_status(dev); > pci_device_deassert_intx(dev); > @@ -207,27 +201,34 @@ void pci_device_reset(PCIDevice *dev) > } > =20 > /* > + * 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 =3D DO_UPCAST(PCIBus, qbus, qbus); > int i; > =20 > for (i =3D 0; i < ARRAY_SIZE(bus->devices); ++i) { > if (bus->devices[i]) { > - pci_device_reset(bus->devices[i]); > + pci_do_device_reset(bus->devices[i]); > } > } > =20 > for (i =3D 0; i < bus->nirq; i++) { > assert(bus->irq_count[i] =3D=3D 0); > } > - > - /* topology traverse is done by pci_bus_reset(). > - Tell qbus/qdev walker not to traverse the tree */ > - return 1; > } > =20 > static void pci_host_bus_register(PCIBus *bus, DeviceState *parent) > diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h > index 21ea2c6..409fd71 100644 > --- a/include/hw/qdev-core.h > +++ b/include/hw/qdev-core.h > @@ -178,7 +178,7 @@ struct BusClass { > * bindings can be found at http://playground.sun.com/1275/binding= s/. > */ > 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; > }; > --=20 > 1.7.1 This seems to break a bunch of stuff: /scm/qemu/hw/s390x/virtio-ccw.c: In function =E2=80=98virtual_css_bus_class_init=E2=80=99: /scm/qemu/hw/s390x/virtio-ccw.c:47:14: error: assignment from incompatible pointer type [-Werror] k->reset =3D virtual_css_bus_reset; ^