[PATCH 2/5] node_device: detect CSS devices

[Erik Skultety]

On Mon, Aug 24, 2020 at 01:59:12PM +0200, Bjoern Walk wrote:
> From: Boris Fiuczynski <fiuczy at linux.ibm.com>
>
> Make channel subsystem (CSS) devices available in the node_device driver.o

Can there be more CSS devices per CPC?

> The CCS devices reside in the computer system and provide CCW devices, e.g.:
>
>   +- css_0_0_003a
>       |
>       +- ccw_0_0_1a2b
>           |
>           +- scsi_host0
>               |
>               +- scsi_target0_0_0
>                   |
>                   +- scsi_0_0_0_0
>
> Reviewed-by: Bjoern Walk <bwalk at linux.ibm.com>
> Signed-off-by: Boris Fiuczynski <fiuczy at linux.ibm.com>
> ---
>  docs/schemas/nodedev.rng                      | 16 ++++++++++++++
>  src/conf/node_device_conf.c                   |  5 +++++
>  src/conf/node_device_conf.h                   |  1 +
>  src/conf/virnodedeviceobj.c                   |  1 +
>  src/node_device/node_device_udev.c            | 22 +++++++++++++++++++
>  .../ccw_0_0_10000-invalid.xml                 |  4 ++--
>  tests/nodedevschemadata/ccw_0_0_ffff.xml      |  4 ++--
>  tests/nodedevschemadata/css_0_0_ffff.xml      | 10 +++++++++
>  tests/nodedevxml2xmltest.c                    |  1 +
>  tools/virsh-nodedev.c                         |  1 +
>  10 files changed, 61 insertions(+), 4 deletions(-)
>  create mode 100644 tests/nodedevschemadata/css_0_0_ffff.xml
>
> diff --git a/docs/schemas/nodedev.rng b/docs/schemas/nodedev.rng
> index 4b2b350f..f7f517b5 100644
> --- a/docs/schemas/nodedev.rng
> +++ b/docs/schemas/nodedev.rng
> @@ -85,6 +85,7 @@
>          <ref name="capdrm"/>
>          <ref name="capmdev"/>
>          <ref name="capccwdev"/>
> +        <ref name="capcssdev"/>
>        </choice>
>      </element>
>    </define>
> @@ -659,6 +660,21 @@
>      </element>
>    </define>
>
> +  <define name='capcssdev'>
> +    <attribute name='type'>
> +      <value>css</value>
> +    </attribute>
> +    <element name='cssid'>

Is ^this one just a different name for CHPID or those are completely unrelated?

> +      <ref name='ccwCssidRange'/>
> +    </element>
> +    <element name='ssid'>
> +      <ref name='ccwSsidRange'/>
> +    </element>
> +    <element name='devno'>
> +      <ref name='ccwDevnoRange'/>
> +    </element>
> +  </define>
> +

Are ^these attributes documented a little more somewhere? I didn't find those
in [1]. I suppose it is available in the z/Architecture: Principles of
Operation publication for which I'd have to get an IBM account.

[snip]

>
>
> +static int
> +udevProcessCSS(struct udev_device *device,
> +               virNodeDeviceDefPtr def)
> +{
> +    /* do not process EADM and CHSC devices to keep the list sane */
> +    if (STREQ(def->driver, "eadm_subchannel") ||
> +        STREQ(def->driver, "chsc_subchannel"))

[2] Also mentions message subchannel, although apparently there are no Linux
kernel drivers for that yet, IIUC that one would have to be added here as well
as some point, is that correct?

> +        return -1;
> +
> +    if (udevGetCCWAddress(def->sysfs_path, &def->caps->data) < 0)
> +        return -1;
> +
> +    if (udevGenerateDeviceName(device, def, NULL) != 0)
> +        return -1;
> +
> +    return 0;
> +}
> +
>  static int
>  udevGetDeviceNodes(struct udev_device *device,
>                     virNodeDeviceDefPtr def)
> @@ -1175,6 +1193,8 @@ udevGetDeviceType(struct udev_device *device,
>              *type = VIR_NODE_DEV_CAP_MDEV;
>          else if (STREQ_NULLABLE(subsystem, "ccw"))
>              *type = VIR_NODE_DEV_CAP_CCW_DEV;
> +        else if (STREQ_NULLABLE(subsystem, "css"))
> +            *type = VIR_NODE_DEV_CAP_CSS_DEV;
>
>          VIR_FREE(subsystem);
>      }
> @@ -1219,6 +1239,8 @@ udevGetDeviceDetails(struct udev_device *device,
>          return udevProcessMediatedDevice(device, def);
>      case VIR_NODE_DEV_CAP_CCW_DEV:
>          return udevProcessCCW(device, def);
> +    case VIR_NODE_DEV_CAP_CSS_DEV:
> +        return udevProcessCSS(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/tests/nodedevschemadata/ccw_0_0_10000-invalid.xml b/tests/nodedevschemadata/ccw_0_0_10000-invalid.xml
> index d840555c..f3cf0c1c 100644
> --- a/tests/nodedevschemadata/ccw_0_0_10000-invalid.xml
> +++ b/tests/nodedevschemadata/ccw_0_0_10000-invalid.xml
> @@ -1,7 +1,7 @@
>  <device>
>    <name>ccw_0_0_10000</name>
> -  <path>/sys/devices/css0/0.0.0000/0.0.10000</path>
> -  <parent>computer</parent>
> +  <path>/sys/devices/css0/0.0.0070/0.0.10000</path>
> +  <parent>css_0_0_0070</parent>

Looking at the architecture diagram at [1], I'm wondering why haven't we add
the CSS channel right away and instead only added CCW devices, which are
basically just end points on the CSS subsystem.

The changes otherwise look good, so I'm inclined to give it an ACK, but I'd
like to understand CSS a bit more before that (since I don't have HW to try
this on)

Erik