Add new mdev type for aggregated resources

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

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

Quoted 11 lines · Zhenyu Wang

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

Quoted 1 line

    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.

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.

Quoted 26 lines

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

Quoted 14 lines · On Thu, 21 Jun 2018 19:57:38 +0530 Kirti Wankhede <[email protected]> wrote:

Quoted 12 lines · Zhenyu Wang

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  

Quoted 2 lines

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.

Quoted 3 lines

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.

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,

Quoted 27 lines

Quoted 27 lines

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

Quoted 25 lines · Alex Williamson

Quoted 12 lines · On Thu, 21 Jun 2018 19:57:38 +0530 Kirti Wankhede <[email protected]> wrote:

Quoted 11 lines · Zhenyu Wang

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

Quoted 4 lines

Quoted 1 line

    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.

Quoted 3 lines

Quoted 3 lines

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.

Quoted 10 lines

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?

thanks

Quoted 28 lines

Quoted 27 lines

Quoted 27 lines

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

For special mdev type which can aggregate instances for mdev device, this extends mdev create interface by allowing extra "instances=xxx" parameter, which is passed to mdev device model to be able to create arbitrary bundled number of instances for target mdev device.

v2: create new create_with_instances operator for vendor driver

Cc Kirti Wankhede <[email protected]>

Cc Alex Williamson <[email protected]>

Cc Kevin Tian <[email protected]>

Signed-off-by Zhenyu Wang <[email protected]>

---
 drivers/vfio/mdev/mdev_core.c    | 18 +++++++++++++----
 drivers/vfio/mdev/mdev_private.h |  5 ++++-
 drivers/vfio/mdev/mdev_sysfs.c   | 34 ++++++++++++++++++++++++++------
 include/linux/mdev.h             | 10 ++++++++++
 4 files changed, 56 insertions(+), 11 deletions(-)

drivers/vfio/mdev/mdev_sysfs.c +0 -1

index 249472f05509..a06e5b7c69d3 100644
@@ -54,11 +54,15 @@ static const struct sysfs_ops mdev_type_sysfs_ops = {
 static ssize_t create_store(struct kobject *kobj, struct device *dev,
 			    const char *buf, size_t count)
 {
-	char *str;

drivers/vfio/mdev/mdev_sysfs.c +0 -0

 		return -EINVAL;
 
 	str = kstrndup(buf, count, GFP_KERNEL);

include/linux/mdev.h +3 -0

  * @remove:		Called to free resources in parent device's driver for a
  *			a mediated device. It is mandatory to provide 'remove'

@@ -71,6 +78,9 @@ struct mdev_parent_ops {
 	struct attribute_group **supported_type_groups;
 
 	int     (*create)(struct kobject *kobj, struct mdev_device *mdev);
+	int     (*create_with_instances)(struct kobject *kobj,
+					 struct mdev_device *mdev,
+					 unsigned int instances);
 	int     (*remove)(struct mdev_device *mdev);
 	int     (*open)(struct mdev_device *mdev);
 	void    (*release)(struct mdev_device *mdev);

Update mdev doc on new aggregration attribute and instances attribute for mdev.

Cc Kirti Wankhede <[email protected]>

Cc Alex Williamson <[email protected]>

Cc Kevin Tian <[email protected]>

Signed-off-by Zhenyu Wang <[email protected]>

---
 Documentation/vfio-mediated-device.txt | 39 ++++++++++++++++++++++----
 1 file changed, 33 insertions(+), 6 deletions(-)

Documentation/vfio-mediated-device.txt +14 -6

index c3f69bcaf96e..9ec9495dcbe7 100644
@@ -211,12 +211,20 @@ Directories and files under the sysfs for Each Physical Device
   |     |   |--- description
   |     |   |--- [devices]
   |     |--- [<type-id>]
-  |          |--- create
-  |          |--- name
-  |          |--- available_instances
-  |          |--- device_api
-  |          |--- description
-  |          |--- [devices]
+  |     |    |--- create
+  |     |    |--- name
+  |     |    |--- available_instances
+  |     |    |--- device_api
+  |     |    |--- description
+  |     |    |--- [devices]
+  |     |--- [<type-id>]
+  |     |    |--- create
+  |     |    |--- name
+  |     |    |--- available_instances
+  |     |    |--- device_api
+  |     |    |--- description
+  |     |    |--- <aggregation>
+  |     |    |--- [devices]
 
 * [mdev_supported_types]
 

Documentation/vfio-mediated-device.txt +11 -0

+
+Example::
+
+	# echo "<uuid>,instances=N" > create
+
 Directories and Files Under the sysfs for Each mdev Device
 ----------------------------------------------------------
 

@@ -268,6 +289,7 @@ Directories and Files Under the sysfs for Each mdev Device
   |- [parent phy device]
   |--- [$MDEV_UUID]
          |--- remove
+	 |--- instances
          |--- mdev_type {link to its type}
          |--- vendor-specific-attributes [optional]
 

@@ -281,6 +303,11 @@ Example::
 
 	# echo 1 > /sys/bus/mdev/devices/$mdev_UUID/remove
 
+* instances
+
+For aggregation type show number of instances assigned for this mdev. For normal
+type or default will just show one instance.
+
 Mediated device Hot plug
 ------------------------
 

Quoted 46 lines · On Fri, 20 Jul 2018 10:19:28 +0800 Zhenyu Wang <[email protected]> wrote:

Update mdev doc on new aggregration attribute and instances attribute for mdev.

Cc Kirti Wankhede <[email protected]>

Cc Alex Williamson <[email protected]>

Cc Kevin Tian <[email protected]>

Signed-off-by Zhenyu Wang <[email protected]>

---
 Documentation/vfio-mediated-device.txt | 39 ++++++++++++++++++++++----
 1 file changed, 33 insertions(+), 6 deletions(-)

Documentation/vfio-mediated-device.txt +14 -6

index c3f69bcaf96e..9ec9495dcbe7 100644
@@ -211,12 +211,20 @@ Directories and files under the sysfs for Each Physical Device
   |     |   |--- description
   |     |   |--- [devices]
   |     |--- [<type-id>]
-  |          |--- create
-  |          |--- name
-  |          |--- available_instances
-  |          |--- device_api
-  |          |--- description
-  |          |--- [devices]
+  |     |    |--- create
+  |     |    |--- name
+  |     |    |--- available_instances
+  |     |    |--- device_api
+  |     |    |--- description
+  |     |    |--- [devices]
+  |     |--- [<type-id>]
+  |     |    |--- create
+  |     |    |--- name
+  |     |    |--- available_instances
+  |     |    |--- device_api
+  |     |    |--- description
+  |     |    |--- <aggregation>
+  |     |    |--- [devices]
 
 * [mdev_supported_types]
 

@@ -260,6 +268,19 @@ Directories and files under the sysfs for Each Physical Device
   This attribute should show brief features/description of the type. This is
   optional attribute.
 

Documentation/vfio-mediated-device.txt Directories and files under the sysfs for Each Physical Device · line 273

Quoted 3 lines

Documentation/vfio-mediated-device.txt +3 -0

+* <aggregation>
+
+  The description is to show feature for one instance of the type. <aggregation>

You are talking about "one instance" here. Can this be different for the same type with different physical devices?

Documentation/vfio-mediated-device.txt Directories and files under the sysfs for Each Physical Device · line 278

Quoted 5 lines

Documentation/vfio-mediated-device.txt +5 -0

+  is an optional attributes to show that [<type-id>]'s instances can be
+  aggregated to be assigned for one mdev device. Set number of instances by
+  appending "instances=N" parameter for create. Instances number can't exceed
+  available_instances number. Without "instances=N" parameter will be default
+  one instance to create.

Could there be a case where available_instances is n, but aggregation is only supported for a value m < n? If yes, should m be discoverable via the "aggregation" attribute?

Quoted 28 lines

Documentation/vfio-mediated-device.txt +11 -0

+
+Example::
+
+	# echo "<uuid>,instances=N" > create
+
 Directories and Files Under the sysfs for Each mdev Device
 ----------------------------------------------------------
 

@@ -268,6 +289,7 @@ Directories and Files Under the sysfs for Each mdev Device
   |- [parent phy device]
   |--- [$MDEV_UUID]
          |--- remove
+	 |--- instances
          |--- mdev_type {link to its type}
          |--- vendor-specific-attributes [optional]
 

@@ -281,6 +303,11 @@ Example::
 
 	# echo 1 > /sys/bus/mdev/devices/$mdev_UUID/remove
 
+* instances
+
+For aggregation type show number of instances assigned for this mdev. For normal
+type or default will just show one instance.
+
 Mediated device Hot plug
 ------------------------
 

Quoted 49 lines · Cornelia Huck

Quoted 46 lines · On Fri, 20 Jul 2018 10:19:28 +0800 Zhenyu Wang <[email protected]> wrote:

Update mdev doc on new aggregration attribute and instances attribute for mdev.

Cc Kirti Wankhede <[email protected]>

Cc Alex Williamson <[email protected]>

Cc Kevin Tian <[email protected]>

Signed-off-by Zhenyu Wang <[email protected]>

---
 Documentation/vfio-mediated-device.txt | 39 ++++++++++++++++++++++----
 1 file changed, 33 insertions(+), 6 deletions(-)

Documentation/vfio-mediated-device.txt +14 -6

index c3f69bcaf96e..9ec9495dcbe7 100644
@@ -211,12 +211,20 @@ Directories and files under the sysfs for Each Physical Device
   |     |   |--- description
   |     |   |--- [devices]
   |     |--- [<type-id>]
-  |          |--- create
-  |          |--- name
-  |          |--- available_instances
-  |          |--- device_api
-  |          |--- description
-  |          |--- [devices]
+  |     |    |--- create
+  |     |    |--- name
+  |     |    |--- available_instances
+  |     |    |--- device_api
+  |     |    |--- description
+  |     |    |--- [devices]
+  |     |--- [<type-id>]
+  |     |    |--- create
+  |     |    |--- name
+  |     |    |--- available_instances
+  |     |    |--- device_api
+  |     |    |--- description
+  |     |    |--- <aggregation>
+  |     |    |--- [devices]
 
 * [mdev_supported_types]
 

@@ -260,6 +268,19 @@ Directories and files under the sysfs for Each Physical Device
   This attribute should show brief features/description of the type. This is
   optional attribute.
 

Quoted 7 lines

Documentation/vfio-mediated-device.txt Directories and files under the sysfs for Each Physical Device · line 273

Quoted 3 lines

Documentation/vfio-mediated-device.txt +3 -0

+* <aggregation>
+
+  The description is to show feature for one instance of the type. <aggregation>  

You are talking about "one instance" here. Can this be different for the same type with different physical devices?

I would expect for normal mdev types, driver might expose like x2, x4, x8 types which split hw resource equally. But for type with aggregation feature, it can set user wanted number of instances. Sorry maybe my use of word was not clear, how about "one example of type"?

Quoted 6 lines

Quoted 5 lines

Documentation/vfio-mediated-device.txt +5 -0

+  is an optional attributes to show that [<type-id>]'s instances can be
+  aggregated to be assigned for one mdev device. Set number of instances by
+  appending "instances=N" parameter for create. Instances number can't exceed
+  available_instances number. Without "instances=N" parameter will be default
+  one instance to create.  

Quoted 4 lines

Could there be a case where available_instances is n, but aggregation is only supported for a value m < n? If yes, should m be discoverable via the "aggregation" attribute?

I don't think there could be a case for that. As for aggregation type, it represents minimal resource allocated for a singleton, I can't see any reason for possible m < n.

Quoted 49 lines · Cornelia Huck

Quoted 46 lines · On Fri, 20 Jul 2018 10:19:28 +0800 Zhenyu Wang <[email protected]> wrote:

Update mdev doc on new aggregration attribute and instances attribute for mdev.

Cc Kirti Wankhede <[email protected]>

Cc Alex Williamson <[email protected]>

Cc Kevin Tian <[email protected]>

Signed-off-by Zhenyu Wang <[email protected]>

---
 Documentation/vfio-mediated-device.txt | 39 ++++++++++++++++++++++----
 1 file changed, 33 insertions(+), 6 deletions(-)

Documentation/vfio-mediated-device.txt +14 -6

index c3f69bcaf96e..9ec9495dcbe7 100644
@@ -211,12 +211,20 @@ Directories and files under the sysfs for Each Physical Device
   |     |   |--- description
   |     |   |--- [devices]
   |     |--- [<type-id>]
-  |          |--- create
-  |          |--- name
-  |          |--- available_instances
-  |          |--- device_api
-  |          |--- description
-  |          |--- [devices]
+  |     |    |--- create
+  |     |    |--- name
+  |     |    |--- available_instances
+  |     |    |--- device_api
+  |     |    |--- description
+  |     |    |--- [devices]
+  |     |--- [<type-id>]
+  |     |    |--- create
+  |     |    |--- name
+  |     |    |--- available_instances
+  |     |    |--- device_api
+  |     |    |--- description
+  |     |    |--- <aggregation>
+  |     |    |--- [devices]
 
 * [mdev_supported_types]
 

@@ -260,6 +268,19 @@ Directories and files under the sysfs for Each Physical Device
   This attribute should show brief features/description of the type. This is
   optional attribute.
 

Quoted 7 lines

Documentation/vfio-mediated-device.txt Directories and files under the sysfs for Each Physical Device · line 273

Quoted 3 lines

Documentation/vfio-mediated-device.txt +3 -0

+* <aggregation>
+
+  The description is to show feature for one instance of the type. <aggregation>  

You are talking about "one instance" here. Can this be different for the same type with different physical devices?

I would expect for normal mdev types, driver might expose like x2, x4, x8 types which split hw resource equally. But for type with aggregation feature, it can set user wanted number of instances. Sorry maybe my use of word was not clear, how about "one example of type"?

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. This requirement comes not only from to be able to allocate flexible resources for KVMGT, but also from Intel scalable IO virtualization which would use vfio/mdev to be able to allocate arbitrary resources on mdev instance. More info on [1] [2] [3].

To allow to create user defined resources for mdev, it 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

VM manager e.g libvirt can check mdev type with "aggregation" attribute which can support this setting. If no "aggregation" attribute found for mdev type, previous behavior is still kept for one instance allocation. And new sysfs attribute "instances" is created for each mdev device to show allocated number.

This trys to create new KVMGT type with minimal vGPU resources which can be combined with "instances=x" setting to allocate for user wanted resources.

Quoted 15 lines · On Fri, 20 Jul 2018 10:19:24 +0800 Zhenyu Wang <[email protected]> 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. This requirement comes not only from to be able to allocate flexible resources for KVMGT, but also from Intel scalable IO virtualization which would use vfio/mdev to be able to allocate arbitrary resources on mdev instance. More info on [1] [2] [3].

To allow to create user defined resources for mdev, it 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

Quoted 7 lines

VM manager e.g libvirt can check mdev type with "aggregation" attribute which can support this setting. If no "aggregation" attribute found for mdev type, previous behavior is still kept for one instance allocation. And new sysfs attribute "instances" is created for each mdev device to show allocated number.

This trys to create new KVMGT type with minimal vGPU resources which can be combined with "instances=x" setting to allocate for user wanted resources.

"instances" makes me think this is arg helps to create multiple mdev instances rather than consuming multiple instances for a single mdev. You're already exposing the "aggregation" attribute, so doesn't "aggregate" perhaps make more sense as the create option? We're asking the driver to aggregate $NUM instances into a single mdev. The mdev attribute should then perhaps also be "aggregated_instances".

The next user question for the interface might be what aspect of the device gets multiplied by this aggregation? In i915 I see you're multiplying the memory sizes by the instance, but clearly the resolution doesn't change. I assume this is sort of like mdev types themselves, ie. some degree of what a type means is buried in the implementation and some degree of what some number of those types aggregated together means is impossible to describe generically.

I'm curious what libvirt folks and Kirti think of this, it looks like it has a nice degree of backwards compatibility, both in the sysfs interface and the vendor driver interface. Thanks,

Quoted 33 lines · Alex Williamson

Quoted 15 lines · On Fri, 20 Jul 2018 10:19:24 +0800 Zhenyu Wang <[email protected]> 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. This requirement comes not only from to be able to allocate flexible resources for KVMGT, but also from Intel scalable IO virtualization which would use vfio/mdev to be able to allocate arbitrary resources on mdev instance. More info on [1] [2] [3].

To allow to create user defined resources for mdev, it 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

Quoted 7 lines

VM manager e.g libvirt can check mdev type with "aggregation" attribute which can support this setting. If no "aggregation" attribute found for mdev type, previous behavior is still kept for one instance allocation. And new sysfs attribute "instances" is created for each mdev device to show allocated number.

This trys to create new KVMGT type with minimal vGPU resources which can be combined with "instances=x" setting to allocate for user wanted resources.

"instances" makes me think this is arg helps to create multiple mdev instances rather than consuming multiple instances for a single mdev. You're already exposing the "aggregation" attribute, so doesn't "aggregate" perhaps make more sense as the create option? We're asking the driver to aggregate $NUM instances into a single mdev. The mdev attribute should then perhaps also be "aggregated_instances".

Quoted 7 lines

The next user question for the interface might be what aspect of the device gets multiplied by this aggregation? In i915 I see you're multiplying the memory sizes by the instance, but clearly the resolution doesn't change. I assume this is sort of like mdev types themselves, ie. some degree of what a type means is buried in the implementation and some degree of what some number of those types aggregated together means is impossible to describe generically.

I don't seem to clearly see the benefit here, so I have to ask, how is this better and/or different from allowing a heterogeneous setup if one needs a more performant instance in terms of more resources? Because to me, once you're able to aggregate instances, I would assume that a simple "echo uuid" with a different type should succeed as well and provide me (from user's perspective) with the same results. Could you please clarify this to me, as well as what resources/parameters are going to be impacted by aggregation?

Quoted 42 lines · On Thu, 26 Jul 2018 15:50:28 +0200 Erik Skultety <[email protected]> wrote:

Quoted 33 lines · Alex Williamson

Quoted 15 lines · On Fri, 20 Jul 2018 10:19:24 +0800 Zhenyu Wang <[email protected]> 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. This requirement comes not only from to be able to allocate flexible resources for KVMGT, but also from Intel scalable IO virtualization which would use vfio/mdev to be able to allocate arbitrary resources on mdev instance. More info on [1] [2] [3].

To allow to create user defined resources for mdev, it 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

Quoted 7 lines

VM manager e.g libvirt can check mdev type with "aggregation" attribute which can support this setting. If no "aggregation" attribute found for mdev type, previous behavior is still kept for one instance allocation. And new sysfs attribute "instances" is created for each mdev device to show allocated number.

This trys to create new KVMGT type with minimal vGPU resources which can be combined with "instances=x" setting to allocate for user wanted resources.

"instances" makes me think this is arg helps to create multiple mdev instances rather than consuming multiple instances for a single mdev. You're already exposing the "aggregation" attribute, so doesn't "aggregate" perhaps make more sense as the create option? We're asking the driver to aggregate $NUM instances into a single mdev. The mdev attribute should then perhaps also be "aggregated_instances".

Quoted 7 lines

The next user question for the interface might be what aspect of the device gets multiplied by this aggregation? In i915 I see you're multiplying the memory sizes by the instance, but clearly the resolution doesn't change. I assume this is sort of like mdev types themselves, ie. some degree of what a type means is buried in the implementation and some degree of what some number of those types aggregated together means is impossible to describe generically.

Quoted 7 lines

I don't seem to clearly see the benefit here, so I have to ask, how is this better and/or different from allowing a heterogeneous setup if one needs a more performant instance in terms of more resources? Because to me, once you're able to aggregate instances, I would assume that a simple "echo uuid" with a different type should succeed as well and provide me (from user's perspective) with the same results. Could you please clarify this to me, as well as what resources/parameters are going to be impacted by aggregation?

I think you're suggesting that we could simply define new mdev types to account for these higher aggregate instances, for example we can define discrete types that are 2x, 3x, 4x, etc. the resource count of a single instance. What I think we're trying to address with this proposal is what happens when the resources available are exceptionally large and they can be combined in arbitrary ways. For example if a parent device can expose 10,000 resources and the granularity with which we can create and mdev instance is 1, is it practical to create 10,000 mdev types or does it make more sense to expose a granularity and method of aggregation.

Using graphics here perhaps falls a little short of the intention of the interface because the possible types are easily enumerable and it would be entirely practical to create discrete types for each. vGPUs also have a lot of variables, so defining which attribute of the device is multiplied by the number of instances is a little more fuzzy. Thanks,

Alex

Quoted 26 lines · Alex Williamson

Quoted 22 lines · On Fri, 20 Jul 2018 10:19:24 +0800 Zhenyu Wang <[email protected]> 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. This requirement comes not only from to be able to allocate flexible resources for KVMGT, but also from Intel scalable IO virtualization which would use vfio/mdev to be able to allocate arbitrary resources on mdev instance. More info on [1] [2] [3].

To allow to create user defined resources for mdev, it 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

VM manager e.g libvirt can check mdev type with "aggregation" attribute which can support this setting. If no "aggregation" attribute found for mdev type, previous behavior is still kept for one instance allocation. And new sysfs attribute "instances" is created for each mdev device to show allocated number.

This trys to create new KVMGT type with minimal vGPU resources which can be combined with "instances=x" setting to allocate for user wanted resources.

Quoted 6 lines

"instances" makes me think this is arg helps to create multiple mdev instances rather than consuming multiple instances for a single mdev. You're already exposing the "aggregation" attribute, so doesn't "aggregate" perhaps make more sense as the create option? We're asking the driver to aggregate $NUM instances into a single mdev. The mdev attribute should then perhaps also be "aggregated_instances".

Quoted 3 lines

I'm curious what libvirt folks and Kirti think of this, it looks like it has a nice degree of backwards compatibility, both in the sysfs interface and the vendor driver interface. Thanks,

Since libvirt doesn't have an API to create mdevs yet, this doesn't pose an issue for us at the moment. I see this adds new optional sysfs attributes which we could expose within our device capabilities XML, provided it doesn't use a free form text, like the description attribute does.

Quoted 27 lines · On Thu, 26 Jul 2018 15:50:28 +0200 Erik Skultety <[email protected]> wrote:

Quoted 26 lines · Alex Williamson

Quoted 22 lines · On Fri, 20 Jul 2018 10:19:24 +0800 Zhenyu Wang <[email protected]> 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. This requirement comes not only from to be able to allocate flexible resources for KVMGT, but also from Intel scalable IO virtualization which would use vfio/mdev to be able to allocate arbitrary resources on mdev instance. More info on [1] [2] [3].

To allow to create user defined resources for mdev, it 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

VM manager e.g libvirt can check mdev type with "aggregation" attribute which can support this setting. If no "aggregation" attribute found for mdev type, previous behavior is still kept for one instance allocation. And new sysfs attribute "instances" is created for each mdev device to show allocated number.

This trys to create new KVMGT type with minimal vGPU resources which can be combined with "instances=x" setting to allocate for user wanted resources.

Quoted 6 lines

Quoted 6 lines

"instances" makes me think this is arg helps to create multiple mdev instances rather than consuming multiple instances for a single mdev. You're already exposing the "aggregation" attribute, so doesn't "aggregate" perhaps make more sense as the create option? We're asking the driver to aggregate $NUM instances into a single mdev. The mdev attribute should then perhaps also be "aggregated_instances".

I agree, that seems like a better naming scheme.

(...)

Quoted 8 lines

Quoted 3 lines

I'm curious what libvirt folks and Kirti think of this, it looks like it has a nice degree of backwards compatibility, both in the sysfs interface and the vendor driver interface. Thanks,

Since libvirt doesn't have an API to create mdevs yet, this doesn't pose an issue for us at the moment. I see this adds new optional sysfs attributes which we could expose within our device capabilities XML, provided it doesn't use a free form text, like the description attribute does.

One thing I noticed is that we have seem to have an optional (?) vendor-driver created "aggregation" attribute (which always prints "true" in the Intel driver). Would it be better or worse for libvirt if it contained some kind of upper boundary or so? Additionally, would it

be easier if the "create" attribute always accepted ",instances=1" (calling the .create ops in that case?)

Quoted 4 lines · Cornelia Huck

One thing I noticed is that we have seem to have an optional (?) vendor-driver created "aggregation" attribute (which always prints "true" in the Intel driver). Would it be better or worse for libvirt if it contained some kind of upper boundary or so? Additionally, would it

Can you be more specific? Although, I wouldn't argue against data that conveys some information, since I would treat the mere presence of the optional attribute as a supported feature that we can expose. Therefore, additional *structured* data which sets clear limits to a certain feature is only a plus that we can expose to the users/management layer.

Quoted 11 lines · On Thu, 26 Jul 2018 17:43:45 +0200 Erik Skultety <[email protected]> wrote:

Quoted 4 lines · Cornelia Huck

One thing I noticed is that we have seem to have an optional (?) vendor-driver created "aggregation" attribute (which always prints "true" in the Intel driver). Would it be better or worse for libvirt if it contained some kind of upper boundary or so? Additionally, would it

Can you be more specific? Although, I wouldn't argue against data that conveys some information, since I would treat the mere presence of the optional attribute as a supported feature that we can expose. Therefore, additional *structured* data which sets clear limits to a certain feature is only a plus that we can expose to the users/management layer.

My question is what would be easiest for libvirt:

- "aggregation" attribute only present when driver supports aggregation (possibly containing max number of resources to be aggregated)
- "aggregation" attribute always present; contains '1' if driver does not support aggregation and 'm' if driver can aggregate 'm' resources

Quoted 15 lines · Cornelia Huck

Quoted 11 lines · On Thu, 26 Jul 2018 17:43:45 +0200 Erik Skultety <[email protected]> wrote:

Quoted 4 lines · Cornelia Huck

One thing I noticed is that we have seem to have an optional (?) vendor-driver created "aggregation" attribute (which always prints "true" in the Intel driver). Would it be better or worse for libvirt if it contained some kind of upper boundary or so? Additionally, would it

Can you be more specific? Although, I wouldn't argue against data that conveys some information, since I would treat the mere presence of the optional attribute as a supported feature that we can expose. Therefore, additional *structured* data which sets clear limits to a certain feature is only a plus that we can expose to the users/management layer.

Quoted 6 lines

My question is what would be easiest for libvirt:

- "aggregation" attribute only present when driver supports aggregation (possibly containing max number of resources to be aggregated)
- "aggregation" attribute always present; contains '1' if driver does not support aggregation and 'm' if driver can aggregate 'm' resources

Both are fine from libvirt's POV, but IMHO the former makes a bit more sense and I'm in favour of that one, IOW the presence of an attribute denotes a new functionality which we can report, if it's missing, the feature is clearly lacking- I don't think we (libvirt) should be reporting the value 1 explicitly in the XML if the feature is missing, we can assume 1 as the default.

Quoted 27 lines · On Thu, 26 Jul 2018 15:50:28 +0200 Erik Skultety <[email protected]> wrote:

Quoted 26 lines · Alex Williamson

Quoted 22 lines · On Fri, 20 Jul 2018 10:19:24 +0800 Zhenyu Wang <[email protected]> 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. This requirement comes not only from to be able to allocate flexible resources for KVMGT, but also from Intel scalable IO virtualization which would use vfio/mdev to be able to allocate arbitrary resources on mdev instance. More info on [1] [2] [3].

To allow to create user defined resources for mdev, it 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

VM manager e.g libvirt can check mdev type with "aggregation" attribute which can support this setting. If no "aggregation" attribute found for mdev type, previous behavior is still kept for one instance allocation. And new sysfs attribute "instances" is created for each mdev device to show allocated number.

This trys to create new KVMGT type with minimal vGPU resources which can be combined with "instances=x" setting to allocate for user wanted resources.

Quoted 6 lines

Quoted 6 lines

"instances" makes me think this is arg helps to create multiple mdev instances rather than consuming multiple instances for a single mdev. You're already exposing the "aggregation" attribute, so doesn't "aggregate" perhaps make more sense as the create option? We're asking the driver to aggregate $NUM instances into a single mdev. The mdev attribute should then perhaps also be "aggregated_instances".

I agree, that seems like a better naming scheme.

(...)

Quoted 8 lines

Quoted 3 lines

I'm curious what libvirt folks and Kirti think of this, it looks like it has a nice degree of backwards compatibility, both in the sysfs interface and the vendor driver interface. Thanks,

Since libvirt doesn't have an API to create mdevs yet, this doesn't pose an issue for us at the moment. I see this adds new optional sysfs attributes which we could expose within our device capabilities XML, provided it doesn't use a free form text, like the description attribute does.

One thing I noticed is that we have seem to have an optional (?) vendor-driver created "aggregation" attribute (which always prints "true" in the Intel driver). Would it be better or worse for libvirt if it contained some kind of upper boundary or so?

Additionally, would it
be easier if the "create" attribute always accepted ",instances=1" (calling the .create ops in that case?)

Quoted 11 lines · On Thu, 26 Jul 2018 17:30:07 +0200 Cornelia Huck <[email protected]> wrote:

Quoted 4 lines · On Thu, 26 Jul 2018 15:50:28 +0200 Erik Skultety <[email protected]> wrote:

Since libvirt doesn't have an API to create mdevs yet, this doesn't pose an issue for us at the moment. I see this adds new optional sysfs attributes which we could expose within our device capabilities XML, provided it doesn't use a free form text, like the description attribute does.

One thing I noticed is that we have seem to have an optional (?) vendor-driver created "aggregation" attribute (which always prints "true" in the Intel driver). Would it be better or worse for libvirt if it contained some kind of upper boundary or so?

Ultimately the aggregation value should be fully specified in Documentation/ABI, but doesn't the kernel generally use 'Y' or 'N' for boolean attributes in sysfs? Maybe mdev core can handle the attribute since it should exist any time the create_with_instances callback is provided.

Quoted 4 lines

Additionally, would it
be easier if the "create" attribute always accepted ",instances=1" (calling the .create ops in that case?)

Unless I misunderstand the code or the question, I think this is exactly what happens:

If the mdev core supplied the aggregation attribute, then the presence of the attribute could indicate to userspace whether it can always provide an instance (aggregate) count, even if limited to '1' when 'N', for that mdev type. Thanks,

Alex

Quoted 15 lines · On Fri, 20 Jul 2018 10:19:24 +0800 Zhenyu Wang <[email protected]> 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. This requirement comes not only from to be able to allocate flexible resources for KVMGT, but also from Intel scalable IO virtualization which would use vfio/mdev to be able to allocate arbitrary resources on mdev instance. More info on [1] [2] [3].

To allow to create user defined resources for mdev, it 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

Quoted 7 lines

VM manager e.g libvirt can check mdev type with "aggregation" attribute which can support this setting. If no "aggregation" attribute found for mdev type, previous behavior is still kept for one instance allocation. And new sysfs attribute "instances" is created for each mdev device to show allocated number.

This trys to create new KVMGT type with minimal vGPU resources which can be combined with "instances=x" setting to allocate for user wanted resources.

"instances" makes me think this is arg helps to create multiple mdev instances rather than consuming multiple instances for a single mdev. You're already exposing the "aggregation" attribute, so doesn't "aggregate" perhaps make more sense as the create option? We're asking the driver to aggregate $NUM instances into a single mdev. The mdev attribute should then perhaps also be "aggregated_instances".

The next user question for the interface might be what aspect of the device gets multiplied by this aggregation? In i915 I see you're multiplying the memory sizes by the instance, but clearly the resolution doesn't change. I assume this is sort of like mdev types themselves, ie. some degree of what a type means is buried in the implementation and some degree of what some number of those types aggregated together means is impossible to describe generically.

We're also going to need to add aggregation to the checklist for device compatibility for migration, for example 1) is it the same mdev_type, 1a) are the aggregation counts the same (new), 2) is the host driver compatible (TBD).

The count handling in create_store(), particularly MDEV_CREATE_OPT_LEN looks a little suspicious. I think we should just be validating that the string before the ',' or the entire string if there is no comma is UUID length. Pass only that to uuid_le_to_bin(). We can then strncmp as you have for "instances=" (or "aggregate=") but then let's be sure to end the string we pass to kstrtouint(), ie. assume there could be further args.

Documentation/ABI/testing/sysfs-bus-vfio-mdev also needs to be updated with this series.

I'm curious what libvirt folks and Kirti think of this, it looks like it has a nice degree of backwards compatibility, both in the sysfs interface and the vendor driver interface. Thanks,

Alex

From: Zhenyu Wang [mailto:[email protected]]
Sent: Friday, July 20, 2018 10:19 AM

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. This requirement comes not only from to be able to
allocate flexible resources for KVMGT, but also from Intel scalable IO virtualization which would use vfio/mdev to be able to allocate arbitrary resources on mdev instance. More info on [1] [2] [3].

To allow to create user defined resources for mdev, it 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

VM manager e.g libvirt can check mdev type with "aggregation" attribute which
can support this setting. If no "aggregation" attribute found for mdev type, previous behavior is still kept for one instance allocation. And new sysfs attribute "instances" is created for each mdev device to show allocated number.

This trys to create new KVMGT type with minimal vGPU resources which can be
combined with "instances=x" setting to allocate for user wanted resources.

References:
[1] https://software.intel.com/en-us/download/intel-virtualization-
technology-for-directed-io-architecture-specification
[2] https://software.intel.com/en-us/download/intel-scalable-io-
virtualization-technical-specification
[3] https://schd.ws/hosted_files/lc32018/00/LC3-SIOV-final.pdf

v2:
- Add new create_with_instances driver hook
- Update doc for new attributes

Zhenyu Wang (4):
vfio/mdev: Add new instances parameter for mdev create vfio/mdev: Add mdev device instances attribute drm/i915/gvt: Add new aggregation type support Documentation/vfio-mediated-device.txt: update for aggregation
attribute

 Documentation/vfio-mediated-device.txt | 39 +++++++++++++++---
 drivers/gpu/drm/i915/gvt/gvt.c         | 26 +++++++++---
 drivers/gpu/drm/i915/gvt/gvt.h         | 14 ++++---
 drivers/gpu/drm/i915/gvt/kvmgt.c       | 30 +++++++++++---
 drivers/gpu/drm/i915/gvt/vgpu.c        | 56 ++++++++++++++++++++++----
 drivers/vfio/mdev/mdev_core.c          | 19 +++++++--
 drivers/vfio/mdev/mdev_private.h       |  6 ++-
 drivers/vfio/mdev/mdev_sysfs.c         | 42 ++++++++++++++++---
 include/linux/mdev.h                   | 10 +++++
 9 files changed, 203 insertions(+), 39 deletions(-)

--
2.18.0

Hi, Zhenyu,

curious about the progress of this series. Is there still some open remaining or a new version coming soon?