On Thu, 12 Nov 2020 13:15:09 +0100 Shalini Chellathurai Saroja <shal...@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 <al...@linux.ibm.com> > Signed-off-by: Shalini Chellathurai Saroja <shal...@linux.ibm.com> > Reviewed-by: Bjoern Walk <bw...@linux.ibm.com> > Reviewed-by: Boris Fiuczynski <fiu...@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; > }