4:52 a.m.
On Fri, Apr 17, 2020 at 04:44:50PM +0800, Cornelia Huck wrote:
On Mon, 13 Apr 2020 01:52:01 -0400
Yan Zhao <yan.y.zhao(a)intel.com> wrote:
> This patchset introduces a migration_version attribute under sysfs of VFIO
> Mediated devices.
>
> This migration_version attribute is used to check migration compatibility
> between two mdev devices.
>
> Currently, it has two locations:
> (1) under mdev_type node,
> which can be used even before device creation, but only for mdev
> devices of the same mdev type.
> (2) under mdev device node,
> which can only be used after the mdev devices are created, but the src
> and target mdev devices are not necessarily be of the same mdev type
> (The second location is newly added in v5, in order to keep consistent
> with the migration_version node for migratable pass-though devices)
What is the relationship between those two attributes?
(1) is for mdev devices specifically, and (2) is provided to keep the same
sysfs interface as with non-mdev cases. so (2) is for both mdev devices and
non-mdev devices.
in future, if we enable vfio-pci vendor ops, (i.e. a non-mdev device
is binding to vfio-pci, but is able to register migration region and do
migration transactions from a vendor provided affiliate driver),
the vendor driver would export (2) directly, under device node.
It is not able to provide (1) as there're no mdev devices involved.
Is existence (and compatibility) of (1) a pre-req for possible
existence (and compatibility) of (2)?
no. (2) does not reply on (1).
Does userspace need to check (1) or can it completely rely on (2),
if
it so chooses?
I think it can completely reply on (2) if compatibility check before
mdev creation is not required.
If devices with a different mdev type are indeed compatible, it
seems
userspace can only find out after the devices have actually been
created, as (1) does not apply?
yes, I think so.
One of my worries is that the existence of an attribute with the
same
name in two similar locations might lead to confusion. But maybe it
isn't a problem.
Yes, I have the same feeling. but as (2) is for sysfs interface
consistency, to make it transparent to userspace tools like libvirt,
I guess the same name is necessary?
Thanks
Yan
>
> Patch 1 defines migration_version attribute for the first location in
> Documentation/vfio-mediated-device.txt
>
> Patch 2 uses GVT as an example for patch 1 to show how to expose
> migration_version attribute and check migration compatibility in vendor
> driver.
>
> Patch 3 defines migration_version attribute for the second location in
> Documentation/vfio-mediated-device.txt
>
> Patch 4 uses GVT as an example for patch 3 to show how to expose
> migration_version attribute and check migration compatibility in vendor
> driver.
>
> (The previous "Reviewed-by" and "Acked-by" for patch 1 and patch
2 are
> kept in v5, as there are only small changes to commit messages of the two
> patches.)
>
> v5:
> added patch 2 and 4 for mdev device part of migration_version attribute.
>
> v4:
> 1. fixed indentation/spell errors, reworded several error messages
> 2. added a missing memory free for error handling in patch 2
>
> v3:
> 1. renamed version to migration_version
> 2. let errno to be freely defined by vendor driver
> 3. let checking mdev_type be prerequisite of migration compatibility check
> 4. reworded most part of patch 1
> 5. print detailed error log in patch 2 and generate migration_version
> string at init time
>
> v2:
> 1. renamed patched 1
> 2. made definition of device version string completely private to vendor
> driver
> 3. reverted changes to sample mdev drivers
> 4. described intent and usage of version attribute more clearly.
>
>
> Yan Zhao (4):
> vfio/mdev: add migration_version attribute for mdev (under mdev_type
> node)
> drm/i915/gvt: export migration_version to mdev sysfs (under mdev_type
> node)
> vfio/mdev: add migration_version attribute for mdev (under mdev device
> node)
> drm/i915/gvt: export migration_version to mdev sysfs (under mdev
> device node)
>
> .../driver-api/vfio-mediated-device.rst | 183 ++++++++++++++++++
> drivers/gpu/drm/i915/gvt/Makefile | 2 +-
> drivers/gpu/drm/i915/gvt/gvt.c | 39 ++++
> drivers/gpu/drm/i915/gvt/gvt.h | 7 +
> drivers/gpu/drm/i915/gvt/kvmgt.c | 55 ++++++
> drivers/gpu/drm/i915/gvt/migration_version.c | 170 ++++++++++++++++
> drivers/gpu/drm/i915/gvt/vgpu.c | 13 +-
> 7 files changed, 466 insertions(+), 3 deletions(-)
> create mode 100644 drivers/gpu/drm/i915/gvt/migration_version.c
>