From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:46257) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bfxuy-0004qh-7u for qemu-devel@nongnu.org; Fri, 02 Sep 2016 19:30:41 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bfxuv-0002EQ-2F for qemu-devel@nongnu.org; Fri, 02 Sep 2016 19:30:40 -0400 Received: from hqemgate16.nvidia.com ([216.228.121.65]:10224) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bfxuu-0002EF-PY for qemu-devel@nongnu.org; Fri, 02 Sep 2016 19:30:36 -0400 Date: Fri, 2 Sep 2016 16:30:31 -0700 From: Neo Jia Message-ID: <20160902233031.GA3886@nvidia.com> References: <1472804172-25542-1-git-send-email-jike.song@intel.com> <1472804172-25542-5-git-send-email-jike.song@intel.com> <72d0964e-492c-41d8-596e-cc42a8d1ea50@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Disposition: inline In-Reply-To: <72d0964e-492c-41d8-596e-cc42a8d1ea50@redhat.com> Subject: Re: [Qemu-devel] [RFC v2 4/4] docs: Add Documentation for Mediated devices List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eric Blake Cc: Jike Song , alex.williamson@redhat.com, kwankhede@nvidia.com, kevin.tian@intel.com, guangrong.xiao@linux.intel.com, kvm@vger.kernel.org, qemu-devel@nongnu.org, zhenyuw@linux.intel.com, zhiyuan.lv@intel.com, pbonzini@redhat.com, bjsdjshi@linux.vnet.ibm.com, kraxel@redhat.com On Fri, Sep 02, 2016 at 05:09:46PM -0500, Eric Blake wrote: > * PGP Signed by an unknown key > > On 09/02/2016 03:16 AM, Jike Song wrote: > > From: Kirti Wankhede > > > > Add file Documentation/vfio-mediated-device.txt that include details of > > mediated device framework. > > > > Signed-off-by: Kirti Wankhede > > Signed-off-by: Neo Jia > > Signed-off-by: Jike Song > > --- > > Documentation/vfio-mediated-device.txt | 203 +++++++++++++++++++++++++++++++++ > > 1 file changed, 203 insertions(+) > > create mode 100644 Documentation/vfio-mediated-device.txt > > > > diff --git a/Documentation/vfio-mediated-device.txt b/Documentation/vfio-mediated-device.txt > > new file mode 100644 > > index 0000000..39bdcd9 > > --- /dev/null > > +++ b/Documentation/vfio-mediated-device.txt > > @@ -0,0 +1,203 @@ > > +VFIO Mediated devices [1] > > +------------------------------------------------------------------------------- > > Many files under Documentation trim the ---- decorator to the length of > the line above. > > Also, since you have no explicit copyright/license notice, your > documentation is under GPLv2+ per the top level. Other files do this, > and if you are okay with it, I won't complain; but if you intended > something else, or even if you wanted to make it explicit rather than > implicit, then you may want to copy the example of files that call out a > quick blurb on copyright and licensing. > Hi Eric, Thanks for the review and really sorry about the extra email threads of this review, we already have one actively going on for a while starting from RFC to currently v7. http://www.spinics.net/lists/kvm/msg137208.html And the related latest v7 document is at: http://www.spinics.net/lists/kvm/msg137210.html We will address all your review comments there. Thanks, Neo > > + > > +There are more and more use cases/demands to virtualize the DMA devices which > > +doesn't have SR_IOV capability built-in. To do this, drivers of different > > s/doesn't/don't/ > > > +devices had to develop their own management interface and set of APIs and then > > +integrate it to user space software. We've identified common requirements and > > +unified management interface for such devices to make user space software > > +integration easier. > > + > > +The VFIO driver framework provides unified APIs for direct device access. It is > > +an IOMMU/device agnostic framework for exposing direct device access to > > +user space, in a secure, IOMMU protected environment. This framework is > > +used for multiple devices like GPUs, network adapters and compute accelerators. > > +With direct device access, virtual machines or user space applications have > > +direct access of physical device. This framework is reused for mediated devices. > > + > > +Mediated core driver provides a common interface for mediated device management > > +that can be used by drivers of different devices. This module provides a generic > > +interface to create/destroy mediated device, add/remove it to mediated bus > > s/mediated/a mediated/ twice > > > +driver, add/remove device to IOMMU group. It also provides an interface to > > s/add/and add/ > s/device to/a device to an/ > > > +register different types of bus drivers, for example, Mediated VFIO PCI driver > > +is designed for mediated PCI devices and supports VFIO APIs. Similarly, driver > > s/driver/the driver/ > > > +can be designed to support any type of mediated device and added to this > > +framework. Mediated bus driver add/delete mediated device to VFIO Group. > > Missing a verb and several articles, but I'm not sure what you meant. > Maybe: > > A mediated bus driver can add/delete mediated devices to a VFIO Group. > > > + > > +Below is the high level block diagram, with NVIDIA, Intel and IBM devices > > +as examples, since these are the devices which are going to actively use > > +this module as of now. > > + > > > + > > + > > +Registration Interfaces > > +------------------------------------------------------------------------------- > > + > > Again, rather long separator, > > > +Mediated core driver provides two types of registration interfaces: > > + > > +1. Registration interface for mediated bus driver: > > +------------------------------------------------- > > while this seems one byte short. I'll quit pointing it out. > > > + > > + /** > > + * struct mdev_driver [2] - Mediated device driver > > + * @name: driver name > > + * @probe: called when new device created > > + * @remove: called when device removed > > + * @driver: device driver structure > > No mention of online, offline. > > > + **/ > > + struct mdev_driver { > > + const char *name; > > + int (*probe)(struct device *dev); > > + void (*remove)(struct device *dev); > > + int (*online)(struct device *dev); > > + int (*offline)(struct device *dev); > > + struct device_driver driver; > > + }; > > + > > + > ... > > + > > +For echo physical device, there is a mdev host device created, it shows in sysfs: > > +/sys/bus/pci/devices/0000:05:00.0/mdev-host/ > > Did you mean 'For echoing a' (as in duplicating the device), or maybe > 'For echoing to a' (as in duplicating data sent to the device)? Or is > this a typo s/echo/each/? > > > -- > Eric Blake eblake redhat com +1-919-301-3266 > Libvirt virtualization library http://libvirt.org > > > * Unknown Key > * 0x2527436A