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 <kwankhede@xxxxxxxxxx> > > > > Add file Documentation/vfio-mediated-device.txt that include details of > > mediated device framework. > > > > Signed-off-by: Kirti Wankhede <kwankhede@xxxxxxxxxx> > > Signed-off-by: Neo Jia <cjia@xxxxxxxxxx> > > Signed-off-by: Jike Song <jike.song@xxxxxxxxx> > > --- > > 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 -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html