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

Re: [PATCH 5/5] node_device: mdev vfio-ccw support



Erik Skultety <eskultet redhat com> [2020-09-08, 06:01PM +0200]:
> On Mon, Aug 24, 2020 at 01:59:15PM +0200, Bjoern Walk wrote:
> > From: Boris Fiuczynski <fiuczy linux ibm com>
> > +        case VIR_NODE_DEV_CAP_SYSTEM:
> > +        case VIR_NODE_DEV_CAP_USB_DEV:
> > +        case VIR_NODE_DEV_CAP_USB_INTERFACE:
> > +        case VIR_NODE_DEV_CAP_NET:
> > +        case VIR_NODE_DEV_CAP_SCSI_HOST:
> > +        case VIR_NODE_DEV_CAP_SCSI_TARGET:
> > +        case VIR_NODE_DEV_CAP_SCSI:
> > +        case VIR_NODE_DEV_CAP_STORAGE:
> > +        case VIR_NODE_DEV_CAP_FC_HOST:
> > +        case VIR_NODE_DEV_CAP_VPORTS:
> > +        case VIR_NODE_DEV_CAP_SCSI_GENERIC:
> > +        case VIR_NODE_DEV_CAP_DRM:
> > +        case VIR_NODE_DEV_CAP_MDEV_TYPES:
> > +        case VIR_NODE_DEV_CAP_MDEV:
> > +        case VIR_NODE_DEV_CAP_CCW_DEV:
> 
> What about ccw mdevs? I would think that it's CCW end point device that
> we would want to assign concurrently, but we're only "slicing" the root channel
> subsystem device. If you have and CSS mdev, are the CCW subchannels separate
> for each VM or does it behave kind of like NPIV with its vHBAs? OR I'm
> completely off here?

You are correct. This is an unfortunate architectural decision on our
platform. For vfio-ccw, we instatiate the mdevs on the CSS layer, which
has a 1-to-1 relation to the underlying CCW device. That's why we need
the information about the CSS devices in libvirt, to give the user a
chance to easily find this relation when he wants to do passthrough with
a DASD for example. It's a bit unintuitive and even silly for the
end user, but it's architecturally correct and hence it was implemented
like this in the vfio-ccw kernel driver. We don't even get the benefits
of something like NPIV because for every subchannel there is only one
CCW device (at least to my knowledge).

> 
> > +        case VIR_NODE_DEV_CAP_LAST:
> > +            continue;
> > +        }
> >      }
> >
> >      virNodeDeviceObjEndAPI(&dev);
> >
> > -    return pci_addr;
> > +    return addr;
> >  }
> >
> >
> > @@ -664,11 +695,11 @@ nodeDeviceGetMdevctlStartCommand(virNodeDeviceDefPtr def,
> >  {
> >      virCommandPtr cmd;
> >      g_autofree char *json = NULL;
> > -    g_autofree char *parent_pci = nodeDeviceFindAddressByName(def->parent);
> > +    g_autofree char *parent_addr = nodeDeviceFindAddressByName(def->parent);
> >
> > -    if (!parent_pci) {
> > +    if (!parent_addr) {
> >          virReportError(VIR_ERR_NO_NODE_DEVICE,
> > -                       _("unable to find PCI address for parent device '%s'"), def->parent);
> > +                       _("unable to find address for parent device '%s'"), def->parent);
> 
> I'm wondering whether "unable to find parent device '%s'" would not suffice,
> since we're not specifying what type of address we were not able to find - I'm
> not even sure the address information is important at all.
> 
> Erik
> 
> >          return NULL;
> >      }
> >
> > @@ -679,7 +710,7 @@ nodeDeviceGetMdevctlStartCommand(virNodeDeviceDefPtr def,
> >      }
> >
> >      cmd = virCommandNewArgList(MDEVCTL, "start",
> > -                               "-p", parent_pci,
> > +                               "-p", parent_addr,
> >                                 "--jsonfile", "/dev/stdin",
> >                                 NULL);
> >
> > --
> > 2.24.1
> >
> 

-- 
IBM Systems
Linux on Z & Virtualization Development
--------------------------------------------------
IBM Deutschland Research & Development GmbH
Schönaicher Str. 220, 71032 Böblingen
Phone: +49 7031 16 1819
--------------------------------------------------
Vorsitzende des Aufsichtsrats: Gregor Pillen
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294

Attachment: signature.asc
Description: PGP signature


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