From 9a4d7d87197e3ff9138981e196aa5021d13a51a8 Mon Sep 17 00:00:00 2001 From: Andreas Noever Date: Thu, 23 Jan 2014 21:59:21 +0100 Subject: PCI: Increment max correctly in pci_scan_bridge() This patch fixes two small issues: - If pci_add_new_bus() fails, max must not be incremented. Otherwise an incorrect value is returned from pci_scan_bridge(). - If the bus is already present, max must be incremented. I think that this case should only be hit if we trigger a manual rescan of a CardBus bridge. Signed-off-by: Andreas Noever Signed-off-by: Bjorn Helgaas --- drivers/pci/probe.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c index 6e34498ec9f0..f340c947d8cb 100644 --- a/drivers/pci/probe.c +++ b/drivers/pci/probe.c @@ -852,11 +852,12 @@ int pci_scan_bridge(struct pci_bus *bus, struct pci_dev *dev, int max, int pass) * this case we only re-scan this bus. */ child = pci_find_bus(pci_domain_nr(bus), max+1); if (!child) { - child = pci_add_new_bus(bus, dev, ++max); + child = pci_add_new_bus(bus, dev, max+1); if (!child) goto out; - pci_bus_insert_busn_res(child, max, 0xff); + pci_bus_insert_busn_res(child, max+1, 0xff); } + max++; buses = (buses & 0xff000000) | ((unsigned int)(child->primary) << 0) | ((unsigned int)(child->busn_res.start) << 8) -- cgit v1.2.2 From 2ed8582341f651ca14d00ab0ada4b46f493e1fcb Mon Sep 17 00:00:00 2001 From: Andreas Noever Date: Thu, 23 Jan 2014 21:59:22 +0100 Subject: PCI: Clarify the "scan anyway" comment in pci_scan_bridge() Initially when we encountered a bus that was already present we skipped it. Since 74710ded8e16 'PCI: always scan child buses' we continue scanning in order to allow user triggered rescans of already existing busses. The old comment suggested that the reason for continuing the scan is a bug in the i450NX chipset. This is not the case. Signed-off-by: Andreas Noever Signed-off-by: Bjorn Helgaas --- drivers/pci/probe.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c index f340c947d8cb..511a8f6d7636 100644 --- a/drivers/pci/probe.c +++ b/drivers/pci/probe.c @@ -805,11 +805,10 @@ int pci_scan_bridge(struct pci_bus *bus, struct pci_dev *dev, int max, int pass) goto out; /* - * If we already got to this bus through a different bridge, - * don't re-add it. This can happen with the i450NX chipset. - * - * However, we continue to descend down the hierarchy and - * scan remaining child buses. + * The bus might already exist for two reasons: Either we are + * rescanning the bus or the bus is reachable through more than + * one bridge. The second case can happen with the i450NX + * chipset. */ child = pci_find_bus(pci_domain_nr(bus), secondary); if (!child) { -- cgit v1.2.2 From 619c8c310f7f21e59a7e2b53795183c34401a599 Mon Sep 17 00:00:00 2001 From: Andreas Noever Date: Thu, 23 Jan 2014 21:59:23 +0100 Subject: PCI: Assign CardBus bus number only during the second pass Right now the CardBus code in pci_scan_bridge() is executed during both passes. Since we always allocate the bus number ourselves it makes sense to put it into the second pass. Signed-off-by: Andreas Noever Signed-off-by: Bjorn Helgaas --- drivers/pci/probe.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c index 511a8f6d7636..5dc8e1a31e4f 100644 --- a/drivers/pci/probe.c +++ b/drivers/pci/probe.c @@ -831,7 +831,7 @@ int pci_scan_bridge(struct pci_bus *bus, struct pci_dev *dev, int max, int pass) * do in the second pass. */ if (!pass) { - if (pcibios_assign_all_busses() || broken) + if (pcibios_assign_all_busses() || broken || is_cardbus) /* Temporarily disable forwarding of the configuration cycles on all bridges in this bus segment to avoid possible -- cgit v1.2.2 From ced04d15519a15d38b46162b94a1f26b4022116e Mon Sep 17 00:00:00 2001 From: Andreas Noever Date: Thu, 23 Jan 2014 21:59:24 +0100 Subject: PCI: Use request_resource_conflict() instead of insert_ for bus numbers If a conflict happens during insert_resource_conflict() and all conflicts fit within the newly inserted resource then they will become children of the new resource. This is almost certainly not what we want for bus numbers. Signed-off-by: Andreas Noever Signed-off-by: Bjorn Helgaas --- drivers/pci/probe.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c index 5dc8e1a31e4f..21f162c5c7bb 100644 --- a/drivers/pci/probe.c +++ b/drivers/pci/probe.c @@ -1835,7 +1835,7 @@ int pci_bus_insert_busn_res(struct pci_bus *b, int bus, int bus_max) res->flags |= IORESOURCE_PCI_FIXED; } - conflict = insert_resource_conflict(parent_res, res); + conflict = request_resource_conflict(parent_res, res); if (conflict) dev_printk(KERN_DEBUG, &b->dev, -- cgit v1.2.2 From 1820ffdccb9b4398c5f0f70360edc68e039c3c72 Mon Sep 17 00:00:00 2001 From: Andreas Noever Date: Thu, 23 Jan 2014 21:59:25 +0100 Subject: PCI: Make sure bus number resources stay within their parents bounds Right now we use 0xff for busn_res.end when probing and later reduce it to the value that is actually used. This does not work if a parent bridge has already a lower subordinate value. For example during hotplug of a new bridge below an already-configured bridge the following message is printed from pci_bus_insert_busn_res(): pci_bus 0000:06: busn_res: can not insert [bus 06-ff] under [bus 05-9b] (conflicts with (null) [bus 05-9b]) This patch clamps the bus range to that of the parent and also ensures that we do not exceed the parents range when assigning the final subordinate value. We also check that busses configured by the firmware fit into their parents bounds. [bhelgaas: reword dev_warn() and fix printk format warning] Signed-off-by: Andreas Noever Signed-off-by: Bjorn Helgaas --- drivers/pci/probe.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c index 21f162c5c7bb..9a641cc4275d 100644 --- a/drivers/pci/probe.c +++ b/drivers/pci/probe.c @@ -782,7 +782,7 @@ int pci_scan_bridge(struct pci_bus *bus, struct pci_dev *dev, int max, int pass) /* Check if setup is sensible at all */ if (!pass && (primary != bus->number || secondary <= bus->number || - secondary > subordinate)) { + secondary > subordinate || subordinate > bus->busn_res.end)) { dev_info(&dev->dev, "bridge configuration invalid ([bus %02x-%02x]), reconfiguring\n", secondary, subordinate); broken = 1; @@ -854,7 +854,8 @@ int pci_scan_bridge(struct pci_bus *bus, struct pci_dev *dev, int max, int pass) child = pci_add_new_bus(bus, dev, max+1); if (!child) goto out; - pci_bus_insert_busn_res(child, max+1, 0xff); + pci_bus_insert_busn_res(child, max+1, + bus->busn_res.end); } max++; buses = (buses & 0xff000000) @@ -927,6 +928,11 @@ int pci_scan_bridge(struct pci_bus *bus, struct pci_dev *dev, int max, int pass) /* * Set the subordinate bus number to its real value. */ + if (max > bus->busn_res.end) { + dev_warn(&dev->dev, "max busn %02x is outside %pR\n", + max, &bus->busn_res); + max = bus->busn_res.end; + } pci_bus_update_busn_res_end(child, max); pci_write_config_byte(dev, PCI_SUBORDINATE_BUS, max); } -- cgit v1.2.2 From f5fb40700fc9a52944fbe07148c858a5025908b1 Mon Sep 17 00:00:00 2001 From: Andreas Noever Date: Thu, 23 Jan 2014 21:59:26 +0100 Subject: PCI: Remove pci_fixup_parent_subordinate_busnr() The function has no effect. If pcibios_assign_all_busses() is not set then the function does nothing. If it is set then in pci_scan_bridge we are always in the branch where we assign the bus numbers ourselves and the subordinate values of all parent busses will be set to 0xff since that is what they inherited from their parent bus and ultimately from the root bus. Signed-off-by: Andreas Noever Signed-off-by: Bjorn Helgaas --- drivers/pci/probe.c | 30 ------------------------------ 1 file changed, 30 deletions(-) diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c index 9a641cc4275d..e5df03669470 100644 --- a/drivers/pci/probe.c +++ b/drivers/pci/probe.c @@ -731,22 +731,6 @@ struct pci_bus *__ref pci_add_new_bus(struct pci_bus *parent, struct pci_dev *de return child; } -static void pci_fixup_parent_subordinate_busnr(struct pci_bus *child, int max) -{ - struct pci_bus *parent = child->parent; - - /* Attempts to fix that up are really dangerous unless - we're going to re-assign all bus numbers. */ - if (!pcibios_assign_all_busses()) - return; - - while (parent->parent && parent->busn_res.end < max) { - parent->busn_res.end = max; - pci_write_config_byte(parent->self, PCI_SUBORDINATE_BUS, max); - parent = parent->parent; - } -} - /* * If it's a bridge, configure it and scan the bus behind it. * For CardBus bridges, we don't scan behind as the devices will @@ -879,20 +863,7 @@ int pci_scan_bridge(struct pci_bus *bus, struct pci_dev *dev, int max, int pass) if (!is_cardbus) { child->bridge_ctl = bctl; - /* - * Adjust subordinate busnr in parent buses. - * We do this before scanning for children because - * some devices may not be detected if the bios - * was lazy. - */ - pci_fixup_parent_subordinate_busnr(child, max); - /* Now we can scan all subordinate buses... */ max = pci_scan_child_bus(child); - /* - * now fix it up again since we have found - * the real value of max. - */ - pci_fixup_parent_subordinate_busnr(child, max); } else { /* * For CardBus bridges, we leave 4 bus numbers @@ -923,7 +894,6 @@ int pci_scan_bridge(struct pci_bus *bus, struct pci_dev *dev, int max, int pass) } } max += i; - pci_fixup_parent_subordinate_busnr(child, max); } /* * Set the subordinate bus number to its real value. -- cgit v1.2.2 From c95b0bd6ca3dbb1abf8394c38d26df65d890cb9a Mon Sep 17 00:00:00 2001 From: Andreas Noever Date: Thu, 23 Jan 2014 21:59:27 +0100 Subject: PCI: Check for child busses which use more bus numbers than allocated pci_scan_child_bus can (potentially) return a bus number higher than the subordinate value of the child bus. Possible reasons are that bus numbers are reserved for SR-IOV or for CardBus (SR-IOV is done without checks and the CardBus checks are sketchy at best). We clamp the returned value to the actual subordinate value and print a warning if too many bus numbers are reserved. [bhelgaas: whitespace] Signed-off-by: Andreas Noever Signed-off-by: Bjorn Helgaas --- drivers/pci/probe.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c index e5df03669470..1436288924c3 100644 --- a/drivers/pci/probe.c +++ b/drivers/pci/probe.c @@ -805,10 +805,12 @@ int pci_scan_bridge(struct pci_bus *bus, struct pci_dev *dev, int max, int pass) } cmax = pci_scan_child_bus(child); - if (cmax > max) - max = cmax; - if (child->busn_res.end > max) - max = child->busn_res.end; + if (cmax > subordinate) + dev_warn(&dev->dev, "bridge has subordinate %02x but max busn %02x\n", + subordinate, cmax); + /* subordinate should equal child->busn_res.end */ + if (subordinate > max) + max = subordinate; } else { /* * We need to assign a number to this bus which we always -- cgit v1.2.2 From fc1b253141b360f9c669d391b4ff663b984ef0c9 Mon Sep 17 00:00:00 2001 From: Andreas Noever Date: Thu, 23 Jan 2014 21:59:28 +0100 Subject: PCI: Don't scan random busses in pci_scan_bridge() When assigning a new bus number in pci_scan_bridge we check whether max+1 is free by calling pci_find_bus. If it does already exist then we assume that we are rescanning and that this is the right bus to scan. This is fragile. If max+1 lies outside of bus->busn_res.end then we will rescan some random bus from somewhere else in the hierachy. This patch checks for this case and prints a warning. [bhelgaas: add parent/child bus number info to dev_warn()] Signed-off-by: Andreas Noever Signed-off-by: Bjorn Helgaas --- drivers/pci/probe.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c index 1436288924c3..509494381a7a 100644 --- a/drivers/pci/probe.c +++ b/drivers/pci/probe.c @@ -829,12 +829,16 @@ int pci_scan_bridge(struct pci_bus *bus, struct pci_dev *dev, int max, int pass) goto out; } + if (max >= bus->busn_res.end) { + dev_warn(&dev->dev, "can't allocate child bus %02x from %pR\n", + max, &bus->busn_res); + goto out; + } + /* Clear errors */ pci_write_config_word(dev, PCI_STATUS, 0xffff); - /* Prevent assigning a bus number that already exists. - * This can happen when a bridge is hot-plugged, so in - * this case we only re-scan this bus. */ + /* The bus will already exist if we are rescanning */ child = pci_find_bus(pci_domain_nr(bus), max+1); if (!child) { child = pci_add_new_bus(bus, dev, max+1); -- cgit v1.2.2