linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Yinghai Lu <yinghai@kernel.org>
To: Jesse Barnes <jbarnes@virtuousgeek.org>
Cc: Ram Pai <linuxram@us.ibm.com>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"linux-pci@vger.kernel.org" <linux-pci@vger.kernel.org>,
	Linus Torvalds <torvalds@linux-foundation.org>,
	Andrew Morton <akpm@linux-foundation.org>
Subject: [PATCH -v2] pci: Check bridge resources after resource allocation.
Date: Sun, 08 May 2011 00:55:04 -0700	[thread overview]
Message-ID: <4DC64C58.70203@kernel.org> (raw)
In-Reply-To: <4DC4B059.3030502@kernel.org>


During pci remove/rescan testing found:

[  541.141614] pci 0000:c0:03.0: PCI bridge to [bus c4-c9]
[  541.141965] pci 0000:c0:03.0:   bridge window [io  0x1000-0x0fff]
[  541.159181] pci 0000:c0:03.0:   bridge window [mem 0xf0000000-0xf00fffff]
[  541.159540] pci 0000:c0:03.0:   bridge window [mem 0xfc180000000-0xfc197ffffff 64bit pref]
[  541.179374] pci 0000:c0:03.0: device not available (can't reserve [io  0x1000-0x0fff])
[  541.199198] pci 0000:c0:03.0: Error enabling bridge (-22), continuing
[  541.199202] pci 0000:c0:03.0: enabling bus mastering
[  541.199209] pci 0000:c0:03.0: setting latency timer to 64
[  541.199917] pcieport 0000:c0:03.0: device not available (can't reserve [io  0x1000-0x0fff])
[  541.199963] pcieport: probe of 0000:c0:03.0 failed with error -22

This bug was uncovered by commit
| commit c8adf9a3e873eddaaec11ac410a99ef6b9656938
| Author: Ram Pai <linuxram@us.ibm.com>
| Date:   Mon Feb 14 17:43:20 2011 -0800
|
|    PCI: pre-allocate additional resources to devices only after successful allocation of essential resources.

After that commit, pci_hotplug_io_size is changed to additional_io_size from minium size.
So it will not go through resource_size(res) != 0 path,  and will not be reset there.

The root cause is: pci_bridge_check_ranges will set RESOURCE_IO flag for pci
bridge, and later if children do not need IO resource. those bridge
resources will not need to be allocated. but flags is still there. that will
confuse the the pci_enable_bridges later.

related code:
| static void assign_requested_resources_sorted(struct resource_list *head,
|                                  struct resource_list_x *fail_head)
| {
|         struct resource *res;
|         struct resource_list *list;
|         int idx;
|
|         for (list = head->next; list; list = list->next) {
|                 res = list->res;
|                 idx = res - &list->dev->resource[0];
|                 if (resource_size(res) && pci_assign_resource(list->dev, idx)) {
| ...
|                         reset_resource(res);
|                 }
|         }
| }

We can not just reset resource there, because will still need to use flag for addition resource allocation afterwards.

At last, We have to add pci_bridge_check_resources() to close the loop.

after patch, will get right result:
[  621.206655] pci 0000:c0:03.0: PCI bridge to [bus c4-c9]
[  621.206912] pci 0000:c0:03.0:   bridge window [io  disabled]
[  621.226594] pci 0000:c0:03.0:   bridge window [mem 0xf0000000-0xf00fffff]
[  621.226904] pci 0000:c0:03.0:   bridge window [mem 0xfc180000000-0xfc197ffffff 64bit pref]
[  621.247012] pci 0000:c0:03.0: enabling bus mastering
[  621.247275] pci 0000:c0:03.0: setting latency timer to 64
[  621.267656] pcieport 0000:c0:03.0: setting latency timer to 64
[  621.268134] pcieport 0000:c0:03.0: irq 160 for MSI/MSI-X
[  621.286832] pcieport 0000:c0:03.0: Signaling PME through PCIe PME interrupt
[  621.306360] pci 0000:c4:00.0: Signaling PME through PCIe PME interrupt
[  621.306684] pcie_pme 0000:c0:03.0:pcie01: service driver pcie_pme loaded
[  621.326512] aer 0000:c0:03.0:pcie02: service driver aer loaded
[  621.326911] pciehp 0000:c0:03.0:pcie04: Hotplug Controller:

-v2: update description.

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


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

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
@@ -811,6 +811,27 @@ void __ref pci_bus_size_bridges(struct p
 }
 EXPORT_SYMBOL(pci_bus_size_bridges);
 
+static void __ref pci_bridge_check_resources(struct pci_dev *bridge)
+{
+	struct resource *b_res;
+	int i;
+
+	b_res = &bridge->resource[PCI_BRIDGE_RESOURCES];
+
+	/*
+	 * We set resource flag in pci_bridge_check_ranges() for further
+	 *  allocation for children devices.
+	 * Then if children do not need some kind of resource (like IO), and
+	 *  bridge resource will not be allocated.
+	 * At last, those flags could be left set, that will confuse
+	 *  pci_setup_bridge() and pci_enable_bridge()
+	 * So use reset_resource to clear them.
+	 */
+	for (i = 0; i < PCI_BRIDGE_RESOURCE_NUM; i++)
+		if (!resource_size(&b_res[i]))
+			reset_resource(&b_res[i]);
+}
+
 static void __ref __pci_bus_assign_resources(const struct pci_bus *bus,
 					 struct resource_list_x *add_head,
 					 struct resource_list_x *fail_head)
@@ -820,6 +841,9 @@ static void __ref __pci_bus_assign_resou
 
 	pbus_assign_resources_sorted(bus, add_head, fail_head);
 
+	if (bus->self)
+		pci_bridge_check_resources(bus->self);
+
 	list_for_each_entry(dev, &bus->devices, bus_list) {
 		b = dev->subordinate;
 		if (!b)
@@ -862,6 +886,8 @@ static void __ref __pci_bridge_assign_re
 	if (!b)
 		return;
 
+	pci_bridge_check_resources((struct pci_dev *)bridge);
+
 	__pci_bus_assign_resources(b, NULL, fail_head);
 
 	switch (bridge->class >> 8) {

  reply	other threads:[~2011-05-08  7:55 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-05-05  7:24 [PATCH] pci: Check bridge resources after resource allocation Yinghai Lu
2011-05-06  8:12 ` Ram Pai
2011-05-06 20:22   ` Yinghai Lu
2011-05-06 20:43     ` [PATCH 1/2] pci: Using add_list in pcie hotplug path Yinghai Lu
2011-05-06 20:44     ` [PATCH 2/2] pci: honor child buses add_size in hot plug configuration Yinghai Lu
2011-05-07  1:52     ` [PATCH] pci: Check bridge resources after resource allocation Ram Pai
2011-05-07  2:37       ` Yinghai Lu
2011-05-08  7:55         ` Yinghai Lu [this message]
2011-05-09 21:20           ` [PATCH -v2] " Jesse Barnes
2011-05-09 22:36             ` Linus Torvalds
2011-05-11  1:19               ` Yinghai Lu
2011-05-12 18:06                 ` Ram Pai
2011-05-12 18:14                   ` Linus Torvalds
2011-05-12 18:22                     ` Ram Pai
2011-05-12 18:37                       ` Jesse Barnes
2011-05-12 19:18                         ` Linus Torvalds
2011-05-12 19:34                           ` Jesse Barnes
2011-05-14  1:06                             ` Yinghai Lu
2011-05-16  7:59                               ` Ram Pai
2011-05-16 20:55                                 ` Yinghai Lu
2011-05-16 22:36                                   ` Ram Pai
2011-05-17  3:52                                     ` Jesse Barnes
2011-05-17  5:22                                       ` Linus Torvalds
2011-05-12 18:22                     ` Yinghai Lu
2011-05-12 18:30                   ` Yinghai Lu

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4DC64C58.70203@kernel.org \
    --to=yinghai@kernel.org \
    --cc=akpm@linux-foundation.org \
    --cc=jbarnes@virtuousgeek.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=linuxram@us.ibm.com \
    --cc=torvalds@linux-foundation.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).