From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=44511 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1PLD4x-0008LT-Uz for qemu-devel@nongnu.org; Wed, 24 Nov 2010 06:00:01 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1PLD4w-0002wL-Lh for qemu-devel@nongnu.org; Wed, 24 Nov 2010 05:59:59 -0500 Received: from mx1.redhat.com ([209.132.183.28]:31400) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1PLD4w-0002w0-Ek for qemu-devel@nongnu.org; Wed, 24 Nov 2010 05:59:58 -0500 Date: Wed, 24 Nov 2010 12:59:43 +0200 From: "Michael S. Tsirkin" Message-ID: <20101124105943.GD23493@redhat.com> References: <20101122075402.GA6892@redhat.com> <20101122104337.GB24265@valinux.co.jp> <20101122225312.GA1359@redhat.com> <20101123181026.GA2752@redhat.com> <20101124023725.GC9591@valinux.co.jp> <20101124052758.GA19320@redhat.com> <20101124071514.GA5118@valinux.co.jp> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20101124071514.GA5118@valinux.co.jp> Subject: [Qemu-devel] Re: [PATCH v2 0/6] qdev reset refactoring and pci bus reset List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Isaku Yamahata Cc: skandasa@cisco.com, etmartin@cisco.com, wexu2@cisco.com, qemu-devel@nongnu.org, kraxel@redhat.com, pbonzini@redhat.com On Wed, Nov 24, 2010 at 04:15:14PM +0900, Isaku Yamahata wrote: > On Wed, Nov 24, 2010 at 07:27:58AM +0200, Michael S. Tsirkin wrote: > > Right. So let's add an inline helper to avoid code duplication here? > > > > pci: fix bus walk under secondary bus reset > > > > Take into account secondary bus reset bit for > > bus walk: devices behind a reset bus should not > > respond to configuration cycles. > > > > Signed-off-by: Michael S. Tsirkin > > > > > > diff --git a/hw/pci.c b/hw/pci.c > > index d02f980..0c15b13 100644 > > --- a/hw/pci.c > > +++ b/hw/pci.c > > @@ -1540,6 +1540,16 @@ void pci_bridge_update_mappings(PCIBus *b) > > } > > } > > > > +/* Whether a given bus number is in range of the secondary > > + * bus of the given bridge device. */ > > +static bool pci_secondary_bus_in_range(PCIDevice *dev, int bus_num) > > +{ > > + return !(pci_get_word(dev->config + PCI_BRIDGE_CONTROL) & > > + PCI_BRIDGE_CTL_BUS_RESET) /* Don't walk the bus if it's reset. */ && > > + dev->config[PCI_SECONDARY_BUS] < bus_num && > > + bus_num <= dev->config[PCI_SUBORDINATE_BUS]; > > +} > > + > > PCIBus *pci_find_bus(PCIBus *bus, int bus_num) > > { > > PCIBus *sec; > > @@ -1552,20 +1562,21 @@ PCIBus *pci_find_bus(PCIBus *bus, int bus_num) > > return bus; > > } > > > > + /* Consider all bus numbers in range for the host pci bridge. */ > > + if (bus->parent_dev && > > + !pci_secondary_bus_in_range(bus->parent_dev, bus_num)) { > > + return NULL; > > + } > > + > > /* try child bus */ > > - if (!bus->parent_dev /* host pci bridge */ || > > - (bus->parent_dev->config[PCI_SECONDARY_BUS] < bus_num && > > - bus_num <= bus->parent_dev->config[PCI_SUBORDINATE_BUS])) { > > - for (; bus; bus = sec) { > > - QLIST_FOREACH(sec, &bus->child, sibling) { > > - assert(sec->parent_dev); > > - if (sec->parent_dev->config[PCI_SECONDARY_BUS] == bus_num) { > > - return sec; > > - } > > - if (sec->parent_dev->config[PCI_SECONDARY_BUS] < bus_num && > > - bus_num <= sec->parent_dev->config[PCI_SUBORDINATE_BUS]) { > > - break; > > - } > > + for (; bus; bus = sec) { > > + QLIST_FOREACH(sec, &bus->child, sibling) { > > + assert(sec->parent_dev); > > + if (sec->parent_dev->config[PCI_SECONDARY_BUS] == bus_num) { > > + return sec; > > + } > > + if (pci_secondary_bus_in_range(sec->parent_dev, bus_num)) { > > This condition should be "if (!pci_...)" Why? We are looking for a device on the given bus that claims the given bus_num. If we find one, break out of the inner loop and walk down to the child. > > > + break; > > } > > } > > } > > > > > -- > > > yamahata > > > > -- > yamahata