From patchwork Wed Sep 23 19:35:26 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Arnd Bergmann X-Patchwork-Id: 7252151 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.29.136]) by patchwork2.web.kernel.org (Postfix) with ESMTP id 644A1BEEC1 for ; Wed, 23 Sep 2015 19:57:38 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 685ED2094A for ; Wed, 23 Sep 2015 19:57:37 +0000 (UTC) Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.9]) (using TLSv1.2 with cipher AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 7969020942 for ; Wed, 23 Sep 2015 19:57:36 +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 1Zeppk-0005TP-Vv; Wed, 23 Sep 2015 19:36:04 +0000 Received: from mout.kundenserver.de ([212.227.17.24]) by bombadil.infradead.org with esmtps (Exim 4.80.1 #2 (Red Hat Linux)) id 1Zepph-0005Ky-Jz for linux-arm-kernel@lists.infradead.org; Wed, 23 Sep 2015 19:36:02 +0000 Received: from wuerfel.localnet ([149.172.15.242]) by mrelayeu.kundenserver.de (mreue103) with ESMTPSA (Nemesis) id 0MJnBU-1Zfvkt2psq-0016ZM; Wed, 23 Sep 2015 21:35:31 +0200 From: Arnd Bergmann To: David Daney Subject: Re: [PATCH v3 4/6] PCI: generic: Correct, and avoid overflow, in bus_max calculation. Date: Wed, 23 Sep 2015 21:35:26 +0200 Message-ID: <1589576.R6XolM4Vbv@wuerfel> User-Agent: KMail/4.11.5 (Linux/3.16.0-10-generic; KDE/4.11.5; x86_64; ; ) In-Reply-To: <5602CA55.10604@caviumnetworks.com> References: <1442965757-12925-1-git-send-email-ddaney.cavm@gmail.com> <1496061.0ds89jTH75@wuerfel> <5602CA55.10604@caviumnetworks.com> MIME-Version: 1.0 X-Provags-ID: V03:K0:OuV1CTvm8B71abO8MixBokIEmdx5LPsyYfSUZ6WD+2ZA8AZbn4u LAXk61KV5fHvwDkpqHFOt2G40oV60LC95rd+uhOHpgiULk938JJhEdEaAmacN8s4Bne4jID 1CuM5CczIHSXSP7boBkojMVucZjTow319E4LHgy9Kbi1PCwdY9xjbala0v/Bp1syivU6lkh Z0UbOltMdXj/dChHLgAEQ== X-UI-Out-Filterresults: notjunk:1; V01:K0:RthesOeUN4U=:JLsNXTdrCdN8SWSQToeRbr 19MSPJw2YTsX21Qg9sJIuBHkLfdEHMP1U0vJvUXq2pMWze9ABZ+35ktyusA+BaUMcbRIc1KhH LuayXsy7aIxlFFbn0OLxyJvqdTZVQ8WB2QIZVK3cQAauuyVoqyqU2A8J0cgf7bvlQmrRxSdKn RD65DCdkbAq4LHv5NoCFQCBpUd4KDStJCAw0p+E4dWBzGBquzt7lqF+msm5KEV7DJfEXYIHyC sULJijdSRkJ0RezGvvN9Npb/B77hRHG1Q1CmjhKp7BUTERefT65+52OK0oeXgJeyU5DeeM3kW ezKvCA8xD7x9kN8HrRE6wZXm1qvhf/rrA35TbkYGCLC/CM3I4gn5vUWsdta/olk/tv0A4L+7x Iv4otLgmwHe0fploxgWpVVt2FNGU1hfRa8OD0M2pSIVhrIZNSQ9/pXc9gsdLNrKnhUqZMkDHK BSHQ/Hb8fvqmBxs5Vyujp1ygBtYhvNH9sLf86nBMYqjmPsXY+2duWibm61TDbqS8pipwK9FsR 2yiVJKXV5TrXEoTirX6O7NGldfpZ/OV4LOD62UoQPAox80Km/pRR0N4GNqri8z58MRP9Q+uBj xOIGU5qJ/LFPR6Ri8entUIW1TaiPZeqx3kxniZaQgiiuf8bdg0ogpVuy1SUlOm8rlORZW8VHR HBn7Xti8+8A297W4qyGLwJkTdXUVv8rFSmrlxk9Ld4812e1/32lB6rmdegGcLB+aQ42xwCTb/ tLoNAVRyKxUQhqcP X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20150923_123601_864102_D42F7B06 X-CRM114-Status: GOOD ( 35.56 ) X-Spam-Score: -2.6 (--) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.20 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Mark Rutland , devicetree@vger.kernel.org, Pawel Moll , Ian Campbell , Marc Zyngier , linux-pci@vger.kernel.org, David Daney , Will Deacon , linux-kernel@vger.kernel.org, Rob Herring , David Daney , Kumar Gala , Bjorn Helgaas , linux-arm-kernel@lists.infradead.org Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+patchwork-linux-arm=patchwork.kernel.org@lists.infradead.org X-Spam-Status: No, score=-4.2 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_MED, 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 On Wednesday 23 September 2015 08:50:45 David Daney wrote: > On 09/23/2015 01:01 AM, Arnd Bergmann wrote: > > On Tuesday 22 September 2015 16:49:15 David Daney wrote: > >> From: David Daney > >> diff --git a/Documentation/devicetree/bindings/pci/host-generic-pci.txt b/Documentation/devicetree/bindings/pci/host-generic-pci.txt > >> index cf3e205..105a968 100644 > >> --- a/Documentation/devicetree/bindings/pci/host-generic-pci.txt > >> +++ b/Documentation/devicetree/bindings/pci/host-generic-pci.txt > >> @@ -34,7 +34,9 @@ Properties of the host controller node: > >> - #size-cells : Must be 2. > >> > >> - reg : The Configuration Space base address and size, as accessed > >> - from the parent bus. > >> + from the parent bus. The base address corresponds to > >> + bus zero, even though the "bus-range" property may specify > >> + a different starting bus number. > > > > This sounds like very unusual behavior. If you have a system with faked > > bus numbers where the registers only physically exist for a subset of the > > buses, this requires defining a reg property that contains MMIO space > > which is outside of the device and potentially contains other devices. > > The pci-host-generic driver only maps the ranges that correspond to the > "bus-range" buses, so mapping of illegal address ranges should not be a > problem. There is still a problem with what the driver is allowed to map, the current behavior is just an implementation detail that should not mean the binding can rely on that. We do the per-bus mapping because the vmalloc space on 32-bit systems is very limited. A driver for a purely 64-bit OS could simplify this by just mapping the entire 'reg' property as most drivers do, and then use the device as an offset into that. > > What would break if we instead defined it the expected way and only > > list the registers for the bus numbers in the "bus-range" property? > > I'm not sure if we have the luxury of being able to change the > definition, although the existing code only works with a starting bus > number of zero. From this we might conclude that non-zero starting bus > numbers cannot exist in the wild, so changing the the definition of > "reg" so that it starts at the starting bus number might be possible. > > My reading of: > > http://www.o3one.org/hwdocs/openfirmware/pci_supplement_2_1.pdf > > Section 3.1.1, does not preclude your interpretation. Although that is > for PCI-PCI bridges, and not this pci-host-generic root complex. > > If we really want to go with a different definition of what the "reg" > property means, then actual code has to change, and we risk breaking > something. My understanding is that the code has to change anyway in one place or another. The change you did in this patch is not needed then, but you say that something else has to change. Is this the only change we'd need? Arnd diff --git a/drivers/pci/host/pci-host-generic.c b/drivers/pci/host/pci-host-generic.c index 265dd25169bf..d8a5c0047155 100644 --- a/drivers/pci/host/pci-host-generic.c +++ b/drivers/pci/host/pci-host-generic.c @@ -196,7 +196,7 @@ static int gen_pci_parse_map_cfg_windows(struct gen_pci *pci) u32 sz = 1 << pci->cfg.ops->bus_shift; pci->cfg.win[idx] = devm_ioremap(dev, - pci->cfg.res.start + busn * sz, + pci->cfg.res.start + idx * sz, sz); if (!pci->cfg.win[idx]) return -ENOMEM;