| Message ID | 1428053164-28277-9-git-send-email-wangyijing@huawei.com (mailing list archive) |
|---|---|
| State | New, archived |
| Delegated to: | Bjorn Helgaas |
| Headers | show |
On Fri, Apr 03, 2015 at 05:25:41PM +0800, Yijing Wang wrote: > If there is no busn resource provided for pci_scan_root_bus(), > we would insert a default bus resource (root_bus_number, 255) > in root bus, and update the max bus number we found after > pci_scan_child_bus(). We also need to hold the default bus > resource in pci_host_bridge, then we could identify whether > the new pci host bridge conflict with existing one. > > Signed-off-by: Yijing Wang <wangyijing@huawei.com> > --- > drivers/pci/host-bridge.c | 20 ++++++++++++++++++++ > drivers/pci/probe.c | 26 ++++++++++---------------- > include/linux/pci.h | 2 ++ > 3 files changed, 32 insertions(+), 16 deletions(-) > > diff --git a/drivers/pci/host-bridge.c b/drivers/pci/host-bridge.c > index 7d52a0a..ecc1a7c 100644 > --- a/drivers/pci/host-bridge.c > +++ b/drivers/pci/host-bridge.c > @@ -19,6 +19,25 @@ static void pci_release_host_bridge_dev(struct device *dev) > kfree(bridge); > } > > +static void pci_host_update_busn_res( > + struct pci_host_bridge *host, int bus, > + struct list_head *resources) > +{ > + struct resource_entry *window; > + > + resource_list_for_each_entry(window, resources) > + if (window->res->flags & IORESOURCE_BUS) > + return; > + > + pr_info( > + "No busn resource found for pci%04x:%02x, will use [bus %02x-ff]\n", > + host->domain, bus, bus); > + host->busn_res.flags = IORESOURCE_BUS; > + host->busn_res.start = bus; > + host->busn_res.end = 255; > + pci_add_resource(resources, &host->busn_res); > +} > + > struct pci_host_bridge *pci_create_host_bridge( > struct device *parent, int domain, int bus, > struct list_head *resources) > @@ -33,6 +52,7 @@ struct pci_host_bridge *pci_create_host_bridge( > > host->dev.parent = parent; > INIT_LIST_HEAD(&host->windows); > + pci_host_update_busn_res(host, bus, resources); > resource_list_for_each_entry_safe(window, n, resources) > list_move_tail(&window->node, &host->windows); > /* > diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c > index 9bc4784..d5a12d9 100644 > --- a/drivers/pci/probe.c > +++ b/drivers/pci/probe.c > @@ -2022,6 +2022,12 @@ int pci_bus_update_busn_res_end(struct pci_bus *b, int bus_max) > return ret; > } > > +static void pci_host_update_busn_res_end( > + struct pci_host_bridge *host, int max) > +{ > + host->busn_res.end = max; > +} > + > void pci_bus_release_busn_res(struct pci_bus *b) > { > struct resource *res = &b->busn_res; > @@ -2040,32 +2046,20 @@ static struct pci_bus *__pci_scan_root_bus(int bus, > struct pci_host_bridge *host, struct pci_ops *ops, > void *sysdata) > { > - struct resource_entry *window; > - bool found = false; > struct pci_bus *b; > int max; > > - resource_list_for_each_entry(window, &host->windows) > - if (window->res->flags & IORESOURCE_BUS) { > - found = true; > - break; > - } > - > b = __pci_create_root_bus(bus, host, ops, sysdata); > if (!b) > return NULL; > > - if (!found) { > - dev_info(&b->dev, > - "No busn resource found for root bus, will use [bus %02x-ff]\n", > - bus); > - pci_bus_insert_busn_res(b, bus, 255); > - } > - > max = pci_scan_child_bus(b); > > - if (!found) > + /* If default busn resource used, update the max bus number */ > + if (host->busn_res.flags & IORESOURCE_BUS) { > + pci_host_update_busn_res_end(host, max); > pci_bus_update_busn_res_end(b, max); > + } > > return b; > } > diff --git a/include/linux/pci.h b/include/linux/pci.h > index 1542df8..f189dfb 100644 > --- a/include/linux/pci.h > +++ b/include/linux/pci.h > @@ -404,6 +404,8 @@ struct pci_host_bridge { > int domain; > struct device dev; > struct pci_bus *bus; /* root bus */ > + /* we use default bus resource if no bus resource provided */ > + struct resource busn_res; I don't understand the need for another busn_res here. The host bridge bus range should be identical to the root bus range. Having two copies will confuse things. And apparently this host->busn_res is only filled in if the arch doesn't provide a busn resource? To check for conflicts between host bridges, can you iterate through the existing ones and check the range of their root buses? > struct list_head windows; /* resource_entry */ > void (*release_fn)(struct pci_host_bridge *); > void *release_data; > -- > 1.7.1 > -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
>> diff --git a/include/linux/pci.h b/include/linux/pci.h >> index 1542df8..f189dfb 100644 >> --- a/include/linux/pci.h >> +++ b/include/linux/pci.h >> @@ -404,6 +404,8 @@ struct pci_host_bridge { >> int domain; >> struct device dev; >> struct pci_bus *bus; /* root bus */ >> + /* we use default bus resource if no bus resource provided */ >> + struct resource busn_res; > > I don't understand the need for another busn_res here. The host bridge bus > range should be identical to the root bus range. Having two copies will > confuse things. > > And apparently this host->busn_res is only filled in if the arch doesn't > provide a busn resource? > > To check for conflicts between host bridges, can you iterate through the > existing ones and check the range of their root buses? Checking root buses is not enough, there is time gap between pci_host_bridge creation and pci root bus creation. E.g. A and B do pci scan sync in two cpus, A first check the buses resource, and find free bus resource, then it go to create pci_host_bridge and scan root bus, at the same time, B also check the buses resource, because A has not created root bus, B also think there are free buses, this is a problem. I agree you that having two copies is not a good idea. For pci bus resource, we have the following request path: 1. arch supplies the busn_res or PCI core add the default bus resource(in pci_scan_bus); 2. Call pci_bus_insert_busn_res() to insert the bus resource for root bus. 3. Call get_pci_domain_busn_res() to return the per-domain bus resource. 4. Request root bus resource under per-domain bus resource. We have the per-domain structure pci_domain_busn_res to manage the bus resources in one domain, so what about add a bitmap to manage the free bus resource ? In pci_create_host_bridge(), first check whether the new pci_host_bridge bus resources required is free, if yes, we set the bitmap to occupy the bus resource at once, then create the host bridge and add it to the global list. struct pci_domain_busn_res { struct list_head list; struct resource res; int domain_nr; bitmap used; }; Another problem, I found it's unnecessary to add bus resource to pci_host_bridge->windows. Unlike the IO and MEM resource, we never use the pci_host_bridge bus resource again after pci enumeration. I think we could clean it up, or in some cases, when arch supplies the bus resource, we still have the two copies. Thanks! Yijing. > >> struct list_head windows; /* resource_entry */ >> void (*release_fn)(struct pci_host_bridge *); >> void *release_data; >> -- >> 1.7.1 >> > > . >
diff --git a/drivers/pci/host-bridge.c b/drivers/pci/host-bridge.c index 7d52a0a..ecc1a7c 100644 --- a/drivers/pci/host-bridge.c +++ b/drivers/pci/host-bridge.c @@ -19,6 +19,25 @@ static void pci_release_host_bridge_dev(struct device *dev) kfree(bridge); } +static void pci_host_update_busn_res( + struct pci_host_bridge *host, int bus, + struct list_head *resources) +{ + struct resource_entry *window; + + resource_list_for_each_entry(window, resources) + if (window->res->flags & IORESOURCE_BUS) + return; + + pr_info( + "No busn resource found for pci%04x:%02x, will use [bus %02x-ff]\n", + host->domain, bus, bus); + host->busn_res.flags = IORESOURCE_BUS; + host->busn_res.start = bus; + host->busn_res.end = 255; + pci_add_resource(resources, &host->busn_res); +} + struct pci_host_bridge *pci_create_host_bridge( struct device *parent, int domain, int bus, struct list_head *resources) @@ -33,6 +52,7 @@ struct pci_host_bridge *pci_create_host_bridge( host->dev.parent = parent; INIT_LIST_HEAD(&host->windows); + pci_host_update_busn_res(host, bus, resources); resource_list_for_each_entry_safe(window, n, resources) list_move_tail(&window->node, &host->windows); /* diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c index 9bc4784..d5a12d9 100644 --- a/drivers/pci/probe.c +++ b/drivers/pci/probe.c @@ -2022,6 +2022,12 @@ int pci_bus_update_busn_res_end(struct pci_bus *b, int bus_max) return ret; } +static void pci_host_update_busn_res_end( + struct pci_host_bridge *host, int max) +{ + host->busn_res.end = max; +} + void pci_bus_release_busn_res(struct pci_bus *b) { struct resource *res = &b->busn_res; @@ -2040,32 +2046,20 @@ static struct pci_bus *__pci_scan_root_bus(int bus, struct pci_host_bridge *host, struct pci_ops *ops, void *sysdata) { - struct resource_entry *window; - bool found = false; struct pci_bus *b; int max; - resource_list_for_each_entry(window, &host->windows) - if (window->res->flags & IORESOURCE_BUS) { - found = true; - break; - } - b = __pci_create_root_bus(bus, host, ops, sysdata); if (!b) return NULL; - if (!found) { - dev_info(&b->dev, - "No busn resource found for root bus, will use [bus %02x-ff]\n", - bus); - pci_bus_insert_busn_res(b, bus, 255); - } - max = pci_scan_child_bus(b); - if (!found) + /* If default busn resource used, update the max bus number */ + if (host->busn_res.flags & IORESOURCE_BUS) { + pci_host_update_busn_res_end(host, max); pci_bus_update_busn_res_end(b, max); + } return b; } diff --git a/include/linux/pci.h b/include/linux/pci.h index 1542df8..f189dfb 100644 --- a/include/linux/pci.h +++ b/include/linux/pci.h @@ -404,6 +404,8 @@ struct pci_host_bridge { int domain; struct device dev; struct pci_bus *bus; /* root bus */ + /* we use default bus resource if no bus resource provided */ + struct resource busn_res; struct list_head windows; /* resource_entry */ void (*release_fn)(struct pci_host_bridge *); void *release_data;
If there is no busn resource provided for pci_scan_root_bus(), we would insert a default bus resource (root_bus_number, 255) in root bus, and update the max bus number we found after pci_scan_child_bus(). We also need to hold the default bus resource in pci_host_bridge, then we could identify whether the new pci host bridge conflict with existing one. Signed-off-by: Yijing Wang <wangyijing@huawei.com> --- drivers/pci/host-bridge.c | 20 ++++++++++++++++++++ drivers/pci/probe.c | 26 ++++++++++---------------- include/linux/pci.h | 2 ++ 3 files changed, 32 insertions(+), 16 deletions(-)