* [PATCH pciutils 0/5] Support for PROGIF, REVID and SUBSYS
@ 2022-01-21 13:57 Pali Rohár
2022-01-21 13:57 ` [PATCH pciutils 1/5] libpci: Add new options for pci_fill_info: " Pali Rohár
` (5 more replies)
0 siblings, 6 replies; 17+ messages in thread
From: Pali Rohár @ 2022-01-21 13:57 UTC (permalink / raw)
To: Martin Mares, Bjorn Helgaas, Krzysztof Wilczyński,
Matthew Wilcox, linux-pci
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(-)
--
2.20.1
^ permalink raw reply [flat|nested] 17+ messages in thread* [PATCH pciutils 1/5] libpci: Add new options for pci_fill_info: PROGIF, REVID and SUBSYS 2022-01-21 13:57 [PATCH pciutils 0/5] Support for PROGIF, REVID and SUBSYS Pali Rohár @ 2022-01-21 13:57 ` Pali Rohár 2022-01-21 13:57 ` [PATCH pciutils 2/5] libpci: generic: Implement PROGIF, REVID and SUBSYS support Pali Rohár ` (4 subsequent siblings) 5 siblings, 0 replies; 17+ messages in thread From: Pali Rohár @ 2022-01-21 13:57 UTC (permalink / raw) To: Martin Mares, Bjorn Helgaas, Krzysztof Wilczyński, Matthew Wilcox, linux-pci This change extends libpci library and allows providers to fill these informations (Programming interface, Revision id and Subsystem ids) via native system APIs, which sometimes may differs from what is stored in PCI config space. Programming interface is part of 24-bit Device Class number but apparently libpci exports only high 16-bit of this number via device_class member. --- lib/pci.h | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/lib/pci.h b/lib/pci.h index b9fd9bfb9b5b..8c3c11b9ebeb 100644 --- a/lib/pci.h +++ b/lib/pci.h @@ -145,6 +145,9 @@ struct pci_dev { 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 */ + 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 */ /* Fields used internally */ struct pci_access *access; @@ -210,6 +213,9 @@ char *pci_get_string_property(struct pci_dev *d, u32 prop) PCI_ABI; #define PCI_FILL_IOMMU_GROUP 0x4000 #define PCI_FILL_BRIDGE_BASES 0x8000 #define PCI_FILL_RESCAN 0x00010000 +#define PCI_FILL_PROGIF 0x00020000 +#define PCI_FILL_REVID 0x00040000 +#define PCI_FILL_SUBSYS 0x00080000 void pci_setup_cache(struct pci_dev *, u8 *cache, int len) PCI_ABI; -- 2.20.1 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH pciutils 2/5] libpci: generic: Implement PROGIF, REVID and SUBSYS support 2022-01-21 13:57 [PATCH pciutils 0/5] Support for PROGIF, REVID and SUBSYS Pali Rohár 2022-01-21 13:57 ` [PATCH pciutils 1/5] libpci: Add new options for pci_fill_info: " Pali Rohár @ 2022-01-21 13:57 ` Pali Rohár 2022-01-21 13:57 ` [PATCH pciutils 3/5] libpci: generic: Implement SUBSYS also for PCI_HEADER_TYPE_BRIDGE Pali Rohár ` (3 subsequent siblings) 5 siblings, 0 replies; 17+ messages in thread From: Pali Rohár @ 2022-01-21 13:57 UTC (permalink / raw) To: Martin Mares, Bjorn Helgaas, Krzysztof Wilczyński, Matthew Wilcox, linux-pci PCI_FILL_SUBSYS is implemented only for PCI_HEADER_TYPE_NORMAL and PCI_HEADER_TYPE_CARDBUS like in lspci. --- lib/generic.c | 31 ++++++++++++++++++++++++++++++- 1 file changed, 30 insertions(+), 1 deletion(-) diff --git a/lib/generic.c b/lib/generic.c index ef9e2a34b4f4..f4b6918cb55b 100644 --- a/lib/generic.c +++ b/lib/generic.c @@ -80,7 +80,7 @@ pci_generic_fill_info(struct pci_dev *d, unsigned int flags) struct pci_access *a = d->access; unsigned int done = 0; - if ((flags & (PCI_FILL_BASES | PCI_FILL_ROM_BASE)) && d->hdrtype < 0) + if ((flags & (PCI_FILL_SUBSYS | PCI_FILL_BASES | PCI_FILL_ROM_BASE)) && d->hdrtype < 0) d->hdrtype = pci_read_byte(d, PCI_HEADER_TYPE) & 0x7f; if (flags & PCI_FILL_IDENT) @@ -96,6 +96,35 @@ pci_generic_fill_info(struct pci_dev *d, unsigned int flags) done |= PCI_FILL_CLASS; } + if (flags & PCI_FILL_PROGIF) + { + d->prog_if = pci_read_byte(d, PCI_CLASS_PROG); + done |= PCI_FILL_PROGIF; + } + + if (flags & PCI_FILL_REVID) + { + d->rev_id = pci_read_byte(d, PCI_REVISION_ID); + done |= PCI_FILL_REVID; + } + + if (flags & PCI_FILL_SUBSYS) + { + switch (d->hdrtype) + { + case PCI_HEADER_TYPE_NORMAL: + d->subsys_vendor_id = pci_read_word(d, PCI_SUBSYSTEM_VENDOR_ID); + d->subsys_id = pci_read_word(d, PCI_SUBSYSTEM_ID); + done |= PCI_FILL_SUBSYS; + break; + case PCI_HEADER_TYPE_CARDBUS: + d->subsys_vendor_id = pci_read_word(d, PCI_CB_SUBSYSTEM_VENDOR_ID); + d->subsys_id = pci_read_word(d, PCI_CB_SUBSYSTEM_ID); + done |= PCI_FILL_SUBSYS; + break; + } + } + if (flags & PCI_FILL_IRQ) { d->irq = pci_read_byte(d, PCI_INTERRUPT_LINE); -- 2.20.1 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH pciutils 3/5] libpci: generic: Implement SUBSYS also for PCI_HEADER_TYPE_BRIDGE 2022-01-21 13:57 [PATCH pciutils 0/5] Support for PROGIF, REVID and SUBSYS Pali Rohár 2022-01-21 13:57 ` [PATCH pciutils 1/5] libpci: Add new options for pci_fill_info: " Pali Rohár 2022-01-21 13:57 ` [PATCH pciutils 2/5] libpci: generic: Implement PROGIF, REVID and SUBSYS support Pali Rohár @ 2022-01-21 13:57 ` Pali Rohár 2022-01-21 14:40 ` Martin Mareš 2022-01-21 13:57 ` [PATCH pciutils 4/5] libpci: sysfs: Implement PROGIF, REVID and SUBSYS support Pali Rohár ` (2 subsequent siblings) 5 siblings, 1 reply; 17+ messages in thread From: Pali Rohár @ 2022-01-21 13:57 UTC (permalink / raw) To: Martin Mares, Bjorn Helgaas, Krzysztof Wilczyński, Matthew Wilcox, linux-pci Subsystem ids for PCI Bridges are stored in extended capability PCI_CAP_ID_SSVID. --- lib/generic.c | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/lib/generic.c b/lib/generic.c index f4b6918cb55b..2cdb7956754c 100644 --- a/lib/generic.c +++ b/lib/generic.c @@ -117,6 +117,30 @@ pci_generic_fill_info(struct pci_dev *d, unsigned int flags) d->subsys_id = pci_read_word(d, PCI_SUBSYSTEM_ID); done |= PCI_FILL_SUBSYS; break; + case PCI_HEADER_TYPE_BRIDGE: + if (pci_read_word(d, PCI_STATUS) & PCI_STATUS_CAP_LIST) + { + byte been_there[256]; + int where, id; + + memset(been_there, 0, 256); + where = pci_read_byte(d, PCI_CAPABILITY_LIST) & ~3; + while (where && !been_there[where]++) + { + id = pci_read_byte(d, where + PCI_CAP_LIST_ID); + if (id == PCI_CAP_ID_SSVID) + { + d->subsys_vendor_id = pci_read_word(d, where + PCI_SSVID_VENDOR); + d->subsys_id = pci_read_word(d, where + PCI_SSVID_DEVICE); + done |= PCI_FILL_SUBSYS; + break; + } + if (id == 0xff) + break; + where = pci_read_byte(d, where + PCI_CAP_LIST_NEXT) & ~3; + } + } + break; case PCI_HEADER_TYPE_CARDBUS: d->subsys_vendor_id = pci_read_word(d, PCI_CB_SUBSYSTEM_VENDOR_ID); d->subsys_id = pci_read_word(d, PCI_CB_SUBSYSTEM_ID); -- 2.20.1 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH pciutils 3/5] libpci: generic: Implement SUBSYS also for PCI_HEADER_TYPE_BRIDGE 2022-01-21 13:57 ` [PATCH pciutils 3/5] libpci: generic: Implement SUBSYS also for PCI_HEADER_TYPE_BRIDGE Pali Rohár @ 2022-01-21 14:40 ` Martin Mareš 2022-01-21 16:11 ` Pali Rohár 0 siblings, 1 reply; 17+ messages in thread From: Martin Mareš @ 2022-01-21 14:40 UTC (permalink / raw) To: Pali Rohár Cc: Bjorn Helgaas, Krzysztof Wilczyński, Matthew Wilcox, linux-pci Hello! > + case PCI_HEADER_TYPE_BRIDGE: > + if (pci_read_word(d, PCI_STATUS) & PCI_STATUS_CAP_LIST) > + { > + byte been_there[256]; > + int where, id; > + > + memset(been_there, 0, 256); > + where = pci_read_byte(d, PCI_CAPABILITY_LIST) & ~3; > + while (where && !been_there[where]++) Please don't. There should be a single implementation of capability list walking in libpci, not everybody doing his own. Martin ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH pciutils 3/5] libpci: generic: Implement SUBSYS also for PCI_HEADER_TYPE_BRIDGE 2022-01-21 14:40 ` Martin Mareš @ 2022-01-21 16:11 ` Pali Rohár 2022-01-21 20:43 ` Martin Mareš 0 siblings, 1 reply; 17+ messages in thread From: Pali Rohár @ 2022-01-21 16:11 UTC (permalink / raw) To: Martin Mareš Cc: Bjorn Helgaas, Krzysztof Wilczyński, Matthew Wilcox, linux-pci On Friday 21 January 2022 15:40:49 Martin Mareš wrote: > Hello! > > > + case PCI_HEADER_TYPE_BRIDGE: > > + if (pci_read_word(d, PCI_STATUS) & PCI_STATUS_CAP_LIST) > > + { > > + byte been_there[256]; > > + int where, id; > > + > > + memset(been_there, 0, 256); > > + where = pci_read_byte(d, PCI_CAPABILITY_LIST) & ~3; > > + while (where && !been_there[where]++) > > Please don't. There should be a single implementation of capability list > walking in libpci, not everybody doing his own. Current libpci code which walks capability list is in functions pci_scan_caps() and pci_find_cap(). But pci_find_cap() calls pci_fill_info() (only with PCI_FILL_CAPS flag). So I'm not sure if it is a good idea to call pci_find_cap() from pci_generic_fill_info(). ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH pciutils 3/5] libpci: generic: Implement SUBSYS also for PCI_HEADER_TYPE_BRIDGE 2022-01-21 16:11 ` Pali Rohár @ 2022-01-21 20:43 ` Martin Mareš 0 siblings, 0 replies; 17+ messages in thread From: Martin Mareš @ 2022-01-21 20:43 UTC (permalink / raw) To: Pali Rohár Cc: Bjorn Helgaas, Krzysztof Wilczyński, Matthew Wilcox, linux-pci Hello! > Current libpci code which walks capability list is in functions > pci_scan_caps() and pci_find_cap(). But pci_find_cap() calls > pci_fill_info() (only with PCI_FILL_CAPS flag). So I'm not sure if it is > a good idea to call pci_find_cap() from pci_generic_fill_info(). Still, the right solution is to change pci_fill_info() to allow recursion :) Here is my try. Martin commit 8e9299e4dde0bb73f2d571dc6e3b522f88d5765d Author: Martin Mares <mj@ucw.cz> Date: Fri Jan 21 21:39:18 2022 +0100 Simplified pci_fill_info() and friends Previously, we kept track of which fields were already filled, which was quite brittle. Now we keep only the set of already known fields in struct pci_dev. We check if the current field is needed against this information. Not only this simplifies the whole thing, but it also enables future back-ends to call pci_fill_info() recursively as needed. diff --git a/lib/access.c b/lib/access.c index b257849..9bd6989 100644 --- a/lib/access.c +++ b/lib/access.c @@ -1,7 +1,7 @@ /* * The PCI Library -- User Access * - * Copyright (c) 1997--2018 Martin Mares <mj@ucw.cz> + * Copyright (c) 1997--2022 Martin Mares <mj@ucw.cz> * * Can be freely distributed and used under the terms of the GNU GPL. */ @@ -198,7 +198,7 @@ pci_fill_info_v35(struct pci_dev *d, int flags) pci_reset_properties(d); } if (uflags & ~d->known_fields) - d->known_fields |= d->methods->fill_info(d, flags & ~d->known_fields); + d->methods->fill_info(d, uflags); return d->known_fields; } diff --git a/lib/caps.c b/lib/caps.c index c3b9180..3c025a9 100644 --- a/lib/caps.c +++ b/lib/caps.c @@ -80,17 +80,16 @@ pci_scan_ext_caps(struct pci_dev *d) while (where); } -unsigned int +void pci_scan_caps(struct pci_dev *d, unsigned int want_fields) { - if ((want_fields & PCI_FILL_EXT_CAPS) && !(d->known_fields & PCI_FILL_CAPS)) + if (want_fields & PCI_FILL_EXT_CAPS) want_fields |= PCI_FILL_CAPS; - if (want_fields & PCI_FILL_CAPS) + if (want_fill(d, want_fields, PCI_FILL_CAPS)) pci_scan_trad_caps(d); - if (want_fields & PCI_FILL_EXT_CAPS) + if (want_fill(d, want_fields, PCI_FILL_EXT_CAPS)) pci_scan_ext_caps(d); - return want_fields; } void diff --git a/lib/fbsd-device.c b/lib/fbsd-device.c index cffab69..396ff1d 100644 --- a/lib/fbsd-device.c +++ b/lib/fbsd-device.c @@ -159,7 +159,7 @@ fbsd_scan(struct pci_access *a) free(matches); } -static int +static void fbsd_fill_info(struct pci_dev *d, int flags) { struct pci_conf_io conf; @@ -200,16 +200,14 @@ fbsd_fill_info(struct pci_dev *d, int flags) d->access->error("fbsd_fill_info: ioctl(PCIOCGETCONF) failed: %s", strerror(errno)); } - if (flags & PCI_FILL_IDENT) + if (want_fill(d, flags, PCI_FILL_IDENT)) { d->vendor_id = match.pc_vendor; d->device_id = match.pc_device; } - if (flags & PCI_FILL_CLASS) - { - d->device_class = (match.pc_class << 8) | match.pc_subclass; - } - if (flags & (PCI_FILL_BASES | PCI_FILL_SIZES)) + if (want_fill(d, flags, PCI_FILL_CLASS)) + d->device_class = (match.pc_class << 8) | match.pc_subclass; + if (want_fill(d, flags PCI_FILL_BASES | PCI_FILL_SIZES)) { d->rom_base_addr = 0; d->rom_size = 0; @@ -242,9 +240,6 @@ fbsd_fill_info(struct pci_dev *d, int flags) } } } - - return flags & (PCI_FILL_IDENT | PCI_FILL_CLASS | PCI_FILL_BASES | - PCI_FILL_SIZES); } static int diff --git a/lib/generic.c b/lib/generic.c index ef9e2a3..e80158f 100644 --- a/lib/generic.c +++ b/lib/generic.c @@ -1,7 +1,7 @@ /* * The PCI Library -- Generic Direct Access Functions * - * Copyright (c) 1997--2000 Martin Mares <mj@ucw.cz> + * Copyright (c) 1997--2022 Martin Mares <mj@ucw.cz> * * Can be freely distributed and used under the terms of the GNU GPL. */ @@ -74,39 +74,36 @@ pci_generic_scan(struct pci_access *a) pci_generic_scan_bus(a, busmap, 0); } -unsigned int +static int +get_hdr_type(struct pci_dev *d) +{ + if (d->hdrtype < 0) + d->hdrtype = pci_read_byte(d, PCI_HEADER_TYPE) & 0x7f; + return d->hdrtype; +} + +void pci_generic_fill_info(struct pci_dev *d, unsigned int flags) { struct pci_access *a = d->access; - unsigned int done = 0; - if ((flags & (PCI_FILL_BASES | PCI_FILL_ROM_BASE)) && d->hdrtype < 0) - d->hdrtype = pci_read_byte(d, PCI_HEADER_TYPE) & 0x7f; - - if (flags & PCI_FILL_IDENT) + if (want_fill(d, flags, PCI_FILL_IDENT)) { d->vendor_id = pci_read_word(d, PCI_VENDOR_ID); d->device_id = pci_read_word(d, PCI_DEVICE_ID); - done |= PCI_FILL_IDENT; } - if (flags & PCI_FILL_CLASS) - { - d->device_class = pci_read_word(d, PCI_CLASS_DEVICE); - done |= PCI_FILL_CLASS; - } + if (want_fill(d, flags, PCI_FILL_CLASS)) + d->device_class = pci_read_word(d, PCI_CLASS_DEVICE); - if (flags & PCI_FILL_IRQ) - { - d->irq = pci_read_byte(d, PCI_INTERRUPT_LINE); - done |= PCI_FILL_IRQ; - } + if (want_fill(d, flags, PCI_FILL_IRQ)) + d->irq = pci_read_byte(d, PCI_INTERRUPT_LINE); - if (flags & PCI_FILL_BASES) + if (want_fill(d, flags, PCI_FILL_BASES)) { int cnt = 0, i; memset(d->base_addr, 0, sizeof(d->base_addr)); - switch (d->hdrtype) + switch (get_hdr_type(d)) { case PCI_HEADER_TYPE_NORMAL: cnt = 6; @@ -148,14 +145,13 @@ pci_generic_fill_info(struct pci_dev *d, unsigned int flags) } } } - done |= PCI_FILL_BASES; } - if (flags & PCI_FILL_ROM_BASE) + if (want_fill(d, flags, PCI_FILL_ROM_BASE)) { int reg = 0; d->rom_base_addr = 0; - switch (d->hdrtype) + switch (get_hdr_type(d)) { case PCI_HEADER_TYPE_NORMAL: reg = PCI_ROM_ADDRESS; @@ -170,13 +166,9 @@ pci_generic_fill_info(struct pci_dev *d, unsigned int flags) if (u != 0xffffffff) d->rom_base_addr = u; } - done |= PCI_FILL_ROM_BASE; } - if (flags & (PCI_FILL_CAPS | PCI_FILL_EXT_CAPS)) - done |= pci_scan_caps(d, flags); - - return done; + pci_scan_caps(d, flags); } static int diff --git a/lib/hurd.c b/lib/hurd.c index 90cf89f..0939a20 100644 --- a/lib/hurd.c +++ b/lib/hurd.c @@ -328,26 +328,16 @@ hurd_fill_rom(struct pci_dev *d) d->rom_size = rom.size; } -static unsigned int +static void hurd_fill_info(struct pci_dev *d, unsigned int flags) { - unsigned int done = 0; - if (!d->access->buscentric) { - if (flags & (PCI_FILL_BASES | PCI_FILL_SIZES)) - { - hurd_fill_regions(d); - done |= PCI_FILL_BASES | PCI_FILL_SIZES; - } - if (flags & PCI_FILL_ROM_BASE) - { - hurd_fill_rom(d); - done |= PCI_FILL_ROM_BASE; - } + if (want_fill(d, flags, PCI_FILL_BASES | PCI_FILL_SIZES)) + hurd_fill_regions(d); + if (want_fill(d, flags, PCI_FILL_ROM_BASE)) + hurd_fill_rom(d); } - - return done | pci_generic_fill_info(d, flags & ~done); } struct pci_methods pm_hurd = { diff --git a/lib/internal.h b/lib/internal.h index 17c27e1..942e3cb 100644 --- a/lib/internal.h +++ b/lib/internal.h @@ -1,7 +1,7 @@ /* * The PCI Library -- Internal Stuff * - * Copyright (c) 1997--2018 Martin Mares <mj@ucw.cz> + * Copyright (c) 1997--2022 Martin Mares <mj@ucw.cz> * * Can be freely distributed and used under the terms of the GNU GPL. */ @@ -41,7 +41,7 @@ struct pci_methods { void (*init)(struct pci_access *); void (*cleanup)(struct pci_access *); void (*scan)(struct pci_access *); - unsigned int (*fill_info)(struct pci_dev *, unsigned int flags); + void (*fill_info)(struct pci_dev *, unsigned int flags); int (*read)(struct pci_dev *, int pos, byte *buf, int len); int (*write)(struct pci_dev *, int pos, byte *buf, int len); int (*read_vpd)(struct pci_dev *, int pos, byte *buf, int len); @@ -52,7 +52,7 @@ struct pci_methods { /* generic.c */ void pci_generic_scan_bus(struct pci_access *, byte *busmap, int bus); void pci_generic_scan(struct pci_access *); -unsigned int pci_generic_fill_info(struct pci_dev *, unsigned int flags); +void pci_generic_fill_info(struct pci_dev *, unsigned int flags); int pci_generic_block_read(struct pci_dev *, int pos, byte *buf, int len); int pci_generic_block_write(struct pci_dev *, int pos, byte *buf, int len); @@ -75,6 +75,18 @@ 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; +static inline int want_fill(struct pci_dev *d, unsigned want_fields, unsigned int try_fields) +{ + want_fields &= try_fields; + if ((d->known_fields & want_fields) == want_fields) + return 0; + else + { + d->known_fields |= try_fields; + return 1; + } +} + struct pci_property { struct pci_property *next; u32 key; @@ -89,7 +101,7 @@ int pci_set_param_internal(struct pci_access *acc, char *param, char *val, int c void pci_free_params(struct pci_access *acc); /* caps.c */ -unsigned int pci_scan_caps(struct pci_dev *, unsigned int want_fields); +void pci_scan_caps(struct pci_dev *, unsigned int want_fields); void pci_free_caps(struct pci_dev *); extern struct pci_methods pm_intel_conf1, pm_intel_conf2, pm_linux_proc, diff --git a/lib/sysfs.c b/lib/sysfs.c index fb64241..4b8b2cd 100644 --- a/lib/sysfs.c +++ b/lib/sysfs.c @@ -288,11 +288,9 @@ sysfs_fill_slots(struct pci_access *a) closedir(dir); } -static unsigned int +static void sysfs_fill_info(struct pci_dev *d, unsigned int flags) { - unsigned int done = 0; - if (!d->access->buscentric) { /* @@ -300,61 +298,45 @@ sysfs_fill_info(struct pci_dev *d, unsigned int flags) * the kernel's view, which has regions and IRQs remapped and other fields * (most importantly classes) possibly fixed if the device is known broken. */ - if (flags & PCI_FILL_IDENT) + if (want_fill(d, flags, PCI_FILL_IDENT)) { d->vendor_id = sysfs_get_value(d, "vendor", 1); d->device_id = sysfs_get_value(d, "device", 1); - done |= PCI_FILL_IDENT; } - if (flags & PCI_FILL_CLASS) - { - d->device_class = sysfs_get_value(d, "class", 1) >> 8; - done |= PCI_FILL_CLASS; - } - if (flags & PCI_FILL_IRQ) - { + if (want_fill(d, flags, PCI_FILL_CLASS)) + d->device_class = sysfs_get_value(d, "class", 1) >> 8; + if (want_fill(d, flags, PCI_FILL_IRQ)) 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 (want_fill(d, flags, PCI_FILL_BASES | PCI_FILL_ROM_BASE | PCI_FILL_SIZES | PCI_FILL_IO_FLAGS)) sysfs_get_resources(d); - done |= PCI_FILL_BASES | PCI_FILL_ROM_BASE | PCI_FILL_SIZES | PCI_FILL_IO_FLAGS; - } } - if (flags & PCI_FILL_PHYS_SLOT) + if (want_fill(d, flags, PCI_FILL_PHYS_SLOT)) { struct pci_dev *pd; sysfs_fill_slots(d->access); for (pd = d->access->devices; pd; pd = pd->next) pd->known_fields |= PCI_FILL_PHYS_SLOT; - done |= PCI_FILL_PHYS_SLOT; } - if (flags & PCI_FILL_MODULE_ALIAS) + if (want_fill(d, flags, PCI_FILL_MODULE_ALIAS)) { char buf[OBJBUFSIZE]; if (sysfs_get_string(d, "modalias", buf, 0)) d->module_alias = pci_set_property(d, PCI_FILL_MODULE_ALIAS, buf); - done |= PCI_FILL_MODULE_ALIAS; } - if (flags & PCI_FILL_LABEL) + if (want_fill(d, flags, PCI_FILL_LABEL)) { char buf[OBJBUFSIZE]; if (sysfs_get_string(d, "label", buf, 0)) d->label = pci_set_property(d, PCI_FILL_LABEL, buf); - done |= PCI_FILL_LABEL; } - if (flags & PCI_FILL_NUMA_NODE) - { - d->numa_node = sysfs_get_value(d, "numa_node", 0); - done |= PCI_FILL_NUMA_NODE; - } + if (want_fill(d, flags, PCI_FILL_NUMA_NODE)) + d->numa_node = sysfs_get_value(d, "numa_node", 0); - if (flags & PCI_FILL_IOMMU_GROUP) + if (want_fill(d, flags, PCI_FILL_IOMMU_GROUP)) { char *group_link = sysfs_deref_link(d, "iommu_group"); if (group_link) @@ -362,10 +344,9 @@ sysfs_fill_info(struct pci_dev *d, unsigned int flags) pci_set_property(d, PCI_FILL_IOMMU_GROUP, basename(group_link)); free(group_link); } - done |= PCI_FILL_IOMMU_GROUP; } - if (flags & PCI_FILL_DT_NODE) + if (want_fill(d, flags, PCI_FILL_DT_NODE)) { char *node = sysfs_deref_link(d, "of_node"); if (node) @@ -373,10 +354,9 @@ sysfs_fill_info(struct pci_dev *d, unsigned int flags) pci_set_property(d, PCI_FILL_DT_NODE, node); free(node); } - done |= PCI_FILL_DT_NODE; } - return done | pci_generic_fill_info(d, flags & ~done); + pci_generic_fill_info(d, flags); } /* Intent of the sysfs_setup() caller */ ^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH pciutils 4/5] libpci: sysfs: Implement PROGIF, REVID and SUBSYS support 2022-01-21 13:57 [PATCH pciutils 0/5] Support for PROGIF, REVID and SUBSYS Pali Rohár ` (2 preceding siblings ...) 2022-01-21 13:57 ` [PATCH pciutils 3/5] libpci: generic: Implement SUBSYS also for PCI_HEADER_TYPE_BRIDGE Pali Rohár @ 2022-01-21 13:57 ` Pali Rohár 2022-01-21 13:57 ` [PATCH pciutils 5/5] lspci: Retrieve prog if, subsystem ids and revision id via libpci Pali Rohár 2022-01-21 14:40 ` [PATCH pciutils 0/5] Support for PROGIF, REVID and SUBSYS Martin Mareš 5 siblings, 0 replies; 17+ messages in thread From: Pali Rohár @ 2022-01-21 13:57 UTC (permalink / raw) To: Martin Mares, Bjorn Helgaas, Krzysztof Wilczyński, Matthew Wilcox, linux-pci In sysfs there are optional nodes with this information. --- lib/sysfs.c | 39 ++++++++++++++++++++++++++++++++++++--- 1 file changed, 36 insertions(+), 3 deletions(-) diff --git a/lib/sysfs.c b/lib/sysfs.c index 7c157a2688ad..7714607f66a0 100644 --- a/lib/sysfs.c +++ b/lib/sysfs.c @@ -329,6 +329,7 @@ static unsigned int sysfs_fill_info(struct pci_dev *d, unsigned int flags) { unsigned int done = 0; + int value; if (!d->access->buscentric) { @@ -343,10 +344,42 @@ sysfs_fill_info(struct pci_dev *d, unsigned int flags) d->device_id = sysfs_get_value(d, "device", 1); done |= PCI_FILL_IDENT; } - if (flags & PCI_FILL_CLASS) + if (flags & (PCI_FILL_CLASS | PCI_FILL_PROGIF)) { - d->device_class = sysfs_get_value(d, "class", 1) >> 8; - done |= PCI_FILL_CLASS; + value = sysfs_get_value(d, "class", 1); + if (flags & PCI_FILL_CLASS) + { + d->device_class = value >> 8; + done |= PCI_FILL_CLASS; + } + if (flags & PCI_FILL_PROGIF) + { + d->prog_if = value & 0xff; + done |= PCI_FILL_PROGIF; + } + } + if (flags & PCI_FILL_REVID) + { + value = sysfs_get_value(d, "revision", 0); + if (value >= 0) + { + d->rev_id = value; + done |= PCI_FILL_REVID; + } + } + if (flags & PCI_FILL_SUBSYS) + { + value = sysfs_get_value(d, "subsystem_vendor", 0); + if (value >= 0) + { + d->subsys_vendor_id = value; + value = sysfs_get_value(d, "subsystem_device", 0); + if (value >= 0) + { + d->subsys_id = value; + done |= PCI_FILL_SUBSYS; + } + } } if (flags & PCI_FILL_IRQ) { -- 2.20.1 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH pciutils 5/5] lspci: Retrieve prog if, subsystem ids and revision id via libpci 2022-01-21 13:57 [PATCH pciutils 0/5] Support for PROGIF, REVID and SUBSYS Pali Rohár ` (3 preceding siblings ...) 2022-01-21 13:57 ` [PATCH pciutils 4/5] libpci: sysfs: Implement PROGIF, REVID and SUBSYS support Pali Rohár @ 2022-01-21 13:57 ` Pali Rohár 2022-01-21 14:40 ` [PATCH pciutils 0/5] Support for PROGIF, REVID and SUBSYS Martin Mareš 5 siblings, 0 replies; 17+ messages in thread From: Pali Rohár @ 2022-01-21 13:57 UTC (permalink / raw) To: Martin Mares, Bjorn Helgaas, Krzysztof Wilczyński, Matthew Wilcox, linux-pci Use pci_fill_info with PROGIF, REVID and SUBSYS to fill this information. lspci in some places reads class from what libpci provider fills in dev->device_class and in some other places it reads directly from config space. In dev->device_class is stored class possible different class as in config space (e.g. if kernel is fixing class because device has bogus information stored in config space). With this change is class always read from dev->device_class which reflects and respects lspci -b option (Bus-centric view). Same applies for subsystem ids and revision id (note that prog if is part of class). --- ls-kernel.c | 8 +++--- lspci.c | 70 ++++++++++++++++++----------------------------------- lspci.h | 2 -- 3 files changed, 27 insertions(+), 53 deletions(-) diff --git a/ls-kernel.c b/ls-kernel.c index ecacd0e65dce..2284b4625b12 100644 --- a/ls-kernel.c +++ b/ls-kernel.c @@ -174,16 +174,14 @@ static int match_pcimap(struct device *d, struct pcimap_entry *e) { struct pci_dev *dev = d->dev; - unsigned int class = get_conf_long(d, PCI_REVISION_ID) >> 8; - word subv, subd; + unsigned int class = (((unsigned int)dev->device_class << 8) | dev->prog_if); #define MATCH(x, y) ((y) > 0xffff || (x) == (y)) - get_subid(d, &subv, &subd); return MATCH(dev->vendor_id, e->vendor) && MATCH(dev->device_id, e->device) && - MATCH(subv, e->subvendor) && - MATCH(subd, e->subdevice) && + MATCH(dev->subsys_vendor_id, e->subvendor) && + MATCH(dev->subsys_id, e->subdevice) && (class & e->class_mask) == e->class; #undef MATCH } diff --git a/lspci.c b/lspci.c index d14d1b9185d6..c4e6c93bc67a 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_info(p, PCI_FILL_IDENT | PCI_FILL_CLASS | PCI_FILL_PROGIF | PCI_FILL_REVID | PCI_FILL_SUBSYS); return d; } @@ -285,25 +285,6 @@ show_slot_name(struct device *d) show_slot_path(d); } -void -get_subid(struct device *d, word *subvp, word *subdp) -{ - byte htype = get_conf_byte(d, PCI_HEADER_TYPE) & 0x7f; - - if (htype == PCI_HEADER_TYPE_NORMAL) - { - *subvp = get_conf_word(d, PCI_SUBSYSTEM_VENDOR_ID); - *subdp = get_conf_word(d, PCI_SUBSYSTEM_ID); - } - else if (htype == PCI_HEADER_TYPE_CARDBUS && d->config_cached >= 128) - { - *subvp = get_conf_word(d, PCI_CB_SUBSYSTEM_VENDOR_ID); - *subdp = get_conf_word(d, PCI_CB_SUBSYSTEM_ID); - } - else - *subvp = *subdp = 0xffff; -} - static void show_terse(struct device *d) { @@ -319,12 +300,12 @@ show_terse(struct device *d) pci_lookup_name(pacc, devbuf, sizeof(devbuf), PCI_LOOKUP_VENDOR | PCI_LOOKUP_DEVICE, p->vendor_id, p->device_id)); - if (c = get_conf_byte(d, PCI_REVISION_ID)) - printf(" (rev %02x)", c); + if ((p->known_fields & PCI_FILL_REVID) && p->rev_id) + printf(" (rev %02x)", p->rev_id); if (verbose) { char *x; - c = get_conf_byte(d, PCI_CLASS_PROG); + c = (p->known_fields & PCI_FILL_PROGIF) ? p->prog_if : 0; x = pci_lookup_name(pacc, devbuf, sizeof(devbuf), PCI_LOOKUP_PROGIF | PCI_LOOKUP_NO_NUMBERS, p->device_class, c); @@ -340,19 +321,18 @@ show_terse(struct device *d) if (verbose || opt_kernel) { - word subsys_v, subsys_d; char ssnamebuf[256]; pci_fill_info(p, PCI_FILL_LABEL); if (p->label) printf("\tDeviceName: %s", p->label); - get_subid(d, &subsys_v, &subsys_d); - if (subsys_v && subsys_v != 0xffff) + if ((p->known_fields & PCI_FILL_SUBSYS) && + p->subsys_vendor_id && p->subsys_vendor_id != 0xffff) printf("\tSubsystem: %s\n", pci_lookup_name(pacc, ssnamebuf, sizeof(ssnamebuf), PCI_LOOKUP_SUBSYSTEM | PCI_LOOKUP_VENDOR | PCI_LOOKUP_DEVICE, - p->vendor_id, p->device_id, subsys_v, subsys_d)); + p->vendor_id, p->device_id, p->subsys_vendor_id, p->subsys_id)); } } @@ -766,7 +746,7 @@ show_verbose(struct device *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_BRIDGE_BASES); + PCI_FILL_BRIDGE_BASES | PCI_FILL_PROGIF | PCI_FILL_REVID | PCI_FILL_SUBSYS); irq = p->irq; switch (htype) @@ -947,13 +927,9 @@ static void show_machine(struct device *d) { struct pci_dev *p = d->dev; - int c; - word sv_id, sd_id; char classbuf[128], vendbuf[128], devbuf[128], svbuf[128], sdbuf[128]; char *dt_node, *iommu_group; - get_subid(d, &sv_id, &sd_id); - if (verbose) { pci_fill_info(p, PCI_FILL_PHYS_SLOT | PCI_FILL_NUMA_NODE | PCI_FILL_DT_NODE | PCI_FILL_IOMMU_GROUP); @@ -966,19 +942,20 @@ show_machine(struct device *d) pci_lookup_name(pacc, vendbuf, sizeof(vendbuf), PCI_LOOKUP_VENDOR, p->vendor_id, p->device_id)); printf("Device:\t%s\n", pci_lookup_name(pacc, devbuf, sizeof(devbuf), PCI_LOOKUP_DEVICE, p->vendor_id, p->device_id)); - if (sv_id && sv_id != 0xffff) + if ((p->known_fields & PCI_FILL_SUBSYS) && + p->subsys_vendor_id && p->subsys_vendor_id != 0xffff) { printf("SVendor:\t%s\n", - pci_lookup_name(pacc, svbuf, sizeof(svbuf), PCI_LOOKUP_SUBSYSTEM | PCI_LOOKUP_VENDOR, sv_id)); + pci_lookup_name(pacc, svbuf, sizeof(svbuf), PCI_LOOKUP_SUBSYSTEM | PCI_LOOKUP_VENDOR, p->subsys_vendor_id)); printf("SDevice:\t%s\n", - pci_lookup_name(pacc, sdbuf, sizeof(sdbuf), PCI_LOOKUP_SUBSYSTEM | PCI_LOOKUP_DEVICE, p->vendor_id, p->device_id, sv_id, sd_id)); + pci_lookup_name(pacc, sdbuf, sizeof(sdbuf), PCI_LOOKUP_SUBSYSTEM | PCI_LOOKUP_DEVICE, p->vendor_id, p->device_id, p->subsys_vendor_id, p->subsys_id)); } if (p->phy_slot) printf("PhySlot:\t%s\n", p->phy_slot); - if (c = get_conf_byte(d, PCI_REVISION_ID)) - printf("Rev:\t%02x\n", c); - if (c = get_conf_byte(d, PCI_CLASS_PROG)) - printf("ProgIf:\t%02x\n", c); + if ((p->known_fields & PCI_FILL_REVID) && p->rev_id) + printf("Rev:\t%02x\n", p->rev_id); + if (p->known_fields & PCI_FILL_PROGIF) + printf("ProgIf:\t%02x\n", p->prog_if); if (opt_kernel) show_kernel_machine(d); if (p->numa_node != -1) @@ -994,14 +971,15 @@ show_machine(struct device *d) print_shell_escaped(pci_lookup_name(pacc, classbuf, sizeof(classbuf), PCI_LOOKUP_CLASS, p->device_class)); print_shell_escaped(pci_lookup_name(pacc, vendbuf, sizeof(vendbuf), PCI_LOOKUP_VENDOR, p->vendor_id, p->device_id)); print_shell_escaped(pci_lookup_name(pacc, devbuf, sizeof(devbuf), PCI_LOOKUP_DEVICE, p->vendor_id, p->device_id)); - if (c = get_conf_byte(d, PCI_REVISION_ID)) - printf(" -r%02x", c); - if (c = get_conf_byte(d, PCI_CLASS_PROG)) - printf(" -p%02x", c); - if (sv_id && sv_id != 0xffff) + if ((p->known_fields & PCI_FILL_REVID) && p->rev_id) + printf(" -r%02x", p->rev_id); + if (p->known_fields & PCI_FILL_PROGIF) + printf(" -p%02x", p->prog_if); + if ((p->known_fields & PCI_FILL_SUBSYS) && + p->subsys_vendor_id && p->subsys_vendor_id != 0xffff) { - print_shell_escaped(pci_lookup_name(pacc, svbuf, sizeof(svbuf), PCI_LOOKUP_SUBSYSTEM | PCI_LOOKUP_VENDOR, sv_id)); - print_shell_escaped(pci_lookup_name(pacc, sdbuf, sizeof(sdbuf), PCI_LOOKUP_SUBSYSTEM | PCI_LOOKUP_DEVICE, p->vendor_id, p->device_id, sv_id, sd_id)); + print_shell_escaped(pci_lookup_name(pacc, svbuf, sizeof(svbuf), PCI_LOOKUP_SUBSYSTEM | PCI_LOOKUP_VENDOR, p->subsys_vendor_id)); + print_shell_escaped(pci_lookup_name(pacc, sdbuf, sizeof(sdbuf), PCI_LOOKUP_SUBSYSTEM | PCI_LOOKUP_DEVICE, p->vendor_id, p->device_id, p->subsys_vendor_id, p->subsys_id)); } else printf(" \"\" \"\""); diff --git a/lspci.h b/lspci.h index 352177fcce7b..6e0bb2492fd5 100644 --- a/lspci.h +++ b/lspci.h @@ -55,8 +55,6 @@ u32 get_conf_long(struct device *d, unsigned int pos); word get_conf_word(struct device *d, unsigned int pos); byte get_conf_byte(struct device *d, unsigned int pos); -void get_subid(struct device *d, word *subvp, word *subdp); - /* Useful macros for decoding of bits and bit fields */ #define FLAG(x,y) ((x & y) ? '+' : '-') -- 2.20.1 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH pciutils 0/5] Support for PROGIF, REVID and SUBSYS 2022-01-21 13:57 [PATCH pciutils 0/5] Support for PROGIF, REVID and SUBSYS Pali Rohár ` (4 preceding siblings ...) 2022-01-21 13:57 ` [PATCH pciutils 5/5] lspci: Retrieve prog if, subsystem ids and revision id via libpci Pali Rohár @ 2022-01-21 14:40 ` Martin Mareš 2022-01-21 15:12 ` Pali Rohár 5 siblings, 1 reply; 17+ messages in thread From: Martin Mareš @ 2022-01-21 14:40 UTC (permalink / raw) To: Pali Rohár Cc: Bjorn Helgaas, Krzysztof Wilczyński, Matthew Wilcox, linux-pci 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 ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH pciutils 0/5] Support for PROGIF, REVID and SUBSYS 2022-01-21 14:40 ` [PATCH pciutils 0/5] Support for PROGIF, REVID and SUBSYS Martin Mareš @ 2022-01-21 15:12 ` Pali Rohár 2022-01-21 15:15 ` Martin Mareš 0 siblings, 1 reply; 17+ messages in thread From: Pali Rohár @ 2022-01-21 15:12 UTC (permalink / raw) To: Martin Mareš Cc: Bjorn Helgaas, Krzysztof Wilczyński, Matthew Wilcox, linux-pci 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. ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH pciutils 0/5] Support for PROGIF, REVID and SUBSYS 2022-01-21 15:12 ` Pali Rohár @ 2022-01-21 15:15 ` Martin Mareš 2022-01-21 15:26 ` Pali Rohár 0 siblings, 1 reply; 17+ messages in thread From: Martin Mareš @ 2022-01-21 15:15 UTC (permalink / raw) To: Pali Rohár Cc: Bjorn Helgaas, Krzysztof Wilczyński, Matthew Wilcox, linux-pci 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 -- Martin `MJ' Mareš <mj@ucw.cz> http://mj.ucw.cz/ United Computer Wizards, Prague, Czech Republic, Europe, Earth, Universe "I don't give a damn for a man that can only spell a word one way." -- M. Twain ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH pciutils 0/5] Support for PROGIF, REVID and SUBSYS 2022-01-21 15:15 ` Martin Mareš @ 2022-01-21 15:26 ` Pali Rohár 2022-01-21 15:29 ` Martin Mareš 0 siblings, 1 reply; 17+ messages in thread From: Pali Rohár @ 2022-01-21 15:26 UTC (permalink / raw) To: Martin Mareš Cc: Bjorn Helgaas, Krzysztof Wilczyński, Matthew Wilcox, linux-pci 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? ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH pciutils 0/5] Support for PROGIF, REVID and SUBSYS 2022-01-21 15:26 ` Pali Rohár @ 2022-01-21 15:29 ` Martin Mareš 2022-01-21 15:35 ` Pali Rohár 0 siblings, 1 reply; 17+ messages in thread From: Martin Mareš @ 2022-01-21 15:29 UTC (permalink / raw) To: Pali Rohár Cc: Bjorn Helgaas, Krzysztof Wilczyński, Matthew Wilcox, linux-pci > 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 -- Martin `MJ' Mareš <mj@ucw.cz> http://mj.ucw.cz/ United Computer Wizards, Prague, Czech Republic, Europe, Earth, Universe Not all rumors are as misleading as this one. ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH pciutils 0/5] Support for PROGIF, REVID and SUBSYS 2022-01-21 15:29 ` Martin Mareš @ 2022-01-21 15:35 ` Pali Rohár 2022-01-21 15:38 ` Martin Mareš 0 siblings, 1 reply; 17+ messages in thread From: Pali Rohár @ 2022-01-21 15:35 UTC (permalink / raw) To: Martin Mareš Cc: Bjorn Helgaas, Krzysztof Wilczyński, Matthew Wilcox, linux-pci 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)? ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH pciutils 0/5] Support for PROGIF, REVID and SUBSYS 2022-01-21 15:35 ` Pali Rohár @ 2022-01-21 15:38 ` Martin Mareš 2022-01-21 15:45 ` Pali Rohár 0 siblings, 1 reply; 17+ messages in thread From: Martin Mareš @ 2022-01-21 15:38 UTC (permalink / raw) To: Pali Rohár Cc: Bjorn Helgaas, Krzysztof Wilczyński, Matthew Wilcox, linux-pci > 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 -- Martin `MJ' Mareš <mj@ucw.cz> http://mj.ucw.cz/ United Computer Wizards, Prague, Czech Republic, Europe, Earth, Universe Nothing is smiple enough to be not screwed up. ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH pciutils 0/5] Support for PROGIF, REVID and SUBSYS 2022-01-21 15:38 ` Martin Mareš @ 2022-01-21 15:45 ` Pali Rohár 0 siblings, 0 replies; 17+ messages in thread From: Pali Rohár @ 2022-01-21 15:45 UTC (permalink / raw) To: Martin Mareš Cc: Bjorn Helgaas, Krzysztof Wilczyński, Matthew Wilcox, linux-pci 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. ^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2022-01-21 20:43 UTC | newest] Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-01-21 13:57 [PATCH pciutils 0/5] Support for PROGIF, REVID and SUBSYS Pali Rohár 2022-01-21 13:57 ` [PATCH pciutils 1/5] libpci: Add new options for pci_fill_info: " Pali Rohár 2022-01-21 13:57 ` [PATCH pciutils 2/5] libpci: generic: Implement PROGIF, REVID and SUBSYS support Pali Rohár 2022-01-21 13:57 ` [PATCH pciutils 3/5] libpci: generic: Implement SUBSYS also for PCI_HEADER_TYPE_BRIDGE Pali Rohár 2022-01-21 14:40 ` Martin Mareš 2022-01-21 16:11 ` Pali Rohár 2022-01-21 20:43 ` Martin Mareš 2022-01-21 13:57 ` [PATCH pciutils 4/5] libpci: sysfs: Implement PROGIF, REVID and SUBSYS support Pali Rohár 2022-01-21 13:57 ` [PATCH pciutils 5/5] lspci: Retrieve prog if, subsystem ids and revision id via libpci Pali Rohár 2022-01-21 14:40 ` [PATCH pciutils 0/5] Support for PROGIF, REVID and SUBSYS Martin Mareš 2022-01-21 15:12 ` Pali Rohár 2022-01-21 15:15 ` Martin Mareš 2022-01-21 15:26 ` Pali Rohár 2022-01-21 15:29 ` Martin Mareš 2022-01-21 15:35 ` Pali Rohár 2022-01-21 15:38 ` Martin Mareš 2022-01-21 15:45 ` Pali Rohár
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.