Re: [PATCH pciutils 3/4] libpci: Add support for filling bridge resources

["Pali Rohár" <pali@kernel.org>]

On Thursday 20 January 2022 14:33:52 Bjorn Helgaas wrote:
> On Fri, Dec 31, 2021 at 11:27:48PM +0100, Pali Rohár wrote:
> > On Sunday 26 December 2021 23:20:27 Pali Rohár wrote:
> > > On Sunday 26 December 2021 23:13:11 Martin Mareš wrote:
> > > > Hi!
> > > > 
> > > > > +      else if (i < 7+6+4)
> > > > > +        {
> > > > > +          /*
> > > > > +           * If kernel was compiled without CONFIG_PCI_IOV option then after
> > > > > +           * the ROM line for configured bridge device (that which had set
> > > > > +           * subordinary bus number to non-zero value) are four additional lines
> > > > > +           * which describe resources behind bridge. For PCI-to-PCI bridges they
> > > > > +           * are: IO, MEM, PREFMEM and empty. For CardBus bridges they are: IO0,
> > > > > +           * IO1, MEM0 and MEM1. For unconfigured bridges and other devices
> > > > > +           * there is no additional line after the ROM line. If kernel was
> > > > > +           * compiled with CONFIG_PCI_IOV option then after the ROM line and
> > > > > +           * before the first bridge resource line are six additional lines
> > > > > +           * which describe IOV resources. Read all remaining lines in resource
> > > > > +           * file and based on the number of remaining lines (0, 4, 6, 10) parse
> > > > > +           * resources behind bridge.
> > > > > +           */
> > > > > +          lines[i-7].flags = flags;
> > > > > +          lines[i-7].base_addr = start;
> > > > > +          lines[i-7].size = size;
> > > > > +        }
> > > > > +    }
> > > > > +  if (i == 7+4 || i == 7+6+4)
> > > > 
> > > > This looks crazy: is there any other way how to tell what the
> > > > bridge entries mean?  Checking the number of entries looks very
> > > > brittle.
> > > 
> > > I do not know any other way. Just for reference, here is a link to
> > > the function resource_show() and DEVICE_COUNT_RESOURCE enum:
> > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/pci/pci-sysfs.c?h=v5.15#n136
> > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/include/linux/pci.h?h=v5.15#n94
> > 
> > I have also checked flags and there is no indication if resource is
> > assigned on bridge as BAR or is forwarded behind the bridge.
> > 
> > Bjorn, Krzysztof: any idea if something better than checking number
> > of entries in "resource" node can be used to determinate type of
> > entry at specified line in "resource" node?
> 
> That *is* crazy.  I'm sorry that resource_show() works that way, and
> that it gives no clue to identify BAR vs ROM vs IOV BAR vs CB window
> vs regular bridge window.
> 
> It's conceivable that we could add "io_window" and "mem_window" files
> or something similar.

Meanwhile I found out that in linux/ioport.h file is IORESOURCE_WINDOW
constant with comment /* forwarded by bridge */
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/include/linux/ioport.h?h=v5.15#n56

But apparently it is not set for resources behind PCI bridges and
therefore it is not available in column of "resources" sysfs file.

So maybe instead of adding new sysfs files, it would be better way to
implement this flag and export it in flags column of "resources" file
for every row which belongs to resources behind bridges?

But in any case changes in kernel does not help lspci/libpci which is
running on existing (unmodified) kernel.

> Does this patch fix a problem?  I'm not clear on what the benefit is.
> 
> Bjorn

My patch for libpci fixes it, but via counting number of rows in
"resources" sysfs file... which is crazy. But I do not see any other
option how to do it via currently available kernel APIs.