Re: [PATCH 6/6] PCI: Try to allocate mem64 above 4G at first

[Yinghai Lu <yinghai@kernel.org>]

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

On Thu, Nov 21, 2013 at 2:30 AM, Guo Chao <yan@linux.vnet.ibm.com> wrote:
> Hi:
>
> On Tue, Nov 19, 2013 at 05:51:57PM -0800, Yinghai Lu wrote:
>> Will fall back to below 4g if it can not find any above 4g.
>>
>
>
>
>
>> x86 32bit without X86_PAE support will have bottom set to 0, because
>> resource_size_t is 32bit.
>>
>> Also for 32bit with resource_size_t 64bit kernel on machine with pae support
>> we are safe because iomem_resource is limited to 32bit according to
>> x86_phys_bits.
>>
>> -v2: update bottom assigning to make it clear for non-pae support machine.
>> -v3: Bjorn's change:
>>         use MAX_RESOURCE instead of -1
>>         use start/end instead of bottom/max
>>         for all arch instead of just x86_64
>> -v4: updated after PCI_MAX_RESOURCE_32 change.
>> -v5: restore io handling to use PCI_MAX_RESOURCE_32 as limit.
>> -v6: checking pcibios_resource_to_bus return for every bus res, to decide it
>>       if we need to try high at first.
>>      It supports all arches instead of just x86_64.
>>

> Work fine in our systems if '[RFC PATCH 3/3] PCI: do not reset bridge's
> IORESOURCE_MEM_64 flag for ROM BAR' applied.
>
> Otherwise, in one system, the 32-bit window is too small to provide
> fallback space for prefetchable windows of root bridge, causing all
> prefethable resources failed to get addresses.
>
> Any comments about that patch?

no, that patch is not right.
That could prevent rom BAR getting allocate under 4G.

solution could be:
1. Just remove pref on rom bar allocation. as attached rom_no_pref.patch
2. or treat pref rom as option resource, as attached rom_option_1_xxx.patch
3. or more generic, treat all pci BAR 32bit prefetechable as normal
MMIO32 during allocation. aka mmio prefectechable will be used for pci
bridge that support 64bit mmio pref and leave bridge's 32bit only pref
bar register blank.

Bjorn, Linus,
Are you happy with No 3?

Thanks

Yinghai

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

Subject: [PATCH] PCI: Don't allocate rom bar in bridge pref resource

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

---
 drivers/pci/probe.c |    5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

Index: linux-2.6/drivers/pci/probe.c
===================================================================
--- linux-2.6.orig/drivers/pci/probe.c
+++ linux-2.6/drivers/pci/probe.c
@@ -336,9 +336,8 @@ static void pci_read_bases(struct pci_de
 	if (rom) {
 		struct resource *res = &dev->resource[PCI_ROM_RESOURCE];
 		dev->rom_base_reg = rom;
-		res->flags = IORESOURCE_MEM | IORESOURCE_PREFETCH |
-				IORESOURCE_READONLY | IORESOURCE_CACHEABLE |
-				IORESOURCE_SIZEALIGN;
+		res->flags = IORESOURCE_MEM | IORESOURCE_READONLY |
+				IORESOURCE_CACHEABLE | IORESOURCE_SIZEALIGN;
 		__pci_read_base(dev, pci_bar_mem32, res, rom);
 	}
 }

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

Subject: [PATCH] PCI: Treat ROM resource as optional during assigning.

So will try to allocate them together with requested ones, if can not assign
them, could go with requested one only, and just skip ROM resource.

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

---
 drivers/pci/setup-bus.c |   21 +++++++--------------
 1 file changed, 7 insertions(+), 14 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
@@ -303,18 +303,10 @@ static void assign_requested_resources_s
 		idx = pci_dev_resource_idx(dev_res->dev, res);
 		if (resource_size(res) &&
 		    pci_assign_resource_fit(dev_res->dev, idx, fit)) {
-			if (fail_head) {
-				/*
-				 * if the failed res is for ROM BAR, and it will
-				 * be enabled later, don't add it to the list
-				 */
-				if (!((idx == PCI_ROM_RESOURCE) &&
-				      (!(res->flags & IORESOURCE_ROM_ENABLE))))
-					add_to_list(fail_head,
-						    dev_res->dev, res,
-						    0 /* don't care */,
-						    0 /* don't care */);
-			}
+			if (fail_head)
+				add_to_list(fail_head, dev_res->dev, res,
+					    0 /* don't care */,
+					    0 /* don't care */);
 			reset_resource(res);
 		}
 	}
@@ -903,8 +895,9 @@ static int pbus_size_mem(struct pci_bus
 				continue;
 			r_size = resource_size(r);
 
-			/* put SRIOV requested res to the optional list */
-			if (realloc_head && is_pci_iov_resource_idx(i)) {
+			/* put SRIOV/ROM requested res to the optional list */
+			if (realloc_head && (is_pci_iov_resource_idx(i) ||
+					     is_pci_rom_resource_idx(i))) {
 				r->end = r->start - 1;
 				add_to_list(realloc_head, dev, r, r_size, 0/* dont' care */);
 				children_add_size += r_size;