Skip to content

Commit

Permalink
PCI: Revert "PCI: Call pci_read_bridge_bases() from core instead of a…
Browse files Browse the repository at this point in the history
…rch code"

Revert dff22d2 ("PCI: Call pci_read_bridge_bases() from core instead
of arch code").

Reading PCI bridge windows is not arch-specific in itself, but there is PCI
core code that doesn't work correctly if we read them too early.  For
example, Hannes found this case on an ARM Freescale i.mx6 board:

  pci_bus 0000:00: root bus resource [mem 0x01000000-0x01efffff]
  pci 0000:00:00.0: PCI bridge to [bus 01-ff]
  pci 0000:00:00.0: BAR 8: no space for [mem size 0x01000000] (mem window)
  pci 0000:01:00.0: BAR 2: failed to assign [mem size 0x00200000]
  pci 0000:01:00.0: BAR 1: failed to assign [mem size 0x00004000]
  pci 0000:01:00.0: BAR 0: failed to assign [mem size 0x00000100]

The 00:00.0 mem window needs to be at least 3MB: the 01:00.0 device needs
0x204100 of space, and mem windows are megabyte-aligned.

Bus sizing can increase a bridge window size, but never *decrease* it (see
d65245c ("PCI: don't shrink bridge resources")).  Prior to
dff22d2, ARM didn't read bridge windows at all, so the "original size"
was zero, and we assigned a 3MB window.

After dff22d2, we read the bridge windows before sizing the bus.  The
firmware programmed a 16MB window (size 0x01000000) in 00:00.0, and since
we never decrease the size, we kept 16MB even though we only needed 3MB.
But 16MB doesn't fit in the host bridge aperture, so we failed to assign
space for the window and the downstream devices.

I think this is a defect in the PCI core: we shouldn't rely on the firmware
to assign sensible windows.

Ray reported a similar problem, also on ARM, with Broadcom iProc.

Issues like this are too hard to fix right now, so revert dff22d2.

Reported-by: Hannes <oe5hpm@gmail.com>
Reported-by: Ray Jui <rjui@broadcom.com>
Link: http://lkml.kernel.org/r/CAAa04yFQEUJm7Jj1qMT57-LG7ZGtnhNDBe=PpSRa70Mj+XhW-A@mail.gmail.com
Link: http://lkml.kernel.org/r/55F75BB8.4070405@broadcom.com
Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
Acked-by: Yinghai Lu <yinghai@kernel.org>
Acked-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
  • Loading branch information
Bjorn Helgaas committed Sep 15, 2015
1 parent 6ff33f3 commit 237865f
Show file tree
Hide file tree
Showing 12 changed files with 42 additions and 11 deletions.
7 changes: 6 additions & 1 deletion arch/alpha/kernel/pci.c
Original file line number Diff line number Diff line change
Expand Up @@ -242,7 +242,12 @@ pci_restore_srm_config(void)

void pcibios_fixup_bus(struct pci_bus *bus)
{
struct pci_dev *dev;
struct pci_dev *dev = bus->self;

if (pci_has_flag(PCI_PROBE_ONLY) && dev &&
(dev->class >> 8) == PCI_CLASS_BRIDGE_PCI) {
pci_read_bridge_bases(bus);
}

list_for_each_entry(dev, &bus->devices, bus_list) {
pdev_save_srm_config(dev);
Expand Down
2 changes: 2 additions & 0 deletions arch/frv/mb93090-mb00/pci-vdk.c
Original file line number Diff line number Diff line change
Expand Up @@ -294,6 +294,8 @@ void pcibios_fixup_bus(struct pci_bus *bus)
printk("### PCIBIOS_FIXUP_BUS(%d)\n",bus->number);
#endif

pci_read_bridge_bases(bus);

if (bus->number == 0) {
struct pci_dev *dev;
list_for_each_entry(dev, &bus->devices, bus_list) {
Expand Down
5 changes: 3 additions & 2 deletions arch/ia64/pci/pci.c
Original file line number Diff line number Diff line change
Expand Up @@ -533,9 +533,10 @@ void pcibios_fixup_bus(struct pci_bus *b)
{
struct pci_dev *dev;

if (b->self)
if (b->self) {
pci_read_bridge_bases(b);
pcibios_fixup_bridge_resources(b->self);

}
list_for_each_entry(dev, &b->devices, bus_list)
pcibios_fixup_device_resources(dev);
platform_pci_fixup_bus(b);
Expand Down
9 changes: 8 additions & 1 deletion arch/microblaze/pci/pci-common.c
Original file line number Diff line number Diff line change
Expand Up @@ -863,7 +863,14 @@ void pcibios_setup_bus_devices(struct pci_bus *bus)

void pcibios_fixup_bus(struct pci_bus *bus)
{
/* Fixup the bus */
/* When called from the generic PCI probe, read PCI<->PCI bridge
* bases. This is -not- called when generating the PCI tree from
* the OF device-tree.
*/
if (bus->self != NULL)
pci_read_bridge_bases(bus);

/* Now fixup the bus bus */
pcibios_setup_bus_self(bus);

/* Now fixup devices on that bus */
Expand Down
6 changes: 6 additions & 0 deletions arch/mips/pci/pci.c
Original file line number Diff line number Diff line change
Expand Up @@ -311,6 +311,12 @@ int pcibios_enable_device(struct pci_dev *dev, int mask)

void pcibios_fixup_bus(struct pci_bus *bus)
{
struct pci_dev *dev = bus->self;

if (pci_has_flag(PCI_PROBE_ONLY) && dev &&
(dev->class >> 8) == PCI_CLASS_BRIDGE_PCI) {
pci_read_bridge_bases(bus);
}
}

EXPORT_SYMBOL(PCIBIOS_MIN_IO);
Expand Down
1 change: 1 addition & 0 deletions arch/mn10300/unit-asb2305/pci.c
Original file line number Diff line number Diff line change
Expand Up @@ -324,6 +324,7 @@ void pcibios_fixup_bus(struct pci_bus *bus)
struct pci_dev *dev;

if (bus->self) {
pci_read_bridge_bases(bus);
pcibios_fixup_bridge_resources(bus->self);
}

Expand Down
8 changes: 7 additions & 1 deletion arch/powerpc/kernel/pci-common.c
Original file line number Diff line number Diff line change
Expand Up @@ -1032,7 +1032,13 @@ void pcibios_set_master(struct pci_dev *dev)

void pcibios_fixup_bus(struct pci_bus *bus)
{
/* Fixup the bus */
/* When called from the generic PCI probe, read PCI<->PCI bridge
* bases. This is -not- called when generating the PCI tree from
* the OF device-tree.
*/
pci_read_bridge_bases(bus);

/* Now fixup the bus bus */
pcibios_setup_bus_self(bus);

/* Now fixup devices on that bus */
Expand Down
1 change: 1 addition & 0 deletions arch/x86/pci/common.c
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,7 @@ void pcibios_fixup_bus(struct pci_bus *b)
{
struct pci_dev *dev;

pci_read_bridge_bases(b);
list_for_each_entry(dev, &b->devices, bus_list)
pcibios_fixup_device_resources(dev);
}
Expand Down
4 changes: 4 additions & 0 deletions arch/xtensa/kernel/pci.c
Original file line number Diff line number Diff line change
Expand Up @@ -210,6 +210,10 @@ subsys_initcall(pcibios_init);

void pcibios_fixup_bus(struct pci_bus *bus)
{
if (bus->parent) {
/* This is a subordinate bridge */
pci_read_bridge_bases(bus);
}
}

void pcibios_set_master(struct pci_dev *dev)
Expand Down
3 changes: 3 additions & 0 deletions drivers/parisc/dino.c
Original file line number Diff line number Diff line change
Expand Up @@ -560,6 +560,9 @@ dino_fixup_bus(struct pci_bus *bus)
} else if (bus->parent) {
int i;

pci_read_bridge_bases(bus);


for(i = PCI_BRIDGE_RESOURCES; i < PCI_NUM_RESOURCES; i++) {
if((bus->self->resource[i].flags &
(IORESOURCE_IO | IORESOURCE_MEM)) == 0)
Expand Down
1 change: 1 addition & 0 deletions drivers/parisc/lba_pci.c
Original file line number Diff line number Diff line change
Expand Up @@ -693,6 +693,7 @@ lba_fixup_bus(struct pci_bus *bus)
if (bus->parent) {
int i;
/* PCI-PCI Bridge */
pci_read_bridge_bases(bus);
for (i = PCI_BRIDGE_RESOURCES; i < PCI_NUM_RESOURCES; i++)
pci_claim_bridge_resource(bus->self, i);
} else {
Expand Down
6 changes: 0 additions & 6 deletions drivers/pci/probe.c
Original file line number Diff line number Diff line change
Expand Up @@ -855,9 +855,6 @@ int pci_scan_bridge(struct pci_bus *bus, struct pci_dev *dev, int max, int pass)
child->bridge_ctl = bctl;
}

/* Read and initialize bridge resources */
pci_read_bridge_bases(child);

cmax = pci_scan_child_bus(child);
if (cmax > subordinate)
dev_warn(&dev->dev, "bridge has subordinate %02x but max busn %02x\n",
Expand Down Expand Up @@ -918,9 +915,6 @@ int pci_scan_bridge(struct pci_bus *bus, struct pci_dev *dev, int max, int pass)

if (!is_cardbus) {
child->bridge_ctl = bctl;

/* Read and initialize bridge resources */
pci_read_bridge_bases(child);
max = pci_scan_child_bus(child);
} else {
/*
Expand Down

0 comments on commit 237865f

Please sign in to comment.