[PATCH pciutils 0/4] Support for PCI_FILL_PARENT

[PATCH pciutils 0/4] Support for PCI_FILL_PARENT

[Pali Rohár]

Extend libpci API with a new option PCI_FILL_PARENT to fill parent
device for the current enumerated device. This can be useful in
situation when non-complaint PCI-to-PCI bridge-like device with Type 0
header is present in the system and behind this bridge are either
endpoint devices or another non-compliant bridges. This applies e.g.
for notoriously broken Galileo and Marvell PCI and PCIe devices.
lspci can will use PCI_FILL_PARENT information from the system if
config space does not provide enough information to build topology.

Pali Rohár (4):
  libpci: Add new option PCI_FILL_PARENT
  libpci: sysfs: Implement support for PCI_FILL_PARENT
  lspci: Build tree based on PCI_FILL_PARENT information
  lspci: Do not show -[00]- bus in tree output

 lib/pci.h   |  2 ++
 lib/sysfs.c | 31 ++++++++++++++++++++++++
 ls-tree.c   | 69 +++++++++++++++++++++++++++++++++++++++++++++++++----
 lspci.c     |  2 +-
 4 files changed, 98 insertions(+), 6 deletions(-)

-- 
2.20.1

[PATCH pciutils 1/4] libpci: Add new option PCI_FILL_PARENT

[Pali Rohár]

This change extends libpci and allows providers to fill parent pci_dev.
This is useful to retrieve topology as it is reported by the system itself.
---
 lib/pci.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/lib/pci.h b/lib/pci.h
index c13387e2b4b1..de031f7a416b 100644
--- a/lib/pci.h
+++ b/lib/pci.h
@@ -148,6 +148,7 @@ struct pci_dev {
   u8 prog_if;				/* Programming interface for device_class */
   u8 rev_id;				/* Revision id */
   u16 subsys_vendor_id, subsys_id;	/* Subsystem vendor id and subsystem id */
+  struct pci_dev *parent;		/* Parent device */
 
   /* Fields used internally */
   struct pci_access *access;
@@ -217,6 +218,7 @@ char *pci_get_string_property(struct pci_dev *d, u32 prop) PCI_ABI;
 #define PCI_FILL_REVID		0x00040000
 #define PCI_FILL_SUBSYS		0x00080000
 #define PCI_FILL_DRIVER		0x00100000
+#define PCI_FILL_PARENT		0x00200000
 
 void pci_setup_cache(struct pci_dev *, u8 *cache, int len) PCI_ABI;
 
-- 
2.20.1

[PATCH pciutils 2/4] libpci: sysfs: Implement support for PCI_FILL_PARENT

[Pali Rohár]

---
 lib/sysfs.c | 31 +++++++++++++++++++++++++++++++
 1 file changed, 31 insertions(+)

diff --git a/lib/sysfs.c b/lib/sysfs.c
index 0dbd127ff53b..1db1b1a7752b 100644
--- a/lib/sysfs.c
+++ b/lib/sysfs.c
@@ -390,6 +390,37 @@ sysfs_fill_info(struct pci_dev *d, unsigned int flags)
 	{
 	  done |= sysfs_get_resources(d);
 	}
+      if (flags & PCI_FILL_PARENT)
+	{
+	  unsigned int domain, bus, dev, func;
+	  char *path_abs, *path_canon, *name;
+	  char path_rel[OBJNAMELEN];
+	  struct pci_dev *parent;
+
+	  /* Construct sysfs path for parent device */
+	  sysfs_obj_name(d, "..", path_rel);
+	  path_abs = realpath(path_rel, NULL);
+	  name = path_abs ? strrchr(path_abs, '/') : NULL;
+	  name = name ? name+1 : name;
+	  parent = NULL;
+
+	  if (name && sscanf(name, "%x:%x:%x.%d", &domain, &bus, &dev, &func) == 4 && domain <= 0x7fffffff)
+	    for (parent = d->access->devices; parent; parent = parent->next)
+	      if (parent->domain == (int)domain && parent->bus == bus && parent->dev == dev && parent->func == func)
+	        break;
+
+	  if (parent)
+	    {
+	      /* Check if parsed BDF address from parent sysfs device is really expected PCI device */
+	      sysfs_obj_name(parent, ".", path_rel);
+	      path_canon = realpath(path_rel, NULL);
+	      if (path_canon && strcmp(path_canon, path_abs) == 0)
+		{
+		  d->parent = parent;
+		  done |= PCI_FILL_PARENT;
+		}
+	    }
+	}
     }
 
   if (flags & PCI_FILL_PHYS_SLOT)
-- 
2.20.1

[PATCH pciutils 3/4] lspci: Build tree based on PCI_FILL_PARENT information

[Pali Rohár]

Topology reported by system (libpci provider) may be different from
topology built based on primary/secondary/subordinate numbers from PCI
bridges by lspci.

This happens for example when some non-compliant PCI-to-PCI bridge
with Type 0 header (e.g. Marvell one) is available in the system.

So add additional edges reported by libpci when building tree in lspci.
---
 ls-tree.c | 65 +++++++++++++++++++++++++++++++++++++++++++++++++++----
 lspci.c   |  2 +-
 2 files changed, 62 insertions(+), 5 deletions(-)

diff --git a/ls-tree.c b/ls-tree.c
index aaa1ee99d9f2..fede581156e4 100644
--- a/ls-tree.c
+++ b/ls-tree.c
@@ -25,6 +25,19 @@ find_bus(struct bridge *b, unsigned int domain, unsigned int n)
   return bus;
 }
 
+static struct device *
+find_device(struct pci_dev *dd)
+{
+  struct device *d;
+
+  if (!dd)
+    return NULL;
+  for (d=first_dev; d; d=d->next)
+    if (d->dev == dd)
+      break;
+  return d;
+}
+
 static struct bus *
 new_bus(struct bridge *b, unsigned int domain, unsigned int n)
 {
@@ -47,9 +60,20 @@ static void
 insert_dev(struct device *d, struct bridge *b)
 {
   struct pci_dev *p = d->dev;
-  struct bus *bus;
+  struct device *parent = NULL;
+  struct bus *bus = NULL;
+
+  if (p->known_fields & PCI_FILL_PARENT)
+    parent = find_device(p->parent);
 
-  if (! (bus = find_bus(b, p->domain, p->bus)))
+  if (parent && parent->bridge)
+    {
+      bus = parent->bridge->first_bus;
+      if (!bus)
+        bus = new_bus(parent->bridge, p->domain, p->bus);
+    }
+
+  if (!bus && ! (bus = find_bus(b, p->domain, p->bus)))
     {
       struct bridge *c;
       for (c=b->child; c; c=c->next)
@@ -113,14 +137,47 @@ grow_tree(void)
 	    b->primary, b->secondary, b->subordinate);
 	}
     }
+
+  /* Append additional bridges reported by libpci via d->parent */
+
+  for (d=first_dev; d; d=d->next)
+    {
+      struct device *parent = NULL;
+      if (d->dev->known_fields & PCI_FILL_PARENT)
+        parent = find_device(d->dev->parent);
+      if (!parent || parent->bridge)
+        continue;
+      b = xmalloc(sizeof(struct bridge));
+      b->domain = parent->dev->domain;
+      b->primary = parent->dev->bus;
+      b->secondary = d->dev->bus;
+      /* At this stage subordinate number is unknown, so set it to secondary bus number. */
+      b->subordinate = b->secondary;
+      *last_br = b;
+      last_br = &b->chain;
+      b->next = b->child = NULL;
+      b->first_bus = NULL;
+      b->last_bus = NULL;
+      b->br_dev = parent;
+      parent->bridge = b;
+      pacc->debug("Tree: bridge %04x:%02x:%02x.%d\n", b->domain,
+        parent->dev->bus, parent->dev->dev, parent->dev->func);
+    }
   *last_br = NULL;
 
   /* Create a bridge tree */
 
   for (b=&host_bridge; b; b=b->chain)
     {
-      struct bridge *c, *best;
-      best = NULL;
+      struct device *br_dev = b->br_dev;
+      struct bridge *c, *best = NULL;
+      struct device *parent = NULL;
+
+      if (br_dev && (br_dev->dev->known_fields & PCI_FILL_PARENT))
+        parent = find_device(br_dev->dev->parent);
+      if (parent)
+        best = parent->bridge;
+      if (!best)
       for (c=&host_bridge; c; c=c->chain)
 	if (c != b && (c == &host_bridge || b->domain == c->domain) &&
 	    b->primary >= c->secondary && b->primary <= c->subordinate &&
diff --git a/lspci.c b/lspci.c
index c4e6c93bc67a..b18ccba89049 100644
--- a/lspci.c
+++ b/lspci.c
@@ -143,7 +143,7 @@ scan_device(struct pci_dev *p)
 	d->config_cached += 64;
     }
   pci_setup_cache(p, d->config, d->config_cached);
-  pci_fill_info(p, PCI_FILL_IDENT | PCI_FILL_CLASS | PCI_FILL_PROGIF | PCI_FILL_REVID | PCI_FILL_SUBSYS);
+  pci_fill_info(p, PCI_FILL_IDENT | PCI_FILL_CLASS | PCI_FILL_PROGIF | PCI_FILL_REVID | PCI_FILL_SUBSYS | (need_topology ? PCI_FILL_PARENT : 0));
   return d;
 }
 
-- 
2.20.1

[PATCH pciutils 4/4] lspci: Do not show -[00]- bus in tree output

[Pali Rohár]

Secondary or subordinate bus cannot be zero. Zero value could indicate
either invalid secondary bus value or the fact that secondary bus value was
not filled or indicates non-compliant PCI-to-PCI bridge. This change makes
tree output better readable when bus numbers are not known or not provided.
---
 ls-tree.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/ls-tree.c b/ls-tree.c
index fede581156e4..f8154a2b034a 100644
--- a/ls-tree.c
+++ b/ls-tree.c
@@ -258,7 +258,9 @@ show_tree_dev(struct device *d, char *line, char *p)
   for (b=&host_bridge; b; b=b->chain)
     if (b->br_dev == d)
       {
-	if (b->secondary == b->subordinate)
+	if (b->secondary == 0)
+	  p = tree_printf(line, p, "-");
+	else if (b->secondary == b->subordinate)
 	  p = tree_printf(line, p, "-[%02x]-", b->secondary);
 	else
 	  p = tree_printf(line, p, "-[%02x-%02x]-", b->secondary, b->subordinate);
-- 
2.20.1

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

[Martin Mareš]

Hello!

> Extend libpci API with a new option PCI_FILL_PARENT to fill parent
> device for the current enumerated device. This can be useful in
> situation when non-complaint PCI-to-PCI bridge-like device with Type 0
> header is present in the system and behind this bridge are either
> endpoint devices or another non-compliant bridges. This applies e.g.
> for notoriously broken Galileo and Marvell PCI and PCIe devices.
> lspci can will use PCI_FILL_PARENT information from the system if
> config space does not provide enough information to build topology.

Looks reasonable, but please put a better explanation in pci.h
(in particular, mention that this is not guaranteed to be available).

When reading the implementation in the sysfs backend, I wonder how you
can guarantee that at the moment of parsing the child device, the parent
device is already known to libpci.

				Martin

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

[Pali Rohár]

On Friday 21 January 2022 15:51:34 Martin Mareš wrote:
> Hello!
> 
> > Extend libpci API with a new option PCI_FILL_PARENT to fill parent
> > device for the current enumerated device. This can be useful in
> > situation when non-complaint PCI-to-PCI bridge-like device with Type 0
> > header is present in the system and behind this bridge are either
> > endpoint devices or another non-compliant bridges. This applies e.g.
> > for notoriously broken Galileo and Marvell PCI and PCIe devices.
> > lspci can will use PCI_FILL_PARENT information from the system if
> > config space does not provide enough information to build topology.
> 
> Looks reasonable, but please put a better explanation in pci.h
> (in particular, mention that this is not guaranteed to be available).

Ok!

> When reading the implementation in the sysfs backend, I wonder how you
> can guarantee that at the moment of parsing the child device, the parent
> device is already known to libpci.

All devices are parsed in sysfs_scan() function. And additional
information, including this "parent" is filled in sysfs_fill_info()
function. sysfs_scan() itself does not call sysfs_fill_info().

So prior calling pci_fill_info() it is required to call pci_scan_bus()
to have ->parent member filled.

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

[Martin Mareš]

> All devices are parsed in sysfs_scan() function. And additional
> information, including this "parent" is filled in sysfs_fill_info()
> function. sysfs_scan() itself does not call sysfs_fill_info().
> 
> So prior calling pci_fill_info() it is required to call pci_scan_bus()
> to have ->parent member filled.

Ah, OK. My fault.

It's OK then.

			Have a nice fortnight
-- 
Martin `MJ' Mareš                        <mj@ucw.cz>   http://mj.ucw.cz/
United Computer Wizards, Prague, Czech Republic, Europe, Earth, Universe
A LISP programmer knows value of everything, but cost of nothing.