[pciutils,0/5] Support for PROGIF, REVID and SUBSYS

libpci currently provides only access to bits [23:8] of class id via
dev->device_class member. Remaining bits [7:0] of class id can be only
accessed via reading config space.

lspci in some places reads class id from dev->device_class and on some
places from config space because it does not have access to all class bits.

For some broken devices kernel reports different class id via sysfs as
what is stored in device config space. Value reported by sysfs should be
the correct one.

lspci has -b option (Bus-centric view) to choose if information from
kernel or from config space should be showed. But this option is not
respected on all places because of missing bits.

Same applies for vendor+device ids, subsystem ids and revision id.

Export all these information from sysfs via libpci API use it in lspci.
With this change lspci should now respect -b option for all these
information.

With this change are in libpci and lspci also available subsystem ids
for PCI-to-PCI bridges.

This patch series is based on top of another patch series:
https://lore.kernel.org/linux-pci/20211220155448.1233-3-pali@kernel.org/

Pali Rohár (5):
  libpci: Add new options for pci_fill_info: PROGIF, REVID and SUBSYS
  libpci: generic: Implement PROGIF, REVID and SUBSYS support
  libpci: generic: Implement SUBSYS also for PCI_HEADER_TYPE_BRIDGE
  libpci: sysfs: Implement PROGIF, REVID and SUBSYS support
  lspci: Retrieve prog if, subsystem ids and revision id via libpci

 lib/generic.c | 55 +++++++++++++++++++++++++++++++++++++++-
 lib/pci.h     |  6 +++++
 lib/sysfs.c   | 39 +++++++++++++++++++++++++---
 ls-kernel.c   |  8 +++---
 lspci.c       | 70 ++++++++++++++++++---------------------------------
 lspci.h       |  2 --
 6 files changed, 123 insertions(+), 57 deletions(-)

Hello!

> libpci currently provides only access to bits [23:8] of class id via
> dev->device_class member. Remaining bits [7:0] of class id can be only
> accessed via reading config space.
[...]

I really do not like the explosion of PCI_FILL_xxx flags for trivial things.

Add a single PCI_FILL_CLASS_EXT, which will fill the class, subclass,
revision and programming interface.

					Martin

On Friday 21 January 2022 15:40:14 Martin Mareš wrote:
> Hello!
> 
> > libpci currently provides only access to bits [23:8] of class id via
> > dev->device_class member. Remaining bits [7:0] of class id can be only
> > accessed via reading config space.
> [...]
> 
> I really do not like the explosion of PCI_FILL_xxx flags for trivial things.
> 
> Add a single PCI_FILL_CLASS_EXT, which will fill the class, subclass,
> revision and programming interface.

Ok!

How to handle situation when "class+subclass+prog_if" is provided and
revision is not provided? What should libpci backends set in this case?
Because on both Linux and Windows systems are these information provided
separately. On Linux you can chmod 000 revision sysfs file and let class
sysfs file still readable. Windows can probably decide itself that it
would not report revision at all...

And what to do with subsystem ids? They are not part of
class/subclass/prog_if/revision fields and different devices have them
stored on different locations... And for PCI-to-PCI bridges they are
only optional and does not have to be present at all.

Hello!

> How to handle situation when "class+subclass+prog_if" is provided and
> revision is not provided? What should libpci backends set in this case?
> Because on both Linux and Windows systems are these information provided
> separately. On Linux you can chmod 000 revision sysfs file and let class
> sysfs file still readable. Windows can probably decide itself that it
> would not report revision at all...

Read it from the config registers in that case.

> And what to do with subsystem ids? They are not part of
> class/subclass/prog_if/revision fields and different devices have them
> stored on different locations... And for PCI-to-PCI bridges they are
> only optional and does not have to be present at all.

I would prefer a separate PCI_FILL_xxx flag for them.

			Have a nice fortnight

On Friday 21 January 2022 16:15:32 Martin Mareš wrote:
> Hello!
> 
> > How to handle situation when "class+subclass+prog_if" is provided and
> > revision is not provided? What should libpci backends set in this case?
> > Because on both Linux and Windows systems are these information provided
> > separately. On Linux you can chmod 000 revision sysfs file and let class
> > sysfs file still readable. Windows can probably decide itself that it
> > would not report revision at all...
> 
> Read it from the config registers in that case.

On Linux "class+subclass+prog_if" can be different than what is in
config registers and the purpose of this patch series with new fields is
to allow user to see difference in lspci.

On Windows access to real config space is not available for non-system
users.

> > And what to do with subsystem ids? They are not part of
> > class/subclass/prog_if/revision fields and different devices have them
> > stored on different locations... And for PCI-to-PCI bridges they are
> > only optional and does not have to be present at all.
> 
> I would prefer a separate PCI_FILL_xxx flag for them.

So like PCI_FILL_SUBSYS flag in this patch series?

> On Linux "class+subclass+prog_if" can be different than what is in
> config registers and the purpose of this patch series with new fields is
> to allow user to see difference in lspci.
> 
> On Windows access to real config space is not available for non-system
> users.

Sure, I meant to read it from the config registers only if the back-end
is unable to supply it.

> > > And what to do with subsystem ids? They are not part of
> > > class/subclass/prog_if/revision fields and different devices have them
> > > stored on different locations... And for PCI-to-PCI bridges they are
> > > only optional and does not have to be present at all.
> > 
> > I would prefer a separate PCI_FILL_xxx flag for them.
> 
> So like PCI_FILL_SUBSYS flag in this patch series?

Yes, that part was OK.

			Have a nice fortnight

On Friday 21 January 2022 16:29:44 Martin Mareš wrote:
> > On Linux "class+subclass+prog_if" can be different than what is in
> > config registers and the purpose of this patch series with new fields is
> > to allow user to see difference in lspci.
> > 
> > On Windows access to real config space is not available for non-system
> > users.
> 
> Sure, I meant to read it from the config registers only if the back-end
> is unable to supply it.

And in case "class+subclass+prog_if" is available but "revision" is not
available and even it is not possible to read revision from config space
(because e.g. due to missing permissions or limitations of win backend)?

> And in case "class+subclass+prog_if" is available but "revision" is not
> available and even it is not possible to read revision from config space
> (because e.g. due to missing permissions or limitations of win backend)?

I bet that nobody will ever check if reading of something as basic as the
revision ID succeeded. It can never fail on sane systems.

In such cases, just return a default value and log a warning.

				Have a nice fortnight

On Friday 21 January 2022 16:38:50 Martin Mareš wrote:
> > And in case "class+subclass+prog_if" is available but "revision" is not
> > available and even it is not possible to read revision from config space
> > (because e.g. due to missing permissions or limitations of win backend)?
> 
> I bet that nobody will ever check if reading of something as basic as the
> revision ID succeeded. It can never fail on sane systems.
> 
> In such cases, just return a default value and log a warning.

Ok! Sorry for a longer discussion as it is only corner case, but it can
happen (probably more often happens on windows) and therefore needs to
be somehow handled in the code.