linux-pci.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v5 0/2] PCI: dwc: fix two MSI issues
@ 2020-09-30  1:12 Jisheng Zhang
  2020-09-30  1:12 ` [PATCH v5 1/2] PCI: dwc: Skip PCIE_MSI_INTR0* programming if MSI is disabled Jisheng Zhang
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Jisheng Zhang @ 2020-09-30  1:12 UTC (permalink / raw)
  To: Kishon Vijay Abraham I, Lorenzo Pieralisi, Rob Herring,
	Bjorn Helgaas, Jingoo Han, Gustavo Pimentel
  Cc: linux-omap, linux-pci, linux-arm-kernel, linux-kernel

Fix two MSI issues. One to skip PCIE_MSI_INTR0* programming if MSI is
disabled, another to use an address in the driver data for MSI address,
to fix the MSI page leakage during suspend/resume.

Since v4:
  - fix pci-dra7xx.c

Since v3:
  - add Acked-by tag
  - change patch2 commit msg to make it clear
  - map the MSI msg with dma_map_single_attrs() for some platforms
    which either has seperate addrs for dma and phy or has mem access
    limitation for the PCIe.

Since v2:
  - add Acked-by tag
  - use an address in the driver data for MSI address. Thank Ard and Rob
    for pointing out this correct direction.
  - Since the MSI page has gone, the leak issue doesn't exist anymore,
    remove unnecessary patches.
  - Remove dw_pcie_free_msi rename and the last patch. They could be
    targeted to next. So will send out patches in a separate series.

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 (2):
  PCI: dwc: Skip PCIE_MSI_INTR0* programming if MSI is disabled
  PCI: dwc: Fix MSI page leakage in suspend/resume

 drivers/pci/controller/dwc/pci-dra7xx.c       | 18 +++++++++-
 .../pci/controller/dwc/pcie-designware-host.c | 34 +++++++++----------
 drivers/pci/controller/dwc/pcie-designware.h  |  2 +-
 3 files changed, 35 insertions(+), 19 deletions(-)

-- 
2.28.0


^ permalink raw reply	[flat|nested] 6+ messages in thread
* [PATCH v5 1/2] PCI: dwc: Skip PCIE_MSI_INTR0* programming if MSI is disabled
  2020-09-30  1:12 [PATCH v5 0/2] PCI: dwc: fix two MSI issues Jisheng Zhang
@ 2020-09-30  1:12 ` Jisheng Zhang
  2020-09-30  1:15 ` [PATCH v5 2/2] PCI: dwc: Fix MSI page leakage in suspend/resume Jisheng Zhang
  2020-10-08 11:36 ` [PATCH v5 0/2] PCI: dwc: fix two MSI issues Lorenzo Pieralisi
  2 siblings, 0 replies; 6+ messages in thread
From: Jisheng Zhang @ 2020-09-30  1:12 UTC (permalink / raw)
  To: Kishon Vijay Abraham I, Lorenzo Pieralisi, Rob Herring,
	Bjorn Helgaas, Jingoo Han, Gustavo Pimentel
  Cc: linux-omap, linux-pci, linux-arm-kernel, linux-kernel

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 <Jisheng.Zhang@synaptics.com>
Reviewed-by: Rob Herring <robh@kernel.org>
Acked-by: Gustavo Pimentel <gustavo.pimentel@synopsys.com>
---
 drivers/pci/controller/dwc/pcie-designware-host.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
index 9dafecba347f..f08f4d97f321 100644
--- a/drivers/pci/controller/dwc/pcie-designware-host.c
+++ b/drivers/pci/controller/dwc/pcie-designware-host.c
@@ -632,7 +632,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 */
-- 
2.28.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread
* [PATCH v5 2/2] PCI: dwc: Fix MSI page leakage in suspend/resume
  2020-09-30  1:12 [PATCH v5 0/2] PCI: dwc: fix two MSI issues Jisheng Zhang
  2020-09-30  1:12 ` [PATCH v5 1/2] PCI: dwc: Skip PCIE_MSI_INTR0* programming if MSI is disabled Jisheng Zhang
@ 2020-09-30  1:15 ` Jisheng Zhang
  2020-10-07 17:41   ` Rob Herring
  2020-10-08 11:36 ` [PATCH v5 0/2] PCI: dwc: fix two MSI issues Lorenzo Pieralisi
  2 siblings, 1 reply; 6+ messages in thread
From: Jisheng Zhang @ 2020-09-30  1:15 UTC (permalink / raw)
  To: Kishon Vijay Abraham I, Lorenzo Pieralisi, Rob Herring,
	Bjorn Helgaas, Jingoo Han, Gustavo Pimentel
  Cc: linux-omap, linux-pci, linux-arm-kernel, linux-kernel

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
msi page will be leaked.

As pointed out by Rob and Ard, there's no need to allocate a page for
the MSI address, we could use an address in the driver data.

To avoid map the MSI msg again during resume, we move the map MSI msg
from dw_pcie_msi_init() to dw_pcie_host_init().

Signed-off-by: Jisheng Zhang <Jisheng.Zhang@synaptics.com>
---
 drivers/pci/controller/dwc/pci-dra7xx.c       | 18 ++++++++++-
 .../pci/controller/dwc/pcie-designware-host.c | 32 +++++++++----------
 drivers/pci/controller/dwc/pcie-designware.h  |  2 +-
 3 files changed, 34 insertions(+), 18 deletions(-)

diff --git a/drivers/pci/controller/dwc/pci-dra7xx.c b/drivers/pci/controller/dwc/pci-dra7xx.c
index dc387724cf08..a0b2deb7f628 100644
--- a/drivers/pci/controller/dwc/pci-dra7xx.c
+++ b/drivers/pci/controller/dwc/pci-dra7xx.c
@@ -490,7 +490,9 @@ static struct irq_chip dra7xx_pci_msi_bottom_irq_chip = {
 static int dra7xx_pcie_msi_host_init(struct pcie_port *pp)
 {
 	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
+	struct device *dev = pci->dev;
 	u32 ctrl, num_ctrls;
+	int ret;
 
 	pp->msi_irq_chip = &dra7xx_pci_msi_bottom_irq_chip;
 
@@ -506,7 +508,21 @@ static int dra7xx_pcie_msi_host_init(struct pcie_port *pp)
 				    ~0);
 	}
 
-	return dw_pcie_allocate_domains(pp);
+	ret = dw_pcie_allocate_domains(pp);
+	if (ret)
+		return ret;
+
+	pp->msi_data = dma_map_single_attrs(dev, &pp->msi_msg,
+					   sizeof(pp->msi_msg),
+					   DMA_FROM_DEVICE,
+					   DMA_ATTR_SKIP_CPU_SYNC);
+	ret = dma_mapping_error(dev, pp->msi_data);
+	if (ret) {
+		dev_err(dev, "Failed to map MSI data\n");
+		pp->msi_data = 0;
+		dw_pcie_free_msi(pp);
+	}
+	return ret;
 }
 
 static const struct dw_pcie_host_ops dra7xx_pcie_host_ops = {
diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
index f08f4d97f321..27bbeee5718f 100644
--- a/drivers/pci/controller/dwc/pcie-designware-host.c
+++ b/drivers/pci/controller/dwc/pcie-designware-host.c
@@ -288,26 +288,17 @@ 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)
-		__free_page(pp->msi_page);
+	if (pp->msi_data) {
+		struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
+		struct device *dev = pci->dev;
+		dma_unmap_single_attrs(dev, pp->msi_data, sizeof(pp->msi_msg),
+				       DMA_FROM_DEVICE, DMA_ATTR_SKIP_CPU_SYNC);
+	}
 }
 
 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);
-	pp->msi_data = dma_map_page(dev, pp->msi_page, 0, PAGE_SIZE,
-				    DMA_FROM_DEVICE);
-	if (dma_mapping_error(dev, pp->msi_data)) {
-		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;
+	u64 msi_target = (u64)pp->msi_data;
 
 	/* Program the msi_data */
 	dw_pcie_wr_own_conf(pp, PCIE_MSI_ADDR_LO, 4,
@@ -440,6 +431,15 @@ int dw_pcie_host_init(struct pcie_port *pp)
 				irq_set_chained_handler_and_data(pp->msi_irq,
 							    dw_chained_msi_isr,
 							    pp);
+			pp->msi_data = dma_map_single_attrs(pci->dev, &pp->msi_msg,
+						      sizeof(pp->msi_msg),
+						      DMA_FROM_DEVICE,
+						      DMA_ATTR_SKIP_CPU_SYNC);
+			if (dma_mapping_error(pci->dev, pp->msi_data)) {
+				dev_err(pci->dev, "Failed to map MSI data\n");
+				pp->msi_data = 0;
+				goto err_free_msi;
+			}
 		} else {
 			ret = pp->ops->msi_host_init(pp);
 			if (ret < 0)
diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
index f911760dcc69..cd9e76904c58 100644
--- a/drivers/pci/controller/dwc/pcie-designware.h
+++ b/drivers/pci/controller/dwc/pcie-designware.h
@@ -194,8 +194,8 @@ struct pcie_port {
 	int			msi_irq;
 	struct irq_domain	*irq_domain;
 	struct irq_domain	*msi_domain;
+	u16			msi_msg;
 	dma_addr_t		msi_data;
-	struct page		*msi_page;
 	struct irq_chip		*msi_irq_chip;
 	u32			num_vectors;
 	u32			irq_mask[MAX_MSI_CTRLS];
-- 
2.28.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread
* Re: [PATCH v5 2/2] PCI: dwc: Fix MSI page leakage in suspend/resume
  2020-09-30  1:15 ` [PATCH v5 2/2] PCI: dwc: Fix MSI page leakage in suspend/resume Jisheng Zhang
@ 2020-10-07 17:41   ` Rob Herring
  0 siblings, 0 replies; 6+ messages in thread
From: Rob Herring @ 2020-10-07 17:41 UTC (permalink / raw)
  To: Jisheng Zhang
  Cc: Kishon Vijay Abraham I, Lorenzo Pieralisi, Bjorn Helgaas,
	Jingoo Han, Gustavo Pimentel, linux-omap, linux-pci,
	linux-arm-kernel, linux-kernel

On Wed, Sep 30, 2020 at 09:15:25AM +0800, Jisheng Zhang wrote:
> 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
> msi page will be leaked.
> 
> As pointed out by Rob and Ard, there's no need to allocate a page for
> the MSI address, we could use an address in the driver data.
> 
> To avoid map the MSI msg again during resume, we move the map MSI msg
> from dw_pcie_msi_init() to dw_pcie_host_init().
> 
> Signed-off-by: Jisheng Zhang <Jisheng.Zhang@synaptics.com>
> ---
>  drivers/pci/controller/dwc/pci-dra7xx.c       | 18 ++++++++++-
>  .../pci/controller/dwc/pcie-designware-host.c | 32 +++++++++----------
>  drivers/pci/controller/dwc/pcie-designware.h  |  2 +-
>  3 files changed, 34 insertions(+), 18 deletions(-)

I'm working on some larger MSI clean-ups which should eliminate the 
dra7xx addition, but this is good enough for now.

Reviewed-by: Rob Herring <robh@kernel.org>

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH v5 0/2] PCI: dwc: fix two MSI issues
  2020-09-30  1:12 [PATCH v5 0/2] PCI: dwc: fix two MSI issues Jisheng Zhang
  2020-09-30  1:12 ` [PATCH v5 1/2] PCI: dwc: Skip PCIE_MSI_INTR0* programming if MSI is disabled Jisheng Zhang
  2020-09-30  1:15 ` [PATCH v5 2/2] PCI: dwc: Fix MSI page leakage in suspend/resume Jisheng Zhang
@ 2020-10-08 11:36 ` Lorenzo Pieralisi
  2020-10-09  7:10   ` Jisheng Zhang
  2 siblings, 1 reply; 6+ messages in thread
From: Lorenzo Pieralisi @ 2020-10-08 11:36 UTC (permalink / raw)
  To: Jisheng Zhang
  Cc: Kishon Vijay Abraham I, Rob Herring, Bjorn Helgaas, Jingoo Han,
	Gustavo Pimentel, linux-omap, linux-pci, linux-arm-kernel,
	linux-kernel

On Wed, Sep 30, 2020 at 09:12:05AM +0800, Jisheng Zhang wrote:
> Fix two MSI issues. One to skip PCIE_MSI_INTR0* programming if MSI is
> disabled, another to use an address in the driver data for MSI address,
> to fix the MSI page leakage during suspend/resume.
> 
> Since v4:
>   - fix pci-dra7xx.c
> 
> Since v3:
>   - add Acked-by tag
>   - change patch2 commit msg to make it clear
>   - map the MSI msg with dma_map_single_attrs() for some platforms
>     which either has seperate addrs for dma and phy or has mem access
>     limitation for the PCIe.
> 
> Since v2:
>   - add Acked-by tag
>   - use an address in the driver data for MSI address. Thank Ard and Rob
>     for pointing out this correct direction.
>   - Since the MSI page has gone, the leak issue doesn't exist anymore,
>     remove unnecessary patches.
>   - Remove dw_pcie_free_msi rename and the last patch. They could be
>     targeted to next. So will send out patches in a separate series.
> 
> 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 (2):
>   PCI: dwc: Skip PCIE_MSI_INTR0* programming if MSI is disabled
>   PCI: dwc: Fix MSI page leakage in suspend/resume
> 
>  drivers/pci/controller/dwc/pci-dra7xx.c       | 18 +++++++++-
>  .../pci/controller/dwc/pcie-designware-host.c | 34 +++++++++----------
>  drivers/pci/controller/dwc/pcie-designware.h  |  2 +-
>  3 files changed, 35 insertions(+), 19 deletions(-)

Hi,

can you rebase this series against my pci/dwc branch please ?

Thanks,
Lorenzo

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH v5 0/2] PCI: dwc: fix two MSI issues
  2020-10-08 11:36 ` [PATCH v5 0/2] PCI: dwc: fix two MSI issues Lorenzo Pieralisi
@ 2020-10-09  7:10   ` Jisheng Zhang
  0 siblings, 0 replies; 6+ messages in thread
From: Jisheng Zhang @ 2020-10-09  7:10 UTC (permalink / raw)
  To: Lorenzo Pieralisi
  Cc: Kishon Vijay Abraham I, Rob Herring, Bjorn Helgaas, Jingoo Han,
	Gustavo Pimentel, linux-omap, linux-pci, linux-arm-kernel,
	linux-kernel

On Thu, 8 Oct 2020 12:36:14 +0100 Lorenzo Pieralisi wrote:

> 
> On Wed, Sep 30, 2020 at 09:12:05AM +0800, Jisheng Zhang wrote:
> > Fix two MSI issues. One to skip PCIE_MSI_INTR0* programming if MSI is
> > disabled, another to use an address in the driver data for MSI address,
> > to fix the MSI page leakage during suspend/resume.
> >
> > Since v4:
> >   - fix pci-dra7xx.c
> >
> > Since v3:
> >   - add Acked-by tag
> >   - change patch2 commit msg to make it clear
> >   - map the MSI msg with dma_map_single_attrs() for some platforms
> >     which either has seperate addrs for dma and phy or has mem access
> >     limitation for the PCIe.
> >
> > Since v2:
> >   - add Acked-by tag
> >   - use an address in the driver data for MSI address. Thank Ard and Rob
> >     for pointing out this correct direction.
> >   - Since the MSI page has gone, the leak issue doesn't exist anymore,
> >     remove unnecessary patches.
> >   - Remove dw_pcie_free_msi rename and the last patch. They could be
> >     targeted to next. So will send out patches in a separate series.
> >
> > 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 (2):
> >   PCI: dwc: Skip PCIE_MSI_INTR0* programming if MSI is disabled
> >   PCI: dwc: Fix MSI page leakage in suspend/resume
> >
> >  drivers/pci/controller/dwc/pci-dra7xx.c       | 18 +++++++++-
> >  .../pci/controller/dwc/pcie-designware-host.c | 34 +++++++++----------
> >  drivers/pci/controller/dwc/pcie-designware.h  |  2 +-
> >  3 files changed, 35 insertions(+), 19 deletions(-)  
> 
> Hi,
> 
> can you rebase this series against my pci/dwc branch please ?
> 

Done in v6.

Thanks

^ permalink raw reply	[flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-10-09  7:11 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-30  1:12 [PATCH v5 0/2] PCI: dwc: fix two MSI issues Jisheng Zhang
2020-09-30  1:12 ` [PATCH v5 1/2] PCI: dwc: Skip PCIE_MSI_INTR0* programming if MSI is disabled Jisheng Zhang
2020-09-30  1:15 ` [PATCH v5 2/2] PCI: dwc: Fix MSI page leakage in suspend/resume Jisheng Zhang
2020-10-07 17:41   ` Rob Herring
2020-10-08 11:36 ` [PATCH v5 0/2] PCI: dwc: fix two MSI issues Lorenzo Pieralisi
2020-10-09  7:10   ` Jisheng Zhang

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).