* [PATCH 1/2] pcie/designware: Keep viewport fixed for IO transaction if num_viewport > 2
2016-07-04 16:14 [PATCH 0/2] pcie/designware: Viewport assignment update Pratyush Anand
@ 2016-07-04 16:14 ` Pratyush Anand
2016-07-04 16:14 ` [PATCH 2/2] pcie/designware: Exchange viewport of `MEMORYs' and `CFGs/IOs' Pratyush Anand
2016-08-22 18:49 ` [PATCH 0/2] pcie/designware: Viewport assignment update Bjorn Helgaas
2 siblings, 0 replies; 7+ messages in thread
From: Pratyush Anand @ 2016-07-04 16:14 UTC (permalink / raw)
To: dongbo4, bhelgaas, jingoohan1
Cc: linux-pci, Pratyush Anand,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Joao Pinto, open list, Mark Rutland, Rob Herring
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
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH 1/2] pcie/designware: Keep viewport fixed for IO transaction if num_viewport > 2
@ 2016-07-04 16:14 ` Pratyush Anand
0 siblings, 0 replies; 7+ messages in thread
From: Pratyush Anand @ 2016-07-04 16:14 UTC (permalink / raw)
To: dongbo4, bhelgaas, jingoohan1
Cc: linux-pci, Pratyush Anand,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Joao Pinto, open list, Mark Rutland, Rob Herring
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
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH 1/2] pcie/designware: Keep viewport fixed for IO transaction if num_viewport > 2
2016-07-04 16:14 ` Pratyush Anand
@ 2016-07-05 16:23 ` Rob Herring
-1 siblings, 0 replies; 7+ messages in thread
From: Rob Herring @ 2016-07-05 16:23 UTC (permalink / raw)
To: Pratyush Anand
Cc: dongbo4, bhelgaas, jingoohan1, linux-pci, Pratyush Anand,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Joao Pinto, open list, Mark Rutland
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>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] pcie/designware: Keep viewport fixed for IO transaction if num_viewport > 2
@ 2016-07-05 16:23 ` Rob Herring
0 siblings, 0 replies; 7+ messages in thread
From: Rob Herring @ 2016-07-05 16:23 UTC (permalink / raw)
To: Pratyush Anand
Cc: dongbo4, bhelgaas, jingoohan1, linux-pci, Pratyush Anand,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Joao Pinto, open list, Mark Rutland
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>
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 2/2] pcie/designware: Exchange viewport of `MEMORYs' and `CFGs/IOs'
2016-07-04 16:14 [PATCH 0/2] pcie/designware: Viewport assignment update Pratyush Anand
2016-07-04 16:14 ` Pratyush Anand
@ 2016-07-04 16:14 ` Pratyush Anand
2016-08-22 18:49 ` [PATCH 0/2] pcie/designware: Viewport assignment update Bjorn Helgaas
2 siblings, 0 replies; 7+ messages in thread
From: Pratyush Anand @ 2016-07-04 16:14 UTC (permalink / raw)
To: dongbo4, bhelgaas, jingoohan1; +Cc: linux-pci, Pratyush Anand, open list
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
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH 0/2] pcie/designware: Viewport assignment update
2016-07-04 16:14 [PATCH 0/2] pcie/designware: Viewport assignment update Pratyush Anand
2016-07-04 16:14 ` Pratyush Anand
2016-07-04 16:14 ` [PATCH 2/2] pcie/designware: Exchange viewport of `MEMORYs' and `CFGs/IOs' Pratyush Anand
@ 2016-08-22 18:49 ` Bjorn Helgaas
2 siblings, 0 replies; 7+ messages in thread
From: Bjorn Helgaas @ 2016-08-22 18:49 UTC (permalink / raw)
To: Pratyush Anand; +Cc: dongbo4, bhelgaas, jingoohan1, linux-pci
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!
^ permalink raw reply [flat|nested] 7+ messages in thread