Re: [v3 PATCH 1/1] PCI: of: fix non-prefetchable region address range check.

[Bjorn Helgaas <helgaas@kernel.org>]

On Wed, Jun 18, 2025 at 01:56:17PM +0000, Wannes Bouwen (Nokia) wrote:
> [v3 PATCH 1/1] PCI: of: fix non-prefetchable region address range check.
> 
> According to the PCIe spec (PCIe r6.3, sec 7.5.1.3.8), non-prefetchable
> memory supports only 32-bit host bridge windows (both base address as
> limit address).
>
> In the kernel there is a check that prints a warning if a
> non-prefetchable resource's size exceeds the 32-bit limit.
> 
> The check currently checks the size of the resource, while actually the
> check should be done on the PCIe end address of the non-prefetchable
> window.
> 
> Move the check to devm_of_pci_get_host_bridge_resources() where the PCIe
> addresses are available and use the end address instead of the size of
> the window.
> 
> Fixes: fede8526cc48 (PCI: of: Warn if non-prefetchable memory aperture
> size is > 32-bit)
> Signed-off-by: Wannes Bouwen <wannes.bouwen@nokia.com>
> ---
> 
> v3:
>   - Update subject and description + add changelog
> 
> v2:
>   - Use PCI address range instead of window size to check that window is
>     within a 32bit boundary.
> 
> ---
>  drivers/pci/of.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/pci/of.c b/drivers/pci/of.c
> index 3579265f1198..4fffa32c7c3d 100644
> --- a/drivers/pci/of.c
> +++ b/drivers/pci/of.c
> @@ -400,6 +400,10 @@ static int devm_of_pci_get_host_bridge_resources(struct device *dev,
>  			*io_base = range.cpu_addr;
>  		} else if (resource_type(res) == IORESOURCE_MEM) {
>  			res->flags &= ~IORESOURCE_MEM_64;
> +
> +			if (!(res->flags & IORESOURCE_PREFETCH))
> +				if (upper_32_bits(range.pci_addr + range.size - 1))
> +					dev_warn(dev, "Memory resource size exceeds max for 32 bits\n");

Seems mostly right to me.  I think we should update the message to
talk about the *address* instead of the size, since that's the real
problem.  Also would be nice to include the resource (%pR) and the
corresponding bus addresses as we do in pci_register_host_bridge().

>  		}
>  
>  		pci_add_resource_offset(resources, res,	res->start - range.pci_addr);
> @@ -622,10 +626,6 @@ 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 (upper_32_bits(resource_size(res)))
> -					dev_warn(dev, "Memory resource size exceeds max for 32 bits\n");
> -
>  			break;
>  		}
>  	}
> -- 
> 2.43.5
>