[PATCH 0/4] PCI: fix cardbus and sriov regressions

[PATCH 0/4] PCI: fix cardbus and sriov regressions

[Ram Pai]

The following patch-set fixes regressions caused by:

the commit "PCI: update bridge resources to get more big ranges when allocating space (again)"
http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdiff;h=da7822e5ad71ec9b745b412639f1e5e0ba795a20

patch 1/4: correctly calculates the additional resource size for hotplug bridges.
patch 2/4: ability to resize assigned pci-resource.
patch 3/4: makes SRIOV BARs a nice-to-have resource, which means resources will
		be attempted to assign, but not gauranteed to succeed.
patch 4/4: makes cardbus bridge resources nice-to-have resource.


The regression was caused because on some platforms with limited i/o and mem
resources, the nice-to-have resources were allocated ahead of
absolutely-required resources, thus starving the latter. The patchset will
ensure that all the mandatory resource requirements are satisfied before any
nice-to-have resource requirements are satisfied.

[PATCH 1/4] PCI: honor child buses add_size in hot plug configuration

[Ram Pai]

From: Yinghai Lu <yinghai@kernel.org>

Recent pci_bus_size change will use add_size for minimum resource size for pcie
hotplug bridge.  But it does not pass children back to parent bridge.

that will have problem on some setup like:
hot add one chassis with more hot plug slots.
for example: if the chassis have 8 slots, we should allocate 8x2M instead
of one 1x2M for parent bus.

So try to get child add_size and compare the sum with parent bus bridge...

Signed-off-by: Yinghai Lu <yinghai@kernel.org>
Reviewed-by: Ram Pai <linuxram@us.ibm.com>
---
 drivers/pci/setup-bus.c |   26 ++++++++++++++++++++++++++
 1 files changed, 26 insertions(+), 0 deletions(-)

diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
index 1e9e5a5..e42b89a 100644
--- a/drivers/pci/setup-bus.c
+++ b/drivers/pci/setup-bus.c
@@ -536,6 +536,20 @@ static resource_size_t calculate_memsize(resource_size_t size,
 	return size;
 }
 
+static resource_size_t get_res_add_size(struct resource_list_x *add_head,
+					struct resource *res)
+{
+	struct resource_list_x *list;
+
+	/* check if it is in add_head list */
+	for (list = add_head->next; list && list->res != res;
+			list = list->next);
+	if (list)
+		return list->add_size;
+
+	return 0;
+}
+
 /**
  * pbus_size_io() - size the io window of a given bus
  *
@@ -555,6 +569,7 @@ static void pbus_size_io(struct pci_bus *bus, resource_size_t min_size,
 	struct pci_dev *dev;
 	struct resource *b_res = find_free_bus_resource(bus, IORESOURCE_IO);
 	unsigned long size = 0, size0 = 0, size1 = 0;
+	resource_size_t children_add_size = 0;
 
 	if (!b_res)
  		return;
@@ -575,10 +590,15 @@ static void pbus_size_io(struct pci_bus *bus, resource_size_t min_size,
 				size += r_size;
 			else
 				size1 += r_size;
+
+			if (add_head)
+				children_add_size += get_res_add_size(add_head, r);
 		}
 	}
 	size0 = calculate_iosize(size, min_size, size1,
 			resource_size(b_res), 4096);
+	if (children_add_size > add_size)
+		add_size = children_add_size;
 	size1 = (!add_head || (add_head && !add_size)) ? size0 :
 		calculate_iosize(size, min_size+add_size, size1,
 			resource_size(b_res), 4096);
@@ -620,6 +640,7 @@ static int pbus_size_mem(struct pci_bus *bus, unsigned long mask,
 	int order, max_order;
 	struct resource *b_res = find_free_bus_resource(bus, type);
 	unsigned int mem64_mask = 0;
+	resource_size_t children_add_size = 0;
 
 	if (!b_res)
 		return 0;
@@ -661,6 +682,9 @@ static int pbus_size_mem(struct pci_bus *bus, unsigned long mask,
 			if (order > max_order)
 				max_order = order;
 			mem64_mask &= r->flags & IORESOURCE_MEM_64;
+
+			if (add_head)
+				children_add_size += get_res_add_size(add_head, r);
 		}
 	}
 	align = 0;
@@ -677,6 +701,8 @@ static int pbus_size_mem(struct pci_bus *bus, unsigned long mask,
 		align += aligns[order];
 	}
 	size0 = calculate_memsize(size, min_size, 0, resource_size(b_res), min_align);
+	if (children_add_size > add_size)
+		add_size = children_add_size;
 	size1 = (!add_head || (add_head && !add_size)) ? size0 :
 		calculate_memsize(size, min_size+add_size, 0,
 				resource_size(b_res), min_align);
-- 
1.7.0.4

[PATCH 2/4] PCI : ability to resize assigned pci-resource

[Ram Pai]

Currently pci-bridges are allocated enough resources to satisfy their immediate
requirements.  Any additional resource-requests fail if additional free space,
contiguous to the one already allocated, is not available. This behavior is not
satisfying when sufficient contiguous resources, that can satisfy the request,
is available in a different location.

This patch provides the ability to expand a allocated resource.  This behavior
is particularly useful to satisfy larger size nice-to-have resource requests,
like SRIOV BARs or cardbus bridges.

	Signed-off-by: Ram Pai <linuxram@us.ibm.com>
---
 drivers/pci/setup-bus.c |   30 ++++++---
 drivers/pci/setup-res.c |  152 +++++++++++++++++++++++++++++------------------
 include/linux/pci.h     |    1 +
 kernel/resource.c       |   98 ++++++++++++++++++++++++++++---
 4 files changed, 206 insertions(+), 75 deletions(-)

diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
index e42b89a..c282c48 100644
--- a/drivers/pci/setup-bus.c
+++ b/drivers/pci/setup-bus.c
@@ -34,6 +34,7 @@ struct resource_list_x {
 	resource_size_t start;
 	resource_size_t end;
 	resource_size_t add_size;
+	resource_size_t min_align;
 	unsigned long flags;
 };
 
@@ -58,7 +59,7 @@ struct resource_list_x {
  */
 static void add_to_list(struct resource_list_x *head,
 		 struct pci_dev *dev, struct resource *res,
-		 resource_size_t add_size)
+		 resource_size_t add_size, resource_size_t min_align)
 {
 	struct resource_list_x *list = head;
 	struct resource_list_x *ln = list->next;
@@ -77,13 +78,16 @@ static void add_to_list(struct resource_list_x *head,
 	tmp->end = res->end;
 	tmp->flags = res->flags;
 	tmp->add_size = add_size;
+	tmp->min_align = min_align;
 	list->next = tmp;
 }
 
 static void add_to_failed_list(struct resource_list_x *head,
 				struct pci_dev *dev, struct resource *res)
 {
-	add_to_list(head, dev, res, 0);
+	add_to_list(head, dev, res,
+			0 /* dont care */,
+			0 /* dont care */);
 }
 
 static void __dev_sort_resources(struct pci_dev *dev,
@@ -152,13 +156,19 @@ static void adjust_resources_sorted(struct resource_list_x *add_head,
 
 		idx = res - &list->dev->resource[0];
 		add_size=list->add_size;
-		if (!resource_size(res) && add_size) {
-			 res->end = res->start + add_size - 1;
-			 if(pci_assign_resource(list->dev, idx))
+		if (!resource_size(res)) {
+			res->end = res->start + add_size - 1;
+			if(pci_assign_resource(list->dev, idx))
 				reset_resource(res);
-		} else if (add_size) {
-			adjust_resource(res, res->start,
-				resource_size(res) + add_size);
+		} else {
+			resource_size_t align = list->min_align;
+			res->flags |= list->flags &
+			   (IORESOURCE_STARTALIGN|IORESOURCE_SIZEALIGN);
+			if (pci_reassign_resource(list->dev, idx, add_size,
+					align))
+				dev_printk(KERN_DEBUG, &list->dev->dev,
+				"failed to add optional resources res=%pR\n",
+				res);
 		}
 out:
 		tmp = list;
@@ -615,7 +625,7 @@ static void pbus_size_io(struct pci_bus *bus, resource_size_t min_size,
 	b_res->end = b_res->start + size0 - 1;
 	b_res->flags |= IORESOURCE_STARTALIGN;
 	if (size1 > size0 && add_head)
-		add_to_list(add_head, bus->self, b_res, size1-size0);
+		add_to_list(add_head, bus->self, b_res, size1-size0, 4096);
 }
 
 /**
@@ -718,7 +728,7 @@ static int pbus_size_mem(struct pci_bus *bus, unsigned long mask,
 	b_res->end = size0 + min_align - 1;
 	b_res->flags |= IORESOURCE_STARTALIGN | mem64_mask;
 	if (size1 > size0 && add_head)
-		add_to_list(add_head, bus->self, b_res, size1-size0);
+		add_to_list(add_head, bus->self, b_res, size1-size0, min_align);
 	return 1;
 }
 
diff --git a/drivers/pci/setup-res.c b/drivers/pci/setup-res.c
index bc0e6ee..e8a94d5 100644
--- a/drivers/pci/setup-res.c
+++ b/drivers/pci/setup-res.c
@@ -129,16 +129,16 @@ void pci_disable_bridge_window(struct pci_dev *dev)
 }
 #endif	/* CONFIG_PCI_QUIRKS */
 
+
+
 static int __pci_assign_resource(struct pci_bus *bus, struct pci_dev *dev,
-				 int resno)
+		int resno, resource_size_t size, resource_size_t align)
 {
 	struct resource *res = dev->resource + resno;
-	resource_size_t size, min, align;
+	resource_size_t min;
 	int ret;
 
-	size = resource_size(res);
 	min = (res->flags & IORESOURCE_IO) ? PCIBIOS_MIN_IO : PCIBIOS_MIN_MEM;
-	align = pci_resource_alignment(dev, res);
 
 	/* First, try exact prefetching match.. */
 	ret = pci_bus_alloc_resource(bus, res, size, align, min,
@@ -155,56 +155,101 @@ static int __pci_assign_resource(struct pci_bus *bus, struct pci_dev *dev,
 		ret = pci_bus_alloc_resource(bus, res, size, align, min, 0,
 					     pcibios_align_resource, dev);
 	}
+	return ret;
+}
 
-	if (ret < 0 && dev->fw_addr[resno]) {
-		struct resource *root, *conflict;
-		resource_size_t start, end;
+static int pci_revert_fw_address(struct resource *res, struct pci_dev *dev, 
+		int resno, resource_size_t size)
+{
+	struct resource *root, *conflict;
+	resource_size_t start, end;
+	int ret = 0;
 
-		/*
-		 * If we failed to assign anything, let's try the address
-		 * where firmware left it.  That at least has a chance of
-		 * working, which is better than just leaving it disabled.
-		 */
+	if (res->flags & IORESOURCE_IO)
+		root = &ioport_resource;
+	else
+		root = &iomem_resource;
+
+	start = res->start;
+	end = res->end;
+	res->start = dev->fw_addr[resno];
+	res->end = res->start + size - 1;
+	dev_info(&dev->dev, "BAR %d: trying firmware assignment %pR\n",
+		 resno, res);
+	conflict = request_resource_conflict(root, res);
+	if (conflict) {
+		dev_info(&dev->dev,
+			 "BAR %d: %pR conflicts with %s %pR\n", resno,
+			 res, conflict->name, conflict);
+		res->start = start;
+		res->end = end;
+		ret = 1;
+	}
+	return ret;
+}
+
+static int _pci_assign_resource(struct pci_dev *dev, int resno, int size, resource_size_t min_align)
+{
+	struct resource *res = dev->resource + resno;
+	struct pci_bus *bus;
+	int ret;
+	char *type;
 
-		if (res->flags & IORESOURCE_IO)
-			root = &ioport_resource;
+	bus = dev->bus;
+	while ((ret = __pci_assign_resource(bus, dev, resno, size, min_align))) {
+		if (!bus->parent || !bus->self->transparent)
+			break;
+		bus = bus->parent;
+	}
+
+	if (ret) {
+		if (res->flags & IORESOURCE_MEM)
+			if (res->flags & IORESOURCE_PREFETCH)
+				type = "mem pref";
+			else
+				type = "mem";
+		else if (res->flags & IORESOURCE_IO)
+			type = "io";
 		else
-			root = &iomem_resource;
-
-		start = res->start;
-		end = res->end;
-		res->start = dev->fw_addr[resno];
-		res->end = res->start + size - 1;
-		dev_info(&dev->dev, "BAR %d: trying firmware assignment %pR\n",
-			 resno, res);
-		conflict = request_resource_conflict(root, res);
-		if (conflict) {
-			dev_info(&dev->dev,
-				 "BAR %d: %pR conflicts with %s %pR\n", resno,
-				 res, conflict->name, conflict);
-			res->start = start;
-			res->end = end;
-		} else
-			ret = 0;
+			type = "unknown";
+		dev_info(&dev->dev,
+			 "BAR %d: can't assign %s (size %#llx)\n",
+			 resno, type, (unsigned long long) resource_size(res));
 	}
 
+	return ret;
+}
+
+int pci_reassign_resource(struct pci_dev *dev, int resno, resource_size_t addsize,
+			resource_size_t min_align)
+{
+	struct resource *res = dev->resource + resno;
+	resource_size_t new_size;
+	int ret;
+
+	if (!res->parent) {
+		dev_info(&dev->dev, "BAR %d: can't reassign an unassigned resouce %pR "
+			 "\n", resno, res);
+		return -EINVAL;
+	}
+
+	new_size = resource_size(res) + addsize + min_align - 1;
+	ret = _pci_assign_resource(dev, resno, new_size, min_align);
 	if (!ret) {
 		res->flags &= ~IORESOURCE_STARTALIGN;
 		dev_info(&dev->dev, "BAR %d: assigned %pR\n", resno, res);
 		if (resno < PCI_BRIDGE_RESOURCES)
 			pci_update_resource(dev, resno);
 	}
-
 	return ret;
 }
 
 int pci_assign_resource(struct pci_dev *dev, int resno)
 {
 	struct resource *res = dev->resource + resno;
-	resource_size_t align;
+	resource_size_t align, size;
 	struct pci_bus *bus;
 	int ret;
-	char *type;
 
 	align = pci_resource_alignment(dev, res);
 	if (!align) {
@@ -214,34 +259,27 @@ int pci_assign_resource(struct pci_dev *dev, int resno)
 	}
 
 	bus = dev->bus;
-	while ((ret = __pci_assign_resource(bus, dev, resno))) {
-		if (bus->parent && bus->self->transparent)
-			bus = bus->parent;
-		else
-			bus = NULL;
-		if (bus)
-			continue;
-		break;
-	}
+	size = resource_size(res);
+	ret = _pci_assign_resource(dev, resno, size, align);
 
-	if (ret) {
-		if (res->flags & IORESOURCE_MEM)
-			if (res->flags & IORESOURCE_PREFETCH)
-				type = "mem pref";
-			else
-				type = "mem";
-		else if (res->flags & IORESOURCE_IO)
-			type = "io";
-		else
-			type = "unknown";
-		dev_info(&dev->dev,
-			 "BAR %d: can't assign %s (size %#llx)\n",
-			 resno, type, (unsigned long long) resource_size(res));
-	}
+	/*
+	 * If we failed to assign anything, let's try the address
+	 * where firmware left it.  That at least has a chance of
+	 * working, which is better than just leaving it disabled.
+	 */
+	if (ret < 0 && dev->fw_addr[resno])
+		ret = pci_revert_fw_address(res, dev, resno, size);
 
+	if (!ret) {
+		res->flags &= ~IORESOURCE_STARTALIGN;
+		dev_info(&dev->dev, "BAR %d: assigned %pR\n", resno, res);
+		if (resno < PCI_BRIDGE_RESOURCES)
+			pci_update_resource(dev, resno);
+	}
 	return ret;
 }
 
+
 /* Sort resources by alignment */
 void pdev_sort_resources(struct pci_dev *dev, struct resource_list *head)
 {
diff --git a/include/linux/pci.h b/include/linux/pci.h
index c446b5c..f39d894 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -800,6 +800,7 @@ int __pci_reset_function(struct pci_dev *dev);
 int pci_reset_function(struct pci_dev *dev);
 void pci_update_resource(struct pci_dev *dev, int resno);
 int __must_check pci_assign_resource(struct pci_dev *dev, int i);
+int __must_check pci_reassign_resource(struct pci_dev *dev, int i, resource_size_t add_size, resource_size_t align);
 int pci_select_bars(struct pci_dev *dev, unsigned long flags);
 
 /* ROM control related routines */
diff --git a/kernel/resource.c b/kernel/resource.c
index 798e2fa..ff3629c 100644
--- a/kernel/resource.c
+++ b/kernel/resource.c
@@ -386,7 +386,8 @@ static bool resource_contains(struct resource *res1, struct resource *res2)
 /*
  * Find empty slot in the resource tree given range and alignment.
  */
-static int find_resource(struct resource *root, struct resource *new,
+static int __find_resource(struct resource *root, struct resource *old,
+			 struct resource *new,
 			 resource_size_t size, resource_size_t min,
 			 resource_size_t max, resource_size_t align,
 			 resource_size_t (*alignf)(void *,
@@ -404,14 +405,22 @@ static int find_resource(struct resource *root, struct resource *new,
 	 * Skip past an allocated resource that starts at 0, since the assignment
 	 * of this->start - 1 to tmp->end below would cause an underflow.
 	 */
-	if (this && this->start == 0) {
-		tmp.start = this->end + 1;
-		this = this->sibling;
+	if (this && this->start == root->start) {
+		if (this == old) {
+			tmp.start = old->start;
+		} else {
+			tmp.start = this->end + 1;
+		}
+ 		this = this->sibling;
 	}
 	for(;;) {
-		if (this)
-			tmp.end = this->start - 1;
-		else
+		if (this) {
+			if (this == old) {
+				tmp.end = this->end;
+			} else {
+				tmp.end = this->start - 1;
+			}
+		} else
 			tmp.end = root->end;
 
 		resource_clip(&tmp, min, max);
@@ -432,12 +441,82 @@ static int find_resource(struct resource *root, struct resource *new,
 		}
 		if (!this)
 			break;
-		tmp.start = this->end + 1;
+		if (this != old)
+			tmp.start = this->end + 1;
 		this = this->sibling;
 	}
 	return -EBUSY;
 }
 
+/*
+ * Find empty slot in the resource tree given range and alignment.
+ */
+static int find_resource(struct resource *root, struct resource *new,
+			 resource_size_t size, resource_size_t min,
+			 resource_size_t max, resource_size_t align,
+			 resource_size_t (*alignf)(void *,
+						   const struct resource *,
+						   resource_size_t,
+						   resource_size_t),
+			 void *alignf_data)
+{
+	return  __find_resource(root, NULL, new, size, min, max, align, alignf, alignf_data);
+}
+
+/**
+ * reallocate_resource - allocate empty slot in the resource tree given range & alignment
+ * @root: root resource descriptor
+ * @new: resource descriptor desired by caller
+ * @size: requested resource region size
+ * @min: minimum size to allocate
+ * @max: maximum size to allocate
+ * @align: alignment requested, in bytes
+ * @alignf: alignment function, optional, called if not NULL
+ * @alignf_data: arbitrary data to pass to the @alignf function
+ */
+int reallocate_resource(struct resource *root, struct resource *old,
+		      resource_size_t newsize, resource_size_t min,
+		      resource_size_t max, resource_size_t align,
+		      resource_size_t (*alignf)(void *,
+						const struct resource *,
+						resource_size_t,
+						resource_size_t),
+		      void *alignf_data)
+{
+	int err=0;
+	struct resource new = *old;
+
+	write_lock(&resource_lock);
+
+	if ((err = __find_resource(root, old, &new, newsize, min, max, align, 
+			alignf, alignf_data)))
+		goto out;
+
+	if (resource_contains(&new, old)) {
+		old->start = new.start;
+		old->end = new.end;
+		goto out;
+	}
+
+	if (old->child) {
+		err = -EBUSY;
+		goto out;
+	}
+
+	if (resource_contains(old, &new)) {
+		old->start = new.start;
+		old->end = new.end;
+	} else {
+		__release_resource(old);
+		*old = new;
+		__request_resource(root, old);
+	}
+out:
+	write_unlock(&resource_lock);
+	return err;
+}
+
+
 /**
  * allocate_resource - allocate empty slot in the resource tree given range & alignment
  * @root: root resource descriptor
@@ -463,6 +542,9 @@ int allocate_resource(struct resource *root, struct resource *new,
 	if (!alignf)
 		alignf = simple_align_resource;
 
+	if ( new->parent )
+		return reallocate_resource(root, new, size, min, max, align, alignf, alignf_data);
+
 	write_lock(&resource_lock);
 	err = find_resource(root, new, size, min, max, align, alignf, alignf_data);
 	if (err >= 0 && __request_resource(root, new))
-- 
1.7.0.4

[PATCH 3/4] PCI: make SRIOV resources nice-to-have

[Ram Pai]

From: Yinghai Lu <yinghai@kernel.org>

Allocate resources to SRIOV BARs only after all other genuine resources
requests are satisfied. Dont retry if resource allocation for SRIOV BARs fail.

Signed-off-by: Ram Pai <linuxram@us.ibm.com>
Signed-off-by: Yinghai Lu <yinghai@kernel.org>
---
 drivers/pci/setup-bus.c |   10 ++++++++++
 1 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
index c282c48..4f8873e 100644
--- a/drivers/pci/setup-bus.c
+++ b/drivers/pci/setup-bus.c
@@ -672,6 +672,16 @@ static int pbus_size_mem(struct pci_bus *bus, unsigned long mask,
 			if (r->parent || (r->flags & mask) != type)
 				continue;
 			r_size = resource_size(r);
+#ifdef CONFIG_PCI_IOV
+			/* add SRIOV BARs to nice-to-have list */
+			if (add_head && i >= PCI_IOV_RESOURCES &&
+					i <= PCI_IOV_RESOURCE_END) {
+				r->end = r->start - 1;
+				add_to_list(add_head, dev, r, r_size, 1);
+				children_add_size += r_size;
+				continue;
+			}
+#endif
 			/* For bridges size != alignment */
 			align = pci_resource_alignment(dev, r);
 			order = __ffs(align) - 20;
-- 
1.7.0.4

[PATCH 4/4] PCI: make cardbus-bridge resources nice-to-have

[Ram Pai]

Allocate resources to cardbus bridge only after all other genuine
resources requests are satisfied. Dont retry if resource allocation
for cardbus-bridge fails.

Tested-by: Oliver Hartkopp <socketcan@hartkopp.net>
Signed-off-by: Ram Pai <linuxram@us.ibm.com>
---
 drivers/pci/setup-bus.c |   20 +++++++++++++-------
 1 files changed, 13 insertions(+), 7 deletions(-)

diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
index 4f8873e..023fc9c 100644
--- a/drivers/pci/setup-bus.c
+++ b/drivers/pci/setup-bus.c
@@ -742,7 +742,8 @@ static int pbus_size_mem(struct pci_bus *bus, unsigned long mask,
 	return 1;
 }
 
-static void pci_bus_size_cardbus(struct pci_bus *bus)
+static void pci_bus_size_cardbus(struct pci_bus *bus,
+			struct resource_list_x *add_head)
 {
 	struct pci_dev *bridge = bus->self;
 	struct resource *b_res = &bridge->resource[PCI_BRIDGE_RESOURCES];
@@ -753,12 +754,14 @@ static void pci_bus_size_cardbus(struct pci_bus *bus)
 	 * a fixed amount of bus space for CardBus bridges.
 	 */
 	b_res[0].start = 0;
-	b_res[0].end = pci_cardbus_io_size - 1;
+	b_res[0].end = 0;
 	b_res[0].flags |= IORESOURCE_IO | IORESOURCE_SIZEALIGN;
+	add_to_list(add_head, bridge, b_res, pci_cardbus_io_size - 1, 1);
 
 	b_res[1].start = 0;
-	b_res[1].end = pci_cardbus_io_size - 1;
+	b_res[1].end = 0;
 	b_res[1].flags |= IORESOURCE_IO | IORESOURCE_SIZEALIGN;
+	add_to_list(add_head, bridge, b_res+1, pci_cardbus_io_size - 1, 1);
 
 	/*
 	 * Check whether prefetchable memory is supported
@@ -778,16 +781,19 @@ static void pci_bus_size_cardbus(struct pci_bus *bus)
 	 */
 	if (ctrl & PCI_CB_BRIDGE_CTL_PREFETCH_MEM0) {
 		b_res[2].start = 0;
-		b_res[2].end = pci_cardbus_mem_size - 1;
+		b_res[2].end = 0;
 		b_res[2].flags |= IORESOURCE_MEM | IORESOURCE_PREFETCH | IORESOURCE_SIZEALIGN;
+		add_to_list(add_head, bridge, b_res+2, pci_cardbus_mem_size - 1, 1);
 
 		b_res[3].start = 0;
-		b_res[3].end = pci_cardbus_mem_size - 1;
+		b_res[3].end = 0;
 		b_res[3].flags |= IORESOURCE_MEM | IORESOURCE_SIZEALIGN;
+		add_to_list(add_head, bridge, b_res+3, pci_cardbus_mem_size - 1, 1);
 	} else {
 		b_res[3].start = 0;
-		b_res[3].end = pci_cardbus_mem_size * 2 - 1;
+		b_res[3].end = 0;
 		b_res[3].flags |= IORESOURCE_MEM | IORESOURCE_SIZEALIGN;
+		add_to_list(add_head, bridge, b_res+3, pci_cardbus_mem_size * 2 - 1, 1);
 	}
 }
 
@@ -805,7 +811,7 @@ void __ref __pci_bus_size_bridges(struct pci_bus *bus,
 
 		switch (dev->class >> 8) {
 		case PCI_CLASS_BRIDGE_CARDBUS:
-			pci_bus_size_cardbus(b);
+			pci_bus_size_cardbus(b, add_head);
 			break;
 
 		case PCI_CLASS_BRIDGE_PCI:
-- 
1.7.0.4

Re: [PATCH 4/4] PCI: make cardbus-bridge resources nice-to-have

[Dominik Brodowski]

On Mon, Jun 20, 2011 at 03:47:17PM -0700, Ram Pai wrote:
> Allocate resources to cardbus bridge only after all other genuine
> resources requests are satisfied. Dont retry if resource allocation
> for cardbus-bridge fails.

Well, for those who use cardbus cards, cardbus resources aren't "nice to
have", they are absolutely required. Of course, not all cardbus cards need
as many resources as are currently assigned, so I wouldn't oppose a patch
which marks _some_ of the currently assigned resources as "nice to have".
But this approach -- 0 required, all "nice to have" -- seems wrong to me.

Best,
	Dominik

Re: [PATCH 4/4] PCI: make cardbus-bridge resources nice-to-have

[Ram Pai]

On Tue, Jun 21, 2011 at 09:57:00AM +0200, Dominik Brodowski wrote:
> On Mon, Jun 20, 2011 at 03:47:17PM -0700, Ram Pai wrote:
> > Allocate resources to cardbus bridge only after all other genuine
> > resources requests are satisfied. Dont retry if resource allocation
> > for cardbus-bridge fails.
> 
> Well, for those who use cardbus cards, cardbus resources aren't "nice to
> have", they are absolutely required. Of course, not all cardbus cards need
> as many resources as are currently assigned, so I wouldn't oppose a patch
> which marks _some_ of the currently assigned resources as "nice to have".
> But this approach -- 0 required, all "nice to have" -- seems wrong to me.

Do you know how much minimal resource is good enough?  The value, before
this patch, was 256 for IO ports and  64M for memory.

BTW: If the BIOS has already assigned enough resources for all the devices on
the system, no devices will be starved including the cardbus. The OS intervenes
and is forced to make this hard choice only when it sees unassigned resources to
some devices along with resource contention.

RP

Re: [PATCH 4/4] PCI: make cardbus-bridge resources nice-to-have

[Jesse Barnes]

On Tue, 21 Jun 2011 09:23:21 -0700
Ram Pai <linuxram@us.ibm.com> wrote:

> On Tue, Jun 21, 2011 at 09:57:00AM +0200, Dominik Brodowski wrote:
> > On Mon, Jun 20, 2011 at 03:47:17PM -0700, Ram Pai wrote:
> > > Allocate resources to cardbus bridge only after all other genuine
> > > resources requests are satisfied. Dont retry if resource allocation
> > > for cardbus-bridge fails.
> > 
> > Well, for those who use cardbus cards, cardbus resources aren't "nice to
> > have", they are absolutely required. Of course, not all cardbus cards need
> > as many resources as are currently assigned, so I wouldn't oppose a patch
> > which marks _some_ of the currently assigned resources as "nice to have".
> > But this approach -- 0 required, all "nice to have" -- seems wrong to me.
> 
> Do you know how much minimal resource is good enough?  The value, before
> this patch, was 256 for IO ports and  64M for memory.
> 
> BTW: If the BIOS has already assigned enough resources for all the devices on
> the system, no devices will be starved including the cardbus. The OS intervenes
> and is forced to make this hard choice only when it sees unassigned resources to
> some devices along with resource contention.

I just know this is going to trigger regressions, so I think Dominik's
concern is valid.  We'll have some existing machine with a device whose
resource was never assigned, but we didn't care because it was unused.
Now this code will try to give it some address space at the expense of
something that *is* being used.

But OTOH this will at least try to allocate *some* space to cardbus, we
just won't try as hard as with some other resources.  I'd mainly like
to avoid the situation Dominik pointed out, where we have perfectly
good cardbus resources assigned (unlike in Oliver's case) but they're
stolen for a bridge that may get a hotplugged device or some other
device that didn't have a BIOS assignment.

-- 
Jesse Barnes, Intel Open Source Technology Center

Re: [PATCH 4/4] PCI: make cardbus-bridge resources nice-to-have

[Jesse Barnes]

On Tue, 21 Jun 2011 09:23:21 -0700
Ram Pai <linuxram@us.ibm.com> wrote:

> On Tue, Jun 21, 2011 at 09:57:00AM +0200, Dominik Brodowski wrote:
> > On Mon, Jun 20, 2011 at 03:47:17PM -0700, Ram Pai wrote:
> > > Allocate resources to cardbus bridge only after all other genuine
> > > resources requests are satisfied. Dont retry if resource allocation
> > > for cardbus-bridge fails.
> > 
> > Well, for those who use cardbus cards, cardbus resources aren't "nice to
> > have", they are absolutely required. Of course, not all cardbus cards need
> > as many resources as are currently assigned, so I wouldn't oppose a patch
> > which marks _some_ of the currently assigned resources as "nice to have".
> > But this approach -- 0 required, all "nice to have" -- seems wrong to me.
> 
> Do you know how much minimal resource is good enough?  The value, before
> this patch, was 256 for IO ports and  64M for memory.
> 
> BTW: If the BIOS has already assigned enough resources for all the devices on
> the system, no devices will be starved including the cardbus. The OS intervenes
> and is forced to make this hard choice only when it sees unassigned resources to
> some devices along with resource contention.

Dominik, presumably you have a few good cardbus test machines; can you
give Ram's patches a try?  If we know they break existing
configurations, I'm afraid we'll just have to revert the whole
re-allocation patch yet again.  If your stuff survives, I'll ping Linus
to see what he thinks, though he'll probably want to revert in any
case...

Thanks,
-- 
Jesse Barnes, Intel Open Source Technology Center

Re: [PATCH 4/4] PCI: make cardbus-bridge resources nice-to-have

[Dominik Brodowski]

Hey,

On Tue, Jun 21, 2011 at 02:36:22PM -0700, Jesse Barnes wrote:
> On Tue, 21 Jun 2011 09:23:21 -0700
> Ram Pai <linuxram@us.ibm.com> wrote:
> 
> > On Tue, Jun 21, 2011 at 09:57:00AM +0200, Dominik Brodowski wrote:
> > > On Mon, Jun 20, 2011 at 03:47:17PM -0700, Ram Pai wrote:
> > > > Allocate resources to cardbus bridge only after all other genuine
> > > > resources requests are satisfied. Dont retry if resource allocation
> > > > for cardbus-bridge fails.
> > > 
> > > Well, for those who use cardbus cards, cardbus resources aren't "nice to
> > > have", they are absolutely required. Of course, not all cardbus cards need
> > > as many resources as are currently assigned, so I wouldn't oppose a patch
> > > which marks _some_ of the currently assigned resources as "nice to have".
> > > But this approach -- 0 required, all "nice to have" -- seems wrong to me.
> > 
> > Do you know how much minimal resource is good enough?  The value, before
> > this patch, was 256 for IO ports and  64M for memory.
> > 
> > BTW: If the BIOS has already assigned enough resources for all the devices on
> > the system, no devices will be starved including the cardbus. The OS intervenes
> > and is forced to make this hard choice only when it sees unassigned resources to
> > some devices along with resource contention.
> 
> Dominik, presumably you have a few good cardbus test machines; can you
> give Ram's patches a try?  If we know they break existing
> configurations, I'm afraid we'll just have to revert the whole
> re-allocation patch yet again.  If your stuff survives, I'll ping Linus
> to see what he thinks, though he'll probably want to revert in any
> case...

Actually, I only have one cardbus-capable test machine, which does work in
very most cases, and also I do care much more about the PCMCIA side of
things than the PCI/CardBus side... Therefore, all I could do is some more
or less informed guessing about how much minimal resource we should try to
allocate...

Best,
	Dominik

Re: [PATCH 4/4] PCI: make cardbus-bridge resources nice-to-have

[Ram Pai]

On Wed, Jun 22, 2011 at 12:13:01AM +0200, Dominik Brodowski wrote:
> Hey,
> 
> On Tue, Jun 21, 2011 at 02:36:22PM -0700, Jesse Barnes wrote:
> > On Tue, 21 Jun 2011 09:23:21 -0700
> > Ram Pai <linuxram@us.ibm.com> wrote:
> > 
> > > On Tue, Jun 21, 2011 at 09:57:00AM +0200, Dominik Brodowski wrote:
> > > > On Mon, Jun 20, 2011 at 03:47:17PM -0700, Ram Pai wrote:
> > > > > Allocate resources to cardbus bridge only after all other genuine
> > > > > resources requests are satisfied. Dont retry if resource allocation
> > > > > for cardbus-bridge fails.
> > > > 
> > > > Well, for those who use cardbus cards, cardbus resources aren't "nice to
> > > > have", they are absolutely required. Of course, not all cardbus cards need
> > > > as many resources as are currently assigned, so I wouldn't oppose a patch
> > > > which marks _some_ of the currently assigned resources as "nice to have".
> > > > But this approach -- 0 required, all "nice to have" -- seems wrong to me.
> > > 
> > > Do you know how much minimal resource is good enough?  The value, before
> > > this patch, was 256 for IO ports and  64M for memory.
> > > 
> > > BTW: If the BIOS has already assigned enough resources for all the devices on
> > > the system, no devices will be starved including the cardbus. The OS intervenes
> > > and is forced to make this hard choice only when it sees unassigned resources to
> > > some devices along with resource contention.
> > 
> > Dominik, presumably you have a few good cardbus test machines; can you
> > give Ram's patches a try?  If we know they break existing
> > configurations, I'm afraid we'll just have to revert the whole
> > re-allocation patch yet again.  If your stuff survives, I'll ping Linus
> > to see what he thinks, though he'll probably want to revert in any
> > case...
> 
> Actually, I only have one cardbus-capable test machine, which does work in
> very most cases, and also I do care much more about the PCMCIA side of
> things than the PCI/CardBus side... Therefore, all I could do is some more
> or less informed guessing about how much minimal resource we should try to
> allocate...

I assume majority of the platforms will have enough resources to satisfy all
the resource requests, and their BIOS would have done a decent job.

Even if the BIOS has not done a decent job, and there are enough resources
available we should not see a regression.

The only platforms that would expose a regression is when resources are under
contention and the BIOS has assigned enough resource to the cardbus bridge but
not to some other device. It will be hard to find such a platform, but I am
sure there is one out somewhere there.

I am sure we will see; some day, reports of regression because that platform
would have the exact right characteristics to expose the issue. But then that
platform is a highly constrained platform in the first place. Its debatable if
that should be characterised as a regression, or a platform that was riding on
good luck till now.

Even Oliver's platform is a highly constrained platform, and we probably can
treat his platform as 'riding on good luck till now'.

We won't be able to satisfy all the platforms with resource constraints.  I
think our probable choice is to select a solution that breaks least number of
platforms and special case those broken platforms through kernel command line
parameters.

RP

Re: [PATCH 4/4] PCI: make cardbus-bridge resources nice-to-have

[Jesse Barnes]

On Tue, 21 Jun 2011 17:48:16 -0700
Ram Pai <linuxram@us.ibm.com> wrote:
> I assume majority of the platforms will have enough resources to satisfy all
> the resource requests, and their BIOS would have done a decent job.
> 
> Even if the BIOS has not done a decent job, and there are enough resources
> available we should not see a regression.
> 
> The only platforms that would expose a regression is when resources are under
> contention and the BIOS has assigned enough resource to the cardbus bridge but
> not to some other device. It will be hard to find such a platform, but I am
> sure there is one out somewhere there.
> 
> I am sure we will see; some day, reports of regression because that platform
> would have the exact right characteristics to expose the issue. But then that
> platform is a highly constrained platform in the first place. Its debatable if
> that should be characterised as a regression, or a platform that was riding on
> good luck till now.
> 
> Even Oliver's platform is a highly constrained platform, and we probably can
> treat his platform as 'riding on good luck till now'.
> 
> We won't be able to satisfy all the platforms with resource constraints.  I
> think our probable choice is to select a solution that breaks least number of
> platforms and special case those broken platforms through kernel command line
> parameters.

Another option is to hide the new allocation behavior behind a kernel
parameter.  I know Bjorn has opposed this in the past because really
this sort of thing should "just work".  But so far it hasn't, and we've
had to revert both Bjorn's resource tracking changes as well as the
re-allocation code.

Hiding it behind a boot option would at least let us improve things
over time and potentially switch over to new resource code in the
future...

Thoughts?

-- 
Jesse Barnes, Intel Open Source Technology Center

Re: [PATCH 4/4] PCI: make cardbus-bridge resources nice-to-have

[Rafael J. Wysocki]

On Thursday, June 23, 2011, Jesse Barnes wrote:
> On Tue, 21 Jun 2011 17:48:16 -0700
> Ram Pai <linuxram@us.ibm.com> wrote:
> > I assume majority of the platforms will have enough resources to satisfy all
> > the resource requests, and their BIOS would have done a decent job.
> > 
> > Even if the BIOS has not done a decent job, and there are enough resources
> > available we should not see a regression.
> > 
> > The only platforms that would expose a regression is when resources are under
> > contention and the BIOS has assigned enough resource to the cardbus bridge but
> > not to some other device. It will be hard to find such a platform, but I am
> > sure there is one out somewhere there.
> > 
> > I am sure we will see; some day, reports of regression because that platform
> > would have the exact right characteristics to expose the issue. But then that
> > platform is a highly constrained platform in the first place. Its debatable if
> > that should be characterised as a regression, or a platform that was riding on
> > good luck till now.
> > 
> > Even Oliver's platform is a highly constrained platform, and we probably can
> > treat his platform as 'riding on good luck till now'.
> > 
> > We won't be able to satisfy all the platforms with resource constraints.  I
> > think our probable choice is to select a solution that breaks least number of
> > platforms and special case those broken platforms through kernel command line
> > parameters.
> 
> Another option is to hide the new allocation behavior behind a kernel
> parameter.  I know Bjorn has opposed this in the past because really
> this sort of thing should "just work".  But so far it hasn't, and we've
> had to revert both Bjorn's resource tracking changes as well as the
> re-allocation code.
> 
> Hiding it behind a boot option would at least let us improve things
> over time and potentially switch over to new resource code in the
> future...
> 
> Thoughts?

Do I understand correctly that at the moment we have two set of systems,
one of which works with the new code and doesn't work with the old code
and the other one conversely?

Rafael

Re: [PATCH 4/4] PCI: make cardbus-bridge resources nice-to-have

[Ram Pai]

On Thu, Jun 23, 2011 at 10:42:29PM +0200, Rafael J. Wysocki wrote:
> On Thursday, June 23, 2011, Jesse Barnes wrote:
> > On Tue, 21 Jun 2011 17:48:16 -0700
> > Ram Pai <linuxram@us.ibm.com> wrote:
> > > I assume majority of the platforms will have enough resources to satisfy all
> > > the resource requests, and their BIOS would have done a decent job.
> > > 
> > > Even if the BIOS has not done a decent job, and there are enough resources
> > > available we should not see a regression.
> > > 
> > > The only platforms that would expose a regression is when resources are under
> > > contention and the BIOS has assigned enough resource to the cardbus bridge but
> > > not to some other device. It will be hard to find such a platform, but I am
> > > sure there is one out somewhere there.
> > > 
> > > I am sure we will see; some day, reports of regression because that platform
> > > would have the exact right characteristics to expose the issue. But then that
> > > platform is a highly constrained platform in the first place. Its debatable if
> > > that should be characterised as a regression, or a platform that was riding on
> > > good luck till now.
> > > 
> > > Even Oliver's platform is a highly constrained platform, and we probably can
> > > treat his platform as 'riding on good luck till now'.
> > > 
> > > We won't be able to satisfy all the platforms with resource constraints.  I
> > > think our probable choice is to select a solution that breaks least number of
> > > platforms and special case those broken platforms through kernel command line
> > > parameters.
> > 
> > Another option is to hide the new allocation behavior behind a kernel
> > parameter.  I know Bjorn has opposed this in the past because really
> > this sort of thing should "just work".  But so far it hasn't, and we've
> > had to revert both Bjorn's resource tracking changes as well as the
> > re-allocation code.
> > 
> > Hiding it behind a boot option would at least let us improve things
> > over time and potentially switch over to new resource code in the
> > future...
> > 
> > Thoughts?
> 
> Do I understand correctly that at the moment we have two set of systems,
> one of which works with the new code and doesn't work with the old code
> and the other one conversely?

Here is the current state:

(a) As of 2.6.39, for platforms whose BIOS have not allocated enough resources to its
devices, those devices will **continue to not work**. An example of such a platform is
the one whose BIOS has not allocated enough resources to SRIOV BARs.

(b) With Yinghai's patch
	the commit "PCI: update bridge resources to get more big ranges when allocating space (again)"
	http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdiff;h=da7822e5ad71ec9b745b412639f1e5e0ba795a20
Most of the  platforms that were not working in (a) will start working, but will break a few platforms, that
have resource constraints and whose BIOS has not allocated enough resources to some of its devices.
Oliver's and Ben Hutching's platform are two of the known platforms; as of now.

(c) with my patch all the above platforms will start working. But the 4th patch in the series
raises a genuine concern that it might break resource-constrained platforms with cardbus bridges.

The question is which one of these is a lesser-evil  :)

Personally I think we should merge all the patches except the 4th patch, and support
Oliver's platform through kernel command line parameter. And I think we should
revert Yinghai's patch for now and merge it with all other patches in the 3.0.1 timeframe
after thorough testing.

RP

Re: [PATCH 4/4] PCI: make cardbus-bridge resources nice-to-have

[Rafael J. Wysocki]

On Friday, June 24, 2011, Ram Pai wrote:
> On Thu, Jun 23, 2011 at 10:42:29PM +0200, Rafael J. Wysocki wrote:
> > On Thursday, June 23, 2011, Jesse Barnes wrote:
> > > On Tue, 21 Jun 2011 17:48:16 -0700
> > > Ram Pai <linuxram@us.ibm.com> wrote:
> > > > I assume majority of the platforms will have enough resources to satisfy all
> > > > the resource requests, and their BIOS would have done a decent job.
> > > > 
> > > > Even if the BIOS has not done a decent job, and there are enough resources
> > > > available we should not see a regression.
> > > > 
> > > > The only platforms that would expose a regression is when resources are under
> > > > contention and the BIOS has assigned enough resource to the cardbus bridge but
> > > > not to some other device. It will be hard to find such a platform, but I am
> > > > sure there is one out somewhere there.
> > > > 
> > > > I am sure we will see; some day, reports of regression because that platform
> > > > would have the exact right characteristics to expose the issue. But then that
> > > > platform is a highly constrained platform in the first place. Its debatable if
> > > > that should be characterised as a regression, or a platform that was riding on
> > > > good luck till now.
> > > > 
> > > > Even Oliver's platform is a highly constrained platform, and we probably can
> > > > treat his platform as 'riding on good luck till now'.
> > > > 
> > > > We won't be able to satisfy all the platforms with resource constraints.  I
> > > > think our probable choice is to select a solution that breaks least number of
> > > > platforms and special case those broken platforms through kernel command line
> > > > parameters.
> > > 
> > > Another option is to hide the new allocation behavior behind a kernel
> > > parameter.  I know Bjorn has opposed this in the past because really
> > > this sort of thing should "just work".  But so far it hasn't, and we've
> > > had to revert both Bjorn's resource tracking changes as well as the
> > > re-allocation code.
> > > 
> > > Hiding it behind a boot option would at least let us improve things
> > > over time and potentially switch over to new resource code in the
> > > future...
> > > 
> > > Thoughts?
> > 
> > Do I understand correctly that at the moment we have two set of systems,
> > one of which works with the new code and doesn't work with the old code
> > and the other one conversely?
> 
> Here is the current state:
> 
> (a) As of 2.6.39, for platforms whose BIOS have not allocated enough resources to its
> devices, those devices will **continue to not work**. An example of such a platform is
> the one whose BIOS has not allocated enough resources to SRIOV BARs.
> 
> (b) With Yinghai's patch
> 	the commit "PCI: update bridge resources to get more big ranges when allocating space (again)"
> 	http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdiff;h=da7822e5ad71ec9b745b412639f1e5e0ba795a20
> Most of the  platforms that were not working in (a) will start working, but will break a few platforms, that
> have resource constraints and whose BIOS has not allocated enough resources to some of its devices.
> Oliver's and Ben Hutching's platform are two of the known platforms; as of now.
> 
> (c) with my patch all the above platforms will start working. But the 4th patch in the series
> raises a genuine concern that it might break resource-constrained platforms with cardbus bridges.
> 
> The question is which one of these is a lesser-evil  :)
> 
> Personally I think we should merge all the patches except the 4th patch, and support
> Oliver's platform through kernel command line parameter. And I think we should
> revert Yinghai's patch for now and merge it with all other patches in the 3.0.1
> timeframe after thorough testing.

Well, I think in that case the default behavior should be as for 2.6.39
and there may be a command line switch turning on some new behavior for
whoever needs that.

This way we'll avoid introducing regressions on systems that don't use the
new command line switch and we'' allow systems that don't work without it to
be handled.

What the new behavior should be is to be determined I guess.

Thanks,
Rafael