From patchwork Tue May 26 02:17:05 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Rafael J. Wysocki" X-Patchwork-Id: 6477621 X-Patchwork-Delegate: bhelgaas@google.com Return-Path: X-Original-To: patchwork-linux-pci@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork1.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork1.web.kernel.org (Postfix) with ESMTP id A92EC9F1CC for ; Tue, 26 May 2015 01:51:44 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 9F9E920592 for ; Tue, 26 May 2015 01:51:43 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 8CA9F2055B for ; Tue, 26 May 2015 01:51:41 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751181AbbEZBvi (ORCPT ); Mon, 25 May 2015 21:51:38 -0400 Received: from v094114.home.net.pl ([79.96.170.134]:45784 "HELO v094114.home.net.pl" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with SMTP id S1751085AbbEZBvi (ORCPT ); Mon, 25 May 2015 21:51:38 -0400 Received: from cms249.neoplus.adsl.tpnet.pl (83.31.146.249) (HELO vostro.rjw.lan) by serwer1319399.home.pl (79.96.170.134) with SMTP (IdeaSmtpServer v0.80) id 18ef46a79fb55160; Tue, 26 May 2015 03:51:35 +0200 From: "Rafael J. Wysocki" To: Boris Ostrovsky , Sander Eikelenboom Cc: david.vrabel@citrix.com, xen-devel@lists.xenproject.org, linux-kernel@vger.kernel.org, Bjorn Helgaas , ACPI Devel Maling List , Linux PCI Subject: [PATCH] PCI / ACPI: Do not set ACPI companions for host bridges with parents Date: Tue, 26 May 2015 04:17:05 +0200 Message-ID: <2625308.mOex6suvmm@vostro.rjw.lan> User-Agent: KMail/4.11.5 (Linux/4.0.0+; KDE/4.11.5; x86_64; ; ) In-Reply-To: <1901357.YAorP09rCo@vostro.rjw.lan> References: <555FDDA1.8030806@oracle.com> <2059136.yfbqz351P1@vostro.rjw.lan> <1901357.YAorP09rCo@vostro.rjw.lan> MIME-Version: 1.0 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.9 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_HI, T_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 Tuesday, May 26, 2015 03:08:17 AM Rafael J. Wysocki wrote: > On Tuesday, May 26, 2015 01:42:16 AM Rafael J. Wysocki wrote: > > On Tuesday, May 26, 2015 01:22:12 AM Rafael J. Wysocki wrote: > > > On Friday, May 22, 2015 09:53:37 PM Boris Ostrovsky wrote: > > > > On 05/22/2015 04:11 AM, Sander Eikelenboom wrote: > > > > > Hello Sander, > > > > > > > > > [cut] > > > > > > (+Rafael again) > > > > > > > > So the immediate cause of those errors is that pdev->evtchn is 0. > > > > Backend is not notified and things not go well then. > > > > > > > > And it is indeed caused by 97badf873ab60e841243b66133ff9eff2a46ef29: > > > > > > > > We allocate pcifront_sd in pcifront_scan_root() and then pass it to > > > > pci_scan_bus_parented() as sysdata. Eventually this sysdata is used in > > > > pcibios_root_bridge_prepare() as pci_sysdata. It is dereferenced as > > > > pci_sysdata->companion (which I believe is aliased to pcifront_sd->pdev) > > > > Well, there is an int node field between them, so I'm not sure. > > > > > > and then set_primary_fwnode() writes it, thus corrupting > > > > pcifront_sd->pdev (and I think this is what sets evtchn to zero). > > > > So the corruption happens when set_primary_fwnode() writes NULL to the > > 'secondary' field of object pointed to by 'fwnode'. > > > > This isn't strictly necessary and we might avoid the crash by only > > writing to fwnode->secondary if fn is not NULL. > > > > So, Sander please test the patch below too if possible. > > > > Of course, that doesn't solve a problem of passing an incorrect pointer > > to ACPI_COMPANION_SET() in pcibios_root_bridge_prepare(). > > And here's one more thing to test. And the below is how I'd fix it, so you can simply test this patch and skip the previous ones. Acked-by: Bjorn Helgaas --- From: Rafael J. Wysocki Subject: PCI / ACPI: Do not set ACPI companions for host bridges with parents Commit 97badf873ab6 (device property: Make it possible to use secondary firmware nodes) uncovered a bug in the x86 (and ia64) PCI host bridge initialization code that assumes bridge->bus->sysdata to always point to a struct pci_sysdata object which need not be the case (in particular, the Xen PCI frontend driver sets it to point to a different data type). If it is not the case, an incorrect pointer (or a piece of data that is not a pointer at all) will be passed to ACPI_COMPANION_SET() and that may cause interesting breakage to happen going forward. To work around this problem use the observation that the ACPI host bridge initialization always passes NULL as parent to pci_create_root_bus(), so if pcibios_root_bridge_prepare() sees a non-NULL parent of the bridge, it should not attempt to set an ACPI companion for it, because that means that pci_create_root_bus() has been called by someone else. Reported-by: Sander Eikelenboom Signed-off-by: Rafael J. Wysocki --- arch/ia64/pci/pci.c | 13 ++++++++++--- arch/x86/pci/acpi.c | 13 ++++++++++--- 2 files changed, 20 insertions(+), 6 deletions(-) -- 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 Index: linux-pm/arch/x86/pci/acpi.c =================================================================== --- linux-pm.orig/arch/x86/pci/acpi.c +++ linux-pm/arch/x86/pci/acpi.c @@ -482,9 +482,16 @@ struct pci_bus *pci_acpi_scan_root(struc int pcibios_root_bridge_prepare(struct pci_host_bridge *bridge) { - struct pci_sysdata *sd = bridge->bus->sysdata; - - ACPI_COMPANION_SET(&bridge->dev, sd->companion); + /* + * We pass NULL as parent to pci_create_root_bus(), so if it is not NULL + * here, pci_create_root_bus() has been called by someone else and + * sysdata is likely to be different from what we expect. Let it go in + * that case. + */ + if (!bridge->dev.parent) { + struct pci_sysdata *sd = bridge->bus->sysdata; + ACPI_COMPANION_SET(&bridge->dev, sd->companion); + } return 0; } Index: linux-pm/arch/ia64/pci/pci.c =================================================================== --- linux-pm.orig/arch/ia64/pci/pci.c +++ linux-pm/arch/ia64/pci/pci.c @@ -478,9 +478,16 @@ struct pci_bus *pci_acpi_scan_root(struc int pcibios_root_bridge_prepare(struct pci_host_bridge *bridge) { - struct pci_controller *controller = bridge->bus->sysdata; - - ACPI_COMPANION_SET(&bridge->dev, controller->companion); + /* + * We pass NULL as parent to pci_create_root_bus(), so if it is not NULL + * here, pci_create_root_bus() has been called by someone else and + * sysdata is likely to be different from what we expect. Let it go in + * that case. + */ + if (!bridge->dev.parent) { + struct pci_controller *controller = bridge->bus->sysdata; + ACPI_COMPANION_SET(&bridge->dev, controller->companion); + } return 0; }