Message-ID: <87h7inm1op.fsf@stealth>
Date:   Fri, 28 May 2021 22:24:06 +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>,
        PCI <linux-pci@...r.kernel.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>,
        Bjorn Helgaas <helgaas@...nel.org>,
        Bjorn Helgaas <bhelgaas@...gle.com>
Subject: Re: [PATCH 1/2] PCI: of: Override 64-bit flag for non-prefetchable
 memory below 4GB

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

> On Thu, May 27, 2021 at 10:06 AM Punit Agrawal <punitagrawal@...il.com> wrote:
>>
>> Some host bridges advertise non-prefetable memory windows that are
>> entirely located below 4GB but are marked as 64-bit address memory.
>>
>> Since commit 9d57e61bf723 ("of/pci: Add IORESOURCE_MEM_64 to resource
>> flags for 64-bit memory addresses"), the OF PCI range parser takes a
>> stricter view and treats 64-bit address ranges as advertised while
>> before such ranges were treated as 32-bit.
>>
>> A PCI-to-PCI bridges cannot forward 64-bit non-prefetchable memory
>> ranges. As a result, the change in behaviour due to the commit causes
>> allocation failure for devices that are connected behind PCI host
>> bridges modelled as PCI-to-PCI bridge and require non-prefetchable bus
>> addresses.
>>
>> In order to not break platforms, override the 64-bit flag for
>> non-prefetchable memory ranges that lie entirely below 4GB.
>>
>> Suggested-by: Ard Biesheuvel <ardb@...nel.org>
>> Link: https://lore.kernel.org/r/7a1e2ebc-f7d8-8431-d844-41a9c36a8911@arm.com
>> Signed-off-by: Punit Agrawal <punitagrawal@...il.com>
>> Cc: Bjorn Helgaas <bhelgaas@...gle.com>
>> Cc: Rob Herring <robh+dt@...nel.org>
>> ---
>>  drivers/pci/of.c | 8 ++++++--
>>  1 file changed, 6 insertions(+), 2 deletions(-)
>>
>> 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");
>
> Based on Ard's explanation, doesn't this need to also check for
> !IORESOURCE_MEM_64?

Right - I was too focussed on the below case.
>
>> -
>> +                               if ((res->flags & IORESOURCE_MEM_64) && !upper_32_bits(res->end)) {
>
> res->end is the CPU address space. Isn't it the PCI address space we
> care about?

Indeed. I suspect the easiest way to check PCI addresses would be to
move the check to where the range property is being parsed.

I'll address both the comments with the next update.

Thanks,
Punit

>
>> +                                       dev_warn(dev, "Overriding 64-bit flag for non-prefetchable memory below 4GB\n");
>> +                                       res->flags &= ~IORESOURCE_MEM_64;
>> +                               }
>> +                       }
>>                         break;
>>                 }
>>         }
>> --
>> 2.30.2
>>
>
> _______________________________________________
> Linux-rockchip mailing list
> Linux-rockchip@...ts.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-rockchip

Powered by blists - more mailing lists