From patchwork Sun May 5 14:40:59 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Nicholas Johnson X-Patchwork-Id: 1095469 X-Patchwork-Delegate: bhelgaas@google.com Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=none (mailfrom) smtp.mailfrom=vger.kernel.org (client-ip=209.132.180.67; helo=vger.kernel.org; envelope-from=linux-pci-owner@vger.kernel.org; receiver=) Authentication-Results: ozlabs.org; dmarc=fail (p=none dis=none) header.from=outlook.com.au Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 44xpTH6dDnz9s6w for ; Mon, 6 May 2019 00:41:07 +1000 (AEST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727994AbfEEOlG (ORCPT ); Sun, 5 May 2019 10:41:06 -0400 Received: from mail-oln040092253063.outbound.protection.outlook.com ([40.92.253.63]:44951 "EHLO APC01-SG2-obe.outbound.protection.outlook.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1727325AbfEEOlG (ORCPT ); Sun, 5 May 2019 10:41:06 -0400 Received: from HK2APC01FT045.eop-APC01.prod.protection.outlook.com (10.152.248.53) by HK2APC01HT118.eop-APC01.prod.protection.outlook.com (10.152.249.82) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA384) id 15.20.1856.11; Sun, 5 May 2019 14:40:59 +0000 Received: from PS2P216MB0642.KORP216.PROD.OUTLOOK.COM (10.152.248.52) by HK2APC01FT045.mail.protection.outlook.com (10.152.249.139) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA384) id 15.20.1856.11 via Frontend Transport; Sun, 5 May 2019 14:40:59 +0000 Received: from PS2P216MB0642.KORP216.PROD.OUTLOOK.COM ([fe80::adb3:4c16:60fd:65]) by PS2P216MB0642.KORP216.PROD.OUTLOOK.COM ([fe80::adb3:4c16:60fd:65%5]) with mapi id 15.20.1856.012; Sun, 5 May 2019 14:40:59 +0000 From: Nicholas Johnson To: "linux-kernel@vger.kernel.org" CC: "linux-pci@vger.kernel.org" , "bhelgaas@google.com" , "mika.westerberg@linux.intel.com" , "corbet@lwn.net" , Nicholas Johnson Subject: [PATCH v5 3/5] PCI: Fix bug resulting in double hpmemsize being assigned to MMIO window Thread-Topic: [PATCH v5 3/5] PCI: Fix bug resulting in double hpmemsize being assigned to MMIO window Thread-Index: AQHVA1COx3JcNUJoSEanByS+7CjcRA== Date: Sun, 5 May 2019 14:40:59 +0000 Message-ID: References: <20190505144001.8106-1-nicholas.johnson-opensource@outlook.com.au> In-Reply-To: <20190505144001.8106-1-nicholas.johnson-opensource@outlook.com.au> Accept-Language: en-AU, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-clientproxiedby: SG2PR01CA0116.apcprd01.prod.exchangelabs.com (2603:1096:4:40::20) To PS2P216MB0642.KORP216.PROD.OUTLOOK.COM (2603:1096:300:1c::16) x-incomingtopheadermarker: OriginalChecksum:2E2A3860F1F43F59222599554FF91EF41F23724C0642FB833E7DC2F25C644241; UpperCasedChecksum:CE7B018451CCD8160BA41D2A6EAC7B6A9026D3A92D7FAAA1C07EA434C2A18193; SizeAsReceived:7927; Count:50 x-ms-exchange-messagesentrepresentingtype: 1 x-mailer: git-send-email 2.19.1 x-tmn: [JrLzKi1TPd8u8ow6G+hcfPNVv7fZtlB6jqpHDpokqH9xwjwpiQ1DhLJen3Vlco3pUIspSf85+8E=] x-microsoft-original-message-id: <20190505144001.8106-4-nicholas.johnson-opensource@outlook.com.au> x-ms-publictraffictype: Email x-incomingheadercount: 50 x-eopattributedmessage: 0 x-ms-exchange-slblob-mailprops: Xo9mRxKQURT0eY5pRcrZj21eed7UjhNvi7FE7Kc18pk46RO+8U82h2pOklhCLZE/UatxfLjARZByivZzUFsbn8xO7h5kiMXG8u0rdLULW1TUKV59HiV+lXVoWPfQSkN5ei7nAqYBRJqiyWibRaKljBaEHHwEqOUifQr4dH1ny3DJNHz1nk77I88FZs1AQAEIcnquFZp7F/8nAcQEtrxhZUHdOCgeigsk7nT1GUyqfUziwM3mP5ysk5z+ZG2/CJq+dW6t4e/pTadWuDpoVErENxWENYDiiXG5NTcymrO7P3+YmeSIYhgknukYjI45A2q8EZplZyRddllQ45Z5ZBoERagBQoM3hOccuTN80lV76xgIs1JTSrq/Qf9pC/86GE0JRjS0PiTQux31L6drydsirAtltz3PbCDyiGZ3x5N03ElNZ8ekmOR1F2gp94+KCR6O0zQP6RzGvDwe+P3IczkIbpXD36Zw1fx5QNxJtDaT1xk9tYmvJFxlNbj7/qTTdoBIEQjHIj+ILWC5gDGAUgBX28b8e4069wGkeRuYuvTArTJzjuYr01O2Yj/vOb3zu9EUkW8IPC+HQOekf9/AkLEofMhHN4HZbXyJiDlijFs6m86kV+CXy2cLsP6PFJK9HISjkJsm6hkaGc6xqI3faUQXC0Pw3dlHOWeGtHiMOiqaODAIap3/pjVA75nyMo5eEsWrPg26ra+qdlB4kSZTybGaUJ4dHjfVKDTqn7g4Q4Y4QmpIbcreeFMPhFEW4L5oNHESZW2Y0c0rav04TTqJX92zEij2g22BV8alzGnyQsbsGw/Kkdl7orL5UkwsQJsK5NtoNYXOjzOb4xFUGgS5+l9v8hPVjC1k9RI/7YAExXAkw81RVWnBa8RBscFJcHc4lyKudGGWZDdW/MY9ZF5viJi1IfJN6Kh+Mt5CogvPRDgWj1mgQU/jw8h+2Q== x-microsoft-antispam: BCL:0; PCL:0; RULEID:(2390118)(5050001)(7020095)(20181119110)(201702061078)(5061506573)(5061507331)(1603103135)(2017031320274)(201702181274)(2017031323274)(2017031324274)(2017031322404)(1601125500)(1603101475)(1701031045); SRVR:HK2APC01HT118; x-ms-traffictypediagnostic: HK2APC01HT118: x-ms-exchange-purlcount: 1 x-microsoft-antispam-message-info: dKA3UxGAHMILO7A0CTl9UrzbyIFl7xM4AWVCZQc7z6p7sEGESrtrEU85/2aGDsV5v10b/8UmqWFRxcC6ldJxBffBqZMBSEAYXo7lbRcbUIrMGdfEceIhZ8yDHnJPfccOkqBALILzOirtg1jcbgUp6kyicOApWpxtIAgWJZK3IfqDcfByFqrDBFeWfBaftj1C MIME-Version: 1.0 X-OriginatorOrg: outlook.com X-MS-Exchange-CrossTenant-RMS-PersistedConsumerOrg: 00000000-0000-0000-0000-000000000000 X-MS-Exchange-CrossTenant-Network-Message-Id: 266becd2-9773-4a91-bba5-08d6d167b04f X-MS-Exchange-CrossTenant-rms-persistedconsumerorg: 00000000-0000-0000-0000-000000000000 X-MS-Exchange-CrossTenant-originalarrivaltime: 05 May 2019 14:40:59.2248 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Internet X-MS-Exchange-CrossTenant-id: 84df9e7f-e9f6-40af-b435-aaaaaaaaaaaa X-MS-Exchange-Transport-CrossTenantHeadersStamped: HK2APC01HT118 Sender: linux-pci-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-pci@vger.kernel.org Background ========================================================================== Solve bug report: https://bugzilla.kernel.org/show_bug.cgi?id=203243 Currently, the kernel can sometimes assign the MMIO_PREF window additional size into the MMIO window, resulting in double the MMIO additional size, even if the MMIO_PREF window was successful. This happens if in the first pass, the MMIO_PREF succeeds but the MMIO fails. In the next pass, because MMIO_PREF is already assigned, the attempt to assign MMIO_PREF returns an error code instead of success (nothing more to do, already allocated). Example of problem (more context can be found in the bug report URL): Mainline kernel: pci 0000:06:01.0: BAR 14: assigned [mem 0x90100000-0xa00fffff] = 256M pci 0000:06:04.0: BAR 14: assigned [mem 0xa0200000-0xb01fffff] = 256M Patched kernel: pci 0000:06:01.0: BAR 14: assigned [mem 0x90100000-0x980fffff] = 128M pci 0000:06:04.0: BAR 14: assigned [mem 0x98200000-0xa01fffff] = 128M This was using pci=realloc,hpmemsize=128M,nocrs - on the same machine with the same configuration, with a Ubuntu mainline kernel and a kernel patched with this patch series. This patch is vital for the next patch in the series. The next patch allows the user to specify MMIO and MMIO_PREF independently. If the MMIO_PREF is set to be very large, this bug will end up more than doubling the MMIO size. The bug results in the MMIO_PREF being added to the MMIO window, which means doubling if MMIO_PREF size == MMIO size. With a large MMIO_PREF, without this patch, the MMIO window will likely fail to be assigned altogether due to lack of 32-bit address space. Patch notes ========================================================================== Change find_free_bus_resource() to not skip assigned resources with non-null parent. Add checks in pbus_size_io() and pbus_size_mem() to return success if resource returned from find_free_bus_resource() is already allocated. This avoids pbus_size_io() and pbus_size_mem() returning error code to __pci_bus_size_bridges() when a resource has been successfully assigned in a previous pass. This fixes the existing behaviour where space for a resource could be reserved multiple times in different parent bridge windows. This also greatly reduces the number of failed BAR messages in dmesg when Linux assigns resources. Signed-off-by: Nicholas Johnson --- drivers/pci/setup-bus.c | 28 +++++++++++++++++++--------- 1 file changed, 19 insertions(+), 9 deletions(-) diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c index 5214815c7..e7126cc0e 100644 --- a/drivers/pci/setup-bus.c +++ b/drivers/pci/setup-bus.c @@ -752,11 +752,17 @@ static void pci_bridge_check_ranges(struct pci_bus *bus) } } -/* Helper function for sizing routines: find first available - bus resource of a given type. Note: we intentionally skip - the bus resources which have already been assigned (that is, - have non-NULL parent resource). */ -static struct resource *find_free_bus_resource(struct pci_bus *bus, +/* + * Helper function for sizing routines: find first bus resource of a given + * type. Note: we do not skip the bus resources which have already been + * assigned (r->parent != NULL). This is because a resource that is already + * assigned (nothing more to be done) will be indistinguishable from one that + * failed due to lack of space if we skip assigned resources. If the caller + * function cannot tell the difference then it might try to place the + * resources in a different window, doubling up on resources or causing + * unforeseeable issues. + */ +static struct resource *find_bus_resource_of_type(struct pci_bus *bus, unsigned long type_mask, unsigned long type) { int i; @@ -765,7 +771,7 @@ static struct resource *find_free_bus_resource(struct pci_bus *bus, pci_bus_for_each_resource(bus, r, i) { if (r == &ioport_resource || r == &iomem_resource) continue; - if (r && (r->flags & type_mask) == type && !r->parent) + if (r && (r->flags & type_mask) == type) return r; } return NULL; @@ -863,14 +869,16 @@ static void pbus_size_io(struct pci_bus *bus, resource_size_t min_size, resource_size_t add_size, struct list_head *realloc_head) { struct pci_dev *dev; - struct resource *b_res = find_free_bus_resource(bus, IORESOURCE_IO, - IORESOURCE_IO); + struct resource *b_res = find_bus_resource_of_type(bus, IORESOURCE_IO, + IORESOURCE_IO); resource_size_t size = 0, size0 = 0, size1 = 0; resource_size_t children_add_size = 0; resource_size_t min_align, align; if (!b_res) return; + if (b_res->parent) + return; min_align = window_alignment(bus, IORESOURCE_IO); list_for_each_entry(dev, &bus->devices, bus_list) { @@ -975,7 +983,7 @@ static int pbus_size_mem(struct pci_bus *bus, unsigned long mask, resource_size_t min_align, align, size, size0, size1; resource_size_t aligns[18]; /* Alignments from 1Mb to 128Gb */ int order, max_order; - struct resource *b_res = find_free_bus_resource(bus, + struct resource *b_res = find_bus_resource_of_type(bus, mask | IORESOURCE_PREFETCH, type); resource_size_t children_add_size = 0; resource_size_t children_add_align = 0; @@ -983,6 +991,8 @@ static int pbus_size_mem(struct pci_bus *bus, unsigned long mask, if (!b_res) return -ENOSPC; + if (b_res->parent) + return 0; memset(aligns, 0, sizeof(aligns)); max_order = 0;