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

[Punit Agrawal <punitagrawal@xxxxxxxxx>]

Hi Rob,

Thanks for taking a look.

Rob Herring <robh+dt@xxxxxxxxxx> writes:

> On Wed, May 26, 2021 at 8:35 AM Punit Agrawal <punitagrawal@xxxxxxxxx> 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@xxxxxxx>
>> Link: https://lore.kernel.org/r/7a1e2ebc-f7d8-8431-d844-41a9c36a8911@xxxxxxx
>> Suggested-by: Robin Murphy <robin.murphy@xxxxxxx>
>> Signed-off-by: Punit Agrawal <punitagrawal@xxxxxxxxx>
>> Cc: Heiko Stuebner <heiko@xxxxxxxxx>
>> Cc: Rob Herring <robh+dt@xxxxxxxxxx>
>> ---
>> 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;
                }
        }