From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:37862) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TkLV4-0002IG-8k for qemu-devel@nongnu.org; Sun, 16 Dec 2012 16:11:58 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1TkLV1-0002EA-Kn for qemu-devel@nongnu.org; Sun, 16 Dec 2012 16:11:54 -0500 Received: from mx1.redhat.com ([209.132.183.28]:5410) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TkLV1-0002Dz-DI for qemu-devel@nongnu.org; Sun, 16 Dec 2012 16:11:51 -0500 Date: Sun, 16 Dec 2012 23:15:00 +0200 From: "Michael S. Tsirkin" Message-ID: <20121216211500.GB21521@redhat.com> References: <1355322396-32026-1-git-send-email-pbonzini@redhat.com> <20121212153821.GA17446@redhat.com> <50C8B2FD.5030200@redhat.com> <20121212171847.GC18597@redhat.com> <50C8BF83.4010307@redhat.com> <20121212212720.GC23087@redhat.com> <50C997BF.80200@redhat.com> <20121216170418.GG15790@redhat.com> <50CE2184.9020908@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <50CE2184.9020908@redhat.com> Subject: Re: [Qemu-devel] [PATCH 0/2] virtio: reset all qbuses too when writing to the status field List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paolo Bonzini Cc: agraf@suse.de, qemu-devel On Sun, Dec 16, 2012 at 08:31:16PM +0100, Paolo Bonzini wrote: > Il 16/12/2012 18:04, Michael S. Tsirkin ha scritto: > > On Thu, Dec 13, 2012 at 09:54:23AM +0100, Paolo Bonzini wrote: > >> Il 12/12/2012 22:27, Michael S. Tsirkin ha scritto: > >>>>> Maybe it's obvious to you that qdev_reset_all(x) > >>>>> does a soft reset and what soft reset means > >>>>> for each bus type > >>>> > >>>> We can define soft reset to be *independent* of the bus type. As you > >>>> said, you access it with a device register. > >>> > >>> I think qemu has one type of reset ATM which is the hard reset. > >> > >> A hard reset would kill BARs and configuration space too. > >> qdev_reset_all doesn't. Ergo, it is not a hard reset. > >> > >> But hey, I'm not wed to names. Let's call it device-level reset and > >> bus-level reset. Whatever. > > > > It's not a question of a name. > > > > ATM qemu supports one kind of reset because it's a kind of reset all > > hardware supports. The moment we start inventing > > our own one we need to document exactly what it means. > > We have two, DeviceClass's and BusClass's reset members. > > I'll make a patch to document them. > > > You have a point about a problem. > > > > My problem is with the solution, this solution depends on the exact > > modeling for correctness and that I don't want to do since we seem to be > > re-shuffling what inherits what so often. > > > > For example I could not figure out how the reset function for virtio pci > > (which clears pending msix vectors so is required) was called by this. > > The same way a PCI bus reset clears pending MSIX vectors. > > qdev_reset_all(pci_dev) > -> qdev_walk_children(pci_dev, qdev_reset_one, qbus_reset_one, NULL); > -> qdev_reset_one(pci_dev, NULL); > -> device_reset(pci_dev); > -> calls dc->reset member set for virtio-*-pci, i.e. virtio_pci_reset > > > Another thing that bothers me is that during regular PCI bus reset > > virtio does not invoke qdev_reset_all but with this reset, it does. > > Inconsistent. > > It does. > > A PCI bus reset (or FLR) calls pci_device_reset which does this > > void pci_device_reset(PCIDevice *dev) > { > int r; > > qdev_reset_all(&dev->qdev); > ... > } > > This is exactly how a PCI bus reset clears pending MSIX vectors. > > Paolo Yes but I mean virtio-pci device is not a bus. virtio could be a bus. Our modeling seems incorrect and relying on it for correctness, looks a bit wrong. -- MST