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@xxxxxxxxxx> 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(-)
>>>>
>>
>
