[PATCH pciutils 0/4] Support for PCI_FILL_DRIVER

[PATCH pciutils 0/4] Support for PCI_FILL_DRIVER

[Pali Rohár]

Both procfs and sysfs provides information about used PCI driver.
Add support for a new libpci string property PCI_FILL_DRIVER, fill it in
both procfs ans sysfs provides and use it in lspci instead of lspci own
sysfs code for retrieving driver.

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

Pali Rohár (4):
  libpci: Define new string property PCI_FILL_DRIVER
  libpci: proc: Implement support for PCI_FILL_DRIVER
  libpci: sysfs: Implement support for PCI_FILL_DRIVER
  lspci: Replace find_driver() via libpci PCI_FILL_DRIVER

 lib/pci.h   |  1 +
 lib/proc.c  | 22 +++++++++++++++++++--
 lib/sysfs.c | 13 +++++++++++++
 ls-kernel.c | 56 +++++++++++++++--------------------------------------
 4 files changed, 50 insertions(+), 42 deletions(-)

-- 
2.20.1

[PATCH pciutils 1/4] libpci: Define new string property PCI_FILL_DRIVER

[Pali Rohár]

This change extends libpci library and allows providers to fill
PCI_FILL_DRIVER via native system APIs. As it is string property there is
no need to increase ABI version.

Intended usage in application is just:

  const char *driver = pci_get_string_property(d->dev, PCI_FILL_DRIVER);
---
 lib/pci.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/lib/pci.h b/lib/pci.h
index 8c3c11b9ebeb..c13387e2b4b1 100644
--- a/lib/pci.h
+++ b/lib/pci.h
@@ -216,6 +216,7 @@ char *pci_get_string_property(struct pci_dev *d, u32 prop) PCI_ABI;
 #define PCI_FILL_PROGIF		0x00020000
 #define PCI_FILL_REVID		0x00040000
 #define PCI_FILL_SUBSYS		0x00080000
+#define PCI_FILL_DRIVER		0x00100000
 
 void pci_setup_cache(struct pci_dev *, u8 *cache, int len) PCI_ABI;
 
-- 
2.20.1

[PATCH pciutils 2/4] libpci: proc: Implement support for PCI_FILL_DRIVER

[Pali Rohár]

File /proc/bus/pci/devices contains optional driver name in the last 18th field.
---
 lib/proc.c | 22 ++++++++++++++++++++--
 1 file changed, 20 insertions(+), 2 deletions(-)

diff --git a/lib/proc.c b/lib/proc.c
index cb9d08d17768..9b33863e69e6 100644
--- a/lib/proc.c
+++ b/lib/proc.c
@@ -11,6 +11,7 @@
 #include <stdio.h>
 #include <string.h>
 #include <unistd.h>
+#include <ctype.h>
 #include <errno.h>
 #include <fcntl.h>
 #include <sys/types.h>
@@ -69,9 +70,11 @@ proc_scan(struct pci_access *a)
     {
       struct pci_dev *d = pci_alloc_dev(a);
       unsigned int dfn, vend, cnt, known;
+      char *driver;
+      int offset;
 
 #define F " " PCIADDR_T_FMT
-      cnt = sscanf(buf, "%x %x %x" F F F F F F F F F F F F F F,
+      cnt = sscanf(buf, "%x %x %x" F F F F F F F F F F F F F F "%n",
 	     &dfn,
 	     &vend,
 	     &d->irq,
@@ -88,7 +91,8 @@ proc_scan(struct pci_access *a)
 	     &d->size[3],
 	     &d->size[4],
 	     &d->size[5],
-	     &d->rom_size);
+	     &d->rom_size,
+	     &offset);
 #undef F
       if (cnt != 9 && cnt != 10 && cnt != 17)
 	a->error("proc: parse error (read only %d items)", cnt);
@@ -106,6 +110,20 @@ proc_scan(struct pci_access *a)
 	  if (cnt >= 17)
 	    known |= PCI_FILL_SIZES;
 	}
+      if (cnt >= 17)
+        {
+          while (buf[offset] && isspace(buf[offset]))
+            ++offset;
+          driver = &buf[offset];
+          while (buf[offset] && !isspace(buf[offset]))
+            ++offset;
+          buf[offset] = '\0';
+          if (driver[0])
+            {
+              pci_set_property(d, PCI_FILL_DRIVER, driver);
+              known |= PCI_FILL_DRIVER;
+            }
+        }
       d->known_fields = known;
       pci_link_dev(a, d);
     }
-- 
2.20.1

[PATCH pciutils 3/4] libpci: sysfs: Implement support for PCI_FILL_DRIVER

[Pali Rohár]

In sysfs is driver name stored as symlink path of "driver" node.
---
 lib/sysfs.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/lib/sysfs.c b/lib/sysfs.c
index 7714607f66a0..0dbd127ff53b 100644
--- a/lib/sysfs.c
+++ b/lib/sysfs.c
@@ -445,6 +445,19 @@ sysfs_fill_info(struct pci_dev *d, unsigned int flags)
       done |= PCI_FILL_DT_NODE;
     }
 
+  if (flags & PCI_FILL_DRIVER)
+    {
+      char *driver_path = sysfs_deref_link(d, "driver");
+      if (driver_path)
+        {
+          char *driver = strrchr(driver_path, '/');
+          driver = driver ? driver+1 : driver_path;
+          pci_set_property(d, PCI_FILL_DRIVER, driver);
+          free(driver_path);
+        }
+      done |= PCI_FILL_DRIVER;
+    }
+
   return done | pci_generic_fill_info(d, flags & ~done);
 }
 
-- 
2.20.1

[PATCH pciutils 4/4] lspci: Replace find_driver() via libpci PCI_FILL_DRIVER

[Pali Rohár]

And implement this show_kernel() and show_kernel_machine() for all
platforms.
---
 ls-kernel.c | 56 +++++++++++++++--------------------------------------
 1 file changed, 16 insertions(+), 40 deletions(-)

diff --git a/ls-kernel.c b/ls-kernel.c
index 2284b4625b12..3c484af7a9e0 100644
--- a/ls-kernel.c
+++ b/ls-kernel.c
@@ -212,40 +212,6 @@ show_kernel_cleanup(void)
 
 #endif
 
-#define DRIVER_BUF_SIZE 1024
-
-static char *
-find_driver(struct device *d, char *buf)
-{
-  struct pci_dev *dev = d->dev;
-  char name[1024], *drv, *base;
-  int n;
-
-  if (dev->access->method != PCI_ACCESS_SYS_BUS_PCI)
-    return NULL;
-
-  base = pci_get_param(dev->access, "sysfs.path");
-  if (!base || !base[0])
-    return NULL;
-
-  n = snprintf(name, sizeof(name), "%s/devices/%04x:%02x:%02x.%d/driver",
-	       base, dev->domain, dev->bus, dev->dev, dev->func);
-  if (n < 0 || n >= (int)sizeof(name))
-    die("show_driver: sysfs device name too long, why?");
-
-  n = readlink(name, buf, DRIVER_BUF_SIZE);
-  if (n < 0)
-    return NULL;
-  if (n >= DRIVER_BUF_SIZE)
-    return "<name-too-long>";
-  buf[n] = 0;
-
-  if (drv = strrchr(buf, '/'))
-    return drv+1;
-  else
-    return buf;
-}
-
 static const char *
 next_module_filtered(struct device *d)
 {
@@ -268,10 +234,10 @@ next_module_filtered(struct device *d)
 void
 show_kernel(struct device *d)
 {
-  char buf[DRIVER_BUF_SIZE];
   const char *driver, *module;
 
-  if (driver = find_driver(d, buf))
+  pci_fill_info(d->dev, PCI_FILL_DRIVER);
+  if (driver = pci_get_string_property(d->dev, PCI_FILL_DRIVER))
     printf("\tKernel driver in use: %s\n", driver);
 
   if (!show_kernel_init())
@@ -287,10 +253,10 @@ show_kernel(struct device *d)
 void
 show_kernel_machine(struct device *d)
 {
-  char buf[DRIVER_BUF_SIZE];
   const char *driver, *module;
 
-  if (driver = find_driver(d, buf))
+  pci_fill_info(d->dev, PCI_FILL_DRIVER);
+  if (driver = pci_get_string_property(d->dev, PCI_FILL_DRIVER))
     printf("Driver:\t%s\n", driver);
 
   if (!show_kernel_init())
@@ -303,13 +269,23 @@ show_kernel_machine(struct device *d)
 #else
 
 void
-show_kernel(struct device *d UNUSED)
+show_kernel(struct device *d)
 {
+  const char *driver;
+
+  pci_fill_info(d->dev, PCI_FILL_DRIVER);
+  if (driver = pci_get_string_property(d->dev, PCI_FILL_DRIVER))
+    printf("\tDriver in use: %s\n", driver);
 }
 
 void
-show_kernel_machine(struct device *d UNUSED)
+show_kernel_machine(struct device *d)
 {
+  const char *driver;
+
+  pci_fill_info(d->dev, PCI_FILL_DRIVER);
+  if (driver = pci_get_string_property(d->dev, PCI_FILL_DRIVER))
+    printf("Driver:\t%s\n", driver);
 }
 
 void
-- 
2.20.1

Re: [PATCH pciutils 0/4] Support for PCI_FILL_DRIVER

[Martin Mareš]

Hello!

> Both procfs and sysfs provides information about used PCI driver.
> Add support for a new libpci string property PCI_FILL_DRIVER, fill it in
> both procfs ans sysfs provides and use it in lspci instead of lspci own
> sysfs code for retrieving driver.

Is there any meaningful difference in the reported data?

				Martin

Re: [PATCH pciutils 0/4] Support for PCI_FILL_DRIVER

[Pali Rohár]

On Friday 21 January 2022 15:31:42 Martin Mareš wrote:
> Hello!
> 
> > Both procfs and sysfs provides information about used PCI driver.
> > Add support for a new libpci string property PCI_FILL_DRIVER, fill it in
> > both procfs ans sysfs provides and use it in lspci instead of lspci own
> > sysfs code for retrieving driver.
> 
> Is there any meaningful difference in the reported data?

Reported data are exactly same. Just with this change lspci can report
driver also for non-sysfs backend (e.g. procfs backend or new WIP win32
backend).

Basically sysfs logic from lspci executable was moved into the libpci
library and therefore now it is available also for other applications.


So difference for -A linux-proc is following:

Without this patch series:
./lspci -A linux-proc -s 00:02.0 -v
00:02.0 VGA compatible controller: Intel Corporation 4th Gen Core Processor Integrated Graphics Controller (rev 06) (prog-if 00 [VGA controller])
        Subsystem: Dell 4th Gen Core Processor Integrated Graphics Controller
        Flags: bus master, fast devsel, latency 0, IRQ 33
        Memory at f5800000 (64-bit, non-prefetchable) [size=4M]
        Memory at d0000000 (64-bit, prefetchable) [size=256M]
        I/O ports at f000 [size=64]
        Expansion ROM at 000c0000 [virtual] [disabled] [size=128K]
        Capabilities: <access denied>

And with this patch series:
./lspci -A linux-proc -s 00:02.0 -v
00:02.0 VGA compatible controller: Intel Corporation 4th Gen Core Processor Integrated Graphics Controller (rev 06) (prog-if 00 [VGA controller])
        Subsystem: Dell 4th Gen Core Processor Integrated Graphics Controller
        Flags: bus master, fast devsel, latency 0, IRQ 33
        Memory at f5800000 (64-bit, non-prefetchable) [size=4M]
        Memory at d0000000 (64-bit, prefetchable) [size=256M]
        I/O ports at f000 [size=64]
        Expansion ROM at 000c0000 [virtual] [disabled] [size=128K]
        Capabilities: <access denied>
        Kernel driver in use: i915

Re: [PATCH pciutils 0/4] Support for PCI_FILL_DRIVER

[Martin Mareš]

Hello!

> Reported data are exactly same. Just with this change lspci can report
> driver also for non-sysfs backend (e.g. procfs backend or new WIP win32
> backend).
> 
> Basically sysfs logic from lspci executable was moved into the libpci
> library and therefore now it is available also for other applications.

I am not convinced that it is right to make something with so vague
semantics a part of libpci API.

The procfs back-end is a pretty weak motivation, because it's long obsolete
and basically I haven't dropped it yet only because it takes more effort
than letting it stay :-)

Now, the win32 back-end could be intersting, but let's make it mergeable
in its basic form first.

				Martin