Re: [PATCH v3 0/2] VFIO mdev aggregated resources handling

[Zhenyu Wang <zhenyuw@linux.intel.com>]

[-- Attachment #1: Type: text/plain, Size: 7265 bytes --]

On 2020.07.08 06:31:00 +0000, Tian, Kevin wrote:
> > From: Alex Williamson <alex.williamson@redhat.com>
> > Sent: Wednesday, July 8, 2020 9:07 AM
> > 
> > On Tue, 7 Jul 2020 23:28:39 +0000
> > "Tian, Kevin" <kevin.tian@intel.com> wrote:
> > 
> > > Hi, Alex,
> > >
> > > Gentle ping... Please let us know whether this version looks good.
> > 
> > I figured this is entangled with the versioning scheme.  There are
> > unanswered questions about how something that assumes a device of a
> > given type is software compatible to another device of the same type
> > handles aggregation and how the type class would indicate compatibility
> > with an aggregated instance.  Thanks,
> > 
> 
> Yes, this open is an interesting topic. I didn't closely follow the versioning
> scheme discussion. Below is some preliminary thought in my mind:
> 
> --
> First, let's consider migrating an aggregated instance:
> 
> A conservative policy is to check whether the compatible type is supported 
> on target device and whether available instances under that type can afford 
> the ask of the aggregated instance. Compatibility check in this scheme is 
> separated from aggregation check, then no change is required to the current 
> versioning interface.

In last mdev's aggregation series, no aggregation info is exposed in mdev type
until instance creates, so that would cause possible conflict w/o that info, e.g
type might have avail instances but not actually provide aggregation. Then from
that point of view, either require to add new flag because current 'description'
is useless or change versioning interface or require to be different type..

> 
> Then there comes a case where the target device doesn't handle aggregation
> but support a different type which however provides compatible capabilities 
> and same resource size as the aggregated instance expects. I guess this is
> one puzzle how to check compatibility between such types. One possible
> extension is to introduce a non_aggregated_list  to indicate compatible 
> non-aggregated types for each aggregated instance. Then mgmt.. stack 
> just loop the compatible list if the conservative policy fails.  I didn't think 
> carefully about what format is reasonable here. But if we agree that an
> separate interface is required to support such usage, then this may come
> later after the basic migration_version interface is completed.
> --
> 
> Another scenario is about migrating a non-aggregated instance to a device
> handling aggregation. Then there is an open whether an aggregated type 
> can be used to back the non-aggregated instance in case of no available 
> instance under the original type claimed by non-aggregated instance. 
> This won't happen in KVMGT, because all vGPU types share the same 
> resource pool. Allocating instance under one type also decrement available 
> instances under other types. So if we fail to find available instance under 
> type-A (with 4x resource of type-B), then we will also fail to create an
>  aggregated instance (aggregate=4) under type-B. therefore, we just 
> need stick to basic type compatibility check for non-aggregated instance. 
> And I feel this assumption can be applied to other devices handling 
> aggregation. It doesn't make sense for two types to claim compatibility 
> (only with resource size difference) when their resources are allocated
> from different pools (which usually implies different capability or QOS/
> SLA difference). With this assumption, we don't need provide another
> interface to indicate compatible aggregated types for non-aggregated
> interface.
> --
> 
> I may definitely overlook something here, but if above analysis sounds
> reasonable, then this series could be decoupled from the versioning 
> scheme discussion based on conservative policy for now. :)
> 
> Thanks
> Kevin
> 
> > 
> > 
> > > > From: Zhenyu Wang <zhenyuw@linux.intel.com>
> > > > Sent: Wednesday, April 8, 2020 1:58 PM
> > > >
> > > > Hi,
> > > >
> > > > This is a refresh on previous series:
> > > > https://patchwork.kernel.org/cover/11208279/
> > > > and https://patchwork.freedesktop.org/series/70425/
> > > >
> > > > 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].
> > > >
> > > > As we agreed that for current opaque mdev device type, we'd still
> > > > explore management interface based on mdev sysfs definition. And this
> > > > one tries to follow Alex's previous suggestion to create generic
> > > > parameters under 'mdev' directory for each device, so vendor driver
> > > > could provide support like as other defined mdev sysfs entries.
> > > >
> > > > For mdev type with aggregation support, files as "aggregated_instances"
> > > > and "max_aggregation" should be created under 'mdev' directory. E.g
> > > >
> > > > /sys/devices/pci0000:00/0000:00:02.0/<UUID>/mdev/
> > > >    |-- aggregated_instances
> > > >    |-- max_aggregation
> > > >
> > > > "aggregated_instances" is used to set or return current number of
> > > > instances for aggregation, which can not be larger than
> > "max_aggregation".
> > > >
> > > > The first patch is to update the document for new mdev parameter
> > directory.
> > > > The second one is to add aggregation support in GVT driver.
> > > >
> > > > 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
> > > >
> > > > Changelog:
> > > > v3:
> > > > - add more description for sysfs entries
> > > > - rebase GVT support
> > > > - rename accounting function
> > > >
> > > > Zhenyu Wang (2):
> > > >   Documentation/driver-api/vfio-mediated-device.rst: update for
> > > >     aggregation support
> > > >   drm/i915/gvt: mdev aggregation type
> > > >
> > > >  .../driver-api/vfio-mediated-device.rst       |  22 +++
> > > >  drivers/gpu/drm/i915/gvt/aperture_gm.c        |  44 +++--
> > > >  drivers/gpu/drm/i915/gvt/gtt.c                |   9 +-
> > > >  drivers/gpu/drm/i915/gvt/gvt.c                |   7 +-
> > > >  drivers/gpu/drm/i915/gvt/gvt.h                |  42 +++--
> > > >  drivers/gpu/drm/i915/gvt/kvmgt.c              | 115 +++++++++++-
> > > >  drivers/gpu/drm/i915/gvt/vgpu.c               | 172 ++++++++++++------
> > > >  7 files changed, 317 insertions(+), 94 deletions(-)
> > > >
> > > > --
> > > > 2.25.1
> > >
> 

-- 
Open Source Technology Center, Intel ltd.

$gpg --keyserver wwwkeys.pgp.net --recv-keys 4D781827

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]