* [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 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
* [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
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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.