From: Robin Murphy <robin.murphy@arm.com>
To: Anand Moon <linux.amoon@gmail.com>, Ard Biesheuvel <ardb@kernel.org>
Cc: Punit Agrawal <punitagrawal@gmail.com>,
Alexandru Elisei <alexandru.elisei@arm.com>,
Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
linux-rockchip@lists.infradead.org,
arm-mail-list <linux-arm-kernel@lists.infradead.org>,
Heiko Stuebner <heiko.stuebner@theobroma-systems.com>,
leobras.c@gmail.com, Rob Herring <robh@kernel.org>,
PCI <linux-pci@vger.kernel.org>
Subject: Re: [BUG] rockpro64: PCI BAR reassignment broken by commit 9d57e61bf723 ("of/pci: Add IORESOURCE_MEM_64 to resource flags for 64-bit memory addresses")
Date: Tue, 25 May 2021 18:14:42 +0100 [thread overview]
Message-ID: <4292e929-779f-9c83-f75c-7d8121a1c6c8@arm.com> (raw)
In-Reply-To: <CANAwSgQEti7=dsYbqYMxrcP9KPFc-s0e4xHPPwOC=gP+scpP3w@mail.gmail.com>
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.
next prev parent reply other threads:[~2021-05-25 17:14 UTC|newest]
Thread overview: 28+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <7a1e2ebc-f7d8-8431-d844-41a9c36a8911@arm.com>
2021-05-19 11:27 ` [BUG] rockpro64: PCI BAR reassignment broken by commit 9d57e61bf723 ("of/pci: Add IORESOURCE_MEM_64 to resource flags for 64-bit memory addresses") Robin Murphy
2021-05-19 13:17 ` Peter Geis
2021-05-23 11:03 ` Punit Agrawal
2021-05-23 12:10 ` Ard Biesheuvel
2021-05-25 13:42 ` Punit Agrawal
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:23 ` Peter Geis
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:37 ` Peter Geis
2021-05-26 13:55 ` Christian König
2021-05-26 14:15 ` Ard Biesheuvel
2021-05-25 17:25 ` Robin Murphy
2021-05-25 17:34 ` Peter Geis
2021-05-25 18:55 ` Robin Murphy
2021-05-25 19:15 ` Bjorn Helgaas
2021-05-25 19:43 ` Ard Biesheuvel
2021-05-25 20:03 ` Peter Geis
2021-05-26 14:18 ` Ard Biesheuvel
2021-05-25 16:59 ` Anand Moon
2021-05-25 17:14 ` Robin Murphy [this message]
2021-05-25 17:42 ` Peter Geis
2021-05-25 22:36 ` Punit Agrawal
2021-05-26 15:37 ` Rob Herring
2021-05-26 16:35 ` Ard Biesheuvel
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=4292e929-779f-9c83-f75c-7d8121a1c6c8@arm.com \
--to=robin.murphy@arm.com \
--cc=alexandru.elisei@arm.com \
--cc=ardb@kernel.org \
--cc=heiko.stuebner@theobroma-systems.com \
--cc=leobras.c@gmail.com \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pci@vger.kernel.org \
--cc=linux-rockchip@lists.infradead.org \
--cc=linux.amoon@gmail.com \
--cc=punitagrawal@gmail.com \
--cc=robh@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).