| Message ID | 20171219232940.659-2-niklas.cassel@axis.com (mailing list archive) |
|---|---|
| State | New, archived |
| Delegated to: | Bjorn Helgaas |
| Headers | show |
Hi Niklas, Às 11:29 PM de 12/19/2017, Niklas Cassel escreveu: > Use the DMA-API to get the MSI address. This address will be written to > our PCI config space and to the register which determines which AXI > address the DWC IP will spoof for incoming MSI irqs. > > Since it is a PCIe endpoint device, rather than the CPU, that is supposed > to write to the MSI address, the proper way to get the MSI address is by > using the DMA API, not by using virt_to_phys(). > > Using virt_to_phys() might work on some systems, but using the DMA API > should work on all systems. > > This is essentially the same thing as allocating a buffer in a driver > to which the endpoint will write to. To do this, we use the DMA API. > > Signed-off-by: Niklas Cassel <niklas.cassel@axis.com> > --- > drivers/pci/dwc/pcie-designware-host.c | 15 ++++++++++++--- > drivers/pci/dwc/pcie-designware.h | 3 ++- > 2 files changed, 14 insertions(+), 4 deletions(-) > > diff --git a/drivers/pci/dwc/pcie-designware-host.c b/drivers/pci/dwc/pcie-designware-host.c > index 81e2157a7cfb..bf558df5b7b3 100644 > --- a/drivers/pci/dwc/pcie-designware-host.c > +++ b/drivers/pci/dwc/pcie-designware-host.c > @@ -83,10 +83,19 @@ irqreturn_t dw_handle_msi_irq(struct pcie_port *pp) > > void dw_pcie_msi_init(struct pcie_port *pp) > { > + struct dw_pcie *pci = to_dw_pcie_from_pp(pp); > + struct device *dev = pci->dev; > + struct page *page; > u64 msi_target; > > - pp->msi_data = __get_free_pages(GFP_KERNEL, 0); > - msi_target = virt_to_phys((void *)pp->msi_data); > + page = alloc_page(GFP_KERNEL); > + pp->msi_data = dma_map_page(dev, 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(page); > + return; > + } > + msi_target = (u64)pp->msi_data; > > /* program the msi_data */ > dw_pcie_wr_own_conf(pp, PCIE_MSI_ADDR_LO, 4, > @@ -187,7 +196,7 @@ static void dw_msi_setup_msg(struct pcie_port *pp, unsigned int irq, u32 pos) > if (pp->ops->get_msi_addr) > msi_target = pp->ops->get_msi_addr(pp); > else > - msi_target = virt_to_phys((void *)pp->msi_data); > + msi_target = (u64)pp->msi_data; > > msg.address_lo = (u32)(msi_target & 0xffffffff); > msg.address_hi = (u32)(msi_target >> 32 & 0xffffffff); > diff --git a/drivers/pci/dwc/pcie-designware.h b/drivers/pci/dwc/pcie-designware.h > index e5d9d77b778e..ecdede68522a 100644 > --- a/drivers/pci/dwc/pcie-designware.h > +++ b/drivers/pci/dwc/pcie-designware.h > @@ -14,6 +14,7 @@ > #ifndef _PCIE_DESIGNWARE_H > #define _PCIE_DESIGNWARE_H > > +#include <linux/dma-mapping.h> > #include <linux/irq.h> > #include <linux/msi.h> > #include <linux/pci.h> > @@ -168,7 +169,7 @@ struct pcie_port { > const struct dw_pcie_host_ops *ops; > int msi_irq; > struct irq_domain *irq_domain; > - unsigned long msi_data; > + dma_addr_t msi_data; > DECLARE_BITMAP(msi_irq_in_use, MAX_MSI_IRQS); > }; > > Makes total sense! Thanks. Acked-by: Joao Pinto <jpinto@synopsys.com>
On Wednesday, December 20, 2017 2:10 PM, Joao Pinto wrote: > > Hi Niklas, > > Às 11:29 PM de 12/19/2017, Niklas Cassel escreveu: > > Use the DMA-API to get the MSI address. This address will be written to > > our PCI config space and to the register which determines which AXI > > address the DWC IP will spoof for incoming MSI irqs. > > > > Since it is a PCIe endpoint device, rather than the CPU, that is > supposed > > to write to the MSI address, the proper way to get the MSI address is by > > using the DMA API, not by using virt_to_phys(). > > > > Using virt_to_phys() might work on some systems, but using the DMA API > > should work on all systems. > > > > This is essentially the same thing as allocating a buffer in a driver > > to which the endpoint will write to. To do this, we use the DMA API. > > > > Signed-off-by: Niklas Cassel <niklas.cassel@axis.com> > > --- > > drivers/pci/dwc/pcie-designware-host.c | 15 ++++++++++++--- > > drivers/pci/dwc/pcie-designware.h | 3 ++- > > 2 files changed, 14 insertions(+), 4 deletions(-) > > > > diff --git a/drivers/pci/dwc/pcie-designware-host.c > b/drivers/pci/dwc/pcie-designware-host.c > > index 81e2157a7cfb..bf558df5b7b3 100644 > > --- a/drivers/pci/dwc/pcie-designware-host.c > > +++ b/drivers/pci/dwc/pcie-designware-host.c > > @@ -83,10 +83,19 @@ irqreturn_t dw_handle_msi_irq(struct pcie_port *pp) > > > > void dw_pcie_msi_init(struct pcie_port *pp) > > { > > + struct dw_pcie *pci = to_dw_pcie_from_pp(pp); > > + struct device *dev = pci->dev; > > + struct page *page; > > u64 msi_target; > > > > - pp->msi_data = __get_free_pages(GFP_KERNEL, 0); > > - msi_target = virt_to_phys((void *)pp->msi_data); > > + page = alloc_page(GFP_KERNEL); > > + pp->msi_data = dma_map_page(dev, 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(page); > > + return; > > + } > > + msi_target = (u64)pp->msi_data; > > > > /* program the msi_data */ > > dw_pcie_wr_own_conf(pp, PCIE_MSI_ADDR_LO, 4, > > @@ -187,7 +196,7 @@ static void dw_msi_setup_msg(struct pcie_port *pp, > unsigned int irq, u32 pos) > > if (pp->ops->get_msi_addr) > > msi_target = pp->ops->get_msi_addr(pp); > > else > > - msi_target = virt_to_phys((void *)pp->msi_data); > > + msi_target = (u64)pp->msi_data; > > > > msg.address_lo = (u32)(msi_target & 0xffffffff); > > msg.address_hi = (u32)(msi_target >> 32 & 0xffffffff); > > diff --git a/drivers/pci/dwc/pcie-designware.h b/drivers/pci/dwc/pcie- > designware.h > > index e5d9d77b778e..ecdede68522a 100644 > > --- a/drivers/pci/dwc/pcie-designware.h > > +++ b/drivers/pci/dwc/pcie-designware.h > > @@ -14,6 +14,7 @@ > > #ifndef _PCIE_DESIGNWARE_H > > #define _PCIE_DESIGNWARE_H > > > > +#include <linux/dma-mapping.h> > > #include <linux/irq.h> > > #include <linux/msi.h> > > #include <linux/pci.h> > > @@ -168,7 +169,7 @@ struct pcie_port { > > const struct dw_pcie_host_ops *ops; > > int msi_irq; > > struct irq_domain *irq_domain; > > - unsigned long msi_data; > > + dma_addr_t msi_data; > > DECLARE_BITMAP(msi_irq_in_use, MAX_MSI_IRQS); > > }; > > > > > > Makes total sense! Thanks. > > Acked-by: Joao Pinto <jpinto@synopsys.com> Acked-by: Jingoo Han <jingoohan1@gmail.com> Best regards, Jingoo Han
On Wed, Dec 20, 2017 at 12:29:22AM +0100, Niklas Cassel wrote: > Use the DMA-API to get the MSI address. This address will be written to > our PCI config space and to the register which determines which AXI > address the DWC IP will spoof for incoming MSI irqs. > > Since it is a PCIe endpoint device, rather than the CPU, that is supposed > to write to the MSI address, the proper way to get the MSI address is by > using the DMA API, not by using virt_to_phys(). > > Using virt_to_phys() might work on some systems, but using the DMA API > should work on all systems. > > This is essentially the same thing as allocating a buffer in a driver > to which the endpoint will write to. To do this, we use the DMA API. > > Signed-off-by: Niklas Cassel <niklas.cassel@axis.com> > --- > drivers/pci/dwc/pcie-designware-host.c | 15 ++++++++++++--- > drivers/pci/dwc/pcie-designware.h | 3 ++- > 2 files changed, 14 insertions(+), 4 deletions(-) Resurrecting an old thread... Did this fix any actual problem for you? I'm asking because this introduces a bug that an unmap is never done in the error path and there's an issue with suspend/resume[1]. I'd like to simplify all this. The whole thing including the existing alloc_page() seems pointless. AIUI, the DWC MSI controller just needs a single address which is never forwarded out of the RC. So we could use any address that an EP would never write to. Some drivers just take the address of a field in their driver data. Perhaps even just the DBI base would work or the address of PCIE_MSI_ADDR_LO. Rob [1] https://lore.kernel.org/r/20200923142607.10c89bd2@xhacker.debian > > diff --git a/drivers/pci/dwc/pcie-designware-host.c b/drivers/pci/dwc/pcie-designware-host.c > index 81e2157a7cfb..bf558df5b7b3 100644 > --- a/drivers/pci/dwc/pcie-designware-host.c > +++ b/drivers/pci/dwc/pcie-designware-host.c > @@ -83,10 +83,19 @@ irqreturn_t dw_handle_msi_irq(struct pcie_port *pp) > > void dw_pcie_msi_init(struct pcie_port *pp) > { > + struct dw_pcie *pci = to_dw_pcie_from_pp(pp); > + struct device *dev = pci->dev; > + struct page *page; > u64 msi_target; > > - pp->msi_data = __get_free_pages(GFP_KERNEL, 0); > - msi_target = virt_to_phys((void *)pp->msi_data); > + page = alloc_page(GFP_KERNEL); > + pp->msi_data = dma_map_page(dev, 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(page); > + return; > + } > + msi_target = (u64)pp->msi_data; > > /* program the msi_data */ > dw_pcie_wr_own_conf(pp, PCIE_MSI_ADDR_LO, 4, > @@ -187,7 +196,7 @@ static void dw_msi_setup_msg(struct pcie_port *pp, unsigned int irq, u32 pos) > if (pp->ops->get_msi_addr) > msi_target = pp->ops->get_msi_addr(pp); > else > - msi_target = virt_to_phys((void *)pp->msi_data); > + msi_target = (u64)pp->msi_data; > > msg.address_lo = (u32)(msi_target & 0xffffffff); > msg.address_hi = (u32)(msi_target >> 32 & 0xffffffff); > diff --git a/drivers/pci/dwc/pcie-designware.h b/drivers/pci/dwc/pcie-designware.h > index e5d9d77b778e..ecdede68522a 100644 > --- a/drivers/pci/dwc/pcie-designware.h > +++ b/drivers/pci/dwc/pcie-designware.h > @@ -14,6 +14,7 @@ > #ifndef _PCIE_DESIGNWARE_H > #define _PCIE_DESIGNWARE_H > > +#include <linux/dma-mapping.h> > #include <linux/irq.h> > #include <linux/msi.h> > #include <linux/pci.h> > @@ -168,7 +169,7 @@ struct pcie_port { > const struct dw_pcie_host_ops *ops; > int msi_irq; > struct irq_domain *irq_domain; > - unsigned long msi_data; > + dma_addr_t msi_data; > DECLARE_BITMAP(msi_irq_in_use, MAX_MSI_IRQS); > }; > > -- > 2.14.2
diff --git a/drivers/pci/dwc/pcie-designware-host.c b/drivers/pci/dwc/pcie-designware-host.c index 81e2157a7cfb..bf558df5b7b3 100644 --- a/drivers/pci/dwc/pcie-designware-host.c +++ b/drivers/pci/dwc/pcie-designware-host.c @@ -83,10 +83,19 @@ irqreturn_t dw_handle_msi_irq(struct pcie_port *pp) void dw_pcie_msi_init(struct pcie_port *pp) { + struct dw_pcie *pci = to_dw_pcie_from_pp(pp); + struct device *dev = pci->dev; + struct page *page; u64 msi_target; - pp->msi_data = __get_free_pages(GFP_KERNEL, 0); - msi_target = virt_to_phys((void *)pp->msi_data); + page = alloc_page(GFP_KERNEL); + pp->msi_data = dma_map_page(dev, 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(page); + return; + } + msi_target = (u64)pp->msi_data; /* program the msi_data */ dw_pcie_wr_own_conf(pp, PCIE_MSI_ADDR_LO, 4, @@ -187,7 +196,7 @@ static void dw_msi_setup_msg(struct pcie_port *pp, unsigned int irq, u32 pos) if (pp->ops->get_msi_addr) msi_target = pp->ops->get_msi_addr(pp); else - msi_target = virt_to_phys((void *)pp->msi_data); + msi_target = (u64)pp->msi_data; msg.address_lo = (u32)(msi_target & 0xffffffff); msg.address_hi = (u32)(msi_target >> 32 & 0xffffffff); diff --git a/drivers/pci/dwc/pcie-designware.h b/drivers/pci/dwc/pcie-designware.h index e5d9d77b778e..ecdede68522a 100644 --- a/drivers/pci/dwc/pcie-designware.h +++ b/drivers/pci/dwc/pcie-designware.h @@ -14,6 +14,7 @@ #ifndef _PCIE_DESIGNWARE_H #define _PCIE_DESIGNWARE_H +#include <linux/dma-mapping.h> #include <linux/irq.h> #include <linux/msi.h> #include <linux/pci.h> @@ -168,7 +169,7 @@ struct pcie_port { const struct dw_pcie_host_ops *ops; int msi_irq; struct irq_domain *irq_domain; - unsigned long msi_data; + dma_addr_t msi_data; DECLARE_BITMAP(msi_irq_in_use, MAX_MSI_IRQS); };
Use the DMA-API to get the MSI address. This address will be written to our PCI config space and to the register which determines which AXI address the DWC IP will spoof for incoming MSI irqs. Since it is a PCIe endpoint device, rather than the CPU, that is supposed to write to the MSI address, the proper way to get the MSI address is by using the DMA API, not by using virt_to_phys(). Using virt_to_phys() might work on some systems, but using the DMA API should work on all systems. This is essentially the same thing as allocating a buffer in a driver to which the endpoint will write to. To do this, we use the DMA API. Signed-off-by: Niklas Cassel <niklas.cassel@axis.com> --- drivers/pci/dwc/pcie-designware-host.c | 15 ++++++++++++--- drivers/pci/dwc/pcie-designware.h | 3 ++- 2 files changed, 14 insertions(+), 4 deletions(-)