| Message ID | 20180804101402.10022-3-hch@lst.de (mailing list archive) |
|---|---|
| State | New, archived |
| Delegated to: | Bjorn Helgaas |
| Headers | show |
| Series | [1/3] PCI: add a callback to struct pci_host_bridge for adding a new device | expand |
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).
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;
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(+)