Re: [Qemu-devel] [PATCH v8 4/6] docs: Add Documentation for Mediated devices

[Kirti Wankhede]

On 10/11/2016 7:44 PM, Daniel P. Berrange wrote:
> On Tue, Oct 11, 2016 at 01:58:35AM +0530, Kirti Wankhede wrote:
>> 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>
>> Change-Id: I137dd646442936090d92008b115908b7b2c7bc5d
>> ---
>>  Documentation/vfio-mdev/vfio-mediated-device.txt | 219 
>> +++++++++++++++++++++++
>>  1 file changed, 219 insertions(+)
>>  create mode 100644 Documentation/vfio-mdev/vfio-mediated-device.txt
>>
>> diff --git a/Documentation/vfio-mdev/vfio-mediated-device.txt 
>> b/Documentation/vfio-mdev/vfio-mediated-device.txt
>> new file mode 100644
>> index 000000000000..c1eacb83807b
>> --- /dev/null
>> +++ b/Documentation/vfio-mdev/vfio-mediated-device.txt
>> @@ -0,0 +1,219 @@
>> +/*
>> + * VFIO Mediated devices
>> + *
>> + * Copyright (c) 2016, NVIDIA CORPORATION. All rights reserved.
> 
> Adding "All rights reserved" is bogus since you're providing it under
> the GPL, but I see countless kernel source files have this, so meh.
> 
>> + *     Author: Neo Jia <address@hidden>
>> + *             Kirti Wankhede <address@hidden>
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License version 2 as
>> + * published by the Free Software Foundation.
>> + */
>> +
> 
>> +Mediated device management interface via sysfs
>> +----------------------------------------------
>> +Management interface via sysfs allows user space software, like libvirt, to
>> +query and configure mediated device in a HW agnostic fashion. This 
>> management
>> +interface provide flexibility to underlying physical device's driver to 
>> support
>> +mediated device hotplug, multiple mediated devices per virtual machine, 
>> multiple
>> +mediated devices from different physical devices, etc.
>> +
>> +Under per-physical device sysfs:
>> +--------------------------------
>> +
>> +* mdev_supported_types:
>> +    List of current supported mediated device types and its details are 
>> added
>> +in this directory in following format:
>> +
>> +|- <parent phy device>
>> +|--- Vendor-specific-attributes [optional]
>> +|--- mdev_supported_types
>> +|     |--- <type id>
>> +|     |   |--- create
>> +|     |   |--- name
>> +|     |   |--- available_instances
>> +|     |   |--- description /class
>> +|     |   |--- [devices]
>> +|     |--- <type id>
>> +|     |   |--- create
>> +|     |   |--- name
>> +|     |   |--- available_instances
>> +|     |   |--- description /class
>> +|     |   |--- [devices]
>> +|     |--- <type id>
>> +|          |--- create
>> +|          |--- name
>> +|          |--- available_instances
>> +|          |--- description /class
>> +|          |--- [devices]
>> +
>> +[TBD : description or class is yet to be decided. This will change.]
> 
> I thought that in previous discussions we had agreed to drop
> the <type id> concept and use the name as the unique identifier.
> When reporting these types in libvirt we won't want to report
> the type id values - we'll want the name strings to be unique.
> 

The 'name' might not be unique but type_id will be. For example that Neo
pointed out in earlier discussion, virtual devices can come from two
different physical devices, end user would be presented with what they
had selected but there will be internal implementation differences. In
that case 'type_id' will be unique.

> Based on this sysfs spec, the only fields we would report in
> libvirt would be name + available_instances.
> 
>> +Under per mdev device:
>> +----------------------
>> +
>> +|- <parent phy device>
>> +|--- $MDEV_UUID
>> +         |--- remove
>> +         |--- {link to its type}
>> +         |--- vendor-specific-attributes [optional]
> 
> Again, I thought we'd agreed to not have arbitrary vendor
> specific attributes ?
> 
> That said, I don't mind them existing in kernel sysfs, just
> be aware that we'll *not* expose any vendor specific attributes
> in libvirt, so your functional implementation must not rely on
> these attributes being used in any way by libvirt.
> 
> 

Right, Libvirt would not use vendor specific attributes but admin can
use these to get/set extra information for a particular device. These
are optional, so its up to vendor to provide such attributes or not.

Thanks,
Kirti

> 
>> +
>> +* remove: (write only)
>> +    Write '1' to 'remove' file would destroy mdev device. Vendor driver can
>> +    fail remove() callback if that device is active and vendor driver
>> +    doesn't support hot-unplug.
>> +    Example:
>> +    # echo 1 > /sys/bus/mdev/devices/$mdev_UUID/remove
> 
>> +Mediated device Hotplug:
>> +------------------------
>> +
>> +Mediated devices can be created and assigned during runtime. Procedure to
>> +hot-plug mediated device is same as hot-plug PCI device.
> 
> Generally this looks much saner now all the grouping stuff has gone.
> 
> 
> 
> Regards,
> Daniel
>