On Tue, 2015-09-15 at 20:47 +0000, Rustad, Mark D wrote: > > On Sep 15, 2015, at 12:04 PM, Alex Williamson <alex.williamson@xxxxxxxxxx> wrote: > > > > > > FRU-type information is only one of the use cases of VPD, the spec also > > defines (PCI rev 3.0, 6.4): > > > > ... a mechanism for storing information such as performance and > > failure data on the device being monitored. > > > > That information could very much be function specific. > > It is open to interpretation. I guess still I see it as the physical device as a whole. > > > When I was looking at whether we should provide VPD access of an > > assigned device at all, I ran across this interesting statement in the > > PCI spec (rev 3.0, I.3.1.1): > > > > CP Extended Capability > > > > This field allows a new capability to be identified in the VPD > > area. Since dynamic control/status cannot be placed in VPD, the > > data for this field identifies where, in the device’s memory or > > I/O address space, the control/status registers for the > > capability can be found. Location of the control/status > > registers is identified by providing the index (a value between > > 0 and 5) of the Base Address register that defines the address > > range that contains the registers, and the offset within that > > Base Address register range where the control/status registers > > reside. The data area for this field is four bytes long. The > > first byte contains the ID of the extended capability. The > > second byte contains the index (zero based) of the Base Address > > register used. The next two bytes contain the offset (in > > little-endian order) within that address range where the > > control/status registers defined for that capability reside. > > > > Again, this sounds like function specific data, and both here and above, > > blocking access to VPD could affect the functionality of drivers. It > > may be the case that Intel would find this use to be madness, but > > there's no PCI spec requirement that separate functions are in any way > > similar and we're looking at an interface that may be used by non-Intel > > devices as well. Thanks, > > It isn't an interface as such, it is a quirk to address some > widespread design problems with multi function devices with VPD. And > you are right that functions can be different. In fact this quirk is > needed only because now they often (usually in fact) are not > different! I do hope to see some non-Intel devices use the quirk, > because I'm pretty sure there are other devices that have the same > issue. > > I realize that I covered a pretty wide swath by making the quirk apply > to all Intel Ethernet devices, but that still seems correct. The > Skylake is not an issue because it does not have VPD so the > pci_find_capability will fail before any handling of the quirk is > possible. The code that applies the quirk could check specific > devices, but it would make the code a lot bigger, and I see this kind > of code as dead weight for so many systems that I tried to make it as > small as possible. Since all Intel Ethernet seems to be correct now > and as far as I can see into the future, that is what I did. > > Going back to something you mentioned before, I think you are right > that the failure case for the pci_vpd_f0_dev_check could be made to > simply clear the quirk and continue, since pci_vpd_f0_dev_check really > should not fail in cases where the quirk is applicable. That does seem > like a reasonable change to me the more I think about it. > > I think a whitelist would be unnecessary dead weight. Yep, a whitelist is probably not the way to go. AFAICT, you're looking for plugin-cards where all the functions meet the criteria of having the same class, vendor, and device ID. If we don't meet that criteria, then it's not a device we're expecting and we should leave it alone. Also, rather than clearing the flag, can we move the tests done by pci_vpd_f0_dev_check() into the quirk setup function? It seems like function 0 should be sufficiently configured by the time we're probing non-zero functions that we can be more selective in setting the flag rather than unsetting it later. Thanks, Alex -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
