[2/3] PCI/xilinx: Work-around for hardware DMA limit (32 bits)

This PCIe bridge only has a 32 bit bus master interface, thus truncating
the DMA capability of all PCIe devices attached beneath it. This caps
the child device capability so that these devices work on systems with
physical memory beyond the 4GiB threshold.

Based on an earlier patch from Wesley W. Terpstra <wesley@sifive.com>.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/pci/controller/pcie-xilinx.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

FYI, This Xilinx PCIe IP 32-bit cap only applies to SOME instances of
the IP. The Ultrascale+ version of Xilinx PCIe hard IP does support
64-bit or 32-bit. The Virtex7 version only supports 32-bit. The
pcie-xilinx driver woks with both of these root complexes. So probably
there should be a conditional hook in the DTS that triggers the
work-around behaviour.

On Sat, Aug 4, 2018 at 3:14 AM, Christoph Hellwig <hch@lst.de> wrote:
> This PCIe bridge only has a 32 bit bus master interface, thus truncating
> the DMA capability of all PCIe devices attached beneath it. This caps
> the child device capability so that these devices work on systems with
> physical memory beyond the 4GiB threshold.
>
> Based on an earlier patch from Wesley W. Terpstra <wesley@sifive.com>.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  drivers/pci/controller/pcie-xilinx.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
>
> diff --git a/drivers/pci/controller/pcie-xilinx.c b/drivers/pci/controller/pcie-xilinx.c
> index 7b1389d8e2a5..ccfd91e0515f 100644
> --- a/drivers/pci/controller/pcie-xilinx.c
> +++ b/drivers/pci/controller/pcie-xilinx.c
> @@ -197,6 +197,16 @@ static void __iomem *xilinx_pcie_map_bus(struct pci_bus *bus,
>         return port->reg_base + relbus + where;
>  }
>
> +/*
> + * This PCIe bridge only has a 32 bit bus master interface, thus truncating
> + * the DMA capability of all PCIe devices attached beneath it.
> + */
> +static int xilinx_pcie_add_device(struct pci_dev *pdev)
> +{
> +       pdev->dev.bus_dma_mask = DMA_BIT_MASK(32);
> +       return 0;
> +}
> +
>  /* PCIe operations */
>  static struct pci_ops xilinx_pcie_ops = {
>         .map_bus = xilinx_pcie_map_bus,
> @@ -665,6 +675,7 @@ static int xilinx_pcie_probe(struct platform_device *pdev)
>         bridge->ops = &xilinx_pcie_ops;
>         bridge->map_irq = of_irq_parse_and_map_pci;
>         bridge->swizzle_irq = pci_common_swizzle;
> +       bridge->add_device = xilinx_pcie_add_device;
>
>  #ifdef CONFIG_PCI_MSI
>         xilinx_pcie_msi_chip.dev = dev;
> --
> 2.18.0
>

On Sun, Aug 05, 2018 at 01:02:58PM -0700, Wesley Terpstra wrote:
> FYI, This Xilinx PCIe IP 32-bit cap only applies to SOME instances of
> the IP. The Ultrascale+ version of Xilinx PCIe hard IP does support
> 64-bit or 32-bit. The Virtex7 version only supports 32-bit. The
> pcie-xilinx driver woks with both of these root complexes. So probably
> there should be a conditional hook in the DTS that triggers the
> work-around behaviour.

Either we'll need to able to detect which version we use based on
registrs, or we will indeed need firmware information.

Note that we already have the mechanism for firmware directed dma limits
in place, it is called the dma-ranges DT property.  If we can get the
SiFive firmware to set it up properly the RISC-V swiotlb code will
just do the right thing.

On Mon, Aug 06, 2018 at 02:35:27PM +0200, Christoph Hellwig wrote:
> On Sun, Aug 05, 2018 at 01:02:58PM -0700, Wesley Terpstra wrote:
> > FYI, This Xilinx PCIe IP 32-bit cap only applies to SOME instances of
> > the IP. The Ultrascale+ version of Xilinx PCIe hard IP does support
> > 64-bit or 32-bit. The Virtex7 version only supports 32-bit. The
> > pcie-xilinx driver woks with both of these root complexes. So probably
> > there should be a conditional hook in the DTS that triggers the
> > work-around behaviour.
> 
> Either we'll need to able to detect which version we use based on
> registrs, or we will indeed need firmware information.
> 
> Note that we already have the mechanism for firmware directed dma limits
> in place, it is called the dma-ranges DT property.  If we can get the
> SiFive firmware to set it up properly the RISC-V swiotlb code will
> just do the right thing.

It would do the right thing without patches 1 and 2 (I would
consider merging patch 1 anyway - see pcibios_add_device()
removal).

It seems to me that the best course of action consists in patching
firmware, that would remove the need for patch 2 (I will merge patch
1 anyway - even if that's for a "different" purpose, see
pcibios_add_device() removal), please let us know.

Thanks,
Lorenzo

On Mon, Aug 06, 2018 at 02:40:43PM +0100, Lorenzo Pieralisi wrote:
> It would do the right thing without patches 1 and 2 (I would
> consider merging patch 1 anyway - see pcibios_add_device()
> removal).
> 
> It seems to me that the best course of action consists in patching
> firmware, that would remove the need for patch 2 (I will merge patch
> 1 anyway - even if that's for a "different" purpose, see
> pcibios_add_device() removal), please let us know.

Sounds good to me.  Patch 1 also doesn't even depend on dma-mapping
bits so feel free to pull it in when you think is the best time.

On Mon, Aug 6, 2018 at 5:35 AM, Christoph Hellwig <hch@lst.de> wrote:
> Note that we already have the mechanism for firmware directed dma limits
> in place, it is called the dma-ranges DT property.  If we can get the
> SiFive firmware to set it up properly the RISC-V swiotlb code will
> just do the right thing.

Does this mean we only need to set the dma-ranges property inside the
pci DTS node? No changes to the driver needed?

On Mon, Aug 06, 2018 at 09:21:40AM -0700, Wesley Terpstra wrote:
> On Mon, Aug 6, 2018 at 5:35 AM, Christoph Hellwig <hch@lst.de> wrote:
> > Note that we already have the mechanism for firmware directed dma limits
> > in place, it is called the dma-ranges DT property.  If we can get the
> > SiFive firmware to set it up properly the RISC-V swiotlb code will
> > just do the right thing.
> 
> Does this mean we only need to set the dma-ranges property inside the
> pci DTS node? No changes to the driver needed?

The code looks at the DT parent of the PCI bridge device.  Take a look
at drivers/pci/pci-driver.c:pci_dma_configure() and
drivers/of/device.c:of_dma_configure() in 4.18-rc (for older kernels
the involved functions are slightly different, but the functionality
is the same).