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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Sat, 23 Nov 2019 19:09:48 -0400
From:   Jason Gunthorpe <jgg@...pe.ca>
To:     Tiwei Bie <tiwei.bie@...el.com>
Cc:     Jason Wang <jasowang@...hat.com>,
        Alex Williamson <alex.williamson@...hat.com>,
        "Michael S. Tsirkin" <mst@...hat.com>,
        Parav Pandit <parav@...lanox.com>,
        Jeff Kirsher <jeffrey.t.kirsher@...el.com>,
        davem@...emloft.net, gregkh@...uxfoundation.org,
        Dave Ertman <david.m.ertman@...el.com>, netdev@...r.kernel.org,
        linux-rdma@...r.kernel.org, nhorman@...hat.com,
        sassmann@...hat.com, Kiran Patil <kiran.patil@...el.com>
Subject: Re: [net-next v2 1/1] virtual-bus: Implementation of Virtual Bus

On Sat, Nov 23, 2019 at 12:39:51PM +0800, Tiwei Bie wrote:
> On Fri, Nov 22, 2019 at 02:02:14PM -0400, Jason Gunthorpe wrote:
> > On Fri, Nov 22, 2019 at 04:45:38PM +0800, Jason Wang wrote:
> > > On 2019/11/21 下午10:17, Jason Gunthorpe wrote:
> > > > On Thu, Nov 21, 2019 at 03:21:29PM +0800, Jason Wang wrote:
> > > > > > The role of vfio has traditionally been around secure device
> > > > > > assignment of a HW resource to a VM. I'm not totally clear on what the
> > > > > > role if mdev is seen to be, but all the mdev drivers in the tree seem
> > > > > > to make 'and pass it to KVM' a big part of their description.
> > > > > > 
> > > > > > So, looking at the virtio patches, I see some intended use is to map
> > > > > > some BAR pages into the VM.
> > > > > Nope, at least not for the current stage. It still depends on the
> > > > > virtio-net-pci emulatio in qemu to work. In the future, we will allow such
> > > > > mapping only for dorbell.
> > > > There has been a lot of emails today, but I think this is the main
> > > > point I want to respond to.
> > > > 
> > > > Using vfio when you don't even assign any part of the device BAR to
> > > > the VM is, frankly, a gigantic misuse, IMHO.
> > > 
> > > That's not a compelling point. 
> > 
> > Well, this discussion is going nowhere.
> 
> You removed JasonW's other reply in above quote. He said it clearly
> that we do want/need to assign parts of device BAR to the VM.

Generally we don't look at patches based on stuff that isn't in them.

> > I mean the library functions in the kernel that vfio uses to implement
> > all the user dma stuff. Other subsystems use them too, it is not
> > exclusive to vfio.
> 
> IIUC, your point is to suggest us invent new DMA API for userspace to
> use instead of leveraging VFIO's well defined DMA API. Even if we don't
> use VFIO at all, I would imagine it could be very VFIO-like (e.g. caps
> for BAR + container/group for DMA) eventually.

None of the other user dma subsystems seem to have the problems you
are imagining here. Perhaps you should try it first?
 
> > > > 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.

Jason