On Thu, 12 Nov 2020 13:15:19 +0100
Shalini Chellathurai Saroja <shalini@xxxxxxxxxxxxx> wrote:
> From: Boris Fiuczynski <fiuczy@xxxxxxxxxxxxx>
>
> Add detection of mdev_types capability to Adjunct Processor Matrix
> device.
>
> Signed-off-by: Boris Fiuczynski <fiuczy@xxxxxxxxxxxxx>
> Signed-off-by: Shalini Chellathurai Saroja <shalini@xxxxxxxxxxxxx>
> ---
> 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@xxxxxxxxxx>
