* [PATCH 1/2] PCI: of: Override 64-bit flag for non-prefetchable memory below 4GB
@ 2021-05-27 15:05 ` Punit Agrawal
0 siblings, 0 replies; 21+ messages in thread
From: Punit Agrawal @ 2021-05-27 15:05 UTC (permalink / raw)
To: linux-rockchip, linux-pci
Cc: Punit Agrawal, linux-arm-kernel, linux-kernel, alexandru.elisei,
wqu, robin.murphy, pgwipeout, ardb, briannorris, shawn.lin,
helgaas, robh+dt, Bjorn Helgaas
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@kernel.org>
Link: https://lore.kernel.org/r/7a1e2ebc-f7d8-8431-d844-41a9c36a8911@arm.com
Signed-off-by: Punit Agrawal <punitagrawal@gmail.com>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: Rob Herring <robh+dt@kernel.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");
-
+ 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;
}
}
--
2.30.2
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH 1/2] PCI: of: Override 64-bit flag for non-prefetchable memory below 4GB
@ 2021-05-27 15:05 ` Punit Agrawal
0 siblings, 0 replies; 21+ messages in thread
From: Punit Agrawal @ 2021-05-27 15:05 UTC (permalink / raw)
To: linux-rockchip, linux-pci
Cc: Punit Agrawal, linux-arm-kernel, linux-kernel, alexandru.elisei,
wqu, robin.murphy, pgwipeout, ardb, briannorris, shawn.lin,
helgaas, robh+dt, Bjorn Helgaas
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@kernel.org>
Link: https://lore.kernel.org/r/7a1e2ebc-f7d8-8431-d844-41a9c36a8911@arm.com
Signed-off-by: Punit Agrawal <punitagrawal@gmail.com>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: Rob Herring <robh+dt@kernel.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");
-
+ 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;
}
}
--
2.30.2
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [PATCH 1/2] PCI: of: Override 64-bit flag for non-prefetchable memory below 4GB
2021-05-27 15:05 ` Punit Agrawal
(?)
@ 2021-05-27 16:21 ` Bjorn Helgaas
-1 siblings, 0 replies; 21+ messages in thread
From: Bjorn Helgaas @ 2021-05-27 16:21 UTC (permalink / raw)
To: Punit Agrawal
Cc: linux-rockchip, linux-pci, linux-arm-kernel, linux-kernel,
alexandru.elisei, wqu, robin.murphy, pgwipeout, ardb,
briannorris, shawn.lin, robh+dt, Bjorn Helgaas
On Fri, May 28, 2021 at 12:05:41AM +0900, Punit Agrawal 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.
Conceptually, I'm not sure why we need IORESOURCE_MEM_64 at all on
resources we get from DT. I think the main point of IORESOURCE_MEM_64
is to convey the information that "this register, e.g., a PCI BAR, has
space for 64-bit values if you need to write to it."
When we're parsing this from DT, I think we're just getting a fixed
value and there's no concept of writing anything back, so it doesn't
seem like we should need to know how wide the hardware register is, or
even whether there *is* a hardware register.
But I'm sure the PCI resource allocation code probably depends on
IORESOURCE_MEM_64 in those host bridge windows in very ugly ways.
> 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@kernel.org>
> Link: https://lore.kernel.org/r/7a1e2ebc-f7d8-8431-d844-41a9c36a8911@arm.com
> Signed-off-by: Punit Agrawal <punitagrawal@gmail.com>
> Cc: Bjorn Helgaas <bhelgaas@google.com>
> Cc: Rob Herring <robh+dt@kernel.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");
> -
> + 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");
Maybe "Clearing 64-bit flag"?
Can you include %pR, so we see the resource in question?
Unrelated but close by, would be nice if the preceding warning ("size
exceeds max") also included %pR.
> + res->flags &= ~IORESOURCE_MEM_64;
> + }
> + }
> break;
> }
> }
> --
> 2.30.2
>
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 1/2] PCI: of: Override 64-bit flag for non-prefetchable memory below 4GB
@ 2021-05-27 16:21 ` Bjorn Helgaas
0 siblings, 0 replies; 21+ messages in thread
From: Bjorn Helgaas @ 2021-05-27 16:21 UTC (permalink / raw)
To: Punit Agrawal
Cc: linux-rockchip, linux-pci, linux-arm-kernel, linux-kernel,
alexandru.elisei, wqu, robin.murphy, pgwipeout, ardb,
briannorris, shawn.lin, robh+dt, Bjorn Helgaas
On Fri, May 28, 2021 at 12:05:41AM +0900, Punit Agrawal 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.
Conceptually, I'm not sure why we need IORESOURCE_MEM_64 at all on
resources we get from DT. I think the main point of IORESOURCE_MEM_64
is to convey the information that "this register, e.g., a PCI BAR, has
space for 64-bit values if you need to write to it."
When we're parsing this from DT, I think we're just getting a fixed
value and there's no concept of writing anything back, so it doesn't
seem like we should need to know how wide the hardware register is, or
even whether there *is* a hardware register.
But I'm sure the PCI resource allocation code probably depends on
IORESOURCE_MEM_64 in those host bridge windows in very ugly ways.
> 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@kernel.org>
> Link: https://lore.kernel.org/r/7a1e2ebc-f7d8-8431-d844-41a9c36a8911@arm.com
> Signed-off-by: Punit Agrawal <punitagrawal@gmail.com>
> Cc: Bjorn Helgaas <bhelgaas@google.com>
> Cc: Rob Herring <robh+dt@kernel.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");
> -
> + 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");
Maybe "Clearing 64-bit flag"?
Can you include %pR, so we see the resource in question?
Unrelated but close by, would be nice if the preceding warning ("size
exceeds max") also included %pR.
> + res->flags &= ~IORESOURCE_MEM_64;
> + }
> + }
> break;
> }
> }
> --
> 2.30.2
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 1/2] PCI: of: Override 64-bit flag for non-prefetchable memory below 4GB
@ 2021-05-27 16:21 ` Bjorn Helgaas
0 siblings, 0 replies; 21+ messages in thread
From: Bjorn Helgaas @ 2021-05-27 16:21 UTC (permalink / raw)
To: Punit Agrawal
Cc: linux-rockchip, linux-pci, linux-arm-kernel, linux-kernel,
alexandru.elisei, wqu, robin.murphy, pgwipeout, ardb,
briannorris, shawn.lin, robh+dt, Bjorn Helgaas
On Fri, May 28, 2021 at 12:05:41AM +0900, Punit Agrawal 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.
Conceptually, I'm not sure why we need IORESOURCE_MEM_64 at all on
resources we get from DT. I think the main point of IORESOURCE_MEM_64
is to convey the information that "this register, e.g., a PCI BAR, has
space for 64-bit values if you need to write to it."
When we're parsing this from DT, I think we're just getting a fixed
value and there's no concept of writing anything back, so it doesn't
seem like we should need to know how wide the hardware register is, or
even whether there *is* a hardware register.
But I'm sure the PCI resource allocation code probably depends on
IORESOURCE_MEM_64 in those host bridge windows in very ugly ways.
> 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@kernel.org>
> Link: https://lore.kernel.org/r/7a1e2ebc-f7d8-8431-d844-41a9c36a8911@arm.com
> Signed-off-by: Punit Agrawal <punitagrawal@gmail.com>
> Cc: Bjorn Helgaas <bhelgaas@google.com>
> Cc: Rob Herring <robh+dt@kernel.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");
> -
> + 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");
Maybe "Clearing 64-bit flag"?
Can you include %pR, so we see the resource in question?
Unrelated but close by, would be nice if the preceding warning ("size
exceeds max") also included %pR.
> + res->flags &= ~IORESOURCE_MEM_64;
> + }
> + }
> break;
> }
> }
> --
> 2.30.2
>
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 1/2] PCI: of: Override 64-bit flag for non-prefetchable memory below 4GB
2021-05-27 16:21 ` Bjorn Helgaas
(?)
@ 2021-05-28 12:42 ` Punit Agrawal
-1 siblings, 0 replies; 21+ messages in thread
From: Punit Agrawal @ 2021-05-28 12:42 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: linux-rockchip, linux-pci, linux-arm-kernel, linux-kernel,
alexandru.elisei, wqu, robin.murphy, pgwipeout, ardb,
briannorris, shawn.lin, robh+dt, Bjorn Helgaas
Hi Bjorn,
Thanks for taking a look.
Bjorn Helgaas <helgaas@kernel.org> writes:
> On Fri, May 28, 2021 at 12:05:41AM +0900, Punit Agrawal 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.
>
> Conceptually, I'm not sure why we need IORESOURCE_MEM_64 at all on
> resources we get from DT. I think the main point of IORESOURCE_MEM_64
> is to convey the information that "this register, e.g., a PCI BAR, has
> space for 64-bit values if you need to write to it."
>
> When we're parsing this from DT, I think we're just getting a fixed
> value and there's no concept of writing anything back, so it doesn't
> seem like we should need to know how wide the hardware register is, or
> even whether there *is* a hardware register.
>
> But I'm sure the PCI resource allocation code probably depends on
> IORESOURCE_MEM_64 in those host bridge windows in very ugly ways.
Thanks for the explanation. From what I can tell, the IORESOURCE_MEM_64
flag is used in pci_bus_alloc_resource() to allocate from high PCI
addresses. Without the flag allocations above 4GB will fail. Not sure
that's legitimate use of the flag though.
>> 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@kernel.org>
>> Link: https://lore.kernel.org/r/7a1e2ebc-f7d8-8431-d844-41a9c36a8911@arm.com
>> Signed-off-by: Punit Agrawal <punitagrawal@gmail.com>
>> Cc: Bjorn Helgaas <bhelgaas@google.com>
>> Cc: Rob Herring <robh+dt@kernel.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");
>> -
>> + 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");
>
> Maybe "Clearing 64-bit flag"?
>
> Can you include %pR, so we see the resource in question?
I'll follow your suggestions in the next update.
>
> Unrelated but close by, would be nice if the preceding warning ("size
> exceeds max") also included %pR.
Makes sense. I'll add the resource print to improve the message.
Thanks,
Punit
>
>> + res->flags &= ~IORESOURCE_MEM_64;
>> + }
>> + }
>> break;
>> }
>> }
>> --
>> 2.30.2
>>
>
> _______________________________________________
> Linux-rockchip mailing list
> Linux-rockchip@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 1/2] PCI: of: Override 64-bit flag for non-prefetchable memory below 4GB
@ 2021-05-28 12:42 ` Punit Agrawal
0 siblings, 0 replies; 21+ messages in thread
From: Punit Agrawal @ 2021-05-28 12:42 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: linux-rockchip, linux-pci, linux-arm-kernel, linux-kernel,
alexandru.elisei, wqu, robin.murphy, pgwipeout, ardb,
briannorris, shawn.lin, robh+dt, Bjorn Helgaas
Hi Bjorn,
Thanks for taking a look.
Bjorn Helgaas <helgaas@kernel.org> writes:
> On Fri, May 28, 2021 at 12:05:41AM +0900, Punit Agrawal 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.
>
> Conceptually, I'm not sure why we need IORESOURCE_MEM_64 at all on
> resources we get from DT. I think the main point of IORESOURCE_MEM_64
> is to convey the information that "this register, e.g., a PCI BAR, has
> space for 64-bit values if you need to write to it."
>
> When we're parsing this from DT, I think we're just getting a fixed
> value and there's no concept of writing anything back, so it doesn't
> seem like we should need to know how wide the hardware register is, or
> even whether there *is* a hardware register.
>
> But I'm sure the PCI resource allocation code probably depends on
> IORESOURCE_MEM_64 in those host bridge windows in very ugly ways.
Thanks for the explanation. From what I can tell, the IORESOURCE_MEM_64
flag is used in pci_bus_alloc_resource() to allocate from high PCI
addresses. Without the flag allocations above 4GB will fail. Not sure
that's legitimate use of the flag though.
>> 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@kernel.org>
>> Link: https://lore.kernel.org/r/7a1e2ebc-f7d8-8431-d844-41a9c36a8911@arm.com
>> Signed-off-by: Punit Agrawal <punitagrawal@gmail.com>
>> Cc: Bjorn Helgaas <bhelgaas@google.com>
>> Cc: Rob Herring <robh+dt@kernel.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");
>> -
>> + 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");
>
> Maybe "Clearing 64-bit flag"?
>
> Can you include %pR, so we see the resource in question?
I'll follow your suggestions in the next update.
>
> Unrelated but close by, would be nice if the preceding warning ("size
> exceeds max") also included %pR.
Makes sense. I'll add the resource print to improve the message.
Thanks,
Punit
>
>> + res->flags &= ~IORESOURCE_MEM_64;
>> + }
>> + }
>> break;
>> }
>> }
>> --
>> 2.30.2
>>
>
> _______________________________________________
> Linux-rockchip mailing list
> Linux-rockchip@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-rockchip
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 1/2] PCI: of: Override 64-bit flag for non-prefetchable memory below 4GB
@ 2021-05-28 12:42 ` Punit Agrawal
0 siblings, 0 replies; 21+ messages in thread
From: Punit Agrawal @ 2021-05-28 12:42 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: linux-rockchip, linux-pci, linux-arm-kernel, linux-kernel,
alexandru.elisei, wqu, robin.murphy, pgwipeout, ardb,
briannorris, shawn.lin, robh+dt, Bjorn Helgaas
Hi Bjorn,
Thanks for taking a look.
Bjorn Helgaas <helgaas@kernel.org> writes:
> On Fri, May 28, 2021 at 12:05:41AM +0900, Punit Agrawal 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.
>
> Conceptually, I'm not sure why we need IORESOURCE_MEM_64 at all on
> resources we get from DT. I think the main point of IORESOURCE_MEM_64
> is to convey the information that "this register, e.g., a PCI BAR, has
> space for 64-bit values if you need to write to it."
>
> When we're parsing this from DT, I think we're just getting a fixed
> value and there's no concept of writing anything back, so it doesn't
> seem like we should need to know how wide the hardware register is, or
> even whether there *is* a hardware register.
>
> But I'm sure the PCI resource allocation code probably depends on
> IORESOURCE_MEM_64 in those host bridge windows in very ugly ways.
Thanks for the explanation. From what I can tell, the IORESOURCE_MEM_64
flag is used in pci_bus_alloc_resource() to allocate from high PCI
addresses. Without the flag allocations above 4GB will fail. Not sure
that's legitimate use of the flag though.
>> 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@kernel.org>
>> Link: https://lore.kernel.org/r/7a1e2ebc-f7d8-8431-d844-41a9c36a8911@arm.com
>> Signed-off-by: Punit Agrawal <punitagrawal@gmail.com>
>> Cc: Bjorn Helgaas <bhelgaas@google.com>
>> Cc: Rob Herring <robh+dt@kernel.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");
>> -
>> + 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");
>
> Maybe "Clearing 64-bit flag"?
>
> Can you include %pR, so we see the resource in question?
I'll follow your suggestions in the next update.
>
> Unrelated but close by, would be nice if the preceding warning ("size
> exceeds max") also included %pR.
Makes sense. I'll add the resource print to improve the message.
Thanks,
Punit
>
>> + res->flags &= ~IORESOURCE_MEM_64;
>> + }
>> + }
>> break;
>> }
>> }
>> --
>> 2.30.2
>>
>
> _______________________________________________
> Linux-rockchip mailing list
> Linux-rockchip@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-rockchip
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 1/2] PCI: of: Override 64-bit flag for non-prefetchable memory below 4GB
2021-05-27 15:05 ` Punit Agrawal
(?)
@ 2021-05-27 16:38 ` Rob Herring
-1 siblings, 0 replies; 21+ messages in thread
From: Rob Herring @ 2021-05-27 16:38 UTC (permalink / raw)
To: Punit Agrawal
Cc: open list:ARM/Rockchip SoC...,
PCI, linux-arm-kernel, linux-kernel, Alexandru Elisei, wqu,
Robin Murphy, Peter Geis, Ard Biesheuvel, Brian Norris,
Shawn Lin, Bjorn Helgaas, Bjorn Helgaas
On Thu, May 27, 2021 at 10:06 AM Punit Agrawal <punitagrawal@gmail.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@kernel.org>
> Link: https://lore.kernel.org/r/7a1e2ebc-f7d8-8431-d844-41a9c36a8911@arm.com
> Signed-off-by: Punit Agrawal <punitagrawal@gmail.com>
> Cc: Bjorn Helgaas <bhelgaas@google.com>
> Cc: Rob Herring <robh+dt@kernel.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?
> -
> + 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?
> + dev_warn(dev, "Overriding 64-bit flag for non-prefetchable memory below 4GB\n");
> + res->flags &= ~IORESOURCE_MEM_64;
> + }
> + }
> break;
> }
> }
> --
> 2.30.2
>
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 1/2] PCI: of: Override 64-bit flag for non-prefetchable memory below 4GB
@ 2021-05-27 16:38 ` Rob Herring
0 siblings, 0 replies; 21+ messages in thread
From: Rob Herring @ 2021-05-27 16:38 UTC (permalink / raw)
To: Punit Agrawal
Cc: open list:ARM/Rockchip SoC...,
PCI, linux-arm-kernel, linux-kernel, Alexandru Elisei, wqu,
Robin Murphy, Peter Geis, Ard Biesheuvel, Brian Norris,
Shawn Lin, Bjorn Helgaas, Bjorn Helgaas
On Thu, May 27, 2021 at 10:06 AM Punit Agrawal <punitagrawal@gmail.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@kernel.org>
> Link: https://lore.kernel.org/r/7a1e2ebc-f7d8-8431-d844-41a9c36a8911@arm.com
> Signed-off-by: Punit Agrawal <punitagrawal@gmail.com>
> Cc: Bjorn Helgaas <bhelgaas@google.com>
> Cc: Rob Herring <robh+dt@kernel.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?
> -
> + 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?
> + dev_warn(dev, "Overriding 64-bit flag for non-prefetchable memory below 4GB\n");
> + res->flags &= ~IORESOURCE_MEM_64;
> + }
> + }
> break;
> }
> }
> --
> 2.30.2
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 1/2] PCI: of: Override 64-bit flag for non-prefetchable memory below 4GB
@ 2021-05-27 16:38 ` Rob Herring
0 siblings, 0 replies; 21+ messages in thread
From: Rob Herring @ 2021-05-27 16:38 UTC (permalink / raw)
To: Punit Agrawal
Cc: open list:ARM/Rockchip SoC...,
PCI, linux-arm-kernel, linux-kernel, Alexandru Elisei, wqu,
Robin Murphy, Peter Geis, Ard Biesheuvel, Brian Norris,
Shawn Lin, Bjorn Helgaas, Bjorn Helgaas
On Thu, May 27, 2021 at 10:06 AM Punit Agrawal <punitagrawal@gmail.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@kernel.org>
> Link: https://lore.kernel.org/r/7a1e2ebc-f7d8-8431-d844-41a9c36a8911@arm.com
> Signed-off-by: Punit Agrawal <punitagrawal@gmail.com>
> Cc: Bjorn Helgaas <bhelgaas@google.com>
> Cc: Rob Herring <robh+dt@kernel.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?
> -
> + 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?
> + 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@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 1/2] PCI: of: Override 64-bit flag for non-prefetchable memory below 4GB
2021-05-27 16:38 ` Rob Herring
(?)
@ 2021-05-28 13:24 ` Punit Agrawal
-1 siblings, 0 replies; 21+ messages in thread
From: Punit Agrawal @ 2021-05-28 13:24 UTC (permalink / raw)
To: Rob Herring
Cc: open list:ARM/Rockchip SoC...,
PCI, linux-arm-kernel, linux-kernel, Alexandru Elisei, wqu,
Robin Murphy, Peter Geis, Ard Biesheuvel, Brian Norris,
Shawn Lin, Bjorn Helgaas, Bjorn Helgaas
Rob Herring <robh+dt@kernel.org> writes:
> On Thu, May 27, 2021 at 10:06 AM Punit Agrawal <punitagrawal@gmail.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@kernel.org>
>> Link: https://lore.kernel.org/r/7a1e2ebc-f7d8-8431-d844-41a9c36a8911@arm.com
>> Signed-off-by: Punit Agrawal <punitagrawal@gmail.com>
>> Cc: Bjorn Helgaas <bhelgaas@google.com>
>> Cc: Rob Herring <robh+dt@kernel.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@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 1/2] PCI: of: Override 64-bit flag for non-prefetchable memory below 4GB
@ 2021-05-28 13:24 ` Punit Agrawal
0 siblings, 0 replies; 21+ messages in thread
From: Punit Agrawal @ 2021-05-28 13:24 UTC (permalink / raw)
To: Rob Herring
Cc: open list:ARM/Rockchip SoC...,
PCI, linux-arm-kernel, linux-kernel, Alexandru Elisei, wqu,
Robin Murphy, Peter Geis, Ard Biesheuvel, Brian Norris,
Shawn Lin, Bjorn Helgaas, Bjorn Helgaas
Rob Herring <robh+dt@kernel.org> writes:
> On Thu, May 27, 2021 at 10:06 AM Punit Agrawal <punitagrawal@gmail.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@kernel.org>
>> Link: https://lore.kernel.org/r/7a1e2ebc-f7d8-8431-d844-41a9c36a8911@arm.com
>> Signed-off-by: Punit Agrawal <punitagrawal@gmail.com>
>> Cc: Bjorn Helgaas <bhelgaas@google.com>
>> Cc: Rob Herring <robh+dt@kernel.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@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-rockchip
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 1/2] PCI: of: Override 64-bit flag for non-prefetchable memory below 4GB
@ 2021-05-28 13:24 ` Punit Agrawal
0 siblings, 0 replies; 21+ messages in thread
From: Punit Agrawal @ 2021-05-28 13:24 UTC (permalink / raw)
To: Rob Herring
Cc: open list:ARM/Rockchip SoC...,
PCI, linux-arm-kernel, linux-kernel, Alexandru Elisei, wqu,
Robin Murphy, Peter Geis, Ard Biesheuvel, Brian Norris,
Shawn Lin, Bjorn Helgaas, Bjorn Helgaas
Rob Herring <robh+dt@kernel.org> writes:
> On Thu, May 27, 2021 at 10:06 AM Punit Agrawal <punitagrawal@gmail.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@kernel.org>
>> Link: https://lore.kernel.org/r/7a1e2ebc-f7d8-8431-d844-41a9c36a8911@arm.com
>> Signed-off-by: Punit Agrawal <punitagrawal@gmail.com>
>> Cc: Bjorn Helgaas <bhelgaas@google.com>
>> Cc: Rob Herring <robh+dt@kernel.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@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-rockchip
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply [flat|nested] 21+ messages in thread