On Mon, Sep 27, 2021 at 10:30:47PM +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. > > A GTree is used as a data structure in order to maintain key ordering > which will be important in XML to XML tests later. Now, I've learnt a bit more about VPD and considering my comments on the XML format in the last patch, I think this use of GTree is unduly opaque and overkill I think we should be representing the data we're extracting as plan struct fields, in the same way that we handle SMBIOS in the virsysinfo.h file.... > diff --git a/src/util/virpcivpd.h b/src/util/virpcivpd.h > new file mode 100644 > index 0000000000..7327806df3 > --- /dev/null > +++ b/src/util/virpcivpd.h > + > +#include "internal.h" > + > +/* > + * PCI Local bus (2.2+, Appendix I) and PCIe 4.0+ (7.9.19 VPD Capability) define > + * the VPD capability structure (8 bytes in total) and VPD registers that can be used to access > + * VPD data including: > + * bit 31 of the first 32-bit DWORD: data transfer completion flag (between the VPD data register > + * and the VPD data storage hardware); > + * bits 30:16 of the first 32-bit DWORD: VPD address of the first VPD data byte to be accessed; > + * bits 31:0 of the second 32-bit DWORD: VPD data bytes with LSB being pointed to by the VPD address. > + * > + * Given that only 15 bits (30:16) are allocated for VPD address its mask is 0x7fff. > +*/ > +#define PCI_VPD_ADDR_MASK 0x7FFF > + > +/* > + * VPD data consists of small and large resource data types. Information within a resource type > + * consists of a 2-byte keyword, 1-byte length and data bytes (up to 255). > +*/ > +#define PCI_VPD_MAX_FIELD_SIZE 255 > +#define PCI_VPD_LARGE_RESOURCE_FLAG 0x80 > +#define PCI_VPD_STRING_RESOURCE_FLAG 0x02 > +#define PCI_VPD_READ_ONLY_LARGE_RESOURCE_FLAG 0x10 > +#define PCI_VPD_READ_WRITE_LARGE_RESOURCE_FLAG 0x11 > +#define PCI_VPD_RESOURCE_END_TAG 0x0F > +#define PCI_VPD_RESOURCE_END_VAL PCI_VPD_RESOURCE_END_TAG << 3 > +#define VIR_TYPE_PCI_VPD_RESOURCE_TYPE vir_pci_vpd_resource_type_get_type() > + > +G_BEGIN_DECLS; > + > +typedef enum { > + VIR_PCI_VPD_RESOURCE_FIELD_VALUE_FORMAT_TEXT = 1, > + VIR_PCI_VPD_RESOURCE_FIELD_VALUE_FORMAT_BINARY, > + VIR_PCI_VPD_RESOURCE_FIELD_VALUE_FORMAT_RESVD, > + VIR_PCI_VPD_RESOURCE_FIELD_VALUE_FORMAT_RDWR, > + VIR_PCI_VPD_RESOURCE_FIELD_VALUE_FORMAT_LAST > +} virPCIVPDResourceFieldValueFormat; > + > +typedef enum { > + VIR_PCI_VPD_RESOURCE_TYPE_STRING = 1, > + VIR_PCI_VPD_RESOURCE_TYPE_VPD_R, > + VIR_PCI_VPD_RESOURCE_TYPE_VPD_W, > + VIR_PCI_VPD_RESOURCE_TYPE_LAST > +} virPCIVPDResourceType; > + > +GType vir_pci_vpd_resource_type_get_type(void); > + > +virPCIVPDResourceFieldValueFormat virPCIVPDResourceGetFieldValueFormat(const gchar *value); > + > +#define VIR_TYPE_PCI_VPD_RESOURCE vir_pci_vpd_resource_get_type() > +G_DECLARE_DERIVABLE_TYPE(virPCIVPDResource, vir_pci_vpd_resource, VIR, PCI_VPD_RESOURCE, GObject); > +struct _virPCIVPDResourceClass { > + GObjectClass parent_class; > +}; > + > +/* Glib 2.59 adds proper support for g_autolist for derivable types > + * see 86c073dba9d82ef3f1bc3d3116b058b9b5c3b1eb. At the time of writing > + * 2.56 is the minimum version used by Libvirt. Without the below code > + * compilation errors will occur. > + */ > +#if !GLIB_CHECK_VERSION(2, 59, 0) > +typedef GList *virPCIVPDResource_listautoptr; > +static inline void > +glib_listautoptr_cleanup_virPCIVPDResource(GList **_l) > +{ > + g_list_free_full(*_l, (GDestroyNotify)g_object_unref); > +} > +#endif > + > +GEnumValue *virPCIVPDResourceGetResourceType(virPCIVPDResource *res); > + > +#define VIR_TYPE_PCI_VPD_STRING_RESOURCE vir_pci_vpd_string_resource_get_type() > +G_DECLARE_FINAL_TYPE(virPCIVPDStringResource, vir_pci_vpd_string_resource, VIR, > + PCI_VPD_STRING_RESOURCE, virPCIVPDResource); > + > +struct _virPCIVPDStringResourceClass { > + virPCIVPDResourceClass parent_class; > +}; > + > +virPCIVPDStringResource *virPCIVPDStringResourceNew(gchar *value); > + > +const gchar *virPCIVPDStringResourceGetValue(const virPCIVPDStringResource *res); > + > +#define VIR_TYPE_PCI_VPD_KEYWORD_RESOURCE vir_pci_vpd_keyword_resource_get_type() > +G_DECLARE_FINAL_TYPE(virPCIVPDKeywordResource, vir_pci_vpd_keyword_resource, VIR, > + PCI_VPD_KEYWORD_RESOURCE, virPCIVPDResource); > +virPCIVPDKeywordResource *virPCIVPDKeywordResourceNew(GTree *resourceFields, bool readOnly); > + > +void virPCIVPDKeywordResourceForEach(virPCIVPDKeywordResource *res, GTraverseFunc func, > + gpointer userData); > + > + > +GList *virPCIVPDParse(int vpdFileFd); ....I feel like all of this can be reduced down to just a couple of public structs and a single method: typedef struct virPCIVPDResourceCustom virPCIVPDResourceCustom; struct virPCIVPDResourceCustom { char idx; char *value; }; typedef struct virPCIVPDResourceRO virPCIVPDResourceRO; struct virPCIVPDResourceRO { char *part_numer; char *change_level; char *fabric_geography; char *location; char *manufcatur_id; char *pci_geography; char *serial_number; size_t nvendor_specific; virPCIVPDResourceCustom *vendor_specific; }; typedef struct virPCIVPDResourceRW virPCIVPDResourceRW; struct virPCIVPDResourceRW { char *asset_tag; size_t nvendor_specific; virPCIVPDResourceCustom *vendor_specific; size_t nsystem_specific; virPCIVPDResourceCustom *system_specific; }; typedef struct virPCIVPDResource virPCIVPDResource; struct virPCIVPDResource { char *name; virPCIVPDResourceRO *ro; virPCIVPDResourceRW *rw; } virPCIVPDResource *virPCIVPDParse(int vpdFileFd); void virPCIVPDResourceFree(virPCIVPDResource *res); G_DEFINE_AUTOPTR_CLEANUP_FUNC(virPCIVPDResource, virPCIVPDResourceFree); 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 :|