From patchwork Tue May 10 13:06:29 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Prarit Bhargava X-Patchwork-Id: 9057261 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.29.136]) by patchwork2.web.kernel.org (Postfix) with ESMTP id 8465CBF29F for ; Tue, 10 May 2016 13:06:36 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 528092010B for ; Tue, 10 May 2016 13:06:35 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id F132D20148 for ; Tue, 10 May 2016 13:06:33 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751262AbcEJNGd (ORCPT ); Tue, 10 May 2016 09:06:33 -0400 Received: from mx1.redhat.com ([209.132.183.28]:44774 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751229AbcEJNGc (ORCPT ); Tue, 10 May 2016 09:06:32 -0400 Received: from int-mx11.intmail.prod.int.phx2.redhat.com (int-mx11.intmail.prod.int.phx2.redhat.com [10.5.11.24]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 10C45A1495; Tue, 10 May 2016 13:06:31 +0000 (UTC) Received: from [10.16.186.145] (prarit-guest.khw.lab.eng.bos.redhat.com [10.16.186.145]) by int-mx11.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id u4AD6TQx007534; Tue, 10 May 2016 09:06:29 -0400 Message-ID: <5731DCD5.4030208@redhat.com> Date: Tue, 10 May 2016 09:06:29 -0400 From: Prarit Bhargava User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.7.0 MIME-Version: 1.0 To: Bjorn Helgaas CC: linux-pci@vger.kernel.org, Bjorn Helgaas , Thomas Gleixner , Ingo Molnar , "H. Peter Anvin" , Myron Stowe , x86@kernel.org, Andi Kleen Subject: Re: [PATCH] x86/PCI: Fix Broadwell-EP Home Agent & PCU non-compliant BARs References: <1462818195-6533-1-git-send-email-prarit@redhat.com> <20160509192030.GA18166@localhost> In-Reply-To: <20160509192030.GA18166@localhost> X-Scanned-By: MIMEDefang 2.68 on 10.5.11.24 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.27]); Tue, 10 May 2016 13:06:32 +0000 (UTC) Sender: linux-pci-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-pci@vger.kernel.org X-Spam-Status: No, score=-9.0 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_HI, 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 05/09/2016 03:20 PM, Bjorn Helgaas wrote: > [+cc Andi] > > Hi Prarit, > > On Mon, May 09, 2016 at 02:23:15PM -0400, Prarit Bhargava wrote: >> commit b894157 ("x86/PCI: Mark Broadwell-EP Home Agent & PCU as having >> non-compliant BARs") marks Home Agent 0 & PCU has having non-compliant >> BARs. > > By convention, I use 12-char SHA1 ("[core] abbrev=12" in .git/config) > when citing commits. > >> Before commit b894157, >> >> pci 0000:ff:12.0: BAR 2: failed to assign [mem size 0x00000040] >> pci 0000:ff:12.0: BAR 4: failed to assign [mem size 0x00000040] >> pci 0000:ff:12.4: BAR 2: failed to assign [mem size 0x00000040] >> pci 0000:ff:12.4: BAR 4: failed to assign [mem size 0x00000040] >> pci 0000:ff:12.0: BAR 1: failed to assign [mem size 0x00000010] >> pci 0000:ff:12.0: BAR 3: failed to assign [mem size 0x00000010] >> pci 0000:ff:12.0: BAR 5: failed to assign [mem size 0x00000010] >> pci 0000:ff:12.4: BAR 1: failed to assign [mem size 0x00000010] >> pci 0000:ff:12.4: BAR 3: failed to assign [mem size 0x00000010] >> pci 0000:ff:12.4: BAR 5: failed to assign [mem size 0x00000010] >> pci 0000:7f:12.0: BAR 2: failed to assign [mem size 0x00000040] >> pci 0000:7f:12.0: BAR 4: failed to assign [mem size 0x00000040] >> pci 0000:7f:12.4: BAR 2: failed to assign [mem size 0x00000040] >> pci 0000:7f:12.4: BAR 4: failed to assign [mem size 0x00000040] >> pci 0000:7f:12.0: BAR 1: failed to assign [mem size 0x00000010] >> pci 0000:7f:12.0: BAR 3: failed to assign [mem size 0x00000010] >> pci 0000:7f:12.0: BAR 5: failed to assign [mem size 0x00000010] >> pci 0000:7f:12.4: BAR 1: failed to assign [mem size 0x00000010] >> pci 0000:7f:12.4: BAR 3: failed to assign [mem size 0x00000010] >> pci 0000:7f:12.4: BAR 5: failed to assign [mem size 0x00000010] >> >> After commit b894157, there are still "failed to assign" messages, >> as well as new "failed to assign" messages for ff:12.0, ff:1e.3, >> 7f:12.0, and 7f:1e.3. >> >> pci 0000:ff:12.4: BAR 2: failed to assign [mem size 0x00000040] >> pci 0000:ff:12.4: BAR 4: failed to assign [mem size 0x00000040] >> pci 0000:ff:12.4: BAR 1: failed to assign [mem size 0x00000010] >> pci 0000:ff:12.4: BAR 3: failed to assign [mem size 0x00000010] >> pci 0000:ff:12.4: BAR 5: failed to assign [mem size 0x00000010] >> pci 0000:ff:12.0: BAR 6: failed to assign [mem size 0x00000001 pref] >> pci 0000:ff:1e.3: BAR 6: failed to assign [mem size 0x00000001 pref] >> pci 0000:7f:12.4: BAR 2: failed to assign [mem size 0x00000040] >> pci 0000:7f:12.4: BAR 4: failed to assign [mem size 0x00000040] >> pci 0000:7f:12.4: BAR 1: failed to assign [mem size 0x00000010] >> pci 0000:7f:12.4: BAR 3: failed to assign [mem size 0x00000010] >> pci 0000:7f:12.4: BAR 5: failed to assign [mem size 0x00000010] >> pci 0000:7f:12.0: BAR 6: failed to assign [mem size 0x00000001 pref] >> pci 0000:7f:1e.3: BAR 6: failed to assign [mem size 0x00000001 pref] >> >> There are two issues with commit b894157. >> >> The first is that there is another device, Home Agent 1 & PCU, that must >> also be quirked in the same way. >> >> \# lspci -n -s 7f:12.4 >> 7f:12.4 0880: 8086:6f60 (rev 01) > > I think we should split this into two patches: one to add quirks for > the Home Agent 1 & PCU, and a second for the resource assignment > issue. > > Can you dig up a spec for these devices? I should have asked Andi for > that the first time around, but I didn't. Maybe there's something > we're not interpreting correctly. I still have a hard time believing > that Intel would produce a PCI device with non-BAR registers where the > BARs are supposed to be. Maybe there's supposed to be an EA > capability or something that tells us to ignore these registers. It looks like Andi has provided this information in his reply. I will add some of that info in my next post. > > Can you collect "lspci -vvxxx" output for one of these devices? # lspci -xxxvv -s 7f:12.4 7f:12.4 System peripheral: Intel Corporation Xeon E7 v4/Xeon E5 v4/Xeon E3 v4/Xeon D Home Agent 1 (rev 01) Subsystem: Intel Corporation Xeon E7 v4/Xeon E5 v4/Xeon E3 v4/Xeon D Home Agent 1 Control: I/O- Mem- BusMaster- SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B- DisINTx- Status: Cap- 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast >TAbort- SERR- >> After applying the quirk patch, we end up with: >> >> pci 0000:ff:12.0: BAR 6: failed to assign [mem size 0x00000001 pref] >> pci 0000:ff:12.4: BAR 6: failed to assign [mem size 0x00000001 pref] >> pci 0000:ff:1e.3: BAR 6: failed to assign [mem size 0x00000001 pref] >> pci 0000:7f:12.0: BAR 6: failed to assign [mem size 0x00000001 pref] >> pci 0000:7f:12.4: BAR 6: failed to assign [mem size 0x00000001 pref] >> pci 0000:7f:1e.3: BAR 6: failed to assign [mem size 0x00000001 pref] >> >> which drives us to the second issue. Since the PCI devices now >> have unnassigned resources (BARs), pcibios_assign_resources() >> call pci_assign_unassigned_root_bus_resources(). This results in the >> messages above. I have added a non_compliant_bars check in >> pbus_assign_resources_sorted() to avoid the unassigned device's resources >> from being added to the failed resources list for the bus. > > I don't understand this part yet. If we mark a device with > non_compliant_bars, __pci_read_base() will return without doing > anything, so we should not fill in the struct resource at all. It > wouldn't have the "mem" or "pref" bits shown above, and it shouldn't > participate in pcibios_assign_resources() at all. All of these are > for BAR 6 (the ROM BAR), so maybe there's something wrong with the way > to handle that in particular. I did some additional debugging after reading your comment. I dumped out the contents of the resources for each bus's devices. I concentrated on one particular device, 7f:12.4 (the output of lspci is above). Before the call to pcibios_assign_resources(), 7f:12.4 has pci 0000:7f:12.4: PRARIT: BAR 6: [mem 0x00000000 pref] so that means it the resource was not changed/modified in the call of pcibios_assign_resources(). I took a closer look at the code, and I think I know what the issue is. In pci_read_bases() the code does if (rom) { struct resource *res = &dev->resource[PCI_ROM_RESOURCE]; dev->rom_base_reg = rom; res->flags = IORESOURCE_MEM | IORESOURCE_PREFETCH | IORESOURCE_READONLY | IORESOURCE_CACHEABLE | IORESOURCE_SIZEALIGN; __pci_read_base(dev, pci_bar_mem32, res, rom); } which initializes the res->flags field. This field is later checked in pcibios_allocate_dev_rom_resource() which is called from pcibios_assign_resources(), and the resource is declared unassigned because it has a res->flags field. Perhaps (sorry for the cut-and-paste) this is a more correct fix which would avoid the setting of res->flags? There is no point in setting dev->rom_base_reg, etc., if this is a non-compliant device. P. --- 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/probe.c b/drivers/pci/probe.c index 2384100..818731a 100644 --- a/drivers/pci/probe.c +++ b/drivers/pci/probe.c @@ -361,7 +361,7 @@ static void pci_read_bases(struct pci_dev *dev, unsigned int pos += __pci_read_base(dev, pci_bar_unknown, res, reg); } - if (rom) { + if (rom && !dev->non_compliant_bars) { struct resource *res = &dev->resource[PCI_ROM_RESOURCE]; dev->rom_base_reg = rom; res->flags = IORESOURCE_MEM | IORESOURCE_PREFETCH |