Re: [libvirt PATCH 1/4] PCI VPD: handle additional edge cases
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 :|