linux-kernel - Re: [PATCH] arm64: dts: rockchip: Update PCI host bridge window to 32-bit address memory

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <87a6ohniec.fsf@stealth>
Date:   Thu, 27 May 2021 09:13:15 +0900
From:   Punit Agrawal <punitagrawal@...il.com>
To:     Rob Herring <robh+dt@...nel.org>
Cc:     "open list:ARM/Rockchip SoC..." <linux-rockchip@...ts.infradead.org>,
        linux-arm-kernel <linux-arm-kernel@...ts.infradead.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Alexandru Elisei <alexandru.elisei@....com>, wqu@...e.com,
        Robin Murphy <robin.murphy@....com>,
        Peter Geis <pgwipeout@...il.com>,
        Ard Biesheuvel <ardb@...nel.org>,
        Brian Norris <briannorris@...omium.org>,
        Shawn Lin <shawn.lin@...k-chips.com>,
        PCI <linux-pci@...r.kernel.org>, Heiko Stuebner <heiko@...ech.de>
Subject: Re: [PATCH] arm64: dts: rockchip: Update PCI host bridge window to
 32-bit address memory

Hi Rob,

Thanks for taking a look.

Rob Herring <robh+dt@...nel.org> writes:

> On Wed, May 26, 2021 at 8:35 AM Punit Agrawal <punitagrawal@...il.com> wrote:
>>
>> The PCIe host bridge on RK3399 advertises a single address range
>> marked as 64-bit memory even though it lies entirely below 4GB. While
>> previously, the OF PCI range parser treated 64-bit ranges more
>> leniently (i.e., as 32-bit), since commit 9d57e61bf723 ("of/pci: Add
>> IORESOURCE_MEM_64 to resource flags for 64-bit memory addresses") the
>> code takes a stricter view and treats the ranges as advertised in the
>> device tree (i.e, as 64-bit).
>>
>> The change in behaviour causes failure when allocating bus addresses
>> to devices connected behind a PCI-to-PCI bridge that require
>> non-prefetchable memory ranges. The allocation failure was observed
>> for certain Samsung NVMe drives connected to RockPro64 boards.
>>
>> Update the host bridge window attributes to treat it as 32-bit address
>> memory. This fixes the allocation failure observed since commit
>> 9d57e61bf723.
>>
>> Reported-by: Alexandru Elisei <alexandru.elisei@....com>
>> Link: https://lore.kernel.org/r/7a1e2ebc-f7d8-8431-d844-41a9c36a8911@arm.com
>> Suggested-by: Robin Murphy <robin.murphy@....com>
>> Signed-off-by: Punit Agrawal <punitagrawal@...il.com>
>> Cc: Heiko Stuebner <heiko@...ech.de>
>> Cc: Rob Herring <robh+dt@...nel.org>
>> ---
>> Hi,
>>
>> The patch fixes the failure observed with detecting certain Samsung
>> NVMe drives on RK3399 based boards.
>>
>> Hopefully, the folks on this thread can provide some input on the
>> reason the host bridge window was originally marked as 64-bit or if
>> there are any downsides to applying the patch.
>
> We can't require *only* a DT update to fix this. Ideally, the Rockchip
> PCI driver should clear the 64-bit flag in the resources though I'm
> not sure if the bridge driver would have access early enough.

Following the discussion in the other thread, I tested the following
changes to fixup 64-bit flag for non-prefetchable memory resources that
fit below 4GB.

If the changes look good, I'll send it out as a proper patch later
today.

---->8----
diff --git a/drivers/pci/of.c b/drivers/pci/of.c
index da5b414d585a..b9d0bee5a088 100644
--- a/drivers/pci/of.c
+++ b/drivers/pci/of.c
@@ -565,10 +565,14 @@ static int pci_parse_request_of_pci_ranges(struct device *dev,
                case IORESOURCE_MEM:
                        res_valid |= !(res->flags & IORESOURCE_PREFETCH);

-                       if (!(res->flags & IORESOURCE_PREFETCH))
+                       if (!(res->flags & IORESOURCE_PREFETCH)) {
                                if (upper_32_bits(resource_size(res)))
                                        dev_warn(dev, "Memory resource size exceeds max for 32 bits\n");
-
+                               if ((res->flags & IORESOURCE_MEM_64) && !upper_32_bits(res->end)) {
+                                       dev_warn(dev, "Overriding 64-bit flag for non-prefetchable memory below 4GB\n");
+                                       res->flags &= ~IORESOURCE_MEM_64;
+                               }
+                       }
                        break;
                }
        }