PCI: dwc: fix two MSI issues
Subsystems (4)
v5 cover PCI: dwc: fix two MSI issues
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(-)
Signature · 1 line
2.28.0
PATCH v5 1/2 PCI: dwc: Skip PCIE_MSI_INTR0* programming if MSI is disabled
If MSI is disabled, there's no need to program PCIE_MSI_INTR0_MASK and PCIE_MSI_INTR0_ENABLE registers.
--- 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
@@ -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 */
Signature · 1 line
2.28.0
PATCH v5 2/2 PCI: dwc: Fix MSI page leakage in suspend/resume
In reply to Jisheng ZhangCurrently, 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().
--- 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(-)
drivers/pci/controller/dwc/pci-dra7xx.c +17 -1
@@ -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 = {
drivers/pci/controller/dwc/pcie-designware-host.c +16 -16
@@ -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 )
drivers/pci/controller/dwc/pcie-designware.h +1 -1
@@ -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 ];
Signature · 1 line
2.28.0
Re: [PATCH v5 2/2] PCI: dwc: Fix MSI page leakage in suspend/resume
Quoted 13 lines · Jisheng Zhang
Currently,
dw_pcie_msi_init()allocates and maps page for msi, then program thePCIE_MSI_ADDR_LOandPCIE_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) callsdw_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()todw_pcie_host_init().
Quoted 6 lines
Signed-off-by Jisheng Zhang <[email protected]>--- 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.
Quoted 29 lines · Jisheng Zhang
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:
- fixpci-dra7xx.cSince v3:
- add Acked-by tag
- change patch2 commit msg to make it clear
- map the MSI msg withdma_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.
- Removedw_pcie_free_msirename 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 movingdw_pcie_msi_init()from each users to designware host
Quoted 8 lines
Jisheng Zhang (2):
PCI: dwc: Skip PCIE_MSI_INTR0* programming if MSI is disabled
PCI: dwc: Fix MSI page leakage in suspend/resumedrivers/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
Quoted 35 lines · On Thu, 8 Oct 2020 12:36:14 +0100 Lorenzo Pieralisi wrote:
Quoted 33 lines · Jisheng Zhang
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:
- fixpci-dra7xx.cSince v3:
- add Acked-by tag
- change patch2 commit msg to make it clear
- map the MSI msg withdma_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.
- Removedw_pcie_free_msirename 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 movingdw_pcie_msi_init()from each users to designware hostJisheng Zhang (2):
PCI: dwc: Skip PCIE_MSI_INTR0* programming if MSI is disabled
PCI: dwc: Fix MSI page leakage in suspend/resume
Quoted 9 lines
Quoted 4 lines
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