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: 4234621 X-Patchwork-Delegate: bhelgaas@google.com Return-Path: X-Original-To: patchwork-linux-pci@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 DFCFBBF90B for ; Fri, 23 May 2014 17:51:41 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id CB169203A9 for ; Fri, 23 May 2014 17:51:40 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 7C91F202C8 for ; Fri, 23 May 2014 17:51:39 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751263AbaEWRvN (ORCPT ); Fri, 23 May 2014 13:51:13 -0400 Received: from mail-pb0-f41.google.com ([209.85.160.41]:34169 "EHLO mail-pb0-f41.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751246AbaEWRvL (ORCPT ); Fri, 23 May 2014 13:51:11 -0400 Received: by mail-pb0-f41.google.com with SMTP id uo5so4500412pbc.28 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=VrILxeOOM4JDxB/BO5SrFy4Xkq8o0A2ekgsyXPtcdcKCimc7iRmXZCcp1PfNKavMSB KLhIK5GNy+BKu0ba/C+SaO6we/bJDjTPx4y/Spms/cDBl97/EpaZmltbMQHJG3NVHPil s5eLzUPcHyVhMTbG2ti/zNBXpFlgxuXU7YLw/U/FkIac4F3AV6mw8YkKtrTZTQ0CLB0u Zg5q4OcYlPp9s5+6GQ9udl+y7SVHVsYpXt9LuzJggc+BvmrrdbUpH16Rflfaf0BJlX3C 5ubCHURZp1sgfFZ0yyoTkFXOo5OuQrFbhqfgfa2LWDGy13z64UPzGzMuGFWvK367v4CR wpSQ== X-Gm-Message-State: ALoCoQnIYrcfdM0Pwg3jti2M6MfMYP/Rcq40yWzfx93/2u/DK0476CTptoGOZ8DMXaDE8bTOCr6M 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 Cc: Alan , linux-pci@vger.kernel.org, linux-kernel@vger.kernel.org, Nikhil P Rao , Olof Johansson , Arnd Bergmann , Jason Cooper , linux-arm-kernel 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 Sender: linux-pci-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-pci@vger.kernel.org X-Spam-Status: No, score=-7.5 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_HI, 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)