From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1ME3Bs-0004Ko-Es for qemu-devel@nongnu.org; Tue, 09 Jun 2009 11:24:44 -0400 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1ME3Bn-0004Ea-CM for qemu-devel@nongnu.org; Tue, 09 Jun 2009 11:24:43 -0400 Received: from [199.232.76.173] (port=56359 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1ME3Bn-0004EV-7Y for qemu-devel@nongnu.org; Tue, 09 Jun 2009 11:24:39 -0400 Received: from mx2.redhat.com ([66.187.237.31]:47081) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1ME3Bm-0006S0-Mb for qemu-devel@nongnu.org; Tue, 09 Jun 2009 11:24:38 -0400 Date: Tue, 9 Jun 2009 18:24:33 +0300 From: Gleb Natapov Subject: Re: [Qemu-devel] [PATCH 2/3] Add pci_bus_reset() function. Message-ID: <20090609152433.GK5558@redhat.com> References: <1244465766-6349-1-git-send-email-gleb@redhat.com> <1244465766-6349-2-git-send-email-gleb@redhat.com> <4A2D219F.1020408@redhat.com> <20090608143710.GM27210@redhat.com> <4A2D85B8.1090301@redhat.com> <20090609053154.GO27210@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Blue Swirl Cc: Yaniv Kamay , Dor Laor , Avi Kivity , qemu-devel@nongnu.org On Tue, Jun 09, 2009 at 06:07:00PM +0300, Blue Swirl wrote: > On 6/9/09, Gleb Natapov wrote: > > On Tue, Jun 09, 2009 at 12:42:16AM +0300, Dor Laor wrote: > > > Gleb Natapov wrote: > > >> On Mon, Jun 08, 2009 at 05:35:11PM +0300, Avi Kivity wrote: > > >> > > >>> Gleb Natapov wrote: > > >>> > > >>>> To reset internal irq handling data structures. > > >>>> > > >>>> Signed-off-by: Gleb Natapov > > >>>> Signed-off-by: Yaniv Kamay > > >>>> --- > > >>>> hw/pci.c | 16 ++++++++++++++++ > > >>>> 1 files changed, 16 insertions(+), 0 deletions(-) > > >>>> > > >>>> diff --git a/hw/pci.c b/hw/pci.c > > >>>> index 02b335f..89fefdf 100644 > > >>>> --- a/hw/pci.c > > >>>> +++ b/hw/pci.c > > >>>> @@ -88,6 +88,21 @@ static int pcibus_load(QEMUFile *f, void *opaque, int version_id) > > >>>> return 0; > > >>>> } > > >>>> +static void pci_bus_reset(void *opaque) > > >>>> +{ > > >>>> + PCIBus *bus = (PCIBus *)opaque; > > >>>> + int i; > > >>>> + > > >>>> + for (i = 0; i < bus->nirq; i++) { > > >>>> + bus->irq_count[i] = 0; > > >>>> + } > > >>>> + for (i = 0; i < PCI_DEVICES_MAX; i++) { > > >>>> + if (bus->devices[i]) > > >>>> + memset(bus->devices[i]->irq_state, 0, > > >>>> + sizeof(bus->devices[i]->irq_state)); > > >>>> + } > > >>>> +} > > >>>> + > > >>>> > > >>> Shouldn't each device's reset function bring its line low, thus > > >>> zeroing the irq_state naturally? > > >>> > > >>> If not, we have a bug somewhere. Note we have exactly the same issue > > >>> with save/restore. > > >>> > > >>> > > >> They should, but I haven't found one that does. > > >> > > > virtio does and so do many others. Sad thing is that all should do it > > > since the line is shared. > > > e1000 and rtl8139 do not register a reset handler. > > > > > > > So somebody (not me) should go and fix all others. > > Here's a 5 min patch to add reset to e1000 and rtl8139. Not too difficult? > Very handy. I just discovered that e1000 continue to write to memory after OS reboot with some OSes. Will check if this fixes it. > > > Maybe we should make it a required callback for pci_qdev_register? or better > > > have every (pci?) device register several callback together. > > > > May be. > > If we go to this, I'd also make savevm/loadvm mandatory. We need this for device hot unplug. We have to be able to reset only one device at a time. -- Gleb.