| Message ID | 20230113171409.30470-9-Sergey.Semin@baikalelectronics.ru (mailing list archive) |
|---|---|
| State | Accepted |
| Delegated to: | Lorenzo Pieralisi |
| Headers | show |
| Series | dmaengine: dw-edma: Add RP/EP local DMA controllers support | expand |
On Fri, Jan 13, 2023 at 08:13:50PM +0300, Serge Semin wrote: > In general the Synopsys PCIe EndPoint IP prototype kit can be attached to > a PCIe bus with any PCIe Host controller including to the one with > distinctive from CPU address space. Due to that we need to make sure that > the source and destination addresses of the DMA-slave devices are properly > converted to the PCIe bus address space, otherwise the DMA transaction > will not only work as expected, but may cause the memory corruption with > subsequent system crash. Let's do that by introducing a new > dw_edma_pcie_address() method defined in the dw-edma-pcie.c, which will > perform the denoted translation by using the pcibios_resource_to_bus() > method. > > Fixes: 41aaff2a2ac0 ("dmaengine: Add Synopsys eDMA IP PCIe glue-logic") > Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru> > Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> > Tested-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> > Acked-by: Vinod Koul <vkoul@kernel.org> > > --- > > Note this patch depends on the patch "dmaengine: dw-edma: Add CPU to PCIe > bus address translation" from this series. > --- > drivers/dma/dw-edma/dw-edma-pcie.c | 15 +++++++++++++++ > 1 file changed, 15 insertions(+) > > diff --git a/drivers/dma/dw-edma/dw-edma-pcie.c b/drivers/dma/dw-edma/dw-edma-pcie.c > index 04c95cba1244..f530bacfd716 100644 > --- a/drivers/dma/dw-edma/dw-edma-pcie.c > +++ b/drivers/dma/dw-edma/dw-edma-pcie.c > @@ -95,8 +95,23 @@ static int dw_edma_pcie_irq_vector(struct device *dev, unsigned int nr) > return pci_irq_vector(to_pci_dev(dev), nr); > } > > +static u64 dw_edma_pcie_address(struct device *dev, phys_addr_t cpu_addr) > +{ > + struct pci_dev *pdev = to_pci_dev(dev); > + struct pci_bus_region region; > + struct resource res = { > + .flags = IORESOURCE_MEM, > + .start = cpu_addr, > + .end = cpu_addr, > + }; > + > + pcibios_resource_to_bus(pdev->bus, ®ion, &res); > + return region.start; > +} This doesn't look DW-specific. Do you expect other implementations that are specific, or could this be a generic function that shares some implementation with pci_bus_address()? > static const struct dw_edma_core_ops dw_edma_pcie_core_ops = { > .irq_vector = dw_edma_pcie_irq_vector, > + .pci_address = dw_edma_pcie_address, > }; > > static void dw_edma_pcie_get_vsec_dma_data(struct pci_dev *pdev, > -- > 2.39.0 > >
On Fri, Jan 20, 2023 at 05:29:20PM -0600, Bjorn Helgaas wrote: > On Fri, Jan 13, 2023 at 08:13:50PM +0300, Serge Semin wrote: > > In general the Synopsys PCIe EndPoint IP prototype kit can be attached to > > a PCIe bus with any PCIe Host controller including to the one with > > distinctive from CPU address space. Due to that we need to make sure that > > the source and destination addresses of the DMA-slave devices are properly > > converted to the PCIe bus address space, otherwise the DMA transaction > > will not only work as expected, but may cause the memory corruption with > > subsequent system crash. Let's do that by introducing a new > > dw_edma_pcie_address() method defined in the dw-edma-pcie.c, which will > > perform the denoted translation by using the pcibios_resource_to_bus() > > method. > > > > Fixes: 41aaff2a2ac0 ("dmaengine: Add Synopsys eDMA IP PCIe glue-logic") > > Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru> > > Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> > > Tested-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> > > Acked-by: Vinod Koul <vkoul@kernel.org> > > > > --- > > > > Note this patch depends on the patch "dmaengine: dw-edma: Add CPU to PCIe > > bus address translation" from this series. > > --- > > drivers/dma/dw-edma/dw-edma-pcie.c | 15 +++++++++++++++ > > 1 file changed, 15 insertions(+) > > > > diff --git a/drivers/dma/dw-edma/dw-edma-pcie.c b/drivers/dma/dw-edma/dw-edma-pcie.c > > index 04c95cba1244..f530bacfd716 100644 > > --- a/drivers/dma/dw-edma/dw-edma-pcie.c > > +++ b/drivers/dma/dw-edma/dw-edma-pcie.c > > @@ -95,8 +95,23 @@ static int dw_edma_pcie_irq_vector(struct device *dev, unsigned int nr) > > return pci_irq_vector(to_pci_dev(dev), nr); > > } > > > > +static u64 dw_edma_pcie_address(struct device *dev, phys_addr_t cpu_addr) > > +{ > > + struct pci_dev *pdev = to_pci_dev(dev); > > + struct pci_bus_region region; > > + struct resource res = { > > + .flags = IORESOURCE_MEM, > > + .start = cpu_addr, > > + .end = cpu_addr, > > + }; > > + > > + pcibios_resource_to_bus(pdev->bus, ®ion, &res); > > + return region.start; > > +} > > This doesn't look DW-specific. Do you expect other implementations > that are specific, or could this be a generic function that shares > some implementation with pci_bus_address()? I have doubts any other implementation would need such method of the address translation. It's specific to the remote DW eDMA setup (DW eDMA embedded into a PCIe end-point and accessed from a PCIe host-side), which support is implemented in the drivers/dma/dw-edma/dw-edma-pcie.c driver and it doesn't seem to be a widespread type of devices anyway. The only known case is the Synopsys PCIe EndPoint IP prototype kit mentioned in the initial DW eDMA driver commit. In case of the local DW eDMA setup (DW eDMA embedded into the DW PCIe Root Port/Complex/End-point controllers and accessible via the CPU-side of the controllers) the CPU->PCIe address translation is performed by the outbound iATU engine if the DMA_BYPASS flag is cleared in the outbound iATU windows control register. The __dw_pcie_prog_outbound_atu() function (see pcie-designware.c) doesn't set that flag. That's why we can freely omit the dw_edma_core_ops.pci_address callback. On the other hand the DW PCIe controller can have a customized ATU (see dw_pcie_rp.bridge->child_ops can be pre-initialized by the DW PCIe low-level drivers). In that case I can't predict of how the eDMA engine would work. It can be as much similar to the iATU DMA_BYPASS-flag-less case or work on the vendor-specific basis. It's up to LLDD to implement the proper dw_edma_core_ops.pci_address callback then. -Serge(y) > > > static const struct dw_edma_core_ops dw_edma_pcie_core_ops = { > > .irq_vector = dw_edma_pcie_irq_vector, > > + .pci_address = dw_edma_pcie_address, > > }; > > > > static void dw_edma_pcie_get_vsec_dma_data(struct pci_dev *pdev, > > -- > > 2.39.0 > > > >
diff --git a/drivers/dma/dw-edma/dw-edma-pcie.c b/drivers/dma/dw-edma/dw-edma-pcie.c index 04c95cba1244..f530bacfd716 100644 --- a/drivers/dma/dw-edma/dw-edma-pcie.c +++ b/drivers/dma/dw-edma/dw-edma-pcie.c @@ -95,8 +95,23 @@ static int dw_edma_pcie_irq_vector(struct device *dev, unsigned int nr) return pci_irq_vector(to_pci_dev(dev), nr); } +static u64 dw_edma_pcie_address(struct device *dev, phys_addr_t cpu_addr) +{ + struct pci_dev *pdev = to_pci_dev(dev); + struct pci_bus_region region; + struct resource res = { + .flags = IORESOURCE_MEM, + .start = cpu_addr, + .end = cpu_addr, + }; + + pcibios_resource_to_bus(pdev->bus, ®ion, &res); + return region.start; +} + static const struct dw_edma_core_ops dw_edma_pcie_core_ops = { .irq_vector = dw_edma_pcie_irq_vector, + .pci_address = dw_edma_pcie_address, }; static void dw_edma_pcie_get_vsec_dma_data(struct pci_dev *pdev,
In general the Synopsys PCIe EndPoint IP prototype kit can be attached to a PCIe bus with any PCIe Host controller including to the one with distinctive from CPU address space. Due to that we need to make sure that the source and destination addresses of the DMA-slave devices are properly converted to the PCIe bus address space, otherwise the DMA transaction will not only work as expected, but may cause the memory corruption with subsequent system crash. Let's do that by introducing a new dw_edma_pcie_address() method defined in the dw-edma-pcie.c, which will perform the denoted translation by using the pcibios_resource_to_bus() method. Fixes: 41aaff2a2ac0 ("dmaengine: Add Synopsys eDMA IP PCIe glue-logic") Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru> Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> Tested-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> Acked-by: Vinod Koul <vkoul@kernel.org> --- Note this patch depends on the patch "dmaengine: dw-edma: Add CPU to PCIe bus address translation" from this series. --- drivers/dma/dw-edma/dw-edma-pcie.c | 15 +++++++++++++++ 1 file changed, 15 insertions(+)