From patchwork Thu Mar 9 07:39:07 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?b?UmFmYcWCIE1pxYJlY2tp?= X-Patchwork-Id: 9612397 X-Patchwork-Delegate: bhelgaas@google.com Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id D0682602B4 for ; Thu, 9 Mar 2017 07:39:41 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id C7C0B2857E for ; Thu, 9 Mar 2017 07:39:41 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id BBE9E285A7; Thu, 9 Mar 2017 07:39:41 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.5 required=2.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, FREEMAIL_FROM, RCVD_IN_DNSWL_HI, RCVD_IN_SORBS_SPAM autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 1F7DE2857E for ; Thu, 9 Mar 2017 07:39:41 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751942AbdCIHjk (ORCPT ); Thu, 9 Mar 2017 02:39:40 -0500 Received: from mail-lf0-f66.google.com ([209.85.215.66]:36441 "EHLO mail-lf0-f66.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751355AbdCIHjj (ORCPT ); Thu, 9 Mar 2017 02:39:39 -0500 Received: by mail-lf0-f66.google.com with SMTP id g70so3943925lfh.3 for ; Wed, 08 Mar 2017 23:39:11 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=subject:to:references:cc:from:message-id:date:user-agent :mime-version:in-reply-to:content-transfer-encoding; bh=jY58JMGmPMkEsQa1M5SIkeaWD4EBcmYuaXwXqlcZqsk=; b=hQWei6Y9mIykOdEfrX27LzwhtmQWYSGymAqlVLB/BnxWgvf47GjnAPqYtsqFhLZ9l5 MfE5LLm70PneUjhU0gRks/utuHBQ/sDNdKXyUaXsrpcbmhfSUYf0CqFx5+wEgs0X7JFJ jC1Tjfi1BLiDlq/YfZGCqKwnkeroZyS6JnPsKXrVKMAlCnRa86i03tp4Y8i9ZsJa0E1Q /i6KqwgFbbR+r74ZeD9Ym8Q3b1sUSkjpVg/0lamnfiocT/wVd28lnqrYkrJw69FbVIxi NFzpjfp+8+96OEuJ7C2tCCsSWQmOpkZ7QpErIvsfp1mF8SSeWsgoNhovC+oIPA7iUHcT tm6g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:references:cc:from:message-id:date :user-agent:mime-version:in-reply-to:content-transfer-encoding; bh=jY58JMGmPMkEsQa1M5SIkeaWD4EBcmYuaXwXqlcZqsk=; b=QyzrMwUQE7JC8CR8muhhjOw6JWr4PpXR9vyOLu9rBDU6W03kRexlfxAjceR3DwYZ50 dsFX4gs6QDIOY44dwKNXiaSf3nTrfnHsWw5X9GF1kEuVBh8nTIZaSjeXcud6a4/dEdm3 hT3rAfIjdM8hkOO9tQf4lGar3C8rtVJBFInTdp7ushNFameENyCH5JnuwQ2yiWEPmCTe vIx9543DGWjYxhRo/f6kP52nAhQvQfEDEpxpSRHQgBGOv1ANdhAuHMu0AgAJUHG3USPW joEeuuqFoCJU3j1fpyP0/RGxkR/g8+yXGd23eJM/06SZkgy7cM0L7sXIguMg3C0iIFTu dRFg== X-Gm-Message-State: AMke39m+uCZbBtyyU06GdjiQixnjbzFgb3mdlNXaV/JyWGBEGu84xhJsMnHUmAZ+l2misw== X-Received: by 10.25.181.194 with SMTP id g63mr2496925lfk.47.1489045150999; Wed, 08 Mar 2017 23:39:10 -0800 (PST) Received: from linux-samsung.lan (ip-194-187-74-233.konfederacka.maverick.com.pl. [194.187.74.233]) by smtp.googlemail.com with ESMTPSA id g65sm1094808lfk.10.2017.03.08.23.39.09 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 08 Mar 2017 23:39:10 -0800 (PST) Subject: Re: pcie-iproc: broken 2nd (& 3rd?) controller support by c3245a566400 ("PCI: iproc: Request host bridge window resources") To: Bjorn Helgaas , Ray Jui , Scott Branden , Jon Mason , Oza Oza , JD Zheng , Andy Gospodarek References: Cc: bcm-kernel-feedback-list@broadcom.com, linux-pci@vger.kernel.org, linux-arm-kernel@lists.infradead.org, Felix Fietkau From: =?UTF-8?B?UmFmYcWCIE1pxYJlY2tp?= Message-ID: Date: Thu, 9 Mar 2017 08:39:07 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.6.0 MIME-Version: 1.0 In-Reply-To: Sender: linux-pci-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-pci@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP On 03/08/2017 01:56 PM, Rafał Miłecki wrote: > I just tried upgrading BCM5301X from 4.4 to 4.9 and noticed I don't see card > connected to the 2nd controller. > > [ 2.593534] pcie_iproc_bcma bcma0:7: PCI host bridge to bus 0000:00 > [ 2.599786] pci_bus 0000:00: root bus resource [mem 0x08000000-0x0fffffff] > [ 2.606663] pcie_iproc_bcma bcma0:7: link: UP > [ 2.611316] PCI: bus0: Fast back to back transfers disabled > [ 2.616899] pci 0000:00:00.0: bridge configuration invalid ([bus 00-00]), reconfiguring > [ 2.625395] PCI: bus1: Fast back to back transfers disabled > [ 2.631011] pci 0000:00:00.0: BAR 8: assigned [mem 0x08000000-0x080fffff] > [ 2.637795] pci 0000:01:00.0: BAR 0: assigned [mem 0x08000000-0x08007fff 64bit] > [ 2.645091] pci 0000:00:00.0: PCI bridge to [bus 01] > [ 2.650042] pci 0000:00:00.0: bridge window [mem 0x08000000-0x080fffff] > > [ 2.657199] pcie_iproc_bcma bcma0:8: resource collision: [mem 0x40000000-0x47ffffff] conflicts with PCIe MEM space [mem 0x40000000-0x47ffffff] > [ 2.669946] pcie_iproc_bcma bcma0:8: PCIe controller setup failed > [ 2.676032] pcie_iproc_bcma: probe of bcma0:8 failed with error -16 > > > This used to work with older kernels because there wasn't any collision check: > > [ 2.587117] pcie_iproc_bcma bcma0:7: PCI host bridge to bus 0000:00 > [ 2.593378] pci_bus 0000:00: root bus resource [mem 0x08000000-0x0fffffff] > [ 2.600256] pcie_iproc_bcma bcma0:7: link: UP > [ 2.604888] PCI: bus0: Fast back to back transfers disabled > [ 2.610474] pci 0000:00:00.0: bridge configuration invalid ([bus 00-00]), reconfiguring > [ 2.618973] PCI: bus1: Fast back to back transfers disabled > [ 2.624591] pci 0000:00:00.0: BAR 8: assigned [mem 0x08000000-0x080fffff] > [ 2.631382] pci 0000:01:00.0: BAR 0: assigned [mem 0x08000000-0x08007fff 64bit] > [ 2.638686] pci 0000:00:00.0: PCI bridge to [bus 01] > [ 2.643633] pci 0000:00:00.0: bridge window [mem 0x08000000-0x080fffff] > > [ 2.777118] pcie_iproc_bcma bcma0:8: PCI host bridge to bus 0001:00 > [ 2.783367] pci_bus 0001:00: root bus resource [mem 0x40000000-0x47ffffff] > [ 2.790245] pcie_iproc_bcma bcma0:8: link: UP > [ 2.794862] PCI: bus0: Fast back to back transfers disabled > [ 2.800452] pci 0001:00:00.0: bridge configuration invalid ([bus 00-00]), reconfiguring > [ 2.808946] PCI: bus1: Fast back to back transfers disabled > [ 2.814558] pci 0001:00:00.0: BAR 8: assigned [mem 0x40000000-0x400fffff] > [ 2.821352] pci 0001:01:00.0: BAR 0: assigned [mem 0x40000000-0x40007fff 64bit] > [ 2.828650] pci 0001:00:00.0: PCI bridge to [bus 01] > [ 2.833600] pci 0001:00:00.0: bridge window [mem 0x40000000-0x400fffff] > > > I guess the check is OK after all and the real problem is iproc driver assigning > the same resource. > > Broadcom team: could you take a look at this, please? I found a reason of this conflict (and probably random crashes I started seeing with 4.9). I believe we have a memory corruption. So that commit c3245a566400 ("PCI: iproc: Request host bridge window resources") adds call to the devm_request_pci_bus_resources passing "res" pointer. The problem is "res" points to the *local* variable of iproc_pcie_bcma_probe function. As soon as the iproc_pcie_bcma_probe exits that resource variable is not accessible anymore, yet it's used as a child for &iomem_resource. Things go even worse on another iproc_pcie_bcma_probe call. Its local variable res_mem gets the same memory address as the one already used for &iomem_resource. We modify local variable modifying already-added resource at the same time! So this is where this whole conflicts comes from. What is stored as resource for [mem 0x08000000-0x0fffffff] range gets modified as "local" variable to the [mem 0x40000000-0x47ffffff] and then we try to re-request the same resource. I'm pasting log & patch that allowed me to debug & notice this problem so you can confirm my observations. [ 2.615055] resource: [__request_resource] root:[mem 0x00000000-0xffffffff] [ 2.621995] resource: [__request_resource] new:c7839be0 new:[mem 0x08000000-0x0fffffff] [ 2.630060] resource: [__request_resource] tmp:c7ffee00 tmp:[mem 0x00000000-0x07ffffff] [ 2.638122] resource: [__request_resource] tmp:c7adf180 tmp:[mem 0x18000300-0x180003ff] [ 2.764941] pcie_iproc_bcma bcma0:7: PCI host bridge to bus 0000:00 [ 2.771194] pci_bus 0000:00: root bus resource [mem 0x08000000-0x0fffffff] [ 2.778066] pcie_iproc_bcma bcma0:7: link: UP [ 2.782693] PCI: bus0: Fast back to back transfers disabled [ 2.788267] pci 0000:00:00.0: bridge configuration invalid ([bus 00-00]), reconfiguring [ 2.796767] PCI: bus1: Fast back to back transfers disabled [ 2.802375] resource: [__request_resource] root:[mem 0x08000000-0x0fffffff] [ 2.809324] resource: [__request_resource] new:c7b08e64 new:[mem size 0x00100000] [ 2.816865] resource: [__request_resource] tmp: (null) tmp: (null) [ 2.823282] pci 0000:00:00.0: BAR 8: assigned [mem 0x08000000-0x080fffff] [ 2.830049] resource: [__request_resource] root:[mem 0x08000000-0x080fffff] [ 2.836986] resource: [__request_resource] new:c7b09164 new:[mem size 0x00008000 64bit] [ 2.845047] resource: [__request_resource] tmp: (null) tmp: (null) [ 2.851464] pci 0000:01:00.0: BAR 0: assigned [mem 0x08000000-0x08007fff 64bit] [ 2.858758] pci 0000:00:00.0: PCI bridge to [bus 01] [ 2.863700] pci 0000:00:00.0: bridge window [mem 0x08000000-0x080fffff] [ 2.870834] resource: [__request_resource] root:[mem 0x00000000-0xffffffff] [ 2.877791] resource: [__request_resource] new:c7839be0 new:[mem 0x40000000-0x47ffffff] [ 2.885855] resource: [__request_resource] tmp:c7ffee00 tmp:[mem 0x00000000-0x07ffffff] [ 2.893911] resource: [__request_resource] tmp:c7839be0 tmp:[mem 0x40000000-0x47ffffff] [ 2.901971] resource: [__request_resource] Found collision with tmp [ 2.908225] pcie_iproc_bcma bcma0:8: resource collision: [mem 0x40000000-0x47ffffff] conflicts with PCIe MEM space [mem 0x40000000-0x47ffffff] [ 2.920952] pcie_iproc_bcma bcma0:8: PCIe controller setup failed [ 2.927037] pcie_iproc_bcma: probe of bcma0:8 failed with error -16 As you can see, for the first controller following resource has been requested: c7839be0 [mem 0x08000000-0x0fffffff] Then when we try to probe second controller there are following resources already in use: c7ffee00 [mem 0x00000000-0x07ffffff] c7839be0 [mem 0x40000000-0x47ffffff] and we try to request: c7839be0 [mem 0x40000000-0x47ffffff] diff --git a/kernel/resource.c b/kernel/resource.c index 9b5f044..fab9405 100644 --- a/kernel/resource.c +++ b/kernel/resource.c @@ -220,6 +220,8 @@ static struct resource * __request_resource(struct resource *root, struct resour resource_size_t end = new->end; struct resource *tmp, **p; + if (new->flags & IORESOURCE_MEM) pr_info("[%s] root:%pR\n", __func__, root); + if (new->flags & IORESOURCE_MEM) pr_info("[%s] new:%p new:%pR\n", __func__, new, new); if (end < start) return root; if (start < root->start) @@ -229,6 +231,7 @@ static struct resource * __request_resource(struct resource *root, struct resour p = &root->child; for (;;) { tmp = *p; + if (new->flags & IORESOURCE_MEM) pr_info("[%s] tmp:%p tmp:%pR\n", __func__, tmp, tmp); if (!tmp || tmp->start > end) { new->sibling = tmp; *p = new; @@ -238,6 +241,7 @@ static struct resource * __request_resource(struct resource *root, struct resour p = &tmp->sibling; if (tmp->end < start) continue; + if (new->flags & IORESOURCE_MEM) pr_info("[%s] Found collision with tmp\n", __func__); return tmp; } }