linux-pci.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH V3] PCI: tegra: Use the DMA-API to get the MSI address
@ 2019-04-16 10:51 Vidya Sagar
  2019-04-16 11:50 ` Robin Murphy
  2019-04-16 14:50 ` Lorenzo Pieralisi
  0 siblings, 2 replies; 3+ messages in thread
From: Vidya Sagar @ 2019-04-16 10:51 UTC (permalink / raw)
  To: bhelgaas, lorenzo.pieralisi, robin.murphy, treding, swarren,
	jonathanh, dev
  Cc: mperttunen, linux-tegra, linux-pci, kthota, mmaddireddy, vidyas,
	sagar.tv

Since the upstream MSI memory writes are generated by downstream devices,
it is logically correct to have MSI target memory coming from the DMA pool
reserved for PCIe than from the general memory pool reserved for CPU
access. This avoids PCIe DMA addresses coinciding with MSI target address
thereby raising unwanted MSI interrupts. This patch also enforces to limit
the MSI target address to 32-bits to make it work for PCIe endponits that
support only 32-bit MSI target address and those endpoints that support
64-bit MSI target address anyway work with 32-bit MSI target address.

Signed-off-by: Vidya Sagar <vidyas@nvidia.com>
Reviewed-by: Thierry Reding <treding@nvidia.com>
Acked-by: Thierry Reding <treding@nvidia.com>
---
v3:
* replaced dma_alloc_coherent(), dma_free_coherent() APIs with
  dma_alloc_attrs(), dma_free_attrs() APIs along with
  DMA_ATTR_NO_KERNEL_MAPPING flag to indicate that there is no
  need to create kernel mapping for it.

v2:
* changed 'phys' type to 'dma_addr_t' from 'u64'
* added a comment on why DMA mask is set to 32-bit
* replaced 'dma' with 'DMA'

 drivers/pci/controller/pci-tegra.c | 37 ++++++++++++++++++++++--------
 1 file changed, 28 insertions(+), 9 deletions(-)

diff --git a/drivers/pci/controller/pci-tegra.c b/drivers/pci/controller/pci-tegra.c
index f4f53d092e00..464ba2538d52 100644
--- a/drivers/pci/controller/pci-tegra.c
+++ b/drivers/pci/controller/pci-tegra.c
@@ -231,9 +231,9 @@ struct tegra_msi {
 	struct msi_controller chip;
 	DECLARE_BITMAP(used, INT_PCI_MSI_NR);
 	struct irq_domain *domain;
-	unsigned long pages;
 	struct mutex lock;
-	u64 phys;
+	void *virt;
+	dma_addr_t phys;
 	int irq;
 };
 
@@ -1536,7 +1536,7 @@ static int tegra_pcie_msi_setup(struct tegra_pcie *pcie)
 	err = platform_get_irq_byname(pdev, "msi");
 	if (err < 0) {
 		dev_err(dev, "failed to get IRQ: %d\n", err);
-		goto err;
+		goto free_irq_domain;
 	}
 
 	msi->irq = err;
@@ -1545,17 +1545,35 @@ static int tegra_pcie_msi_setup(struct tegra_pcie *pcie)
 			  tegra_msi_irq_chip.name, pcie);
 	if (err < 0) {
 		dev_err(dev, "failed to request IRQ: %d\n", err);
-		goto err;
+		goto free_irq_domain;
+	}
+
+	/* Though the PCIe controller can address >32-bit address space, to
+	 * facilitate endpoints that support only 32-bit MSI target address,
+	 * the mask is set to 32-bit to make sure that MSI target address is
+	 * always a 32-bit address
+	 */
+	err = dma_set_coherent_mask(dev, DMA_BIT_MASK(32));
+	if (err < 0) {
+		dev_err(dev, "failed to set DMA coherent mask: %d\n", err);
+		goto free_irq;
+	}
+
+	msi->virt = dma_alloc_attrs(dev, PAGE_SIZE, &msi->phys, GFP_KERNEL,
+				    DMA_ATTR_NO_KERNEL_MAPPING);
+	if (!msi->virt) {
+		dev_err(dev, "failed to allocate DMA memory for MSI\n");
+		err = -ENOMEM;
+		goto free_irq;
 	}
 
-	/* setup AFI/FPCI range */
-	msi->pages = __get_free_pages(GFP_KERNEL, 0);
-	msi->phys = virt_to_phys((void *)msi->pages);
 	host->msi = &msi->chip;
 
 	return 0;
 
-err:
+free_irq:
+	free_irq(msi->irq, pcie);
+free_irq_domain:
 	irq_domain_remove(msi->domain);
 	return err;
 }
@@ -1592,7 +1610,8 @@ static void tegra_pcie_msi_teardown(struct tegra_pcie *pcie)
 	struct tegra_msi *msi = &pcie->msi;
 	unsigned int i, irq;
 
-	free_pages(msi->pages, 0);
+	dma_free_attrs(pcie->dev, PAGE_SIZE, msi->virt, msi->phys,
+		       DMA_ATTR_NO_KERNEL_MAPPING);
 
 	if (msi->irq > 0)
 		free_irq(msi->irq, pcie);
-- 
2.17.1


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

* Re: [PATCH V3] PCI: tegra: Use the DMA-API to get the MSI address
  2019-04-16 10:51 [PATCH V3] PCI: tegra: Use the DMA-API to get the MSI address Vidya Sagar
@ 2019-04-16 11:50 ` Robin Murphy
  2019-04-16 14:50 ` Lorenzo Pieralisi
  1 sibling, 0 replies; 3+ messages in thread
From: Robin Murphy @ 2019-04-16 11:50 UTC (permalink / raw)
  To: Vidya Sagar, bhelgaas, lorenzo.pieralisi, treding, swarren,
	jonathanh, dev
  Cc: mperttunen, linux-tegra, linux-pci, kthota, mmaddireddy, sagar.tv

On 16/04/2019 11:51, Vidya Sagar wrote:
> Since the upstream MSI memory writes are generated by downstream devices,
> it is logically correct to have MSI target memory coming from the DMA pool
> reserved for PCIe than from the general memory pool reserved for CPU
> access. This avoids PCIe DMA addresses coinciding with MSI target address
> thereby raising unwanted MSI interrupts. This patch also enforces to limit
> the MSI target address to 32-bits to make it work for PCIe endponits that
> support only 32-bit MSI target address and those endpoints that support
> 64-bit MSI target address anyway work with 32-bit MSI target address.

Although I hope we can solve the general problem even better in future,

Reviewed-by: Robin Murphy <robin.murphy@arm.com>

Since I took a look at the wider context, it also seems a little suspect 
that the tegra_pcie_msi_teardown() call from tegra_pcie_remove() is 
gated by IS_ENABLED(CONFIG_PCI_MSI), whereas tegra_pcie_msi_setup() 
seems to be called from tegra_pcie_probe() unconditionally, but that's 
not this patch's problem.

Robin.

> Signed-off-by: Vidya Sagar <vidyas@nvidia.com>
> Reviewed-by: Thierry Reding <treding@nvidia.com>
> Acked-by: Thierry Reding <treding@nvidia.com>
> ---
> v3:
> * replaced dma_alloc_coherent(), dma_free_coherent() APIs with
>    dma_alloc_attrs(), dma_free_attrs() APIs along with
>    DMA_ATTR_NO_KERNEL_MAPPING flag to indicate that there is no
>    need to create kernel mapping for it.
> 
> v2:
> * changed 'phys' type to 'dma_addr_t' from 'u64'
> * added a comment on why DMA mask is set to 32-bit
> * replaced 'dma' with 'DMA'
> 
>   drivers/pci/controller/pci-tegra.c | 37 ++++++++++++++++++++++--------
>   1 file changed, 28 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/pci/controller/pci-tegra.c b/drivers/pci/controller/pci-tegra.c
> index f4f53d092e00..464ba2538d52 100644
> --- a/drivers/pci/controller/pci-tegra.c
> +++ b/drivers/pci/controller/pci-tegra.c
> @@ -231,9 +231,9 @@ struct tegra_msi {
>   	struct msi_controller chip;
>   	DECLARE_BITMAP(used, INT_PCI_MSI_NR);
>   	struct irq_domain *domain;
> -	unsigned long pages;
>   	struct mutex lock;
> -	u64 phys;
> +	void *virt;
> +	dma_addr_t phys;
>   	int irq;
>   };
>   
> @@ -1536,7 +1536,7 @@ static int tegra_pcie_msi_setup(struct tegra_pcie *pcie)
>   	err = platform_get_irq_byname(pdev, "msi");
>   	if (err < 0) {
>   		dev_err(dev, "failed to get IRQ: %d\n", err);
> -		goto err;
> +		goto free_irq_domain;
>   	}
>   
>   	msi->irq = err;
> @@ -1545,17 +1545,35 @@ static int tegra_pcie_msi_setup(struct tegra_pcie *pcie)
>   			  tegra_msi_irq_chip.name, pcie);
>   	if (err < 0) {
>   		dev_err(dev, "failed to request IRQ: %d\n", err);
> -		goto err;
> +		goto free_irq_domain;
> +	}
> +
> +	/* Though the PCIe controller can address >32-bit address space, to
> +	 * facilitate endpoints that support only 32-bit MSI target address,
> +	 * the mask is set to 32-bit to make sure that MSI target address is
> +	 * always a 32-bit address
> +	 */
> +	err = dma_set_coherent_mask(dev, DMA_BIT_MASK(32));
> +	if (err < 0) {
> +		dev_err(dev, "failed to set DMA coherent mask: %d\n", err);
> +		goto free_irq;
> +	}
> +
> +	msi->virt = dma_alloc_attrs(dev, PAGE_SIZE, &msi->phys, GFP_KERNEL,
> +				    DMA_ATTR_NO_KERNEL_MAPPING);
> +	if (!msi->virt) {
> +		dev_err(dev, "failed to allocate DMA memory for MSI\n");
> +		err = -ENOMEM;
> +		goto free_irq;
>   	}
>   
> -	/* setup AFI/FPCI range */
> -	msi->pages = __get_free_pages(GFP_KERNEL, 0);
> -	msi->phys = virt_to_phys((void *)msi->pages);
>   	host->msi = &msi->chip;
>   
>   	return 0;
>   
> -err:
> +free_irq:
> +	free_irq(msi->irq, pcie);
> +free_irq_domain:
>   	irq_domain_remove(msi->domain);
>   	return err;
>   }
> @@ -1592,7 +1610,8 @@ static void tegra_pcie_msi_teardown(struct tegra_pcie *pcie)
>   	struct tegra_msi *msi = &pcie->msi;
>   	unsigned int i, irq;
>   
> -	free_pages(msi->pages, 0);
> +	dma_free_attrs(pcie->dev, PAGE_SIZE, msi->virt, msi->phys,
> +		       DMA_ATTR_NO_KERNEL_MAPPING);
>   
>   	if (msi->irq > 0)
>   		free_irq(msi->irq, pcie);
> 

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

* Re: [PATCH V3] PCI: tegra: Use the DMA-API to get the MSI address
  2019-04-16 10:51 [PATCH V3] PCI: tegra: Use the DMA-API to get the MSI address Vidya Sagar
  2019-04-16 11:50 ` Robin Murphy
@ 2019-04-16 14:50 ` Lorenzo Pieralisi
  1 sibling, 0 replies; 3+ messages in thread
From: Lorenzo Pieralisi @ 2019-04-16 14:50 UTC (permalink / raw)
  To: Vidya Sagar
  Cc: bhelgaas, robin.murphy, treding, swarren, jonathanh, dev,
	mperttunen, linux-tegra, linux-pci, kthota, mmaddireddy,
	sagar.tv

On Tue, Apr 16, 2019 at 04:21:44PM +0530, Vidya Sagar wrote:
> Since the upstream MSI memory writes are generated by downstream devices,
> it is logically correct to have MSI target memory coming from the DMA pool
> reserved for PCIe than from the general memory pool reserved for CPU
> access. This avoids PCIe DMA addresses coinciding with MSI target address
> thereby raising unwanted MSI interrupts. This patch also enforces to limit
> the MSI target address to 32-bits to make it work for PCIe endponits that
> support only 32-bit MSI target address and those endpoints that support
> 64-bit MSI target address anyway work with 32-bit MSI target address.
> 
> Signed-off-by: Vidya Sagar <vidyas@nvidia.com>
> Reviewed-by: Thierry Reding <treding@nvidia.com>
> Acked-by: Thierry Reding <treding@nvidia.com>
> ---
> v3:
> * replaced dma_alloc_coherent(), dma_free_coherent() APIs with
>   dma_alloc_attrs(), dma_free_attrs() APIs along with
>   DMA_ATTR_NO_KERNEL_MAPPING flag to indicate that there is no
>   need to create kernel mapping for it.
> 
> v2:
> * changed 'phys' type to 'dma_addr_t' from 'u64'
> * added a comment on why DMA mask is set to 32-bit
> * replaced 'dma' with 'DMA'
> 
>  drivers/pci/controller/pci-tegra.c | 37 ++++++++++++++++++++++--------
>  1 file changed, 28 insertions(+), 9 deletions(-)

Applied to pci/tegra for v5.2.

Thanks,
Lorenzo

> diff --git a/drivers/pci/controller/pci-tegra.c b/drivers/pci/controller/pci-tegra.c
> index f4f53d092e00..464ba2538d52 100644
> --- a/drivers/pci/controller/pci-tegra.c
> +++ b/drivers/pci/controller/pci-tegra.c
> @@ -231,9 +231,9 @@ struct tegra_msi {
>  	struct msi_controller chip;
>  	DECLARE_BITMAP(used, INT_PCI_MSI_NR);
>  	struct irq_domain *domain;
> -	unsigned long pages;
>  	struct mutex lock;
> -	u64 phys;
> +	void *virt;
> +	dma_addr_t phys;
>  	int irq;
>  };
>  
> @@ -1536,7 +1536,7 @@ static int tegra_pcie_msi_setup(struct tegra_pcie *pcie)
>  	err = platform_get_irq_byname(pdev, "msi");
>  	if (err < 0) {
>  		dev_err(dev, "failed to get IRQ: %d\n", err);
> -		goto err;
> +		goto free_irq_domain;
>  	}
>  
>  	msi->irq = err;
> @@ -1545,17 +1545,35 @@ static int tegra_pcie_msi_setup(struct tegra_pcie *pcie)
>  			  tegra_msi_irq_chip.name, pcie);
>  	if (err < 0) {
>  		dev_err(dev, "failed to request IRQ: %d\n", err);
> -		goto err;
> +		goto free_irq_domain;
> +	}
> +
> +	/* Though the PCIe controller can address >32-bit address space, to
> +	 * facilitate endpoints that support only 32-bit MSI target address,
> +	 * the mask is set to 32-bit to make sure that MSI target address is
> +	 * always a 32-bit address
> +	 */
> +	err = dma_set_coherent_mask(dev, DMA_BIT_MASK(32));
> +	if (err < 0) {
> +		dev_err(dev, "failed to set DMA coherent mask: %d\n", err);
> +		goto free_irq;
> +	}
> +
> +	msi->virt = dma_alloc_attrs(dev, PAGE_SIZE, &msi->phys, GFP_KERNEL,
> +				    DMA_ATTR_NO_KERNEL_MAPPING);
> +	if (!msi->virt) {
> +		dev_err(dev, "failed to allocate DMA memory for MSI\n");
> +		err = -ENOMEM;
> +		goto free_irq;
>  	}
>  
> -	/* setup AFI/FPCI range */
> -	msi->pages = __get_free_pages(GFP_KERNEL, 0);
> -	msi->phys = virt_to_phys((void *)msi->pages);
>  	host->msi = &msi->chip;
>  
>  	return 0;
>  
> -err:
> +free_irq:
> +	free_irq(msi->irq, pcie);
> +free_irq_domain:
>  	irq_domain_remove(msi->domain);
>  	return err;
>  }
> @@ -1592,7 +1610,8 @@ static void tegra_pcie_msi_teardown(struct tegra_pcie *pcie)
>  	struct tegra_msi *msi = &pcie->msi;
>  	unsigned int i, irq;
>  
> -	free_pages(msi->pages, 0);
> +	dma_free_attrs(pcie->dev, PAGE_SIZE, msi->virt, msi->phys,
> +		       DMA_ATTR_NO_KERNEL_MAPPING);
>  
>  	if (msi->irq > 0)
>  		free_irq(msi->irq, pcie);
> -- 
> 2.17.1
> 

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

end of thread, other threads:[~2019-04-16 14:50 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-04-16 10:51 [PATCH V3] PCI: tegra: Use the DMA-API to get the MSI address Vidya Sagar
2019-04-16 11:50 ` Robin Murphy
2019-04-16 14:50 ` Lorenzo Pieralisi

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).