From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([2001:4830:134:3::10]:55750)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <kevin.tian@intel.com>) id 1bm9oO-0001Mo-7Y
	for qemu-devel@nongnu.org; Mon, 19 Sep 2016 21:25:29 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <kevin.tian@intel.com>) id 1bm9oK-0006di-0v
	for qemu-devel@nongnu.org; Mon, 19 Sep 2016 21:25:27 -0400
Received: from mga06.intel.com ([134.134.136.31]:34264)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <kevin.tian@intel.com>) id 1bm9oJ-0006da-M1
	for qemu-devel@nongnu.org; Mon, 19 Sep 2016 21:25:23 -0400
From: "Tian, Kevin" <kevin.tian@intel.com>
Date: Tue, 20 Sep 2016 01:25:19 +0000
Message-ID: <AADFC41AFE54684AB9EE6CBC0274A5D18DF7EAD0@SHSMSX101.ccr.corp.intel.com>
References: <00d96f24-5df0-d16b-d4e1-838333989dee@nvidia.com>
	<20160919153600.70599974@t450s.home>
In-Reply-To: <20160919153600.70599974@t450s.home>
Content-Language: en-US
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
Subject: Re: [Qemu-devel] [RFC v2] libvirt vGPU QEMU integration
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel/>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Alex Williamson <alex.williamson@redhat.com>, Kirti Wankhede <kwankhede@nvidia.com>
Cc: "libvir-list@redhat.com" <libvir-list@redhat.com>, qemu-devel <qemu-devel@nongnu.org>, Paolo Bonzini <pbonzini@redhat.com>, Neo Jia <cjia@nvidia.com>, Andy Currid <ACurrid@nvidia.com>, "Song, Jike" <jike.song@intel.com>, "bjsdjshi@linux.vnet.ibm.com" <bjsdjshi@linux.vnet.ibm.com>, Gerd Hoffmann <kraxel@redhat.com>"Dong Jia
	(bjsdjshi@linux.vnet.ibm.com)" <bjsdjshi@linux.vnet.ibm.com>

> From: Alex Williamson [mailto:alex.williamson@redhat.com]
> Sent: Tuesday, September 20, 2016 5:36 AM
>=20
> > In the above example directory '11' represents a type id of mdev device=
.
> > 'name', 'fb_length', 'resolution', 'heads', 'max_instance' and
> > 'requires_group' would be Read-Only files that vendor would provide to
> > describe about that type.
> >
> > 'create':
> >     Write-only file. Mandatory.
> >     Accepts string to create mediated device.
> >
> > 'name':
> >     Read-Only file. Mandatory.
> >     Returns string, the name of that type id.
> >
> > 'fb_length':
> >     Read-only file. Mandatory.
> >     Returns <number>{K,M,G}, size of framebuffer.
>=20
> This can't be mandatory, it's only relevant to vGPU devices, vGPUs are
> just one user of mediated devices.

Agree. at least it doesn't make sense for s390 passthrough usage.

>=20
> >
> > 'max_instance':
> >     Read-Only file. Mandatory.
> >     Returns integer.  Returns maximum mdev device could be created
> > at the moment when this file is read. This count would be updated by
> > vendor driver. Before creating mdev device of this type, check if
> > max_instance is > 0.
>=20
> Didn't we discuss this being being something like "available_instances"
> with a "devices" sub-directory linking current devices of that type?

Right. "available_instances" which is dynamically changed under two scenari=
os:

a) create a new mdev under one type decrements "available_instances" of thi=
s
type by 1;
b) create a new mdev under one type decrements "available_instances" under
other types by type-specific number, if vendor supports creating multiple t=
ypes
simultaneously like in Intel solution;

>=20
> > 'params'
> >     Write-Only file. Optional.
> >     String input. Libvirt would pass the string given in XML file to
> > this file and then create mdev device. Set empty string to clear params=
.
> > For example, set parameter 'frame_rate_limiter=3D0' to disable frame ra=
te
> > limiter for performance benchmarking, then create device of type 11. Th=
e
> > device created would have that parameter set by vendor driver.
>=20
> Might as well just call it "magic", there's absolutely no ability to
> introspect what parameters are allowed or even valid here.  Can all
> parameters be changed dynamically?  Do parameter changes apply to
> existing devices or only future devices?  This is a poorly specified
> interface.  I'd prefer this be done via module options on the vendor
> driver.

I guess it might be mdev specific for nvidia's case, then module option
is not suitable. Then this magic string should be placed under mdev
directory instead of here, otherwise there'll be race condition.

>=20
> >
> > 2. Create/destroy mediated device
> >
> > With above example, vGPU device XML would look like:
> >
> >    <device>
> >      <name>my-vgpu</name>
> >      <parent>pci_0000_86_00_0</parent>
> >      <capability type=3D'mdev'>
> >        <type id=3D'11'/>
> >        <group>1</group>
> >        <params>'frame_rate_limiter=3D0'</params>
> >      </capability>
> >    </device>
> >
> > 'type id' is mandatory.
>=20
> I was under the impression that the vendor supplied "name" was the one
> unique identifier.  We're certainly not going to create a registrar to
> hand out type ids to each vendor, so what makes type id unique?  I have
> a hard time believing that a given vendor can even allocate unique type
> ids for their own devices.  Unique type id across vendors is not
> practical.  So which attribute are we actually using to identify which
> type of mdev device we need and why would we ever specify additional
> attributes like fb_length?  Doesn't the vendor guarantee that "GRID
> M60-0B" has a fixed setup of those attributes?

I'm a bit confused about type ID and type name here. Just keeping one=20
should be enough (ether a number or descriptive name), which is=20
per-device scope, not necessarily to cross vendors. As long as the vendor
defines the ID or name unique cross its own generations, looks should be
sufficient (I guess cross-vendor migration is not an interest here)

>=20
> > 'params' is optional field. User should set this field if extra
> > parameters need to be set for a particular vGPU device. Libvirt don't
> > need to parse these params. These are meant for vendor driver.
>=20
> ie. magic black hole.  nope.

A curious question here, though it's not a mandatory requirement for
Intel's GPU virtualization solution now, but allow such extension might
be useful in the long term. Has libvirt ever supported similar black=20
string in other usages? I can think it's a problem for public cloud usage,
where any configuration on the managed device should be controlled
by orchestration layer (e.g. openstack) automatically. It's not appropriate
to allow a cloud tenant specifying such parameter, so in that manner
any configuration parameter should be explicitly understood from upper
level stack to libvirt then vendor specific parameters would be hard to
handle. On the other hand, if we consider enterprise virtualization=20
scenario which is more proprietary, administrator may add such vendor
specific parameters as a black string through libvirt, as long as he
knows the underlying capability on purchased devices. Then is the latter
a valid value to consider such extension? :-)

Thanks
Kevin