PCI: dwc: improve msi handling

PATCH In review Jisheng Zhang 7 participants 30 messages Sep 24, 2020 – Mar 11, 2021

Subsystems (19)

Arm Qualcomm mailing list Arm i.MX and Layerscape PCI Arm/Amlogic Meson SoC support +15 more subsystems

lore

Share & tools

lore

Jisheng Zhang

author

2020-09-24 11:09 UTC #0 lore raw

v2 cover PCI: dwc: improve msi handling

Improve the msi code:
1. Add proper error handling.
2. Move dw_pcie_msi_init() from each users to designware host to solve msi page leakage in resume path.

Since v1:
- add proper error handling patches.
- solve the msi page leakage by moving dw_pcie_msi_init() from each users to designware host

Jisheng Zhang (5):
PCI: dwc: Call dma_unmap_page() before freeing the msi page
PCI: dwc: Check alloc_page() return value
PCI: dwc: Rename dw_pcie_free_msi to dw_pcie_msi_deinit
PCI: dwc: Skip PCIE_MSI_INTR0* programming if MSI is disabled
PCI: dwc: Move dw_pcie_msi_init() from each users to designware host

 drivers/pci/controller/dwc/pci-dra7xx.c       |  1 +
 drivers/pci/controller/dwc/pci-exynos.c       |  2 -
 drivers/pci/controller/dwc/pci-imx6.c         |  3 --
 drivers/pci/controller/dwc/pci-meson.c        |  8 ----
 drivers/pci/controller/dwc/pcie-artpec6.c     | 10 -----
 .../pci/controller/dwc/pcie-designware-host.c | 43 +++++++++++++------
 .../pci/controller/dwc/pcie-designware-plat.c |  3 --
 drivers/pci/controller/dwc/pcie-designware.h  |  9 +++-
 drivers/pci/controller/dwc/pcie-histb.c       |  3 --
 drivers/pci/controller/dwc/pcie-kirin.c       |  3 --
 drivers/pci/controller/dwc/pcie-qcom.c        |  3 --
 drivers/pci/controller/dwc/pcie-spear13xx.c   |  1 -
 drivers/pci/controller/dwc/pcie-tegra194.c    |  2 -
 drivers/pci/controller/dwc/pcie-uniphier.c    |  9 +---
 14 files changed, 38 insertions(+), 62 deletions(-)

Signature · 1 line

2.28.0

Jisheng Zhang

author

2020-09-24 11:10 UTC #1 lore raw

PATCH v2 1/5 PCI: dwc: Call dma_unmap_page() before freeing the msi page

In dw_pcie_free_msi(), call dma_unmap_page() before freeing.

Signed-off-by Jisheng Zhang <[email protected]>

---
 drivers/pci/controller/dwc/pcie-designware-host.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

drivers/pci/controller/dwc/pcie-designware-host.c +5 -1

index 9dafecba347f..0a19de946351 100644
@@ -288,8 +288,12 @@ void dw_pcie_free_msi(struct pcie_port *pp)
 	irq_domain_remove(pp->msi_domain);
 	irq_domain_remove(pp->irq_domain);
 
-	if (pp->msi_page)
+	if (pp->msi_page) {
+		struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
+		struct device *dev = pci->dev;
+		dma_unmap_page(dev, pp->msi_data, PAGE_SIZE, DMA_FROM_DEVICE);
 		__free_page(pp->msi_page);
+	}
 }
 
 void dw_pcie_msi_init(struct pcie_port *pp)

Signature · 1 line

2.28.0

Gustavo Pimentel

Acked-by on v2 1/5

2020-09-24 11:48 UTC #2 lore raw

RE: [PATCH v2 1/5] PCI: dwc: Call dma_unmap_page() before freeing the msi page

Quoted 27 lines · Jisheng Zhang

In dw_pcie_free_msi(), call dma_unmap_page() before freeing.

Signed-off-by Jisheng Zhang <[email protected]>

---
 drivers/pci/controller/dwc/pcie-designware-host.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

drivers/pci/controller/dwc/pcie-designware-host.c +5 -1

index 9dafecba347f..0a19de946351 100644
@@ -288,8 +288,12 @@ void dw_pcie_free_msi(struct pcie_port *pp)
 	irq_domain_remove(pp->msi_domain);
 	irq_domain_remove(pp->irq_domain);
 
-	if (pp->msi_page)
+	if (pp->msi_page) {
+		struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
+		struct device *dev = pci->dev;
+		dma_unmap_page(dev, pp->msi_data, PAGE_SIZE, DMA_FROM_DEVICE);
 		__free_page(pp->msi_page);
+	}
 }
 
 void dw_pcie_msi_init(struct pcie_port *pp)

Signature · 1 line

2.28.0

Acked-by Gustavo Pimentel <[email protected]>

Jisheng Zhang

author

2020-09-24 11:10 UTC #3 lore raw

PATCH v2 2/5 PCI: dwc: Check alloc_page() return value

In reply to Jisheng Zhang

We need to check alloc_page() succeed or not before continuing.

Signed-off-by Jisheng Zhang <[email protected]>

---
 drivers/pci/controller/dwc/pcie-designware-host.c | 5 +++++
 1 file changed, 5 insertions(+)

drivers/pci/controller/dwc/pcie-designware-host.c +5 -0

index 0a19de946351..9e04e8ef3aa4 100644
@@ -303,6 +303,11 @@ void dw_pcie_msi_init(struct pcie_port *pp)
 	u64 msi_target;
 
 	pp->msi_page = alloc_page(GFP_KERNEL);
+	if (!pp->msi_page) {
+		dev_err(dev, "Failed to alloc MSI page\n");
+		return;
+	}
+
 	pp->msi_data = dma_map_page(dev, pp->msi_page, 0, PAGE_SIZE,
 				    DMA_FROM_DEVICE);
 	if (dma_mapping_error(dev, pp->msi_data)) {

Signature · 1 line

2.28.0

Gustavo Pimentel

Acked-by on v2 2/5

2020-09-24 11:47 UTC #4 lore raw

RE: [PATCH v2 2/5] PCI: dwc: Check alloc_page() return value

Quoted 25 lines · Jisheng Zhang

We need to check alloc_page() succeed or not before continuing.

Signed-off-by Jisheng Zhang <[email protected]>

---
 drivers/pci/controller/dwc/pcie-designware-host.c | 5 +++++
 1 file changed, 5 insertions(+)

drivers/pci/controller/dwc/pcie-designware-host.c +5 -0

index 0a19de946351..9e04e8ef3aa4 100644
@@ -303,6 +303,11 @@ void dw_pcie_msi_init(struct pcie_port *pp)
 	u64 msi_target;
 
 	pp->msi_page = alloc_page(GFP_KERNEL);
+	if (!pp->msi_page) {
+		dev_err(dev, "Failed to alloc MSI page\n");
+		return;
+	}
+
 	pp->msi_data = dma_map_page(dev, pp->msi_page, 0, PAGE_SIZE,
 				    DMA_FROM_DEVICE);
 	if (dma_mapping_error(dev, pp->msi_data)) {

Signature · 1 line

2.28.0

Acked-by Gustavo Pimentel <[email protected]>

Marc Zyngier

2020-09-29 17:30 UTC #5 lore raw

Re: [PATCH v2 2/5] PCI: dwc: Check alloc_page() return value

In reply to Jisheng Zhang

Quoted 13 lines · Jisheng Zhang

We need to check alloc_page() succeed or not before continuing.
Signed-off-by Jisheng Zhang <[email protected]>
---
 drivers/pci/controller/dwc/pcie-designware-host.c | 5 +++++
 1 file changed, 5 insertions(+)
drivers/pci/controller/dwc/pcie-designware-host.c +0 -0
index 0a19de946351..9e04e8ef3aa4 100644

drivers/pci/controller/dwc/pcie-designware-host.c void dw_pcie_msi_init(struct pcie_port *pp) · line 307

Quoted 5 lines

drivers/pci/controller/dwc/pcie-designware-host.c +2 -0

@@ -303,6 +303,11 @@ void dw_pcie_msi_init(struct pcie_port *pp)
 	u64 msi_target;
 
 	pp->msi_page = alloc_page(GFP_KERNEL);
+	if (!pp->msi_page) {
+		dev_err(dev, "Failed to alloc MSI page\n");

A failing allocation will already scream, so there is no need to add insult to injury.

More importantly, what is this MSI page ever used for? If I remember well, this is just a random address that never gets written to.

So why do we allocate a page here? Why do we bother with this DMA mapping?

M.

Signature · 1 line

Who you jivin' with that Cosmik Debris?

Jisheng Zhang

author

2020-09-30 01:23 UTC #6 lore raw

Re: [PATCH v2 2/5] PCI: dwc: Check alloc_page() return value

Hi Marc,

Quoted 25 lines · On Tue, 29 Sep 2020 18:29:52 +0100 Marc Zyngier wrote:

Quoted 13 lines · Jisheng Zhang
We need to check alloc_page() succeed or not before continuing.
Signed-off-by Jisheng Zhang <[email protected]>
---
 drivers/pci/controller/dwc/pcie-designware-host.c | 5 +++++
 1 file changed, 5 insertions(+)
drivers/pci/controller/dwc/pcie-designware-host.c +0 -0
index 0a19de946351..9e04e8ef3aa4 100644
drivers/pci/controller/dwc/pcie-designware-host.c void dw_pcie_msi_init(struct pcie_port *pp) · line 307
Quoted 5 lines
drivers/pci/controller/dwc/pcie-designware-host.c +2 -0
@@ -303,6 +303,11 @@ void dw_pcie_msi_init(struct pcie_port *pp)
      u64 msi_target;
 
      pp->msi_page = alloc_page(GFP_KERNEL);
+     if (!pp->msi_page) {
+             dev_err(dev, "Failed to alloc MSI page\n");  
A failing allocation will already scream, so there is no need to add insult to injury.

Quoted 6 lines

More importantly, what is this MSI page ever used for? If I remember well, this is just a random address that never gets written to.
So why do we allocate a page here? Why do we bother with this DMA mapping?

Ard and Rob also pointed out that there's no need to allocate a page, instead, we could use an address in the driver data for MSI address. So I refactored the patches and verified this solution works fine. Could you please review the V5?

Thanks

Jisheng Zhang

author

2020-09-24 11:09 UTC #7 lore raw

PATCH v2 3/5 PCI: dwc: Rename dw_pcie_free_msi to dw_pcie_msi_deinit

In reply to Jisheng Zhang

The dw_pcie_free_msi() does more things than freeing the msi page, for example, remove irq domain etc., rename it as dw_pcie_msi_deinit.

Signed-off-by Jisheng Zhang <[email protected]>

---
 drivers/pci/controller/dwc/pcie-designware-host.c | 6 +++---
 drivers/pci/controller/dwc/pcie-designware.h      | 4 ++--
 2 files changed, 5 insertions(+), 5 deletions(-)

drivers/pci/controller/dwc/pcie-designware-host.c +3 -3

index 9e04e8ef3aa4..d2de8bc5db91 100644
@@ -278,7 +278,7 @@ int dw_pcie_allocate_domains(struct pcie_port *pp)
 	return 0;
 }
 
-void dw_pcie_free_msi(struct pcie_port *pp)
+void dw_pcie_msi_deinit(struct pcie_port *pp)
 {
 	if (pp->msi_irq) {
 		irq_set_chained_handler(pp->msi_irq, NULL);

@@ -500,7 +500,7 @@ int dw_pcie_host_init(struct pcie_port *pp)
 
 err_free_msi:
 	if (pci_msi_enabled() && !pp->ops->msi_host_init)
-		dw_pcie_free_msi(pp);
+		dw_pcie_msi_deinit(pp);
 	return ret;
 }
 EXPORT_SYMBOL_GPL(dw_pcie_host_init);

@@ -510,7 +510,7 @@ void dw_pcie_host_deinit(struct pcie_port *pp)
 	pci_stop_root_bus(pp->root_bus);
 	pci_remove_root_bus(pp->root_bus);
 	if (pci_msi_enabled() && !pp->ops->msi_host_init)
-		dw_pcie_free_msi(pp);
+		dw_pcie_msi_deinit(pp);
 }
 EXPORT_SYMBOL_GPL(dw_pcie_host_deinit);

drivers/pci/controller/dwc/pcie-designware.h +2 -2

index f911760dcc69..43b8061e1bec 100644
@@ -371,7 +371,7 @@ static inline void dw_pcie_dbi_ro_wr_dis(struct dw_pcie *pci)
 #ifdef CONFIG_PCIE_DW_HOST
 irqreturn_t dw_handle_msi_irq(struct pcie_port *pp);
 void dw_pcie_msi_init(struct pcie_port *pp);
-void dw_pcie_free_msi(struct pcie_port *pp);
+void dw_pcie_msi_deinit(struct pcie_port *pp);
 void dw_pcie_setup_rc(struct pcie_port *pp);
 int dw_pcie_host_init(struct pcie_port *pp);
 void dw_pcie_host_deinit(struct pcie_port *pp);

@@ -386,7 +386,7 @@ static inline void dw_pcie_msi_init(struct pcie_port *pp)
 {
 }
 
-static inline void dw_pcie_free_msi(struct pcie_port *pp)
+static inline void dw_pcie_msi_deinit(struct pcie_port *pp)
 {
 }

Signature · 1 line

2.28.0

Gustavo Pimentel

Acked-by on v2 3/5

2020-09-24 11:49 UTC #8 lore raw

RE: [PATCH v2 3/5] PCI: dwc: Rename dw_pcie_free_msi to dw_pcie_msi_deinit

Quoted 64 lines · Jisheng Zhang

The dw_pcie_free_msi() does more things than freeing the msi page, for example, remove irq domain etc., rename it as dw_pcie_msi_deinit.

Signed-off-by Jisheng Zhang <[email protected]>

---
 drivers/pci/controller/dwc/pcie-designware-host.c | 6 +++---
 drivers/pci/controller/dwc/pcie-designware.h      | 4 ++--
 2 files changed, 5 insertions(+), 5 deletions(-)

drivers/pci/controller/dwc/pcie-designware-host.c +3 -3

index 9e04e8ef3aa4..d2de8bc5db91 100644
@@ -278,7 +278,7 @@ int dw_pcie_allocate_domains(struct pcie_port *pp)
 	return 0;
 }
 
-void dw_pcie_free_msi(struct pcie_port *pp)
+void dw_pcie_msi_deinit(struct pcie_port *pp)
 {
 	if (pp->msi_irq) {
 		irq_set_chained_handler(pp->msi_irq, NULL);

@@ -500,7 +500,7 @@ int dw_pcie_host_init(struct pcie_port *pp)
 
 err_free_msi:
 	if (pci_msi_enabled() && !pp->ops->msi_host_init)
-		dw_pcie_free_msi(pp);
+		dw_pcie_msi_deinit(pp);
 	return ret;
 }
 EXPORT_SYMBOL_GPL(dw_pcie_host_init);

@@ -510,7 +510,7 @@ void dw_pcie_host_deinit(struct pcie_port *pp)
 	pci_stop_root_bus(pp->root_bus);
 	pci_remove_root_bus(pp->root_bus);
 	if (pci_msi_enabled() && !pp->ops->msi_host_init)
-		dw_pcie_free_msi(pp);
+		dw_pcie_msi_deinit(pp);
 }
 EXPORT_SYMBOL_GPL(dw_pcie_host_deinit);

drivers/pci/controller/dwc/pcie-designware.h +2 -2

index f911760dcc69..43b8061e1bec 100644
@@ -371,7 +371,7 @@ static inline void dw_pcie_dbi_ro_wr_dis(struct dw_pcie *pci)
 #ifdef CONFIG_PCIE_DW_HOST
 irqreturn_t dw_handle_msi_irq(struct pcie_port *pp);
 void dw_pcie_msi_init(struct pcie_port *pp);
-void dw_pcie_free_msi(struct pcie_port *pp);
+void dw_pcie_msi_deinit(struct pcie_port *pp);
 void dw_pcie_setup_rc(struct pcie_port *pp);
 int dw_pcie_host_init(struct pcie_port *pp);
 void dw_pcie_host_deinit(struct pcie_port *pp);

@@ -386,7 +386,7 @@ static inline void dw_pcie_msi_init(struct pcie_port *pp)
 {
 }
 
-static inline void dw_pcie_free_msi(struct pcie_port *pp)
+static inline void dw_pcie_msi_deinit(struct pcie_port *pp)
 {
 }

Signature · 1 line

2.28.0

Acked-by Gustavo Pimentel <[email protected]>

Jisheng Zhang

author

2020-09-24 11:11 UTC #9 lore raw

PATCH v2 4/5 PCI: dwc: Skip PCIE_MSI_INTR0* programming if MSI is disabled

In reply to Jisheng Zhang

If MSI is disabled, there's no need to program PCIE_MSI_INTR0_MASK and PCIE_MSI_INTR0_ENABLE registers.

Signed-off-by Jisheng Zhang <[email protected]>

---
 drivers/pci/controller/dwc/pcie-designware-host.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

drivers/pci/controller/dwc/pcie-designware-host.c +1 -1

index d2de8bc5db91..7a8adf597803 100644
@@ -641,7 +641,7 @@ void dw_pcie_setup_rc(struct pcie_port *pp)
 
 	dw_pcie_setup(pci);
 
-	if (!pp->ops->msi_host_init) {
+	if (pci_msi_enabled() && !pp->ops->msi_host_init) {
 		num_ctrls = pp->num_vectors / MAX_MSI_IRQS_PER_CTRL;
 
 		/* Initialize IRQ Status array */

Signature · 1 line

2.28.0

Gustavo Pimentel

Acked-by on v2 4/5

2020-09-24 11:49 UTC #10 lore raw

RE: [PATCH v2 4/5] PCI: dwc: Skip PCIE_MSI_INTR0* programming if MSI is disabled

Quoted 23 lines · Jisheng Zhang

If MSI is disabled, there's no need to program PCIE_MSI_INTR0_MASK and PCIE_MSI_INTR0_ENABLE registers.
Signed-off-by Jisheng Zhang <[email protected]>
---
 drivers/pci/controller/dwc/pcie-designware-host.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/pci/controller/dwc/pcie-designware-host.c +1 -1
index d2de8bc5db91..7a8adf597803 100644
@@ -641,7 +641,7 @@ void dw_pcie_setup_rc(struct pcie_port *pp)
 
 	dw_pcie_setup(pci);
 
-	if (!pp->ops->msi_host_init) {
+	if (pci_msi_enabled() && !pp->ops->msi_host_init) {
 		num_ctrls = pp->num_vectors / MAX_MSI_IRQS_PER_CTRL;
 
 		/* Initialize IRQ Status array */
Signature · 1 line
2.28.0

Acked-by Gustavo Pimentel <[email protected]>

Jisheng Zhang

author

2020-09-24 11:09 UTC #11 lore raw

PATCH v2 5/5 PCI: dwc: Move dw_pcie_msi_init() from each users to designware host

In reply to Jisheng Zhang

Currently, dw_pcie_msi_init() allocates and maps page for msi, then program the PCIE_MSI_ADDR_LO and PCIE_MSI_ADDR_HI. The Root Complex may lose power during suspend-to-RAM, so when we resume, we want to redo the latter but not the former. If designware based driver (for example, pcie-tegra194.c) calls dw_pcie_msi_init() in resume path, the previous msi page will be leaked. From another side, except pci-dra7xx.c we can move the dw_pcie_msi_init() from each users to designware host, I.E move the msi page allocation and mapping to dw_pcie_host_init() and move the PCIE_MSI_ADDR_* programming to dw_pcie_setup_rc(). After this moving, we solve the msi page leakage as well.

Signed-off-by Jisheng Zhang <[email protected]>

---
 drivers/pci/controller/dwc/pci-dra7xx.c       |  1 +
 drivers/pci/controller/dwc/pci-exynos.c       |  2 --
 drivers/pci/controller/dwc/pci-imx6.c         |  3 ---
 drivers/pci/controller/dwc/pci-meson.c        |  8 -------
 drivers/pci/controller/dwc/pcie-artpec6.c     | 10 --------
 .../pci/controller/dwc/pcie-designware-host.c | 24 ++++++++++++-------
 .../pci/controller/dwc/pcie-designware-plat.c |  3 ---
 drivers/pci/controller/dwc/pcie-designware.h  |  5 ++++
 drivers/pci/controller/dwc/pcie-histb.c       |  3 ---
 drivers/pci/controller/dwc/pcie-kirin.c       |  3 ---
 drivers/pci/controller/dwc/pcie-qcom.c        |  3 ---
 drivers/pci/controller/dwc/pcie-spear13xx.c   |  1 -
 drivers/pci/controller/dwc/pcie-tegra194.c    |  2 --
 drivers/pci/controller/dwc/pcie-uniphier.c    |  9 +------
 14 files changed, 22 insertions(+), 55 deletions(-)

drivers/pci/controller/dwc/pci-dra7xx.c +1 -0

index dc387724cf08..d8b74389e353 100644
@@ -210,6 +210,7 @@ static int dra7xx_pcie_host_init(struct pcie_port *pp)
 	dra7xx_pcie_establish_link(pci);
 	dw_pcie_wait_for_link(pci);
 	dw_pcie_msi_init(pp);
+	dw_pcie_msi_config(pp);
 	dra7xx_pcie_enable_interrupts(dra7xx);
 
 	return 0;

drivers/pci/controller/dwc/pci-exynos.c +0 -2

index 8d82c43ae299..9cca0ce79777 100644
@@ -298,8 +298,6 @@ static void exynos_pcie_msi_init(struct exynos_pcie *ep)
 	struct pcie_port *pp = &pci->pp;
 	u32 val;
 
-	dw_pcie_msi_init(pp);
-
 	/* enable MSI interrupt */
 	val = exynos_pcie_readl(ep->mem_res->elbi_base, PCIE_IRQ_EN_LEVEL);
 	val |= IRQ_MSI_ENABLE;

drivers/pci/controller/dwc/pci-imx6.c +0 -3

index 5fef2613b223..dba6e351e3df 100644
@@ -848,9 +848,6 @@ static int imx6_pcie_host_init(struct pcie_port *pp)
 	dw_pcie_setup_rc(pp);
 	imx6_pcie_establish_link(imx6_pcie);
 
-	if (IS_ENABLED(CONFIG_PCI_MSI))
-		dw_pcie_msi_init(pp);
-
 	return 0;
 }

drivers/pci/controller/dwc/pci-meson.c +0 -8

index 4f183b96afbb..cd0d9dd8dd61 100644
@@ -377,12 +377,6 @@ static int meson_pcie_establish_link(struct meson_pcie *mp)
 	return dw_pcie_wait_for_link(pci);
 }
 
-static void meson_pcie_enable_interrupts(struct meson_pcie *mp)
-{
-	if (IS_ENABLED(CONFIG_PCI_MSI))
-		dw_pcie_msi_init(&mp->pci.pp);
-}
-
 static int meson_pcie_rd_own_conf(struct pcie_port *pp, int where, int size,
 				  u32 *val)
 {

@@ -467,8 +461,6 @@ static int meson_pcie_host_init(struct pcie_port *pp)
 	if (ret)
 		return ret;
 
-	meson_pcie_enable_interrupts(mp);
-
 	return 0;
 }

drivers/pci/controller/dwc/pcie-artpec6.c +0 ~~-10~~

index 97d50bb50f06..af1e6bb28e7a 100644
@@ -346,15 +346,6 @@ static void artpec6_pcie_deassert_core_reset(struct artpec6_pcie *artpec6_pcie)
 	usleep_range(100, 200);
 }
 
-static void artpec6_pcie_enable_interrupts(struct artpec6_pcie *artpec6_pcie)
-{
-	struct dw_pcie *pci = artpec6_pcie->pci;
-	struct pcie_port *pp = &pci->pp;
-
-	if (IS_ENABLED(CONFIG_PCI_MSI))
-		dw_pcie_msi_init(pp);
-}
-
 static int artpec6_pcie_host_init(struct pcie_port *pp)
 {
 	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);

@@ -368,7 +359,6 @@ static int artpec6_pcie_host_init(struct pcie_port *pp)
 	dw_pcie_setup_rc(pp);
 	artpec6_pcie_establish_link(pci);
 	dw_pcie_wait_for_link(pci);
-	artpec6_pcie_enable_interrupts(artpec6_pcie);
 
 	return 0;
 }

drivers/pci/controller/dwc/pcie-designware-host.c +15 -9

index 7a8adf597803..6603d7c36f2e 100644
@@ -296,11 +296,23 @@ void dw_pcie_msi_deinit(struct pcie_port *pp)
 	}
 }
 
+void dw_pcie_msi_config(struct pcie_port *pp)
+{
+	if (pp->msi_page) {
+		u64 msi_target = (u64)pp->msi_data;
+
+		/* Program the msi_data */
+		dw_pcie_wr_own_conf(pp, PCIE_MSI_ADDR_LO, 4,
+				    lower_32_bits(msi_target));
+		dw_pcie_wr_own_conf(pp, PCIE_MSI_ADDR_HI, 4,
+				    upper_32_bits(msi_target));
+	}
+}
+
 void dw_pcie_msi_init(struct pcie_port *pp)
 {
 	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
 	struct device *dev = pci->dev;
-	u64 msi_target;
 
 	pp->msi_page = alloc_page(GFP_KERNEL);
 	if (!pp->msi_page) {

@@ -314,15 +326,7 @@ void dw_pcie_msi_init(struct pcie_port *pp)
 		dev_err(dev, "Failed to map MSI data\n");
 		__free_page(pp->msi_page);
 		pp->msi_page = NULL;
-		return;
 	}
-	msi_target = (u64)pp->msi_data;
-
-	/* Program the msi_data */
-	dw_pcie_wr_own_conf(pp, PCIE_MSI_ADDR_LO, 4,
-			    lower_32_bits(msi_target));
-	dw_pcie_wr_own_conf(pp, PCIE_MSI_ADDR_HI, 4,
-			    upper_32_bits(msi_target));
 }
 EXPORT_SYMBOL_GPL(dw_pcie_msi_init);

@@ -449,6 +453,7 @@ int dw_pcie_host_init(struct pcie_port *pp)
 				irq_set_chained_handler_and_data(pp->msi_irq,
 							    dw_chained_msi_isr,
 							    pp);
+			dw_pcie_msi_init(pp);
 		} else {
 			ret = pp->ops->msi_host_init(pp);
 			if (ret < 0)

@@ -654,6 +659,7 @@ void dw_pcie_setup_rc(struct pcie_port *pp)
 					    (ctrl * MSI_REG_CTRL_BLOCK_SIZE),
 					    4, ~0);
 		}
+		dw_pcie_msi_config(pp);
 	}
 
 	/* Setup RC BARs */

drivers/pci/controller/dwc/pcie-designware-plat.c +0 -3

index 712456f6ce36..9ccf69a3dcf4 100644
@@ -40,9 +40,6 @@ static int dw_plat_pcie_host_init(struct pcie_port *pp)
 	dw_pcie_setup_rc(pp);
 	dw_pcie_wait_for_link(pci);
 
-	if (IS_ENABLED(CONFIG_PCI_MSI))
-		dw_pcie_msi_init(pp);
-
 	return 0;
 }

drivers/pci/controller/dwc/pcie-designware.h +5 -0

index 43b8061e1bec..40d22fe33afe 100644
@@ -372,6 +372,7 @@ static inline void dw_pcie_dbi_ro_wr_dis(struct dw_pcie *pci)
 irqreturn_t dw_handle_msi_irq(struct pcie_port *pp);
 void dw_pcie_msi_init(struct pcie_port *pp);
 void dw_pcie_msi_deinit(struct pcie_port *pp);
+void dw_pcie_msi_config(struct pcie_port *pp);
 void dw_pcie_setup_rc(struct pcie_port *pp);
 int dw_pcie_host_init(struct pcie_port *pp);
 void dw_pcie_host_deinit(struct pcie_port *pp);

@@ -390,6 +391,10 @@ static inline void dw_pcie_msi_deinit(struct pcie_port *pp)
 {
 }
 
+static inline void dw_pcie_msi_config(struct pcie_port *pp)
+{
+}
+
 static inline void dw_pcie_setup_rc(struct pcie_port *pp)
 {
 }

drivers/pci/controller/dwc/pcie-histb.c +0 -3

index 2a2835746077..fbf53e897ca7 100644
@@ -196,9 +196,6 @@ static int histb_pcie_host_init(struct pcie_port *pp)
 {
 	histb_pcie_establish_link(pp);
 
-	if (IS_ENABLED(CONFIG_PCI_MSI))
-		dw_pcie_msi_init(pp);
-
 	return 0;
 }

drivers/pci/controller/dwc/pcie-kirin.c +0 -3

index e496f51e0152..d7246995daf0 100644
@@ -425,9 +425,6 @@ static int kirin_pcie_host_init(struct pcie_port *pp)
 {
 	kirin_pcie_establish_link(pp);
 
-	if (IS_ENABLED(CONFIG_PCI_MSI))
-		dw_pcie_msi_init(pp);
-
 	return 0;
 }

drivers/pci/controller/dwc/pcie-qcom.c +0 -3

index 3aac77a295ba..2abbb850fb56 100644
@@ -1281,9 +1281,6 @@ static int qcom_pcie_host_init(struct pcie_port *pp)
 
 	dw_pcie_setup_rc(pp);
 
-	if (IS_ENABLED(CONFIG_PCI_MSI))
-		dw_pcie_msi_init(pp);
-
 	qcom_ep_reset_deassert(pcie);
 
 	ret = qcom_pcie_establish_link(pcie);

drivers/pci/controller/dwc/pcie-spear13xx.c +0 -1

index 62846562da0b..760e27de0082 100644
@@ -157,7 +157,6 @@ static void spear13xx_pcie_enable_interrupts(struct spear13xx_pcie *spear13xx_pc
 
 	/* Enable MSI interrupt */
 	if (IS_ENABLED(CONFIG_PCI_MSI)) {
-		dw_pcie_msi_init(pp);
 		writel(readl(&app_reg->int_mask) |
 				MSI_CTRL_INT, &app_reg->int_mask);
 	}

drivers/pci/controller/dwc/pcie-tegra194.c +0 -2

index 70498689d0c0..b51fe136d345 100644
@@ -783,8 +783,6 @@ static void tegra_pcie_enable_msi_interrupts(struct pcie_port *pp)
 	struct tegra_pcie_dw *pcie = to_tegra_pcie(pci);
 	u32 val;
 
-	dw_pcie_msi_init(pp);
-
 	/* Enable MSI interrupt generation */
 	val = appl_readl(pcie, APPL_INTR_EN_L0_0);
 	val |= APPL_INTR_EN_L0_0_SYS_MSI_INTR_EN;

drivers/pci/controller/dwc/pcie-uniphier.c +1 -8

index 3a7f403b57b8..d7b465b669f4 100644
@@ -318,14 +318,7 @@ static int uniphier_pcie_host_init(struct pcie_port *pp)
 	uniphier_pcie_irq_enable(priv);
 
 	dw_pcie_setup_rc(pp);
-	ret = uniphier_pcie_establish_link(pci);
-	if (ret)
-		return ret;
-
-	if (IS_ENABLED(CONFIG_PCI_MSI))
-		dw_pcie_msi_init(pp);
-
-	return 0;
+	return uniphier_pcie_establish_link(pci);
 }
 
 static const struct dw_pcie_host_ops uniphier_pcie_host_ops = {

Signature · 1 line

2.28.0

Vidya Sagar

2020-10-08 05:44 UTC #12 lore raw

Re: [PATCH v2 5/5] PCI: dwc: Move dw_pcie_msi_init() from each users to designware host

Quoted 144 lines · Jisheng Zhang

External email: Use caution opening links or attachments

Signed-off-by Jisheng Zhang <[email protected]>

---
  drivers/pci/controller/dwc/pci-dra7xx.c       |  1 +
  drivers/pci/controller/dwc/pci-exynos.c       |  2 --
  drivers/pci/controller/dwc/pci-imx6.c         |  3 ---
  drivers/pci/controller/dwc/pci-meson.c        |  8 -------
  drivers/pci/controller/dwc/pcie-artpec6.c     | 10 --------
  .../pci/controller/dwc/pcie-designware-host.c | 24 ++++++++++++-------
  .../pci/controller/dwc/pcie-designware-plat.c |  3 ---
  drivers/pci/controller/dwc/pcie-designware.h  |  5 ++++
  drivers/pci/controller/dwc/pcie-histb.c       |  3 ---
  drivers/pci/controller/dwc/pcie-kirin.c       |  3 ---
  drivers/pci/controller/dwc/pcie-qcom.c        |  3 ---
  drivers/pci/controller/dwc/pcie-spear13xx.c   |  1 -
  drivers/pci/controller/dwc/pcie-tegra194.c    |  2 --
  drivers/pci/controller/dwc/pcie-uniphier.c    |  9 +------
  14 files changed, 22 insertions(+), 55 deletions(-)

drivers/pci/controller/dwc/pci-dra7xx.c +1 -0

index dc387724cf08..d8b74389e353 100644
@@ -210,6 +210,7 @@ static int dra7xx_pcie_host_init(struct pcie_port *pp)
         dra7xx_pcie_establish_link(pci);
         dw_pcie_wait_for_link(pci);
         dw_pcie_msi_init(pp);
+       dw_pcie_msi_config(pp);
         dra7xx_pcie_enable_interrupts(dra7xx);
 
         return 0;

drivers/pci/controller/dwc/pci-exynos.c +0 -2

index 8d82c43ae299..9cca0ce79777 100644
@@ -298,8 +298,6 @@ static void exynos_pcie_msi_init(struct exynos_pcie *ep)
         struct pcie_port *pp = &pci->pp;
         u32 val;
 
-       dw_pcie_msi_init(pp);
-
         /* enable MSI interrupt */
         val = exynos_pcie_readl(ep->mem_res->elbi_base, PCIE_IRQ_EN_LEVEL);
         val |= IRQ_MSI_ENABLE;

drivers/pci/controller/dwc/pci-imx6.c +0 -3

index 5fef2613b223..dba6e351e3df 100644
@@ -848,9 +848,6 @@ static int imx6_pcie_host_init(struct pcie_port *pp)
         dw_pcie_setup_rc(pp);
         imx6_pcie_establish_link(imx6_pcie);
 
-       if (IS_ENABLED(CONFIG_PCI_MSI))
-               dw_pcie_msi_init(pp);
-
         return 0;
  }

drivers/pci/controller/dwc/pci-meson.c +0 -8

index 4f183b96afbb..cd0d9dd8dd61 100644
@@ -377,12 +377,6 @@ static int meson_pcie_establish_link(struct meson_pcie *mp)
         return dw_pcie_wait_for_link(pci);
  }
 
-static void meson_pcie_enable_interrupts(struct meson_pcie *mp)
-{
-       if (IS_ENABLED(CONFIG_PCI_MSI))
-               dw_pcie_msi_init(&mp->pci.pp);
-}
-
  static int meson_pcie_rd_own_conf(struct pcie_port *pp, int where, int size,
                                   u32 *val)
  {

@@ -467,8 +461,6 @@ static int meson_pcie_host_init(struct pcie_port *pp)
         if (ret)
                 return ret;
 
-       meson_pcie_enable_interrupts(mp);
-
         return 0;
  }

drivers/pci/controller/dwc/pcie-artpec6.c +0 ~~-10~~

index 97d50bb50f06..af1e6bb28e7a 100644
@@ -346,15 +346,6 @@ static void artpec6_pcie_deassert_core_reset(struct artpec6_pcie *artpec6_pcie)
         usleep_range(100, 200);
  }
 
-static void artpec6_pcie_enable_interrupts(struct artpec6_pcie *artpec6_pcie)
-{
-       struct dw_pcie *pci = artpec6_pcie->pci;
-       struct pcie_port *pp = &pci->pp;
-
-       if (IS_ENABLED(CONFIG_PCI_MSI))
-               dw_pcie_msi_init(pp);
-}
-
  static int artpec6_pcie_host_init(struct pcie_port *pp)
  {
         struct dw_pcie *pci = to_dw_pcie_from_pp(pp);

@@ -368,7 +359,6 @@ static int artpec6_pcie_host_init(struct pcie_port *pp)
         dw_pcie_setup_rc(pp);
         artpec6_pcie_establish_link(pci);
         dw_pcie_wait_for_link(pci);
-       artpec6_pcie_enable_interrupts(artpec6_pcie);
 
         return 0;
  }

drivers/pci/controller/dwc/pcie-designware-host.c +10 -0

index 7a8adf597803..6603d7c36f2e 100644
@@ -296,11 +296,23 @@ void dw_pcie_msi_deinit(struct pcie_port *pp)
         }
  }
 
+void dw_pcie_msi_config(struct pcie_port *pp)
+{
+       if (pp->msi_page) {
+               u64 msi_target = (u64)pp->msi_data;
+
+               /* Program the msi_data */
+               dw_pcie_wr_own_conf(pp, PCIE_MSI_ADDR_LO, 4,
+                                   lower_32_bits(msi_target));
+               dw_pcie_wr_own_conf(pp, PCIE_MSI_ADDR_HI, 4,
+                                   upper_32_bits(msi_target));

drivers/pci/controller/dwc/pcie-designware-host.c void dw_pcie_msi_deinit(struct pcie_port *pp) · line 317

Quoted 10 lines

drivers/pci/controller/dwc/pcie-designware-host.c +3 -1

+       }
+}
+
  void dw_pcie_msi_init(struct pcie_port *pp)
  {
         struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
         struct device *dev = pci->dev;
-       u64 msi_target;
 
         pp->msi_page = alloc_page(GFP_KERNEL);

I'm not sure if it can be addressed in the same patch, but I think it is required to call dma_set_coherent_mask() with 32-bit mask before calling dma_map_single() as there are endpoint devices which are only 32-bit MSI capable and it is required to restrict the MSI target address to always be in 32-bit domain so that both kinds of endpoints (only 32-bit capable and 64-bit capable) can work with this 32-bit MSI target.

Quoted 160 lines

drivers/pci/controller/dwc/pcie-designware-host.c +2 -8

         if (!pp->msi_page) {

@@ -314,15 +326,7 @@ void dw_pcie_msi_init(struct pcie_port *pp)
                 dev_err(dev, "Failed to map MSI data\n");
                 __free_page(pp->msi_page);
                 pp->msi_page = NULL;
-               return;
         }
-       msi_target = (u64)pp->msi_data;
-
-       /* Program the msi_data */
-       dw_pcie_wr_own_conf(pp, PCIE_MSI_ADDR_LO, 4,
-                           lower_32_bits(msi_target));
-       dw_pcie_wr_own_conf(pp, PCIE_MSI_ADDR_HI, 4,
-                           upper_32_bits(msi_target));
  }
  EXPORT_SYMBOL_GPL(dw_pcie_msi_init);

@@ -449,6 +453,7 @@ int dw_pcie_host_init(struct pcie_port *pp)
                                 irq_set_chained_handler_and_data(pp->msi_irq,
                                                             dw_chained_msi_isr,
                                                             pp);
+                       dw_pcie_msi_init(pp);
                 } else {
                         ret = pp->ops->msi_host_init(pp);
                         if (ret < 0)

@@ -654,6 +659,7 @@ void dw_pcie_setup_rc(struct pcie_port *pp)
                                             (ctrl * MSI_REG_CTRL_BLOCK_SIZE),
                                             4, ~0);
                 }
+               dw_pcie_msi_config(pp);
         }
 
         /* Setup RC BARs */

drivers/pci/controller/dwc/pcie-designware-plat.c +0 -3

index 712456f6ce36..9ccf69a3dcf4 100644
@@ -40,9 +40,6 @@ static int dw_plat_pcie_host_init(struct pcie_port *pp)
         dw_pcie_setup_rc(pp);
         dw_pcie_wait_for_link(pci);
 
-       if (IS_ENABLED(CONFIG_PCI_MSI))
-               dw_pcie_msi_init(pp);
-
         return 0;
  }

drivers/pci/controller/dwc/pcie-designware.h +5 -0

index 43b8061e1bec..40d22fe33afe 100644
@@ -372,6 +372,7 @@ static inline void dw_pcie_dbi_ro_wr_dis(struct dw_pcie *pci)
  irqreturn_t dw_handle_msi_irq(struct pcie_port *pp);
  void dw_pcie_msi_init(struct pcie_port *pp);
  void dw_pcie_msi_deinit(struct pcie_port *pp);
+void dw_pcie_msi_config(struct pcie_port *pp);
  void dw_pcie_setup_rc(struct pcie_port *pp);
  int dw_pcie_host_init(struct pcie_port *pp);
  void dw_pcie_host_deinit(struct pcie_port *pp);

@@ -390,6 +391,10 @@ static inline void dw_pcie_msi_deinit(struct pcie_port *pp)
  {
  }
 
+static inline void dw_pcie_msi_config(struct pcie_port *pp)
+{
+}
+
  static inline void dw_pcie_setup_rc(struct pcie_port *pp)
  {
  }

drivers/pci/controller/dwc/pcie-histb.c +0 -3

index 2a2835746077..fbf53e897ca7 100644
@@ -196,9 +196,6 @@ static int histb_pcie_host_init(struct pcie_port *pp)
  {
         histb_pcie_establish_link(pp);
 
-       if (IS_ENABLED(CONFIG_PCI_MSI))
-               dw_pcie_msi_init(pp);
-
         return 0;
  }

drivers/pci/controller/dwc/pcie-kirin.c +0 -3

index e496f51e0152..d7246995daf0 100644
@@ -425,9 +425,6 @@ static int kirin_pcie_host_init(struct pcie_port *pp)
  {
         kirin_pcie_establish_link(pp);
 
-       if (IS_ENABLED(CONFIG_PCI_MSI))
-               dw_pcie_msi_init(pp);
-
         return 0;
  }

drivers/pci/controller/dwc/pcie-qcom.c +0 -3

index 3aac77a295ba..2abbb850fb56 100644
@@ -1281,9 +1281,6 @@ static int qcom_pcie_host_init(struct pcie_port *pp)
 
         dw_pcie_setup_rc(pp);
 
-       if (IS_ENABLED(CONFIG_PCI_MSI))
-               dw_pcie_msi_init(pp);
-
         qcom_ep_reset_deassert(pcie);
 
         ret = qcom_pcie_establish_link(pcie);

drivers/pci/controller/dwc/pcie-spear13xx.c +0 -1

index 62846562da0b..760e27de0082 100644
@@ -157,7 +157,6 @@ static void spear13xx_pcie_enable_interrupts(struct spear13xx_pcie *spear13xx_pc
 
         /* Enable MSI interrupt */
         if (IS_ENABLED(CONFIG_PCI_MSI)) {
-               dw_pcie_msi_init(pp);
                 writel(readl(&app_reg->int_mask) |
                                 MSI_CTRL_INT, &app_reg->int_mask);
         }

drivers/pci/controller/dwc/pcie-tegra194.c +0 -2

index 70498689d0c0..b51fe136d345 100644
@@ -783,8 +783,6 @@ static void tegra_pcie_enable_msi_interrupts(struct pcie_port *pp)
         struct tegra_pcie_dw *pcie = to_tegra_pcie(pci);
         u32 val;
 
-       dw_pcie_msi_init(pp);
-
         /* Enable MSI interrupt generation */
         val = appl_readl(pcie, APPL_INTR_EN_L0_0);
         val |= APPL_INTR_EN_L0_0_SYS_MSI_INTR_EN;

drivers/pci/controller/dwc/pcie-uniphier.c +1 -9

index 3a7f403b57b8..d7b465b669f4 100644
@@ -318,14 +318,7 @@ static int uniphier_pcie_host_init(struct pcie_port *pp)
         uniphier_pcie_irq_enable(priv);
 
         dw_pcie_setup_rc(pp);
-       ret = uniphier_pcie_establish_link(pci);
-       if (ret)
-               return ret;
-
-       if (IS_ENABLED(CONFIG_PCI_MSI))
-               dw_pcie_msi_init(pp);
-
-       return 0;
+       return uniphier_pcie_establish_link(pci);
  }
 
  static const struct dw_pcie_host_ops uniphier_pcie_host_ops = {
--

2.28.0

Krzysztof Wilczyński

2021-03-07 22:10 UTC #13 lore raw

Re: [PATCH v2 5/5] PCI: dwc: Move dw_pcie_msi_init() from each users to designware host

In reply to Jisheng Zhang

Hi,

Quoted 11 lines

Currently, dw_pcie_msi_init() allocates and maps page for msi, then program the PCIE_MSI_ADDR_LO and PCIE_MSI_ADDR_HI. The Root Complex may lose power during suspend-to-RAM, so when we resume, we want to redo the latter but not the former. If designware based driver (for example, pcie-tegra194.c) calls dw_pcie_msi_init() in resume path, the previous msi page will be leaked. From another side, except pci-dra7xx.c we can move the dw_pcie_msi_init() from each users to designware host, I.E move the msi page allocation and mapping to dw_pcie_host_init() and move the PCIE_MSI_ADDR_* programming to dw_pcie_setup_rc(). After this moving, we solve the msi page leakage as well.

[...]

A small nitpick. All the "designware" should be "DesignWare" both in the commit message and the subject. Similarly, "msi" would be "MSI", and "I.E" would become "i.e.,". If you ever sent another version of the patch, that is.

See the following for reference:
https://lore.kernel.org/linux-pci/[email protected]/

Krzysztof

_______________________________________________
linux-amlogic mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-amlogic

Jisheng Zhang

author

2021-03-11 06:51 UTC #14 lore raw

Re: [PATCH v2 5/5] PCI: dwc: Move dw_pcie_msi_init() from each users to designware host

Quoted 15 lines · On Sun, 7 Mar 2021 23:10:12 +0100 Krzysztof Wilczyński wrote:

Hi,
Quoted 11 lines
Currently, dw_pcie_msi_init() allocates and maps page for msi, then program the PCIE_MSI_ADDR_LO and PCIE_MSI_ADDR_HI. The Root Complex may lose power during suspend-to-RAM, so when we resume, we want to redo the latter but not the former. If designware based driver (for example, pcie-tegra194.c) calls dw_pcie_msi_init() in resume path, the previous msi page will be leaked. From another side, except pci-dra7xx.c we can move the dw_pcie_msi_init() from each users to designware host, I.E move the msi page allocation and mapping to dw_pcie_host_init() and move the PCIE_MSI_ADDR_* programming to dw_pcie_setup_rc(). After this moving, we solve the msi page leakage as well.

Quoted 7 lines

[...]
A small nitpick. All the "designware" should be "DesignWare" both in the commit message and the subject. Similarly, "msi" would be "MSI", and "I.E" would become "i.e.,". If you ever sent another version of the patch, that is.

This series was dropped and useless after Rob's excellent dw pcie code clean up.

Thanks

_______________________________________________
linux-amlogic mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-amlogic

Jon Hunter

maintainer

2020-09-25 08:54 UTC #15 lore raw

In reply to Jisheng Zhang

Quoted 4 lines · Jisheng Zhang

Improve the msi code:
1. Add proper error handling.
2. Move dw_pcie_msi_init() from each users to designware host to solve msi page leakage in resume path.

Apologies if this is slightly off topic, but I have been meaning to ask about MSIs and PCI. On Tegra194 which uses the DWC PCI driver, whenever we hotplug CPUs we see the following warnings ...

 [      79.068351] WARNING KERN IRQ70: set affinity failed(-22).
 [      79.068362] WARNING KERN IRQ71: set affinity failed(-22).

These interrupts are the MSIs ...

70:          0          0          0          0          0          0          0          0   PCI-MSI 134217728 Edge      PCIe PME, aerdrv
71:          0          0          0          0          0          0          0          0   PCI-MSI 134742016 Edge      ahci[0001:01:00.0]

This caused because ...

static int dw_pci_msi_set_affinity(struct irq_data *d,
const struct cpumask *mask, bool force)

 {
         return -EINVAL;

}

Now the above is not unique to the DWC PCI host driver, it appears that most PCIe drivers also do the same. However, I am curious if there is any way to avoid the above warnings given that setting the affinity does not appear to be supported in anyway AFAICT.

Cheers
Jon

Signature · 1 line

nvpublic

Jisheng Zhang

author

2020-09-25 09:17 UTC #16 lore raw

Hi Jon,

Quoted 23 lines · On Fri, 25 Sep 2020 09:53:45 +0100 Jon Hunter wrote:

Quoted 4 lines · Jisheng Zhang
Improve the msi code:
1. Add proper error handling.
2. Move dw_pcie_msi_init() from each users to designware host to solve msi page leakage in resume path.
Apologies if this is slightly off topic, but I have been meaning to ask about MSIs and PCI. On Tegra194 which uses the DWC PCI driver, whenever we hotplug CPUs we see the following warnings ...
 [      79.068351] WARNING KERN IRQ70: set affinity failed(-22).
 [      79.068362] WARNING KERN IRQ71: set affinity failed(-22).
These interrupts are the MSIs ...
70:          0          0          0          0          0          0          0          0   PCI-MSI 134217728 Edge      PCIe PME, aerdrv
71:          0          0          0          0          0          0          0          0   PCI-MSI 134742016 Edge      ahci[0001:01:00.0]
This caused because ...
static int dw_pci_msi_set_affinity(struct irq_data *d,
const struct cpumask *mask, bool force)

Quoted 9 lines

 {
         return -EINVAL;
}
Now the above is not unique to the DWC PCI host driver, it appears that most PCIe drivers also do the same. However, I am curious if there is any way to avoid the above warnings given that setting the affinity does not appear to be supported in anyway AFAICT.

Could you please try below patch?

drivers/pci/controller/dwc/pcie-designware-host.c +0 -1

index bf25d783b5c5..7e5dc54d060e 100644
@@ -197,7 +197,6 @@ static struct irq_chip dw_pci_msi_bottom_irq_chip = {
        .name = "DWPCI-MSI",
        .irq_ack = dw_pci_bottom_ack,
        .irq_compose_msi_msg = dw_pci_setup_msi_msg,
-       .irq_set_affinity = dw_pci_msi_set_affinity,
        .irq_mask = dw_pci_bottom_mask,
        .irq_unmask = dw_pci_bottom_unmask,
 };

Jisheng Zhang

author

2020-09-25 09:27 UTC #17 lore raw

Quoted 50 lines · On Fri, 25 Sep 2020 17:17:12 +0800 Jisheng Zhang <[email protected]>…

CAUTION: Email originated externally, do not click links or open attachments unless you recognize the sender and know the content is safe.
Hi Jon,
Quoted 23 lines · On Fri, 25 Sep 2020 09:53:45 +0100 Jon Hunter wrote:
Quoted 4 lines · Jisheng Zhang
Improve the msi code:
1. Add proper error handling.
2. Move dw_pcie_msi_init() from each users to designware host to solve msi page leakage in resume path.
Apologies if this is slightly off topic, but I have been meaning to ask about MSIs and PCI. On Tegra194 which uses the DWC PCI driver, whenever we hotplug CPUs we see the following warnings ...
 [      79.068351] WARNING KERN IRQ70: set affinity failed(-22).
 [      79.068362] WARNING KERN IRQ71: set affinity failed(-22).
These interrupts are the MSIs ...
70:          0          0          0          0          0          0          0          0   PCI-MSI 134217728 Edge      PCIe PME, aerdrv
71:          0          0          0          0          0          0          0          0   PCI-MSI 134742016 Edge      ahci[0001:01:00.0]
This caused because ...
static int dw_pci_msi_set_affinity(struct irq_data *d,
const struct cpumask *mask, bool force)
Quoted 9 lines
 {
         return -EINVAL;
}
Now the above is not unique to the DWC PCI host driver, it appears that most PCIe drivers also do the same. However, I am curious if there is any way to avoid the above warnings given that setting the affinity does not appear to be supported in anyway AFAICT.
Could you please try below patch?
drivers/pci/controller/dwc/pcie-designware-host.c +0 -0
index bf25d783b5c5..7e5dc54d060e 100644

drivers/pci/controller/dwc/pcie-designware-host.c static struct irq_chip dw_pci_msi_bottom_irq_chip = { · line 202

Quoted 7 lines

drivers/pci/controller/dwc/pcie-designware-host.c +0 -1

@@ -197,7 +197,6 @@ static struct irq_chip dw_pci_msi_bottom_irq_chip = {
        .name = "DWPCI-MSI",
        .irq_ack = dw_pci_bottom_ack,
        .irq_compose_msi_msg = dw_pci_setup_msi_msg,
-       .irq_set_affinity = dw_pci_msi_set_affinity,
        .irq_mask = dw_pci_bottom_mask,
        .irq_unmask = dw_pci_bottom_unmask,
 };

A complete patch w/o compiler warning:

drivers/pci/controller/dwc/pcie-designware-host.c +0 -7

index bf25d783b5c5..18f719cfed0b 100644
@@ -137,12 +137,6 @@ static void dw_pci_setup_msi_msg(struct irq_data *d, struct msi_msg *msg)
 		(int)d->hwirq, msg->address_hi, msg->address_lo);
 }
 
-static int dw_pci_msi_set_affinity(struct irq_data *d,
-				   const struct cpumask *mask, bool force)
-{
-	return -EINVAL;
-}
-
 static void dw_pci_bottom_mask(struct irq_data *d)
 {
 	struct pcie_port *pp = irq_data_get_irq_chip_data(d);

@@ -197,7 +191,6 @@ static struct irq_chip dw_pci_msi_bottom_irq_chip = {
 	.name = "DWPCI-MSI",
 	.irq_ack = dw_pci_bottom_ack,
 	.irq_compose_msi_msg = dw_pci_setup_msi_msg,
-	.irq_set_affinity = dw_pci_msi_set_affinity,
 	.irq_mask = dw_pci_bottom_mask,
 	.irq_unmask = dw_pci_bottom_unmask,
 };

Jon Hunter

maintainer

2020-09-25 15:13 UTC #18 lore raw

Hi Jisheng,

On 25/09/2020 10:27, Jisheng Zhang wrote:

...

Quoted 37 lines

Quoted 8 lines

Could you please try below patch?
drivers/pci/controller/dwc/pcie-designware-host.c +0 -0
index bf25d783b5c5..7e5dc54d060e 100644

drivers/pci/controller/dwc/pcie-designware-host.c static struct irq_chip dw_pci_msi_bottom_irq_chip = { · line 202

Quoted 7 lines

drivers/pci/controller/dwc/pcie-designware-host.c +0 -1

@@ -197,7 +197,6 @@ static struct irq_chip dw_pci_msi_bottom_irq_chip = {
        .name = "DWPCI-MSI",
        .irq_ack = dw_pci_bottom_ack,
        .irq_compose_msi_msg = dw_pci_setup_msi_msg,
-       .irq_set_affinity = dw_pci_msi_set_affinity,
        .irq_mask = dw_pci_bottom_mask,
        .irq_unmask = dw_pci_bottom_unmask,
 };

A complete patch w/o compiler warning:

drivers/pci/controller/dwc/pcie-designware-host.c +0 -6

index bf25d783b5c5..18f719cfed0b 100644
@@ -137,12 +137,6 @@ static void dw_pci_setup_msi_msg(struct irq_data *d, struct msi_msg *msg)
 		(int)d->hwirq, msg->address_hi, msg->address_lo);
 }
 
-static int dw_pci_msi_set_affinity(struct irq_data *d,
-				   const struct cpumask *mask, bool force)
-{
-	return -EINVAL;
-}
-
 static void dw_pci_bottom_mask(struct irq_data *d)
 {
 	struct pcie_port *pp = irq_data_get_irq_chip_data(d);

drivers/pci/controller/dwc/pcie-designware-host.c static struct irq_chip dw_pci_msi_bottom_irq_chip = { · line 196

Quoted 7 lines

drivers/pci/controller/dwc/pcie-designware-host.c +0 -1

@@ -197,7 +191,6 @@ static struct irq_chip dw_pci_msi_bottom_irq_chip = {
 	.name = "DWPCI-MSI",
 	.irq_ack = dw_pci_bottom_ack,
 	.irq_compose_msi_msg = dw_pci_setup_msi_msg,
-	.irq_set_affinity = dw_pci_msi_set_affinity,
 	.irq_mask = dw_pci_bottom_mask,
 	.irq_unmask = dw_pci_bottom_unmask,
 };

Thanks I was not expecting this to work because ...

int irq_do_set_affinity(struct irq_data *data, const struct cpumask *mask,
bool force)

 {
         struct irq_desc *desc = irq_data_to_desc(data);
         struct irq_chip *chip = irq_data_get_irq_chip(data);
         int ret;
 
         if (!chip || !chip->irq_set_affinity)
                 return -EINVAL;

However, with your patch Tegra crashes on boot ...

[   11.613853] Unable to handle kernel NULL pointer dereference at virtual address 0000000000000000
[   11.622500] Mem abort info:
[   11.622515]   ESR = 0x86000004
[   11.622524]   EC = 0x21: IABT (current EL), IL = 32 bits
[   11.622540]   SET = 0, FnV = 0
[   11.636544]   EA = 0, S1PTW = 0
[   11.636554] user pgtable: 4k pages, 48-bit VAs, pgdp=000000046a28e000
[   11.636559] [0000000000000000] pgd=0000000000000000, p4d=0000000000000000
[   11.652652] Internal error: Oops: 86000004 [#1] PREEMPT SMP
[   11.652658] Modules linked in: pwm_tegra phy_tegra194_p2u crct10dif_ce lm90 pwm_fan tegra_bpmp_thermal pcie_tegra194 ip_tables x_tables ipv6
[   11.670525] CPU: 3 PID: 138 Comm: kworker/3:3 Not tainted 5.9.0-rc4-dirty #12
[   11.670534] Hardware name: NVIDIA Jetson AGX Xavier Developer Kit (DT)
[   11.683967] Workqueue: events deferred_probe_work_func
[   11.683974] pstate: 60c00089 (nZCv daIf +PAN +UAO BTYPE=--)
[   11.683985] pc : 0x0
[   11.696669] lr : msi_domain_set_affinity+0x44/0xc0
[   11.696672] sp : ffff800012bcb390
[   11.696680] x29: ffff800012bcb390 x28: ffff0003e3033c20 
[   11.709891] x27: ffff0003e76cfe58 x26: 0000000000000000 
[   11.709900] x25: ffff800011d7e850 x24: ffff800011d7e878 
[   11.709908] x23: 0000000000000000 x22: ffff0003e76cfe00 
[   11.709914] x21: ffff0003e76cfe58 x20: ffff0003e76cfe58 
[   11.709921] x19: ffff800011b19000 x18: ffffffffffffffff 
[   11.709927] x17: 0000000000000000 x16: 0000000000000000 
[   11.741262] x15: ffff800011b19948 x14: 0000000000000040 
[   11.741267] x13: 0000000000000228 x12: 0000000000000030 
[   11.741272] x11: 0101010101010101 x10: 0000000000000040 
[   11.741277] x9 : 0000000000000000 x8 : 0000000000000004 
[   11.741281] x7 : ffffffffffffffff x6 : 00000000000000ff 
[   11.767374] x5 : 0000000000000000 x4 : 0000000000000000 
[   11.767379] x3 : 0000000000000000 x2 : 0000000000000000 
[   11.767384] x1 : ffff800011d7e898 x0 : ffff0003e262bf00 
[   11.767406] Call trace:
[   11.767410]  0x0
[   11.767424]  irq_do_set_affinity+0x4c/0x178
[   11.791400]  irq_setup_affinity+0x124/0x1b0
[   11.791423]  irq_startup+0x6c/0x118
[   11.791434]  __setup_irq+0x810/0x8a0
[   11.802510]  request_threaded_irq+0xdc/0x188
[   11.802517]  pcie_pme_probe+0x98/0x110
[   11.802536]  pcie_port_probe_service+0x34/0x60
[   11.814799]  really_probe+0x110/0x400
[   11.814809]  driver_probe_device+0x54/0xb8
[   11.822438]  __device_attach_driver+0x90/0xc0
[   11.822463]  bus_for_each_drv+0x70/0xc8
[   11.822471]  __device_attach+0xec/0x150
[   11.834307]  device_initial_probe+0x10/0x18
[   11.834311]  bus_probe_device+0x94/0xa0
[   11.834315]  device_add+0x464/0x730
[   11.834338]  device_register+0x1c/0x28
[   11.834349]  pcie_port_device_register+0x2d0/0x3e8
[   11.854056]  pcie_portdrv_probe+0x34/0xd8
[   11.854063]  local_pci_probe+0x3c/0xa0
[   11.854088]  pci_device_probe+0x128/0x1c8
[   11.854103]  really_probe+0x110/0x400
[   11.869283]  driver_probe_device+0x54/0xb8
[   11.869311]  __device_attach_driver+0x90/0xc0
[   11.877638]  bus_for_each_drv+0x70/0xc8
[   11.877645]  __device_attach+0xec/0x150
[   11.877669]  device_attach+0x10/0x18
[   11.877680]  pci_bus_add_device+0x4c/0xb0
[   11.892642]  pci_bus_add_devices+0x44/0x90
[   11.892646]  dw_pcie_host_init+0x370/0x4f8
[   11.892653]  tegra_pcie_dw_probe+0x5e8/0xb50 [pcie_tegra194]
[   11.892661]  platform_drv_probe+0x50/0xa8
[   11.910179]  really_probe+0x110/0x400
[   11.910183]  driver_probe_device+0x54/0xb8
[   11.910186]  __device_attach_driver+0x90/0xc0
[   11.910213]  bus_for_each_drv+0x70/0xc8
[   11.910240]  __device_attach+0xec/0x150
[   11.929689]  device_initial_probe+0x10/0x18
[   11.929694]  bus_probe_device+0x94/0xa0
[   11.929719]  deferred_probe_work_func+0x6c/0xa0
[   11.929730]  process_one_work+0x1cc/0x360
[   11.946008]  worker_thread+0x48/0x450
[   11.949602]  kthread+0x120/0x150
[   11.952803]  ret_from_fork+0x10/0x1c
[   11.956332] Code: bad PC value
[   11.959360] ---[ end trace 03c30e252fe4e40b ]---

To be honest, I am not sure I completely understand why it crashes here.

Cheers
Jon

Signature · 1 line

nvpublic

Jisheng Zhang

author

2020-09-27 08:29 UTC #19 lore raw

Hi,

Quoted 147 lines · On Fri, 25 Sep 2020 16:13:02 +0100 Jon Hunter wrote:

Hi Jisheng,

On 25/09/2020 10:27, Jisheng Zhang wrote:

...

Quoted 37 lines

Quoted 8 lines

Could you please try below patch?
drivers/pci/controller/dwc/pcie-designware-host.c +0 -0
index bf25d783b5c5..7e5dc54d060e 100644

drivers/pci/controller/dwc/pcie-designware-host.c static struct irq_chip dw_pci_msi_bottom_irq_chip = { · line 202

Quoted 7 lines

drivers/pci/controller/dwc/pcie-designware-host.c +0 -1

@@ -197,7 +197,6 @@ static struct irq_chip dw_pci_msi_bottom_irq_chip = {
        .name = "DWPCI-MSI",
        .irq_ack = dw_pci_bottom_ack,
        .irq_compose_msi_msg = dw_pci_setup_msi_msg,
-       .irq_set_affinity = dw_pci_msi_set_affinity,
        .irq_mask = dw_pci_bottom_mask,
        .irq_unmask = dw_pci_bottom_unmask,
 };

A complete patch w/o compiler warning:

drivers/pci/controller/dwc/pcie-designware-host.c +0 -6

index bf25d783b5c5..18f719cfed0b 100644
@@ -137,12 +137,6 @@ static void dw_pci_setup_msi_msg(struct irq_data *d, struct msi_msg *msg)
              (int)d->hwirq, msg->address_hi, msg->address_lo);
 }
 
-static int dw_pci_msi_set_affinity(struct irq_data *d,
-                                const struct cpumask *mask, bool force)
-{
-     return -EINVAL;
-}
-
 static void dw_pci_bottom_mask(struct irq_data *d)
 {
      struct pcie_port *pp = irq_data_get_irq_chip_data(d);

drivers/pci/controller/dwc/pcie-designware-host.c static struct irq_chip dw_pci_msi_bottom_irq_chip = { · line 196

Quoted 7 lines

drivers/pci/controller/dwc/pcie-designware-host.c +0 -1

@@ -197,7 +191,6 @@ static struct irq_chip dw_pci_msi_bottom_irq_chip = {
      .name = "DWPCI-MSI",
      .irq_ack = dw_pci_bottom_ack,
      .irq_compose_msi_msg = dw_pci_setup_msi_msg,
-     .irq_set_affinity = dw_pci_msi_set_affinity,
      .irq_mask = dw_pci_bottom_mask,
      .irq_unmask = dw_pci_bottom_unmask,
 };

Thanks I was not expecting this to work because ...

int irq_do_set_affinity(struct irq_data *data, const struct cpumask *mask,
bool force)

 {
         struct irq_desc *desc = irq_data_to_desc(data);
         struct irq_chip *chip = irq_data_get_irq_chip(data);
         int ret;

         if (!chip || !chip->irq_set_affinity)
                 return -EINVAL;

However, with your patch Tegra crashes on boot ...

[   11.613853] Unable to handle kernel NULL pointer dereference at virtual address 0000000000000000
[   11.622500] Mem abort info:
[   11.622515]   ESR = 0x86000004
[   11.622524]   EC = 0x21: IABT (current EL), IL = 32 bits
[   11.622540]   SET = 0, FnV = 0
[   11.636544]   EA = 0, S1PTW = 0
[   11.636554] user pgtable: 4k pages, 48-bit VAs, pgdp=000000046a28e000
[   11.636559] [0000000000000000] pgd=0000000000000000, p4d=0000000000000000
[   11.652652] Internal error: Oops: 86000004 [#1] PREEMPT SMP
[   11.652658] Modules linked in: pwm_tegra phy_tegra194_p2u crct10dif_ce lm90 pwm_fan tegra_bpmp_thermal pcie_tegra194 ip_tables x_tables ipv6
[   11.670525] CPU: 3 PID: 138 Comm: kworker/3:3 Not tainted 5.9.0-rc4-dirty #12
[   11.670534] Hardware name: NVIDIA Jetson AGX Xavier Developer Kit (DT)
[   11.683967] Workqueue: events deferred_probe_work_func
[   11.683974] pstate: 60c00089 (nZCv daIf +PAN +UAO BTYPE=--)
[   11.683985] pc : 0x0
[   11.696669] lr : msi_domain_set_affinity+0x44/0xc0
[   11.696672] sp : ffff800012bcb390
[   11.696680] x29: ffff800012bcb390 x28: ffff0003e3033c20
[   11.709891] x27: ffff0003e76cfe58 x26: 0000000000000000
[   11.709900] x25: ffff800011d7e850 x24: ffff800011d7e878
[   11.709908] x23: 0000000000000000 x22: ffff0003e76cfe00
[   11.709914] x21: ffff0003e76cfe58 x20: ffff0003e76cfe58
[   11.709921] x19: ffff800011b19000 x18: ffffffffffffffff
[   11.709927] x17: 0000000000000000 x16: 0000000000000000
[   11.741262] x15: ffff800011b19948 x14: 0000000000000040
[   11.741267] x13: 0000000000000228 x12: 0000000000000030
[   11.741272] x11: 0101010101010101 x10: 0000000000000040
[   11.741277] x9 : 0000000000000000 x8 : 0000000000000004
[   11.741281] x7 : ffffffffffffffff x6 : 00000000000000ff
[   11.767374] x5 : 0000000000000000 x4 : 0000000000000000
[   11.767379] x3 : 0000000000000000 x2 : 0000000000000000
[   11.767384] x1 : ffff800011d7e898 x0 : ffff0003e262bf00
[   11.767406] Call trace:
[   11.767410]  0x0
[   11.767424]  irq_do_set_affinity+0x4c/0x178
[   11.791400]  irq_setup_affinity+0x124/0x1b0
[   11.791423]  irq_startup+0x6c/0x118
[   11.791434]  __setup_irq+0x810/0x8a0
[   11.802510]  request_threaded_irq+0xdc/0x188
[   11.802517]  pcie_pme_probe+0x98/0x110
[   11.802536]  pcie_port_probe_service+0x34/0x60
[   11.814799]  really_probe+0x110/0x400
[   11.814809]  driver_probe_device+0x54/0xb8
[   11.822438]  __device_attach_driver+0x90/0xc0
[   11.822463]  bus_for_each_drv+0x70/0xc8
[   11.822471]  __device_attach+0xec/0x150
[   11.834307]  device_initial_probe+0x10/0x18
[   11.834311]  bus_probe_device+0x94/0xa0
[   11.834315]  device_add+0x464/0x730
[   11.834338]  device_register+0x1c/0x28
[   11.834349]  pcie_port_device_register+0x2d0/0x3e8
[   11.854056]  pcie_portdrv_probe+0x34/0xd8
[   11.854063]  local_pci_probe+0x3c/0xa0
[   11.854088]  pci_device_probe+0x128/0x1c8
[   11.854103]  really_probe+0x110/0x400
[   11.869283]  driver_probe_device+0x54/0xb8
[   11.869311]  __device_attach_driver+0x90/0xc0
[   11.877638]  bus_for_each_drv+0x70/0xc8
[   11.877645]  __device_attach+0xec/0x150
[   11.877669]  device_attach+0x10/0x18
[   11.877680]  pci_bus_add_device+0x4c/0xb0
[   11.892642]  pci_bus_add_devices+0x44/0x90
[   11.892646]  dw_pcie_host_init+0x370/0x4f8
[   11.892653]  tegra_pcie_dw_probe+0x5e8/0xb50 [pcie_tegra194]
[   11.892661]  platform_drv_probe+0x50/0xa8
[   11.910179]  really_probe+0x110/0x400
[   11.910183]  driver_probe_device+0x54/0xb8
[   11.910186]  __device_attach_driver+0x90/0xc0
[   11.910213]  bus_for_each_drv+0x70/0xc8
[   11.910240]  __device_attach+0xec/0x150
[   11.929689]  device_initial_probe+0x10/0x18
[   11.929694]  bus_probe_device+0x94/0xa0
[   11.929719]  deferred_probe_work_func+0x6c/0xa0
[   11.929730]  process_one_work+0x1cc/0x360
[   11.946008]  worker_thread+0x48/0x450
[   11.949602]  kthread+0x120/0x150
[   11.952803]  ret_from_fork+0x10/0x1c
[   11.956332] Code: bad PC value
[   11.959360] ---[ end trace 03c30e252fe4e40b ]---

Quoted 2 lines

To be honest, I am not sure I completely understand why it crashes here.

I see, the msi_domain_set_affinity() calls parent->chip->irq_set_affinity without checking, grepping the irqchip and pci dir, I found that if the MSI is based on some cascaded interrupt mechanism, they all point the irq_set_affinity to irq_chip_set_affinity_parent(), so I believe below patch works:

drivers/pci/controller/dwc/pcie-designware-host.c +1 -7

index bf25d783b5c5..093fba616736 100644
@@ -137,12 +137,6 @@ static void dw_pci_setup_msi_msg(struct irq_data *d, struct msi_msg *msg)
 		(int)d->hwirq, msg->address_hi, msg->address_lo);
 }
 
-static int dw_pci_msi_set_affinity(struct irq_data *d,
-				   const struct cpumask *mask, bool force)
-{
-	return -EINVAL;
-}
-
 static void dw_pci_bottom_mask(struct irq_data *d)
 {
 	struct pcie_port *pp = irq_data_get_irq_chip_data(d);

@@ -197,7 +191,7 @@ static struct irq_chip dw_pci_msi_bottom_irq_chip = {
 	.name = "DWPCI-MSI",
 	.irq_ack = dw_pci_bottom_ack,
 	.irq_compose_msi_msg = dw_pci_setup_msi_msg,
-	.irq_set_affinity = dw_pci_msi_set_affinity,
+	.irq_set_affinity = irq_chip_set_affinity_parent,
 	.irq_mask = dw_pci_bottom_mask,
 	.irq_unmask = dw_pci_bottom_unmask,
 };

Jon Hunter

maintainer

2020-09-28 17:46 UTC #20 lore raw

On 27/09/2020 09:28, Jisheng Zhang wrote:

...

Quoted 25 lines

I see, the msi_domain_set_affinity() calls parent->chip->irq_set_affinity without checking, grepping the irqchip and pci dir, I found that if the MSI is based on some cascaded interrupt mechanism, they all point the irq_set_affinity to irq_chip_set_affinity_parent(), so I believe below patch works:
drivers/pci/controller/dwc/pcie-designware-host.c +0 -6
index bf25d783b5c5..093fba616736 100644
@@ -137,12 +137,6 @@ static void dw_pci_setup_msi_msg(struct irq_data *d, struct msi_msg *msg)
 		(int)d->hwirq, msg->address_hi, msg->address_lo);
 }
 
-static int dw_pci_msi_set_affinity(struct irq_data *d,
-				   const struct cpumask *mask, bool force)
-{
-	return -EINVAL;
-}
-
 static void dw_pci_bottom_mask(struct irq_data *d)
 {
 	struct pcie_port *pp = irq_data_get_irq_chip_data(d);

drivers/pci/controller/dwc/pcie-designware-host.c static struct irq_chip dw_pci_msi_bottom_irq_chip = { · line 197

Quoted 8 lines

drivers/pci/controller/dwc/pcie-designware-host.c +1 -1

@@ -197,7 +191,7 @@ static struct irq_chip dw_pci_msi_bottom_irq_chip = {
 	.name = "DWPCI-MSI",
 	.irq_ack = dw_pci_bottom_ack,
 	.irq_compose_msi_msg = dw_pci_setup_msi_msg,
-	.irq_set_affinity = dw_pci_msi_set_affinity,
+	.irq_set_affinity = irq_chip_set_affinity_parent,
 	.irq_mask = dw_pci_bottom_mask,
 	.irq_unmask = dw_pci_bottom_unmask,
 };

Unfortunately, this still crashes ...

[   11.521674] Unable to handle kernel NULL pointer dereference at virtual address 0000000000000018
[   11.530324] Mem abort info:
[   11.533089]   ESR = 0x96000004
[   11.536105]   EC = 0x25: DABT (current EL), IL = 32 bits
[   11.541333]   SET = 0, FnV = 0
[   11.544344]   EA = 0, S1PTW = 0
[   11.547441] Data abort info:
[   11.550279]   ISV = 0, ISS = 0x00000004
[   11.554056]   CM = 0, WnR = 0
[   11.557007] user pgtable: 4k pages, 48-bit VAs, pgdp=0000000467341000
[   11.563333] [0000000000000018] pgd=0000000000000000, p4d=0000000000000000
[   11.570024] Internal error: Oops: 96000004 [#1] PREEMPT SMP
[   11.575517] Modules linked in: crct10dif_ce pwm_tegra snd_hda_core phy_tegra194_p2u lm90 pcie_tegra194 tegra_bpmp_thermal pwm_fan ip_tables x_tables ipv6
[   11.589046] CPU: 3 PID: 148 Comm: kworker/3:1 Not tainted 5.9.0-rc4-00009-g6fdf18edb995-dirty #7
[   11.597669] Hardware name: NVIDIA Jetson AGX Xavier Developer Kit (DT)
[   11.604110] Workqueue: events deferred_probe_work_func
[   11.609174] pstate: 60c00089 (nZCv daIf +PAN +UAO BTYPE=--)
[   11.614657] pc : irq_chip_set_affinity_parent+0x4/0x30
[   11.619735] lr : msi_domain_set_affinity+0x44/0xc0
[   11.624448] sp : ffff800012d4b390
[   11.627744] x29: ffff800012d4b390 x28: ffff0003e7234c20 
[   11.632983] x27: ffff0003e913e460 x26: 0000000000000000 
[   11.638231] x25: ffff800011d7e890 x24: ffff800011d7e8b8 
[   11.643466] x23: 0000000000000000 x22: ffff0003e913e400 
[   11.648701] x21: ffff0003e913e460 x20: ffff0003e913e460 
[   11.653932] x19: ffff800011b19000 x18: ffffffffffffffff 
[   11.659160] x17: 0000000000000000 x16: 0000000000000000 
[   11.664390] x15: 0000000000000001 x14: 0000000000000040 
[   11.669636] x13: 0000000000000228 x12: 0000000000000030 
[   11.674864] x11: 0101010101010101 x10: 0000000000000040 
[   11.680111] x9 : 0000000000000000 x8 : 0000000000000004 
[   11.685363] x7 : ffffffffffffffff x6 : 00000000000000ff 
[   11.690596] x5 : 0000000000000000 x4 : 0000000000000000 
[   11.695843] x3 : ffff8000100d89a8 x2 : 0000000000000000 
[   11.701058] x1 : ffff800011d7e8d8 x0 : 0000000000000000 
[   11.706288] Call trace:
[   11.708708]  irq_chip_set_affinity_parent+0x4/0x30
[   11.713431]  irq_do_set_affinity+0x4c/0x178
[   11.717540]  irq_setup_affinity+0x124/0x1b0
[   11.721650]  irq_startup+0x6c/0x118
[   11.725081]  __setup_irq+0x810/0x8a0
[   11.728580]  request_threaded_irq+0xdc/0x188
[   11.732793]  pcie_pme_probe+0x98/0x110
[   11.736481]  pcie_port_probe_service+0x34/0x60
[   11.740848]  really_probe+0x110/0x400
[   11.744445]  driver_probe_device+0x54/0xb8
[   11.748482]  __device_attach_driver+0x90/0xc0
[   11.752758]  bus_for_each_drv+0x70/0xc8
[   11.756526]  __device_attach+0xec/0x150
[   11.760306]  device_initial_probe+0x10/0x18
[   11.764413]  bus_probe_device+0x94/0xa0
[   11.768203]  device_add+0x464/0x730
[   11.771630]  device_register+0x1c/0x28
[   11.775311]  pcie_port_device_register+0x2d0/0x3e8
[   11.780017]  pcie_portdrv_probe+0x34/0xd8
[   11.783957]  local_pci_probe+0x3c/0xa0
[   11.787647]  pci_device_probe+0x128/0x1c8
[   11.791588]  really_probe+0x110/0x400
[   11.795179]  driver_probe_device+0x54/0xb8
[   11.799202]  __device_attach_driver+0x90/0xc0
[   11.803480]  bus_for_each_drv+0x70/0xc8
[   11.807244]  __device_attach+0xec/0x150
[   11.811009]  device_attach+0x10/0x18
[   11.814518]  pci_bus_add_device+0x4c/0xb0
[   11.818456]  pci_bus_add_devices+0x44/0x90
[   11.822478]  dw_pcie_host_init+0x370/0x4f8
[   11.826504]  tegra_pcie_dw_probe+0x5e8/0xb50 [pcie_tegra194]
[   11.832044]  platform_drv_probe+0x50/0xa8
[   11.835984]  really_probe+0x110/0x400
[   11.839580]  driver_probe_device+0x54/0xb8
[   11.843608]  __device_attach_driver+0x90/0xc0
[   11.847887]  bus_for_each_drv+0x70/0xc8
[   11.851655]  __device_attach+0xec/0x150
[   11.855424]  device_initial_probe+0x10/0x18
[   11.859548]  bus_probe_device+0x94/0xa0
[   11.863317]  deferred_probe_work_func+0x6c/0xa0
[   11.867781]  process_one_work+0x1cc/0x360
[   11.871720]  worker_thread+0x48/0x450
[   11.875318]  kthread+0x120/0x150
[   11.878495]  ret_from_fork+0x10/0x1c
[   11.882027] Code: a8c17bfd d65f03c0 d503201f f9401400 (f9400c03)

Cheers
Jon

Signature · 1 line

nvpublic

Jisheng Zhang

author

2020-09-29 10:49 UTC #21 lore raw

In reply to Jon Hunter

Hi Jon,

Quoted 7 lines · On Fri, 25 Sep 2020 09:53:45 +0100 Jon Hunter wrote:

Quoted 4 lines · Jisheng Zhang
Improve the msi code:
1. Add proper error handling.
2. Move dw_pcie_msi_init() from each users to designware host to solve msi page leakage in resume path.

Quoted 7 lines

Apologies if this is slightly off topic, but I have been meaning to ask about MSIs and PCI. On Tegra194 which uses the DWC PCI driver, whenever we hotplug CPUs we see the following warnings ...
 [      79.068351] WARNING KERN IRQ70: set affinity failed(-22).
 [      79.068362] WARNING KERN IRQ71: set affinity failed(-22).

I tried to reproduce this issue on Synaptics SoC, but can't reproduce it. Per my understanding of the code in kernel/irq/cpuhotplug.c, this warning happened when we migrate irqs away from the offline cpu, this implicitly implies that before this point the irq has bind to the offline cpu, but how could this happen given current dw_pci_msi_set_affinity() implementation always return -EINVAL

thanks

Jon Hunter

maintainer

2020-09-29 13:23 UTC #22 lore raw

Hi Jisheng,

Quoted 19 lines · Jisheng Zhang

Hi Jon,
Quoted 7 lines · On Fri, 25 Sep 2020 09:53:45 +0100 Jon Hunter wrote:
Quoted 4 lines · Jisheng Zhang
Improve the msi code:
1. Add proper error handling.
2. Move dw_pcie_msi_init() from each users to designware host to solve msi page leakage in resume path.
Quoted 7 lines
Apologies if this is slightly off topic, but I have been meaning to ask about MSIs and PCI. On Tegra194 which uses the DWC PCI driver, whenever we hotplug CPUs we see the following warnings ...
 [      79.068351] WARNING KERN IRQ70: set affinity failed(-22).
 [      79.068362] WARNING KERN IRQ71: set affinity failed(-22).

Quoted 6 lines

I tried to reproduce this issue on Synaptics SoC, but can't reproduce it. Per my understanding of the code in kernel/irq/cpuhotplug.c, this warning happened when we migrate irqs away from the offline cpu, this implicitly implies that before this point the irq has bind to the offline cpu, but how could this happen given current dw_pci_msi_set_affinity() implementation always return -EINVAL

By default the smp_affinity should be set so that all CPUs can be interrupted ...

$ cat /proc/irq/70/smp_affinity
0xff

In my case there are 8 CPUs and so 0xff implies that the interrupt can be triggered on any of the 8 CPUs.

Do you see the set_affinity callback being called for the DWC irqchip in migrate_one_irq()?

Cheers
Jon

Signature · 1 line

nvpublic

Marc Zyngier

2020-09-29 17:25 UTC #23 lore raw

Quoted 32 lines · Jon Hunter

Hi Jisheng,
Quoted 19 lines · Jisheng Zhang
Hi Jon,
Quoted 7 lines · On Fri, 25 Sep 2020 09:53:45 +0100 Jon Hunter wrote:
Quoted 4 lines · Jisheng Zhang
Improve the msi code:
1. Add proper error handling.
2. Move dw_pcie_msi_init() from each users to designware host to solve msi page leakage in resume path.
Quoted 7 lines
Apologies if this is slightly off topic, but I have been meaning to ask about MSIs and PCI. On Tegra194 which uses the DWC PCI driver, whenever we hotplug CPUs we see the following warnings ...
 [      79.068351] WARNING KERN IRQ70: set affinity failed(-22).
 [      79.068362] WARNING KERN IRQ71: set affinity failed(-22).
Quoted 6 lines
I tried to reproduce this issue on Synaptics SoC, but can't reproduce it. Per my understanding of the code in kernel/irq/cpuhotplug.c, this warning happened when we migrate irqs away from the offline cpu, this implicitly implies that before this point the irq has bind to the offline cpu, but how could this happen given current dw_pci_msi_set_affinity() implementation always return -EINVAL
By default the smp_affinity should be set so that all CPUs can be interrupted ...

Quoted 8 lines

$ cat /proc/irq/70/smp_affinity
0xff
In my case there are 8 CPUs and so 0xff implies that the interrupt can be triggered on any of the 8 CPUs.
Do you see the set_affinity callback being called for the DWC irqchip in migrate_one_irq()?

The problem is common to all MSI implementations that end up muxing all the end-point MSIs into a single interrupt. With these systems, you cannot set the affinity of individual MSIs (they don't target a CPU, they target another interrupt... braindead). Only the mux interrupt can have its affinity changed.

So returning -EINVAL is the right thing to do.

M.

Signature · 1 line

Jazz is not dead. It just smells funny...

Jon Hunter

maintainer

2020-09-29 18:02 UTC #24 lore raw

Quoted 46 lines · Marc Zyngier

Quoted 36 lines · Jon Hunter
Hi Jisheng,
Quoted 20 lines · Jisheng Zhang
Hi Jon,
Quoted 7 lines · On Fri, 25 Sep 2020 09:53:45 +0100 Jon Hunter wrote:
Quoted 4 lines · Jisheng Zhang
Improve the msi code:
1. Add proper error handling.
2. Move dw_pcie_msi_init() from each users to designware host to solve msi page leakage in resume path.
Quoted 8 lines
Apologies if this is slightly off topic, but I have been meaning to ask about MSIs and PCI. On Tegra194 which uses the DWC PCI driver, whenever we
hotplug CPUs we see the following warnings ...
[ 79.068351] WARNING KERN IRQ70: set affinity failed(-22). [ 79.068362] WARNING KERN IRQ71: set affinity failed(-22).
Quoted 9 lines
I tried to reproduce this issue on Synaptics SoC, but can't reproduce it.
Per my understanding of the code in kernel/irq/cpuhotplug.c, this warning
happened when we migrate irqs away from the offline cpu, this implicitly implies that before this point the irq has bind to the offline cpu, but how
could this happen given current dw_pci_msi_set_affinity() implementation always return -EINVAL
By default the smp_affinity should be set so that all CPUs can be interrupted ...
Quoted 8 lines
$ cat /proc/irq/70/smp_affinity
0xff
In my case there are 8 CPUs and so 0xff implies that the interrupt can be triggered on any of the 8 CPUs.
Do you see the set_affinity callback being called for the DWC irqchip in migrate_one_irq()?

Quoted 7 lines

The problem is common to all MSI implementations that end up muxing all the end-point MSIs into a single interrupt. With these systems, you cannot set the affinity of individual MSIs (they don't target a CPU, they target another interrupt... braindead). Only the mux interrupt can have its affinity changed.
So returning -EINVAL is the right thing to do.

Right, so if that is the case, then surely there should be some way to avoid these warnings because they are not relevant?

Cheers
Jon

Signature · 1 line

nvpublic

Marc Zyngier

2020-09-29 18:13 UTC #25 lore raw

Quoted 53 lines · Jon Hunter

Quoted 52 lines · Marc Zyngier
Quoted 36 lines · Jon Hunter
Hi Jisheng,
Quoted 20 lines · Jisheng Zhang
Hi Jon,
Quoted 7 lines · On Fri, 25 Sep 2020 09:53:45 +0100 Jon Hunter wrote:
Quoted 4 lines · Jisheng Zhang
Improve the msi code:
1. Add proper error handling.
2. Move dw_pcie_msi_init() from each users to designware host to solve msi page leakage in resume path.
Quoted 8 lines
Apologies if this is slightly off topic, but I have been meaning to ask about MSIs and PCI. On Tegra194 which uses the DWC PCI driver, whenever we
hotplug CPUs we see the following warnings ...
[ 79.068351] WARNING KERN IRQ70: set affinity failed(-22). [ 79.068362] WARNING KERN IRQ71: set affinity failed(-22).
Quoted 9 lines
I tried to reproduce this issue on Synaptics SoC, but can't reproduce it.
Per my understanding of the code in kernel/irq/cpuhotplug.c, this warning
happened when we migrate irqs away from the offline cpu, this implicitly implies that before this point the irq has bind to the offline cpu, but how
could this happen given current dw_pci_msi_set_affinity() implementation always return -EINVAL
By default the smp_affinity should be set so that all CPUs can be interrupted ...
Quoted 8 lines
$ cat /proc/irq/70/smp_affinity
0xff
In my case there are 8 CPUs and so 0xff implies that the interrupt can be triggered on any of the 8 CPUs.
Do you see the set_affinity callback being called for the DWC irqchip in migrate_one_irq()?
The problem is common to all MSI implementations that end up muxing all the end-point MSIs into a single interrupt. With these systems, you cannot set the affinity of individual MSIs (they don't target a CPU, they target another interrupt... braindead). Only the mux interrupt can have its affinity changed.

Quoted 4 lines

Quoted 1 line
So returning -EINVAL is the right thing to do.
Right, so if that is the case, then surely there should be some way to avoid these warnings because they are not relevant?

I don't think there is a way to do this, because the core code doesn't (and cannot) know the exact interrupt topology.

The only alternative would be to change the affinity of the mux interrupt when a MSI affinity changes, but that tends to break userspace (irqbalance, for example).

M.

Signature · 1 line

Jazz is not dead. It just smells funny...

Vidya Sagar

2020-10-06 06:27 UTC #26 lore raw

In reply to Jisheng Zhang

Hi,
I would like to verify this series along with the other series "PCI: dwc: fix two MSI issues" on Tegra194. I tried to apply these series on both linux-next and Lorenzo's pci/dwc branches but there seem to be non trivial conflicts. Could you please tell me which branch I can use and apply these series cleanly? FWIW, I acknowledge that the existing code does leak MSI target page every time system goes through suspend-resume sequence on Tegra194.

Thanks,
Vidya Sagar

Quoted 40 lines · Jisheng Zhang

External email: Use caution opening links or attachments
Improve the msi code:
1. Add proper error handling.
2. Move dw_pcie_msi_init() from each users to designware host to solve msi page leakage in resume path.
Since v1:
- add proper error handling patches.
- solve the msi page leakage by moving dw_pcie_msi_init() from each users to designware host
Jisheng Zhang (5):
PCI: dwc: Call dma_unmap_page() before freeing the msi page
PCI: dwc: Check alloc_page() return value
PCI: dwc: Rename dw_pcie_free_msi to dw_pcie_msi_deinit
PCI: dwc: Skip PCIE_MSI_INTR0* programming if MSI is disabled
PCI: dwc: Move dw_pcie_msi_init() from each users to designware host
  drivers/pci/controller/dwc/pci-dra7xx.c       |  1 +
  drivers/pci/controller/dwc/pci-exynos.c       |  2 -
  drivers/pci/controller/dwc/pci-imx6.c         |  3 --
  drivers/pci/controller/dwc/pci-meson.c        |  8 ----
  drivers/pci/controller/dwc/pcie-artpec6.c     | 10 -----
  .../pci/controller/dwc/pcie-designware-host.c | 43 +++++++++++++------
  .../pci/controller/dwc/pcie-designware-plat.c |  3 --
  drivers/pci/controller/dwc/pcie-designware.h  |  9 +++-
  drivers/pci/controller/dwc/pcie-histb.c       |  3 --
  drivers/pci/controller/dwc/pcie-kirin.c       |  3 --
  drivers/pci/controller/dwc/pcie-qcom.c        |  3 --
  drivers/pci/controller/dwc/pcie-spear13xx.c   |  1 -
  drivers/pci/controller/dwc/pcie-tegra194.c    |  2 -
  drivers/pci/controller/dwc/pcie-uniphier.c    |  9 +---
  14 files changed, 38 insertions(+), 62 deletions(-)
--
2.28.0

Jisheng Zhang

author

2020-10-06 06:37 UTC #27 lore raw

Quoted 3 lines · On Tue, 6 Oct 2020 11:56:34 +0530 Vidya Sagar wrote:

Hi,

Hi,

Quoted 5 lines

I would like to verify this series along with the other series "PCI:
dwc: fix two MSI issues" on Tegra194. I tried to apply these series on both linux-next and Lorenzo's pci/dwc branches but there seem to be non trivial conflicts. Could you please tell me which branch I can use and apply these series cleanly?

This is a fix, so I thought the series would be picked up in v5.9, so the series is patched against v5.9-rcN

could you please try v5 https://lkml.org/lkml/2020/9/29/2511 on v5.9-rc7?

Thanks

Quoted 47 lines

FWIW, I acknowledge that the existing code does leak MSI target page every time system goes through suspend-resume sequence on Tegra194.
Thanks,
Vidya Sagar
Quoted 40 lines · Jisheng Zhang
External email: Use caution opening links or attachments
Improve the msi code:
1. Add proper error handling.
2. Move dw_pcie_msi_init() from each users to designware host to solve msi page leakage in resume path.
Since v1:
- add proper error handling patches.
- solve the msi page leakage by moving dw_pcie_msi_init() from each users to designware host
Jisheng Zhang (5):
PCI: dwc: Call dma_unmap_page() before freeing the msi page
PCI: dwc: Check alloc_page() return value
PCI: dwc: Rename dw_pcie_free_msi to dw_pcie_msi_deinit
PCI: dwc: Skip PCIE_MSI_INTR0* programming if MSI is disabled
PCI: dwc: Move dw_pcie_msi_init() from each users to designware host
  drivers/pci/controller/dwc/pci-dra7xx.c       |  1 +
  drivers/pci/controller/dwc/pci-exynos.c       |  2 -
  drivers/pci/controller/dwc/pci-imx6.c         |  3 --
  drivers/pci/controller/dwc/pci-meson.c        |  8 ----
  drivers/pci/controller/dwc/pcie-artpec6.c     | 10 -----
  .../pci/controller/dwc/pcie-designware-host.c | 43 +++++++++++++------
  .../pci/controller/dwc/pcie-designware-plat.c |  3 --
  drivers/pci/controller/dwc/pcie-designware.h  |  9 +++-
  drivers/pci/controller/dwc/pcie-histb.c       |  3 --
  drivers/pci/controller/dwc/pcie-kirin.c       |  3 --
  drivers/pci/controller/dwc/pcie-qcom.c        |  3 --
  drivers/pci/controller/dwc/pcie-spear13xx.c   |  1 -
  drivers/pci/controller/dwc/pcie-tegra194.c    |  2 -
  drivers/pci/controller/dwc/pcie-uniphier.c    |  9 +---
  14 files changed, 38 insertions(+), 62 deletions(-)
--
2.28.0

Vidya Sagar

2020-10-08 05:33 UTC #28 lore raw

Quoted 17 lines · Jisheng Zhang

External email: Use caution opening links or attachments
Quoted 3 lines · On Tue, 6 Oct 2020 11:56:34 +0530 Vidya Sagar wrote:
Hi,
Hi,
Quoted 5 lines
I would like to verify this series along with the other series "PCI:
dwc: fix two MSI issues" on Tegra194. I tried to apply these series on both linux-next and Lorenzo's pci/dwc branches but there seem to be non trivial conflicts. Could you please tell me which branch I can use and apply these series cleanly?

Quoted 4 lines

This is a fix, so I thought the series would be picked up in v5.9, so the series is patched against v5.9-rcN
could you please try v5 https://lkml.org/lkml/2020/9/29/2511 on v5.9-rc7?

I tried this series on top of v5.9-rc7 and it worked as expected on Tegra194 platform. Also, I couldn't cleanly apply the other series 'PCI: dwc: fix two MSI issues' on top. Could you please rebase them?

Thanks,
Vidya Sagar

Quoted 52 lines

Thanks
Quoted 47 lines
FWIW, I acknowledge that the existing code does leak MSI target page every time system goes through suspend-resume sequence on Tegra194.
Thanks,
Vidya Sagar
Quoted 40 lines · Jisheng Zhang
External email: Use caution opening links or attachments
Improve the msi code:
1. Add proper error handling.
2. Move dw_pcie_msi_init() from each users to designware host to solve msi page leakage in resume path.
Since v1:
- add proper error handling patches.
- solve the msi page leakage by moving dw_pcie_msi_init() from each users to designware host
Jisheng Zhang (5):
PCI: dwc: Call dma_unmap_page() before freeing the msi page
PCI: dwc: Check alloc_page() return value
PCI: dwc: Rename dw_pcie_free_msi to dw_pcie_msi_deinit
PCI: dwc: Skip PCIE_MSI_INTR0* programming if MSI is disabled
PCI: dwc: Move dw_pcie_msi_init() from each users to designware host
   drivers/pci/controller/dwc/pci-dra7xx.c       |  1 +
   drivers/pci/controller/dwc/pci-exynos.c       |  2 -
   drivers/pci/controller/dwc/pci-imx6.c         |  3 --
   drivers/pci/controller/dwc/pci-meson.c        |  8 ----
   drivers/pci/controller/dwc/pcie-artpec6.c     | 10 -----
   .../pci/controller/dwc/pcie-designware-host.c | 43 +++++++++++++------
   .../pci/controller/dwc/pcie-designware-plat.c |  3 --
   drivers/pci/controller/dwc/pcie-designware.h  |  9 +++-
   drivers/pci/controller/dwc/pcie-histb.c       |  3 --
   drivers/pci/controller/dwc/pcie-kirin.c       |  3 --
   drivers/pci/controller/dwc/pcie-qcom.c        |  3 --
   drivers/pci/controller/dwc/pcie-spear13xx.c   |  1 -
   drivers/pci/controller/dwc/pcie-tegra194.c    |  2 -
   drivers/pci/controller/dwc/pcie-uniphier.c    |  9 +---
   14 files changed, 38 insertions(+), 62 deletions(-)
--
2.28.0

Jisheng Zhang

author

2020-10-09 08:38 UTC #29 lore raw

PATCH 1/1 PCI: dwc: Move dw_pcie_msi_init() from each users to designware host

Let the designware host take care the integrated msi init rather than duplicate dw_pcie_msi_init() in each users.

Signed-off-by Jisheng Zhang <[email protected]>

---

Hi Vidya,

After V7, only this patch is left, others in v2 are not needed. There's one more clean up chance -- we can also move dw_pcie_free_msi() to designware host and make it static if we can clean up dra7xx. I see Rob is working on some larger MSI clean-ups, maybe this will be done in his clean-ups.

Thanks

 drivers/pci/controller/dwc/pci-dra7xx.c           | 1 -
 drivers/pci/controller/dwc/pci-exynos.c           | 2 --
 drivers/pci/controller/dwc/pci-imx6.c             | 1 -
 drivers/pci/controller/dwc/pci-meson.c            | 2 --
 drivers/pci/controller/dwc/pcie-artpec6.c         | 1 -
 drivers/pci/controller/dwc/pcie-designware-host.c | 5 +++--
 drivers/pci/controller/dwc/pcie-designware-plat.c | 1 -
 drivers/pci/controller/dwc/pcie-designware.h      | 5 -----
 drivers/pci/controller/dwc/pcie-histb.c           | 1 -
 drivers/pci/controller/dwc/pcie-kirin.c           | 1 -
 drivers/pci/controller/dwc/pcie-qcom.c            | 1 -
 drivers/pci/controller/dwc/pcie-spear13xx.c       | 4 +---
 drivers/pci/controller/dwc/pcie-tegra194.c        | 2 --
 drivers/pci/controller/dwc/pcie-uniphier.c        | 2 --
 14 files changed, 4 insertions(+), 25 deletions(-)

drivers/pci/controller/dwc/pci-dra7xx.c +0 -1

index 6d012d2b1e90..a5edaa6b6b58 100644
@@ -185,7 +185,6 @@ static int dra7xx_pcie_host_init(struct pcie_port *pp)
 
 	dra7xx_pcie_establish_link(pci);
 	dw_pcie_wait_for_link(pci);
-	dw_pcie_msi_init(pp);
 	dra7xx_pcie_enable_interrupts(dra7xx);
 
 	return 0;

drivers/pci/controller/dwc/pci-exynos.c +0 -2

index 242683cde04a..97c166885277 100644
@@ -298,8 +298,6 @@ static void exynos_pcie_msi_init(struct exynos_pcie *ep)
 	struct pcie_port *pp = &pci->pp;
 	u32 val;
 
-	dw_pcie_msi_init(pp);
-
 	/* enable MSI interrupt */
 	val = exynos_pcie_readl(ep->mem_res->elbi_base, PCIE_IRQ_EN_LEVEL);
 	val |= IRQ_MSI_ENABLE;

drivers/pci/controller/dwc/pci-imx6.c +0 -1

index 337c74cbdfdb..cf52eb5d7d2e 100644
@@ -836,7 +836,6 @@ static int imx6_pcie_host_init(struct pcie_port *pp)
 	imx6_setup_phy_mpll(imx6_pcie);
 	dw_pcie_setup_rc(pp);
 	imx6_pcie_establish_link(imx6_pcie);
-	dw_pcie_msi_init(pp);
 
 	return 0;
 }

drivers/pci/controller/dwc/pci-meson.c +0 -2

index 33deb290c4e7..11bfc42fac1c 100644
@@ -387,8 +387,6 @@ static int meson_pcie_host_init(struct pcie_port *pp)
 	if (ret)
 		return ret;
 
-	dw_pcie_msi_init(pp);
-
 	return 0;
 }

drivers/pci/controller/dwc/pcie-artpec6.c +0 -1

index 929448e9e0bc..73d4bf99c737 100644
@@ -331,7 +331,6 @@ static int artpec6_pcie_host_init(struct pcie_port *pp)
 	dw_pcie_setup_rc(pp);
 	artpec6_pcie_establish_link(pci);
 	dw_pcie_wait_for_link(pci);
-	dw_pcie_msi_init(pp);
 
 	return 0;
 }

drivers/pci/controller/dwc/pcie-designware-host.c +3 -2

index d02c7e74738d..7622f114223e 100644
@@ -275,7 +275,7 @@ void dw_pcie_free_msi(struct pcie_port *pp)
 	}
 }
 
-void dw_pcie_msi_init(struct pcie_port *pp)
+static void dw_pcie_msi_init(struct pcie_port *pp)
 {
 	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
 	u64 msi_target = (u64)pp->msi_data;

@@ -287,7 +287,6 @@ void dw_pcie_msi_init(struct pcie_port *pp)
 	dw_pcie_writel_dbi(pci, PCIE_MSI_ADDR_LO, lower_32_bits(msi_target));
 	dw_pcie_writel_dbi(pci, PCIE_MSI_ADDR_HI, upper_32_bits(msi_target));
 }
-EXPORT_SYMBOL_GPL(dw_pcie_msi_init);
 
 int dw_pcie_host_init(struct pcie_port *pp)
 {

@@ -545,6 +544,8 @@ void dw_pcie_setup_rc(struct pcie_port *pp)
 					    ~0);
 		}
 	}
+	if (pci_msi_enabled() && pp->msi_data)
+		dw_pcie_msi_init(pp);
 
 	/* Setup RC BARs */
 	dw_pcie_writel_dbi(pci, PCI_BASE_ADDRESS_0, 0x00000004);

drivers/pci/controller/dwc/pcie-designware-plat.c +0 -1

index e3e300669ed5..9ccf69a3dcf4 100644
@@ -39,7 +39,6 @@ static int dw_plat_pcie_host_init(struct pcie_port *pp)
 
 	dw_pcie_setup_rc(pp);
 	dw_pcie_wait_for_link(pci);
-	dw_pcie_msi_init(pp);
 
 	return 0;
 }

drivers/pci/controller/dwc/pcie-designware.h +0 -5

index 9d2f511f13fa..f9f6b276a11a 100644
@@ -365,7 +365,6 @@ static inline void dw_pcie_dbi_ro_wr_dis(struct dw_pcie *pci)
 
 #ifdef CONFIG_PCIE_DW_HOST
 irqreturn_t dw_handle_msi_irq(struct pcie_port *pp);
-void dw_pcie_msi_init(struct pcie_port *pp);
 void dw_pcie_free_msi(struct pcie_port *pp);
 void dw_pcie_setup_rc(struct pcie_port *pp);
 int dw_pcie_host_init(struct pcie_port *pp);

@@ -379,10 +378,6 @@ static inline irqreturn_t dw_handle_msi_irq(struct pcie_port *pp)
 	return IRQ_NONE;
 }
 
-static inline void dw_pcie_msi_init(struct pcie_port *pp)
-{
-}
-
 static inline void dw_pcie_free_msi(struct pcie_port *pp)
 {
 }

drivers/pci/controller/dwc/pcie-histb.c +0 -1

index afc1abbe49aa..aa9eaee2c4bd 100644
@@ -202,7 +202,6 @@ static int histb_pcie_host_init(struct pcie_port *pp)
 	pp->bridge->ops = &histb_pci_ops;
 
 	histb_pcie_establish_link(pp);
-	dw_pcie_msi_init(pp);
 
 	return 0;
 }

drivers/pci/controller/dwc/pcie-kirin.c +0 -1

index 6f01ae013326..dc30e43a6be9 100644
@@ -429,7 +429,6 @@ static int kirin_pcie_host_init(struct pcie_port *pp)
 	pp->bridge->ops = &kirin_pci_ops;
 
 	kirin_pcie_establish_link(pp);
-	dw_pcie_msi_init(pp);
 
 	return 0;
 }

drivers/pci/controller/dwc/pcie-qcom.c +0 -1

index 5eb28251dbee..4f66e534e011 100644
@@ -1271,7 +1271,6 @@ static int qcom_pcie_host_init(struct pcie_port *pp)
 	}
 
 	dw_pcie_setup_rc(pp);
-	dw_pcie_msi_init(pp);
 
 	qcom_ep_reset_deassert(pcie);

drivers/pci/controller/dwc/pcie-spear13xx.c +1 -3

index e348225f651f..c75550573a1e 100644
@@ -129,11 +129,9 @@ static void spear13xx_pcie_enable_interrupts(struct spear13xx_pcie *spear13xx_pc
 	struct pcie_app_reg *app_reg = spear13xx_pcie->app_base;
 
 	/* Enable MSI interrupt */
-	if (IS_ENABLED(CONFIG_PCI_MSI)) {
-		dw_pcie_msi_init(pp);
+	if (IS_ENABLED(CONFIG_PCI_MSI))
 		writel(readl(&app_reg->int_mask) |
 				MSI_CTRL_INT, &app_reg->int_mask);
-	}
 }
 
 static int spear13xx_pcie_link_up(struct dw_pcie *pci)

drivers/pci/controller/dwc/pcie-tegra194.c +0 -2

index aa511ec0d800..b093be21cab2 100644
@@ -772,8 +772,6 @@ static void tegra_pcie_enable_msi_interrupts(struct pcie_port *pp)
 	struct tegra_pcie_dw *pcie = to_tegra_pcie(pci);
 	u32 val;
 
-	dw_pcie_msi_init(pp);
-
 	/* Enable MSI interrupt generation */
 	val = appl_readl(pcie, APPL_INTR_EN_L0_0);
 	val |= APPL_INTR_EN_L0_0_SYS_MSI_INTR_EN;

drivers/pci/controller/dwc/pcie-uniphier.c +0 -2

index 48176265c867..c19bdfed4337 100644
@@ -322,8 +322,6 @@ static int uniphier_pcie_host_init(struct pcie_port *pp)
 	if (ret)
 		return ret;
 
-	dw_pcie_msi_init(pp);
-
 	return 0;
 }

Signature · 1 line

2.28.0