* [PATCH V2] PCI: tegra: Use the DMA-API to get the MSI address @ 2019-03-19 14:02 Vidya Sagar 2019-03-27 11:01 ` vidya sagar ` (2 more replies) 0 siblings, 3 replies; 8+ messages in thread From: Vidya Sagar @ 2019-03-19 14:02 UTC (permalink / raw) To: bhelgaas, lorenzo.pieralisi, treding, swarren, mperttunen, jonathanh, dev, vidyas Cc: linux-tegra, linux-pci, kthota, mmaddireddy 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> --- 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 | 35 ++++++++++++++++++++++++++--------- 1 file changed, 26 insertions(+), 9 deletions(-) diff --git a/drivers/pci/controller/pci-tegra.c b/drivers/pci/controller/pci-tegra.c index f4f53d092e00..f8173a5e352d 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,34 @@ 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_coherent(dev, PAGE_SIZE, &msi->phys, GFP_KERNEL); + 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 +1609,7 @@ 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_coherent(pcie->dev, PAGE_SIZE, msi->virt, msi->phys); if (msi->irq > 0) free_irq(msi->irq, pcie); -- 2.7.4 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH V2] PCI: tegra: Use the DMA-API to get the MSI address 2019-03-19 14:02 [PATCH V2] PCI: tegra: Use the DMA-API to get the MSI address Vidya Sagar @ 2019-03-27 11:01 ` vidya sagar [not found] ` <CAN7O0+LG8vbB+hApe2tz2LLE_a9XF1iPfD7tKY6=MmhqgoJm8w@mail.gmail.com> 2019-04-11 9:40 ` Lorenzo Pieralisi 2 siblings, 0 replies; 8+ messages in thread From: vidya sagar @ 2019-03-27 11:01 UTC (permalink / raw) To: Vidya Sagar Cc: bhelgaas, lorenzo.pieralisi, treding, swarren, mperttunen, jonathanh, dev, linux-tegra, linux-pci, kthota, NManikanta Hi Bjorn/Lorenzo, Can you please review this patch? Thierry has reviewed it and I already took care of his comments. Thanks, Vidya Sagar On Tue, Mar 19, 2019 at 7:33 PM Vidya Sagar <vidyas@nvidia.com> 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> > --- > 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 | 35 ++++++++++++++++++++++++++--------- > 1 file changed, 26 insertions(+), 9 deletions(-) > > diff --git a/drivers/pci/controller/pci-tegra.c b/drivers/pci/controller/pci-tegra.c > index f4f53d092e00..f8173a5e352d 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,34 @@ 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_coherent(dev, PAGE_SIZE, &msi->phys, GFP_KERNEL); > + 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 +1609,7 @@ 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_coherent(pcie->dev, PAGE_SIZE, msi->virt, msi->phys); > > if (msi->irq > 0) > free_irq(msi->irq, pcie); > -- > 2.7.4 > ^ permalink raw reply [flat|nested] 8+ messages in thread
[parent not found: <CAN7O0+LG8vbB+hApe2tz2LLE_a9XF1iPfD7tKY6=MmhqgoJm8w@mail.gmail.com>]
* Re: [PATCH V2] PCI: tegra: Use the DMA-API to get the MSI address [not found] ` <CAN7O0+LG8vbB+hApe2tz2LLE_a9XF1iPfD7tKY6=MmhqgoJm8w@mail.gmail.com> @ 2019-04-01 5:43 ` Vidya Sagar 2019-04-04 21:30 ` Vidya Sagar 0 siblings, 1 reply; 8+ messages in thread From: Vidya Sagar @ 2019-04-01 5:43 UTC (permalink / raw) To: bhelgaas, lorenzo.pieralisi Cc: vidya sagar, treding, swarren, mperttunen, jonathanh, dev, linux-tegra, linux-pci, kthota, NManikanta Hi Bjorn / Lorenzo, Can you please review this patch? Thanks, Vidya Sagar On 3/27/2019 4:29 PM, vidya sagar wrote: > Hi Bjorn/Lorenzo, > Can you please review this patch? > Thierry has reviewed it and I already took care of his comments. > > Thanks, > Vidya Sagar > > On Tue, Mar 19, 2019 at 7:33 PM Vidya Sagar <vidyas@nvidia.com <mailto:vidyas@nvidia.com>> 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 <mailto:vidyas@nvidia.com>> > Reviewed-by: Thierry Reding <treding@nvidia.com <mailto:treding@nvidia.com>> > Acked-by: Thierry Reding <treding@nvidia.com <mailto:treding@nvidia.com>> > --- > 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 | 35 ++++++++++++++++++++++++++--------- > 1 file changed, 26 insertions(+), 9 deletions(-) > > diff --git a/drivers/pci/controller/pci-tegra.c b/drivers/pci/controller/pci-tegra.c > index f4f53d092e00..f8173a5e352d 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,34 @@ static int tegra_pcie_msi_setup(struct tegra_pcie *pcie) > tegra_msi_irq_chip.name <http://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_coherent(dev, PAGE_SIZE, &msi->phys, GFP_KERNEL); > + 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 +1609,7 @@ 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_coherent(pcie->dev, PAGE_SIZE, msi->virt, msi->phys); > > if (msi->irq > 0) > free_irq(msi->irq, pcie); > -- > 2.7.4 > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH V2] PCI: tegra: Use the DMA-API to get the MSI address 2019-04-01 5:43 ` Vidya Sagar @ 2019-04-04 21:30 ` Vidya Sagar 2019-04-05 12:46 ` Lorenzo Pieralisi 0 siblings, 1 reply; 8+ messages in thread From: Vidya Sagar @ 2019-04-04 21:30 UTC (permalink / raw) To: bhelgaas, lorenzo.pieralisi Cc: vidya sagar, treding, swarren, mperttunen, jonathanh, dev, linux-tegra, linux-pci, kthota, NManikanta On 4/1/2019 11:13 AM, Vidya Sagar wrote: Hi Bjorn / Lorenzo, Apologies for reminding you again. Could you please review this patch? Thanks, Vidya Sagar > Hi Bjorn / Lorenzo, > Can you please review this patch? > > Thanks, > Vidya Sagar > > On 3/27/2019 4:29 PM, vidya sagar wrote: >> Hi Bjorn/Lorenzo, >> Can you please review this patch? >> Thierry has reviewed it and I already took care of his comments. >> >> Thanks, >> Vidya Sagar >> >> On Tue, Mar 19, 2019 at 7:33 PM Vidya Sagar <vidyas@nvidia.com <mailto:vidyas@nvidia.com>> 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 <mailto:vidyas@nvidia.com>> >> Reviewed-by: Thierry Reding <treding@nvidia.com <mailto:treding@nvidia.com>> >> Acked-by: Thierry Reding <treding@nvidia.com <mailto:treding@nvidia.com>> >> --- >> 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 | 35 ++++++++++++++++++++++++++--------- >> 1 file changed, 26 insertions(+), 9 deletions(-) >> >> diff --git a/drivers/pci/controller/pci-tegra.c b/drivers/pci/controller/pci-tegra.c >> index f4f53d092e00..f8173a5e352d 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,34 @@ static int tegra_pcie_msi_setup(struct tegra_pcie *pcie) >> tegra_msi_irq_chip.name <http://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_coherent(dev, PAGE_SIZE, &msi->phys, GFP_KERNEL); >> + 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 +1609,7 @@ 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_coherent(pcie->dev, PAGE_SIZE, msi->virt, msi->phys); >> >> if (msi->irq > 0) >> free_irq(msi->irq, pcie); >> -- 2.7.4 >> > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH V2] PCI: tegra: Use the DMA-API to get the MSI address 2019-04-04 21:30 ` Vidya Sagar @ 2019-04-05 12:46 ` Lorenzo Pieralisi 0 siblings, 0 replies; 8+ messages in thread From: Lorenzo Pieralisi @ 2019-04-05 12:46 UTC (permalink / raw) To: Vidya Sagar Cc: bhelgaas, vidya sagar, treding, swarren, mperttunen, jonathanh, dev, linux-tegra, linux-pci, kthota, NManikanta On Fri, Apr 05, 2019 at 03:00:36AM +0530, Vidya Sagar wrote: > On 4/1/2019 11:13 AM, Vidya Sagar wrote: > > Hi Bjorn / Lorenzo, > Apologies for reminding you again. Could you please review this patch? It is in the patchwork queue - I will get to it, thank you for your patience. Lorenzo > Thanks, > Vidya Sagar > > >Hi Bjorn / Lorenzo, > >Can you please review this patch? > > > >Thanks, > >Vidya Sagar > > > >On 3/27/2019 4:29 PM, vidya sagar wrote: > >>Hi Bjorn/Lorenzo, > >>Can you please review this patch? > >>Thierry has reviewed it and I already took care of his comments. > >> > >>Thanks, > >>Vidya Sagar > >> > >>On Tue, Mar 19, 2019 at 7:33 PM Vidya Sagar <vidyas@nvidia.com <mailto:vidyas@nvidia.com>> 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 <mailto:vidyas@nvidia.com>> > >> Reviewed-by: Thierry Reding <treding@nvidia.com <mailto:treding@nvidia.com>> > >> Acked-by: Thierry Reding <treding@nvidia.com <mailto:treding@nvidia.com>> > >> --- > >> 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 | 35 ++++++++++++++++++++++++++--------- > >> 1 file changed, 26 insertions(+), 9 deletions(-) > >> > >> diff --git a/drivers/pci/controller/pci-tegra.c b/drivers/pci/controller/pci-tegra.c > >> index f4f53d092e00..f8173a5e352d 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,34 @@ static int tegra_pcie_msi_setup(struct tegra_pcie *pcie) > >> tegra_msi_irq_chip.name <http://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_coherent(dev, PAGE_SIZE, &msi->phys, GFP_KERNEL); > >> + 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 +1609,7 @@ 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_coherent(pcie->dev, PAGE_SIZE, msi->virt, msi->phys); > >> > >> if (msi->irq > 0) > >> free_irq(msi->irq, pcie); > >> -- 2.7.4 > >> > > > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH V2] PCI: tegra: Use the DMA-API to get the MSI address 2019-03-19 14:02 [PATCH V2] PCI: tegra: Use the DMA-API to get the MSI address Vidya Sagar 2019-03-27 11:01 ` vidya sagar [not found] ` <CAN7O0+LG8vbB+hApe2tz2LLE_a9XF1iPfD7tKY6=MmhqgoJm8w@mail.gmail.com> @ 2019-04-11 9:40 ` Lorenzo Pieralisi 2019-04-12 15:00 ` Robin Murphy 2 siblings, 1 reply; 8+ messages in thread From: Lorenzo Pieralisi @ 2019-04-11 9:40 UTC (permalink / raw) To: Vidya Sagar Cc: bhelgaas, treding, swarren, mperttunen, jonathanh, dev, linux-tegra, linux-pci, kthota, mmaddireddy, robin.murphy [+Robin] I would like Robin to have a look before merging this patch so that we agree that's the expected approach. Lorenzo On Tue, Mar 19, 2019 at 07:32:01PM +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> > --- > 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 | 35 ++++++++++++++++++++++++++--------- > 1 file changed, 26 insertions(+), 9 deletions(-) > > diff --git a/drivers/pci/controller/pci-tegra.c b/drivers/pci/controller/pci-tegra.c > index f4f53d092e00..f8173a5e352d 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,34 @@ 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_coherent(dev, PAGE_SIZE, &msi->phys, GFP_KERNEL); > + 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 +1609,7 @@ 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_coherent(pcie->dev, PAGE_SIZE, msi->virt, msi->phys); > > if (msi->irq > 0) > free_irq(msi->irq, pcie); > -- > 2.7.4 > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH V2] PCI: tegra: Use the DMA-API to get the MSI address 2019-04-11 9:40 ` Lorenzo Pieralisi @ 2019-04-12 15:00 ` Robin Murphy 2019-04-16 10:48 ` Vidya Sagar 0 siblings, 1 reply; 8+ messages in thread From: Robin Murphy @ 2019-04-12 15:00 UTC (permalink / raw) To: Lorenzo Pieralisi, Vidya Sagar Cc: bhelgaas, treding, swarren, mperttunen, jonathanh, dev, linux-tegra, linux-pci, kthota, mmaddireddy On 11/04/2019 10:40, Lorenzo Pieralisi wrote: > [+Robin] > > I would like Robin to have a look before merging this patch so > that we agree that's the expected approach. It's a bit crazy, but I guess it's not really any worse than the existing implementation. As long the comments and commit message clearly document that this is just trickery to reserve a 32-bit DMA address (which AFAICS they more or less do) I don't think I have a significant objection. One suggestion I would make is using dma_alloc_attrs() with DMA_ATTR_NO_KERNEL_MAPPING, to make it clear that this is being set aside for 'special' device purposes and is not intended to be accessed as a regular buffer (plus I believe this is non-coherent, so that should also skip the small overhead of creating a non-cacheable remap). Ultimately, it might make sense to have an API in the PCI core which can look at memblock/bridge windows/etc. to find a suitable non-overlapping address for this kind of root-complex-integrated doorbell without having to subvert the page allocator, as it seems to be a fairly common setup in 'embedded' IPs. Robin. > > Lorenzo > > On Tue, Mar 19, 2019 at 07:32:01PM +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> >> --- >> 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 | 35 ++++++++++++++++++++++++++--------- >> 1 file changed, 26 insertions(+), 9 deletions(-) >> >> diff --git a/drivers/pci/controller/pci-tegra.c b/drivers/pci/controller/pci-tegra.c >> index f4f53d092e00..f8173a5e352d 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,34 @@ 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_coherent(dev, PAGE_SIZE, &msi->phys, GFP_KERNEL); >> + 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 +1609,7 @@ 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_coherent(pcie->dev, PAGE_SIZE, msi->virt, msi->phys); >> >> if (msi->irq > 0) >> free_irq(msi->irq, pcie); >> -- >> 2.7.4 >> ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH V2] PCI: tegra: Use the DMA-API to get the MSI address 2019-04-12 15:00 ` Robin Murphy @ 2019-04-16 10:48 ` Vidya Sagar 0 siblings, 0 replies; 8+ messages in thread From: Vidya Sagar @ 2019-04-16 10:48 UTC (permalink / raw) To: Robin Murphy, Lorenzo Pieralisi Cc: bhelgaas, treding, swarren, mperttunen, jonathanh, dev, linux-tegra, linux-pci, kthota, mmaddireddy On 4/12/2019 8:30 PM, Robin Murphy wrote: > On 11/04/2019 10:40, Lorenzo Pieralisi wrote: >> [+Robin] >> >> I would like Robin to have a look before merging this patch so >> that we agree that's the expected approach. > > It's a bit crazy, but I guess it's not really any worse than the existing implementation. As long the comments and commit message clearly document that this is just trickery to reserve a 32-bit DMA address (which AFAICS they more or less do) I don't think I have a significant objection. > > One suggestion I would make is using dma_alloc_attrs() with DMA_ATTR_NO_KERNEL_MAPPING, to make it clear that this is being set aside for 'special' device purposes and is not intended to be accessed as a regular buffer (plus I believe this is non-coherent, so that should also skip the small overhead of creating a non-cacheable remap). Thanks Robin for your inputs. I'll take care of changing API from dma_alloc_coherent()/dma_free_coherent() to dma_alloc_attrs()/dma_free_attrs() in V3 patch. > > Ultimately, it might make sense to have an API in the PCI core which can look at memblock/bridge windows/etc. to find a suitable non-overlapping address for this kind of root-complex-integrated doorbell without having to subvert the page allocator, as it seems to be a fairly common setup in 'embedded' IPs. > > Robin. > >> >> Lorenzo >> >> On Tue, Mar 19, 2019 at 07:32:01PM +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> >>> --- >>> 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 | 35 ++++++++++++++++++++++++++--------- >>> 1 file changed, 26 insertions(+), 9 deletions(-) >>> >>> diff --git a/drivers/pci/controller/pci-tegra.c b/drivers/pci/controller/pci-tegra.c >>> index f4f53d092e00..f8173a5e352d 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,34 @@ 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_coherent(dev, PAGE_SIZE, &msi->phys, GFP_KERNEL); >>> + 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 +1609,7 @@ 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_coherent(pcie->dev, PAGE_SIZE, msi->virt, msi->phys); >>> if (msi->irq > 0) >>> free_irq(msi->irq, pcie); >>> -- >>> 2.7.4 >>> ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2019-04-16 10:49 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-03-19 14:02 [PATCH V2] PCI: tegra: Use the DMA-API to get the MSI address Vidya Sagar 2019-03-27 11:01 ` vidya sagar [not found] ` <CAN7O0+LG8vbB+hApe2tz2LLE_a9XF1iPfD7tKY6=MmhqgoJm8w@mail.gmail.com> 2019-04-01 5:43 ` Vidya Sagar 2019-04-04 21:30 ` Vidya Sagar 2019-04-05 12:46 ` Lorenzo Pieralisi 2019-04-11 9:40 ` Lorenzo Pieralisi 2019-04-12 15:00 ` Robin Murphy 2019-04-16 10:48 ` Vidya Sagar
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).