Re: [PATCH 3/3] PCI: Set pref for mem64 resource of pcie device

[Yinghai Lu <yinghai@kernel.org>]

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

On Wed, Apr 8, 2015 at 2:12 PM, Benjamin Herrenschmidt
<benh@kernel.crashing.org> wrote:

> Thanks Bjorn. We can fix Yinghai patch for 4.2, it would be indeed handy
> even for us to be able to support putting 64-bit NP BARs in prefetch
> windows (For some SR-IOV adapters for example) too, but we need to do it
> right.

Please check if you are ok with attached.

        Yinghai

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

Subject: [PATCH v3] PCI: Set under_pref for mem64 resource of pcie device

We still get "no compatible bridge window" warning on sparc T5-8
after we add support for 64bit resource parsing for root bus.

 PCI: scan_bus[/pci@300/pci@1/pci@0/pci@6] bus no 8
 PCI: Claiming 0000:00:01.0: Resource 15: 0000800100000000..00008004afffffff [220c]
 PCI: Claiming 0000:01:00.0: Resource 15: 0000800100000000..00008004afffffff [220c]
 PCI: Claiming 0000:02:04.0: Resource 15: 0000800100000000..000080012fffffff [220c]
 PCI: Claiming 0000:03:00.0: Resource 15: 0000800100000000..000080012fffffff [220c]
 PCI: Claiming 0000:04:06.0: Resource 14: 0000800100000000..000080010fffffff [220c]
 PCI: Claiming 0000:05:00.0: Resource 0: 0000800100000000..0000800100001fff [204]
 pci 0000:05:00.0: can't claim BAR 0 [mem 0x800100000000-0x800100001fff]: no compatible bridge window

All the bridges 64-bit resource have pref bit, but the device resource does not
have pref set, then we can not find parent for the device resource,
as we can not put non-pref mem under pref mem.

According to pcie spec errta
https://www.pcisig.com/specifications/pciexpress/base2/PCIe_Base_r2.1_Errata_08Jun10.pdf
page 13, in some case it is ok to mark some as pref.

Only set pref for 64bit mmio when the entire path from the host to the adapter is
over PCI Express.

Fixes: commit d63e2e1f3df9 ("sparc/PCI: Clip bridge windows to fit in upstream windows")
Link: http://lkml.kernel.org/r/CAE9FiQU1gJY1LYrxs+ma5LCTEEe4xmtjRG0aXJ9K_Tsu+m9Wuw@mail.gmail.com
Reported-by: David Ahern <david.ahern@oracle.com>
Tested-by: David Ahern <david.ahern@oracle.com>
Signed-off-by: Yinghai Lu <yinghai@kernel.org>
Cc:  <stable@vger.kernel.org> #3.19

---
-v2: set pref for mmio 64 when whole path is PCI Express, according to David Miller.
-v3: don't set pref directly, change to UNDER_PREF, and set PREF before
     sizing and assign resource, and cleart PREF afterwards. requested by BenH.
---


---
 drivers/pci/bus.c       |    9 +++++++-
 drivers/pci/pci.h       |   11 ++++++++++
 drivers/pci/probe.c     |   50 ++++++++++++++++++++++++++++++++++++++++++++++++
 drivers/pci/setup-bus.c |    8 ++++++-
 drivers/pci/setup-res.c |   10 +++++++++
 include/linux/ioport.h  |    2 +
 6 files changed, 88 insertions(+), 2 deletions(-)

Index: linux-2.6/drivers/pci/bus.c
===================================================================
--- linux-2.6.orig/drivers/pci/bus.c
+++ linux-2.6/drivers/pci/bus.c
@@ -139,6 +139,8 @@ static int pci_bus_alloc_from_region(str
 
 	type_mask |= IORESOURCE_TYPE_BITS;
 
+	pci_set_pref_under_pref(res);
+
 	pci_bus_for_each_resource(bus, r, i) {
 		if (!r)
 			continue;
@@ -170,9 +172,14 @@ static int pci_bus_alloc_from_region(str
 		/* Ok, try it out.. */
 		ret = allocate_resource(r, res, size, min, max,
 					align, alignf, alignf_data);
-		if (ret == 0)
+		if (ret == 0) {
+			pci_clear_pref_under_pref(res);
 			return 0;
+		}
 	}
+
+	pci_clear_pref_under_pref(res);
+
 	return -ENOMEM;
 }
 
Index: linux-2.6/drivers/pci/pci.h
===================================================================
--- linux-2.6.orig/drivers/pci/pci.h
+++ linux-2.6/drivers/pci/pci.h
@@ -216,6 +216,17 @@ void __pci_bus_assign_resources(const st
 				struct list_head *fail_head);
 bool pci_bus_clip_resource(struct pci_dev *dev, int idx);
 
+static inline void pci_set_pref_under_pref(struct resource *res)
+{
+	if (res->flags & IORESOURCE_UNDER_PREF)
+		res->flags |= IORESOURCE_PREFETCH;
+}
+static inline void pci_clear_pref_under_pref(struct resource *res)
+{
+	if (res->flags & IORESOURCE_UNDER_PREF)
+		res->flags &= ~IORESOURCE_PREFETCH;
+}
+
 /**
  * pci_ari_enabled - query ARI forwarding status
  * @bus: the PCI bus
Index: linux-2.6/drivers/pci/probe.c
===================================================================
--- linux-2.6.orig/drivers/pci/probe.c
+++ linux-2.6/drivers/pci/probe.c
@@ -1508,6 +1508,53 @@ static void pci_init_capabilities(struct
 	pci_enable_acs(dev);
 }
 
+static bool pci_up_path_over_pcie(struct pci_bus *bus)
+{
+	if (!bus)
+		return true;
+
+	if (bus->self && !pci_is_pcie(bus->self))
+		return false;
+
+	return pci_up_path_over_pcie(bus->parent);
+}
+
+/*
+ * According to
+ * https://www.pcisig.com/specifications/pciexpress/base2/PCIe_Base_r2.1_Errata_08Jun10.pdf
+ * page 13, system firmware could put some 64bit non-pref under 64bit pref,
+ * on some cases.
+ * Let's set pref bit for 64bit mmio when entire path from the host to
+ * the adapter is over PCI Express.
+ */
+static void set_pcie_64bit_under_pref(struct pci_dev *dev)
+{
+	int i;
+
+	if (!pci_is_pcie(dev))
+		return;
+
+	if (!pci_up_path_over_pcie(dev->bus))
+		return;
+
+	for (i = 0; i < PCI_BRIDGE_RESOURCES; i++) {
+		struct resource *res = &dev->resource[i];
+		enum pci_bar_type type;
+		int reg;
+
+		if (!(res->flags & IORESOURCE_MEM_64))
+			continue;
+
+		if (res->flags & IORESOURCE_PREFETCH)
+			continue;
+
+		reg = pci_resource_bar(dev, i, &type);
+		dev_printk(KERN_DEBUG, &dev->dev, "reg 0x%x %pR + under_pref\n",
+			   reg, res);
+		res->flags |= IORESOURCE_UNDER_PREF;
+	}
+}
+
 void pci_device_add(struct pci_dev *dev, struct pci_bus *bus)
 {
 	int ret;
@@ -1538,6 +1585,9 @@ void pci_device_add(struct pci_dev *dev,
 	/* Initialize various capabilities */
 	pci_init_capabilities(dev);
 
+	/* After pcie_cap is assigned and sriov bar is probed */
+	set_pcie_64bit_under_pref(dev);
+
 	/*
 	 * Add the device to our list of discovered devices
 	 * and the bus list for fixup functions, etc.
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
@@ -977,10 +977,13 @@ static int pbus_size_mem(struct pci_bus
 			struct resource *r = &dev->resource[i];
 			resource_size_t r_size;
 
+			pci_set_pref_under_pref(r);
 			if (r->parent || ((r->flags & mask) != type &&
 					  (r->flags & mask) != type2 &&
-					  (r->flags & mask) != type3))
+					  (r->flags & mask) != type3)) {
+				pci_clear_pref_under_pref(r);
 				continue;
+			}
 			r_size = resource_size(r);
 #ifdef CONFIG_PCI_IOV
 			/* put SRIOV requested res to the optional list */
@@ -989,6 +992,7 @@ static int pbus_size_mem(struct pci_bus
 				r->end = r->start - 1;
 				add_to_list(realloc_head, dev, r, r_size, 0/* don't care */);
 				children_add_size += r_size;
+				pci_clear_pref_under_pref(r);
 				continue;
 			}
 #endif
@@ -1018,6 +1022,8 @@ static int pbus_size_mem(struct pci_bus
 
 			if (realloc_head)
 				children_add_size += get_res_add_size(realloc_head, r);
+
+			pci_clear_pref_under_pref(r);
 		}
 	}
 
Index: linux-2.6/drivers/pci/setup-res.c
===================================================================
--- linux-2.6.orig/drivers/pci/setup-res.c
+++ linux-2.6/drivers/pci/setup-res.c
@@ -116,11 +116,14 @@ int pci_claim_resource(struct pci_dev *d
 		return -EINVAL;
 	}
 
+	pci_set_pref_under_pref(res);
+
 	root = pci_find_parent_resource(dev, res);
 	if (!root) {
 		dev_info(&dev->dev, "can't claim BAR %d %pR: no compatible bridge window\n",
 			 resource, res);
 		res->flags |= IORESOURCE_UNSET;
+		pci_clear_pref_under_pref(res);
 		return -EINVAL;
 	}
 
@@ -129,9 +132,12 @@ int pci_claim_resource(struct pci_dev *d
 		dev_info(&dev->dev, "can't claim BAR %d %pR: address conflict with %s %pR\n",
 			 resource, res, conflict->name, conflict);
 		res->flags |= IORESOURCE_UNSET;
+		pci_clear_pref_under_pref(res);
 		return -EBUSY;
 	}
 
+	pci_clear_pref_under_pref(res);
+
 	return 0;
 }
 EXPORT_SYMBOL(pci_claim_resource);
@@ -250,9 +256,11 @@ static int __pci_assign_resource(struct
 static int _pci_assign_resource(struct pci_dev *dev, int resno,
 				resource_size_t size, resource_size_t min_align)
 {
+	struct resource *res = dev->resource + resno;
 	struct pci_bus *bus;
 	int ret;
 
+	pci_set_pref_under_pref(res);
 	bus = dev->bus;
 	while ((ret = __pci_assign_resource(bus, dev, resno, size, min_align))) {
 		if (!bus->parent || !bus->self->transparent)
@@ -260,6 +268,8 @@ static int _pci_assign_resource(struct p
 		bus = bus->parent;
 	}
 
+	pci_clear_pref_under_pref(res);
+
 	return ret;
 }
 
Index: linux-2.6/include/linux/ioport.h
===================================================================
--- linux-2.6.orig/include/linux/ioport.h
+++ linux-2.6/include/linux/ioport.h
@@ -49,6 +49,8 @@ struct resource {
 #define IORESOURCE_WINDOW	0x00200000	/* forwarded by bridge */
 #define IORESOURCE_MUXED	0x00400000	/* Resource is software muxed */
 
+#define IORESOURCE_UNDER_PREF	0x00800000	/* non-pref could under pref */
+
 #define IORESOURCE_EXCLUSIVE	0x08000000	/* Userland may not map this resource */
 #define IORESOURCE_DISABLED	0x10000000
 #define IORESOURCE_UNSET	0x20000000	/* No address assigned yet */