[REGRESSION][v3.4-rc1] PCI: add a PCI resource reallocation config option

[REGRESSION][v3.4-rc1] PCI: add a PCI resource reallocation config option

[Joseph Salisbury]

Hi Yinghai,

A kernel bug was opened against Ubuntu [0].  After a kernel bisect, it
was found that the following commit introduced the bug:

commit b07f2ebc109b607789f648dedcff4b125f9afec6
Author: Yinghai Lu <yinghai@kernel.org>
Date:   Thu Feb 23 19:23:32 2012 -0800

    PCI: add a PCI resource reallocation config option



The regression was introduced as of v3.4-rc1 and also exists in current
mainline.

The bug seems platform specific since we have not had allot of other
reports.  The bug is preventing ixgbe to probe Intel x520 NIC's.

It would be easy enough to disable the PCI_REALLOC_ENABLE_AUTO config
option, but I wanted to get your feedback since you are the author.  It
looks like the bug reporter may also be able to work around the problem
with the pci=realloc=.
   

Thanks,
   
Joe

[0] pad.lv/1245938

Re: [REGRESSION][v3.4-rc1] PCI: add a PCI resource reallocation config option

[Yinghai Lu]

[-- Attachment #1: Type: text/plain, Size: 1138 bytes --]

On Wed, Nov 27, 2013 at 12:22 PM, Joseph Salisbury
<joseph.salisbury@canonical.com> wrote:
> Hi Yinghai,
>
> A kernel bug was opened against Ubuntu [0].  After a kernel bisect, it
> was found that the following commit introduced the bug:
>
> commit b07f2ebc109b607789f648dedcff4b125f9afec6
> Author: Yinghai Lu <yinghai@kernel.org>
> Date:   Thu Feb 23 19:23:32 2012 -0800
>
>     PCI: add a PCI resource reallocation config option
>
>
>
> The regression was introduced as of v3.4-rc1 and also exists in current
> mainline.
>
> The bug seems platform specific since we have not had allot of other
> reports.  The bug is preventing ixgbe to probe Intel x520 NIC's.
>
> It would be easy enough to disable the PCI_REALLOC_ENABLE_AUTO config
> option, but I wanted to get your feedback since you are the author.  It
> looks like the bug reporter may also be able to work around the problem
> with the pci=realloc=.

The bios has problem to have two functions rom bar to same place, and
root bus does
not enough mmio range and confuse realloc logic.

Please try attached patches to top of linus's tree, or your internal tree.

Thanks

Yinghai

[-- Attachment #2: pcibus_addr_converting_bus.patch --]
[-- Type: text/x-patch, Size: 4049 bytes --]

Subject: [PATCH] PCI: pcibus address to resource converting take bus instead of dev

For allocating resource under bus path, we do not have dev to pass along,
and we only have bus to use instead.

-v2: drop pcibios_bus_addr_to_resource().

Signed-off-by: Yinghai Lu <yinghai@kernel.org>

---
 drivers/pci/host-bridge.c |   34 +++++++++++++++++++++-------------
 include/linux/pci.h       |    5 +++++
 2 files changed, 26 insertions(+), 13 deletions(-)

Index: linux-2.6/drivers/pci/host-bridge.c
===================================================================
--- linux-2.6.orig/drivers/pci/host-bridge.c
+++ linux-2.6/drivers/pci/host-bridge.c
@@ -9,22 +9,19 @@
 
 #include "pci.h"
 
-static struct pci_bus *find_pci_root_bus(struct pci_dev *dev)
+static struct pci_bus *find_pci_root_bus(struct pci_bus *bus)
 {
-	struct pci_bus *bus;
-
-	bus = dev->bus;
 	while (bus->parent)
 		bus = bus->parent;
 
 	return bus;
 }
 
-static struct pci_host_bridge *find_pci_host_bridge(struct pci_dev *dev)
+static struct pci_host_bridge *find_pci_host_bridge(struct pci_bus *bus)
 {
-	struct pci_bus *bus = find_pci_root_bus(dev);
+	struct pci_bus *root_bus = find_pci_root_bus(bus);
 
-	return to_pci_host_bridge(bus->bridge);
+	return to_pci_host_bridge(root_bus->bridge);
 }
 
 void pci_set_host_bridge_release(struct pci_host_bridge *bridge,
@@ -40,10 +37,11 @@ static bool resource_contains(struct res
 	return res1->start <= res2->start && res1->end >= res2->end;
 }
 
-void pcibios_resource_to_bus(struct pci_dev *dev, struct pci_bus_region *region,
-			     struct resource *res)
+void __pcibios_resource_to_bus(struct pci_bus *bus,
+				      struct pci_bus_region *region,
+				      struct resource *res)
 {
-	struct pci_host_bridge *bridge = find_pci_host_bridge(dev);
+	struct pci_host_bridge *bridge = find_pci_host_bridge(bus);
 	struct pci_host_bridge_window *window;
 	resource_size_t offset = 0;
 
@@ -60,6 +58,11 @@ void pcibios_resource_to_bus(struct pci_
 	region->start = res->start - offset;
 	region->end = res->end - offset;
 }
+void pcibios_resource_to_bus(struct pci_dev *dev, struct pci_bus_region *region,
+			     struct resource *res)
+{
+	__pcibios_resource_to_bus(dev->bus, region, res);
+}
 EXPORT_SYMBOL(pcibios_resource_to_bus);
 
 static bool region_contains(struct pci_bus_region *region1,
@@ -68,10 +71,10 @@ static bool region_contains(struct pci_b
 	return region1->start <= region2->start && region1->end >= region2->end;
 }
 
-void pcibios_bus_to_resource(struct pci_dev *dev, struct resource *res,
-			     struct pci_bus_region *region)
+void __pcibios_bus_to_resource(struct pci_bus *bus, struct resource *res,
+				      struct pci_bus_region *region)
 {
-	struct pci_host_bridge *bridge = find_pci_host_bridge(dev);
+	struct pci_host_bridge *bridge = find_pci_host_bridge(bus);
 	struct pci_host_bridge_window *window;
 	resource_size_t offset = 0;
 
@@ -93,4 +96,9 @@ void pcibios_bus_to_resource(struct pci_
 	res->start = region->start + offset;
 	res->end = region->end + offset;
 }
+void pcibios_bus_to_resource(struct pci_dev *dev, struct resource *res,
+			     struct pci_bus_region *region)
+{
+	__pcibios_bus_to_resource(dev->bus, res, region);
+}
 EXPORT_SYMBOL(pcibios_bus_to_resource);
Index: linux-2.6/include/linux/pci.h
===================================================================
--- linux-2.6.orig/include/linux/pci.h
+++ linux-2.6/include/linux/pci.h
@@ -738,8 +738,13 @@ void pci_fixup_cardbus(struct pci_bus *)
 
 /* Generic PCI functions used internally */
 
+void __pcibios_resource_to_bus(struct pci_bus *bus,
+			       struct pci_bus_region *region,
+			       struct resource *res);
 void pcibios_resource_to_bus(struct pci_dev *dev, struct pci_bus_region *region,
 			     struct resource *res);
+void __pcibios_bus_to_resource(struct pci_bus *bus, struct resource *res,
+				      struct pci_bus_region *region);
 void pcibios_bus_to_resource(struct pci_dev *dev, struct resource *res,
 			     struct pci_bus_region *region);
 void pcibios_scan_specific_bus(int busn);

[-- Attachment #3: auto_need_mmio64.patch --]
[-- Type: text/x-patch, Size: 3600 bytes --]

Subject: [PATCH] PCI: Only enable realloc auto when root bus has 64bit mmio

Joseph found
| commit b07f2ebc109b607789f648dedcff4b125f9afec6
| Date:   Thu Feb 23 19:23:32 2012 -0800
|
|    PCI: add a PCI resource reallocation config option

cause one system can not load driver for Intel x520 NIC's.

The root resource:
[    1.212470] PCI host bridge to bus 0000:20
[    1.212475] pci_bus 0000:20: root bus resource [bus 20-3e]
[    1.212479] pci_bus 0000:20: root bus resource [io  0xc000-0xdfff]
[    1.212483] pci_bus 0000:20: root bus resource [mem 0xfecc0000-0xfecfffff]
[    1.212487] pci_bus 0000:20: root bus resource [mem 0xe9400000-0xe97fffff]

and bios does not assign sriov, also have two function ROM bar point to same
position.
[    1.213197] pci 0000:22:00.0: [8086:10fb] type 00 class 0x020000
...
[    1.213240] pci 0000:22:00.0: reg 0x30: [mem 0xe9500000-0xe957ffff pref]
[    1.213303] pci 0000:22:00.0: reg 0x184: [mem 0x00000000-0x00003fff 64bit]
[    1.213317] pci 0000:22:00.0: reg 0x190: [mem 0x00000000-0x00003fff 64bit]
[    1.213366] pci 0000:22:00.1: [8086:10fb] type 00 class 0x020000
...
[    1.213408] pci 0000:22:00.1: reg 0x30: [mem 0xe9500000-0xe957ffff pref]
[    1.213468] pci 0000:22:00.1: reg 0x184: [mem 0x00000000-0x00003fff 64bit]
[    1.213481] pci 0000:22:00.1: reg 0x190: [mem 0x00000000-0x00003fff 64bit]
[    1.218527] pci 0000:20:03.0: PCI bridge to [bus 22]
[    1.218534] pci 0000:20:03.0:   bridge window [io  0xd000-0xdfff]
[    1.218537] pci 0000:20:03.0:   bridge window [mem 0xe9400000-0xe95fffff]
...
[    1.254103] pci 0000:22:00.1: address space collision: [mem 0xe9500000-0xe957ffff pref] conflicts with 0000:22:00.0 [mem 0xe9500000-0xe957ffff pref]
[    1.254111] pci 0000:23:00.1: address space collision: [mem 0xe9700000-0xe977ffff pref] conflicts with 0000:23:00.0 [mem 0xe9700000-0xe977ffff pref]

We don't need to enable realloc for this case, as we can not alter root bus mmio
range to get big one to hold two rom bar, and sriov under 4G.

Add checking if pci root bus have 4G above mmio res, and don't enable
realloc auto accordingly.

bug report at: https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1245938

Reported-by: Joseph Salisbury <joseph.salisbury@canonical.com>
Signed-off-by: Yinghai Lu <yinghai@kernel.org>


---
 drivers/pci/setup-bus.c |   32 +++++++++++++++++++++++++++-----
 1 file changed, 27 insertions(+), 5 deletions(-)

Index: linux-2.6/drivers/pci/setup-bus.c
===================================================================
--- linux-2.6.orig/drivers/pci/setup-bus.c
+++ linux-2.6/drivers/pci/setup-bus.c
@@ -1432,17 +1432,39 @@ static int iov_resources_unassigned(stru
 	return 0;
 }
 
+static bool pci_bus_mem_above_4g(struct pci_bus *bus)
+{
+	int i;
+	struct resource *res;
+
+	pci_bus_for_each_resource(bus, res, i) {
+		struct pci_bus_region region;
+
+		if (!res || !(res->flags & IORESOURCE_MEM))
+			continue;
+
+		__pcibios_resource_to_bus(bus, &region, res);
+		if (region.end > 0xffffffff)
+			return true;
+	}
+
+	return false;
+}
+
 static enum enable_type pci_realloc_detect(struct pci_bus *bus,
 			 enum enable_type enable_local)
 {
-	bool unassigned = false;
-
 	if (enable_local != undefined)
 		return enable_local;
 
-	pci_walk_bus(bus, iov_resources_unassigned, &unassigned);
-	if (unassigned)
-		return auto_enabled;
+	/* only enable auto when root bus does support 64bit mmio */
+	if (pci_bus_mem_above_4g(bus)) {
+		bool unassigned = false;
+
+		pci_walk_bus(bus, iov_resources_unassigned, &unassigned);
+		if (unassigned)
+			return auto_enabled;
+	}
 
 	return enable_local;
 }

Re: [REGRESSION][v3.4-rc1] PCI: add a PCI resource reallocation config option

[Joseph Salisbury]

On 11/28/2013 02:17 AM, Yinghai Lu wrote:
> On Wed, Nov 27, 2013 at 12:22 PM, Joseph Salisbury
> <joseph.salisbury@canonical.com> wrote:
>> Hi Yinghai,
>>
>> A kernel bug was opened against Ubuntu [0].  After a kernel bisect, it
>> was found that the following commit introduced the bug:
>>
>> commit b07f2ebc109b607789f648dedcff4b125f9afec6
>> Author: Yinghai Lu <yinghai@kernel.org>
>> Date:   Thu Feb 23 19:23:32 2012 -0800
>>
>>     PCI: add a PCI resource reallocation config option
>>
>>
>>
>> The regression was introduced as of v3.4-rc1 and also exists in current
>> mainline.
>>
>> The bug seems platform specific since we have not had allot of other
>> reports.  The bug is preventing ixgbe to probe Intel x520 NIC's.
>>
>> It would be easy enough to disable the PCI_REALLOC_ENABLE_AUTO config
>> option, but I wanted to get your feedback since you are the author.  It
>> looks like the bug reporter may also be able to work around the problem
>> with the pci=realloc=.
> The bios has problem to have two functions rom bar to same place, and
> root bus does
> not enough mmio range and confuse realloc logic.
>
> Please try attached patches to top of linus's tree, or your internal tree.
>
> Thanks
>
> Yinghai
Thanks so much for the patches, Yinghai.  I'll report back with the
testing results.

Thanks,

Joe

Re: [REGRESSION][v3.4-rc1] PCI: add a PCI resource reallocation config option

[Joseph Salisbury]

On 11/28/2013 02:17 AM, Yinghai Lu wrote:
> On Wed, Nov 27, 2013 at 12:22 PM, Joseph Salisbury
> <joseph.salisbury@canonical.com> wrote:
>> Hi Yinghai,
>>
>> A kernel bug was opened against Ubuntu [0].  After a kernel bisect, it
>> was found that the following commit introduced the bug:
>>
>> commit b07f2ebc109b607789f648dedcff4b125f9afec6
>> Author: Yinghai Lu <yinghai@kernel.org>
>> Date:   Thu Feb 23 19:23:32 2012 -0800
>>
>>     PCI: add a PCI resource reallocation config option
>>
>>
>>
>> The regression was introduced as of v3.4-rc1 and also exists in current
>> mainline.
>>
>> The bug seems platform specific since we have not had allot of other
>> reports.  The bug is preventing ixgbe to probe Intel x520 NIC's.
>>
>> It would be easy enough to disable the PCI_REALLOC_ENABLE_AUTO config
>> option, but I wanted to get your feedback since you are the author.  It
>> looks like the bug reporter may also be able to work around the problem
>> with the pci=realloc=.
> The bios has problem to have two functions rom bar to same place, and
> root bus does
> not enough mmio range and confuse realloc logic.
>
> Please try attached patches to top of linus's tree, or your internal tree.
>
> Thanks
>
> Yinghai

Hi Yinghai,

Your patches were tested against the 3.5 tree.  The patches did fix the
bug, and allowed the Intel x520 NIC's to work properly.  We were unable
to test this with Linus' tree due to another bug, which prevents the
system from booting.  We are looking deeper into that bug and should be
able to test the mainline tree at some point.

Testing against the 3.5 tree required that I cherry-pick the following
commits: ff35147, 55ed83a, 967260c and 223d96f.  However, that shouldn't
affect the test results of your patches.

Thanks for your assistance with this!


Joe

Re: [REGRESSION][v3.4-rc1] PCI: add a PCI resource reallocation config option

[Yinghai Lu]

On Mon, Dec 2, 2013 at 1:38 PM, Joseph Salisbury
<joseph.salisbury@canonical.com> wrote:
> Your patches were tested against the 3.5 tree.  The patches did fix the
> bug, and allowed the Intel x520 NIC's to work properly.  We were unable
> to test this with Linus' tree due to another bug, which prevents the
> system from booting.  We are looking deeper into that bug and should be
> able to test the mainline tree at some point.
>
> Testing against the 3.5 tree required that I cherry-pick the following
> commits: ff35147, 55ed83a, 967260c and 223d96f.  However, that shouldn't
> affect the test results of your patches.

Thanks.

Will repost them separately with stable tag.

Yinghai