* [Qemu-devel] [PATCH] pci: fix pci_find_bus(). @ 2010-04-09 10:13 Isaku Yamahata 2010-04-09 23:48 ` [Qemu-devel] " Isaku Yamahata 2010-04-11 10:46 ` [Qemu-devel] Re: [PATCH] " Michael S. Tsirkin 0 siblings, 2 replies; 6+ messages in thread From: Isaku Yamahata @ 2010-04-09 10:13 UTC (permalink / raw) To: qemu-devel; +Cc: Blue Swirl, Michael S. Tsirkin When looking down child bus, it looked parent bridge's bus number. It should look child bridge's. Cc: Blue Swirl <blauwirbel@gmail.com> Cc: "Michael S. Tsirkin" <mst@redhat.com> Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp> --- hw/pci.c | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/hw/pci.c b/hw/pci.c index 0dbca17..2f6907b 100644 --- a/hw/pci.c +++ b/hw/pci.c @@ -1557,9 +1557,9 @@ PCIBus *pci_find_bus(PCIBus *bus, int bus_num) /* try child bus */ QLIST_FOREACH(sec, &bus->child, sibling) { - if (!bus->parent_dev /* pci host bridge */ + if (!sec->parent_dev /* pci host bridge */ || (pci_bus_num(sec) <= bus_num && - bus_num <= bus->parent_dev->config[PCI_SUBORDINATE_BUS]) ) { + bus_num <= sec->parent_dev->config[PCI_SUBORDINATE_BUS]) ) { ret = pci_find_bus(sec, bus_num); if (ret) { return ret; -- 1.6.6.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [Qemu-devel] Re: [PATCH] pci: fix pci_find_bus(). 2010-04-09 10:13 [Qemu-devel] [PATCH] pci: fix pci_find_bus() Isaku Yamahata @ 2010-04-09 23:48 ` Isaku Yamahata 2010-04-11 10:51 ` Michael S. Tsirkin 2010-04-11 10:46 ` [Qemu-devel] Re: [PATCH] " Michael S. Tsirkin 1 sibling, 1 reply; 6+ messages in thread From: Isaku Yamahata @ 2010-04-09 23:48 UTC (permalink / raw) To: qemu-devel; +Cc: Blue Swirl, Michael S. Tsirkin Oh I sent out the wrong one. This is the correct one. Sorry for noise. >From 370c23c837070f78ca5715f032eacbf6747d42fb Mon Sep 17 00:00:00 2001 Message-Id: <370c23c837070f78ca5715f032eacbf6747d42fb.1270856742.git.yamahata@valinux.co.jp> In-Reply-To: <cover.1270856742.git.yamahata@valinux.co.jp> References: <cover.1270856742.git.yamahata@valinux.co.jp> From: Isaku Yamahata <yamahata@valinux.co.jp> Date: Fri, 9 Apr 2010 19:09:35 +0900 Subject: [PATCH] pci: fix pci_find_bus(). When looking down child bus, it should look parent bridge's bus number, not child bus's. Cc: Blue Swirl <blauwirbel@gmail.com> Cc: "Michael S. Tsirkin" <mst@redhat.com> Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp> --- hw/pci.c | 8 ++++---- 1 files changed, 4 insertions(+), 4 deletions(-) diff --git a/hw/pci.c b/hw/pci.c index 0dbca17..b6e6bbe 100644 --- a/hw/pci.c +++ b/hw/pci.c @@ -1556,10 +1556,10 @@ PCIBus *pci_find_bus(PCIBus *bus, int bus_num) } /* try child bus */ - QLIST_FOREACH(sec, &bus->child, sibling) { - if (!bus->parent_dev /* pci host bridge */ - || (pci_bus_num(sec) <= bus_num && - bus_num <= bus->parent_dev->config[PCI_SUBORDINATE_BUS]) ) { + if (!bus->parent_dev /* pci host bridge */ + || (pci_bus_num(bus) <= bus_num && + bus_num <= bus->parent_dev->config[PCI_SUBORDINATE_BUS])) { + QLIST_FOREACH(sec, &bus->child, sibling) { ret = pci_find_bus(sec, bus_num); if (ret) { return ret; -- 1.6.6.1 -- yamahata ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [Qemu-devel] Re: [PATCH] pci: fix pci_find_bus(). 2010-04-09 23:48 ` [Qemu-devel] " Isaku Yamahata @ 2010-04-11 10:51 ` Michael S. Tsirkin 2010-04-12 2:58 ` [Qemu-devel] [PATCH v2] " Isaku Yamahata 0 siblings, 1 reply; 6+ messages in thread From: Michael S. Tsirkin @ 2010-04-11 10:51 UTC (permalink / raw) To: Isaku Yamahata; +Cc: Blue Swirl, qemu-devel On Sat, Apr 10, 2010 at 08:48:35AM +0900, Isaku Yamahata wrote: > Oh I sent out the wrong one. This is the correct one. > Sorry for noise. > > >From 370c23c837070f78ca5715f032eacbf6747d42fb Mon Sep 17 00:00:00 2001 > Message-Id: <370c23c837070f78ca5715f032eacbf6747d42fb.1270856742.git.yamahata@valinux.co.jp> > In-Reply-To: <cover.1270856742.git.yamahata@valinux.co.jp> > References: <cover.1270856742.git.yamahata@valinux.co.jp> > From: Isaku Yamahata <yamahata@valinux.co.jp> > Date: Fri, 9 Apr 2010 19:09:35 +0900 > Subject: [PATCH] pci: fix pci_find_bus(). > > When looking down child bus, it should look parent bridge's > bus number, not child bus's. > > Cc: Blue Swirl <blauwirbel@gmail.com> > Cc: "Michael S. Tsirkin" <mst@redhat.com> > Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp> > --- > hw/pci.c | 8 ++++---- > 1 files changed, 4 insertions(+), 4 deletions(-) > > diff --git a/hw/pci.c b/hw/pci.c > index 0dbca17..b6e6bbe 100644 > --- a/hw/pci.c > +++ b/hw/pci.c > @@ -1556,10 +1556,10 @@ PCIBus *pci_find_bus(PCIBus *bus, int bus_num) > } > > /* try child bus */ > - QLIST_FOREACH(sec, &bus->child, sibling) { > - if (!bus->parent_dev /* pci host bridge */ > - || (pci_bus_num(sec) <= bus_num && > - bus_num <= bus->parent_dev->config[PCI_SUBORDINATE_BUS]) ) { > + if (!bus->parent_dev /* pci host bridge */ > + || (pci_bus_num(bus) <= bus_num && Better use PCI_SECONDARY_BUS here directly, we know parent_dev is non-NULL. > + bus_num <= bus->parent_dev->config[PCI_SUBORDINATE_BUS])) { > + QLIST_FOREACH(sec, &bus->child, sibling) { > ret = pci_find_bus(sec, bus_num); > if (ret) { > return ret; What do you think about converting code to loop as I suggested earlier? > -- > 1.6.6.1 > > > -- > yamahata ^ permalink raw reply [flat|nested] 6+ messages in thread
* [Qemu-devel] [PATCH v2] pci: fix pci_find_bus(). 2010-04-11 10:51 ` Michael S. Tsirkin @ 2010-04-12 2:58 ` Isaku Yamahata 2010-04-20 17:19 ` [Qemu-devel] " Blue Swirl 0 siblings, 1 reply; 6+ messages in thread From: Isaku Yamahata @ 2010-04-12 2:58 UTC (permalink / raw) To: Michael S. Tsirkin; +Cc: Blue Swirl, qemu-devel When looking down child bus, it should look parent bridge's bus number, not child bus's. Optimized tail recursion and style fix. Cc: Blue Swirl <blauwirbel@gmail.com> Cc: "Michael S. Tsirkin" <mst@redhat.com> Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp> --- hw/pci.c | 25 ++++++++++++++++--------- 1 files changed, 16 insertions(+), 9 deletions(-) diff --git a/hw/pci.c b/hw/pci.c index 2355232..6c0cc7b 100644 --- a/hw/pci.c +++ b/hw/pci.c @@ -1546,23 +1546,30 @@ static void pci_bridge_write_config(PCIDevice *d, PCIBus *pci_find_bus(PCIBus *bus, int bus_num) { - PCIBus *sec, *ret; + PCIBus *sec; - if (!bus) + if (!bus) { return NULL; + } if (pci_bus_num(bus) == bus_num) { return bus; } /* try child bus */ - QLIST_FOREACH(sec, &bus->child, sibling) { - if (!bus->parent_dev /* pci host bridge */ - || (pci_bus_num(sec) <= bus_num && - bus_num <= bus->parent_dev->config[PCI_SUBORDINATE_BUS]) ) { - ret = pci_find_bus(sec, bus_num); - if (ret) { - return ret; + 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; + } } } } -- 1.6.6.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [Qemu-devel] Re: [PATCH v2] pci: fix pci_find_bus(). 2010-04-12 2:58 ` [Qemu-devel] [PATCH v2] " Isaku Yamahata @ 2010-04-20 17:19 ` Blue Swirl 0 siblings, 0 replies; 6+ messages in thread From: Blue Swirl @ 2010-04-20 17:19 UTC (permalink / raw) To: Isaku Yamahata; +Cc: qemu-devel, Michael S. Tsirkin Thanks, applied. On 4/12/10, Isaku Yamahata <yamahata@valinux.co.jp> wrote: > When looking down child bus, it should look parent bridge's > bus number, not child bus's. > > Optimized tail recursion and style fix. > > > Cc: Blue Swirl <blauwirbel@gmail.com> > Cc: "Michael S. Tsirkin" <mst@redhat.com> > Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp> > --- > > hw/pci.c | 25 ++++++++++++++++--------- > 1 files changed, 16 insertions(+), 9 deletions(-) > > > diff --git a/hw/pci.c b/hw/pci.c > > index 2355232..6c0cc7b 100644 > > --- a/hw/pci.c > +++ b/hw/pci.c > > @@ -1546,23 +1546,30 @@ static void pci_bridge_write_config(PCIDevice *d, > > > PCIBus *pci_find_bus(PCIBus *bus, int bus_num) > > { > - PCIBus *sec, *ret; > + PCIBus *sec; > > > - if (!bus) > + if (!bus) { > return NULL; > + } > > > if (pci_bus_num(bus) == bus_num) { > return bus; > > } > > /* try child bus */ > - QLIST_FOREACH(sec, &bus->child, sibling) { > - if (!bus->parent_dev /* pci host bridge */ > - || (pci_bus_num(sec) <= bus_num && > - bus_num <= bus->parent_dev->config[PCI_SUBORDINATE_BUS]) ) { > > - ret = pci_find_bus(sec, bus_num); > - if (ret) { > - return ret; > + 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; > + } > } > } > } > > -- > 1.6.6.1 > > > ^ permalink raw reply [flat|nested] 6+ messages in thread
* [Qemu-devel] Re: [PATCH] pci: fix pci_find_bus(). 2010-04-09 10:13 [Qemu-devel] [PATCH] pci: fix pci_find_bus() Isaku Yamahata 2010-04-09 23:48 ` [Qemu-devel] " Isaku Yamahata @ 2010-04-11 10:46 ` Michael S. Tsirkin 1 sibling, 0 replies; 6+ messages in thread From: Michael S. Tsirkin @ 2010-04-11 10:46 UTC (permalink / raw) To: Isaku Yamahata; +Cc: Blue Swirl, qemu-devel On Fri, Apr 09, 2010 at 07:13:24PM +0900, Isaku Yamahata wrote: > When looking down child bus, it looked parent bridge's > bus number. > It should look child bridge's. > > Cc: Blue Swirl <blauwirbel@gmail.com> > Cc: "Michael S. Tsirkin" <mst@redhat.com> > Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp> > --- > hw/pci.c | 4 ++-- > 1 files changed, 2 insertions(+), 2 deletions(-) > > diff --git a/hw/pci.c b/hw/pci.c > index 0dbca17..2f6907b 100644 > --- a/hw/pci.c > +++ b/hw/pci.c > @@ -1557,9 +1557,9 @@ PCIBus *pci_find_bus(PCIBus *bus, int bus_num) > > /* try child bus */ > QLIST_FOREACH(sec, &bus->child, sibling) { > - if (!bus->parent_dev /* pci host bridge */ > + if (!sec->parent_dev /* pci host bridge */ I don't understand this first test. As far as I can tell secondary bus must always have a device, as set by pci_register_secondary_bus. Should this rather be assert(sec->parent_dev)? > || (pci_bus_num(sec) <= bus_num && And so the above should just use sec->parent_dev->config[PCI_SECONDARY_BUS] instead of a wrapper that tests sec->parent_dev. > - bus_num <= bus->parent_dev->config[PCI_SUBORDINATE_BUS]) ) { > + bus_num <= sec->parent_dev->config[PCI_SUBORDINATE_BUS]) ) { > ret = pci_find_bus(sec, bus_num); > if (ret) { I think that in the above, we can just as well do return pci_find_bus() - if multiple children claim the same bus range, on real hardware only one of them will claim the transaction. > return ret; I find the use of recursion here confusing. Since as pointed out above this can be a tail recursion, it can easily be converted to a loop. How does the below look, for example? Note: completely untested, likely broken: diff --git a/hw/pci.c b/hw/pci.c index b6abd67..e9d6def 100644 --- a/hw/pci.c +++ b/hw/pci.c @@ -1546,26 +1546,33 @@ static void pci_bridge_write_config(PCIDevice *d, PCIBus *pci_find_bus(PCIBus *bus, int bus_num) { - PCIBus *sec, *ret; + PCIBus *sec; + bool found; - if (!bus) + if (!bus) { return NULL; + } if (pci_bus_num(bus) == bus_num) { return bus; } /* try child bus */ - QLIST_FOREACH(sec, &bus->child, sibling) { - if (!bus->parent_dev /* pci host bridge */ - || (pci_bus_num(sec) <= bus_num && - bus_num <= bus->parent_dev->config[PCI_SUBORDINATE_BUS]) ) { - ret = pci_find_bus(sec, bus_num); - if (ret) { - return ret; + do { + found = false; + 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]) { + bus = sec; + found = true; + break; } } - } + } while (found); return NULL; } > -- > 1.6.6.1 > ^ permalink raw reply related [flat|nested] 6+ messages in thread
end of thread, other threads:[~2010-04-20 17:19 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2010-04-09 10:13 [Qemu-devel] [PATCH] pci: fix pci_find_bus() Isaku Yamahata 2010-04-09 23:48 ` [Qemu-devel] " Isaku Yamahata 2010-04-11 10:51 ` Michael S. Tsirkin 2010-04-12 2:58 ` [Qemu-devel] [PATCH v2] " Isaku Yamahata 2010-04-20 17:19 ` [Qemu-devel] " Blue Swirl 2010-04-11 10:46 ` [Qemu-devel] Re: [PATCH] " Michael S. Tsirkin
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).