From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from bh-25.webhostbox.net ([208.91.199.152]:47969 "EHLO bh-25.webhostbox.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751670AbbIOVHZ (ORCPT ); Tue, 15 Sep 2015 17:07:25 -0400 Subject: Re: trouble with PCI: Call pci_read_bridge_bases() from core instead of arch code To: Yinghai Lu References: <20150904164412.GD22997@red-moon> <20150907091230.GB29293@red-moon> <20150914100929.GB18886@red-moon> <20150914162834.GA11199@red-moon> <20150915094610.GD11199@red-moon> <20150915163000.GA16240@red-moon> <55F84CAA.8060901@roeck-us.net> Cc: Lorenzo Pieralisi , Bjorn Helgaas , "oe5hpm@gmail.com" , Ralf Baechle , "James E.J. Bottomley" , Michael Ellerman , Richard Henderson , Benjamin Herrenschmidt , David Howells , Russell King , Tony Luck , "David S. Miller" , Ingo Molnar , Michal Simek , Chris Zankel , "linux-pci@vger.kernel.org" From: Guenter Roeck Message-ID: <55F88885.9070707@roeck-us.net> Date: Tue, 15 Sep 2015 14:07:17 -0700 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Sender: linux-pci-owner@vger.kernel.org List-ID: On 09/15/2015 01:17 PM, Yinghai Lu wrote: > On Tue, Sep 15, 2015 at 9:51 AM, Guenter Roeck wrote: >> On 09/15/2015 09:30 AM, Lorenzo Pieralisi wrote: >> >> It looks like me and Yinghai disagree how the problem I was trying to fix >> should be handled, I don't understand Yinghai's concerns, and unfortunately >> I just don't have the time I would need to get a better understanding. >> It is fine with me to revert your patch and abandon mine. > > I put one simplified version in one my branch. > > https://git.kernel.org/cgit/linux/kernel/git/yinghai/linux-yinghai.git/patch/?id=cdf4bfbc1ce12d826abd49a8987eb3ca7e129332 > > and will sent that later if needed. > >>>From cdf4bfbc1ce12d826abd49a8987eb3ca7e129332 Mon Sep 17 00:00:00 2001 > From: Guenter Roeck Yinghai, that isn't really my patch, so you should not send it under my name. Thanks, Guenter > Date: Tue, 15 Sep 2015 12:59:11 -0700 > Subject: PCI: Only try to assign io port only for root bus that support it > > The PCI subsystem always assumes that I/O is supported on root bus 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 root does not support I/O in the first place. > > To avoid this message, check if root bus supports I/O, then child bus > will inherit the setting from parent bus, and later during sizing and > assigning, check that bus flags and skip those resources. > > [bhelgaas: reverse sense of new pci_bus_flags_t value] > [yinghai: only check root bus io port, and use flag on sizing and assigning] > Signed-off-by: Guenter Roeck > Signed-off-by: Yinghai Lu > Cc: Bjorn Helgaas > CC: Lorenzo Pieralisi > > diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c > index c4c6947..5d946bd 100644 > --- a/drivers/pci/probe.c > +++ b/drivers/pci/probe.c > @@ -339,6 +339,9 @@ 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_ROOT_SUPPORTS_IO)) > + return; > + > io_mask = PCI_IO_RANGE_MASK; > io_granularity = 0x1000; > if (dev->io_window_1k) { > @@ -2126,6 +2129,8 @@ struct pci_bus *pci_create_root_bus(struct > device *parent, int bus, > } else > bus_addr[0] = '\0'; > dev_info(&b->dev, "root bus resource %pR%s\n", res, bus_addr); > + if (resource_type(res) == IORESOURCE_IO) > + b->bus_flags |= PCI_BUS_FLAGS_ROOT_SUPPORTS_IO; > } > > resource_list_for_each_entry(window, &bridge->windows) { > diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c > index 9d5e415..8ee5d17 100644 > --- a/drivers/pci/setup-bus.c > +++ b/drivers/pci/setup-bus.c > @@ -223,6 +223,10 @@ static void pdev_assign_resources_prepare(struct > pci_dev *dev, > if (r->flags & IORESOURCE_PCI_FIXED) > continue; > > + if ((r->flags & IORESOURCE_IO) && > + !(dev->bus->bus_flags & PCI_BUS_FLAGS_ROOT_SUPPORTS_IO)) > + continue; > + > if (resource_disabled(r) || r->parent) > continue; > > @@ -1178,6 +1182,11 @@ static void pbus_size_io(struct pci_bus *bus, > resource_size_t min_size, > min_size = 0; > } > > + if (!(bus->bus_flags & PCI_BUS_FLAGS_ROOT_SUPPORTS_IO)) { > + b_res->flags |= IORESOURCE_UNSET | IORESOURCE_DISABLED; > + return; > + } > + > min_align = window_alignment(bus, IORESOURCE_IO); > list_for_each_entry(dev, &bus->devices, bus_list) { > int i; > diff --git a/include/linux/pci.h b/include/linux/pci.h > index 5ed9bf1..790c534 100644 > --- a/include/linux/pci.h > +++ b/include/linux/pci.h > @@ -193,6 +193,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_ROOT_SUPPORTS_IO = (__force pci_bus_flags_t) 4, > }; > > /* These values come from the PCI Express Spec */ >