From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([2001:4830:134:3::10]:58916)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <alex.williamson@redhat.com>) id 1bmMP2-0001zI-Vt
	for qemu-devel@nongnu.org; Tue, 20 Sep 2016 10:52:10 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <alex.williamson@redhat.com>) id 1bmMOy-00010T-Oj
	for qemu-devel@nongnu.org; Tue, 20 Sep 2016 10:52:08 -0400
Received: from mx1.redhat.com ([209.132.183.28]:50940)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <alex.williamson@redhat.com>) id 1bmMOy-000100-FX
	for qemu-devel@nongnu.org; Tue, 20 Sep 2016 10:52:04 -0400
Date: Tue, 20 Sep 2016 08:52:02 -0600
From: Alex Williamson <alex.williamson@redhat.com>
Message-ID: <20160920085202.76bf4e14@t450s.home>
In-Reply-To: <32b91537-0d83-a312-db19-7341650c3d4a@nvidia.com>
References: <00d96f24-5df0-d16b-d4e1-838333989dee@nvidia.com>
	<20160919153600.70599974@t450s.home>
	<8e7e7357-cad4-f9ca-4104-97cadd347a13@redhat.com>
	<20160919162521.3569caa2@t450s.home>
	<32b91537-0d83-a312-db19-7341650c3d4a@nvidia.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit
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: Kirti Wankhede <kwankhede@nvidia.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>, "libvir-list@redhat.com" <libvir-list@redhat.com>, qemu-devel <qemu-devel@nongnu.org>, Neo Jia <cjia@nvidia.com>, Andy Currid <ACurrid@nvidia.com>, "Tian, Kevin" <kevin.tian@intel.com>, "Song, Jike" <jike.song@intel.com>, "bjsdjshi@linux.vnet.ibm.com" <bjsdjshi@linux.vnet.ibm.com>, Gerd Hoffmann <kraxel@redhat.com>

On Tue, 20 Sep 2016 20:05:01 +0530
Kirti Wankhede <kwankhede@nvidia.com> wrote:

> On 9/20/2016 3:55 AM, Alex Williamson wrote:
> > On Mon, 19 Sep 2016 23:50:56 +0200
> > Paolo Bonzini <pbonzini@redhat.com> wrote:
> >   
> >> On 19/09/2016 23:36, Alex Williamson wrote:  
> >>> On Tue, 20 Sep 2016 02:05:52 +0530
> >>> Kirti Wankhede <kwankhede@nvidia.com> wrote:    
> >>>> 'fb_length':
> >>>>     Read-only file. Mandatory.
> >>>>     Returns <number>{K,M,G}, size of framebuffer.    
> >>>
> >>> This can't be mandatory, it's only relevant to vGPU devices, vGPUs are
> >>> just one user of mediated devices. [...]
> >>>     
> >>>> '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=0' to disable frame rate
> >>>> limiter for performance benchmarking, then create device of type 11. The
> >>>> device created would have that parameter set by vendor driver.    
> >>>
> >>> 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.    
> >>
> >> Or additional files in the mdev's sysfs directory?  
> > 
> > Sure, the vendor driver could certainly support that, it'd be vendor
> > specific of course.
> >  
> 
> Right, it would be vendor specific. But if this is not set through mdev
> device XML, user have to create mdev device with XML first and then
> manually set such parameter. Also these parameters would not retain if
> mdev device is destroyed and created again. That's why if libvirt sets
> these parameters by reading it from XML file, it would be simple for user.
> 
> If we move 'params' file to mdev device, could libvirt do the following
> to create mdev device:
> 
> * Read /sys/../0000\:86\:00.0/11/max_instances. If it is greater than 0,
> then only proceed else fail.
> 
> * Autogenerate UUID
> * Create device:
> 
>     echo "$UUID" > /sys/../0000\:86\:00.0/11/create
> 
> * Set extra params if 'params' field exist in device XML and 'params'
> file exist mdev device directory:
> 
>     echo "frame_rate_limiter=0" > /sys/bus/mdev/devices/$UUID/params

Why does libvirt even need to participate in setting these additional
parameters?  The example here is for benchmarking, which doesn't
express to me a strong need to be automated through libvirt.  For
hostdev devices libvirt supports a managed='no' option that requires
the user to setup the device in advance.  Something similar sounds
reasonable here too.

This is also where I see that a vendor module option could be used,
something like:

modprobe nvidia enable-no-frl-mdev

You say you don't want to apply this to all mdev devices, but maybe it
just makes an additional type-id available "GRID-M60-0B-NOFRL".  Then
you've removed libvirt from the equation, the VM simply specifies this
type-id that incorporates that feature.  Obviously it gets complicated
if there are multiple features to enable, but we've only seen one
example so far of why this parameter is needed.  Thanks,

Alex