On Tue, Oct 26, 2021 at 08:28:20PM +0300, Dmitrii Shcherbakov wrote: > Hi Daniel, > > > Something is still not right with the logic here as this error report > is triggering on my machines. If I comment out this check, then I can > get data reported by libvirt > > The VPD example you shared previously has multiple violations of the VPD format: > > * Invalid field values ("N/A" in the read-only section, 0xFF in the > read-write section); > * The lack of the RW field at the end of the read-write section. > > Previously, the VPD was discarded due to invalid values. > > I thought honoring the RW requirement would still make sense and kept > the RW presence check. > > I added it as a test case here (converted to hex): > https://listman.redhat.com/archives/libvir-list/2021-October/msg00871.html > > The spec says the following (PCIe 4.0 "6.28.2.3 Read/Write Fields"): > > "RW | Remaining Read/Write Area | > This descriptor is used to identify the unused portion of the > read/write space. The product > vendor initializes this parameter based on the size of the read/write > space or the space remaining > following the Vx VPD items. One or more of the Vx, Yx, and RW items > are required." > > I can also confirm that by looking at the hexdump and looking for hex > values for R (0x52) and W (0x57): > > hexdump -e '16/1 "0x%02x, " "\n"' vpd > > I could ease it up and allow it to be missing as we do not care much > about it when reading so long as the resource > length is correct. However, if we were to allow Libvirt to write to > the read-write section in the future we would need to > know how much free space is there in the first place. The issue I have is that 'lspci -vvv' will happily report the VPD data on my machine: Capabilities: [e0] Vital Product Data Product Name: HP Ethernet 1Gb 2-port 361i Adapter Read-only fields: [PN] Part number: N/A [EC] Engineering changes: N/A [SN] Serial number: N/A [V0] Vendor specific: 4W/1W PCIeG2x4 2p 1GbE RJ45 Intel i350 [RV] Reserved: checksum good, 0 byte(s) reserved Read/write fields: [V1] Vendor specific: 5.7.06 [V3] Vendor specific: 2.8.20 [V6] Vendor specific: 1.5.35 [YA] Asset tag: N/A [YB] System specific: <FF><FF><FF><FF><FF><FF><FF><FF><FF><FF><FF><FF><FF><FF><FF><FF> [YC] System specific: <FF><FF><FF><FF><FF><FF><FF><FF><FF><FF><FF><FF><FF><FF><FF><FF> End while libvirt refuses to report it. Even if the BIOS is not perfectly following the spec, it is clearly still possible to extract the data and display it to the user. So I don't think it is reasonable for libvirt to refuse todo this. I'd have sympathy if this machine was pre-production hardware or from a no-name vendor, but this machine is typical production quality hardware from a major vendor. The only think there I think it is reasonable for libvirt to reject the data is in the cae of the two fields that contain non-printable characters <FF>. Other than that I expect libvirt to report exactly the same data as lspci will report, and not try to secondguess whether the values provided by the vendor are semantically valid or not. Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|