[libvirt PATCH v7 1/5] Add a PCI/PCIe device VPD Parser

Daniel P. Berrangé berrange at redhat.com
Thu Oct 21 15:46:27 UTC 2021


On Wed, Oct 20, 2021 at 11:30:31AM +0300, Dmitrii Shcherbakov wrote:
> Add support for deserializing the binary PCI/PCIe VPD format and storing
> results in memory.
> 
> 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.
> 
> Signed-off-by: Dmitrii Shcherbakov <dmitrii.shcherbakov at canonical.com>
> ---
>  build-aux/syntax-check.mk |   4 +-
>  po/POTFILES.in            |   1 +
>  src/libvirt_private.syms  |  18 +
>  src/util/meson.build      |   1 +
>  src/util/virpcivpd.c      | 754 +++++++++++++++++++++++++++++++++++
>  src/util/virpcivpd.h      |  76 ++++
>  src/util/virpcivpdpriv.h  |  83 ++++
>  tests/meson.build         |   1 +
>  tests/testutils.c         |  35 ++
>  tests/testutils.h         |   4 +
>  tests/virpcivpdtest.c     | 809 ++++++++++++++++++++++++++++++++++++++
>  11 files changed, 1784 insertions(+), 2 deletions(-)
>  create mode 100644 src/util/virpcivpd.c
>  create mode 100644 src/util/virpcivpd.h
>  create mode 100644 src/util/virpcivpdpriv.h
>  create mode 100644 tests/virpcivpdtest.c
> 
> diff --git a/build-aux/syntax-check.mk b/build-aux/syntax-check.mk
> index cb12b64532..2a6e2f86a1 100644
> --- a/build-aux/syntax-check.mk
> +++ b/build-aux/syntax-check.mk
> @@ -775,9 +775,9 @@ sc_prohibit_windows_special_chars_in_filename:
>  	{ echo '$(ME): Windows special chars in filename not allowed' 1>&2; echo exit 1; } || :
>  
>  sc_prohibit_mixed_case_abbreviations:
> -	@prohibit='Pci|Usb|Scsi' \
> +	@prohibit='Pci|Usb|Scsi|Vpd' \
>  	in_vc_files='\.[ch]$$' \
> -	halt='Use PCI, USB, SCSI, not Pci, Usb, Scsi' \
> +	halt='Use PCI, USB, SCSI, VPD, not Pci, Usb, Scsi, Vpd' \
>  	  $(_sc_search_regexp)
>  
>  # Require #include <locale.h> in all files that call setlocale()
> diff --git a/po/POTFILES.in b/po/POTFILES.in
> index b554cf08ca..8a726f624e 100644
> --- a/po/POTFILES.in
> +++ b/po/POTFILES.in
> @@ -302,6 +302,7 @@
>  @SRCDIR at src/util/virnvme.c
>  @SRCDIR at src/util/virobject.c
>  @SRCDIR at src/util/virpci.c
> + at SRCDIR@src/util/virpcivpd.c
>  @SRCDIR at src/util/virperf.c
>  @SRCDIR at src/util/virpidfile.c
>  @SRCDIR at src/util/virpolkit.c
> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> index c5d788285e..444b51c880 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -3576,6 +3576,24 @@ virVHBAManageVport;
>  virVHBAPathExists;
>  
>  
> +# util/virpcivpd.h
> +
> +virPCIVPDParse;
> +virPCIVPDParseVPDLargeResourceFields;
> +virPCIVPDParseVPDLargeResourceString;
> +virPCIVPDReadVPDBytes;
> +virPCIVPDResourceCustomCompareIndex;
> +virPCIVPDResourceCustomFree;
> +virPCIVPDResourceCustomUpsertValue;
> +virPCIVPDResourceFree;
> +virPCIVPDResourceGetFieldValueFormat;
> +virPCIVPDResourceIsValidTextValue;
> +virPCIVPDResourceROFree;
> +virPCIVPDResourceRONew;
> +virPCIVPDResourceRWFree;
> +virPCIVPDResourceRWNew;
> +virPCIVPDResourceUpdateKeyword;
> +
>  # util/virvsock.h
>  virVsockAcquireGuestCid;
>  virVsockSetGuestCid;
> diff --git a/src/util/meson.build b/src/util/meson.build
> index 05934f6841..24350a3e67 100644
> --- a/src/util/meson.build
> +++ b/src/util/meson.build
> @@ -105,6 +105,7 @@ util_sources = [
>    'virutil.c',
>    'viruuid.c',
>    'virvhba.c',
> +  'virpcivpd.c',
>    'virvsock.c',
>    'virxml.c',
>  ]
> diff --git a/src/util/virpcivpd.c b/src/util/virpcivpd.c
> new file mode 100644
> index 0000000000..a208224228
> --- /dev/null
> +++ b/src/util/virpcivpd.c

> +
> +bool
> +virPCIVPDResourceCustomCompareIndex(virPCIVPDResourceCustom *a, virPCIVPDResourceCustom *b)
> +{
> +    if (a == b) {
> +        return true;
> +    } else if (a == NULL || b == NULL) {
> +        return false;
> +    } else {
> +        return a->idx == b->idx;
> +    }
> +    return true;
> +}

Subtle issue here - this function is used as a callback with GLib
and thus it needs to still use gboolean / TRUE / FALSE, because thats
a different sized data type to bool. This caused a horribly
non-deterministic problem on 32-bit builds.



> +
> +#endif /* __linux__ */

We needed an '#else' block here with stubs for non-Linux otherwise
we got link failures on Windows builds due to symbols not being
exported.


> +static int
> +mymain(void)
> +{
> +    int ret = 0;
> +
> +    if (virTestRun("Basic functionality of virPCIVPDResource ", testPCIVPDResourceBasic, NULL) < 0)
> +        ret = -1;
> +    if (virTestRun("Custom field index comparison",
> +                   testPCIVPDResourceCustomCompareIndex, NULL) < 0)
> +        ret = -1;
> +    if (virTestRun("Custom field value insertion and updates ",
> +                   testPCIVPDResourceCustomUpsertValue, NULL) < 0)
> +        ret = -1;
> +    if (virTestRun("Valid text values ", testPCIVPDIsValidTextValue, NULL) < 0)
> +        ret = -1;
> +    if (virTestRun("Determining a field value format by a key ",
> +                   testPCIVPDGetFieldValueFormat, NULL) < 0)
> +        ret = -1;
> +    if (virTestRun("Reading VPD bytes ", testVirPCIVPDReadVPDBytes, NULL) < 0)
> +        ret = -1;
> +    if (virTestRun("Parsing VPD string resources ", testVirPCIVPDParseVPDStringResource, NULL) < 0)
> +        ret = -1;
> +    if (virTestRun("Parsing a VPD resource with an invalid keyword ",
> +                   testVirPCIVPDParseFullVPDSkipInvalidKeywords, NULL) < 0)
> +        ret = -1;
> +    if (virTestRun("Parsing VPD resources from a full VPD ", testVirPCIVPDParseFullVPD, NULL) < 0)
> +        ret = -1;
> +    if (virTestRun("Parsing invalid VPD records ", testVirPCIVPDParseFullVPDInvalid, NULL) < 0)
> +        ret = -1;
> +
> +    return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE;
> +}
> +
> +VIR_TEST_MAIN(mymain)
> +#endif

Also needed a dummy 'mymain' in an #else block here for non-linux
too.


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




More information about the libvir-list mailing list