Re: [PATCH V2] PCI: tegra: Use the DMA-API to get the MSI address

[Vidya Sagar <vidyas@nvidia.com>]

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
>