[RFC PATCH 0/3] Make 64-bit prefetchable MMIO work

[RFC PATCH 0/3] Make 64-bit prefetchable MMIO work

[Guo Chao]

Resource assignment code does not work properly after we expose 64-bit
prefetchable MMIO window in PowerNV platform.

  # ROM BAR get 4G-above address.
	This is because default value of PCIBIOS_MAX_MEM_32 is -1
	which makes it useless in 64-bit platform. After
	fixing this ...

  # ROM BAR can't get any address.
	This is because PCIBIOS_MAX_MEM_32 is actually a PCI address
	but functions as a CPU address. After fixing this ...

  # 64-bit prefetchable address is never used.
	This is because root bridge's IORESOURCE_MEM_64 is reset due to
	ROM BARs, which makes it not quilify to get 4G-above prefetchable
	address from host bridge. The 64-bit 4G-above prefetchable window
	is not used at all in the end.

Nothing is really broken currently. This is just a RFC series, preparing
for later PowerNV 64-bit MMIO work.

Guo Chao (3):
  PCI: do not compare CPU address with PCI address
  PCI: set proper default value of PCIBIOS_MAX_MEM_32
  PCI: do not reset bridge's IORESOURCE_MEM_64 flag for ROM BAR

 arch/x86/include/asm/pci.h |  1 -
 drivers/pci/bus.c          | 65 ++++++++++++++++++++++++++++++++++++++++++++--
 drivers/pci/setup-bus.c    |  3 ++-
 include/linux/pci.h        |  2 +-
 4 files changed, 66 insertions(+), 5 deletions(-)

-- 
1.8.3.2

[RFC PATCH 1/3] PCI: do not compare CPU address with PCI address

[Guo Chao]

In resource assignment code, limits are exerted to restrict allocated
resource range. However these limits are PCI address but compared to
CPU address in the end. Translated them before comparing.

We can't just use pcibios_bus_to_resource because the limits may not
included in the host bridge window. Introduce a help function to
do this translation, if address missed, return an approximate one.

Signed-off-by: Guo Chao <yan@linux.vnet.ibm.com>
---
 drivers/pci/bus.c | 65 +++++++++++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 63 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/bus.c b/drivers/pci/bus.c
index fc1b740..532c0a4 100644
--- a/drivers/pci/bus.c
+++ b/drivers/pci/bus.c
@@ -99,6 +99,64 @@ void pci_bus_remove_resources(struct pci_bus *bus)
 }
 
 /**
+ * pci_bus_to_resource
+ *
+ * Much like pcibios_bus_to_resource() except it takes a single address
+ * and returns an approximate one if target address is not included
+ * in the bridge window. The approximate address is smaller than required
+ * one is 'bound' is 1, larger than required one if 'bound' is 0.
+ */
+static resource_size_t pci_bus_to_resource(struct pci_bus *bus, int flags,
+				    int mask, resource_size_t addr, int bound)
+{
+	struct pci_host_bridge *bridge;
+	struct pci_host_bridge_window *window, *match = NULL;
+	resource_size_t max = 0, min = -1;
+	resource_size_t offset = -1, start, end;
+
+	while (bus->parent)
+		bus = bus->parent;
+
+	bridge = to_pci_host_bridge(bus->bridge);
+
+	list_for_each_entry(window, &bridge->windows, list) {
+		if ((flags ^ window->res->flags) & mask)
+			continue;
+
+		start = window->res->start - window->offset;
+		end = window->res->end - window->offset;
+
+		if (addr >= start && addr <= end) {
+			offset = window->offset;
+			break;
+		}
+
+		if (bound && addr > end && end > max) {
+			max = end;
+			match = window;
+		} else if (!bound && addr < start && start < min) {
+			min = start;
+			match = window;
+		}
+	}
+
+	if (offset == -1) {
+		/*
+		 * Not even found the matched type. This may happen,
+		 * for example, if try to translate IO address in a HB
+		 * without IO window. Just return the original address,
+		 * it will fail later anyway.
+		 */
+		if (match == NULL)
+			return addr;
+
+		return (bound ? max : min) + match->offset;
+	}
+
+	return addr + offset;
+}
+
+/**
  * pci_bus_alloc_resource - allocate a resource from a parent bus
  * @bus: PCI bus
  * @res: resource to allocate
@@ -129,9 +187,12 @@ pci_bus_alloc_resource(struct pci_bus *bus, struct resource *res,
 
 	type_mask |= IORESOURCE_IO | IORESOURCE_MEM;
 
-	/* don't allocate too high if the pref mem doesn't support 64bit*/
+	/* don't allocate too high if the pref mem doesn't support 64bit */
 	if (!(res->flags & IORESOURCE_MEM_64))
-		max = PCIBIOS_MAX_MEM_32;
+		max = pci_bus_to_resource(bus, res->flags, type_mask,
+							PCIBIOS_MAX_MEM_32, 1);
+
+	min = pci_bus_to_resource(bus, res->flags, type_mask, min, 0);
 
 	pci_bus_for_each_resource(bus, r, i) {
 		if (!r)
-- 
1.8.3.2

[RFC PATCH 2/3] PCI: set proper default value of PCIBIOS_MAX_MEM_32

[Guo Chao]

PCIBIOS_MAX_MEM_32 is used to prevent 32-bit BAR from getting 4G-above
address. Its default value -1 extends to 0xffffffffffffffff in 64-bit
platform which make it useless at all. While arch can overwrite it,
0xffffffff should be good enough for those who does not have special
requirement. Set PCIBIOS_MAX_MEM_32 to 0xffffffff and kill this
definition in x86.

Signed-off-by: Guo Chao <yan@linux.vnet.ibm.com>
---
 arch/x86/include/asm/pci.h | 1 -
 include/linux/pci.h        | 2 +-
 2 files changed, 1 insertion(+), 2 deletions(-)

diff --git a/arch/x86/include/asm/pci.h b/arch/x86/include/asm/pci.h
index 7d74432..73ff4bc 100644
--- a/arch/x86/include/asm/pci.h
+++ b/arch/x86/include/asm/pci.h
@@ -125,7 +125,6 @@ int setup_msi_irq(struct pci_dev *dev, struct msi_desc *msidesc,
 
 /* generic pci stuff */
 #include <asm-generic/pci.h>
-#define PCIBIOS_MAX_MEM_32 0xffffffff
 
 #ifdef CONFIG_NUMA
 /* Returns the node based on pci bus */
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 835ec7b..f43405d 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -1487,7 +1487,7 @@ static inline struct pci_dev *pci_dev_get(struct pci_dev *dev)
 #include <asm/pci.h>
 
 #ifndef PCIBIOS_MAX_MEM_32
-#define PCIBIOS_MAX_MEM_32 (-1)
+#define PCIBIOS_MAX_MEM_32 (0xffffffff)
 #endif
 
 /* these helpers provide future and backwards compatibility
-- 
1.8.3.2

[RFC PATCH 3/3] PCI: do not reset bridge's IORESOURCE_MEM_64 flag for ROM BAR

[Guo Chao]

If 32-bit prefetchable BARs detected, the prefetchable resource's
IORESOURCE_MEM_64 flag of its upstream bridge will be reset. This in
turn causes all upstream bridges up to the root bridge have their
IORESOURCE_MEM_64 reset. If prefetchable windows in a system located
above 4G, then they are never used for the sake of 32-bit prefetchable BAR.

Desert 64-bit prefetchable windows for ROM BAR seems unreasonable.
Just don't reset bridge's IORESOURCE_MEM_64 flag in case of ROM BARs.
They will fail to get address from 4G-above prefetchable window and
fall back to non-prefetchable window within 4G.

Signed-off-by: Guo Chao <yan@linux.vnet.ibm.com>
---
 drivers/pci/setup-bus.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
index 4ce83b2..e9006b9 100644
--- a/drivers/pci/setup-bus.c
+++ b/drivers/pci/setup-bus.c
@@ -974,7 +974,8 @@ static int pbus_size_mem(struct pci_bus *bus, unsigned long mask,
 				aligns[order] += align;
 			if (order > max_order)
 				max_order = order;
-			mem64_mask &= r->flags & IORESOURCE_MEM_64;
+			if (i != PCI_ROM_RESOURCE)
+				mem64_mask &= r->flags & IORESOURCE_MEM_64;
 
 			if (realloc_head)
 				children_add_size += get_res_add_size(realloc_head, r);
-- 
1.8.3.2

Re: [RFC PATCH 1/3] PCI: do not compare CPU address with PCI address

[Yinghai Lu]

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

On Mon, Nov 18, 2013 at 10:36 PM, Guo Chao <yan@linux.vnet.ibm.com> wrote:
> In resource assignment code, limits are exerted to restrict allocated
> resource range. However these limits are PCI address but compared to
> CPU address in the end. Translated them before comparing.
>
> We can't just use pcibios_bus_to_resource because the limits may not
> included in the host bridge window. Introduce a help function to
> do this translation, if address missed, return an approximate one.
>
> Signed-off-by: Guo Chao <yan@linux.vnet.ibm.com>
> ---
>  drivers/pci/bus.c | 65 +++++++++++++++++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 63 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/pci/bus.c b/drivers/pci/bus.c
> index fc1b740..532c0a4 100644
> --- a/drivers/pci/bus.c
> +++ b/drivers/pci/bus.c
> @@ -99,6 +99,64 @@ void pci_bus_remove_resources(struct pci_bus *bus)
>  }
>
>  /**
> + * pci_bus_to_resource
> + *
> + * Much like pcibios_bus_to_resource() except it takes a single address
> + * and returns an approximate one if target address is not included
> + * in the bridge window. The approximate address is smaller than required
> + * one is 'bound' is 1, larger than required one if 'bound' is 0.
> + */
> +static resource_size_t pci_bus_to_resource(struct pci_bus *bus, int flags,
> +                                   int mask, resource_size_t addr, int bound)
> +{
> +       struct pci_host_bridge *bridge;
> +       struct pci_host_bridge_window *window, *match = NULL;
> +       resource_size_t max = 0, min = -1;
> +       resource_size_t offset = -1, start, end;
> +
> +       while (bus->parent)
> +               bus = bus->parent;
> +
> +       bridge = to_pci_host_bridge(bus->bridge);
> +
> +       list_for_each_entry(window, &bridge->windows, list) {
> +               if ((flags ^ window->res->flags) & mask)
> +                       continue;
> +
> +               start = window->res->start - window->offset;
> +               end = window->res->end - window->offset;
> +
> +               if (addr >= start && addr <= end) {
> +                       offset = window->offset;
> +                       break;
> +               }
> +
> +               if (bound && addr > end && end > max) {
> +                       max = end;
> +                       match = window;
> +               } else if (!bound && addr < start && start < min) {
> +                       min = start;
> +                       match = window;
> +               }
> +       }
> +
> +       if (offset == -1) {
> +               /*
> +                * Not even found the matched type. This may happen,
> +                * for example, if try to translate IO address in a HB
> +                * without IO window. Just return the original address,
> +                * it will fail later anyway.
> +                */
> +               if (match == NULL)
> +                       return addr;
> +
> +               return (bound ? max : min) + match->offset;
> +       }
> +
> +       return addr + offset;
> +}

that is confusing.

Can we use http://lkml.indiana.edu/hypermail/linux/kernel/1206.0/00437/pcibus_addr_converting_bus.patch
instead?

or as attached:

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

Subject: [PATCH] PCI: pcibus address to resource converting take bus directly

For allocating resource under bus path, we do have dev pass along, and we
could just use bus instead.

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

---
 drivers/pci/host-bridge.c |   34 +++++++++++++++++++++-------------
 include/linux/pci.h       |    3 +++
 2 files changed, 24 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)
+static 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
@@ -792,6 +792,9 @@ 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_dev *dev, struct resource *res,

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

Subject: [PATCH] PCI: Add pcibios_bus_addr_to_res()

it takes addr and return converted address only.

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

---
 drivers/pci/host-bridge.c |   14 ++++++++++++++
 include/linux/pci.h       |    2 ++
 2 files changed, 16 insertions(+)

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
@@ -102,3 +102,17 @@ void pcibios_bus_to_resource(struct pci_
 	__pcibios_bus_to_resource(dev->bus, res, region);
 }
 EXPORT_SYMBOL(pcibios_bus_to_resource);
+
+resource_size_t pcibios_bus_addr_to_res(struct pci_bus *bus, int flags,
+					resource_size_t addr)
+{
+	struct pci_bus_region region;
+	struct resource r;
+
+	r.flags = flags;
+	region.start = addr;
+	region.end = addr;
+	__pcibios_bus_to_resource(bus, &r, &region);
+
+	return r.end;
+}
Index: linux-2.6/include/linux/pci.h
===================================================================
--- linux-2.6.orig/include/linux/pci.h
+++ linux-2.6/include/linux/pci.h
@@ -799,6 +799,8 @@ void pcibios_resource_to_bus(struct pci_
 			     struct resource *res);
 void pcibios_bus_to_resource(struct pci_dev *dev, struct resource *res,
 			     struct pci_bus_region *region);
+resource_size_t pcibios_bus_addr_to_res(struct pci_bus *bus, int flags,
+					resource_size_t addr);
 void pcibios_scan_specific_bus(int busn);
 struct pci_bus *pci_find_bus(int domain, int busnr);
 void pci_bus_add_devices(const struct pci_bus *bus);

Re: [RFC PATCH 1/3] PCI: do not compare CPU address with PCI address

[Yinghai Lu]

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

On Tue, Nov 19, 2013 at 11:31 AM, Yinghai Lu <yinghai@kernel.org> wrote:
> On Mon, Nov 18, 2013 at 10:36 PM, Guo Chao <yan@linux.vnet.ibm.com>
> Can we use http://lkml.indiana.edu/hypermail/linux/kernel/1206.0/00437/pcibus_addr_converting_bus.patch
> instead?
>
> or as attached:

and

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

Subject: [PATCH] PCI: Try to allocate mem64 above 4G at first

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.

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

---
 arch/x86/include/asm/pci.h |    1 -
 drivers/pci/bus.c          |   42 ++++++++++++++++++++++++++++++++++--------
 drivers/pci/pci.h          |    2 ++
 include/linux/pci.h        |    4 ----
 4 files changed, 36 insertions(+), 13 deletions(-)

Index: linux-2.6/arch/x86/include/asm/pci.h
===================================================================
--- linux-2.6.orig/arch/x86/include/asm/pci.h
+++ linux-2.6/arch/x86/include/asm/pci.h
@@ -155,7 +155,6 @@ void default_restore_msi_irqs(struct pci
 
 /* generic pci stuff */
 #include <asm-generic/pci.h>
-#define PCIBIOS_MAX_MEM_32 0xffffffff
 
 #ifdef CONFIG_NUMA
 /* Returns the node based on pci bus */
Index: linux-2.6/drivers/pci/bus.c
===================================================================
--- linux-2.6.orig/drivers/pci/bus.c
+++ linux-2.6/drivers/pci/bus.c
@@ -125,15 +125,13 @@ pci_bus_alloc_resource(struct pci_bus *b
 {
 	int i, ret = -ENOMEM;
 	struct resource *r;
-	resource_size_t max = -1;
 
 	type_mask |= IORESOURCE_IO | IORESOURCE_MEM;
 
-	/* don't allocate too high if the pref mem doesn't support 64bit*/
-	if (!(res->flags & IORESOURCE_MEM_64))
-		max = PCIBIOS_MAX_MEM_32;
-
 	pci_bus_for_each_resource(bus, r, i) {
+		resource_size_t start, end, middle;
+		struct pci_bus_region region;
+
 		if (!r)
 			continue;
 
@@ -147,14 +145,42 @@ pci_bus_alloc_resource(struct pci_bus *b
 		    !(res->flags & IORESOURCE_PREFETCH))
 			continue;
 
+		start = 0;
+		end = MAX_RESOURCE;
+		/*
+		 * don't allocate too high if the pref mem doesn't
+		 * support 64bit, also if this is a 64-bit mem
+		 * resource, try above 4GB first
+		 */
+		__pcibios_resource_to_bus(bus, &region, r);
+		if (region.start <= PCI_MAX_ADDR_32 &&
+		    region.end > PCI_MAX_ADDR_32) {
+			middle = pcibios_bus_addr_to_res(bus, res->flags,
+						      PCI_MAX_ADDR_32);
+			if (res->flags & IORESOURCE_MEM_64)
+				start = middle + 1;
+			else
+				end = middle;
+		} else if (region.start > PCI_MAX_ADDR_32 &&
+			   !(res->flags & IORESOURCE_MEM_64))
+				continue;
+
+again:
 		/* Ok, try it out.. */
 		ret = allocate_resource(r, res, size,
-					r->start ? : min,
-					max, align,
+					max(start, r->start ? : min),
+					end, align,
 					alignf, alignf_data);
 		if (ret == 0)
-			break;
+			return 0;
+
+		if (start != 0) {
+			start = 0;
+			goto again;
+		}
 	}
+
+
 	return ret;
 }
 
Index: linux-2.6/drivers/pci/pci.h
===================================================================
--- linux-2.6.orig/drivers/pci/pci.h
+++ linux-2.6/drivers/pci/pci.h
@@ -195,6 +195,8 @@ enum pci_bar_type {
 	pci_bar_mem64,		/* A 64-bit memory BAR */
 };
 
+#define PCI_MAX_ADDR_32	((resource_size_t)0xffffffff)
+
 bool pci_bus_read_dev_vendor_id(struct pci_bus *bus, int devfn, u32 *pl,
 				int crs_timeout);
 int pci_setup_device(struct pci_dev *dev);
Index: linux-2.6/include/linux/pci.h
===================================================================
--- linux-2.6.orig/include/linux/pci.h
+++ linux-2.6/include/linux/pci.h
@@ -1539,10 +1539,6 @@ static inline struct pci_host_bridge *pc
 
 #include <asm/pci.h>
 
-#ifndef PCIBIOS_MAX_MEM_32
-#define PCIBIOS_MAX_MEM_32 (-1)
-#endif
-
 /* these helpers provide future and backwards compatibility
  * for accessing popular PCI BAR info */
 static inline resource_size_t pci_resource_start(const struct pci_dev *dev,

Re: [RFC PATCH 1/3] PCI: do not compare CPU address with PCI address

[Bjorn Helgaas]

On Tue, Nov 19, 2013 at 12:31 PM, Yinghai Lu <yinghai@kernel.org> wrote:
> On Mon, Nov 18, 2013 at 10:36 PM, Guo Chao <yan@linux.vnet.ibm.com> wrote:
>> In resource assignment code, limits are exerted to restrict allocated
>> resource range. However these limits are PCI address but compared to
>> CPU address in the end. Translated them before comparing.
>>
>> We can't just use pcibios_bus_to_resource because the limits may not
>> included in the host bridge window. Introduce a help function to
>> do this translation, if address missed, return an approximate one.
>>
>> Signed-off-by: Guo Chao <yan@linux.vnet.ibm.com>
>> ---
>>  drivers/pci/bus.c | 65 +++++++++++++++++++++++++++++++++++++++++++++++++++++--
>>  1 file changed, 63 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/pci/bus.c b/drivers/pci/bus.c
>> index fc1b740..532c0a4 100644
>> --- a/drivers/pci/bus.c
>> +++ b/drivers/pci/bus.c
>> @@ -99,6 +99,64 @@ void pci_bus_remove_resources(struct pci_bus *bus)
>>  }
>>
>>  /**
>> + * pci_bus_to_resource
>> + *
>> + * Much like pcibios_bus_to_resource() except it takes a single address
>> + * and returns an approximate one if target address is not included
>> + * in the bridge window. The approximate address is smaller than required
>> + * one is 'bound' is 1, larger than required one if 'bound' is 0.
>> + */
>> +static resource_size_t pci_bus_to_resource(struct pci_bus *bus, int flags,
>> +                                   int mask, resource_size_t addr, int bound)
>> +{
>> +       struct pci_host_bridge *bridge;
>> +       struct pci_host_bridge_window *window, *match = NULL;
>> +       resource_size_t max = 0, min = -1;
>> +       resource_size_t offset = -1, start, end;
>> +
>> +       while (bus->parent)
>> +               bus = bus->parent;
>> +
>> +       bridge = to_pci_host_bridge(bus->bridge);
>> +
>> +       list_for_each_entry(window, &bridge->windows, list) {
>> +               if ((flags ^ window->res->flags) & mask)
>> +                       continue;
>> +
>> +               start = window->res->start - window->offset;
>> +               end = window->res->end - window->offset;
>> +
>> +               if (addr >= start && addr <= end) {
>> +                       offset = window->offset;
>> +                       break;
>> +               }
>> +
>> +               if (bound && addr > end && end > max) {
>> +                       max = end;
>> +                       match = window;
>> +               } else if (!bound && addr < start && start < min) {
>> +                       min = start;
>> +                       match = window;
>> +               }
>> +       }
>> +
>> +       if (offset == -1) {
>> +               /*
>> +                * Not even found the matched type. This may happen,
>> +                * for example, if try to translate IO address in a HB
>> +                * without IO window. Just return the original address,
>> +                * it will fail later anyway.
>> +                */
>> +               if (match == NULL)
>> +                       return addr;
>> +
>> +               return (bound ? max : min) + match->offset;
>> +       }
>> +
>> +       return addr + offset;
>> +}
>
> that is confusing.

I agree, this is way too confusing.

I certainly agree that the current code is wrong -- PCIBIOS_MAX_MEM_32
is a limit on the *bus* address, but we're using it to limit CPU
addresses.  And it's not arch-dependent at all; it's strictly a
function of PCI, so I think you should fix that, too.  Your [2/3]
patch is a start, but I don't think there's any reason to allow an
arch to override it, and it's only used in one place, so it doesn't
need to be in include/linux/pci.h where it's effectively exported to
the world.

It's completely bogus to try to compute "max" outside the loop because
there's no requirement that the translation offset be the same for all
the resources.  But *inside* the loop, you have a valid struct
resource, and you can easily compute the corresponding bus addresses
and determine whether they fit in 32 bits.

Bjorn