On Sun, Nov 24, 2019 at 10:51:24PM +0800, Tiwei Bie wrote: > On Sat, Nov 23, 2019 at 07:09:48PM -0400, Jason Gunthorpe wrote: > > 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. > > The hardware is ready, and it's something really necessary (for > the performance). It was planned to be added immediately after > current series. If you want, it certainly can be included right now. It can't hurt, for sure. Can be a separate patch if you feel review is easier that way. > > > > > > 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? > > Actually VFIO DMA API wasn't used at the beginning of vhost-mdev. But > after the discussion in upstream during the RFC stage since the last > year, the conclusion is that leveraging VFIO's existing DMA API would > be the better choice and then vhost-mdev switched to that direction. > > > > > > > > > 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. > > No, I meant the same thing. Michael helped me explain that. > https://patchwork.ozlabs.org/patch/1195895/#2311180 > > > > > Jason
