On Thu, Jan 20, 2022 at 09:45:05PM +0100, Pali Rohár wrote:
> 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?
I looked at that, too. Today we only set IORESOURCE_WINDOW for host
bridge windows. Maybe it could be set for PCI-to-PCI bridge windows,
too. Would have to audit users to make sure it wouldn't break
anything.
> But in any case changes in kernel does not help lspci/libpci which is
> running on existing (unmodified) kernel.
Of course.
> > Does this patch fix a problem? I'm not clear on what the benefit is.
>
> 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.
The current subject and commit log are:
libpci: Add support for filling bridge resources
Extend libpci API and ABI to fill bridge resources from sysfs.
That doesn't give a reason why Martin should include this patch. Does
it fix a problem? Does it help lspci show more information? If so,
what is the difference in output?
Bjorn
