| Message ID | 20231011071423.249458-9-yoshihiro.shimoda.uh@renesas.com |
|---|---|
| State | New |
| Headers | show |
| Series | PCI: dwc: rcar-gen4: Add R-Car Gen4 PCIe support | expand |
[+cc Siddharth, Ravi, Sriramakrishnan] On Wed, Oct 11, 2023 at 04:14:15PM +0900, Yoshihiro Shimoda wrote: > According to the section 3.5.7.2 "RC Mode" in DWC PCIe Dual Mode > Rev.5.20a, we should disable two BARs to avoid unnecessary memory > assignment during device enumeration. Otherwise, Renesas R-Car Gen4 > PCIe controllers cannot work correctly in host mode. > > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> > --- > drivers/pci/controller/dwc/pcie-designware-host.c | 8 ++++++++ > 1 file changed, 8 insertions(+) > > diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c > index a7170fd0e847..56cc7ff6d508 100644 > --- a/drivers/pci/controller/dwc/pcie-designware-host.c > +++ b/drivers/pci/controller/dwc/pcie-designware-host.c > @@ -737,6 +737,14 @@ int dw_pcie_setup_rc(struct dw_pcie_rp *pp) > u32 val, ctrl, num_ctrls; > int ret; > > + /* > + * According to the section 3.5.7.2 "RC Mode" in DWC PCIe Dual Mode > + * Rev.5.20a, we should disable two BARs to avoid unnecessary memory > + * assignment during device enumeration. > + */ > + dw_pcie_writel_dbi2(pci, PCI_BASE_ADDRESS_0, 0x0); > + dw_pcie_writel_dbi2(pci, PCI_BASE_ADDRESS_1, 0x0); I cc'd Siddharth and others because they are working on a Keystone issue with MSI-X that requires BAR0; see https://lore.kernel.org/r/20231011123451.34827-1-s-vadapalli@ti.com I assume any DWC controller that uses MSI-X would require BAR0 or BAR1 for the MSI-X Table. I don't have any of the DWC specs and don't know whether any controllers use MSI-X, so just heads up in case they do. This patch was recently merged and will appear in v6.7. Bjorn
Hello Bjorn, On 17/10/23 03:18, Bjorn Helgaas wrote: > [+cc Siddharth, Ravi, Sriramakrishnan] > > On Wed, Oct 11, 2023 at 04:14:15PM +0900, Yoshihiro Shimoda wrote: >> According to the section 3.5.7.2 "RC Mode" in DWC PCIe Dual Mode >> Rev.5.20a, we should disable two BARs to avoid unnecessary memory >> assignment during device enumeration. Otherwise, Renesas R-Car Gen4 >> PCIe controllers cannot work correctly in host mode. >> >> Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> >> --- >> drivers/pci/controller/dwc/pcie-designware-host.c | 8 ++++++++ >> 1 file changed, 8 insertions(+) >> >> diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c >> index a7170fd0e847..56cc7ff6d508 100644 >> --- a/drivers/pci/controller/dwc/pcie-designware-host.c >> +++ b/drivers/pci/controller/dwc/pcie-designware-host.c >> @@ -737,6 +737,14 @@ int dw_pcie_setup_rc(struct dw_pcie_rp *pp) >> u32 val, ctrl, num_ctrls; >> int ret; >> >> + /* >> + * According to the section 3.5.7.2 "RC Mode" in DWC PCIe Dual Mode >> + * Rev.5.20a, we should disable two BARs to avoid unnecessary memory >> + * assignment during device enumeration. >> + */ >> + dw_pcie_writel_dbi2(pci, PCI_BASE_ADDRESS_0, 0x0); >> + dw_pcie_writel_dbi2(pci, PCI_BASE_ADDRESS_1, 0x0); > > I cc'd Siddharth and others because they are working on a Keystone > issue with MSI-X that requires BAR0; see > https://lore.kernel.org/r/20231011123451.34827-1-s-vadapalli@ti.com > > I assume any DWC controller that uses MSI-X would require BAR0 or BAR1 > for the MSI-X Table. Disabling BAR0 in this section will not affect the pci-keystone.c driver. The MSI-X setup in the pci-keystone.c driver is done via the ks_pcie_v3_65_add_bus() function which is invoked in the pci_host_probe(bridge) function call within dw_pcie_host_init(). Since pci_host_probe(bridge) is invoked *after* dw_pcie_setup_rc(), the MSI-X setup which involves enabling BAR0 will be performed after BAR0 is disabled in this patch, due to which BAR0 will remain enabled as far as MSI-X setup is concerned. Also, the DW PCIe IP version corresponding to the Keystone PCIe host controller is 4.90a. Even in the 4.90a Databook, in section 3.5.7.2 RC Mode, it is suggested that the BARs should be disabled. > > I don't have any of the DWC specs and don't know whether any > controllers use MSI-X, so just heads up in case they do. This patch > was recently merged and will appear in v6.7. > > Bjorn
Dear All, On 11.10.2023 09:14, Yoshihiro Shimoda wrote: > According to the section 3.5.7.2 "RC Mode" in DWC PCIe Dual Mode > Rev.5.20a, we should disable two BARs to avoid unnecessary memory > assignment during device enumeration. Otherwise, Renesas R-Car Gen4 > PCIe controllers cannot work correctly in host mode. > > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> This patch landed in today's linux-next 20231017 as commit e308528cac3e ("PCI: dwc: Disable two BARs to avoid unnecessary memory assignment"). Unfortunately it causes the following kernel panic on Samsung Exynos5433-based TM2e board: exynos-pcie 15700000.pcie: host bridge /soc@0/pcie@15700000 ranges: exynos-pcie 15700000.pcie: IO 0x000c001000..0x000c010fff -> 0x0000000000 exynos-pcie 15700000.pcie: MEM 0x000c011000..0x000ffffffe -> 0x000c011000 exynos-pcie 15700000.pcie: iATU: unroll F, 3 ob, 5 ib, align 4K, limit 4G Unable to handle kernel paging request at virtual address ffff800084196010 Mem abort info: ... Data abort info: ... swapper pgtable: 4k pages, 48-bit VAs, pgdp=0000000022047000 [ffff800084196010] pgd=10000000df6ff003, p4d=10000000df6ff003, pud=10000000df6fe003, pmd=1000000024ad9003, pte=0000000000000000 Internal error: Oops: 0000000096000047 [#1] PREEMPT SMP Modules linked in: CPU: 4 PID: 55 Comm: kworker/u18:0 Not tainted 6.6.0-rc1+ #14129 Hardware name: Samsung TM2E board (DT) Workqueue: events_unbound deferred_probe_work_func pstate: 60000005 (nZCv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--) pc : dw_pcie_write_dbi2+0xb8/0xc8 lr : dw_pcie_setup_rc+0x30/0x4e4 ... Call trace: dw_pcie_write_dbi2+0xb8/0xc8 dw_pcie_setup_rc+0x30/0x4e4 dw_pcie_host_init+0x238/0x608 exynos_pcie_probe+0x23c/0x340 platform_probe+0x68/0xd8 really_probe+0x148/0x2b4 __driver_probe_device+0x78/0x12c driver_probe_device+0xd8/0x160 __device_attach_driver+0xb8/0x138 bus_for_each_drv+0x84/0xe0 __device_attach+0xa8/0x1b0 device_initial_probe+0x14/0x20 bus_probe_device+0xb0/0xb4 deferred_probe_work_func+0x8c/0xc8 process_one_work+0x1ec/0x53c worker_thread+0x298/0x408 kthread+0x124/0x128 ret_from_fork+0x10/0x20 Code: d50332bf 79000023 17ffffe2 d50332bf (b9000023) ---[ end trace 0000000000000000 ]--- Kernel panic - not syncing: Oops: Fatal exception SMP: stopping secondary CPUs Kernel Offset: disabled CPU features: 0x8c00020e,3c020000,0000421b Memory Limit: none ---[ end Kernel panic - not syncing: Oops: Fatal exception ]--- I've observed similar issue on Qualcomm's RB5 platform with some additional not-yet merged patches enabling PCIe support. Reverting $subject on top of linux-next fixes this issue. Let me know if you need more information. > --- > drivers/pci/controller/dwc/pcie-designware-host.c | 8 ++++++++ > 1 file changed, 8 insertions(+) > > diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c > index a7170fd0e847..56cc7ff6d508 100644 > --- a/drivers/pci/controller/dwc/pcie-designware-host.c > +++ b/drivers/pci/controller/dwc/pcie-designware-host.c > @@ -737,6 +737,14 @@ int dw_pcie_setup_rc(struct dw_pcie_rp *pp) > u32 val, ctrl, num_ctrls; > int ret; > > + /* > + * According to the section 3.5.7.2 "RC Mode" in DWC PCIe Dual Mode > + * Rev.5.20a, we should disable two BARs to avoid unnecessary memory > + * assignment during device enumeration. > + */ > + dw_pcie_writel_dbi2(pci, PCI_BASE_ADDRESS_0, 0x0); > + dw_pcie_writel_dbi2(pci, PCI_BASE_ADDRESS_1, 0x0); > + > /* > * Enable DBI read-only registers for writing/updating configuration. > * Write permission gets disabled towards the end of this function. Best regards
On Tue, Oct 17, 2023 at 12:05:12PM +0000, Yoshihiro Shimoda wrote: > Dear Marek, > > > From: Marek Szyprowski, Sent: Tuesday, October 17, 2023 6:19 PM > > > > Dear All, > > > > On 11.10.2023 09:14, Yoshihiro Shimoda wrote: > > > According to the section 3.5.7.2 "RC Mode" in DWC PCIe Dual Mode > > > Rev.5.20a, we should disable two BARs to avoid unnecessary memory > > > assignment during device enumeration. Otherwise, Renesas R-Car Gen4 > > > PCIe controllers cannot work correctly in host mode. > > > > > > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> > > > > This patch landed in today's linux-next 20231017 as commit e308528cac3e > > ("PCI: dwc: Disable two BARs to avoid unnecessary memory assignment"). > > Unfortunately it causes the following kernel panic on Samsung > > Exynos5433-based TM2e board: > > > > exynos-pcie 15700000.pcie: host bridge /soc@0/pcie@15700000 ranges: > > exynos-pcie 15700000.pcie: IO 0x000c001000..0x000c010fff -> > > 0x0000000000 > > exynos-pcie 15700000.pcie: MEM 0x000c011000..0x000ffffffe -> > > 0x000c011000 > > exynos-pcie 15700000.pcie: iATU: unroll F, 3 ob, 5 ib, align 4K, limit 4G > > Unable to handle kernel paging request at virtual address ffff800084196010 > > Mem abort info: > > ... > > Data abort info: > > ... > > swapper pgtable: 4k pages, 48-bit VAs, pgdp=0000000022047000 > > [ffff800084196010] pgd=10000000df6ff003, p4d=10000000df6ff003, > > pud=10000000df6fe003, pmd=1000000024ad9003, pte=0000000000000000 > > Internal error: Oops: 0000000096000047 [#1] PREEMPT SMP > > Modules linked in: > > CPU: 4 PID: 55 Comm: kworker/u18:0 Not tainted 6.6.0-rc1+ #14129 > > Hardware name: Samsung TM2E board (DT) > > Workqueue: events_unbound deferred_probe_work_func > > pstate: 60000005 (nZCv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--) > > pc : dw_pcie_write_dbi2+0xb8/0xc8 > > lr : dw_pcie_setup_rc+0x30/0x4e4 > > ... > > Call trace: > > dw_pcie_write_dbi2+0xb8/0xc8 > > dw_pcie_setup_rc+0x30/0x4e4 > > dw_pcie_host_init+0x238/0x608 > > exynos_pcie_probe+0x23c/0x340 > > platform_probe+0x68/0xd8 > > really_probe+0x148/0x2b4 > > __driver_probe_device+0x78/0x12c > > driver_probe_device+0xd8/0x160 > > __device_attach_driver+0xb8/0x138 > > bus_for_each_drv+0x84/0xe0 > > __device_attach+0xa8/0x1b0 > > device_initial_probe+0x14/0x20 > > bus_probe_device+0xb0/0xb4 > > deferred_probe_work_func+0x8c/0xc8 > > process_one_work+0x1ec/0x53c > > worker_thread+0x298/0x408 > > kthread+0x124/0x128 > > ret_from_fork+0x10/0x20 > > Code: d50332bf 79000023 17ffffe2 d50332bf (b9000023) > > ---[ end trace 0000000000000000 ]--- > > Kernel panic - not syncing: Oops: Fatal exception > > SMP: stopping secondary CPUs > > Kernel Offset: disabled > > CPU features: 0x8c00020e,3c020000,0000421b > > Memory Limit: none > > ---[ end Kernel panic - not syncing: Oops: Fatal exception ]--- > > > > I've observed similar issue on Qualcomm's RB5 platform with some > > additional not-yet merged patches enabling PCIe support. Reverting > > $subject on top of linux-next fixes this issue. > > > > Let me know if you need more information. > > Thank you for the report. I guess that the issue is related to > out-of-range access of dbi2: > - In arch/arm64/boot/dts/exynos/exynos5433.dtsi, the dbi reg size is 0x1000 > like below: > ----- > pcie: pcie@15700000 { > compatible = "samsung,exynos5433-pcie"; > reg = <0x15700000 0x1000>, <0x156b0000 0x1000>, > <0x0c000000 0x1000>; > reg-names = "dbi", "elbi", "config"; > ... > ----- > > - In drivers/pci/controller/dwc/pcie-designware.c, "dbi2" area is calculated > by the following if reg-names "dbi2" didn't exist: > ----- > pci->dbi_base2 = pci->dbi_base + SZ_4K; > ----- > > - However, this is out-of-memory on exynos5433.dtsi because the "dbi" size is > 0x1000 only. > - And then, this patch always writes PCI_BASE_ADDRESS_[01] to dbi2 area. > So, since this is out-of-range, the kernel panic happens. > I could reproduce the issue Marek reported on RB5. As you pointed out, it is due to not mapping dbi2 explicitly. But we were not using DBI2 on the host earlier and it looks to me that DBI2 may not be implemented on Qcom host platforms. Atleast on EP, I confirmed with Qcom that DBI=DBI2 as represented in the driver, but I couldn't confirm if it is same for host as well. > Perhaps, we should revert this patch at first. And, add the settings into > my environment (pcie-rcar-gen4.c) only. I also have alternative solution which > modifies the "dbi2" area calculation and avoid out-of-range access somehow. > But, it may complicate source code... > Yes, let's revert this patch for now and move it to rcar driver. - Mani > Best regards, > Yoshihiro Shimoda > > > > --- > > > drivers/pci/controller/dwc/pcie-designware-host.c | 8 ++++++++ > > > 1 file changed, 8 insertions(+) > > > > > > diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c > > > index a7170fd0e847..56cc7ff6d508 100644 > > > --- a/drivers/pci/controller/dwc/pcie-designware-host.c > > > +++ b/drivers/pci/controller/dwc/pcie-designware-host.c > > > @@ -737,6 +737,14 @@ int dw_pcie_setup_rc(struct dw_pcie_rp *pp) > > > u32 val, ctrl, num_ctrls; > > > int ret; > > > > > > + /* > > > + * According to the section 3.5.7.2 "RC Mode" in DWC PCIe Dual Mode > > > + * Rev.5.20a, we should disable two BARs to avoid unnecessary memory > > > + * assignment during device enumeration. > > > + */ > > > + dw_pcie_writel_dbi2(pci, PCI_BASE_ADDRESS_0, 0x0); > > > + dw_pcie_writel_dbi2(pci, PCI_BASE_ADDRESS_1, 0x0); > > > + > > > /* > > > * Enable DBI read-only registers for writing/updating configuration. > > > * Write permission gets disabled towards the end of this function. > > > > Best regards > > -- > > Marek Szyprowski, PhD > > Samsung R&D Institute Poland >
diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c index a7170fd0e847..56cc7ff6d508 100644 --- a/drivers/pci/controller/dwc/pcie-designware-host.c +++ b/drivers/pci/controller/dwc/pcie-designware-host.c @@ -737,6 +737,14 @@ int dw_pcie_setup_rc(struct dw_pcie_rp *pp) u32 val, ctrl, num_ctrls; int ret; + /* + * According to the section 3.5.7.2 "RC Mode" in DWC PCIe Dual Mode + * Rev.5.20a, we should disable two BARs to avoid unnecessary memory + * assignment during device enumeration. + */ + dw_pcie_writel_dbi2(pci, PCI_BASE_ADDRESS_0, 0x0); + dw_pcie_writel_dbi2(pci, PCI_BASE_ADDRESS_1, 0x0); + /* * Enable DBI read-only registers for writing/updating configuration. * Write permission gets disabled towards the end of this function.
According to the section 3.5.7.2 "RC Mode" in DWC PCIe Dual Mode Rev.5.20a, we should disable two BARs to avoid unnecessary memory assignment during device enumeration. Otherwise, Renesas R-Car Gen4 PCIe controllers cannot work correctly in host mode. Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> --- drivers/pci/controller/dwc/pcie-designware-host.c | 8 ++++++++ 1 file changed, 8 insertions(+)