Subject: Re: [net-next v2 1/1] virtual-bus: Implementation of Virtual Bus

From: Jason Gunthorpe <[email-masked]>
Date: Sun, 24 Nov 2019 20:07:20 -0400

On Sun, Nov 24, 2019 at 06:00:23AM -0500, Michael S. Tsirkin wrote:
> On Sat, Nov 23, 2019 at 07:09:48PM -0400, Jason Gunthorpe wrote:
> > > > > > Further, I do not think it is wise to design the userspace ABI around
> > > > > > a simplistict implementation that can't do BAR assignment,
> > > > > 
> > > > > Again, the vhost-mdev follow the VFIO ABI, no new ABI is invented, and
> > > > > mmap() was kept their for mapping device regions.
> > > > 
> > > > The patches have a new file in include/uapi.
> > > 
> > > I guess you didn't look at the code. Just to clarify, there is no
> > > new file introduced in include/uapi. Only small vhost extensions to
> > > the existing vhost uapi are involved in vhost-mdev.
> > 
> > You know, I review alot of patches every week, and sometimes I make
> > mistakes, but not this time. From the ICF cover letter:
> > 
> > https://lkml.org/lkml/2019/11/7/62
> > 
> >  drivers/vfio/mdev/mdev_core.c    |  21 ++
> >  drivers/vhost/Kconfig            |  12 +
> >  drivers/vhost/Makefile           |   3 +
> >  drivers/vhost/mdev.c             | 556 +++++++++++++++++++++++++++++++
> >  include/linux/mdev.h             |   5 +
> >  include/uapi/linux/vhost.h       |  21 ++
> >  include/uapi/linux/vhost_types.h |   8 +
> >       ^^^^^^^^^^^^^^
> > 
> > Perhaps you thought I ment ICF was adding uapi? My remarks cover all
> > three of the series involved here.
> 
> Tiwei seems to be right - include/uapi/linux/vhost.h and
> include/uapi/linux/vhost_types.h are both existing files.  vhost uapi
> extensions included here are very modest. They
> just add virtio spec things that vhost was missing.

Sigh, fine whatever, I mispoke and called the 7 new ioctls a 'new
file' instead of 'new ioctls' when responding to someone who denied
they even existed. 

Anyhow why do both of you keep saying "small vhost extensions to the
existing vhost uapi" when these 7 new ioctls appear to be connected to
vfio_device_ops, and /dev/vfio ?

Oh, gross, this is running some existing ioctl interface over
/dev/vfio - the new uABI here is really putting all 10 new ioctls on
/dev/vfio that didn't exist there before.

Jason