* [PATCH] Exchange the Assignments of `MEMORYs' and `CFGs/IOs' in Designware PCIe Driver
[not found] <1467100302-221983-1-git-send-email-zhaonenglong@hisilicon.com>
@ 2016-06-28 8:12 ` dongbo (E)
2016-07-03 15:27 ` Pratyush Anand
0 siblings, 1 reply; 4+ messages in thread
From: dongbo (E) @ 2016-06-28 8:12 UTC (permalink / raw)
To: Pratyush Anand, jingoohan1, bhelgaas
Cc: Linuxarm, linux-kernel, linux-pci, Zhanweitao
From: Dong Bo <dongbo4@huawei.com>
In designware PCIe driver, the iatu0 is used for both CFG and IO accesses.
When sending CFGs to peripherals (e.g. lspci), iatu0 frequently switches
between CFG and IO alternatively.
A MEMORY probably 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.
Signed-off-by: Dong Bo <dongbo4@huawei.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 aafd766..1bd88d9 100644
--- a/drivers/pci/host/pcie-designware.c
+++ b/drivers/pci/host/pcie-designware.c
@@ -599,11 +599,11 @@ 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);
- 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);
@@ -636,11 +636,11 @@ 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);
- 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);
@@ -779,7 +779,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);
-- 1.9.1
.
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] Exchange the Assignments of `MEMORYs' and `CFGs/IOs' in Designware PCIe Driver
2016-06-28 8:12 ` [PATCH] Exchange the Assignments of `MEMORYs' and `CFGs/IOs' in Designware PCIe Driver dongbo (E)
@ 2016-07-03 15:27 ` Pratyush Anand
2016-07-04 7:36 ` dongbo (E)
0 siblings, 1 reply; 4+ messages in thread
From: Pratyush Anand @ 2016-07-03 15:27 UTC (permalink / raw)
To: dongbo (E)
Cc: jingoohan1, bhelgaas, Linuxarm, linux-kernel, linux-pci, Zhanweitao
On Tue, Jun 28, 2016 at 1:42 PM, dongbo (E) <dongbo4@huawei.com> wrote:
>
> From: Dong Bo <dongbo4@huawei.com>
>
> In designware PCIe driver, the iatu0 is used for both CFG and IO accesses.
> When sending CFGs to peripherals (e.g. lspci), iatu0 frequently switches
> between CFG and IO alternatively.
>
> A MEMORY probably 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.
Had a re-thought on it. While it will fix wrong memory access in your
case, it can still cause issues with IO access for some other
platform.
Can you please test [1] and check it that works for you. You will need
to define num-viewport in your device tree file.
~Pratyush
[1] https://github.com/pratyushanand/linux/commit/131b83ea7db0834d77ee5df65c6696bccbf8a1ce
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] Exchange the Assignments of `MEMORYs' and `CFGs/IOs' in Designware PCIe Driver
2016-07-03 15:27 ` Pratyush Anand
@ 2016-07-04 7:36 ` dongbo (E)
2016-07-04 9:00 ` Pratyush Anand
0 siblings, 1 reply; 4+ messages in thread
From: dongbo (E) @ 2016-07-04 7:36 UTC (permalink / raw)
To: Pratyush Anand
Cc: jingoohan1, bhelgaas, Linuxarm, linux-kernel, linux-pci, Zhanweitao
On 2016/7/3 23:27, Pratyush Anand wrote:
> On Tue, Jun 28, 2016 at 1:42 PM, dongbo (E) <dongbo4@huawei.com> wrote:
>>
>> From: Dong Bo <dongbo4@huawei.com>
>>
>> In designware PCIe driver, the iatu0 is used for both CFG and IO accesses.
>> When sending CFGs to peripherals (e.g. lspci), iatu0 frequently switches
>> between CFG and IO alternatively.
>>
>> A MEMORY probably 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.
>
>
> Had a re-thought on it. While it will fix wrong memory access in your
> case, it can still cause issues with IO access for some other
> platform.
>
> Can you please test [1] and check it that works for you. You will need
> to define num-viewport in your device tree file.
>
> ~Pratyush
>
> [1] https://github.com/pratyushanand/linux/commit/131b83ea7db0834d77ee5df65c6696bccbf8a1ce
>
> .
>
Checked, it works for us.
I think it would be better to exchange the assignments of MEMORYs and
CFGs/IOs when num_viewports <= 2, cause it fixes wrong memory access.
As you mentioned, other corner point for failure is still there while
there are only two viewports. It seems that there is not a perfect
solution.
Best Regards
Dong Bo
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] Exchange the Assignments of `MEMORYs' and `CFGs/IOs' in Designware PCIe Driver
2016-07-04 7:36 ` dongbo (E)
@ 2016-07-04 9:00 ` Pratyush Anand
0 siblings, 0 replies; 4+ messages in thread
From: Pratyush Anand @ 2016-07-04 9:00 UTC (permalink / raw)
To: dongbo (E)
Cc: jingoohan1, bhelgaas, Linuxarm, linux-kernel, linux-pci, Zhanweitao
On Mon, Jul 4, 2016 at 1:06 PM, dongbo (E) <dongbo4@huawei.com> wrote:
> On 2016/7/3 23:27, Pratyush Anand wrote:
>> On Tue, Jun 28, 2016 at 1:42 PM, dongbo (E) <dongbo4@huawei.com> wrote:
>>>
>>> From: Dong Bo <dongbo4@huawei.com>
>>>
>>> In designware PCIe driver, the iatu0 is used for both CFG and IO accesses.
>>> When sending CFGs to peripherals (e.g. lspci), iatu0 frequently switches
>>> between CFG and IO alternatively.
>>>
>>> A MEMORY probably 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.
>>
>>
>> Had a re-thought on it. While it will fix wrong memory access in your
>> case, it can still cause issues with IO access for some other
>> platform.
>>
>> Can you please test [1] and check it that works for you. You will need
>> to define num-viewport in your device tree file.
>>
>> ~Pratyush
>>
>> [1] https://github.com/pratyushanand/linux/commit/131b83ea7db0834d77ee5df65c6696bccbf8a1ce
>>
>> .
>>
>
> Checked, it works for us.
Thanks for testing.
>
> I think it would be better to exchange the assignments of MEMORYs and
> CFGs/IOs when num_viewports <= 2, cause it fixes wrong memory access.
OK.. I think that can be accommodated. I have rebased your patch on
top of mine with some change in commit log.
https://github.com/pratyushanand/linux/commit/6d3805a5e0fbbbd73beba80c2c9151b26399ea67
Will send both of the patches to the list for review.
>
> As you mentioned, other corner point for failure is still there while
> there are only two viewports. It seems that there is not a perfect
> solution.
Yes, unfortunately we will have to live with either remote possibility
of less frequent IO transfer corruption or we can disable IO transfer
for <=2 viewports. IMHO, former is the better way to go with.
~Pratyush
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2016-07-04 9:00 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <1467100302-221983-1-git-send-email-zhaonenglong@hisilicon.com>
2016-06-28 8:12 ` [PATCH] Exchange the Assignments of `MEMORYs' and `CFGs/IOs' in Designware PCIe Driver dongbo (E)
2016-07-03 15:27 ` Pratyush Anand
2016-07-04 7:36 ` dongbo (E)
2016-07-04 9:00 ` Pratyush Anand
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.