From 7c950b9e53732f574e3a46d37c62f1f33d0b218c Mon Sep 17 00:00:00 2001 From: Dongdong Liu Date: Wed, 11 Oct 2017 18:52:58 +0800 Subject: PCI/portdrv: Add #defines for AER and DPC Interrupt Message Number masks In the AER case, the mask isn't strictly necessary because there are no higher-order bits above the Interrupt Message Number, but using a #define will make it possible to grep for it. Suggested-by: Bjorn Helgaas Signed-off-by: Dongdong Liu Signed-off-by: Bjorn Helgaas Reviewed-by: Christoph Hellwig --- drivers/pci/pcie/portdrv_core.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'drivers/pci/pcie/portdrv_core.c') diff --git a/drivers/pci/pcie/portdrv_core.c b/drivers/pci/pcie/portdrv_core.c index 313a21df1692..72fcbe5567dd 100644 --- a/drivers/pci/pcie/portdrv_core.c +++ b/drivers/pci/pcie/portdrv_core.c @@ -114,7 +114,7 @@ static int pcie_port_enable_irq_vec(struct pci_dev *dev, int *irqs, int mask) */ pos = pci_find_ext_capability(dev, PCI_EXT_CAP_ID_ERR); pci_read_config_dword(dev, pos + PCI_ERR_ROOT_STATUS, ®32); - entry = reg32 >> 27; + entry = (reg32 & PCI_ERR_ROOT_AER_IRQ) >> 27; if (entry >= nr_entries) goto out_free_irqs; @@ -141,7 +141,7 @@ static int pcie_port_enable_irq_vec(struct pci_dev *dev, int *irqs, int mask) */ pos = pci_find_ext_capability(dev, PCI_EXT_CAP_ID_DPC); pci_read_config_word(dev, pos + PCI_EXP_DPC_CAP, ®16); - entry = reg16 & 0x1f; + entry = reg16 & PCI_EXP_DPC_IRQ; if (entry >= nr_entries) goto out_free_irqs; -- cgit v1.2.3-59-g8ed1b From b8acfd7c0f88c49dc0089cd40e02040187160f6a Mon Sep 17 00:00:00 2001 From: Bjorn Helgaas Date: Thu, 19 Oct 2017 16:09:26 -0500 Subject: PCI/portdrv: Consolidate comments Consolidate some repetitive comments so we can see the code better. No functional change. Signed-off-by: Bjorn Helgaas Reviewed-by: Christoph Hellwig --- drivers/pci/pcie/portdrv_core.c | 63 ++++++----------------------------------- 1 file changed, 9 insertions(+), 54 deletions(-) (limited to 'drivers/pci/pcie/portdrv_core.c') diff --git a/drivers/pci/pcie/portdrv_core.c b/drivers/pci/pcie/portdrv_core.c index 72fcbe5567dd..accd16082348 100644 --- a/drivers/pci/pcie/portdrv_core.c +++ b/drivers/pci/pcie/portdrv_core.c @@ -56,40 +56,27 @@ 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. - */ + /* Allocate the maximum possible number of MSI/MSI-X vectors */ nr_entries = pci_alloc_irq_vectors(dev, 1, PCIE_PORT_MAX_MSI_ENTRIES, PCI_IRQ_MSIX | PCI_IRQ_MSI); if (nr_entries < 0) return nr_entries; + /* + * The Interrupt Message Number indicates which vector is used, i.e., + * the MSI-X table entry or the MSI offset between the base Message + * Data and the generated interrupt message. See PCIe r3.1, sec + * 7.8.2, 7.10.10, 7.31.2. + */ if (mask & (PCIE_PORT_SERVICE_PME | PCIE_PORT_SERVICE_HP)) { u16 reg16; - /* - * Per PCIe r3.1, sec 6.1.6, "PME and Hot-Plug Event - * interrupts (when both are implemented) always share the - * same MSI or MSI-X vector, as indicated by the Interrupt - * Message Number field in the PCI Express Capabilities - * register". - * - * Per sec 7.8.2, "For MSI, the [Interrupt Message Number] - * indicates the offset between the base Message Data and - * the interrupt message that is generated." - * - * "For MSI-X, the [Interrupt Message Number] indicates - * which MSI-X Table entry is used to generate the - * interrupt message." - */ pcie_capability_read_word(dev, PCI_EXP_FLAGS, ®16); entry = (reg16 & PCI_EXP_FLAGS_IRQ) >> 9; if (entry >= nr_entries) goto out_free_irqs; + /* PME and hotplug share an MSI/MSI-X vector */ irqs[PCIE_PORT_SERVICE_PME_SHIFT] = pci_irq_vector(dev, entry); irqs[PCIE_PORT_SERVICE_HP_SHIFT] = pci_irq_vector(dev, entry); @@ -99,19 +86,6 @@ static int pcie_port_enable_irq_vec(struct pci_dev *dev, int *irqs, int mask) if (mask & PCIE_PORT_SERVICE_AER) { u32 reg32, pos; - /* - * Per PCIe r3.1, sec 7.10.10, the Advanced Error Interrupt - * Message Number in the Root Error Status register - * indicates which MSI/MSI-X vector is used for AER. - * - * "For MSI, the [Advanced Error Interrupt Message Number] - * indicates the offset between the base Message Data and - * the interrupt message that is generated." - * - * "For MSI-X, the [Advanced Error Interrupt Message - * Number] indicates which MSI-X Table entry is used to - * generate the interrupt message." - */ pos = pci_find_ext_capability(dev, PCI_EXT_CAP_ID_ERR); pci_read_config_dword(dev, pos + PCI_ERR_ROOT_STATUS, ®32); entry = (reg32 & PCI_ERR_ROOT_AER_IRQ) >> 27; @@ -126,19 +100,6 @@ static int pcie_port_enable_irq_vec(struct pci_dev *dev, int *irqs, int mask) if (mask & PCIE_PORT_SERVICE_DPC) { u16 reg16, pos; - /* - * Per PCIe r4.0 (v0.9), sec 7.9.15.2, the DPC Interrupt - * Message Number in the DPC Capability register indicates - * which MSI/MSI-X vector is used for DPC. - * - * "For MSI, the [DPC Interrupt Message Number] indicates - * the offset between the base Message Data and the - * interrupt message that is generated." - * - * "For MSI-X, the [DPC Interrupt Message Number] indicates - * which MSI-X Table entry is used to generate the - * interrupt message." - */ pos = pci_find_ext_capability(dev, PCI_EXT_CAP_ID_DPC); pci_read_config_word(dev, pos + PCI_EXP_DPC_CAP, ®16); entry = reg16 & PCI_EXP_DPC_IRQ; @@ -150,16 +111,10 @@ static int pcie_port_enable_irq_vec(struct pci_dev *dev, int *irqs, int mask) 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 we allocated more than we need, free them and allocate fewer */ 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) -- cgit v1.2.3-59-g8ed1b From 3321eafd2a79f15126ebaa82f1b5d7fce89c02cb Mon Sep 17 00:00:00 2001 From: Bjorn Helgaas Date: Fri, 20 Oct 2017 08:48:06 -0500 Subject: PCI/portdrv: Factor out Interrupt Message Number lookup Factor out Interrupt Message Number lookup from the MSI/MSI-X interrupt setup. One side effect is that we only have to check once to see if we have enough vectors for all the services. No functional change intended. Signed-off-by: Bjorn Helgaas Reviewed-by: Christoph Hellwig --- drivers/pci/pcie/portdrv_core.c | 110 ++++++++++++++++++++++------------------ 1 file changed, 62 insertions(+), 48 deletions(-) (limited to 'drivers/pci/pcie/portdrv_core.c') diff --git a/drivers/pci/pcie/portdrv_core.c b/drivers/pci/pcie/portdrv_core.c index accd16082348..102f5c5fcfe0 100644 --- a/drivers/pci/pcie/portdrv_core.c +++ b/drivers/pci/pcie/portdrv_core.c @@ -43,6 +43,53 @@ static void release_pcie_device(struct device *dev) kfree(to_pcie_device(dev)); } +/* + * Fill in *pme, *aer, *dpc with the relevant Interrupt Message Numbers if + * services are enabled in "mask". Return the number of MSI/MSI-X vectors + * required to accommodate the largest Message Number. + */ +static int pcie_message_numbers(struct pci_dev *dev, int mask, + u32 *pme, u32 *aer, u32 *dpc) +{ + u32 nvec = 0, pos, reg32; + u16 reg16; + + /* + * The Interrupt Message Number indicates which vector is used, i.e., + * the MSI-X table entry or the MSI offset between the base Message + * Data and the generated interrupt message. See PCIe r3.1, sec + * 7.8.2, 7.10.10, 7.31.2. + */ + + if (mask & (PCIE_PORT_SERVICE_PME | PCIE_PORT_SERVICE_HP)) { + pcie_capability_read_word(dev, PCI_EXP_FLAGS, ®16); + *pme = (reg16 & PCI_EXP_FLAGS_IRQ) >> 9; + nvec = *pme + 1; + } + + if (mask & PCIE_PORT_SERVICE_AER) { + pos = pci_find_ext_capability(dev, PCI_EXT_CAP_ID_ERR); + if (pos) { + pci_read_config_dword(dev, pos + PCI_ERR_ROOT_STATUS, + ®32); + *aer = (reg32 & PCI_ERR_ROOT_AER_IRQ) >> 27; + nvec = max(nvec, *aer + 1); + } + } + + if (mask & PCIE_PORT_SERVICE_DPC) { + pos = pci_find_ext_capability(dev, PCI_EXT_CAP_ID_DPC); + if (pos) { + pci_read_config_word(dev, pos + PCI_EXP_DPC_CAP, + ®16); + *dpc = reg16 & PCI_EXP_DPC_IRQ; + nvec = max(nvec, *dpc + 1); + } + } + + return nvec; +} + /** * pcie_port_enable_irq_vec - try to set up MSI-X or MSI as interrupt mode * for given port @@ -54,7 +101,8 @@ static void release_pcie_device(struct device *dev) */ static int pcie_port_enable_irq_vec(struct pci_dev *dev, int *irqs, int mask) { - int nr_entries, entry, nvec = 0; + int nr_entries, nvec; + u32 pme = 0, aer = 0, dpc = 0; /* Allocate the maximum possible number of MSI/MSI-X vectors */ nr_entries = pci_alloc_irq_vectors(dev, 1, PCIE_PORT_MAX_MSI_ENTRIES, @@ -62,54 +110,24 @@ static int pcie_port_enable_irq_vec(struct pci_dev *dev, int *irqs, int mask) if (nr_entries < 0) return nr_entries; - /* - * The Interrupt Message Number indicates which vector is used, i.e., - * the MSI-X table entry or the MSI offset between the base Message - * Data and the generated interrupt message. See PCIe r3.1, sec - * 7.8.2, 7.10.10, 7.31.2. - */ - if (mask & (PCIE_PORT_SERVICE_PME | PCIE_PORT_SERVICE_HP)) { - u16 reg16; - - pcie_capability_read_word(dev, PCI_EXP_FLAGS, ®16); - entry = (reg16 & PCI_EXP_FLAGS_IRQ) >> 9; - if (entry >= nr_entries) - goto out_free_irqs; - - /* PME and hotplug share an MSI/MSI-X vector */ - 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); + /* See how many and which Interrupt Message Numbers we actually use */ + nvec = pcie_message_numbers(dev, mask, &pme, &aer, &dpc); + if (nvec > nr_entries) { + pci_free_irq_vectors(dev); + return -EIO; } - if (mask & PCIE_PORT_SERVICE_AER) { - u32 reg32, pos; - - pos = pci_find_ext_capability(dev, PCI_EXT_CAP_ID_ERR); - pci_read_config_dword(dev, pos + PCI_ERR_ROOT_STATUS, ®32); - entry = (reg32 & PCI_ERR_ROOT_AER_IRQ) >> 27; - if (entry >= nr_entries) - goto out_free_irqs; - - irqs[PCIE_PORT_SERVICE_AER_SHIFT] = pci_irq_vector(dev, entry); - - nvec = max(nvec, entry + 1); + /* PME and hotplug share an MSI/MSI-X vector */ + if (mask & (PCIE_PORT_SERVICE_PME | PCIE_PORT_SERVICE_HP)) { + irqs[PCIE_PORT_SERVICE_PME_SHIFT] = pci_irq_vector(dev, pme); + irqs[PCIE_PORT_SERVICE_HP_SHIFT] = pci_irq_vector(dev, pme); } - if (mask & PCIE_PORT_SERVICE_DPC) { - u16 reg16, pos; - - pos = pci_find_ext_capability(dev, PCI_EXT_CAP_ID_DPC); - pci_read_config_word(dev, pos + PCI_EXP_DPC_CAP, ®16); - entry = reg16 & PCI_EXP_DPC_IRQ; - if (entry >= nr_entries) - goto out_free_irqs; - - irqs[PCIE_PORT_SERVICE_DPC_SHIFT] = pci_irq_vector(dev, entry); + if (mask & PCIE_PORT_SERVICE_AER) + irqs[PCIE_PORT_SERVICE_AER_SHIFT] = pci_irq_vector(dev, aer); - nvec = max(nvec, entry + 1); - } + if (mask & PCIE_PORT_SERVICE_DPC) + irqs[PCIE_PORT_SERVICE_DPC_SHIFT] = pci_irq_vector(dev, dpc); /* If we allocated more than we need, free them and allocate fewer */ if (nvec != nr_entries) { @@ -122,10 +140,6 @@ static int pcie_port_enable_irq_vec(struct pci_dev *dev, int *irqs, int mask) } return 0; - -out_free_irqs: - pci_free_irq_vectors(dev); - return -EIO; } /** -- cgit v1.2.3-59-g8ed1b From a579ba49a9e27bfc1d5cb69b0ea3781d8df46b5b Mon Sep 17 00:00:00 2001 From: Bjorn Helgaas Date: Fri, 20 Oct 2017 08:57:16 -0500 Subject: PCI/portdrv: Compute MSI/MSI-X IRQ vectors after final allocation When setting up portdrv MSI/MSI-X interrupts, we previously allocated the maximum possible number of vectors, read the Interrupt Message Numbers for each service, saved the IRQ for each, freed the vectors, and finally used the largest Message Number to reallocate only as many vectors as we need. The problem is that freeing the vectors invalidates their IRQs, so the saved IRQ numbers may now be invalid, which can result in errors like this: pcie_pme: probe of 0000:00:00.0:pcie001 failed with error -22 pciehp 0000:00:00.0:pcie004: Cannot get irq 20 for the hotplug controller aer: probe of 0000:00:00.0:pcie002 failed with error -22 dpc 0000:00:00.0:pcie010: request IRQ22 failed: -22 Change the setup so we save the Interrupt Message Numbers (not the IRQs) before we free the original setup, then use the Message Numbers to compute the IRQs (via pci_irq_vector()) *after* we reallocate the vectors. This should always be safe for MSI-X because the Message Numbers are fixed. For MSI, the hardware is allowed to change Message Numbers when we update the MSI Multiple Message Enable field when reallocating the vectors, but since we allocate enough vectors to accommodate the largest Message Number we found, that's unlikely. See PCIe r3.1, sec 7.8.2, 7.10.10, 7.31.2. Fixes: 3674cc49da9a ("PCI/portdrv: Use pci_irq_alloc_vectors()") Based-on-patch-by: Dongdong Liu Tested-by: Dongdong Liu # HiSilicon hip08 Signed-off-by: Bjorn Helgaas Reviewed-by: Christoph Hellwig --- drivers/pci/pcie/portdrv_core.c | 30 ++++++++++++++++++++---------- 1 file changed, 20 insertions(+), 10 deletions(-) (limited to 'drivers/pci/pcie/portdrv_core.c') diff --git a/drivers/pci/pcie/portdrv_core.c b/drivers/pci/pcie/portdrv_core.c index 102f5c5fcfe0..3cd5eb48644a 100644 --- a/drivers/pci/pcie/portdrv_core.c +++ b/drivers/pci/pcie/portdrv_core.c @@ -117,6 +117,26 @@ static int pcie_port_enable_irq_vec(struct pci_dev *dev, int *irqs, int mask) return -EIO; } + /* + * If we allocated more than we need, free them and reallocate fewer. + * + * Reallocating may change the specific vectors we get, so + * pci_irq_vector() must be done *after* the reallocation. + * + * If we're using MSI, hardware is *allowed* to change the Interrupt + * Message Numbers when we free and reallocate the vectors, but we + * assume it won't because we allocate enough vectors for the + * biggest Message Number we found. + */ + if (nvec != nr_entries) { + pci_free_irq_vectors(dev); + + nr_entries = pci_alloc_irq_vectors(dev, nvec, nvec, + PCI_IRQ_MSIX | PCI_IRQ_MSI); + if (nr_entries < 0) + return nr_entries; + } + /* PME and hotplug share an MSI/MSI-X vector */ if (mask & (PCIE_PORT_SERVICE_PME | PCIE_PORT_SERVICE_HP)) { irqs[PCIE_PORT_SERVICE_PME_SHIFT] = pci_irq_vector(dev, pme); @@ -129,16 +149,6 @@ static int pcie_port_enable_irq_vec(struct pci_dev *dev, int *irqs, int mask) if (mask & PCIE_PORT_SERVICE_DPC) irqs[PCIE_PORT_SERVICE_DPC_SHIFT] = pci_irq_vector(dev, dpc); - /* If we allocated more than we need, free them and allocate fewer */ - if (nvec != nr_entries) { - pci_free_irq_vectors(dev); - - nr_entries = pci_alloc_irq_vectors(dev, nvec, nvec, - PCI_IRQ_MSIX | PCI_IRQ_MSI); - if (nr_entries < 0) - return nr_entries; - } - return 0; } -- cgit v1.2.3-59-g8ed1b