* XML serialization and deserialization of PCI VPD resources;
* PCI VPD capability flags added and used in relevant places;
* XML to XML tests for the added capability.
Signed-off-by: Dmitrii Shcherbakov <dmitrii.shcherbakov@canonical.com>
---
docs/schemas/nodedev.rng | 40 +++
include/libvirt/libvirt-nodedev.h | 1 +
src/conf/node_device_conf.c | 271 ++++++++++++++++++
src/conf/node_device_conf.h | 6 +-
src/conf/virnodedeviceobj.c | 7 +-
src/node_device/node_device_driver.c | 2 +
src/node_device/node_device_udev.c | 2 +
.../pci_0000_42_00_0_vpd.xml | 33 +++
.../pci_0000_42_00_0_vpd.xml | 1 +
tests/nodedevxml2xmltest.c | 1 +
tools/virsh-nodedev.c | 3 +
11 files changed, 365 insertions(+), 2 deletions(-)
create mode 100644 tests/nodedevschemadata/pci_0000_42_00_0_vpd.xml
create mode 120000 tests/nodedevxml2xmlout/pci_0000_42_00_0_vpd.xml
diff --git a/docs/schemas/nodedev.rng b/docs/schemas/nodedev.rng
index e089e66858..fbbee4d0c6 100644
--- a/docs/schemas/nodedev.rng
+++ b/docs/schemas/nodedev.rng
@@ -223,6 +223,10 @@
<ref name="mdev_types"/>
</optional>
+ <optional>
+ <ref name="vpd"/>
+ </optional>
+
<optional>
<element name="iommuGroup">
<attribute name="number">
@@ -770,6 +774,42 @@
</element>
</define>
+ <define name="vpd">
+ <element name="capability">
+ <attribute name="type">
+ <value>vpd</value>
+ </attribute>
+ <oneOrMore>
+ <choice>
+ <element name="resource">
+ <attribute name="type">
+ <choice>
+ <value>string</value>
+ </choice>
+ </attribute>
+ <text/>
+ </element>
+ <element name="resource">
+ <attribute name="type">
+ <choice>
+ <value>vpd-r</value>
+ <value>vpd-w</value>
+ </choice>
+ </attribute>
+ <oneOrMore>
+ <element name="field">
+ <attribute name="keyword">
+ <data type="string"/>
+ </attribute>
+ <text/>
+ </element>
+ </oneOrMore>
+ </element>
+ </choice>
+ </oneOrMore>
+ </element>
+ </define>
+
<define name="apDomainRange">
<choice>
<data type="string">
diff --git a/include/libvirt/libvirt-nodedev.h b/include/libvirt/libvirt-nodedev.h
index e492634217..245365b07f 100644
--- a/include/libvirt/libvirt-nodedev.h
+++ b/include/libvirt/libvirt-nodedev.h
@@ -84,6 +84,7 @@ typedef enum {
VIR_CONNECT_LIST_NODE_DEVICES_CAP_AP_CARD = 1 << 18, /* s390 AP Card device */
VIR_CONNECT_LIST_NODE_DEVICES_CAP_AP_QUEUE = 1 << 19, /* s390 AP Queue */
VIR_CONNECT_LIST_NODE_DEVICES_CAP_AP_MATRIX = 1 << 20, /* s390 AP Matrix */
+ VIR_CONNECT_LIST_NODE_DEVICES_CAP_VPD = 1 << 21, /* Device with VPD */
/* filter the devices by active state */
VIR_CONNECT_LIST_NODE_DEVICES_INACTIVE = 1 << 30, /* Inactive devices */
diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c
index 9bbff97ffd..7d9f74f45b 100644
--- a/src/conf/node_device_conf.c
+++ b/src/conf/node_device_conf.c
@@ -36,6 +36,7 @@
#include "virrandom.h"
#include "virlog.h"
#include "virfcp.h"
+#include "virpcivpd.h"
#define VIR_FROM_THIS VIR_FROM_NODEDEV
@@ -70,6 +71,7 @@ VIR_ENUM_IMPL(virNodeDevCap,
"ap_card",
"ap_queue",
"ap_matrix",
+ "vpd",
);
VIR_ENUM_IMPL(virNodeDevNetCap,
@@ -240,6 +242,90 @@ virNodeDeviceCapMdevTypesFormat(virBuffer *buf,
}
}
+static void
+virNodeDeviceCapVPDStringResourceFormat(virBuffer *buf, virPCIVPDResource *res)
+{
+ virBufferAdjustIndent(buf, 2);
+ virBufferEscapeString(buf, "%s", virPCIVPDStringResourceGetValue((virPCIVPDStringResource *)res));
+ virBufferAdjustIndent(buf, -2);
+}
+
+static gboolean
+virNodeDeviceCapVPDWriteTextField(gpointer key, gpointer val, gpointer userData)
+{
+ const gchar *k = (const gchar*)key, *v = (const gchar*)val;
+ virBuffer *buf = userData;
+ virPCIVPDResourceFieldValueFormat format = VIR_PCI_VPD_RESOURCE_FIELD_VALUE_FORMAT_LAST;
+
+ format = virPCIVPDResourceGetFieldValueFormat(k);
+
+ if (format == VIR_PCI_VPD_RESOURCE_FIELD_VALUE_FORMAT_TEXT) {
+ virBufferEscapeString(buf, "<field keyword='%s'>", k);
+ virBufferEscapeString(buf, "%s", v);
+ virBufferEscapeString(buf, "</field>\n", k);
+ }
+ return false;
+}
+
+static void
+virNodeDeviceCapVPDKeywordResourceFormat(virBuffer *buf, virPCIVPDResource *res)
+{
+ virPCIVPDKeywordResource *keywordRes = NULL;
+ g_autofree GHashTableIter *iter = NULL;
+
+ virBufferAddLit(buf, "\n");
+ virBufferAdjustIndent(buf, 2);
+ keywordRes = (virPCIVPDKeywordResource*)res;
+
+ virPCIVPDKeywordResourceForEach(keywordRes, virNodeDeviceCapVPDWriteTextField, buf);
+ virBufferAdjustIndent(buf, -2);
+}
+
+static void
+virNodeDeviceCapVPDResourceFormat(virBuffer *buf, virPCIVPDResource *res)
+{
+ GEnumValue *resRype = NULL;
+ void (*resFormatFunc)(virBuffer *buf, virPCIVPDResource *res);
+
+ if (G_TYPE_CHECK_INSTANCE_TYPE(res, VIR_TYPE_PCI_VPD_STRING_RESOURCE)) {
+ resFormatFunc = virNodeDeviceCapVPDStringResourceFormat;
+ } else if (G_TYPE_CHECK_INSTANCE_TYPE(res, VIR_TYPE_PCI_VPD_KEYWORD_RESOURCE)) {
+ resFormatFunc = virNodeDeviceCapVPDKeywordResourceFormat;
+ } else {
+ /* Unexpected resource type. This should not happen unless the PCI(e) specs
+ * change and new resource types are introduced into util.virpcivpd. Either way,
+ * we can only return the control back to the caller here.
+ */
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("Unexpected VPD resource type encountered during formatting"));
+ return;
+ }
+ virBufferAdjustIndent(buf, 2);
+
+ resRype = virPCIVPDResourceGetResourceType(res);
+ virBufferEscapeString(buf, "<resource type='%s'>", resRype->value_nick);
+ /* Format the resource in a type-specific way. */
+ resFormatFunc(buf, res);
+ virBufferAddLit(buf, "</resource>\n");
+
+ virBufferAdjustIndent(buf, -2);
+}
+
+static void
+virNodeDeviceCapVPDFormat(virBuffer *buf, GList *vpdResources)
+{
+ if (!g_list_length(vpdResources)) {
+ return;
+ }
+
+ virBufferAddLit(buf, "<capability type='vpd'>\n");
+ while (vpdResources) {
+ GList *next = vpdResources->next;
+ virNodeDeviceCapVPDResourceFormat(buf, vpdResources->data);
+ vpdResources = next;
+ }
+ virBufferAddLit(buf, "</capability>\n");
+}
static void
virNodeDeviceCapPCIDefFormat(virBuffer *buf,
@@ -315,6 +401,9 @@ virNodeDeviceCapPCIDefFormat(virBuffer *buf,
data->pci_dev.mdev_types,
data->pci_dev.nmdev_types);
}
+ if (data->pci_dev.flags & VIR_NODE_DEV_CAP_FLAG_PCI_VPD) {
+ virNodeDeviceCapVPDFormat(buf, data->pci_dev.vpd_resources);
+ }
if (data->pci_dev.nIommuGroupDevices) {
virBufferAsprintf(buf, "<iommuGroup number='%d'>\n",
data->pci_dev.iommuGroupNumber);
@@ -673,6 +762,7 @@ virNodeDeviceDefFormat(const virNodeDeviceDef *def)
case VIR_NODE_DEV_CAP_MDEV_TYPES:
case VIR_NODE_DEV_CAP_FC_HOST:
case VIR_NODE_DEV_CAP_VPORTS:
+ case VIR_NODE_DEV_CAP_VPD:
case VIR_NODE_DEV_CAP_LAST:
break;
}
@@ -859,6 +949,132 @@ virNodeDevCapMdevTypesParseXML(xmlXPathContextPtr ctxt,
return ret;
}
+static GTree *
+virNodeDeviceCapVPDParseXMLKeywordResource(xmlXPathContextPtr ctxt)
+{
+ int nfields = -1;
+ g_autofree gchar* fieldValue = NULL;
+ g_autofree gchar* fieldKeyword = NULL;
+ g_autoptr(GTree) resourceFields = NULL;
+ g_autofree xmlNodePtr *nodes = NULL;
+ xmlNodePtr orignode = NULL;
+ size_t i = 0;
+
+ resourceFields = g_tree_new_full(
+ (GCompareDataFunc)g_strcmp0, NULL, g_free, g_free);
+
+ if ((nfields = virXPathNodeSet("./field[@keyword]", ctxt, &nodes)) < 0) {
+ virReportError(VIR_ERR_XML_ERROR, "%s",
+ _("no VPD <field> elements with a keyword specified found"));
+ ctxt->node = orignode;
+ return NULL;
+ }
+
+ orignode = ctxt->node;
+ for (i = 0; i < nfields; i++) {
+ ctxt->node = nodes[i];
+ if (!(fieldKeyword = virXPathString("string(./@keyword[1])", ctxt))) {
+ virReportError(VIR_ERR_XML_ERROR, "%s",
+ _("VPD resource field keyword parsing has failed"));
+ continue;
+ }
+ if (!(fieldValue = virXPathString("string(./text())", ctxt))) {
+ virReportError(VIR_ERR_XML_ERROR, "%s",
+ _("VPD resource field keyword parsing has failed"));
+ continue;
+ }
+ g_tree_insert(resourceFields, g_steal_pointer(&fieldKeyword),
+ g_steal_pointer(&fieldValue));
+ }
+ ctxt->node = orignode;
+ return g_steal_pointer(&resourceFields);
+}
+
+static int
+virNodeDeviceCapVPDParseXML(xmlXPathContextPtr ctxt, GList **vpdResources)
+{
+ xmlNodePtr orignode = NULL;
+ g_autofree gchar* resText = NULL;
+ g_autofree GTree *resourceFields = NULL;
+ g_autofree xmlNodePtr *nodes = NULL;
+ int nresources = -1;
+ g_autofree gchar* resTypeStr = NULL;
+ virPCIVPDResourceType type = VIR_PCI_VPD_RESOURCE_TYPE_LAST;
+ GEnumClass *class;
+ size_t i = 0;
+
+ if ((nresources = virXPathNodeSet("./resource[@type]", ctxt, &nodes)) < 0) {
+ virReportError(VIR_ERR_XML_ERROR, "%s",
+ _("no VPD <resource> elements with a type specified found"));
+ return -1;
+ }
+
+ orignode = ctxt->node;
+ for (i = 0; i < nresources; i++) {
+ ctxt->node = nodes[i];
+ if (!(resTypeStr = virXPathString("string(./@type[1])", ctxt))) {
+ virReportError(VIR_ERR_XML_ERROR, "%s",
+ _("VPD resource type parsing has failed"));
+ ctxt->node = orignode;
+ return -1;
+ }
+
+ class = g_type_class_ref(VIR_TYPE_PCI_VPD_RESOURCE_TYPE);
+ type = g_enum_get_value_by_nick(class, resTypeStr)->value;
+ g_type_class_unref(class);
+ if (!type) {
+ virReportError(VIR_ERR_XML_ERROR, "%s",
+ _("Unexpected VPD resource type encountered"));
+ /* Skip resources with unknown types. */
+ continue;
+ }
+
+ /* Create in-memory representations of resources based on their type. If a resource is not
+ * valid, report an error and skip to parsing another resource.
+ */
+ switch (type) {
+ case VIR_PCI_VPD_RESOURCE_TYPE_STRING:
+ if (!(resText = virXPathString("string(./text())", ctxt))) {
+ virReportError(VIR_ERR_XML_ERROR, "%s",
+ _("Could not read a string resource text"));
+ continue;
+ }
+ *vpdResources = g_list_append(*vpdResources, virPCIVPDStringResourceNew(
+ g_steal_pointer(&resText)));
+ break;
+ case VIR_PCI_VPD_RESOURCE_TYPE_VPD_R:
+ if (!(resourceFields = virNodeDeviceCapVPDParseXMLKeywordResource(ctxt))) {
+ virReportError(VIR_ERR_XML_ERROR,
+ _("Could not parse %s VPD resource fields"), resTypeStr);
+ continue;
+ }
+ *vpdResources = g_list_append(*vpdResources,
+ virPCIVPDKeywordResourceNew(g_steal_pointer(&resourceFields), true));
+ break;
+ case VIR_PCI_VPD_RESOURCE_TYPE_VPD_W:
+ if (!(resourceFields = virNodeDeviceCapVPDParseXMLKeywordResource(ctxt))) {
+ virReportError(VIR_ERR_XML_ERROR,
+ _("Could not parse %s VPD resource fields"), resTypeStr);
+ continue;
+ }
+ *vpdResources = g_list_append(*vpdResources,
+ virPCIVPDKeywordResourceNew(g_steal_pointer(&resourceFields), false));
+ break;
+ case VIR_PCI_VPD_RESOURCE_TYPE_LAST:
+ default:
+ /* If the VPD module is aware of a resource type and it has been serialized into
+ * XML while the parser does not then this is a bug.
+ */
+ virReportError(VIR_ERR_XML_ERROR,
+ _("The XML parser has encountered an unsupported resource type %s"),
+ resTypeStr);
+ ctxt->node = orignode;
+ return -1;
+ }
+ }
+ ctxt->node = orignode;
+ return 0;
+}
static int
virNodeDevAPMatrixCapabilityParseXML(xmlXPathContextPtr ctxt,
@@ -1718,6 +1934,11 @@ virNodeDevPCICapabilityParseXML(xmlXPathContextPtr ctxt,
&pci_dev->nmdev_types) < 0)
return -1;
pci_dev->flags |= VIR_NODE_DEV_CAP_FLAG_PCI_MDEV;
+ } else if (STREQ(type, "vpd")) {
+ if (virNodeDeviceCapVPDParseXML(ctxt, &pci_dev->vpd_resources) < 0) {
+ return -1;
+ }
+ pci_dev->flags |= VIR_NODE_DEV_CAP_FLAG_PCI_VPD;
} else {
int hdrType = virPCIHeaderTypeFromString(type);
@@ -2024,6 +2245,7 @@ virNodeDevCapsDefParseXML(xmlXPathContextPtr ctxt,
case VIR_NODE_DEV_CAP_VPORTS:
case VIR_NODE_DEV_CAP_SCSI_GENERIC:
case VIR_NODE_DEV_CAP_VDPA:
+ case VIR_NODE_DEV_CAP_VPD:
case VIR_NODE_DEV_CAP_LAST:
virReportError(VIR_ERR_INTERNAL_ERROR,
_("unknown capability type '%d' for '%s'"),
@@ -2287,6 +2509,8 @@ virNodeDevCapsDefFree(virNodeDevCapsDef *caps)
for (i = 0; i < data->pci_dev.nmdev_types; i++)
virMediatedDeviceTypeFree(data->pci_dev.mdev_types[i]);
g_free(data->pci_dev.mdev_types);
+ g_list_free_full(g_steal_pointer(&data->pci_dev.vpd_resources),
+ g_object_unref);
break;
case VIR_NODE_DEV_CAP_USB_DEV:
g_free(data->usb_dev.product_name);
@@ -2352,6 +2576,7 @@ virNodeDevCapsDefFree(virNodeDevCapsDef *caps)
case VIR_NODE_DEV_CAP_VDPA:
case VIR_NODE_DEV_CAP_AP_CARD:
case VIR_NODE_DEV_CAP_AP_QUEUE:
+ case VIR_NODE_DEV_CAP_VPD:
case VIR_NODE_DEV_CAP_LAST:
/* This case is here to shutup the compiler */
break;
@@ -2418,6 +2643,7 @@ virNodeDeviceUpdateCaps(virNodeDeviceDef *def)
case VIR_NODE_DEV_CAP_VDPA:
case VIR_NODE_DEV_CAP_AP_CARD:
case VIR_NODE_DEV_CAP_AP_QUEUE:
+ case VIR_NODE_DEV_CAP_VPD:
case VIR_NODE_DEV_CAP_LAST:
break;
}
@@ -2489,6 +2715,10 @@ virNodeDeviceCapsListExport(virNodeDeviceDef *def,
MAYBE_ADD_CAP(VIR_NODE_DEV_CAP_MDEV_TYPES);
ncaps++;
}
+ if (flags & VIR_NODE_DEV_CAP_FLAG_PCI_VPD) {
+ MAYBE_ADD_CAP(VIR_NODE_DEV_CAP_VPD);
+ ncaps++;
+ }
}
if (caps->data.type == VIR_NODE_DEV_CAP_CSS_DEV) {
@@ -2749,6 +2979,44 @@ virNodeDeviceGetMdevTypesCaps(const char *sysfspath,
}
+/**
+ * virNodeDeviceGetPCIVPDDynamicCap:
+ * @devCapPCIDev: a virNodeDevCapPCIDev for which to add VPD resources.
+ *
+ * While VPD has a read-only portion, there may be a read-write portion per
+ * the specs which may change dynamically.
+ *
+ * Returns: 0 if the operation was successful (even if VPD is not present for
+ * that device since it is optional in the specs, -1 otherwise.
+ */
+static int
+virNodeDeviceGetPCIVPDDynamicCap(virNodeDevCapPCIDev *devCapPCIDev)
+{
+ g_autoptr(virPCIDevice) pciDev = NULL;
+ virPCIDeviceAddress devAddr;
+ g_autolist(virPCIVPDResource) resourceList = NULL;
+
+ devAddr.domain = devCapPCIDev->domain;
+ devAddr.bus = devCapPCIDev->bus;
+ devAddr.slot = devCapPCIDev->slot;
+ devAddr.function = devCapPCIDev->function;
+
+ if (!(pciDev = virPCIDeviceNew(&devAddr)))
+ return -1;
+
+ if (virPCIDeviceHasVPD(pciDev)) {
+ /* VPD is optional in PCI(e) specs. If it is there, attempt to add it. */
+ if ((resourceList = virPCIDeviceGetVPDResources(pciDev))) {
+ devCapPCIDev->flags |= VIR_NODE_DEV_CAP_FLAG_PCI_VPD;
+ devCapPCIDev->vpd_resources = g_steal_pointer(&resourceList);
+ } else {
+ g_list_free_full(devCapPCIDev->vpd_resources, g_object_unref);
+ }
+ }
+ return 0;
+}
+
+
/* virNodeDeviceGetPCIDynamicCaps() get info that is stored in sysfs
* about devices related to this device, i.e. things that can change
* without this device itself changing. These must be refreshed
@@ -2771,6 +3039,9 @@ virNodeDeviceGetPCIDynamicCaps(const char *sysfsPath,
if (pci_dev->nmdev_types > 0)
pci_dev->flags |= VIR_NODE_DEV_CAP_FLAG_PCI_MDEV;
+ if (virNodeDeviceGetPCIVPDDynamicCap(pci_dev) < 0)
+ return -1;
+
return 0;
}
diff --git a/src/conf/node_device_conf.h b/src/conf/node_device_conf.h
index 5a4d9c7a55..32e59fa52a 100644
--- a/src/conf/node_device_conf.h
+++ b/src/conf/node_device_conf.h
@@ -69,6 +69,7 @@ typedef enum {
VIR_NODE_DEV_CAP_AP_CARD, /* s390 AP Card device */
VIR_NODE_DEV_CAP_AP_QUEUE, /* s390 AP Queue */
VIR_NODE_DEV_CAP_AP_MATRIX, /* s390 AP Matrix device */
+ VIR_NODE_DEV_CAP_VPD, /* Device provides VPD */
VIR_NODE_DEV_CAP_LAST
} virNodeDevCapType;
@@ -103,6 +104,7 @@ typedef enum {
VIR_NODE_DEV_CAP_FLAG_PCI_VIRTUAL_FUNCTION = (1 << 1),
VIR_NODE_DEV_CAP_FLAG_PCIE = (1 << 2),
VIR_NODE_DEV_CAP_FLAG_PCI_MDEV = (1 << 3),
+ VIR_NODE_DEV_CAP_FLAG_PCI_VPD = (1 << 4),
} virNodeDevPCICapFlags;
typedef enum {
@@ -181,6 +183,7 @@ struct _virNodeDevCapPCIDev {
int hdrType; /* enum virPCIHeaderType or -1 */
virMediatedDeviceType **mdev_types;
size_t nmdev_types;
+ GList *vpd_resources;
};
typedef struct _virNodeDevCapUSBDev virNodeDevCapUSBDev;
@@ -418,7 +421,8 @@ G_DEFINE_AUTOPTR_CLEANUP_FUNC(virNodeDevCapsDef, virNodeDevCapsDefFree);
VIR_CONNECT_LIST_NODE_DEVICES_CAP_VDPA | \
VIR_CONNECT_LIST_NODE_DEVICES_CAP_AP_CARD | \
VIR_CONNECT_LIST_NODE_DEVICES_CAP_AP_QUEUE | \
- VIR_CONNECT_LIST_NODE_DEVICES_CAP_AP_MATRIX)
+ VIR_CONNECT_LIST_NODE_DEVICES_CAP_AP_MATRIX | \
+ VIR_CONNECT_LIST_NODE_DEVICES_CAP_VPD)
#define VIR_CONNECT_LIST_NODE_DEVICES_FILTERS_ACTIVE \
VIR_CONNECT_LIST_NODE_DEVICES_ACTIVE | \
diff --git a/src/conf/virnodedeviceobj.c b/src/conf/virnodedeviceobj.c
index 9a9841576a..165ec1f1dd 100644
--- a/src/conf/virnodedeviceobj.c
+++ b/src/conf/virnodedeviceobj.c
@@ -701,6 +701,9 @@ virNodeDeviceObjHasCap(const virNodeDeviceObj *obj,
if (type == VIR_NODE_DEV_CAP_MDEV_TYPES &&
(cap->data.pci_dev.flags & VIR_NODE_DEV_CAP_FLAG_PCI_MDEV))
return true;
+ if (type == VIR_NODE_DEV_CAP_VPD &&
+ (cap->data.pci_dev.flags & VIR_NODE_DEV_CAP_FLAG_PCI_VPD))
+ return true;
break;
case VIR_NODE_DEV_CAP_SCSI_HOST:
@@ -742,6 +745,7 @@ virNodeDeviceObjHasCap(const virNodeDeviceObj *obj,
case VIR_NODE_DEV_CAP_VDPA:
case VIR_NODE_DEV_CAP_AP_CARD:
case VIR_NODE_DEV_CAP_AP_QUEUE:
+ case VIR_NODE_DEV_CAP_VPD:
case VIR_NODE_DEV_CAP_LAST:
break;
}
@@ -899,7 +903,8 @@ virNodeDeviceObjMatch(virNodeDeviceObj *obj,
MATCH_CAP(VDPA) ||
MATCH_CAP(AP_CARD) ||
MATCH_CAP(AP_QUEUE) ||
- MATCH_CAP(AP_MATRIX)))
+ MATCH_CAP(AP_MATRIX) ||
+ MATCH_CAP(VPD)))
return false;
}
diff --git a/src/node_device/node_device_driver.c b/src/node_device/node_device_driver.c
index 3bc6eb1c11..d19ed7d948 100644
--- a/src/node_device/node_device_driver.c
+++ b/src/node_device/node_device_driver.c
@@ -708,6 +708,7 @@ nodeDeviceObjFormatAddress(virNodeDeviceObj *obj)
case VIR_NODE_DEV_CAP_VDPA:
case VIR_NODE_DEV_CAP_AP_CARD:
case VIR_NODE_DEV_CAP_AP_QUEUE:
+ case VIR_NODE_DEV_CAP_VPD:
case VIR_NODE_DEV_CAP_LAST:
break;
}
@@ -1983,6 +1984,7 @@ int nodeDeviceDefValidate(virNodeDeviceDef *def,
case VIR_NODE_DEV_CAP_AP_CARD:
case VIR_NODE_DEV_CAP_AP_QUEUE:
case VIR_NODE_DEV_CAP_AP_MATRIX:
+ case VIR_NODE_DEV_CAP_VPD:
case VIR_NODE_DEV_CAP_LAST:
break;
}
diff --git a/src/node_device/node_device_udev.c b/src/node_device/node_device_udev.c
index 71f0bef827..7c3bb762b3 100644
--- a/src/node_device/node_device_udev.c
+++ b/src/node_device/node_device_udev.c
@@ -42,6 +42,7 @@
#include "virnetdev.h"
#include "virmdev.h"
#include "virutil.h"
+#include "virpcivpd.h"
#include "configmake.h"
@@ -1397,6 +1398,7 @@ udevGetDeviceDetails(struct udev_device *device,
case VIR_NODE_DEV_CAP_AP_MATRIX:
return udevProcessAPMatrix(device, def);
case VIR_NODE_DEV_CAP_MDEV_TYPES:
+ case VIR_NODE_DEV_CAP_VPD:
case VIR_NODE_DEV_CAP_SYSTEM:
case VIR_NODE_DEV_CAP_FC_HOST:
case VIR_NODE_DEV_CAP_VPORTS:
diff --git a/tests/nodedevschemadata/pci_0000_42_00_0_vpd.xml b/tests/nodedevschemadata/pci_0000_42_00_0_vpd.xml
new file mode 100644
index 0000000000..831b6feb24
--- /dev/null
+++ b/tests/nodedevschemadata/pci_0000_42_00_0_vpd.xml
@@ -0,0 +1,33 @@
+<device>
+ <name>pci_0000_42_00_0</name>
+ <capability type='pci'>
+ <class>0x020000</class>
+ <domain>0</domain>
+ <bus>66</bus>
+ <slot>0</slot>
+ <function>0</function>
+ <product id='0xa2d6'>MT42822 BlueField-2 integrated ConnectX-6 Dx network controller</product>
+ <vendor id='0x15b3'>Mellanox Technologies</vendor>
+ <capability type='virt_functions' maxCount='16'/>
+ <capability type='vpd'>
+ <resource type='string'>BlueField-2 DPU 25GbE Dual-Port SFP56, Crypto Enabled, 16GB on-board DDR, 1GbE OOB management, Tall Bracket</resource>
+ <resource type='vpd-r'>
+ <field keyword='EC'>B1</field>
+ <field keyword='PN'>MBF2H332A-AEEOT</field>
+ <field keyword='SN'>MT2113X00000</field>
+ <field keyword='V0'>PCIeGen4 x8</field>
+ <field keyword='V2'>MBF2H332A-AEEOT</field>
+ <field keyword='V3'>3c53d07eec484d8aab34dabd24fe575aa</field>
+ <field keyword='VA'>MLX:MN=MLNX:CSKU=V2:UUID=V3:PCI=V0:MODL=BF2H332A</field>
+ </resource>
+ </capability>
+ <iommuGroup number='65'>
+ <address domain='0x0000' bus='0x42' slot='0x00' function='0x0'/>
+ </iommuGroup>
+ <numa node='0'/>
+ <pci-express>
+ <link validity='cap' port='0' speed='16' width='8'/>
+ <link validity='sta' speed='8' width='8'/>
+ </pci-express>
+ </capability>
+</device>
diff --git a/tests/nodedevxml2xmlout/pci_0000_42_00_0_vpd.xml b/tests/nodedevxml2xmlout/pci_0000_42_00_0_vpd.xml
new file mode 120000
index 0000000000..a0b5372ca0
--- /dev/null
+++ b/tests/nodedevxml2xmlout/pci_0000_42_00_0_vpd.xml
@@ -0,0 +1 @@
+../nodedevschemadata/pci_0000_42_00_0_vpd.xml
\ No newline at end of file
diff --git a/tests/nodedevxml2xmltest.c b/tests/nodedevxml2xmltest.c
index 9e32e7d553..557347fb07 100644
--- a/tests/nodedevxml2xmltest.c
+++ b/tests/nodedevxml2xmltest.c
@@ -121,6 +121,7 @@ mymain(void)
DO_TEST("pci_0000_02_10_7_sriov_pf_vfs_all_header_type");
DO_TEST("drm_renderD129");
DO_TEST("pci_0000_02_10_7_mdev_types");
+ DO_TEST("pci_0000_42_00_0_vpd");
DO_TEST("mdev_3627463d_b7f0_4fea_b468_f1da537d301b");
DO_TEST("ccw_0_0_ffff");
DO_TEST("css_0_0_ffff");
diff --git a/tools/virsh-nodedev.c b/tools/virsh-nodedev.c
index f72359121f..63725587fc 100644
--- a/tools/virsh-nodedev.c
+++ b/tools/virsh-nodedev.c
@@ -476,6 +476,9 @@ cmdNodeListDevices(vshControl *ctl, const vshCmd *cmd G_GNUC_UNUSED)
case VIR_NODE_DEV_CAP_MDEV:
flags |= VIR_CONNECT_LIST_NODE_DEVICES_CAP_MDEV;
break;
+ case VIR_NODE_DEV_CAP_VPD:
+ flags |= VIR_CONNECT_LIST_NODE_DEVICES_CAP_VPD;
+ break;
case VIR_NODE_DEV_CAP_CCW_DEV:
flags |= VIR_CONNECT_LIST_NODE_DEVICES_CAP_CCW_DEV;
break;
--
2.30.2
On 9/27/21 3:30 PM, Dmitrii Shcherbakov wrote:
[...]
> diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c
[...]
> +
> +static void
> +virNodeDeviceCapVPDResourceFormat(virBuffer *buf, virPCIVPDResource *res)
> +{
> + GEnumValue *resRype = NULL;
> + void (*resFormatFunc)(virBuffer *buf, virPCIVPDResource *res);
> +
> + if (G_TYPE_CHECK_INSTANCE_TYPE(res, VIR_TYPE_PCI_VPD_STRING_RESOURCE)) {
> + resFormatFunc = virNodeDeviceCapVPDStringResourceFormat;
> + } else if (G_TYPE_CHECK_INSTANCE_TYPE(res, VIR_TYPE_PCI_VPD_KEYWORD_RESOURCE)) {
> + resFormatFunc = virNodeDeviceCapVPDKeywordResourceFormat;
> + } else {
> + /* Unexpected resource type. This should not happen unless the PCI(e) specs
> + * change and new resource types are introduced into util.virpcivpd. Either way,
> + * we can only return the control back to the caller here.
> + */
> + virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> + _("Unexpected VPD resource type encountered during formatting"));
> + return;
> + }
> + virBufferAdjustIndent(buf, 2);
> +
> + resRype = virPCIVPDResourceGetResourceType(res);
> + virBufferEscapeString(buf, "<resource type='%s'>", resRype->value_nick);
> + /* Format the resource in a type-specific way. */
> + resFormatFunc(buf, res);
It's probably irrelevant since this will be mostly rewritten based on
Dan's suggestions, but the resFormatFunc() pointer seems like an
unnecessary complication - you could just move the if/else construct
down to here and call the appropriate function directly.
> > It's probably irrelevant since this will be mostly rewritten based on > Dan's suggestions, but the resFormatFunc() pointer seems like an > unnecessary complication - you could just move the if/else construct > down to here and call the appropriate function directly. > Ack, I was trying to reuse the same parent type and a function signature to handle resource formatting in a generic way - in case new resource types ever got added. But that was probably too forward-looking.
On Mon, Sep 27, 2021 at 10:30:51PM +0300, Dmitrii Shcherbakov wrote: > * XML serialization and deserialization of PCI VPD resources; > * PCI VPD capability flags added and used in relevant places; > * XML to XML tests for the added capability. > > Signed-off-by: Dmitrii Shcherbakov <dmitrii.shcherbakov@canonical.com> > diff --git a/tests/nodedevschemadata/pci_0000_42_00_0_vpd.xml b/tests/nodedevschemadata/pci_0000_42_00_0_vpd.xml > new file mode 100644 > index 0000000000..831b6feb24 > --- /dev/null > +++ b/tests/nodedevschemadata/pci_0000_42_00_0_vpd.xml > @@ -0,0 +1,33 @@ > +<device> > + <name>pci_0000_42_00_0</name> > + <capability type='pci'> > + <class>0x020000</class> > + <domain>0</domain> > + <bus>66</bus> > + <slot>0</slot> > + <function>0</function> > + <product id='0xa2d6'>MT42822 BlueField-2 integrated ConnectX-6 Dx network controller</product> > + <vendor id='0x15b3'>Mellanox Technologies</vendor> > + <capability type='virt_functions' maxCount='16'/> > + <capability type='vpd'> > + <resource type='string'>BlueField-2 DPU 25GbE Dual-Port SFP56, Crypto Enabled, 16GB on-board DDR, 1GbE OOB management, Tall Bracket</resource> > + <resource type='vpd-r'> > + <field keyword='EC'>B1</field> > + <field keyword='PN'>MBF2H332A-AEEOT</field> > + <field keyword='SN'>MT2113X00000</field> > + <field keyword='V0'>PCIeGen4 x8</field> > + <field keyword='V2'>MBF2H332A-AEEOT</field> > + <field keyword='V3'>3c53d07eec484d8aab34dabd24fe575aa</field> > + <field keyword='VA'>MLX:MN=MLNX:CSKU=V2:UUID=V3:PCI=V0:MODL=BF2H332A</field> I've got a general comment about what do any of these 2-letter keywords actually mean. I presume they are explaned in the PCI spec, but AFAICT the spec is not publically available for free. So at the very least we need to document each one's meaning in libvirt docs IMHO. If I had insight into what they meant, then I might also suggest giving them meaningful names in libvirt. These two level codes are presumably chosen for reasons of space efficiency at the low level. This is not a constraint we especially care about in libvirt, where ease of understanding is usually more important. 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 :|
On Fri, Oct 01, 2021 at 01:11:19PM +0100, Daniel P. Berrangé wrote:
> On Mon, Sep 27, 2021 at 10:30:51PM +0300, Dmitrii Shcherbakov wrote:
> > * XML serialization and deserialization of PCI VPD resources;
> > * PCI VPD capability flags added and used in relevant places;
> > * XML to XML tests for the added capability.
> >
> > Signed-off-by: Dmitrii Shcherbakov <dmitrii.shcherbakov@canonical.com>
>
>
> > diff --git a/tests/nodedevschemadata/pci_0000_42_00_0_vpd.xml b/tests/nodedevschemadata/pci_0000_42_00_0_vpd.xml
> > new file mode 100644
> > index 0000000000..831b6feb24
> > --- /dev/null
> > +++ b/tests/nodedevschemadata/pci_0000_42_00_0_vpd.xml
> > @@ -0,0 +1,33 @@
> > +<device>
> > + <name>pci_0000_42_00_0</name>
> > + <capability type='pci'>
> > + <class>0x020000</class>
> > + <domain>0</domain>
> > + <bus>66</bus>
> > + <slot>0</slot>
> > + <function>0</function>
> > + <product id='0xa2d6'>MT42822 BlueField-2 integrated ConnectX-6 Dx network controller</product>
> > + <vendor id='0x15b3'>Mellanox Technologies</vendor>
> > + <capability type='virt_functions' maxCount='16'/>
> > + <capability type='vpd'>
> > + <resource type='string'>BlueField-2 DPU 25GbE Dual-Port SFP56, Crypto Enabled, 16GB on-board DDR, 1GbE OOB management, Tall Bracket</resource>
> > + <resource type='vpd-r'>
> > + <field keyword='EC'>B1</field>
> > + <field keyword='PN'>MBF2H332A-AEEOT</field>
> > + <field keyword='SN'>MT2113X00000</field>
> > + <field keyword='V0'>PCIeGen4 x8</field>
> > + <field keyword='V2'>MBF2H332A-AEEOT</field>
> > + <field keyword='V3'>3c53d07eec484d8aab34dabd24fe575aa</field>
> > + <field keyword='VA'>MLX:MN=MLNX:CSKU=V2:UUID=V3:PCI=V0:MODL=BF2H332A</field>
>
> I've got a general comment about what do any of these 2-letter
> keywords actually mean. I presume they are explaned in the
> PCI spec, but AFAICT the spec is not publically available for
> free.
>
> So at the very least we need to document each one's meaning
> in libvirt docs IMHO.
>
> If I had insight into what they meant, then I might also
> suggest giving them meaningful names in libvirt. These two
> level codes are presumably chosen for reasons of space
> efficiency at the low level. This is not a constraint
> we especially care about in libvirt, where ease of
> understanding is usually more important.
Ok, I found a copy of the PCI spec
I see the data is classified at 2 levels, with the first
level being:
String Tag: This tag is the first item in the VPD storage
component. It contains the name of the add-in card
in alphanumeric characters.
VPD-R Tag: This tag contains the read only VPD keywords for an
add-in card.
VPD-W Tag: This tag contains the read/write VPD keywords for
an add-in card
Then for VPD-R, the next level
PN: Add-in Card Part Number
EC: Engineering change level of the card
FG: Fabric Geography
LC: Location
MN: Manufacture ID
PG: PCI Geography
SN: Serial number
Vx: Vendor string (Repeated many times for 'x' in range 0-9, A-Z)
CP: Extended capability
RV: Checksum + reserved space
I don't see a need to report the "CP" and "RV" tags in the XML.
Then for VPD-W,the next level
Vx: Vendor string (Repeated many times for 'x' in range 0-9, A-Z)
Sx: System string (Repeated many times for 'x' in range 0-9, B-Z)
YA: Asset tag
With all this in mind, I think we a better to represent
these all as meaingfully named elements
<capability type="vpd">
<name>BlueField-2 DPU 25GbE Dual-Port SFP56, Crypto Enabled, 16GB on-board DDR, 1GbE OOB management, Tall Bracket</name>
<tags access="readonly">
<change_level>B1</change_level>
<part_number>MBF2H332A-AEEOT</part_number>
<serial_number>MT2113X00000</serial_number>
<vendor_string index="0">PCIeGen4 x8</vendor_string>
<vendor_string index="2">MBF2H332A-AEEOT</vendor_string>
<vendor_string index="3">3c53d07eec484d8aab34dabd24fe575aa</vendor_string>
</tags>
<tags access="readwrite">
...
</tags>
</capability>
> > + <capability type='vpd'>
> > + <resource type='string'>BlueField-2 DPU 25GbE Dual-Port SFP56, Crypto Enabled, 16GB on-board DDR, 1GbE OOB management, Tall Bracket</resource>
> > + <resource type='vpd-r'>
> > + <field keyword='EC'>B1</field>
> > + <field keyword='PN'>MBF2H332A-AEEOT</field>
> > + <field keyword='SN'>MT2113X00000</field>
> > + <field keyword='V0'>PCIeGen4 x8</field>
> > + <field keyword='V2'>MBF2H332A-AEEOT</field>
> > + <field keyword='V3'>3c53d07eec484d8aab34dabd24fe575aa</field>
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 :|
> > > > I've got a general comment about what do any of these 2-letter > > keywords actually mean. I presume they are explaned in the > > PCI spec, but AFAICT the spec is not publically available for > > free. > > So at the very least we need to document each one's meaning > > in libvirt docs IMHO. > > > > If I had insight into what they meant, then I might also > > suggest giving them meaningful names in libvirt. These two > > level codes are presumably chosen for reasons of space > > efficiency at the low level. This is not a constraint > > we especially care about in libvirt, where ease of > > understanding is usually more important. > > Ok, I found a copy of the PCI spec Ack, I could document those fields in a doc update for users to understand what to expect. Just a note: those specs are available if an e-mail of a PCI SIG member company is used during registration at the PCI SIG website so I think it should be available to you too. > > I see the data is classified at 2 levels, with the first > level being: > > String Tag: This tag is the first item in the VPD storage > component. It contains the name of the add-in card > in alphanumeric characters. > > VPD-R Tag: This tag contains the read only VPD keywords for an > add-in card. > > VPD-W Tag: This tag contains the read/write VPD keywords for > an add-in card > > > Then for VPD-R, the next level > > PN: Add-in Card Part Number > EC: Engineering change level of the card > FG: Fabric Geography > LC: Location > MN: Manufacture ID > PG: PCI Geography > SN: Serial number > Vx: Vendor string (Repeated many times for 'x' in range 0-9, A-Z) > CP: Extended capability > RV: Checksum + reserved space > > I don't see a need to report the "CP" and "RV" tags in the XML. Agreed, "CP" is currently ignored and "RV" is used for checksum validation only in the current code. There is also the "RW" keyword which is used to identify an unused portion of the read-write space: AFAIU the read-write VPD memory will have some free space for future writes and the "RW" keyword value probably needs to be updated by the software making writes to this area. I have not actually seen any devices with a pre-programmed VPD-W yet but so far the focus has been on reading what's in there properly. There is only one string resource specified in the spec at the moment (the device name). I suppose we could assume that there will only be one and deal with possible VPD format changes as they arise. Likewise, for VPD-R and VPD-W: there is currently no use-case for having multiple instances for the same type - so maybe I was too forward-looking in this case (i.e. the format hasn't seen much change in years). > > > Then for VPD-W,the next level > > Vx: Vendor string (Repeated many times for 'x' in range 0-9, A-Z) > Sx: System string (Repeated many times for 'x' in range 0-9, B-Z) > YA: Asset tag > > With all this in mind, I think we a better to represent > these all as meaingfully named elements > > <capability type="vpd"> > <name>BlueField-2 DPU 25GbE Dual-Port SFP56, Crypto Enabled, 16GB on-board DDR, 1GbE OOB management, Tall Bracket</name> > <tags access="readonly"> > <change_level>B1</change_level> > <part_number>MBF2H332A-AEEOT</part_number> > <serial_number>MT2113X00000</serial_number> > <vendor_string index="0">PCIeGen4 x8</vendor_string> > <vendor_string index="2">MBF2H332A-AEEOT</vendor_string> > <vendor_string index="3">3c53d07eec484d8aab34dabd24fe575aa</vendor_string> > </tags> > <tags access="readwrite"> > ... > </tags> > </capability> > > Ack, this certainly has benefits for human readability but maybe introduces slightly more work to map human-readable names to spec keywords at the client side (not a big deal for the purposes I plan to use it though). Going to adjust what I have to match the above.
© 2016 - 2026 Red Hat, Inc.