add support for Xilinx PCIe root ports on RISC-V

add support for Xilinx PCIe root ports on RISC-V

[Christoph Hellwig]

Hi all,

this series with patches originally from Palmer and Wesley adds support
for the pcie-xilinx host driver on RISC-V boards.  The interesting part
about that is that the IP blocks is limited to 32-bit DMA internally,
which didn't seem to be an issue with the existing users, but shows
up easily with the Sifive RISC-V boards that have physical memory
wired up above 4G.  To support this the per-device flag I've added last
merge window is set through a new hook in struct pci_ops.

add support for Xilinx PCIe root ports on RISC-V

[Christoph Hellwig]

Hi all,

this series with patches originally from Palmer and Wesley adds support
for the pcie-xilinx host driver on RISC-V boards.  The interesting part
about that is that the IP blocks is limited to 32-bit DMA internally,
which didn't seem to be an issue with the existing users, but shows
up easily with the Sifive RISC-V boards that have physical memory
wired up above 4G.  To support this the per-device flag I've added last
merge window is set through a new hook in struct pci_ops.

[PATCH 1/3] PCI: Add hooks for bus/bridge-specific fixups

[Christoph Hellwig]

From: "Wesley W. Terpstra" <wesley@sifive.com>

There is currently no way for a PCIe bridge to impose constraints on
devices added to it.  For example, the Xilinx PCIe host bridge only
supports 32-bit physical addresses (due to a limitation on the AXI
port's address width).  Thus, even devices that claim to support 64-bit
DMA addresses must be restricted to 32-bit addresses when attached to
this host controller.

This patch adds at "add_dev" hook to pci_ops that allows the PCI
infastructure to interpose when adding a device, which in the case of
the Xilinx driver will be used to restrict the address ranges used for
DMA.

Signed-off-by: Wesley W. Terpstra <wesley@sifive.com>
[hch: simplified the prototype]
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/pci/probe.c | 3 +++
 include/linux/pci.h | 1 +
 2 files changed, 4 insertions(+)

diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index ac876e32de4b..978e684cba2c 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -2331,6 +2331,9 @@ void pci_device_add(struct pci_dev *dev, struct pci_bus *bus)
 	/* Set up MSI IRQ domain */
 	pci_set_msi_domain(dev);
 
+	if (bus->ops->add_dev)
+		bus->ops->add_dev(dev, bus);
+
 	/* Notifier could use PCI capabilities */
 	dev->match_driver = false;
 	ret = device_add(&dev->dev);
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 340029b2fb38..ea9609fc44fc 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -662,6 +662,7 @@ static inline int pcibios_err_to_errno(int err)
 struct pci_ops {
 	int (*add_bus)(struct pci_bus *bus);
 	void (*remove_bus)(struct pci_bus *bus);
+	void (*add_dev)(struct pci_dev *dev, struct pci_bus *bus);
 	void __iomem *(*map_bus)(struct pci_bus *bus, unsigned int devfn, int where);
 	int (*read)(struct pci_bus *bus, unsigned int devfn, int where, int size, u32 *val);
 	int (*write)(struct pci_bus *bus, unsigned int devfn, int where, int size, u32 val);
-- 
2.17.1

[PATCH 1/3] PCI: Add hooks for bus/bridge-specific fixups

[Christoph Hellwig]

From: "Wesley W. Terpstra" <wesley@sifive.com>

There is currently no way for a PCIe bridge to impose constraints on
devices added to it.  For example, the Xilinx PCIe host bridge only
supports 32-bit physical addresses (due to a limitation on the AXI
port's address width).  Thus, even devices that claim to support 64-bit
DMA addresses must be restricted to 32-bit addresses when attached to
this host controller.

This patch adds at "add_dev" hook to pci_ops that allows the PCI
infastructure to interpose when adding a device, which in the case of
the Xilinx driver will be used to restrict the address ranges used for
DMA.

Signed-off-by: Wesley W. Terpstra <wesley@sifive.com>
[hch: simplified the prototype]
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/pci/probe.c | 3 +++
 include/linux/pci.h | 1 +
 2 files changed, 4 insertions(+)

diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index ac876e32de4b..978e684cba2c 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -2331,6 +2331,9 @@ void pci_device_add(struct pci_dev *dev, struct pci_bus *bus)
 	/* Set up MSI IRQ domain */
 	pci_set_msi_domain(dev);
 
+	if (bus->ops->add_dev)
+		bus->ops->add_dev(dev, bus);
+
 	/* Notifier could use PCI capabilities */
 	dev->match_driver = false;
 	ret = device_add(&dev->dev);
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 340029b2fb38..ea9609fc44fc 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -662,6 +662,7 @@ static inline int pcibios_err_to_errno(int err)
 struct pci_ops {
 	int (*add_bus)(struct pci_bus *bus);
 	void (*remove_bus)(struct pci_bus *bus);
+	void (*add_dev)(struct pci_dev *dev, struct pci_bus *bus);
 	void __iomem *(*map_bus)(struct pci_bus *bus, unsigned int devfn, int where);
 	int (*read)(struct pci_bus *bus, unsigned int devfn, int where, int size, u32 *val);
 	int (*write)(struct pci_bus *bus, unsigned int devfn, int where, int size, u32 val);
-- 
2.17.1

Re: [PATCH 1/3] PCI: Add hooks for bus/bridge-specific fixups

[Arnd Bergmann]

On Tue, Jun 19, 2018 at 4:16 PM, Christoph Hellwig <hch@lst.de> wrote:
> From: "Wesley W. Terpstra" <wesley@sifive.com>

> index 340029b2fb38..ea9609fc44fc 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -662,6 +662,7 @@ static inline int pcibios_err_to_errno(int err)
>  struct pci_ops {
>         int (*add_bus)(struct pci_bus *bus);
>         void (*remove_bus)(struct pci_bus *bus);
> +       void (*add_dev)(struct pci_dev *dev, struct pci_bus *bus);
>         void __iomem *(*map_bus)(struct pci_bus *bus, unsigned int devfn, int where);
>         int (*read)(struct pci_bus *bus, unsigned int devfn, int where, int size, u32 *val);
>         int (*write)(struct pci_bus *bus, unsigned int devfn, int where, int size, u32 val);

We are a bit inconsistent about adding callbacks to pci_ops or pci_host_bridge.
I would argue this should be part of pci_host_bridge, and the other two should
probably be there as well, or possibly moved into a separate
'pci_host_bridge_ops'
structure referenced from pci_host_bridge along with the other callbacks there.

One of the things we discussed in the past (but never implemented) is that
many of the current '__weak' functions in the PCI core can be turned into
host_bridge specific operations.

Some config space operations by contrast are currently shared across host
bridge drivers (see pci_bus_set_ops, or drivers/pci/ecam.c) and probably
better left out of the host bridge.

      Arnd

[PATCH 1/3] PCI: Add hooks for bus/bridge-specific fixups

[Arnd Bergmann]

On Tue, Jun 19, 2018 at 4:16 PM, Christoph Hellwig <hch@lst.de> wrote:
> From: "Wesley W. Terpstra" <wesley@sifive.com>

> index 340029b2fb38..ea9609fc44fc 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -662,6 +662,7 @@ static inline int pcibios_err_to_errno(int err)
>  struct pci_ops {
>         int (*add_bus)(struct pci_bus *bus);
>         void (*remove_bus)(struct pci_bus *bus);
> +       void (*add_dev)(struct pci_dev *dev, struct pci_bus *bus);
>         void __iomem *(*map_bus)(struct pci_bus *bus, unsigned int devfn, int where);
>         int (*read)(struct pci_bus *bus, unsigned int devfn, int where, int size, u32 *val);
>         int (*write)(struct pci_bus *bus, unsigned int devfn, int where, int size, u32 val);

We are a bit inconsistent about adding callbacks to pci_ops or pci_host_bridge.
I would argue this should be part of pci_host_bridge, and the other two should
probably be there as well, or possibly moved into a separate
'pci_host_bridge_ops'
structure referenced from pci_host_bridge along with the other callbacks there.

One of the things we discussed in the past (but never implemented) is that
many of the current '__weak' functions in the PCI core can be turned into
host_bridge specific operations.

Some config space operations by contrast are currently shared across host
bridge drivers (see pci_bus_set_ops, or drivers/pci/ecam.c) and probably
better left out of the host bridge.

      Arnd

Re: [PATCH 1/3] PCI: Add hooks for bus/bridge-specific fixups

[Christoph Hellwig]

On Tue, Jun 19, 2018 at 04:31:06PM +0200, Arnd Bergmann wrote:
> We are a bit inconsistent about adding callbacks to pci_ops or pci_host_bridge.
> I would argue this should be part of pci_host_bridge, and the other two should
> probably be there as well, or possibly moved into a separate
> 'pci_host_bridge_ops'
> structure referenced from pci_host_bridge along with the other callbacks there.

How do we find the proper pci_host_bridge from an arbitrary
device deep down the hierachy below it?

[PATCH 1/3] PCI: Add hooks for bus/bridge-specific fixups

[Christoph Hellwig]

On Tue, Jun 19, 2018 at 04:31:06PM +0200, Arnd Bergmann wrote:
> We are a bit inconsistent about adding callbacks to pci_ops or pci_host_bridge.
> I would argue this should be part of pci_host_bridge, and the other two should
> probably be there as well, or possibly moved into a separate
> 'pci_host_bridge_ops'
> structure referenced from pci_host_bridge along with the other callbacks there.

How do we find the proper pci_host_bridge from an arbitrary
device deep down the hierachy below it?

Re: [PATCH 1/3] PCI: Add hooks for bus/bridge-specific fixups

[Sinan Kaya]

On Tue, Jun 19, 2018 at 07:32:40PM +0200, Christoph Hellwig wrote:
> On Tue, Jun 19, 2018 at 04:31:06PM +0200, Arnd Bergmann wrote:
> > We are a bit inconsistent about adding callbacks to pci_ops or pci_host_bridge.
> > I would argue this should be part of pci_host_bridge, and the other two should
> > probably be there as well, or possibly moved into a separate
> > 'pci_host_bridge_ops'
> > structure referenced from pci_host_bridge along with the other callbacks there.
> 
> How do we find the proper pci_host_bridge from an arbitrary
> device deep down the hierachy below it?

pci_get_host_bridge_device() should do the work.

[PATCH 1/3] PCI: Add hooks for bus/bridge-specific fixups

[Sinan Kaya]

On Tue, Jun 19, 2018 at 07:32:40PM +0200, Christoph Hellwig wrote:
> On Tue, Jun 19, 2018 at 04:31:06PM +0200, Arnd Bergmann wrote:
> > We are a bit inconsistent about adding callbacks to pci_ops or pci_host_bridge.
> > I would argue this should be part of pci_host_bridge, and the other two should
> > probably be there as well, or possibly moved into a separate
> > 'pci_host_bridge_ops'
> > structure referenced from pci_host_bridge along with the other callbacks there.
> 
> How do we find the proper pci_host_bridge from an arbitrary
> device deep down the hierachy below it?

pci_get_host_bridge_device() should do the work.

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

[Christoph Hellwig]

From: "Wesley W. Terpstra" <wesley@sifive.com>

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.

Signed-off-by: Wesley W. Terpstra <wesley@sifive.com>
[hch: switched to setting the dma_32bit_limit flag instead of overriding
 the dma_map_ops]
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/pci/controller/pcie-xilinx.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/pci/controller/pcie-xilinx.c b/drivers/pci/controller/pcie-xilinx.c
index b110a3a814e3..f6b41df59886 100644
--- a/drivers/pci/controller/pcie-xilinx.c
+++ b/drivers/pci/controller/pcie-xilinx.c
@@ -197,11 +197,21 @@ 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 void xilinx_pcie_add_dev(struct pci_dev *pdev, struct pci_bus *bus)
+{
+	pdev->dev.dma_32bit_limit = true;
+}
+
 /* PCIe operations */
 static struct pci_ops xilinx_pcie_ops = {
 	.map_bus = xilinx_pcie_map_bus,
 	.read	= pci_generic_config_read,
 	.write	= pci_generic_config_write,
+	.add_dev = xilinx_pcie_add_dev,
 };
 
 /* MSI functions */
-- 
2.17.1

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

[Christoph Hellwig]

From: "Wesley W. Terpstra" <wesley@sifive.com>

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.

Signed-off-by: Wesley W. Terpstra <wesley@sifive.com>
[hch: switched to setting the dma_32bit_limit flag instead of overriding
 the dma_map_ops]
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/pci/controller/pcie-xilinx.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/pci/controller/pcie-xilinx.c b/drivers/pci/controller/pcie-xilinx.c
index b110a3a814e3..f6b41df59886 100644
--- a/drivers/pci/controller/pcie-xilinx.c
+++ b/drivers/pci/controller/pcie-xilinx.c
@@ -197,11 +197,21 @@ 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 void xilinx_pcie_add_dev(struct pci_dev *pdev, struct pci_bus *bus)
+{
+	pdev->dev.dma_32bit_limit = true;
+}
+
 /* PCIe operations */
 static struct pci_ops xilinx_pcie_ops = {
 	.map_bus = xilinx_pcie_map_bus,
 	.read	= pci_generic_config_read,
 	.write	= pci_generic_config_write,
+	.add_dev = xilinx_pcie_add_dev,
 };
 
 /* MSI functions */
-- 
2.17.1

[PATCH 3/3] PCI/xilinx: Depend on OF instead of the ARCH

[Christoph Hellwig]

From: Palmer Dabbelt <palmer@dabbelt.com>

There isn't a hard dependency of the Xilinx AXI-PCIe host bridge on any
architecture.  For example: at SiFive we map RISC-V cores to Xilinx FPGAs
and connect the Xilinx IP via a TileLink adapter, so the RISC-V Linux
port will need to be able to enable PCIE_XILINX in order to have PCIe
support.

This patch decouples the PCIE_XILINX support from ARCH.  Instead it just
depends on OF, which I believe is the only true dependency.

Signed-off-by: Palmer Dabbelt <palmer@dabbelt.com>
[hch: replaced the now removed OF_PCI dependency with just OF]
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/pci/controller/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/pci/controller/Kconfig b/drivers/pci/controller/Kconfig
index 18fa09b3ac8f..1fc3d4eb373d 100644
--- a/drivers/pci/controller/Kconfig
+++ b/drivers/pci/controller/Kconfig
@@ -103,7 +103,7 @@ config PCI_HOST_GENERIC
 
 config PCIE_XILINX
 	bool "Xilinx AXI PCIe host bridge support"
-	depends on ARCH_ZYNQ || MICROBLAZE || (MIPS && PCI_DRIVERS_GENERIC) || COMPILE_TEST
+	depends on OF || COMPILE_TEST
 	help
 	  Say 'Y' here if you want kernel to support the Xilinx AXI PCIe
 	  Host Bridge driver.
-- 
2.17.1

[PATCH 3/3] PCI/xilinx: Depend on OF instead of the ARCH

[Christoph Hellwig]

From: Palmer Dabbelt <palmer@dabbelt.com>

There isn't a hard dependency of the Xilinx AXI-PCIe host bridge on any
architecture.  For example: at SiFive we map RISC-V cores to Xilinx FPGAs
and connect the Xilinx IP via a TileLink adapter, so the RISC-V Linux
port will need to be able to enable PCIE_XILINX in order to have PCIe
support.

This patch decouples the PCIE_XILINX support from ARCH.  Instead it just
depends on OF, which I believe is the only true dependency.

Signed-off-by: Palmer Dabbelt <palmer@dabbelt.com>
[hch: replaced the now removed OF_PCI dependency with just OF]
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/pci/controller/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/pci/controller/Kconfig b/drivers/pci/controller/Kconfig
index 18fa09b3ac8f..1fc3d4eb373d 100644
--- a/drivers/pci/controller/Kconfig
+++ b/drivers/pci/controller/Kconfig
@@ -103,7 +103,7 @@ config PCI_HOST_GENERIC
 
 config PCIE_XILINX
 	bool "Xilinx AXI PCIe host bridge support"
-	depends on ARCH_ZYNQ || MICROBLAZE || (MIPS && PCI_DRIVERS_GENERIC) || COMPILE_TEST
+	depends on OF || COMPILE_TEST
 	help
 	  Say 'Y' here if you want kernel to support the Xilinx AXI PCIe
 	  Host Bridge driver.
-- 
2.17.1