[libvirt] [RFC PATCH 0/2] Add new mdev type for aggregated resources

[Kirti Wankhede]

On 6/22/2018 1:12 PM, Zhenyu Wang wrote:
> On 2018.06.21 09:00:28 -0600, Alex Williamson wrote:
>> On Thu, 21 Jun 2018 19:57:38 +0530
>> Kirti Wankhede <kwankhede at nvidia.com> wrote:
>>
>>> On 6/20/2018 1:10 PM, Zhenyu Wang wrote:
>>>> Current mdev device create interface depends on fixed mdev type, which get uuid
>>>> from user to create instance of mdev device. If user wants to use customized
>>>> number of resource for mdev device, then only can create new mdev type for that
>>>> which may not be flexible.
>>>>
>>>> To allow to create user defined resources for mdev, this RFC trys
>>>> to extend mdev create interface by adding new "instances=xxx" parameter
>>>> following uuid, for target mdev type if aggregation is supported, it can
>>>> create new mdev device which contains resources combined by number of
>>>> instances, e.g
>>>>
>>>>     echo "<uuid>,instances=10" > create  
>>>
>>> This seems orthogonal to the way mdev types are meant to be used. Vendor
>>> driver can provide the possible types to provide flexibility to the users.
>>
>> I think the goal here is to define how we create a type that supports
>> arbitrary combinations of resources where the total number of those
>> resources my be sufficiently large that the parent driver enumerating
>> them all is not feasible.
>>
>>> Secondly, not always all resources defined for a particular mdev type
>>> can be multiplied, for example, a mdev type for vGPU that supports 2
>>> heads, that can't be multiplied to use with 20 heads.
> 
> yeah, for vGPU we actually can have flexible config for memory size but
> currently fixed by type definition. Although like for display config, we
> are just sticking with 1 head config even for aggregate type.
> 

A mdev type is set of parameters. If aggregation is on one parameter,
how can user know which parameter can be aggregated?

>>
>> Not all types need to define themselves this way, aiui this is an
>> optional extension.  Userspace can determine if this feature is
>> available with the new attribute and if they're not aware of the new
>> attribute, we operate in a backwards compatible mode where 'echo $UUID >
>> create' consumes one instance of that type.  Mdev, like vfio, is device
>> agnostic, so while a vGPU example may have scaling issues that need to
>> be ironed out to implement this, those don't immediately negate the
>> value of this proposal.  Thanks,
>>
> 
> yep, backward compatible is always ensured by this, so for no
> aggregation attrib type, still keep current behavior, driver should
> refuse "instances=" if set by user. One thing I'm concern is that
> looks currently without changing mdev create ops interface, can't
> carry that option for driver, or should we do with more general parameter?
> 

I think, if aggregation is not supported then vendor driver should fail
'create' with instance >1. That means every vendor driver would need a
change to handle this case.

Other way could be, structure mdev_parent_ops can have other function
pointer 'create_with_instances' which has instances as an argument.
Vendor driver which support aggregation will have to provide this
callback and existing vendor driver will need no change. Then in mdev core:

if (instances > 1) {
	if (parent->ops->create_with_instances)
		ret = parent->ops->create_with_instances(kobj, mdev, instances);
	else
		return -EINVAL;
} else
	 ret = parent->ops->create(kobj, mdev);

Thanks,
Kirti



> thanks
> 
>>
>>>> VM manager e.g libvirt can check mdev type with "aggregation" attribute
>>>> which can support this setting. And new sysfs attribute "instances" is
>>>> created for each mdev device to show allocated number. Default number
>>>> of 1 or no "instances" file can be used for compatibility check.
>>>>
>>>> This RFC trys to create new KVMGT type with minimal vGPU resources which
>>>> can be combined with "instances=x" setting to allocate for user wanted resources.
>>>>
>>>> Zhenyu Wang (2):
>>>>   vfio/mdev: Add new instances parameters for mdev create
>>>>   drm/i915/gvt: Add new aggregation type
>>>>
>>>>  drivers/gpu/drm/i915/gvt/gvt.c   | 26 ++++++++++++---
>>>>  drivers/gpu/drm/i915/gvt/gvt.h   | 14 +++++---
>>>>  drivers/gpu/drm/i915/gvt/kvmgt.c |  9 +++--
>>>>  drivers/gpu/drm/i915/gvt/vgpu.c  | 56 ++++++++++++++++++++++++++++----
>>>>  drivers/s390/cio/vfio_ccw_ops.c  |  3 +-
>>>>  drivers/vfio/mdev/mdev_core.c    | 11 ++++---
>>>>  drivers/vfio/mdev/mdev_private.h |  6 +++-
>>>>  drivers/vfio/mdev/mdev_sysfs.c   | 42 ++++++++++++++++++++----
>>>>  include/linux/mdev.h             |  3 +-
>>>>  samples/vfio-mdev/mbochs.c       |  3 +-
>>>>  samples/vfio-mdev/mdpy.c         |  3 +-
>>>>  samples/vfio-mdev/mtty.c         |  3 +-
>>>>  12 files changed, 141 insertions(+), 38 deletions(-)
>>>>   
>>
>