Re: Subject: [PATCH 1/1] PCI: of: avoid warning for 4 GiB non-prefetchable

[Bjorn Helgaas <helgaas@kernel.org>]

[+cc Vidya and reviewers of fede8526cc48]

On Thu, Nov 14, 2024 at 02:05:08PM +0000, Wannes Bouwen (Nokia) wrote:
> Subject: [PATCH 1/1] PCI: of: avoid warning for 4 GiB non-prefetchable
> windows.
> 
> According to the PCIe spec, non-prefetchable memory supports only 32-bit
> BAR registers and are hence limited to 4 GiB. In the kernel there is a
> check that prints a warning if a non-prefetchable resource exceeds the
> 32-bit limit.
> 
> This check however prints a warning when a 4 GiB window on the host
> bridge is used. This is perfectly possible according to the PCIe spec,
> so in my opinion the warning is a bit too strict. This changeset
> subtracts 1 from the resource_size to avoid printing a warning in the
> case of a 4 GiB non-prefetchable window.
> 
> Signed-off-by: Wannes Bouwen <wannes.bouwen@nokia.com>
> ---
>  drivers/pci/of.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/pci/of.c b/drivers/pci/of.c
> index dacea3fc5128..ccbb1f1c2212 100644
> --- a/drivers/pci/of.c
> +++ b/drivers/pci/of.c
> @@ -622,7 +622,7 @@ 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 (upper_32_bits(resource_size(res) - 1))
>                     dev_warn(dev, "Memory resource size exceeds max for 32 bits\n");

I guess this relies on the fact that BARs must be a power of two in
size, right?  So anything where the upper 32 bits of the size are
non-zero is either 0x1_0000_0000 (4GiB window that we shouldn't warn
about), or 0x2_0000_0000 or bigger (where we *do* want to warn about
it).

But it looks like this is used for host bridge resources, which are
windows, not BARs, so they don't have to be a power of two size.  A
window of size 0x1_8000_0000 is perfectly legal and would fit the
criteria for this warning, but this patch would turn off the warning.

I don't really understand this warning in the first place, though.  It
was added by fede8526cc48 ("PCI: of: Warn if non-prefetchable memory
aperture size is > 32-bit").  But I think the real issue would be
related to the highest address, not the size.  For example, an
aperture of 0x0_c000_0000 - 0x1_4000_0000 is only 0x8000_0000 in size,
but the upper half of it it would be invalid for non-prefetchable
32-bit BARs.

Bjorn