From patchwork Mon Dec 5 12:36:01 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Tomasz Nowicki X-Patchwork-Id: 9460945 X-Patchwork-Delegate: bhelgaas@google.com Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 4824B60459 for ; Mon, 5 Dec 2016 12:36:07 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 38EFC24EE5 for ; Mon, 5 Dec 2016 12:36:07 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 2D92027C0B; Mon, 5 Dec 2016 12:36:07 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.3 required=2.0 tests=BAYES_00,DKIM_SIGNED, RCVD_IN_DNSWL_HI, RCVD_IN_SORBS_SPAM, T_DKIM_INVALID autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 51DB524EE5 for ; Mon, 5 Dec 2016 12:36:06 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751373AbcLEMgF (ORCPT ); Mon, 5 Dec 2016 07:36:05 -0500 Received: from mail-wm0-f44.google.com ([74.125.82.44]:35698 "EHLO mail-wm0-f44.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750951AbcLEMgE (ORCPT ); Mon, 5 Dec 2016 07:36:04 -0500 Received: by mail-wm0-f44.google.com with SMTP id a197so94920455wmd.0 for ; Mon, 05 Dec 2016 04:36:04 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=semihalf-com.20150623.gappssmtp.com; s=20150623; h=subject:to:references:cc:from:message-id:date:user-agent :mime-version:in-reply-to:content-transfer-encoding; bh=DTBcQvIMggP9ySl8Mmsmdo+ms5pMUmz9oVGjxWrHt9U=; b=Nb5QzcXJGEs3QUwe4pKlIw1qi4xrxoCydhf2bURilT+bs+LAhTsiXrXzudtFvrpzk4 tFGnfGKYO9Y0/8WeDVUKb3LulUXms6FXkpl0hcNvDa0KvbjZ5+OYhLATtq7M8vSfC+m3 uXYjs2MA1VXtDyyajMV+bru2gwoCyJOmKJuWtY+1THh3QXcoIY+0l9SkkqTpWUENjqDU jUk3CyvPUPyhmG6u2rbbc7qjIEaTgTDUEoUVQ2hskuIv47I1L52ZvLgrbzvgOH+RaRDt j+m++LFCmSL966eC/PlEZttiuO1r4KbUrLbugei4f2/OiVTVAZyMXMlBYuWOUdBAN25B BmQA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:subject:to:references:cc:from:message-id:date :user-agent:mime-version:in-reply-to:content-transfer-encoding; bh=DTBcQvIMggP9ySl8Mmsmdo+ms5pMUmz9oVGjxWrHt9U=; b=RPfYTlS6dzb7zsrG9LI9dHWVrMMR0PUpjOr2kKoRx8nFee26V7U7u/6YudrsdlH4kl 8DcUT3qu6agn/U41iGbQ0yH92OhL5gMkI33PoveHl/98i/Om5e/v/4bwsfyWQ6SQBCh4 8fOHKbCEB6z6g5SLijeOhG0AN8yl4Z5Zd9rHEWGFo03aP8MkQ3OhTq8esg5hGPsSdyS5 bXEMP3eh2IS7CrxkesE2VzBHRe8lDWMC1oFez+x6/uPbXY6jvhuXyfDy0SANiAxL6pIF CKoF7XsMk7BYUh4joPcT+n174dMnH+PHsyiYbyygsUoIBBxxtDwsysOTaWiSr2AOrGVt HHVg== X-Gm-Message-State: AKaTC02FomjVRLs8w+2LFww19+Sewqgiv6IBrOHgGKIgkcDioCh1+zq+NhbmnL9a4vSK5Q== X-Received: by 10.46.77.65 with SMTP id a62mr27171177ljb.61.1480941363072; Mon, 05 Dec 2016 04:36:03 -0800 (PST) Received: from [10.0.0.85] (31-172-191-173.noc.fibertech.net.pl. [31.172.191.173]) by smtp.googlemail.com with ESMTPSA id r21sm2908216lff.17.2016.12.05.04.36.01 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Mon, 05 Dec 2016 04:36:02 -0800 (PST) Subject: Re: [PATCH v10 00/12] PCI: ARM64 ECAM quirks To: Bjorn Helgaas References: <20161201075131.12247.2211.stgit@bhelgaas-glaptop.roam.corp.google.com> <20161202215711.GE9903@bhelgaas-glaptop.roam.corp.google.com> Cc: linux-pci@vger.kernel.org, Lorenzo Pieralisi , Gabriele Paoloni , "Rafael J. Wysocki" , Duc Dang , Sinan Kaya , Christopher Covington , Dongdong Liu From: Tomasz Nowicki Message-ID: <2bef38c3-be46-918e-8ab3-1eb39df23edc@semihalf.com> Date: Mon, 5 Dec 2016 13:36:01 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Firefox/45.0 Thunderbird/45.4.0 MIME-Version: 1.0 In-Reply-To: <20161202215711.GE9903@bhelgaas-glaptop.roam.corp.google.com> Sender: linux-pci-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-pci@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP Hi Bjorn, On 02.12.2016 22:57, Bjorn Helgaas wrote: > On Fri, Dec 02, 2016 at 03:20:28PM +0100, Tomasz Nowicki wrote: >> dmesg from ThunderX pass2.0 (1 socket board) + small fix: >> >> diff --git a/drivers/acpi/pci_mcfg.c b/drivers/acpi/pci_mcfg.c >> - THUNDER_PEM_QUIRK(2, 0), /* off-chip devices */ >> - THUNDER_PEM_QUIRK(2, 1), /* off-chip devices */ >> + THUNDER_PEM_QUIRK(2, 0UL), /* off-chip devices */ >> + THUNDER_PEM_QUIRK(2, 1UL), /* off-chip devices */ > > Thanks! I folded this change into my branch (possibly to be updated > if Robert sends something better). I believe Robert meant: pass1.x */ #endif #ifdef CONFIG_PCI_HOST_GENERIC > >> ACPI: MCFG table detected, 4 entries >> ACPI: PCI Root Bridge [PCI0] (domain 0000 [bus 00-1f]) >> acpi PNP0A08:00: _OSC: OS supports [ExtendedConfig ASPM ClockPM >> Segments MSI] >> acpi PNP0A08:00: _OSC: platform does not support [PCIeHotplug PME AER] >> acpi PNP0A08:00: _OSC: OS now controls [PCIeCapability] >> acpi PNP0A08:00: ECAM area [mem 0x848000000000-0x848001ffffff] >> reserved by THRX0001:00 >> acpi PNP0A08:00: ECAM at [mem 0x848000000000-0x848001ffffff] for [bus 00-1f] > > I guess we don't need MCFG quirks for these bridges, since I don't see > the "MCFG quirk" message? Yes, this is ThunderX 1-socket pass2.x so quirks are needed for 4-9 host bridges only. Below summary of where/how quirks should be applied: ThunderX pass2.x NUMA node -> segments -> ECAM compliant note 0 -> 0-3 -> ECAM compliant 0 -> 4-9 -> ECAM quirks (thunder_pem_ecam_ops) 1 (optionally) -> 10-13 -> ECAM compliant 1 (optionally) -> 14-19 -> ECAM quirks (thunder_pem_ecam_ops) ThunderX pass1.x NUMA node -> segments -> ECAM compliant note 0 -> 0-3 -> ECAM quirks (pci_thunder_ecam_ops) 0 -> 4-9 -> ECAM quirks (thunder_pem_ecam_ops) 1 (optionally) -> 10-13 -> ECAM quirks (pci_thunder_ecam_ops) 1 (optionally) -> 14-19 -> ECAM quirks (thunder_pem_ecam_ops) > >> system 00:00: [mem 0x848000000000-0x848001ffffff] could not be reserved >> system 00:01: [mem 0x849000000000-0x849001ffffff] could not be reserved >> system 00:02: [mem 0x84a000000000-0x84a001ffffff] could not be reserved >> system 00:03: [mem 0x84b000000000-0x84b001ffffff] could not be reserved >> system 00:04: [mem 0x87e0c0000000-0x87e0c0ffffff] could not be reserved >> system 00:04: [mem 0x88001f000000-0x880057ffffff] could not be reserved >> system 00:05: [mem 0x87e0c2000000-0x87e0c2ffffff] could not be reserved >> system 00:05: [mem 0x88808f000000-0x8880c7ffffff] could not be reserved > > Most of these match ECAM spaces, which is good. 00:04 and 00:05 each > have one ECAM space and one "other" space, which might be PEMx host > bridge registers? That all seems good. Yes, "other" space is PEMx host bridge registers. > > But I assume the other bridges (PCIx) also have register space in > addition to ECAM, and that should be reported also. 00:00, 00:01, 00:02, 00:03 are ECAM compliant so for those we need ECAM region only. > >> root@ubuntu:~# cat /proc/iomem >> ... >> 838000000000-841fffffffff : PCI Bus 0000:00 >> 838000000000-8380003fffff : 0000:03:00.0 > > Something's missing here: we should have a clue about how we got from > bus 00 to bus 03. From your dmesg, 0000:00:15.0 is a bridge from bus > 00 to bus 03, and its windows should appear here. I'd expect > something like: > > 838000000000-841fffffffff : PCI Bus 0000:00 > 838000000000-838fffffffff : PCI Bus 0000:03 <- 00:15.0 window > 838000000000-8380003fffff : 0000:03:00.0 > > This window should be inserted by generic code, so I don't know how > this could be broken. Your dmesg should also have something like > this: > > pci 0000:00:15.0: PCI bridge to [bus 03] > pci 0000:00:15.0: bridge window [mem 0x838000000000-0x838fffffffff] > > I don't see that, maybe because this is actually a console log > collected without "ignore_loglevel"? But that obviously doesn't > affect /proc/iomem, so I'm still puzzled about that. Enhanced Allocation is used for devices but not for bridges which have invalid BARs in standard configuration header. Thus devices request resources from first valid parent bridge (host bridge in this case). > >> 87e024000000-87e024000fff : ARMH0011:00 >> 87e024000000-87e024000fff : ARMH0011:00 > > This is interesting. This must be a driver reserving these areas? > Normally a driver would use the driver name, not the device name. > > Ideally, I think the core should reserve the region with the device > name, and the driver would request it using the driver name, like > this: > > 843000000000-84303fffffff : 0002:01:00.0 <-- PCI core reservation > 843000000000-84303fffffff : thunder-nic <-- driver request > > The ACPI core doesn't actually do the reservation, so I assume the > ARMH0011:00 stuff is from the driver, and it's curious that it has the > same region twice. > >> 87e026000000-87e0bfffffff : PCI Bus 0000:00 >> 87e027000000-87e0277fffff : CAVA02A:00 > > This is interesting, too. CAVA02A:00 looks like an ACPI device, but > apparently it consumes some of the space that we think is routed to > PCI bus 0000:00. I think this happens on some x86 boxes, too, but it > is somewhat confusing. I will take a look. > > Based on this, I don't see any problems with the ThunderX quirks. > I'd like to understand what's going on with the PCI-to-PCI bridge > windows in /proc/iomem, but I doubt that's related to your quirks. Yes it is not related to quirks. However, CAVA02A:00 and ARMH0011 things should be investigated. Thanks, Tomasz --- 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/acpi/pci_mcfg.c b/drivers/acpi/pci_mcfg.c index d34d196..8ca8ca8 100644 --- a/drivers/acpi/pci_mcfg.c +++ b/drivers/acpi/pci_mcfg.c @@ -76,7 +76,7 @@ static struct mcfg_fixup mcfg_quirks[] = { HISI_QUAD_DOM("HIP07 ", 12, &hisi_pcie_ops), #define THUNDER_PEM_RES(addr, node) \ - DEFINE_RES_MEM(addr + (node << 44), 0x39 * SZ_16M) + DEFINE_RES_MEM(addr + ((u64)node << 44), 0x39 * SZ_16M) Which means we can forget UL suffix for THUNDER_PEM_QUIRK macro: @@ -91,8 +91,8 @@ static struct mcfg_fixup mcfg_quirks[] = { { "CAVIUM", "THUNDERX", rev, 9 + (10 * node), MCFG_BUS_ANY, \ &thunder_pem_ecam_ops, THUNDER_PEM_RES(0x89808f000000UL, node) } /* SoC pass2.x */ - THUNDER_PEM_QUIRK(1, 0UL), - THUNDER_PEM_QUIRK(1, 1UL), + THUNDER_PEM_QUIRK(1, 0), + THUNDER_PEM_QUIRK(1, 1), Also, my apologies, I should have fixed parentheses in this macro from the very beginning. For you convenience below incremental patch where I fixed both issues: diff --git a/drivers/acpi/pci_mcfg.c b/drivers/acpi/pci_mcfg.c index d34d196..cf6c321 100644 --- a/drivers/acpi/pci_mcfg.c +++ b/drivers/acpi/pci_mcfg.c @@ -76,23 +76,23 @@ static struct mcfg_fixup mcfg_quirks[] = { HISI_QUAD_DOM("HIP07 ", 12, &hisi_pcie_ops), #define THUNDER_PEM_RES(addr, node) \ - DEFINE_RES_MEM(addr + (node << 44), 0x39 * SZ_16M) + DEFINE_RES_MEM((addr) + ((u64)(node) << 44), 0x39 * SZ_16M) #define THUNDER_PEM_QUIRK(rev, node) \ - { "CAVIUM", "THUNDERX", rev, 4 + (10 * node), MCFG_BUS_ANY, \ - &thunder_pem_ecam_ops, THUNDER_PEM_RES(0x88001f000000UL, node) }, \ - { "CAVIUM", "THUNDERX", rev, 5 + (10 * node), MCFG_BUS_ANY, \ - &thunder_pem_ecam_ops, THUNDER_PEM_RES(0x884057000000UL, node) }, \ - { "CAVIUM", "THUNDERX", rev, 6 + (10 * node), MCFG_BUS_ANY, \ - &thunder_pem_ecam_ops, THUNDER_PEM_RES(0x88808f000000UL, node) }, \ - { "CAVIUM", "THUNDERX", rev, 7 + (10 * node), MCFG_BUS_ANY, \ - &thunder_pem_ecam_ops, THUNDER_PEM_RES(0x89001f000000UL, node) }, \ - { "CAVIUM", "THUNDERX", rev, 8 + (10 * node), MCFG_BUS_ANY, \ - &thunder_pem_ecam_ops, THUNDER_PEM_RES(0x894057000000UL, node) }, \ - { "CAVIUM", "THUNDERX", rev, 9 + (10 * node), MCFG_BUS_ANY, \ - &thunder_pem_ecam_ops, THUNDER_PEM_RES(0x89808f000000UL, node) } + { "CAVIUM", "THUNDERX", (rev), 4 + (10 * (node)), MCFG_BUS_ANY, \ + &thunder_pem_ecam_ops, THUNDER_PEM_RES(0x88001f000000UL, (node)) }, \ + { "CAVIUM", "THUNDERX", (rev), 5 + (10 * (node)), MCFG_BUS_ANY, \ + &thunder_pem_ecam_ops, THUNDER_PEM_RES(0x884057000000UL, (node)) }, \ + { "CAVIUM", "THUNDERX", (rev), 6 + (10 * (node)), MCFG_BUS_ANY, \ + &thunder_pem_ecam_ops, THUNDER_PEM_RES(0x88808f000000UL, (node)) }, \ + { "CAVIUM", "THUNDERX", (rev), 7 + (10 * (node)), MCFG_BUS_ANY, \ + &thunder_pem_ecam_ops, THUNDER_PEM_RES(0x89001f000000UL, (node)) }, \ + { "CAVIUM", "THUNDERX", (rev), 8 + (10 * (node)), MCFG_BUS_ANY, \ + &thunder_pem_ecam_ops, THUNDER_PEM_RES(0x894057000000UL, (node)) }, \ + { "CAVIUM", "THUNDERX", (rev), 9 + (10 * (node)), MCFG_BUS_ANY, \ + &thunder_pem_ecam_ops, THUNDER_PEM_RES(0x89808f000000UL, (node)) } /* SoC pass2.x */ - THUNDER_PEM_QUIRK(1, 0UL), - THUNDER_PEM_QUIRK(1, 1UL), + THUNDER_PEM_QUIRK(1, 0), + THUNDER_PEM_QUIRK(1, 1), #define THUNDER_ECAM_QUIRK(rev, seg) \ { "CAVIUM", "THUNDERX", rev, seg, MCFG_BUS_ANY, \ diff --git a/include/linux/pci-ecam.h b/include/linux/pci-ecam.h index 00eb8eb..643c9e7 100644 --- a/include/linux/pci-ecam.h +++ b/include/linux/pci-ecam.h @@ -62,8 +62,9 @@ extern struct pci_ecam_ops pci_generic_ecam_ops; #if defined(CONFIG_ACPI) && defined(CONFIG_PCI_QUIRKS) extern struct pci_ecam_ops pci_32b_ops; /* 32-bit accesses only */ extern struct pci_ecam_ops hisi_pcie_ops; /* HiSilicon */ -extern struct pci_ecam_ops thunder_pem_ecam_ops; /* Cavium ThunderX 1.x & 2.x */ -extern struct pci_ecam_ops pci_thunder_ecam_ops; /* Cavium ThunderX 1.x */ +extern struct pci_ecam_ops thunder_pem_ecam_ops; /* Cavium ThunderX pass1.x & + pass2.x */ +extern struct pci_ecam_ops pci_thunder_ecam_ops; /* Cavium ThunderX