From patchwork Wed May 11 01:19:17 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Yinghai Lu X-Patchwork-Id: 775642 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by demeter2.kernel.org (8.14.4/8.14.3) with ESMTP id p4B1IV6Y007355 for ; Wed, 11 May 2011 01:20:04 GMT Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752565Ab1EKBUE (ORCPT ); Tue, 10 May 2011 21:20:04 -0400 Received: from rcsinet10.oracle.com ([148.87.113.121]:53031 "EHLO rcsinet10.oracle.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752536Ab1EKBUD (ORCPT ); Tue, 10 May 2011 21:20:03 -0400 Received: from rtcsinet21.oracle.com (rtcsinet21.oracle.com [66.248.204.29]) by rcsinet10.oracle.com (Switch-3.4.2/Switch-3.4.2) with ESMTP id p4B1JQ1R014268 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Wed, 11 May 2011 01:19:29 GMT Received: from acsmt358.oracle.com (acsmt358.oracle.com [141.146.40.158]) by rtcsinet21.oracle.com (8.14.4+Sun/8.14.4) with ESMTP id p4B1JOWs011131 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Wed, 11 May 2011 01:19:25 GMT Received: from abhmt014.oracle.com (abhmt014.oracle.com [141.146.116.23]) by acsmt358.oracle.com (8.12.11.20060308/8.12.11) with ESMTP id p4B1JIof023496; Tue, 10 May 2011 20:19:18 -0500 Received: from linux-siqj.site (/129.146.63.245) by default (Oracle Beehive Gateway v4.0) with ESMTP ; Tue, 10 May 2011 18:19:17 -0700 Message-ID: <4DC9E415.5010402@kernel.org> Date: Tue, 10 May 2011 18:19:17 -0700 From: Yinghai Lu User-Agent: Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.2.17) Gecko/20110414 SUSE/3.1.10 Thunderbird/3.1.10 MIME-Version: 1.0 To: Linus Torvalds CC: Jesse Barnes , Ram Pai , "linux-kernel@vger.kernel.org" , "linux-pci@vger.kernel.org" , Andrew Morton , Bjorn Helgaas Subject: Re: [PATCH -v2] pci: Check bridge resources after resource allocation. References: <4DC250A6.9040802@kernel.org> <20110506081243.GF16782@ram-laptop> <4DC4586C.6020308@kernel.org> <20110507015240.GN16782@ram-laptop> <4DC4B059.3030502@kernel.org> <4DC64C58.70203@kernel.org> <20110509142014.617e3100@jbarnes-desktop> In-Reply-To: X-Source-IP: rtcsinet21.oracle.com [66.248.204.29] X-CT-RefId: str=0001.0A090208.4DC9E422.00E7,ss=1,fgs=0 Sender: linux-pci-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-pci@vger.kernel.org X-Greylist: IP, sender and recipient auto-whitelisted, not delayed by milter-greylist-4.2.6 (demeter2.kernel.org [140.211.167.43]); Wed, 11 May 2011 01:20:04 +0000 (UTC) On 05/09/2011 03:36 PM, Linus Torvalds wrote: > On Mon, May 9, 2011 at 2:20 PM, Jesse Barnes wrote: >> >> Linus? Bjorn? Ram? > > I dunno. The patch really makes me go "that looks *broken*". I really > dislike it. But maybe the crappiness of the patch comes from the > horror that is the current code. please check if this one is ok. [PATCH] pci: Clear bridge resource flags if requested size is 0 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 cause by commit | commit c8adf9a3e873eddaaec11ac410a99ef6b9656938 | Author: Ram Pai | 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); | } | } | } At last, We have to clear the flags in pbus_size_mem/io and etc. also need to update adjust_resources_sorted() to handle this special case if requested_size is 0, but add_size is not 0. 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: Signed-off-by: Yinghai Lu --- drivers/pci/setup-bus.c | 31 +++++++++++++++++++++---------- 1 file changed, 21 insertions(+), 10 deletions(-) -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html 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 @@ -138,9 +138,26 @@ static void adjust_resources_sorted(stru for (list = add_head->next; list;) { res = list->res; /* skip resource that has been reset */ - if (!res->flags) + if (!res->flags && !res->start && !res->end) goto out; + idx = res - &list->dev->resource[0]; + add_size = list->add_size; + if (!add_size) { + dev_warn(&list->dev->dev, "idx %d add_size == 0\n", + idx); + goto out; + } + + if (!resource_size(res)) { + /* need to restore the flag */ + res->flags = list->flags; + res->end = res->start + add_size - 1; + if (pci_assign_resource(list->dev, idx)) + reset_resource(res); + goto out; + } + /* skip this resource if not found in head list */ for (hlist = head->next; hlist && hlist->res != res; hlist = hlist->next); @@ -150,16 +167,8 @@ static void adjust_resources_sorted(stru continue; } - 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)) - reset_resource(res); - } else if (add_size) { - adjust_resource(res, res->start, + adjust_resource(res, res->start, resource_size(res) + add_size); - } out: tmp = list; prev->next = list = list->next; @@ -596,6 +605,8 @@ static void pbus_size_io(struct pci_bus b_res->flags |= IORESOURCE_STARTALIGN; if (size1 > size0 && add_head) add_to_list(add_head, bus->self, b_res, size1-size0); + if (!size0) + b_res->flags = 0; } /** @@ -693,6 +704,8 @@ static int pbus_size_mem(struct pci_bus b_res->flags |= IORESOURCE_STARTALIGN | mem64_mask; if (size1 > size0 && add_head) add_to_list(add_head, bus->self, b_res, size1-size0); + if (!size0) + b_res->flags = 0; return 1; }