[PATCH 2/5] node_device: detect CSS devices
Cornelia Huck
cohuck at redhat.com
Wed Sep 9 07:03:36 UTC 2020
On Wed, 9 Sep 2020 08:18:08 +0200
Bjoern Walk <bwalk at linux.ibm.com> wrote:
> Erik Skultety <eskultet at redhat.com> [2020-09-08, 05:46PM +0200]:
> > 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?
>
> Yes, several typically. One for each I/O device attached to the LPAR.
>
> >
> > > 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?
>
> 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_SA22-7832-03.pdf
>
> 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.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 833 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20200909/268c60be/attachment-0001.sig>
More information about the libvir-list
mailing list