pcie/designware: Viewport assignment update
cover pcie/designware: Viewport assignment update
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(-)
Signature · 1 line
2.5.5
PATCH 1/2 pcie/designware: Keep viewport fixed for IO transaction if num_viewport > 2
From: Pratyush Anand <[email protected]>
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.
--- .../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(-)
Documentation/devicetree/bindings/pci/designware-pcie.txt +3 -0
@@ -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>; };
drivers/pci/host/pcie-designware.c +19 -7
@@ -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 );
drivers/pci/host/pcie-designware.h +1 -0
@@ -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;
Signature · 1 line
2.5.5
Re: [PATCH 1/2] pcie/designware: Keep viewport fixed for IO transaction if num_viewport > 2
Quoted 6 lines · Pratyush Anand
From: Pratyush Anand <[email protected]>
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.
Quoted 7 lines
Signed-off-by Pratyush Anand <[email protected]>Tested-by Dong Bo <[email protected]>--- .../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(-)
PATCH 2/2 pcie/designware: Exchange viewport of `MEMORYs' and `CFGs/IOs'
In reply to Pratyush AnandFrom: Dong Bo <[email protected]>
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.
--- drivers/pci/host/pcie-designware.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)
drivers/pci/host/pcie-designware.c +5 -5
@@ -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 )
Signature · 1 line
2.5.5
Quoted 11 lines · 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 ofMEMORYs' andCFGs/IOs'
Quoted 8 lines
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!