[PATCH v2 0/4] PCI: of: Improvements to handle 64-bit attribute for non-prefetchable ranges

[PATCH v2 0/4] PCI: of: Improvements to handle 64-bit attribute for non-prefetchable ranges

[Punit Agrawal]

Hi,

Here's an updated version of changes to improve handling of the 64-bit
attribute on non-prefetchable host bridge ranges. Previous version can
be found at [0].

The series addresses Rob and Bjorn's comments on the previous version
and updates the checks for 32-bit non-prefetchable window size to only
apply to non 64-bit ranges.

Thanks,
Punit

Changes:
v2:
* Check ranges PCI / bus addresses rather than CPU addresses
* (new) Restrict 32-bit size warnings on ranges that don't have the 64-bit attribute set
* Refactor the 32-bit size warning to the range parsing loop. This
  change also prints the warnings right after the window mappings are
  logged.


[0] https://lore.kernel.org/linux-arm-kernel/20210527150541.3130505-1-punitagrawal@gmail.com/

Punit Agrawal (4):
  PCI: of: Override 64-bit flag for non-prefetchable memory below 4GB
  PCI: of: Relax the condition for warning about non-prefetchable memory
    aperture size
  PCI: of: Refactor the check for non-prefetchable 32-bit window
  arm64: dts: rockchip: Update PCI host bridge window to 32-bit address
    memory

 arch/arm64/boot/dts/rockchip/rk3399.dtsi |  2 +-
 drivers/pci/of.c                         | 17 ++++++++++++-----
 2 files changed, 13 insertions(+), 6 deletions(-)

-- 
2.30.2

[PATCH v2 1/4] PCI: of: Override 64-bit flag for non-prefetchable memory below 4GB

[Punit Agrawal]

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 host bridge that is modelled as PCI-to-PCI bridge 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
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>

Signed-off-by: Punit Agrawal <punitagrawal@gmail.com>
---
 drivers/pci/of.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/pci/of.c b/drivers/pci/of.c
index da5b414d585a..e2e64c5c55cb 100644
--- a/drivers/pci/of.c
+++ b/drivers/pci/of.c
@@ -346,6 +346,14 @@ static int devm_of_pci_get_host_bridge_resources(struct device *dev,
 				dev_warn(dev, "More than one I/O resource converted for %pOF. CPU base address for old range lost!\n",
 					 dev_node);
 			*io_base = range.cpu_addr;
+		} else if (resource_type(res) == IORESOURCE_MEM) {
+			if (!(res->flags & IORESOURCE_PREFETCH)) {
+				if (res->flags & IORESOURCE_MEM_64)
+					if (!upper_32_bits(range.pci_addr + range.size - 1)) {
+						dev_warn(dev, "Clearing 64-bit flag for non-prefetchable memory below 4GB\n");
+						res->flags &= ~IORESOURCE_MEM_64;
+					}
+			}
 		}
 
 		pci_add_resource_offset(resources, res,	res->start - range.pci_addr);
-- 
2.30.2

Re: [PATCH v2 1/4] PCI: of: Override 64-bit flag for non-prefetchable memory below 4GB

[Ard Biesheuvel]

Hi Punit,

On Tue, 1 Jun 2021 at 00:11, Punit Agrawal <punitagrawal@gmail.com> wrote:
>
> Some host bridges advertise non-prefetable memory windows that are

typo ^

> 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 host bridge that is modelled as PCI-to-PCI bridge cannot forward

It is the root port which is modeled as a P2P bridge. The root port(s)
together with the host bridge is what makes up the root complex.


> 64-bit non-prefetchable memory ranges. As a result, the change in
> behaviour due to the commit causes allocation failure for devices that
> require non-prefetchable bus addresses.
>

AIUI, the problem is not that the device requires a non-prefetchable
bus address, but that it fails to allocate a 32-bit BAR from a 64-bit
non-prefetchable window.

> 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>
>
> Signed-off-by: Punit Agrawal <punitagrawal@gmail.com>
> ---
>  drivers/pci/of.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
>
> diff --git a/drivers/pci/of.c b/drivers/pci/of.c
> index da5b414d585a..e2e64c5c55cb 100644
> --- a/drivers/pci/of.c
> +++ b/drivers/pci/of.c
> @@ -346,6 +346,14 @@ static int devm_of_pci_get_host_bridge_resources(struct device *dev,
>                                 dev_warn(dev, "More than one I/O resource converted for %pOF. CPU base address for old range lost!\n",
>                                          dev_node);
>                         *io_base = range.cpu_addr;
> +               } else if (resource_type(res) == IORESOURCE_MEM) {
> +                       if (!(res->flags & IORESOURCE_PREFETCH)) {
> +                               if (res->flags & IORESOURCE_MEM_64)
> +                                       if (!upper_32_bits(range.pci_addr + range.size - 1)) {
> +                                               dev_warn(dev, "Clearing 64-bit flag for non-prefetchable memory below 4GB\n");
> +                                               res->flags &= ~IORESOURCE_MEM_64;
> +                                       }
> +                       }
>                 }
>
>                 pci_add_resource_offset(resources, res, res->start - range.pci_addr);
> --
> 2.30.2
>

Re: [PATCH v2 1/4] PCI: of: Override 64-bit flag for non-prefetchable memory below 4GB

[Punit Agrawal]

Hi Ard,

Thanks for the comments.

Ard Biesheuvel <ardb@kernel.org> writes:

> Hi Punit,
>
> On Tue, 1 Jun 2021 at 00:11, Punit Agrawal <punitagrawal@gmail.com> wrote:
>>
>> Some host bridges advertise non-prefetable memory windows that are
>
> typo ^

Oops. Fixed locally.

>> 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 host bridge that is modelled as PCI-to-PCI bridge cannot forward
>
> It is the root port which is modeled as a P2P bridge. The root port(s)
> together with the host bridge is what makes up the root complex.
>
>
>> 64-bit non-prefetchable memory ranges. As a result, the change in
>> behaviour due to the commit causes allocation failure for devices that
>> require non-prefetchable bus addresses.
>>
>
> AIUI, the problem is not that the device requires a non-prefetchable
> bus address, but that it fails to allocate a 32-bit BAR from a 64-bit
> non-prefetchable window.

That's right. Is the below (borrowed from your explanation) clearer?

    A PCI root port modelled as a PCI-to-PCI bridge cannot forward 64-bit
    non-prefetchable memory ranges. As a result, the change in behaviour due
    to the commit causes failure to allocate 32-bit BAR from a 64-bit
    non-prefetchable window.


If there are no further comments on the set, I'll send an update in a
couple of days.

Thanks,
Punit

>> 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>
>>
>> Signed-off-by: Punit Agrawal <punitagrawal@gmail.com>
>> ---
>>  drivers/pci/of.c | 8 ++++++++
>>  1 file changed, 8 insertions(+)
>>
>> diff --git a/drivers/pci/of.c b/drivers/pci/of.c
>> index da5b414d585a..e2e64c5c55cb 100644
>> --- a/drivers/pci/of.c
>> +++ b/drivers/pci/of.c
>> @@ -346,6 +346,14 @@ static int devm_of_pci_get_host_bridge_resources(struct device *dev,
>>                                 dev_warn(dev, "More than one I/O resource converted for %pOF. CPU base address for old range lost!\n",
>>                                          dev_node);
>>                         *io_base = range.cpu_addr;
>> +               } else if (resource_type(res) == IORESOURCE_MEM) {
>> +                       if (!(res->flags & IORESOURCE_PREFETCH)) {
>> +                               if (res->flags & IORESOURCE_MEM_64)
>> +                                       if (!upper_32_bits(range.pci_addr + range.size - 1)) {
>> +                                               dev_warn(dev, "Clearing 64-bit flag for non-prefetchable memory below 4GB\n");
>> +                                               res->flags &= ~IORESOURCE_MEM_64;
>> +                                       }
>> +                       }
>>                 }
>>
>>                 pci_add_resource_offset(resources, res, res->start - range.pci_addr);
>> --
>> 2.30.2
>>
>
> _______________________________________________
> Linux-rockchip mailing list
> Linux-rockchip@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-rockchip

[PATCH v2 2/4] PCI: of: Relax the condition for warning about non-prefetchable memory aperture size

[Punit Agrawal]

Commit fede8526cc48 ("PCI: of: Warn if non-prefetchable memory
aperture size is > 32-bit") introduced a warning for non-prefetchable
resources that need more than 32bits to resolve. It turns out that the
check is too restrictive and should be applicable to only resources
that are limited to host bridge windows that don't have the ability to
map 64-bit address space.

Relax the condition to only warn when the resource size requires >
32-bits and doesn't allow mapping to 64-bit addresses.

Link: https://lore.kernel.org/r/7a1e2ebc-f7d8-8431-d844-41a9c36a8911@arm.com
Signed-off-by: Punit Agrawal <punitagrawal@gmail.com>
Cc: Vidya Sagar <vidyas@nvidia.com>
---
 drivers/pci/of.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/pci/of.c b/drivers/pci/of.c
index e2e64c5c55cb..c2a57c61f1d1 100644
--- a/drivers/pci/of.c
+++ b/drivers/pci/of.c
@@ -574,7 +574,8 @@ static int pci_parse_request_of_pci_ranges(struct device *dev,
 			res_valid |= !(res->flags & IORESOURCE_PREFETCH);
 
 			if (!(res->flags & IORESOURCE_PREFETCH))
-				if (upper_32_bits(resource_size(res)))
+				if (!(res->flags & IORESOURCE_MEM_64) &&
+				    upper_32_bits(resource_size(res)))
 					dev_warn(dev, "Memory resource size exceeds max for 32 bits\n");
 
 			break;
-- 
2.30.2

[PATCH v2 3/4] PCI: of: Refactor the check for non-prefetchable 32-bit window

[Punit Agrawal]

Recently, an override was added for non-prefetchable host bridge
windows below 4GB that have the 64-bit address attribute set. As many
of the conditions for the check overlap with the check for
non-prefetchable window size, refactor the code to unify the ranges
validation into devm_of_pci_get_host_bridge_resources().

As an added benefit, the warning message is now printed right after
the range mapping giving the user a better indication of where the
issue is.

Signed-off-by: Punit Agrawal <punitagrawal@gmail.com>
Cc: Vidya Sagar <vidyas@nvidia.com>
---
 drivers/pci/of.c | 12 +++++-------
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/drivers/pci/of.c b/drivers/pci/of.c
index c2a57c61f1d1..836d2787510f 100644
--- a/drivers/pci/of.c
+++ b/drivers/pci/of.c
@@ -348,11 +348,15 @@ static int devm_of_pci_get_host_bridge_resources(struct device *dev,
 			*io_base = range.cpu_addr;
 		} else if (resource_type(res) == IORESOURCE_MEM) {
 			if (!(res->flags & IORESOURCE_PREFETCH)) {
-				if (res->flags & IORESOURCE_MEM_64)
+				if (res->flags & IORESOURCE_MEM_64) {
 					if (!upper_32_bits(range.pci_addr + range.size - 1)) {
 						dev_warn(dev, "Clearing 64-bit flag for non-prefetchable memory below 4GB\n");
 						res->flags &= ~IORESOURCE_MEM_64;
 					}
+				} else {
+					if (upper_32_bits(resource_size(res)))
+						dev_warn(dev, "Memory resource size exceeds max for 32 bits\n");
+				}
 			}
 		}
 
@@ -572,12 +576,6 @@ static int pci_parse_request_of_pci_ranges(struct device *dev,
 			break;
 		case IORESOURCE_MEM:
 			res_valid |= !(res->flags & IORESOURCE_PREFETCH);
-
-			if (!(res->flags & IORESOURCE_PREFETCH))
-				if (!(res->flags & IORESOURCE_MEM_64) &&
-				    upper_32_bits(resource_size(res)))
-					dev_warn(dev, "Memory resource size exceeds max for 32 bits\n");
-
 			break;
 		}
 	}
-- 
2.30.2

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

[Punit Agrawal]

The PCIe host bridge on RK3399 advertises a single 64-bit memory
address range even though it lies entirely below 4GB.

Previously the OF PCI range parser treated 64-bit ranges more
leniently (i.e., as 32-bit), but 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@arm.com>
Link: https://lore.kernel.org/r/7a1e2ebc-f7d8-8431-d844-41a9c36a8911@arm.com
Suggested-by: Robin Murphy <robin.murphy@arm.com>
Signed-off-by: Punit Agrawal <punitagrawal@gmail.com>
Cc: Heiko Stuebner <heiko@sntech.de>
Cc: Rob Herring <robh+dt@kernel.org>
---
 arch/arm64/boot/dts/rockchip/rk3399.dtsi | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
index 634a91af8e83..4b854eb21f72 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
@@ -227,7 +227,7 @@ 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>,
+		ranges = <0x82000000 0x0 0xfa000000 0x0 0xfa000000 0x0 0x1e00000>,
 			 <0x81000000 0x0 0xfbe00000 0x0 0xfbe00000 0x0 0x100000>;
 		resets = <&cru SRST_PCIE_CORE>, <&cru SRST_PCIE_MGMT>,
 			 <&cru SRST_PCIE_MGMT_STICKY>, <&cru SRST_PCIE_PIPE>,
-- 
2.30.2

Re: [PATCH v2 0/4] PCI: of: Improvements to handle 64-bit attribute for non-prefetchable ranges

[Alexandru Elisei]

Hi Punit,

On 5/31/21 11:10 PM, Punit Agrawal wrote:
> Hi,
>
> Here's an updated version of changes to improve handling of the 64-bit
> attribute on non-prefetchable host bridge ranges. Previous version can
> be found at [0].
>
> The series addresses Rob and Bjorn's comments on the previous version
> and updates the checks for 32-bit non-prefetchable window size to only
> apply to non 64-bit ranges.

Many thanks for the series. I've tested it on my rockpro64, and the NVME works as
expected:

Tested-by: Alexandru Elisei <alexandru.elisei@arm.com>

Thanks,

Alex

>
> Thanks,
> Punit
>
> Changes:
> v2:
> * Check ranges PCI / bus addresses rather than CPU addresses
> * (new) Restrict 32-bit size warnings on ranges that don't have the 64-bit attribute set
> * Refactor the 32-bit size warning to the range parsing loop. This
>   change also prints the warnings right after the window mappings are
>   logged.
>
>
> [0] https://lore.kernel.org/linux-arm-kernel/20210527150541.3130505-1-punitagrawal@gmail.com/
>
> Punit Agrawal (4):
>   PCI: of: Override 64-bit flag for non-prefetchable memory below 4GB
>   PCI: of: Relax the condition for warning about non-prefetchable memory
>     aperture size
>   PCI: of: Refactor the check for non-prefetchable 32-bit window
>   arm64: dts: rockchip: Update PCI host bridge window to 32-bit address
>     memory
>
>  arch/arm64/boot/dts/rockchip/rk3399.dtsi |  2 +-
>  drivers/pci/of.c                         | 17 ++++++++++++-----
>  2 files changed, 13 insertions(+), 6 deletions(-)
>

Re: [PATCH v2 0/4] PCI: of: Improvements to handle 64-bit attribute for non-prefetchable ranges

[Punit Agrawal]

Hi Alex,

Alexandru Elisei <alexandru.elisei@arm.com> writes:

> Hi Punit,
>
> On 5/31/21 11:10 PM, Punit Agrawal wrote:
>> Hi,
>>
>> Here's an updated version of changes to improve handling of the 64-bit
>> attribute on non-prefetchable host bridge ranges. Previous version can
>> be found at [0].
>>
>> The series addresses Rob and Bjorn's comments on the previous version
>> and updates the checks for 32-bit non-prefetchable window size to only
>> apply to non 64-bit ranges.
>
> Many thanks for the series. I've tested it on my rockpro64, and the NVME works as
> expected:
>
> Tested-by: Alexandru Elisei <alexandru.elisei@arm.com>

Thanks for taking the patches for a spin.

Punit

[...]