Re: [libvirt] [PATCH 0/6] VFIO mdev aggregated resources handling

2019-11-19 Thread Tian, Kevin
> From: Alex Williamson
> Sent: Wednesday, November 20, 2019 6:58 AM
> 
> On Fri, 15 Nov 2019 04:24:35 +
> "Tian, Kevin"  wrote:
> 
> > > From: Alex Williamson
> > > Sent: Thursday, November 7, 2019 2:45 AM
> > >
> > > On Wed, 6 Nov 2019 12:20:31 +0800
> > > Zhenyu Wang  wrote:
> > >
> > > > On 2019.11.05 14:10:42 -0700, Alex Williamson wrote:
> > > > > On Thu, 24 Oct 2019 13:08:23 +0800
> > > > > Zhenyu Wang  wrote:
> > > > >
> > > > > > Hi,
> > > > > >
> > > > > > This is a refresh for previous send of this series. I got impression
> that
> > > > > > some SIOV drivers would still deploy their own create and config
> > > method so
> > > > > > stopped effort on this. But seems this would still be useful for
> some
> > > other
> > > > > > SIOV driver which may simply want capability to aggregate
> resources.
> > > So here's
> > > > > > refreshed series.
> > > > > >
> > > > > > 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 "aggregate=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 ",aggregate=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 "aggregated_instances" is created for each mdev device
> to
> > > show allocated number.
> > > > >
> > > > > Given discussions we've had recently around libvirt interacting with
> > > > > mdev, I think that libvirt would rather have an abstract interface via
> > > > > mdevctl[1].  Therefore can you evaluate how mdevctl would support
> > > this
> > > > > creation extension?  It seems like it would fit within the existing
> > > > > mdev and mdevctl framework if aggregation were simply a sysfs
> > > attribute
> > > > > for the device.  For example, the mdevctl steps might look like this:
> > > > >
> > > > > mdevctl define -u UUID -p PARENT -t TYPE
> > > > > mdevctl modify -u UUID --addattr=mdev/aggregation --value=2
> > > > > mdevctl start -u UUID
> >
> > Hi, Alex, can you elaborate why a sysfs attribute is more friendly
> > to mdevctl? what is the complexity if having mdevctl to pass
> > additional parameter at creation time as this series originally
> > proposed? Just want to clearly understand the limitation of the
> > parameter way. :-)
> 
> We could also flip this question around, vfio-ap already uses sysfs to
> finish composing a device after it's created, therefore why shouldn't
> aggregation use this existing mechanism.  Extending the creation
> interface is a more fundamental change than simply standardizing an
> optional sysfs namespace entry.
> 
> > > > >
> > > > > When mdevctl starts the mdev, it will first create it using the
> > > > > existing mechanism, then apply aggregation attribute, which can
> > > consume
> > > > > the necessary additional instances from the parent device, or return
> an
> > > > > error, which would unwind and return a failure code to the caller
> > > > > (libvirt).  I think the vendor driver would then have freedom to
> decide
> > > > > when the attribute could be modified, for instance it would be
> entirely
> > > > > reasonable to return -EBUSY if the user attempts to modify the
> > > > > attribute while the mdev device is in-use.  Effectively aggregation
> > > > > simply becomes a standardized attribute with common meaning.
> > > Thoughts?
> > > > > [cc libvirt folks for their impression] Thanks,
> > > >
> > > > I think one problem is that before mdevctl start to create mdev you
> > > > don't know what vendor attributes are, as we apply mdev attributes
> > > > after create. You may need some lookup depending on parent.. I think
> > > > making aggregation like other vendor attribute for mdev might be the
> > > > simplest way, but do we want to define its behavior in formal? e.g
> > > > like previous discussed it should show maxium instances for
> aggregation,
> > > etc.
> > >
> > > Yes, we'd still want to standardize how we enable and discover
> 

Re: [libvirt] [PATCH 0/6] VFIO mdev aggregated resources handling

2019-11-19 Thread Alex Williamson
On Fri, 15 Nov 2019 04:24:35 +
"Tian, Kevin"  wrote:

> > From: Alex Williamson
> > Sent: Thursday, November 7, 2019 2:45 AM
> > 
> > On Wed, 6 Nov 2019 12:20:31 +0800
> > Zhenyu Wang  wrote:
> >   
> > > On 2019.11.05 14:10:42 -0700, Alex Williamson wrote:  
> > > > On Thu, 24 Oct 2019 13:08:23 +0800
> > > > Zhenyu Wang  wrote:
> > > >  
> > > > > Hi,
> > > > >
> > > > > This is a refresh for previous send of this series. I got impression 
> > > > > that
> > > > > some SIOV drivers would still deploy their own create and config  
> > method so  
> > > > > stopped effort on this. But seems this would still be useful for some 
> > > > >  
> > other  
> > > > > SIOV driver which may simply want capability to aggregate resources.  
> > So here's  
> > > > > refreshed series.
> > > > >
> > > > > 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 "aggregate=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 ",aggregate=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 "aggregated_instances" is created for each mdev device to  
> > show allocated number.  
> > > >
> > > > Given discussions we've had recently around libvirt interacting with
> > > > mdev, I think that libvirt would rather have an abstract interface via
> > > > mdevctl[1].  Therefore can you evaluate how mdevctl would support  
> > this  
> > > > creation extension?  It seems like it would fit within the existing
> > > > mdev and mdevctl framework if aggregation were simply a sysfs  
> > attribute  
> > > > for the device.  For example, the mdevctl steps might look like this:
> > > >
> > > > mdevctl define -u UUID -p PARENT -t TYPE
> > > > mdevctl modify -u UUID --addattr=mdev/aggregation --value=2
> > > > mdevctl start -u UUID  
> 
> Hi, Alex, can you elaborate why a sysfs attribute is more friendly
> to mdevctl? what is the complexity if having mdevctl to pass
> additional parameter at creation time as this series originally 
> proposed? Just want to clearly understand the limitation of the
> parameter way. :-)

We could also flip this question around, vfio-ap already uses sysfs to
finish composing a device after it's created, therefore why shouldn't
aggregation use this existing mechanism.  Extending the creation
interface is a more fundamental change than simply standardizing an
optional sysfs namespace entry.

> > > >
> > > > When mdevctl starts the mdev, it will first create it using the
> > > > existing mechanism, then apply aggregation attribute, which can  
> > consume  
> > > > the necessary additional instances from the parent device, or return an
> > > > error, which would unwind and return a failure code to the caller
> > > > (libvirt).  I think the vendor driver would then have freedom to decide
> > > > when the attribute could be modified, for instance it would be entirely
> > > > reasonable to return -EBUSY if the user attempts to modify the
> > > > attribute while the mdev device is in-use.  Effectively aggregation
> > > > simply becomes a standardized attribute with common meaning.  
> > Thoughts?  
> > > > [cc libvirt folks for their impression] Thanks,  
> > >
> > > I think one problem is that before mdevctl start to create mdev you
> > > don't know what vendor attributes are, as we apply mdev attributes
> > > after create. You may need some lookup depending on parent.. I think
> > > making aggregation like other vendor attribute for mdev might be the
> > > simplest way, but do we want to define its behavior in formal? e.g
> > > like previous discussed it should show maxium instances for aggregation,  
> > etc.
> > 
> > Yes, we'd still want to standardize how we enable and discover
> > aggregation since we expect multiple users.  Even if libvirt were to
> > use mdevctl as it's mdev interface, higher level tools should have an
> > introspection mechanism 

Re: [libvirt] [PATCH 0/6] VFIO mdev aggregated resources handling

2019-11-15 Thread Tian, Kevin
> From: Alex Williamson
> Sent: Thursday, November 7, 2019 2:45 AM
> 
> On Wed, 6 Nov 2019 12:20:31 +0800
> Zhenyu Wang  wrote:
> 
> > On 2019.11.05 14:10:42 -0700, Alex Williamson wrote:
> > > On Thu, 24 Oct 2019 13:08:23 +0800
> > > Zhenyu Wang  wrote:
> > >
> > > > Hi,
> > > >
> > > > This is a refresh for previous send of this series. I got impression 
> > > > that
> > > > some SIOV drivers would still deploy their own create and config
> method so
> > > > stopped effort on this. But seems this would still be useful for some
> other
> > > > SIOV driver which may simply want capability to aggregate resources.
> So here's
> > > > refreshed series.
> > > >
> > > > 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 "aggregate=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 ",aggregate=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 "aggregated_instances" is created for each mdev device to
> show allocated number.
> > >
> > > Given discussions we've had recently around libvirt interacting with
> > > mdev, I think that libvirt would rather have an abstract interface via
> > > mdevctl[1].  Therefore can you evaluate how mdevctl would support
> this
> > > creation extension?  It seems like it would fit within the existing
> > > mdev and mdevctl framework if aggregation were simply a sysfs
> attribute
> > > for the device.  For example, the mdevctl steps might look like this:
> > >
> > > mdevctl define -u UUID -p PARENT -t TYPE
> > > mdevctl modify -u UUID --addattr=mdev/aggregation --value=2
> > > mdevctl start -u UUID

Hi, Alex, can you elaborate why a sysfs attribute is more friendly
to mdevctl? what is the complexity if having mdevctl to pass
additional parameter at creation time as this series originally 
proposed? Just want to clearly understand the limitation of the
parameter way. :-)

> > >
> > > When mdevctl starts the mdev, it will first create it using the
> > > existing mechanism, then apply aggregation attribute, which can
> consume
> > > the necessary additional instances from the parent device, or return an
> > > error, which would unwind and return a failure code to the caller
> > > (libvirt).  I think the vendor driver would then have freedom to decide
> > > when the attribute could be modified, for instance it would be entirely
> > > reasonable to return -EBUSY if the user attempts to modify the
> > > attribute while the mdev device is in-use.  Effectively aggregation
> > > simply becomes a standardized attribute with common meaning.
> Thoughts?
> > > [cc libvirt folks for their impression] Thanks,
> >
> > I think one problem is that before mdevctl start to create mdev you
> > don't know what vendor attributes are, as we apply mdev attributes
> > after create. You may need some lookup depending on parent.. I think
> > making aggregation like other vendor attribute for mdev might be the
> > simplest way, but do we want to define its behavior in formal? e.g
> > like previous discussed it should show maxium instances for aggregation,
> etc.
> 
> Yes, we'd still want to standardize how we enable and discover
> aggregation since we expect multiple users.  Even if libvirt were to
> use mdevctl as it's mdev interface, higher level tools should have an
> introspection mechanism available.  Possibly the sysfs interfaces
> proposed in this series remains largely the same, but I think perhaps
> the implementation of them moves out to the vendor driver.  In fact,
> perhaps the only change to mdev core is to define the standard.  For
> example, the "aggregation" attribute on the type is potentially simply
> a defined, optional, per type attribute, similar to "name" and
> "description".  For "aggregated_instances" we already have the
> mdev_attr_groups of the mdev_parent_ops, we could define an
> attribute_group with .name = "mdev" as a set of standardized
> attributes, such that vendors could provide both their own vendor
> specific attributes and per 

Re: [libvirt] [PATCH 0/6] VFIO mdev aggregated resources handling

2019-11-07 Thread Cornelia Huck
On Wed, 6 Nov 2019 11:44:40 -0700
Alex Williamson  wrote:

> On Wed, 6 Nov 2019 12:20:31 +0800
> Zhenyu Wang  wrote:
> 
> > On 2019.11.05 14:10:42 -0700, Alex Williamson wrote:  
> > > On Thu, 24 Oct 2019 13:08:23 +0800
> > > Zhenyu Wang  wrote:
> > > 
> > > > Hi,
> > > > 
> > > > This is a refresh for previous send of this series. I got impression 
> > > > that
> > > > some SIOV drivers would still deploy their own create and config method 
> > > > so
> > > > stopped effort on this. But seems this would still be useful for some 
> > > > other
> > > > SIOV driver which may simply want capability to aggregate resources. So 
> > > > here's
> > > > refreshed series.
> > > > 
> > > > 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 "aggregate=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 ",aggregate=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 "aggregated_instances" is created for each mdev device to 
> > > > show allocated number.
> > > 
> > > Given discussions we've had recently around libvirt interacting with
> > > mdev, I think that libvirt would rather have an abstract interface via
> > > mdevctl[1].  Therefore can you evaluate how mdevctl would support this
> > > creation extension?  It seems like it would fit within the existing
> > > mdev and mdevctl framework if aggregation were simply a sysfs attribute
> > > for the device.  For example, the mdevctl steps might look like this:
> > > 
> > > mdevctl define -u UUID -p PARENT -t TYPE
> > > mdevctl modify -u UUID --addattr=mdev/aggregation --value=2
> > > mdevctl start -u UUID
> > > 
> > > When mdevctl starts the mdev, it will first create it using the
> > > existing mechanism, then apply aggregation attribute, which can consume
> > > the necessary additional instances from the parent device, or return an
> > > error, which would unwind and return a failure code to the caller
> > > (libvirt).  I think the vendor driver would then have freedom to decide
> > > when the attribute could be modified, for instance it would be entirely
> > > reasonable to return -EBUSY if the user attempts to modify the
> > > attribute while the mdev device is in-use.  Effectively aggregation
> > > simply becomes a standardized attribute with common meaning.  Thoughts?
> > > [cc libvirt folks for their impression] Thanks,
> > 
> > I think one problem is that before mdevctl start to create mdev you
> > don't know what vendor attributes are, as we apply mdev attributes
> > after create. You may need some lookup depending on parent.. I think
> > making aggregation like other vendor attribute for mdev might be the
> > simplest way, but do we want to define its behavior in formal? e.g
> > like previous discussed it should show maxium instances for aggregation, 
> > etc.  
> 
> Yes, we'd still want to standardize how we enable and discover
> aggregation since we expect multiple users.  Even if libvirt were to
> use mdevctl as it's mdev interface, higher level tools should have an
> introspection mechanism available.  Possibly the sysfs interfaces
> proposed in this series remains largely the same, but I think perhaps
> the implementation of them moves out to the vendor driver.  In fact,
> perhaps the only change to mdev core is to define the standard.  For
> example, the "aggregation" attribute on the type is potentially simply
> a defined, optional, per type attribute, similar to "name" and
> "description".  For "aggregated_instances" we already have the
> mdev_attr_groups of the mdev_parent_ops, we could define an
> attribute_group with .name = "mdev" as a set of standardized
> attributes, such that vendors could provide both their own vendor
> specific attributes and per device attributes with a common meaning and
> semantic defined in the mdev ABI.

+1 to standardizing this. While not every vendor driver will support
aggregation, 

Re: [libvirt] [PATCH 0/6] VFIO mdev aggregated resources handling

2019-11-06 Thread Alex Williamson
On Wed, 6 Nov 2019 12:20:31 +0800
Zhenyu Wang  wrote:

> On 2019.11.05 14:10:42 -0700, Alex Williamson wrote:
> > On Thu, 24 Oct 2019 13:08:23 +0800
> > Zhenyu Wang  wrote:
> >   
> > > Hi,
> > > 
> > > This is a refresh for previous send of this series. I got impression that
> > > some SIOV drivers would still deploy their own create and config method so
> > > stopped effort on this. But seems this would still be useful for some 
> > > other
> > > SIOV driver which may simply want capability to aggregate resources. So 
> > > here's
> > > refreshed series.
> > > 
> > > 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 "aggregate=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 ",aggregate=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 "aggregated_instances" is created for each mdev device to show 
> > > allocated number.  
> > 
> > Given discussions we've had recently around libvirt interacting with
> > mdev, I think that libvirt would rather have an abstract interface via
> > mdevctl[1].  Therefore can you evaluate how mdevctl would support this
> > creation extension?  It seems like it would fit within the existing
> > mdev and mdevctl framework if aggregation were simply a sysfs attribute
> > for the device.  For example, the mdevctl steps might look like this:
> > 
> > mdevctl define -u UUID -p PARENT -t TYPE
> > mdevctl modify -u UUID --addattr=mdev/aggregation --value=2
> > mdevctl start -u UUID
> > 
> > When mdevctl starts the mdev, it will first create it using the
> > existing mechanism, then apply aggregation attribute, which can consume
> > the necessary additional instances from the parent device, or return an
> > error, which would unwind and return a failure code to the caller
> > (libvirt).  I think the vendor driver would then have freedom to decide
> > when the attribute could be modified, for instance it would be entirely
> > reasonable to return -EBUSY if the user attempts to modify the
> > attribute while the mdev device is in-use.  Effectively aggregation
> > simply becomes a standardized attribute with common meaning.  Thoughts?
> > [cc libvirt folks for their impression] Thanks,  
> 
> I think one problem is that before mdevctl start to create mdev you
> don't know what vendor attributes are, as we apply mdev attributes
> after create. You may need some lookup depending on parent.. I think
> making aggregation like other vendor attribute for mdev might be the
> simplest way, but do we want to define its behavior in formal? e.g
> like previous discussed it should show maxium instances for aggregation, etc.

Yes, we'd still want to standardize how we enable and discover
aggregation since we expect multiple users.  Even if libvirt were to
use mdevctl as it's mdev interface, higher level tools should have an
introspection mechanism available.  Possibly the sysfs interfaces
proposed in this series remains largely the same, but I think perhaps
the implementation of them moves out to the vendor driver.  In fact,
perhaps the only change to mdev core is to define the standard.  For
example, the "aggregation" attribute on the type is potentially simply
a defined, optional, per type attribute, similar to "name" and
"description".  For "aggregated_instances" we already have the
mdev_attr_groups of the mdev_parent_ops, we could define an
attribute_group with .name = "mdev" as a set of standardized
attributes, such that vendors could provide both their own vendor
specific attributes and per device attributes with a common meaning and
semantic defined in the mdev ABI.

> The behavior change for driver is that previously aggregation is
> handled at create time, but for sysfs attr it should handle any
> resource allocation before it's really in-use. I think some SIOV
> driver which already requires some specific config should be ok,
> but not sure for other driver which might not be explored in this before.
> Would that be a problem? Kevin?


Re: [libvirt] [PATCH 0/6] VFIO mdev aggregated resources handling

2019-11-05 Thread Zhenyu Wang
On 2019.11.05 14:10:42 -0700, Alex Williamson wrote:
> On Thu, 24 Oct 2019 13:08:23 +0800
> Zhenyu Wang  wrote:
> 
> > Hi,
> > 
> > This is a refresh for previous send of this series. I got impression that
> > some SIOV drivers would still deploy their own create and config method so
> > stopped effort on this. But seems this would still be useful for some other
> > SIOV driver which may simply want capability to aggregate resources. So 
> > here's
> > refreshed series.
> > 
> > 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 "aggregate=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 ",aggregate=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 "aggregated_instances" is created for each mdev device to show 
> > allocated number.
> 
> Given discussions we've had recently around libvirt interacting with
> mdev, I think that libvirt would rather have an abstract interface via
> mdevctl[1].  Therefore can you evaluate how mdevctl would support this
> creation extension?  It seems like it would fit within the existing
> mdev and mdevctl framework if aggregation were simply a sysfs attribute
> for the device.  For example, the mdevctl steps might look like this:
> 
> mdevctl define -u UUID -p PARENT -t TYPE
> mdevctl modify -u UUID --addattr=mdev/aggregation --value=2
> mdevctl start -u UUID
> 
> When mdevctl starts the mdev, it will first create it using the
> existing mechanism, then apply aggregation attribute, which can consume
> the necessary additional instances from the parent device, or return an
> error, which would unwind and return a failure code to the caller
> (libvirt).  I think the vendor driver would then have freedom to decide
> when the attribute could be modified, for instance it would be entirely
> reasonable to return -EBUSY if the user attempts to modify the
> attribute while the mdev device is in-use.  Effectively aggregation
> simply becomes a standardized attribute with common meaning.  Thoughts?
> [cc libvirt folks for their impression] Thanks,

I think one problem is that before mdevctl start to create mdev you
don't know what vendor attributes are, as we apply mdev attributes
after create. You may need some lookup depending on parent.. I think
making aggregation like other vendor attribute for mdev might be the
simplest way, but do we want to define its behavior in formal? e.g
like previous discussed it should show maxium instances for aggregation, etc.

The behavior change for driver is that previously aggregation is
handled at create time, but for sysfs attr it should handle any
resource allocation before it's really in-use. I think some SIOV
driver which already requires some specific config should be ok,
but not sure for other driver which might not be explored in this before.
Would that be a problem? Kevin?

Thanks

> 
> Alex
> 
> [1] https://github.com/mdevctl/mdevctl
> 
> > 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
> > 
> > Zhenyu Wang (6):
> >   vfio/mdev: Add new "aggregate" parameter for mdev create
> >   vfio/mdev: Add "aggregation" attribute for supported mdev type
> >   vfio/mdev: Add "aggregated_instances" attribute for supported mdev
> > device
> >   Documentation/driver-api/vfio-mediated-device.rst: Update for
> > vfio/mdev aggregation support
> >   Documentation/ABI/testing/sysfs-bus-vfio-mdev: Update for vfio/mdev
> > aggregation support
> >   drm/i915/gvt: Add new type with aggregation support
> > 
> >  Documentation/ABI/testing/sysfs-bus-vfio-mdev | 24 ++
> >  .../driver-api/vfio-mediated-device.rst   | 23 ++
> >  drivers/gpu/drm/i915/gvt/gvt.c|  4 +-
> >  drivers/gpu/drm/i915/gvt/gvt.h| 11 ++-
> >  drivers/gpu/drm/i915/gvt/kvmgt.c  | 53 

Re: [libvirt] [PATCH 0/6] VFIO mdev aggregated resources handling

2019-11-05 Thread Alex Williamson
On Thu, 24 Oct 2019 13:08:23 +0800
Zhenyu Wang  wrote:

> Hi,
> 
> This is a refresh for previous send of this series. I got impression that
> some SIOV drivers would still deploy their own create and config method so
> stopped effort on this. But seems this would still be useful for some other
> SIOV driver which may simply want capability to aggregate resources. So here's
> refreshed series.
> 
> 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 "aggregate=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 ",aggregate=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 "aggregated_instances" is created for each mdev device to show 
> allocated number.

Given discussions we've had recently around libvirt interacting with
mdev, I think that libvirt would rather have an abstract interface via
mdevctl[1].  Therefore can you evaluate how mdevctl would support this
creation extension?  It seems like it would fit within the existing
mdev and mdevctl framework if aggregation were simply a sysfs attribute
for the device.  For example, the mdevctl steps might look like this:

mdevctl define -u UUID -p PARENT -t TYPE
mdevctl modify -u UUID --addattr=mdev/aggregation --value=2
mdevctl start -u UUID

When mdevctl starts the mdev, it will first create it using the
existing mechanism, then apply aggregation attribute, which can consume
the necessary additional instances from the parent device, or return an
error, which would unwind and return a failure code to the caller
(libvirt).  I think the vendor driver would then have freedom to decide
when the attribute could be modified, for instance it would be entirely
reasonable to return -EBUSY if the user attempts to modify the
attribute while the mdev device is in-use.  Effectively aggregation
simply becomes a standardized attribute with common meaning.  Thoughts?
[cc libvirt folks for their impression] Thanks,

Alex

[1] https://github.com/mdevctl/mdevctl

> 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
> 
> Zhenyu Wang (6):
>   vfio/mdev: Add new "aggregate" parameter for mdev create
>   vfio/mdev: Add "aggregation" attribute for supported mdev type
>   vfio/mdev: Add "aggregated_instances" attribute for supported mdev
> device
>   Documentation/driver-api/vfio-mediated-device.rst: Update for
> vfio/mdev aggregation support
>   Documentation/ABI/testing/sysfs-bus-vfio-mdev: Update for vfio/mdev
> aggregation support
>   drm/i915/gvt: Add new type with aggregation support
> 
>  Documentation/ABI/testing/sysfs-bus-vfio-mdev | 24 ++
>  .../driver-api/vfio-mediated-device.rst   | 23 ++
>  drivers/gpu/drm/i915/gvt/gvt.c|  4 +-
>  drivers/gpu/drm/i915/gvt/gvt.h| 11 ++-
>  drivers/gpu/drm/i915/gvt/kvmgt.c  | 53 -
>  drivers/gpu/drm/i915/gvt/vgpu.c   | 56 -
>  drivers/vfio/mdev/mdev_core.c | 36 -
>  drivers/vfio/mdev/mdev_private.h  |  6 +-
>  drivers/vfio/mdev/mdev_sysfs.c| 79 ++-
>  include/linux/mdev.h  | 19 +
>  10 files changed, 294 insertions(+), 17 deletions(-)
> 

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