Re: [Qemu-devel] [PATCH v6 1/4] vfio: Mediated device Core driver

[Alex Williamson]

On Tue, 16 Aug 2016 13:30:06 -0700
Neo Jia <address@hidden> wrote:

> On Mon, Aug 15, 2016 at 04:47:41PM -0600, Alex Williamson wrote:
> > On Mon, 15 Aug 2016 12:59:08 -0700
> > Neo Jia <address@hidden> wrote:
> >   
> > > > > >
> > > > > > I'm not sure a comma separated list makes sense here, for both
> > > > > > simplicity in the kernel and more fine grained error reporting, we
> > > > > > probably want to start/stop them individually.  Actually, why is it
> > > > > > that we can't use the mediated device being opened and released to
> > > > > > automatically signal to the backend vendor driver to commit and 
> > > > > > release
> > > > > > resources? I don't fully understand why userspace needs this 
> > > > > > interface.    
> > > >   
> > 
> > That doesn't give an individual user the ability to stop and start
> > their devices though, because in order for a user to have write
> > permissions there, they get permission to DoS other users by pumping
> > arbitrary UUIDs into those files.  By placing start/stop per mdev, we
> > have mdev level granularity of granting start/stop privileges.  Really
> > though, do we want QEMU fumbling around through sysfs or do we want an
> > interface through the vfio API to perform start/stop?  Thanks,  
> 
> Hi Alex,
> 
> I think those two suggests make sense, so we will move the "start/stop"
> under mdev sysfs. 
> 
> This will be incorporated in our next v7 patch and by doing that, it will make
> the locking scheme easier.

Thanks Neo.  Also note that the semantics change when we move to per
device control.  It would be redundant to 'echo $UUID' into a start
file which only controls a single device.  So that means we probably
just want an 'echo 1'.  But if we can 'echo 1' then we can also 'echo
0', so we can reduce this to a single sysfs file.  Sysfs already has a
common interface for starting and stopping devices, the "online" file.
So I think we should probably move in that direction.  Additionally, an
"online" file should support a _show() function, so if we have an Intel
vGPU that perhaps does not need start/stop support, online could report
"1" after create to show that it's already online, possibly even
generate an error trying to change the online state.  Thanks,

Alex