[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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



On Wed, 9 Sep 2020 08:18:08 +0200
Bjoern Walk <bwalk linux ibm com> wrote:

> Erik Skultety <eskultet 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 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 linux ibm com>
> > > Signed-off-by: Boris Fiuczynski <fiuczy 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.

Attachment: pgpjxoTD2Adwx.pgp
Description: OpenPGP digital signature


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]