On Mon, Sep 15, 2014 at 12:10:27AM +0200, Andreas Noever wrote:
>[+cc Thomas, your regression has probably the same cause]
>
>This looks a like it is going to be a little more complicated than anticipated.
>
>pci_scan_bus has two main branches. One for "broken" hardware and cardbus and
>another one for bridges whose configuration looks sane.
>
>David's working 3.2 Kernel does the following:
>pci 0000:00:1e.0: PCI bridge to [bus 01-01] (subtractive decode)
>pci 0000:01:01.0: bridge configuration invalid ([bus 00-00]), reconfiguring
>
>The 00:1e bridge is deemed sane and the 01:01 (cardbus) bridge below it is
>deemed broken. pci_scan_bridge than assigns bus [2-5] to 01:01 without touching
>1e (so we just got 4 imaginary busses). We just print a warning:
>pci_bus 0000:02: [bus 02-05] partially hidden behind transparent bridge 0000:01 [bus 01-01]
>
>After returning to 00:1e (in the sane branch) we also do not update our
>subordinate and just return. Later yenta_socket sees that this is nuts and
>carefully increases the subordinate of 00:1e.
>
>My patch series for 3.15 was trying to make sure that pci_scan_bus does not
>overstep its resources. So now we now refuse to create bus 2 under [01-01]
>(fc1b2531 PCI: Don't scan random busses in pci_scan_bridge()) and a little
>later we would forbid the cardbus code in pci_scan_bridge from reserving 4 more
>busses (1820ffdc PCI: Make sure bus number resources stay within their parents
>bounds). At least these two would need to be reverted.
>
>As an alternative the following patch tries grow the bus window, if necessary
>by growing its parents bus window (recursively). This should make the yenta
>fixup unnecessary. I have tested the patch with normal pci bridges + setpci,
>since I don't have access to a cardbus system.
>
>So if you have time please test this and/or try to revert the two mentioned
>commits.
>
>Thanks,
>Andreas
>---
> drivers/pci/probe.c | 45 ++++++++++++++++++++++++++++++++++++---------
> 1 file changed, 36 insertions(+), 9 deletions(-)
>
>diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
>index e3cf8a2..81dd668 100644
>--- a/drivers/pci/probe.c
>+++ b/drivers/pci/probe.c
>@@ -740,6 +740,30 @@ struct pci_bus *pci_add_new_bus(struct pci_bus *parent, struct pci_dev *dev,
> }
> EXPORT_SYMBOL(pci_add_new_bus);
>
>+int pci_grow_bus(struct pci_bus *bus, int bus_max)
>+{
>+ struct resource *res = &bus->busn_res;
>+ struct resource old_res = *res;
>+ int ret;
>+ if (res->end >= bus_max)
>+ return 0;
>+ if (!bus->self || pci_is_root_bus(bus)) {
>+ dev_printk(KERN_DEBUG, &bus->dev,
>+ "root busn_res %pR cannot grow\n", &res);
>+ return -EBUSY;
>+ }
>+ ret = pci_grow_bus(bus->parent, bus_max);
>+ if (ret)
>+ return ret;
>+ ret = adjust_resource(res, res->start, bus_max - res->start + 1);
>+ dev_printk(KERN_DEBUG, &bus->dev,
>+ "busn_res: %pR end %s grown to %02x\n",
>+ &old_res, ret ? "cannot be" : "has", bus_max);
>+
If adjust_resource fails, I think we can't write the bus_max into the bridge
register.
>+ pci_write_config_byte(bus->self, PCI_SUBORDINATE_BUS, bus_max);
>+ return ret;
>+}
>+
> /*
> * 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
>@@ -814,12 +838,11 @@ int pci_scan_bridge(struct pci_bus *bus, struct pci_dev *dev, int max, int pass)
> }
>
> cmax = pci_scan_child_bus(child);
>- 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;
>+ if (cmax > child->busn_res.end)
>+ dev_warn(&dev->dev, "bridge has bus resource %pR but max busn %02x\n",
>+ &child->busn_res, cmax);
>+ if (child->busn_res.end > max)
>+ max = child->busn_res.end;
By a quick look, I don't see some place change the busn_res.end. And in the
pci_bus_insert_busn_res(), the child->busn_res.end is set to subordinate. So I
don't see the reason you change it.
Do I miss something?
> } else {
> /*
> * We need to assign a number to this bus which we always
>@@ -840,8 +863,10 @@ int pci_scan_bridge(struct pci_bus *bus, struct pci_dev *dev, int max, int pass)
>
> 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;
>+ max + 1, &bus->busn_res);
>+ /* Try to resize bus */
>+ if (pci_grow_bus(bus, max + 1))
>+ goto out;
On some platforms, like powerpc, we have some limitations of the bus number a
bridge could have. Sometimes, we need the start bus number to be power 2
aligned.
Hmm... hope I don't make a mistake, add Gavin in CC list. Gavin, is my
understanding correct?
> }
>
> /* Clear errors */
>@@ -908,6 +933,7 @@ int pci_scan_bridge(struct pci_bus *bus, struct pci_dev *dev, int max, int pass)
> break;
> }
> }
>+ dev_info(&dev->dev, "adding %02x bus numbers for cardbus\n", i);
> max += i;
> }
> /*
>@@ -916,7 +942,8 @@ int pci_scan_bridge(struct pci_bus *bus, struct pci_dev *dev, int max, int pass)
> 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;
>+ if (pci_grow_bus(bus, max))
>+ max = bus->busn_res.end;
> }
> pci_bus_update_busn_res_end(child, max);
> pci_write_config_byte(dev, PCI_SUBORDINATE_BUS, max);
>--
>2.1.0
>
>--
>To unsubscribe from this list: send the line "unsubscribe linux-pci" in
>the body of a message to majordomo@xxxxxxxxxxxxxxx
>More majordomo info at http://vger.kernel.org/majordomo-info.html
--
Richard Yang
Help you, Help me
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
