Re: [PATCH 2/5] node_device: detect CSS devices

> > > 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? > > As far as I understood, there is a 1-to-1 relation between CHPIDs and > channel paths, but they are not the same. For example, there are only > 256 CHPIDs per system available, compared to 2^16 channel paths. 'CHPID' is short for 'channel path identifier'. You can have up to 256 channel paths per channel subsystem image (a given LPAR only sees one channel subsystem image[a]). Any subchannel can use up to 8 channel paths, and a given channel path is usually used by many subchannels. The 'cssid' is the number of the channel subsystem image for the subchannel.[b] > > > > > > + <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 [BTW: what are [1] and [2] referring to?]

> > Operation publication for which I'd have to get an IBM account. > > > > Here's a freely available version of the PoP: > > https://www.ibm.com/support/pages/sites/default/files/inline-files/690450... > > I/O is described in chapter 13. Let me also point to a series of articles I did on my blog: https://virtualpenguins.blogspot.com/2017/02/channel-io-demystified.html

> > > [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? > > We have never seen those, but we would want to add them here if they > have no relevance for the user. I think you want to filter message subchannels as well, my assumption is that libvirt will only care about I/O subchannels.

> > > > > > + 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. > > Simply because they were not relevant for the user at that time. For the > typical Linux user, the channel subsystem was an implementaion detail of > I/O and every interaction with I/O devices happenend via the > corresponding CCW device. That's why I didn't implement them in the > first place. This changes now (unfortunately) with mdevs, so we might > want this information in libvirt.

> > > > > 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) > > Thanks a bunch. Understandably, I am not too deeply into I/O and I am > also confused more times than I'd like to admit. Maybe Boris can give > some more details. I know I/O, but I'm not that deeply into libvirt; I hope I could help a bit nevertheless :)

[a] You could have more than one channel subsystem image visible in theory, if something called MCSS-E is available and enabled by the operating system. AFAIK, the only implementation of MCSS-E is the one in QEMU, and there has never been any operating system that supported it, so it's probably best to just ignore this. [b] Always 0 on LPARs, and within guests. We wanted to use 0xfe for virtio devices (see QEMU device definitions), but as MCSS-E turned out to be never properly implemented, they show up as 0 as well, which makes it a bit useless.