* Re: [BUG] rockpro64: PCI BAR reassignment broken by commit 9d57e61bf723 ("of/pci: Add IORESOURCE_MEM_64 to resource flags for 64-bit memory addresses")
2021-05-25 13:54 ` Ard Biesheuvel
@ 2021-05-25 15:34 ` Peter Geis
2021-05-25 15:54 ` Ard Biesheuvel
2021-05-25 16:59 ` Anand Moon
` (2 subsequent siblings)
3 siblings, 1 reply; 28+ messages in thread
From: Peter Geis @ 2021-05-25 15:34 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: Punit Agrawal, Robin Murphy, Alexandru Elisei,
Linux Kernel Mailing List, open list:ARM/Rockchip SoC...,
arm-mail-list, Heiko Stuebner, Leonardo Bras, Rob Herring, PCI,
Christian König
On Tue, May 25, 2021 at 9:57 AM Ard Biesheuvel <ardb@kernel.org> wrote:
>
> On Tue, 25 May 2021 at 15:42, Punit Agrawal <punitagrawal@gmail.com> wrote:
> >
> > Hi Ard,
> >
> > Ard Biesheuvel <ardb@kernel.org> writes:
> >
> > > On Sun, 23 May 2021 at 13:06, Punit Agrawal <punitagrawal@gmail.com> wrote:
> > >>
> > >> Robin Murphy <robin.murphy@arm.com> writes:
> > >>
> > >> > [ +linux-pci for visibility ]
> > >> >
> > >> > On 2021-05-18 10:09, Alexandru Elisei wrote:
> > >> >> After doing a git bisect I was able to trace the following error when booting my
> > >> >> rockpro64 v2 (rk3399 SoC) with a PCIE NVME expansion card:
> > >> >> [..]
> > >> >> [ 0.305183] rockchip-pcie f8000000.pcie: host bridge /pcie@f8000000 ranges:
> > >> >> [ 0.305248] rockchip-pcie f8000000.pcie: MEM 0x00fa000000..0x00fbdfffff ->
> > >> >> 0x00fa000000
> > >> >> [ 0.305285] rockchip-pcie f8000000.pcie: IO 0x00fbe00000..0x00fbefffff ->
> > >> >> 0x00fbe00000
> > >> >> [ 0.306201] rockchip-pcie f8000000.pcie: supply vpcie1v8 not found, using dummy
> > >> >> regulator
> > >> >> [ 0.306334] rockchip-pcie f8000000.pcie: supply vpcie0v9 not found, using dummy
> > >> >> regulator
> > >> >> [ 0.373705] rockchip-pcie f8000000.pcie: PCI host bridge to bus 0000:00
> > >> >> [ 0.373730] pci_bus 0000:00: root bus resource [bus 00-1f]
> > >> >> [ 0.373751] pci_bus 0000:00: root bus resource [mem 0xfa000000-0xfbdfffff 64bit]
> > >> >> [ 0.373777] pci_bus 0000:00: root bus resource [io 0x0000-0xfffff] (bus
> > >> >> address [0xfbe00000-0xfbefffff])
> > >> >> [ 0.373839] pci 0000:00:00.0: [1d87:0100] type 01 class 0x060400
> > >> >> [ 0.373973] pci 0000:00:00.0: supports D1
> > >> >> [ 0.373992] pci 0000:00:00.0: PME# supported from D0 D1 D3hot
> > >> >> [ 0.378518] pci 0000:00:00.0: bridge configuration invalid ([bus 00-00]),
> > >> >> reconfiguring
> > >> >> [ 0.378765] pci 0000:01:00.0: [144d:a808] type 00 class 0x010802
> > >> >> [ 0.378869] pci 0000:01:00.0: reg 0x10: [mem 0x00000000-0x00003fff 64bit]
> > >> >> [ 0.379051] pci 0000:01:00.0: Max Payload Size set to 256 (was 128, max 256)
> > >> >> [ 0.379661] pci 0000:01:00.0: 8.000 Gb/s available PCIe bandwidth, limited by
> > >> >> 2.5 GT/s PCIe x4 link at 0000:00:00.0 (capable of 31.504 Gb/s with 8.0 GT/s PCIe
> > >> >> x4 link)
> > >> >> [ 0.393269] pci_bus 0000:01: busn_res: [bus 01-1f] end is updated to 01
> > >> >> [ 0.393311] pci 0000:00:00.0: BAR 14: no space for [mem size 0x00100000]
> > >> >> [ 0.393333] pci 0000:00:00.0: BAR 14: failed to assign [mem size 0x00100000]
> > >> >> [ 0.393356] pci 0000:01:00.0: BAR 0: no space for [mem size 0x00004000 64bit]
> > >> >> [ 0.393375] pci 0000:01:00.0: BAR 0: failed to assign [mem size 0x00004000 64bit]
> > >> >> [ 0.393397] pci 0000:00:00.0: PCI bridge to [bus 01]
> > >> >> [ 0.393839] pcieport 0000:00:00.0: PME: Signaling with IRQ 78
> > >> >> [ 0.394165] pcieport 0000:00:00.0: AER: enabled with IRQ 78
> > >> >> [..]
> > >> >> to the commit 9d57e61bf723 ("of/pci: Add IORESOURCE_MEM_64 to
> > >> >> resource flags for
> > >> >> 64-bit memory addresses").
> > >> >
> > >> > FWFW, my hunch is that the host bridge advertising no 32-bit memory
> > >> > resource, only only a single 64-bit non-prefetchable one (even though
> > >> > it's entirely below 4GB) might be a bit weird and tripping something
> > >> > up in the resource assignment code. It certainly seems like the thing
> > >> > most directly related to the offending commit.
> > >> >
> > >> > I'd be tempted to try fiddling with that in the DT (i.e. changing
> > >> > 0x83000000 to 0x82000000 in the PCIe node's "ranges" property) to see
> > >> > if it makes any difference. Note that even if it helps, though, I
> > >> > don't know whether that's the correct fix or just a bodge around a
> > >> > corner-case bug somewhere in the resource code.
> > >>
> > >> From digging into this further the failure seems to be due to a mismatch
> > >> of flags when allocating resources in pci_bus_alloc_from_region() -
> > >>
> > >> if ((res->flags ^ r->flags) & type_mask)
> > >> continue;
> > >>
> > >> Though I am also not sure why the failure is only being reported on
> > >> RK3399 - does a single 64-bit window have anything to do with it?
> > >>
> > >
> > > The NVMe in the example exposes a single 64-bit non-prefetchable BAR.
> > > Such BARs can not be allocated in a prefetchable host bridge window
> > > (unlike the converse, i.e., allocating a prefetchable BAR in a
> > > non-prefetchable host bridge window is fine)
> > >
> > > 64-bit non-prefetchable host bridge windows cannot be forwarded by PCI
> > > to PCI bridges, they simply lack the BAR registers to describe them.
> > > Therefore, non-prefetchable endpoint BARs (even 64-bit ones) need to
> > > be carved out of a host bridge's non-prefetchable 32-bit window if
> > > they need to pass through a bridge.
> >
> > Thank you for the explanation. I also looked at the PCI-to-PCI Bridge
> > spec to understand where some of the limitations are coming from.
> >
> > > So the error seems to be here that the host bridge's 32-bit
> > > non-prefetchable window has the 64-bit attribute set, even though it
> > > resides below 4 GB entirely. I suppose that the resource allocation
> > > could be made more forgiving (and it was in the past, before commit
> > > 9d57e61bf723 was applied). However, I would strongly recommend not
> > > deviating from common practice, and just describe the 32-bit
> > > addressable non-prefetchable resource window as such.
> >
> > IIUC, the host bridge's configuration (64-bit on non-prefetchable
> > window) is based on what the hardware advertises.
> >
>
> What do you mean by 'what the hardware advertises'? The host bridge is
> apparently configured to decode a 32-bit addressable window as MMIO,
> and the question is why this window has the 64-bit attribute set in
> the DT description.
>
> > Can you elaborate on what you have in mind to correct the
> > non-prefetchable resource window? Are you thinking of adding a quirk
> > somewhere to address this?
> >
>
> No. Just fix the DT.
Good Morning,
I believe Robin is correct that there is more to this.
While attempting to work out why dGPUs won't work with the rk356x
series PCIe controllers, Christian König from the amd-gpu driver
mailing list noticed the gpu was incorrectly allocated a 64bit
non-prefetchable BAR which should instead be a 32 non-prefetchable
BAR.
The ranges currently set are:
ranges = <0x81000000 0x0 0x00800000 0x3 0x00800000 0x0 0x00100000
0x82000000 0x0 0x00900000 0x3 0x00900000 0x0 0x3f700000>;
but the final allocation was:
lspci -v
00:00.0 PCI bridge: Fuzhou Rockchip Electronics Co., Ltd Device 3566
(rev 01) (prog-if 00 [Normal decode])
Flags: bus master, fast devsel, latency 0, IRQ 96
Bus: primary=00, secondary=01, subordinate=ff, sec-latency=0
I/O behind bridge: 00001000-00001fff [size=4K]
Memory behind bridge: 00900000-009fffff [size=1M]
Prefetchable memory behind bridge:
0000000010000000-000000001fffffff [size=256M]
Expansion ROM at 300a00000 [virtual] [disabled] [size=64K]
Capabilities: [40] Power Management version 3
Capabilities: [50] MSI: Enable+ Count=1/32 Maskable- 64bit+
Capabilities: [70] Express Root Port (Slot-), MSI 00
Capabilities: [b0] MSI-X: Enable- Count=1 Masked-
Capabilities: [100] Advanced Error Reporting
Capabilities: [148] Secondary PCI Express
Capabilities: [160] L1 PM Substates
Capabilities: [170] Vendor Specific Information: ID=0002 Rev=4
Len=100 <?>
Kernel driver in use: pcieport
01:00.0 VGA compatible controller: Advanced Micro Devices, Inc.
[AMD/ATI] Turks PRO [Radeon HD 7570] (prog-if 00 [VGA controller])
Subsystem: Dell Turks PRO [Radeon HD 7570]
Flags: bus master, fast devsel, latency 0, IRQ 95
Memory at 310000000 (64-bit, prefetchable) [size=256M]
Memory at 300900000 (64-bit, non-prefetchable) [size=128K]
I/O ports at 1000 [size=256]
Expansion ROM at 300920000 [disabled] [size=128K]
Capabilities: [50] Power Management version 3
Capabilities: [58] Express Legacy Endpoint, MSI 00
Capabilities: [a0] MSI: Enable- Count=1/1 Maskable- 64bit+
Capabilities: [100] Vendor Specific Information: ID=0001 Rev=1
Len=010 <?>
Capabilities: [150] Advanced Error Reporting
Kernel driver in use: radeon
01:00.1 Audio device: Advanced Micro Devices, Inc. [AMD/ATI] Turks
HDMI Audio [Radeon HD 6500/6600 / 6700M Series]
Subsystem: Dell Turks HDMI Audio [Radeon HD 6500/6600 / 6700M Series]
Flags: bus master, fast devsel, latency 0, IRQ 98
Memory at 300940000 (64-bit, non-prefetchable) [size=16K]
Capabilities: [50] Power Management version 3
Capabilities: [58] Express Legacy Endpoint, MSI 00
Capabilities: [a0] MSI: Enable+ Count=1/1 Maskable- 64bit+
Capabilities: [100] Vendor Specific Information: ID=0001 Rev=1
Len=010 <?>
Capabilities: [150] Advanced Error Reporting
Kernel driver in use: snd_hda_intel
This will obviously clobber registers during writes.
Also, if <0x82000000> (32 bit) is changed to <0x83000000> (64 bit),
most of the allocations for the dGPU fail due to no valid regions
available.
>
> > I am happy to put something together once I understand the preferred way
> > to go about it.
> >
> > Thanks,
> > Punit
> >
> > [...]
> >
>
> _______________________________________________
> Linux-rockchip mailing list
> Linux-rockchip@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [BUG] rockpro64: PCI BAR reassignment broken by commit 9d57e61bf723 ("of/pci: Add IORESOURCE_MEM_64 to resource flags for 64-bit memory addresses")
2021-05-25 15:34 ` Peter Geis
@ 2021-05-25 15:54 ` Ard Biesheuvel
2021-05-25 16:23 ` Peter Geis
2021-05-25 19:15 ` Bjorn Helgaas
0 siblings, 2 replies; 28+ messages in thread
From: Ard Biesheuvel @ 2021-05-25 15:54 UTC (permalink / raw)
To: Peter Geis
Cc: Punit Agrawal, Robin Murphy, Alexandru Elisei,
Linux Kernel Mailing List, open list:ARM/Rockchip SoC...,
arm-mail-list, Heiko Stuebner, Leonardo Bras, Rob Herring, PCI,
Christian König
On Tue, 25 May 2021 at 17:34, Peter Geis <pgwipeout@gmail.com> wrote:
>
> On Tue, May 25, 2021 at 9:57 AM Ard Biesheuvel <ardb@kernel.org> wrote:
> >
> > On Tue, 25 May 2021 at 15:42, Punit Agrawal <punitagrawal@gmail.com> wrote:
> > >
> > > Hi Ard,
> > >
> > > Ard Biesheuvel <ardb@kernel.org> writes:
> > >
> > > > On Sun, 23 May 2021 at 13:06, Punit Agrawal <punitagrawal@gmail.com> wrote:
> > > >>
> > > >> Robin Murphy <robin.murphy@arm.com> writes:
> > > >>
> > > >> > [ +linux-pci for visibility ]
> > > >> >
> > > >> > On 2021-05-18 10:09, Alexandru Elisei wrote:
> > > >> >> After doing a git bisect I was able to trace the following error when booting my
> > > >> >> rockpro64 v2 (rk3399 SoC) with a PCIE NVME expansion card:
> > > >> >> [..]
> > > >> >> [ 0.305183] rockchip-pcie f8000000.pcie: host bridge /pcie@f8000000 ranges:
> > > >> >> [ 0.305248] rockchip-pcie f8000000.pcie: MEM 0x00fa000000..0x00fbdfffff ->
> > > >> >> 0x00fa000000
> > > >> >> [ 0.305285] rockchip-pcie f8000000.pcie: IO 0x00fbe00000..0x00fbefffff ->
> > > >> >> 0x00fbe00000
> > > >> >> [ 0.306201] rockchip-pcie f8000000.pcie: supply vpcie1v8 not found, using dummy
> > > >> >> regulator
> > > >> >> [ 0.306334] rockchip-pcie f8000000.pcie: supply vpcie0v9 not found, using dummy
> > > >> >> regulator
> > > >> >> [ 0.373705] rockchip-pcie f8000000.pcie: PCI host bridge to bus 0000:00
> > > >> >> [ 0.373730] pci_bus 0000:00: root bus resource [bus 00-1f]
> > > >> >> [ 0.373751] pci_bus 0000:00: root bus resource [mem 0xfa000000-0xfbdfffff 64bit]
> > > >> >> [ 0.373777] pci_bus 0000:00: root bus resource [io 0x0000-0xfffff] (bus
> > > >> >> address [0xfbe00000-0xfbefffff])
> > > >> >> [ 0.373839] pci 0000:00:00.0: [1d87:0100] type 01 class 0x060400
> > > >> >> [ 0.373973] pci 0000:00:00.0: supports D1
> > > >> >> [ 0.373992] pci 0000:00:00.0: PME# supported from D0 D1 D3hot
> > > >> >> [ 0.378518] pci 0000:00:00.0: bridge configuration invalid ([bus 00-00]),
> > > >> >> reconfiguring
> > > >> >> [ 0.378765] pci 0000:01:00.0: [144d:a808] type 00 class 0x010802
> > > >> >> [ 0.378869] pci 0000:01:00.0: reg 0x10: [mem 0x00000000-0x00003fff 64bit]
> > > >> >> [ 0.379051] pci 0000:01:00.0: Max Payload Size set to 256 (was 128, max 256)
> > > >> >> [ 0.379661] pci 0000:01:00.0: 8.000 Gb/s available PCIe bandwidth, limited by
> > > >> >> 2.5 GT/s PCIe x4 link at 0000:00:00.0 (capable of 31.504 Gb/s with 8.0 GT/s PCIe
> > > >> >> x4 link)
> > > >> >> [ 0.393269] pci_bus 0000:01: busn_res: [bus 01-1f] end is updated to 01
> > > >> >> [ 0.393311] pci 0000:00:00.0: BAR 14: no space for [mem size 0x00100000]
> > > >> >> [ 0.393333] pci 0000:00:00.0: BAR 14: failed to assign [mem size 0x00100000]
> > > >> >> [ 0.393356] pci 0000:01:00.0: BAR 0: no space for [mem size 0x00004000 64bit]
> > > >> >> [ 0.393375] pci 0000:01:00.0: BAR 0: failed to assign [mem size 0x00004000 64bit]
> > > >> >> [ 0.393397] pci 0000:00:00.0: PCI bridge to [bus 01]
> > > >> >> [ 0.393839] pcieport 0000:00:00.0: PME: Signaling with IRQ 78
> > > >> >> [ 0.394165] pcieport 0000:00:00.0: AER: enabled with IRQ 78
> > > >> >> [..]
> > > >> >> to the commit 9d57e61bf723 ("of/pci: Add IORESOURCE_MEM_64 to
> > > >> >> resource flags for
> > > >> >> 64-bit memory addresses").
> > > >> >
> > > >> > FWFW, my hunch is that the host bridge advertising no 32-bit memory
> > > >> > resource, only only a single 64-bit non-prefetchable one (even though
> > > >> > it's entirely below 4GB) might be a bit weird and tripping something
> > > >> > up in the resource assignment code. It certainly seems like the thing
> > > >> > most directly related to the offending commit.
> > > >> >
> > > >> > I'd be tempted to try fiddling with that in the DT (i.e. changing
> > > >> > 0x83000000 to 0x82000000 in the PCIe node's "ranges" property) to see
> > > >> > if it makes any difference. Note that even if it helps, though, I
> > > >> > don't know whether that's the correct fix or just a bodge around a
> > > >> > corner-case bug somewhere in the resource code.
> > > >>
> > > >> From digging into this further the failure seems to be due to a mismatch
> > > >> of flags when allocating resources in pci_bus_alloc_from_region() -
> > > >>
> > > >> if ((res->flags ^ r->flags) & type_mask)
> > > >> continue;
> > > >>
> > > >> Though I am also not sure why the failure is only being reported on
> > > >> RK3399 - does a single 64-bit window have anything to do with it?
> > > >>
> > > >
> > > > The NVMe in the example exposes a single 64-bit non-prefetchable BAR.
> > > > Such BARs can not be allocated in a prefetchable host bridge window
> > > > (unlike the converse, i.e., allocating a prefetchable BAR in a
> > > > non-prefetchable host bridge window is fine)
> > > >
> > > > 64-bit non-prefetchable host bridge windows cannot be forwarded by PCI
> > > > to PCI bridges, they simply lack the BAR registers to describe them.
> > > > Therefore, non-prefetchable endpoint BARs (even 64-bit ones) need to
> > > > be carved out of a host bridge's non-prefetchable 32-bit window if
> > > > they need to pass through a bridge.
> > >
> > > Thank you for the explanation. I also looked at the PCI-to-PCI Bridge
> > > spec to understand where some of the limitations are coming from.
> > >
> > > > So the error seems to be here that the host bridge's 32-bit
> > > > non-prefetchable window has the 64-bit attribute set, even though it
> > > > resides below 4 GB entirely. I suppose that the resource allocation
> > > > could be made more forgiving (and it was in the past, before commit
> > > > 9d57e61bf723 was applied). However, I would strongly recommend not
> > > > deviating from common practice, and just describe the 32-bit
> > > > addressable non-prefetchable resource window as such.
> > >
> > > IIUC, the host bridge's configuration (64-bit on non-prefetchable
> > > window) is based on what the hardware advertises.
> > >
> >
> > What do you mean by 'what the hardware advertises'? The host bridge is
> > apparently configured to decode a 32-bit addressable window as MMIO,
> > and the question is why this window has the 64-bit attribute set in
> > the DT description.
> >
> > > Can you elaborate on what you have in mind to correct the
> > > non-prefetchable resource window? Are you thinking of adding a quirk
> > > somewhere to address this?
> > >
> >
> > No. Just fix the DT.
>
> Good Morning,
>
> I believe Robin is correct that there is more to this.
> While attempting to work out why dGPUs won't work with the rk356x
> series PCIe controllers, Christian König from the amd-gpu driver
> mailing list noticed the gpu was incorrectly allocated a 64bit
> non-prefetchable BAR which should instead be a 32 non-prefetchable
> BAR.
>
This is due to the translation. For some reason, lspci translates the
BAR values to CPU addresses, but the PCI side addresses are within
32-bits.
Are you sure the amdgpu driver can even deal with non-1:1 host bridges?
> The ranges currently set are:
> ranges = <0x81000000 0x0 0x00800000 0x3 0x00800000 0x0 0x00100000
> 0x82000000 0x0 0x00900000 0x3 0x00900000 0x0 0x3f700000>;
>
So you have two ranges here.
> but the final allocation was:
>
> lspci -v
> 00:00.0 PCI bridge: Fuzhou Rockchip Electronics Co., Ltd Device 3566
> (rev 01) (prog-if 00 [Normal decode])
> Flags: bus master, fast devsel, latency 0, IRQ 96
> Bus: primary=00, secondary=01, subordinate=ff, sec-latency=0
> I/O behind bridge: 00001000-00001fff [size=4K]
> Memory behind bridge: 00900000-009fffff [size=1M]
> Prefetchable memory behind bridge:
> 0000000010000000-000000001fffffff [size=256M]
But the host bridge/root port decodes two disjoint regions??
> Expansion ROM at 300a00000 [virtual] [disabled] [size=64K]
> Capabilities: [40] Power Management version 3
> Capabilities: [50] MSI: Enable+ Count=1/32 Maskable- 64bit+
> Capabilities: [70] Express Root Port (Slot-), MSI 00
> Capabilities: [b0] MSI-X: Enable- Count=1 Masked-
> Capabilities: [100] Advanced Error Reporting
> Capabilities: [148] Secondary PCI Express
> Capabilities: [160] L1 PM Substates
> Capabilities: [170] Vendor Specific Information: ID=0002 Rev=4
> Len=100 <?>
> Kernel driver in use: pcieport
>
> 01:00.0 VGA compatible controller: Advanced Micro Devices, Inc.
> [AMD/ATI] Turks PRO [Radeon HD 7570] (prog-if 00 [VGA controller])
> Subsystem: Dell Turks PRO [Radeon HD 7570]
> Flags: bus master, fast devsel, latency 0, IRQ 95
> Memory at 310000000 (64-bit, prefetchable) [size=256M]
> Memory at 300900000 (64-bit, non-prefetchable) [size=128K]
> I/O ports at 1000 [size=256]
> Expansion ROM at 300920000 [disabled] [size=128K]
> Capabilities: [50] Power Management version 3
> Capabilities: [58] Express Legacy Endpoint, MSI 00
> Capabilities: [a0] MSI: Enable- Count=1/1 Maskable- 64bit+
> Capabilities: [100] Vendor Specific Information: ID=0001 Rev=1
> Len=010 <?>
> Capabilities: [150] Advanced Error Reporting
> Kernel driver in use: radeon
>
> 01:00.1 Audio device: Advanced Micro Devices, Inc. [AMD/ATI] Turks
> HDMI Audio [Radeon HD 6500/6600 / 6700M Series]
> Subsystem: Dell Turks HDMI Audio [Radeon HD 6500/6600 / 6700M Series]
> Flags: bus master, fast devsel, latency 0, IRQ 98
> Memory at 300940000 (64-bit, non-prefetchable) [size=16K]
> Capabilities: [50] Power Management version 3
> Capabilities: [58] Express Legacy Endpoint, MSI 00
> Capabilities: [a0] MSI: Enable+ Count=1/1 Maskable- 64bit+
> Capabilities: [100] Vendor Specific Information: ID=0001 Rev=1
> Len=010 <?>
> Capabilities: [150] Advanced Error Reporting
> Kernel driver in use: snd_hda_intel
>
> This will obviously clobber registers during writes.
I don't follow. Which writes will clobber which registers, and how is
it obvious?
> Also, if <0x82000000> (32 bit) is changed to <0x83000000> (64 bit),
> most of the allocations for the dGPU fail due to no valid regions
> available.
>
But wasn't the original problem that the resource window was 64-bit to
begin with? Are you sure we are talking about the same problem here?
> >
> > > I am happy to put something together once I understand the preferred way
> > > to go about it.
> > >
> > > Thanks,
> > > Punit
> > >
> > > [...]
> > >
> >
> > _______________________________________________
> > Linux-rockchip mailing list
> > Linux-rockchip@lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [BUG] rockpro64: PCI BAR reassignment broken by commit 9d57e61bf723 ("of/pci: Add IORESOURCE_MEM_64 to resource flags for 64-bit memory addresses")
2021-05-25 15:54 ` Ard Biesheuvel
@ 2021-05-25 16:23 ` Peter Geis
2021-05-25 16:44 ` Ard Biesheuvel
2021-05-25 19:15 ` Bjorn Helgaas
1 sibling, 1 reply; 28+ messages in thread
From: Peter Geis @ 2021-05-25 16:23 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: Punit Agrawal, Robin Murphy, Alexandru Elisei,
Linux Kernel Mailing List, open list:ARM/Rockchip SoC...,
arm-mail-list, Heiko Stuebner, Leonardo Bras, Rob Herring, PCI,
Christian König
On Tue, May 25, 2021 at 11:55 AM Ard Biesheuvel <ardb@kernel.org> wrote:
>
> On Tue, 25 May 2021 at 17:34, Peter Geis <pgwipeout@gmail.com> wrote:
> >
> > On Tue, May 25, 2021 at 9:57 AM Ard Biesheuvel <ardb@kernel.org> wrote:
> > >
> > > On Tue, 25 May 2021 at 15:42, Punit Agrawal <punitagrawal@gmail.com> wrote:
> > > >
> > > > Hi Ard,
> > > >
> > > > Ard Biesheuvel <ardb@kernel.org> writes:
> > > >
> > > > > On Sun, 23 May 2021 at 13:06, Punit Agrawal <punitagrawal@gmail.com> wrote:
> > > > >>
> > > > >> Robin Murphy <robin.murphy@arm.com> writes:
> > > > >>
> > > > >> > [ +linux-pci for visibility ]
> > > > >> >
> > > > >> > On 2021-05-18 10:09, Alexandru Elisei wrote:
> > > > >> >> After doing a git bisect I was able to trace the following error when booting my
> > > > >> >> rockpro64 v2 (rk3399 SoC) with a PCIE NVME expansion card:
> > > > >> >> [..]
> > > > >> >> [ 0.305183] rockchip-pcie f8000000.pcie: host bridge /pcie@f8000000 ranges:
> > > > >> >> [ 0.305248] rockchip-pcie f8000000.pcie: MEM 0x00fa000000..0x00fbdfffff ->
> > > > >> >> 0x00fa000000
> > > > >> >> [ 0.305285] rockchip-pcie f8000000.pcie: IO 0x00fbe00000..0x00fbefffff ->
> > > > >> >> 0x00fbe00000
> > > > >> >> [ 0.306201] rockchip-pcie f8000000.pcie: supply vpcie1v8 not found, using dummy
> > > > >> >> regulator
> > > > >> >> [ 0.306334] rockchip-pcie f8000000.pcie: supply vpcie0v9 not found, using dummy
> > > > >> >> regulator
> > > > >> >> [ 0.373705] rockchip-pcie f8000000.pcie: PCI host bridge to bus 0000:00
> > > > >> >> [ 0.373730] pci_bus 0000:00: root bus resource [bus 00-1f]
> > > > >> >> [ 0.373751] pci_bus 0000:00: root bus resource [mem 0xfa000000-0xfbdfffff 64bit]
> > > > >> >> [ 0.373777] pci_bus 0000:00: root bus resource [io 0x0000-0xfffff] (bus
> > > > >> >> address [0xfbe00000-0xfbefffff])
> > > > >> >> [ 0.373839] pci 0000:00:00.0: [1d87:0100] type 01 class 0x060400
> > > > >> >> [ 0.373973] pci 0000:00:00.0: supports D1
> > > > >> >> [ 0.373992] pci 0000:00:00.0: PME# supported from D0 D1 D3hot
> > > > >> >> [ 0.378518] pci 0000:00:00.0: bridge configuration invalid ([bus 00-00]),
> > > > >> >> reconfiguring
> > > > >> >> [ 0.378765] pci 0000:01:00.0: [144d:a808] type 00 class 0x010802
> > > > >> >> [ 0.378869] pci 0000:01:00.0: reg 0x10: [mem 0x00000000-0x00003fff 64bit]
> > > > >> >> [ 0.379051] pci 0000:01:00.0: Max Payload Size set to 256 (was 128, max 256)
> > > > >> >> [ 0.379661] pci 0000:01:00.0: 8.000 Gb/s available PCIe bandwidth, limited by
> > > > >> >> 2.5 GT/s PCIe x4 link at 0000:00:00.0 (capable of 31.504 Gb/s with 8.0 GT/s PCIe
> > > > >> >> x4 link)
> > > > >> >> [ 0.393269] pci_bus 0000:01: busn_res: [bus 01-1f] end is updated to 01
> > > > >> >> [ 0.393311] pci 0000:00:00.0: BAR 14: no space for [mem size 0x00100000]
> > > > >> >> [ 0.393333] pci 0000:00:00.0: BAR 14: failed to assign [mem size 0x00100000]
> > > > >> >> [ 0.393356] pci 0000:01:00.0: BAR 0: no space for [mem size 0x00004000 64bit]
> > > > >> >> [ 0.393375] pci 0000:01:00.0: BAR 0: failed to assign [mem size 0x00004000 64bit]
> > > > >> >> [ 0.393397] pci 0000:00:00.0: PCI bridge to [bus 01]
> > > > >> >> [ 0.393839] pcieport 0000:00:00.0: PME: Signaling with IRQ 78
> > > > >> >> [ 0.394165] pcieport 0000:00:00.0: AER: enabled with IRQ 78
> > > > >> >> [..]
> > > > >> >> to the commit 9d57e61bf723 ("of/pci: Add IORESOURCE_MEM_64 to
> > > > >> >> resource flags for
> > > > >> >> 64-bit memory addresses").
> > > > >> >
> > > > >> > FWFW, my hunch is that the host bridge advertising no 32-bit memory
> > > > >> > resource, only only a single 64-bit non-prefetchable one (even though
> > > > >> > it's entirely below 4GB) might be a bit weird and tripping something
> > > > >> > up in the resource assignment code. It certainly seems like the thing
> > > > >> > most directly related to the offending commit.
> > > > >> >
> > > > >> > I'd be tempted to try fiddling with that in the DT (i.e. changing
> > > > >> > 0x83000000 to 0x82000000 in the PCIe node's "ranges" property) to see
> > > > >> > if it makes any difference. Note that even if it helps, though, I
> > > > >> > don't know whether that's the correct fix or just a bodge around a
> > > > >> > corner-case bug somewhere in the resource code.
> > > > >>
> > > > >> From digging into this further the failure seems to be due to a mismatch
> > > > >> of flags when allocating resources in pci_bus_alloc_from_region() -
> > > > >>
> > > > >> if ((res->flags ^ r->flags) & type_mask)
> > > > >> continue;
> > > > >>
> > > > >> Though I am also not sure why the failure is only being reported on
> > > > >> RK3399 - does a single 64-bit window have anything to do with it?
> > > > >>
> > > > >
> > > > > The NVMe in the example exposes a single 64-bit non-prefetchable BAR.
> > > > > Such BARs can not be allocated in a prefetchable host bridge window
> > > > > (unlike the converse, i.e., allocating a prefetchable BAR in a
> > > > > non-prefetchable host bridge window is fine)
> > > > >
> > > > > 64-bit non-prefetchable host bridge windows cannot be forwarded by PCI
> > > > > to PCI bridges, they simply lack the BAR registers to describe them.
> > > > > Therefore, non-prefetchable endpoint BARs (even 64-bit ones) need to
> > > > > be carved out of a host bridge's non-prefetchable 32-bit window if
> > > > > they need to pass through a bridge.
> > > >
> > > > Thank you for the explanation. I also looked at the PCI-to-PCI Bridge
> > > > spec to understand where some of the limitations are coming from.
> > > >
> > > > > So the error seems to be here that the host bridge's 32-bit
> > > > > non-prefetchable window has the 64-bit attribute set, even though it
> > > > > resides below 4 GB entirely. I suppose that the resource allocation
> > > > > could be made more forgiving (and it was in the past, before commit
> > > > > 9d57e61bf723 was applied). However, I would strongly recommend not
> > > > > deviating from common practice, and just describe the 32-bit
> > > > > addressable non-prefetchable resource window as such.
> > > >
> > > > IIUC, the host bridge's configuration (64-bit on non-prefetchable
> > > > window) is based on what the hardware advertises.
> > > >
> > >
> > > What do you mean by 'what the hardware advertises'? The host bridge is
> > > apparently configured to decode a 32-bit addressable window as MMIO,
> > > and the question is why this window has the 64-bit attribute set in
> > > the DT description.
> > >
> > > > Can you elaborate on what you have in mind to correct the
> > > > non-prefetchable resource window? Are you thinking of adding a quirk
> > > > somewhere to address this?
> > > >
> > >
> > > No. Just fix the DT.
> >
> > Good Morning,
> >
> > I believe Robin is correct that there is more to this.
> > While attempting to work out why dGPUs won't work with the rk356x
> > series PCIe controllers, Christian König from the amd-gpu driver
> > mailing list noticed the gpu was incorrectly allocated a 64bit
> > non-prefetchable BAR which should instead be a 32 non-prefetchable
> > BAR.
> >
>
> This is due to the translation. For some reason, lspci translates the
> BAR values to CPU addresses, but the PCI side addresses are within
> 32-bits.
The kernel log reports the same thing:
[ 6.662141] pci 0000:01:00.0: reg 0x10: [mem 0x00000000-0x0fffffff
64bit pref]
[ 6.662963] pci 0000:01:00.0: reg 0x18: [mem 0x00000000-0x0001ffff 64bit]
You are saying this is a display only issue?
>
> Are you sure the amdgpu driver can even deal with non-1:1 host bridges?
I cannot answer this as I'm not an amdgpu dev.
>
> > The ranges currently set are:
> > ranges = <0x81000000 0x0 0x00800000 0x3 0x00800000 0x0 0x00100000
> > 0x82000000 0x0 0x00900000 0x3 0x00900000 0x0 0x3f700000>;
> >
>
> So you have two ranges here.
The IO and PCI memory ranges.
There is a third range, the configuration range, which is defined in
the reg block:
<0x3 0x00000000 0x0 0x800000>
All three are shared in the same 1GB window on the rk356x.
https://elixir.bootlin.com/linux/v5.13-rc3/source/Documentation/devicetree/bindings/pci/designware-pcie.txt#L12
>
> > but the final allocation was:
> >
> > lspci -v
> > 00:00.0 PCI bridge: Fuzhou Rockchip Electronics Co., Ltd Device 3566
> > (rev 01) (prog-if 00 [Normal decode])
> > Flags: bus master, fast devsel, latency 0, IRQ 96
> > Bus: primary=00, secondary=01, subordinate=ff, sec-latency=0
> > I/O behind bridge: 00001000-00001fff [size=4K]
> > Memory behind bridge: 00900000-009fffff [size=1M]
> > Prefetchable memory behind bridge:
> > 0000000010000000-000000001fffffff [size=256M]
>
> But the host bridge/root port decodes two disjoint regions??
>
> > Expansion ROM at 300a00000 [virtual] [disabled] [size=64K]
> > Capabilities: [40] Power Management version 3
> > Capabilities: [50] MSI: Enable+ Count=1/32 Maskable- 64bit+
> > Capabilities: [70] Express Root Port (Slot-), MSI 00
> > Capabilities: [b0] MSI-X: Enable- Count=1 Masked-
> > Capabilities: [100] Advanced Error Reporting
> > Capabilities: [148] Secondary PCI Express
> > Capabilities: [160] L1 PM Substates
> > Capabilities: [170] Vendor Specific Information: ID=0002 Rev=4
> > Len=100 <?>
> > Kernel driver in use: pcieport
> >
> > 01:00.0 VGA compatible controller: Advanced Micro Devices, Inc.
> > [AMD/ATI] Turks PRO [Radeon HD 7570] (prog-if 00 [VGA controller])
> > Subsystem: Dell Turks PRO [Radeon HD 7570]
> > Flags: bus master, fast devsel, latency 0, IRQ 95
> > Memory at 310000000 (64-bit, prefetchable) [size=256M]
> > Memory at 300900000 (64-bit, non-prefetchable) [size=128K]
> > I/O ports at 1000 [size=256]
> > Expansion ROM at 300920000 [disabled] [size=128K]
> > Capabilities: [50] Power Management version 3
> > Capabilities: [58] Express Legacy Endpoint, MSI 00
> > Capabilities: [a0] MSI: Enable- Count=1/1 Maskable- 64bit+
> > Capabilities: [100] Vendor Specific Information: ID=0001 Rev=1
> > Len=010 <?>
> > Capabilities: [150] Advanced Error Reporting
> > Kernel driver in use: radeon
> >
> > 01:00.1 Audio device: Advanced Micro Devices, Inc. [AMD/ATI] Turks
> > HDMI Audio [Radeon HD 6500/6600 / 6700M Series]
> > Subsystem: Dell Turks HDMI Audio [Radeon HD 6500/6600 / 6700M Series]
> > Flags: bus master, fast devsel, latency 0, IRQ 98
> > Memory at 300940000 (64-bit, non-prefetchable) [size=16K]
> > Capabilities: [50] Power Management version 3
> > Capabilities: [58] Express Legacy Endpoint, MSI 00
> > Capabilities: [a0] MSI: Enable+ Count=1/1 Maskable- 64bit+
> > Capabilities: [100] Vendor Specific Information: ID=0001 Rev=1
> > Len=010 <?>
> > Capabilities: [150] Advanced Error Reporting
> > Kernel driver in use: snd_hda_intel
> >
> > This will obviously clobber registers during writes.
>
> I don't follow. Which writes will clobber which registers, and how is
> it obvious?
Writing a 64 bit word into a 32 bit register will either clobber the
next higher 32 bit register.
Quoting Christian:
"When you program a 32bit BAR as 64bit you overwrite the register behind
the BAR address with the upper 32bits of the 64bit address value.
So even if the allocation fits into 32bits, the extra register write
will certainly put your device into a banana state."
https://lists.freedesktop.org/archives/amd-gfx/2021-May/064232.html
>
> > Also, if <0x82000000> (32 bit) is changed to <0x83000000> (64 bit),
> > most of the allocations for the dGPU fail due to no valid regions
> > available.
> >
>
> But wasn't the original problem that the resource window was 64-bit to
> begin with? Are you sure we are talking about the same problem here?
The rk3399 in the original report has a 32MB memory window in the
upper end of the 4GB range.
The rk356x has a similar layout, or it can use a 1GB window available
at <0x3 0x00000000>.
Rockchip's default windows are defined as 64bit.
The rk3399 doesn't have enough space to reasonably define two windows,
one 32bit, one 64bit, to work around an allocation bug.
These are the defined regions in the rk3399:
ranges = <0x83000000 0x0 0xfa000000 0x0 0xfa000000 0x0 0x1e00000>,
<0x81000000 0x0 0xfbe00000 0x0 0xfbe00000 0x0 0x100000>;
>
>
> > >
> > > > I am happy to put something together once I understand the preferred way
> > > > to go about it.
> > > >
> > > > Thanks,
> > > > Punit
> > > >
> > > > [...]
> > > >
> > >
> > > _______________________________________________
> > > Linux-rockchip mailing list
> > > Linux-rockchip@lists.infradead.org
> > > http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [BUG] rockpro64: PCI BAR reassignment broken by commit 9d57e61bf723 ("of/pci: Add IORESOURCE_MEM_64 to resource flags for 64-bit memory addresses")
2021-05-25 16:23 ` Peter Geis
@ 2021-05-25 16:44 ` Ard Biesheuvel
2021-05-25 17:01 ` Peter Geis
0 siblings, 1 reply; 28+ messages in thread
From: Ard Biesheuvel @ 2021-05-25 16:44 UTC (permalink / raw)
To: Peter Geis
Cc: Punit Agrawal, Robin Murphy, Alexandru Elisei,
Linux Kernel Mailing List, open list:ARM/Rockchip SoC...,
arm-mail-list, Heiko Stuebner, Leonardo Bras, Rob Herring, PCI,
Christian König
On Tue, 25 May 2021 at 18:23, Peter Geis <pgwipeout@gmail.com> wrote:
>
> On Tue, May 25, 2021 at 11:55 AM Ard Biesheuvel <ardb@kernel.org> wrote:
> >
> > On Tue, 25 May 2021 at 17:34, Peter Geis <pgwipeout@gmail.com> wrote:
> > >
> > > On Tue, May 25, 2021 at 9:57 AM Ard Biesheuvel <ardb@kernel.org> wrote:
> > > >
> > > > On Tue, 25 May 2021 at 15:42, Punit Agrawal <punitagrawal@gmail.com> wrote:
> > > > >
> > > > > Hi Ard,
> > > > >
> > > > > Ard Biesheuvel <ardb@kernel.org> writes:
> > > > >
> > > > > > On Sun, 23 May 2021 at 13:06, Punit Agrawal <punitagrawal@gmail.com> wrote:
> > > > > >>
> > > > > >> Robin Murphy <robin.murphy@arm.com> writes:
> > > > > >>
> > > > > >> > [ +linux-pci for visibility ]
> > > > > >> >
> > > > > >> > On 2021-05-18 10:09, Alexandru Elisei wrote:
> > > > > >> >> After doing a git bisect I was able to trace the following error when booting my
> > > > > >> >> rockpro64 v2 (rk3399 SoC) with a PCIE NVME expansion card:
> > > > > >> >> [..]
> > > > > >> >> [ 0.305183] rockchip-pcie f8000000.pcie: host bridge /pcie@f8000000 ranges:
> > > > > >> >> [ 0.305248] rockchip-pcie f8000000.pcie: MEM 0x00fa000000..0x00fbdfffff ->
> > > > > >> >> 0x00fa000000
> > > > > >> >> [ 0.305285] rockchip-pcie f8000000.pcie: IO 0x00fbe00000..0x00fbefffff ->
> > > > > >> >> 0x00fbe00000
> > > > > >> >> [ 0.306201] rockchip-pcie f8000000.pcie: supply vpcie1v8 not found, using dummy
> > > > > >> >> regulator
> > > > > >> >> [ 0.306334] rockchip-pcie f8000000.pcie: supply vpcie0v9 not found, using dummy
> > > > > >> >> regulator
> > > > > >> >> [ 0.373705] rockchip-pcie f8000000.pcie: PCI host bridge to bus 0000:00
> > > > > >> >> [ 0.373730] pci_bus 0000:00: root bus resource [bus 00-1f]
> > > > > >> >> [ 0.373751] pci_bus 0000:00: root bus resource [mem 0xfa000000-0xfbdfffff 64bit]
> > > > > >> >> [ 0.373777] pci_bus 0000:00: root bus resource [io 0x0000-0xfffff] (bus
> > > > > >> >> address [0xfbe00000-0xfbefffff])
> > > > > >> >> [ 0.373839] pci 0000:00:00.0: [1d87:0100] type 01 class 0x060400
> > > > > >> >> [ 0.373973] pci 0000:00:00.0: supports D1
> > > > > >> >> [ 0.373992] pci 0000:00:00.0: PME# supported from D0 D1 D3hot
> > > > > >> >> [ 0.378518] pci 0000:00:00.0: bridge configuration invalid ([bus 00-00]),
> > > > > >> >> reconfiguring
> > > > > >> >> [ 0.378765] pci 0000:01:00.0: [144d:a808] type 00 class 0x010802
> > > > > >> >> [ 0.378869] pci 0000:01:00.0: reg 0x10: [mem 0x00000000-0x00003fff 64bit]
> > > > > >> >> [ 0.379051] pci 0000:01:00.0: Max Payload Size set to 256 (was 128, max 256)
> > > > > >> >> [ 0.379661] pci 0000:01:00.0: 8.000 Gb/s available PCIe bandwidth, limited by
> > > > > >> >> 2.5 GT/s PCIe x4 link at 0000:00:00.0 (capable of 31.504 Gb/s with 8.0 GT/s PCIe
> > > > > >> >> x4 link)
> > > > > >> >> [ 0.393269] pci_bus 0000:01: busn_res: [bus 01-1f] end is updated to 01
> > > > > >> >> [ 0.393311] pci 0000:00:00.0: BAR 14: no space for [mem size 0x00100000]
> > > > > >> >> [ 0.393333] pci 0000:00:00.0: BAR 14: failed to assign [mem size 0x00100000]
> > > > > >> >> [ 0.393356] pci 0000:01:00.0: BAR 0: no space for [mem size 0x00004000 64bit]
> > > > > >> >> [ 0.393375] pci 0000:01:00.0: BAR 0: failed to assign [mem size 0x00004000 64bit]
> > > > > >> >> [ 0.393397] pci 0000:00:00.0: PCI bridge to [bus 01]
> > > > > >> >> [ 0.393839] pcieport 0000:00:00.0: PME: Signaling with IRQ 78
> > > > > >> >> [ 0.394165] pcieport 0000:00:00.0: AER: enabled with IRQ 78
> > > > > >> >> [..]
> > > > > >> >> to the commit 9d57e61bf723 ("of/pci: Add IORESOURCE_MEM_64 to
> > > > > >> >> resource flags for
> > > > > >> >> 64-bit memory addresses").
> > > > > >> >
> > > > > >> > FWFW, my hunch is that the host bridge advertising no 32-bit memory
> > > > > >> > resource, only only a single 64-bit non-prefetchable one (even though
> > > > > >> > it's entirely below 4GB) might be a bit weird and tripping something
> > > > > >> > up in the resource assignment code. It certainly seems like the thing
> > > > > >> > most directly related to the offending commit.
> > > > > >> >
> > > > > >> > I'd be tempted to try fiddling with that in the DT (i.e. changing
> > > > > >> > 0x83000000 to 0x82000000 in the PCIe node's "ranges" property) to see
> > > > > >> > if it makes any difference. Note that even if it helps, though, I
> > > > > >> > don't know whether that's the correct fix or just a bodge around a
> > > > > >> > corner-case bug somewhere in the resource code.
> > > > > >>
> > > > > >> From digging into this further the failure seems to be due to a mismatch
> > > > > >> of flags when allocating resources in pci_bus_alloc_from_region() -
> > > > > >>
> > > > > >> if ((res->flags ^ r->flags) & type_mask)
> > > > > >> continue;
> > > > > >>
> > > > > >> Though I am also not sure why the failure is only being reported on
> > > > > >> RK3399 - does a single 64-bit window have anything to do with it?
> > > > > >>
> > > > > >
> > > > > > The NVMe in the example exposes a single 64-bit non-prefetchable BAR.
> > > > > > Such BARs can not be allocated in a prefetchable host bridge window
> > > > > > (unlike the converse, i.e., allocating a prefetchable BAR in a
> > > > > > non-prefetchable host bridge window is fine)
> > > > > >
> > > > > > 64-bit non-prefetchable host bridge windows cannot be forwarded by PCI
> > > > > > to PCI bridges, they simply lack the BAR registers to describe them.
> > > > > > Therefore, non-prefetchable endpoint BARs (even 64-bit ones) need to
> > > > > > be carved out of a host bridge's non-prefetchable 32-bit window if
> > > > > > they need to pass through a bridge.
> > > > >
> > > > > Thank you for the explanation. I also looked at the PCI-to-PCI Bridge
> > > > > spec to understand where some of the limitations are coming from.
> > > > >
> > > > > > So the error seems to be here that the host bridge's 32-bit
> > > > > > non-prefetchable window has the 64-bit attribute set, even though it
> > > > > > resides below 4 GB entirely. I suppose that the resource allocation
> > > > > > could be made more forgiving (and it was in the past, before commit
> > > > > > 9d57e61bf723 was applied). However, I would strongly recommend not
> > > > > > deviating from common practice, and just describe the 32-bit
> > > > > > addressable non-prefetchable resource window as such.
> > > > >
> > > > > IIUC, the host bridge's configuration (64-bit on non-prefetchable
> > > > > window) is based on what the hardware advertises.
> > > > >
> > > >
> > > > What do you mean by 'what the hardware advertises'? The host bridge is
> > > > apparently configured to decode a 32-bit addressable window as MMIO,
> > > > and the question is why this window has the 64-bit attribute set in
> > > > the DT description.
> > > >
> > > > > Can you elaborate on what you have in mind to correct the
> > > > > non-prefetchable resource window? Are you thinking of adding a quirk
> > > > > somewhere to address this?
> > > > >
> > > >
> > > > No. Just fix the DT.
> > >
> > > Good Morning,
> > >
> > > I believe Robin is correct that there is more to this.
> > > While attempting to work out why dGPUs won't work with the rk356x
> > > series PCIe controllers, Christian König from the amd-gpu driver
> > > mailing list noticed the gpu was incorrectly allocated a 64bit
> > > non-prefetchable BAR which should instead be a 32 non-prefetchable
> > > BAR.
> > >
> >
> > This is due to the translation. For some reason, lspci translates the
> > BAR values to CPU addresses, but the PCI side addresses are within
> > 32-bits.
>
> The kernel log reports the same thing:
> [ 6.662141] pci 0000:01:00.0: reg 0x10: [mem 0x00000000-0x0fffffff
> 64bit pref]
> [ 6.662963] pci 0000:01:00.0: reg 0x18: [mem 0x00000000-0x0001ffff 64bit]
>
> You are saying this is a display only issue?
>
Yes. What do the 'root bus resource' log lines say for these regions?
Those should give you both the CPU address as well as the bus address.
> >
> > Are you sure the amdgpu driver can even deal with non-1:1 host bridges?
>
> I cannot answer this as I'm not an amdgpu dev.
>
> >
> > > The ranges currently set are:
> > > ranges = <0x81000000 0x0 0x00800000 0x3 0x00800000 0x0 0x00100000
> > > 0x82000000 0x0 0x00900000 0x3 0x00900000 0x0 0x3f700000>;
> > >
> >
> > So you have two ranges here.
>
> The IO and PCI memory ranges.
>
> There is a third range, the configuration range, which is defined in
> the reg block:
> <0x3 0x00000000 0x0 0x800000>
> All three are shared in the same 1GB window on the rk356x.
>
But the reg block is not a resource window, it is a configuration
range to program the host bridge.
> https://elixir.bootlin.com/linux/v5.13-rc3/source/Documentation/devicetree/bindings/pci/designware-pcie.txt#L12
>
> >
> > > but the final allocation was:
> > >
> > > lspci -v
> > > 00:00.0 PCI bridge: Fuzhou Rockchip Electronics Co., Ltd Device 3566
> > > (rev 01) (prog-if 00 [Normal decode])
> > > Flags: bus master, fast devsel, latency 0, IRQ 96
> > > Bus: primary=00, secondary=01, subordinate=ff, sec-latency=0
> > > I/O behind bridge: 00001000-00001fff [size=4K]
> > > Memory behind bridge: 00900000-009fffff [size=1M]
> > > Prefetchable memory behind bridge:
> > > 0000000010000000-000000001fffffff [size=256M]
> >
> > But the host bridge/root port decodes two disjoint regions??
> >
> > > Expansion ROM at 300a00000 [virtual] [disabled] [size=64K]
> > > Capabilities: [40] Power Management version 3
> > > Capabilities: [50] MSI: Enable+ Count=1/32 Maskable- 64bit+
> > > Capabilities: [70] Express Root Port (Slot-), MSI 00
> > > Capabilities: [b0] MSI-X: Enable- Count=1 Masked-
> > > Capabilities: [100] Advanced Error Reporting
> > > Capabilities: [148] Secondary PCI Express
> > > Capabilities: [160] L1 PM Substates
> > > Capabilities: [170] Vendor Specific Information: ID=0002 Rev=4
> > > Len=100 <?>
> > > Kernel driver in use: pcieport
> > >
> > > 01:00.0 VGA compatible controller: Advanced Micro Devices, Inc.
> > > [AMD/ATI] Turks PRO [Radeon HD 7570] (prog-if 00 [VGA controller])
> > > Subsystem: Dell Turks PRO [Radeon HD 7570]
> > > Flags: bus master, fast devsel, latency 0, IRQ 95
> > > Memory at 310000000 (64-bit, prefetchable) [size=256M]
> > > Memory at 300900000 (64-bit, non-prefetchable) [size=128K]
> > > I/O ports at 1000 [size=256]
> > > Expansion ROM at 300920000 [disabled] [size=128K]
> > > Capabilities: [50] Power Management version 3
> > > Capabilities: [58] Express Legacy Endpoint, MSI 00
> > > Capabilities: [a0] MSI: Enable- Count=1/1 Maskable- 64bit+
> > > Capabilities: [100] Vendor Specific Information: ID=0001 Rev=1
> > > Len=010 <?>
> > > Capabilities: [150] Advanced Error Reporting
> > > Kernel driver in use: radeon
> > >
> > > 01:00.1 Audio device: Advanced Micro Devices, Inc. [AMD/ATI] Turks
> > > HDMI Audio [Radeon HD 6500/6600 / 6700M Series]
> > > Subsystem: Dell Turks HDMI Audio [Radeon HD 6500/6600 / 6700M Series]
> > > Flags: bus master, fast devsel, latency 0, IRQ 98
> > > Memory at 300940000 (64-bit, non-prefetchable) [size=16K]
> > > Capabilities: [50] Power Management version 3
> > > Capabilities: [58] Express Legacy Endpoint, MSI 00
> > > Capabilities: [a0] MSI: Enable+ Count=1/1 Maskable- 64bit+
> > > Capabilities: [100] Vendor Specific Information: ID=0001 Rev=1
> > > Len=010 <?>
> > > Capabilities: [150] Advanced Error Reporting
> > > Kernel driver in use: snd_hda_intel
> > >
> > > This will obviously clobber registers during writes.
> >
> > I don't follow. Which writes will clobber which registers, and how is
> > it obvious?
>
> Writing a 64 bit word into a 32 bit register will either clobber the
> next higher 32 bit register.
> Quoting Christian:
> "When you program a 32bit BAR as 64bit you overwrite the register behind
> the BAR address with the upper 32bits of the 64bit address value.
> So even if the allocation fits into 32bits, the extra register write
> will certainly put your device into a banana state."
>
> https://lists.freedesktop.org/archives/amd-gfx/2021-May/064232.html
>
I seriously doubt that this is what is going on here.
lspci -x will give you the bare BAR values - I suspect that those are
probably fine.
> >
> > > Also, if <0x82000000> (32 bit) is changed to <0x83000000> (64 bit),
> > > most of the allocations for the dGPU fail due to no valid regions
> > > available.
> > >
> >
> > But wasn't the original problem that the resource window was 64-bit to
> > begin with? Are you sure we are talking about the same problem here?
>
> The rk3399 in the original report has a 32MB memory window in the
> upper end of the 4GB range.
> The rk356x has a similar layout, or it can use a 1GB window available
> at <0x3 0x00000000>.
> Rockchip's default windows are defined as 64bit.
>
> The rk3399 doesn't have enough space to reasonably define two windows,
> one 32bit, one 64bit, to work around an allocation bug.
> These are the defined regions in the rk3399:
> ranges = <0x83000000 0x0 0xfa000000 0x0 0xfa000000 0x0 0x1e00000>,
> <0x81000000 0x0 0xfbe00000 0x0 0xfbe00000 0x0 0x100000>;
>
All you really need is a 32-bit non-prefetchable resource window: any
BAR can be allocated from that. A 64-bit BAR can carry a 32-bit number
(just add zeroes at the top), and a prefetchable BAR can happily live
in a non-prefetchable window, with a theoretical performance impact if
the OS actually does use different memory attributes for the
prefetchable window (but I don't think Linux ever handles it this way)
>
> >
> >
> > > >
> > > > > I am happy to put something together once I understand the preferred way
> > > > > to go about it.
> > > > >
> > > > > Thanks,
> > > > > Punit
> > > > >
> > > > > [...]
> > > > >
> > > >
> > > > _______________________________________________
> > > > Linux-rockchip mailing list
> > > > Linux-rockchip@lists.infradead.org
> > > > http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [BUG] rockpro64: PCI BAR reassignment broken by commit 9d57e61bf723 ("of/pci: Add IORESOURCE_MEM_64 to resource flags for 64-bit memory addresses")
2021-05-25 16:44 ` Ard Biesheuvel
@ 2021-05-25 17:01 ` Peter Geis
2021-05-25 17:18 ` Ard Biesheuvel
2021-05-25 17:25 ` Robin Murphy
0 siblings, 2 replies; 28+ messages in thread
From: Peter Geis @ 2021-05-25 17:01 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: Punit Agrawal, Robin Murphy, Alexandru Elisei,
Linux Kernel Mailing List, open list:ARM/Rockchip SoC...,
arm-mail-list, Heiko Stuebner, Leonardo Bras, Rob Herring, PCI,
Christian König
On Tue, May 25, 2021 at 12:44 PM Ard Biesheuvel <ardb@kernel.org> wrote:
>
> On Tue, 25 May 2021 at 18:23, Peter Geis <pgwipeout@gmail.com> wrote:
> >
> > On Tue, May 25, 2021 at 11:55 AM Ard Biesheuvel <ardb@kernel.org> wrote:
> > >
> > > On Tue, 25 May 2021 at 17:34, Peter Geis <pgwipeout@gmail.com> wrote:
> > > >
> > > > On Tue, May 25, 2021 at 9:57 AM Ard Biesheuvel <ardb@kernel.org> wrote:
> > > > >
> > > > > On Tue, 25 May 2021 at 15:42, Punit Agrawal <punitagrawal@gmail.com> wrote:
> > > > > >
> > > > > > Hi Ard,
> > > > > >
> > > > > > Ard Biesheuvel <ardb@kernel.org> writes:
> > > > > >
> > > > > > > On Sun, 23 May 2021 at 13:06, Punit Agrawal <punitagrawal@gmail.com> wrote:
> > > > > > >>
> > > > > > >> Robin Murphy <robin.murphy@arm.com> writes:
> > > > > > >>
> > > > > > >> > [ +linux-pci for visibility ]
> > > > > > >> >
> > > > > > >> > On 2021-05-18 10:09, Alexandru Elisei wrote:
> > > > > > >> >> After doing a git bisect I was able to trace the following error when booting my
> > > > > > >> >> rockpro64 v2 (rk3399 SoC) with a PCIE NVME expansion card:
> > > > > > >> >> [..]
> > > > > > >> >> [ 0.305183] rockchip-pcie f8000000.pcie: host bridge /pcie@f8000000 ranges:
> > > > > > >> >> [ 0.305248] rockchip-pcie f8000000.pcie: MEM 0x00fa000000..0x00fbdfffff ->
> > > > > > >> >> 0x00fa000000
> > > > > > >> >> [ 0.305285] rockchip-pcie f8000000.pcie: IO 0x00fbe00000..0x00fbefffff ->
> > > > > > >> >> 0x00fbe00000
> > > > > > >> >> [ 0.306201] rockchip-pcie f8000000.pcie: supply vpcie1v8 not found, using dummy
> > > > > > >> >> regulator
> > > > > > >> >> [ 0.306334] rockchip-pcie f8000000.pcie: supply vpcie0v9 not found, using dummy
> > > > > > >> >> regulator
> > > > > > >> >> [ 0.373705] rockchip-pcie f8000000.pcie: PCI host bridge to bus 0000:00
> > > > > > >> >> [ 0.373730] pci_bus 0000:00: root bus resource [bus 00-1f]
> > > > > > >> >> [ 0.373751] pci_bus 0000:00: root bus resource [mem 0xfa000000-0xfbdfffff 64bit]
> > > > > > >> >> [ 0.373777] pci_bus 0000:00: root bus resource [io 0x0000-0xfffff] (bus
> > > > > > >> >> address [0xfbe00000-0xfbefffff])
> > > > > > >> >> [ 0.373839] pci 0000:00:00.0: [1d87:0100] type 01 class 0x060400
> > > > > > >> >> [ 0.373973] pci 0000:00:00.0: supports D1
> > > > > > >> >> [ 0.373992] pci 0000:00:00.0: PME# supported from D0 D1 D3hot
> > > > > > >> >> [ 0.378518] pci 0000:00:00.0: bridge configuration invalid ([bus 00-00]),
> > > > > > >> >> reconfiguring
> > > > > > >> >> [ 0.378765] pci 0000:01:00.0: [144d:a808] type 00 class 0x010802
> > > > > > >> >> [ 0.378869] pci 0000:01:00.0: reg 0x10: [mem 0x00000000-0x00003fff 64bit]
> > > > > > >> >> [ 0.379051] pci 0000:01:00.0: Max Payload Size set to 256 (was 128, max 256)
> > > > > > >> >> [ 0.379661] pci 0000:01:00.0: 8.000 Gb/s available PCIe bandwidth, limited by
> > > > > > >> >> 2.5 GT/s PCIe x4 link at 0000:00:00.0 (capable of 31.504 Gb/s with 8.0 GT/s PCIe
> > > > > > >> >> x4 link)
> > > > > > >> >> [ 0.393269] pci_bus 0000:01: busn_res: [bus 01-1f] end is updated to 01
> > > > > > >> >> [ 0.393311] pci 0000:00:00.0: BAR 14: no space for [mem size 0x00100000]
> > > > > > >> >> [ 0.393333] pci 0000:00:00.0: BAR 14: failed to assign [mem size 0x00100000]
> > > > > > >> >> [ 0.393356] pci 0000:01:00.0: BAR 0: no space for [mem size 0x00004000 64bit]
> > > > > > >> >> [ 0.393375] pci 0000:01:00.0: BAR 0: failed to assign [mem size 0x00004000 64bit]
> > > > > > >> >> [ 0.393397] pci 0000:00:00.0: PCI bridge to [bus 01]
> > > > > > >> >> [ 0.393839] pcieport 0000:00:00.0: PME: Signaling with IRQ 78
> > > > > > >> >> [ 0.394165] pcieport 0000:00:00.0: AER: enabled with IRQ 78
> > > > > > >> >> [..]
> > > > > > >> >> to the commit 9d57e61bf723 ("of/pci: Add IORESOURCE_MEM_64 to
> > > > > > >> >> resource flags for
> > > > > > >> >> 64-bit memory addresses").
> > > > > > >> >
> > > > > > >> > FWFW, my hunch is that the host bridge advertising no 32-bit memory
> > > > > > >> > resource, only only a single 64-bit non-prefetchable one (even though
> > > > > > >> > it's entirely below 4GB) might be a bit weird and tripping something
> > > > > > >> > up in the resource assignment code. It certainly seems like the thing
> > > > > > >> > most directly related to the offending commit.
> > > > > > >> >
> > > > > > >> > I'd be tempted to try fiddling with that in the DT (i.e. changing
> > > > > > >> > 0x83000000 to 0x82000000 in the PCIe node's "ranges" property) to see
> > > > > > >> > if it makes any difference. Note that even if it helps, though, I
> > > > > > >> > don't know whether that's the correct fix or just a bodge around a
> > > > > > >> > corner-case bug somewhere in the resource code.
> > > > > > >>
> > > > > > >> From digging into this further the failure seems to be due to a mismatch
> > > > > > >> of flags when allocating resources in pci_bus_alloc_from_region() -
> > > > > > >>
> > > > > > >> if ((res->flags ^ r->flags) & type_mask)
> > > > > > >> continue;
> > > > > > >>
> > > > > > >> Though I am also not sure why the failure is only being reported on
> > > > > > >> RK3399 - does a single 64-bit window have anything to do with it?
> > > > > > >>
> > > > > > >
> > > > > > > The NVMe in the example exposes a single 64-bit non-prefetchable BAR.
> > > > > > > Such BARs can not be allocated in a prefetchable host bridge window
> > > > > > > (unlike the converse, i.e., allocating a prefetchable BAR in a
> > > > > > > non-prefetchable host bridge window is fine)
> > > > > > >
> > > > > > > 64-bit non-prefetchable host bridge windows cannot be forwarded by PCI
> > > > > > > to PCI bridges, they simply lack the BAR registers to describe them.
> > > > > > > Therefore, non-prefetchable endpoint BARs (even 64-bit ones) need to
> > > > > > > be carved out of a host bridge's non-prefetchable 32-bit window if
> > > > > > > they need to pass through a bridge.
> > > > > >
> > > > > > Thank you for the explanation. I also looked at the PCI-to-PCI Bridge
> > > > > > spec to understand where some of the limitations are coming from.
> > > > > >
> > > > > > > So the error seems to be here that the host bridge's 32-bit
> > > > > > > non-prefetchable window has the 64-bit attribute set, even though it
> > > > > > > resides below 4 GB entirely. I suppose that the resource allocation
> > > > > > > could be made more forgiving (and it was in the past, before commit
> > > > > > > 9d57e61bf723 was applied). However, I would strongly recommend not
> > > > > > > deviating from common practice, and just describe the 32-bit
> > > > > > > addressable non-prefetchable resource window as such.
> > > > > >
> > > > > > IIUC, the host bridge's configuration (64-bit on non-prefetchable
> > > > > > window) is based on what the hardware advertises.
> > > > > >
> > > > >
> > > > > What do you mean by 'what the hardware advertises'? The host bridge is
> > > > > apparently configured to decode a 32-bit addressable window as MMIO,
> > > > > and the question is why this window has the 64-bit attribute set in
> > > > > the DT description.
> > > > >
> > > > > > Can you elaborate on what you have in mind to correct the
> > > > > > non-prefetchable resource window? Are you thinking of adding a quirk
> > > > > > somewhere to address this?
> > > > > >
> > > > >
> > > > > No. Just fix the DT.
> > > >
> > > > Good Morning,
> > > >
> > > > I believe Robin is correct that there is more to this.
> > > > While attempting to work out why dGPUs won't work with the rk356x
> > > > series PCIe controllers, Christian König from the amd-gpu driver
> > > > mailing list noticed the gpu was incorrectly allocated a 64bit
> > > > non-prefetchable BAR which should instead be a 32 non-prefetchable
> > > > BAR.
> > > >
> > >
> > > This is due to the translation. For some reason, lspci translates the
> > > BAR values to CPU addresses, but the PCI side addresses are within
> > > 32-bits.
> >
> > The kernel log reports the same thing:
> > [ 6.662141] pci 0000:01:00.0: reg 0x10: [mem 0x00000000-0x0fffffff
> > 64bit pref]
> > [ 6.662963] pci 0000:01:00.0: reg 0x18: [mem 0x00000000-0x0001ffff 64bit]
> >
> > You are saying this is a display only issue?
> >
>
> Yes. What do the 'root bus resource' log lines say for these regions?
> Those should give you both the CPU address as well as the bus address.
[ 6.673497] pci_bus 0000:00: root bus resource [io 0x0000-0xfffff]
(bus address [0x3f700000-0x3f7fffff])
[ 6.674642] pci_bus 0000:00: root bus resource [mem
0x300000000-0x33f6fffff] (bus address [0x00000000-0x3f6fffff])
I tweaked the original Rockchip values to place the non-prefetchable
memory first with the configuration and io later in this boot.
>
>
> > >
> > > Are you sure the amdgpu driver can even deal with non-1:1 host bridges?
> >
> > I cannot answer this as I'm not an amdgpu dev.
> >
> > >
> > > > The ranges currently set are:
> > > > ranges = <0x81000000 0x0 0x00800000 0x3 0x00800000 0x0 0x00100000
> > > > 0x82000000 0x0 0x00900000 0x3 0x00900000 0x0 0x3f700000>;
> > > >
> > >
> > > So you have two ranges here.
> >
> > The IO and PCI memory ranges.
> >
> > There is a third range, the configuration range, which is defined in
> > the reg block:
> > <0x3 0x00000000 0x0 0x800000>
> > All three are shared in the same 1GB window on the rk356x.
> >
>
> But the reg block is not a resource window, it is a configuration
> range to program the host bridge.
>
> > https://elixir.bootlin.com/linux/v5.13-rc3/source/Documentation/devicetree/bindings/pci/designware-pcie.txt#L12
> >
> > >
> > > > but the final allocation was:
> > > >
> > > > lspci -v
> > > > 00:00.0 PCI bridge: Fuzhou Rockchip Electronics Co., Ltd Device 3566
> > > > (rev 01) (prog-if 00 [Normal decode])
> > > > Flags: bus master, fast devsel, latency 0, IRQ 96
> > > > Bus: primary=00, secondary=01, subordinate=ff, sec-latency=0
> > > > I/O behind bridge: 00001000-00001fff [size=4K]
> > > > Memory behind bridge: 00900000-009fffff [size=1M]
> > > > Prefetchable memory behind bridge:
> > > > 0000000010000000-000000001fffffff [size=256M]
> > >
> > > But the host bridge/root port decodes two disjoint regions??
> > >
> > > > Expansion ROM at 300a00000 [virtual] [disabled] [size=64K]
> > > > Capabilities: [40] Power Management version 3
> > > > Capabilities: [50] MSI: Enable+ Count=1/32 Maskable- 64bit+
> > > > Capabilities: [70] Express Root Port (Slot-), MSI 00
> > > > Capabilities: [b0] MSI-X: Enable- Count=1 Masked-
> > > > Capabilities: [100] Advanced Error Reporting
> > > > Capabilities: [148] Secondary PCI Express
> > > > Capabilities: [160] L1 PM Substates
> > > > Capabilities: [170] Vendor Specific Information: ID=0002 Rev=4
> > > > Len=100 <?>
> > > > Kernel driver in use: pcieport
> > > >
> > > > 01:00.0 VGA compatible controller: Advanced Micro Devices, Inc.
> > > > [AMD/ATI] Turks PRO [Radeon HD 7570] (prog-if 00 [VGA controller])
> > > > Subsystem: Dell Turks PRO [Radeon HD 7570]
> > > > Flags: bus master, fast devsel, latency 0, IRQ 95
> > > > Memory at 310000000 (64-bit, prefetchable) [size=256M]
> > > > Memory at 300900000 (64-bit, non-prefetchable) [size=128K]
> > > > I/O ports at 1000 [size=256]
> > > > Expansion ROM at 300920000 [disabled] [size=128K]
> > > > Capabilities: [50] Power Management version 3
> > > > Capabilities: [58] Express Legacy Endpoint, MSI 00
> > > > Capabilities: [a0] MSI: Enable- Count=1/1 Maskable- 64bit+
> > > > Capabilities: [100] Vendor Specific Information: ID=0001 Rev=1
> > > > Len=010 <?>
> > > > Capabilities: [150] Advanced Error Reporting
> > > > Kernel driver in use: radeon
> > > >
> > > > 01:00.1 Audio device: Advanced Micro Devices, Inc. [AMD/ATI] Turks
> > > > HDMI Audio [Radeon HD 6500/6600 / 6700M Series]
> > > > Subsystem: Dell Turks HDMI Audio [Radeon HD 6500/6600 / 6700M Series]
> > > > Flags: bus master, fast devsel, latency 0, IRQ 98
> > > > Memory at 300940000 (64-bit, non-prefetchable) [size=16K]
> > > > Capabilities: [50] Power Management version 3
> > > > Capabilities: [58] Express Legacy Endpoint, MSI 00
> > > > Capabilities: [a0] MSI: Enable+ Count=1/1 Maskable- 64bit+
> > > > Capabilities: [100] Vendor Specific Information: ID=0001 Rev=1
> > > > Len=010 <?>
> > > > Capabilities: [150] Advanced Error Reporting
> > > > Kernel driver in use: snd_hda_intel
> > > >
> > > > This will obviously clobber registers during writes.
> > >
> > > I don't follow. Which writes will clobber which registers, and how is
> > > it obvious?
> >
> > Writing a 64 bit word into a 32 bit register will either clobber the
> > next higher 32 bit register.
> > Quoting Christian:
> > "When you program a 32bit BAR as 64bit you overwrite the register behind
> > the BAR address with the upper 32bits of the 64bit address value.
> > So even if the allocation fits into 32bits, the extra register write
> > will certainly put your device into a banana state."
> >
> > https://lists.freedesktop.org/archives/amd-gfx/2021-May/064232.html
> >
>
> I seriously doubt that this is what is going on here.
>
> lspci -x will give you the bare BAR values - I suspect that those are
> probably fine.
lspci -x
00:00.0 PCI bridge: Fuzhou Rockchip Electronics Co., Ltd Device 3566 (rev 01)
00: 87 1d 66 35 07 05 10 40 01 00 04 06 00 00 01 00
10: 00 00 00 00 00 00 00 00 00 01 ff 00 10 10 00 20
20: 00 10 00 10 01 00 f1 0f 00 00 00 00 00 00 00 00
30: 00 00 00 00 40 00 00 00 00 00 00 00 5f 01 02 00
01:00.0 VGA compatible controller: Advanced Micro Devices, Inc.
[AMD/ATI] Turks PRO [Radeon HD 7570]
00: 02 10 5d 67 07 00 10 20 00 00 00 03 00 00 80 00
10: 0c 00 00 00 00 00 00 00 04 00 00 10 00 00 00 00
20: 01 10 70 3f 00 00 00 00 00 00 00 00 28 10 20 2b
30: 00 00 02 10 50 00 00 00 00 00 00 00 5f 01 00 00
01:00.1 Audio device: Advanced Micro Devices, Inc. [AMD/ATI] Turks
HDMI Audio [Radeon HD 6500/6600 / 6700M Series]
00: 02 10 90 aa 06 00 10 20 00 00 03 04 00 00 80 00
10: 04 00 04 10 00 00 00 00 00 00 00 00 00 00 00 00
20: 00 00 00 00 00 00 00 00 00 00 00 00 28 10 90 aa
30: 00 00 00 00 50 00 00 00 00 00 00 00 ff 02 00 00
>
>
> > >
> > > > Also, if <0x82000000> (32 bit) is changed to <0x83000000> (64 bit),
> > > > most of the allocations for the dGPU fail due to no valid regions
> > > > available.
> > > >
> > >
> > > But wasn't the original problem that the resource window was 64-bit to
> > > begin with? Are you sure we are talking about the same problem here?
> >
> > The rk3399 in the original report has a 32MB memory window in the
> > upper end of the 4GB range.
> > The rk356x has a similar layout, or it can use a 1GB window available
> > at <0x3 0x00000000>.
> > Rockchip's default windows are defined as 64bit.
> >
> > The rk3399 doesn't have enough space to reasonably define two windows,
> > one 32bit, one 64bit, to work around an allocation bug.
> > These are the defined regions in the rk3399:
> > ranges = <0x83000000 0x0 0xfa000000 0x0 0xfa000000 0x0 0x1e00000>,
> > <0x81000000 0x0 0xfbe00000 0x0 0xfbe00000 0x0 0x100000>;
> >
>
> All you really need is a 32-bit non-prefetchable resource window: any
> BAR can be allocated from that. A 64-bit BAR can carry a 32-bit number
> (just add zeroes at the top), and a prefetchable BAR can happily live
> in a non-prefetchable window, with a theoretical performance impact if
> the OS actually does use different memory attributes for the
> prefetchable window (but I don't think Linux ever handles it this way)
So is the IO range necessary as well or will it be automatically
allocated as well?
>
>
> >
> > >
> > >
> > > > >
> > > > > > I am happy to put something together once I understand the preferred way
> > > > > > to go about it.
> > > > > >
> > > > > > Thanks,
> > > > > > Punit
> > > > > >
> > > > > > [...]
> > > > > >
> > > > >
> > > > > _______________________________________________
> > > > > Linux-rockchip mailing list
> > > > > Linux-rockchip@lists.infradead.org
> > > > > http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [BUG] rockpro64: PCI BAR reassignment broken by commit 9d57e61bf723 ("of/pci: Add IORESOURCE_MEM_64 to resource flags for 64-bit memory addresses")
2021-05-25 17:01 ` Peter Geis
@ 2021-05-25 17:18 ` Ard Biesheuvel
2021-05-25 17:37 ` Peter Geis
2021-05-26 13:55 ` Christian König
2021-05-25 17:25 ` Robin Murphy
1 sibling, 2 replies; 28+ messages in thread
From: Ard Biesheuvel @ 2021-05-25 17:18 UTC (permalink / raw)
To: Peter Geis
Cc: Punit Agrawal, Robin Murphy, Alexandru Elisei,
Linux Kernel Mailing List, open list:ARM/Rockchip SoC...,
arm-mail-list, Heiko Stuebner, Leonardo Bras, Rob Herring, PCI,
Christian König
On Tue, 25 May 2021 at 19:01, Peter Geis <pgwipeout@gmail.com> wrote:
>
> On Tue, May 25, 2021 at 12:44 PM Ard Biesheuvel <ardb@kernel.org> wrote:
> >
> > On Tue, 25 May 2021 at 18:23, Peter Geis <pgwipeout@gmail.com> wrote:
> > >
> > > On Tue, May 25, 2021 at 11:55 AM Ard Biesheuvel <ardb@kernel.org> wrote:
> > > >
> > > > On Tue, 25 May 2021 at 17:34, Peter Geis <pgwipeout@gmail.com> wrote:
> > > > >
> > > > > On Tue, May 25, 2021 at 9:57 AM Ard Biesheuvel <ardb@kernel.org> wrote:
> > > > > >
> > > > > > On Tue, 25 May 2021 at 15:42, Punit Agrawal <punitagrawal@gmail.com> wrote:
> > > > > > >
> > > > > > > Hi Ard,
> > > > > > >
> > > > > > > Ard Biesheuvel <ardb@kernel.org> writes:
> > > > > > >
> > > > > > > > On Sun, 23 May 2021 at 13:06, Punit Agrawal <punitagrawal@gmail.com> wrote:
> > > > > > > >>
> > > > > > > >> Robin Murphy <robin.murphy@arm.com> writes:
> > > > > > > >>
> > > > > > > >> > [ +linux-pci for visibility ]
> > > > > > > >> >
> > > > > > > >> > On 2021-05-18 10:09, Alexandru Elisei wrote:
> > > > > > > >> >> After doing a git bisect I was able to trace the following error when booting my
> > > > > > > >> >> rockpro64 v2 (rk3399 SoC) with a PCIE NVME expansion card:
> > > > > > > >> >> [..]
> > > > > > > >> >> [ 0.305183] rockchip-pcie f8000000.pcie: host bridge /pcie@f8000000 ranges:
> > > > > > > >> >> [ 0.305248] rockchip-pcie f8000000.pcie: MEM 0x00fa000000..0x00fbdfffff ->
> > > > > > > >> >> 0x00fa000000
> > > > > > > >> >> [ 0.305285] rockchip-pcie f8000000.pcie: IO 0x00fbe00000..0x00fbefffff ->
> > > > > > > >> >> 0x00fbe00000
> > > > > > > >> >> [ 0.306201] rockchip-pcie f8000000.pcie: supply vpcie1v8 not found, using dummy
> > > > > > > >> >> regulator
> > > > > > > >> >> [ 0.306334] rockchip-pcie f8000000.pcie: supply vpcie0v9 not found, using dummy
> > > > > > > >> >> regulator
> > > > > > > >> >> [ 0.373705] rockchip-pcie f8000000.pcie: PCI host bridge to bus 0000:00
> > > > > > > >> >> [ 0.373730] pci_bus 0000:00: root bus resource [bus 00-1f]
> > > > > > > >> >> [ 0.373751] pci_bus 0000:00: root bus resource [mem 0xfa000000-0xfbdfffff 64bit]
> > > > > > > >> >> [ 0.373777] pci_bus 0000:00: root bus resource [io 0x0000-0xfffff] (bus
> > > > > > > >> >> address [0xfbe00000-0xfbefffff])
> > > > > > > >> >> [ 0.373839] pci 0000:00:00.0: [1d87:0100] type 01 class 0x060400
> > > > > > > >> >> [ 0.373973] pci 0000:00:00.0: supports D1
> > > > > > > >> >> [ 0.373992] pci 0000:00:00.0: PME# supported from D0 D1 D3hot
> > > > > > > >> >> [ 0.378518] pci 0000:00:00.0: bridge configuration invalid ([bus 00-00]),
> > > > > > > >> >> reconfiguring
> > > > > > > >> >> [ 0.378765] pci 0000:01:00.0: [144d:a808] type 00 class 0x010802
> > > > > > > >> >> [ 0.378869] pci 0000:01:00.0: reg 0x10: [mem 0x00000000-0x00003fff 64bit]
> > > > > > > >> >> [ 0.379051] pci 0000:01:00.0: Max Payload Size set to 256 (was 128, max 256)
> > > > > > > >> >> [ 0.379661] pci 0000:01:00.0: 8.000 Gb/s available PCIe bandwidth, limited by
> > > > > > > >> >> 2.5 GT/s PCIe x4 link at 0000:00:00.0 (capable of 31.504 Gb/s with 8.0 GT/s PCIe
> > > > > > > >> >> x4 link)
> > > > > > > >> >> [ 0.393269] pci_bus 0000:01: busn_res: [bus 01-1f] end is updated to 01
> > > > > > > >> >> [ 0.393311] pci 0000:00:00.0: BAR 14: no space for [mem size 0x00100000]
> > > > > > > >> >> [ 0.393333] pci 0000:00:00.0: BAR 14: failed to assign [mem size 0x00100000]
> > > > > > > >> >> [ 0.393356] pci 0000:01:00.0: BAR 0: no space for [mem size 0x00004000 64bit]
> > > > > > > >> >> [ 0.393375] pci 0000:01:00.0: BAR 0: failed to assign [mem size 0x00004000 64bit]
> > > > > > > >> >> [ 0.393397] pci 0000:00:00.0: PCI bridge to [bus 01]
> > > > > > > >> >> [ 0.393839] pcieport 0000:00:00.0: PME: Signaling with IRQ 78
> > > > > > > >> >> [ 0.394165] pcieport 0000:00:00.0: AER: enabled with IRQ 78
> > > > > > > >> >> [..]
> > > > > > > >> >> to the commit 9d57e61bf723 ("of/pci: Add IORESOURCE_MEM_64 to
> > > > > > > >> >> resource flags for
> > > > > > > >> >> 64-bit memory addresses").
> > > > > > > >> >
> > > > > > > >> > FWFW, my hunch is that the host bridge advertising no 32-bit memory
> > > > > > > >> > resource, only only a single 64-bit non-prefetchable one (even though
> > > > > > > >> > it's entirely below 4GB) might be a bit weird and tripping something
> > > > > > > >> > up in the resource assignment code. It certainly seems like the thing
> > > > > > > >> > most directly related to the offending commit.
> > > > > > > >> >
> > > > > > > >> > I'd be tempted to try fiddling with that in the DT (i.e. changing
> > > > > > > >> > 0x83000000 to 0x82000000 in the PCIe node's "ranges" property) to see
> > > > > > > >> > if it makes any difference. Note that even if it helps, though, I
> > > > > > > >> > don't know whether that's the correct fix or just a bodge around a
> > > > > > > >> > corner-case bug somewhere in the resource code.
> > > > > > > >>
> > > > > > > >> From digging into this further the failure seems to be due to a mismatch
> > > > > > > >> of flags when allocating resources in pci_bus_alloc_from_region() -
> > > > > > > >>
> > > > > > > >> if ((res->flags ^ r->flags) & type_mask)
> > > > > > > >> continue;
> > > > > > > >>
> > > > > > > >> Though I am also not sure why the failure is only being reported on
> > > > > > > >> RK3399 - does a single 64-bit window have anything to do with it?
> > > > > > > >>
> > > > > > > >
> > > > > > > > The NVMe in the example exposes a single 64-bit non-prefetchable BAR.
> > > > > > > > Such BARs can not be allocated in a prefetchable host bridge window
> > > > > > > > (unlike the converse, i.e., allocating a prefetchable BAR in a
> > > > > > > > non-prefetchable host bridge window is fine)
> > > > > > > >
> > > > > > > > 64-bit non-prefetchable host bridge windows cannot be forwarded by PCI
> > > > > > > > to PCI bridges, they simply lack the BAR registers to describe them.
> > > > > > > > Therefore, non-prefetchable endpoint BARs (even 64-bit ones) need to
> > > > > > > > be carved out of a host bridge's non-prefetchable 32-bit window if
> > > > > > > > they need to pass through a bridge.
> > > > > > >
> > > > > > > Thank you for the explanation. I also looked at the PCI-to-PCI Bridge
> > > > > > > spec to understand where some of the limitations are coming from.
> > > > > > >
> > > > > > > > So the error seems to be here that the host bridge's 32-bit
> > > > > > > > non-prefetchable window has the 64-bit attribute set, even though it
> > > > > > > > resides below 4 GB entirely. I suppose that the resource allocation
> > > > > > > > could be made more forgiving (and it was in the past, before commit
> > > > > > > > 9d57e61bf723 was applied). However, I would strongly recommend not
> > > > > > > > deviating from common practice, and just describe the 32-bit
> > > > > > > > addressable non-prefetchable resource window as such.
> > > > > > >
> > > > > > > IIUC, the host bridge's configuration (64-bit on non-prefetchable
> > > > > > > window) is based on what the hardware advertises.
> > > > > > >
> > > > > >
> > > > > > What do you mean by 'what the hardware advertises'? The host bridge is
> > > > > > apparently configured to decode a 32-bit addressable window as MMIO,
> > > > > > and the question is why this window has the 64-bit attribute set in
> > > > > > the DT description.
> > > > > >
> > > > > > > Can you elaborate on what you have in mind to correct the
> > > > > > > non-prefetchable resource window? Are you thinking of adding a quirk
> > > > > > > somewhere to address this?
> > > > > > >
> > > > > >
> > > > > > No. Just fix the DT.
> > > > >
> > > > > Good Morning,
> > > > >
> > > > > I believe Robin is correct that there is more to this.
> > > > > While attempting to work out why dGPUs won't work with the rk356x
> > > > > series PCIe controllers, Christian König from the amd-gpu driver
> > > > > mailing list noticed the gpu was incorrectly allocated a 64bit
> > > > > non-prefetchable BAR which should instead be a 32 non-prefetchable
> > > > > BAR.
> > > > >
> > > >
> > > > This is due to the translation. For some reason, lspci translates the
> > > > BAR values to CPU addresses, but the PCI side addresses are within
> > > > 32-bits.
> > >
> > > The kernel log reports the same thing:
> > > [ 6.662141] pci 0000:01:00.0: reg 0x10: [mem 0x00000000-0x0fffffff
> > > 64bit pref]
> > > [ 6.662963] pci 0000:01:00.0: reg 0x18: [mem 0x00000000-0x0001ffff 64bit]
> > >
> > > You are saying this is a display only issue?
> > >
> >
> > Yes. What do the 'root bus resource' log lines say for these regions?
> > Those should give you both the CPU address as well as the bus address.
>
> [ 6.673497] pci_bus 0000:00: root bus resource [io 0x0000-0xfffff]
> (bus address [0x3f700000-0x3f7fffff])
> [ 6.674642] pci_bus 0000:00: root bus resource [mem
> 0x300000000-0x33f6fffff] (bus address [0x00000000-0x3f6fffff])
>
> I tweaked the original Rockchip values to place the non-prefetchable
> memory first with the configuration and io later in this boot.
>
> >
> >
> > > >
> > > > Are you sure the amdgpu driver can even deal with non-1:1 host bridges?
> > >
> > > I cannot answer this as I'm not an amdgpu dev.
> > >
> > > >
> > > > > The ranges currently set are:
> > > > > ranges = <0x81000000 0x0 0x00800000 0x3 0x00800000 0x0 0x00100000
> > > > > 0x82000000 0x0 0x00900000 0x3 0x00900000 0x0 0x3f700000>;
> > > > >
> > > >
> > > > So you have two ranges here.
> > >
> > > The IO and PCI memory ranges.
> > >
> > > There is a third range, the configuration range, which is defined in
> > > the reg block:
> > > <0x3 0x00000000 0x0 0x800000>
> > > All three are shared in the same 1GB window on the rk356x.
> > >
> >
> > But the reg block is not a resource window, it is a configuration
> > range to program the host bridge.
> >
> > > https://elixir.bootlin.com/linux/v5.13-rc3/source/Documentation/devicetree/bindings/pci/designware-pcie.txt#L12
> > >
> > > >
> > > > > but the final allocation was:
> > > > >
> > > > > lspci -v
> > > > > 00:00.0 PCI bridge: Fuzhou Rockchip Electronics Co., Ltd Device 3566
> > > > > (rev 01) (prog-if 00 [Normal decode])
> > > > > Flags: bus master, fast devsel, latency 0, IRQ 96
> > > > > Bus: primary=00, secondary=01, subordinate=ff, sec-latency=0
> > > > > I/O behind bridge: 00001000-00001fff [size=4K]
> > > > > Memory behind bridge: 00900000-009fffff [size=1M]
> > > > > Prefetchable memory behind bridge:
> > > > > 0000000010000000-000000001fffffff [size=256M]
> > > >
> > > > But the host bridge/root port decodes two disjoint regions??
> > > >
> > > > > Expansion ROM at 300a00000 [virtual] [disabled] [size=64K]
> > > > > Capabilities: [40] Power Management version 3
> > > > > Capabilities: [50] MSI: Enable+ Count=1/32 Maskable- 64bit+
> > > > > Capabilities: [70] Express Root Port (Slot-), MSI 00
> > > > > Capabilities: [b0] MSI-X: Enable- Count=1 Masked-
> > > > > Capabilities: [100] Advanced Error Reporting
> > > > > Capabilities: [148] Secondary PCI Express
> > > > > Capabilities: [160] L1 PM Substates
> > > > > Capabilities: [170] Vendor Specific Information: ID=0002 Rev=4
> > > > > Len=100 <?>
> > > > > Kernel driver in use: pcieport
> > > > >
> > > > > 01:00.0 VGA compatible controller: Advanced Micro Devices, Inc.
> > > > > [AMD/ATI] Turks PRO [Radeon HD 7570] (prog-if 00 [VGA controller])
> > > > > Subsystem: Dell Turks PRO [Radeon HD 7570]
> > > > > Flags: bus master, fast devsel, latency 0, IRQ 95
> > > > > Memory at 310000000 (64-bit, prefetchable) [size=256M]
> > > > > Memory at 300900000 (64-bit, non-prefetchable) [size=128K]
> > > > > I/O ports at 1000 [size=256]
> > > > > Expansion ROM at 300920000 [disabled] [size=128K]
> > > > > Capabilities: [50] Power Management version 3
> > > > > Capabilities: [58] Express Legacy Endpoint, MSI 00
> > > > > Capabilities: [a0] MSI: Enable- Count=1/1 Maskable- 64bit+
> > > > > Capabilities: [100] Vendor Specific Information: ID=0001 Rev=1
> > > > > Len=010 <?>
> > > > > Capabilities: [150] Advanced Error Reporting
> > > > > Kernel driver in use: radeon
> > > > >
> > > > > 01:00.1 Audio device: Advanced Micro Devices, Inc. [AMD/ATI] Turks
> > > > > HDMI Audio [Radeon HD 6500/6600 / 6700M Series]
> > > > > Subsystem: Dell Turks HDMI Audio [Radeon HD 6500/6600 / 6700M Series]
> > > > > Flags: bus master, fast devsel, latency 0, IRQ 98
> > > > > Memory at 300940000 (64-bit, non-prefetchable) [size=16K]
> > > > > Capabilities: [50] Power Management version 3
> > > > > Capabilities: [58] Express Legacy Endpoint, MSI 00
> > > > > Capabilities: [a0] MSI: Enable+ Count=1/1 Maskable- 64bit+
> > > > > Capabilities: [100] Vendor Specific Information: ID=0001 Rev=1
> > > > > Len=010 <?>
> > > > > Capabilities: [150] Advanced Error Reporting
> > > > > Kernel driver in use: snd_hda_intel
> > > > >
> > > > > This will obviously clobber registers during writes.
> > > >
> > > > I don't follow. Which writes will clobber which registers, and how is
> > > > it obvious?
> > >
> > > Writing a 64 bit word into a 32 bit register will either clobber the
> > > next higher 32 bit register.
> > > Quoting Christian:
> > > "When you program a 32bit BAR as 64bit you overwrite the register behind
> > > the BAR address with the upper 32bits of the 64bit address value.
> > > So even if the allocation fits into 32bits, the extra register write
> > > will certainly put your device into a banana state."
> > >
> > > https://lists.freedesktop.org/archives/amd-gfx/2021-May/064232.html
> > >
> >
> > I seriously doubt that this is what is going on here.
> >
> > lspci -x will give you the bare BAR values - I suspect that those are
> > probably fine.
>
> lspci -x
> 00:00.0 PCI bridge: Fuzhou Rockchip Electronics Co., Ltd Device 3566 (rev 01)
> 00: 87 1d 66 35 07 05 10 40 01 00 04 06 00 00 01 00
> 10: 00 00 00 00 00 00 00 00 00 01 ff 00 10 10 00 20
> 20: 00 10 00 10 01 00 f1 0f 00 00 00 00 00 00 00 00
> 30: 00 00 00 00 40 00 00 00 00 00 00 00 5f 01 02 00
>
> 01:00.0 VGA compatible controller: Advanced Micro Devices, Inc.
> [AMD/ATI] Turks PRO [Radeon HD 7570]
> 00: 02 10 5d 67 07 00 10 20 00 00 00 03 00 00 80 00
> 10: 0c 00 00 00 00 00 00 00
This is a 64-bit prefetchable BAR programmed with bus address 0x0
> 04 00 00 10 00 00 00 00
This is a 64-bit non-prefetchable BAR programmed with bus address 0x1000_0000
(https://en.wikipedia.org/wiki/PCI_configuration_space describes the
meaning of the low order BAR bits)
> 20: 01 10 70 3f 00 00 00 00
This looks odd. This looks like a 32-bit MMIO address poked into a I/O BAR.
> 00 00 00 00 28 10 20 2b
> 30: 00 00 02 10 50 00 00 00 00 00 00 00 5f 01 00 00
>
> 01:00.1 Audio device: Advanced Micro Devices, Inc. [AMD/ATI] Turks
> HDMI Audio [Radeon HD 6500/6600 / 6700M Series]
> 00: 02 10 90 aa 06 00 10 20 00 00 03 04 00 00 80 00
> 10: 04 00 04 10 00 00 00 00 00 00 00 00 00 00 00 00
> 20: 00 00 00 00 00 00 00 00 00 00 00 00 28 10 90 aa
> 30: 00 00 00 00 50 00 00 00 00 00 00 00 ff 02 00 00
>
> >
> >
> > > >
> > > > > Also, if <0x82000000> (32 bit) is changed to <0x83000000> (64 bit),
> > > > > most of the allocations for the dGPU fail due to no valid regions
> > > > > available.
> > > > >
> > > >
> > > > But wasn't the original problem that the resource window was 64-bit to
> > > > begin with? Are you sure we are talking about the same problem here?
> > >
> > > The rk3399 in the original report has a 32MB memory window in the
> > > upper end of the 4GB range.
> > > The rk356x has a similar layout, or it can use a 1GB window available
> > > at <0x3 0x00000000>.
> > > Rockchip's default windows are defined as 64bit.
> > >
> > > The rk3399 doesn't have enough space to reasonably define two windows,
> > > one 32bit, one 64bit, to work around an allocation bug.
> > > These are the defined regions in the rk3399:
> > > ranges = <0x83000000 0x0 0xfa000000 0x0 0xfa000000 0x0 0x1e00000>,
> > > <0x81000000 0x0 0xfbe00000 0x0 0xfbe00000 0x0 0x100000>;
> > >
> >
> > All you really need is a 32-bit non-prefetchable resource window: any
> > BAR can be allocated from that. A 64-bit BAR can carry a 32-bit number
> > (just add zeroes at the top), and a prefetchable BAR can happily live
> > in a non-prefetchable window, with a theoretical performance impact if
> > the OS actually does use different memory attributes for the
> > prefetchable window (but I don't think Linux ever handles it this way)
>
> So is the IO range necessary as well or will it be automatically
> allocated as well?
>
You need one I/O range and one 32-bit non-prefetchable MMIO window at
the very least, even though the I/O range is rarely used, even by
endpoints that expose I/O BARs.
The translation is tricky to get right, and confuses some drivers, so
it is better avoided if possible. If you do need translation, make
sure to translate in the right direction.
> >
> >
> > >
> > > >
> > > >
> > > > > >
> > > > > > > I am happy to put something together once I understand the preferred way
> > > > > > > to go about it.
> > > > > > >
> > > > > > > Thanks,
> > > > > > > Punit
> > > > > > >
> > > > > > > [...]
> > > > > > >
> > > > > >
> > > > > > _______________________________________________
> > > > > > Linux-rockchip mailing list
> > > > > > Linux-rockchip@lists.infradead.org
> > > > > > http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [BUG] rockpro64: PCI BAR reassignment broken by commit 9d57e61bf723 ("of/pci: Add IORESOURCE_MEM_64 to resource flags for 64-bit memory addresses")
2021-05-25 17:18 ` Ard Biesheuvel
@ 2021-05-25 17:37 ` Peter Geis
2021-05-26 13:55 ` Christian König
1 sibling, 0 replies; 28+ messages in thread
From: Peter Geis @ 2021-05-25 17:37 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: Punit Agrawal, Robin Murphy, Alexandru Elisei,
Linux Kernel Mailing List, open list:ARM/Rockchip SoC...,
arm-mail-list, Heiko Stuebner, Leonardo Bras, Rob Herring, PCI,
Christian König
On Tue, May 25, 2021 at 1:18 PM Ard Biesheuvel <ardb@kernel.org> wrote:
>
> On Tue, 25 May 2021 at 19:01, Peter Geis <pgwipeout@gmail.com> wrote:
> >
> > On Tue, May 25, 2021 at 12:44 PM Ard Biesheuvel <ardb@kernel.org> wrote:
> > >
> > > On Tue, 25 May 2021 at 18:23, Peter Geis <pgwipeout@gmail.com> wrote:
> > > >
> > > > On Tue, May 25, 2021 at 11:55 AM Ard Biesheuvel <ardb@kernel.org> wrote:
> > > > >
> > > > > On Tue, 25 May 2021 at 17:34, Peter Geis <pgwipeout@gmail.com> wrote:
> > > > > >
> > > > > > On Tue, May 25, 2021 at 9:57 AM Ard Biesheuvel <ardb@kernel.org> wrote:
> > > > > > >
> > > > > > > On Tue, 25 May 2021 at 15:42, Punit Agrawal <punitagrawal@gmail.com> wrote:
> > > > > > > >
> > > > > > > > Hi Ard,
> > > > > > > >
> > > > > > > > Ard Biesheuvel <ardb@kernel.org> writes:
> > > > > > > >
> > > > > > > > > On Sun, 23 May 2021 at 13:06, Punit Agrawal <punitagrawal@gmail.com> wrote:
> > > > > > > > >>
> > > > > > > > >> Robin Murphy <robin.murphy@arm.com> writes:
> > > > > > > > >>
> > > > > > > > >> > [ +linux-pci for visibility ]
> > > > > > > > >> >
> > > > > > > > >> > On 2021-05-18 10:09, Alexandru Elisei wrote:
> > > > > > > > >> >> After doing a git bisect I was able to trace the following error when booting my
> > > > > > > > >> >> rockpro64 v2 (rk3399 SoC) with a PCIE NVME expansion card:
> > > > > > > > >> >> [..]
> > > > > > > > >> >> [ 0.305183] rockchip-pcie f8000000.pcie: host bridge /pcie@f8000000 ranges:
> > > > > > > > >> >> [ 0.305248] rockchip-pcie f8000000.pcie: MEM 0x00fa000000..0x00fbdfffff ->
> > > > > > > > >> >> 0x00fa000000
> > > > > > > > >> >> [ 0.305285] rockchip-pcie f8000000.pcie: IO 0x00fbe00000..0x00fbefffff ->
> > > > > > > > >> >> 0x00fbe00000
> > > > > > > > >> >> [ 0.306201] rockchip-pcie f8000000.pcie: supply vpcie1v8 not found, using dummy
> > > > > > > > >> >> regulator
> > > > > > > > >> >> [ 0.306334] rockchip-pcie f8000000.pcie: supply vpcie0v9 not found, using dummy
> > > > > > > > >> >> regulator
> > > > > > > > >> >> [ 0.373705] rockchip-pcie f8000000.pcie: PCI host bridge to bus 0000:00
> > > > > > > > >> >> [ 0.373730] pci_bus 0000:00: root bus resource [bus 00-1f]
> > > > > > > > >> >> [ 0.373751] pci_bus 0000:00: root bus resource [mem 0xfa000000-0xfbdfffff 64bit]
> > > > > > > > >> >> [ 0.373777] pci_bus 0000:00: root bus resource [io 0x0000-0xfffff] (bus
> > > > > > > > >> >> address [0xfbe00000-0xfbefffff])
> > > > > > > > >> >> [ 0.373839] pci 0000:00:00.0: [1d87:0100] type 01 class 0x060400
> > > > > > > > >> >> [ 0.373973] pci 0000:00:00.0: supports D1
> > > > > > > > >> >> [ 0.373992] pci 0000:00:00.0: PME# supported from D0 D1 D3hot
> > > > > > > > >> >> [ 0.378518] pci 0000:00:00.0: bridge configuration invalid ([bus 00-00]),
> > > > > > > > >> >> reconfiguring
> > > > > > > > >> >> [ 0.378765] pci 0000:01:00.0: [144d:a808] type 00 class 0x010802
> > > > > > > > >> >> [ 0.378869] pci 0000:01:00.0: reg 0x10: [mem 0x00000000-0x00003fff 64bit]
> > > > > > > > >> >> [ 0.379051] pci 0000:01:00.0: Max Payload Size set to 256 (was 128, max 256)
> > > > > > > > >> >> [ 0.379661] pci 0000:01:00.0: 8.000 Gb/s available PCIe bandwidth, limited by
> > > > > > > > >> >> 2.5 GT/s PCIe x4 link at 0000:00:00.0 (capable of 31.504 Gb/s with 8.0 GT/s PCIe
> > > > > > > > >> >> x4 link)
> > > > > > > > >> >> [ 0.393269] pci_bus 0000:01: busn_res: [bus 01-1f] end is updated to 01
> > > > > > > > >> >> [ 0.393311] pci 0000:00:00.0: BAR 14: no space for [mem size 0x00100000]
> > > > > > > > >> >> [ 0.393333] pci 0000:00:00.0: BAR 14: failed to assign [mem size 0x00100000]
> > > > > > > > >> >> [ 0.393356] pci 0000:01:00.0: BAR 0: no space for [mem size 0x00004000 64bit]
> > > > > > > > >> >> [ 0.393375] pci 0000:01:00.0: BAR 0: failed to assign [mem size 0x00004000 64bit]
> > > > > > > > >> >> [ 0.393397] pci 0000:00:00.0: PCI bridge to [bus 01]
> > > > > > > > >> >> [ 0.393839] pcieport 0000:00:00.0: PME: Signaling with IRQ 78
> > > > > > > > >> >> [ 0.394165] pcieport 0000:00:00.0: AER: enabled with IRQ 78
> > > > > > > > >> >> [..]
> > > > > > > > >> >> to the commit 9d57e61bf723 ("of/pci: Add IORESOURCE_MEM_64 to
> > > > > > > > >> >> resource flags for
> > > > > > > > >> >> 64-bit memory addresses").
> > > > > > > > >> >
> > > > > > > > >> > FWFW, my hunch is that the host bridge advertising no 32-bit memory
> > > > > > > > >> > resource, only only a single 64-bit non-prefetchable one (even though
> > > > > > > > >> > it's entirely below 4GB) might be a bit weird and tripping something
> > > > > > > > >> > up in the resource assignment code. It certainly seems like the thing
> > > > > > > > >> > most directly related to the offending commit.
> > > > > > > > >> >
> > > > > > > > >> > I'd be tempted to try fiddling with that in the DT (i.e. changing
> > > > > > > > >> > 0x83000000 to 0x82000000 in the PCIe node's "ranges" property) to see
> > > > > > > > >> > if it makes any difference. Note that even if it helps, though, I
> > > > > > > > >> > don't know whether that's the correct fix or just a bodge around a
> > > > > > > > >> > corner-case bug somewhere in the resource code.
> > > > > > > > >>
> > > > > > > > >> From digging into this further the failure seems to be due to a mismatch
> > > > > > > > >> of flags when allocating resources in pci_bus_alloc_from_region() -
> > > > > > > > >>
> > > > > > > > >> if ((res->flags ^ r->flags) & type_mask)
> > > > > > > > >> continue;
> > > > > > > > >>
> > > > > > > > >> Though I am also not sure why the failure is only being reported on
> > > > > > > > >> RK3399 - does a single 64-bit window have anything to do with it?
> > > > > > > > >>
> > > > > > > > >
> > > > > > > > > The NVMe in the example exposes a single 64-bit non-prefetchable BAR.
> > > > > > > > > Such BARs can not be allocated in a prefetchable host bridge window
> > > > > > > > > (unlike the converse, i.e., allocating a prefetchable BAR in a
> > > > > > > > > non-prefetchable host bridge window is fine)
> > > > > > > > >
> > > > > > > > > 64-bit non-prefetchable host bridge windows cannot be forwarded by PCI
> > > > > > > > > to PCI bridges, they simply lack the BAR registers to describe them.
> > > > > > > > > Therefore, non-prefetchable endpoint BARs (even 64-bit ones) need to
> > > > > > > > > be carved out of a host bridge's non-prefetchable 32-bit window if
> > > > > > > > > they need to pass through a bridge.
> > > > > > > >
> > > > > > > > Thank you for the explanation. I also looked at the PCI-to-PCI Bridge
> > > > > > > > spec to understand where some of the limitations are coming from.
> > > > > > > >
> > > > > > > > > So the error seems to be here that the host bridge's 32-bit
> > > > > > > > > non-prefetchable window has the 64-bit attribute set, even though it
> > > > > > > > > resides below 4 GB entirely. I suppose that the resource allocation
> > > > > > > > > could be made more forgiving (and it was in the past, before commit
> > > > > > > > > 9d57e61bf723 was applied). However, I would strongly recommend not
> > > > > > > > > deviating from common practice, and just describe the 32-bit
> > > > > > > > > addressable non-prefetchable resource window as such.
> > > > > > > >
> > > > > > > > IIUC, the host bridge's configuration (64-bit on non-prefetchable
> > > > > > > > window) is based on what the hardware advertises.
> > > > > > > >
> > > > > > >
> > > > > > > What do you mean by 'what the hardware advertises'? The host bridge is
> > > > > > > apparently configured to decode a 32-bit addressable window as MMIO,
> > > > > > > and the question is why this window has the 64-bit attribute set in
> > > > > > > the DT description.
> > > > > > >
> > > > > > > > Can you elaborate on what you have in mind to correct the
> > > > > > > > non-prefetchable resource window? Are you thinking of adding a quirk
> > > > > > > > somewhere to address this?
> > > > > > > >
> > > > > > >
> > > > > > > No. Just fix the DT.
> > > > > >
> > > > > > Good Morning,
> > > > > >
> > > > > > I believe Robin is correct that there is more to this.
> > > > > > While attempting to work out why dGPUs won't work with the rk356x
> > > > > > series PCIe controllers, Christian König from the amd-gpu driver
> > > > > > mailing list noticed the gpu was incorrectly allocated a 64bit
> > > > > > non-prefetchable BAR which should instead be a 32 non-prefetchable
> > > > > > BAR.
> > > > > >
> > > > >
> > > > > This is due to the translation. For some reason, lspci translates the
> > > > > BAR values to CPU addresses, but the PCI side addresses are within
> > > > > 32-bits.
> > > >
> > > > The kernel log reports the same thing:
> > > > [ 6.662141] pci 0000:01:00.0: reg 0x10: [mem 0x00000000-0x0fffffff
> > > > 64bit pref]
> > > > [ 6.662963] pci 0000:01:00.0: reg 0x18: [mem 0x00000000-0x0001ffff 64bit]
> > > >
> > > > You are saying this is a display only issue?
> > > >
> > >
> > > Yes. What do the 'root bus resource' log lines say for these regions?
> > > Those should give you both the CPU address as well as the bus address.
> >
> > [ 6.673497] pci_bus 0000:00: root bus resource [io 0x0000-0xfffff]
> > (bus address [0x3f700000-0x3f7fffff])
> > [ 6.674642] pci_bus 0000:00: root bus resource [mem
> > 0x300000000-0x33f6fffff] (bus address [0x00000000-0x3f6fffff])
> >
> > I tweaked the original Rockchip values to place the non-prefetchable
> > memory first with the configuration and io later in this boot.
> >
> > >
> > >
> > > > >
> > > > > Are you sure the amdgpu driver can even deal with non-1:1 host bridges?
> > > >
> > > > I cannot answer this as I'm not an amdgpu dev.
> > > >
> > > > >
> > > > > > The ranges currently set are:
> > > > > > ranges = <0x81000000 0x0 0x00800000 0x3 0x00800000 0x0 0x00100000
> > > > > > 0x82000000 0x0 0x00900000 0x3 0x00900000 0x0 0x3f700000>;
> > > > > >
> > > > >
> > > > > So you have two ranges here.
> > > >
> > > > The IO and PCI memory ranges.
> > > >
> > > > There is a third range, the configuration range, which is defined in
> > > > the reg block:
> > > > <0x3 0x00000000 0x0 0x800000>
> > > > All three are shared in the same 1GB window on the rk356x.
> > > >
> > >
> > > But the reg block is not a resource window, it is a configuration
> > > range to program the host bridge.
> > >
> > > > https://elixir.bootlin.com/linux/v5.13-rc3/source/Documentation/devicetree/bindings/pci/designware-pcie.txt#L12
> > > >
> > > > >
> > > > > > but the final allocation was:
> > > > > >
> > > > > > lspci -v
> > > > > > 00:00.0 PCI bridge: Fuzhou Rockchip Electronics Co., Ltd Device 3566
> > > > > > (rev 01) (prog-if 00 [Normal decode])
> > > > > > Flags: bus master, fast devsel, latency 0, IRQ 96
> > > > > > Bus: primary=00, secondary=01, subordinate=ff, sec-latency=0
> > > > > > I/O behind bridge: 00001000-00001fff [size=4K]
> > > > > > Memory behind bridge: 00900000-009fffff [size=1M]
> > > > > > Prefetchable memory behind bridge:
> > > > > > 0000000010000000-000000001fffffff [size=256M]
> > > > >
> > > > > But the host bridge/root port decodes two disjoint regions??
> > > > >
> > > > > > Expansion ROM at 300a00000 [virtual] [disabled] [size=64K]
> > > > > > Capabilities: [40] Power Management version 3
> > > > > > Capabilities: [50] MSI: Enable+ Count=1/32 Maskable- 64bit+
> > > > > > Capabilities: [70] Express Root Port (Slot-), MSI 00
> > > > > > Capabilities: [b0] MSI-X: Enable- Count=1 Masked-
> > > > > > Capabilities: [100] Advanced Error Reporting
> > > > > > Capabilities: [148] Secondary PCI Express
> > > > > > Capabilities: [160] L1 PM Substates
> > > > > > Capabilities: [170] Vendor Specific Information: ID=0002 Rev=4
> > > > > > Len=100 <?>
> > > > > > Kernel driver in use: pcieport
> > > > > >
> > > > > > 01:00.0 VGA compatible controller: Advanced Micro Devices, Inc.
> > > > > > [AMD/ATI] Turks PRO [Radeon HD 7570] (prog-if 00 [VGA controller])
> > > > > > Subsystem: Dell Turks PRO [Radeon HD 7570]
> > > > > > Flags: bus master, fast devsel, latency 0, IRQ 95
> > > > > > Memory at 310000000 (64-bit, prefetchable) [size=256M]
> > > > > > Memory at 300900000 (64-bit, non-prefetchable) [size=128K]
> > > > > > I/O ports at 1000 [size=256]
> > > > > > Expansion ROM at 300920000 [disabled] [size=128K]
> > > > > > Capabilities: [50] Power Management version 3
> > > > > > Capabilities: [58] Express Legacy Endpoint, MSI 00
> > > > > > Capabilities: [a0] MSI: Enable- Count=1/1 Maskable- 64bit+
> > > > > > Capabilities: [100] Vendor Specific Information: ID=0001 Rev=1
> > > > > > Len=010 <?>
> > > > > > Capabilities: [150] Advanced Error Reporting
> > > > > > Kernel driver in use: radeon
> > > > > >
> > > > > > 01:00.1 Audio device: Advanced Micro Devices, Inc. [AMD/ATI] Turks
> > > > > > HDMI Audio [Radeon HD 6500/6600 / 6700M Series]
> > > > > > Subsystem: Dell Turks HDMI Audio [Radeon HD 6500/6600 / 6700M Series]
> > > > > > Flags: bus master, fast devsel, latency 0, IRQ 98
> > > > > > Memory at 300940000 (64-bit, non-prefetchable) [size=16K]
> > > > > > Capabilities: [50] Power Management version 3
> > > > > > Capabilities: [58] Express Legacy Endpoint, MSI 00
> > > > > > Capabilities: [a0] MSI: Enable+ Count=1/1 Maskable- 64bit+
> > > > > > Capabilities: [100] Vendor Specific Information: ID=0001 Rev=1
> > > > > > Len=010 <?>
> > > > > > Capabilities: [150] Advanced Error Reporting
> > > > > > Kernel driver in use: snd_hda_intel
> > > > > >
> > > > > > This will obviously clobber registers during writes.
> > > > >
> > > > > I don't follow. Which writes will clobber which registers, and how is
> > > > > it obvious?
> > > >
> > > > Writing a 64 bit word into a 32 bit register will either clobber the
> > > > next higher 32 bit register.
> > > > Quoting Christian:
> > > > "When you program a 32bit BAR as 64bit you overwrite the register behind
> > > > the BAR address with the upper 32bits of the 64bit address value.
> > > > So even if the allocation fits into 32bits, the extra register write
> > > > will certainly put your device into a banana state."
> > > >
> > > > https://lists.freedesktop.org/archives/amd-gfx/2021-May/064232.html
> > > >
> > >
> > > I seriously doubt that this is what is going on here.
> > >
> > > lspci -x will give you the bare BAR values - I suspect that those are
> > > probably fine.
> >
> > lspci -x
> > 00:00.0 PCI bridge: Fuzhou Rockchip Electronics Co., Ltd Device 3566 (rev 01)
> > 00: 87 1d 66 35 07 05 10 40 01 00 04 06 00 00 01 00
> > 10: 00 00 00 00 00 00 00 00 00 01 ff 00 10 10 00 20
> > 20: 00 10 00 10 01 00 f1 0f 00 00 00 00 00 00 00 00
> > 30: 00 00 00 00 40 00 00 00 00 00 00 00 5f 01 02 00
> >
> > 01:00.0 VGA compatible controller: Advanced Micro Devices, Inc.
> > [AMD/ATI] Turks PRO [Radeon HD 7570]
> > 00: 02 10 5d 67 07 00 10 20 00 00 00 03 00 00 80 00
> > 10: 0c 00 00 00 00 00 00 00
>
> This is a 64-bit prefetchable BAR programmed with bus address 0x0
>
> > 04 00 00 10 00 00 00 00
>
> This is a 64-bit non-prefetchable BAR programmed with bus address 0x1000_0000
According to the AMD mailing list, this is a problem, since it should be 32-bit.
>
> (https://en.wikipedia.org/wiki/PCI_configuration_space describes the
> meaning of the low order BAR bits)
>
> > 20: 01 10 70 3f 00 00 00 00
>
> This looks odd. This looks like a 32-bit MMIO address poked into a I/O BAR.
Could this be because it's the only memory that is listed as 32-bit,
since the IO space is forced as such?
What are the implications of that?
>
>
> > 00 00 00 00 28 10 20 2b
> > 30: 00 00 02 10 50 00 00 00 00 00 00 00 5f 01 00 00
> >
> > 01:00.1 Audio device: Advanced Micro Devices, Inc. [AMD/ATI] Turks
> > HDMI Audio [Radeon HD 6500/6600 / 6700M Series]
> > 00: 02 10 90 aa 06 00 10 20 00 00 03 04 00 00 80 00
> > 10: 04 00 04 10 00 00 00 00 00 00 00 00 00 00 00 00
> > 20: 00 00 00 00 00 00 00 00 00 00 00 00 28 10 90 aa
> > 30: 00 00 00 00 50 00 00 00 00 00 00 00 ff 02 00 00
> >
> > >
> > >
> > > > >
> > > > > > Also, if <0x82000000> (32 bit) is changed to <0x83000000> (64 bit),
> > > > > > most of the allocations for the dGPU fail due to no valid regions
> > > > > > available.
> > > > > >
> > > > >
> > > > > But wasn't the original problem that the resource window was 64-bit to
> > > > > begin with? Are you sure we are talking about the same problem here?
> > > >
> > > > The rk3399 in the original report has a 32MB memory window in the
> > > > upper end of the 4GB range.
> > > > The rk356x has a similar layout, or it can use a 1GB window available
> > > > at <0x3 0x00000000>.
> > > > Rockchip's default windows are defined as 64bit.
> > > >
> > > > The rk3399 doesn't have enough space to reasonably define two windows,
> > > > one 32bit, one 64bit, to work around an allocation bug.
> > > > These are the defined regions in the rk3399:
> > > > ranges = <0x83000000 0x0 0xfa000000 0x0 0xfa000000 0x0 0x1e00000>,
> > > > <0x81000000 0x0 0xfbe00000 0x0 0xfbe00000 0x0 0x100000>;
> > > >
> > >
> > > All you really need is a 32-bit non-prefetchable resource window: any
> > > BAR can be allocated from that. A 64-bit BAR can carry a 32-bit number
> > > (just add zeroes at the top), and a prefetchable BAR can happily live
> > > in a non-prefetchable window, with a theoretical performance impact if
> > > the OS actually does use different memory attributes for the
> > > prefetchable window (but I don't think Linux ever handles it this way)
> >
> > So is the IO range necessary as well or will it be automatically
> > allocated as well?
> >
>
> You need one I/O range and one 32-bit non-prefetchable MMIO window at
> the very least, even though the I/O range is rarely used, even by
> endpoints that expose I/O BARs.
>
> The translation is tricky to get right, and confuses some drivers, so
> it is better avoided if possible. If you do need translation, make
> sure to translate in the right direction.
>
> > >
> > >
> > > >
> > > > >
> > > > >
> > > > > > >
> > > > > > > > I am happy to put something together once I understand the preferred way
> > > > > > > > to go about it.
> > > > > > > >
> > > > > > > > Thanks,
> > > > > > > > Punit
> > > > > > > >
> > > > > > > > [...]
> > > > > > > >
> > > > > > >
> > > > > > > _______________________________________________
> > > > > > > Linux-rockchip mailing list
> > > > > > > Linux-rockchip@lists.infradead.org
> > > > > > > http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [BUG] rockpro64: PCI BAR reassignment broken by commit 9d57e61bf723 ("of/pci: Add IORESOURCE_MEM_64 to resource flags for 64-bit memory addresses")
2021-05-25 17:18 ` Ard Biesheuvel
2021-05-25 17:37 ` Peter Geis
@ 2021-05-26 13:55 ` Christian König
2021-05-26 14:15 ` Ard Biesheuvel
1 sibling, 1 reply; 28+ messages in thread
From: Christian König @ 2021-05-26 13:55 UTC (permalink / raw)
To: Ard Biesheuvel, Peter Geis
Cc: Punit Agrawal, Robin Murphy, Alexandru Elisei,
Linux Kernel Mailing List, open list:ARM/Rockchip SoC...,
arm-mail-list, Heiko Stuebner, Leonardo Bras, Rob Herring, PCI
Hi Ard,
Am 25.05.21 um 19:18 schrieb Ard Biesheuvel:
> [SNIP]
>>> I seriously doubt that this is what is going on here.
>>>
>>> lspci -x will give you the bare BAR values - I suspect that those are
>>> probably fine.
>> lspci -x
>> 00:00.0 PCI bridge: Fuzhou Rockchip Electronics Co., Ltd Device 3566 (rev 01)
>> 00: 87 1d 66 35 07 05 10 40 01 00 04 06 00 00 01 00
>> 10: 00 00 00 00 00 00 00 00 00 01 ff 00 10 10 00 20
>> 20: 00 10 00 10 01 00 f1 0f 00 00 00 00 00 00 00 00
>> 30: 00 00 00 00 40 00 00 00 00 00 00 00 5f 01 02 00
>>
>> 01:00.0 VGA compatible controller: Advanced Micro Devices, Inc.
>> [AMD/ATI] Turks PRO [Radeon HD 7570]
>> 00: 02 10 5d 67 07 00 10 20 00 00 00 03 00 00 80 00
>> 10: 0c 00 00 00 00 00 00 00
> This is a 64-bit prefetchable BAR programmed with bus address 0x0
>
>> 04 00 00 10 00 00 00 00
> This is a 64-bit non-prefetchable BAR programmed with bus address 0x1000_0000
>
> (https://en.wikipedia.org/wiki/PCI_configuration_space describes the
> meaning of the low order BAR bits)
Sorry for jumping into the middle of the discussion and to be honest I
haven't fully read it.
This looks a bit odd since on AMD VGA hardware the non-prefetchable BAR
is usually only 32bit, not 64bit.
But this hardware generation is rather old and I'm not sure what the BAR
assignment for that generation was. I would need to dig up the register
description in our archives as well.
Christian.
>
>> 20: 01 10 70 3f 00 00 00 00
> This looks odd. This looks like a 32-bit MMIO address poked into a I/O BAR.
>
>
>> 00 00 00 00 28 10 20 2b
>> 30: 00 00 02 10 50 00 00 00 00 00 00 00 5f 01 00 00
>>
>> 01:00.1 Audio device: Advanced Micro Devices, Inc. [AMD/ATI] Turks
>> HDMI Audio [Radeon HD 6500/6600 / 6700M Series]
>> 00: 02 10 90 aa 06 00 10 20 00 00 03 04 00 00 80 00
>> 10: 04 00 04 10 00 00 00 00 00 00 00 00 00 00 00 00
>> 20: 00 00 00 00 00 00 00 00 00 00 00 00 28 10 90 aa
>> 30: 00 00 00 00 50 00 00 00 00 00 00 00 ff 02 00 00
>>
>>>
>>>>>> Also, if <0x82000000> (32 bit) is changed to <0x83000000> (64 bit),
>>>>>> most of the allocations for the dGPU fail due to no valid regions
>>>>>> available.
>>>>>>
>>>>> But wasn't the original problem that the resource window was 64-bit to
>>>>> begin with? Are you sure we are talking about the same problem here?
>>>> The rk3399 in the original report has a 32MB memory window in the
>>>> upper end of the 4GB range.
>>>> The rk356x has a similar layout, or it can use a 1GB window available
>>>> at <0x3 0x00000000>.
>>>> Rockchip's default windows are defined as 64bit.
>>>>
>>>> The rk3399 doesn't have enough space to reasonably define two windows,
>>>> one 32bit, one 64bit, to work around an allocation bug.
>>>> These are the defined regions in the rk3399:
>>>> ranges = <0x83000000 0x0 0xfa000000 0x0 0xfa000000 0x0 0x1e00000>,
>>>> <0x81000000 0x0 0xfbe00000 0x0 0xfbe00000 0x0 0x100000>;
>>>>
>>> All you really need is a 32-bit non-prefetchable resource window: any
>>> BAR can be allocated from that. A 64-bit BAR can carry a 32-bit number
>>> (just add zeroes at the top), and a prefetchable BAR can happily live
>>> in a non-prefetchable window, with a theoretical performance impact if
>>> the OS actually does use different memory attributes for the
>>> prefetchable window (but I don't think Linux ever handles it this way)
>> So is the IO range necessary as well or will it be automatically
>> allocated as well?
>>
> You need one I/O range and one 32-bit non-prefetchable MMIO window at
> the very least, even though the I/O range is rarely used, even by
> endpoints that expose I/O BARs.
>
> The translation is tricky to get right, and confuses some drivers, so
> it is better avoided if possible. If you do need translation, make
> sure to translate in the right direction.
>
>>>
>>>>>
>>>>>>>> I am happy to put something together once I understand the preferred way
>>>>>>>> to go about it.
>>>>>>>>
>>>>>>>> Thanks,
>>>>>>>> Punit
>>>>>>>>
>>>>>>>> [...]
>>>>>>>>
>>>>>>> _______________________________________________
>>>>>>> Linux-rockchip mailing list
>>>>>>> Linux-rockchip@lists.infradead.org
>>>>>>> http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [BUG] rockpro64: PCI BAR reassignment broken by commit 9d57e61bf723 ("of/pci: Add IORESOURCE_MEM_64 to resource flags for 64-bit memory addresses")
2021-05-26 13:55 ` Christian König
@ 2021-05-26 14:15 ` Ard Biesheuvel
0 siblings, 0 replies; 28+ messages in thread
From: Ard Biesheuvel @ 2021-05-26 14:15 UTC (permalink / raw)
To: Christian König
Cc: Peter Geis, Punit Agrawal, Robin Murphy, Alexandru Elisei,
Linux Kernel Mailing List, open list:ARM/Rockchip SoC...,
arm-mail-list, Heiko Stuebner, Leonardo Bras, Rob Herring, PCI
On Wed, 26 May 2021 at 15:55, Christian König
<ckoenig.leichtzumerken@gmail.com> wrote:
>
> Hi Ard,
>
> Am 25.05.21 um 19:18 schrieb Ard Biesheuvel:
> > [SNIP]
> >>> I seriously doubt that this is what is going on here.
> >>>
> >>> lspci -x will give you the bare BAR values - I suspect that those are
> >>> probably fine.
> >> lspci -x
> >> 00:00.0 PCI bridge: Fuzhou Rockchip Electronics Co., Ltd Device 3566 (rev 01)
> >> 00: 87 1d 66 35 07 05 10 40 01 00 04 06 00 00 01 00
> >> 10: 00 00 00 00 00 00 00 00 00 01 ff 00 10 10 00 20
> >> 20: 00 10 00 10 01 00 f1 0f 00 00 00 00 00 00 00 00
> >> 30: 00 00 00 00 40 00 00 00 00 00 00 00 5f 01 02 00
> >>
> >> 01:00.0 VGA compatible controller: Advanced Micro Devices, Inc.
> >> [AMD/ATI] Turks PRO [Radeon HD 7570]
> >> 00: 02 10 5d 67 07 00 10 20 00 00 00 03 00 00 80 00
> >> 10: 0c 00 00 00 00 00 00 00
> > This is a 64-bit prefetchable BAR programmed with bus address 0x0
> >
> >> 04 00 00 10 00 00 00 00
> > This is a 64-bit non-prefetchable BAR programmed with bus address 0x1000_0000
> >
> > (https://en.wikipedia.org/wiki/PCI_configuration_space describes the
> > meaning of the low order BAR bits)
>
> Sorry for jumping into the middle of the discussion and to be honest I
> haven't fully read it.
>
> This looks a bit odd since on AMD VGA hardware the non-prefetchable BAR
> is usually only 32bit, not 64bit.
>
> But this hardware generation is rather old and I'm not sure what the BAR
> assignment for that generation was. I would need to dig up the register
> description in our archives as well.
>
I have another museum piece in my AMD Seattle:
02:00.0 VGA compatible controller: Advanced Micro Devices, Inc.
[AMD/ATI] Oland XT [Radeon HD 8670 / R7 250/350] (rev 81) (prog-if 00
[VGA controller])
Subsystem: Dell Oland XT [Radeon HD 8670 / R7 250/350]
Flags: bus master, fast devsel, latency 0, IRQ 255
Memory at 100000000 (64-bit, prefetchable) [size=4G]
Memory at 40000000 (64-bit, non-prefetchable) [size=256K]
I/O ports at 1000 [disabled] [size=256]
Expansion ROM at 40060000 [disabled] [size=128K]
Capabilities: <access denied>
Kernel modules: radeon, amdgpu
So AMD/ATI ASICs definitely exist that expose a 64-bit pref and a
64-bit non-pref BAR.
--
Ard.
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [BUG] rockpro64: PCI BAR reassignment broken by commit 9d57e61bf723 ("of/pci: Add IORESOURCE_MEM_64 to resource flags for 64-bit memory addresses")
2021-05-25 17:01 ` Peter Geis
2021-05-25 17:18 ` Ard Biesheuvel
@ 2021-05-25 17:25 ` Robin Murphy
2021-05-25 17:34 ` Peter Geis
1 sibling, 1 reply; 28+ messages in thread
From: Robin Murphy @ 2021-05-25 17:25 UTC (permalink / raw)
To: Peter Geis, Ard Biesheuvel
Cc: Punit Agrawal, Alexandru Elisei, Linux Kernel Mailing List,
open list:ARM/Rockchip SoC...,
arm-mail-list, Heiko Stuebner, Leonardo Bras, Rob Herring, PCI,
Christian König
On 2021-05-25 18:01, Peter Geis wrote:
> On Tue, May 25, 2021 at 12:44 PM Ard Biesheuvel <ardb@kernel.org> wrote:
>>
>> On Tue, 25 May 2021 at 18:23, Peter Geis <pgwipeout@gmail.com> wrote:
>>>
>>> On Tue, May 25, 2021 at 11:55 AM Ard Biesheuvel <ardb@kernel.org> wrote:
>>>>
>>>> On Tue, 25 May 2021 at 17:34, Peter Geis <pgwipeout@gmail.com> wrote:
>>>>>
>>>>> On Tue, May 25, 2021 at 9:57 AM Ard Biesheuvel <ardb@kernel.org> wrote:
>>>>>>
>>>>>> On Tue, 25 May 2021 at 15:42, Punit Agrawal <punitagrawal@gmail.com> wrote:
>>>>>>>
>>>>>>> Hi Ard,
>>>>>>>
>>>>>>> Ard Biesheuvel <ardb@kernel.org> writes:
>>>>>>>
>>>>>>>> On Sun, 23 May 2021 at 13:06, Punit Agrawal <punitagrawal@gmail.com> wrote:
>>>>>>>>>
>>>>>>>>> Robin Murphy <robin.murphy@arm.com> writes:
>>>>>>>>>
>>>>>>>>>> [ +linux-pci for visibility ]
>>>>>>>>>>
>>>>>>>>>> On 2021-05-18 10:09, Alexandru Elisei wrote:
>>>>>>>>>>> After doing a git bisect I was able to trace the following error when booting my
>>>>>>>>>>> rockpro64 v2 (rk3399 SoC) with a PCIE NVME expansion card:
>>>>>>>>>>> [..]
>>>>>>>>>>> [ 0.305183] rockchip-pcie f8000000.pcie: host bridge /pcie@f8000000 ranges:
>>>>>>>>>>> [ 0.305248] rockchip-pcie f8000000.pcie: MEM 0x00fa000000..0x00fbdfffff ->
>>>>>>>>>>> 0x00fa000000
>>>>>>>>>>> [ 0.305285] rockchip-pcie f8000000.pcie: IO 0x00fbe00000..0x00fbefffff ->
>>>>>>>>>>> 0x00fbe00000
>>>>>>>>>>> [ 0.306201] rockchip-pcie f8000000.pcie: supply vpcie1v8 not found, using dummy
>>>>>>>>>>> regulator
>>>>>>>>>>> [ 0.306334] rockchip-pcie f8000000.pcie: supply vpcie0v9 not found, using dummy
>>>>>>>>>>> regulator
>>>>>>>>>>> [ 0.373705] rockchip-pcie f8000000.pcie: PCI host bridge to bus 0000:00
>>>>>>>>>>> [ 0.373730] pci_bus 0000:00: root bus resource [bus 00-1f]
>>>>>>>>>>> [ 0.373751] pci_bus 0000:00: root bus resource [mem 0xfa000000-0xfbdfffff 64bit]
>>>>>>>>>>> [ 0.373777] pci_bus 0000:00: root bus resource [io 0x0000-0xfffff] (bus
>>>>>>>>>>> address [0xfbe00000-0xfbefffff])
>>>>>>>>>>> [ 0.373839] pci 0000:00:00.0: [1d87:0100] type 01 class 0x060400
>>>>>>>>>>> [ 0.373973] pci 0000:00:00.0: supports D1
>>>>>>>>>>> [ 0.373992] pci 0000:00:00.0: PME# supported from D0 D1 D3hot
>>>>>>>>>>> [ 0.378518] pci 0000:00:00.0: bridge configuration invalid ([bus 00-00]),
>>>>>>>>>>> reconfiguring
>>>>>>>>>>> [ 0.378765] pci 0000:01:00.0: [144d:a808] type 00 class 0x010802
>>>>>>>>>>> [ 0.378869] pci 0000:01:00.0: reg 0x10: [mem 0x00000000-0x00003fff 64bit]
>>>>>>>>>>> [ 0.379051] pci 0000:01:00.0: Max Payload Size set to 256 (was 128, max 256)
>>>>>>>>>>> [ 0.379661] pci 0000:01:00.0: 8.000 Gb/s available PCIe bandwidth, limited by
>>>>>>>>>>> 2.5 GT/s PCIe x4 link at 0000:00:00.0 (capable of 31.504 Gb/s with 8.0 GT/s PCIe
>>>>>>>>>>> x4 link)
>>>>>>>>>>> [ 0.393269] pci_bus 0000:01: busn_res: [bus 01-1f] end is updated to 01
>>>>>>>>>>> [ 0.393311] pci 0000:00:00.0: BAR 14: no space for [mem size 0x00100000]
>>>>>>>>>>> [ 0.393333] pci 0000:00:00.0: BAR 14: failed to assign [mem size 0x00100000]
>>>>>>>>>>> [ 0.393356] pci 0000:01:00.0: BAR 0: no space for [mem size 0x00004000 64bit]
>>>>>>>>>>> [ 0.393375] pci 0000:01:00.0: BAR 0: failed to assign [mem size 0x00004000 64bit]
>>>>>>>>>>> [ 0.393397] pci 0000:00:00.0: PCI bridge to [bus 01]
>>>>>>>>>>> [ 0.393839] pcieport 0000:00:00.0: PME: Signaling with IRQ 78
>>>>>>>>>>> [ 0.394165] pcieport 0000:00:00.0: AER: enabled with IRQ 78
>>>>>>>>>>> [..]
>>>>>>>>>>> to the commit 9d57e61bf723 ("of/pci: Add IORESOURCE_MEM_64 to
>>>>>>>>>>> resource flags for
>>>>>>>>>>> 64-bit memory addresses").
>>>>>>>>>>
>>>>>>>>>> FWFW, my hunch is that the host bridge advertising no 32-bit memory
>>>>>>>>>> resource, only only a single 64-bit non-prefetchable one (even though
>>>>>>>>>> it's entirely below 4GB) might be a bit weird and tripping something
>>>>>>>>>> up in the resource assignment code. It certainly seems like the thing
>>>>>>>>>> most directly related to the offending commit.
>>>>>>>>>>
>>>>>>>>>> I'd be tempted to try fiddling with that in the DT (i.e. changing
>>>>>>>>>> 0x83000000 to 0x82000000 in the PCIe node's "ranges" property) to see
>>>>>>>>>> if it makes any difference. Note that even if it helps, though, I
>>>>>>>>>> don't know whether that's the correct fix or just a bodge around a
>>>>>>>>>> corner-case bug somewhere in the resource code.
>>>>>>>>>
>>>>>>>>> From digging into this further the failure seems to be due to a mismatch
>>>>>>>>> of flags when allocating resources in pci_bus_alloc_from_region() -
>>>>>>>>>
>>>>>>>>> if ((res->flags ^ r->flags) & type_mask)
>>>>>>>>> continue;
>>>>>>>>>
>>>>>>>>> Though I am also not sure why the failure is only being reported on
>>>>>>>>> RK3399 - does a single 64-bit window have anything to do with it?
>>>>>>>>>
>>>>>>>>
>>>>>>>> The NVMe in the example exposes a single 64-bit non-prefetchable BAR.
>>>>>>>> Such BARs can not be allocated in a prefetchable host bridge window
>>>>>>>> (unlike the converse, i.e., allocating a prefetchable BAR in a
>>>>>>>> non-prefetchable host bridge window is fine)
>>>>>>>>
>>>>>>>> 64-bit non-prefetchable host bridge windows cannot be forwarded by PCI
>>>>>>>> to PCI bridges, they simply lack the BAR registers to describe them.
>>>>>>>> Therefore, non-prefetchable endpoint BARs (even 64-bit ones) need to
>>>>>>>> be carved out of a host bridge's non-prefetchable 32-bit window if
>>>>>>>> they need to pass through a bridge.
>>>>>>>
>>>>>>> Thank you for the explanation. I also looked at the PCI-to-PCI Bridge
>>>>>>> spec to understand where some of the limitations are coming from.
>>>>>>>
>>>>>>>> So the error seems to be here that the host bridge's 32-bit
>>>>>>>> non-prefetchable window has the 64-bit attribute set, even though it
>>>>>>>> resides below 4 GB entirely. I suppose that the resource allocation
>>>>>>>> could be made more forgiving (and it was in the past, before commit
>>>>>>>> 9d57e61bf723 was applied). However, I would strongly recommend not
>>>>>>>> deviating from common practice, and just describe the 32-bit
>>>>>>>> addressable non-prefetchable resource window as such.
>>>>>>>
>>>>>>> IIUC, the host bridge's configuration (64-bit on non-prefetchable
>>>>>>> window) is based on what the hardware advertises.
>>>>>>>
>>>>>>
>>>>>> What do you mean by 'what the hardware advertises'? The host bridge is
>>>>>> apparently configured to decode a 32-bit addressable window as MMIO,
>>>>>> and the question is why this window has the 64-bit attribute set in
>>>>>> the DT description.
>>>>>>
>>>>>>> Can you elaborate on what you have in mind to correct the
>>>>>>> non-prefetchable resource window? Are you thinking of adding a quirk
>>>>>>> somewhere to address this?
>>>>>>>
>>>>>>
>>>>>> No. Just fix the DT.
>>>>>
>>>>> Good Morning,
>>>>>
>>>>> I believe Robin is correct that there is more to this.
>>>>> While attempting to work out why dGPUs won't work with the rk356x
>>>>> series PCIe controllers, Christian König from the amd-gpu driver
>>>>> mailing list noticed the gpu was incorrectly allocated a 64bit
>>>>> non-prefetchable BAR which should instead be a 32 non-prefetchable
>>>>> BAR.
>>>>>
>>>>
>>>> This is due to the translation. For some reason, lspci translates the
>>>> BAR values to CPU addresses, but the PCI side addresses are within
>>>> 32-bits.
>>>
>>> The kernel log reports the same thing:
>>> [ 6.662141] pci 0000:01:00.0: reg 0x10: [mem 0x00000000-0x0fffffff
>>> 64bit pref]
>>> [ 6.662963] pci 0000:01:00.0: reg 0x18: [mem 0x00000000-0x0001ffff 64bit]
>>>
>>> You are saying this is a display only issue?
>>>
>>
>> Yes. What do the 'root bus resource' log lines say for these regions?
>> Those should give you both the CPU address as well as the bus address.
>
> [ 6.673497] pci_bus 0000:00: root bus resource [io 0x0000-0xfffff]
> (bus address [0x3f700000-0x3f7fffff])
> [ 6.674642] pci_bus 0000:00: root bus resource [mem
> 0x300000000-0x33f6fffff] (bus address [0x00000000-0x3f6fffff])
Assuming RK356x has a similar memory map to other Rockchip SoCs, I
suspect you may have a larger issue with your mem space window shadowing
a significant chunk of your RAM.
Robin.
>
> I tweaked the original Rockchip values to place the non-prefetchable
> memory first with the configuration and io later in this boot.
>
>>
>>
>>>>
>>>> Are you sure the amdgpu driver can even deal with non-1:1 host bridges?
>>>
>>> I cannot answer this as I'm not an amdgpu dev.
>>>
>>>>
>>>>> The ranges currently set are:
>>>>> ranges = <0x81000000 0x0 0x00800000 0x3 0x00800000 0x0 0x00100000
>>>>> 0x82000000 0x0 0x00900000 0x3 0x00900000 0x0 0x3f700000>;
>>>>>
>>>>
>>>> So you have two ranges here.
>>>
>>> The IO and PCI memory ranges.
>>>
>>> There is a third range, the configuration range, which is defined in
>>> the reg block:
>>> <0x3 0x00000000 0x0 0x800000>
>>> All three are shared in the same 1GB window on the rk356x.
>>>
>>
>> But the reg block is not a resource window, it is a configuration
>> range to program the host bridge.
>>
>>> https://elixir.bootlin.com/linux/v5.13-rc3/source/Documentation/devicetree/bindings/pci/designware-pcie.txt#L12
>>>
>>>>
>>>>> but the final allocation was:
>>>>>
>>>>> lspci -v
>>>>> 00:00.0 PCI bridge: Fuzhou Rockchip Electronics Co., Ltd Device 3566
>>>>> (rev 01) (prog-if 00 [Normal decode])
>>>>> Flags: bus master, fast devsel, latency 0, IRQ 96
>>>>> Bus: primary=00, secondary=01, subordinate=ff, sec-latency=0
>>>>> I/O behind bridge: 00001000-00001fff [size=4K]
>>>>> Memory behind bridge: 00900000-009fffff [size=1M]
>>>>> Prefetchable memory behind bridge:
>>>>> 0000000010000000-000000001fffffff [size=256M]
>>>>
>>>> But the host bridge/root port decodes two disjoint regions??
>>>>
>>>>> Expansion ROM at 300a00000 [virtual] [disabled] [size=64K]
>>>>> Capabilities: [40] Power Management version 3
>>>>> Capabilities: [50] MSI: Enable+ Count=1/32 Maskable- 64bit+
>>>>> Capabilities: [70] Express Root Port (Slot-), MSI 00
>>>>> Capabilities: [b0] MSI-X: Enable- Count=1 Masked-
>>>>> Capabilities: [100] Advanced Error Reporting
>>>>> Capabilities: [148] Secondary PCI Express
>>>>> Capabilities: [160] L1 PM Substates
>>>>> Capabilities: [170] Vendor Specific Information: ID=0002 Rev=4
>>>>> Len=100 <?>
>>>>> Kernel driver in use: pcieport
>>>>>
>>>>> 01:00.0 VGA compatible controller: Advanced Micro Devices, Inc.
>>>>> [AMD/ATI] Turks PRO [Radeon HD 7570] (prog-if 00 [VGA controller])
>>>>> Subsystem: Dell Turks PRO [Radeon HD 7570]
>>>>> Flags: bus master, fast devsel, latency 0, IRQ 95
>>>>> Memory at 310000000 (64-bit, prefetchable) [size=256M]
>>>>> Memory at 300900000 (64-bit, non-prefetchable) [size=128K]
>>>>> I/O ports at 1000 [size=256]
>>>>> Expansion ROM at 300920000 [disabled] [size=128K]
>>>>> Capabilities: [50] Power Management version 3
>>>>> Capabilities: [58] Express Legacy Endpoint, MSI 00
>>>>> Capabilities: [a0] MSI: Enable- Count=1/1 Maskable- 64bit+
>>>>> Capabilities: [100] Vendor Specific Information: ID=0001 Rev=1
>>>>> Len=010 <?>
>>>>> Capabilities: [150] Advanced Error Reporting
>>>>> Kernel driver in use: radeon
>>>>>
>>>>> 01:00.1 Audio device: Advanced Micro Devices, Inc. [AMD/ATI] Turks
>>>>> HDMI Audio [Radeon HD 6500/6600 / 6700M Series]
>>>>> Subsystem: Dell Turks HDMI Audio [Radeon HD 6500/6600 / 6700M Series]
>>>>> Flags: bus master, fast devsel, latency 0, IRQ 98
>>>>> Memory at 300940000 (64-bit, non-prefetchable) [size=16K]
>>>>> Capabilities: [50] Power Management version 3
>>>>> Capabilities: [58] Express Legacy Endpoint, MSI 00
>>>>> Capabilities: [a0] MSI: Enable+ Count=1/1 Maskable- 64bit+
>>>>> Capabilities: [100] Vendor Specific Information: ID=0001 Rev=1
>>>>> Len=010 <?>
>>>>> Capabilities: [150] Advanced Error Reporting
>>>>> Kernel driver in use: snd_hda_intel
>>>>>
>>>>> This will obviously clobber registers during writes.
>>>>
>>>> I don't follow. Which writes will clobber which registers, and how is
>>>> it obvious?
>>>
>>> Writing a 64 bit word into a 32 bit register will either clobber the
>>> next higher 32 bit register.
>>> Quoting Christian:
>>> "When you program a 32bit BAR as 64bit you overwrite the register behind
>>> the BAR address with the upper 32bits of the 64bit address value.
>>> So even if the allocation fits into 32bits, the extra register write
>>> will certainly put your device into a banana state."
>>>
>>> https://lists.freedesktop.org/archives/amd-gfx/2021-May/064232.html
>>>
>>
>> I seriously doubt that this is what is going on here.
>>
>> lspci -x will give you the bare BAR values - I suspect that those are
>> probably fine.
>
> lspci -x
> 00:00.0 PCI bridge: Fuzhou Rockchip Electronics Co., Ltd Device 3566 (rev 01)
> 00: 87 1d 66 35 07 05 10 40 01 00 04 06 00 00 01 00
> 10: 00 00 00 00 00 00 00 00 00 01 ff 00 10 10 00 20
> 20: 00 10 00 10 01 00 f1 0f 00 00 00 00 00 00 00 00
> 30: 00 00 00 00 40 00 00 00 00 00 00 00 5f 01 02 00
>
> 01:00.0 VGA compatible controller: Advanced Micro Devices, Inc.
> [AMD/ATI] Turks PRO [Radeon HD 7570]
> 00: 02 10 5d 67 07 00 10 20 00 00 00 03 00 00 80 00
> 10: 0c 00 00 00 00 00 00 00 04 00 00 10 00 00 00 00
> 20: 01 10 70 3f 00 00 00 00 00 00 00 00 28 10 20 2b
> 30: 00 00 02 10 50 00 00 00 00 00 00 00 5f 01 00 00
>
> 01:00.1 Audio device: Advanced Micro Devices, Inc. [AMD/ATI] Turks
> HDMI Audio [Radeon HD 6500/6600 / 6700M Series]
> 00: 02 10 90 aa 06 00 10 20 00 00 03 04 00 00 80 00
> 10: 04 00 04 10 00 00 00 00 00 00 00 00 00 00 00 00
> 20: 00 00 00 00 00 00 00 00 00 00 00 00 28 10 90 aa
> 30: 00 00 00 00 50 00 00 00 00 00 00 00 ff 02 00 00
>
>>
>>
>>>>
>>>>> Also, if <0x82000000> (32 bit) is changed to <0x83000000> (64 bit),
>>>>> most of the allocations for the dGPU fail due to no valid regions
>>>>> available.
>>>>>
>>>>
>>>> But wasn't the original problem that the resource window was 64-bit to
>>>> begin with? Are you sure we are talking about the same problem here?
>>>
>>> The rk3399 in the original report has a 32MB memory window in the
>>> upper end of the 4GB range.
>>> The rk356x has a similar layout, or it can use a 1GB window available
>>> at <0x3 0x00000000>.
>>> Rockchip's default windows are defined as 64bit.
>>>
>>> The rk3399 doesn't have enough space to reasonably define two windows,
>>> one 32bit, one 64bit, to work around an allocation bug.
>>> These are the defined regions in the rk3399:
>>> ranges = <0x83000000 0x0 0xfa000000 0x0 0xfa000000 0x0 0x1e00000>,
>>> <0x81000000 0x0 0xfbe00000 0x0 0xfbe00000 0x0 0x100000>;
>>>
>>
>> All you really need is a 32-bit non-prefetchable resource window: any
>> BAR can be allocated from that. A 64-bit BAR can carry a 32-bit number
>> (just add zeroes at the top), and a prefetchable BAR can happily live
>> in a non-prefetchable window, with a theoretical performance impact if
>> the OS actually does use different memory attributes for the
>> prefetchable window (but I don't think Linux ever handles it this way)
>
> So is the IO range necessary as well or will it be automatically
> allocated as well?
>
>>
>>
>>>
>>>>
>>>>
>>>>>>
>>>>>>> I am happy to put something together once I understand the preferred way
>>>>>>> to go about it.
>>>>>>>
>>>>>>> Thanks,
>>>>>>> Punit
>>>>>>>
>>>>>>> [...]
>>>>>>>
>>>>>>
>>>>>> _______________________________________________
>>>>>> Linux-rockchip mailing list
>>>>>> Linux-rockchip@lists.infradead.org
>>>>>> http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [BUG] rockpro64: PCI BAR reassignment broken by commit 9d57e61bf723 ("of/pci: Add IORESOURCE_MEM_64 to resource flags for 64-bit memory addresses")
2021-05-25 17:25 ` Robin Murphy
@ 2021-05-25 17:34 ` Peter Geis
2021-05-25 18:55 ` Robin Murphy
0 siblings, 1 reply; 28+ messages in thread
From: Peter Geis @ 2021-05-25 17:34 UTC (permalink / raw)
To: Robin Murphy
Cc: Ard Biesheuvel, Punit Agrawal, Alexandru Elisei,
Linux Kernel Mailing List, open list:ARM/Rockchip SoC...,
arm-mail-list, Heiko Stuebner, Leonardo Bras, Rob Herring, PCI,
Christian König
On Tue, May 25, 2021 at 1:25 PM Robin Murphy <robin.murphy@arm.com> wrote:
>
> On 2021-05-25 18:01, Peter Geis wrote:
> > On Tue, May 25, 2021 at 12:44 PM Ard Biesheuvel <ardb@kernel.org> wrote:
> >>
> >> On Tue, 25 May 2021 at 18:23, Peter Geis <pgwipeout@gmail.com> wrote:
> >>>
> >>> On Tue, May 25, 2021 at 11:55 AM Ard Biesheuvel <ardb@kernel.org> wrote:
> >>>>
> >>>> On Tue, 25 May 2021 at 17:34, Peter Geis <pgwipeout@gmail.com> wrote:
> >>>>>
> >>>>> On Tue, May 25, 2021 at 9:57 AM Ard Biesheuvel <ardb@kernel.org> wrote:
> >>>>>>
> >>>>>> On Tue, 25 May 2021 at 15:42, Punit Agrawal <punitagrawal@gmail.com> wrote:
> >>>>>>>
> >>>>>>> Hi Ard,
> >>>>>>>
> >>>>>>> Ard Biesheuvel <ardb@kernel.org> writes:
> >>>>>>>
> >>>>>>>> On Sun, 23 May 2021 at 13:06, Punit Agrawal <punitagrawal@gmail.com> wrote:
> >>>>>>>>>
> >>>>>>>>> Robin Murphy <robin.murphy@arm.com> writes:
> >>>>>>>>>
> >>>>>>>>>> [ +linux-pci for visibility ]
> >>>>>>>>>>
> >>>>>>>>>> On 2021-05-18 10:09, Alexandru Elisei wrote:
> >>>>>>>>>>> After doing a git bisect I was able to trace the following error when booting my
> >>>>>>>>>>> rockpro64 v2 (rk3399 SoC) with a PCIE NVME expansion card:
> >>>>>>>>>>> [..]
> >>>>>>>>>>> [ 0.305183] rockchip-pcie f8000000.pcie: host bridge /pcie@f8000000 ranges:
> >>>>>>>>>>> [ 0.305248] rockchip-pcie f8000000.pcie: MEM 0x00fa000000..0x00fbdfffff ->
> >>>>>>>>>>> 0x00fa000000
> >>>>>>>>>>> [ 0.305285] rockchip-pcie f8000000.pcie: IO 0x00fbe00000..0x00fbefffff ->
> >>>>>>>>>>> 0x00fbe00000
> >>>>>>>>>>> [ 0.306201] rockchip-pcie f8000000.pcie: supply vpcie1v8 not found, using dummy
> >>>>>>>>>>> regulator
> >>>>>>>>>>> [ 0.306334] rockchip-pcie f8000000.pcie: supply vpcie0v9 not found, using dummy
> >>>>>>>>>>> regulator
> >>>>>>>>>>> [ 0.373705] rockchip-pcie f8000000.pcie: PCI host bridge to bus 0000:00
> >>>>>>>>>>> [ 0.373730] pci_bus 0000:00: root bus resource [bus 00-1f]
> >>>>>>>>>>> [ 0.373751] pci_bus 0000:00: root bus resource [mem 0xfa000000-0xfbdfffff 64bit]
> >>>>>>>>>>> [ 0.373777] pci_bus 0000:00: root bus resource [io 0x0000-0xfffff] (bus
> >>>>>>>>>>> address [0xfbe00000-0xfbefffff])
> >>>>>>>>>>> [ 0.373839] pci 0000:00:00.0: [1d87:0100] type 01 class 0x060400
> >>>>>>>>>>> [ 0.373973] pci 0000:00:00.0: supports D1
> >>>>>>>>>>> [ 0.373992] pci 0000:00:00.0: PME# supported from D0 D1 D3hot
> >>>>>>>>>>> [ 0.378518] pci 0000:00:00.0: bridge configuration invalid ([bus 00-00]),
> >>>>>>>>>>> reconfiguring
> >>>>>>>>>>> [ 0.378765] pci 0000:01:00.0: [144d:a808] type 00 class 0x010802
> >>>>>>>>>>> [ 0.378869] pci 0000:01:00.0: reg 0x10: [mem 0x00000000-0x00003fff 64bit]
> >>>>>>>>>>> [ 0.379051] pci 0000:01:00.0: Max Payload Size set to 256 (was 128, max 256)
> >>>>>>>>>>> [ 0.379661] pci 0000:01:00.0: 8.000 Gb/s available PCIe bandwidth, limited by
> >>>>>>>>>>> 2.5 GT/s PCIe x4 link at 0000:00:00.0 (capable of 31.504 Gb/s with 8.0 GT/s PCIe
> >>>>>>>>>>> x4 link)
> >>>>>>>>>>> [ 0.393269] pci_bus 0000:01: busn_res: [bus 01-1f] end is updated to 01
> >>>>>>>>>>> [ 0.393311] pci 0000:00:00.0: BAR 14: no space for [mem size 0x00100000]
> >>>>>>>>>>> [ 0.393333] pci 0000:00:00.0: BAR 14: failed to assign [mem size 0x00100000]
> >>>>>>>>>>> [ 0.393356] pci 0000:01:00.0: BAR 0: no space for [mem size 0x00004000 64bit]
> >>>>>>>>>>> [ 0.393375] pci 0000:01:00.0: BAR 0: failed to assign [mem size 0x00004000 64bit]
> >>>>>>>>>>> [ 0.393397] pci 0000:00:00.0: PCI bridge to [bus 01]
> >>>>>>>>>>> [ 0.393839] pcieport 0000:00:00.0: PME: Signaling with IRQ 78
> >>>>>>>>>>> [ 0.394165] pcieport 0000:00:00.0: AER: enabled with IRQ 78
> >>>>>>>>>>> [..]
> >>>>>>>>>>> to the commit 9d57e61bf723 ("of/pci: Add IORESOURCE_MEM_64 to
> >>>>>>>>>>> resource flags for
> >>>>>>>>>>> 64-bit memory addresses").
> >>>>>>>>>>
> >>>>>>>>>> FWFW, my hunch is that the host bridge advertising no 32-bit memory
> >>>>>>>>>> resource, only only a single 64-bit non-prefetchable one (even though
> >>>>>>>>>> it's entirely below 4GB) might be a bit weird and tripping something
> >>>>>>>>>> up in the resource assignment code. It certainly seems like the thing
> >>>>>>>>>> most directly related to the offending commit.
> >>>>>>>>>>
> >>>>>>>>>> I'd be tempted to try fiddling with that in the DT (i.e. changing
> >>>>>>>>>> 0x83000000 to 0x82000000 in the PCIe node's "ranges" property) to see
> >>>>>>>>>> if it makes any difference. Note that even if it helps, though, I
> >>>>>>>>>> don't know whether that's the correct fix or just a bodge around a
> >>>>>>>>>> corner-case bug somewhere in the resource code.
> >>>>>>>>>
> >>>>>>>>> From digging into this further the failure seems to be due to a mismatch
> >>>>>>>>> of flags when allocating resources in pci_bus_alloc_from_region() -
> >>>>>>>>>
> >>>>>>>>> if ((res->flags ^ r->flags) & type_mask)
> >>>>>>>>> continue;
> >>>>>>>>>
> >>>>>>>>> Though I am also not sure why the failure is only being reported on
> >>>>>>>>> RK3399 - does a single 64-bit window have anything to do with it?
> >>>>>>>>>
> >>>>>>>>
> >>>>>>>> The NVMe in the example exposes a single 64-bit non-prefetchable BAR.
> >>>>>>>> Such BARs can not be allocated in a prefetchable host bridge window
> >>>>>>>> (unlike the converse, i.e., allocating a prefetchable BAR in a
> >>>>>>>> non-prefetchable host bridge window is fine)
> >>>>>>>>
> >>>>>>>> 64-bit non-prefetchable host bridge windows cannot be forwarded by PCI
> >>>>>>>> to PCI bridges, they simply lack the BAR registers to describe them.
> >>>>>>>> Therefore, non-prefetchable endpoint BARs (even 64-bit ones) need to
> >>>>>>>> be carved out of a host bridge's non-prefetchable 32-bit window if
> >>>>>>>> they need to pass through a bridge.
> >>>>>>>
> >>>>>>> Thank you for the explanation. I also looked at the PCI-to-PCI Bridge
> >>>>>>> spec to understand where some of the limitations are coming from.
> >>>>>>>
> >>>>>>>> So the error seems to be here that the host bridge's 32-bit
> >>>>>>>> non-prefetchable window has the 64-bit attribute set, even though it
> >>>>>>>> resides below 4 GB entirely. I suppose that the resource allocation
> >>>>>>>> could be made more forgiving (and it was in the past, before commit
> >>>>>>>> 9d57e61bf723 was applied). However, I would strongly recommend not
> >>>>>>>> deviating from common practice, and just describe the 32-bit
> >>>>>>>> addressable non-prefetchable resource window as such.
> >>>>>>>
> >>>>>>> IIUC, the host bridge's configuration (64-bit on non-prefetchable
> >>>>>>> window) is based on what the hardware advertises.
> >>>>>>>
> >>>>>>
> >>>>>> What do you mean by 'what the hardware advertises'? The host bridge is
> >>>>>> apparently configured to decode a 32-bit addressable window as MMIO,
> >>>>>> and the question is why this window has the 64-bit attribute set in
> >>>>>> the DT description.
> >>>>>>
> >>>>>>> Can you elaborate on what you have in mind to correct the
> >>>>>>> non-prefetchable resource window? Are you thinking of adding a quirk
> >>>>>>> somewhere to address this?
> >>>>>>>
> >>>>>>
> >>>>>> No. Just fix the DT.
> >>>>>
> >>>>> Good Morning,
> >>>>>
> >>>>> I believe Robin is correct that there is more to this.
> >>>>> While attempting to work out why dGPUs won't work with the rk356x
> >>>>> series PCIe controllers, Christian König from the amd-gpu driver
> >>>>> mailing list noticed the gpu was incorrectly allocated a 64bit
> >>>>> non-prefetchable BAR which should instead be a 32 non-prefetchable
> >>>>> BAR.
> >>>>>
> >>>>
> >>>> This is due to the translation. For some reason, lspci translates the
> >>>> BAR values to CPU addresses, but the PCI side addresses are within
> >>>> 32-bits.
> >>>
> >>> The kernel log reports the same thing:
> >>> [ 6.662141] pci 0000:01:00.0: reg 0x10: [mem 0x00000000-0x0fffffff
> >>> 64bit pref]
> >>> [ 6.662963] pci 0000:01:00.0: reg 0x18: [mem 0x00000000-0x0001ffff 64bit]
> >>>
> >>> You are saying this is a display only issue?
> >>>
> >>
> >> Yes. What do the 'root bus resource' log lines say for these regions?
> >> Those should give you both the CPU address as well as the bus address.
> >
> > [ 6.673497] pci_bus 0000:00: root bus resource [io 0x0000-0xfffff]
> > (bus address [0x3f700000-0x3f7fffff])
> > [ 6.674642] pci_bus 0000:00: root bus resource [mem
> > 0x300000000-0x33f6fffff] (bus address [0x00000000-0x3f6fffff])
>
> Assuming RK356x has a similar memory map to other Rockchip SoCs, I
> suspect you may have a larger issue with your mem space window shadowing
> a significant chunk of your RAM.
>
> Robin.
Delightfully they seem to have learned a thing or two.
The 1GB window resides outside the system ram space.
Though the mmio devices do still take up the tail end of the 4G space
at 0xf0000000 - 0xffffffff.
The system ram covers up to 0x2 0x00000000, the rk356x supports 8G of
ram (I know, on a 32 bit bus, I've raged about it already).
Then the PCIe controllers start at 0x3 0x00000000.
This makes it seem the controllers have dedicated on chip ram which
doesn't have direct access to system ram, though I don't know the
implications of this (if I'm even interpreting it correctly).
>
> >
> > I tweaked the original Rockchip values to place the non-prefetchable
> > memory first with the configuration and io later in this boot.
> >
> >>
> >>
> >>>>
> >>>> Are you sure the amdgpu driver can even deal with non-1:1 host bridges?
> >>>
> >>> I cannot answer this as I'm not an amdgpu dev.
> >>>
> >>>>
> >>>>> The ranges currently set are:
> >>>>> ranges = <0x81000000 0x0 0x00800000 0x3 0x00800000 0x0 0x00100000
> >>>>> 0x82000000 0x0 0x00900000 0x3 0x00900000 0x0 0x3f700000>;
> >>>>>
> >>>>
> >>>> So you have two ranges here.
> >>>
> >>> The IO and PCI memory ranges.
> >>>
> >>> There is a third range, the configuration range, which is defined in
> >>> the reg block:
> >>> <0x3 0x00000000 0x0 0x800000>
> >>> All three are shared in the same 1GB window on the rk356x.
> >>>
> >>
> >> But the reg block is not a resource window, it is a configuration
> >> range to program the host bridge.
> >>
> >>> https://elixir.bootlin.com/linux/v5.13-rc3/source/Documentation/devicetree/bindings/pci/designware-pcie.txt#L12
> >>>
> >>>>
> >>>>> but the final allocation was:
> >>>>>
> >>>>> lspci -v
> >>>>> 00:00.0 PCI bridge: Fuzhou Rockchip Electronics Co., Ltd Device 3566
> >>>>> (rev 01) (prog-if 00 [Normal decode])
> >>>>> Flags: bus master, fast devsel, latency 0, IRQ 96
> >>>>> Bus: primary=00, secondary=01, subordinate=ff, sec-latency=0
> >>>>> I/O behind bridge: 00001000-00001fff [size=4K]
> >>>>> Memory behind bridge: 00900000-009fffff [size=1M]
> >>>>> Prefetchable memory behind bridge:
> >>>>> 0000000010000000-000000001fffffff [size=256M]
> >>>>
> >>>> But the host bridge/root port decodes two disjoint regions??
> >>>>
> >>>>> Expansion ROM at 300a00000 [virtual] [disabled] [size=64K]
> >>>>> Capabilities: [40] Power Management version 3
> >>>>> Capabilities: [50] MSI: Enable+ Count=1/32 Maskable- 64bit+
> >>>>> Capabilities: [70] Express Root Port (Slot-), MSI 00
> >>>>> Capabilities: [b0] MSI-X: Enable- Count=1 Masked-
> >>>>> Capabilities: [100] Advanced Error Reporting
> >>>>> Capabilities: [148] Secondary PCI Express
> >>>>> Capabilities: [160] L1 PM Substates
> >>>>> Capabilities: [170] Vendor Specific Information: ID=0002 Rev=4
> >>>>> Len=100 <?>
> >>>>> Kernel driver in use: pcieport
> >>>>>
> >>>>> 01:00.0 VGA compatible controller: Advanced Micro Devices, Inc.
> >>>>> [AMD/ATI] Turks PRO [Radeon HD 7570] (prog-if 00 [VGA controller])
> >>>>> Subsystem: Dell Turks PRO [Radeon HD 7570]
> >>>>> Flags: bus master, fast devsel, latency 0, IRQ 95
> >>>>> Memory at 310000000 (64-bit, prefetchable) [size=256M]
> >>>>> Memory at 300900000 (64-bit, non-prefetchable) [size=128K]
> >>>>> I/O ports at 1000 [size=256]
> >>>>> Expansion ROM at 300920000 [disabled] [size=128K]
> >>>>> Capabilities: [50] Power Management version 3
> >>>>> Capabilities: [58] Express Legacy Endpoint, MSI 00
> >>>>> Capabilities: [a0] MSI: Enable- Count=1/1 Maskable- 64bit+
> >>>>> Capabilities: [100] Vendor Specific Information: ID=0001 Rev=1
> >>>>> Len=010 <?>
> >>>>> Capabilities: [150] Advanced Error Reporting
> >>>>> Kernel driver in use: radeon
> >>>>>
> >>>>> 01:00.1 Audio device: Advanced Micro Devices, Inc. [AMD/ATI] Turks
> >>>>> HDMI Audio [Radeon HD 6500/6600 / 6700M Series]
> >>>>> Subsystem: Dell Turks HDMI Audio [Radeon HD 6500/6600 / 6700M Series]
> >>>>> Flags: bus master, fast devsel, latency 0, IRQ 98
> >>>>> Memory at 300940000 (64-bit, non-prefetchable) [size=16K]
> >>>>> Capabilities: [50] Power Management version 3
> >>>>> Capabilities: [58] Express Legacy Endpoint, MSI 00
> >>>>> Capabilities: [a0] MSI: Enable+ Count=1/1 Maskable- 64bit+
> >>>>> Capabilities: [100] Vendor Specific Information: ID=0001 Rev=1
> >>>>> Len=010 <?>
> >>>>> Capabilities: [150] Advanced Error Reporting
> >>>>> Kernel driver in use: snd_hda_intel
> >>>>>
> >>>>> This will obviously clobber registers during writes.
> >>>>
> >>>> I don't follow. Which writes will clobber which registers, and how is
> >>>> it obvious?
> >>>
> >>> Writing a 64 bit word into a 32 bit register will either clobber the
> >>> next higher 32 bit register.
> >>> Quoting Christian:
> >>> "When you program a 32bit BAR as 64bit you overwrite the register behind
> >>> the BAR address with the upper 32bits of the 64bit address value.
> >>> So even if the allocation fits into 32bits, the extra register write
> >>> will certainly put your device into a banana state."
> >>>
> >>> https://lists.freedesktop.org/archives/amd-gfx/2021-May/064232.html
> >>>
> >>
> >> I seriously doubt that this is what is going on here.
> >>
> >> lspci -x will give you the bare BAR values - I suspect that those are
> >> probably fine.
> >
> > lspci -x
> > 00:00.0 PCI bridge: Fuzhou Rockchip Electronics Co., Ltd Device 3566 (rev 01)
> > 00: 87 1d 66 35 07 05 10 40 01 00 04 06 00 00 01 00
> > 10: 00 00 00 00 00 00 00 00 00 01 ff 00 10 10 00 20
> > 20: 00 10 00 10 01 00 f1 0f 00 00 00 00 00 00 00 00
> > 30: 00 00 00 00 40 00 00 00 00 00 00 00 5f 01 02 00
> >
> > 01:00.0 VGA compatible controller: Advanced Micro Devices, Inc.
> > [AMD/ATI] Turks PRO [Radeon HD 7570]
> > 00: 02 10 5d 67 07 00 10 20 00 00 00 03 00 00 80 00
> > 10: 0c 00 00 00 00 00 00 00 04 00 00 10 00 00 00 00
> > 20: 01 10 70 3f 00 00 00 00 00 00 00 00 28 10 20 2b
> > 30: 00 00 02 10 50 00 00 00 00 00 00 00 5f 01 00 00
> >
> > 01:00.1 Audio device: Advanced Micro Devices, Inc. [AMD/ATI] Turks
> > HDMI Audio [Radeon HD 6500/6600 / 6700M Series]
> > 00: 02 10 90 aa 06 00 10 20 00 00 03 04 00 00 80 00
> > 10: 04 00 04 10 00 00 00 00 00 00 00 00 00 00 00 00
> > 20: 00 00 00 00 00 00 00 00 00 00 00 00 28 10 90 aa
> > 30: 00 00 00 00 50 00 00 00 00 00 00 00 ff 02 00 00
> >
> >>
> >>
> >>>>
> >>>>> Also, if <0x82000000> (32 bit) is changed to <0x83000000> (64 bit),
> >>>>> most of the allocations for the dGPU fail due to no valid regions
> >>>>> available.
> >>>>>
> >>>>
> >>>> But wasn't the original problem that the resource window was 64-bit to
> >>>> begin with? Are you sure we are talking about the same problem here?
> >>>
> >>> The rk3399 in the original report has a 32MB memory window in the
> >>> upper end of the 4GB range.
> >>> The rk356x has a similar layout, or it can use a 1GB window available
> >>> at <0x3 0x00000000>.
> >>> Rockchip's default windows are defined as 64bit.
> >>>
> >>> The rk3399 doesn't have enough space to reasonably define two windows,
> >>> one 32bit, one 64bit, to work around an allocation bug.
> >>> These are the defined regions in the rk3399:
> >>> ranges = <0x83000000 0x0 0xfa000000 0x0 0xfa000000 0x0 0x1e00000>,
> >>> <0x81000000 0x0 0xfbe00000 0x0 0xfbe00000 0x0 0x100000>;
> >>>
> >>
> >> All you really need is a 32-bit non-prefetchable resource window: any
> >> BAR can be allocated from that. A 64-bit BAR can carry a 32-bit number
> >> (just add zeroes at the top), and a prefetchable BAR can happily live
> >> in a non-prefetchable window, with a theoretical performance impact if
> >> the OS actually does use different memory attributes for the
> >> prefetchable window (but I don't think Linux ever handles it this way)
> >
> > So is the IO range necessary as well or will it be automatically
> > allocated as well?
> >
> >>
> >>
> >>>
> >>>>
> >>>>
> >>>>>>
> >>>>>>> I am happy to put something together once I understand the preferred way
> >>>>>>> to go about it.
> >>>>>>>
> >>>>>>> Thanks,
> >>>>>>> Punit
> >>>>>>>
> >>>>>>> [...]
> >>>>>>>
> >>>>>>
> >>>>>> _______________________________________________
> >>>>>> Linux-rockchip mailing list
> >>>>>> Linux-rockchip@lists.infradead.org
> >>>>>> http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [BUG] rockpro64: PCI BAR reassignment broken by commit 9d57e61bf723 ("of/pci: Add IORESOURCE_MEM_64 to resource flags for 64-bit memory addresses")
2021-05-25 17:34 ` Peter Geis
@ 2021-05-25 18:55 ` Robin Murphy
0 siblings, 0 replies; 28+ messages in thread
From: Robin Murphy @ 2021-05-25 18:55 UTC (permalink / raw)
To: Peter Geis
Cc: Ard Biesheuvel, Punit Agrawal, Alexandru Elisei,
Linux Kernel Mailing List, open list:ARM/Rockchip SoC...,
arm-mail-list, Heiko Stuebner, Leonardo Bras, Rob Herring, PCI,
Christian König
On 2021-05-25 18:34, Peter Geis wrote:
> On Tue, May 25, 2021 at 1:25 PM Robin Murphy <robin.murphy@arm.com> wrote:
>>
>> On 2021-05-25 18:01, Peter Geis wrote:
>>> On Tue, May 25, 2021 at 12:44 PM Ard Biesheuvel <ardb@kernel.org> wrote:
>>>>
>>>> On Tue, 25 May 2021 at 18:23, Peter Geis <pgwipeout@gmail.com> wrote:
>>>>>
>>>>> On Tue, May 25, 2021 at 11:55 AM Ard Biesheuvel <ardb@kernel.org> wrote:
>>>>>>
>>>>>> On Tue, 25 May 2021 at 17:34, Peter Geis <pgwipeout@gmail.com> wrote:
>>>>>>>
>>>>>>> On Tue, May 25, 2021 at 9:57 AM Ard Biesheuvel <ardb@kernel.org> wrote:
>>>>>>>>
>>>>>>>> On Tue, 25 May 2021 at 15:42, Punit Agrawal <punitagrawal@gmail.com> wrote:
>>>>>>>>>
>>>>>>>>> Hi Ard,
>>>>>>>>>
>>>>>>>>> Ard Biesheuvel <ardb@kernel.org> writes:
>>>>>>>>>
>>>>>>>>>> On Sun, 23 May 2021 at 13:06, Punit Agrawal <punitagrawal@gmail.com> wrote:
>>>>>>>>>>>
>>>>>>>>>>> Robin Murphy <robin.murphy@arm.com> writes:
>>>>>>>>>>>
>>>>>>>>>>>> [ +linux-pci for visibility ]
>>>>>>>>>>>>
>>>>>>>>>>>> On 2021-05-18 10:09, Alexandru Elisei wrote:
>>>>>>>>>>>>> After doing a git bisect I was able to trace the following error when booting my
>>>>>>>>>>>>> rockpro64 v2 (rk3399 SoC) with a PCIE NVME expansion card:
>>>>>>>>>>>>> [..]
>>>>>>>>>>>>> [ 0.305183] rockchip-pcie f8000000.pcie: host bridge /pcie@f8000000 ranges:
>>>>>>>>>>>>> [ 0.305248] rockchip-pcie f8000000.pcie: MEM 0x00fa000000..0x00fbdfffff ->
>>>>>>>>>>>>> 0x00fa000000
>>>>>>>>>>>>> [ 0.305285] rockchip-pcie f8000000.pcie: IO 0x00fbe00000..0x00fbefffff ->
>>>>>>>>>>>>> 0x00fbe00000
>>>>>>>>>>>>> [ 0.306201] rockchip-pcie f8000000.pcie: supply vpcie1v8 not found, using dummy
>>>>>>>>>>>>> regulator
>>>>>>>>>>>>> [ 0.306334] rockchip-pcie f8000000.pcie: supply vpcie0v9 not found, using dummy
>>>>>>>>>>>>> regulator
>>>>>>>>>>>>> [ 0.373705] rockchip-pcie f8000000.pcie: PCI host bridge to bus 0000:00
>>>>>>>>>>>>> [ 0.373730] pci_bus 0000:00: root bus resource [bus 00-1f]
>>>>>>>>>>>>> [ 0.373751] pci_bus 0000:00: root bus resource [mem 0xfa000000-0xfbdfffff 64bit]
>>>>>>>>>>>>> [ 0.373777] pci_bus 0000:00: root bus resource [io 0x0000-0xfffff] (bus
>>>>>>>>>>>>> address [0xfbe00000-0xfbefffff])
>>>>>>>>>>>>> [ 0.373839] pci 0000:00:00.0: [1d87:0100] type 01 class 0x060400
>>>>>>>>>>>>> [ 0.373973] pci 0000:00:00.0: supports D1
>>>>>>>>>>>>> [ 0.373992] pci 0000:00:00.0: PME# supported from D0 D1 D3hot
>>>>>>>>>>>>> [ 0.378518] pci 0000:00:00.0: bridge configuration invalid ([bus 00-00]),
>>>>>>>>>>>>> reconfiguring
>>>>>>>>>>>>> [ 0.378765] pci 0000:01:00.0: [144d:a808] type 00 class 0x010802
>>>>>>>>>>>>> [ 0.378869] pci 0000:01:00.0: reg 0x10: [mem 0x00000000-0x00003fff 64bit]
>>>>>>>>>>>>> [ 0.379051] pci 0000:01:00.0: Max Payload Size set to 256 (was 128, max 256)
>>>>>>>>>>>>> [ 0.379661] pci 0000:01:00.0: 8.000 Gb/s available PCIe bandwidth, limited by
>>>>>>>>>>>>> 2.5 GT/s PCIe x4 link at 0000:00:00.0 (capable of 31.504 Gb/s with 8.0 GT/s PCIe
>>>>>>>>>>>>> x4 link)
>>>>>>>>>>>>> [ 0.393269] pci_bus 0000:01: busn_res: [bus 01-1f] end is updated to 01
>>>>>>>>>>>>> [ 0.393311] pci 0000:00:00.0: BAR 14: no space for [mem size 0x00100000]
>>>>>>>>>>>>> [ 0.393333] pci 0000:00:00.0: BAR 14: failed to assign [mem size 0x00100000]
>>>>>>>>>>>>> [ 0.393356] pci 0000:01:00.0: BAR 0: no space for [mem size 0x00004000 64bit]
>>>>>>>>>>>>> [ 0.393375] pci 0000:01:00.0: BAR 0: failed to assign [mem size 0x00004000 64bit]
>>>>>>>>>>>>> [ 0.393397] pci 0000:00:00.0: PCI bridge to [bus 01]
>>>>>>>>>>>>> [ 0.393839] pcieport 0000:00:00.0: PME: Signaling with IRQ 78
>>>>>>>>>>>>> [ 0.394165] pcieport 0000:00:00.0: AER: enabled with IRQ 78
>>>>>>>>>>>>> [..]
>>>>>>>>>>>>> to the commit 9d57e61bf723 ("of/pci: Add IORESOURCE_MEM_64 to
>>>>>>>>>>>>> resource flags for
>>>>>>>>>>>>> 64-bit memory addresses").
>>>>>>>>>>>>
>>>>>>>>>>>> FWFW, my hunch is that the host bridge advertising no 32-bit memory
>>>>>>>>>>>> resource, only only a single 64-bit non-prefetchable one (even though
>>>>>>>>>>>> it's entirely below 4GB) might be a bit weird and tripping something
>>>>>>>>>>>> up in the resource assignment code. It certainly seems like the thing
>>>>>>>>>>>> most directly related to the offending commit.
>>>>>>>>>>>>
>>>>>>>>>>>> I'd be tempted to try fiddling with that in the DT (i.e. changing
>>>>>>>>>>>> 0x83000000 to 0x82000000 in the PCIe node's "ranges" property) to see
>>>>>>>>>>>> if it makes any difference. Note that even if it helps, though, I
>>>>>>>>>>>> don't know whether that's the correct fix or just a bodge around a
>>>>>>>>>>>> corner-case bug somewhere in the resource code.
>>>>>>>>>>>
>>>>>>>>>>> From digging into this further the failure seems to be due to a mismatch
>>>>>>>>>>> of flags when allocating resources in pci_bus_alloc_from_region() -
>>>>>>>>>>>
>>>>>>>>>>> if ((res->flags ^ r->flags) & type_mask)
>>>>>>>>>>> continue;
>>>>>>>>>>>
>>>>>>>>>>> Though I am also not sure why the failure is only being reported on
>>>>>>>>>>> RK3399 - does a single 64-bit window have anything to do with it?
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> The NVMe in the example exposes a single 64-bit non-prefetchable BAR.
>>>>>>>>>> Such BARs can not be allocated in a prefetchable host bridge window
>>>>>>>>>> (unlike the converse, i.e., allocating a prefetchable BAR in a
>>>>>>>>>> non-prefetchable host bridge window is fine)
>>>>>>>>>>
>>>>>>>>>> 64-bit non-prefetchable host bridge windows cannot be forwarded by PCI
>>>>>>>>>> to PCI bridges, they simply lack the BAR registers to describe them.
>>>>>>>>>> Therefore, non-prefetchable endpoint BARs (even 64-bit ones) need to
>>>>>>>>>> be carved out of a host bridge's non-prefetchable 32-bit window if
>>>>>>>>>> they need to pass through a bridge.
>>>>>>>>>
>>>>>>>>> Thank you for the explanation. I also looked at the PCI-to-PCI Bridge
>>>>>>>>> spec to understand where some of the limitations are coming from.
>>>>>>>>>
>>>>>>>>>> So the error seems to be here that the host bridge's 32-bit
>>>>>>>>>> non-prefetchable window has the 64-bit attribute set, even though it
>>>>>>>>>> resides below 4 GB entirely. I suppose that the resource allocation
>>>>>>>>>> could be made more forgiving (and it was in the past, before commit
>>>>>>>>>> 9d57e61bf723 was applied). However, I would strongly recommend not
>>>>>>>>>> deviating from common practice, and just describe the 32-bit
>>>>>>>>>> addressable non-prefetchable resource window as such.
>>>>>>>>>
>>>>>>>>> IIUC, the host bridge's configuration (64-bit on non-prefetchable
>>>>>>>>> window) is based on what the hardware advertises.
>>>>>>>>>
>>>>>>>>
>>>>>>>> What do you mean by 'what the hardware advertises'? The host bridge is
>>>>>>>> apparently configured to decode a 32-bit addressable window as MMIO,
>>>>>>>> and the question is why this window has the 64-bit attribute set in
>>>>>>>> the DT description.
>>>>>>>>
>>>>>>>>> Can you elaborate on what you have in mind to correct the
>>>>>>>>> non-prefetchable resource window? Are you thinking of adding a quirk
>>>>>>>>> somewhere to address this?
>>>>>>>>>
>>>>>>>>
>>>>>>>> No. Just fix the DT.
>>>>>>>
>>>>>>> Good Morning,
>>>>>>>
>>>>>>> I believe Robin is correct that there is more to this.
>>>>>>> While attempting to work out why dGPUs won't work with the rk356x
>>>>>>> series PCIe controllers, Christian König from the amd-gpu driver
>>>>>>> mailing list noticed the gpu was incorrectly allocated a 64bit
>>>>>>> non-prefetchable BAR which should instead be a 32 non-prefetchable
>>>>>>> BAR.
>>>>>>>
>>>>>>
>>>>>> This is due to the translation. For some reason, lspci translates the
>>>>>> BAR values to CPU addresses, but the PCI side addresses are within
>>>>>> 32-bits.
>>>>>
>>>>> The kernel log reports the same thing:
>>>>> [ 6.662141] pci 0000:01:00.0: reg 0x10: [mem 0x00000000-0x0fffffff
>>>>> 64bit pref]
>>>>> [ 6.662963] pci 0000:01:00.0: reg 0x18: [mem 0x00000000-0x0001ffff 64bit]
>>>>>
>>>>> You are saying this is a display only issue?
>>>>>
>>>>
>>>> Yes. What do the 'root bus resource' log lines say for these regions?
>>>> Those should give you both the CPU address as well as the bus address.
>>>
>>> [ 6.673497] pci_bus 0000:00: root bus resource [io 0x0000-0xfffff]
>>> (bus address [0x3f700000-0x3f7fffff])
>>> [ 6.674642] pci_bus 0000:00: root bus resource [mem
>>> 0x300000000-0x33f6fffff] (bus address [0x00000000-0x3f6fffff])
>>
>> Assuming RK356x has a similar memory map to other Rockchip SoCs, I
>> suspect you may have a larger issue with your mem space window shadowing
>> a significant chunk of your RAM.
>>
>> Robin.
>
> Delightfully they seem to have learned a thing or two.
> The 1GB window resides outside the system ram space.
> Though the mmio devices do still take up the tail end of the 4G space
> at 0xf0000000 - 0xffffffff.
> The system ram covers up to 0x2 0x00000000, the rk356x supports 8G of
> ram (I know, on a 32 bit bus, I've raged about it already).
> Then the PCIe controllers start at 0x3 0x00000000.
> This makes it seem the controllers have dedicated on chip ram which
> doesn't have direct access to system ram, though I don't know the
> implications of this (if I'm even interpreting it correctly).
That is indeed the view of things in the *CPU* memory map, but what I'm
getting at is the *bus* address space, i.e. from the device's point of
view. Say you have a page at CPU physical address 0x1000 that you want
your PCI device to DMA to, consider how that's going to work if the bus
address range 0x0-0x1fffff has already been assigned to some device's BAR?
I don't have time to get into a lesson about how PCI works, but the key
point here is that PCI memory space is a distinct thing from the system
physical address space upstream of the host bridge. As Ard said,
translations between the two can be tricky to get right. In your case
you could at least identity-map your 64-bit window(s) - for 32-bit
resources it's unavoidable if your CPU view starts up at 12GB, but
you're probably better off pointing that window at 0xf0000000 on the bus
side rather than 0x0. At least that way it shouldn't conflict with
anything useful. Be thankful there *is* still some non-PCI-relevant
space in the bottom 4GB of the CPU memory map - if it was all RAM you'd
be pretty stuffed!
Robin.
>
>>
>>>
>>> I tweaked the original Rockchip values to place the non-prefetchable
>>> memory first with the configuration and io later in this boot.
>>>
>>>>
>>>>
>>>>>>
>>>>>> Are you sure the amdgpu driver can even deal with non-1:1 host bridges?
>>>>>
>>>>> I cannot answer this as I'm not an amdgpu dev.
>>>>>
>>>>>>
>>>>>>> The ranges currently set are:
>>>>>>> ranges = <0x81000000 0x0 0x00800000 0x3 0x00800000 0x0 0x00100000
>>>>>>> 0x82000000 0x0 0x00900000 0x3 0x00900000 0x0 0x3f700000>;
>>>>>>>
>>>>>>
>>>>>> So you have two ranges here.
>>>>>
>>>>> The IO and PCI memory ranges.
>>>>>
>>>>> There is a third range, the configuration range, which is defined in
>>>>> the reg block:
>>>>> <0x3 0x00000000 0x0 0x800000>
>>>>> All three are shared in the same 1GB window on the rk356x.
>>>>>
>>>>
>>>> But the reg block is not a resource window, it is a configuration
>>>> range to program the host bridge.
>>>>
>>>>> https://elixir.bootlin.com/linux/v5.13-rc3/source/Documentation/devicetree/bindings/pci/designware-pcie.txt#L12
>>>>>
>>>>>>
>>>>>>> but the final allocation was:
>>>>>>>
>>>>>>> lspci -v
>>>>>>> 00:00.0 PCI bridge: Fuzhou Rockchip Electronics Co., Ltd Device 3566
>>>>>>> (rev 01) (prog-if 00 [Normal decode])
>>>>>>> Flags: bus master, fast devsel, latency 0, IRQ 96
>>>>>>> Bus: primary=00, secondary=01, subordinate=ff, sec-latency=0
>>>>>>> I/O behind bridge: 00001000-00001fff [size=4K]
>>>>>>> Memory behind bridge: 00900000-009fffff [size=1M]
>>>>>>> Prefetchable memory behind bridge:
>>>>>>> 0000000010000000-000000001fffffff [size=256M]
>>>>>>
>>>>>> But the host bridge/root port decodes two disjoint regions??
>>>>>>
>>>>>>> Expansion ROM at 300a00000 [virtual] [disabled] [size=64K]
>>>>>>> Capabilities: [40] Power Management version 3
>>>>>>> Capabilities: [50] MSI: Enable+ Count=1/32 Maskable- 64bit+
>>>>>>> Capabilities: [70] Express Root Port (Slot-), MSI 00
>>>>>>> Capabilities: [b0] MSI-X: Enable- Count=1 Masked-
>>>>>>> Capabilities: [100] Advanced Error Reporting
>>>>>>> Capabilities: [148] Secondary PCI Express
>>>>>>> Capabilities: [160] L1 PM Substates
>>>>>>> Capabilities: [170] Vendor Specific Information: ID=0002 Rev=4
>>>>>>> Len=100 <?>
>>>>>>> Kernel driver in use: pcieport
>>>>>>>
>>>>>>> 01:00.0 VGA compatible controller: Advanced Micro Devices, Inc.
>>>>>>> [AMD/ATI] Turks PRO [Radeon HD 7570] (prog-if 00 [VGA controller])
>>>>>>> Subsystem: Dell Turks PRO [Radeon HD 7570]
>>>>>>> Flags: bus master, fast devsel, latency 0, IRQ 95
>>>>>>> Memory at 310000000 (64-bit, prefetchable) [size=256M]
>>>>>>> Memory at 300900000 (64-bit, non-prefetchable) [size=128K]
>>>>>>> I/O ports at 1000 [size=256]
>>>>>>> Expansion ROM at 300920000 [disabled] [size=128K]
>>>>>>> Capabilities: [50] Power Management version 3
>>>>>>> Capabilities: [58] Express Legacy Endpoint, MSI 00
>>>>>>> Capabilities: [a0] MSI: Enable- Count=1/1 Maskable- 64bit+
>>>>>>> Capabilities: [100] Vendor Specific Information: ID=0001 Rev=1
>>>>>>> Len=010 <?>
>>>>>>> Capabilities: [150] Advanced Error Reporting
>>>>>>> Kernel driver in use: radeon
>>>>>>>
>>>>>>> 01:00.1 Audio device: Advanced Micro Devices, Inc. [AMD/ATI] Turks
>>>>>>> HDMI Audio [Radeon HD 6500/6600 / 6700M Series]
>>>>>>> Subsystem: Dell Turks HDMI Audio [Radeon HD 6500/6600 / 6700M Series]
>>>>>>> Flags: bus master, fast devsel, latency 0, IRQ 98
>>>>>>> Memory at 300940000 (64-bit, non-prefetchable) [size=16K]
>>>>>>> Capabilities: [50] Power Management version 3
>>>>>>> Capabilities: [58] Express Legacy Endpoint, MSI 00
>>>>>>> Capabilities: [a0] MSI: Enable+ Count=1/1 Maskable- 64bit+
>>>>>>> Capabilities: [100] Vendor Specific Information: ID=0001 Rev=1
>>>>>>> Len=010 <?>
>>>>>>> Capabilities: [150] Advanced Error Reporting
>>>>>>> Kernel driver in use: snd_hda_intel
>>>>>>>
>>>>>>> This will obviously clobber registers during writes.
>>>>>>
>>>>>> I don't follow. Which writes will clobber which registers, and how is
>>>>>> it obvious?
>>>>>
>>>>> Writing a 64 bit word into a 32 bit register will either clobber the
>>>>> next higher 32 bit register.
>>>>> Quoting Christian:
>>>>> "When you program a 32bit BAR as 64bit you overwrite the register behind
>>>>> the BAR address with the upper 32bits of the 64bit address value.
>>>>> So even if the allocation fits into 32bits, the extra register write
>>>>> will certainly put your device into a banana state."
>>>>>
>>>>> https://lists.freedesktop.org/archives/amd-gfx/2021-May/064232.html
>>>>>
>>>>
>>>> I seriously doubt that this is what is going on here.
>>>>
>>>> lspci -x will give you the bare BAR values - I suspect that those are
>>>> probably fine.
>>>
>>> lspci -x
>>> 00:00.0 PCI bridge: Fuzhou Rockchip Electronics Co., Ltd Device 3566 (rev 01)
>>> 00: 87 1d 66 35 07 05 10 40 01 00 04 06 00 00 01 00
>>> 10: 00 00 00 00 00 00 00 00 00 01 ff 00 10 10 00 20
>>> 20: 00 10 00 10 01 00 f1 0f 00 00 00 00 00 00 00 00
>>> 30: 00 00 00 00 40 00 00 00 00 00 00 00 5f 01 02 00
>>>
>>> 01:00.0 VGA compatible controller: Advanced Micro Devices, Inc.
>>> [AMD/ATI] Turks PRO [Radeon HD 7570]
>>> 00: 02 10 5d 67 07 00 10 20 00 00 00 03 00 00 80 00
>>> 10: 0c 00 00 00 00 00 00 00 04 00 00 10 00 00 00 00
>>> 20: 01 10 70 3f 00 00 00 00 00 00 00 00 28 10 20 2b
>>> 30: 00 00 02 10 50 00 00 00 00 00 00 00 5f 01 00 00
>>>
>>> 01:00.1 Audio device: Advanced Micro Devices, Inc. [AMD/ATI] Turks
>>> HDMI Audio [Radeon HD 6500/6600 / 6700M Series]
>>> 00: 02 10 90 aa 06 00 10 20 00 00 03 04 00 00 80 00
>>> 10: 04 00 04 10 00 00 00 00 00 00 00 00 00 00 00 00
>>> 20: 00 00 00 00 00 00 00 00 00 00 00 00 28 10 90 aa
>>> 30: 00 00 00 00 50 00 00 00 00 00 00 00 ff 02 00 00
>>>
>>>>
>>>>
>>>>>>
>>>>>>> Also, if <0x82000000> (32 bit) is changed to <0x83000000> (64 bit),
>>>>>>> most of the allocations for the dGPU fail due to no valid regions
>>>>>>> available.
>>>>>>>
>>>>>>
>>>>>> But wasn't the original problem that the resource window was 64-bit to
>>>>>> begin with? Are you sure we are talking about the same problem here?
>>>>>
>>>>> The rk3399 in the original report has a 32MB memory window in the
>>>>> upper end of the 4GB range.
>>>>> The rk356x has a similar layout, or it can use a 1GB window available
>>>>> at <0x3 0x00000000>.
>>>>> Rockchip's default windows are defined as 64bit.
>>>>>
>>>>> The rk3399 doesn't have enough space to reasonably define two windows,
>>>>> one 32bit, one 64bit, to work around an allocation bug.
>>>>> These are the defined regions in the rk3399:
>>>>> ranges = <0x83000000 0x0 0xfa000000 0x0 0xfa000000 0x0 0x1e00000>,
>>>>> <0x81000000 0x0 0xfbe00000 0x0 0xfbe00000 0x0 0x100000>;
>>>>>
>>>>
>>>> All you really need is a 32-bit non-prefetchable resource window: any
>>>> BAR can be allocated from that. A 64-bit BAR can carry a 32-bit number
>>>> (just add zeroes at the top), and a prefetchable BAR can happily live
>>>> in a non-prefetchable window, with a theoretical performance impact if
>>>> the OS actually does use different memory attributes for the
>>>> prefetchable window (but I don't think Linux ever handles it this way)
>>>
>>> So is the IO range necessary as well or will it be automatically
>>> allocated as well?
>>>
>>>>
>>>>
>>>>>
>>>>>>
>>>>>>
>>>>>>>>
>>>>>>>>> I am happy to put something together once I understand the preferred way
>>>>>>>>> to go about it.
>>>>>>>>>
>>>>>>>>> Thanks,
>>>>>>>>> Punit
>>>>>>>>>
>>>>>>>>> [...]
>>>>>>>>>
>>>>>>>>
>>>>>>>> _______________________________________________
>>>>>>>> Linux-rockchip mailing list
>>>>>>>> Linux-rockchip@lists.infradead.org
>>>>>>>> http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [BUG] rockpro64: PCI BAR reassignment broken by commit 9d57e61bf723 ("of/pci: Add IORESOURCE_MEM_64 to resource flags for 64-bit memory addresses")
2021-05-25 15:54 ` Ard Biesheuvel
2021-05-25 16:23 ` Peter Geis
@ 2021-05-25 19:15 ` Bjorn Helgaas
2021-05-25 19:43 ` Ard Biesheuvel
1 sibling, 1 reply; 28+ messages in thread
From: Bjorn Helgaas @ 2021-05-25 19:15 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: Peter Geis, Punit Agrawal, Robin Murphy, Alexandru Elisei,
Linux Kernel Mailing List, open list:ARM/Rockchip SoC...,
arm-mail-list, Heiko Stuebner, Leonardo Bras, Rob Herring, PCI,
Christian König
On Tue, May 25, 2021 at 05:54:56PM +0200, Ard Biesheuvel wrote:
> On Tue, 25 May 2021 at 17:34, Peter Geis <pgwipeout@gmail.com> wrote:
> > > > >> > On 2021-05-18 10:09, Alexandru Elisei wrote:
> > > > >> >> [..]
> > > > >> >> [ 0.305183] rockchip-pcie f8000000.pcie: host bridge /pcie@f8000000 ranges:
> > > > >> >> [ 0.305248] rockchip-pcie f8000000.pcie: MEM 0x00fa000000..0x00fbdfffff -> 0x00fa000000
> > > > >> >> [ 0.305285] rockchip-pcie f8000000.pcie: IO 0x00fbe00000..0x00fbefffff -> 0x00fbe00000
> > > > >> >> [ 0.373705] rockchip-pcie f8000000.pcie: PCI host bridge to bus 0000:00
> > > > >> >> [ 0.373730] pci_bus 0000:00: root bus resource [bus 00-1f]
> > > > >> >> [ 0.373751] pci_bus 0000:00: root bus resource [mem 0xfa000000-0xfbdfffff 64bit]
> > > > >> >> [ 0.373777] pci_bus 0000:00: root bus resource [io 0x0000-0xfffff] (bus address [0xfbe00000-0xfbefffff])
> ... For some reason, lspci translates the BAR values to CPU
> addresses, but the PCI side addresses are within 32-bits.
lspci shows BARs as CPU physical addresses by default. These are the
same addresses you would see in pdev->resource[n] and the same as BAR
values you would see in dmesg.
A 64-bit CPU physical address can certainly be translated by the host
bridge to a 32-bit PCI address. But that's not happening here because
this host bridge applies no translation (CPU physical 0xfa000000 maps
to bus address 0xfa000000).
"lspci -b" shows the PCI bus addresses.
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [BUG] rockpro64: PCI BAR reassignment broken by commit 9d57e61bf723 ("of/pci: Add IORESOURCE_MEM_64 to resource flags for 64-bit memory addresses")
2021-05-25 19:15 ` Bjorn Helgaas
@ 2021-05-25 19:43 ` Ard Biesheuvel
2021-05-25 20:03 ` Peter Geis
0 siblings, 1 reply; 28+ messages in thread
From: Ard Biesheuvel @ 2021-05-25 19:43 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: Peter Geis, Punit Agrawal, Robin Murphy, Alexandru Elisei,
Linux Kernel Mailing List, open list:ARM/Rockchip SoC...,
arm-mail-list, Heiko Stuebner, Leonardo Bras, Rob Herring, PCI,
Christian König
On Tue, 25 May 2021 at 21:15, Bjorn Helgaas <helgaas@kernel.org> wrote:
>
> On Tue, May 25, 2021 at 05:54:56PM +0200, Ard Biesheuvel wrote:
> > On Tue, 25 May 2021 at 17:34, Peter Geis <pgwipeout@gmail.com> wrote:
>
> > > > > >> > On 2021-05-18 10:09, Alexandru Elisei wrote:
> > > > > >> >> [..]
> > > > > >> >> [ 0.305183] rockchip-pcie f8000000.pcie: host bridge /pcie@f8000000 ranges:
> > > > > >> >> [ 0.305248] rockchip-pcie f8000000.pcie: MEM 0x00fa000000..0x00fbdfffff -> 0x00fa000000
> > > > > >> >> [ 0.305285] rockchip-pcie f8000000.pcie: IO 0x00fbe00000..0x00fbefffff -> 0x00fbe00000
> > > > > >> >> [ 0.373705] rockchip-pcie f8000000.pcie: PCI host bridge to bus 0000:00
> > > > > >> >> [ 0.373730] pci_bus 0000:00: root bus resource [bus 00-1f]
> > > > > >> >> [ 0.373751] pci_bus 0000:00: root bus resource [mem 0xfa000000-0xfbdfffff 64bit]
> > > > > >> >> [ 0.373777] pci_bus 0000:00: root bus resource [io 0x0000-0xfffff] (bus address [0xfbe00000-0xfbefffff])
>
> > ... For some reason, lspci translates the BAR values to CPU
> > addresses, but the PCI side addresses are within 32-bits.
>
> lspci shows BARs as CPU physical addresses by default. These are the
> same addresses you would see in pdev->resource[n] and the same as BAR
> values you would see in dmesg.
>
> A 64-bit CPU physical address can certainly be translated by the host
> bridge to a 32-bit PCI address. But that's not happening here because
> this host bridge applies no translation (CPU physical 0xfa000000 maps
> to bus address 0xfa000000).
>
> "lspci -b" shows the PCI bus addresses.
Ah, thanks.
It does seem, though, that the information overload in this thread is
causing confusion now. Peter shared some log output where there is
definitely MMIO translation being applied.
> > [ 6.673497] pci_bus 0000:00: root bus resource [io 0x0000-0xfffff]
> > (bus address [0x3f700000-0x3f7fffff])
> > [ 6.674642] pci_bus 0000:00: root bus resource [mem
> > 0x300000000-0x33f6fffff] (bus address [0x00000000-0x3f6fffff])
In this case, the I/O translation definitely looks wrong. On a typical
ARM DT system, you will see something like
[ 1.500324] Remapped I/O 0x0000000067f00000 to [io 0x0000-0xffff window]
[ 1.500522] pci_bus 0000:00: root bus resource [io 0x0000-0xffff window]
The MMIO window looks correct, but I suspect that both 0x82000000 and
0x83000000 in the DT ranges are describing the resource window as
prefetchable, preventing the allocation of non-prefetchable BARs in
this window.
Peter, for the configuration listed here, could you try something like
ranges = <0x1000000 0x0 0x0 [IO base in the CPU address map] [IO size]>,
<0x2000000 0x0 0x0 [MMIO base in the CPU address map] [MMIO size]>;
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [BUG] rockpro64: PCI BAR reassignment broken by commit 9d57e61bf723 ("of/pci: Add IORESOURCE_MEM_64 to resource flags for 64-bit memory addresses")
2021-05-25 19:43 ` Ard Biesheuvel
@ 2021-05-25 20:03 ` Peter Geis
2021-05-26 14:18 ` Ard Biesheuvel
0 siblings, 1 reply; 28+ messages in thread
From: Peter Geis @ 2021-05-25 20:03 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: Bjorn Helgaas, Punit Agrawal, Robin Murphy, Alexandru Elisei,
Linux Kernel Mailing List, open list:ARM/Rockchip SoC...,
arm-mail-list, Heiko Stuebner, Leonardo Bras, Rob Herring, PCI,
Christian König
On Tue, May 25, 2021 at 3:43 PM Ard Biesheuvel <ardb@kernel.org> wrote:
>
> On Tue, 25 May 2021 at 21:15, Bjorn Helgaas <helgaas@kernel.org> wrote:
> >
> > On Tue, May 25, 2021 at 05:54:56PM +0200, Ard Biesheuvel wrote:
> > > On Tue, 25 May 2021 at 17:34, Peter Geis <pgwipeout@gmail.com> wrote:
> >
> > > > > > >> > On 2021-05-18 10:09, Alexandru Elisei wrote:
> > > > > > >> >> [..]
> > > > > > >> >> [ 0.305183] rockchip-pcie f8000000.pcie: host bridge /pcie@f8000000 ranges:
> > > > > > >> >> [ 0.305248] rockchip-pcie f8000000.pcie: MEM 0x00fa000000..0x00fbdfffff -> 0x00fa000000
> > > > > > >> >> [ 0.305285] rockchip-pcie f8000000.pcie: IO 0x00fbe00000..0x00fbefffff -> 0x00fbe00000
> > > > > > >> >> [ 0.373705] rockchip-pcie f8000000.pcie: PCI host bridge to bus 0000:00
> > > > > > >> >> [ 0.373730] pci_bus 0000:00: root bus resource [bus 00-1f]
> > > > > > >> >> [ 0.373751] pci_bus 0000:00: root bus resource [mem 0xfa000000-0xfbdfffff 64bit]
> > > > > > >> >> [ 0.373777] pci_bus 0000:00: root bus resource [io 0x0000-0xfffff] (bus address [0xfbe00000-0xfbefffff])
> >
> > > ... For some reason, lspci translates the BAR values to CPU
> > > addresses, but the PCI side addresses are within 32-bits.
> >
> > lspci shows BARs as CPU physical addresses by default. These are the
> > same addresses you would see in pdev->resource[n] and the same as BAR
> > values you would see in dmesg.
> >
> > A 64-bit CPU physical address can certainly be translated by the host
> > bridge to a 32-bit PCI address. But that's not happening here because
> > this host bridge applies no translation (CPU physical 0xfa000000 maps
> > to bus address 0xfa000000).
> >
> > "lspci -b" shows the PCI bus addresses.
>
> Ah, thanks.
>
> It does seem, though, that the information overload in this thread is
> causing confusion now. Peter shared some log output where there is
> definitely MMIO translation being applied.
Yes, I've done work on the rk3399 pcie controller which is why this
caught my attention.
The original issue still seems to exist:
For some reason:
commit 9d57e61bf723 ("of/pci: Add IORESOURCE_MEM_64 to resource flags for
64-bit memory addresses")
causes allocation issues now.
The original description of the issue aligned with issues I was having
bringing up the rk356x pcie controller.
>
> > > [ 6.673497] pci_bus 0000:00: root bus resource [io 0x0000-0xfffff]
> > > (bus address [0x3f700000-0x3f7fffff])
> > > [ 6.674642] pci_bus 0000:00: root bus resource [mem
> > > 0x300000000-0x33f6fffff] (bus address [0x00000000-0x3f6fffff])
>
> In this case, the I/O translation definitely looks wrong. On a typical
> ARM DT system, you will see something like
>
> [ 1.500324] Remapped I/O 0x0000000067f00000 to [io 0x0000-0xffff window]
> [ 1.500522] pci_bus 0000:00: root bus resource [io 0x0000-0xffff window]
>
> The MMIO window looks correct, but I suspect that both 0x82000000 and
> 0x83000000 in the DT ranges are describing the resource window as
> prefetchable, preventing the allocation of non-prefetchable BARs in
> this window.
I checked with lspci -vvvbxxxxnn:
Before your changes:
00:00.0 PCI bridge [0604]: Fuzhou Rockchip Electronics Co., Ltd Device
[1d87:3566] (rev 01) (prog-if 00 [Normal decode])
I/O behind bridge: 00001000-00001fff [size=4K]
Memory behind bridge: 50000000-500fffff [size=1M]
Prefetchable memory behind bridge:
0000000040000000-000000004fffffff [size=256M]
01:00.0 VGA compatible controller [0300]: Advanced Micro Devices, Inc.
[AMD/ATI] Turks PRO [Radeon HD 7570] [1002:675d] (prog-if 00 [VGA
controller])
Region 0: Memory at 40000000 (64-bit, prefetchable)
Region 2: Memory at 50000000 (64-bit, non-prefetchable)
Region 4: I/O ports at 7f701000
Expansion ROM at 50020000 [disabled]
After your changes:
lspci -vvvbxxxxnn
00:00.0 PCI bridge: Fuzhou Rockchip Electronics Co., Ltd Device 3566
(rev 01) (prog-if 00 [Normal decode])
I/O behind bridge: 00001000-00001fff [size=4K]
Memory behind bridge: 10000000-100fffff [size=1M]
Prefetchable memory behind bridge:
0000000000000000-000000000fffffff [size=256M]
01:00.0 VGA compatible controller: Advanced Micro Devices, Inc.
[AMD/ATI] Turks PRO [Radeon HD 7570] (prog-if 00 [VGA controller])
Region 0: Memory at <unassigned> (64-bit, prefetchable) [virtual]
Region 2: Memory at 10000000 (64-bit, non-prefetchable) [virtual]
Region 4: I/O ports at 1000 [virtual]
Expansion ROM at 10020000 [disabled]
>
> Peter, for the configuration listed here, could you try something like
>
> ranges = <0x1000000 0x0 0x0 [IO base in the CPU address map] [IO size]>,
> <0x2000000 0x0 0x0 [MMIO base in the CPU address map] [MMIO size]>;
That was similar to what I already had, removing the relocatable flag
and setting both addresses to 0x0 are the changes.
Here is the result:
[ 6.410670] rockchip-dw-pcie 3c0000000.pcie: Looking up
vpcie3v3-supply from device tree
[ 6.413924] rockchip-dw-pcie 3c0000000.pcie: host bridge
/pcie@fe260000 ranges:
[ 6.414653] rockchip-dw-pcie 3c0000000.pcie: Parsing ranges property...
[ 6.415321] rockchip-dw-pcie 3c0000000.pcie: IO
0x033f700000..0x033f7fffff -> 0x0000000000
[ 6.416375] rockchip-dw-pcie 3c0000000.pcie: MEM
0x0300000000..0x033f6fffff -> 0x0000000000
[ 6.417363] rockchip-dw-pcie 3c0000000.pcie: got 49 for legacy interrupt
[ 6.418676] rockchip-dw-pcie 3c0000000.pcie: found 5 interrupts
[ 6.419224] rockchip-dw-pcie 3c0000000.pcie: invalid resource
[ 6.419764] rockchip-dw-pcie 3c0000000.pcie: iATU unroll: enabled
[ 6.420388] rockchip-dw-pcie 3c0000000.pcie: Detected iATU regions:
8 outbound, 8 inbound
[ 6.628630] rockchip-dw-pcie 3c0000000.pcie: Link up
[ 6.631026] rockchip-dw-pcie 3c0000000.pcie: PCI host bridge to bus 0000:00
[ 6.631734] pci_bus 0000:00: root bus resource [bus 00]
[ 6.632548] pci_bus 0000:00: root bus resource [io 0x0000-0xfffff]
[ 6.633175] pci_bus 0000:00: root bus resource [mem
0x300000000-0x33f6fffff] (bus address [0x00000000-0x3f6fffff])
[ 6.634170] pci_bus 0000:00: scanning bus
[ 6.635052] pci 0000:00:00.0: disabling Extended Tags (this device
can't handle them)
[ 6.635813] pci 0000:00:00.0: [1d87:3566] type 01 class 0x060400
[ 6.636782] pci 0000:00:00.0: reg 0x38: [mem 0x300000000-0x30000ffff pref]
[ 6.638302] pci 0000:00:00.0: supports D1 D2
[ 6.638742] pci 0000:00:00.0: PME# supported from D0 D1 D3hot
[ 6.639432] pci 0000:00:00.0: PME# disabled
[ 6.650984] pci_bus 0000:00: fixups for bus
[ 6.651465] pci 0000:00:00.0: scanning [bus 01-ff] behind bridge, pass 0
[ 6.653454] pci_bus 0000:01: busn_res: can not insert [bus 01-ff]
under [bus 00] (conflicts with (null) [bus 00])
[ 6.654427] pci_bus 0000:01: scanning bus
[ 6.655134] pci 0000:01:00.0: [1002:675d] type 00 class 0x030000
[ 6.655938] pci 0000:01:00.0: reg 0x10: [mem
0x300000000-0x30fffffff 64bit pref]
[ 6.656934] pci 0000:01:00.0: reg 0x18: [mem 0x300000000-0x30001ffff 64bit]
[ 6.657677] pci 0000:01:00.0: reg 0x20: [io 0x0000-0x00ff]
[ 6.658363] pci 0000:01:00.0: reg 0x30: [mem 0x300000000-0x30001ffff pref]
[ 6.659845] pci 0000:01:00.0: supports D1 D2
[ 6.660715] pci 0000:01:00.0: 2.000 Gb/s available PCIe bandwidth,
limited by 2.5 GT/s PCIe x1 link at 0000:00:00.0 (capable of 32.000
Gb/s with 2.5 GT/s PCIe x16 link)
[ 6.664276] pci 0000:01:00.0: vgaarb: VGA device added:
decodes=io+mem,owns=none,locks=none
[ 6.666120] pci 0000:01:00.1: [1002:aa90] type 00 class 0x040300
[ 6.666922] pci 0000:01:00.1: reg 0x10: [mem 0x300000000-0x300003fff 64bit]
[ 6.668888] pci 0000:01:00.1: supports D1 D2
[ 6.680490] pci_bus 0000:01: fixups for bus
[ 6.680941] pci_bus 0000:01: bus scan returning with max=01
[ 6.681507] pci 0000:00:00.0: scanning [bus 01-ff] behind bridge, pass 1
[ 6.682180] pci_bus 0000:00: bus scan returning with max=ff
[ 6.682780] pci 0000:00:00.0: BAR 15: assigned [mem
0x300000000-0x30fffffff 64bit pref]
[ 6.683535] pci 0000:00:00.0: BAR 14: assigned [mem 0x310000000-0x3100fffff]
[ 6.684322] pci 0000:00:00.0: BAR 6: assigned [mem
0x310100000-0x31010ffff pref]
[ 6.685021] pci 0000:00:00.0: BAR 13: assigned [io 0x1000-0x1fff]
[ 6.685669] pci 0000:01:00.0: BAR 0: assigned [mem
0x300000000-0x30fffffff 64bit pref]
[ 6.686500] pci 0000:01:00.0: BAR 2: assigned [mem
0x310000000-0x31001ffff 64bit]
[ 6.687281] pci 0000:01:00.0: BAR 6: assigned [mem
0x310020000-0x31003ffff pref]
[ 6.687985] pci 0000:01:00.1: BAR 0: assigned [mem
0x310040000-0x310043fff 64bit]
[ 6.688872] pci 0000:01:00.0: BAR 4: assigned [io 0x1000-0x10ff]
[ 6.689492] pci 0000:00:00.0: PCI bridge to [bus 01-ff]
[ 6.690000] pci 0000:00:00.0: bridge window [io 0x1000-0x1fff]
[ 6.690593] pci 0000:00:00.0: bridge window [mem 0x310000000-0x3100fffff]
[ 6.691254] pci 0000:00:00.0: bridge window [mem
0x300000000-0x30fffffff 64bit pref]
[ 6.694100] pcieport 0000:00:00.0: assign IRQ: got 95
[ 6.695189] ITS ALLOCATE 2nd level WORKAROUND
[ 6.727845] pcieport 0000:00:00.0: PME: Signaling with IRQ 96
[ 6.729830] pcieport 0000:00:00.0: saving config space at offset
0x0 (reading 0x35661d87)
[ 6.730587] pcieport 0000:00:00.0: saving config space at offset
0x4 (reading 0x100507)
[ 6.731315] pcieport 0000:00:00.0: saving config space at offset
0x8 (reading 0x6040001)
[ 6.732051] pcieport 0000:00:00.0: saving config space at offset
0xc (reading 0x10000)
[ 6.733095] pcieport 0000:00:00.0: saving config space at offset
0x10 (reading 0x0)
[ 6.733797] pcieport 0000:00:00.0: saving config space at offset
0x14 (reading 0x0)
[ 6.734496] pcieport 0000:00:00.0: saving config space at offset
0x18 (reading 0xff0100)
[ 6.735231] pcieport 0000:00:00.0: saving config space at offset
0x1c (reading 0x20001010)
[ 6.735981] pcieport 0000:00:00.0: saving config space at offset
0x20 (reading 0x10001000)
[ 6.736786] pcieport 0000:00:00.0: saving config space at offset
0x24 (reading 0xff10001)
[ 6.737528] pcieport 0000:00:00.0: saving config space at offset
0x28 (reading 0x0)
[ 6.738223] pcieport 0000:00:00.0: saving config space at offset
0x2c (reading 0x0)
[ 6.738918] pcieport 0000:00:00.0: saving config space at offset
0x30 (reading 0x0)
[ 6.739613] pcieport 0000:00:00.0: saving config space at offset
0x34 (reading 0x40)
[ 6.740369] pcieport 0000:00:00.0: saving config space at offset
0x38 (reading 0x0)
[ 6.741067] pcieport 0000:00:00.0: saving config space at offset
0x3c (reading 0x2015f)
[ 6.742525] radeon 0000:01:00.0: assign IRQ: got 95
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [BUG] rockpro64: PCI BAR reassignment broken by commit 9d57e61bf723 ("of/pci: Add IORESOURCE_MEM_64 to resource flags for 64-bit memory addresses")
2021-05-25 20:03 ` Peter Geis
@ 2021-05-26 14:18 ` Ard Biesheuvel
0 siblings, 0 replies; 28+ messages in thread
From: Ard Biesheuvel @ 2021-05-26 14:18 UTC (permalink / raw)
To: Peter Geis
Cc: Bjorn Helgaas, Punit Agrawal, Robin Murphy, Alexandru Elisei,
Linux Kernel Mailing List, open list:ARM/Rockchip SoC...,
arm-mail-list, Heiko Stuebner, Leonardo Bras, Rob Herring, PCI,
Christian König
On Tue, 25 May 2021 at 22:03, Peter Geis <pgwipeout@gmail.com> wrote:
>
> On Tue, May 25, 2021 at 3:43 PM Ard Biesheuvel <ardb@kernel.org> wrote:
> >
> > On Tue, 25 May 2021 at 21:15, Bjorn Helgaas <helgaas@kernel.org> wrote:
> > >
> > > On Tue, May 25, 2021 at 05:54:56PM +0200, Ard Biesheuvel wrote:
> > > > On Tue, 25 May 2021 at 17:34, Peter Geis <pgwipeout@gmail.com> wrote:
> > >
> > > > > > > >> > On 2021-05-18 10:09, Alexandru Elisei wrote:
> > > > > > > >> >> [..]
> > > > > > > >> >> [ 0.305183] rockchip-pcie f8000000.pcie: host bridge /pcie@f8000000 ranges:
> > > > > > > >> >> [ 0.305248] rockchip-pcie f8000000.pcie: MEM 0x00fa000000..0x00fbdfffff -> 0x00fa000000
> > > > > > > >> >> [ 0.305285] rockchip-pcie f8000000.pcie: IO 0x00fbe00000..0x00fbefffff -> 0x00fbe00000
> > > > > > > >> >> [ 0.373705] rockchip-pcie f8000000.pcie: PCI host bridge to bus 0000:00
> > > > > > > >> >> [ 0.373730] pci_bus 0000:00: root bus resource [bus 00-1f]
> > > > > > > >> >> [ 0.373751] pci_bus 0000:00: root bus resource [mem 0xfa000000-0xfbdfffff 64bit]
> > > > > > > >> >> [ 0.373777] pci_bus 0000:00: root bus resource [io 0x0000-0xfffff] (bus address [0xfbe00000-0xfbefffff])
> > >
> > > > ... For some reason, lspci translates the BAR values to CPU
> > > > addresses, but the PCI side addresses are within 32-bits.
> > >
> > > lspci shows BARs as CPU physical addresses by default. These are the
> > > same addresses you would see in pdev->resource[n] and the same as BAR
> > > values you would see in dmesg.
> > >
> > > A 64-bit CPU physical address can certainly be translated by the host
> > > bridge to a 32-bit PCI address. But that's not happening here because
> > > this host bridge applies no translation (CPU physical 0xfa000000 maps
> > > to bus address 0xfa000000).
> > >
> > > "lspci -b" shows the PCI bus addresses.
> >
> > Ah, thanks.
> >
> > It does seem, though, that the information overload in this thread is
> > causing confusion now. Peter shared some log output where there is
> > definitely MMIO translation being applied.
>
> Yes, I've done work on the rk3399 pcie controller which is why this
> caught my attention.
> The original issue still seems to exist:
> For some reason:
> commit 9d57e61bf723 ("of/pci: Add IORESOURCE_MEM_64 to resource flags for
> 64-bit memory addresses")
> causes allocation issues now.
> The original description of the issue aligned with issues I was having
> bringing up the rk356x pcie controller.
>
> >
> > > > [ 6.673497] pci_bus 0000:00: root bus resource [io 0x0000-0xfffff]
> > > > (bus address [0x3f700000-0x3f7fffff])
> > > > [ 6.674642] pci_bus 0000:00: root bus resource [mem
> > > > 0x300000000-0x33f6fffff] (bus address [0x00000000-0x3f6fffff])
> >
> > In this case, the I/O translation definitely looks wrong. On a typical
> > ARM DT system, you will see something like
> >
> > [ 1.500324] Remapped I/O 0x0000000067f00000 to [io 0x0000-0xffff window]
> > [ 1.500522] pci_bus 0000:00: root bus resource [io 0x0000-0xffff window]
> >
> > The MMIO window looks correct, but I suspect that both 0x82000000 and
> > 0x83000000 in the DT ranges are describing the resource window as
> > prefetchable, preventing the allocation of non-prefetchable BARs in
> > this window.
>
> I checked with lspci -vvvbxxxxnn:
>
> Before your changes:
> 00:00.0 PCI bridge [0604]: Fuzhou Rockchip Electronics Co., Ltd Device
> [1d87:3566] (rev 01) (prog-if 00 [Normal decode])
> I/O behind bridge: 00001000-00001fff [size=4K]
> Memory behind bridge: 50000000-500fffff [size=1M]
> Prefetchable memory behind bridge:
> 0000000040000000-000000004fffffff [size=256M]
> 01:00.0 VGA compatible controller [0300]: Advanced Micro Devices, Inc.
> [AMD/ATI] Turks PRO [Radeon HD 7570] [1002:675d] (prog-if 00 [VGA
> controller])
> Region 0: Memory at 40000000 (64-bit, prefetchable)
> Region 2: Memory at 50000000 (64-bit, non-prefetchable)
> Region 4: I/O ports at 7f701000
> Expansion ROM at 50020000 [disabled]
>
> After your changes:
> lspci -vvvbxxxxnn
> 00:00.0 PCI bridge: Fuzhou Rockchip Electronics Co., Ltd Device 3566
> (rev 01) (prog-if 00 [Normal decode])
> I/O behind bridge: 00001000-00001fff [size=4K]
> Memory behind bridge: 10000000-100fffff [size=1M]
> Prefetchable memory behind bridge:
> 0000000000000000-000000000fffffff [size=256M]
> 01:00.0 VGA compatible controller: Advanced Micro Devices, Inc.
> [AMD/ATI] Turks PRO [Radeon HD 7570] (prog-if 00 [VGA controller])
> Region 0: Memory at <unassigned> (64-bit, prefetchable) [virtual]
> Region 2: Memory at 10000000 (64-bit, non-prefetchable) [virtual]
> Region 4: I/O ports at 1000 [virtual]
> Expansion ROM at 10020000 [disabled]
>
> >
> > Peter, for the configuration listed here, could you try something like
> >
> > ranges = <0x1000000 0x0 0x0 [IO base in the CPU address map] [IO size]>,
> > <0x2000000 0x0 0x0 [MMIO base in the CPU address map] [MMIO size]>;
>
> That was similar to what I already had, removing the relocatable flag
> and setting both addresses to 0x0 are the changes.
>
> Here is the result:
<snip>
I'm not sure which conclusion I am supposed to draw from that output.
Are you saying the output looks ok but the GPU card does not work?
Is your system DMA coherent? I would not be very optimistic about
these drivers working out of the box on non-coherent systems.
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [BUG] rockpro64: PCI BAR reassignment broken by commit 9d57e61bf723 ("of/pci: Add IORESOURCE_MEM_64 to resource flags for 64-bit memory addresses")
2021-05-25 13:54 ` Ard Biesheuvel
2021-05-25 15:34 ` Peter Geis
@ 2021-05-25 16:59 ` Anand Moon
2021-05-25 17:14 ` Robin Murphy
2021-05-25 22:36 ` Punit Agrawal
2021-05-26 15:37 ` Rob Herring
3 siblings, 1 reply; 28+ messages in thread
From: Anand Moon @ 2021-05-25 16:59 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: Punit Agrawal, Robin Murphy, Alexandru Elisei,
Linux Kernel Mailing List, linux-rockchip, arm-mail-list,
Heiko Stuebner, leobras.c, Rob Herring, PCI
Hi Ard,
On Tue, 25 May 2021 at 19:27, Ard Biesheuvel <ardb@kernel.org> wrote:
>
> On Tue, 25 May 2021 at 15:42, Punit Agrawal <punitagrawal@gmail.com> wrote:
> >
> > Hi Ard,
> >
> > Ard Biesheuvel <ardb@kernel.org> writes:
> >
> > > On Sun, 23 May 2021 at 13:06, Punit Agrawal <punitagrawal@gmail.com> wrote:
> > >>
> > >> Robin Murphy <robin.murphy@arm.com> writes:
> > >>
> > >> > [ +linux-pci for visibility ]
> > >> >
> > >> > On 2021-05-18 10:09, Alexandru Elisei wrote:
> > >> >> After doing a git bisect I was able to trace the following error when booting my
> > >> >> rockpro64 v2 (rk3399 SoC) with a PCIE NVME expansion card:
> > >> >> [..]
> > >> >> [ 0.305183] rockchip-pcie f8000000.pcie: host bridge /pcie@f8000000 ranges:
> > >> >> [ 0.305248] rockchip-pcie f8000000.pcie: MEM 0x00fa000000..0x00fbdfffff ->
> > >> >> 0x00fa000000
> > >> >> [ 0.305285] rockchip-pcie f8000000.pcie: IO 0x00fbe00000..0x00fbefffff ->
> > >> >> 0x00fbe00000
> > >> >> [ 0.306201] rockchip-pcie f8000000.pcie: supply vpcie1v8 not found, using dummy
> > >> >> regulator
> > >> >> [ 0.306334] rockchip-pcie f8000000.pcie: supply vpcie0v9 not found, using dummy
> > >> >> regulator
> > >> >> [ 0.373705] rockchip-pcie f8000000.pcie: PCI host bridge to bus 0000:00
> > >> >> [ 0.373730] pci_bus 0000:00: root bus resource [bus 00-1f]
> > >> >> [ 0.373751] pci_bus 0000:00: root bus resource [mem 0xfa000000-0xfbdfffff 64bit]
> > >> >> [ 0.373777] pci_bus 0000:00: root bus resource [io 0x0000-0xfffff] (bus
> > >> >> address [0xfbe00000-0xfbefffff])
> > >> >> [ 0.373839] pci 0000:00:00.0: [1d87:0100] type 01 class 0x060400
> > >> >> [ 0.373973] pci 0000:00:00.0: supports D1
> > >> >> [ 0.373992] pci 0000:00:00.0: PME# supported from D0 D1 D3hot
> > >> >> [ 0.378518] pci 0000:00:00.0: bridge configuration invalid ([bus 00-00]),
> > >> >> reconfiguring
> > >> >> [ 0.378765] pci 0000:01:00.0: [144d:a808] type 00 class 0x010802
> > >> >> [ 0.378869] pci 0000:01:00.0: reg 0x10: [mem 0x00000000-0x00003fff 64bit]
> > >> >> [ 0.379051] pci 0000:01:00.0: Max Payload Size set to 256 (was 128, max 256)
> > >> >> [ 0.379661] pci 0000:01:00.0: 8.000 Gb/s available PCIe bandwidth, limited by
> > >> >> 2.5 GT/s PCIe x4 link at 0000:00:00.0 (capable of 31.504 Gb/s with 8.0 GT/s PCIe
> > >> >> x4 link)
> > >> >> [ 0.393269] pci_bus 0000:01: busn_res: [bus 01-1f] end is updated to 01
> > >> >> [ 0.393311] pci 0000:00:00.0: BAR 14: no space for [mem size 0x00100000]
> > >> >> [ 0.393333] pci 0000:00:00.0: BAR 14: failed to assign [mem size 0x00100000]
> > >> >> [ 0.393356] pci 0000:01:00.0: BAR 0: no space for [mem size 0x00004000 64bit]
> > >> >> [ 0.393375] pci 0000:01:00.0: BAR 0: failed to assign [mem size 0x00004000 64bit]
> > >> >> [ 0.393397] pci 0000:00:00.0: PCI bridge to [bus 01]
> > >> >> [ 0.393839] pcieport 0000:00:00.0: PME: Signaling with IRQ 78
> > >> >> [ 0.394165] pcieport 0000:00:00.0: AER: enabled with IRQ 78
> > >> >> [..]
> > >> >> to the commit 9d57e61bf723 ("of/pci: Add IORESOURCE_MEM_64 to
> > >> >> resource flags for
> > >> >> 64-bit memory addresses").
> > >> >
> > >> > FWFW, my hunch is that the host bridge advertising no 32-bit memory
> > >> > resource, only only a single 64-bit non-prefetchable one (even though
> > >> > it's entirely below 4GB) might be a bit weird and tripping something
> > >> > up in the resource assignment code. It certainly seems like the thing
> > >> > most directly related to the offending commit.
> > >> >
> > >> > I'd be tempted to try fiddling with that in the DT (i.e. changing
> > >> > 0x83000000 to 0x82000000 in the PCIe node's "ranges" property) to see
> > >> > if it makes any difference. Note that even if it helps, though, I
> > >> > don't know whether that's the correct fix or just a bodge around a
> > >> > corner-case bug somewhere in the resource code.
> > >>
> > >> From digging into this further the failure seems to be due to a mismatch
> > >> of flags when allocating resources in pci_bus_alloc_from_region() -
> > >>
> > >> if ((res->flags ^ r->flags) & type_mask)
> > >> continue;
> > >>
> > >> Though I am also not sure why the failure is only being reported on
> > >> RK3399 - does a single 64-bit window have anything to do with it?
> > >>
> > >
> > > The NVMe in the example exposes a single 64-bit non-prefetchable BAR.
> > > Such BARs can not be allocated in a prefetchable host bridge window
> > > (unlike the converse, i.e., allocating a prefetchable BAR in a
> > > non-prefetchable host bridge window is fine)
> > >
> > > 64-bit non-prefetchable host bridge windows cannot be forwarded by PCI
> > > to PCI bridges, they simply lack the BAR registers to describe them.
> > > Therefore, non-prefetchable endpoint BARs (even 64-bit ones) need to
> > > be carved out of a host bridge's non-prefetchable 32-bit window if
> > > they need to pass through a bridge.
> >
> > Thank you for the explanation. I also looked at the PCI-to-PCI Bridge
> > spec to understand where some of the limitations are coming from.
> >
> > > So the error seems to be here that the host bridge's 32-bit
> > > non-prefetchable window has the 64-bit attribute set, even though it
> > > resides below 4 GB entirely. I suppose that the resource allocation
> > > could be made more forgiving (and it was in the past, before commit
> > > 9d57e61bf723 was applied). However, I would strongly recommend not
> > > deviating from common practice, and just describe the 32-bit
> > > addressable non-prefetchable resource window as such.
> >
> > IIUC, the host bridge's configuration (64-bit on non-prefetchable
> > window) is based on what the hardware advertises.
> >
>
> What do you mean by 'what the hardware advertises'? The host bridge is
> apparently configured to decode a 32-bit addressable window as MMIO,
> and the question is why this window has the 64-bit attribute set in
> the DT description.
>
> > Can you elaborate on what you have in mind to correct the
> > non-prefetchable resource window? Are you thinking of adding a quirk
> > somewhere to address this?
> >
>
> No. Just fix the DT.
Yes DTS changes are needed as well as some more core driver changes.
As per the Rk3399 TRM (Rockchip RK3399 TRM V1.3 Part2.pdf)
[0] https://rockchip.fr/Rockchip%20RK3399%20TRM%20V1.3%20Part2.pdf
I had made the following dts changes relates to ranges as per PCI below.
*17.6.1 Internal Register Address Mapping
Table 17-23 Global Address Map for Core Local Management*
diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
index 634a91af8e83..796b44e07be1 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
@@ -199,7 +199,7 @@ xin24m: xin24m {
pcie0: pcie@f8000000 {
compatible = "rockchip,rk3399-pcie";
- reg = <0x0 0xf8000000 0x0 0x2000000>,
+ reg = <0x0 0xf8000000 0x0 0x6000000>,
<0x0 0xfd000000 0x0 0x1000000>;
reg-names = "axi-base", "apb-base";
device_type = "pci";
@@ -227,8 +227,8 @@ pcie0: pcie@f8000000 {
<&pcie_phy 2>, <&pcie_phy 3>;
phy-names = "pcie-phy-0", "pcie-phy-1",
"pcie-phy-2", "pcie-phy-3";
- ranges = <0x83000000 0x0 0xfa000000 0x0 0xfa000000 0x0
0x1e00000>,
- <0x81000000 0x0 0xfbe00000 0x0 0xfbe00000 0x0
0x100000>;
+ ranges = <0x83000000 0x0 0xfd800000 0x0 0xfd810000 0x0
0x100000>,
+ <0x81000000 0x0 0xfd800000 0x0 0xfda00000 0x0
0x100000>;
resets = <&cru SRST_PCIE_CORE>, <&cru SRST_PCIE_MGMT>,
<&cru SRST_PCIE_MGMT_STICKY>, <&cru SRST_PCIE_PIPE>,
<&cru SRST_PCIE_PM>, <&cru SRST_P_PCIE>,
@@ -2040,6 +2040,21 @@ pcfg_pull_up_2ma: pcfg-pull-up-2ma {
drive-strength = <2>;
Also, the BAR configuration is missing some tuning bits missing,
* 17.6.7.1.45 Root Complex BAR Configuration Register.*
Earlier I had to face this issue on my Rk3399 board (Odroid n1), but I
could not resolve the issue.
[1] https://patchwork.kernel.org/project/linux-rockchip/patch/1590023130-137406-1-git-send-email-shawn.lin@rock-chips.com/
How can I debug the PCIe handshake messages to explore further?
[alarm@alarm ~]$ dmesg | grep pci
[ 1.399919] ehci-pci: EHCI PCI platform driver
[ 1.538434] ohci-pci: OHCI PCI platform driver
[ 7.112556] rockchip-pcie f8000000.pcie: host bridge /pcie@f8000000 ranges:
[ 7.120583] rockchip-pcie f8000000.pcie: Parsing ranges property...
[ 7.134628] rockchip-pcie f8000000.pcie: MEM
0x00fd810000..0x00fd90ffff -> 0x00fd800000
[ 7.144148] rockchip-pcie f8000000.pcie: IO
0x00fda00000..0x00fdafffff -> 0x00fd800000
[ 7.165435] rockchip-pcie f8000000.pcie: can't request region for
resource [mem 0xf8000000-0xfdffffff]
[ 7.182904] rockchip-pcie: probe of f8000000.pcie failed with error -16
Best Regards
-Anand
>
> > I am happy to put something together once I understand the preferred way
> > to go about it.
> >
> > Thanks,
> > Punit
> >
> > [...]
> >
>
> _______________________________________________
> Linux-rockchip mailing list
> Linux-rockchip@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply related [flat|nested] 28+ messages in thread* Re: [BUG] rockpro64: PCI BAR reassignment broken by commit 9d57e61bf723 ("of/pci: Add IORESOURCE_MEM_64 to resource flags for 64-bit memory addresses")
2021-05-25 16:59 ` Anand Moon
@ 2021-05-25 17:14 ` Robin Murphy
2021-05-25 17:42 ` Peter Geis
0 siblings, 1 reply; 28+ messages in thread
From: Robin Murphy @ 2021-05-25 17:14 UTC (permalink / raw)
To: Anand Moon, Ard Biesheuvel
Cc: Punit Agrawal, Alexandru Elisei, Linux Kernel Mailing List,
linux-rockchip, arm-mail-list, Heiko Stuebner, leobras.c,
Rob Herring, PCI
On 2021-05-25 17:59, Anand Moon wrote:
> Hi Ard,
>
> On Tue, 25 May 2021 at 19:27, Ard Biesheuvel <ardb@kernel.org> wrote:
>>
>> On Tue, 25 May 2021 at 15:42, Punit Agrawal <punitagrawal@gmail.com> wrote:
>>>
>>> Hi Ard,
>>>
>>> Ard Biesheuvel <ardb@kernel.org> writes:
>>>
>>>> On Sun, 23 May 2021 at 13:06, Punit Agrawal <punitagrawal@gmail.com> wrote:
>>>>>
>>>>> Robin Murphy <robin.murphy@arm.com> writes:
>>>>>
>>>>>> [ +linux-pci for visibility ]
>>>>>>
>>>>>> On 2021-05-18 10:09, Alexandru Elisei wrote:
>>>>>>> After doing a git bisect I was able to trace the following error when booting my
>>>>>>> rockpro64 v2 (rk3399 SoC) with a PCIE NVME expansion card:
>>>>>>> [..]
>>>>>>> [ 0.305183] rockchip-pcie f8000000.pcie: host bridge /pcie@f8000000 ranges:
>>>>>>> [ 0.305248] rockchip-pcie f8000000.pcie: MEM 0x00fa000000..0x00fbdfffff ->
>>>>>>> 0x00fa000000
>>>>>>> [ 0.305285] rockchip-pcie f8000000.pcie: IO 0x00fbe00000..0x00fbefffff ->
>>>>>>> 0x00fbe00000
>>>>>>> [ 0.306201] rockchip-pcie f8000000.pcie: supply vpcie1v8 not found, using dummy
>>>>>>> regulator
>>>>>>> [ 0.306334] rockchip-pcie f8000000.pcie: supply vpcie0v9 not found, using dummy
>>>>>>> regulator
>>>>>>> [ 0.373705] rockchip-pcie f8000000.pcie: PCI host bridge to bus 0000:00
>>>>>>> [ 0.373730] pci_bus 0000:00: root bus resource [bus 00-1f]
>>>>>>> [ 0.373751] pci_bus 0000:00: root bus resource [mem 0xfa000000-0xfbdfffff 64bit]
>>>>>>> [ 0.373777] pci_bus 0000:00: root bus resource [io 0x0000-0xfffff] (bus
>>>>>>> address [0xfbe00000-0xfbefffff])
>>>>>>> [ 0.373839] pci 0000:00:00.0: [1d87:0100] type 01 class 0x060400
>>>>>>> [ 0.373973] pci 0000:00:00.0: supports D1
>>>>>>> [ 0.373992] pci 0000:00:00.0: PME# supported from D0 D1 D3hot
>>>>>>> [ 0.378518] pci 0000:00:00.0: bridge configuration invalid ([bus 00-00]),
>>>>>>> reconfiguring
>>>>>>> [ 0.378765] pci 0000:01:00.0: [144d:a808] type 00 class 0x010802
>>>>>>> [ 0.378869] pci 0000:01:00.0: reg 0x10: [mem 0x00000000-0x00003fff 64bit]
>>>>>>> [ 0.379051] pci 0000:01:00.0: Max Payload Size set to 256 (was 128, max 256)
>>>>>>> [ 0.379661] pci 0000:01:00.0: 8.000 Gb/s available PCIe bandwidth, limited by
>>>>>>> 2.5 GT/s PCIe x4 link at 0000:00:00.0 (capable of 31.504 Gb/s with 8.0 GT/s PCIe
>>>>>>> x4 link)
>>>>>>> [ 0.393269] pci_bus 0000:01: busn_res: [bus 01-1f] end is updated to 01
>>>>>>> [ 0.393311] pci 0000:00:00.0: BAR 14: no space for [mem size 0x00100000]
>>>>>>> [ 0.393333] pci 0000:00:00.0: BAR 14: failed to assign [mem size 0x00100000]
>>>>>>> [ 0.393356] pci 0000:01:00.0: BAR 0: no space for [mem size 0x00004000 64bit]
>>>>>>> [ 0.393375] pci 0000:01:00.0: BAR 0: failed to assign [mem size 0x00004000 64bit]
>>>>>>> [ 0.393397] pci 0000:00:00.0: PCI bridge to [bus 01]
>>>>>>> [ 0.393839] pcieport 0000:00:00.0: PME: Signaling with IRQ 78
>>>>>>> [ 0.394165] pcieport 0000:00:00.0: AER: enabled with IRQ 78
>>>>>>> [..]
>>>>>>> to the commit 9d57e61bf723 ("of/pci: Add IORESOURCE_MEM_64 to
>>>>>>> resource flags for
>>>>>>> 64-bit memory addresses").
>>>>>>
>>>>>> FWFW, my hunch is that the host bridge advertising no 32-bit memory
>>>>>> resource, only only a single 64-bit non-prefetchable one (even though
>>>>>> it's entirely below 4GB) might be a bit weird and tripping something
>>>>>> up in the resource assignment code. It certainly seems like the thing
>>>>>> most directly related to the offending commit.
>>>>>>
>>>>>> I'd be tempted to try fiddling with that in the DT (i.e. changing
>>>>>> 0x83000000 to 0x82000000 in the PCIe node's "ranges" property) to see
>>>>>> if it makes any difference. Note that even if it helps, though, I
>>>>>> don't know whether that's the correct fix or just a bodge around a
>>>>>> corner-case bug somewhere in the resource code.
>>>>>
>>>>> From digging into this further the failure seems to be due to a mismatch
>>>>> of flags when allocating resources in pci_bus_alloc_from_region() -
>>>>>
>>>>> if ((res->flags ^ r->flags) & type_mask)
>>>>> continue;
>>>>>
>>>>> Though I am also not sure why the failure is only being reported on
>>>>> RK3399 - does a single 64-bit window have anything to do with it?
>>>>>
>>>>
>>>> The NVMe in the example exposes a single 64-bit non-prefetchable BAR.
>>>> Such BARs can not be allocated in a prefetchable host bridge window
>>>> (unlike the converse, i.e., allocating a prefetchable BAR in a
>>>> non-prefetchable host bridge window is fine)
>>>>
>>>> 64-bit non-prefetchable host bridge windows cannot be forwarded by PCI
>>>> to PCI bridges, they simply lack the BAR registers to describe them.
>>>> Therefore, non-prefetchable endpoint BARs (even 64-bit ones) need to
>>>> be carved out of a host bridge's non-prefetchable 32-bit window if
>>>> they need to pass through a bridge.
>>>
>>> Thank you for the explanation. I also looked at the PCI-to-PCI Bridge
>>> spec to understand where some of the limitations are coming from.
>>>
>>>> So the error seems to be here that the host bridge's 32-bit
>>>> non-prefetchable window has the 64-bit attribute set, even though it
>>>> resides below 4 GB entirely. I suppose that the resource allocation
>>>> could be made more forgiving (and it was in the past, before commit
>>>> 9d57e61bf723 was applied). However, I would strongly recommend not
>>>> deviating from common practice, and just describe the 32-bit
>>>> addressable non-prefetchable resource window as such.
>>>
>>> IIUC, the host bridge's configuration (64-bit on non-prefetchable
>>> window) is based on what the hardware advertises.
>>>
>>
>> What do you mean by 'what the hardware advertises'? The host bridge is
>> apparently configured to decode a 32-bit addressable window as MMIO,
>> and the question is why this window has the 64-bit attribute set in
>> the DT description.
>>
>>> Can you elaborate on what you have in mind to correct the
>>> non-prefetchable resource window? Are you thinking of adding a quirk
>>> somewhere to address this?
>>>
>>
>> No. Just fix the DT.
>
> Yes DTS changes are needed as well as some more core driver changes.
>
> As per the Rk3399 TRM (Rockchip RK3399 TRM V1.3 Part2.pdf)
> [0] https://rockchip.fr/Rockchip%20RK3399%20TRM%20V1.3%20Part2.pdf
>
> I had made the following dts changes relates to ranges as per PCI below.
>
> *17.6.1 Internal Register Address Mapping
> Table 17-23 Global Address Map for Core Local Management*
>
> diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
> b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
> index 634a91af8e83..796b44e07be1 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
> +++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
> @@ -199,7 +199,7 @@ xin24m: xin24m {
>
> pcie0: pcie@f8000000 {
> compatible = "rockchip,rk3399-pcie";
> - reg = <0x0 0xf8000000 0x0 0x2000000>,
> + reg = <0x0 0xf8000000 0x0 0x6000000>,
> <0x0 0xfd000000 0x0 0x1000000>;
> reg-names = "axi-base", "apb-base";
> device_type = "pci";
> @@ -227,8 +227,8 @@ pcie0: pcie@f8000000 {
> <&pcie_phy 2>, <&pcie_phy 3>;
> phy-names = "pcie-phy-0", "pcie-phy-1",
> "pcie-phy-2", "pcie-phy-3";
> - ranges = <0x83000000 0x0 0xfa000000 0x0 0xfa000000 0x0
> 0x1e00000>,
> - <0x81000000 0x0 0xfbe00000 0x0 0xfbe00000 0x0
> 0x100000>;
> + ranges = <0x83000000 0x0 0xfd800000 0x0 0xfd810000 0x0
> 0x100000>,
> + <0x81000000 0x0 0xfd800000 0x0 0xfda00000 0x0
> 0x100000>;
> resets = <&cru SRST_PCIE_CORE>, <&cru SRST_PCIE_MGMT>,
> <&cru SRST_PCIE_MGMT_STICKY>, <&cru SRST_PCIE_PIPE>,
> <&cru SRST_PCIE_PM>, <&cru SRST_P_PCIE>,
> @@ -2040,6 +2040,21 @@ pcfg_pull_up_2ma: pcfg-pull-up-2ma {
> drive-strength = <2>;
>
> Also, the BAR configuration is missing some tuning bits missing,
> * 17.6.7.1.45 Root Complex BAR Configuration Register.*
>
> Earlier I had to face this issue on my Rk3399 board (Odroid n1), but I
> could not resolve the issue.
>
> [1] https://patchwork.kernel.org/project/linux-rockchip/patch/1590023130-137406-1-git-send-email-shawn.lin@rock-chips.com/
>
> How can I debug the PCIe handshake messages to explore further?
>
> [alarm@alarm ~]$ dmesg | grep pci
> [ 1.399919] ehci-pci: EHCI PCI platform driver
> [ 1.538434] ohci-pci: OHCI PCI platform driver
> [ 7.112556] rockchip-pcie f8000000.pcie: host bridge /pcie@f8000000 ranges:
> [ 7.120583] rockchip-pcie f8000000.pcie: Parsing ranges property...
> [ 7.134628] rockchip-pcie f8000000.pcie: MEM
> 0x00fd810000..0x00fd90ffff -> 0x00fd800000
> [ 7.144148] rockchip-pcie f8000000.pcie: IO
> 0x00fda00000..0x00fdafffff -> 0x00fd800000
> [ 7.165435] rockchip-pcie f8000000.pcie: can't request region for
> resource [mem 0xf8000000-0xfdffffff]
> [ 7.182904] rockchip-pcie: probe of f8000000.pcie failed with error -16
Sorry, you've changed your DT for unknown reasons to put the memory and
I/O windows at the same bus address, and now you want help debugging why
trying to put two things at the same address gives -EBUSY?
:/
Is it Friday already?
Robin.
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [BUG] rockpro64: PCI BAR reassignment broken by commit 9d57e61bf723 ("of/pci: Add IORESOURCE_MEM_64 to resource flags for 64-bit memory addresses")
2021-05-25 17:14 ` Robin Murphy
@ 2021-05-25 17:42 ` Peter Geis
0 siblings, 0 replies; 28+ messages in thread
From: Peter Geis @ 2021-05-25 17:42 UTC (permalink / raw)
To: Robin Murphy
Cc: Anand Moon, Ard Biesheuvel, Punit Agrawal, Alexandru Elisei,
Linux Kernel Mailing List, open list:ARM/Rockchip SoC...,
arm-mail-list, Heiko Stuebner, Leonardo Bras, Rob Herring, PCI
On Tue, May 25, 2021 at 1:18 PM Robin Murphy <robin.murphy@arm.com> wrote:
>
> On 2021-05-25 17:59, Anand Moon wrote:
> > Hi Ard,
> >
> > On Tue, 25 May 2021 at 19:27, Ard Biesheuvel <ardb@kernel.org> wrote:
> >>
> >> On Tue, 25 May 2021 at 15:42, Punit Agrawal <punitagrawal@gmail.com> wrote:
> >>>
> >>> Hi Ard,
> >>>
> >>> Ard Biesheuvel <ardb@kernel.org> writes:
> >>>
> >>>> On Sun, 23 May 2021 at 13:06, Punit Agrawal <punitagrawal@gmail.com> wrote:
> >>>>>
> >>>>> Robin Murphy <robin.murphy@arm.com> writes:
> >>>>>
> >>>>>> [ +linux-pci for visibility ]
> >>>>>>
> >>>>>> On 2021-05-18 10:09, Alexandru Elisei wrote:
> >>>>>>> After doing a git bisect I was able to trace the following error when booting my
> >>>>>>> rockpro64 v2 (rk3399 SoC) with a PCIE NVME expansion card:
> >>>>>>> [..]
> >>>>>>> [ 0.305183] rockchip-pcie f8000000.pcie: host bridge /pcie@f8000000 ranges:
> >>>>>>> [ 0.305248] rockchip-pcie f8000000.pcie: MEM 0x00fa000000..0x00fbdfffff ->
> >>>>>>> 0x00fa000000
> >>>>>>> [ 0.305285] rockchip-pcie f8000000.pcie: IO 0x00fbe00000..0x00fbefffff ->
> >>>>>>> 0x00fbe00000
> >>>>>>> [ 0.306201] rockchip-pcie f8000000.pcie: supply vpcie1v8 not found, using dummy
> >>>>>>> regulator
> >>>>>>> [ 0.306334] rockchip-pcie f8000000.pcie: supply vpcie0v9 not found, using dummy
> >>>>>>> regulator
> >>>>>>> [ 0.373705] rockchip-pcie f8000000.pcie: PCI host bridge to bus 0000:00
> >>>>>>> [ 0.373730] pci_bus 0000:00: root bus resource [bus 00-1f]
> >>>>>>> [ 0.373751] pci_bus 0000:00: root bus resource [mem 0xfa000000-0xfbdfffff 64bit]
> >>>>>>> [ 0.373777] pci_bus 0000:00: root bus resource [io 0x0000-0xfffff] (bus
> >>>>>>> address [0xfbe00000-0xfbefffff])
> >>>>>>> [ 0.373839] pci 0000:00:00.0: [1d87:0100] type 01 class 0x060400
> >>>>>>> [ 0.373973] pci 0000:00:00.0: supports D1
> >>>>>>> [ 0.373992] pci 0000:00:00.0: PME# supported from D0 D1 D3hot
> >>>>>>> [ 0.378518] pci 0000:00:00.0: bridge configuration invalid ([bus 00-00]),
> >>>>>>> reconfiguring
> >>>>>>> [ 0.378765] pci 0000:01:00.0: [144d:a808] type 00 class 0x010802
> >>>>>>> [ 0.378869] pci 0000:01:00.0: reg 0x10: [mem 0x00000000-0x00003fff 64bit]
> >>>>>>> [ 0.379051] pci 0000:01:00.0: Max Payload Size set to 256 (was 128, max 256)
> >>>>>>> [ 0.379661] pci 0000:01:00.0: 8.000 Gb/s available PCIe bandwidth, limited by
> >>>>>>> 2.5 GT/s PCIe x4 link at 0000:00:00.0 (capable of 31.504 Gb/s with 8.0 GT/s PCIe
> >>>>>>> x4 link)
> >>>>>>> [ 0.393269] pci_bus 0000:01: busn_res: [bus 01-1f] end is updated to 01
> >>>>>>> [ 0.393311] pci 0000:00:00.0: BAR 14: no space for [mem size 0x00100000]
> >>>>>>> [ 0.393333] pci 0000:00:00.0: BAR 14: failed to assign [mem size 0x00100000]
> >>>>>>> [ 0.393356] pci 0000:01:00.0: BAR 0: no space for [mem size 0x00004000 64bit]
> >>>>>>> [ 0.393375] pci 0000:01:00.0: BAR 0: failed to assign [mem size 0x00004000 64bit]
> >>>>>>> [ 0.393397] pci 0000:00:00.0: PCI bridge to [bus 01]
> >>>>>>> [ 0.393839] pcieport 0000:00:00.0: PME: Signaling with IRQ 78
> >>>>>>> [ 0.394165] pcieport 0000:00:00.0: AER: enabled with IRQ 78
> >>>>>>> [..]
> >>>>>>> to the commit 9d57e61bf723 ("of/pci: Add IORESOURCE_MEM_64 to
> >>>>>>> resource flags for
> >>>>>>> 64-bit memory addresses").
> >>>>>>
> >>>>>> FWFW, my hunch is that the host bridge advertising no 32-bit memory
> >>>>>> resource, only only a single 64-bit non-prefetchable one (even though
> >>>>>> it's entirely below 4GB) might be a bit weird and tripping something
> >>>>>> up in the resource assignment code. It certainly seems like the thing
> >>>>>> most directly related to the offending commit.
> >>>>>>
> >>>>>> I'd be tempted to try fiddling with that in the DT (i.e. changing
> >>>>>> 0x83000000 to 0x82000000 in the PCIe node's "ranges" property) to see
> >>>>>> if it makes any difference. Note that even if it helps, though, I
> >>>>>> don't know whether that's the correct fix or just a bodge around a
> >>>>>> corner-case bug somewhere in the resource code.
> >>>>>
> >>>>> From digging into this further the failure seems to be due to a mismatch
> >>>>> of flags when allocating resources in pci_bus_alloc_from_region() -
> >>>>>
> >>>>> if ((res->flags ^ r->flags) & type_mask)
> >>>>> continue;
> >>>>>
> >>>>> Though I am also not sure why the failure is only being reported on
> >>>>> RK3399 - does a single 64-bit window have anything to do with it?
> >>>>>
> >>>>
> >>>> The NVMe in the example exposes a single 64-bit non-prefetchable BAR.
> >>>> Such BARs can not be allocated in a prefetchable host bridge window
> >>>> (unlike the converse, i.e., allocating a prefetchable BAR in a
> >>>> non-prefetchable host bridge window is fine)
> >>>>
> >>>> 64-bit non-prefetchable host bridge windows cannot be forwarded by PCI
> >>>> to PCI bridges, they simply lack the BAR registers to describe them.
> >>>> Therefore, non-prefetchable endpoint BARs (even 64-bit ones) need to
> >>>> be carved out of a host bridge's non-prefetchable 32-bit window if
> >>>> they need to pass through a bridge.
> >>>
> >>> Thank you for the explanation. I also looked at the PCI-to-PCI Bridge
> >>> spec to understand where some of the limitations are coming from.
> >>>
> >>>> So the error seems to be here that the host bridge's 32-bit
> >>>> non-prefetchable window has the 64-bit attribute set, even though it
> >>>> resides below 4 GB entirely. I suppose that the resource allocation
> >>>> could be made more forgiving (and it was in the past, before commit
> >>>> 9d57e61bf723 was applied). However, I would strongly recommend not
> >>>> deviating from common practice, and just describe the 32-bit
> >>>> addressable non-prefetchable resource window as such.
> >>>
> >>> IIUC, the host bridge's configuration (64-bit on non-prefetchable
> >>> window) is based on what the hardware advertises.
> >>>
> >>
> >> What do you mean by 'what the hardware advertises'? The host bridge is
> >> apparently configured to decode a 32-bit addressable window as MMIO,
> >> and the question is why this window has the 64-bit attribute set in
> >> the DT description.
> >>
> >>> Can you elaborate on what you have in mind to correct the
> >>> non-prefetchable resource window? Are you thinking of adding a quirk
> >>> somewhere to address this?
> >>>
> >>
> >> No. Just fix the DT.
> >
> > Yes DTS changes are needed as well as some more core driver changes.
> >
> > As per the Rk3399 TRM (Rockchip RK3399 TRM V1.3 Part2.pdf)
> > [0] https://rockchip.fr/Rockchip%20RK3399%20TRM%20V1.3%20Part2.pdf
> >
> > I had made the following dts changes relates to ranges as per PCI below.
> >
> > *17.6.1 Internal Register Address Mapping
> > Table 17-23 Global Address Map for Core Local Management*
> >
> > diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
> > b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
> > index 634a91af8e83..796b44e07be1 100644
> > --- a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
> > +++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
> > @@ -199,7 +199,7 @@ xin24m: xin24m {
> >
> > pcie0: pcie@f8000000 {
> > compatible = "rockchip,rk3399-pcie";
> > - reg = <0x0 0xf8000000 0x0 0x2000000>,
> > + reg = <0x0 0xf8000000 0x0 0x6000000>,
> > <0x0 0xfd000000 0x0 0x1000000>;
> > reg-names = "axi-base", "apb-base";
> > device_type = "pci";
> > @@ -227,8 +227,8 @@ pcie0: pcie@f8000000 {
> > <&pcie_phy 2>, <&pcie_phy 3>;
> > phy-names = "pcie-phy-0", "pcie-phy-1",
> > "pcie-phy-2", "pcie-phy-3";
> > - ranges = <0x83000000 0x0 0xfa000000 0x0 0xfa000000 0x0
> > 0x1e00000>,
> > - <0x81000000 0x0 0xfbe00000 0x0 0xfbe00000 0x0
> > 0x100000>;
> > + ranges = <0x83000000 0x0 0xfd800000 0x0 0xfd810000 0x0
> > 0x100000>,
> > + <0x81000000 0x0 0xfd800000 0x0 0xfda00000 0x0
> > 0x100000>;
> > resets = <&cru SRST_PCIE_CORE>, <&cru SRST_PCIE_MGMT>,
> > <&cru SRST_PCIE_MGMT_STICKY>, <&cru SRST_PCIE_PIPE>,
> > <&cru SRST_PCIE_PM>, <&cru SRST_P_PCIE>,
> > @@ -2040,6 +2040,21 @@ pcfg_pull_up_2ma: pcfg-pull-up-2ma {
> > drive-strength = <2>;
> >
> > Also, the BAR configuration is missing some tuning bits missing,
> > * 17.6.7.1.45 Root Complex BAR Configuration Register.*
> >
> > Earlier I had to face this issue on my Rk3399 board (Odroid n1), but I
> > could not resolve the issue.
> >
> > [1] https://patchwork.kernel.org/project/linux-rockchip/patch/1590023130-137406-1-git-send-email-shawn.lin@rock-chips.com/
> >
> > How can I debug the PCIe handshake messages to explore further?
> >
> > [alarm@alarm ~]$ dmesg | grep pci
> > [ 1.399919] ehci-pci: EHCI PCI platform driver
> > [ 1.538434] ohci-pci: OHCI PCI platform driver
> > [ 7.112556] rockchip-pcie f8000000.pcie: host bridge /pcie@f8000000 ranges:
> > [ 7.120583] rockchip-pcie f8000000.pcie: Parsing ranges property...
> > [ 7.134628] rockchip-pcie f8000000.pcie: MEM
> > 0x00fd810000..0x00fd90ffff -> 0x00fd800000
> > [ 7.144148] rockchip-pcie f8000000.pcie: IO
> > 0x00fda00000..0x00fdafffff -> 0x00fd800000
> > [ 7.165435] rockchip-pcie f8000000.pcie: can't request region for
> > resource [mem 0xf8000000-0xfdffffff]
> > [ 7.182904] rockchip-pcie: probe of f8000000.pcie failed with error -16
>
> Sorry, you've changed your DT for unknown reasons to put the memory and
> I/O windows at the same bus address, and now you want help debugging why
> trying to put two things at the same address gives -EBUSY?
>
> :/
>
> Is it Friday already?
I wish, I don't know where they pulled those values from but they
could never work.
Anand Moon:
The apb registers are fine, please don't try to increase them.
By doing so you have now reserved that memory space to the driver and
it cannot be assigned to the bridge.
The rk3399 has a single 32MB memory space and 32 1MB memory spaces.
Only the single large coherent memory space is supported, since using
the other 32 spaces would involve a complicated memory dance that the
driver doesn't support.
The recommendation was changing the PCIe memory space from 64-bit to
32-bit, by changing 0x83000000 to 0x82000000.
>
> Robin.
>
> _______________________________________________
> Linux-rockchip mailing list
> Linux-rockchip@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [BUG] rockpro64: PCI BAR reassignment broken by commit 9d57e61bf723 ("of/pci: Add IORESOURCE_MEM_64 to resource flags for 64-bit memory addresses")
2021-05-25 13:54 ` Ard Biesheuvel
2021-05-25 15:34 ` Peter Geis
2021-05-25 16:59 ` Anand Moon
@ 2021-05-25 22:36 ` Punit Agrawal
2021-05-26 15:37 ` Rob Herring
3 siblings, 0 replies; 28+ messages in thread
From: Punit Agrawal @ 2021-05-25 22:36 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: Robin Murphy, Alexandru Elisei, Linux Kernel Mailing List,
linux-rockchip, arm-mail-list, Heiko Stuebner, leobras.c,
Rob Herring, PCI
Ard Biesheuvel <ardb@kernel.org> writes:
> On Tue, 25 May 2021 at 15:42, Punit Agrawal <punitagrawal@gmail.com> wrote:
>>
>> Hi Ard,
>>
>> Ard Biesheuvel <ardb@kernel.org> writes:
>>
>> > On Sun, 23 May 2021 at 13:06, Punit Agrawal <punitagrawal@gmail.com> wrote:
>> >>
>> >> Robin Murphy <robin.murphy@arm.com> writes:
>> >>
>> >> > [ +linux-pci for visibility ]
>> >> >
>> >> > On 2021-05-18 10:09, Alexandru Elisei wrote:
>> >> >> After doing a git bisect I was able to trace the following error when booting my
>> >> >> rockpro64 v2 (rk3399 SoC) with a PCIE NVME expansion card:
>> >> >> [..]
>> >> >> [ 0.305183] rockchip-pcie f8000000.pcie: host bridge /pcie@f8000000 ranges:
>> >> >> [ 0.305248] rockchip-pcie f8000000.pcie: MEM 0x00fa000000..0x00fbdfffff ->
>> >> >> 0x00fa000000
>> >> >> [ 0.305285] rockchip-pcie f8000000.pcie: IO 0x00fbe00000..0x00fbefffff ->
>> >> >> 0x00fbe00000
>> >> >> [ 0.306201] rockchip-pcie f8000000.pcie: supply vpcie1v8 not found, using dummy
>> >> >> regulator
>> >> >> [ 0.306334] rockchip-pcie f8000000.pcie: supply vpcie0v9 not found, using dummy
>> >> >> regulator
>> >> >> [ 0.373705] rockchip-pcie f8000000.pcie: PCI host bridge to bus 0000:00
>> >> >> [ 0.373730] pci_bus 0000:00: root bus resource [bus 00-1f]
>> >> >> [ 0.373751] pci_bus 0000:00: root bus resource [mem 0xfa000000-0xfbdfffff 64bit]
>> >> >> [ 0.373777] pci_bus 0000:00: root bus resource [io 0x0000-0xfffff] (bus
>> >> >> address [0xfbe00000-0xfbefffff])
>> >> >> [ 0.373839] pci 0000:00:00.0: [1d87:0100] type 01 class 0x060400
>> >> >> [ 0.373973] pci 0000:00:00.0: supports D1
>> >> >> [ 0.373992] pci 0000:00:00.0: PME# supported from D0 D1 D3hot
>> >> >> [ 0.378518] pci 0000:00:00.0: bridge configuration invalid ([bus 00-00]),
>> >> >> reconfiguring
>> >> >> [ 0.378765] pci 0000:01:00.0: [144d:a808] type 00 class 0x010802
>> >> >> [ 0.378869] pci 0000:01:00.0: reg 0x10: [mem 0x00000000-0x00003fff 64bit]
>> >> >> [ 0.379051] pci 0000:01:00.0: Max Payload Size set to 256 (was 128, max 256)
>> >> >> [ 0.379661] pci 0000:01:00.0: 8.000 Gb/s available PCIe bandwidth, limited by
>> >> >> 2.5 GT/s PCIe x4 link at 0000:00:00.0 (capable of 31.504 Gb/s with 8.0 GT/s PCIe
>> >> >> x4 link)
>> >> >> [ 0.393269] pci_bus 0000:01: busn_res: [bus 01-1f] end is updated to 01
>> >> >> [ 0.393311] pci 0000:00:00.0: BAR 14: no space for [mem size 0x00100000]
>> >> >> [ 0.393333] pci 0000:00:00.0: BAR 14: failed to assign [mem size 0x00100000]
>> >> >> [ 0.393356] pci 0000:01:00.0: BAR 0: no space for [mem size 0x00004000 64bit]
>> >> >> [ 0.393375] pci 0000:01:00.0: BAR 0: failed to assign [mem size 0x00004000 64bit]
>> >> >> [ 0.393397] pci 0000:00:00.0: PCI bridge to [bus 01]
>> >> >> [ 0.393839] pcieport 0000:00:00.0: PME: Signaling with IRQ 78
>> >> >> [ 0.394165] pcieport 0000:00:00.0: AER: enabled with IRQ 78
>> >> >> [..]
>> >> >> to the commit 9d57e61bf723 ("of/pci: Add IORESOURCE_MEM_64 to
>> >> >> resource flags for
>> >> >> 64-bit memory addresses").
>> >> >
>> >> > FWFW, my hunch is that the host bridge advertising no 32-bit memory
>> >> > resource, only only a single 64-bit non-prefetchable one (even though
>> >> > it's entirely below 4GB) might be a bit weird and tripping something
>> >> > up in the resource assignment code. It certainly seems like the thing
>> >> > most directly related to the offending commit.
>> >> >
>> >> > I'd be tempted to try fiddling with that in the DT (i.e. changing
>> >> > 0x83000000 to 0x82000000 in the PCIe node's "ranges" property) to see
>> >> > if it makes any difference. Note that even if it helps, though, I
>> >> > don't know whether that's the correct fix or just a bodge around a
>> >> > corner-case bug somewhere in the resource code.
>> >>
>> >> From digging into this further the failure seems to be due to a mismatch
>> >> of flags when allocating resources in pci_bus_alloc_from_region() -
>> >>
>> >> if ((res->flags ^ r->flags) & type_mask)
>> >> continue;
>> >>
>> >> Though I am also not sure why the failure is only being reported on
>> >> RK3399 - does a single 64-bit window have anything to do with it?
>> >>
>> >
>> > The NVMe in the example exposes a single 64-bit non-prefetchable BAR.
>> > Such BARs can not be allocated in a prefetchable host bridge window
>> > (unlike the converse, i.e., allocating a prefetchable BAR in a
>> > non-prefetchable host bridge window is fine)
>> >
>> > 64-bit non-prefetchable host bridge windows cannot be forwarded by PCI
>> > to PCI bridges, they simply lack the BAR registers to describe them.
>> > Therefore, non-prefetchable endpoint BARs (even 64-bit ones) need to
>> > be carved out of a host bridge's non-prefetchable 32-bit window if
>> > they need to pass through a bridge.
>>
>> Thank you for the explanation. I also looked at the PCI-to-PCI Bridge
>> spec to understand where some of the limitations are coming from.
>>
>> > So the error seems to be here that the host bridge's 32-bit
>> > non-prefetchable window has the 64-bit attribute set, even though it
>> > resides below 4 GB entirely. I suppose that the resource allocation
>> > could be made more forgiving (and it was in the past, before commit
>> > 9d57e61bf723 was applied). However, I would strongly recommend not
>> > deviating from common practice, and just describe the 32-bit
>> > addressable non-prefetchable resource window as such.
>>
>> IIUC, the host bridge's configuration (64-bit on non-prefetchable
>> window) is based on what the hardware advertises.
>>
>
> What do you mean by 'what the hardware advertises'? The host bridge is
> apparently configured to decode a 32-bit addressable window as MMIO,
> and the question is why this window has the 64-bit attribute set in
> the DT description.
Right - I completely missed the fact that the ranges property is also
encoding the window attributes. Thanks for setting me straight.
git archaeology doesn't provide any explanation - I am wondering if it
is just an oversight.
>> Can you elaborate on what you have in mind to correct the
>> non-prefetchable resource window? Are you thinking of adding a quirk
>> somewhere to address this?
>>
>
> No. Just fix the DT.
After updating the DT to mark the non-prefetchable window as 32-bit
things work as expected.
Let me send a patch to update the DT - I'll include previous authors
who've touched that DT fragment. Hopefully somebody will jump in to
explain the reason it was done that way.
Thanks,
Punit
[...]
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [BUG] rockpro64: PCI BAR reassignment broken by commit 9d57e61bf723 ("of/pci: Add IORESOURCE_MEM_64 to resource flags for 64-bit memory addresses")
2021-05-25 13:54 ` Ard Biesheuvel
` (2 preceding siblings ...)
2021-05-25 22:36 ` Punit Agrawal
@ 2021-05-26 15:37 ` Rob Herring
2021-05-26 16:35 ` Ard Biesheuvel
3 siblings, 1 reply; 28+ messages in thread
From: Rob Herring @ 2021-05-26 15:37 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: Punit Agrawal, Robin Murphy, Alexandru Elisei,
Linux Kernel Mailing List, linux-rockchip, arm-mail-list,
Heiko Stuebner, leobras.c, PCI
On Tue, May 25, 2021 at 03:54:30PM +0200, Ard Biesheuvel wrote:
> On Tue, 25 May 2021 at 15:42, Punit Agrawal <punitagrawal@gmail.com> wrote:
> >
> > Hi Ard,
> >
> > Ard Biesheuvel <ardb@kernel.org> writes:
> >
> > > On Sun, 23 May 2021 at 13:06, Punit Agrawal <punitagrawal@gmail.com> wrote:
> > >>
> > >> Robin Murphy <robin.murphy@arm.com> writes:
> > >>
> > >> > [ +linux-pci for visibility ]
> > >> >
> > >> > On 2021-05-18 10:09, Alexandru Elisei wrote:
> > >> >> After doing a git bisect I was able to trace the following error when booting my
> > >> >> rockpro64 v2 (rk3399 SoC) with a PCIE NVME expansion card:
> > >> >> [..]
> > >> >> [ 0.305183] rockchip-pcie f8000000.pcie: host bridge /pcie@f8000000 ranges:
> > >> >> [ 0.305248] rockchip-pcie f8000000.pcie: MEM 0x00fa000000..0x00fbdfffff ->
> > >> >> 0x00fa000000
> > >> >> [ 0.305285] rockchip-pcie f8000000.pcie: IO 0x00fbe00000..0x00fbefffff ->
> > >> >> 0x00fbe00000
> > >> >> [ 0.306201] rockchip-pcie f8000000.pcie: supply vpcie1v8 not found, using dummy
> > >> >> regulator
> > >> >> [ 0.306334] rockchip-pcie f8000000.pcie: supply vpcie0v9 not found, using dummy
> > >> >> regulator
> > >> >> [ 0.373705] rockchip-pcie f8000000.pcie: PCI host bridge to bus 0000:00
> > >> >> [ 0.373730] pci_bus 0000:00: root bus resource [bus 00-1f]
> > >> >> [ 0.373751] pci_bus 0000:00: root bus resource [mem 0xfa000000-0xfbdfffff 64bit]
> > >> >> [ 0.373777] pci_bus 0000:00: root bus resource [io 0x0000-0xfffff] (bus
> > >> >> address [0xfbe00000-0xfbefffff])
> > >> >> [ 0.373839] pci 0000:00:00.0: [1d87:0100] type 01 class 0x060400
> > >> >> [ 0.373973] pci 0000:00:00.0: supports D1
> > >> >> [ 0.373992] pci 0000:00:00.0: PME# supported from D0 D1 D3hot
> > >> >> [ 0.378518] pci 0000:00:00.0: bridge configuration invalid ([bus 00-00]),
> > >> >> reconfiguring
> > >> >> [ 0.378765] pci 0000:01:00.0: [144d:a808] type 00 class 0x010802
> > >> >> [ 0.378869] pci 0000:01:00.0: reg 0x10: [mem 0x00000000-0x00003fff 64bit]
> > >> >> [ 0.379051] pci 0000:01:00.0: Max Payload Size set to 256 (was 128, max 256)
> > >> >> [ 0.379661] pci 0000:01:00.0: 8.000 Gb/s available PCIe bandwidth, limited by
> > >> >> 2.5 GT/s PCIe x4 link at 0000:00:00.0 (capable of 31.504 Gb/s with 8.0 GT/s PCIe
> > >> >> x4 link)
> > >> >> [ 0.393269] pci_bus 0000:01: busn_res: [bus 01-1f] end is updated to 01
> > >> >> [ 0.393311] pci 0000:00:00.0: BAR 14: no space for [mem size 0x00100000]
> > >> >> [ 0.393333] pci 0000:00:00.0: BAR 14: failed to assign [mem size 0x00100000]
> > >> >> [ 0.393356] pci 0000:01:00.0: BAR 0: no space for [mem size 0x00004000 64bit]
> > >> >> [ 0.393375] pci 0000:01:00.0: BAR 0: failed to assign [mem size 0x00004000 64bit]
> > >> >> [ 0.393397] pci 0000:00:00.0: PCI bridge to [bus 01]
> > >> >> [ 0.393839] pcieport 0000:00:00.0: PME: Signaling with IRQ 78
> > >> >> [ 0.394165] pcieport 0000:00:00.0: AER: enabled with IRQ 78
> > >> >> [..]
> > >> >> to the commit 9d57e61bf723 ("of/pci: Add IORESOURCE_MEM_64 to
> > >> >> resource flags for
> > >> >> 64-bit memory addresses").
> > >> >
> > >> > FWFW, my hunch is that the host bridge advertising no 32-bit memory
> > >> > resource, only only a single 64-bit non-prefetchable one (even though
> > >> > it's entirely below 4GB) might be a bit weird and tripping something
> > >> > up in the resource assignment code. It certainly seems like the thing
> > >> > most directly related to the offending commit.
> > >> >
> > >> > I'd be tempted to try fiddling with that in the DT (i.e. changing
> > >> > 0x83000000 to 0x82000000 in the PCIe node's "ranges" property) to see
> > >> > if it makes any difference. Note that even if it helps, though, I
> > >> > don't know whether that's the correct fix or just a bodge around a
> > >> > corner-case bug somewhere in the resource code.
> > >>
> > >> From digging into this further the failure seems to be due to a mismatch
> > >> of flags when allocating resources in pci_bus_alloc_from_region() -
> > >>
> > >> if ((res->flags ^ r->flags) & type_mask)
> > >> continue;
> > >>
> > >> Though I am also not sure why the failure is only being reported on
> > >> RK3399 - does a single 64-bit window have anything to do with it?
> > >>
> > >
> > > The NVMe in the example exposes a single 64-bit non-prefetchable BAR.
> > > Such BARs can not be allocated in a prefetchable host bridge window
> > > (unlike the converse, i.e., allocating a prefetchable BAR in a
> > > non-prefetchable host bridge window is fine)
> > >
> > > 64-bit non-prefetchable host bridge windows cannot be forwarded by PCI
> > > to PCI bridges, they simply lack the BAR registers to describe them.
> > > Therefore, non-prefetchable endpoint BARs (even 64-bit ones) need to
> > > be carved out of a host bridge's non-prefetchable 32-bit window if
> > > they need to pass through a bridge.
> >
> > Thank you for the explanation. I also looked at the PCI-to-PCI Bridge
> > spec to understand where some of the limitations are coming from.
> >
> > > So the error seems to be here that the host bridge's 32-bit
> > > non-prefetchable window has the 64-bit attribute set, even though it
> > > resides below 4 GB entirely. I suppose that the resource allocation
> > > could be made more forgiving (and it was in the past, before commit
> > > 9d57e61bf723 was applied). However, I would strongly recommend not
> > > deviating from common practice, and just describe the 32-bit
> > > addressable non-prefetchable resource window as such.
> >
> > IIUC, the host bridge's configuration (64-bit on non-prefetchable
> > window) is based on what the hardware advertises.
> >
>
> What do you mean by 'what the hardware advertises'? The host bridge is
> apparently configured to decode a 32-bit addressable window as MMIO,
> and the question is why this window has the 64-bit attribute set in
> the DT description.
>
> > Can you elaborate on what you have in mind to correct the
> > non-prefetchable resource window? Are you thinking of adding a quirk
> > somewhere to address this?
> >
>
> No. Just fix the DT.
We can't only fix the DT as we shouldn't require a DT update due to a
kernel change. Especially for RK3399 which is pretty stable and widely
used.
Do I understand correctly that 64-bit non-prefetchable never correct? We
recently gained a warning for this in commit fede8526cc48 ("PCI: of:
Warn if non-prefetchable memory aperture size is > 32-bit"), but that
only looks at addresses, not the 64-bit flag. Can't we just not set
the 64-bit flag if non-prefetchable?
BTW, the DT schema is checking ranges hi cell:
- 0x01000000
- 0x02000000
- 0x03000000
- 0x42000000
- 0x43000000
- 0x81000000
- 0x82000000
- 0x83000000
- 0xc2000000
- 0xc3000000
I just went with what I found in dts files. Sounds like 03 and 83 should
be removed. (Really, bit 31 or relocatable should probably not be set
either.)
There's a number of other cases:
Documentation/devicetree/bindings/numa.txt: ranges = <0x03000000 0x8010 0x00000000 0x8010 0x00000000 0x70 0x00000000>;
Documentation/devicetree/bindings/numa.txt: ranges = <0x03000000 0x9010 0x00000000 0x9010 0x00000000 0x70 0x00000000>;
Documentation/devicetree/bindings/pci/mediatek-pcie.txt- 0x83000000 0 0x60000000 0 0x60000000 0 0x10000000>; /* memory space */
Documentation/devicetree/bindings/pci/microchip,pcie-host.yaml: ranges = <0x03000000 0x0 0x78000000 0x0 0x78000000 0x0 0x04000000>;
Documentation/devicetree/bindings/pci/mobiveil-pcie.txt: ranges = < 0x83000000 0 0x00000000 0xa8000000 0 0x8000000>;
Documentation/devicetree/bindings/pci/rockchip-pcie-host.txt: ranges = <0x83000000 0x0 0xfa000000 0x0 0xfa000000 0x0 0x600000
arch/arm/boot/dts/mt7623.dtsi- 0x83000000 0 0x60000000 0 0x60000000 0 0x10000000>;
arch/arm64/boot/dts/broadcom/northstar2/ns2.dtsi: ranges = <0x83000000 0 0x00000000 0 0x00000000 0 0x20000000>;
arch/arm64/boot/dts/broadcom/northstar2/ns2.dtsi: ranges = <0x83000000 0 0x00000000 0 0x30000000 0 0x20000000>;
arch/arm64/boot/dts/broadcom/northstar2/ns2.dtsi: ranges = <0x83000000 0 0x00000000 0 0x60000000 0 0x00c00000>;
arch/arm64/boot/dts/broadcom/stingray/stingray-pcie.dtsi: ranges = <0x83000000 0 0x10000000 0 0x10000000 0 0x20000000>;
arch/arm64/boot/dts/rockchip/rk3399.dtsi: ranges = <0x83000000 0x0 0xfa000000 0x0 0xfa000000 0x0 0x1e00000>,
Rob
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [BUG] rockpro64: PCI BAR reassignment broken by commit 9d57e61bf723 ("of/pci: Add IORESOURCE_MEM_64 to resource flags for 64-bit memory addresses")
2021-05-26 15:37 ` Rob Herring
@ 2021-05-26 16:35 ` Ard Biesheuvel
0 siblings, 0 replies; 28+ messages in thread
From: Ard Biesheuvel @ 2021-05-26 16:35 UTC (permalink / raw)
To: Rob Herring
Cc: Punit Agrawal, Robin Murphy, Alexandru Elisei,
Linux Kernel Mailing List, linux-rockchip, arm-mail-list,
Heiko Stuebner, leobras.c, PCI
On Wed, 26 May 2021 at 17:38, Rob Herring <robh@kernel.org> wrote:
>
> On Tue, May 25, 2021 at 03:54:30PM +0200, Ard Biesheuvel wrote:
> > On Tue, 25 May 2021 at 15:42, Punit Agrawal <punitagrawal@gmail.com> wrote:
> > >
> > > Hi Ard,
> > >
> > > Ard Biesheuvel <ardb@kernel.org> writes:
> > >
> > > > On Sun, 23 May 2021 at 13:06, Punit Agrawal <punitagrawal@gmail.com> wrote:
> > > >>
> > > >> Robin Murphy <robin.murphy@arm.com> writes:
> > > >>
> > > >> > [ +linux-pci for visibility ]
> > > >> >
> > > >> > On 2021-05-18 10:09, Alexandru Elisei wrote:
> > > >> >> After doing a git bisect I was able to trace the following error when booting my
> > > >> >> rockpro64 v2 (rk3399 SoC) with a PCIE NVME expansion card:
> > > >> >> [..]
> > > >> >> [ 0.305183] rockchip-pcie f8000000.pcie: host bridge /pcie@f8000000 ranges:
> > > >> >> [ 0.305248] rockchip-pcie f8000000.pcie: MEM 0x00fa000000..0x00fbdfffff ->
> > > >> >> 0x00fa000000
> > > >> >> [ 0.305285] rockchip-pcie f8000000.pcie: IO 0x00fbe00000..0x00fbefffff ->
> > > >> >> 0x00fbe00000
> > > >> >> [ 0.306201] rockchip-pcie f8000000.pcie: supply vpcie1v8 not found, using dummy
> > > >> >> regulator
> > > >> >> [ 0.306334] rockchip-pcie f8000000.pcie: supply vpcie0v9 not found, using dummy
> > > >> >> regulator
> > > >> >> [ 0.373705] rockchip-pcie f8000000.pcie: PCI host bridge to bus 0000:00
> > > >> >> [ 0.373730] pci_bus 0000:00: root bus resource [bus 00-1f]
> > > >> >> [ 0.373751] pci_bus 0000:00: root bus resource [mem 0xfa000000-0xfbdfffff 64bit]
> > > >> >> [ 0.373777] pci_bus 0000:00: root bus resource [io 0x0000-0xfffff] (bus
> > > >> >> address [0xfbe00000-0xfbefffff])
> > > >> >> [ 0.373839] pci 0000:00:00.0: [1d87:0100] type 01 class 0x060400
> > > >> >> [ 0.373973] pci 0000:00:00.0: supports D1
> > > >> >> [ 0.373992] pci 0000:00:00.0: PME# supported from D0 D1 D3hot
> > > >> >> [ 0.378518] pci 0000:00:00.0: bridge configuration invalid ([bus 00-00]),
> > > >> >> reconfiguring
> > > >> >> [ 0.378765] pci 0000:01:00.0: [144d:a808] type 00 class 0x010802
> > > >> >> [ 0.378869] pci 0000:01:00.0: reg 0x10: [mem 0x00000000-0x00003fff 64bit]
> > > >> >> [ 0.379051] pci 0000:01:00.0: Max Payload Size set to 256 (was 128, max 256)
> > > >> >> [ 0.379661] pci 0000:01:00.0: 8.000 Gb/s available PCIe bandwidth, limited by
> > > >> >> 2.5 GT/s PCIe x4 link at 0000:00:00.0 (capable of 31.504 Gb/s with 8.0 GT/s PCIe
> > > >> >> x4 link)
> > > >> >> [ 0.393269] pci_bus 0000:01: busn_res: [bus 01-1f] end is updated to 01
> > > >> >> [ 0.393311] pci 0000:00:00.0: BAR 14: no space for [mem size 0x00100000]
> > > >> >> [ 0.393333] pci 0000:00:00.0: BAR 14: failed to assign [mem size 0x00100000]
> > > >> >> [ 0.393356] pci 0000:01:00.0: BAR 0: no space for [mem size 0x00004000 64bit]
> > > >> >> [ 0.393375] pci 0000:01:00.0: BAR 0: failed to assign [mem size 0x00004000 64bit]
> > > >> >> [ 0.393397] pci 0000:00:00.0: PCI bridge to [bus 01]
> > > >> >> [ 0.393839] pcieport 0000:00:00.0: PME: Signaling with IRQ 78
> > > >> >> [ 0.394165] pcieport 0000:00:00.0: AER: enabled with IRQ 78
> > > >> >> [..]
> > > >> >> to the commit 9d57e61bf723 ("of/pci: Add IORESOURCE_MEM_64 to
> > > >> >> resource flags for
> > > >> >> 64-bit memory addresses").
> > > >> >
> > > >> > FWFW, my hunch is that the host bridge advertising no 32-bit memory
> > > >> > resource, only only a single 64-bit non-prefetchable one (even though
> > > >> > it's entirely below 4GB) might be a bit weird and tripping something
> > > >> > up in the resource assignment code. It certainly seems like the thing
> > > >> > most directly related to the offending commit.
> > > >> >
> > > >> > I'd be tempted to try fiddling with that in the DT (i.e. changing
> > > >> > 0x83000000 to 0x82000000 in the PCIe node's "ranges" property) to see
> > > >> > if it makes any difference. Note that even if it helps, though, I
> > > >> > don't know whether that's the correct fix or just a bodge around a
> > > >> > corner-case bug somewhere in the resource code.
> > > >>
> > > >> From digging into this further the failure seems to be due to a mismatch
> > > >> of flags when allocating resources in pci_bus_alloc_from_region() -
> > > >>
> > > >> if ((res->flags ^ r->flags) & type_mask)
> > > >> continue;
> > > >>
> > > >> Though I am also not sure why the failure is only being reported on
> > > >> RK3399 - does a single 64-bit window have anything to do with it?
> > > >>
> > > >
> > > > The NVMe in the example exposes a single 64-bit non-prefetchable BAR.
> > > > Such BARs can not be allocated in a prefetchable host bridge window
> > > > (unlike the converse, i.e., allocating a prefetchable BAR in a
> > > > non-prefetchable host bridge window is fine)
> > > >
> > > > 64-bit non-prefetchable host bridge windows cannot be forwarded by PCI
> > > > to PCI bridges, they simply lack the BAR registers to describe them.
> > > > Therefore, non-prefetchable endpoint BARs (even 64-bit ones) need to
> > > > be carved out of a host bridge's non-prefetchable 32-bit window if
> > > > they need to pass through a bridge.
> > >
> > > Thank you for the explanation. I also looked at the PCI-to-PCI Bridge
> > > spec to understand where some of the limitations are coming from.
> > >
> > > > So the error seems to be here that the host bridge's 32-bit
> > > > non-prefetchable window has the 64-bit attribute set, even though it
> > > > resides below 4 GB entirely. I suppose that the resource allocation
> > > > could be made more forgiving (and it was in the past, before commit
> > > > 9d57e61bf723 was applied). However, I would strongly recommend not
> > > > deviating from common practice, and just describe the 32-bit
> > > > addressable non-prefetchable resource window as such.
> > >
> > > IIUC, the host bridge's configuration (64-bit on non-prefetchable
> > > window) is based on what the hardware advertises.
> > >
> >
> > What do you mean by 'what the hardware advertises'? The host bridge is
> > apparently configured to decode a 32-bit addressable window as MMIO,
> > and the question is why this window has the 64-bit attribute set in
> > the DT description.
> >
> > > Can you elaborate on what you have in mind to correct the
> > > non-prefetchable resource window? Are you thinking of adding a quirk
> > > somewhere to address this?
> > >
> >
> > No. Just fix the DT.
>
> We can't only fix the DT as we shouldn't require a DT update due to a
> kernel change. Especially for RK3399 which is pretty stable and widely
> used.
>
> Do I understand correctly that 64-bit non-prefetchable never correct?
No. It just makes little sense for PCIe, given that root ports are
modeled as PCI bridges, which cannot forward non-prefetchable 64-bit
windows. But a funky root complex with integrated endpoints as well as
root ports might have some use for it, I suppose, so we cannot rule it
out.
The bottom line is really that, given that 64-bit BARs can be
allocated in 32-bit space, and that prefetchable BARs can be allocated
in non-prefetchable space, a 32-bit non-prefetchable window is the
bare minimum if you are after compatibility. However, PCIe only
permits 32-bit BARs for legacy endpoints (which includes most VGA and
SATA controllers, for instance, but not XHCI or NVMe). So it also does
not make a lot of sense to complain about the lack of a 32-bit
non-prefetchable window in the general case, as there are
embedded/integrated scenarios imaginable where you simply never plug
such a legacy endpoint in the first place.
Note that having a 32-bit non-prefetchable and a 32-bit prefetchable
window could also work for PCIe - the prefetchable bridge window does
not *require* any of the upper bits to be set.
> We
> recently gained a warning for this in commit fede8526cc48 ("PCI: of:
> Warn if non-prefetchable memory aperture size is > 32-bit"), but that
> only looks at addresses, not the 64-bit flag. Can't we just not set
> the 64-bit flag if non-prefetchable?
>
No, that may be too restrictive - see the integrated endpoint case above.
But perhaps we could clear the 64-bit flag on the resource window if
the bus addresses are guaranteed to fit into 32 bits? Because that is
actually the culprit here.
> BTW, the DT schema is checking ranges hi cell:
>
> - 0x01000000
> - 0x02000000
> - 0x03000000
> - 0x42000000
> - 0x43000000
> - 0x81000000
> - 0x82000000
> - 0x83000000
> - 0xc2000000
> - 0xc3000000
>
> I just went with what I found in dts files. Sounds like 03 and 83 should
> be removed. (Really, bit 31 or relocatable should probably not be set
> either.)
>
I don't think this needs to change.
> There's a number of other cases:
>
> Documentation/devicetree/bindings/numa.txt: ranges = <0x03000000 0x8010 0x00000000 0x8010 0x00000000 0x70 0x00000000>;
> Documentation/devicetree/bindings/numa.txt: ranges = <0x03000000 0x9010 0x00000000 0x9010 0x00000000 0x70 0x00000000>;
> Documentation/devicetree/bindings/pci/mediatek-pcie.txt- 0x83000000 0 0x60000000 0 0x60000000 0 0x10000000>; /* memory space */
> Documentation/devicetree/bindings/pci/microchip,pcie-host.yaml: ranges = <0x03000000 0x0 0x78000000 0x0 0x78000000 0x0 0x04000000>;
> Documentation/devicetree/bindings/pci/mobiveil-pcie.txt: ranges = < 0x83000000 0 0x00000000 0xa8000000 0 0x8000000>;
> Documentation/devicetree/bindings/pci/rockchip-pcie-host.txt: ranges = <0x83000000 0x0 0xfa000000 0x0 0xfa000000 0x0 0x600000
> arch/arm/boot/dts/mt7623.dtsi- 0x83000000 0 0x60000000 0 0x60000000 0 0x10000000>;
> arch/arm64/boot/dts/broadcom/northstar2/ns2.dtsi: ranges = <0x83000000 0 0x00000000 0 0x00000000 0 0x20000000>;
> arch/arm64/boot/dts/broadcom/northstar2/ns2.dtsi: ranges = <0x83000000 0 0x00000000 0 0x30000000 0 0x20000000>;
> arch/arm64/boot/dts/broadcom/northstar2/ns2.dtsi: ranges = <0x83000000 0 0x00000000 0 0x60000000 0 0x00c00000>;
> arch/arm64/boot/dts/broadcom/stingray/stingray-pcie.dtsi: ranges = <0x83000000 0 0x10000000 0 0x10000000 0 0x20000000>;
> arch/arm64/boot/dts/rockchip/rk3399.dtsi: ranges = <0x83000000 0x0 0xfa000000 0x0 0xfa000000 0x0 0x1e00000>,
>
> Rob
>
^ permalink raw reply [flat|nested] 28+ messages in thread