[PATCH libvirt v2 00/11] Support AP card, AP queues and AP matrix
Add support for AP card devices, AP queues and AP matrix devices in libvirt node device driver. --- v2: - The tests included in the patches to detect the node devices (AP card, AP queues and AP matrix) are moved to separate patches. These patches are not divided further due to the following reasons: - The previous patches that add support for node devices in libvirt are not divided further into smaller patches (eg: 53aec799fa31711ffaeacc7ec17ec6d3c2e3cadf). - It would be easier to identify the patches relevant to support AP node devices. - The number of lines in each of these patches are around 125, which is not too much. - Modified according to review comments. - New patch to detect mdev_types capabilty of AP matrix device is added to this patch series. Boris Fiuczynski (1): node_device: detecting mdev_types capability on ap_matrix device Farhan Ali (1): virsh: nodedev: Filter by AP card and AP queue capabilities Shalini Chellathurai Saroja (9): nodedev: detect AP card device tests: AP card node device nodedev: detect AP queues tests: AP queue node device nodedev: detect AP matrix device tests: AP matrix node device virsh: nodedev: filter by AP Matrix capability node_device: refactor address retrieval of node device node_device: mdev matrix support docs/formatnode.html.in | 44 +++- docs/manpages/virsh.rst | 2 +- docs/schemas/nodedev.rng | 70 +++++- include/libvirt/libvirt-nodedev.h | 3 + src/conf/node_device_conf.c | 213 +++++++++++++++++- src/conf/node_device_conf.h | 41 +++- src/conf/virnodedeviceobj.c | 13 +- src/libvirt-nodedev.c | 3 + src/libvirt_private.syms | 1 + src/node_device/node_device_driver.c | 33 ++- src/node_device/node_device_udev.c | 83 +++++++ tests/nodedevschemadata/ap_07_0038.xml | 9 + tests/nodedevschemadata/ap_card07.xml | 8 + tests/nodedevschemadata/ap_matrix.xml | 7 + .../ap_matrix_mdev_types.xml | 14 ++ ...v_ee0b88c4_f554_4dc1_809d_b2a01e8e48ad.xml | 9 + tests/nodedevxml2xmltest.c | 5 + tools/virsh-nodedev.c | 9 + 18 files changed, 554 insertions(+), 13 deletions(-) create mode 100644 tests/nodedevschemadata/ap_07_0038.xml create mode 100644 tests/nodedevschemadata/ap_card07.xml create mode 100644 tests/nodedevschemadata/ap_matrix.xml create mode 100644 tests/nodedevschemadata/ap_matrix_mdev_types.xml create mode 100644 tests/nodedevschemadata/mdev_ee0b88c4_f554_4dc1_809d_b2a01e8e48ad.xml -- 2.26.2
Introduce support for the Adjunct Processor (AP) crypto card device. Udev already detects the device, so add support for libvirt nodedev driver. Signed-off-by: Farhan Ali <alifm@linux.ibm.com> Signed-off-by: Shalini Chellathurai Saroja <shalini@linux.ibm.com> Reviewed-by: Bjoern Walk <bwalk@linux.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com> --- docs/formatnode.html.in | 8 ++++++ docs/schemas/nodedev.rng | 34 ++++++++++++++++++++----- src/conf/node_device_conf.c | 41 ++++++++++++++++++++++++++++++ src/conf/node_device_conf.h | 8 ++++++ src/conf/virnodedeviceobj.c | 1 + src/node_device/node_device_udev.c | 29 +++++++++++++++++++++ tools/virsh-nodedev.c | 1 + 7 files changed, 116 insertions(+), 6 deletions(-) diff --git a/docs/formatnode.html.in b/docs/formatnode.html.in index c7df69c9..d10a79e3 100644 --- a/docs/formatnode.html.in +++ b/docs/formatnode.html.in @@ -431,6 +431,14 @@ device.</dd> </dl> </dd> + <dt><code>ap_card</code></dt> + <dd>Describes the Adjunct Processor (AP) Card device on a S390 host. + Sub-elements include: + <dl> + <dt><code>ap-adapter</code></dt> + <dd>AP Card identifier.</dd> + </dl> + </dd> </dl> </dd> </dl> diff --git a/docs/schemas/nodedev.rng b/docs/schemas/nodedev.rng index b3e98665..d02e5377 100644 --- a/docs/schemas/nodedev.rng +++ b/docs/schemas/nodedev.rng @@ -87,6 +87,7 @@ <ref name="capccwdev"/> <ref name="capcssdev"/> <ref name="capvdpa"/> + <ref name="capapcard"/> </choice> </element> </define> @@ -668,12 +669,21 @@ </element> </define> - <define name="address"> - <element name="address"> - <attribute name="domain"><ref name="hexuint"/></attribute> - <attribute name="bus"><ref name="hexuint"/></attribute> - <attribute name="slot"><ref name="hexuint"/></attribute> - <attribute name="function"><ref name="hexuint"/></attribute> + <define name='capapcard'> + <attribute name='type'> + <value>ap_card</value> + </attribute> + <element name='ap-adapter'> + <ref name='apAdapterRange'/> + </element> + </define> + + <define name='address'> + <element name='address'> + <attribute name='domain'><ref name='hexuint'/></attribute> + <attribute name='bus'><ref name='hexuint'/></attribute> + <attribute name='slot'><ref name='hexuint'/></attribute> + <attribute name='function'><ref name='hexuint'/></attribute> </element> </define> @@ -716,4 +726,16 @@ </element> </define> + <define name="apAdapterRange"> + <choice> + <data type="string"> + <param name="pattern">(0x)?[0-9a-fA-F]{1,2}</param> + </data> + <data type="int"> + <param name="minInclusive">0</param> + <param name="maxInclusive">255</param> + </data> + </choice> + </define> + </grammar> diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c index 4e2837c1..5d7a23cb 100644 --- a/src/conf/node_device_conf.c +++ b/src/conf/node_device_conf.c @@ -67,6 +67,7 @@ VIR_ENUM_IMPL(virNodeDevCap, "ccw", "css", "vdpa", + "ap_card", ); VIR_ENUM_IMPL(virNodeDevNetCap, @@ -650,6 +651,10 @@ virNodeDeviceDefFormat(const virNodeDeviceDef *def) case VIR_NODE_DEV_CAP_VDPA: virNodeDeviceCapVDPADefFormat(&buf, data); break; + case VIR_NODE_DEV_CAP_AP_CARD: + virBufferAsprintf(&buf, "<ap-adapter>0x%02x</ap-adapter>\n", + data->ap_card.ap_adapter); + break; case VIR_NODE_DEV_CAP_MDEV_TYPES: case VIR_NODE_DEV_CAP_FC_HOST: case VIR_NODE_DEV_CAP_VPORTS: @@ -941,6 +946,36 @@ virNodeDevCapCCWParseXML(xmlXPathContextPtr ctxt, } +static int +virNodeDevCapAPCardParseXML(xmlXPathContextPtr ctxt, + virNodeDeviceDefPtr def, + xmlNodePtr node, + virNodeDevCapAPCardPtr ap_card) +{ + xmlNodePtr orig; + g_autofree char *adapter = NULL; + + orig = ctxt->node; + ctxt->node = node; + + if (!(adapter = virXPathString("string(./ap-adapter[1])", ctxt))) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("missing ap-adapter value for '%s'"), def->name); + return -1; + } + + if (virStrToLong_uip(adapter, NULL, 0, &ap_card->ap_adapter) < 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("invalid ap-adapter value '%s' for '%s'"), + adapter, def->name); + return -1; + } + + ctxt->node = orig; + return 0; +} + + static int virNodeDevCapStorageParseXML(xmlXPathContextPtr ctxt, virNodeDeviceDefPtr def, @@ -1979,6 +2014,10 @@ virNodeDevCapsDefParseXML(xmlXPathContextPtr ctxt, case VIR_NODE_DEV_CAP_CSS_DEV: ret = virNodeDevCapCCWParseXML(ctxt, def, node, &caps->data.ccw_dev); break; + case VIR_NODE_DEV_CAP_AP_CARD: + ret = virNodeDevCapAPCardParseXML(ctxt, def, node, + &caps->data.ap_card); + break; case VIR_NODE_DEV_CAP_MDEV_TYPES: case VIR_NODE_DEV_CAP_FC_HOST: case VIR_NODE_DEV_CAP_VPORTS: @@ -2306,6 +2345,7 @@ virNodeDevCapsDefFree(virNodeDevCapsDefPtr caps) case VIR_NODE_DEV_CAP_VPORTS: case VIR_NODE_DEV_CAP_CCW_DEV: case VIR_NODE_DEV_CAP_VDPA: + case VIR_NODE_DEV_CAP_AP_CARD: case VIR_NODE_DEV_CAP_LAST: /* This case is here to shutup the compiler */ break; @@ -2365,6 +2405,7 @@ virNodeDeviceUpdateCaps(virNodeDeviceDefPtr def) case VIR_NODE_DEV_CAP_MDEV: case VIR_NODE_DEV_CAP_CCW_DEV: case VIR_NODE_DEV_CAP_VDPA: + case VIR_NODE_DEV_CAP_AP_CARD: case VIR_NODE_DEV_CAP_LAST: break; } diff --git a/src/conf/node_device_conf.h b/src/conf/node_device_conf.h index 262524fc..f86f880c 100644 --- a/src/conf/node_device_conf.h +++ b/src/conf/node_device_conf.h @@ -66,6 +66,7 @@ typedef enum { VIR_NODE_DEV_CAP_CCW_DEV, /* s390 CCW device */ VIR_NODE_DEV_CAP_CSS_DEV, /* s390 channel subsystem device */ VIR_NODE_DEV_CAP_VDPA, /* vDPA device */ + VIR_NODE_DEV_CAP_AP_CARD, /* s390 AP Card device */ VIR_NODE_DEV_CAP_LAST } virNodeDevCapType; @@ -289,6 +290,12 @@ struct _virNodeDevCapVDPA { char *chardev; }; +typedef struct _virNodeDevCapAPCard virNodeDevCapAPCard; +typedef virNodeDevCapAPCard *virNodeDevCapAPCardPtr; +struct _virNodeDevCapAPCard { + unsigned int ap_adapter; +}; + typedef struct _virNodeDevCapData virNodeDevCapData; typedef virNodeDevCapData *virNodeDevCapDataPtr; struct _virNodeDevCapData { @@ -308,6 +315,7 @@ struct _virNodeDevCapData { virNodeDevCapMdev mdev; virNodeDevCapCCW ccw_dev; virNodeDevCapVDPA vdpa; + virNodeDevCapAPCard ap_card; }; }; diff --git a/src/conf/virnodedeviceobj.c b/src/conf/virnodedeviceobj.c index 9e46d22a..0aa59289 100644 --- a/src/conf/virnodedeviceobj.c +++ b/src/conf/virnodedeviceobj.c @@ -717,6 +717,7 @@ virNodeDeviceObjHasCap(const virNodeDeviceObj *obj, case VIR_NODE_DEV_CAP_MDEV: case VIR_NODE_DEV_CAP_CCW_DEV: case VIR_NODE_DEV_CAP_VDPA: + case VIR_NODE_DEV_CAP_AP_CARD: 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 65f312d8..b4eb4553 100644 --- a/src/node_device/node_device_udev.c +++ b/src/node_device/node_device_udev.c @@ -1193,6 +1193,31 @@ udevProcessVDPA(struct udev_device *device, } +static int +udevProcessAPCard(struct udev_device *device, + virNodeDeviceDefPtr def) +{ + char *c; + virNodeDevCapDataPtr data = &def->caps->data; + + /* The sysfs path would be in the format /sys/bus/ap/devices/cardXX, + where XX is the ap adapter id */ + if ((c = strrchr(def->sysfs_path, '/')) == NULL || + virStrToLong_ui(c + 1 + strlen("card"), NULL, 16, + &data->ap_card.ap_adapter) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("failed to parse the AP Card from sysfs path: '%s'"), + def->sysfs_path); + return -1; + } + + if (udevGenerateDeviceName(device, def, NULL) != 0) + return -1; + + return 0; +} + + static int udevGetDeviceNodes(struct udev_device *device, virNodeDeviceDefPtr def) @@ -1247,6 +1272,8 @@ udevGetDeviceType(struct udev_device *device, *type = VIR_NODE_DEV_CAP_NET; else if (STREQ(devtype, "drm_minor")) *type = VIR_NODE_DEV_CAP_DRM; + else if (STREQ(devtype, "ap_card")) + *type = VIR_NODE_DEV_CAP_AP_CARD; } else { /* PCI devices don't set the DEVTYPE property. */ if (udevHasDeviceProperty(device, "PCI_CLASS")) @@ -1322,6 +1349,8 @@ udevGetDeviceDetails(struct udev_device *device, return udevProcessCSS(device, def); case VIR_NODE_DEV_CAP_VDPA: return udevProcessVDPA(device, def); + case VIR_NODE_DEV_CAP_AP_CARD: + return udevProcessAPCard(device, def); case VIR_NODE_DEV_CAP_MDEV_TYPES: case VIR_NODE_DEV_CAP_SYSTEM: case VIR_NODE_DEV_CAP_FC_HOST: diff --git a/tools/virsh-nodedev.c b/tools/virsh-nodedev.c index 527bf49f..32c12553 100644 --- a/tools/virsh-nodedev.c +++ b/tools/virsh-nodedev.c @@ -467,6 +467,7 @@ cmdNodeListDevices(vshControl *ctl, const vshCmd *cmd G_GNUC_UNUSED) case VIR_NODE_DEV_CAP_VDPA: flags |= VIR_CONNECT_LIST_NODE_DEVICES_CAP_VDPA; break; + case VIR_NODE_DEV_CAP_AP_CARD: case VIR_NODE_DEV_CAP_LAST: break; } -- 2.26.2
On Thu, 12 Nov 2020 13:15:09 +0100 Shalini Chellathurai Saroja <shalini@linux.ibm.com> wrote:
Introduce support for the Adjunct Processor (AP) crypto card device. Udev already detects the device, so add support for libvirt nodedev driver.
Signed-off-by: Farhan Ali <alifm@linux.ibm.com> Signed-off-by: Shalini Chellathurai Saroja <shalini@linux.ibm.com> Reviewed-by: Bjoern Walk <bwalk@linux.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com> --- docs/formatnode.html.in | 8 ++++++ docs/schemas/nodedev.rng | 34 ++++++++++++++++++++----- src/conf/node_device_conf.c | 41 ++++++++++++++++++++++++++++++ src/conf/node_device_conf.h | 8 ++++++ src/conf/virnodedeviceobj.c | 1 + src/node_device/node_device_udev.c | 29 +++++++++++++++++++++ tools/virsh-nodedev.c | 1 + 7 files changed, 116 insertions(+), 6 deletions(-)
diff --git a/docs/formatnode.html.in b/docs/formatnode.html.in index c7df69c9..d10a79e3 100644 --- a/docs/formatnode.html.in +++ b/docs/formatnode.html.in @@ -431,6 +431,14 @@ device.</dd> </dl> </dd> + <dt><code>ap_card</code></dt> + <dd>Describes the Adjunct Processor (AP) Card device on a S390 host. + Sub-elements include: + <dl> + <dt><code>ap-adapter</code></dt> + <dd>AP Card identifier.</dd> + </dl> + </dd> </dl> </dd> </dl> diff --git a/docs/schemas/nodedev.rng b/docs/schemas/nodedev.rng index b3e98665..d02e5377 100644 --- a/docs/schemas/nodedev.rng +++ b/docs/schemas/nodedev.rng @@ -87,6 +87,7 @@ <ref name="capccwdev"/> <ref name="capcssdev"/> <ref name="capvdpa"/> + <ref name="capapcard"/> </choice> </element> </define> @@ -668,12 +669,21 @@ </element> </define>
- <define name="address"> - <element name="address"> - <attribute name="domain"><ref name="hexuint"/></attribute> - <attribute name="bus"><ref name="hexuint"/></attribute> - <attribute name="slot"><ref name="hexuint"/></attribute> - <attribute name="function"><ref name="hexuint"/></attribute> + <define name='capapcard'> + <attribute name='type'> + <value>ap_card</value> + </attribute> + <element name='ap-adapter'> + <ref name='apAdapterRange'/> + </element> + </define> + + <define name='address'> + <element name='address'> + <attribute name='domain'><ref name='hexuint'/></attribute> + <attribute name='bus'><ref name='hexuint'/></attribute> + <attribute name='slot'><ref name='hexuint'/></attribute> + <attribute name='function'><ref name='hexuint'/></attribute>
It seems that you're unnecessarily changing double-quotes to single-quotes here, which adds spurious changes to the diff. The rest of the file uses double-quotes. Let's stick with that.
</element> </define>
@@ -716,4 +726,16 @@ </element> </define>
+ <define name="apAdapterRange"> + <choice> + <data type="string"> + <param name="pattern">(0x)?[0-9a-fA-F]{1,2}</param> + </data> + <data type="int"> + <param name="minInclusive">0</param> + <param name="maxInclusive">255</param> + </data> + </choice> + </define>
As far as I can tell, this is identical to the definition of the 'uint8' type in basictypes.rng. Is there a reason for defining a different type?
+ </grammar> diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c index 4e2837c1..5d7a23cb 100644 --- a/src/conf/node_device_conf.c +++ b/src/conf/node_device_conf.c @@ -67,6 +67,7 @@ VIR_ENUM_IMPL(virNodeDevCap, "ccw", "css", "vdpa", + "ap_card", );
VIR_ENUM_IMPL(virNodeDevNetCap, @@ -650,6 +651,10 @@ virNodeDeviceDefFormat(const virNodeDeviceDef *def) case VIR_NODE_DEV_CAP_VDPA: virNodeDeviceCapVDPADefFormat(&buf, data); break; + case VIR_NODE_DEV_CAP_AP_CARD: + virBufferAsprintf(&buf, "<ap-adapter>0x%02x</ap-adapter>\n", + data->ap_card.ap_adapter); + break; case VIR_NODE_DEV_CAP_MDEV_TYPES: case VIR_NODE_DEV_CAP_FC_HOST: case VIR_NODE_DEV_CAP_VPORTS: @@ -941,6 +946,36 @@ virNodeDevCapCCWParseXML(xmlXPathContextPtr ctxt, }
+static int +virNodeDevCapAPCardParseXML(xmlXPathContextPtr ctxt, + virNodeDeviceDefPtr def, + xmlNodePtr node, + virNodeDevCapAPCardPtr ap_card) +{ + xmlNodePtr orig; + g_autofree char *adapter = NULL; + + orig = ctxt->node; + ctxt->node = node; + + if (!(adapter = virXPathString("string(./ap-adapter[1])", ctxt))) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("missing ap-adapter value for '%s'"), def->name); + return -1; + } + + if (virStrToLong_uip(adapter, NULL, 0, &ap_card->ap_adapter) < 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("invalid ap-adapter value '%s' for '%s'"), + adapter, def->name); + return -1; + } + + ctxt->node = orig; + return 0; +} + + static int virNodeDevCapStorageParseXML(xmlXPathContextPtr ctxt, virNodeDeviceDefPtr def, @@ -1979,6 +2014,10 @@ virNodeDevCapsDefParseXML(xmlXPathContextPtr ctxt, case VIR_NODE_DEV_CAP_CSS_DEV: ret = virNodeDevCapCCWParseXML(ctxt, def, node, &caps->data.ccw_dev); break; + case VIR_NODE_DEV_CAP_AP_CARD: + ret = virNodeDevCapAPCardParseXML(ctxt, def, node, + &caps->data.ap_card); + break; case VIR_NODE_DEV_CAP_MDEV_TYPES: case VIR_NODE_DEV_CAP_FC_HOST: case VIR_NODE_DEV_CAP_VPORTS: @@ -2306,6 +2345,7 @@ virNodeDevCapsDefFree(virNodeDevCapsDefPtr caps) case VIR_NODE_DEV_CAP_VPORTS: case VIR_NODE_DEV_CAP_CCW_DEV: case VIR_NODE_DEV_CAP_VDPA: + case VIR_NODE_DEV_CAP_AP_CARD: case VIR_NODE_DEV_CAP_LAST: /* This case is here to shutup the compiler */ break; @@ -2365,6 +2405,7 @@ virNodeDeviceUpdateCaps(virNodeDeviceDefPtr def) case VIR_NODE_DEV_CAP_MDEV: case VIR_NODE_DEV_CAP_CCW_DEV: case VIR_NODE_DEV_CAP_VDPA: + case VIR_NODE_DEV_CAP_AP_CARD: case VIR_NODE_DEV_CAP_LAST: break; } diff --git a/src/conf/node_device_conf.h b/src/conf/node_device_conf.h index 262524fc..f86f880c 100644 --- a/src/conf/node_device_conf.h +++ b/src/conf/node_device_conf.h @@ -66,6 +66,7 @@ typedef enum { VIR_NODE_DEV_CAP_CCW_DEV, /* s390 CCW device */ VIR_NODE_DEV_CAP_CSS_DEV, /* s390 channel subsystem device */ VIR_NODE_DEV_CAP_VDPA, /* vDPA device */ + VIR_NODE_DEV_CAP_AP_CARD, /* s390 AP Card device */
VIR_NODE_DEV_CAP_LAST } virNodeDevCapType; @@ -289,6 +290,12 @@ struct _virNodeDevCapVDPA { char *chardev; };
+typedef struct _virNodeDevCapAPCard virNodeDevCapAPCard; +typedef virNodeDevCapAPCard *virNodeDevCapAPCardPtr; +struct _virNodeDevCapAPCard { + unsigned int ap_adapter; +}; + typedef struct _virNodeDevCapData virNodeDevCapData; typedef virNodeDevCapData *virNodeDevCapDataPtr; struct _virNodeDevCapData { @@ -308,6 +315,7 @@ struct _virNodeDevCapData { virNodeDevCapMdev mdev; virNodeDevCapCCW ccw_dev; virNodeDevCapVDPA vdpa; + virNodeDevCapAPCard ap_card; }; };
diff --git a/src/conf/virnodedeviceobj.c b/src/conf/virnodedeviceobj.c index 9e46d22a..0aa59289 100644 --- a/src/conf/virnodedeviceobj.c +++ b/src/conf/virnodedeviceobj.c @@ -717,6 +717,7 @@ virNodeDeviceObjHasCap(const virNodeDeviceObj *obj, case VIR_NODE_DEV_CAP_MDEV: case VIR_NODE_DEV_CAP_CCW_DEV: case VIR_NODE_DEV_CAP_VDPA: + case VIR_NODE_DEV_CAP_AP_CARD: 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 65f312d8..b4eb4553 100644 --- a/src/node_device/node_device_udev.c +++ b/src/node_device/node_device_udev.c @@ -1193,6 +1193,31 @@ udevProcessVDPA(struct udev_device *device, }
+static int +udevProcessAPCard(struct udev_device *device, + virNodeDeviceDefPtr def) +{ + char *c; + virNodeDevCapDataPtr data = &def->caps->data; + + /* The sysfs path would be in the format /sys/bus/ap/devices/cardXX, + where XX is the ap adapter id */ + if ((c = strrchr(def->sysfs_path, '/')) == NULL || + virStrToLong_ui(c + 1 + strlen("card"), NULL, 16, + &data->ap_card.ap_adapter) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("failed to parse the AP Card from sysfs path: '%s'"), + def->sysfs_path); + return -1; + } + + if (udevGenerateDeviceName(device, def, NULL) != 0) + return -1; + + return 0; +} + + static int udevGetDeviceNodes(struct udev_device *device, virNodeDeviceDefPtr def) @@ -1247,6 +1272,8 @@ udevGetDeviceType(struct udev_device *device, *type = VIR_NODE_DEV_CAP_NET; else if (STREQ(devtype, "drm_minor")) *type = VIR_NODE_DEV_CAP_DRM; + else if (STREQ(devtype, "ap_card")) + *type = VIR_NODE_DEV_CAP_AP_CARD; } else { /* PCI devices don't set the DEVTYPE property. */ if (udevHasDeviceProperty(device, "PCI_CLASS")) @@ -1322,6 +1349,8 @@ udevGetDeviceDetails(struct udev_device *device, return udevProcessCSS(device, def); case VIR_NODE_DEV_CAP_VDPA: return udevProcessVDPA(device, def); + case VIR_NODE_DEV_CAP_AP_CARD: + return udevProcessAPCard(device, def); case VIR_NODE_DEV_CAP_MDEV_TYPES: case VIR_NODE_DEV_CAP_SYSTEM: case VIR_NODE_DEV_CAP_FC_HOST: diff --git a/tools/virsh-nodedev.c b/tools/virsh-nodedev.c index 527bf49f..32c12553 100644 --- a/tools/virsh-nodedev.c +++ b/tools/virsh-nodedev.c @@ -467,6 +467,7 @@ cmdNodeListDevices(vshControl *ctl, const vshCmd *cmd G_GNUC_UNUSED) case VIR_NODE_DEV_CAP_VDPA: flags |= VIR_CONNECT_LIST_NODE_DEVICES_CAP_VDPA; break; + case VIR_NODE_DEV_CAP_AP_CARD: case VIR_NODE_DEV_CAP_LAST: break; }
Hi Jonathon, Thank you for the quick review:-) On 11/12/20 5:27 PM, Jonathon Jongsma wrote:
+ <define name='address'> + <element name='address'> + <attribute name='domain'><ref name='hexuint'/></attribute> + <attribute name='bus'><ref name='hexuint'/></attribute> + <attribute name='slot'><ref name='hexuint'/></attribute> + <attribute name='function'><ref name='hexuint'/></attribute> It seems that you're unnecessarily changing double-quotes to single-quotes here, which adds spurious changes to the diff. The rest of the file uses double-quotes. Let's stick with that. Sure.
</element> </define>
@@ -716,4 +726,16 @@ </element> </define>
+ <define name="apAdapterRange"> + <choice> + <data type="string"> + <param name="pattern">(0x)?[0-9a-fA-F]{1,2}</param> + </data> + <data type="int"> + <param name="minInclusive">0</param> + <param name="maxInclusive">255</param> + </data> + </choice> + </define>
As far as I can tell, this is identical to the definition of the 'uint8' type in basictypes.rng. Is there a reason for defining a different type?
Yes, In definition apAdapterRange, the prefix '0x' is optional. -- Kind regards Shalini Chellathurai Saroja Linux on Z and Virtualization Development Dept 1419 Vorsitzende des Aufsichtsrats: Gregor Pillen Geschäftsführung: Dirk Wittkopp Sitz der Gesellschaft: Böblingen Registergericht: Amtsgericht Stuttgart, HRB 243294
On Fri, 13 Nov 2020 12:09:26 +0100 Shalini Chellathurai Saroja <shalini@linux.ibm.com> wrote:
Hi Jonathon,
Thank you for the quick review:-)
On 11/12/20 5:27 PM, Jonathon Jongsma wrote:
+ <define name='address'> + <element name='address'> + <attribute name='domain'><ref name='hexuint'/></attribute> + <attribute name='bus'><ref name='hexuint'/></attribute> + <attribute name='slot'><ref name='hexuint'/></attribute> + <attribute name='function'><ref name='hexuint'/></attribute> It seems that you're unnecessarily changing double-quotes to single-quotes here, which adds spurious changes to the diff. The rest of the file uses double-quotes. Let's stick with that. Sure.
</element> </define>
@@ -716,4 +726,16 @@ </element> </define>
+ <define name="apAdapterRange"> + <choice> + <data type="string"> + <param name="pattern">(0x)?[0-9a-fA-F]{1,2}</param> + </data> + <data type="int"> + <param name="minInclusive">0</param> + <param name="maxInclusive">255</param> + </data> + </choice> + </define>
As far as I can tell, this is identical to the definition of the 'uint8' type in basictypes.rng. Is there a reason for defining a different type?
Yes, In definition apAdapterRange, the prefix '0x' is optional.
Oh, I guess I missed that part. Now that I look at basictypes.rng, I see that the '0x' is required for uint8 and uint24, but it is optional for uint16 and uint32. Does anybody know the justification for these differences? That said, I don't believe that your parsing code actually supports an optional '0x' prefix. In virNodeDevCapAPCardParseXML(), you call virStrToLong_uip(adapter, NULL, 0, &ap_card->ap_adapter) But I'm quite sure that passing a value of e.g. 'ff' for adapter will result in a parsing failure. Try changing the ap-adapter value in tests/nodedevschemadata/ap_card07.xml to some different values and see what happens. Jonathon
Jonathon Jongsma <jjongsma@redhat.com> [2020-11-13, 09:28AM -0600]:
That said, I don't believe that your parsing code actually supports an optional '0x' prefix. In virNodeDevCapAPCardParseXML(), you call
virStrToLong_uip(adapter, NULL, 0, &ap_card->ap_adapter)
But I'm quite sure that passing a value of e.g. 'ff' for adapter will result in a parsing failure. Try changing the ap-adapter value in tests/nodedevschemadata/ap_card07.xml to some different values and see what happens.
As it should, because 'ff' is not a valid hex value in libvirt context. Maybe this changed for for the newer types. I can remember a long discussion couple of years ago about this disambiguity and if we should require the '0x'-prefix and make the base for the conversion explicit, but this was rejected.
Jonathon
-- IBM Systems Linux on Z & Virtualization Development -------------------------------------------------- IBM Deutschland Research & Development GmbH Schönaicher Str. 220, 71032 Böblingen Phone: +49 7031 16 1819 -------------------------------------------------- Vorsitzende des Aufsichtsrats: Gregor Pillen Geschäftsführung: Dirk Wittkopp Sitz der Gesellschaft: Böblingen Registergericht: Amtsgericht Stuttgart, HRB 243294
On 11/13/20 4:28 PM, Jonathon Jongsma wrote:
On Fri, 13 Nov 2020 12:09:26 +0100 Shalini Chellathurai Saroja <shalini@linux.ibm.com> wrote:
Hi Jonathon,
Thank you for the quick review:-)
On 11/12/20 5:27 PM, Jonathon Jongsma wrote:
+ <define name='address'> + <element name='address'> + <attribute name='domain'><ref name='hexuint'/></attribute> + <attribute name='bus'><ref name='hexuint'/></attribute> + <attribute name='slot'><ref name='hexuint'/></attribute> + <attribute name='function'><ref name='hexuint'/></attribute> It seems that you're unnecessarily changing double-quotes to single-quotes here, which adds spurious changes to the diff. The rest of the file uses double-quotes. Let's stick with that. Sure.
</element> </define>
@@ -716,4 +726,16 @@ </element> </define>
+ <define name="apAdapterRange"> + <choice> + <data type="string"> + <param name="pattern">(0x)?[0-9a-fA-F]{1,2}</param> + </data> + <data type="int"> + <param name="minInclusive">0</param> + <param name="maxInclusive">255</param> + </data> + </choice> + </define>
As far as I can tell, this is identical to the definition of the 'uint8' type in basictypes.rng. Is there a reason for defining a different type?
Yes, In definition apAdapterRange, the prefix '0x' is optional.
Oh, I guess I missed that part. Now that I look at basictypes.rng, I see that the '0x' is required for uint8 and uint24, but it is optional for uint16 and uint32. Does anybody know the justification for these differences?
That said, I don't believe that your parsing code actually supports an optional '0x' prefix. In virNodeDevCapAPCardParseXML(), you call
virStrToLong_uip(adapter, NULL, 0, &ap_card->ap_adapter)
But I'm quite sure that passing a value of e.g. 'ff' for adapter will result in a parsing failure. Try changing the ap-adapter value in tests/nodedevschemadata/ap_card07.xml to some different values and see what happens.
Jonathon
Hi Jonathon, That's true, I will use the uint8 definition instead, thank you:-) -- Kind regards Shalini Chellathurai Saroja Linux on Z and Virtualization Development Vorsitzende des Aufsichtsrats: Gregor Pillen Geschäftsführung: Dirk Wittkopp Sitz der Gesellschaft: Böblingen Registergericht: Amtsgericht Stuttgart, HRB 243294
Add tests to verify libvirt node device driver support for AP card device. Signed-off-by: Farhan Ali <alifm@linux.ibm.com> Signed-off-by: Shalini Chellathurai Saroja <shalini@linux.ibm.com> Reviewed-by: Bjoern Walk <bwalk@linux.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com> --- tests/nodedevschemadata/ap_card07.xml | 8 ++++++++ tests/nodedevxml2xmltest.c | 1 + 2 files changed, 9 insertions(+) create mode 100644 tests/nodedevschemadata/ap_card07.xml diff --git a/tests/nodedevschemadata/ap_card07.xml b/tests/nodedevschemadata/ap_card07.xml new file mode 100644 index 00000000..14a845fd --- /dev/null +++ b/tests/nodedevschemadata/ap_card07.xml @@ -0,0 +1,8 @@ +<device> + <name>ap_card07</name> + <path>/sys/devices/ap/card07</path> + <parent>computer</parent> + <capability type='ap_card'> + <ap-adapter>0x07</ap-adapter> + </capability> +</device> diff --git a/tests/nodedevxml2xmltest.c b/tests/nodedevxml2xmltest.c index a009ecb3..7b8a5a82 100644 --- a/tests/nodedevxml2xmltest.c +++ b/tests/nodedevxml2xmltest.c @@ -125,6 +125,7 @@ mymain(void) DO_TEST("ccw_0_0_ffff"); DO_TEST("css_0_0_ffff"); DO_TEST("css_0_0_fffe_mdev_types"); + DO_TEST("ap_card07"); return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE; } -- 2.26.2
Each AP card device can support upto 256 AP queues. AP queues are also detected by udev, so add support for libvirt nodedev driver. Signed-off-by: Farhan Ali <alifm@linux.ibm.com> Signed-off-by: Shalini Chellathurai Saroja <shalini@linux.ibm.com> Reviewed-by: Bjoern Walk <bwalk@linux.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com> --- docs/formatnode.html.in | 11 ++++++ docs/schemas/nodedev.rng | 25 +++++++++++++ src/conf/node_device_conf.c | 59 ++++++++++++++++++++++++++++++ src/conf/node_device_conf.h | 9 +++++ src/conf/virnodedeviceobj.c | 1 + src/node_device/node_device_udev.c | 27 ++++++++++++++ tools/virsh-nodedev.c | 1 + 7 files changed, 133 insertions(+) diff --git a/docs/formatnode.html.in b/docs/formatnode.html.in index d10a79e3..45281363 100644 --- a/docs/formatnode.html.in +++ b/docs/formatnode.html.in @@ -439,6 +439,17 @@ <dd>AP Card identifier.</dd> </dl> </dd> + <dt><code>ap_queue</code></dt> + <dd>Describes the AP Queue on a S390 host. An AP Queue is + identified by it's ap-adapter and ap-domain id. Sub-elements include: + <dl> + <dt><code>ap-adapter</code></dt> + <dd>The ap-adapter of an AP Queue identifies AP Card to which + this queue belongs.</dd> + <dt><code>ap-domain</code></dt> + <dd>AP Queue identifier.</dd> + </dl> + </dd> </dl> </dd> </dl> diff --git a/docs/schemas/nodedev.rng b/docs/schemas/nodedev.rng index d02e5377..51cb8854 100644 --- a/docs/schemas/nodedev.rng +++ b/docs/schemas/nodedev.rng @@ -88,6 +88,7 @@ <ref name="capcssdev"/> <ref name="capvdpa"/> <ref name="capapcard"/> + <ref name="capapqueue"/> </choice> </element> </define> @@ -678,6 +679,18 @@ </element> </define> + <define name='capapqueue'> + <attribute name='type'> + <value>ap_queue</value> + </attribute> + <element name='ap-adapter'> + <ref name='apAdapterRange'/> + </element> + <element name='ap-domain'> + <ref name='apDomainRange'/> + </element> + </define> + <define name='address'> <element name='address'> <attribute name='domain'><ref name='hexuint'/></attribute> @@ -738,4 +751,16 @@ </choice> </define> + <define name="apDomainRange"> + <choice> + <data type="string"> + <param name="pattern">(0x)?[0-9a-fA-F]{1,4}</param> + </data> + <data type="int"> + <param name="minInclusive">0</param> + <param name="maxInclusive">255</param> + </data> + </choice> + </define> + </grammar> diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c index 5d7a23cb..fa3b823f 100644 --- a/src/conf/node_device_conf.c +++ b/src/conf/node_device_conf.c @@ -68,6 +68,7 @@ VIR_ENUM_IMPL(virNodeDevCap, "css", "vdpa", "ap_card", + "ap_queue", ); VIR_ENUM_IMPL(virNodeDevNetCap, @@ -655,6 +656,12 @@ virNodeDeviceDefFormat(const virNodeDeviceDef *def) virBufferAsprintf(&buf, "<ap-adapter>0x%02x</ap-adapter>\n", data->ap_card.ap_adapter); break; + case VIR_NODE_DEV_CAP_AP_QUEUE: + virBufferAsprintf(&buf, "<ap-adapter>0x%02x</ap-adapter>\n", + data->ap_queue.ap_adapter); + virBufferAsprintf(&buf, "<ap-domain>0x%04x</ap-domain>\n", + data->ap_queue.ap_domain); + break; case VIR_NODE_DEV_CAP_MDEV_TYPES: case VIR_NODE_DEV_CAP_FC_HOST: case VIR_NODE_DEV_CAP_VPORTS: @@ -976,6 +983,52 @@ virNodeDevCapAPCardParseXML(xmlXPathContextPtr ctxt, } +static int +virNodeDevCapAPQueueParseXML(xmlXPathContextPtr ctxt, + virNodeDeviceDefPtr def, + xmlNodePtr node, + virNodeDevCapAPQueuePtr ap_queue) +{ + xmlNodePtr orig; + int ret = -1; + g_autofree char *adapter = NULL, *dom = NULL; + + orig = ctxt->node; + ctxt->node = node; + + if (!(adapter = virXPathString("string(./ap-adapter[1])", ctxt))) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("missing ap-adapter value for '%s'"), def->name); + return -1; + } + + if (virStrToLong_uip(adapter, NULL, 0, &ap_queue->ap_adapter) < 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("invalid ap-adapter value '%s' for '%s'"), + adapter, def->name); + goto out; + } + + if (!(dom = virXPathString("string(./ap-domain[1])", ctxt))) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("missing ap-domain value for '%s'"), def->name); + goto out; + } + + if (virStrToLong_uip(dom, NULL, 0, &ap_queue->ap_domain) < 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("invalid ap-domain value '%s' for '%s'"), + dom, def->name); + goto out; + } + + ret = 0; + out: + ctxt->node = orig; + return ret; +} + + static int virNodeDevCapStorageParseXML(xmlXPathContextPtr ctxt, virNodeDeviceDefPtr def, @@ -2018,6 +2071,10 @@ virNodeDevCapsDefParseXML(xmlXPathContextPtr ctxt, ret = virNodeDevCapAPCardParseXML(ctxt, def, node, &caps->data.ap_card); break; + case VIR_NODE_DEV_CAP_AP_QUEUE: + ret = virNodeDevCapAPQueueParseXML(ctxt, def, node, + &caps->data.ap_queue); + break; case VIR_NODE_DEV_CAP_MDEV_TYPES: case VIR_NODE_DEV_CAP_FC_HOST: case VIR_NODE_DEV_CAP_VPORTS: @@ -2346,6 +2403,7 @@ virNodeDevCapsDefFree(virNodeDevCapsDefPtr caps) case VIR_NODE_DEV_CAP_CCW_DEV: 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_LAST: /* This case is here to shutup the compiler */ break; @@ -2406,6 +2464,7 @@ virNodeDeviceUpdateCaps(virNodeDeviceDefPtr def) case VIR_NODE_DEV_CAP_CCW_DEV: 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_LAST: break; } diff --git a/src/conf/node_device_conf.h b/src/conf/node_device_conf.h index f86f880c..27cb0004 100644 --- a/src/conf/node_device_conf.h +++ b/src/conf/node_device_conf.h @@ -67,6 +67,7 @@ typedef enum { VIR_NODE_DEV_CAP_CSS_DEV, /* s390 channel subsystem device */ VIR_NODE_DEV_CAP_VDPA, /* vDPA device */ VIR_NODE_DEV_CAP_AP_CARD, /* s390 AP Card device */ + VIR_NODE_DEV_CAP_AP_QUEUE, /* s390 AP Queue */ VIR_NODE_DEV_CAP_LAST } virNodeDevCapType; @@ -296,6 +297,13 @@ struct _virNodeDevCapAPCard { unsigned int ap_adapter; }; +typedef struct _virNodeDevCapAPQueue virNodeDevCapAPQueue; +typedef virNodeDevCapAPQueue *virNodeDevCapAPQueuePtr; +struct _virNodeDevCapAPQueue { + unsigned int ap_adapter; + unsigned int ap_domain; +}; + typedef struct _virNodeDevCapData virNodeDevCapData; typedef virNodeDevCapData *virNodeDevCapDataPtr; struct _virNodeDevCapData { @@ -316,6 +324,7 @@ struct _virNodeDevCapData { virNodeDevCapCCW ccw_dev; virNodeDevCapVDPA vdpa; virNodeDevCapAPCard ap_card; + virNodeDevCapAPQueue ap_queue; }; }; diff --git a/src/conf/virnodedeviceobj.c b/src/conf/virnodedeviceobj.c index 0aa59289..8b4302d7 100644 --- a/src/conf/virnodedeviceobj.c +++ b/src/conf/virnodedeviceobj.c @@ -718,6 +718,7 @@ virNodeDeviceObjHasCap(const virNodeDeviceObj *obj, case VIR_NODE_DEV_CAP_CCW_DEV: 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_LAST: break; } diff --git a/src/node_device/node_device_udev.c b/src/node_device/node_device_udev.c index b4eb4553..6bbff571 100644 --- a/src/node_device/node_device_udev.c +++ b/src/node_device/node_device_udev.c @@ -1218,6 +1218,29 @@ udevProcessAPCard(struct udev_device *device, } +static int +udevProcessAPQueue(struct udev_device *device, + virNodeDeviceDefPtr def) +{ + char *c; + virNodeDevCapDataPtr data = &def->caps->data; + + if ((c = strrchr(def->sysfs_path, '/')) == NULL || + virStrToLong_ui(c + 1, &c, 16, &data->ap_queue.ap_adapter) < 0 || + virStrToLong_ui(c + 1, &c, 16, &data->ap_queue.ap_domain) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("failed to parse the AP Queue from sysfs path: '%s'"), + def->sysfs_path); + return -1; + } + + if (udevGenerateDeviceName(device, def, NULL) != 0) + return -1; + + return 0; +} + + static int udevGetDeviceNodes(struct udev_device *device, virNodeDeviceDefPtr def) @@ -1274,6 +1297,8 @@ udevGetDeviceType(struct udev_device *device, *type = VIR_NODE_DEV_CAP_DRM; else if (STREQ(devtype, "ap_card")) *type = VIR_NODE_DEV_CAP_AP_CARD; + else if (STREQ(devtype, "ap_queue")) + *type = VIR_NODE_DEV_CAP_AP_QUEUE; } else { /* PCI devices don't set the DEVTYPE property. */ if (udevHasDeviceProperty(device, "PCI_CLASS")) @@ -1351,6 +1376,8 @@ udevGetDeviceDetails(struct udev_device *device, return udevProcessVDPA(device, def); case VIR_NODE_DEV_CAP_AP_CARD: return udevProcessAPCard(device, def); + case VIR_NODE_DEV_CAP_AP_QUEUE: + return udevProcessAPQueue(device, def); case VIR_NODE_DEV_CAP_MDEV_TYPES: case VIR_NODE_DEV_CAP_SYSTEM: case VIR_NODE_DEV_CAP_FC_HOST: diff --git a/tools/virsh-nodedev.c b/tools/virsh-nodedev.c index 32c12553..81752e85 100644 --- a/tools/virsh-nodedev.c +++ b/tools/virsh-nodedev.c @@ -468,6 +468,7 @@ cmdNodeListDevices(vshControl *ctl, const vshCmd *cmd G_GNUC_UNUSED) flags |= VIR_CONNECT_LIST_NODE_DEVICES_CAP_VDPA; break; case VIR_NODE_DEV_CAP_AP_CARD: + case VIR_NODE_DEV_CAP_AP_QUEUE: case VIR_NODE_DEV_CAP_LAST: break; } -- 2.26.2
On Thu, 12 Nov 2020 13:15:11 +0100 Shalini Chellathurai Saroja <shalini@linux.ibm.com> wrote:
Each AP card device can support upto 256 AP queues. AP queues are also detected by udev, so add support for libvirt nodedev driver.
Signed-off-by: Farhan Ali <alifm@linux.ibm.com> Signed-off-by: Shalini Chellathurai Saroja <shalini@linux.ibm.com> Reviewed-by: Bjoern Walk <bwalk@linux.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com> --- docs/formatnode.html.in | 11 ++++++ docs/schemas/nodedev.rng | 25 +++++++++++++ src/conf/node_device_conf.c | 59 ++++++++++++++++++++++++++++++ src/conf/node_device_conf.h | 9 +++++ src/conf/virnodedeviceobj.c | 1 + src/node_device/node_device_udev.c | 27 ++++++++++++++ tools/virsh-nodedev.c | 1 + 7 files changed, 133 insertions(+)
diff --git a/docs/formatnode.html.in b/docs/formatnode.html.in index d10a79e3..45281363 100644 --- a/docs/formatnode.html.in +++ b/docs/formatnode.html.in @@ -439,6 +439,17 @@ <dd>AP Card identifier.</dd> </dl> </dd> + <dt><code>ap_queue</code></dt> + <dd>Describes the AP Queue on a S390 host. An AP Queue is + identified by it's ap-adapter and ap-domain id.
"it's" should be "its". Is it worth a very brief description of what an AP queue actually is?
Sub-elements include: + <dl> + <dt><code>ap-adapter</code></dt> + <dd>The ap-adapter of an AP Queue identifies AP Card to which + this queue belongs.</dd> + <dt><code>ap-domain</code></dt> + <dd>AP Queue identifier.</dd> + </dl> + </dd> </dl> </dd> </dl> diff --git a/docs/schemas/nodedev.rng b/docs/schemas/nodedev.rng index d02e5377..51cb8854 100644 --- a/docs/schemas/nodedev.rng +++ b/docs/schemas/nodedev.rng @@ -88,6 +88,7 @@ <ref name="capcssdev"/> <ref name="capvdpa"/> <ref name="capapcard"/> + <ref name="capapqueue"/> </choice> </element> </define> @@ -678,6 +679,18 @@ </element> </define>
+ <define name='capapqueue'> + <attribute name='type'> + <value>ap_queue</value> + </attribute> + <element name='ap-adapter'> + <ref name='apAdapterRange'/> + </element> + <element name='ap-domain'> + <ref name='apDomainRange'/> + </element> + </define>
Let's use double quotes to keep the file consistent.
+ <define name='address'> <element name='address'> <attribute name='domain'><ref name='hexuint'/></attribute> @@ -738,4 +751,16 @@ </choice> </define>
+ <define name="apDomainRange"> + <choice> + <data type="string"> + <param name="pattern">(0x)?[0-9a-fA-F]{1,4}</param> + </data> + <data type="int"> + <param name="minInclusive">0</param> + <param name="maxInclusive">255</param>
Is 255 correct here? the hex pattern above implies that it is a 16 bit value, but here you're limiting it to 255. If it is a 16 bit value, can we just use the already-defined 'uint16' type from basictypes.rng?
+ </data> + </choice> + </define> + </grammar> diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c index 5d7a23cb..fa3b823f 100644 --- a/src/conf/node_device_conf.c +++ b/src/conf/node_device_conf.c @@ -68,6 +68,7 @@ VIR_ENUM_IMPL(virNodeDevCap, "css", "vdpa", "ap_card", + "ap_queue", );
VIR_ENUM_IMPL(virNodeDevNetCap, @@ -655,6 +656,12 @@ virNodeDeviceDefFormat(const virNodeDeviceDef *def) virBufferAsprintf(&buf, "<ap-adapter>0x%02x</ap-adapter>\n", data->ap_card.ap_adapter); break; + case VIR_NODE_DEV_CAP_AP_QUEUE: + virBufferAsprintf(&buf, "<ap-adapter>0x%02x</ap-adapter>\n", + data->ap_queue.ap_adapter); + virBufferAsprintf(&buf, "<ap-domain>0x%04x</ap-domain>\n", + data->ap_queue.ap_domain); + break; case VIR_NODE_DEV_CAP_MDEV_TYPES: case VIR_NODE_DEV_CAP_FC_HOST: case VIR_NODE_DEV_CAP_VPORTS: @@ -976,6 +983,52 @@ virNodeDevCapAPCardParseXML(xmlXPathContextPtr ctxt, }
+static int +virNodeDevCapAPQueueParseXML(xmlXPathContextPtr ctxt, + virNodeDeviceDefPtr def, + xmlNodePtr node, + virNodeDevCapAPQueuePtr ap_queue) +{ + xmlNodePtr orig; + int ret = -1; + g_autofree char *adapter = NULL, *dom = NULL; + + orig = ctxt->node; + ctxt->node = node; + + if (!(adapter = virXPathString("string(./ap-adapter[1])", ctxt))) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("missing ap-adapter value for '%s'"), def->name); + return -1; + } + + if (virStrToLong_uip(adapter, NULL, 0, &ap_queue->ap_adapter) < 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("invalid ap-adapter value '%s' for '%s'"), + adapter, def->name); + goto out; + } + + if (!(dom = virXPathString("string(./ap-domain[1])", ctxt))) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("missing ap-domain value for '%s'"), def->name); + goto out; + } + + if (virStrToLong_uip(dom, NULL, 0, &ap_queue->ap_domain) < 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("invalid ap-domain value '%s' for '%s'"), + dom, def->name); + goto out; + } + + ret = 0; + out: + ctxt->node = orig; + return ret; +} + + static int virNodeDevCapStorageParseXML(xmlXPathContextPtr ctxt, virNodeDeviceDefPtr def, @@ -2018,6 +2071,10 @@ virNodeDevCapsDefParseXML(xmlXPathContextPtr ctxt, ret = virNodeDevCapAPCardParseXML(ctxt, def, node, &caps->data.ap_card); break; + case VIR_NODE_DEV_CAP_AP_QUEUE: + ret = virNodeDevCapAPQueueParseXML(ctxt, def, node, + &caps->data.ap_queue); + break; case VIR_NODE_DEV_CAP_MDEV_TYPES: case VIR_NODE_DEV_CAP_FC_HOST: case VIR_NODE_DEV_CAP_VPORTS: @@ -2346,6 +2403,7 @@ virNodeDevCapsDefFree(virNodeDevCapsDefPtr caps) case VIR_NODE_DEV_CAP_CCW_DEV: 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_LAST: /* This case is here to shutup the compiler */ break; @@ -2406,6 +2464,7 @@ virNodeDeviceUpdateCaps(virNodeDeviceDefPtr def) case VIR_NODE_DEV_CAP_CCW_DEV: 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_LAST: break; } diff --git a/src/conf/node_device_conf.h b/src/conf/node_device_conf.h index f86f880c..27cb0004 100644 --- a/src/conf/node_device_conf.h +++ b/src/conf/node_device_conf.h @@ -67,6 +67,7 @@ typedef enum { VIR_NODE_DEV_CAP_CSS_DEV, /* s390 channel subsystem device */ VIR_NODE_DEV_CAP_VDPA, /* vDPA device */ VIR_NODE_DEV_CAP_AP_CARD, /* s390 AP Card device */ + VIR_NODE_DEV_CAP_AP_QUEUE, /* s390 AP Queue */
VIR_NODE_DEV_CAP_LAST } virNodeDevCapType; @@ -296,6 +297,13 @@ struct _virNodeDevCapAPCard { unsigned int ap_adapter; };
+typedef struct _virNodeDevCapAPQueue virNodeDevCapAPQueue; +typedef virNodeDevCapAPQueue *virNodeDevCapAPQueuePtr; +struct _virNodeDevCapAPQueue { + unsigned int ap_adapter; + unsigned int ap_domain; +}; + typedef struct _virNodeDevCapData virNodeDevCapData; typedef virNodeDevCapData *virNodeDevCapDataPtr; struct _virNodeDevCapData { @@ -316,6 +324,7 @@ struct _virNodeDevCapData { virNodeDevCapCCW ccw_dev; virNodeDevCapVDPA vdpa; virNodeDevCapAPCard ap_card; + virNodeDevCapAPQueue ap_queue; }; };
diff --git a/src/conf/virnodedeviceobj.c b/src/conf/virnodedeviceobj.c index 0aa59289..8b4302d7 100644 --- a/src/conf/virnodedeviceobj.c +++ b/src/conf/virnodedeviceobj.c @@ -718,6 +718,7 @@ virNodeDeviceObjHasCap(const virNodeDeviceObj *obj, case VIR_NODE_DEV_CAP_CCW_DEV: 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_LAST: break; } diff --git a/src/node_device/node_device_udev.c b/src/node_device/node_device_udev.c index b4eb4553..6bbff571 100644 --- a/src/node_device/node_device_udev.c +++ b/src/node_device/node_device_udev.c @@ -1218,6 +1218,29 @@ udevProcessAPCard(struct udev_device *device, }
+static int +udevProcessAPQueue(struct udev_device *device, + virNodeDeviceDefPtr def) +{ + char *c; + virNodeDevCapDataPtr data = &def->caps->data; +
In the previous patch, you added a comment explaining the format of the sysfs path. I found that helpful. Without knowing the format, it's a bit difficult to judge whether this code below is correct.
+ if ((c = strrchr(def->sysfs_path, '/')) == NULL || + virStrToLong_ui(c + 1, &c, 16, &data->ap_queue.ap_adapter) < 0 || + virStrToLong_ui(c + 1, &c, 16, &data->ap_queue.ap_domain) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("failed to parse the AP Queue from sysfs path: '%s'"), + def->sysfs_path); + return -1; + } + + if (udevGenerateDeviceName(device, def, NULL) != 0) + return -1; + + return 0; +} + + static int udevGetDeviceNodes(struct udev_device *device, virNodeDeviceDefPtr def) @@ -1274,6 +1297,8 @@ udevGetDeviceType(struct udev_device *device, *type = VIR_NODE_DEV_CAP_DRM; else if (STREQ(devtype, "ap_card")) *type = VIR_NODE_DEV_CAP_AP_CARD; + else if (STREQ(devtype, "ap_queue")) + *type = VIR_NODE_DEV_CAP_AP_QUEUE; } else { /* PCI devices don't set the DEVTYPE property. */ if (udevHasDeviceProperty(device, "PCI_CLASS")) @@ -1351,6 +1376,8 @@ udevGetDeviceDetails(struct udev_device *device, return udevProcessVDPA(device, def); case VIR_NODE_DEV_CAP_AP_CARD: return udevProcessAPCard(device, def); + case VIR_NODE_DEV_CAP_AP_QUEUE: + return udevProcessAPQueue(device, def); case VIR_NODE_DEV_CAP_MDEV_TYPES: case VIR_NODE_DEV_CAP_SYSTEM: case VIR_NODE_DEV_CAP_FC_HOST: diff --git a/tools/virsh-nodedev.c b/tools/virsh-nodedev.c index 32c12553..81752e85 100644 --- a/tools/virsh-nodedev.c +++ b/tools/virsh-nodedev.c @@ -468,6 +468,7 @@ cmdNodeListDevices(vshControl *ctl, const vshCmd *cmd G_GNUC_UNUSED) flags |= VIR_CONNECT_LIST_NODE_DEVICES_CAP_VDPA; break; case VIR_NODE_DEV_CAP_AP_CARD: + case VIR_NODE_DEV_CAP_AP_QUEUE: case VIR_NODE_DEV_CAP_LAST: break; }
On 11/12/20 6:01 PM, Jonathon Jongsma wrote:
diff --git a/docs/formatnode.html.in b/docs/formatnode.html.in index d10a79e3..45281363 100644 --- a/docs/formatnode.html.in +++ b/docs/formatnode.html.in @@ -439,6 +439,17 @@ <dd>AP Card identifier.</dd> </dl> </dd> + <dt><code>ap_queue</code></dt> + <dd>Describes the AP Queue on a S390 host. An AP Queue is + identified by it's ap-adapter and ap-domain id. "it's" should be "its". Is it worth a very brief description of what an AP queue actually is?
Sure, I will provide a brief description on AP queue. ...
+ <define name='capapqueue'> + <attribute name='type'> + <value>ap_queue</value> + </attribute> + <element name='ap-adapter'> + <ref name='apAdapterRange'/> + </element> + <element name='ap-domain'> + <ref name='apDomainRange'/> + </element> + </define>
Let's use double quotes to keep the file consistent.
Sure:-)
+ <define name='address'> <element name='address'> <attribute name='domain'><ref name='hexuint'/></attribute> @@ -738,4 +751,16 @@ </choice> </define>
+ <define name="apDomainRange"> + <choice> + <data type="string"> + <param name="pattern">(0x)?[0-9a-fA-F]{1,4}</param> + </data> + <data type="int"> + <param name="minInclusive">0</param> + <param name="maxInclusive">255</param> Is 255 correct here? the hex pattern above implies that it is a 16 bit value, but here you're limiting it to 255. If it is a 16 bit value, can we just use the already-defined 'uint16' type from basictypes.rng?
Yes, it is. Current architecture limit is 256 (0-255) for domains. The address schema of domains was created in linux on z, with a future architectural change in mind. ...
diff --git a/src/node_device/node_device_udev.c b/src/node_device/node_device_udev.c index b4eb4553..6bbff571 100644 --- a/src/node_device/node_device_udev.c +++ b/src/node_device/node_device_udev.c @@ -1218,6 +1218,29 @@ udevProcessAPCard(struct udev_device *device, }
+static int +udevProcessAPQueue(struct udev_device *device, + virNodeDeviceDefPtr def) +{ + char *c; + virNodeDevCapDataPtr data = &def->caps->data; + In the previous patch, you added a comment explaining the format of the sysfs path. I found that helpful. Without knowing the format, it's a bit difficult to judge whether this code below is correct.
I will add the below comment /* The sysfs path would be in the format /sys/bus/ap/devices/xx.yyyy, where xx is ap adapter id and yyyy is ap domain id. eg:/sys/bus/ap/devices/08.0001 */ -- Kind regards Shalini Chellathurai Saroja Vorsitzende des Aufsichtsrats: Gregor Pillen Geschäftsführung: Dirk Wittkopp Sitz der Gesellschaft: Böblingen Registergericht: Amtsgericht Stuttgart, HRB 243294
On Fri, 13 Nov 2020 15:19:11 +0100 Shalini Chellathurai Saroja <shalini@linux.ibm.com> wrote:
On 11/12/20 6:01 PM, Jonathon Jongsma wrote:
diff --git a/docs/formatnode.html.in b/docs/formatnode.html.in index d10a79e3..45281363 100644 --- a/docs/formatnode.html.in +++ b/docs/formatnode.html.in @@ -439,6 +439,17 @@ <dd>AP Card identifier.</dd> </dl> </dd> + <dt><code>ap_queue</code></dt> + <dd>Describes the AP Queue on a S390 host. An AP Queue is + identified by it's ap-adapter and ap-domain id. "it's" should be "its". Is it worth a very brief description of what an AP queue actually is?
Sure, I will provide a brief description on AP queue.
...
+ <define name='capapqueue'> + <attribute name='type'> + <value>ap_queue</value> + </attribute> + <element name='ap-adapter'> + <ref name='apAdapterRange'/> + </element> + <element name='ap-domain'> + <ref name='apDomainRange'/> + </element> + </define>
Let's use double quotes to keep the file consistent.
Sure:-)
+ <define name='address'> <element name='address'> <attribute name='domain'><ref name='hexuint'/></attribute> @@ -738,4 +751,16 @@ </choice> </define>
+ <define name="apDomainRange"> + <choice> + <data type="string"> + <param name="pattern">(0x)?[0-9a-fA-F]{1,4}</param> + </data> + <data type="int"> + <param name="minInclusive">0</param> + <param name="maxInclusive">255</param> Is 255 correct here? the hex pattern above implies that it is a 16 bit value, but here you're limiting it to 255. If it is a 16 bit value, can we just use the already-defined 'uint16' type from basictypes.rng?
Yes, it is. Current architecture limit is 256 (0-255) for domains.
The address schema of domains was created in linux on z, with a future architectural change in mind.
...
So, this is a bit confusing to me. The xml schema appears to be trying to prevent me from specifying a value above 255 if I enter it as an integer, yet it allows me to specify a value of 0xFFFF if I specify it as a hex value. Side note: I can also enter plain integers beyond 255 because of the fact that the '0x' prefix is optional. 9999 will validate according to the schema because it matches the hex string pattern. However I note that your parsing code does not enforce this 255 limit anywhere.
diff --git a/src/node_device/node_device_udev.c b/src/node_device/node_device_udev.c index b4eb4553..6bbff571 100644 --- a/src/node_device/node_device_udev.c +++ b/src/node_device/node_device_udev.c @@ -1218,6 +1218,29 @@ udevProcessAPCard(struct udev_device *device, }
+static int +udevProcessAPQueue(struct udev_device *device, + virNodeDeviceDefPtr def) +{ + char *c; + virNodeDevCapDataPtr data = &def->caps->data; + In the previous patch, you added a comment explaining the format of the sysfs path. I found that helpful. Without knowing the format, it's a bit difficult to judge whether this code below is correct.
I will add the below comment
/* The sysfs path would be in the format /sys/bus/ap/devices/xx.yyyy, where xx is ap adapter id and yyyy is ap domain id. eg:/sys/bus/ap/devices/08.0001 */
On 11/13/20 4:50 PM, Jonathon Jongsma wrote:
On Fri, 13 Nov 2020 15:19:11 +0100 Shalini Chellathurai Saroja <shalini@linux.ibm.com> wrote:
On 11/12/20 6:01 PM, Jonathon Jongsma wrote:
diff --git a/docs/formatnode.html.in b/docs/formatnode.html.in index d10a79e3..45281363 100644 --- a/docs/formatnode.html.in +++ b/docs/formatnode.html.in @@ -439,6 +439,17 @@ <dd>AP Card identifier.</dd> </dl> </dd> + <dt><code>ap_queue</code></dt> + <dd>Describes the AP Queue on a S390 host. An AP Queue is + identified by it's ap-adapter and ap-domain id. "it's" should be "its". Is it worth a very brief description of what an AP queue actually is? Sure, I will provide a brief description on AP queue.
...
+ <define name='capapqueue'> + <attribute name='type'> + <value>ap_queue</value> + </attribute> + <element name='ap-adapter'> + <ref name='apAdapterRange'/> + </element> + <element name='ap-domain'> + <ref name='apDomainRange'/> + </element> + </define>
Let's use double quotes to keep the file consistent.
Sure:-)
+ <define name='address'> <element name='address'> <attribute name='domain'><ref name='hexuint'/></attribute> @@ -738,4 +751,16 @@ </choice> </define>
+ <define name="apDomainRange"> + <choice> + <data type="string"> + <param name="pattern">(0x)?[0-9a-fA-F]{1,4}</param> + </data> + <data type="int"> + <param name="minInclusive">0</param> + <param name="maxInclusive">255</param> Is 255 correct here? the hex pattern above implies that it is a 16 bit value, but here you're limiting it to 255. If it is a 16 bit value, can we just use the already-defined 'uint16' type from basictypes.rng?
Yes, it is. Current architecture limit is 256 (0-255) for domains.
The address schema of domains was created in linux on z, with a future architectural change in mind.
...
So, this is a bit confusing to me. The xml schema appears to be trying to prevent me from specifying a value above 255 if I enter it as an integer, yet it allows me to specify a value of 0xFFFF if I specify it as a hex value.
Side note: I can also enter plain integers beyond 255 because of the fact that the '0x' prefix is optional. 9999 will validate according to the schema because it matches the hex string pattern.
However I note that your parsing code does not enforce this 255 limit anywhere.
I will enforce the maximum limit of domain value as 255 in the parsing code. I will modify the apDomainRange definition to have the prefix '0x' as required instead of optional. Thank you for pointing it out:-)
diff --git a/src/node_device/node_device_udev.c b/src/node_device/node_device_udev.c index b4eb4553..6bbff571 100644 --- a/src/node_device/node_device_udev.c +++ b/src/node_device/node_device_udev.c @@ -1218,6 +1218,29 @@ udevProcessAPCard(struct udev_device *device, }
+static int +udevProcessAPQueue(struct udev_device *device, + virNodeDeviceDefPtr def) +{ + char *c; + virNodeDevCapDataPtr data = &def->caps->data; + In the previous patch, you added a comment explaining the format of the sysfs path. I found that helpful. Without knowing the format, it's a bit difficult to judge whether this code below is correct.
I will add the below comment
/* The sysfs path would be in the format /sys/bus/ap/devices/xx.yyyy, where xx is ap adapter id and yyyy is ap domain id. eg:/sys/bus/ap/devices/08.0001 */
-- Kind regards Shalini Chellathurai Saroja Linux on Z and Virtualization Development Vorsitzende des Aufsichtsrats: Gregor Pillen Geschäftsführung: Dirk Wittkopp Sitz der Gesellschaft: Böblingen Registergericht: Amtsgericht Stuttgart, HRB 243294
Add tests to verify libvirt node device driver support for AP queues Signed-off-by: Farhan Ali <alifm@linux.ibm.com> Signed-off-by: Shalini Chellathurai Saroja <shalini@linux.ibm.com> Reviewed-by: Bjoern Walk <bwalk@linux.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com> --- tests/nodedevschemadata/ap_07_0038.xml | 9 +++++++++ tests/nodedevxml2xmltest.c | 1 + 2 files changed, 10 insertions(+) create mode 100644 tests/nodedevschemadata/ap_07_0038.xml diff --git a/tests/nodedevschemadata/ap_07_0038.xml b/tests/nodedevschemadata/ap_07_0038.xml new file mode 100644 index 00000000..553c68f2 --- /dev/null +++ b/tests/nodedevschemadata/ap_07_0038.xml @@ -0,0 +1,9 @@ +<device> + <name>ap_07_0038</name> + <path>/sys/devices/ap/card07/07.0038</path> + <parent>ap_card07</parent> + <capability type='ap_queue'> + <ap-adapter>0x07</ap-adapter> + <ap-domain>0x0038</ap-domain> + </capability> +</device> diff --git a/tests/nodedevxml2xmltest.c b/tests/nodedevxml2xmltest.c index 7b8a5a82..a8cbe6a9 100644 --- a/tests/nodedevxml2xmltest.c +++ b/tests/nodedevxml2xmltest.c @@ -126,6 +126,7 @@ mymain(void) DO_TEST("css_0_0_ffff"); DO_TEST("css_0_0_fffe_mdev_types"); DO_TEST("ap_card07"); + DO_TEST("ap_07_0038"); return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE; } -- 2.26.2
From: Farhan Ali <alifm@linux.ibm.com> Add support to filter by 'ap_card' and 'ap_queue' capabilities. Signed-off-by: Farhan Ali <alifm@linux.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com> Reviewed-by: Bjoern Walk <bwalk@linux.ibm.com> Signed-off-by: Shalini Chellathurai Saroja <shalini@linux.ibm.com> --- docs/manpages/virsh.rst | 2 +- include/libvirt/libvirt-nodedev.h | 2 ++ src/conf/node_device_conf.h | 4 +++- src/conf/virnodedeviceobj.c | 4 +++- src/libvirt-nodedev.c | 2 ++ tools/virsh-nodedev.c | 4 ++++ 6 files changed, 15 insertions(+), 3 deletions(-) diff --git a/docs/manpages/virsh.rst b/docs/manpages/virsh.rst index bfd26e31..1e71818a 100644 --- a/docs/manpages/virsh.rst +++ b/docs/manpages/virsh.rst @@ -4972,7 +4972,7 @@ List all of the devices available on the node that are known by libvirt. separated by comma, e.g. --cap pci,scsi. Valid capability types include 'system', 'pci', 'usb_device', 'usb', 'net', 'scsi_host', 'scsi_target', 'scsi', 'storage', 'fc_host', 'vports', 'scsi_generic', 'drm', 'mdev', -'mdev_types', 'ccw', 'css'. +'mdev_types', 'ccw', 'css', 'ap_card', 'ap_queue'. If *--tree* is used, the output is formatted in a tree representing parents of each node. *cap* and *--tree* are mutually exclusive. diff --git a/include/libvirt/libvirt-nodedev.h b/include/libvirt/libvirt-nodedev.h index b73b076f..d5091aa2 100644 --- a/include/libvirt/libvirt-nodedev.h +++ b/include/libvirt/libvirt-nodedev.h @@ -83,6 +83,8 @@ typedef enum { VIR_CONNECT_LIST_NODE_DEVICES_CAP_CCW_DEV = 1 << 15, /* CCW device */ VIR_CONNECT_LIST_NODE_DEVICES_CAP_CSS_DEV = 1 << 16, /* CSS device */ VIR_CONNECT_LIST_NODE_DEVICES_CAP_VDPA = 1 << 17, /* vDPA device */ + 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 */ } virConnectListAllNodeDeviceFlags; int virConnectListAllNodeDevices (virConnectPtr conn, diff --git a/src/conf/node_device_conf.h b/src/conf/node_device_conf.h index 27cb0004..b580d6cf 100644 --- a/src/conf/node_device_conf.h +++ b/src/conf/node_device_conf.h @@ -402,7 +402,9 @@ virNodeDevCapsDefFree(virNodeDevCapsDefPtr caps); VIR_CONNECT_LIST_NODE_DEVICES_CAP_MDEV | \ VIR_CONNECT_LIST_NODE_DEVICES_CAP_CCW_DEV | \ VIR_CONNECT_LIST_NODE_DEVICES_CAP_CSS_DEV | \ - VIR_CONNECT_LIST_NODE_DEVICES_CAP_VDPA) + VIR_CONNECT_LIST_NODE_DEVICES_CAP_VDPA | \ + VIR_CONNECT_LIST_NODE_DEVICES_CAP_AP_CARD | \ + VIR_CONNECT_LIST_NODE_DEVICES_CAP_AP_QUEUE) int virNodeDeviceGetSCSIHostCaps(virNodeDevCapSCSIHostPtr scsi_host); diff --git a/src/conf/virnodedeviceobj.c b/src/conf/virnodedeviceobj.c index 8b4302d7..691632c6 100644 --- a/src/conf/virnodedeviceobj.c +++ b/src/conf/virnodedeviceobj.c @@ -871,7 +871,9 @@ virNodeDeviceObjMatch(virNodeDeviceObjPtr obj, MATCH(MDEV) || MATCH(CCW_DEV) || MATCH(CSS_DEV) || - MATCH(VDPA))) + MATCH(VDPA) || + MATCH(AP_CARD) || + MATCH(AP_QUEUE))) return false; } diff --git a/src/libvirt-nodedev.c b/src/libvirt-nodedev.c index 28765b9c..762413eb 100644 --- a/src/libvirt-nodedev.c +++ b/src/libvirt-nodedev.c @@ -102,6 +102,8 @@ virNodeNumOfDevices(virConnectPtr conn, const char *cap, unsigned int flags) * VIR_CONNECT_LIST_NODE_DEVICES_CAP_MDEV * VIR_CONNECT_LIST_NODE_DEVICES_CAP_CCW_DEV * VIR_CONNECT_LIST_NODE_DEVICES_CAP_CSS_DEV + * VIR_CONNECT_LIST_NODE_DEVICES_CAP_AP_CARD + * VIR_CONNECT_LIST_NODE_DEVICES_CAP_AP_QUEUE * * Returns the number of node devices found or -1 and sets @devices to NULL in * case of error. On success, the array stored into @devices is guaranteed to diff --git a/tools/virsh-nodedev.c b/tools/virsh-nodedev.c index 81752e85..f5ffe525 100644 --- a/tools/virsh-nodedev.c +++ b/tools/virsh-nodedev.c @@ -468,7 +468,11 @@ cmdNodeListDevices(vshControl *ctl, const vshCmd *cmd G_GNUC_UNUSED) flags |= VIR_CONNECT_LIST_NODE_DEVICES_CAP_VDPA; break; case VIR_NODE_DEV_CAP_AP_CARD: + flags |= VIR_CONNECT_LIST_NODE_DEVICES_CAP_AP_CARD; + break; case VIR_NODE_DEV_CAP_AP_QUEUE: + flags |= VIR_CONNECT_LIST_NODE_DEVICES_CAP_AP_QUEUE; + break; case VIR_NODE_DEV_CAP_LAST: break; } -- 2.26.2
Add support for AP matrix device in libvirt node device driver. Signed-off-by: Shalini Chellathurai Saroja <shalini@linux.ibm.com> Reviewed-by: Bjoern Walk <bwalk@linux.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com> --- docs/schemas/nodedev.rng | 7 +++++++ src/conf/node_device_conf.c | 11 ++++++++++- src/conf/node_device_conf.h | 8 ++++++++ src/conf/virnodedeviceobj.c | 1 + src/node_device/node_device_udev.c | 23 +++++++++++++++++++++++ tools/virsh-nodedev.c | 1 + 6 files changed, 50 insertions(+), 1 deletion(-) diff --git a/docs/schemas/nodedev.rng b/docs/schemas/nodedev.rng index 51cb8854..5be21145 100644 --- a/docs/schemas/nodedev.rng +++ b/docs/schemas/nodedev.rng @@ -89,6 +89,7 @@ <ref name="capvdpa"/> <ref name="capapcard"/> <ref name="capapqueue"/> + <ref name="capapmatrix"/> </choice> </element> </define> @@ -691,6 +692,12 @@ </element> </define> + <define name='capapmatrix'> + <attribute name='type'> + <value>ap_matrix</value> + </attribute> + </define> + <define name='address'> <element name='address'> <attribute name='domain'><ref name='hexuint'/></attribute> diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c index fa3b823f..f3d146a5 100644 --- a/src/conf/node_device_conf.c +++ b/src/conf/node_device_conf.c @@ -69,6 +69,7 @@ VIR_ENUM_IMPL(virNodeDevCap, "vdpa", "ap_card", "ap_queue", + "ap_matrix", ); VIR_ENUM_IMPL(virNodeDevNetCap, @@ -665,6 +666,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_AP_MATRIX: case VIR_NODE_DEV_CAP_LAST: break; } @@ -2075,6 +2077,9 @@ virNodeDevCapsDefParseXML(xmlXPathContextPtr ctxt, ret = virNodeDevCapAPQueueParseXML(ctxt, def, node, &caps->data.ap_queue); break; + case VIR_NODE_DEV_CAP_AP_MATRIX: + ret = 0; + break; case VIR_NODE_DEV_CAP_MDEV_TYPES: case VIR_NODE_DEV_CAP_FC_HOST: case VIR_NODE_DEV_CAP_VPORTS: @@ -2396,6 +2401,9 @@ virNodeDevCapsDefFree(virNodeDevCapsDefPtr caps) virMediatedDeviceTypeFree(data->ccw_dev.mdev_types[i]); VIR_FREE(data->ccw_dev.mdev_types); break; + case VIR_NODE_DEV_CAP_AP_MATRIX: + VIR_FREE(data->ap_matrix.addr); + break; case VIR_NODE_DEV_CAP_MDEV_TYPES: case VIR_NODE_DEV_CAP_DRM: case VIR_NODE_DEV_CAP_FC_HOST: @@ -2465,6 +2473,7 @@ virNodeDeviceUpdateCaps(virNodeDeviceDefPtr 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_AP_MATRIX: case VIR_NODE_DEV_CAP_LAST: break; } @@ -2806,6 +2815,7 @@ virNodeDeviceGetPCIDynamicCaps(const char *sysfsPath, &pci_dev->mdev_types, &pci_dev->nmdev_types) < 0) return -1; + if (pci_dev->nmdev_types > 0) pci_dev->flags |= VIR_NODE_DEV_CAP_FLAG_PCI_MDEV; @@ -2849,7 +2859,6 @@ virNodeDeviceGetPCIDynamicCaps(const char *sysfsPath G_GNUC_UNUSED, return -1; } - int virNodeDeviceGetSCSITargetCaps(const char *sysfsPath G_GNUC_UNUSED, virNodeDevCapSCSITargetPtr scsi_target G_GNUC_UNUSED) { diff --git a/src/conf/node_device_conf.h b/src/conf/node_device_conf.h index b580d6cf..b863653b 100644 --- a/src/conf/node_device_conf.h +++ b/src/conf/node_device_conf.h @@ -68,6 +68,7 @@ typedef enum { VIR_NODE_DEV_CAP_VDPA, /* vDPA device */ 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_LAST } virNodeDevCapType; @@ -304,6 +305,12 @@ struct _virNodeDevCapAPQueue { unsigned int ap_domain; }; +typedef struct _virNodeDevCapAPMatrix virNodeDevCapAPMatrix; +typedef virNodeDevCapAPMatrix *virNodeDevCapAPMatrixPtr; +struct _virNodeDevCapAPMatrix { + char *addr; +}; + typedef struct _virNodeDevCapData virNodeDevCapData; typedef virNodeDevCapData *virNodeDevCapDataPtr; struct _virNodeDevCapData { @@ -325,6 +332,7 @@ struct _virNodeDevCapData { virNodeDevCapVDPA vdpa; virNodeDevCapAPCard ap_card; virNodeDevCapAPQueue ap_queue; + virNodeDevCapAPMatrix ap_matrix; }; }; diff --git a/src/conf/virnodedeviceobj.c b/src/conf/virnodedeviceobj.c index 691632c6..8fbef5f5 100644 --- a/src/conf/virnodedeviceobj.c +++ b/src/conf/virnodedeviceobj.c @@ -719,6 +719,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_AP_MATRIX: 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 6bbff571..5f57000e 100644 --- a/src/node_device/node_device_udev.c +++ b/src/node_device/node_device_udev.c @@ -1241,6 +1241,25 @@ udevProcessAPQueue(struct udev_device *device, } +static int +udevProcessAPMatrix(struct udev_device *device, + virNodeDeviceDefPtr def) +{ + size_t i; + virNodeDevCapDataPtr data = &def->caps->data; + + data->ap_matrix.addr = g_strdup(udev_device_get_sysname(device)); + def->name = g_strdup_printf("ap_%s", data->ap_matrix.addr); + + for (i = 0; i < strlen(def->name); i++) { + if (!(g_ascii_isalnum(*(def->name + i)))) + *(def->name + i) = '_'; + } + + return 0; +} + + static int udevGetDeviceNodes(struct udev_device *device, virNodeDeviceDefPtr def) @@ -1326,6 +1345,8 @@ udevGetDeviceType(struct udev_device *device, *type = VIR_NODE_DEV_CAP_CSS_DEV; else if (STREQ_NULLABLE(subsystem, "vdpa")) *type = VIR_NODE_DEV_CAP_VDPA; + else if (STREQ_NULLABLE(subsystem, "matrix")) + *type = VIR_NODE_DEV_CAP_AP_MATRIX; VIR_FREE(subsystem); } @@ -1378,6 +1399,8 @@ udevGetDeviceDetails(struct udev_device *device, return udevProcessAPCard(device, def); case VIR_NODE_DEV_CAP_AP_QUEUE: return udevProcessAPQueue(device, def); + case VIR_NODE_DEV_CAP_AP_MATRIX: + return udevProcessAPMatrix(device, def); case VIR_NODE_DEV_CAP_MDEV_TYPES: case VIR_NODE_DEV_CAP_SYSTEM: case VIR_NODE_DEV_CAP_FC_HOST: diff --git a/tools/virsh-nodedev.c b/tools/virsh-nodedev.c index f5ffe525..f69030d2 100644 --- a/tools/virsh-nodedev.c +++ b/tools/virsh-nodedev.c @@ -473,6 +473,7 @@ cmdNodeListDevices(vshControl *ctl, const vshCmd *cmd G_GNUC_UNUSED) case VIR_NODE_DEV_CAP_AP_QUEUE: flags |= VIR_CONNECT_LIST_NODE_DEVICES_CAP_AP_QUEUE; break; + case VIR_NODE_DEV_CAP_AP_MATRIX: case VIR_NODE_DEV_CAP_LAST: break; } -- 2.26.2
On Thu, 12 Nov 2020 13:15:14 +0100 Shalini Chellathurai Saroja <shalini@linux.ibm.com> wrote:
Add support for AP matrix device in libvirt node device driver.
Signed-off-by: Shalini Chellathurai Saroja <shalini@linux.ibm.com> Reviewed-by: Bjoern Walk <bwalk@linux.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com> --- docs/schemas/nodedev.rng | 7 +++++++ src/conf/node_device_conf.c | 11 ++++++++++- src/conf/node_device_conf.h | 8 ++++++++ src/conf/virnodedeviceobj.c | 1 + src/node_device/node_device_udev.c | 23 +++++++++++++++++++++++ tools/virsh-nodedev.c | 1 + 6 files changed, 50 insertions(+), 1 deletion(-)
diff --git a/docs/schemas/nodedev.rng b/docs/schemas/nodedev.rng index 51cb8854..5be21145 100644 --- a/docs/schemas/nodedev.rng +++ b/docs/schemas/nodedev.rng @@ -89,6 +89,7 @@ <ref name="capvdpa"/> <ref name="capapcard"/> <ref name="capapqueue"/> + <ref name="capapmatrix"/> </choice> </element> </define> @@ -691,6 +692,12 @@ </element> </define>
+ <define name='capapmatrix'> + <attribute name='type'> + <value>ap_matrix</value> + </attribute> + </define> + <define name='address'> <element name='address'> <attribute name='domain'><ref name='hexuint'/></attribute> diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c index fa3b823f..f3d146a5 100644 --- a/src/conf/node_device_conf.c +++ b/src/conf/node_device_conf.c @@ -69,6 +69,7 @@ VIR_ENUM_IMPL(virNodeDevCap, "vdpa", "ap_card", "ap_queue", + "ap_matrix", );
VIR_ENUM_IMPL(virNodeDevNetCap, @@ -665,6 +666,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_AP_MATRIX: case VIR_NODE_DEV_CAP_LAST: break; } @@ -2075,6 +2077,9 @@ virNodeDevCapsDefParseXML(xmlXPathContextPtr ctxt, ret = virNodeDevCapAPQueueParseXML(ctxt, def, node, &caps->data.ap_queue); break; + case VIR_NODE_DEV_CAP_AP_MATRIX: + ret = 0; + break; case VIR_NODE_DEV_CAP_MDEV_TYPES: case VIR_NODE_DEV_CAP_FC_HOST: case VIR_NODE_DEV_CAP_VPORTS: @@ -2396,6 +2401,9 @@ virNodeDevCapsDefFree(virNodeDevCapsDefPtr caps) virMediatedDeviceTypeFree(data->ccw_dev.mdev_types[i]); VIR_FREE(data->ccw_dev.mdev_types); break; + case VIR_NODE_DEV_CAP_AP_MATRIX: + VIR_FREE(data->ap_matrix.addr); + break; case VIR_NODE_DEV_CAP_MDEV_TYPES: case VIR_NODE_DEV_CAP_DRM: case VIR_NODE_DEV_CAP_FC_HOST: @@ -2465,6 +2473,7 @@ virNodeDeviceUpdateCaps(virNodeDeviceDefPtr 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_AP_MATRIX: case VIR_NODE_DEV_CAP_LAST: break; } @@ -2806,6 +2815,7 @@ virNodeDeviceGetPCIDynamicCaps(const char *sysfsPath, &pci_dev->mdev_types, &pci_dev->nmdev_types) < 0) return -1; +
unrelated change
if (pci_dev->nmdev_types > 0) pci_dev->flags |= VIR_NODE_DEV_CAP_FLAG_PCI_MDEV;
@@ -2849,7 +2859,6 @@ virNodeDeviceGetPCIDynamicCaps(const char *sysfsPath G_GNUC_UNUSED, return -1; }
-
unrelated change
int virNodeDeviceGetSCSITargetCaps(const char *sysfsPath G_GNUC_UNUSED, virNodeDevCapSCSITargetPtr scsi_target G_GNUC_UNUSED) { diff --git a/src/conf/node_device_conf.h b/src/conf/node_device_conf.h index b580d6cf..b863653b 100644 --- a/src/conf/node_device_conf.h +++ b/src/conf/node_device_conf.h @@ -68,6 +68,7 @@ typedef enum { VIR_NODE_DEV_CAP_VDPA, /* vDPA device */ 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_LAST } virNodeDevCapType; @@ -304,6 +305,12 @@ struct _virNodeDevCapAPQueue { unsigned int ap_domain; };
+typedef struct _virNodeDevCapAPMatrix virNodeDevCapAPMatrix; +typedef virNodeDevCapAPMatrix *virNodeDevCapAPMatrixPtr; +struct _virNodeDevCapAPMatrix { + char *addr; +}; + typedef struct _virNodeDevCapData virNodeDevCapData; typedef virNodeDevCapData *virNodeDevCapDataPtr; struct _virNodeDevCapData { @@ -325,6 +332,7 @@ struct _virNodeDevCapData { virNodeDevCapVDPA vdpa; virNodeDevCapAPCard ap_card; virNodeDevCapAPQueue ap_queue; + virNodeDevCapAPMatrix ap_matrix; }; };
diff --git a/src/conf/virnodedeviceobj.c b/src/conf/virnodedeviceobj.c index 691632c6..8fbef5f5 100644 --- a/src/conf/virnodedeviceobj.c +++ b/src/conf/virnodedeviceobj.c @@ -719,6 +719,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_AP_MATRIX: 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 6bbff571..5f57000e 100644 --- a/src/node_device/node_device_udev.c +++ b/src/node_device/node_device_udev.c @@ -1241,6 +1241,25 @@ udevProcessAPQueue(struct udev_device *device, }
+static int +udevProcessAPMatrix(struct udev_device *device, + virNodeDeviceDefPtr def) +{ + size_t i; + virNodeDevCapDataPtr data = &def->caps->data; + + data->ap_matrix.addr = g_strdup(udev_device_get_sysname(device)); + def->name = g_strdup_printf("ap_%s", data->ap_matrix.addr);
So you're setting this 'addr' field, but it is not used anywhere else in this patch. Perhaps you'll use it in upcoming patches. But it is not formatted into the node device xml or anything like that. Is that intentional?
+ + for (i = 0; i < strlen(def->name); i++) { + if (!(g_ascii_isalnum(*(def->name + i)))) + *(def->name + i) = '_'; + } + + return 0; +}
Out of curiosity, what's the reason that you're hard-coding an "ap_" prefix to the nodedev name rather than just using udevGenerateDeviceName() like all of the other device types?
+ + static int udevGetDeviceNodes(struct udev_device *device, virNodeDeviceDefPtr def) @@ -1326,6 +1345,8 @@ udevGetDeviceType(struct udev_device *device, *type = VIR_NODE_DEV_CAP_CSS_DEV; else if (STREQ_NULLABLE(subsystem, "vdpa")) *type = VIR_NODE_DEV_CAP_VDPA; + else if (STREQ_NULLABLE(subsystem, "matrix")) + *type = VIR_NODE_DEV_CAP_AP_MATRIX;
VIR_FREE(subsystem); } @@ -1378,6 +1399,8 @@ udevGetDeviceDetails(struct udev_device *device, return udevProcessAPCard(device, def); case VIR_NODE_DEV_CAP_AP_QUEUE: return udevProcessAPQueue(device, def); + case VIR_NODE_DEV_CAP_AP_MATRIX: + return udevProcessAPMatrix(device, def); case VIR_NODE_DEV_CAP_MDEV_TYPES: case VIR_NODE_DEV_CAP_SYSTEM: case VIR_NODE_DEV_CAP_FC_HOST: diff --git a/tools/virsh-nodedev.c b/tools/virsh-nodedev.c index f5ffe525..f69030d2 100644 --- a/tools/virsh-nodedev.c +++ b/tools/virsh-nodedev.c @@ -473,6 +473,7 @@ cmdNodeListDevices(vshControl *ctl, const vshCmd *cmd G_GNUC_UNUSED) case VIR_NODE_DEV_CAP_AP_QUEUE: flags |= VIR_CONNECT_LIST_NODE_DEVICES_CAP_AP_QUEUE; break; + case VIR_NODE_DEV_CAP_AP_MATRIX: case VIR_NODE_DEV_CAP_LAST: break; }
On 11/12/20 9:29 PM, Jonathon Jongsma wrote:
diff --git a/src/node_device/node_device_udev.c b/src/node_device/node_device_udev.c index 6bbff571..5f57000e 100644 --- a/src/node_device/node_device_udev.c +++ b/src/node_device/node_device_udev.c @@ -1241,6 +1241,25 @@ udevProcessAPQueue(struct udev_device *device, }
+static int +udevProcessAPMatrix(struct udev_device *device, + virNodeDeviceDefPtr def) +{ + size_t i; + virNodeDevCapDataPtr data = &def->caps->data; + + data->ap_matrix.addr = g_strdup(udev_device_get_sysname(device)); + def->name = g_strdup_printf("ap_%s", data->ap_matrix.addr); So you're setting this 'addr' field, but it is not used anywhere else in this patch. Perhaps you'll use it in upcoming patches. But it is not formatted into the node device xml or anything like that. Is that intentional?
Yes, it is not formatted into the node device xml. I will include this change in the patch 10.
+ + for (i = 0; i < strlen(def->name); i++) { + if (!(g_ascii_isalnum(*(def->name + i)))) + *(def->name + i) = '_'; + } + + return 0; +} Out of curiosity, what's the reason that you're hard-coding an "ap_" prefix to the nodedev name rather than just using udevGenerateDeviceName() like all of the other device types?
The name generated with udevGenerateDeviceName() is matrix_matrix (as both udev_device_get_subsystem and udev_device_get_sysname returns matrix), which is not very helpful, so we decided to go with ap_matrix instead. -- Kind regards Shalini Chellathurai Saroja Linux on Z and Virtualization Development Vorsitzende des Aufsichtsrats: Gregor Pillen Geschäftsführung: Dirk Wittkopp Sitz der Gesellschaft: Böblingen Registergericht: Amtsgericht Stuttgart, HRB 243294
On Fri, Nov 13, 2020 at 03:45:33PM +0100, Shalini Chellathurai Saroja wrote:
On 11/12/20 9:29 PM, Jonathon Jongsma wrote:
diff --git a/src/node_device/node_device_udev.c b/src/node_device/node_device_udev.c index 6bbff571..5f57000e 100644 --- a/src/node_device/node_device_udev.c +++ b/src/node_device/node_device_udev.c @@ -1241,6 +1241,25 @@ udevProcessAPQueue(struct udev_device *device, } +static int +udevProcessAPMatrix(struct udev_device *device, + virNodeDeviceDefPtr def) +{ + size_t i; + virNodeDevCapDataPtr data = &def->caps->data; + + data->ap_matrix.addr = g_strdup(udev_device_get_sysname(device)); + def->name = g_strdup_printf("ap_%s", data->ap_matrix.addr); So you're setting this 'addr' field, but it is not used anywhere else in this patch. Perhaps you'll use it in upcoming patches. But it is not formatted into the node device xml or anything like that. Is that intentional?
Yes, it is not formatted into the node device xml.
I will include this change in the patch 10.
+ + for (i = 0; i < strlen(def->name); i++) { + if (!(g_ascii_isalnum(*(def->name + i)))) + *(def->name + i) = '_'; + } + + return 0; +} Out of curiosity, what's the reason that you're hard-coding an "ap_" prefix to the nodedev name rather than just using udevGenerateDeviceName() like all of the other device types?
The name generated with udevGenerateDeviceName() is matrix_matrix (as both udev_device_get_subsystem and udev_device_get_sysname returns matrix), which is not very helpful, so we decided to go with ap_matrix instead.
But if that is the case, I'm wondering why are you trying to generate it dynamically in the first place, why not hardcoding it to "ap_matrix"? I must be missing something here. Regards, Erik
On 11/30/20 2:46 PM, Erik Skultety wrote:
On Fri, Nov 13, 2020 at 03:45:33PM +0100, Shalini Chellathurai Saroja wrote:
On 11/12/20 9:29 PM, Jonathon Jongsma wrote:
diff --git a/src/node_device/node_device_udev.c b/src/node_device/node_device_udev.c index 6bbff571..5f57000e 100644 --- a/src/node_device/node_device_udev.c +++ b/src/node_device/node_device_udev.c @@ -1241,6 +1241,25 @@ udevProcessAPQueue(struct udev_device *device, } +static int +udevProcessAPMatrix(struct udev_device *device, + virNodeDeviceDefPtr def) +{ + size_t i; + virNodeDevCapDataPtr data = &def->caps->data; + + data->ap_matrix.addr = g_strdup(udev_device_get_sysname(device)); + def->name = g_strdup_printf("ap_%s", data->ap_matrix.addr); So you're setting this 'addr' field, but it is not used anywhere else in this patch. Perhaps you'll use it in upcoming patches. But it is not formatted into the node device xml or anything like that. Is that intentional?
Yes, it is not formatted into the node device xml.
I will include this change in the patch 10.
+ + for (i = 0; i < strlen(def->name); i++) { + if (!(g_ascii_isalnum(*(def->name + i)))) + *(def->name + i) = '_'; + } + + return 0; +} Out of curiosity, what's the reason that you're hard-coding an "ap_" prefix to the nodedev name rather than just using udevGenerateDeviceName() like all of the other device types? The name generated with udevGenerateDeviceName() is matrix_matrix (as both udev_device_get_subsystem and udev_device_get_sysname returns matrix), which is not very helpful, so we decided to go with ap_matrix instead. But if that is the case, I'm wondering why are you trying to generate it dynamically in the first place, why not hardcoding it to "ap_matrix"? I must be missing something here.
Regards, Erik ok, I will hard code it. Thank you for the review.
-- Kind regards Shalini Chellathurai Saroja Linux on Z and Virtualization Development Vorsitzende des Aufsichtsrats: Gregor Pillen Geschäftsführung: Dirk Wittkopp Sitz der Gesellschaft: Böblingen Registergericht: Amtsgericht Stuttgart, HRB 243294
Add tests to verify libvirt node device driver support for AP matrix device. Signed-off-by: Shalini Chellathurai Saroja <shalini@linux.ibm.com> Reviewed-by: Bjoern Walk <bwalk@linux.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com> --- tests/nodedevschemadata/ap_matrix.xml | 7 +++++++ .../mdev_ee0b88c4_f554_4dc1_809d_b2a01e8e48ad.xml | 9 +++++++++ tests/nodedevxml2xmltest.c | 2 ++ 3 files changed, 18 insertions(+) create mode 100644 tests/nodedevschemadata/ap_matrix.xml create mode 100644 tests/nodedevschemadata/mdev_ee0b88c4_f554_4dc1_809d_b2a01e8e48ad.xml diff --git a/tests/nodedevschemadata/ap_matrix.xml b/tests/nodedevschemadata/ap_matrix.xml new file mode 100644 index 00000000..30dab9cf --- /dev/null +++ b/tests/nodedevschemadata/ap_matrix.xml @@ -0,0 +1,7 @@ +<device> + <name>ap_matrix</name> + <path>/sys/devices/vfio_ap/matrix</path> + <parent>computer</parent> + <capability type='ap_matrix'> + </capability> +</device> diff --git a/tests/nodedevschemadata/mdev_ee0b88c4_f554_4dc1_809d_b2a01e8e48ad.xml b/tests/nodedevschemadata/mdev_ee0b88c4_f554_4dc1_809d_b2a01e8e48ad.xml new file mode 100644 index 00000000..106f7593 --- /dev/null +++ b/tests/nodedevschemadata/mdev_ee0b88c4_f554_4dc1_809d_b2a01e8e48ad.xml @@ -0,0 +1,9 @@ +<device> + <name>mdev_ee0b88c4-f554-4dc1-809d-b2a01e8e48ad</name> + <path>/sys/devices/vfio_ap/matrix/mdev_ee0b88c4-f554-4dc1-809d-b2a01e8e48ad</path> + <parent>ap_matrix</parent> + <capability type='mdev'> + <type id='vfio_ap-passthrough'/> + <iommuGroup number='0'/> + </capability> +</device> diff --git a/tests/nodedevxml2xmltest.c b/tests/nodedevxml2xmltest.c index a8cbe6a9..dc8cb04f 100644 --- a/tests/nodedevxml2xmltest.c +++ b/tests/nodedevxml2xmltest.c @@ -127,6 +127,8 @@ mymain(void) DO_TEST("css_0_0_fffe_mdev_types"); DO_TEST("ap_card07"); DO_TEST("ap_07_0038"); + DO_TEST("ap_matrix"); + DO_TEST("mdev_ee0b88c4_f554_4dc1_809d_b2a01e8e48ad"); return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE; } -- 2.26.2
Add support to filter by 'ap_matrix' capability. Signed-off-by: Shalini Chellathurai Saroja <shalini@linux.ibm.com> Reviewed-by: Bjoern Walk <bwalk@linux.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com> --- docs/formatnode.html.in | 3 +++ docs/manpages/virsh.rst | 2 +- include/libvirt/libvirt-nodedev.h | 1 + src/conf/node_device_conf.h | 3 ++- src/conf/virnodedeviceobj.c | 3 ++- src/libvirt-nodedev.c | 1 + tools/virsh-nodedev.c | 2 ++ 7 files changed, 12 insertions(+), 3 deletions(-) diff --git a/docs/formatnode.html.in b/docs/formatnode.html.in index 45281363..8a84d0dc 100644 --- a/docs/formatnode.html.in +++ b/docs/formatnode.html.in @@ -450,6 +450,9 @@ <dd>AP Queue identifier.</dd> </dl> </dd> + <dt><code>ap_matrix</code></dt> + <dd>Describes an AP Matrix device on a S390 architecture providing + cryptographic host resources usable for virtualization.</dd> </dl> </dd> </dl> diff --git a/docs/manpages/virsh.rst b/docs/manpages/virsh.rst index 1e71818a..23068dcb 100644 --- a/docs/manpages/virsh.rst +++ b/docs/manpages/virsh.rst @@ -4972,7 +4972,7 @@ List all of the devices available on the node that are known by libvirt. separated by comma, e.g. --cap pci,scsi. Valid capability types include 'system', 'pci', 'usb_device', 'usb', 'net', 'scsi_host', 'scsi_target', 'scsi', 'storage', 'fc_host', 'vports', 'scsi_generic', 'drm', 'mdev', -'mdev_types', 'ccw', 'css', 'ap_card', 'ap_queue'. +'mdev_types', 'ccw', 'css', 'ap_card', 'ap_queue', 'ap_matrix'. If *--tree* is used, the output is formatted in a tree representing parents of each node. *cap* and *--tree* are mutually exclusive. diff --git a/include/libvirt/libvirt-nodedev.h b/include/libvirt/libvirt-nodedev.h index d5091aa2..eab8abf6 100644 --- a/include/libvirt/libvirt-nodedev.h +++ b/include/libvirt/libvirt-nodedev.h @@ -85,6 +85,7 @@ typedef enum { VIR_CONNECT_LIST_NODE_DEVICES_CAP_VDPA = 1 << 17, /* vDPA device */ 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 */ } virConnectListAllNodeDeviceFlags; int virConnectListAllNodeDevices (virConnectPtr conn, diff --git a/src/conf/node_device_conf.h b/src/conf/node_device_conf.h index b863653b..b8397128 100644 --- a/src/conf/node_device_conf.h +++ b/src/conf/node_device_conf.h @@ -412,7 +412,8 @@ virNodeDevCapsDefFree(virNodeDevCapsDefPtr caps); VIR_CONNECT_LIST_NODE_DEVICES_CAP_CSS_DEV | \ 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_QUEUE | \ + VIR_CONNECT_LIST_NODE_DEVICES_CAP_AP_MATRIX) int virNodeDeviceGetSCSIHostCaps(virNodeDevCapSCSIHostPtr scsi_host); diff --git a/src/conf/virnodedeviceobj.c b/src/conf/virnodedeviceobj.c index 8fbef5f5..25d12776 100644 --- a/src/conf/virnodedeviceobj.c +++ b/src/conf/virnodedeviceobj.c @@ -874,7 +874,8 @@ virNodeDeviceObjMatch(virNodeDeviceObjPtr obj, MATCH(CSS_DEV) || MATCH(VDPA) || MATCH(AP_CARD) || - MATCH(AP_QUEUE))) + MATCH(AP_QUEUE) || + MATCH(AP_MATRIX))) return false; } diff --git a/src/libvirt-nodedev.c b/src/libvirt-nodedev.c index 762413eb..eb8c735a 100644 --- a/src/libvirt-nodedev.c +++ b/src/libvirt-nodedev.c @@ -104,6 +104,7 @@ virNodeNumOfDevices(virConnectPtr conn, const char *cap, unsigned int flags) * VIR_CONNECT_LIST_NODE_DEVICES_CAP_CSS_DEV * VIR_CONNECT_LIST_NODE_DEVICES_CAP_AP_CARD * VIR_CONNECT_LIST_NODE_DEVICES_CAP_AP_QUEUE + * VIR_CONNECT_LIST_NODE_DEVICES_CAP_AP_MATRIX * * Returns the number of node devices found or -1 and sets @devices to NULL in * case of error. On success, the array stored into @devices is guaranteed to diff --git a/tools/virsh-nodedev.c b/tools/virsh-nodedev.c index f69030d2..69422e20 100644 --- a/tools/virsh-nodedev.c +++ b/tools/virsh-nodedev.c @@ -474,6 +474,8 @@ cmdNodeListDevices(vshControl *ctl, const vshCmd *cmd G_GNUC_UNUSED) flags |= VIR_CONNECT_LIST_NODE_DEVICES_CAP_AP_QUEUE; break; case VIR_NODE_DEV_CAP_AP_MATRIX: + flags |= VIR_CONNECT_LIST_NODE_DEVICES_CAP_AP_MATRIX; + break; case VIR_NODE_DEV_CAP_LAST: break; } -- 2.26.2
Use switch statements instead of if-else condition in the method nodeDeviceFindAddressByName to retrieve address of a node device. Signed-off-by: Shalini Chellathurai Saroja <shalini@linux.ibm.com> Reviewed-by: Bjoern Walk <bwalk@linux.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com> --- src/node_device/node_device_driver.c | 30 ++++++++++++++++++++++++++-- 1 file changed, 28 insertions(+), 2 deletions(-) diff --git a/src/node_device/node_device_driver.c b/src/node_device/node_device_driver.c index f5ea973c..65c647f5 100644 --- a/src/node_device/node_device_driver.c +++ b/src/node_device/node_device_driver.c @@ -638,7 +638,8 @@ nodeDeviceFindAddressByName(const char *name) def = virNodeDeviceObjGetDef(dev); for (caps = def->caps; caps != NULL; caps = caps->next) { - if (caps->data.type == VIR_NODE_DEV_CAP_PCI_DEV) { + switch (caps->data.type) { + case VIR_NODE_DEV_CAP_PCI_DEV: { virPCIDeviceAddress pci_addr = { .domain = caps->data.pci_dev.domain, .bus = caps->data.pci_dev.bus, @@ -648,7 +649,9 @@ nodeDeviceFindAddressByName(const char *name) addr = virPCIDeviceAddressAsString(&pci_addr); break; - } else if (caps->data.type == VIR_NODE_DEV_CAP_CSS_DEV) { + } + + case VIR_NODE_DEV_CAP_CSS_DEV: { virDomainDeviceCCWAddress ccw_addr = { .cssid = caps->data.ccw_dev.cssid, .ssid = caps->data.ccw_dev.ssid, @@ -657,6 +660,29 @@ nodeDeviceFindAddressByName(const char *name) addr = virDomainCCWAddressAsString(&ccw_addr); break; + } + + case VIR_NODE_DEV_CAP_SYSTEM: + case VIR_NODE_DEV_CAP_USB_DEV: + case VIR_NODE_DEV_CAP_USB_INTERFACE: + case VIR_NODE_DEV_CAP_NET: + case VIR_NODE_DEV_CAP_SCSI_HOST: + case VIR_NODE_DEV_CAP_SCSI_TARGET: + case VIR_NODE_DEV_CAP_SCSI: + case VIR_NODE_DEV_CAP_STORAGE: + case VIR_NODE_DEV_CAP_FC_HOST: + case VIR_NODE_DEV_CAP_VPORTS: + case VIR_NODE_DEV_CAP_SCSI_GENERIC: + case VIR_NODE_DEV_CAP_DRM: + case VIR_NODE_DEV_CAP_MDEV_TYPES: + case VIR_NODE_DEV_CAP_MDEV: + case VIR_NODE_DEV_CAP_CCW_DEV: + 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_AP_MATRIX: + case VIR_NODE_DEV_CAP_LAST: + break; } } -- 2.26.2
Allow mdev devices to be created on the matrix device. Signed-off-by: Shalini Chellathurai Saroja <shalini@linux.ibm.com> Reviewed-by: Bjoern Walk <bwalk@linux.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com> --- src/node_device/node_device_driver.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/node_device/node_device_driver.c b/src/node_device/node_device_driver.c index 65c647f5..e254b492 100644 --- a/src/node_device/node_device_driver.c +++ b/src/node_device/node_device_driver.c @@ -662,6 +662,10 @@ nodeDeviceFindAddressByName(const char *name) break; } + case VIR_NODE_DEV_CAP_AP_MATRIX: + addr = g_strdup(caps->data.ap_matrix.addr); + break; + case VIR_NODE_DEV_CAP_SYSTEM: case VIR_NODE_DEV_CAP_USB_DEV: case VIR_NODE_DEV_CAP_USB_INTERFACE: @@ -680,7 +684,6 @@ nodeDeviceFindAddressByName(const char *name) 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_AP_MATRIX: case VIR_NODE_DEV_CAP_LAST: break; } -- 2.26.2
On Thu, 12 Nov 2020 13:15:18 +0100 Shalini Chellathurai Saroja <shalini@linux.ibm.com> wrote:
Allow mdev devices to be created on the matrix device.
Signed-off-by: Shalini Chellathurai Saroja <shalini@linux.ibm.com> Reviewed-by: Bjoern Walk <bwalk@linux.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com> --- src/node_device/node_device_driver.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/src/node_device/node_device_driver.c b/src/node_device/node_device_driver.c index 65c647f5..e254b492 100644 --- a/src/node_device/node_device_driver.c +++ b/src/node_device/node_device_driver.c @@ -662,6 +662,10 @@ nodeDeviceFindAddressByName(const char *name) break; }
+ case VIR_NODE_DEV_CAP_AP_MATRIX: + addr = g_strdup(caps->data.ap_matrix.addr); + break; + case VIR_NODE_DEV_CAP_SYSTEM: case VIR_NODE_DEV_CAP_USB_DEV: case VIR_NODE_DEV_CAP_USB_INTERFACE: @@ -680,7 +684,6 @@ nodeDeviceFindAddressByName(const char *name) 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_AP_MATRIX: case VIR_NODE_DEV_CAP_LAST: break; }
So here is where you use the saved address field. Perhaps the field should be introduced in this patch instead?
On 11/12/20 9:42 PM, Jonathon Jongsma wrote:
On Thu, 12 Nov 2020 13:15:18 +0100 Shalini Chellathurai Saroja <shalini@linux.ibm.com> wrote:
Allow mdev devices to be created on the matrix device.
Signed-off-by: Shalini Chellathurai Saroja <shalini@linux.ibm.com> Reviewed-by: Bjoern Walk <bwalk@linux.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com> --- src/node_device/node_device_driver.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/src/node_device/node_device_driver.c b/src/node_device/node_device_driver.c index 65c647f5..e254b492 100644 --- a/src/node_device/node_device_driver.c +++ b/src/node_device/node_device_driver.c @@ -662,6 +662,10 @@ nodeDeviceFindAddressByName(const char *name) break; }
+ case VIR_NODE_DEV_CAP_AP_MATRIX: + addr = g_strdup(caps->data.ap_matrix.addr); + break; + case VIR_NODE_DEV_CAP_SYSTEM: case VIR_NODE_DEV_CAP_USB_DEV: case VIR_NODE_DEV_CAP_USB_INTERFACE: @@ -680,7 +684,6 @@ nodeDeviceFindAddressByName(const char *name) 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_AP_MATRIX: case VIR_NODE_DEV_CAP_LAST: break; }
So here is where you use the saved address field. Perhaps the field should be introduced in this patch instead? ok, I will do so.
-- Kind regards Shalini Chellathurai Saroja Linux on Z and Virtualization Development Vorsitzende des Aufsichtsrats: Gregor Pillen Geschäftsführung: Dirk Wittkopp Sitz der Gesellschaft: Böblingen Registergericht: Amtsgericht Stuttgart, HRB 243294
From: Boris Fiuczynski <fiuczy@linux.ibm.com> Add detection of mdev_types capability to Adjunct Processor Matrix device. Signed-off-by: Boris Fiuczynski <fiuczy@linux.ibm.com> Signed-off-by: Shalini Chellathurai Saroja <shalini@linux.ibm.com> --- docs/formatnode.html.in | 24 +++- docs/schemas/nodedev.rng | 4 + src/conf/node_device_conf.c | 108 +++++++++++++++++- src/conf/node_device_conf.h | 11 ++ src/conf/virnodedeviceobj.c | 7 +- src/libvirt_private.syms | 1 + src/node_device/node_device_udev.c | 4 + .../ap_matrix_mdev_types.xml | 14 +++ tests/nodedevxml2xmltest.c | 1 + 9 files changed, 168 insertions(+), 6 deletions(-) create mode 100644 tests/nodedevschemadata/ap_matrix_mdev_types.xml diff --git a/docs/formatnode.html.in b/docs/formatnode.html.in index 8a84d0dc..66f38c51 100644 --- a/docs/formatnode.html.in +++ b/docs/formatnode.html.in @@ -452,7 +452,26 @@ </dd> <dt><code>ap_matrix</code></dt> <dd>Describes an AP Matrix device on a S390 architecture providing - cryptographic host resources usable for virtualization.</dd> + cryptographic host resources usable for virtualization. + Sub-elements include: + <dl> + <dt><code>capability</code></dt> + <dd> + This optional element can occur multiple times. If it + exists, it has a mandatory <code>type</code> attribute + which will be set to: + <dl> + <dt><code><a id="MDEVTypesCapAP">mdev_types</a></code></dt> + <dd> + <span class="since">Since 6.10.0</span> + This device is capable of creating mediated devices. + The sub-elements are summarized in + <a href="#MDEVTypesCap">mdev_types capability</a>. + </dd> + </dl> + </dd> + </dl> + </dd> </dl> </dd> </dl> @@ -460,7 +479,8 @@ <h3><a id="MDEVTypesCap">mdev_types capability</a></h3> <p> - <a href="#MDEVTypesCapPCI">PCI</a> and <a href="#MDEVTypesCapCSS">CSS</a> + <a href="#MDEVTypesCapPCI">PCI</a>, <a href="#MDEVTypesCapCSS">CSS</a> + and <a href="#MDEVTypesCapAP">AP Matrix</a> devices can be capable of creating mediated devices. If they indeed are capable, then the parent <code>capability</code> element for <code>mdev_types</code> type will contain a list of diff --git a/docs/schemas/nodedev.rng b/docs/schemas/nodedev.rng index 5be21145..8ea74f23 100644 --- a/docs/schemas/nodedev.rng +++ b/docs/schemas/nodedev.rng @@ -696,6 +696,9 @@ <attribute name='type'> <value>ap_matrix</value> </attribute> + <optional> + <ref name="mdev_types"/> + </optional> </define> <define name='address'> @@ -736,6 +739,7 @@ <choice> <value>vfio-pci</value> <value>vfio-ccw</value> + <value>vfio-ap</value> </choice> </element> <element name="availableInstances"> diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c index f3d146a5..01a4d5cd 100644 --- a/src/conf/node_device_conf.c +++ b/src/conf/node_device_conf.c @@ -663,10 +663,15 @@ virNodeDeviceDefFormat(const virNodeDeviceDef *def) virBufferAsprintf(&buf, "<ap-domain>0x%04x</ap-domain>\n", data->ap_queue.ap_domain); break; + case VIR_NODE_DEV_CAP_AP_MATRIX: + if (data->ap_matrix.flags & VIR_NODE_DEV_CAP_FLAG_AP_MATRIX_MDEV) + virNodeDeviceCapMdevTypesFormat(&buf, + data->ap_matrix.mdev_types, + data->ap_matrix.nmdev_types); + 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_AP_MATRIX: case VIR_NODE_DEV_CAP_LAST: break; } @@ -861,6 +866,33 @@ virNodeDevCapMdevTypesParseXML(xmlXPathContextPtr ctxt, } +static int +virNodeDevAPMatrixCapabilityParseXML(xmlXPathContextPtr ctxt, + xmlNodePtr node, + virNodeDevCapAPMatrixPtr apm_dev) +{ + g_autofree char *type = virXMLPropString(node, "type"); + VIR_XPATH_NODE_AUTORESTORE(ctxt) + + ctxt->node = node; + + if (!type) { + virReportError(VIR_ERR_XML_ERROR, "%s", _("Missing capability type")); + return -1; + } + + if (STREQ(type, "mdev_types")) { + if (virNodeDevCapMdevTypesParseXML(ctxt, + &apm_dev->mdev_types, + &apm_dev->nmdev_types) < 0) + return -1; + apm_dev->flags |= VIR_NODE_DEV_CAP_FLAG_AP_MATRIX_MDEV; + } + + return 0; +} + + static int virNodeDevCSSCapabilityParseXML(xmlXPathContextPtr ctxt, xmlNodePtr node, @@ -1031,6 +1063,31 @@ virNodeDevCapAPQueueParseXML(xmlXPathContextPtr ctxt, } +static int +virNodeDevCapAPMatrixParseXML(xmlXPathContextPtr ctxt, + virNodeDeviceDefPtr def G_GNUC_UNUSED, + xmlNodePtr node, + virNodeDevCapAPMatrixPtr ap_matrix) +{ + VIR_XPATH_NODE_AUTORESTORE(ctxt) + g_autofree xmlNodePtr *nodes = NULL; + int n = 0; + size_t i = 0; + + ctxt->node = node; + + if ((n = virXPathNodeSet("./capability", ctxt, &nodes)) < 0) + return -1; + + for (i = 0; i < n; i++) { + if (virNodeDevAPMatrixCapabilityParseXML(ctxt, nodes[i], ap_matrix) < 0) + return -1; + } + + return 0; +} + + static int virNodeDevCapStorageParseXML(xmlXPathContextPtr ctxt, virNodeDeviceDefPtr def, @@ -2078,7 +2135,8 @@ virNodeDevCapsDefParseXML(xmlXPathContextPtr ctxt, &caps->data.ap_queue); break; case VIR_NODE_DEV_CAP_AP_MATRIX: - ret = 0; + ret = virNodeDevCapAPMatrixParseXML(ctxt, def, node, + &caps->data.ap_matrix); break; case VIR_NODE_DEV_CAP_MDEV_TYPES: case VIR_NODE_DEV_CAP_FC_HOST: @@ -2403,6 +2461,9 @@ virNodeDevCapsDefFree(virNodeDevCapsDefPtr caps) break; case VIR_NODE_DEV_CAP_AP_MATRIX: VIR_FREE(data->ap_matrix.addr); + for (i = 0; i < data->ap_matrix.nmdev_types; i++) + virMediatedDeviceTypeFree(data->ap_matrix.mdev_types[i]); + VIR_FREE(data->ap_matrix.mdev_types); break; case VIR_NODE_DEV_CAP_MDEV_TYPES: case VIR_NODE_DEV_CAP_DRM: @@ -2454,6 +2515,11 @@ virNodeDeviceUpdateCaps(virNodeDeviceDefPtr def) &cap->data.ccw_dev) < 0) return -1; break; + case VIR_NODE_DEV_CAP_AP_MATRIX: + if (virNodeDeviceGetAPMatrixDynamicCaps(def->sysfs_path, + &cap->data.ap_matrix) < 0) + return -1; + break; /* all types that (supposedly) don't require any updates * relative to what's in the cache. @@ -2473,7 +2539,6 @@ virNodeDeviceUpdateCaps(virNodeDeviceDefPtr 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_AP_MATRIX: case VIR_NODE_DEV_CAP_LAST: break; } @@ -2556,6 +2621,15 @@ virNodeDeviceCapsListExport(virNodeDeviceDefPtr def, ncaps++; } } + + if (caps->data.type == VIR_NODE_DEV_CAP_AP_MATRIX) { + flags = caps->data.ap_matrix.flags; + + if (flags & VIR_NODE_DEV_CAP_FLAG_AP_MATRIX_MDEV) { + MAYBE_ADD_CAP(VIR_NODE_DEV_CAP_MDEV_TYPES); + ncaps++; + } + } } #undef MAYBE_ADD_CAP @@ -2844,6 +2918,27 @@ virNodeDeviceGetCSSDynamicCaps(const char *sysfsPath, return 0; } +/* virNodeDeviceGetAPMatrixDynamicCaps() 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 + * anytime full XML of the device is requested, because they can + * change with no corresponding notification from the kernel/udev. + */ +int +virNodeDeviceGetAPMatrixDynamicCaps(const char *sysfsPath, + virNodeDevCapAPMatrixPtr ap_matrix) +{ + ap_matrix->flags &= ~VIR_NODE_DEV_CAP_FLAG_AP_MATRIX_MDEV; + if (virNodeDeviceGetMdevTypesCaps(sysfsPath, + &ap_matrix->mdev_types, + &ap_matrix->nmdev_types) < 0) + return -1; + if (ap_matrix->nmdev_types > 0) + ap_matrix->flags |= VIR_NODE_DEV_CAP_FLAG_AP_MATRIX_MDEV; + + return 0; +} + #else int @@ -2872,4 +2967,11 @@ virNodeDeviceGetCSSDynamicCaps(const char *sysfsPath G_GNUC_UNUSED, return -1; } +int +virNodeDeviceGetAPMatrixDynamicCaps(const char *sysfsPath G_GNUC_UNUSED, + virNodeDevCapAPMatrixPtr ap_matrix G_GNUC_UNUSED) +{ + return -1; +} + #endif /* __linux__ */ diff --git a/src/conf/node_device_conf.h b/src/conf/node_device_conf.h index b8397128..c67b8e2a 100644 --- a/src/conf/node_device_conf.h +++ b/src/conf/node_device_conf.h @@ -109,6 +109,10 @@ typedef enum { VIR_NODE_DEV_CAP_FLAG_CSS_MDEV = (1 << 0), } virNodeDevCCWCapFlags; +typedef enum { + VIR_NODE_DEV_CAP_FLAG_AP_MATRIX_MDEV = (1 << 0), +} virNodeDevAPMatrixCapFlags; + typedef enum { /* Keep in sync with VIR_ENUM_IMPL in node_device_conf.c */ VIR_NODE_DEV_DRM_PRIMARY, @@ -309,6 +313,9 @@ typedef struct _virNodeDevCapAPMatrix virNodeDevCapAPMatrix; typedef virNodeDevCapAPMatrix *virNodeDevCapAPMatrixPtr; struct _virNodeDevCapAPMatrix { char *addr; + unsigned int flags; /* enum virNodeDevAPMatrixCapFlags */ + virMediatedDeviceTypePtr *mdev_types; + size_t nmdev_types; }; typedef struct _virNodeDevCapData virNodeDevCapData; @@ -430,6 +437,10 @@ int virNodeDeviceGetCSSDynamicCaps(const char *sysfsPath, virNodeDevCapCCWPtr ccw_dev); +int +virNodeDeviceGetAPMatrixDynamicCaps(const char *sysfsPath, + virNodeDevCapAPMatrixPtr ap_matrix); + int virNodeDeviceUpdateCaps(virNodeDeviceDefPtr def); diff --git a/src/conf/virnodedeviceobj.c b/src/conf/virnodedeviceobj.c index 25d12776..c9bda77b 100644 --- a/src/conf/virnodedeviceobj.c +++ b/src/conf/virnodedeviceobj.c @@ -702,6 +702,12 @@ virNodeDeviceObjHasCap(const virNodeDeviceObj *obj, return true; break; + case VIR_NODE_DEV_CAP_AP_MATRIX: + if (type == VIR_NODE_DEV_CAP_MDEV_TYPES && + (cap->data.ap_matrix.flags & VIR_NODE_DEV_CAP_FLAG_AP_MATRIX_MDEV)) + return true; + break; + case VIR_NODE_DEV_CAP_SYSTEM: case VIR_NODE_DEV_CAP_USB_DEV: case VIR_NODE_DEV_CAP_USB_INTERFACE: @@ -719,7 +725,6 @@ 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_AP_MATRIX: case VIR_NODE_DEV_CAP_LAST: break; } diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 491bd5bd..25c3456a 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -824,6 +824,7 @@ virNodeDeviceDefFree; virNodeDeviceDefParseFile; virNodeDeviceDefParseNode; virNodeDeviceDefParseString; +virNodeDeviceGetAPMatrixDynamicCaps; virNodeDeviceGetCSSDynamicCaps; virNodeDeviceGetPCIDynamicCaps; virNodeDeviceGetSCSIHostCaps; diff --git a/src/node_device/node_device_udev.c b/src/node_device/node_device_udev.c index 5f57000e..53f3c24b 100644 --- a/src/node_device/node_device_udev.c +++ b/src/node_device/node_device_udev.c @@ -1256,6 +1256,10 @@ udevProcessAPMatrix(struct udev_device *device, *(def->name + i) = '_'; } + if (virNodeDeviceGetAPMatrixDynamicCaps(def->sysfs_path, + &data->ap_matrix) < 0) + return -1; + return 0; } diff --git a/tests/nodedevschemadata/ap_matrix_mdev_types.xml b/tests/nodedevschemadata/ap_matrix_mdev_types.xml new file mode 100644 index 00000000..0ca83680 --- /dev/null +++ b/tests/nodedevschemadata/ap_matrix_mdev_types.xml @@ -0,0 +1,14 @@ +<device> + <name>ap_matrix</name> + <path>/sys/devices/vfio_ap/matrix</path> + <parent>computer</parent> + <capability type='ap_matrix'> + <capability type='mdev_types'> + <type id='vfio_api-passthrough'> + <name>VFIO AP Passthrough Device</name> + <deviceAPI>vfio-ap</deviceAPI> + <availableInstances>65536</availableInstances> + </type> + </capability> + </capability> +</device> diff --git a/tests/nodedevxml2xmltest.c b/tests/nodedevxml2xmltest.c index dc8cb04f..a2321d13 100644 --- a/tests/nodedevxml2xmltest.c +++ b/tests/nodedevxml2xmltest.c @@ -128,6 +128,7 @@ mymain(void) DO_TEST("ap_card07"); DO_TEST("ap_07_0038"); DO_TEST("ap_matrix"); + DO_TEST("ap_matrix_mdev_types"); DO_TEST("mdev_ee0b88c4_f554_4dc1_809d_b2a01e8e48ad"); return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE; -- 2.26.2
On Thu, 12 Nov 2020 13:15:19 +0100 Shalini Chellathurai Saroja <shalini@linux.ibm.com> wrote:
From: Boris Fiuczynski <fiuczy@linux.ibm.com>
Add detection of mdev_types capability to Adjunct Processor Matrix device.
Signed-off-by: Boris Fiuczynski <fiuczy@linux.ibm.com> Signed-off-by: Shalini Chellathurai Saroja <shalini@linux.ibm.com> --- docs/formatnode.html.in | 24 +++- docs/schemas/nodedev.rng | 4 + src/conf/node_device_conf.c | 108 +++++++++++++++++- src/conf/node_device_conf.h | 11 ++ src/conf/virnodedeviceobj.c | 7 +- src/libvirt_private.syms | 1 + src/node_device/node_device_udev.c | 4 + .../ap_matrix_mdev_types.xml | 14 +++ tests/nodedevxml2xmltest.c | 1 + 9 files changed, 168 insertions(+), 6 deletions(-) create mode 100644 tests/nodedevschemadata/ap_matrix_mdev_types.xml
diff --git a/docs/formatnode.html.in b/docs/formatnode.html.in index 8a84d0dc..66f38c51 100644 --- a/docs/formatnode.html.in +++ b/docs/formatnode.html.in @@ -452,7 +452,26 @@ </dd> <dt><code>ap_matrix</code></dt> <dd>Describes an AP Matrix device on a S390 architecture providing - cryptographic host resources usable for virtualization.</dd> + cryptographic host resources usable for virtualization. + Sub-elements include: + <dl> + <dt><code>capability</code></dt> + <dd> + This optional element can occur multiple times. If it + exists, it has a mandatory <code>type</code> attribute + which will be set to: + <dl> + <dt><code><a id="MDEVTypesCapAP">mdev_types</a></code></dt> + <dd> + <span class="since">Since 6.10.0</span> + This device is capable of creating mediated devices. + The sub-elements are summarized in + <a href="#MDEVTypesCap">mdev_types capability</a>. + </dd> + </dl> + </dd> + </dl> + </dd> </dl> </dd> </dl> @@ -460,7 +479,8 @@ <h3><a id="MDEVTypesCap">mdev_types capability</a></h3>
<p> - <a href="#MDEVTypesCapPCI">PCI</a> and <a href="#MDEVTypesCapCSS">CSS</a> + <a href="#MDEVTypesCapPCI">PCI</a>, <a href="#MDEVTypesCapCSS">CSS</a> + and <a href="#MDEVTypesCapAP">AP Matrix</a> devices can be capable of creating mediated devices. If they indeed are capable, then the parent <code>capability</code> element for <code>mdev_types</code> type will contain a list of diff --git a/docs/schemas/nodedev.rng b/docs/schemas/nodedev.rng index 5be21145..8ea74f23 100644 --- a/docs/schemas/nodedev.rng +++ b/docs/schemas/nodedev.rng @@ -696,6 +696,9 @@ <attribute name='type'> <value>ap_matrix</value> </attribute> + <optional> + <ref name="mdev_types"/> + </optional> </define>
<define name='address'> @@ -736,6 +739,7 @@ <choice> <value>vfio-pci</value> <value>vfio-ccw</value> + <value>vfio-ap</value> </choice> </element> <element name="availableInstances"> diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c index f3d146a5..01a4d5cd 100644 --- a/src/conf/node_device_conf.c +++ b/src/conf/node_device_conf.c @@ -663,10 +663,15 @@ virNodeDeviceDefFormat(const virNodeDeviceDef *def) virBufferAsprintf(&buf, "<ap-domain>0x%04x</ap-domain>\n", data->ap_queue.ap_domain); break; + case VIR_NODE_DEV_CAP_AP_MATRIX: + if (data->ap_matrix.flags & VIR_NODE_DEV_CAP_FLAG_AP_MATRIX_MDEV) + virNodeDeviceCapMdevTypesFormat(&buf, + data->ap_matrix.mdev_types, + data->ap_matrix.nmdev_types); + 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_AP_MATRIX: case VIR_NODE_DEV_CAP_LAST: break; } @@ -861,6 +866,33 @@ virNodeDevCapMdevTypesParseXML(xmlXPathContextPtr ctxt, }
+static int +virNodeDevAPMatrixCapabilityParseXML(xmlXPathContextPtr ctxt, + xmlNodePtr node, + virNodeDevCapAPMatrixPtr apm_dev) +{ + g_autofree char *type = virXMLPropString(node, "type"); + VIR_XPATH_NODE_AUTORESTORE(ctxt) + + ctxt->node = node; + + if (!type) { + virReportError(VIR_ERR_XML_ERROR, "%s", _("Missing capability type")); + return -1; + } + + if (STREQ(type, "mdev_types")) { + if (virNodeDevCapMdevTypesParseXML(ctxt, + &apm_dev->mdev_types, + &apm_dev->nmdev_types) < 0) + return -1; + apm_dev->flags |= VIR_NODE_DEV_CAP_FLAG_AP_MATRIX_MDEV; + } + + return 0; +} + + static int virNodeDevCSSCapabilityParseXML(xmlXPathContextPtr ctxt, xmlNodePtr node, @@ -1031,6 +1063,31 @@ virNodeDevCapAPQueueParseXML(xmlXPathContextPtr ctxt, }
+static int +virNodeDevCapAPMatrixParseXML(xmlXPathContextPtr ctxt, + virNodeDeviceDefPtr def G_GNUC_UNUSED, + xmlNodePtr node, + virNodeDevCapAPMatrixPtr ap_matrix) +{ + VIR_XPATH_NODE_AUTORESTORE(ctxt) + g_autofree xmlNodePtr *nodes = NULL; + int n = 0; + size_t i = 0; + + ctxt->node = node; + + if ((n = virXPathNodeSet("./capability", ctxt, &nodes)) < 0) + return -1; + + for (i = 0; i < n; i++) { + if (virNodeDevAPMatrixCapabilityParseXML(ctxt, nodes[i], ap_matrix) < 0) + return -1; + } + + return 0; +} + + static int virNodeDevCapStorageParseXML(xmlXPathContextPtr ctxt, virNodeDeviceDefPtr def, @@ -2078,7 +2135,8 @@ virNodeDevCapsDefParseXML(xmlXPathContextPtr ctxt, &caps->data.ap_queue); break; case VIR_NODE_DEV_CAP_AP_MATRIX: - ret = 0; + ret = virNodeDevCapAPMatrixParseXML(ctxt, def, node, + &caps->data.ap_matrix); break; case VIR_NODE_DEV_CAP_MDEV_TYPES: case VIR_NODE_DEV_CAP_FC_HOST: @@ -2403,6 +2461,9 @@ virNodeDevCapsDefFree(virNodeDevCapsDefPtr caps) break; case VIR_NODE_DEV_CAP_AP_MATRIX: VIR_FREE(data->ap_matrix.addr); + for (i = 0; i < data->ap_matrix.nmdev_types; i++) + virMediatedDeviceTypeFree(data->ap_matrix.mdev_types[i]); + VIR_FREE(data->ap_matrix.mdev_types); break; case VIR_NODE_DEV_CAP_MDEV_TYPES: case VIR_NODE_DEV_CAP_DRM: @@ -2454,6 +2515,11 @@ virNodeDeviceUpdateCaps(virNodeDeviceDefPtr def) &cap->data.ccw_dev) < 0) return -1; break; + case VIR_NODE_DEV_CAP_AP_MATRIX: + if (virNodeDeviceGetAPMatrixDynamicCaps(def->sysfs_path, + &cap->data.ap_matrix) < 0) + return -1; + break;
/* all types that (supposedly) don't require any updates * relative to what's in the cache. @@ -2473,7 +2539,6 @@ virNodeDeviceUpdateCaps(virNodeDeviceDefPtr 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_AP_MATRIX: case VIR_NODE_DEV_CAP_LAST: break; } @@ -2556,6 +2621,15 @@ virNodeDeviceCapsListExport(virNodeDeviceDefPtr def, ncaps++; } } + + if (caps->data.type == VIR_NODE_DEV_CAP_AP_MATRIX) { + flags = caps->data.ap_matrix.flags; + + if (flags & VIR_NODE_DEV_CAP_FLAG_AP_MATRIX_MDEV) { + MAYBE_ADD_CAP(VIR_NODE_DEV_CAP_MDEV_TYPES); + ncaps++; + } + } }
#undef MAYBE_ADD_CAP @@ -2844,6 +2918,27 @@ virNodeDeviceGetCSSDynamicCaps(const char *sysfsPath, return 0; }
+/* virNodeDeviceGetAPMatrixDynamicCaps() 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 + * anytime full XML of the device is requested, because they can + * change with no corresponding notification from the kernel/udev. + */ +int +virNodeDeviceGetAPMatrixDynamicCaps(const char *sysfsPath, + virNodeDevCapAPMatrixPtr ap_matrix) +{ + ap_matrix->flags &= ~VIR_NODE_DEV_CAP_FLAG_AP_MATRIX_MDEV; + if (virNodeDeviceGetMdevTypesCaps(sysfsPath, + &ap_matrix->mdev_types, + &ap_matrix->nmdev_types) < 0) + return -1; + if (ap_matrix->nmdev_types > 0) + ap_matrix->flags |= VIR_NODE_DEV_CAP_FLAG_AP_MATRIX_MDEV; + + return 0; +} + #else
int @@ -2872,4 +2967,11 @@ virNodeDeviceGetCSSDynamicCaps(const char *sysfsPath G_GNUC_UNUSED, return -1; }
+int +virNodeDeviceGetAPMatrixDynamicCaps(const char *sysfsPath G_GNUC_UNUSED, + virNodeDevCapAPMatrixPtr ap_matrix G_GNUC_UNUSED) +{ + return -1; +} + #endif /* __linux__ */ diff --git a/src/conf/node_device_conf.h b/src/conf/node_device_conf.h index b8397128..c67b8e2a 100644 --- a/src/conf/node_device_conf.h +++ b/src/conf/node_device_conf.h @@ -109,6 +109,10 @@ typedef enum { VIR_NODE_DEV_CAP_FLAG_CSS_MDEV = (1 << 0), } virNodeDevCCWCapFlags;
+typedef enum { + VIR_NODE_DEV_CAP_FLAG_AP_MATRIX_MDEV = (1 << 0), +} virNodeDevAPMatrixCapFlags; + typedef enum { /* Keep in sync with VIR_ENUM_IMPL in node_device_conf.c */ VIR_NODE_DEV_DRM_PRIMARY, @@ -309,6 +313,9 @@ typedef struct _virNodeDevCapAPMatrix virNodeDevCapAPMatrix; typedef virNodeDevCapAPMatrix *virNodeDevCapAPMatrixPtr; struct _virNodeDevCapAPMatrix { char *addr; + unsigned int flags; /* enum virNodeDevAPMatrixCapFlags */ + virMediatedDeviceTypePtr *mdev_types; + size_t nmdev_types; };
typedef struct _virNodeDevCapData virNodeDevCapData; @@ -430,6 +437,10 @@ int virNodeDeviceGetCSSDynamicCaps(const char *sysfsPath, virNodeDevCapCCWPtr ccw_dev);
+int +virNodeDeviceGetAPMatrixDynamicCaps(const char *sysfsPath, + virNodeDevCapAPMatrixPtr ap_matrix); + int virNodeDeviceUpdateCaps(virNodeDeviceDefPtr def);
diff --git a/src/conf/virnodedeviceobj.c b/src/conf/virnodedeviceobj.c index 25d12776..c9bda77b 100644 --- a/src/conf/virnodedeviceobj.c +++ b/src/conf/virnodedeviceobj.c @@ -702,6 +702,12 @@ virNodeDeviceObjHasCap(const virNodeDeviceObj *obj, return true; break;
+ case VIR_NODE_DEV_CAP_AP_MATRIX: + if (type == VIR_NODE_DEV_CAP_MDEV_TYPES && + (cap->data.ap_matrix.flags & VIR_NODE_DEV_CAP_FLAG_AP_MATRIX_MDEV)) + return true; + break; + case VIR_NODE_DEV_CAP_SYSTEM: case VIR_NODE_DEV_CAP_USB_DEV: case VIR_NODE_DEV_CAP_USB_INTERFACE: @@ -719,7 +725,6 @@ 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_AP_MATRIX: case VIR_NODE_DEV_CAP_LAST: break; } diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 491bd5bd..25c3456a 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -824,6 +824,7 @@ virNodeDeviceDefFree; virNodeDeviceDefParseFile; virNodeDeviceDefParseNode; virNodeDeviceDefParseString; +virNodeDeviceGetAPMatrixDynamicCaps; virNodeDeviceGetCSSDynamicCaps; virNodeDeviceGetPCIDynamicCaps; virNodeDeviceGetSCSIHostCaps; diff --git a/src/node_device/node_device_udev.c b/src/node_device/node_device_udev.c index 5f57000e..53f3c24b 100644 --- a/src/node_device/node_device_udev.c +++ b/src/node_device/node_device_udev.c @@ -1256,6 +1256,10 @@ udevProcessAPMatrix(struct udev_device *device, *(def->name + i) = '_'; }
+ if (virNodeDeviceGetAPMatrixDynamicCaps(def->sysfs_path, + &data->ap_matrix) < 0) + return -1; + return 0; }
diff --git a/tests/nodedevschemadata/ap_matrix_mdev_types.xml b/tests/nodedevschemadata/ap_matrix_mdev_types.xml new file mode 100644 index 00000000..0ca83680 --- /dev/null +++ b/tests/nodedevschemadata/ap_matrix_mdev_types.xml @@ -0,0 +1,14 @@ +<device> + <name>ap_matrix</name> + <path>/sys/devices/vfio_ap/matrix</path> + <parent>computer</parent> + <capability type='ap_matrix'> + <capability type='mdev_types'> + <type id='vfio_api-passthrough'>
nit: I guess this is just an arbitrary string representing an mdev type, but I would guess that you intended it to say vfio_ap- instead of vfio_api-?
+ <name>VFIO AP Passthrough Device</name> + <deviceAPI>vfio-ap</deviceAPI> + <availableInstances>65536</availableInstances> + </type> + </capability> + </capability> +</device> diff --git a/tests/nodedevxml2xmltest.c b/tests/nodedevxml2xmltest.c index dc8cb04f..a2321d13 100644 --- a/tests/nodedevxml2xmltest.c +++ b/tests/nodedevxml2xmltest.c @@ -128,6 +128,7 @@ mymain(void) DO_TEST("ap_card07"); DO_TEST("ap_07_0038"); DO_TEST("ap_matrix"); + DO_TEST("ap_matrix_mdev_types"); DO_TEST("mdev_ee0b88c4_f554_4dc1_809d_b2a01e8e48ad");
return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE;
Reviewed-by: Jonathon Jongsma <jjongsma@redhat.com>
On 11/12/20 10:08 PM, Jonathon Jongsma wrote:
diff --git a/tests/nodedevschemadata/ap_matrix_mdev_types.xml b/tests/nodedevschemadata/ap_matrix_mdev_types.xml new file mode 100644 index 00000000..0ca83680 --- /dev/null +++ b/tests/nodedevschemadata/ap_matrix_mdev_types.xml @@ -0,0 +1,14 @@ +<device> + <name>ap_matrix</name> + <path>/sys/devices/vfio_ap/matrix</path> + <parent>computer</parent> + <capability type='ap_matrix'> + <capability type='mdev_types'> + <type id='vfio_api-passthrough'> nit: I guess this is just an arbitrary string representing an mdev type, but I would guess that you intended it to say vfio_ap- instead of vfio_api-?
Your guess is correct. It must read: 'vfio_ap-passthrough' Good catch!
+ <name>VFIO AP Passthrough Device</name> + <deviceAPI>vfio-ap</deviceAPI> + <availableInstances>65536</availableInstances> + </type> + </capability> + </capability> +</device> diff --git a/tests/nodedevxml2xmltest.c b/tests/nodedevxml2xmltest.c index dc8cb04f..a2321d13 100644 --- a/tests/nodedevxml2xmltest.c +++ b/tests/nodedevxml2xmltest.c @@ -128,6 +128,7 @@ mymain(void) DO_TEST("ap_card07"); DO_TEST("ap_07_0038"); DO_TEST("ap_matrix"); + DO_TEST("ap_matrix_mdev_types"); DO_TEST("mdev_ee0b88c4_f554_4dc1_809d_b2a01e8e48ad");
return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE;
Reviewed-by: Jonathon Jongsma<jjongsma@redhat.com>
Thanks, Jonathon! -- Mit freundlichen Grüßen/Kind regards Boris Fiuczynski IBM Deutschland Research & Development GmbH Vorsitzender des Aufsichtsrats: Gregor Pillen Geschäftsführung: Dirk Wittkopp Sitz der Gesellschaft: Böblingen Registergericht: Amtsgericht Stuttgart, HRB 243294
participants (5)
-
Bjoern Walk -
Boris Fiuczynski -
Erik Skultety -
Jonathon Jongsma -
Shalini Chellathurai Saroja