From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:35781) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TsI5Y-0002Te-45 for qemu-devel@nongnu.org; Mon, 07 Jan 2013 14:10:25 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1TsI5V-0005hO-UZ for qemu-devel@nongnu.org; Mon, 07 Jan 2013 14:10:24 -0500 Received: from e7.ny.us.ibm.com ([32.97.182.137]:55662) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TsI5V-0005gL-PO for qemu-devel@nongnu.org; Mon, 07 Jan 2013 14:10:21 -0500 Received: from /spool/local by e7.ny.us.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Mon, 7 Jan 2013 14:10:19 -0500 Received: from d01relay04.pok.ibm.com (d01relay04.pok.ibm.com [9.56.227.236]) by d01dlp02.pok.ibm.com (Postfix) with ESMTP id 5369D6E803C for ; Mon, 7 Jan 2013 14:10:16 -0500 (EST) Received: from d01av01.pok.ibm.com (d01av01.pok.ibm.com [9.56.224.215]) by d01relay04.pok.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id r07JAEjv191448 for ; Mon, 7 Jan 2013 14:10:14 -0500 Received: from d01av01.pok.ibm.com (loopback [127.0.0.1]) by d01av01.pok.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP id r07JABQF014893 for ; Mon, 7 Jan 2013 14:10:11 -0500 From: Anthony Liguori In-Reply-To: <1355761490-10073-1-git-send-email-pbonzini@redhat.com> References: <1355761490-10073-1-git-send-email-pbonzini@redhat.com> Date: Mon, 07 Jan 2013 13:10:04 -0600 Message-ID: <877gnovmgj.fsf@codemonkey.ws> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Subject: Re: [Qemu-devel] [PATCH 00/15] qdev: make reset semantics more clear and consistent, reset qbuses under virtio devices List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paolo Bonzini , qemu-devel@nongnu.org Cc: mst@redhat.com Paolo Bonzini writes: > After discussion with mst on the topic of resetting virtio devices, > here is a series that hopefully clarifies the semantics of bus and > device resets. > > After this series, there are two kinds of resets: > > 1) device-level reset is the kind of reset that you get with a register > write on the device. It will clear interrupts and DMAs among other things, > but not any bus-level state, for example it will not clear PCI BARs and > other configuration space data. It is done with qdev_reset_all. > > 2) bus-level reset is the kind of reset that you get with a register > write on the device that exports the bus (including triggering a device-level > reset on the device that exports the bus). It will do a device-level > reset on the child, but also clear bus-level state such as PCI BARs and > other configuration space data. It can be triggered for all devices > on a bus with qbus_reset_all. There is still no API for a bus-level > reset of a single device (like PCI FLR), this can be added later. I don't really understand this dual abstraction. I suspect it's overgeneralizing something that's the result of poor modeling. Very often with PCI devices, you have an otherwise independent chipset embedded on a PCI card There's a clear separate between the chipset and the card. It may be possible to poke the chipset directly to do a reset and that may only affect state that's on the chipset but not any card-specific state. But this has nothing to do with busses, this is just about encapsulation. IOW, what you have is: E1000 is-a PCIDevice has-a E1000 chipset PCIDevice::reset() -> calls chipset->reset() But with the right register write, chipset->reset() can be called w/o PCIDevice::reset(). But again, this has nothing to do with busses. What I'm missing with this series is what problem are we trying to solve? I don't think we model reset correctly today because I don't think there's a single notion of reset. I think reset really ought to just be a bus level concept with individual implementations for each bus. Regards, Anthony Liguori > > Patches 1-5 are miscellaneous fixes to the reset paths. > > Patches 6-8 introduce qbus_reset_all and uses it. > > Patches 9-12 switch qdev_reset_all and qbus_reset_all from pre-order > to post-order, and document the resulting semantics. > > Finally, patches 13-15 adjust the virtio implementations to use qdev > device-level reset more extensively. > > Paolo Bonzini (15): > qdev: do not reset a device until the parent has been initialized > intel-hda: do not reset codecs from intel_hda_reset > pci: clean up resetting of IRQs > virtio-pci: reset device before PCI layer > virtio-s390: add a reset function to virtio-s390 devices > qdev: add qbus_reset_all > pci: do not export pci_bus_reset > lsi: use qbus_reset_all to reset SCSI bus > qdev: allow both pre- and post-order vists in qdev walking functions > qdev: switch reset to post-order > qdev: remove device_reset > qdev: document reset semantics > virtio-pci: reset all qbuses too when writing to the status field > virtio-s390: reset all qbuses too when writing to the status field > virtio-serial: do not perform bus reset by hand > > hw/intel-hda.c | 1 - > hw/lsi53c895a.c | 7 +---- > hw/pci.c | 16 ++++------ > hw/pci.h | 1 - > hw/pci_bridge.c | 2 +- > hw/qdev-core.h | 83 ++++++++++++++++++++++++++++++++++++++++++-------- > hw/qdev.c | 70 +++++++++++++++++++++++++++--------------- > hw/s390-virtio-bus.c | 16 +++++++++- > hw/virtio-pci.c | 27 +++++++--------- > hw/virtio-serial-bus.c | 19 +++--------- > 10 files changed, 156 insertions(+), 86 deletions(-) > > -- > 1.8.0.2