[libvirt PATCH v7 0/5] Add a PCI/PCIe device VPD Capability

[Daniel P. Berrangé]

On Wed, Oct 20, 2021 at 11:30:30AM +0300, Dmitrii Shcherbakov wrote:
> Add support for deserializing the binary PCI/PCIe VPD format and
> exposing VPD resources as XML elements in a new nested capability
> of PCI/PCIe devices called 'vpd'.
> 
> The series contains the following incremental changes:
> 
> * The PCI VPD parser module, in-memory resource representation
>   and tests;
> * VPD-related helpers added to the virpci module;
> * VPD capability support: XML serialization/deserialization from/into
>   VPD resource data structures;
> * Documentation.
> 
> The VPD format is specified in "I.3. VPD Definitions" in PCI specs
> (2.2+) and "6.28.1 VPD Format" PCIe 4.0. As section 6.28 in PCIe 4.0
> notes, the PCI Local Bus and PCIe VPD formats are binary compatible
> and PCIe 4.0 merely started incorporating what was already present in
> PCI specs.
> 
> Linux kernel exposes a binary blob in the VPD format via sysfs since
> v2.6.26 (commit 94e6108803469a37ee1e3c92dafdd1d59298602f) which requires
> a parser to interpret.
> 
> There are usage scenarios where information such as the board serial
> number needs to be retrieved from PCI(e) VPD. Projects like Nova can
> utilize this information for cases which involve virtual interface
> plugging on SmartNIC DPUs but there may be other scenarios and types of
> information useful to retrieve from VPD. The fact that the format is
> binary requires proper parsing instead of substring searching hence the
> full parser is proposed. Likewise, checksum validation requires proper
> parsing as well.
> 
> A usage example is present here:
> https://review.opendev.org/c/openstack/nova/+/808199
> 
> The patch follows a prior discussion on the mailing list which has
> additional context about the use-case but a narrower proposal:
> 
> https://listman.redhat.com/archives/libvir-list/2021-May/msg00873.html
> https://www.mail-archive.com/libvir-list@redhat.com/msg218165.html
> 
> The new functionality is mostly contained in virpcivpd with a
> couple of new functions added to virpci. Additionally, the necessary XML
> serialization/deserialization and glue code is added to expose the VPD
> capability to external clients as XML.
> 
> A new capability flag is added along with a new capability in order to
> allow for filtering of PCI devices with the VPD capability using virsh:
> 
> virsh nodedev-list --cap vpd
> sudo virsh nodedev-dumpxml --device pci_dddd_bb_ss_f
> 
> In this example having the root uid is required in order to access the
> vpd sysfs entry, therefore, the nodedev XML output will only contain
> the VPD capability if virsh is run as root.
> 
> The capability is treated as dynamic due to the presence of read-write
> sections in the VPD format per PCI/PCIe specs (the idea being that
> read-write resource fields may potentially be altered by the DPU OS
> over time independently from the host OS).
> 
> Unit tests cover the parser functionality (including many possible
> invalid cases), in-memory representation as well as XML serialization
> and deserialization.
> 
> Manual functional testing was performed with 2 DPUs and several other
> NIC models which expose PCI(e) VPD. Testing have also been performed
> for devices that do not have VPD or those that expose a VPD capability
> but exhibit invalid behavior (I/O errors while reading a sysfs entry).
> 
> v7 changes:
> 
> * Fixed a number of memleaks in virpcivpd.c, virpcivpdtest.c,
>   node_device_conf.c (see the test results in a paste below);
> * Moved some preprocessor definitions and virPCIVPDResourceFieldValueFormat
>   to virpcivpdpriv.h (not the .c file since those are used in unit tests);
> * virPCIVPDResourceUpdateKeyword now prints a warning and returns true for
>   unexpected keywords, whereas virPCIVPDParseVPDLargeResourceFields fails
>   on errors returned from virPCIVPDResourceUpdateKeyword;
> * Updates to static fields now free the memory allocated to old values.


Since the issues i pointed out are minimal, I've just made them
myself and will push this once I see gitlab CI passing for it.


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 :|