From patchwork Mon Feb 2 21:06:26 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Bjorn Helgaas X-Patchwork-Id: 5764291 Return-Path: X-Original-To: patchwork-kvm@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 B34609F380 for ; Mon, 2 Feb 2015 21:06:37 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id A99442097A for ; Mon, 2 Feb 2015 21:06:36 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 8C9E72086F for ; Mon, 2 Feb 2015 21:06:35 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S964781AbbBBVGc (ORCPT ); Mon, 2 Feb 2015 16:06:32 -0500 Received: from mail-oi0-f49.google.com ([209.85.218.49]:45882 "EHLO mail-oi0-f49.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754448AbbBBVGa (ORCPT ); Mon, 2 Feb 2015 16:06:30 -0500 Received: by mail-oi0-f49.google.com with SMTP id a3so46009639oib.8 for ; Mon, 02 Feb 2015 13:06:29 -0800 (PST) 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=jFw7LPiKK0K2iFMFjVLyvVxUZTHe1Be7dsN2MyYykUg=; b=jF3XD0kuYjcw+ZAixzJBOYTRIw7O70BnJYVK2ecdMVmdvlYiiIO+OwJvgHxUcGNdTa x5M0Vv+SDP0SLnSPAxzYauMbw6mx5J5xJ2NniNioq6C4w0XWc0prSFresotFmVdSCU/V Q6sq8F4nrj1N6vJ9UChQkleyPdOb7hInLbMz1zmdYousqvBeQgJrxqI/sI/stpafHpz8 aRALq33O9C4YINbFxEhml4oVZLJrYIR6INS9w0w2gRsDEzrmukCmxstfSIwnqxm8Tskm RIfRzS5jM/q0O407XLB2RDoSdosfchkBwC8eHGRqpOi02nKHCvot9z13TXG1HuuJqe5e 7CLw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; 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=jFw7LPiKK0K2iFMFjVLyvVxUZTHe1Be7dsN2MyYykUg=; b=hf3lEa52rsM+oQdsnB0BZNBrkXDJv3kBlH25q6PNGKOs/KZ9nyVjrkxkUEn/t4+aAA e/VHEaYqZ97Op6vpJintYAVaFZGwQOqvJvzmbZJcvUpfeNGZXOR4jiSl49GYMzV2dhk6 SddjrVPeJ1vxFxcMMcV043K4yRq+Cx+uXMc17EXxCq3hhRILKUDTDMyzM5+hVzRvtx0O St7ZuND53rY4j4Wby+fkMnD2vEkv9kwO7QlU/pA2DEUGQFQ7d8emC4k7VErzLc5xnLd1 GrL4Vt0XJk2a6zaAZbqCocoToxU3Bab+Xn42CHayIsOnDNVGLAWcqqnItRtljXzqTfMM 7ctg== X-Gm-Message-State: ALoCoQl4SPg7LY5xGEKEZtqOPpiCFHjdx1IiAUWtqb3s8zo4OKIF8Mh46XW+aLH9zPtcVpKBzwds X-Received: by 10.202.210.86 with SMTP id j83mr12703853oig.131.1422911189521; Mon, 02 Feb 2015 13:06:29 -0800 (PST) Received: from google.com ([69.71.1.1]) by mx.google.com with ESMTPSA id d10sm10095195obf.17.2015.02.02.13.06.28 (version=TLSv1.2 cipher=RC4-SHA bits=128/128); Mon, 02 Feb 2015 13:06:28 -0800 (PST) Date: Mon, 2 Feb 2015 15:06:26 -0600 From: Bjorn Helgaas To: Yijing Wang Cc: linux-pci@vger.kernel.org, boris.ostrovsky@oracle.com, yinghai@kernel.org, xen-devel@lists.xenproject.org, konrad.wilk@oracle.com, david.vrabel@citrix.com, alex.williamson@redhat.com, kvm@vger.kernel.org, Jan Beulich Subject: Re: [PATCH v3] PCI: Add guard to avoid mapping a invalid msix base address Message-ID: <20150202210626.GC5176@google.com> References: <1422503683-19709-1-git-send-email-wangyijing@huawei.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <1422503683-19709-1-git-send-email-wangyijing@huawei.com> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: kvm-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: kvm@vger.kernel.org X-Spam-Status: No, score=-4.0 required=5.0 tests=BAYES_00, DKIM_ADSP_CUSTOM_MED, DKIM_SIGNED, FSL_HELO_FAKE, RCVD_IN_DNSWL_HI, T_DKIM_INVALID, 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 [+cc Jan] On Thu, Jan 29, 2015 at 11:54:43AM +0800, Yijing Wang wrote: > Sometimes, a pci bridge device BAR was not assigned > properly. After we call pci_bus_assign_resources(), the > resource of the BAR would be reseted. So if we try to > enable msix for this device, it will map a invalid > resource as the msix base address, and a warning call trace > will report. > > pci_bus_assign_resources() > __pci_bus_assign_resources() > pbus_assign_resources_sorted() > __assign_resources_sorted() > assign_requested_resources_sorted() > pci_assign_resource() -->fail > reset_resource() -->res->start/end/flags = 0 > > pcie_port_device_register() > init_service_irqs() > pcie_port_enable_msix() > ... > msix_capability_init() > msix_map_region() > phys_addr = pci_resource_start(dev, bir) + table_offset; > If BAR(index=bir) was not assign properly, pci_resource_start(dev, bir) > here would return 0, so phys_addr is a invalid physical > address of msix. > > [ 43.094087] ------------[ cut here ]------------ > [ 43.097418] WARNING: CPU: 1 PID: 1800 at arch/arm64/mm/ioremap.c:58 __ioremap_caller+0xd4/0xe8() > ... > [ 43.121694] CPU: 1 PID: 1800 Comm: insmod Tainted: G O 3.16.0 #5 > [ 43.127374] Call trace: > [ 43.128522] [] dump_backtrace+0x0/0x130 > [ 43.132637] [] show_stack+0x10/0x1c > [ 43.136402] [] dump_stack+0x74/0x94 > [ 43.140166] [] warn_slowpath_common+0x8c/0xb4 > [ 43.144804] [] warn_slowpath_null+0x14/0x20 > [ 43.149266] [] __ioremap_caller+0xd0/0xe8 > [ 43.153555] [] __ioremap+0xc/0x18 > [ 43.157145] [] pci_enable_msix+0x238/0x44c > [ 43.161521] [] pci_enable_msix_range+0x34/0x80 > [ 43.166243] [] pcie_port_device_register+0x104/0x480 > [ 43.171491] [] pcie_portdrv_probe+0x38/0xa0 > [ 43.175952] [] pci_device_probe+0x78/0xd4 > [ 43.180238] [] really_probe+0x6c/0x22c > [ 43.184265] [] __device_attach+0x5c/0x6c > [ 43.188466] [] bus_for_each_drv+0x50/0x94 > [ 43.192755] [] device_attach+0x9c/0xc0 > [ 43.196780] [] pci_bus_add_device+0x38/0x80 > [ 43.201243] [] pci_bus_add_devices+0x4c/0xd4 > [ 43.205791] [] pci_common_init+0x274/0x378 > [ 43.210170] [] $x+0xb8c/0xc88 [pcie] > [ 43.214024] [] platform_drv_probe+0x20/0x58 > [ 43.218483] [] really_probe+0xc4/0x22c > [ 43.222510] [] __driver_attach+0xa0/0xa8 > [ 43.226708] [] bus_for_each_dev+0x54/0x98 > [ 43.231000] [] driver_attach+0x1c/0x28 > [ 43.235024] [] bus_add_driver+0x14c/0x204 > [ 43.239309] [] driver_register+0x64/0x130 > [ 43.243598] [] __platform_driver_register+0x5c/0x68 > [ 43.248757] [] platform_driver_probe+0x28/0xac > [ 43.253485] [] pcie_init+0x30/0x3c [pcie] > [ 43.258293] [] do_one_initcall+0x88/0x19c > [ 43.262585] [] load_module+0xc2c/0xf2c > [ 43.266609] [] SyS_finit_module+0x78/0x88 > [ 43.270897] ---[ end trace ea5eb60837afb5aa ]--- > > Reported-by: Zhang Jukuo > Tested-by: Zhang Jukuo > Signed-off-by: Yijing Wang I applied this to pci/msi for v3.20, thanks! I reworked the changelog as follows; let me know if it still doesn't make things clear: commit 6a878e5085fe97bd1e222b7883a1b815fcbbe4ed Author: Yijing Wang Date: Wed Jan 28 09:52:17 2015 +0800 PCI: Fail MSI-X mappings if there's no space assigned to MSI-X BAR Unlike MSI, which is configured via registers in the MSI capability in Configuration Space, MSI-X is configured via tables in Memory Space. These MSI-X tables are mapped by a device BAR, and if no Memory Space has been assigned to the BAR, MSI-X cannot be used. Fail MSI-X setup if no space has been assigned for the BAR. Previously, we ioremapped the MSI-X table even if the resource hadn't been assigned. In this case, the resource address is undefined (and is often zero), which may lead to warnings or oopses in this path: pci_enable_msix msix_capability_init msix_map_region ioremap_nocache The PCI core sets resource flags to zero when it can't assign space for the resource (see reset_resource()). There are also some cases where it sets the IORESOURCE_UNSET flag, e.g., pci_reassigndev_resource_alignment(), pci_assign_resource(), etc. So we must check for both cases. [bhelgaas: changelog] Reported-by: Zhang Jukuo Tested-by: Zhang Jukuo Signed-off-by: Yijing Wang Signed-off-by: Bjorn Helgaas --- To unsubscribe from this list: send the line "unsubscribe kvm" 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/arch/x86/pci/xen.c b/arch/x86/pci/xen.c index c489ef2c1a39..34fc4189ebf0 100644 --- a/arch/x86/pci/xen.c +++ b/arch/x86/pci/xen.c @@ -298,12 +298,16 @@ static int xen_initdom_setup_msi_irqs(struct pci_dev *dev, int nvec, int type) map_irq.entry_nr = nvec; } else if (type == PCI_CAP_ID_MSIX) { int pos; + unsigned long flags; u32 table_offset, bir; pos = dev->msix_cap; pci_read_config_dword(dev, pos + PCI_MSIX_TABLE, &table_offset); bir = (u8)(table_offset & PCI_MSIX_TABLE_BIR); + flags = pci_resource_flags(dev, bir); + if (!flags || (flags & IORESOURCE_UNSET)) + return -EINVAL; map_irq.table_base = pci_resource_start(dev, bir); map_irq.entry_nr = msidesc->msi_attrib.entry_nr; diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c index fd60806d3fd0..c3e7dfcf9ff5 100644 --- a/drivers/pci/msi.c +++ b/drivers/pci/msi.c @@ -694,11 +694,16 @@ static void __iomem *msix_map_region(struct pci_dev *dev, unsigned nr_entries) { resource_size_t phys_addr; u32 table_offset; + unsigned long flags; u8 bir; pci_read_config_dword(dev, dev->msix_cap + PCI_MSIX_TABLE, &table_offset); bir = (u8)(table_offset & PCI_MSIX_TABLE_BIR); + flags = pci_resource_flags(dev, bir); + if (!flags || (flags & IORESOURCE_UNSET)) + return NULL; + table_offset &= PCI_MSIX_TABLE_OFFSET; phys_addr = pci_resource_start(dev, bir) + table_offset;