Re: [libvirt] [Qemu-devel] [RFC v2] libvirt vGPU QEMU integration

[Alex Williamson]

On Fri, 7 Oct 2016 10:46:22 +0530
Kirti Wankhede  wrote:

> Ping..
> 
> Pulling the questions at the top.
> 
> >> Will libvirt report 'description' RO attribute, its output would be
> >> string, so that user could be able to see the configuration of that
> >> profile?
> >>  
> >
> > Daniel,
> > Waiting for your input on this.
> >  
> >> We can have 'class' as optional attribute. So Intel don't have to
> >> provide 'class' attribute and they don't have to specify mandatory
> >> attributes of that class. We would provide 'class' attribute and provide
> >> mandatory attributes.  

Hi Kirti,

I would proceed with your design and posting, this is not a
sufficiently significant issue to delay a new version.  The next
posting will likely not be the last, more feedback will come.

Personally I don't see an issue.  We should have a small number
of mandatory sysfs interfaces for mdev devices, essentially just
enough to manage the life cycle of a device, even if it requires
vendor specific knowledge to understand the exact composition of
that device.  Some sort of human readable description seems like
a reasonable optional feature for a userspace tool like libvirt
to take advantage of.  A class definition with an available set
of attributes per class would clearly enhance the ability of a
userspace tool to understand the device, but does not seem
strictly required for managing the device.  Thanks,

Alex

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [Qemu-devel] [RFC v2] libvirt vGPU QEMU integration

[Kirti Wankhede]

Ping..

Pulling the questions at the top.

>> Will libvirt report 'description' RO attribute, its output would be
>> string, so that user could be able to see the configuration of that
>> profile?
>>
>
> Daniel,
> Waiting for your input on this.
>
>> We can have 'class' as optional attribute. So Intel don't have to
>> provide 'class' attribute and they don't have to specify mandatory
>> attributes of that class. We would provide 'class' attribute and provide
>> mandatory attributes.
>

Thanks,
Kirti


On 10/3/2016 1:50 PM, Kirti Wankhede wrote:
> 
> 
> On 9/30/2016 10:49 AM, Kirti Wankhede wrote:
>>
> ...
> 
>>> Hi Daniel,
>>>
>>> Here you are proposing to add a class named "gpu", which will make all 
>>> those gpu
>>> related attributes mandatory, which libvirt can allow user to better
>>> parse/present a particular mdev configuration?
>>>
>>> I am just wondering if there is another option that we just make all 
>>> those
>>> attributes that a mdev device can have as optional but still meaningful 
>>> to
>>> libvirt, so libvirt can still parse / recognize them as an class "mdev".
>>
>> 'mdev' isn't a class - mdev is the name of the kernel module. The class
>> refers to the broad capability of the device. class would be things
>> like "gpu", "nic", "fpga" or other such things. The point of the class
>> is to identify which other attributes will be considered mandatory.
>>
>>
>
> Thanks Daniel. This class definition makes sense to me.
>
> However I'm not sure whether we should define such common mandatory 
> attributes
> of a 'gpu' class now. Intel will go with a 2's power sharing of type 
> definition... actual
> type name to be finalized, but an example looks like below:
>
> [GVTG-SKL-x2]: available instances (2)
> [GVTG-SKL-x4]: available instances (4)
> [GVTG-SKL-x8]: available instances (8)
> ...
>
> User can create different types of vGPUs simultaneously. A GVTG-SKL-x2 
> type
> vGPU will get half of the physical GPU resource, while a GVTG-SKL-x4 type 
> will
> get a quarter. However it's unclear to me how we want to enumerate those
> resources into resolution or heads. I feel it'd be more reasonable for us 
> to push
> initial libvirt mdev support w/o vgpu specific class definition, until we 
> see
> a clear value of doing so (at that time we then follow Daniel's guideline 
> to define
> mandatory attributes common to all GPU vendors).

 Libvirt won't report arbitrary vendor define attributes. So if we are not
 going to define a gpu class & associated attributes, then there will be
 no reporting of the 'heads', 'resolution', 'fb_length' data described
 above.

>>>
>>> yes, that's my point. I think nvidia may put them into the 'description' 
>>> attribute
>>> just for descriptive purpose for now.
>>
>>
>> Will libvirt report 'description' RO attribute, its output would be
>> string, so that user could be able to see the configuration of that
>> profile?
>>
> 
> Daniel,
> Waiting for your input on this.
> 
>> We can have 'class' as optional attribute. So Intel don't have to
>> provide 'class' attribute and they don't have to specify mandatory
>> attributes of that class. We would provide 'class' attribute and provide
>> mandatory attributes.
> 
> 
> Thanks,
> Kirti
> 

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [Qemu-devel] [RFC v2] libvirt vGPU QEMU integration

[Kirti Wankhede]

On 9/30/2016 10:49 AM, Kirti Wankhede wrote:
> 
...

>> Hi Daniel,
>>
>> Here you are proposing to add a class named "gpu", which will make all 
>> those gpu
>> related attributes mandatory, which libvirt can allow user to better
>> parse/present a particular mdev configuration?
>>
>> I am just wondering if there is another option that we just make all 
>> those
>> attributes that a mdev device can have as optional but still meaningful 
>> to
>> libvirt, so libvirt can still parse / recognize them as an class "mdev".
>
> 'mdev' isn't a class - mdev is the name of the kernel module. The class
> refers to the broad capability of the device. class would be things
> like "gpu", "nic", "fpga" or other such things. The point of the class
> is to identify which other attributes will be considered mandatory.
>
>

 Thanks Daniel. This class definition makes sense to me.

 However I'm not sure whether we should define such common mandatory 
 attributes
 of a 'gpu' class now. Intel will go with a 2's power sharing of type 
 definition... actual
 type name to be finalized, but an example looks like below:

 [GVTG-SKL-x2]: available instances (2)
 [GVTG-SKL-x4]: available instances (4)
 [GVTG-SKL-x8]: available instances (8)
 ...

 User can create different types of vGPUs simultaneously. A GVTG-SKL-x2 type
 vGPU will get half of the physical GPU resource, while a GVTG-SKL-x4 type 
 will
 get a quarter. However it's unclear to me how we want to enumerate those
 resources into resolution or heads. I feel it'd be more reasonable for us 
 to push
 initial libvirt mdev support w/o vgpu specific class definition, until we 
 see
 a clear value of doing so (at that time we then follow Daniel's guideline 
 to define
 mandatory attributes common to all GPU vendors).
>>>
>>> Libvirt won't report arbitrary vendor define attributes. So if we are not
>>> going to define a gpu class & associated attributes, then there will be
>>> no reporting of the 'heads', 'resolution', 'fb_length' data described
>>> above.
>>>
>>
>> yes, that's my point. I think nvidia may put them into the 'description' 
>> attribute
>> just for descriptive purpose for now.
> 
> 
> Will libvirt report 'description' RO attribute, its output would be
> string, so that user could be able to see the configuration of that
> profile?
>

Daniel,
Waiting for your input on this.

> We can have 'class' as optional attribute. So Intel don't have to
> provide 'class' attribute and they don't have to specify mandatory
> attributes of that class. We would provide 'class' attribute and provide
> mandatory attributes.


Thanks,
Kirti

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [Qemu-devel] [RFC v2] libvirt vGPU QEMU integration

[Kirti Wankhede]

On 9/29/2016 8:12 PM, Tian, Kevin wrote:
>> From: Daniel P. Berrange [mailto:berra...@redhat.com]
>> Sent: Thursday, September 29, 2016 10:39 PM
>>
>> On Thu, Sep 29, 2016 at 02:35:48PM +, Tian, Kevin wrote:
 From: Daniel P. Berrange [mailto:berra...@redhat.com]
 Sent: Thursday, September 29, 2016 4:06 PM

 On Wed, Sep 28, 2016 at 12:48:33PM -0700, Neo Jia wrote:
> On Tue, Sep 20, 2016 at 10:47:53AM +0100, Daniel P. Berrange wrote:
>> On Tue, Sep 20, 2016 at 02:05:52AM +0530, Kirti Wankhede wrote:
>>>
>>> Hi libvirt experts,
>>>
>>> Thanks for valuable input on v1 version of RFC.
>>>
>>> Quick brief, VFIO based mediated device framework provides a way to
>>> virtualize their devices without SR-IOV, like NVIDIA vGPU, Intel KVMGT
>>> and IBM's channel IO. This framework reuses VFIO APIs for all the
>>> functionalities for mediated devices which are currently being used for
>>> pass through devices. This framework introduces a set of new sysfs files
>>> for device creation and its life cycle management.
>>>
>>> Here is the summary of discussion on v1:
>>> 1. Discover mediated device:
>>> As part of physical device initialization process, vendor driver will
>>> register their physical devices, which will be used to create virtual
>>> device (mediated device, aka mdev) to the mediated framework.
>>>
>>> Vendor driver should specify mdev_supported_types in directory format.
>>> This format is class based, for example, display class directory format
>>> should be as below. We need to define such set for each class of devices
>>> which would be supported by mediated device framework.
>>>
>>>  --- mdev_destroy
>>>  --- mdev_supported_types
>>>  |-- 11
>>>  |   |-- create
>>>  |   |-- name
>>>  |   |-- fb_length
>>>  |   |-- resolution
>>>  |   |-- heads
>>>  |   |-- max_instances
>>>  |   |-- params
>>>  |   |-- requires_group
>>>  |-- 12
>>>  |   |-- create
>>>  |   |-- name
>>>  |   |-- fb_length
>>>  |   |-- resolution
>>>  |   |-- heads
>>>  |   |-- max_instances
>>>  |   |-- params
>>>  |   |-- requires_group
>>>  |-- 13
>>>  |-- create
>>>  |-- name
>>>  |-- fb_length
>>>  |-- resolution
>>>  |-- heads
>>>  |-- max_instances
>>>  |-- params
>>>  |-- requires_group
>>>
>>>
>>> In the above example directory '11' represents a type id of mdev device.
>>> 'name', 'fb_length', 'resolution', 'heads', 'max_instance' and
>>> 'requires_group' would be Read-Only files that vendor would provide to
>>> describe about that type.
>>>
>>> 'create':
>>> Write-only file. Mandatory.
>>> Accepts string to create mediated device.
>>>
>>> 'name':
>>> Read-Only file. Mandatory.
>>> Returns string, the name of that type id.
>>
>> Presumably this is a human-targetted title/description of
>> the device.
>>
>>>
>>> 'fb_length':
>>> Read-only file. Mandatory.
>>> Returns {K,M,G}, size of framebuffer.
>>>
>>> 'resolution':
>>> Read-Only file. Mandatory.
>>> Returns 'hres x vres' format. Maximum supported resolution.
>>>
>>> 'heads':
>>> Read-Only file. Mandatory.
>>> Returns integer. Number of maximum heads supported.
>>
>> None of these should be mandatory as that makes the mdev
>> useless for non-GPU devices.
>>
>> I'd expect to see a 'class' or 'type' attribute in the
>> directory whcih tells you what kind of mdev it is. A
>> valid 'class' value would be 'gpu'. The fb_length,
>> resolution, and heads parameters would only be mandatory
>> when class==gpu.
>>
>
> Hi Daniel,
>
> Here you are proposing to add a class named "gpu", which will make all 
> those gpu
> related attributes mandatory, which libvirt can allow user to better
> parse/present a particular mdev configuration?
>
> I am just wondering if there is another option that we just make all those
> attributes that a mdev device can have as optional but still meaningful to
> libvirt, so libvirt can still parse / recognize them as an class "mdev".

 'mdev' isn't a class - mdev is the name of the kernel module. The class
 refers to the broad capability of the device. class would be things
 like "gpu", "nic", "fpga" or other such things. The point of the class
 is to identify which other attributes will be considered mandatory.


>>>
>>> Thanks Daniel. This class definition makes sense to me.
>>>
>>> However I'm not sure whether we should define such common mandatory 
>>> attributes
>>> of a 'gpu' class now. Intel will go with a 2's power 

Re: [libvirt] [Qemu-devel] [RFC v2] libvirt vGPU QEMU integration

[Tian, Kevin]

> From: Daniel P. Berrange [mailto:berra...@redhat.com]
> Sent: Thursday, September 29, 2016 10:39 PM
> 
> On Thu, Sep 29, 2016 at 02:35:48PM +, Tian, Kevin wrote:
> > > From: Daniel P. Berrange [mailto:berra...@redhat.com]
> > > Sent: Thursday, September 29, 2016 4:06 PM
> > >
> > > On Wed, Sep 28, 2016 at 12:48:33PM -0700, Neo Jia wrote:
> > > > On Tue, Sep 20, 2016 at 10:47:53AM +0100, Daniel P. Berrange wrote:
> > > > > On Tue, Sep 20, 2016 at 02:05:52AM +0530, Kirti Wankhede wrote:
> > > > > >
> > > > > > Hi libvirt experts,
> > > > > >
> > > > > > Thanks for valuable input on v1 version of RFC.
> > > > > >
> > > > > > Quick brief, VFIO based mediated device framework provides a way to
> > > > > > virtualize their devices without SR-IOV, like NVIDIA vGPU, Intel 
> > > > > > KVMGT
> > > > > > and IBM's channel IO. This framework reuses VFIO APIs for all the
> > > > > > functionalities for mediated devices which are currently being used 
> > > > > > for
> > > > > > pass through devices. This framework introduces a set of new sysfs 
> > > > > > files
> > > > > > for device creation and its life cycle management.
> > > > > >
> > > > > > Here is the summary of discussion on v1:
> > > > > > 1. Discover mediated device:
> > > > > > As part of physical device initialization process, vendor driver 
> > > > > > will
> > > > > > register their physical devices, which will be used to create 
> > > > > > virtual
> > > > > > device (mediated device, aka mdev) to the mediated framework.
> > > > > >
> > > > > > Vendor driver should specify mdev_supported_types in directory 
> > > > > > format.
> > > > > > This format is class based, for example, display class directory 
> > > > > > format
> > > > > > should be as below. We need to define such set for each class of 
> > > > > > devices
> > > > > > which would be supported by mediated device framework.
> > > > > >
> > > > > >  --- mdev_destroy
> > > > > >  --- mdev_supported_types
> > > > > >  |-- 11
> > > > > >  |   |-- create
> > > > > >  |   |-- name
> > > > > >  |   |-- fb_length
> > > > > >  |   |-- resolution
> > > > > >  |   |-- heads
> > > > > >  |   |-- max_instances
> > > > > >  |   |-- params
> > > > > >  |   |-- requires_group
> > > > > >  |-- 12
> > > > > >  |   |-- create
> > > > > >  |   |-- name
> > > > > >  |   |-- fb_length
> > > > > >  |   |-- resolution
> > > > > >  |   |-- heads
> > > > > >  |   |-- max_instances
> > > > > >  |   |-- params
> > > > > >  |   |-- requires_group
> > > > > >  |-- 13
> > > > > >  |-- create
> > > > > >  |-- name
> > > > > >  |-- fb_length
> > > > > >  |-- resolution
> > > > > >  |-- heads
> > > > > >  |-- max_instances
> > > > > >  |-- params
> > > > > >  |-- requires_group
> > > > > >
> > > > > >
> > > > > > In the above example directory '11' represents a type id of mdev 
> > > > > > device.
> > > > > > 'name', 'fb_length', 'resolution', 'heads', 'max_instance' and
> > > > > > 'requires_group' would be Read-Only files that vendor would provide 
> > > > > > to
> > > > > > describe about that type.
> > > > > >
> > > > > > 'create':
> > > > > > Write-only file. Mandatory.
> > > > > > Accepts string to create mediated device.
> > > > > >
> > > > > > 'name':
> > > > > > Read-Only file. Mandatory.
> > > > > > Returns string, the name of that type id.
> > > > >
> > > > > Presumably this is a human-targetted title/description of
> > > > > the device.
> > > > >
> > > > > >
> > > > > > 'fb_length':
> > > > > > Read-only file. Mandatory.
> > > > > > Returns {K,M,G}, size of framebuffer.
> > > > > >
> > > > > > 'resolution':
> > > > > > Read-Only file. Mandatory.
> > > > > > Returns 'hres x vres' format. Maximum supported resolution.
> > > > > >
> > > > > > 'heads':
> > > > > > Read-Only file. Mandatory.
> > > > > > Returns integer. Number of maximum heads supported.
> > > > >
> > > > > None of these should be mandatory as that makes the mdev
> > > > > useless for non-GPU devices.
> > > > >
> > > > > I'd expect to see a 'class' or 'type' attribute in the
> > > > > directory whcih tells you what kind of mdev it is. A
> > > > > valid 'class' value would be 'gpu'. The fb_length,
> > > > > resolution, and heads parameters would only be mandatory
> > > > > when class==gpu.
> > > > >
> > > >
> > > > Hi Daniel,
> > > >
> > > > Here you are proposing to add a class named "gpu", which will make all 
> > > > those gpu
> > > > related attributes mandatory, which libvirt can allow user to better
> > > > parse/present a particular mdev configuration?
> > > >
> > > > I am just wondering if there is another option that we just make all 
> > > > those
> > > > attributes that a mdev device can have as optional but still meaningful 
> > > > to
> > > > libvirt, so libvirt can still parse / recognize them as an class "mdev".
> > >
> > > 'mdev' isn't a class - mdev is 

Re: [libvirt] [Qemu-devel] [RFC v2] libvirt vGPU QEMU integration

[Daniel P. Berrange]

On Thu, Sep 29, 2016 at 02:35:48PM +, Tian, Kevin wrote:
> > From: Daniel P. Berrange [mailto:berra...@redhat.com]
> > Sent: Thursday, September 29, 2016 4:06 PM
> > 
> > On Wed, Sep 28, 2016 at 12:48:33PM -0700, Neo Jia wrote:
> > > On Tue, Sep 20, 2016 at 10:47:53AM +0100, Daniel P. Berrange wrote:
> > > > On Tue, Sep 20, 2016 at 02:05:52AM +0530, Kirti Wankhede wrote:
> > > > >
> > > > > Hi libvirt experts,
> > > > >
> > > > > Thanks for valuable input on v1 version of RFC.
> > > > >
> > > > > Quick brief, VFIO based mediated device framework provides a way to
> > > > > virtualize their devices without SR-IOV, like NVIDIA vGPU, Intel KVMGT
> > > > > and IBM's channel IO. This framework reuses VFIO APIs for all the
> > > > > functionalities for mediated devices which are currently being used 
> > > > > for
> > > > > pass through devices. This framework introduces a set of new sysfs 
> > > > > files
> > > > > for device creation and its life cycle management.
> > > > >
> > > > > Here is the summary of discussion on v1:
> > > > > 1. Discover mediated device:
> > > > > As part of physical device initialization process, vendor driver will
> > > > > register their physical devices, which will be used to create virtual
> > > > > device (mediated device, aka mdev) to the mediated framework.
> > > > >
> > > > > Vendor driver should specify mdev_supported_types in directory format.
> > > > > This format is class based, for example, display class directory 
> > > > > format
> > > > > should be as below. We need to define such set for each class of 
> > > > > devices
> > > > > which would be supported by mediated device framework.
> > > > >
> > > > >  --- mdev_destroy
> > > > >  --- mdev_supported_types
> > > > >  |-- 11
> > > > >  |   |-- create
> > > > >  |   |-- name
> > > > >  |   |-- fb_length
> > > > >  |   |-- resolution
> > > > >  |   |-- heads
> > > > >  |   |-- max_instances
> > > > >  |   |-- params
> > > > >  |   |-- requires_group
> > > > >  |-- 12
> > > > >  |   |-- create
> > > > >  |   |-- name
> > > > >  |   |-- fb_length
> > > > >  |   |-- resolution
> > > > >  |   |-- heads
> > > > >  |   |-- max_instances
> > > > >  |   |-- params
> > > > >  |   |-- requires_group
> > > > >  |-- 13
> > > > >  |-- create
> > > > >  |-- name
> > > > >  |-- fb_length
> > > > >  |-- resolution
> > > > >  |-- heads
> > > > >  |-- max_instances
> > > > >  |-- params
> > > > >  |-- requires_group
> > > > >
> > > > >
> > > > > In the above example directory '11' represents a type id of mdev 
> > > > > device.
> > > > > 'name', 'fb_length', 'resolution', 'heads', 'max_instance' and
> > > > > 'requires_group' would be Read-Only files that vendor would provide to
> > > > > describe about that type.
> > > > >
> > > > > 'create':
> > > > > Write-only file. Mandatory.
> > > > > Accepts string to create mediated device.
> > > > >
> > > > > 'name':
> > > > > Read-Only file. Mandatory.
> > > > > Returns string, the name of that type id.
> > > >
> > > > Presumably this is a human-targetted title/description of
> > > > the device.
> > > >
> > > > >
> > > > > 'fb_length':
> > > > > Read-only file. Mandatory.
> > > > > Returns {K,M,G}, size of framebuffer.
> > > > >
> > > > > 'resolution':
> > > > > Read-Only file. Mandatory.
> > > > > Returns 'hres x vres' format. Maximum supported resolution.
> > > > >
> > > > > 'heads':
> > > > > Read-Only file. Mandatory.
> > > > > Returns integer. Number of maximum heads supported.
> > > >
> > > > None of these should be mandatory as that makes the mdev
> > > > useless for non-GPU devices.
> > > >
> > > > I'd expect to see a 'class' or 'type' attribute in the
> > > > directory whcih tells you what kind of mdev it is. A
> > > > valid 'class' value would be 'gpu'. The fb_length,
> > > > resolution, and heads parameters would only be mandatory
> > > > when class==gpu.
> > > >
> > >
> > > Hi Daniel,
> > >
> > > Here you are proposing to add a class named "gpu", which will make all 
> > > those gpu
> > > related attributes mandatory, which libvirt can allow user to better
> > > parse/present a particular mdev configuration?
> > >
> > > I am just wondering if there is another option that we just make all those
> > > attributes that a mdev device can have as optional but still meaningful to
> > > libvirt, so libvirt can still parse / recognize them as an class "mdev".
> > 
> > 'mdev' isn't a class - mdev is the name of the kernel module. The class
> > refers to the broad capability of the device. class would be things
> > like "gpu", "nic", "fpga" or other such things. The point of the class
> > is to identify which other attributes will be considered mandatory.
> > 
> > 
> 
> Thanks Daniel. This class definition makes sense to me.
> 
> However I'm not sure whether we should define such common mandatory attributes
> of 

Re: [libvirt] [Qemu-devel] [RFC v2] libvirt vGPU QEMU integration

[Tian, Kevin]

> From: Daniel P. Berrange [mailto:berra...@redhat.com]
> Sent: Thursday, September 29, 2016 4:06 PM
> 
> On Wed, Sep 28, 2016 at 12:48:33PM -0700, Neo Jia wrote:
> > On Tue, Sep 20, 2016 at 10:47:53AM +0100, Daniel P. Berrange wrote:
> > > On Tue, Sep 20, 2016 at 02:05:52AM +0530, Kirti Wankhede wrote:
> > > >
> > > > Hi libvirt experts,
> > > >
> > > > Thanks for valuable input on v1 version of RFC.
> > > >
> > > > Quick brief, VFIO based mediated device framework provides a way to
> > > > virtualize their devices without SR-IOV, like NVIDIA vGPU, Intel KVMGT
> > > > and IBM's channel IO. This framework reuses VFIO APIs for all the
> > > > functionalities for mediated devices which are currently being used for
> > > > pass through devices. This framework introduces a set of new sysfs files
> > > > for device creation and its life cycle management.
> > > >
> > > > Here is the summary of discussion on v1:
> > > > 1. Discover mediated device:
> > > > As part of physical device initialization process, vendor driver will
> > > > register their physical devices, which will be used to create virtual
> > > > device (mediated device, aka mdev) to the mediated framework.
> > > >
> > > > Vendor driver should specify mdev_supported_types in directory format.
> > > > This format is class based, for example, display class directory format
> > > > should be as below. We need to define such set for each class of devices
> > > > which would be supported by mediated device framework.
> > > >
> > > >  --- mdev_destroy
> > > >  --- mdev_supported_types
> > > >  |-- 11
> > > >  |   |-- create
> > > >  |   |-- name
> > > >  |   |-- fb_length
> > > >  |   |-- resolution
> > > >  |   |-- heads
> > > >  |   |-- max_instances
> > > >  |   |-- params
> > > >  |   |-- requires_group
> > > >  |-- 12
> > > >  |   |-- create
> > > >  |   |-- name
> > > >  |   |-- fb_length
> > > >  |   |-- resolution
> > > >  |   |-- heads
> > > >  |   |-- max_instances
> > > >  |   |-- params
> > > >  |   |-- requires_group
> > > >  |-- 13
> > > >  |-- create
> > > >  |-- name
> > > >  |-- fb_length
> > > >  |-- resolution
> > > >  |-- heads
> > > >  |-- max_instances
> > > >  |-- params
> > > >  |-- requires_group
> > > >
> > > >
> > > > In the above example directory '11' represents a type id of mdev device.
> > > > 'name', 'fb_length', 'resolution', 'heads', 'max_instance' and
> > > > 'requires_group' would be Read-Only files that vendor would provide to
> > > > describe about that type.
> > > >
> > > > 'create':
> > > > Write-only file. Mandatory.
> > > > Accepts string to create mediated device.
> > > >
> > > > 'name':
> > > > Read-Only file. Mandatory.
> > > > Returns string, the name of that type id.
> > >
> > > Presumably this is a human-targetted title/description of
> > > the device.
> > >
> > > >
> > > > 'fb_length':
> > > > Read-only file. Mandatory.
> > > > Returns {K,M,G}, size of framebuffer.
> > > >
> > > > 'resolution':
> > > > Read-Only file. Mandatory.
> > > > Returns 'hres x vres' format. Maximum supported resolution.
> > > >
> > > > 'heads':
> > > > Read-Only file. Mandatory.
> > > > Returns integer. Number of maximum heads supported.
> > >
> > > None of these should be mandatory as that makes the mdev
> > > useless for non-GPU devices.
> > >
> > > I'd expect to see a 'class' or 'type' attribute in the
> > > directory whcih tells you what kind of mdev it is. A
> > > valid 'class' value would be 'gpu'. The fb_length,
> > > resolution, and heads parameters would only be mandatory
> > > when class==gpu.
> > >
> >
> > Hi Daniel,
> >
> > Here you are proposing to add a class named "gpu", which will make all 
> > those gpu
> > related attributes mandatory, which libvirt can allow user to better
> > parse/present a particular mdev configuration?
> >
> > I am just wondering if there is another option that we just make all those
> > attributes that a mdev device can have as optional but still meaningful to
> > libvirt, so libvirt can still parse / recognize them as an class "mdev".
> 
> 'mdev' isn't a class - mdev is the name of the kernel module. The class
> refers to the broad capability of the device. class would be things
> like "gpu", "nic", "fpga" or other such things. The point of the class
> is to identify which other attributes will be considered mandatory.
> 
> 

Thanks Daniel. This class definition makes sense to me.

However I'm not sure whether we should define such common mandatory attributes
of a 'gpu' class now. Intel will go with a 2's power sharing of type 
definition... actual 
type name to be finalized, but an example looks like below:

[GVTG-SKL-x2]: available instances (2)
[GVTG-SKL-x4]: available instances (4)
[GVTG-SKL-x8]: available instances (8)
...

User can create different types of vGPUs simultaneously. A GVTG-SKL-x2 type
vGPU will 

Re: [libvirt] [Qemu-devel] [RFC v2] libvirt vGPU QEMU integration

[Daniel P. Berrange]

On Wed, Sep 28, 2016 at 12:48:33PM -0700, Neo Jia wrote:
> On Tue, Sep 20, 2016 at 10:47:53AM +0100, Daniel P. Berrange wrote:
> > On Tue, Sep 20, 2016 at 02:05:52AM +0530, Kirti Wankhede wrote:
> > > 
> > > Hi libvirt experts,
> > > 
> > > Thanks for valuable input on v1 version of RFC.
> > > 
> > > Quick brief, VFIO based mediated device framework provides a way to
> > > virtualize their devices without SR-IOV, like NVIDIA vGPU, Intel KVMGT
> > > and IBM's channel IO. This framework reuses VFIO APIs for all the
> > > functionalities for mediated devices which are currently being used for
> > > pass through devices. This framework introduces a set of new sysfs files
> > > for device creation and its life cycle management.
> > > 
> > > Here is the summary of discussion on v1:
> > > 1. Discover mediated device:
> > > As part of physical device initialization process, vendor driver will
> > > register their physical devices, which will be used to create virtual
> > > device (mediated device, aka mdev) to the mediated framework.
> > > 
> > > Vendor driver should specify mdev_supported_types in directory format.
> > > This format is class based, for example, display class directory format
> > > should be as below. We need to define such set for each class of devices
> > > which would be supported by mediated device framework.
> > > 
> > >  --- mdev_destroy
> > >  --- mdev_supported_types
> > >  |-- 11
> > >  |   |-- create
> > >  |   |-- name
> > >  |   |-- fb_length
> > >  |   |-- resolution
> > >  |   |-- heads
> > >  |   |-- max_instances
> > >  |   |-- params
> > >  |   |-- requires_group
> > >  |-- 12
> > >  |   |-- create
> > >  |   |-- name
> > >  |   |-- fb_length
> > >  |   |-- resolution
> > >  |   |-- heads
> > >  |   |-- max_instances
> > >  |   |-- params
> > >  |   |-- requires_group
> > >  |-- 13
> > >  |-- create
> > >  |-- name
> > >  |-- fb_length
> > >  |-- resolution
> > >  |-- heads
> > >  |-- max_instances
> > >  |-- params
> > >  |-- requires_group
> > > 
> > > 
> > > In the above example directory '11' represents a type id of mdev device.
> > > 'name', 'fb_length', 'resolution', 'heads', 'max_instance' and
> > > 'requires_group' would be Read-Only files that vendor would provide to
> > > describe about that type.
> > > 
> > > 'create':
> > > Write-only file. Mandatory.
> > > Accepts string to create mediated device.
> > > 
> > > 'name':
> > > Read-Only file. Mandatory.
> > > Returns string, the name of that type id.
> > 
> > Presumably this is a human-targetted title/description of
> > the device.
> > 
> > > 
> > > 'fb_length':
> > > Read-only file. Mandatory.
> > > Returns {K,M,G}, size of framebuffer.
> > > 
> > > 'resolution':
> > > Read-Only file. Mandatory.
> > > Returns 'hres x vres' format. Maximum supported resolution.
> > > 
> > > 'heads':
> > > Read-Only file. Mandatory.
> > > Returns integer. Number of maximum heads supported.
> > 
> > None of these should be mandatory as that makes the mdev
> > useless for non-GPU devices.
> > 
> > I'd expect to see a 'class' or 'type' attribute in the
> > directory whcih tells you what kind of mdev it is. A
> > valid 'class' value would be 'gpu'. The fb_length,
> > resolution, and heads parameters would only be mandatory
> > when class==gpu.
> > 
> 
> Hi Daniel,
> 
> Here you are proposing to add a class named "gpu", which will make all those 
> gpu
> related attributes mandatory, which libvirt can allow user to better
> parse/present a particular mdev configuration?
> 
> I am just wondering if there is another option that we just make all those
> attributes that a mdev device can have as optional but still meaningful to
> libvirt, so libvirt can still parse / recognize them as an class "mdev".

'mdev' isn't a class - mdev is the name of the kernel module. The class
refers to the broad capability of the device. class would be things
like "gpu", "nic", "fpga" or other such things. The point of the class
is to identify which other attributes will be considered mandatory.


Regards,
Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://entangle-photo.org   -o-http://search.cpan.org/~danberr/ :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [Qemu-devel] [RFC v2] libvirt vGPU QEMU integration

[Neo Jia]

On Tue, Sep 20, 2016 at 10:47:53AM +0100, Daniel P. Berrange wrote:
> On Tue, Sep 20, 2016 at 02:05:52AM +0530, Kirti Wankhede wrote:
> > 
> > Hi libvirt experts,
> > 
> > Thanks for valuable input on v1 version of RFC.
> > 
> > Quick brief, VFIO based mediated device framework provides a way to
> > virtualize their devices without SR-IOV, like NVIDIA vGPU, Intel KVMGT
> > and IBM's channel IO. This framework reuses VFIO APIs for all the
> > functionalities for mediated devices which are currently being used for
> > pass through devices. This framework introduces a set of new sysfs files
> > for device creation and its life cycle management.
> > 
> > Here is the summary of discussion on v1:
> > 1. Discover mediated device:
> > As part of physical device initialization process, vendor driver will
> > register their physical devices, which will be used to create virtual
> > device (mediated device, aka mdev) to the mediated framework.
> > 
> > Vendor driver should specify mdev_supported_types in directory format.
> > This format is class based, for example, display class directory format
> > should be as below. We need to define such set for each class of devices
> > which would be supported by mediated device framework.
> > 
> >  --- mdev_destroy
> >  --- mdev_supported_types
> >  |-- 11
> >  |   |-- create
> >  |   |-- name
> >  |   |-- fb_length
> >  |   |-- resolution
> >  |   |-- heads
> >  |   |-- max_instances
> >  |   |-- params
> >  |   |-- requires_group
> >  |-- 12
> >  |   |-- create
> >  |   |-- name
> >  |   |-- fb_length
> >  |   |-- resolution
> >  |   |-- heads
> >  |   |-- max_instances
> >  |   |-- params
> >  |   |-- requires_group
> >  |-- 13
> >  |-- create
> >  |-- name
> >  |-- fb_length
> >  |-- resolution
> >  |-- heads
> >  |-- max_instances
> >  |-- params
> >  |-- requires_group
> > 
> > 
> > In the above example directory '11' represents a type id of mdev device.
> > 'name', 'fb_length', 'resolution', 'heads', 'max_instance' and
> > 'requires_group' would be Read-Only files that vendor would provide to
> > describe about that type.
> > 
> > 'create':
> > Write-only file. Mandatory.
> > Accepts string to create mediated device.
> > 
> > 'name':
> > Read-Only file. Mandatory.
> > Returns string, the name of that type id.
> 
> Presumably this is a human-targetted title/description of
> the device.
> 
> > 
> > 'fb_length':
> > Read-only file. Mandatory.
> > Returns {K,M,G}, size of framebuffer.
> > 
> > 'resolution':
> > Read-Only file. Mandatory.
> > Returns 'hres x vres' format. Maximum supported resolution.
> > 
> > 'heads':
> > Read-Only file. Mandatory.
> > Returns integer. Number of maximum heads supported.
> 
> None of these should be mandatory as that makes the mdev
> useless for non-GPU devices.
> 
> I'd expect to see a 'class' or 'type' attribute in the
> directory whcih tells you what kind of mdev it is. A
> valid 'class' value would be 'gpu'. The fb_length,
> resolution, and heads parameters would only be mandatory
> when class==gpu.
> 

Hi Daniel,

Here you are proposing to add a class named "gpu", which will make all those gpu
related attributes mandatory, which libvirt can allow user to better
parse/present a particular mdev configuration?

I am just wondering if there is another option that we just make all those
attributes that a mdev device can have as optional but still meaningful to
libvirt, so libvirt can still parse / recognize them as an class "mdev".

In general, I am just trying to understand the requirement from libvirt and see
how we can fit in this requirement for both Intel and NVIDIA since Intel is also
moving to the type-based interface although they don't have "class" concept yet.

Thanks,
Neo

> > 'max_instance':
> > Read-Only file. Mandatory.
> > Returns integer.  Returns maximum mdev device could be created
> > at the moment when this file is read. This count would be updated by
> > vendor driver. Before creating mdev device of this type, check if
> > max_instance is > 0.
> > 
> > 'params'
> > Write-Only file. Optional.
> > String input. Libvirt would pass the string given in XML file to
> > this file and then create mdev device. Set empty string to clear params.
> > For example, set parameter 'frame_rate_limiter=0' to disable frame rate
> > limiter for performance benchmarking, then create device of type 11. The
> > device created would have that parameter set by vendor driver.
> 
> Nope, libvirt will explicitly *NEVER* allow arbitrary opaque
> passthrough of vendor specific data in this way.
> 
> > The parent device would look like:
> > 
> >
> >  pci__86_00_0
> >  
> >0
> >134
> >0
> >0
> >
> >  
> >  
> >
> >GRID M60-0B
> >512M
> >

Re: [libvirt] [Qemu-devel] [RFC v2] libvirt vGPU QEMU integration

[Paolo Bonzini]

On 20/09/2016 17:14, Daniel P. Berrange wrote:
> Any VM which
> uses the separate namespace is tainted, which means if theres a bug
> report we'll require the reported to remove whatever config caused
> the tainting and then reproduce the problem.
> 
> If the vendor specific mdev parameters are things that apps will often
> need to be able to set, then allowing it via a separate namespace is
> just providing a backdoor that everyone needs to use, defeating the
> point of using a separate namespace.

No, they don't need to be often set, and tainting the domain is a good idea.

Paolo

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [Qemu-devel] [RFC v2] libvirt vGPU QEMU integration

[Daniel P. Berrange]

On Tue, Sep 20, 2016 at 10:01:18PM +0530, Kirti Wankhede wrote:
> 
> 
> On 9/20/2016 8:44 PM, Daniel P. Berrange wrote:
> > On Tue, Sep 20, 2016 at 05:05:43PM +0200, Paolo Bonzini wrote:
> >>
> >>
> >> On 20/09/2016 16:58, Daniel P. Berrange wrote:
> > As I've said in my earlier reply - libvirt will *NOT* support passing
> > arbitrary vendor specific parameters as a blob via the XML. Everything
> > that appears in the XML must be *fully* specified and explicitly
> > represented in the XML  as a distinct attribute or element.
> 
>  Are generic key/value attributes (e.g. a  element) acceptable?
> >>>
> >>> Only if libvirt has a known list of valid attribute key names upfront.
> >>> We don't want to just blindly expose arbitary vendor specific keys exposed
> >>> by the kernel. Libvirt's job is to ensure the XML representation is vendor
> >>> portable
> >>
> 
> In key/value attributes (taking example from proposed xml file)
> 
> 2560x1600
> 
> 'Key' (i.e. 'resolution') should be known upfront, not the value, right?

NB, resolution is a good example of why we would *not* use the
generic $value format in
the XML. Instead we'd represent it as

   

as applications should not have to further parse data after extracting
it from an XML attribute/element.

Regards,
Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [Qemu-devel] [RFC v2] libvirt vGPU QEMU integration

[Daniel P. Berrange]

On Tue, Sep 20, 2016 at 10:12:20PM +0530, Kirti Wankhede wrote:
> 
> 
> On 9/20/2016 10:06 PM, Daniel P. Berrange wrote:
> > On Tue, Sep 20, 2016 at 10:01:18PM +0530, Kirti Wankhede wrote:
> >>
> >>
> >> On 9/20/2016 8:44 PM, Daniel P. Berrange wrote:
> >>> On Tue, Sep 20, 2016 at 05:05:43PM +0200, Paolo Bonzini wrote:
> 
> 
>  On 20/09/2016 16:58, Daniel P. Berrange wrote:
> >>> As I've said in my earlier reply - libvirt will *NOT* support passing
> >>> arbitrary vendor specific parameters as a blob via the XML. Everything
> >>> that appears in the XML must be *fully* specified and explicitly
> >>> represented in the XML  as a distinct attribute or element.
> >>
> >> Are generic key/value attributes (e.g. a  element) 
> >> acceptable?
> >
> > Only if libvirt has a known list of valid attribute key names upfront.
> > We don't want to just blindly expose arbitary vendor specific keys 
> > exposed
> > by the kernel. Libvirt's job is to ensure the XML representation is 
> > vendor
> > portable
> 
> >>
> >> In key/value attributes (taking example from proposed xml file)
> >>
> >> 2560x1600
> >>
> >> 'Key' (i.e. 'resolution') should be known upfront, not the value, right?
> > 
> > Yes, the actual value is not important - only its structured.
> > ie, libvirt would check that it is in the format '$WIDTHx$HEIGHT'
> > and reject it if not.
> > 
> 
> In this particular example, libvirt checks if its integer? or value
> could be 2560x1600 or 4096x4096 both are valid?
>
> Does libvirt accept string value?

Err, as I said, we'd validate that its in the format '$WIDTHx$HEIGHT'


Regards,
Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [Qemu-devel] [RFC v2] libvirt vGPU QEMU integration

[Kirti Wankhede]

On 9/20/2016 10:06 PM, Daniel P. Berrange wrote:
> On Tue, Sep 20, 2016 at 10:01:18PM +0530, Kirti Wankhede wrote:
>>
>>
>> On 9/20/2016 8:44 PM, Daniel P. Berrange wrote:
>>> On Tue, Sep 20, 2016 at 05:05:43PM +0200, Paolo Bonzini wrote:


 On 20/09/2016 16:58, Daniel P. Berrange wrote:
>>> As I've said in my earlier reply - libvirt will *NOT* support passing
>>> arbitrary vendor specific parameters as a blob via the XML. Everything
>>> that appears in the XML must be *fully* specified and explicitly
>>> represented in the XML  as a distinct attribute or element.
>>
>> Are generic key/value attributes (e.g. a  element) acceptable?
>
> Only if libvirt has a known list of valid attribute key names upfront.
> We don't want to just blindly expose arbitary vendor specific keys exposed
> by the kernel. Libvirt's job is to ensure the XML representation is vendor
> portable

>>
>> In key/value attributes (taking example from proposed xml file)
>>
>> 2560x1600
>>
>> 'Key' (i.e. 'resolution') should be known upfront, not the value, right?
> 
> Yes, the actual value is not important - only its structured.
> ie, libvirt would check that it is in the format '$WIDTHx$HEIGHT'
> and reject it if not.
> 

In this particular example, libvirt checks if its integer? or value
could be 2560x1600 or 4096x4096 both are valid?

Does libvirt accept string value?

Kirti

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [Qemu-devel] [RFC v2] libvirt vGPU QEMU integration

[Daniel P. Berrange]

On Tue, Sep 20, 2016 at 10:01:18PM +0530, Kirti Wankhede wrote:
> 
> 
> On 9/20/2016 8:44 PM, Daniel P. Berrange wrote:
> > On Tue, Sep 20, 2016 at 05:05:43PM +0200, Paolo Bonzini wrote:
> >>
> >>
> >> On 20/09/2016 16:58, Daniel P. Berrange wrote:
> > As I've said in my earlier reply - libvirt will *NOT* support passing
> > arbitrary vendor specific parameters as a blob via the XML. Everything
> > that appears in the XML must be *fully* specified and explicitly
> > represented in the XML  as a distinct attribute or element.
> 
>  Are generic key/value attributes (e.g. a  element) acceptable?
> >>>
> >>> Only if libvirt has a known list of valid attribute key names upfront.
> >>> We don't want to just blindly expose arbitary vendor specific keys exposed
> >>> by the kernel. Libvirt's job is to ensure the XML representation is vendor
> >>> portable
> >>
> 
> In key/value attributes (taking example from proposed xml file)
> 
> 2560x1600
> 
> 'Key' (i.e. 'resolution') should be known upfront, not the value, right?

Yes, the actual value is not important - only its structured.
ie, libvirt would check that it is in the format '$WIDTHx$HEIGHT'
and reject it if not.

Regards,
Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [Qemu-devel] [RFC v2] libvirt vGPU QEMU integration

[Kirti Wankhede]

On 9/20/2016 8:44 PM, Daniel P. Berrange wrote:
> On Tue, Sep 20, 2016 at 05:05:43PM +0200, Paolo Bonzini wrote:
>>
>>
>> On 20/09/2016 16:58, Daniel P. Berrange wrote:
> As I've said in my earlier reply - libvirt will *NOT* support passing
> arbitrary vendor specific parameters as a blob via the XML. Everything
> that appears in the XML must be *fully* specified and explicitly
> represented in the XML  as a distinct attribute or element.

 Are generic key/value attributes (e.g. a  element) acceptable?
>>>
>>> Only if libvirt has a known list of valid attribute key names upfront.
>>> We don't want to just blindly expose arbitary vendor specific keys exposed
>>> by the kernel. Libvirt's job is to ensure the XML representation is vendor
>>> portable
>>

In key/value attributes (taking example from proposed xml file)

2560x1600

'Key' (i.e. 'resolution') should be known upfront, not the value, right?

Kirti


>> Ok, then I guess vendor-specific mdev parameters are out completely.  Or
>> could they be added under a separate namespace, like QEMU passthrough?
> 
> The QEMU XML namespace that allows passthrough is intended either as a
> short term hack to let people experiment with new QEMU features, before
> they get explicitly represented in the main XML namespace, or in a few
> cases, for things we never want to support officially.  Any VM which
> uses the separate namespace is tainted, which means if theres a bug
> report we'll require the reported to remove whatever config caused
> the tainting and then reproduce the problem.
> 
> If the vendor specific mdev parameters are things that apps will often
> need to be able to set, then allowing it via a separate namespace is
> just providing a backdoor that everyone needs to use, defeating the
> point of using a separate namespace.
> 
> We don't want to knowingly design a system which defacto requires
> the app to use the separate namespace most/all of the time.
> 
> 
> Regards,
> Daniel
> 

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [Qemu-devel] [RFC v2] libvirt vGPU QEMU integration

[Daniel P. Berrange]

On Tue, Sep 20, 2016 at 05:05:43PM +0200, Paolo Bonzini wrote:
> 
> 
> On 20/09/2016 16:58, Daniel P. Berrange wrote:
> > > > As I've said in my earlier reply - libvirt will *NOT* support passing
> > > > arbitrary vendor specific parameters as a blob via the XML. Everything
> > > > that appears in the XML must be *fully* specified and explicitly
> > > > represented in the XML  as a distinct attribute or element.
> > > 
> > > Are generic key/value attributes (e.g. a  element) acceptable?
> >
> > Only if libvirt has a known list of valid attribute key names upfront.
> > We don't want to just blindly expose arbitary vendor specific keys exposed
> > by the kernel. Libvirt's job is to ensure the XML representation is vendor
> > portable
> 
> Ok, then I guess vendor-specific mdev parameters are out completely.  Or
> could they be added under a separate namespace, like QEMU passthrough?

The QEMU XML namespace that allows passthrough is intended either as a
short term hack to let people experiment with new QEMU features, before
they get explicitly represented in the main XML namespace, or in a few
cases, for things we never want to support officially.  Any VM which
uses the separate namespace is tainted, which means if theres a bug
report we'll require the reported to remove whatever config caused
the tainting and then reproduce the problem.

If the vendor specific mdev parameters are things that apps will often
need to be able to set, then allowing it via a separate namespace is
just providing a backdoor that everyone needs to use, defeating the
point of using a separate namespace.

We don't want to knowingly design a system which defacto requires
the app to use the separate namespace most/all of the time.


Regards,
Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [Qemu-devel] [RFC v2] libvirt vGPU QEMU integration

[Paolo Bonzini]

On 20/09/2016 16:58, Daniel P. Berrange wrote:
> > > As I've said in my earlier reply - libvirt will *NOT* support passing
> > > arbitrary vendor specific parameters as a blob via the XML. Everything
> > > that appears in the XML must be *fully* specified and explicitly
> > > represented in the XML  as a distinct attribute or element.
> > 
> > Are generic key/value attributes (e.g. a  element) acceptable?
>
> Only if libvirt has a known list of valid attribute key names upfront.
> We don't want to just blindly expose arbitary vendor specific keys exposed
> by the kernel. Libvirt's job is to ensure the XML representation is vendor
> portable

Ok, then I guess vendor-specific mdev parameters are out completely.  Or
could they be added under a separate namespace, like QEMU passthrough?

Paolo

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [Qemu-devel] [RFC v2] libvirt vGPU QEMU integration

[Daniel P. Berrange]

On Tue, Sep 20, 2016 at 04:49:09PM +0200, Paolo Bonzini wrote:
> 
> 
> On 20/09/2016 16:41, Daniel P. Berrange wrote:
> > As I've said in my earlier reply - libvirt will *NOT* support passing
> > arbitrary vendor specific parameters as a blob via the XML. Everything
> > that appears in the XML must be *fully* specified and explicitly
> > represented in the XML  as a distinct attribute or element.
> 
> Are generic key/value attributes (e.g. a  element) acceptable?

Only if libvirt has a known list of valid attribute key names upfront.
We don't want to just blindly expose arbitary vendor specific keys exposed
by the kernel. Libvirt's job is to ensure the XML representation is vendor
portable and stable across software versions. If we just blindly expose
the strings reported by the host, then the XML will change if the vendor
arbitararily renames an attribute in a software update, or if two
vendors have the same concept there's no guaranteed name between them.


Regards,
Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [Qemu-devel] [RFC v2] libvirt vGPU QEMU integration

[Paolo Bonzini]

On 20/09/2016 16:41, Daniel P. Berrange wrote:
> As I've said in my earlier reply - libvirt will *NOT* support passing
> arbitrary vendor specific parameters as a blob via the XML. Everything
> that appears in the XML must be *fully* specified and explicitly
> represented in the XML  as a distinct attribute or element.

Are generic key/value attributes (e.g. a  element) acceptable?

Paolo

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [Qemu-devel] [RFC v2] libvirt vGPU QEMU integration

[Daniel P. Berrange]

On Tue, Sep 20, 2016 at 08:05:01PM +0530, Kirti Wankhede wrote:
> 
> 
> On 9/20/2016 3:55 AM, Alex Williamson wrote:
> > On Mon, 19 Sep 2016 23:50:56 +0200
> > Paolo Bonzini  wrote:
> > 
> >> On 19/09/2016 23:36, Alex Williamson wrote:
> >>> On Tue, 20 Sep 2016 02:05:52 +0530
> >>> Kirti Wankhede  wrote:  
>  'fb_length':
>  Read-only file. Mandatory.
>  Returns {K,M,G}, size of framebuffer.  
> >>>
> >>> This can't be mandatory, it's only relevant to vGPU devices, vGPUs are
> >>> just one user of mediated devices. [...]
> >>>   
>  'params'
>  Write-Only file. Optional.
>  String input. Libvirt would pass the string given in XML file to
>  this file and then create mdev device. Set empty string to clear params.
>  For example, set parameter 'frame_rate_limiter=0' to disable frame rate
>  limiter for performance benchmarking, then create device of type 11. The
>  device created would have that parameter set by vendor driver.  
> >>>
> >>> Might as well just call it "magic", there's absolutely no ability to
> >>> introspect what parameters are allowed or even valid here.  Can all
> >>> parameters be changed dynamically?  Do parameter changes apply to
> >>> existing devices or only future devices?  This is a poorly specified
> >>> interface.  I'd prefer this be done via module options on the vendor
> >>> driver.  
> >>
> >> Or additional files in the mdev's sysfs directory?
> > 
> > Sure, the vendor driver could certainly support that, it'd be vendor
> > specific of course.
> >
> 
> Right, it would be vendor specific. But if this is not set through mdev
> device XML, user have to create mdev device with XML first and then
> manually set such parameter. Also these parameters would not retain if
> mdev device is destroyed and created again. That's why if libvirt sets
> these parameters by reading it from XML file, it would be simple for user.
> 
> If we move 'params' file to mdev device, could libvirt do the following
> to create mdev device:
> 
> * Read /sys/../\:86\:00.0/11/max_instances. If it is greater than 0,
> then only proceed else fail.
> 
> * Autogenerate UUID
> * Create device:
> 
> echo "$UUID" > /sys/../\:86\:00.0/11/create
> 
> * Set extra params if 'params' field exist in device XML and 'params'
> file exist mdev device directory:
> 
> echo "frame_rate_limiter=0" > /sys/bus/mdev/devices/$UUID/params

As I've said in my earlier reply - libvirt will *NOT* support passing
arbitrary vendor specific parameters as a blob via the XML. Everything
that appears in the XML must be *fully* specified and explicitly
represented in the XML  as a distinct attribute or element.

Regards,
Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [Qemu-devel] [RFC v2] libvirt vGPU QEMU integration

[Daniel P. Berrange]

On Tue, Sep 20, 2016 at 02:05:52AM +0530, Kirti Wankhede wrote:
> 
> Hi libvirt experts,
> 
> Thanks for valuable input on v1 version of RFC.
> 
> Quick brief, VFIO based mediated device framework provides a way to
> virtualize their devices without SR-IOV, like NVIDIA vGPU, Intel KVMGT
> and IBM's channel IO. This framework reuses VFIO APIs for all the
> functionalities for mediated devices which are currently being used for
> pass through devices. This framework introduces a set of new sysfs files
> for device creation and its life cycle management.
> 
> Here is the summary of discussion on v1:
> 1. Discover mediated device:
> As part of physical device initialization process, vendor driver will
> register their physical devices, which will be used to create virtual
> device (mediated device, aka mdev) to the mediated framework.
> 
> Vendor driver should specify mdev_supported_types in directory format.
> This format is class based, for example, display class directory format
> should be as below. We need to define such set for each class of devices
> which would be supported by mediated device framework.
> 
>  --- mdev_destroy
>  --- mdev_supported_types
>  |-- 11
>  |   |-- create
>  |   |-- name
>  |   |-- fb_length
>  |   |-- resolution
>  |   |-- heads
>  |   |-- max_instances
>  |   |-- params
>  |   |-- requires_group
>  |-- 12
>  |   |-- create
>  |   |-- name
>  |   |-- fb_length
>  |   |-- resolution
>  |   |-- heads
>  |   |-- max_instances
>  |   |-- params
>  |   |-- requires_group
>  |-- 13
>  |-- create
>  |-- name
>  |-- fb_length
>  |-- resolution
>  |-- heads
>  |-- max_instances
>  |-- params
>  |-- requires_group
> 
> 
> In the above example directory '11' represents a type id of mdev device.
> 'name', 'fb_length', 'resolution', 'heads', 'max_instance' and
> 'requires_group' would be Read-Only files that vendor would provide to
> describe about that type.
> 
> 'create':
> Write-only file. Mandatory.
> Accepts string to create mediated device.
> 
> 'name':
> Read-Only file. Mandatory.
> Returns string, the name of that type id.

Presumably this is a human-targetted title/description of
the device.

> 
> 'fb_length':
> Read-only file. Mandatory.
> Returns {K,M,G}, size of framebuffer.
> 
> 'resolution':
> Read-Only file. Mandatory.
> Returns 'hres x vres' format. Maximum supported resolution.
> 
> 'heads':
> Read-Only file. Mandatory.
> Returns integer. Number of maximum heads supported.

None of these should be mandatory as that makes the mdev
useless for non-GPU devices.

I'd expect to see a 'class' or 'type' attribute in the
directory whcih tells you what kind of mdev it is. A
valid 'class' value would be 'gpu'. The fb_length,
resolution, and heads parameters would only be mandatory
when class==gpu.

> 'max_instance':
> Read-Only file. Mandatory.
> Returns integer.  Returns maximum mdev device could be created
> at the moment when this file is read. This count would be updated by
> vendor driver. Before creating mdev device of this type, check if
> max_instance is > 0.
> 
> 'params'
> Write-Only file. Optional.
> String input. Libvirt would pass the string given in XML file to
> this file and then create mdev device. Set empty string to clear params.
> For example, set parameter 'frame_rate_limiter=0' to disable frame rate
> limiter for performance benchmarking, then create device of type 11. The
> device created would have that parameter set by vendor driver.

Nope, libvirt will explicitly *NEVER* allow arbitrary opaque
passthrough of vendor specific data in this way.

> The parent device would look like:
> 
>
>  pci__86_00_0
>  
>0
>134
>0
>0
>
>  
>  
>
>GRID M60-0B
>512M
>2560x1600
>2
>16
>1
>  

There would need to be a  element, eg gpu

We would then have further elements based on the class. eg

  

GRID M60-0B
512M
2560x1600
2
16
1
  



>
>GRID M60
>NVIDIA
>  
>
> 
> 2. Create/destroy mediated device
> 
> With above example, vGPU device XML would look like:
> 
>
>  my-vgpu
>  pci__86_00_0
>  
>
>1
>'frame_rate_limiter=0'

No, we will not support  in this manner in libvirt.

The entire purpose of libvirt is to represent data in a
vendor agnostic manner and not do abitrary passthrough
of vendor specific data. Simply saying this field is
optional does not get around that either.

>  
>
> 
> 'type id' is mandatory.
> 'group' is optional. It should be a unique number in the system among
> all the groups created for mdev devices. Its usage is:
>