[PATCH 0/2] pcie/designware: Viewport assignment update

[PATCH 0/2] pcie/designware: Viewport assignment update

[Pratyush Anand]

Most of the designware PCIe platforms have more than 2 viewports. So, patch
1/2 allows to fix viewport-2 for IO transaction for such cases.
When we have only two viewports, then patch 2/2 exchange viewport
assignment of memory and cfg transaction. It helps to fix a possibility of
corruption for memory transaction. However, there still exist possibility
of an IO transaction to be corrupted. We can not do much for <=2 viewports.
See, patch log for detail.

Dong Bo (1):
  pcie/designware: Exchange viewport of `MEMORYs' and `CFGs/IOs'

Pratyush Anand (1):
  pcie/designware: Keep viewport fixed for IO transaction if
    num_viewport > 2

 .../devicetree/bindings/pci/designware-pcie.txt    |  3 ++
 drivers/pci/host/pcie-designware.c                 | 32 +++++++++++++++-------
 drivers/pci/host/pcie-designware.h                 |  1 +
 3 files changed, 26 insertions(+), 10 deletions(-)

-- 
2.5.5

[PATCH 1/2] pcie/designware: Keep viewport fixed for IO transaction if num_viewport > 2

[Pratyush Anand]

From: Pratyush Anand <pratyush.anand@gmail.com>

Most of the platform have 3 or more viewports. We do not need to share
viewport for such platform between IO and CFG. Assign viewport 2 to IO
transactions in such cases.

Signed-off-by: Pratyush Anand <pratyush.anand@gmail.com>
Tested-by: Dong Bo <dongbo4@huawei.com>
---
 .../devicetree/bindings/pci/designware-pcie.txt    |  3 +++
 drivers/pci/host/pcie-designware.c                 | 26 ++++++++++++++++------
 drivers/pci/host/pcie-designware.h                 |  1 +
 3 files changed, 23 insertions(+), 7 deletions(-)

diff --git a/Documentation/devicetree/bindings/pci/designware-pcie.txt b/Documentation/devicetree/bindings/pci/designware-pcie.txt
index 6c5322c55411..a72b7ecb80ee 100644
--- a/Documentation/devicetree/bindings/pci/designware-pcie.txt
+++ b/Documentation/devicetree/bindings/pci/designware-pcie.txt
@@ -17,6 +17,8 @@ Required properties:
 - num-lanes: number of lanes to use
 
 Optional properties:
+- num-viewport: number of view port configured in hardware. If a platform
+  does not specify it then driver will consider num-viewport as 2.
 - num-lanes: number of lanes to use (this property should be specified unless
   the link is brought already up in BIOS)
 - reset-gpio: gpio pin number of power good signal
@@ -44,4 +46,5 @@ Example configuration:
 		interrupts = <25>, <24>;
 		#interrupt-cells = <1>;
 		num-lanes = <1>;
+		num-viewport = <3>;
 	};
diff --git a/drivers/pci/host/pcie-designware.c b/drivers/pci/host/pcie-designware.c
index aafd766546f3..4c17c02ad72b 100644
--- a/drivers/pci/host/pcie-designware.c
+++ b/drivers/pci/host/pcie-designware.c
@@ -51,6 +51,7 @@
 #define PCIE_ATU_VIEWPORT		0x900
 #define PCIE_ATU_REGION_INBOUND		(0x1 << 31)
 #define PCIE_ATU_REGION_OUTBOUND	(0x0 << 31)
+#define PCIE_ATU_REGION_INDEX2		(0x2 << 0)
 #define PCIE_ATU_REGION_INDEX1		(0x1 << 0)
 #define PCIE_ATU_REGION_INDEX0		(0x0 << 0)
 #define PCIE_ATU_CR1			0x904
@@ -521,6 +522,10 @@ int dw_pcie_host_init(struct pcie_port *pp)
 	if (ret)
 		pp->lanes = 0;
 
+	ret = of_property_read_u32(np, "num-viewport", &pp->num_viewport);
+	if (ret)
+		pp->num_viewport = 2;
+
 	if (IS_ENABLED(CONFIG_PCI_MSI)) {
 		if (!pp->ops->msi_host_init) {
 			pp->irq_domain = irq_domain_add_linear(pp->dev->of_node,
@@ -603,9 +608,10 @@ static int dw_pcie_rd_other_conf(struct pcie_port *pp, struct pci_bus *bus,
 				  type, cpu_addr,
 				  busdev, cfg_size);
 	ret = dw_pcie_cfg_read(va_cfg_base + where, size, val);
-	dw_pcie_prog_outbound_atu(pp, PCIE_ATU_REGION_INDEX0,
-				  PCIE_ATU_TYPE_IO, pp->io_base,
-				  pp->io_bus_addr, pp->io_size);
+	if (pp->num_viewport <= 2)
+		dw_pcie_prog_outbound_atu(pp, PCIE_ATU_REGION_INDEX0,
+					  PCIE_ATU_TYPE_IO, pp->io_base,
+					  pp->io_bus_addr, pp->io_size);
 
 	return ret;
 }
@@ -640,9 +646,10 @@ static int dw_pcie_wr_other_conf(struct pcie_port *pp, struct pci_bus *bus,
 				  type, cpu_addr,
 				  busdev, cfg_size);
 	ret = dw_pcie_cfg_write(va_cfg_base + where, size, val);
-	dw_pcie_prog_outbound_atu(pp, PCIE_ATU_REGION_INDEX0,
-				  PCIE_ATU_TYPE_IO, pp->io_base,
-				  pp->io_bus_addr, pp->io_size);
+	if (pp->num_viewport <= 2)
+		dw_pcie_prog_outbound_atu(pp, PCIE_ATU_REGION_INDEX0,
+					  PCIE_ATU_TYPE_IO, pp->io_base,
+					  pp->io_bus_addr, pp->io_size);
 
 	return ret;
 }
@@ -778,10 +785,15 @@ void dw_pcie_setup_rc(struct pcie_port *pp)
 	 * uses its own address translation component rather than ATU, so
 	 * we should not program the ATU here.
 	 */
-	if (!pp->ops->rd_other_conf)
+	if (!pp->ops->rd_other_conf) {
 		dw_pcie_prog_outbound_atu(pp, PCIE_ATU_REGION_INDEX1,
 					  PCIE_ATU_TYPE_MEM, pp->mem_base,
 					  pp->mem_bus_addr, pp->mem_size);
+		if (pp->num_viewport > 2)
+			dw_pcie_prog_outbound_atu(pp, PCIE_ATU_REGION_INDEX2,
+						  PCIE_ATU_TYPE_IO, pp->io_base,
+						  pp->io_bus_addr, pp->io_size);
+	}
 
 	dw_pcie_wr_own_conf(pp, PCI_BASE_ADDRESS_0, 4, 0);
 
diff --git a/drivers/pci/host/pcie-designware.h b/drivers/pci/host/pcie-designware.h
index f437f9b5be04..61f479dd181d 100644
--- a/drivers/pci/host/pcie-designware.h
+++ b/drivers/pci/host/pcie-designware.h
@@ -49,6 +49,7 @@ struct pcie_port {
 	struct resource		*busn;
 	int			irq;
 	u32			lanes;
+	u32			num_viewport;
 	struct pcie_host_ops	*ops;
 	int			msi_irq;
 	struct irq_domain	*irq_domain;
-- 
2.5.5

Re: [PATCH 1/2] pcie/designware: Keep viewport fixed for IO transaction if num_viewport > 2

[Rob Herring]

On Mon, Jul 04, 2016 at 09:44:42PM +0530, Pratyush Anand wrote:
> From: Pratyush Anand <pratyush.anand@gmail.com>
> 
> Most of the platform have 3 or more viewports. We do not need to share
> viewport for such platform between IO and CFG. Assign viewport 2 to IO
> transactions in such cases.
> 
> Signed-off-by: Pratyush Anand <pratyush.anand@gmail.com>
> Tested-by: Dong Bo <dongbo4@huawei.com>
> ---
>  .../devicetree/bindings/pci/designware-pcie.txt    |  3 +++
>  drivers/pci/host/pcie-designware.c                 | 26 ++++++++++++++++------
>  drivers/pci/host/pcie-designware.h                 |  1 +
>  3 files changed, 23 insertions(+), 7 deletions(-)

Acked-by: Rob Herring <robh@kernel.org>

[PATCH 2/2] pcie/designware: Exchange viewport of `MEMORYs' and `CFGs/IOs'

[Pratyush Anand]

From: Dong Bo <dongbo4@huawei.com>

When we have only two view ports in a designware PCIe platform, then iatu0
is used for both CFG and IO accesses.  When CFGs are sent to peripherals
(e.g. lspci), iatu0 frequently switches between CFG and IO alternatively.

For such scenarios, a MEMORY might be sent as an IOs by mistake.
Considering the following configurations:
MEMORY          ->      BASE_ADDR: 0xb4100000, LIMIT: 0xb4100FFF, TYPE=mem
CFG             ->      BASE_ADDR: 0xb4000000, LIMIT: 0xb4000FFF, TYPE=cfg
IO              ->      BASE_ADDR: 0xFFFFFFFF, LIMIT: 0xFFFFFFFE, TYPE=io

Suppose PCIe has just completed a CFG access, to switch back to IO, it set
the BASE_ADDR to 0xFFFFFFFF, LIMIT 0xFFFFFFFE and TYPE to io. When another
CFG comes, the BASE_ADDR is set to 0xb4000000 to switch to CFG. At this
moment, a MEMORY access shows up, since it matches with iatu0
(due to 0xb4000000 <= MEMORY BASE_ADDR <= MEMORY LIMIE <= 0xFFFFFFF), it
is treated as an IO access by mistake, then sent to perpheral.

This patch fixes the problem by exchanging the assignments of `MEMORYs'
and `CFGs/IOs', which assigning MEMEORYs to iatu0, CFGs and IOs to iatu1.

We can still have issues with IO transfer, however memory transfer is used
predominantly therefore we are just minimizing the risk of failure.
Actually, we can not do much when we have only two viewports. We can either
not allow the less frequent IO transfers at all, or can live with a
remote possibility of getting it corrupted.

Signed-off-by: Dong Bo <dongbo4@huawei.com>
[pratyush.anand@gmail.com: Modified commit log to capture remote risk]
Signed-off-by: Pratyush Anand <pratyush.anand@gmail.com>
---
 drivers/pci/host/pcie-designware.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/pci/host/pcie-designware.c b/drivers/pci/host/pcie-designware.c
index 4c17c02ad72b..fe3d48c318df 100644
--- a/drivers/pci/host/pcie-designware.c
+++ b/drivers/pci/host/pcie-designware.c
@@ -604,12 +604,12 @@ static int dw_pcie_rd_other_conf(struct pcie_port *pp, struct pci_bus *bus,
 		va_cfg_base = pp->va_cfg1_base;
 	}
 
-	dw_pcie_prog_outbound_atu(pp, PCIE_ATU_REGION_INDEX0,
+	dw_pcie_prog_outbound_atu(pp, PCIE_ATU_REGION_INDEX1,
 				  type, cpu_addr,
 				  busdev, cfg_size);
 	ret = dw_pcie_cfg_read(va_cfg_base + where, size, val);
 	if (pp->num_viewport <= 2)
-		dw_pcie_prog_outbound_atu(pp, PCIE_ATU_REGION_INDEX0,
+		dw_pcie_prog_outbound_atu(pp, PCIE_ATU_REGION_INDEX1,
 					  PCIE_ATU_TYPE_IO, pp->io_base,
 					  pp->io_bus_addr, pp->io_size);
 
@@ -642,12 +642,12 @@ static int dw_pcie_wr_other_conf(struct pcie_port *pp, struct pci_bus *bus,
 		va_cfg_base = pp->va_cfg1_base;
 	}
 
-	dw_pcie_prog_outbound_atu(pp, PCIE_ATU_REGION_INDEX0,
+	dw_pcie_prog_outbound_atu(pp, PCIE_ATU_REGION_INDEX1,
 				  type, cpu_addr,
 				  busdev, cfg_size);
 	ret = dw_pcie_cfg_write(va_cfg_base + where, size, val);
 	if (pp->num_viewport <= 2)
-		dw_pcie_prog_outbound_atu(pp, PCIE_ATU_REGION_INDEX0,
+		dw_pcie_prog_outbound_atu(pp, PCIE_ATU_REGION_INDEX1,
 					  PCIE_ATU_TYPE_IO, pp->io_base,
 					  pp->io_bus_addr, pp->io_size);
 
@@ -786,7 +786,7 @@ void dw_pcie_setup_rc(struct pcie_port *pp)
 	 * we should not program the ATU here.
 	 */
 	if (!pp->ops->rd_other_conf) {
-		dw_pcie_prog_outbound_atu(pp, PCIE_ATU_REGION_INDEX1,
+		dw_pcie_prog_outbound_atu(pp, PCIE_ATU_REGION_INDEX0,
 					  PCIE_ATU_TYPE_MEM, pp->mem_base,
 					  pp->mem_bus_addr, pp->mem_size);
 		if (pp->num_viewport > 2)
-- 
2.5.5

Re: [PATCH 0/2] pcie/designware: Viewport assignment update

[Bjorn Helgaas]

On Mon, Jul 04, 2016 at 09:44:41PM +0530, Pratyush Anand wrote:
> Most of the designware PCIe platforms have more than 2 viewports. So, patch
> 1/2 allows to fix viewport-2 for IO transaction for such cases.
> When we have only two viewports, then patch 2/2 exchange viewport
> assignment of memory and cfg transaction. It helps to fix a possibility of
> corruption for memory transaction. However, there still exist possibility
> of an IO transaction to be corrupted. We can not do much for <=2 viewports.
> See, patch log for detail.
> 
> Dong Bo (1):
>   pcie/designware: Exchange viewport of `MEMORYs' and `CFGs/IOs'
> 
> Pratyush Anand (1):
>   pcie/designware: Keep viewport fixed for IO transaction if
>     num_viewport > 2
> 
>  .../devicetree/bindings/pci/designware-pcie.txt    |  3 ++
>  drivers/pci/host/pcie-designware.c                 | 32 +++++++++++++++-------
>  drivers/pci/host/pcie-designware.h                 |  1 +
>  3 files changed, 26 insertions(+), 10 deletions(-)

Applied (with Rob's ack on 1/2) to pci/host-designware for v4.9, thanks!