All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v6 0/4] PCI/portdrv: Fix MSI/MSI-X bug for PCIe port service drivers
@ 2017-10-20 16:21 Bjorn Helgaas
  2017-10-20 16:22 ` [PATCH v6 1/4] PCI/portdrv: Add #defines for AER and DPC Interrupt Message Number masks Bjorn Helgaas
                   ` (4 more replies)
  0 siblings, 5 replies; 8+ messages in thread
From: Bjorn Helgaas @ 2017-10-20 16:21 UTC (permalink / raw)
  To: Dongdong Liu
  Cc: Charles Chenxin, linuxarm, Christoph Hellwig, Gabriele Paoloni,
	linux-pci

This is based on Dongdong's series:
  v3: http://lkml.kernel.org/r/1507553167-27833-1-git-send-email-liudongdong3@huawei.com
  v4: http://lkml.kernel.org/r/1507719178-112556-1-git-send-email-liudongdong3@huawei.com
  v5: http://lkml.kernel.org/r/20171019225726.17806.62380.stgit@bhelgaas-glaptop.roam.corp.google.com

The main idea is to fix IRQ issues when removing and adding back a Root
Port.  The problem is that when we allocate, free, and reallocate MSI/MSI-X
vectors for the device, we may not get the same vectors the second time,
but we saved IRQs based on the first set of vectors.

Changes in v5:
  - add back a general Interrupt Message Number comment
  - factor out Interrupt Message Number helper separately

This is on pci/msi for v4.15.

---

Bjorn Helgaas (3):
      PCI/portdrv: Consolidate comments
      PCI/portdrv: Factor out Interrupt Message Number lookup
      PCI/portdrv: Compute MSI/MSI-X IRQ vectors after final allocation

Dongdong Liu (1):
      PCI/portdrv: Add #defines for AER and DPC Interrupt Message Number masks


 drivers/pci/pcie/portdrv_core.c |  171 +++++++++++++++++----------------------
 include/uapi/linux/pci_regs.h   |    2 
 2 files changed, 77 insertions(+), 96 deletions(-)

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v6 1/4] PCI/portdrv: Add #defines for AER and DPC Interrupt Message Number masks
  2017-10-20 16:21 [PATCH v6 0/4] PCI/portdrv: Fix MSI/MSI-X bug for PCIe port service drivers Bjorn Helgaas
@ 2017-10-20 16:22 ` Bjorn Helgaas
  2017-10-20 16:22 ` [PATCH v6 2/4] PCI/portdrv: Consolidate comments Bjorn Helgaas
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 8+ messages in thread
From: Bjorn Helgaas @ 2017-10-20 16:22 UTC (permalink / raw)
  To: Dongdong Liu
  Cc: Charles Chenxin, linuxarm, Christoph Hellwig, Gabriele Paoloni,
	linux-pci

From: Dongdong Liu <liudongdong3@huawei.com>

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 <bhelgaas@google.com>
Signed-off-by: Dongdong Liu <liudongdong3@huawei.com>
Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
---
 drivers/pci/pcie/portdrv_core.c |    4 ++--
 include/uapi/linux/pci_regs.h   |    2 ++
 2 files changed, 4 insertions(+), 2 deletions(-)

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, &reg32);
-		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, &reg16);
-		entry = reg16 & 0x1f;
+		entry = reg16 & PCI_EXP_DPC_IRQ;
 		if (entry >= nr_entries)
 			goto out_free_irqs;
 
diff --git a/include/uapi/linux/pci_regs.h b/include/uapi/linux/pci_regs.h
index f8d58045926f..f7c09a4c494a 100644
--- a/include/uapi/linux/pci_regs.h
+++ b/include/uapi/linux/pci_regs.h
@@ -746,6 +746,7 @@
 #define PCI_ERR_ROOT_FIRST_FATAL	0x00000010 /* First UNC is Fatal */
 #define PCI_ERR_ROOT_NONFATAL_RCV	0x00000020 /* Non-Fatal Received */
 #define PCI_ERR_ROOT_FATAL_RCV		0x00000040 /* Fatal Received */
+#define PCI_ERR_ROOT_AER_IRQ		0xf8000000 /* Advanced Error Interrupt Message Number */
 #define PCI_ERR_ROOT_ERR_SRC	52	/* Error Source Identification */
 
 /* Virtual Channel */
@@ -960,6 +961,7 @@
 
 /* Downstream Port Containment */
 #define PCI_EXP_DPC_CAP			4	/* DPC Capability */
+#define PCI_EXP_DPC_IRQ			0x1f	/* DPC Interrupt Message Number */
 #define  PCI_EXP_DPC_CAP_RP_EXT		0x20	/* Root Port Extensions for DPC */
 #define  PCI_EXP_DPC_CAP_POISONED_TLP	0x40	/* Poisoned TLP Egress Blocking Supported */
 #define  PCI_EXP_DPC_CAP_SW_TRIGGER	0x80	/* Software Triggering Supported */

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH v6 2/4] PCI/portdrv: Consolidate comments
  2017-10-20 16:21 [PATCH v6 0/4] PCI/portdrv: Fix MSI/MSI-X bug for PCIe port service drivers Bjorn Helgaas
  2017-10-20 16:22 ` [PATCH v6 1/4] PCI/portdrv: Add #defines for AER and DPC Interrupt Message Number masks Bjorn Helgaas
@ 2017-10-20 16:22 ` Bjorn Helgaas
  2017-10-20 16:22 ` [PATCH v6 3/4] PCI/portdrv: Factor out Interrupt Message Number lookup Bjorn Helgaas
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 8+ messages in thread
From: Bjorn Helgaas @ 2017-10-20 16:22 UTC (permalink / raw)
  To: Dongdong Liu
  Cc: Charles Chenxin, linuxarm, Christoph Hellwig, Gabriele Paoloni,
	linux-pci

From: Bjorn Helgaas <bhelgaas@google.com>

Consolidate some repetitive comments so we can see the code better.  No
functional change.

Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
---
 drivers/pci/pcie/portdrv_core.c |   63 ++++++---------------------------------
 1 file changed, 9 insertions(+), 54 deletions(-)

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, &reg16);
 		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, &reg32);
 		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, &reg16);
 		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)

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH v6 3/4] PCI/portdrv: Factor out Interrupt Message Number lookup
  2017-10-20 16:21 [PATCH v6 0/4] PCI/portdrv: Fix MSI/MSI-X bug for PCIe port service drivers Bjorn Helgaas
  2017-10-20 16:22 ` [PATCH v6 1/4] PCI/portdrv: Add #defines for AER and DPC Interrupt Message Number masks Bjorn Helgaas
  2017-10-20 16:22 ` [PATCH v6 2/4] PCI/portdrv: Consolidate comments Bjorn Helgaas
@ 2017-10-20 16:22 ` Bjorn Helgaas
  2017-10-21  8:09   ` Christoph Hellwig
  2017-10-20 16:22 ` [PATCH v6 4/4] PCI/portdrv: Compute MSI/MSI-X IRQ vectors after final allocation Bjorn Helgaas
  2017-10-21  2:22 ` [PATCH v6 0/4] PCI/portdrv: Fix MSI/MSI-X bug for PCIe port service drivers Dongdong Liu
  4 siblings, 1 reply; 8+ messages in thread
From: Bjorn Helgaas @ 2017-10-20 16:22 UTC (permalink / raw)
  To: Dongdong Liu
  Cc: Charles Chenxin, linuxarm, Christoph Hellwig, Gabriele Paoloni,
	linux-pci

From: Bjorn Helgaas <bhelgaas@google.com>

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 <bhelgaas@google.com>
---
 drivers/pci/pcie/portdrv_core.c |  110 ++++++++++++++++++++++-----------------
 1 file changed, 62 insertions(+), 48 deletions(-)

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, &reg16);
+		*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,
+					      &reg32);
+			*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,
+					     &reg16);
+			*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, &reg16);
-		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, &reg32);
-		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, &reg16);
-		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;
 }
 
 /**

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH v6 4/4] PCI/portdrv: Compute MSI/MSI-X IRQ vectors after final allocation
  2017-10-20 16:21 [PATCH v6 0/4] PCI/portdrv: Fix MSI/MSI-X bug for PCIe port service drivers Bjorn Helgaas
                   ` (2 preceding siblings ...)
  2017-10-20 16:22 ` [PATCH v6 3/4] PCI/portdrv: Factor out Interrupt Message Number lookup Bjorn Helgaas
@ 2017-10-20 16:22 ` Bjorn Helgaas
  2017-10-21  8:09   ` Christoph Hellwig
  2017-10-21  2:22 ` [PATCH v6 0/4] PCI/portdrv: Fix MSI/MSI-X bug for PCIe port service drivers Dongdong Liu
  4 siblings, 1 reply; 8+ messages in thread
From: Bjorn Helgaas @ 2017-10-20 16:22 UTC (permalink / raw)
  To: Dongdong Liu
  Cc: Charles Chenxin, linuxarm, Christoph Hellwig, Gabriele Paoloni,
	linux-pci

From: Bjorn Helgaas <bhelgaas@google.com>

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 <liudongdong3@huawei.com>
Tested-by: Dongdong Liu <liudongdong3@huawei.com>  # HiSilicon hip08
Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
---
 drivers/pci/pcie/portdrv_core.c |   30 ++++++++++++++++++++----------
 1 file changed, 20 insertions(+), 10 deletions(-)

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;
 }
 

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH v6 0/4] PCI/portdrv: Fix MSI/MSI-X bug for PCIe port service drivers
  2017-10-20 16:21 [PATCH v6 0/4] PCI/portdrv: Fix MSI/MSI-X bug for PCIe port service drivers Bjorn Helgaas
                   ` (3 preceding siblings ...)
  2017-10-20 16:22 ` [PATCH v6 4/4] PCI/portdrv: Compute MSI/MSI-X IRQ vectors after final allocation Bjorn Helgaas
@ 2017-10-21  2:22 ` Dongdong Liu
  4 siblings, 0 replies; 8+ messages in thread
From: Dongdong Liu @ 2017-10-21  2:22 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: Charles Chenxin, linuxarm, Christoph Hellwig, Gabriele Paoloni,
	linux-pci

Hi Bjorn.

The patch v6 looks good to me.
It works ok on HiSilicon hip08 platform.
Tested-by: Dongdong Liu <liudongdong3@huawei.com>

在 2017/10/21 0:21, Bjorn Helgaas 写道:
> This is based on Dongdong's series:
>   v3: http://lkml.kernel.org/r/1507553167-27833-1-git-send-email-liudongdong3@huawei.com
>   v4: http://lkml.kernel.org/r/1507719178-112556-1-git-send-email-liudongdong3@huawei.com
>   v5: http://lkml.kernel.org/r/20171019225726.17806.62380.stgit@bhelgaas-glaptop.roam.corp.google.com
>
> The main idea is to fix IRQ issues when removing and adding back a Root
> Port.  The problem is that when we allocate, free, and reallocate MSI/MSI-X
> vectors for the device, we may not get the same vectors the second time,
> but we saved IRQs based on the first set of vectors.
>
> Changes in v5:
>   - add back a general Interrupt Message Number comment
>   - factor out Interrupt Message Number helper separately
>
> This is on pci/msi for v4.15.
>
> ---
>
> Bjorn Helgaas (3):
>       PCI/portdrv: Consolidate comments
>       PCI/portdrv: Factor out Interrupt Message Number lookup
>       PCI/portdrv: Compute MSI/MSI-X IRQ vectors after final allocation
>
> Dongdong Liu (1):
>       PCI/portdrv: Add #defines for AER and DPC Interrupt Message Number masks
>
>
>  drivers/pci/pcie/portdrv_core.c |  171 +++++++++++++++++----------------------
>  include/uapi/linux/pci_regs.h   |    2
>  2 files changed, 77 insertions(+), 96 deletions(-)
>
> .
>

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v6 3/4] PCI/portdrv: Factor out Interrupt Message Number lookup
  2017-10-20 16:22 ` [PATCH v6 3/4] PCI/portdrv: Factor out Interrupt Message Number lookup Bjorn Helgaas
@ 2017-10-21  8:09   ` Christoph Hellwig
  0 siblings, 0 replies; 8+ messages in thread
From: Christoph Hellwig @ 2017-10-21  8:09 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: Dongdong Liu, Charles Chenxin, linuxarm, Christoph Hellwig,
	Gabriele Paoloni, linux-pci

Looks fine,

Reviewed-by: Christoph Hellwig <hch@lst.de>

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v6 4/4] PCI/portdrv: Compute MSI/MSI-X IRQ vectors after final allocation
  2017-10-20 16:22 ` [PATCH v6 4/4] PCI/portdrv: Compute MSI/MSI-X IRQ vectors after final allocation Bjorn Helgaas
@ 2017-10-21  8:09   ` Christoph Hellwig
  0 siblings, 0 replies; 8+ messages in thread
From: Christoph Hellwig @ 2017-10-21  8:09 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: Dongdong Liu, Charles Chenxin, linuxarm, Christoph Hellwig,
	Gabriele Paoloni, linux-pci

Looks fine,

Reviewed-by: Christoph Hellwig <hch@lst.de>

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2017-10-21  8:09 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-10-20 16:21 [PATCH v6 0/4] PCI/portdrv: Fix MSI/MSI-X bug for PCIe port service drivers Bjorn Helgaas
2017-10-20 16:22 ` [PATCH v6 1/4] PCI/portdrv: Add #defines for AER and DPC Interrupt Message Number masks Bjorn Helgaas
2017-10-20 16:22 ` [PATCH v6 2/4] PCI/portdrv: Consolidate comments Bjorn Helgaas
2017-10-20 16:22 ` [PATCH v6 3/4] PCI/portdrv: Factor out Interrupt Message Number lookup Bjorn Helgaas
2017-10-21  8:09   ` Christoph Hellwig
2017-10-20 16:22 ` [PATCH v6 4/4] PCI/portdrv: Compute MSI/MSI-X IRQ vectors after final allocation Bjorn Helgaas
2017-10-21  8:09   ` Christoph Hellwig
2017-10-21  2:22 ` [PATCH v6 0/4] PCI/portdrv: Fix MSI/MSI-X bug for PCIe port service drivers Dongdong Liu

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.