From patchwork Wed Oct 11 10:52:57 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Dongdong Liu X-Patchwork-Id: 824331 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=) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 3yBqrG1S37z9t16 for ; Wed, 11 Oct 2017 21:25:50 +1100 (AEDT) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751964AbdJKKZr (ORCPT ); Wed, 11 Oct 2017 06:25:47 -0400 Received: from szxga04-in.huawei.com ([45.249.212.190]:7968 "EHLO szxga04-in.huawei.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751613AbdJKKZq (ORCPT ); Wed, 11 Oct 2017 06:25:46 -0400 Received: from 172.30.72.60 (EHLO DGGEMS404-HUB.china.huawei.com) ([172.30.72.60]) by dggrg04-dlp.huawei.com (MOS 4.4.6-GA FastPath queued) with ESMTP id DIV86702; Wed, 11 Oct 2017 18:25:43 +0800 (CST) Received: from linux-ioko.site (10.71.200.31) by DGGEMS404-HUB.china.huawei.com (10.3.19.204) with Microsoft SMTP Server id 14.3.301.0; Wed, 11 Oct 2017 18:25:11 +0800 From: Dongdong Liu To: , CC: , , , , , Dongdong Liu Subject: [PATCH V4 1/2] PCI/portdrv: Fix MSI/MSI-X bug for PCIe port service drivers Date: Wed, 11 Oct 2017 18:52:57 +0800 Message-ID: <1507719178-112556-2-git-send-email-liudongdong3@huawei.com> X-Mailer: git-send-email 1.9.1 In-Reply-To: <1507719178-112556-1-git-send-email-liudongdong3@huawei.com> References: <1507719178-112556-1-git-send-email-liudongdong3@huawei.com> MIME-Version: 1.0 X-Originating-IP: [10.71.200.31] X-CFilter-Loop: Reflected X-Mirapoint-Virus-RAPID-Raw: score=unknown(0), refid=str=0001.0A020206.59DDF1A8.005E, ss=1, re=0.000, recu=0.000, reip=0.000, cl=1, cld=1, fgs=0, ip=0.0.0.0, so=2014-11-16 11:51:01, dmn=2013-03-21 17:37:32 X-Mirapoint-Loop-Id: 9f91f3ca0cd80bd09acdaeff105d029f Sender: linux-pci-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-pci@vger.kernel.org After removing and adding back the PCI root port device, we see the PCIe port service drivers request irq failed. pcie_pme: probe of 0000:00:00.0:pcie001 failed with error -22 aer: probe of 0000:00:00.0:pcie002 failed with error -22 pciehp 0000:00:00.0:pcie004: Slot #0 AttnBtn- PwrCtrl- MRL- AttnInd- PwrInd- HotPlug+ Surprise+ Interlock- NoCompl- LLActRep+ pciehp 0000:00:00.0:pcie004: Cannot get irq 20 for the hotplug controller pciehp 0000:00:00.0:pcie004: Notification initialization failed (-1) dpc 0000:00:00.0:pcie010: request IRQ22 failed: -22 dpc: probe of 0000:00:00.0:pcie010 failed with error -22 The current code basically does this: - allocate 32 vectors - figure out vector used by PME and hotplug - figure out vector used by AER - figure out vector used by DPC - free the 32 vectors we allocated - allocate only as many vectors as we need but it is broken as calling pci_free_irq_vectors() invalidates the IRQ numbers returned before by pci_irq_vectors(); The hardware works: - PME and hotplug use the Interrupt Message Number from the PCIe Capability register. - AER uses the AER Interrupt Message Number from the AER Root Error Status register. - DPC uses the DPC Interrupt Message Number from the DPC Capability register. - FRS (not supported by Linux yet) uses the FRS Interrupt Message Number from the FRS Queuing Capability register. - That's a total of 4 possible MSI/MSI-X vectors used for PME, hotplug, AER, DPC, and FRS, so there's no point in trying to allocate more than 4 vectors (we currently start with 32). - All these Interrupt Message Numbers are read-only to software but are updated by hardware when software writes the Multiple Message Enable field of the MSI Message Control register. Thanks for pointing this out; I didn't realize this before. - Therefore, we must read the Interrupt Message Numbers *after* we write Multiple Message Enable. Since we can use at most 4 vectors, this patch is to optimize the interrupt vector allocation strategy in pcie_port_enable_irq_vec(). First figure out how many vectors we need,allocate them, and then fill in the slots in the irqs[] table. Cc: Fixes: 3674cc49da9a ("PCI/portdrv: Use pci_irq_alloc_vectors()") Suggested-by: Bjorn Helgaas Signed-off-by: Dongdong Liu Signed-off-by: Gabriele Paoloni --- drivers/pci/pcie/portdrv_core.c | 45 +++++++++++++---------------------------- 1 file changed, 14 insertions(+), 31 deletions(-) diff --git a/drivers/pci/pcie/portdrv_core.c b/drivers/pci/pcie/portdrv_core.c index 313a21d..9692379 100644 --- a/drivers/pci/pcie/portdrv_core.c +++ b/drivers/pci/pcie/portdrv_core.c @@ -56,14 +56,16 @@ static int pcie_port_enable_irq_vec(struct pci_dev *dev, int *irqs, int mask) { int nr_entries, entry, nvec = 0; - /* - * Allocate as many entries as the port wants, so that we can check - * which of them will be useful. Moreover, if nr_entries is correctly - * equal to the number of entries this port actually uses, we'll happily - * go through without any tricks. - */ - nr_entries = pci_alloc_irq_vectors(dev, 1, PCIE_PORT_MAX_MSI_ENTRIES, - PCI_IRQ_MSIX | PCI_IRQ_MSI); + if (mask & (PCIE_PORT_SERVICE_PME | PCIE_PORT_SERVICE_HP)) + nvec++; + if (mask & PCIE_PORT_SERVICE_AER) + nvec++; + if (mask & PCIE_PORT_SERVICE_DPC) + nvec++; + + /* Allocate as many vectors as the we need */ + nr_entries = pci_alloc_irq_vectors(dev, 1, nvec, + PCI_IRQ_MSIX | PCI_IRQ_MSI); if (nr_entries < 0) return nr_entries; @@ -90,10 +92,10 @@ static int pcie_port_enable_irq_vec(struct pci_dev *dev, int *irqs, int mask) if (entry >= nr_entries) goto out_free_irqs; - irqs[PCIE_PORT_SERVICE_PME_SHIFT] = pci_irq_vector(dev, entry); - irqs[PCIE_PORT_SERVICE_HP_SHIFT] = pci_irq_vector(dev, entry); - - nvec = max(nvec, entry + 1); + if (mask & PCIE_PORT_SERVICE_PME) + irqs[PCIE_PORT_SERVICE_PME_SHIFT] = pci_irq_vector(dev, entry); + if (mask & PCIE_PORT_SERVICE_HP) + irqs[PCIE_PORT_SERVICE_HP_SHIFT] = pci_irq_vector(dev, entry); } if (mask & PCIE_PORT_SERVICE_AER) { @@ -120,7 +122,6 @@ static int pcie_port_enable_irq_vec(struct pci_dev *dev, int *irqs, int mask) irqs[PCIE_PORT_SERVICE_AER_SHIFT] = pci_irq_vector(dev, entry); - nvec = max(nvec, entry + 1); } if (mask & PCIE_PORT_SERVICE_DPC) { @@ -146,24 +147,6 @@ static int pcie_port_enable_irq_vec(struct pci_dev *dev, int *irqs, int mask) goto out_free_irqs; irqs[PCIE_PORT_SERVICE_DPC_SHIFT] = pci_irq_vector(dev, entry); - - nvec = max(nvec, entry + 1); - } - - /* - * If nvec is equal to the allocated number of entries, we can just use - * what we have. Otherwise, the port has some extra entries not for the - * services we know and we need to work around that. - */ - if (nvec != nr_entries) { - /* Drop the temporary MSI-X setup */ - pci_free_irq_vectors(dev); - - /* Now allocate the MSI-X vectors for real */ - nr_entries = pci_alloc_irq_vectors(dev, nvec, nvec, - PCI_IRQ_MSIX | PCI_IRQ_MSI); - if (nr_entries < 0) - return nr_entries; } return 0;