Re: [Qemu-devel] [RFC v2 4/4] docs: Add Documentation for Mediated devices

[Eric Blake]

On 09/02/2016 03:16 AM, Jike Song wrote:
> From: Kirti Wankhede <address@hidden>
> 
> Add file Documentation/vfio-mediated-device.txt that include details of
> mediated device framework.
> 
> Signed-off-by: Kirti Wankhede <address@hidden>
> Signed-off-by: Neo Jia <address@hidden>
> Signed-off-by: Jike Song <address@hidden>
> ---
>  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.

> +
> +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

signature.asc
Description: OpenPGP digital signature