[PATCH pciutils 1/4] lspci: Show 16/32/64 bit width for address ranges behind bridge

[PATCH pciutils 1/4] lspci: Show 16/32/64 bit width for address ranges behind bridge

[Pali Rohár]

Type of address range is encoded in lower bits.
---
 lspci.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/lspci.c b/lspci.c
index aba2745a9192..17649a0540fa 100644
--- a/lspci.c
+++ b/lspci.c
@@ -374,12 +374,12 @@ show_size(u64 x)
 }
 
 static void
-show_range(char *prefix, u64 base, u64 limit, int is_64bit)
+show_range(char *prefix, u64 base, u64 limit, int bits)
 {
   printf("%s:", prefix);
   if (base <= limit || verbose > 2)
     {
-      if (is_64bit)
+      if (bits > 32)
         printf(" %016" PCI_U64_FMT_X "-%016" PCI_U64_FMT_X, base, limit);
       else
         printf(" %08x-%08x", (unsigned) base, (unsigned) limit);
@@ -388,6 +388,7 @@ show_range(char *prefix, u64 base, u64 limit, int is_64bit)
     show_size(limit - base + 1);
   else
     printf(" [disabled]");
+  printf(" [%d-bit]", bits);
   putchar('\n');
 }
 
@@ -578,7 +579,7 @@ show_htype1(struct device *d)
 	  io_base |= (get_conf_word(d, PCI_IO_BASE_UPPER16) << 16);
 	  io_limit |= (get_conf_word(d, PCI_IO_LIMIT_UPPER16) << 16);
 	}
-      show_range("\tI/O behind bridge", io_base, io_limit+0xfff, 0);
+      show_range("\tI/O behind bridge", io_base, io_limit+0xfff, (io_type == PCI_IO_RANGE_TYPE_32) ? 32 : 16);
     }
 
   if (mem_type != (mem_limit & PCI_MEMORY_RANGE_TYPE_MASK) ||
@@ -588,7 +589,7 @@ show_htype1(struct device *d)
     {
       mem_base = (mem_base & PCI_MEMORY_RANGE_MASK) << 16;
       mem_limit = (mem_limit & PCI_MEMORY_RANGE_MASK) << 16;
-      show_range("\tMemory behind bridge", mem_base, mem_limit + 0xfffff, 0);
+      show_range("\tMemory behind bridge", mem_base, mem_limit + 0xfffff, 32);
     }
 
   if (pref_type != (pref_limit & PCI_PREF_RANGE_TYPE_MASK) ||
@@ -603,7 +604,7 @@ show_htype1(struct device *d)
 	  pref_base_64 |= (u64) get_conf_long(d, PCI_PREF_BASE_UPPER32) << 32;
 	  pref_limit_64 |= (u64) get_conf_long(d, PCI_PREF_LIMIT_UPPER32) << 32;
 	}
-      show_range("\tPrefetchable memory behind bridge", pref_base_64, pref_limit_64 + 0xfffff, (pref_type == PCI_PREF_RANGE_TYPE_64));
+      show_range("\tPrefetchable memory behind bridge", pref_base_64, pref_limit_64 + 0xfffff, (pref_type == PCI_PREF_RANGE_TYPE_64) ? 64 : 32);
     }
 
   if (verbose > 1)
-- 
2.20.1

[PATCH pciutils 2/4] lspci: Simplify printing range in show_range()

[Pali Rohár]

Use just one printf() call with width format argument based on number of bits.
---
 lspci.c | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/lspci.c b/lspci.c
index 17649a0540fa..67ac19b61a29 100644
--- a/lspci.c
+++ b/lspci.c
@@ -378,12 +378,7 @@ show_range(char *prefix, u64 base, u64 limit, int bits)
 {
   printf("%s:", prefix);
   if (base <= limit || verbose > 2)
-    {
-      if (bits > 32)
-        printf(" %016" PCI_U64_FMT_X "-%016" PCI_U64_FMT_X, base, limit);
-      else
-        printf(" %08x-%08x", (unsigned) base, (unsigned) limit);
-    }
+    printf(" %0*" PCI_U64_FMT_X "-%0*" PCI_U64_FMT_X, (bits+3)/4, base, (bits+3)/4, limit);
   if (base <= limit)
     show_size(limit - base + 1);
   else
-- 
2.20.1

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

[Pali Rohár]

Extend libpci API and ABI to fill bridge resources from sysfs.
---
 lib/access.c   | 20 +++++++++++---------
 lib/caps.c     |  2 +-
 lib/filter.c   |  2 +-
 lib/internal.h |  1 +
 lib/libpci.ver |  5 +++++
 lib/pci.h      |  4 ++++
 lib/sysfs.c    | 48 ++++++++++++++++++++++++++++++++++++++++++------
 7 files changed, 65 insertions(+), 17 deletions(-)

diff --git a/lib/access.c b/lib/access.c
index b257849a685e..a33e067e2389 100644
--- a/lib/access.c
+++ b/lib/access.c
@@ -189,7 +189,7 @@ pci_reset_properties(struct pci_dev *d)
 }
 
 int
-pci_fill_info_v35(struct pci_dev *d, int flags)
+pci_fill_info_v38(struct pci_dev *d, int flags)
 {
   unsigned int uflags = flags;
   if (uflags & PCI_FILL_RESCAN)
@@ -203,19 +203,21 @@ pci_fill_info_v35(struct pci_dev *d, int flags)
 }
 
 /* In version 3.1, pci_fill_info got new flags => versioned alias */
-/* In versions 3.2, 3.3, 3.4 and 3.5, the same has happened */
-STATIC_ALIAS(int pci_fill_info(struct pci_dev *d, int flags), pci_fill_info_v35(d, flags));
-DEFINE_ALIAS(int pci_fill_info_v30(struct pci_dev *d, int flags), pci_fill_info_v35);
-DEFINE_ALIAS(int pci_fill_info_v31(struct pci_dev *d, int flags), pci_fill_info_v35);
-DEFINE_ALIAS(int pci_fill_info_v32(struct pci_dev *d, int flags), pci_fill_info_v35);
-DEFINE_ALIAS(int pci_fill_info_v33(struct pci_dev *d, int flags), pci_fill_info_v35);
-DEFINE_ALIAS(int pci_fill_info_v34(struct pci_dev *d, int flags), pci_fill_info_v35);
+/* In versions 3.2, 3.3, 3.4, 3.5 and 3.8, the same has happened */
+STATIC_ALIAS(int pci_fill_info(struct pci_dev *d, int flags), pci_fill_info_v38(d, flags));
+DEFINE_ALIAS(int pci_fill_info_v30(struct pci_dev *d, int flags), pci_fill_info_v38);
+DEFINE_ALIAS(int pci_fill_info_v31(struct pci_dev *d, int flags), pci_fill_info_v38);
+DEFINE_ALIAS(int pci_fill_info_v32(struct pci_dev *d, int flags), pci_fill_info_v38);
+DEFINE_ALIAS(int pci_fill_info_v33(struct pci_dev *d, int flags), pci_fill_info_v38);
+DEFINE_ALIAS(int pci_fill_info_v34(struct pci_dev *d, int flags), pci_fill_info_v38);
+DEFINE_ALIAS(int pci_fill_info_v35(struct pci_dev *d, int flags), pci_fill_info_v38);
 SYMBOL_VERSION(pci_fill_info_v30, pci_fill_info@LIBPCI_3.0);
 SYMBOL_VERSION(pci_fill_info_v31, pci_fill_info@LIBPCI_3.1);
 SYMBOL_VERSION(pci_fill_info_v32, pci_fill_info@LIBPCI_3.2);
 SYMBOL_VERSION(pci_fill_info_v33, pci_fill_info@LIBPCI_3.3);
 SYMBOL_VERSION(pci_fill_info_v34, pci_fill_info@LIBPCI_3.4);
-SYMBOL_VERSION(pci_fill_info_v35, pci_fill_info@@LIBPCI_3.5);
+SYMBOL_VERSION(pci_fill_info_v35, pci_fill_info@LIBPCI_3.5);
+SYMBOL_VERSION(pci_fill_info_v38, pci_fill_info@@LIBPCI_3.8);
 
 void
 pci_setup_cache(struct pci_dev *d, byte *cache, int len)
diff --git a/lib/caps.c b/lib/caps.c
index c3b918059fe1..70a41b836e31 100644
--- a/lib/caps.c
+++ b/lib/caps.c
@@ -129,7 +129,7 @@ pci_find_cap_nr(struct pci_dev *d, unsigned int id, unsigned int type,
   unsigned int target = (cap_number ? *cap_number : 0);
   unsigned int index = 0;
 
-  pci_fill_info_v35(d, ((type == PCI_CAP_NORMAL) ? PCI_FILL_CAPS : PCI_FILL_EXT_CAPS));
+  pci_fill_info_v38(d, ((type == PCI_CAP_NORMAL) ? PCI_FILL_CAPS : PCI_FILL_EXT_CAPS));
 
   for (c=d->first_cap; c; c=c->next)
     {
diff --git a/lib/filter.c b/lib/filter.c
index 573fb2810363..195f813193c4 100644
--- a/lib/filter.c
+++ b/lib/filter.c
@@ -129,7 +129,7 @@ pci_filter_match_v33(struct pci_filter *f, struct pci_dev *d)
     return 0;
   if (f->device >= 0 || f->vendor >= 0)
     {
-      pci_fill_info_v35(d, PCI_FILL_IDENT);
+      pci_fill_info_v38(d, PCI_FILL_IDENT);
       if ((f->device >= 0 && f->device != d->device_id) ||
 	  (f->vendor >= 0 && f->vendor != d->vendor_id))
 	return 0;
diff --git a/lib/internal.h b/lib/internal.h
index 17c27e194a29..4df8dd70c2c6 100644
--- a/lib/internal.h
+++ b/lib/internal.h
@@ -74,6 +74,7 @@ int pci_fill_info_v32(struct pci_dev *, int flags) VERSIONED_ABI;
 int pci_fill_info_v33(struct pci_dev *, int flags) VERSIONED_ABI;
 int pci_fill_info_v34(struct pci_dev *, int flags) VERSIONED_ABI;
 int pci_fill_info_v35(struct pci_dev *, int flags) VERSIONED_ABI;
+int pci_fill_info_v38(struct pci_dev *, int flags) VERSIONED_ABI;
 
 struct pci_property {
   struct pci_property *next;
diff --git a/lib/libpci.ver b/lib/libpci.ver
index e20c3f581c71..73f7fa71e357 100644
--- a/lib/libpci.ver
+++ b/lib/libpci.ver
@@ -82,3 +82,8 @@ LIBPCI_3.7 {
 	global:
 		pci_find_cap_nr;
 };
+
+LIBPCI_3.8 {
+	global:
+		pci_fill_info;
+};
diff --git a/lib/pci.h b/lib/pci.h
index 814247691086..0ec7f211ca24 100644
--- a/lib/pci.h
+++ b/lib/pci.h
@@ -142,6 +142,9 @@ struct pci_dev {
   pciaddr_t flags[6];			/* PCI_IORESOURCE_* flags for regions */
   pciaddr_t rom_flags;			/* PCI_IORESOURCE_* flags for expansion ROM */
   int domain;				/* PCI domain (host bridge) */
+  pciaddr_t bridge_base_addr[4];	/* Bridge base addresses (without flags) */
+  pciaddr_t bridge_size[4];		/* Bridge sizes */
+  pciaddr_t bridge_flags[4];		/* PCI_IORESOURCE_* flags for bridge addresses */
 
   /* Fields used internally */
   struct pci_access *access;
@@ -205,6 +208,7 @@ char *pci_get_string_property(struct pci_dev *d, u32 prop) PCI_ABI;
 #define PCI_FILL_IO_FLAGS	0x1000
 #define PCI_FILL_DT_NODE	0x2000		/* Device tree node */
 #define PCI_FILL_IOMMU_GROUP	0x4000
+#define PCI_FILL_BRIDGE_BASES	0x8000
 #define PCI_FILL_RESCAN		0x00010000
 
 void pci_setup_cache(struct pci_dev *, u8 *cache, int len) PCI_ABI;
diff --git a/lib/sysfs.c b/lib/sysfs.c
index fb6424105e84..7c157a2688ad 100644
--- a/lib/sysfs.c
+++ b/lib/sysfs.c
@@ -148,19 +148,22 @@ sysfs_get_value(struct pci_dev *d, char *object, int mandatory)
     return -1;
 }
 
-static void
+static unsigned int
 sysfs_get_resources(struct pci_dev *d)
 {
   struct pci_access *a = d->access;
   char namebuf[OBJNAMELEN], buf[256];
+  struct { pciaddr_t flags, base_addr, size; } lines[10];
+  unsigned int done;
   FILE *file;
   int i;
 
+  done = 0;
   sysfs_obj_name(d, "resource", namebuf);
   file = fopen(namebuf, "r");
   if (!file)
     a->error("Cannot open %s: %s", namebuf, strerror(errno));
-  for (i = 0; i < 7; i++)
+  for (i = 0; i < 7+6+4+1; i++)
     {
       unsigned long long start, end, size, flags;
       if (!fgets(buf, sizeof(buf), file))
@@ -177,16 +180,50 @@ sysfs_get_resources(struct pci_dev *d)
 	  flags &= PCI_ADDR_FLAG_MASK;
 	  d->base_addr[i] = start | flags;
 	  d->size[i] = size;
+	  done |= PCI_FILL_BASES | PCI_FILL_SIZES | PCI_FILL_IO_FLAGS;
 	}
-      else
+      else if (i == 6)
 	{
 	  d->rom_flags = flags;
 	  flags &= PCI_ADDR_FLAG_MASK;
 	  d->rom_base_addr = start | flags;
 	  d->rom_size = size;
+	  done |= PCI_FILL_ROM_BASE;
 	}
+      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)
+    {
+      int offset = (i == 7+6+4) ? 6 : 0;
+      for (i = 0; i < 4; i++)
+        {
+          d->bridge_flags[i] = lines[offset+i].flags;
+          d->bridge_base_addr[i] = lines[offset+i].base_addr;
+          d->bridge_size[i] = lines[offset+i].size;
+        }
+      done |= PCI_FILL_BRIDGE_BASES;
     }
   fclose(file);
+  return done;
 }
 
 static void sysfs_scan(struct pci_access *a)
@@ -316,10 +353,9 @@ sysfs_fill_info(struct pci_dev *d, unsigned int flags)
 	  d->irq = sysfs_get_value(d, "irq", 1);
 	  done |= PCI_FILL_IRQ;
 	}
-      if (flags & (PCI_FILL_BASES | PCI_FILL_ROM_BASE | PCI_FILL_SIZES | PCI_FILL_IO_FLAGS))
+      if (flags & (PCI_FILL_BASES | PCI_FILL_ROM_BASE | PCI_FILL_SIZES | PCI_FILL_IO_FLAGS | PCI_FILL_BRIDGE_BASES))
 	{
-	  sysfs_get_resources(d);
-	  done |= PCI_FILL_BASES | PCI_FILL_ROM_BASE | PCI_FILL_SIZES | PCI_FILL_IO_FLAGS;
+	  done |= sysfs_get_resources(d);
 	}
     }
 
-- 
2.20.1

[PATCH pciutils 4/4] lspci: Use PCI_FILL_BRIDGE_BASES to detect if range behind bridge is disabled or unsupported

[Pali Rohár]

Show resources behind bridge as reported by PCI_FILL_BRIDGE_BASES.

I/O or Prefetchable memory behind bridge is unsupported by bridge if both
base and limit bridge registers are read-only and returns zero. So if base
and limit registers returns zero (which is valid enabled range) and kernel
reports that particular resource is disabled it means that resource is
unsupported. Both I/O or Prefetchable memory resources are only optional.
---
 lspci.c | 58 ++++++++++++++++++++++++++++++++++++++++++++++++---------
 1 file changed, 49 insertions(+), 9 deletions(-)

diff --git a/lspci.c b/lspci.c
index 67ac19b61a29..d14d1b9185d6 100644
--- a/lspci.c
+++ b/lspci.c
@@ -374,12 +374,12 @@ show_size(u64 x)
 }
 
 static void
-show_range(char *prefix, u64 base, u64 limit, int bits)
+show_range(char *prefix, u64 base, u64 limit, int bits, int disabled)
 {
   printf("%s:", prefix);
   if (base <= limit || verbose > 2)
     printf(" %0*" PCI_U64_FMT_X "-%0*" PCI_U64_FMT_X, (bits+3)/4, base, (bits+3)/4, limit);
-  if (base <= limit)
+  if (!disabled && base <= limit)
     show_size(limit - base + 1);
   else
     printf(" [disabled]");
@@ -543,6 +543,7 @@ show_htype0(struct device *d)
 static void
 show_htype1(struct device *d)
 {
+  struct pci_dev *p = d->dev;
   u32 io_base = get_conf_byte(d, PCI_IO_BASE);
   u32 io_limit = get_conf_byte(d, PCI_IO_LIMIT);
   u32 io_type = io_base & PCI_IO_RANGE_TYPE_MASK;
@@ -554,6 +555,10 @@ show_htype1(struct device *d)
   u32 pref_type = pref_base & PCI_PREF_RANGE_TYPE_MASK;
   word sec_stat = get_conf_word(d, PCI_SEC_STATUS);
   word brc = get_conf_word(d, PCI_BRIDGE_CONTROL);
+  int io_disabled = (p->known_fields & PCI_FILL_BRIDGE_BASES) && !p->bridge_size[0];
+  int mem_disabled = (p->known_fields & PCI_FILL_BRIDGE_BASES) && !p->bridge_size[1];
+  int pref_disabled = (p->known_fields & PCI_FILL_BRIDGE_BASES) && !p->bridge_size[2];
+  int io_bits, pref_bits;
 
   show_bases(d, 2);
   printf("\tBus: primary=%02x, secondary=%02x, subordinate=%02x, sec-latency=%d\n",
@@ -562,7 +567,15 @@ show_htype1(struct device *d)
 	 get_conf_byte(d, PCI_SUBORDINATE_BUS),
 	 get_conf_byte(d, PCI_SEC_LATENCY_TIMER));
 
-  if (io_type != (io_limit & PCI_IO_RANGE_TYPE_MASK) ||
+  if ((p->known_fields & PCI_FILL_BRIDGE_BASES) && !io_disabled)
+    {
+      io_base = p->bridge_base_addr[0] & PCI_IO_RANGE_MASK;
+      io_limit = io_base + p->bridge_size[0] - 1;
+      io_type = p->bridge_base_addr[0] & PCI_IO_RANGE_TYPE_MASK;
+      io_bits = (io_type == PCI_IO_RANGE_TYPE_32) ? 32 : 16;
+      show_range("\tI/O behind bridge", io_base, io_limit, io_bits, io_disabled);
+    }
+  else if (io_type != (io_limit & PCI_IO_RANGE_TYPE_MASK) ||
       (io_type != PCI_IO_RANGE_TYPE_16 && io_type != PCI_IO_RANGE_TYPE_32))
     printf("\t!!! Unknown I/O range types %x/%x\n", io_base, io_limit);
   else
@@ -574,20 +587,40 @@ show_htype1(struct device *d)
 	  io_base |= (get_conf_word(d, PCI_IO_BASE_UPPER16) << 16);
 	  io_limit |= (get_conf_word(d, PCI_IO_LIMIT_UPPER16) << 16);
 	}
-      show_range("\tI/O behind bridge", io_base, io_limit+0xfff, (io_type == PCI_IO_RANGE_TYPE_32) ? 32 : 16);
+      /* I/O is unsupported if both base and limit are zeros and resource is disabled */
+      if (!(io_base == 0x0 && io_limit == 0x0 && io_disabled))
+        {
+          io_limit += 0xfff;
+          io_bits = (io_type == PCI_IO_RANGE_TYPE_32) ? 32 : 16;
+          show_range("\tI/O behind bridge", io_base, io_limit, io_bits, io_disabled);
+        }
     }
 
-  if (mem_type != (mem_limit & PCI_MEMORY_RANGE_TYPE_MASK) ||
+  if ((p->known_fields & PCI_FILL_BRIDGE_BASES) && !mem_disabled)
+    {
+      mem_base = p->bridge_base_addr[1] & PCI_MEMORY_RANGE_MASK;
+      mem_limit = mem_base + p->bridge_size[1] - 1;
+      show_range("\tMemory behind bridge", mem_base, mem_limit, 32, mem_disabled);
+    }
+  else if (mem_type != (mem_limit & PCI_MEMORY_RANGE_TYPE_MASK) ||
       mem_type)
     printf("\t!!! Unknown memory range types %x/%x\n", mem_base, mem_limit);
   else
     {
       mem_base = (mem_base & PCI_MEMORY_RANGE_MASK) << 16;
       mem_limit = (mem_limit & PCI_MEMORY_RANGE_MASK) << 16;
-      show_range("\tMemory behind bridge", mem_base, mem_limit + 0xfffff, 32);
+      show_range("\tMemory behind bridge", mem_base, mem_limit + 0xfffff, 32, mem_disabled);
     }
 
-  if (pref_type != (pref_limit & PCI_PREF_RANGE_TYPE_MASK) ||
+  if ((p->known_fields & PCI_FILL_BRIDGE_BASES) && !pref_disabled)
+    {
+      u64 pref_base_64 = p->bridge_base_addr[2] & PCI_MEMORY_RANGE_MASK;
+      u64 pref_limit_64 = pref_base_64 + p->bridge_size[2] - 1;
+      pref_type = p->bridge_base_addr[2] & PCI_MEMORY_RANGE_TYPE_MASK;
+      pref_bits = (pref_type == PCI_PREF_RANGE_TYPE_64) ? 64 : 32;
+      show_range("\tPrefetchable memory behind bridge", pref_base_64, pref_limit_64, pref_bits, pref_disabled);
+    }
+  else if (pref_type != (pref_limit & PCI_PREF_RANGE_TYPE_MASK) ||
       (pref_type != PCI_PREF_RANGE_TYPE_32 && pref_type != PCI_PREF_RANGE_TYPE_64))
     printf("\t!!! Unknown prefetchable memory range types %x/%x\n", pref_base, pref_limit);
   else
@@ -599,7 +632,13 @@ show_htype1(struct device *d)
 	  pref_base_64 |= (u64) get_conf_long(d, PCI_PREF_BASE_UPPER32) << 32;
 	  pref_limit_64 |= (u64) get_conf_long(d, PCI_PREF_LIMIT_UPPER32) << 32;
 	}
-      show_range("\tPrefetchable memory behind bridge", pref_base_64, pref_limit_64 + 0xfffff, (pref_type == PCI_PREF_RANGE_TYPE_64) ? 64 : 32);
+      /* Prefetchable memory is unsupported if both base and limit are zeros and resource is disabled */
+      if (!(pref_base_64 == 0x0 && pref_limit_64 == 0x0 && pref_disabled))
+        {
+          pref_limit_64 += 0xfffff;
+          pref_bits = (pref_type == PCI_PREF_RANGE_TYPE_64) ? 64 : 32;
+          show_range("\tPrefetchable memory behind bridge", pref_base_64, pref_limit_64, pref_bits, pref_disabled);
+        }
     }
 
   if (verbose > 1)
@@ -726,7 +765,8 @@ show_verbose(struct device *d)
   show_terse(d);
 
   pci_fill_info(p, PCI_FILL_IRQ | PCI_FILL_BASES | PCI_FILL_ROM_BASE | PCI_FILL_SIZES |
-    PCI_FILL_PHYS_SLOT | PCI_FILL_NUMA_NODE | PCI_FILL_DT_NODE | PCI_FILL_IOMMU_GROUP);
+    PCI_FILL_PHYS_SLOT | PCI_FILL_NUMA_NODE | PCI_FILL_DT_NODE | PCI_FILL_IOMMU_GROUP |
+    PCI_FILL_BRIDGE_BASES);
   irq = p->irq;
 
   switch (htype)
-- 
2.20.1

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

[Martin Mareš]

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.

				Martin

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

[Pali Rohár]

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

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

[Pali Rohár]

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?

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

[Bjorn Helgaas]

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.

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

Bjorn

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

[Pali Rohár]

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.

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

[Bjorn Helgaas]

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

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

[Pali Rohár]

On Thursday 20 January 2022 15:02:12 Bjorn Helgaas wrote:
> 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

Usage is in patch 4/4. lspci with this change and also with patch 4/4 can
distinguish between states: 1) PCI-to-PCI bridge does not support IO and
2) PCI-to-PCI bridge has enabled IO forwarding range set to 0x0000-0x0fff.
Both these two states have IO_BASE and IO_LIMIT registers set to zeros.
lspci currently decodes IO_BASE=IO_LIMIT=0x00 as IO forwarding range is
enabled and set to range 0x0000-0x0fff.

IORESOURCE_WINDOW for PCI-to-PCI bridges

[Pali Rohár]

On Thursday 20 January 2022 15:02:12 Bjorn Helgaas wrote:
> 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.

Hello Bjorn, I would like to remind this older issue. Did you have a time
to audit usage of IORESOURCE_WINDOW? Some flag for resource forwarding
windows in PCI-to-PCI bridges would really help userspace application to
distinguish between IO/MEM BARs an IO/MEM forwarding windows.

Re: IORESOURCE_WINDOW for PCI-to-PCI bridges

[Bjorn Helgaas]

On Fri, Nov 11, 2022 at 09:09:45PM +0100, Pali Rohár wrote:
> On Thursday 20 January 2022 15:02:12 Bjorn Helgaas wrote:
> > On Thu, Jan 20, 2022 at 09:45:05PM +0100, Pali Rohár wrote:

[trimmed material; beginning of thread is at
https://lore.kernel.org/r/20211220155448.1233-3-pali@kernel.org]

> > > 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.
> 
> Hello Bjorn, I would like to remind this older issue. Did you have a time
> to audit usage of IORESOURCE_WINDOW? Some flag for resource forwarding
> windows in PCI-to-PCI bridges would really help userspace application to
> distinguish between IO/MEM BARs an IO/MEM forwarding windows.

I had forgotten all about this issue.  IIUC, the ultimate goal here
is to help lspci distinguish between an I/O window that's disabled and
one that's enabled at [io 0x0000-0x0fff].

I have not done the research to see whether it would be safe to set
IORESOURCE_WINDOW for PCI-to-PCI bridge windows.  I'm sorry if I left
the impression that I intended to do that.  I would welcome your help
to do that.

Bjorn

Re: IORESOURCE_WINDOW for PCI-to-PCI bridges

[Pali Rohár]

On Friday 11 November 2022 15:05:55 Bjorn Helgaas wrote:
> On Fri, Nov 11, 2022 at 09:09:45PM +0100, Pali Rohár wrote:
> > On Thursday 20 January 2022 15:02:12 Bjorn Helgaas wrote:
> > > On Thu, Jan 20, 2022 at 09:45:05PM +0100, Pali Rohár wrote:
> 
> [trimmed material; beginning of thread is at
> https://lore.kernel.org/r/20211220155448.1233-3-pali@kernel.org]
> 
> > > > 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.
> > 
> > Hello Bjorn, I would like to remind this older issue. Did you have a time
> > to audit usage of IORESOURCE_WINDOW? Some flag for resource forwarding
> > windows in PCI-to-PCI bridges would really help userspace application to
> > distinguish between IO/MEM BARs an IO/MEM forwarding windows.
> 
> I had forgotten all about this issue.  IIUC, the ultimate goal here
> is to help lspci distinguish between an I/O window that's disabled and
> one that's enabled at [io 0x0000-0x0fff].
> 
> I have not done the research to see whether it would be safe to set
> IORESOURCE_WINDOW for PCI-to-PCI bridge windows.  I'm sorry if I left
> the impression that I intended to do that.  I would welcome your help
> to do that.
> 
> Bjorn

Ok, do you have some resources or other information at which I should
look? I just do not know where to start or what to check for that
research.

I looked into kernel sources and the only places where is code checking
for IORESOURCE_WINDOW is ACPI related: arch/arm64/kernel/pci.c and
drivers/pnp/resource.c. And I do not fully understand how is ACPI
connected with PCI resources at this level. Other places which check
(lib/vsprintf.c and drivers/pnp/interface.c) just use it for
printf-formats.

Re: IORESOURCE_WINDOW for PCI-to-PCI bridges

[Bjorn Helgaas]

On Fri, Nov 11, 2022 at 10:48:16PM +0100, Pali Rohár wrote:
> On Friday 11 November 2022 15:05:55 Bjorn Helgaas wrote:
> > On Fri, Nov 11, 2022 at 09:09:45PM +0100, Pali Rohár wrote:
> > > On Thursday 20 January 2022 15:02:12 Bjorn Helgaas wrote:
> > > > On Thu, Jan 20, 2022 at 09:45:05PM +0100, Pali Rohár wrote:
> > 
> > [trimmed material; beginning of thread is at
> > https://lore.kernel.org/r/20211220155448.1233-3-pali@kernel.org]
> > 
> > > > > 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.
> > > 
> > > Hello Bjorn, I would like to remind this older issue. Did you have a time
> > > to audit usage of IORESOURCE_WINDOW? Some flag for resource forwarding
> > > windows in PCI-to-PCI bridges would really help userspace application to
> > > distinguish between IO/MEM BARs an IO/MEM forwarding windows.
> > 
> > I had forgotten all about this issue.  IIUC, the ultimate goal here
> > is to help lspci distinguish between an I/O window that's disabled and
> > one that's enabled at [io 0x0000-0x0fff].
> > 
> > I have not done the research to see whether it would be safe to set
> > IORESOURCE_WINDOW for PCI-to-PCI bridge windows.  I'm sorry if I left
> > the impression that I intended to do that.  I would welcome your help
> > to do that.
> 
> Ok, do you have some resources or other information at which I should
> look? I just do not know where to start or what to check for that
> research.
> 
> I looked into kernel sources and the only places where is code checking
> for IORESOURCE_WINDOW is ACPI related: arch/arm64/kernel/pci.c and
> drivers/pnp/resource.c. And I do not fully understand how is ACPI
> connected with PCI resources at this level. Other places which check
> (lib/vsprintf.c and drivers/pnp/interface.c) just use it for
> printf-formats.

Yeah, that's the kind of thing I have in mind.  I can't remember if I
had any specific concern.

Bjorn