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@xxxxxxxxxxxxx>
> ---
> 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@src/util/virnvme.c
> @SRCDIR@src/util/virobject.c
> @SRCDIR@src/util/virpci.c
> +@SRCDIR@src/util/virpcivpd.c
> @SRCDIR@src/util/virperf.c
> @SRCDIR@src/util/virpidfile.c
> @SRCDIR@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 :|
