Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Sat, 21 Jan 2023 23:37:02 +0300
From:   Serge Semin <fancer.lancer@gmail.com>
To:     Bjorn Helgaas <helgaas@kernel.org>
Cc:     Serge Semin <Sergey.Semin@baikalelectronics.ru>,
        Gustavo Pimentel <gustavo.pimentel@synopsys.com>,
        Vinod Koul <vkoul@kernel.org>, Rob Herring <robh@kernel.org>,
        Bjorn Helgaas <bhelgaas@google.com>,
        Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>,
        Cai Huoqing <cai.huoqing@linux.dev>,
        Robin Murphy <robin.murphy@arm.com>,
        Jingoo Han <jingoohan1@gmail.com>, Frank Li <Frank.Li@nxp.com>,
        Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>,
        Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>,
        Pavel Parkhomenko <Pavel.Parkhomenko@baikalelectronics.ru>,
        Krzysztof =?utf-8?Q?Wilczy=C5=84ski?= <kw@linux.com>,
        caihuoqing <caihuoqing@baidu.com>,
        Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>,
        linux-pci@vger.kernel.org, dmaengine@vger.kernel.org,
        linux-kernel@vger.kernel.org
Subject: Re: [PATCH v9 08/27] dmaengine: dw-edma: Add PCIe bus address getter
 to the remote EP glue-driver
Message-ID: <20230121203702.kgo7rdeiu7pvwh2b@mobilestation>
References: <20230113171409.30470-9-Sergey.Semin@baikalelectronics.ru>
 <20230120232920.GA681120@bhelgaas>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20230120232920.GA681120@bhelgaas>
Precedence: bulk

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, &region, &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
> > 
> >