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: 30839 Return-Path: X-Original-To: linaro@patches.linaro.org Delivered-To: linaro@patches.linaro.org Received: from mail-pa0-f69.google.com (mail-pa0-f69.google.com [209.85.220.69]) by ip-10-151-82-157.ec2.internal (Postfix) with ESMTPS id D9F6D20369 for ; Fri, 23 May 2014 17:51:36 +0000 (UTC) Received: by mail-pa0-f69.google.com with SMTP id ey11sf19525447pad.0 for ; Fri, 23 May 2014 10:51:36 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:delivered-to:from:to:cc:subject:references:date :in-reply-to:message-id:user-agent:mime-version:sender:precedence :list-id:x-original-sender:x-original-authentication-results :mailing-list:list-post:list-help:list-archive:list-unsubscribe :content-type; bh=YP5GfoC+6/Cz7Qh3K2FCr1gehAtt6OjTvJLhaoRFdzY=; b=b0eup5lXt6nWFGCd0pgL7DPeHwLUEIJUYoA5I/1abLIiFI5Y5LOurdo+CMEJEOfoZl NbkYabbcfIJS/u8pPNCCIHEhHPiu5iM6aVSks/yEZlTRtp7cDLmPICwc+WlUMCIFyu3J gsbaRjpTe093uzFc+1zcKa+ca1DUU9AOB+8u5N7d2O+gMf1tPZNAsRA3fbqsqSWxTaAX 67d/hSjGELpqIaO49Sa49W2sDE7l7dJdI3HHXHRfRY8JAcgOA7Sy2vX3iJJ60+VCqlr2 CMwaM+IguvVDjQmLlXT5lv/xrLHkMm9LahXKJ5OldBKEozvyjEMssVg9lqkfVVqRbktJ vMVg== X-Gm-Message-State: ALoCoQnJSxA2227B2GE0a1jzEhGwg3iKWkDxxa9CwINiBpAUSCZj9s84OUefxBIt7NDFFyJ78GBs X-Received: by 10.66.149.67 with SMTP id ty3mr2650715pab.27.1400867496217; Fri, 23 May 2014 10:51:36 -0700 (PDT) X-BeenThere: patchwork-forward@linaro.org Received: by 10.140.81.145 with SMTP id f17ls1764721qgd.95.gmail; Fri, 23 May 2014 10:51:36 -0700 (PDT) X-Received: by 10.58.209.233 with SMTP id mp9mr5704680vec.30.1400867496015; Fri, 23 May 2014 10:51:36 -0700 (PDT) Received: from mail-ve0-f171.google.com (mail-ve0-f171.google.com [209.85.128.171]) by mx.google.com with ESMTPS id sw4si2064119vdc.69.2014.05.23.10.51.35 for (version=TLSv1 cipher=ECDHE-RSA-RC4-SHA bits=128/128); Fri, 23 May 2014 10:51:35 -0700 (PDT) Received-SPF: pass (google.com: domain of patch+caf_=patchwork-forward=linaro.org@linaro.org designates 209.85.128.171 as permitted sender) client-ip=209.85.128.171; Received: by mail-ve0-f171.google.com with SMTP id oz11so6650560veb.16 for ; Fri, 23 May 2014 10:51:35 -0700 (PDT) X-Received: by 10.52.11.230 with SMTP id t6mr4626860vdb.27.1400867495885; Fri, 23 May 2014 10:51:35 -0700 (PDT) X-Forwarded-To: patchwork-forward@linaro.org X-Forwarded-For: patch@linaro.org patchwork-forward@linaro.org Delivered-To: patch@linaro.org Received: by 10.220.221.72 with SMTP id ib8csp50685vcb; Fri, 23 May 2014 10:51:35 -0700 (PDT) X-Received: by 10.68.245.162 with SMTP id xp2mr8019280pbc.69.1400867494164; Fri, 23 May 2014 10:51:34 -0700 (PDT) Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id la14si4905124pab.163.2014.05.23.10.51.33 for ; Fri, 23 May 2014 10:51:33 -0700 (PDT) Received-SPF: none (google.com: linux-kernel-owner@vger.kernel.org does not designate permitted sender hosts) client-ip=209.132.180.67; Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751386AbaEWRvO (ORCPT + 27 others); Fri, 23 May 2014 13:51:14 -0400 Received: from mail-pb0-f54.google.com ([209.85.160.54]:56039 "EHLO mail-pb0-f54.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751168AbaEWRvL (ORCPT ); Fri, 23 May 2014 13:51:11 -0400 Received: by mail-pb0-f54.google.com with SMTP id jt11so4487448pbb.27 for ; Fri, 23 May 2014 10:51:10 -0700 (PDT) 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-kernel-owner@vger.kernel.org Precedence: list List-ID: X-Mailing-List: linux-kernel@vger.kernel.org X-Removed-Original-Auth: Dkim didn't pass. X-Original-Sender: khilman@linaro.org X-Original-Authentication-Results: mx.google.com; spf=pass (google.com: domain of patch+caf_=patchwork-forward=linaro.org@linaro.org designates 209.85.128.171 as permitted sender) smtp.mail=patch+caf_=patchwork-forward=linaro.org@linaro.org Mailing-list: list patchwork-forward@linaro.org; contact patchwork-forward+owners@linaro.org X-Google-Group-Id: 836684582541 List-Post: , List-Help: , List-Archive: List-Unsubscribe: , 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)