Re: [PATCH v1] sysfs: Include bridge window address size flags

[Pali Rohár]

On Friday 26 June 2026 16:30:47 Bjorn Helgaas wrote:
> sysfs_get_resources() fills d->base_addr[] with CPU addresses of PCI BARs
> (the CPU addresses in sysfs 'resource' differ from the BARs when a host
> bridge applies an offset between CPU and PCI addresses).
> 
> 119c1376f9ca ("libpci: Add support for filling bridge resources") fills
> d->bridge_base_addr[] for bridge windows, but unlike for d->base_addr[], it
> did not include the low-order four bits of 'flags'.  For I/O windows, those
> bits indicate 16- or 32-bit addressing; for prefetchable memory windows,
> they indicate 32- or 64-bit addressing.
> 
> ccf68033a452 ("lspci: Use PCI_FILL_BRIDGE_BASES to detect if range behind
> bridge is disabled or unsupported") tests the d->bridge_base_addr[] flag
> bits to learn those properties, but since they weren't included by
> 119c1376f9ca, 32-bit I/O windows were labeled as "[16-bit]" and 64-bit
> prefetchable windows were labeled as "[32-bit]".

Is not an issue that lspci.c is looking for flags in ->bridge_base_addr[] ?
For me it seems to be better to look for flags in ->bridge_flags[] member.

> Previous output of "lspci -v" on a bridge:
> 
>   I/O behind bridge: 2000-2fff [size=4K] [16-bit]
>   Prefetchable memory behind bridge: 48030000000-480401fffff [size=258M] [32-bit]
> 
> Similar output after this commit:
> 
>   I/O behind bridge: 00002000-00002fff [size=4K] [32-bit]
>   Prefetchable memory behind bridge: 0000048030000000-00000480401fffff [size=258M] [64-bit]
> 
> Signed-off-by: Bjorn Helgaas <bhelgaas@xxxxxxxxxx>
> ---
>  lib/sysfs.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/lib/sysfs.c b/lib/sysfs.c
> index 55ac7a6f0699..77ab99e2d5d2 100644
> --- a/lib/sysfs.c
> +++ b/lib/sysfs.c
> @@ -219,11 +219,14 @@ sysfs_get_resources(struct pci_dev *d)
>  	}
>        if (i == 7+4 || i == 7+6+4)
>  	{
> +	  unsigned long long flags;
>  	  int offset = (i == 7+6+4) ? 6 : 0;
>  	  for (i = 0; i < 4; i++)
>  	    {
> -	      d->bridge_flags[i] = lines[offset+i].flags;
> -	      d->bridge_base_addr[i] = lines[offset+i].base_addr;
> +	      flags = lines[offset+i].flags;
> +	      d->bridge_flags[i] = flags;
> +	      flags &= PCI_ADDR_FLAG_MASK;
> +	      d->bridge_base_addr[i] = lines[offset+i].base_addr | flags;
>  	      d->bridge_size[i] = lines[offset+i].size;
>  	    }
>  	  have_bridge_bases = 1;
> -- 
> 2.53.0
> 

lib/pci.h says:

  pciaddr_t bridge_base_addr[4];	/* Bridge base addresses (without flags) */
  pciaddr_t bridge_size[4];		/* Bridge sizes */
  pciaddr_t bridge_flags[4];		/* PCI_IORESOURCE_* flags for bridge addresses */

And at least two backends (sysfs.c and win32-cfgmgr32.c) are filling the
bridge_base_addr[] without flags.

So I would prefer to change lspci.c code instead to look for flags into
the flags field. Otherwise other backends and documentation in pci.h
would be inconsistent with sysfs.