Re: [libvirt] [RFC PATCH v1 1/1] Add attribute multiple_mdev_support for mdev type-id

On Thu, 27 Sep 2018 06:48:27 +0000 "Tian, Kevin" <kevin.tian(a)intel.com&gt; wrote: >> From: Kirti Wankhede >> Sent: Thursday, September 27, 2018 2:22 PM >> >> Generally a single instance of mdev device, a share of physical device, is >> assigned to user space application or a VM. There are cases when multiple >> instances of mdev devices of same or different types are required by User >> space application or VM. For example in case of vGPU, multiple mdev >> devices >> of type which represents whole GPU can be assigned to one instance of >> application or VM. >> >> All types of mdev devices may not support assigning multiple mdev devices >> to a user space application. In that case vendor driver can fail open() >> call of mdev device. But there is no way to know User space application >> about the configuration supported by vendor driver. >> >> To expose supported configuration, vendor driver should add >> 'multiple_mdev_support' attribute to type-id directory if vendor driver >> supports assigning multiple mdev devices of a particular type-id to one >> instance of user space application or a VM. >> >> User space application should read if 'multiple_mdev_support' attibute is >> present in type-id directory of all mdev devices which are going to be >> used. If all read 1 then user space application can proceed with multiple >> mdev devices. >> >> This is optional and readonly attribute. > > I didn't get what is the exact problem from above description. Isn't it the > basic point behind mdev concept that parent driver can create multiple > mdev instances on a physical device? VFIO usually doesn't care whether > multiple devices (pci or mdev) are assigned to same process/VM or not. > Can you give some example of actual problem behind this change? The situation here is that mdev imposes no restrictions regarding how mdev devices can be used by the user. Multiple mdevs, regardless of type, can be combined and used in any way the user sees fit, at least that's the theory. However, in practice, certain vendor drivers impose a limitation that prevents multiple mdev devices from being used in the same VM. This is done by returning an error when the user attempts to open those additional devices. Now we're in the very predictable situation that we want to consider that some of the vendor's mdev types might be combined in the same userspace instance, while others still impose a restriction, and how can we optionally expose such per mdev type restrictions to the userspace so we have something better than "try it and see if it works". The below proposal assumes that a single instance per VM is the norm and that we add something to the API to indicate multiple instances are supported. However, that's really never been the position of the mdev core, where there's no concept of limiting devices per user instance; it's a vendor driver restriction. So I think the question is then why should we burden vendor drivers who do not impose a restriction with an additional field? Does the below extension provide a better backwards compatibility scenario?

With the current code, I think it's reasonable for userspace to assume there are no mdev limits per userspace instance, those that exist are vendor specific. The below proposal is for an optional field, what does the management tool assume when it's not supplied? We have both cases currently, mdev devices that allow multiple instances per VM and those that do not, so I don't see that the user is more informed with this addition and no attempt is made here to synchronously update all drivers to expose this new attribute.

Does it make more sense then to add an attribute to expose the restriction rather than the lack of restriction. Perhaps something like "singleton_usage_restriction".

Also if the type is meant to be a boolean, I think we have Y/N support for that in sysfs rather than using integers which raise the question what >1 implies.

Is there a more compelling backwards compatibility store for the below proposal than what I'm thinking? Thanks, Alex >> Signed-off-by: Neo Jia <cjia(a)nvidia.com&gt; >> Signed-off-by: Kirti Wankhede <kwankhede(a)nvidia.com&gt; >> --- >> Documentation/ABI/testing/sysfs-bus-vfio-mdev | 13 +++++++++++++ >> 1 file changed, 13 insertions(+) >> >> diff --git a/Documentation/ABI/testing/sysfs-bus-vfio-mdev >> b/Documentation/ABI/testing/sysfs-bus-vfio-mdev >> index 452dbe39270e..69e1291479ce 100644 >> --- a/Documentation/ABI/testing/sysfs-bus-vfio-mdev >> +++ b/Documentation/ABI/testing/sysfs-bus-vfio-mdev >> @@ -85,6 +85,19 @@ Users: >> a particular <type-id> that can help in understanding the >> features provided by that type of mediated device. >> >> +What: /sys/.../mdev_supported_types/<type- >> id>/multiple_mdev_support >> +Date: September 2018 >> +Contact: Kirti Wankhede <kwankhede(a)nvidia.com&gt; >> +Description: >> + Reading this attribute will return 0 or 1. Returning 1 >> indicates >> + multiple mdev devices of a particular <type-id> assigned to >> one >> + User space application is supported by vendor driver. This is >> + optional and readonly attribute. >> +Users: >> + Userspace applications interested in knowing if multiple >> mdev >> + devices of a particular <type-id> can be assigned to one >> + instance of application. >> + >> What: /sys/.../<device>/<UUID>/ >> Date: October 2016 >> Contact: Kirti Wankhede <kwankhede(a)nvidia.com&gt; >> -- >> 2.7.0 >