From patchwork Fri May 23 17:51:08 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Kevin Hilman X-Patchwork-Id: 4234631 Return-Path: X-Original-To: patchwork-linux-arm@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork2.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.19.201]) by patchwork2.web.kernel.org (Postfix) with ESMTP id C7644BF90B for ; Fri, 23 May 2014 17:54:47 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 99C2A203AD for ; Fri, 23 May 2014 17:54:46 +0000 (UTC) Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.9]) (using TLSv1.2 with cipher DHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 5C83B203A9 for ; Fri, 23 May 2014 17:54:45 +0000 (UTC) Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.80.1 #2 (Red Hat Linux)) id 1Wntd2-000815-Ve; Fri, 23 May 2014 17:51:36 +0000 Received: from mail-pb0-f46.google.com ([209.85.160.46]) by bombadil.infradead.org with esmtps (Exim 4.80.1 #2 (Red Hat Linux)) id 1Wntcy-0007wE-Tu for linux-arm-kernel@lists.infradead.org; Fri, 23 May 2014 17:51:34 +0000 Received: by mail-pb0-f46.google.com with SMTP id rq2so4431928pbb.5 for ; Fri, 23 May 2014 10:51:10 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:from:to:cc:subject:references:date:in-reply-to :message-id:user-agent:mime-version:content-type; bh=lEPE+KRSd6iNcSK+bfJQ7Q/QTwguexK9Wt/Ur9R2VQE=; b=SLiIbko6LaKNXXWBPxqrNJZFoVio4e54CbZGJHxnLLpZjXB0F3qqsfQdWH0yTuXd/Z 7hS87kqTc0LrGE64LJGOqq/eQ8XPS+APtN1y6+fHTVhBNPsmHGkglxzQMIsUkkd9zKIH hClwevnXPRug/7RY0SfVh5AdxqX3LpWHDxUvC/LXN67u+4Aog8X36KrPhKwSQ3kjVu6o mS4u8JPUK/hhwk6LsvfoVXY7T0WFGpPNs3oDzwxd+uiGyVUIVwtaZhSy8oquPVcDplv8 UQ1W7APFz9POEmml7CFhROeFqXet6wP5+8/GuKRD3LQyW50ii5S2FlYao5NXmwFiNMsG NA/A== X-Gm-Message-State: ALoCoQm1h3oz6CliPIE9WZpHATn3swHSvGvlrXhPwYRTFiwzJhjKI0RIGjcHZtrmn9dlyBoMJ4JM X-Received: by 10.68.250.3 with SMTP id yy3mr8157242pbc.56.1400867470751; Fri, 23 May 2014 10:51:10 -0700 (PDT) Received: from localhost (c-67-183-17-239.hsd1.wa.comcast.net. [67.183.17.239]) by mx.google.com with ESMTPSA id om6sm5635904pbc.43.2014.05.23.10.51.09 for (version=TLSv1.2 cipher=RC4-SHA bits=128/128); Fri, 23 May 2014 10:51:10 -0700 (PDT) From: Kevin Hilman To: Bjorn Helgaas Subject: Re: [PATCH] pci: Allow very large resource windows References: <20140519130255.15364.49358.stgit@alan.etchedpixels.co.uk> <20140519202853.GA1371@google.com> Date: Fri, 23 May 2014 10:51:08 -0700 In-Reply-To: <20140519202853.GA1371@google.com> (Bjorn Helgaas's message of "Mon, 19 May 2014 14:28:53 -0600") Message-ID: <7h38g0e643.fsf@paris.lan> User-Agent: Gnus/5.130008 (Ma Gnus v0.8) Emacs/24.3 (gnu/linux) MIME-Version: 1.0 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20140523_105133_039993_A047CC4C X-CRM114-Status: GOOD ( 29.41 ) X-Spam-Score: -0.7 (/) Cc: Alan , Jason Cooper , Arnd Bergmann , linux-pci@vger.kernel.org, linux-kernel@vger.kernel.org, Olof Johansson , Nikhil P Rao , linux-arm-kernel X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+patchwork-linux-arm=patchwork.kernel.org@lists.infradead.org X-Spam-Status: No, score=-2.5 required=5.0 tests=BAYES_00,RP_MATCHES_RCVD, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP Hi Bjorn, Bjorn Helgaas writes: > On Mon, May 19, 2014 at 02:03:14PM +0100, Alan wrote: >> From: Alan >> >> This is needed for some of the Xeon Phi type systems. >> >> Signed-off-by: Alan Cox > > I applied this to my pci/resource branch for v3.16. Nikhil > posted essentially the same patch a couple years ago, so I added > his signed-off-by and adopted his use of ARRAY_SIZE() to connect the > "order > 13" test with the aligns[] declaration. This patch (with the ARRAY_SIZE changes) has hit next-20140253 (in the form of commit 272c5a886c57) and according to my bisect, is the cause of a new boot failure on a 32-bit ARM platform (Marvell Armada 370, Mirabox, boot log excerpt down below[2].) While debugging, I found that Alan's original patch (without the ARRAY_SIZE change) booted just fine so I started looking closely at the ARRAY_SIZE() change. After some high-tech printk debugging, I noticed that order was negative when doing the compare, and found that this hack got things booting again: diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c index 47482b27fa12..792db3477fd5 100644 --- a/drivers/pci/setup-bus.c +++ b/drivers/pci/setup-bus.c @@ -966,7 +966,7 @@ static int pbus_size_mem(struct pci_bus *bus, unsigned long mask, /* For bridges size != alignment */ align = pci_resource_alignment(dev, r); order = __ffs(align) - 20; - if (order >= ARRAY_SIZE(aligns)) { + if (order >= (int)ARRAY_SIZE(aligns)) { dev_warn(&dev->dev, "disabling BAR %d: %pR " "(bad alignment %#llx)\n", i, r, (unsigned long long) align); which led me to realize that the ARRAY_SIZE() change converted the >= into an unsigned compare where before it was a signed one, and as an unsigned compare, it was always true, resulting in those BARs being disabled. Below is a patch[1] which fixes the problem for me, and attempts a more detailed description of the problem in the changelog. Kevin [1] From 386d39a22e586fc1b060ad18c79247a50f2c0f8c Mon Sep 17 00:00:00 2001 From: Kevin Hilman Date: Fri, 23 May 2014 10:24:40 -0700 Subject: [PATCH] PCI: pbus_mem_size(): don't disable small BAR windows Since commit 272c5a886c57 (PCI: Support BAR sizes up to 8GB), small BAR windows get disabled. For small BAR sizes, order (as computed by __ffs(align) - 20) may be negative. If order is negative, when checking if it's >= ARRAY_SIZE(aligns) will always be true (since the result of ARRAY_SIZE() is unsigned) and so those BARs will be disabled. It doesn't make sense for order to be negative, and the code already converts it to non-negative later on, so to fix this bug, ensure that order is non-negative before comapring with ARRAY_SIZE(). NOTE: Before commit 272c5a886c57, this worked just fine because order wasn't compared with ARRAY_SIZE() but with a hard-coded int, so the resulting signed compare worked fine. Signed-off-by: Kevin Hilman --- drivers/pci/setup-bus.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c index 47482b27fa12..a4152566f531 100644 --- a/drivers/pci/setup-bus.c +++ b/drivers/pci/setup-bus.c @@ -966,6 +966,8 @@ static int pbus_size_mem(struct pci_bus *bus, unsigned long mask, /* For bridges size != alignment */ align = pci_resource_alignment(dev, r); order = __ffs(align) - 20; + if (order < 0) + order = 0; if (order >= ARRAY_SIZE(aligns)) { dev_warn(&dev->dev, "disabling BAR %d: %pR " "(bad alignment %#llx)\n", i, r, @@ -974,8 +976,6 @@ static int pbus_size_mem(struct pci_bus *bus, unsigned long mask, continue; } size += r_size; - if (order < 0) - order = 0; /* Exclude ranges with size > align from calculation of the alignment. */ if (r_size == align)