PCI: Clear bridge MEM_64 flag if one child does not support it

Zermond and Marek reported 3.16 and later kernel broke the ati radeon support.
And it points to commit 5b28541552ef ("PCI: Restrict 64-bit prefetchable bridge windows to 64-bit resources").

The root causes:
BIOS does not assign resource for the bridge correctly.

[    0.113672] pci 0000:01:00.0: reg 0x10: [mem 0xc0000000-0xcfffffff pref]
[    0.113683] pci 0000:01:00.0: reg 0x14: [io  0xa000-0xa0ff]
[    0.113695] pci 0000:01:00.0: reg 0x18: [mem 0xfdff0000-0xfdffffff]
[    0.113729] pci 0000:01:00.0: reg 0x30: [mem 0xfdfc0000-0xfdfdffff pref]
[    0.113776] pci 0000:01:00.0: supports D1 D2
[    0.115016] pci 0000:00:01.0: PCI bridge to [bus 01]
[    0.115022] pci 0000:00:01.0:   bridge window [io  0x8000-0xafff]
[    0.115027] pci 0000:00:01.0:   bridge window [mem 0xfdf00000-0xfdffffff]
[    0.115034] pci 0000:00:01.0:   bridge window [mem 0xbdf00000-0xddefffff 64bit pref]
but
[    0.158601] pci 0000:00:01.0: address space collision: [mem

0xbdf00000-0xddefffff 64bit pref] conflicts with System RAM [mem 0x00100000-0xbff9ffff]
so kernel reject them, and try to allocate the resource to it.

Current code after the commit
1. when bridge does not support 64bit mmio pref, child mmio non 64bit will be
allocated from bridge mmio pref.
2. when bridge does support 64bit mmio pref, child mmio non 64bit will be allocated from bridge mmio.

In this case, mmio bar is not big enough and realloc is not triggerd and it will not be increased, so child device 01:00.0 will not get allocated mmio pref range.

Solution would be:
1. specify pci=realloc to scratch the bridge mmio register and get bigger range for it and then allocate to child mmio pref.
2. or scan the children resource other than ROM to clear bridge MEM_64 for mmio pref.

The patch is using second way so will keep child mmio pref into bridge mmio pref range.

---
 drivers/pci/setup-bus.c |   47 +++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 47 insertions(+)

Index: linux-2.6/drivers/pci/setup-bus.c
===================================================================
@@ -1118,6 +1118,52 @@ handle_done:
 	;
 }
 
+static void pbus_check_mem64(struct pci_bus *bus, unsigned long mask,
+			 unsigned long type, struct list_head *realloc_head)
+{
+	struct pci_dev *dev;
+	resource_size_t align;
+	resource_size_t aligns[18];	/* Alignments from 1Mb to 128Gb */
+	int order;
+	unsigned int mem64_mask = 0;
+	struct resource *b_res = find_free_bus_resource(bus, mask, type);
+
+	if (!b_res || !(b_res->flags & IORESOURCE_MEM_64))
+		return;
+
+	memset(aligns, 0, sizeof(aligns));
+
+	mem64_mask = IORESOURCE_MEM_64;
+	b_res->flags &= ~IORESOURCE_MEM_64;
+
+	list_for_each_entry(dev, &bus->devices, bus_list) {
+		int i;
+
+		for (i = 0; i < PCI_NUM_RESOURCES; i++) {
+			struct resource *r = &dev->resource[i];
+
+			if (r->parent || (r->flags & mask) != type)
+				continue;
+#ifdef CONFIG_PCI_IOV
+			if (realloc_head && i >= PCI_IOV_RESOURCES &&
+					i <= PCI_IOV_RESOURCE_END)
+				continue;
+#endif
+			align = pci_resource_alignment(dev, r);
+			order = __ffs(align) - 20;
+			if (order < 0)
+				order = 0;
+			if (order >= ARRAY_SIZE(aligns))
+				continue;
+
+			if (i != PCI_ROM_RESOURCE)
+				mem64_mask &= r->flags & IORESOURCE_MEM_64;
+		}
+	}
+
+	b_res->flags |= mem64_mask;
+}
+
 void __pci_bus_size_bridges(struct pci_bus *bus, struct list_head *realloc_head)
 {
 	struct pci_dev *dev;

@@ -1171,6 +1217,7 @@ void __pci_bus_size_bridges(struct pci_b
 		b_res = &bus->self->resource[PCI_BRIDGE_RESOURCES];
 		mask = IORESOURCE_MEM;
 		prefmask = IORESOURCE_MEM | IORESOURCE_PREFETCH;
+		pbus_check_mem64(bus, prefmask, prefmask, realloc_head);
 		if (b_res[2].flags & IORESOURCE_MEM_64) {
 			prefmask |= IORESOURCE_MEM_64;
 			ret = pbus_size_mem(bus, prefmask, prefmask,

That means that having a single ROM BAR that is 32-bit and prefetchable will downgrade the entire window to 32-bit ? That's not going to work either.

I have GPUs with 16G BARs for example... suddenly they don't fit anaymore because we downgraded the window to 32 bit because somewhere there's a 32-bit pref resource ?

That will break more than it fixes...

Ben.

I think an option would be to keep track of whether the PCI host bridge can do 64-bit pref above 4G.

If it can't then we know 64-bit pref is always fair game for 32-bit resources and we can thus downgrade all the bridges 64-pref to 32-perf.

If it can, then we probably do need to leave it there do real 64-bit pref, and shoot anything that is 32-bit only down the non-pref windows.

We *could* try to be smart and scan first to check if anything under the bridge actually has large/64-bit BARs but that's going to be a heuristic at best and isn't going to do any good with hotplug.

I tend to think that treating anything 32-bit pref as non-pref is in fact the best solution unless we know that the platform doesn't do

Ben.

Yes, that could be enhancement.

That should fix the problem Marek's

But not on Zermond's, as his system is not using _CRS.

Or we can enforce new rule that no _CRS then only use under 4G ?

Yes, but will need to enable pci=realloc automatically, so we can take back control
of bridge mmio bar.

Thanks

Yinghai

http://patchwork.ozlabs.org/patch/419295/

PCI: Clear all bridge res MEM_64 if host bridge has non mem64

the ROM BAR get skipped during the checking.

+ if (i != PCI_ROM_RESOURCE)
+ mem64_mask &= r->flags & IORESOURCE_MEM_64;
+ }

Please check if this patch break your platform. I tried on my setup on x86. and it is still
working on 64 bit resource allocation.

Thanks

Yinghai

I'm going to give it a spin and Richard, could you please apply Yinghai's patch to see if your SRIOV code can work properly?

Thanks,
Gavin

Yinghai & Gavin,

I did a quick test on my machine. This patch doesn't affect the MMIO allocation on out platform, so SRIOV works fine.

I will spend more time to read the patch to get more understanding about the problem.

Richard Yang
Help you, Help me

Hi Marek,

Can you boot following branch with "debug ignore_loglevel pci=realloc" on your setup?

git://git.kernel.org/pub/scm/linux/kernel/git/yinghai/linux-yinghai.git
for-pci-allocate-fit-3.18

it has
8f74af9: PCI, x86: Allocate mmio near end of free range 1ff91c1: PCI: Don't allocate small resource in big free space. 108b43b: resources: Add allocate_resource_fit()
b6a22f0: resources: Make find_resource could return just fit resource
cf50e16: resources: Split out __allocate_resource()

on top the v3.18, and it will allocate resource in fit way.

Thanks

Yinghai

Hi, Yinghai

Just spent some time to understand the problem and your patch.

Two comments to this patch:
1. realloc_head is not necessary here
2. In case we have two MEM BAR under a bus, one is MEM64|PREF and one is PREF, after this patch, the MEM64 flag will be cleared, right? If so, this may fall back to the same situation in bug 74151.

I am looking at the bugzilla and try to understand the root cause for this allocation failure. Currently I don't get the reason. In my mind, the resource will be sized in the non-pref bridge window. But seems not. I have asked more info in the bugzilla, so that I could understand the problem. (Yeah, maybe I am not that smart, if one of you would help me understand the problem, I am happy to offer you a cup of coffee :-) )

Bjorin,

As you mentioned in another thread, "5b28541552ef is taking the wrong approach". (http://www.spinics.net/lists/linux-pci/msg37374.html) Maybe I don't catch it clearly. Put a 32bit prefetchable resource in a 32bit non-prefetchable bridge window is a bad idea? But in my mind, if the bridge prefetchable window is 64bit, we can't put a 32bit prefetchable resource in it.

I am trying to catch up with the updates of the bug, if I missed something just let me know.

Richard Yang
Help you, Help me

A 32-bit prefetchable resource *can* be put in a 32-bit non-prefetchable window, but the device won't perform as well as it would if the resource were in a prefetchable window.

What I object to is the fact that we put a 32-bit prefetchable resource in the non-prefetchable window and leave the 64-bit prefetchable window unused. This gives up performance for no benefit.

If the window is programmed to be above 4GB, of course we can't put a 32-bit resource in it. My point is that if the bridge *supports* a 64-bit prefetchable window, we can decide where to place it. If we put the window below 4GB, we can put a 32-bit prefetchable resource in it.

I think maybe you're thinking of "64-bit window" as "a window programmed to be above 4GB." I'm using "64-bit window" to mean "a window that supports 64-bit addressing," i.e., one where PCI_PREF_BASE_UPPER32 and PCI_PREF_LIMIT_UPPER32 are implemented. That's analogous to the way we talk about 64-bit BARs. A 64-bit BAR is still a 64-bit BAR even if it is currently programmed to be below 4GB.

Bjorn

OK, I get your point. This is really a waste to put a 32-bit prefetchable resource in non-prefetchable window and no one use the 64-bit prefetchable window. (That's very interesting even no performance lost by doing so as mentioned in reply from Marek. Maybe the reason is the most important IO is performed in DMA.)

Below is what's in my mind, if not correct please let me know.

1. Yinghai's patch needs a little modification

Now back to Yinghai's patch. If this patch wants to apply this logic it should check all prefetchable resource, including M64 and non-M64

if *none* of them has M64 prefetchable resource, then clear the M64 flag in bridge.

But the logic in patch is

if *any* of them has non-M64 prefetchable resource, then clear the M64 flag in bridge.

2. Yes, the strategy will improve the performance in some case, but with limited case.

Then suppose we use the strategy, clear the M64 flag in bridge when none of the child resource need it.

I imagined this scenario:

                       +--------------+
                       |B1            |
                       +------+-------+
                              |  Bus#1

---+--------------+-------------------+----
| Bus#2 | Bus#3
+------+------+ +------+-------+
|B2 | |B3 |
+-------------+ +--------------+
res[1] 32-bit non-pref res[1] 32-bit non-pref
res[2] 64-bit pref res[2] 32-bit pref

Suppose we have a PCI sub-tree like this. And suppose all the bus->resource[2] is with M64 flag before sizing. (The initial value is retrieved in enumeration stage by pci_read_bridge_bases(). This happens before sizing.)

We will first size Bus#2, then Bus#3, at last Bus#1. As shown in the chart,

    For Bus#2, there is a 64-bit pref, so Bus#2->resource[2] with M64 flag set.
    For Bus#3, there is no 64-bit pref, so Bus#3->resource[2] with M64 flag
    cleared.
    Then up to Bus#1, since there is a 64-bit prefetchable resource, the
    Bus#1->resource[2] is with M64 flag set.

The final B1's window will look like this:

         +--------------------------------------+
         | B1                                   |
         |                                      |
	 | res[1] 32-bit non-pref               |
         | +---------+----------+----------+    |
	 | |B2.res[1]| B3.res[1]| B3.res[2]|    |
         | +---------+----------+----------+    |
	 |                                      |
         | res[2] 64-bit pref                   |
         | +----------------+                   |
         | |B2.res[2]       |                   |
         | +----------------+                   |
         +--------------------------------------+

B3.res[2] will bring some performance improvement to those devices under it. But from its grand parent's point of view, B3.res[2] still sits under a non-prefetchable window. So the performance improvement will be limited. Hope my understanding is correct.

So the best case for the performance improvement is all this PCI tree has no 64-bit prefetchable resource.

B3.res[2] still contribute to the root's non-prefetchable window, once there is a 64-bit prefetchable in the PCI tree. If we don't have enough non-prefetchable window in root, we would fail.

Bjorn, I agree to apply the logic you mentioned to clear the M64 flag when no child need it. But the benefits is limited.

I did a grep on the lspci -vvv output and see there is no 64-bit prefetchable BAR in the system.

$ grep Region lspci | grep Me
	Region 0: Memory at febff400 (32-bit, non-prefetchable) [size=1K]
	Region 0: Memory at febf8000 (64-bit, non-prefetchable) [size=16K]
	Region 0: Memory at febff000 (32-bit, non-prefetchable) [size=1K]
	Region 5: Memory at febff800 (32-bit, non-prefetchable) [size=2K]
	Region 0: Memory at c0000000 (32-bit, prefetchable) [size=256M]
	Region 2: Memory at fdff0000 (32-bit, non-prefetchable) [size=64K]
	Region 0: Memory at fe0c0000 (64-bit, non-prefetchable) [size=256K]
	Region 0: Memory at fe1fe000 (64-bit, non-prefetchable) [size=8K]
	Region 0: Memory at feaffc00 (32-bit, non-prefetchable) [size=1K]

I think this is the reason why this patch could help on this case.

3. Not sure why the non-prefetchable window becomes bigger on 3.16

I past the fragment of /proc/iomem from Marek.

3.15 good version

c0000000-ffffffff : PCI Bus 0000:00
c0000000-cfffffff : PCI Bus 0000:01

    c0000000-cfffffff : 0000:01:00.0
  d0000000-d01fffff : PCI Bus 0000:08
  ddf00000-dfefffff : PCI Bus 0000:06
  e0000000-efffffff : PCI MMCONFIG 0000 [bus 00-ff]
    e0000000-efffffff : pnp 00:0b
  fdf00000-fdffffff : PCI Bus 0000:01
    fdfc0000-fdfdffff : 0000:01:00.0
    fdff0000-fdffffff : 0000:01:00.0

3.16 bad version

c0000000-ffffffff : PCI Bus 0000:00
c0000000-c01fffff : PCI Bus 0000:01 c0200000-c03fffff : PCI Bus 0000:08 ddf00000-dfefffff : PCI Bus 0000:06 e0000000-efffffff : PCI MMCONFIG 0000 [bus 00-ff]

    e0000000-efffffff : pnp 00:07
  fdf00000-fdffffff : PCI Bus 0000:01
    fdfc0000-fdfdffff : 0000:01:00.0
    fdff0000-fdffffff : 0000:01:00.0

On 3.15, one BAR in device 01:00.0 is allocated from c0000000-cffffffff. While on 3.16, we need to allocate the same BAR from fdf0000000 range. But we see this range keeps the same size as in 3.15. I don't catch the reason for this.

My thoughts is if we could have a bigger range in fdf00000, the non-prefetch window in Bus#01, the Graphic card could work too.

Yes, agree.

If the 64-bit prefetchable window contains some space under 4G, we could put some 32-bit prefetchable resource in it. But this would bring more complexity to the sizing algorithm. We need a separate value to track the size below 4G in 64-bit prefetchable window, then to see who would have the luck to fit in. And those can't the ticket would be sit in non-prefetchable window.

Thanks for reminding. It makes me more clear.

Let me make my statement more clear.

If the 64-bit window is enabled, like the PCI_PREF_BASE_UPPER32/PCI_PREF_BASE_UPPER32 is set. And this address is set the M64 flag, which means the range contains space above 4G. In this case, we can't put a 32-bit resource in this window.

If my understanding is not correct, please let me know.

Richard Yang
Help you, Help me

why ?

want to align that with pbus_size_mem.

Yes.

Thanks

Yinghai

Hi Yinghai,

I have built and booted your branch and it works well. Do you want me to attach some logs?
(I am new here and I have read http://www.tux.org/lkml/ and I don't want to break some rules for attachment size)

Marek

Please attach logs to the bugzilla and make their type "text/plain". That's the easiest to browse.

Bjorn

Hi Marek,

Can you run some graphics benchmark program to check the performance between
1.

git://git.kernel.org/pub/scm/linux/kernel/git/yinghai/linux-yinghai.git
branch: for-pci-allocate-fit-3.18

2.
v 3.18 + clear mmio64 flags when children device does not support it

Thanks

Yinghai

I have run Unigine Heaven benchmark on both versions (I tried version 2. with and also without kernel parameters "debug ignore_loglevel pci=realloc") and the performance of each version was the same (70-71 points). I tried to run this benchmark also with kernel 3.15.10 and the result was 67 points. Tomorrow I can try to run some more benchmarks, today I didn't have much time.

Marek

so putting mem pref under bridge mem does not cause performance loss?

Thanks

Yinghai

Yinghai, With the patch, the Power8 machine boots up and have resources assigned properly: 64-bits pref BARs are covered by 64-bits pref windows and left memory BARs are covered by 32-bits non-pref windows. Note that I didn't find an 32-bits-pref BAR on the system.

For SRIOV case, Richard still need test.

Thanks,
Gavin

Good, Thanks for the testing.

Yinghai