Re: [Qemu-devel] [RFC v2 0/4] adding mdev bus and vfio support

[Neo Jia]

On Wed, Sep 07, 2016 at 10:22:26AM +0800, Jike Song wrote:
> On 09/02/2016 11:03 PM, Alex Williamson wrote:
> > On Fri,  2 Sep 2016 16:16:08 +0800
> > Jike Song <address@hidden> wrote:
> > 
> >> This patchset is based on NVidia's "Add Mediated device support" series, 
> >> version 6:
> >>
> >>    http://www.spinics.net/lists/kvm/msg136472.html
> > 
> > 
> > Hi Jike,
> > 
> > I'm thrilled by your active participation here, but I'm confused which
> > versions I should be reviewing and where the primary development is
> > going.  Kirti sent v7 a week ago, so I would have expected a revision
> > based on that rather than a re-write based on v6 plus incorporation of a
> > few of Kirti's patches directly.
> 
> Hi Alex,
> 
> [Sorry! replied this on Monday but it was silently dropped by the our 
> firewall]
> 
> 
> 
> The v1 of this patchset was send as incremental ones, basing on Nvidia's v6, 
> to
> demonstrate how is it possible and beneficial to:
> 
>       1, Introduce an independent device between physical and mdev;
>       2, Simplify vfio-mdev and make it the most flexible for vendor drivers;
> 
> Unfortunately neither was understood or adopted in v7:
> 
>       http://www.spinics.net/lists/kvm/msg137081.html
> 
> So here came the v2, as a standalone series, to give a whole and straight
> demonstration. The reason of still basing on v6:
> 
>       - Addressed all v6 comments (except the iommu part);
>       - There is no comments yet for v7 (except the sysfs ones);
> 
> 
> 
> > I liked the last version of these
> > changes a lot, but we need to figure out how to combine development
> > because we do not have infinite cycles for review available :-\  Thanks!
> 
> Fully understand.
> 
> Here is the dilemma: v6 is an obsolete version to work upon, v7 is still not
> at the direction we prefer. 

Hi Jike,

I wish I could meet you in person in KVM forum couple weeks ago so we can have a
better discussion.

We are trying our best to accommodate almost all requirements / comments from 
use cases and code reviews while keeping little (or none) architectural changes
between revisions.

> We would be highly glad and thankful if Neo/Kirti
> would adopt the code in their next version, which will certainly form a
> more simple and consolidated base for future co-development; otherwise
> and we could at least discuss the concerns, in case of any.
> 

As I have said in my previous response to you, if you have any questions about
adopting the framework that we have developed, you are very welcome to
comment/speak out on the code review thread like others. And if it is reasonable
request and won't break other vendors' use case, we will adopt it (one example
is the online file and removing the mdev pci dependency).

Just some update for you regarding the v7 patches, currently we are very
actively trying to lock down the sysfs and management interfaces discussion.

So, if you would like to make the upstream happen sooner, please join us in the
v7 and following patch discussion instead of rewriting them.

Thanks,
Neo

> 
> --
> Thanks,
> Jike
> 
> >>
> >>
> >> Key Changes from Nvidia v6:
> >>
> >>    - Introduced an independent struct device to host device, thereby
> >>      formed a physical-host-mdev hierarchy, and highly reused Linux
> >>      driver core support;
> >>
> >>    - Added online/offline to mdev_bus_type, leveraging the 'online'
> >>      attr support from Linux driver core;
> >>
> >>    - Removed mdev_class and other unecessary stuff;
> >>
> >>    /*
> >>     * Given the changes above, the code volume of mdev core driver
> >>     * dramatically reduced by ~50%.
> >>     */
> >>
> >>
> >>    - Interfaces between vfio_mdev and vendor driver are high-level,
> >>      e.g. ioctl instead of get_irq_info/set_irq_info and reset,
> >>      start/stop became mdev oriented, etc.;
> >>
> >>    /*
> >>     * Given the changes above, the code volume of mdev core driver
> >>     * dramatically reduced by ~64%.
> >>     */
> >>
> >>
> >> Test
> >>
> >>    - Tested with KVMGT
> >>
> >> TODO
> >>
> >>    - Re-implement the attribute group of host device as long as the
> >>      sysfs hierarchy in discussion gets finalized;
> >>
> >>    - Move common routines from current vfio-pci into a higher location,
> >>      export them for various VFIO bus drivers and/or mdev vendor drivers;
> >>
> >>    - Add implementation examples for vendor drivers to Documentation;
> >>
> >>    - Refine IOMMU changes
> >>
> >>
> >>
> >> Jike Song (2):
> >>   Mediated device Core driver
> >>   vfio: VFIO bus driver for MDEV devices
> >>
> >> Kirti Wankhede (2):
> >>   vfio iommu: Add support for mediated devices
> >>   docs: Add Documentation for Mediated devices
> >>
> >>  Documentation/vfio-mediated-device.txt | 203 ++++++++++++++
> >>  drivers/vfio/Kconfig                   |   1 +
> >>  drivers/vfio/Makefile                  |   1 +
> >>  drivers/vfio/mdev/Kconfig              |  18 ++
> >>  drivers/vfio/mdev/Makefile             |   5 +
> >>  drivers/vfio/mdev/mdev_core.c          | 250 +++++++++++++++++
> >>  drivers/vfio/mdev/mdev_driver.c        | 155 ++++++++++
> >>  drivers/vfio/mdev/mdev_private.h       |  29 ++
> >>  drivers/vfio/mdev/mdev_sysfs.c         | 155 ++++++++++
> >>  drivers/vfio/mdev/vfio_mdev.c          | 187 ++++++++++++
> >>  drivers/vfio/vfio.c                    |  82 ++++++
> >>  drivers/vfio/vfio_iommu_type1.c        | 499 
> >> +++++++++++++++++++++++++++++----
> >>  include/linux/mdev.h                   | 159 +++++++++++
> >>  include/linux/vfio.h                   |  13 +-
> >>  14 files changed, 1709 insertions(+), 48 deletions(-)
> >>  create mode 100644 Documentation/vfio-mediated-device.txt
> >>  create mode 100644 drivers/vfio/mdev/Kconfig
> >>  create mode 100644 drivers/vfio/mdev/Makefile
> >>  create mode 100644 drivers/vfio/mdev/mdev_core.c
> >>  create mode 100644 drivers/vfio/mdev/mdev_driver.c
> >>  create mode 100644 drivers/vfio/mdev/mdev_private.h
> >>  create mode 100644 drivers/vfio/mdev/mdev_sysfs.c
> >>  create mode 100644 drivers/vfio/mdev/vfio_mdev.c
> >>  create mode 100644 include/linux/mdev.h
> >>
> >