[PATCH libvirt v2 01/11] nodedev: detect AP card device

Jonathon Jongsma jjongsma at redhat.com
Thu Nov 12 16:27:37 UTC 2020


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




More information about the libvir-list mailing list