| Message ID | 1436292680-25111-1-git-send-email-linux@roeck-us.net (mailing list archive) |
|---|---|
| State | New, archived |
| Delegated to: | Bjorn Helgaas |
| Headers | show |
On 07/07/2015 11:11 AM, Guenter Roeck wrote: > The PCI subsystem always assumes that I/O is supported on PCIe bridges > and tries to assign an I/O window to each child bus even if that is not > the case. > > This may result in messages such as > > pcieport 0000:02:00.0: res[7]=[io 0x1000-0x0fff] > get_res_add_size add_size 1000 > pcieport 0000:02:00.0: BAR 7: no space for [io size 0x1000] > pcieport 0000:02:00.0: BAR 7: failed to assign [io size 0x1000] > > for each bridge port, even if a bus or its parent does not support > I/O in the first place. > > To avoid this message, check if a bus supports I/O before trying to > enable it. Also check if the root bus has an IO window assigned; > if not, it does not make sense to try to assign one to any of its > child busses. > > Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> > Signed-off-by: Guenter Roeck <linux@roeck-us.net> > --- > v2: Use a new bus flag to indicate if IO is supported on a bus or not. > Using IORESOURCE_DISABLED in resource flags turned out to be futile, > since the term "!res->flags" is widely used to detect if a resource > window is enabled or not, and setting IORESOURCE_DISABLED would > affect all this code. > > This patch depends on 'PCI: move pci_read_bridge_bases to the generic > PCI layer' by Lorenzo Pieralisi; without it, pci_read_bridge_io() > is not always called. > > With this version of the patch, pci_bridge_check_ranges() still sets > IORESOURCE_IO. Moving this into pci_read_bridge_io() had undesirable > side effects and resulted in missing IO window assignments on one of > the x86 platforms I tested with. I'll have to explore this further. > Hi Bjorn, any comments on this patch ? Is the new bus flag acceptable ? Thanks, Guenter > drivers/pci/probe.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++ > drivers/pci/setup-bus.c | 9 +-------- > include/linux/pci.h | 1 + > 3 files changed, 48 insertions(+), 8 deletions(-) > > diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c > index cefd636681b6..b21cba7aeb79 100644 > --- a/drivers/pci/probe.c > +++ b/drivers/pci/probe.c > @@ -332,6 +332,35 @@ static void pci_read_bases(struct pci_dev *dev, unsigned int howmany, int rom) > } > } > > +static bool pci_bus_supports_io(struct pci_bus *bus) > +{ > + struct pci_dev *dev = bus->self; > + u16 io; > + > + pci_read_config_word(dev, PCI_IO_BASE, &io); > + if (!io) { > + pci_write_config_word(dev, PCI_IO_BASE, 0xe0f0); > + pci_read_config_word(dev, PCI_IO_BASE, &io); > + pci_write_config_word(dev, PCI_IO_BASE, 0x0); > + } > + return !!io; > +} > + > +static bool pci_root_has_io_resource(struct pci_bus *bus) > +{ > + struct resource *res; > + int i; > + > + while (bus->parent) > + bus = bus->parent; > + > + pci_bus_for_each_resource(bus, res, i) { > + if (res && (res->flags & IORESOURCE_IO)) > + return true; > + } > + return false; > +} > + > static void pci_read_bridge_io(struct pci_bus *child) > { > struct pci_dev *dev = child->self; > @@ -340,6 +369,23 @@ static void pci_read_bridge_io(struct pci_bus *child) > struct pci_bus_region region; > struct resource *res; > > + if (child->bus_flags & PCI_BUS_FLAGS_NO_IO) > + return; > + > + if (!pci_bus_supports_io(child)) { > + dev_printk(KERN_DEBUG, &dev->dev, > + " bus does not support IO\n"); > + child->bus_flags |= PCI_BUS_FLAGS_NO_IO; > + return; > + } > + > + if (!pci_root_has_io_resource(child)) { > + dev_printk(KERN_DEBUG, &dev->dev, > + " no IO window on root bus\n"); > + child->bus_flags |= PCI_BUS_FLAGS_NO_IO; > + return; > + } > + > io_mask = PCI_IO_RANGE_MASK; > io_granularity = 0x1000; > if (dev->io_window_1k) { > diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c > index 508cc56130e3..c8c7eecadbfe 100644 > --- a/drivers/pci/setup-bus.c > +++ b/drivers/pci/setup-bus.c > @@ -744,7 +744,6 @@ int pci_claim_bridge_resource(struct pci_dev *bridge, int i) > base/limit registers must be read-only and read as 0. */ > static void pci_bridge_check_ranges(struct pci_bus *bus) > { > - u16 io; > u32 pmem; > struct pci_dev *bridge = bus->self; > struct resource *b_res; > @@ -752,13 +751,7 @@ static void pci_bridge_check_ranges(struct pci_bus *bus) > b_res = &bridge->resource[PCI_BRIDGE_RESOURCES]; > b_res[1].flags |= IORESOURCE_MEM; > > - pci_read_config_word(bridge, PCI_IO_BASE, &io); > - if (!io) { > - pci_write_config_word(bridge, PCI_IO_BASE, 0xe0f0); > - pci_read_config_word(bridge, PCI_IO_BASE, &io); > - pci_write_config_word(bridge, PCI_IO_BASE, 0x0); > - } > - if (io) > + if (!(bus->bus_flags & PCI_BUS_FLAGS_NO_IO)) > b_res[0].flags |= IORESOURCE_IO; > > /* DECchip 21050 pass 2 errata: the bridge may miss an address > diff --git a/include/linux/pci.h b/include/linux/pci.h > index 8a0321a8fb59..b910ed04aa0b 100644 > --- a/include/linux/pci.h > +++ b/include/linux/pci.h > @@ -191,6 +191,7 @@ typedef unsigned short __bitwise pci_bus_flags_t; > enum pci_bus_flags { > PCI_BUS_FLAGS_NO_MSI = (__force pci_bus_flags_t) 1, > PCI_BUS_FLAGS_NO_MMRBC = (__force pci_bus_flags_t) 2, > + PCI_BUS_FLAGS_NO_IO = (__force pci_bus_flags_t) 4, > }; > > /* These values come from the PCI Express Spec */ > -- 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
On Tue, Jul 07, 2015 at 11:11:20AM -0700, Guenter Roeck wrote: > The PCI subsystem always assumes that I/O is supported on PCIe bridges > and tries to assign an I/O window to each child bus even if that is not > the case. > > This may result in messages such as > > pcieport 0000:02:00.0: res[7]=[io 0x1000-0x0fff] > get_res_add_size add_size 1000 > pcieport 0000:02:00.0: BAR 7: no space for [io size 0x1000] > pcieport 0000:02:00.0: BAR 7: failed to assign [io size 0x1000] > > for each bridge port, even if a bus or its parent does not support > I/O in the first place. > > To avoid this message, check if a bus supports I/O before trying to > enable it. Also check if the root bus has an IO window assigned; > if not, it does not make sense to try to assign one to any of its > child busses. > > Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> > Signed-off-by: Guenter Roeck <linux@roeck-us.net> Applied to pci/resource for v4.3, thanks! > --- > v2: Use a new bus flag to indicate if IO is supported on a bus or not. > Using IORESOURCE_DISABLED in resource flags turned out to be futile, > since the term "!res->flags" is widely used to detect if a resource > window is enabled or not, and setting IORESOURCE_DISABLED would > affect all this code. > > This patch depends on 'PCI: move pci_read_bridge_bases to the generic > PCI layer' by Lorenzo Pieralisi; without it, pci_read_bridge_io() > is not always called. > > With this version of the patch, pci_bridge_check_ranges() still sets > IORESOURCE_IO. Moving this into pci_read_bridge_io() had undesirable > side effects and resulted in missing IO window assignments on one of > the x86 platforms I tested with. I'll have to explore this further. > > drivers/pci/probe.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++ > drivers/pci/setup-bus.c | 9 +-------- > include/linux/pci.h | 1 + > 3 files changed, 48 insertions(+), 8 deletions(-) > > diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c > index cefd636681b6..b21cba7aeb79 100644 > --- a/drivers/pci/probe.c > +++ b/drivers/pci/probe.c > @@ -332,6 +332,35 @@ static void pci_read_bases(struct pci_dev *dev, unsigned int howmany, int rom) > } > } > > +static bool pci_bus_supports_io(struct pci_bus *bus) > +{ > + struct pci_dev *dev = bus->self; > + u16 io; > + > + pci_read_config_word(dev, PCI_IO_BASE, &io); > + if (!io) { > + pci_write_config_word(dev, PCI_IO_BASE, 0xe0f0); > + pci_read_config_word(dev, PCI_IO_BASE, &io); > + pci_write_config_word(dev, PCI_IO_BASE, 0x0); > + } > + return !!io; > +} > + > +static bool pci_root_has_io_resource(struct pci_bus *bus) > +{ > + struct resource *res; > + int i; > + > + while (bus->parent) > + bus = bus->parent; > + > + pci_bus_for_each_resource(bus, res, i) { > + if (res && (res->flags & IORESOURCE_IO)) > + return true; > + } > + return false; > +} > + > static void pci_read_bridge_io(struct pci_bus *child) > { > struct pci_dev *dev = child->self; > @@ -340,6 +369,23 @@ static void pci_read_bridge_io(struct pci_bus *child) > struct pci_bus_region region; > struct resource *res; > > + if (child->bus_flags & PCI_BUS_FLAGS_NO_IO) > + return; > + > + if (!pci_bus_supports_io(child)) { > + dev_printk(KERN_DEBUG, &dev->dev, > + " bus does not support IO\n"); > + child->bus_flags |= PCI_BUS_FLAGS_NO_IO; > + return; > + } > + > + if (!pci_root_has_io_resource(child)) { > + dev_printk(KERN_DEBUG, &dev->dev, > + " no IO window on root bus\n"); > + child->bus_flags |= PCI_BUS_FLAGS_NO_IO; > + return; > + } > + > io_mask = PCI_IO_RANGE_MASK; > io_granularity = 0x1000; > if (dev->io_window_1k) { > diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c > index 508cc56130e3..c8c7eecadbfe 100644 > --- a/drivers/pci/setup-bus.c > +++ b/drivers/pci/setup-bus.c > @@ -744,7 +744,6 @@ int pci_claim_bridge_resource(struct pci_dev *bridge, int i) > base/limit registers must be read-only and read as 0. */ > static void pci_bridge_check_ranges(struct pci_bus *bus) > { > - u16 io; > u32 pmem; > struct pci_dev *bridge = bus->self; > struct resource *b_res; > @@ -752,13 +751,7 @@ static void pci_bridge_check_ranges(struct pci_bus *bus) > b_res = &bridge->resource[PCI_BRIDGE_RESOURCES]; > b_res[1].flags |= IORESOURCE_MEM; > > - pci_read_config_word(bridge, PCI_IO_BASE, &io); > - if (!io) { > - pci_write_config_word(bridge, PCI_IO_BASE, 0xe0f0); > - pci_read_config_word(bridge, PCI_IO_BASE, &io); > - pci_write_config_word(bridge, PCI_IO_BASE, 0x0); > - } > - if (io) > + if (!(bus->bus_flags & PCI_BUS_FLAGS_NO_IO)) > b_res[0].flags |= IORESOURCE_IO; > > /* DECchip 21050 pass 2 errata: the bridge may miss an address > diff --git a/include/linux/pci.h b/include/linux/pci.h > index 8a0321a8fb59..b910ed04aa0b 100644 > --- a/include/linux/pci.h > +++ b/include/linux/pci.h > @@ -191,6 +191,7 @@ typedef unsigned short __bitwise pci_bus_flags_t; > enum pci_bus_flags { > PCI_BUS_FLAGS_NO_MSI = (__force pci_bus_flags_t) 1, > PCI_BUS_FLAGS_NO_MMRBC = (__force pci_bus_flags_t) 2, > + PCI_BUS_FLAGS_NO_IO = (__force pci_bus_flags_t) 4, > }; > > /* These values come from the PCI Express Spec */ > -- > 2.1.0 > -- 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
On Wed, Jul 29, 2015 at 9:09 AM, Bjorn Helgaas <bhelgaas@google.com> wrote: > On Tue, Jul 07, 2015 at 11:11:20AM -0700, Guenter Roeck wrote: >> The PCI subsystem always assumes that I/O is supported on PCIe bridges >> and tries to assign an I/O window to each child bus even if that is not >> the case. >> >> This may result in messages such as >> >> pcieport 0000:02:00.0: res[7]=[io 0x1000-0x0fff] >> get_res_add_size add_size 1000 >> pcieport 0000:02:00.0: BAR 7: no space for [io size 0x1000] >> pcieport 0000:02:00.0: BAR 7: failed to assign [io size 0x1000] >> > > Applied to pci/resource for v4.3, thanks! >> @@ -340,6 +369,23 @@ static void pci_read_bridge_io(struct pci_bus *child) >> struct pci_bus_region region; >> struct resource *res; >> >> + if (child->bus_flags & PCI_BUS_FLAGS_NO_IO) >> + return; >> + >> + if (!pci_bus_supports_io(child)) { >> + dev_printk(KERN_DEBUG, &dev->dev, >> + " bus does not support IO\n"); >> + child->bus_flags |= PCI_BUS_FLAGS_NO_IO; >> + return; >> + } >> + >> + if (!pci_root_has_io_resource(child)) { >> + dev_printk(KERN_DEBUG, &dev->dev, >> + " no IO window on root bus\n"); >> + child->bus_flags |= PCI_BUS_FLAGS_NO_IO; >> + return; >> + } >> + >> io_mask = PCI_IO_RANGE_MASK; >> io_granularity = 0x1000; >> if (dev->io_window_1k) { Hi Bjorn, Looks like you flip the PCI_BUS_FLAGS_NO_IO to PCI_BUS_FLAGS_SUPPORT_IO, and now have @@ -340,6 +373,21 @@ static void pci_read_bridge_io(struct pci_bus *child) struct pci_bus_region region; struct resource *res; + if (!(child->bus_flags & PCI_BUS_FLAGS_SUPPORTS_IO)) + return; + + if (!pci_bridge_supports_io(dev)) { + dev_printk(KERN_DEBUG, &dev->dev, " no I/O window\n"); + return; + } + + if (!pci_root_has_io_resource(child)) { + dev_printk(KERN_DEBUG, &dev->dev, " no I/O resource on root bus\n"); + return; + } + + child->bus_flags |= PCI_BUS_FLAGS_SUPPORTS_IO; + so PCI_BUS_FLAGS_SUPPORTS_IO will never get set. Yinghai -- 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
On 07/29/2015 12:30 PM, Yinghai Lu wrote: > On Wed, Jul 29, 2015 at 9:09 AM, Bjorn Helgaas <bhelgaas@google.com> wrote: >> On Tue, Jul 07, 2015 at 11:11:20AM -0700, Guenter Roeck wrote: >>> The PCI subsystem always assumes that I/O is supported on PCIe bridges >>> and tries to assign an I/O window to each child bus even if that is not >>> the case. >>> >>> This may result in messages such as >>> >>> pcieport 0000:02:00.0: res[7]=[io 0x1000-0x0fff] >>> get_res_add_size add_size 1000 >>> pcieport 0000:02:00.0: BAR 7: no space for [io size 0x1000] >>> pcieport 0000:02:00.0: BAR 7: failed to assign [io size 0x1000] >>> >> >> Applied to pci/resource for v4.3, thanks! >>> @@ -340,6 +369,23 @@ static void pci_read_bridge_io(struct pci_bus *child) >>> struct pci_bus_region region; >>> struct resource *res; >>> >>> + if (child->bus_flags & PCI_BUS_FLAGS_NO_IO) >>> + return; >>> + >>> + if (!pci_bus_supports_io(child)) { >>> + dev_printk(KERN_DEBUG, &dev->dev, >>> + " bus does not support IO\n"); >>> + child->bus_flags |= PCI_BUS_FLAGS_NO_IO; >>> + return; >>> + } >>> + >>> + if (!pci_root_has_io_resource(child)) { >>> + dev_printk(KERN_DEBUG, &dev->dev, >>> + " no IO window on root bus\n"); >>> + child->bus_flags |= PCI_BUS_FLAGS_NO_IO; >>> + return; >>> + } >>> + >>> io_mask = PCI_IO_RANGE_MASK; >>> io_granularity = 0x1000; >>> if (dev->io_window_1k) { > > Hi Bjorn, > > Looks like you flip the PCI_BUS_FLAGS_NO_IO to > PCI_BUS_FLAGS_SUPPORT_IO, and now have > > @@ -340,6 +373,21 @@ static void pci_read_bridge_io(struct pci_bus *child) > struct pci_bus_region region; > struct resource *res; > > + if (!(child->bus_flags & PCI_BUS_FLAGS_SUPPORTS_IO)) > + return; > + > + if (!pci_bridge_supports_io(dev)) { > + dev_printk(KERN_DEBUG, &dev->dev, " no I/O window\n"); > + return; > + } > + > + if (!pci_root_has_io_resource(child)) { > + dev_printk(KERN_DEBUG, &dev->dev, " no I/O resource > on root bus\n"); > + return; > + } > + > + child->bus_flags |= PCI_BUS_FLAGS_SUPPORTS_IO; > + > > so PCI_BUS_FLAGS_SUPPORTS_IO will never get set. > Hi Yinghai, excellent catch. Unfortunately, I don't know how to make it work with the reversed flag. The idea here was that the flag propagates from parent to child. This makes sense for an "it doesn't work" flag to be inherited from the child, but not for an "it works" flag. Guenter -- 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
On Wed, Jul 29, 2015 at 12:46 PM, Guenter Roeck <linux@roeck-us.net> wrote: > On 07/29/2015 12:30 PM, Yinghai Lu wrote: > >> >> so PCI_BUS_FLAGS_SUPPORTS_IO will never get set. >> > > excellent catch. Unfortunately, I don't know how to make it > work with the reversed flag. The idea here was that the flag > propagates from parent to child. This makes sense for an > "it doesn't work" flag to be inherited from the child, > but not for an "it works" flag. > also would be better if we can add has_ioport in hostbridge instead. like has_mem64 in https://patchwork.ozlabs.org/patch/500926/ and use to_pci_host_bridge(bus->bridge)->has_ioport to replace pci_root_has_io_resource() Thanks Yinghai -- 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
On 07/29/2015 12:53 PM, Yinghai Lu wrote: > On Wed, Jul 29, 2015 at 12:46 PM, Guenter Roeck <linux@roeck-us.net> wrote: >> On 07/29/2015 12:30 PM, Yinghai Lu wrote: >> >>> >>> so PCI_BUS_FLAGS_SUPPORTS_IO will never get set. >>> >> >> excellent catch. Unfortunately, I don't know how to make it >> work with the reversed flag. The idea here was that the flag >> propagates from parent to child. This makes sense for an >> "it doesn't work" flag to be inherited from the child, >> but not for an "it works" flag. >> > > also would be better if we can add has_ioport in hostbridge instead. > like has_mem64 in https://patchwork.ozlabs.org/patch/500926/ > and use > > to_pci_host_bridge(bus->bridge)->has_ioport > > to replace pci_root_has_io_resource() > Sure, that would make sense. Bjorn, how do you want to handle the flag problem ? Do you have an idea on how to make it work with the reversed definition ? Thanks, Guenter -- 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
On 07/29/2015 12:53 PM, Yinghai Lu wrote: > On Wed, Jul 29, 2015 at 12:46 PM, Guenter Roeck <linux@roeck-us.net> wrote: >> On 07/29/2015 12:30 PM, Yinghai Lu wrote: >> >>> >>> so PCI_BUS_FLAGS_SUPPORTS_IO will never get set. >>> >> >> excellent catch. Unfortunately, I don't know how to make it >> work with the reversed flag. The idea here was that the flag >> propagates from parent to child. This makes sense for an >> "it doesn't work" flag to be inherited from the child, >> but not for an "it works" flag. >> > > also would be better if we can add has_ioport in hostbridge instead. > like has_mem64 in https://patchwork.ozlabs.org/patch/500926/ > and use > > to_pci_host_bridge(bus->bridge)->has_ioport > > to replace pci_root_has_io_resource() > Is there a function to get the root bus ? Otherwise I still need to find the root bus first. Guenter -- 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
On Wed, Jul 29, 2015 at 1:18 PM, Guenter Roeck <linux@roeck-us.net> wrote: > On 07/29/2015 12:53 PM, Yinghai Lu wrote: >> >> On Wed, Jul 29, 2015 at 12:46 PM, Guenter Roeck <linux@roeck-us.net> >> wrote: >>> >>> On 07/29/2015 12:30 PM, Yinghai Lu wrote: >>> >>>> >>>> so PCI_BUS_FLAGS_SUPPORTS_IO will never get set. >>>> >>> >>> excellent catch. Unfortunately, I don't know how to make it >>> work with the reversed flag. The idea here was that the flag >>> propagates from parent to child. This makes sense for an >>> "it doesn't work" flag to be inherited from the child, >>> but not for an "it works" flag. >>> >> >> also would be better if we can add has_ioport in hostbridge instead. >> like has_mem64 in https://patchwork.ozlabs.org/patch/500926/ >> and use >> >> to_pci_host_bridge(bus->bridge)->has_ioport >> >> to replace pci_root_has_io_resource() >> > > Is there a function to get the root bus ? Otherwise I still need to > find the root bus first. host-bridge and rootbus is one to one mapping. drivers/pci/host-bridge.c has related function. Yinghai -- 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
On Wed, Jul 29, 2015 at 01:02:25PM -0700, Guenter Roeck wrote: > On 07/29/2015 12:53 PM, Yinghai Lu wrote: > >On Wed, Jul 29, 2015 at 12:46 PM, Guenter Roeck <linux@roeck-us.net> wrote: > >>On 07/29/2015 12:30 PM, Yinghai Lu wrote: > >> > >>> > >>>so PCI_BUS_FLAGS_SUPPORTS_IO will never get set. > >>> > >> > >>excellent catch. Unfortunately, I don't know how to make it > >>work with the reversed flag. The idea here was that the flag > >>propagates from parent to child. This makes sense for an > >>"it doesn't work" flag to be inherited from the child, > >>but not for an "it works" flag. > >> > > > >also would be better if we can add has_ioport in hostbridge instead. > >like has_mem64 in https://patchwork.ozlabs.org/patch/500926/ > >and use > > > >to_pci_host_bridge(bus->bridge)->has_ioport > > > >to replace pci_root_has_io_resource() > > > > Sure, that would make sense. > > Bjorn, how do you want to handle the flag problem ? > Do you have an idea on how to make it work with > the reversed definition ? I'll wait for the revised patch. Sorry for screwing this up. -- 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/drivers/pci/probe.c b/drivers/pci/probe.c index cefd636681b6..b21cba7aeb79 100644 --- a/drivers/pci/probe.c +++ b/drivers/pci/probe.c @@ -332,6 +332,35 @@ static void pci_read_bases(struct pci_dev *dev, unsigned int howmany, int rom) } } +static bool pci_bus_supports_io(struct pci_bus *bus) +{ + struct pci_dev *dev = bus->self; + u16 io; + + pci_read_config_word(dev, PCI_IO_BASE, &io); + if (!io) { + pci_write_config_word(dev, PCI_IO_BASE, 0xe0f0); + pci_read_config_word(dev, PCI_IO_BASE, &io); + pci_write_config_word(dev, PCI_IO_BASE, 0x0); + } + return !!io; +} + +static bool pci_root_has_io_resource(struct pci_bus *bus) +{ + struct resource *res; + int i; + + while (bus->parent) + bus = bus->parent; + + pci_bus_for_each_resource(bus, res, i) { + if (res && (res->flags & IORESOURCE_IO)) + return true; + } + return false; +} + static void pci_read_bridge_io(struct pci_bus *child) { struct pci_dev *dev = child->self; @@ -340,6 +369,23 @@ static void pci_read_bridge_io(struct pci_bus *child) struct pci_bus_region region; struct resource *res; + if (child->bus_flags & PCI_BUS_FLAGS_NO_IO) + return; + + if (!pci_bus_supports_io(child)) { + dev_printk(KERN_DEBUG, &dev->dev, + " bus does not support IO\n"); + child->bus_flags |= PCI_BUS_FLAGS_NO_IO; + return; + } + + if (!pci_root_has_io_resource(child)) { + dev_printk(KERN_DEBUG, &dev->dev, + " no IO window on root bus\n"); + child->bus_flags |= PCI_BUS_FLAGS_NO_IO; + return; + } + io_mask = PCI_IO_RANGE_MASK; io_granularity = 0x1000; if (dev->io_window_1k) { diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c index 508cc56130e3..c8c7eecadbfe 100644 --- a/drivers/pci/setup-bus.c +++ b/drivers/pci/setup-bus.c @@ -744,7 +744,6 @@ int pci_claim_bridge_resource(struct pci_dev *bridge, int i) base/limit registers must be read-only and read as 0. */ static void pci_bridge_check_ranges(struct pci_bus *bus) { - u16 io; u32 pmem; struct pci_dev *bridge = bus->self; struct resource *b_res; @@ -752,13 +751,7 @@ static void pci_bridge_check_ranges(struct pci_bus *bus) b_res = &bridge->resource[PCI_BRIDGE_RESOURCES]; b_res[1].flags |= IORESOURCE_MEM; - pci_read_config_word(bridge, PCI_IO_BASE, &io); - if (!io) { - pci_write_config_word(bridge, PCI_IO_BASE, 0xe0f0); - pci_read_config_word(bridge, PCI_IO_BASE, &io); - pci_write_config_word(bridge, PCI_IO_BASE, 0x0); - } - if (io) + if (!(bus->bus_flags & PCI_BUS_FLAGS_NO_IO)) b_res[0].flags |= IORESOURCE_IO; /* DECchip 21050 pass 2 errata: the bridge may miss an address diff --git a/include/linux/pci.h b/include/linux/pci.h index 8a0321a8fb59..b910ed04aa0b 100644 --- a/include/linux/pci.h +++ b/include/linux/pci.h @@ -191,6 +191,7 @@ typedef unsigned short __bitwise pci_bus_flags_t; enum pci_bus_flags { PCI_BUS_FLAGS_NO_MSI = (__force pci_bus_flags_t) 1, PCI_BUS_FLAGS_NO_MMRBC = (__force pci_bus_flags_t) 2, + PCI_BUS_FLAGS_NO_IO = (__force pci_bus_flags_t) 4, }; /* These values come from the PCI Express Spec */
The PCI subsystem always assumes that I/O is supported on PCIe bridges and tries to assign an I/O window to each child bus even if that is not the case. This may result in messages such as pcieport 0000:02:00.0: res[7]=[io 0x1000-0x0fff] get_res_add_size add_size 1000 pcieport 0000:02:00.0: BAR 7: no space for [io size 0x1000] pcieport 0000:02:00.0: BAR 7: failed to assign [io size 0x1000] for each bridge port, even if a bus or its parent does not support I/O in the first place. To avoid this message, check if a bus supports I/O before trying to enable it. Also check if the root bus has an IO window assigned; if not, it does not make sense to try to assign one to any of its child busses. Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> Signed-off-by: Guenter Roeck <linux@roeck-us.net> --- v2: Use a new bus flag to indicate if IO is supported on a bus or not. Using IORESOURCE_DISABLED in resource flags turned out to be futile, since the term "!res->flags" is widely used to detect if a resource window is enabled or not, and setting IORESOURCE_DISABLED would affect all this code. This patch depends on 'PCI: move pci_read_bridge_bases to the generic PCI layer' by Lorenzo Pieralisi; without it, pci_read_bridge_io() is not always called. With this version of the patch, pci_bridge_check_ranges() still sets IORESOURCE_IO. Moving this into pci_read_bridge_io() had undesirable side effects and resulted in missing IO window assignments on one of the x86 platforms I tested with. I'll have to explore this further. drivers/pci/probe.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++ drivers/pci/setup-bus.c | 9 +-------- include/linux/pci.h | 1 + 3 files changed, 48 insertions(+), 8 deletions(-)