From patchwork Tue Aug 6 20:56:46 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Bjorn Helgaas X-Patchwork-Id: 2839609 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 9FCC2BF535 for ; Tue, 6 Aug 2013 20:56:55 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 90EF920173 for ; Tue, 6 Aug 2013 20:56:54 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id B4D6120179 for ; Tue, 6 Aug 2013 20:56:52 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756177Ab3HFU4v (ORCPT ); Tue, 6 Aug 2013 16:56:51 -0400 Received: from mail-qc0-f177.google.com ([209.85.216.177]:45465 "EHLO mail-qc0-f177.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1756146Ab3HFU4v (ORCPT ); Tue, 6 Aug 2013 16:56:51 -0400 Received: by mail-qc0-f177.google.com with SMTP id e11so487285qcx.36 for ; Tue, 06 Aug 2013 13:56:50 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20120113; h=date:from:to:cc:subject:message-id:references:mime-version :content-type:content-disposition:in-reply-to:user-agent; bh=DgTlifqYP6Eu0nBEi5mgyofnHRZYcAOCS1nUU26BaaM=; b=USODqRowr2r7LRLCJ1EQo3l2yDuJvqdp1KpQgtgFDKaLrtcdCPAeEEbJYnir5VF5cM SA+iZXNp5Unmdez21GUI5IUoWvSBN9B08Hm12W/82RRvXJ4rqgSQMGt/mS4WbXTzHpHm IGdpA82AY4u7AY3waDuKj+TXVY+eo8QSr6SvuvKSwknZEcUH6rB/C2rCLLu0tbBdRo2v BYUw2aQnwg4TE5c7tfniWB/Zbq2urAEMSmGO8ABZxxeKtdj4smQbyg62dkFKMSXhDSV+ 4UmJ2YhwfqkDrLTQhfjm4lQSYXo2QSm6GI2Ycg5HN4uJpYmPVdK5y4T+VjNQlZN/XOK+ P8Zw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20120113; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-type:content-disposition:in-reply-to :user-agent; bh=DgTlifqYP6Eu0nBEi5mgyofnHRZYcAOCS1nUU26BaaM=; b=nkh0LJMCvhN66tw1+VXJhovwsp0enAE+ogrY1DceXlhjj9hiWSSgU0gVDwwk0lsOYo DiyR3AW8igStIcq2Xgcrc411GCB4Xg3EYBxSisanhLWvw5HeFGqTjwLrURuacfrJ7qbW 1OWH9CXm+qLzKXFHrYDQBcexJwjvEQZ+l8mp8o/10O9ouCIXwBul0xnvsQxEjSKiU4ms Gnc9ZsXXBSHpbAxGwFQfTDkD9DWQ0Hm57u77n+5rHcJP8N1pGxMuQ6qdVLKXwzhvoUAN y11apAjj04+wpo3iHQz47VZXIwnL/cIinWRNbhVtsXVw7pHLAG/M+hdWRQzfI8embK1J gAbg== X-Gm-Message-State: ALoCoQmqRBuFu8vMHj1APolnboDgYhcmv8RNmZJz0YU15ffscljnwnNr3BZ54KF7SUn9iyyuFwH33EvWOerDeBoyIlvNXuarWjY7iZO2DIL3Rpm/kFf7150KO/rzOMOVOvdprHdsYQLpClljAh4LmfSABVmkPO8dBO/aHWPLTT8q3JSO65Bhg5aqMnTqwDPIrC4wgTYOgfLoHOh+2ATgLOwX8M4e/F7dBA== X-Received: by 10.49.37.4 with SMTP id u4mr4226498qej.87.1375822610135; Tue, 06 Aug 2013 13:56:50 -0700 (PDT) Received: from google.com ([172.16.51.6]) by mx.google.com with ESMTPSA id c4sm6866753qad.0.2013.08.06.13.56.48 for (version=TLSv1.2 cipher=RC4-SHA bits=128/128); Tue, 06 Aug 2013 13:56:49 -0700 (PDT) Date: Tue, 6 Aug 2013 14:56:46 -0600 From: Bjorn Helgaas To: Wei Yang Cc: "linux-pci@vger.kernel.org" , Ram Pai , Gavin Shan , Yinghai Lu Subject: Re: [PATCH 4/4] PCI: fix the io resource alignment calculation in pbus_size_io() Message-ID: <20130806205646.GA3590@google.com> References: <1375435866-16332-1-git-send-email-weiyang@linux.vnet.ibm.com> <1375435866-16332-5-git-send-email-weiyang@linux.vnet.ibm.com> <20130806061924.GB10876@weiyang.vnet.ibm.com> <20130806134421.GD31970@google.com> <20130806154742.GB10680@weiyang.vnet.ibm.com> <20130806180155.GB1246@google.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20130806180155.GB1246@google.com> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-pci-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-pci@vger.kernel.org X-Spam-Status: No, score=-6.8 required=5.0 tests=BAYES_00, DKIM_ADSP_CUSTOM_MED, DKIM_SIGNED, RCVD_IN_DNSWL_HI, RP_MATCHES_RCVD, T_DKIM_INVALID, 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 Tue, Aug 06, 2013 at 12:01:55PM -0600, Bjorn Helgaas wrote: > On Tue, Aug 06, 2013 at 11:47:42PM +0800, Wei Yang wrote: > > On Tue, Aug 06, 2013 at 07:44:21AM -0600, Bjorn Helgaas wrote: > > >On Tue, Aug 06, 2013 at 02:19:24PM +0800, Wei Yang wrote: > > >> On Mon, Aug 05, 2013 at 11:58:50AM -0600, Bjorn Helgaas wrote: > > >> >[+cc Yinghai] > > >> > > > >> >On Fri, Aug 2, 2013 at 3:31 AM, Wei Yang wrote: > > >> >> In commit 462d9303 ("PCI: Align P2P windows using pcibios_window_alignment()"), > > >> >> it introduce a new method to calculate the window alignment of P2P bridge. > > >> >> > > >> >> When the io_window_1k is set, the calculation for the io resource alignment > > >> >> is different from the original one. In the original logic before 462d9303, > > >> >> the alignment is no bigger than 4K even the io_window_1k is set. The logic > > >> >> introduced in 462d9303 will limit the alignment to 1k in this case. > > >> >> > > >> >> This patch fix this issue. > > >> > > > >> >Presumably this fixes a bug, but you don't say what it is. "different > > >> >from the original" is not a description of a problem. You need > > >> >something like "with the current code, we allocate the wrong window > > >> >size in situation X, or we allocate a window with incorrect alignment > > >> >in situation Y, etc." > > >> > > > >> > > >> With current code, we allocate the wrong window size when upstream bridge > > >> could be 1k aligned and one of the downstream port is 4k aligned. > > >> > > >> In this case, the "min_align" should be 4k. But the current code set > > >> "min_align" to 1k. > > > > > > > Hmm... sorry I should say. > > > > With current code, we allocate the wrong window size and alignment when upstream > > bridge could be 1k aligned and one of the downstream port is 4k aligned. > > > > In this case, the "min_align" should be 4k. But the current code set > > "min_align" to 1k. > > Actually, I think only the alignment is wrong (not the size). But I do > agree that this looks like a problem in the current code. I'll write > this up and post the patch soon. Here's the patch I propose. The code change is the same as I posted yesterday; only the changelog is new. I'll put these in next pending comments. Bjorn commit 2d1d66780ecd12c9518835303f5302fc5262d49b Author: Bjorn Helgaas Date: Mon Aug 5 16:15:10 2013 -0600 PCI: Align bridge I/O windows as required by downstream devices & bridges An upstream bridge's I/O window must be at least as aligned as any downstream device or bridge requires. In particular, if the upstream bridge supports 1K alignment but a downstream bridge requires 4K alignment, the upstream window must also be 4K aligned. Therefore, do not reduce the required alignment ("min_align") based on the upstream bridge's capabilities. Reported-by: Wei Yang Suggested-by: Yinghai Lu Signed-off-by: Bjorn Helgaas --- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c index d4f1ad9..8333c92 100644 --- a/drivers/pci/setup-bus.c +++ b/drivers/pci/setup-bus.c @@ -749,12 +749,12 @@ static void pbus_size_io(struct pci_bus *bus, resource_size_t min_size, struct resource *b_res = find_free_bus_resource(bus, IORESOURCE_IO); resource_size_t size = 0, size0 = 0, size1 = 0; resource_size_t children_add_size = 0; - resource_size_t min_align, io_align, align; + resource_size_t min_align, align; if (!b_res) return; - io_align = min_align = window_alignment(bus, IORESOURCE_IO); + min_align = window_alignment(bus, IORESOURCE_IO); list_for_each_entry(dev, &bus->devices, bus_list) { int i; @@ -781,9 +781,6 @@ static void pbus_size_io(struct pci_bus *bus, resource_size_t min_size, } } - if (min_align > io_align) - min_align = io_align; - size0 = calculate_iosize(size, min_size, size1, resource_size(b_res), min_align); if (children_add_size > add_size)