[PATCH v3 0/2] introduction of migration_version attribute for VFIO live migration

[PATCH v3 0/2] introduction of migration_version attribute for VFIO live migration

[Yan Zhao]

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 of the same mdev type.

Patch 1 defines migration_version attribute in
Documentation/vfio-mediated-device.txt

Patch 2 uses GVT as an example to show how to expose migration_version
attribute and check migration compatibility in vendor driver.


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 (2):
  vfio/mdev: add migration_version attribute for mdev device
  drm/i915/gvt: export migration_version to mdev sysfs for Intel vGPU

 Documentation/vfio-mediated-device.txt       | 113 +++++++++++++
 drivers/gpu/drm/i915/gvt/Makefile            |   2 +-
 drivers/gpu/drm/i915/gvt/gvt.c               |  39 +++++
 drivers/gpu/drm/i915/gvt/gvt.h               |   5 +
 drivers/gpu/drm/i915/gvt/migration_version.c | 167 +++++++++++++++++++
 drivers/gpu/drm/i915/gvt/vgpu.c              |  13 +-
 6 files changed, 336 insertions(+), 3 deletions(-)
 create mode 100644 drivers/gpu/drm/i915/gvt/migration_version.c

-- 
2.17.1

[Qemu-devel] [PATCH v3 0/2] introduction of migration_version attribute for VFIO live migration

[Yan Zhao]

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 of the same mdev type.

Patch 1 defines migration_version attribute in
Documentation/vfio-mediated-device.txt

Patch 2 uses GVT as an example to show how to expose migration_version
attribute and check migration compatibility in vendor driver.


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 (2):
  vfio/mdev: add migration_version attribute for mdev device
  drm/i915/gvt: export migration_version to mdev sysfs for Intel vGPU

 Documentation/vfio-mediated-device.txt       | 113 +++++++++++++
 drivers/gpu/drm/i915/gvt/Makefile            |   2 +-
 drivers/gpu/drm/i915/gvt/gvt.c               |  39 +++++
 drivers/gpu/drm/i915/gvt/gvt.h               |   5 +
 drivers/gpu/drm/i915/gvt/migration_version.c | 167 +++++++++++++++++++
 drivers/gpu/drm/i915/gvt/vgpu.c              |  13 +-
 6 files changed, 336 insertions(+), 3 deletions(-)
 create mode 100644 drivers/gpu/drm/i915/gvt/migration_version.c

-- 
2.17.1

[PATCH v3 1/2] vfio/mdev: add migration_version attribute for mdev device

[Yan Zhao]

migration_version attribute is used to check migration compatibility
between two mdev device of the same mdev type.
The key is that it's rw and its data is opaque to userspace.

Userspace reads migration_version of mdev device at source side and
writes the value to migration_version attribute of mdev device at target
side. It judges migration compatibility according to whether the read
and write operations succeed or fail.

As this attribute is under mdev_type node, userspace is able to know
whether two mdev devices are compatible before a mdev device is created.

userspace needs to check whether the two mdev devices are of the same
mdev type before checking the migration_version attribute. It also needs
to check device creation parameters if aggregation is supported in
future.

             __    userspace
              /\              \
             /                 \write
            / read              \
   ________/__________       ___\|/_____________
  | migration_version |     | migration_version |-->check migration
  ---------------------     ---------------------   compatibility
    mdev device A               mdev device B

Cc: Alex Williamson <alex.williamson@redhat.com>
Cc: Erik Skultety <eskultet@redhat.com>
Cc: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
Cc: Cornelia Huck <cohuck@redhat.com>
Cc: "Tian, Kevin" <kevin.tian@intel.com>
Cc: Zhenyu Wang <zhenyuw@linux.intel.com>
Cc: "Wang, Zhi A" <zhi.a.wang@intel.com>
Cc: Neo Jia <cjia@nvidia.com>
Cc: Kirti Wankhede <kwankhede@nvidia.com>
Cc: Daniel P. Berrangé <berrange@redhat.com>
Cc: Christophe de Dinechin <dinechin@redhat.com>

Signed-off-by: Yan Zhao <yan.y.zhao@intel.com>

---
v3:
1. renamed version to migration_version
(Christophe de Dinechin, Cornelia Huck, Alex Williamson)
2. let errno to be freely defined by vendor driver
(Alex Williamson, Erik Skultety, Cornelia Huck, Dr. David Alan Gilbert)
3. let checking mdev_type be prerequisite of migration compatibility
check. (Alex Williamson)
4. reworded example usage section.
(most of this section came from Alex Williamson)
5. reworded attribute intention section (Cornelia Huck)

v2:
1. added detailed intent and usage
2. made definition of version string completely private to vendor driver
   (Alex Williamson)
3. abandoned changes to sample mdev drivers (Alex Williamson)
4. mandatory --> optional (Cornelia Huck)
5. added description for errno (Cornelia Huck)
---
 Documentation/vfio-mediated-device.txt | 113 +++++++++++++++++++++++++
 1 file changed, 113 insertions(+)

diff --git a/Documentation/vfio-mediated-device.txt b/Documentation/vfio-mediated-device.txt
index c3f69bcaf96e..1241e1cee64e 100644
--- a/Documentation/vfio-mediated-device.txt
+++ b/Documentation/vfio-mediated-device.txt
@@ -202,6 +202,7 @@ Directories and files under the sysfs for Each Physical Device
   |     |   |--- available_instances
   |     |   |--- device_api
   |     |   |--- description
+  |     |   |--- migration_version
   |     |   |--- [devices]
   |     |--- [<type-id>]
   |     |   |--- create
@@ -209,6 +210,7 @@ Directories and files under the sysfs for Each Physical Device
   |     |   |--- available_instances
   |     |   |--- device_api
   |     |   |--- description
+  |     |   |--- migration_version
   |     |   |--- [devices]
   |     |--- [<type-id>]
   |          |--- create
@@ -216,6 +218,7 @@ Directories and files under the sysfs for Each Physical Device
   |          |--- available_instances
   |          |--- device_api
   |          |--- description
+  |          |--- migration_version
   |          |--- [devices]
 
 * [mdev_supported_types]
@@ -246,6 +249,116 @@ Directories and files under the sysfs for Each Physical Device
   This attribute should show the number of devices of type <type-id> that can be
   created.
 
+* migration_version
+
+  This attribute is rw, and is optional.
+  It is used to check migration compatibility between two mdev devices of the
+  same mdev type. Absence of this attribute means the device of type <type-id>
+  does not support migration.
+  This attribute provides a way to check migration compatibility between two
+  mdev devices from userspace even before device creation. The intended usage is
+  for userspace to read the migration_version attribute from one mdev device and
+  then writing that value to the migration_version attribute of the other mdev
+  device. The second mdev device indicates compatibility via the return code of
+  the write operation. This makes compatibility between mdev devices completely
+  vendor-defined and opaque to userspace. Userspace should do nothing more
+  than verify the mdev types match and then use the migration_version attribute
+  to confirm source to target compatibility.
+
+  Reading/Writing Attribute Data:
+  read(2) will fail if device of type <type-id> does not support migration and
+          otherwise succeed and return migration_version string of the device of
+          type <type-id>.
+
+          This migration_version string is vendor defined and opaque to the
+          userspace. Vendor is free to include whatever they feel is relevant.
+          e.g. <pciid of parent device>-<software version>.
+
+          Restrictions on this migration_version string:
+            1. It should only contain ascii characters
+            2. MAX Length is PATH_MAX (4096)
+
+  write(2) expects migration_version string of source mdev device, and will
+          succeed if it is determined to be compatible and otherwise fail with
+          vendor specific errno.
+
+  Errno:
+  -An errno on read(2) indicates the device of type <type-id> does not support
+  migration;
+  -An errno on write(2) indicates the devices are incompatible or the target
+  doesn't support migration.
+  Vendor driver is free to define specific errno and is suggested to
+  print detailed error in syslog for diagnose purpose.
+
+  Userspace should treat ANY of below conditions as two mdev devices not
+  compatible:
+  (0) The mdev devices are not of the same type
+  (1) any one of the two mdev devices does not have a migration_version
+  attribute
+  (2) error when reading from migration_version attribute of one mdev device
+  (3) error when writing migration_version string of one mdev device to
+  migration_version attribute of the other mdev device
+
+  Userspace should regard two mdev devices compatible when ALL of below
+  conditions are met:
+  (0) The mdev devices are of the same type
+  (1) success when reading from migration_version attribute of one mdev device.
+  (2) success when writing migration_version string of one mdev device to
+  migration_version attribute of the other mdev device.
+
+  Example Usage:
+  (1) Compare mdev types:
+
+  The mdev type of an instantiated device can be read from the mdev_type link
+  within the device instance in sysfs, for example:
+
+  # basename $(readlink -f /sys/bus/mdev/devices/$MDEV_UUID/mdev_type/)
+
+  The mdev types available on a given host system can also be found through
+  /sys/class/mdev_bus, for example:
+
+  # ls /sys/class/mdev_bus/*/mdev_supported_types/
+
+  Migration is only possible between devices of the same mdev type.
+
+  (2) Retrieve the mdev source migration_version:
+
+  The migration_version information can either be read from the mdev_type link
+  on an instantiated device:
+
+  # cat /sys/bus/mdev/devices/$UUID1/mdev_type/migration_version
+
+  Or it can be read from the mdev type definition, for example:
+
+  # cat /sys/class/mdev_bus/*/mdev_supported_types/$MDEV_TYPE/migration_version
+
+  If reading the source migration_version generates an error, migration is not
+  possible.
+  NB, there might be several parent devices for a given mdev type on a host
+  system, each may support or expose different migration_versions.
+  Matching the specific mdev type to a parent may become important in such
+  configurations.
+
+  (3) Test source migration_version at target:
+
+  Given a migration_version as outlined above, its compatibility to an
+  instantiated device of the same mdev type can be tested as:
+  # echo $VERSION > /sys/bus/mdev/devices/$UUID2/mdev_type/migration_version
+
+  If this write fails, the source and target migration versions are not
+  compatible or the target does not support migration.
+
+  Compatibility can also be tested prior to target device creation using the
+  mdev type definition for a parent device with a previously found matching mdev
+  type, for example:
+
+  # echo $VERSION > \
+  /sys/class/mdev_bus/$PARENT/mdev_supported_types/$MDEV_TYPE/migration_version
+
+  Again, an error writing the migration_version indicates that an instance of
+  this mdev type would not support a migration from the provided migration
+  version.
+
 * [device]
 
   This directory contains links to the devices of type <type-id> that have been
-- 
2.17.1

[Qemu-devel] [PATCH v3 1/2] vfio/mdev: add migration_version attribute for mdev device

[Yan Zhao]

migration_version attribute is used to check migration compatibility
between two mdev device of the same mdev type.
The key is that it's rw and its data is opaque to userspace.

Userspace reads migration_version of mdev device at source side and
writes the value to migration_version attribute of mdev device at target
side. It judges migration compatibility according to whether the read
and write operations succeed or fail.

As this attribute is under mdev_type node, userspace is able to know
whether two mdev devices are compatible before a mdev device is created.

userspace needs to check whether the two mdev devices are of the same
mdev type before checking the migration_version attribute. It also needs
to check device creation parameters if aggregation is supported in
future.

             __    userspace
              /\              \
             /                 \write
            / read              \
   ________/__________       ___\|/_____________
  | migration_version |     | migration_version |-->check migration
  ---------------------     ---------------------   compatibility
    mdev device A               mdev device B

Cc: Alex Williamson <alex.williamson@redhat.com>
Cc: Erik Skultety <eskultet@redhat.com>
Cc: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
Cc: Cornelia Huck <cohuck@redhat.com>
Cc: "Tian, Kevin" <kevin.tian@intel.com>
Cc: Zhenyu Wang <zhenyuw@linux.intel.com>
Cc: "Wang, Zhi A" <zhi.a.wang@intel.com>
Cc: Neo Jia <cjia@nvidia.com>
Cc: Kirti Wankhede <kwankhede@nvidia.com>
Cc: Daniel P. Berrangé <berrange@redhat.com>
Cc: Christophe de Dinechin <dinechin@redhat.com>

Signed-off-by: Yan Zhao <yan.y.zhao@intel.com>

---
v3:
1. renamed version to migration_version
(Christophe de Dinechin, Cornelia Huck, Alex Williamson)
2. let errno to be freely defined by vendor driver
(Alex Williamson, Erik Skultety, Cornelia Huck, Dr. David Alan Gilbert)
3. let checking mdev_type be prerequisite of migration compatibility
check. (Alex Williamson)
4. reworded example usage section.
(most of this section came from Alex Williamson)
5. reworded attribute intention section (Cornelia Huck)

v2:
1. added detailed intent and usage
2. made definition of version string completely private to vendor driver
   (Alex Williamson)
3. abandoned changes to sample mdev drivers (Alex Williamson)
4. mandatory --> optional (Cornelia Huck)
5. added description for errno (Cornelia Huck)
---
 Documentation/vfio-mediated-device.txt | 113 +++++++++++++++++++++++++
 1 file changed, 113 insertions(+)

diff --git a/Documentation/vfio-mediated-device.txt b/Documentation/vfio-mediated-device.txt
index c3f69bcaf96e..1241e1cee64e 100644
--- a/Documentation/vfio-mediated-device.txt
+++ b/Documentation/vfio-mediated-device.txt
@@ -202,6 +202,7 @@ Directories and files under the sysfs for Each Physical Device
   |     |   |--- available_instances
   |     |   |--- device_api
   |     |   |--- description
+  |     |   |--- migration_version
   |     |   |--- [devices]
   |     |--- [<type-id>]
   |     |   |--- create
@@ -209,6 +210,7 @@ Directories and files under the sysfs for Each Physical Device
   |     |   |--- available_instances
   |     |   |--- device_api
   |     |   |--- description
+  |     |   |--- migration_version
   |     |   |--- [devices]
   |     |--- [<type-id>]
   |          |--- create
@@ -216,6 +218,7 @@ Directories and files under the sysfs for Each Physical Device
   |          |--- available_instances
   |          |--- device_api
   |          |--- description
+  |          |--- migration_version
   |          |--- [devices]
 
 * [mdev_supported_types]
@@ -246,6 +249,116 @@ Directories and files under the sysfs for Each Physical Device
   This attribute should show the number of devices of type <type-id> that can be
   created.
 
+* migration_version
+
+  This attribute is rw, and is optional.
+  It is used to check migration compatibility between two mdev devices of the
+  same mdev type. Absence of this attribute means the device of type <type-id>
+  does not support migration.
+  This attribute provides a way to check migration compatibility between two
+  mdev devices from userspace even before device creation. The intended usage is
+  for userspace to read the migration_version attribute from one mdev device and
+  then writing that value to the migration_version attribute of the other mdev
+  device. The second mdev device indicates compatibility via the return code of
+  the write operation. This makes compatibility between mdev devices completely
+  vendor-defined and opaque to userspace. Userspace should do nothing more
+  than verify the mdev types match and then use the migration_version attribute
+  to confirm source to target compatibility.
+
+  Reading/Writing Attribute Data:
+  read(2) will fail if device of type <type-id> does not support migration and
+          otherwise succeed and return migration_version string of the device of
+          type <type-id>.
+
+          This migration_version string is vendor defined and opaque to the
+          userspace. Vendor is free to include whatever they feel is relevant.
+          e.g. <pciid of parent device>-<software version>.
+
+          Restrictions on this migration_version string:
+            1. It should only contain ascii characters
+            2. MAX Length is PATH_MAX (4096)
+
+  write(2) expects migration_version string of source mdev device, and will
+          succeed if it is determined to be compatible and otherwise fail with
+          vendor specific errno.
+
+  Errno:
+  -An errno on read(2) indicates the device of type <type-id> does not support
+  migration;
+  -An errno on write(2) indicates the devices are incompatible or the target
+  doesn't support migration.
+  Vendor driver is free to define specific errno and is suggested to
+  print detailed error in syslog for diagnose purpose.
+
+  Userspace should treat ANY of below conditions as two mdev devices not
+  compatible:
+  (0) The mdev devices are not of the same type
+  (1) any one of the two mdev devices does not have a migration_version
+  attribute
+  (2) error when reading from migration_version attribute of one mdev device
+  (3) error when writing migration_version string of one mdev device to
+  migration_version attribute of the other mdev device
+
+  Userspace should regard two mdev devices compatible when ALL of below
+  conditions are met:
+  (0) The mdev devices are of the same type
+  (1) success when reading from migration_version attribute of one mdev device.
+  (2) success when writing migration_version string of one mdev device to
+  migration_version attribute of the other mdev device.
+
+  Example Usage:
+  (1) Compare mdev types:
+
+  The mdev type of an instantiated device can be read from the mdev_type link
+  within the device instance in sysfs, for example:
+
+  # basename $(readlink -f /sys/bus/mdev/devices/$MDEV_UUID/mdev_type/)
+
+  The mdev types available on a given host system can also be found through
+  /sys/class/mdev_bus, for example:
+
+  # ls /sys/class/mdev_bus/*/mdev_supported_types/
+
+  Migration is only possible between devices of the same mdev type.
+
+  (2) Retrieve the mdev source migration_version:
+
+  The migration_version information can either be read from the mdev_type link
+  on an instantiated device:
+
+  # cat /sys/bus/mdev/devices/$UUID1/mdev_type/migration_version
+
+  Or it can be read from the mdev type definition, for example:
+
+  # cat /sys/class/mdev_bus/*/mdev_supported_types/$MDEV_TYPE/migration_version
+
+  If reading the source migration_version generates an error, migration is not
+  possible.
+  NB, there might be several parent devices for a given mdev type on a host
+  system, each may support or expose different migration_versions.
+  Matching the specific mdev type to a parent may become important in such
+  configurations.
+
+  (3) Test source migration_version at target:
+
+  Given a migration_version as outlined above, its compatibility to an
+  instantiated device of the same mdev type can be tested as:
+  # echo $VERSION > /sys/bus/mdev/devices/$UUID2/mdev_type/migration_version
+
+  If this write fails, the source and target migration versions are not
+  compatible or the target does not support migration.
+
+  Compatibility can also be tested prior to target device creation using the
+  mdev type definition for a parent device with a previously found matching mdev
+  type, for example:
+
+  # echo $VERSION > \
+  /sys/class/mdev_bus/$PARENT/mdev_supported_types/$MDEV_TYPE/migration_version
+
+  Again, an error writing the migration_version indicates that an instance of
+  this mdev type would not support a migration from the provided migration
+  version.
+
 * [device]
 
   This directory contains links to the devices of type <type-id> that have been
-- 
2.17.1

[PATCH v3 2/2] drm/i915/gvt: export migration_version to mdev sysfs for Intel vGPU

[Yan Zhao]

This feature implements the migration_version attribute for Intel's vGPU
mdev devices.

migration_version attribute is rw.
It's used to check migration compatibility for two mdev devices of the
same mdev type.
migration_version string is defined by vendor driver and opaque to
userspace.

For Intel vGPU of gen8 and gen9, the format of migration_version string
is:
  <vendor id>-<device id>-<vgpu type>-<software version>.

For future platforms, the format of migration_version string is to be
expanded to include more meta data to identify Intel vGPUs for live
migration compatibility check

For old platforms, and for GVT not supporting vGPU live migration
feature, -ENODEV is returned on read(2)/write(2) of migration_version
attribute.
For vGPUs running old GVT who do not expose migration_version
attribute, live migration is regarded as not supported for those vGPUs.

Cc: Alex Williamson <alex.williamson@redhat.com>
Cc: Erik Skultety <eskultet@redhat.com>
Cc: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
Cc: Cornelia Huck <cohuck@redhat.com>
Cc: "Tian, Kevin" <kevin.tian@intel.com>
Cc: Zhenyu Wang <zhenyuw@linux.intel.com>
Cc: "Wang, Zhi A" <zhi.a.wang@intel.com>
c: Neo Jia <cjia@nvidia.com>
Cc: Kirti Wankhede <kwankhede@nvidia.com>

Signed-off-by: Yan Zhao <yan.y.zhao@intel.com>

---
v3:
1. renamed version to migration_version
(Christophe de Dinechin, Cornelia Huck, Alex Williamson)
2. instead of generating migration version strings each time, storing
them in vgpu types generated during initialization.
(Zhenyu Wang, Cornelia Huck)
3. replaced multiple snprintf to one big snprintf in
intel_gvt_get_vfio_migration_version()
(Dr. David Alan Gilbert)
4. printed detailed error log
(Alex Williamson, Erik Skultety, Cornelia Huck, Dr. David Alan Gilbert)
5. incorporated <software version> into migration_version string
(Alex Williamson)
6. do not use ifndef macro to switch off migration_version attribute
(Zhenyu Wang)

v2:
1. removed 32 common part of version string
(Alex Williamson)
2. do not register version attribute for GVT not supporting live
migration.(Cornelia Huck)
3. for platforms out of gen8, gen9, return -EINVAL --> -ENODEV for
incompatible. (Cornelia Huck)
---
 drivers/gpu/drm/i915/gvt/Makefile            |   2 +-
 drivers/gpu/drm/i915/gvt/gvt.c               |  39 +++++
 drivers/gpu/drm/i915/gvt/gvt.h               |   5 +
 drivers/gpu/drm/i915/gvt/migration_version.c | 167 +++++++++++++++++++
 drivers/gpu/drm/i915/gvt/vgpu.c              |  13 +-
 5 files changed, 223 insertions(+), 3 deletions(-)
 create mode 100644 drivers/gpu/drm/i915/gvt/migration_version.c

diff --git a/drivers/gpu/drm/i915/gvt/Makefile b/drivers/gpu/drm/i915/gvt/Makefile
index 271fb46d4dd0..a9d561c93ab8 100644
--- a/drivers/gpu/drm/i915/gvt/Makefile
+++ b/drivers/gpu/drm/i915/gvt/Makefile
@@ -3,7 +3,7 @@ GVT_DIR := gvt
 GVT_SOURCE := gvt.o aperture_gm.o handlers.o vgpu.o trace_points.o firmware.o \
 	interrupt.o gtt.o cfg_space.o opregion.o mmio.o display.o edid.o \
 	execlist.o scheduler.o sched_policy.o mmio_context.o cmd_parser.o debugfs.o \
-	fb_decoder.o dmabuf.o page_track.o
+	fb_decoder.o dmabuf.o page_track.o migration_version.o
 
 ccflags-y				+= -I$(src) -I$(src)/$(GVT_DIR)
 i915-y					+= $(addprefix $(GVT_DIR)/, $(GVT_SOURCE))
diff --git a/drivers/gpu/drm/i915/gvt/gvt.c b/drivers/gpu/drm/i915/gvt/gvt.c
index 43f4242062dd..be2980e8ac75 100644
--- a/drivers/gpu/drm/i915/gvt/gvt.c
+++ b/drivers/gpu/drm/i915/gvt/gvt.c
@@ -105,14 +105,53 @@ static ssize_t description_show(struct kobject *kobj, struct device *dev,
 		       type->weight);
 }
 
+static ssize_t migration_version_show(struct kobject *kobj, struct device *dev,
+		char *buf)
+{
+	struct intel_vgpu_type *type;
+	void *gvt = kdev_to_i915(dev)->gvt;
+
+	type = intel_gvt_find_vgpu_type(gvt, kobject_name(kobj));
+	if (!type || !type->migration_version) {
+		gvt_err("Does not support migraion on type %s. Please search previous detailed log\n",
+				kobject_name(kobj));
+		return -ENODEV;
+	}
+
+	return snprintf(buf, strlen(type->migration_version) + 2,
+			"%s\n", type->migration_version);
+}
+
+static ssize_t migration_version_store(struct kobject *kobj, struct device *dev,
+		const char *buf, size_t count)
+{
+	int ret = 0;
+	struct intel_vgpu_type *type;
+	void *gvt = kdev_to_i915(dev)->gvt;
+
+	type = intel_gvt_find_vgpu_type(gvt, kobject_name(kobj));
+	if (!type || !type->migration_version) {
+		gvt_err("Does not support migraion on type %s. Please search previous detailed log\n",
+				kobject_name(kobj));
+		return -ENODEV;
+	}
+
+	ret = intel_gvt_check_vfio_migration_version(gvt,
+			type->migration_version, buf);
+
+	return (ret < 0 ? ret : count);
+}
+
 static MDEV_TYPE_ATTR_RO(available_instances);
 static MDEV_TYPE_ATTR_RO(device_api);
 static MDEV_TYPE_ATTR_RO(description);
+static MDEV_TYPE_ATTR_RW(migration_version);
 
 static struct attribute *gvt_type_attrs[] = {
 	&mdev_type_attr_available_instances.attr,
 	&mdev_type_attr_device_api.attr,
 	&mdev_type_attr_description.attr,
+	&mdev_type_attr_migration_version.attr,
 	NULL,
 };
 
diff --git a/drivers/gpu/drm/i915/gvt/gvt.h b/drivers/gpu/drm/i915/gvt/gvt.h
index f5a328b5290a..265319d35c25 100644
--- a/drivers/gpu/drm/i915/gvt/gvt.h
+++ b/drivers/gpu/drm/i915/gvt/gvt.h
@@ -296,6 +296,7 @@ struct intel_vgpu_type {
 	unsigned int fence;
 	unsigned int weight;
 	enum intel_vgpu_edid resolution;
+	char *migration_version;
 };
 
 struct intel_gvt {
@@ -687,6 +688,10 @@ void intel_gvt_debugfs_remove_vgpu(struct intel_vgpu *vgpu);
 int intel_gvt_debugfs_init(struct intel_gvt *gvt);
 void intel_gvt_debugfs_clean(struct intel_gvt *gvt);
 
+ssize_t intel_gvt_check_vfio_migration_version(struct intel_gvt *gvt,
+		const char *self, const char *remote);
+char *intel_gvt_get_vfio_migration_version(struct intel_gvt *gvt,
+		const char *vgpu_type);
 
 #include "trace.h"
 #include "mpt.h"
diff --git a/drivers/gpu/drm/i915/gvt/migration_version.c b/drivers/gpu/drm/i915/gvt/migration_version.c
new file mode 100644
index 000000000000..e6acf188b29a
--- /dev/null
+++ b/drivers/gpu/drm/i915/gvt/migration_version.c
@@ -0,0 +1,167 @@
+/*
+ * Copyright(c) 2011-2017 Intel Corporation. All rights reserved.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
+ * SOFTWARE.
+ *
+ * Authors:
+ *    Yan Zhao <yan.y.zhao@intel.com>
+ */
+#include <linux/vfio.h>
+#include "i915_drv.h"
+#include "gvt.h"
+
+#define INV_SOFTWARE_VERSION (-1U)
+#define VENDOR_ID_LEN (4)
+#define DEVICE_ID_LEN (4)
+#define VGPU_TYPE_LEN (16)
+#define SOFTWARE_VER_LEN (8)
+
+/* total length of vfio migration version string.
+ * never exceed limit of PATH_MAX (4096)
+ */
+#define MIGRATION_VERSION_TOTAL_LEN (VENDOR_ID_LEN + DEVICE_ID_LEN + \
+					VGPU_TYPE_LEN + SOFTWARE_VER_LEN + 4)
+
+#define GVT_VFIO_MIGRATION_SOFTWARE_VERSION INV_SOFTWARE_VERSION
+
+
+#define PRINTF_FORMAT "%04x-%04x-%s-%08x"
+#define SCANF_FORMAT "%x-%x-%16[^-]-%x"
+
+enum incompatible_reason {
+	IREASON_WRONG_REMOTE_FORMAT = 0,
+	IREASON_HARDWARE_MISMATCH,
+	IREASON_SOFTWARE_VERSION_MISMATCH,
+	IREASON_VGPU_TYPE_MISMATCH,
+};
+
+static const char *const incompatible_reason_str[] = {
+	[IREASON_WRONG_REMOTE_FORMAT] =
+		"wrong string format. probably wrong GVT version",
+	[IREASON_HARDWARE_MISMATCH] =
+		"physical device not matched",
+	[IREASON_SOFTWARE_VERSION_MISMATCH] =
+		"migration software version not matched",
+	[IREASON_VGPU_TYPE_MISMATCH] =
+		"vgpu type not matched"
+};
+
+static bool is_compatible(const char *local, const char *remote)
+{
+	bool ret;
+
+	ret = sysfs_streq(local, remote);
+
+	if (!ret) {
+		int vid_l = 0, did_l = 0, vid_r = 0, did_r = 0;
+		char type_l[VGPU_TYPE_LEN], type_r[VGPU_TYPE_LEN];
+		u32 sv_l = 0, sv_r = 0;
+		int rl = 0, rr = 0;
+		enum incompatible_reason reason = IREASON_WRONG_REMOTE_FORMAT;
+
+		memset(type_l, 0, sizeof(type_l));
+		memset(type_r, 0, sizeof(type_r));
+
+		rl = sscanf(local, SCANF_FORMAT,
+				&vid_l, &did_l, type_l, &sv_l);
+		rr = sscanf(remote, SCANF_FORMAT,
+				&vid_r, &did_r, type_r, &sv_r);
+
+		if (rl == rr) {
+			if (vid_l != vid_r || did_l != did_r)
+				reason = IREASON_HARDWARE_MISMATCH;
+			else if (sv_l != sv_r)
+				reason = IREASON_SOFTWARE_VERSION_MISMATCH;
+			else if (strncmp(type_l, type_r, VGPU_TYPE_LEN))
+				reason = IREASON_VGPU_TYPE_MISMATCH;
+		}
+
+		gvt_err("Migration version mismatched. Possible reason: %s. Local migration version:%s, Remote migration version:%s\n",
+				incompatible_reason_str[reason], local,	remote);
+
+	}
+	return ret;
+
+}
+
+
+char *
+intel_gvt_get_vfio_migration_version(struct intel_gvt *gvt,
+		const char *vgpu_type)
+{
+	int cnt = 0;
+	struct drm_i915_private *dev_priv = gvt->dev_priv;
+	char *version = NULL;
+
+	/* currently only gen8 & gen9 are supported */
+	if (!IS_GEN(dev_priv, 8) && !IS_GEN(dev_priv, 9)) {
+		gvt_err("Local hardware does not support migration on %d\n",
+				INTEL_INFO(dev_priv)->gen);
+		return NULL;
+	}
+
+	if (GVT_VFIO_MIGRATION_SOFTWARE_VERSION == INV_SOFTWARE_VERSION) {
+		gvt_err("Local GVT does not support migration\n");
+		return NULL;
+	}
+
+	version = kzalloc(MIGRATION_VERSION_TOTAL_LEN, GFP_KERNEL);
+
+	if (unlikely(!version)) {
+		gvt_err("memory allocation failed when get local migraiton version\n");
+		return NULL;
+	}
+
+	/* vendor id + device id + vgpu type + software version */
+	cnt = snprintf(version, MIGRATION_VERSION_TOTAL_LEN, PRINTF_FORMAT,
+			PCI_VENDOR_ID_INTEL,
+			INTEL_DEVID(dev_priv),
+			vgpu_type,
+			GVT_VFIO_MIGRATION_SOFTWARE_VERSION);
+
+	if (cnt)
+		return version;
+
+	gvt_err("string generation failed when get local migration version\n");
+	return NULL;
+}
+
+ssize_t intel_gvt_check_vfio_migration_version(struct intel_gvt *gvt,
+		const char *self, const char *remote)
+{
+	struct drm_i915_private *dev_priv = gvt->dev_priv;
+
+	/* currently only gen8 & gen9 are supported */
+	if (!IS_GEN(dev_priv, 8) && !IS_GEN(dev_priv, 9)) {
+		gvt_err("Local hardware does not support migration on %d\n",
+				INTEL_INFO(dev_priv)->gen);
+		return -ENODEV;
+	}
+
+	if (GVT_VFIO_MIGRATION_SOFTWARE_VERSION == INV_SOFTWARE_VERSION) {
+		gvt_err("Local GVT does not support migration\n");
+		return -ENODEV;
+	}
+
+	if (!is_compatible(self, remote))
+		return -EINVAL;
+
+	return 0;
+}
diff --git a/drivers/gpu/drm/i915/gvt/vgpu.c b/drivers/gpu/drm/i915/gvt/vgpu.c
index 44ce3c2b9ac1..7642b21641bd 100644
--- a/drivers/gpu/drm/i915/gvt/vgpu.c
+++ b/drivers/gpu/drm/i915/gvt/vgpu.c
@@ -155,13 +155,18 @@ int intel_gvt_init_vgpu_types(struct intel_gvt *gvt)
 			sprintf(gvt->types[i].name, "GVTg_V5_%s",
 						vgpu_types[i].name);
 
-		gvt_dbg_core("type[%d]: %s avail %u low %u high %u fence %u weight %u res %s\n",
+		gvt->types[i].migration_version =
+			intel_gvt_get_vfio_migration_version(gvt,
+					gvt->types[i].name);
+		gvt_dbg_core("type[%d]: %s avail %u low %u high %u fence %u weight %u res %s, migratio_version:%s\n",
 			     i, gvt->types[i].name,
 			     gvt->types[i].avail_instance,
 			     gvt->types[i].low_gm_size,
 			     gvt->types[i].high_gm_size, gvt->types[i].fence,
 			     gvt->types[i].weight,
-			     vgpu_edid_str(gvt->types[i].resolution));
+			     vgpu_edid_str(gvt->types[i].resolution),
+			     (gvt->types[i].migration_version ?
+			     gvt->types[i].migration_version : "null"));
 	}
 
 	gvt->num_types = i;
@@ -170,6 +175,10 @@ int intel_gvt_init_vgpu_types(struct intel_gvt *gvt)
 
 void intel_gvt_clean_vgpu_types(struct intel_gvt *gvt)
 {
+	int i;
+
+	for (i = 0; i < gvt->num_types; i++)
+		kfree(gvt->types[i].migration_version);
 	kfree(gvt->types);
 }
 
-- 
2.17.1

[Qemu-devel] [PATCH v3 2/2] drm/i915/gvt: export migration_version to mdev sysfs for Intel vGPU

[Yan Zhao]

This feature implements the migration_version attribute for Intel's vGPU
mdev devices.

migration_version attribute is rw.
It's used to check migration compatibility for two mdev devices of the
same mdev type.
migration_version string is defined by vendor driver and opaque to
userspace.

For Intel vGPU of gen8 and gen9, the format of migration_version string
is:
  <vendor id>-<device id>-<vgpu type>-<software version>.

For future platforms, the format of migration_version string is to be
expanded to include more meta data to identify Intel vGPUs for live
migration compatibility check

For old platforms, and for GVT not supporting vGPU live migration
feature, -ENODEV is returned on read(2)/write(2) of migration_version
attribute.
For vGPUs running old GVT who do not expose migration_version
attribute, live migration is regarded as not supported for those vGPUs.

Cc: Alex Williamson <alex.williamson@redhat.com>
Cc: Erik Skultety <eskultet@redhat.com>
Cc: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
Cc: Cornelia Huck <cohuck@redhat.com>
Cc: "Tian, Kevin" <kevin.tian@intel.com>
Cc: Zhenyu Wang <zhenyuw@linux.intel.com>
Cc: "Wang, Zhi A" <zhi.a.wang@intel.com>
c: Neo Jia <cjia@nvidia.com>
Cc: Kirti Wankhede <kwankhede@nvidia.com>

Signed-off-by: Yan Zhao <yan.y.zhao@intel.com>

---
v3:
1. renamed version to migration_version
(Christophe de Dinechin, Cornelia Huck, Alex Williamson)
2. instead of generating migration version strings each time, storing
them in vgpu types generated during initialization.
(Zhenyu Wang, Cornelia Huck)
3. replaced multiple snprintf to one big snprintf in
intel_gvt_get_vfio_migration_version()
(Dr. David Alan Gilbert)
4. printed detailed error log
(Alex Williamson, Erik Skultety, Cornelia Huck, Dr. David Alan Gilbert)
5. incorporated <software version> into migration_version string
(Alex Williamson)
6. do not use ifndef macro to switch off migration_version attribute
(Zhenyu Wang)

v2:
1. removed 32 common part of version string
(Alex Williamson)
2. do not register version attribute for GVT not supporting live
migration.(Cornelia Huck)
3. for platforms out of gen8, gen9, return -EINVAL --> -ENODEV for
incompatible. (Cornelia Huck)
---
 drivers/gpu/drm/i915/gvt/Makefile            |   2 +-
 drivers/gpu/drm/i915/gvt/gvt.c               |  39 +++++
 drivers/gpu/drm/i915/gvt/gvt.h               |   5 +
 drivers/gpu/drm/i915/gvt/migration_version.c | 167 +++++++++++++++++++
 drivers/gpu/drm/i915/gvt/vgpu.c              |  13 +-
 5 files changed, 223 insertions(+), 3 deletions(-)
 create mode 100644 drivers/gpu/drm/i915/gvt/migration_version.c

diff --git a/drivers/gpu/drm/i915/gvt/Makefile b/drivers/gpu/drm/i915/gvt/Makefile
index 271fb46d4dd0..a9d561c93ab8 100644
--- a/drivers/gpu/drm/i915/gvt/Makefile
+++ b/drivers/gpu/drm/i915/gvt/Makefile
@@ -3,7 +3,7 @@ GVT_DIR := gvt
 GVT_SOURCE := gvt.o aperture_gm.o handlers.o vgpu.o trace_points.o firmware.o \
 	interrupt.o gtt.o cfg_space.o opregion.o mmio.o display.o edid.o \
 	execlist.o scheduler.o sched_policy.o mmio_context.o cmd_parser.o debugfs.o \
-	fb_decoder.o dmabuf.o page_track.o
+	fb_decoder.o dmabuf.o page_track.o migration_version.o
 
 ccflags-y				+= -I$(src) -I$(src)/$(GVT_DIR)
 i915-y					+= $(addprefix $(GVT_DIR)/, $(GVT_SOURCE))
diff --git a/drivers/gpu/drm/i915/gvt/gvt.c b/drivers/gpu/drm/i915/gvt/gvt.c
index 43f4242062dd..be2980e8ac75 100644
--- a/drivers/gpu/drm/i915/gvt/gvt.c
+++ b/drivers/gpu/drm/i915/gvt/gvt.c
@@ -105,14 +105,53 @@ static ssize_t description_show(struct kobject *kobj, struct device *dev,
 		       type->weight);
 }
 
+static ssize_t migration_version_show(struct kobject *kobj, struct device *dev,
+		char *buf)
+{
+	struct intel_vgpu_type *type;
+	void *gvt = kdev_to_i915(dev)->gvt;
+
+	type = intel_gvt_find_vgpu_type(gvt, kobject_name(kobj));
+	if (!type || !type->migration_version) {
+		gvt_err("Does not support migraion on type %s. Please search previous detailed log\n",
+				kobject_name(kobj));
+		return -ENODEV;
+	}
+
+	return snprintf(buf, strlen(type->migration_version) + 2,
+			"%s\n", type->migration_version);
+}
+
+static ssize_t migration_version_store(struct kobject *kobj, struct device *dev,
+		const char *buf, size_t count)
+{
+	int ret = 0;
+	struct intel_vgpu_type *type;
+	void *gvt = kdev_to_i915(dev)->gvt;
+
+	type = intel_gvt_find_vgpu_type(gvt, kobject_name(kobj));
+	if (!type || !type->migration_version) {
+		gvt_err("Does not support migraion on type %s. Please search previous detailed log\n",
+				kobject_name(kobj));
+		return -ENODEV;
+	}
+
+	ret = intel_gvt_check_vfio_migration_version(gvt,
+			type->migration_version, buf);
+
+	return (ret < 0 ? ret : count);
+}
+
 static MDEV_TYPE_ATTR_RO(available_instances);
 static MDEV_TYPE_ATTR_RO(device_api);
 static MDEV_TYPE_ATTR_RO(description);
+static MDEV_TYPE_ATTR_RW(migration_version);
 
 static struct attribute *gvt_type_attrs[] = {
 	&mdev_type_attr_available_instances.attr,
 	&mdev_type_attr_device_api.attr,
 	&mdev_type_attr_description.attr,
+	&mdev_type_attr_migration_version.attr,
 	NULL,
 };
 
diff --git a/drivers/gpu/drm/i915/gvt/gvt.h b/drivers/gpu/drm/i915/gvt/gvt.h
index f5a328b5290a..265319d35c25 100644
--- a/drivers/gpu/drm/i915/gvt/gvt.h
+++ b/drivers/gpu/drm/i915/gvt/gvt.h
@@ -296,6 +296,7 @@ struct intel_vgpu_type {
 	unsigned int fence;
 	unsigned int weight;
 	enum intel_vgpu_edid resolution;
+	char *migration_version;
 };
 
 struct intel_gvt {
@@ -687,6 +688,10 @@ void intel_gvt_debugfs_remove_vgpu(struct intel_vgpu *vgpu);
 int intel_gvt_debugfs_init(struct intel_gvt *gvt);
 void intel_gvt_debugfs_clean(struct intel_gvt *gvt);
 
+ssize_t intel_gvt_check_vfio_migration_version(struct intel_gvt *gvt,
+		const char *self, const char *remote);
+char *intel_gvt_get_vfio_migration_version(struct intel_gvt *gvt,
+		const char *vgpu_type);
 
 #include "trace.h"
 #include "mpt.h"
diff --git a/drivers/gpu/drm/i915/gvt/migration_version.c b/drivers/gpu/drm/i915/gvt/migration_version.c
new file mode 100644
index 000000000000..e6acf188b29a
--- /dev/null
+++ b/drivers/gpu/drm/i915/gvt/migration_version.c
@@ -0,0 +1,167 @@
+/*
+ * Copyright(c) 2011-2017 Intel Corporation. All rights reserved.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
+ * SOFTWARE.
+ *
+ * Authors:
+ *    Yan Zhao <yan.y.zhao@intel.com>
+ */
+#include <linux/vfio.h>
+#include "i915_drv.h"
+#include "gvt.h"
+
+#define INV_SOFTWARE_VERSION (-1U)
+#define VENDOR_ID_LEN (4)
+#define DEVICE_ID_LEN (4)
+#define VGPU_TYPE_LEN (16)
+#define SOFTWARE_VER_LEN (8)
+
+/* total length of vfio migration version string.
+ * never exceed limit of PATH_MAX (4096)
+ */
+#define MIGRATION_VERSION_TOTAL_LEN (VENDOR_ID_LEN + DEVICE_ID_LEN + \
+					VGPU_TYPE_LEN + SOFTWARE_VER_LEN + 4)
+
+#define GVT_VFIO_MIGRATION_SOFTWARE_VERSION INV_SOFTWARE_VERSION
+
+
+#define PRINTF_FORMAT "%04x-%04x-%s-%08x"
+#define SCANF_FORMAT "%x-%x-%16[^-]-%x"
+
+enum incompatible_reason {
+	IREASON_WRONG_REMOTE_FORMAT = 0,
+	IREASON_HARDWARE_MISMATCH,
+	IREASON_SOFTWARE_VERSION_MISMATCH,
+	IREASON_VGPU_TYPE_MISMATCH,
+};
+
+static const char *const incompatible_reason_str[] = {
+	[IREASON_WRONG_REMOTE_FORMAT] =
+		"wrong string format. probably wrong GVT version",
+	[IREASON_HARDWARE_MISMATCH] =
+		"physical device not matched",
+	[IREASON_SOFTWARE_VERSION_MISMATCH] =
+		"migration software version not matched",
+	[IREASON_VGPU_TYPE_MISMATCH] =
+		"vgpu type not matched"
+};
+
+static bool is_compatible(const char *local, const char *remote)
+{
+	bool ret;
+
+	ret = sysfs_streq(local, remote);
+
+	if (!ret) {
+		int vid_l = 0, did_l = 0, vid_r = 0, did_r = 0;
+		char type_l[VGPU_TYPE_LEN], type_r[VGPU_TYPE_LEN];
+		u32 sv_l = 0, sv_r = 0;
+		int rl = 0, rr = 0;
+		enum incompatible_reason reason = IREASON_WRONG_REMOTE_FORMAT;
+
+		memset(type_l, 0, sizeof(type_l));
+		memset(type_r, 0, sizeof(type_r));
+
+		rl = sscanf(local, SCANF_FORMAT,
+				&vid_l, &did_l, type_l, &sv_l);
+		rr = sscanf(remote, SCANF_FORMAT,
+				&vid_r, &did_r, type_r, &sv_r);
+
+		if (rl == rr) {
+			if (vid_l != vid_r || did_l != did_r)
+				reason = IREASON_HARDWARE_MISMATCH;
+			else if (sv_l != sv_r)
+				reason = IREASON_SOFTWARE_VERSION_MISMATCH;
+			else if (strncmp(type_l, type_r, VGPU_TYPE_LEN))
+				reason = IREASON_VGPU_TYPE_MISMATCH;
+		}
+
+		gvt_err("Migration version mismatched. Possible reason: %s. Local migration version:%s, Remote migration version:%s\n",
+				incompatible_reason_str[reason], local,	remote);
+
+	}
+	return ret;
+
+}
+
+
+char *
+intel_gvt_get_vfio_migration_version(struct intel_gvt *gvt,
+		const char *vgpu_type)
+{
+	int cnt = 0;
+	struct drm_i915_private *dev_priv = gvt->dev_priv;
+	char *version = NULL;
+
+	/* currently only gen8 & gen9 are supported */
+	if (!IS_GEN(dev_priv, 8) && !IS_GEN(dev_priv, 9)) {
+		gvt_err("Local hardware does not support migration on %d\n",
+				INTEL_INFO(dev_priv)->gen);
+		return NULL;
+	}
+
+	if (GVT_VFIO_MIGRATION_SOFTWARE_VERSION == INV_SOFTWARE_VERSION) {
+		gvt_err("Local GVT does not support migration\n");
+		return NULL;
+	}
+
+	version = kzalloc(MIGRATION_VERSION_TOTAL_LEN, GFP_KERNEL);
+
+	if (unlikely(!version)) {
+		gvt_err("memory allocation failed when get local migraiton version\n");
+		return NULL;
+	}
+
+	/* vendor id + device id + vgpu type + software version */
+	cnt = snprintf(version, MIGRATION_VERSION_TOTAL_LEN, PRINTF_FORMAT,
+			PCI_VENDOR_ID_INTEL,
+			INTEL_DEVID(dev_priv),
+			vgpu_type,
+			GVT_VFIO_MIGRATION_SOFTWARE_VERSION);
+
+	if (cnt)
+		return version;
+
+	gvt_err("string generation failed when get local migration version\n");
+	return NULL;
+}
+
+ssize_t intel_gvt_check_vfio_migration_version(struct intel_gvt *gvt,
+		const char *self, const char *remote)
+{
+	struct drm_i915_private *dev_priv = gvt->dev_priv;
+
+	/* currently only gen8 & gen9 are supported */
+	if (!IS_GEN(dev_priv, 8) && !IS_GEN(dev_priv, 9)) {
+		gvt_err("Local hardware does not support migration on %d\n",
+				INTEL_INFO(dev_priv)->gen);
+		return -ENODEV;
+	}
+
+	if (GVT_VFIO_MIGRATION_SOFTWARE_VERSION == INV_SOFTWARE_VERSION) {
+		gvt_err("Local GVT does not support migration\n");
+		return -ENODEV;
+	}
+
+	if (!is_compatible(self, remote))
+		return -EINVAL;
+
+	return 0;
+}
diff --git a/drivers/gpu/drm/i915/gvt/vgpu.c b/drivers/gpu/drm/i915/gvt/vgpu.c
index 44ce3c2b9ac1..7642b21641bd 100644
--- a/drivers/gpu/drm/i915/gvt/vgpu.c
+++ b/drivers/gpu/drm/i915/gvt/vgpu.c
@@ -155,13 +155,18 @@ int intel_gvt_init_vgpu_types(struct intel_gvt *gvt)
 			sprintf(gvt->types[i].name, "GVTg_V5_%s",
 						vgpu_types[i].name);
 
-		gvt_dbg_core("type[%d]: %s avail %u low %u high %u fence %u weight %u res %s\n",
+		gvt->types[i].migration_version =
+			intel_gvt_get_vfio_migration_version(gvt,
+					gvt->types[i].name);
+		gvt_dbg_core("type[%d]: %s avail %u low %u high %u fence %u weight %u res %s, migratio_version:%s\n",
 			     i, gvt->types[i].name,
 			     gvt->types[i].avail_instance,
 			     gvt->types[i].low_gm_size,
 			     gvt->types[i].high_gm_size, gvt->types[i].fence,
 			     gvt->types[i].weight,
-			     vgpu_edid_str(gvt->types[i].resolution));
+			     vgpu_edid_str(gvt->types[i].resolution),
+			     (gvt->types[i].migration_version ?
+			     gvt->types[i].migration_version : "null"));
 	}
 
 	gvt->num_types = i;
@@ -170,6 +175,10 @@ int intel_gvt_init_vgpu_types(struct intel_gvt *gvt)
 
 void intel_gvt_clean_vgpu_types(struct intel_gvt *gvt)
 {
+	int i;
+
+	for (i = 0; i < gvt->num_types; i++)
+		kfree(gvt->types[i].migration_version);
 	kfree(gvt->types);
 }
 
-- 
2.17.1

Re: [PATCH v3 1/2] vfio/mdev: add migration_version attribute for mdev device

[Cornelia Huck]

On Sun, 26 May 2019 23:43:42 -0400
Yan Zhao <yan.y.zhao@intel.com> wrote:

> migration_version attribute is used to check migration compatibility
> between two mdev device of the same mdev type.

s/device/devices/

> The key is that it's rw and its data is opaque to userspace.
> 
> Userspace reads migration_version of mdev device at source side and
> writes the value to migration_version attribute of mdev device at target
> side. It judges migration compatibility according to whether the read
> and write operations succeed or fail.
> 
> As this attribute is under mdev_type node, userspace is able to know
> whether two mdev devices are compatible before a mdev device is created.
> 
> userspace needs to check whether the two mdev devices are of the same
> mdev type before checking the migration_version attribute. It also needs
> to check device creation parameters if aggregation is supported in
> future.
> 
>              __    userspace
>               /\              \
>              /                 \write
>             / read              \
>    ________/__________       ___\|/_____________
>   | migration_version |     | migration_version |-->check migration
>   ---------------------     ---------------------   compatibility
>     mdev device A               mdev device B
> 
> Cc: Alex Williamson <alex.williamson@redhat.com>
> Cc: Erik Skultety <eskultet@redhat.com>
> Cc: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> Cc: Cornelia Huck <cohuck@redhat.com>
> Cc: "Tian, Kevin" <kevin.tian@intel.com>
> Cc: Zhenyu Wang <zhenyuw@linux.intel.com>
> Cc: "Wang, Zhi A" <zhi.a.wang@intel.com>
> Cc: Neo Jia <cjia@nvidia.com>
> Cc: Kirti Wankhede <kwankhede@nvidia.com>
> Cc: Daniel P. Berrangé <berrange@redhat.com>
> Cc: Christophe de Dinechin <dinechin@redhat.com>
> 
> Signed-off-by: Yan Zhao <yan.y.zhao@intel.com>
> 
> ---
> v3:
> 1. renamed version to migration_version
> (Christophe de Dinechin, Cornelia Huck, Alex Williamson)
> 2. let errno to be freely defined by vendor driver
> (Alex Williamson, Erik Skultety, Cornelia Huck, Dr. David Alan Gilbert)
> 3. let checking mdev_type be prerequisite of migration compatibility
> check. (Alex Williamson)
> 4. reworded example usage section.
> (most of this section came from Alex Williamson)
> 5. reworded attribute intention section (Cornelia Huck)
> 
> v2:
> 1. added detailed intent and usage
> 2. made definition of version string completely private to vendor driver
>    (Alex Williamson)
> 3. abandoned changes to sample mdev drivers (Alex Williamson)
> 4. mandatory --> optional (Cornelia Huck)
> 5. added description for errno (Cornelia Huck)
> ---
>  Documentation/vfio-mediated-device.txt | 113 +++++++++++++++++++++++++
>  1 file changed, 113 insertions(+)
> 

While I probably would have written a more compact description, your
version is fine with me as well.

Reviewed-by: Cornelia Huck <cohuck@redhat.com>

Re: [Qemu-devel] [PATCH v3 1/2] vfio/mdev: add migration_version attribute for mdev device

[Cornelia Huck]

On Sun, 26 May 2019 23:43:42 -0400
Yan Zhao <yan.y.zhao@intel.com> wrote:

> migration_version attribute is used to check migration compatibility
> between two mdev device of the same mdev type.

s/device/devices/

> The key is that it's rw and its data is opaque to userspace.
> 
> Userspace reads migration_version of mdev device at source side and
> writes the value to migration_version attribute of mdev device at target
> side. It judges migration compatibility according to whether the read
> and write operations succeed or fail.
> 
> As this attribute is under mdev_type node, userspace is able to know
> whether two mdev devices are compatible before a mdev device is created.
> 
> userspace needs to check whether the two mdev devices are of the same
> mdev type before checking the migration_version attribute. It also needs
> to check device creation parameters if aggregation is supported in
> future.
> 
>              __    userspace
>               /\              \
>              /                 \write
>             / read              \
>    ________/__________       ___\|/_____________
>   | migration_version |     | migration_version |-->check migration
>   ---------------------     ---------------------   compatibility
>     mdev device A               mdev device B
> 
> Cc: Alex Williamson <alex.williamson@redhat.com>
> Cc: Erik Skultety <eskultet@redhat.com>
> Cc: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> Cc: Cornelia Huck <cohuck@redhat.com>
> Cc: "Tian, Kevin" <kevin.tian@intel.com>
> Cc: Zhenyu Wang <zhenyuw@linux.intel.com>
> Cc: "Wang, Zhi A" <zhi.a.wang@intel.com>
> Cc: Neo Jia <cjia@nvidia.com>
> Cc: Kirti Wankhede <kwankhede@nvidia.com>
> Cc: Daniel P. Berrangé <berrange@redhat.com>
> Cc: Christophe de Dinechin <dinechin@redhat.com>
> 
> Signed-off-by: Yan Zhao <yan.y.zhao@intel.com>
> 
> ---
> v3:
> 1. renamed version to migration_version
> (Christophe de Dinechin, Cornelia Huck, Alex Williamson)
> 2. let errno to be freely defined by vendor driver
> (Alex Williamson, Erik Skultety, Cornelia Huck, Dr. David Alan Gilbert)
> 3. let checking mdev_type be prerequisite of migration compatibility
> check. (Alex Williamson)
> 4. reworded example usage section.
> (most of this section came from Alex Williamson)
> 5. reworded attribute intention section (Cornelia Huck)
> 
> v2:
> 1. added detailed intent and usage
> 2. made definition of version string completely private to vendor driver
>    (Alex Williamson)
> 3. abandoned changes to sample mdev drivers (Alex Williamson)
> 4. mandatory --> optional (Cornelia Huck)
> 5. added description for errno (Cornelia Huck)
> ---
>  Documentation/vfio-mediated-device.txt | 113 +++++++++++++++++++++++++
>  1 file changed, 113 insertions(+)
> 

While I probably would have written a more compact description, your
version is fine with me as well.

Reviewed-by: Cornelia Huck <cohuck@redhat.com>

Re: [PATCH v3 2/2] drm/i915/gvt: export migration_version to mdev sysfs for Intel vGPU

[Cornelia Huck]

On Sun, 26 May 2019 23:44:37 -0400
Yan Zhao <yan.y.zhao@intel.com> wrote:

> This feature implements the migration_version attribute for Intel's vGPU
> mdev devices.
> 
> migration_version attribute is rw.
> It's used to check migration compatibility for two mdev devices of the
> same mdev type.
> migration_version string is defined by vendor driver and opaque to
> userspace.
> 
> For Intel vGPU of gen8 and gen9, the format of migration_version string
> is:
>   <vendor id>-<device id>-<vgpu type>-<software version>.
> 
> For future platforms, the format of migration_version string is to be
> expanded to include more meta data to identify Intel vGPUs for live
> migration compatibility check
> 
> For old platforms, and for GVT not supporting vGPU live migration
> feature, -ENODEV is returned on read(2)/write(2) of migration_version
> attribute.
> For vGPUs running old GVT who do not expose migration_version
> attribute, live migration is regarded as not supported for those vGPUs.
> 
> Cc: Alex Williamson <alex.williamson@redhat.com>
> Cc: Erik Skultety <eskultet@redhat.com>
> Cc: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> Cc: Cornelia Huck <cohuck@redhat.com>
> Cc: "Tian, Kevin" <kevin.tian@intel.com>
> Cc: Zhenyu Wang <zhenyuw@linux.intel.com>
> Cc: "Wang, Zhi A" <zhi.a.wang@intel.com>
> c: Neo Jia <cjia@nvidia.com>
> Cc: Kirti Wankhede <kwankhede@nvidia.com>
> 
> Signed-off-by: Yan Zhao <yan.y.zhao@intel.com>
> 
> ---
> v3:
> 1. renamed version to migration_version
> (Christophe de Dinechin, Cornelia Huck, Alex Williamson)
> 2. instead of generating migration version strings each time, storing
> them in vgpu types generated during initialization.
> (Zhenyu Wang, Cornelia Huck)
> 3. replaced multiple snprintf to one big snprintf in
> intel_gvt_get_vfio_migration_version()
> (Dr. David Alan Gilbert)
> 4. printed detailed error log
> (Alex Williamson, Erik Skultety, Cornelia Huck, Dr. David Alan Gilbert)
> 5. incorporated <software version> into migration_version string
> (Alex Williamson)
> 6. do not use ifndef macro to switch off migration_version attribute
> (Zhenyu Wang)
> 
> v2:
> 1. removed 32 common part of version string
> (Alex Williamson)
> 2. do not register version attribute for GVT not supporting live
> migration.(Cornelia Huck)
> 3. for platforms out of gen8, gen9, return -EINVAL --> -ENODEV for
> incompatible. (Cornelia Huck)
> ---
>  drivers/gpu/drm/i915/gvt/Makefile            |   2 +-
>  drivers/gpu/drm/i915/gvt/gvt.c               |  39 +++++
>  drivers/gpu/drm/i915/gvt/gvt.h               |   5 +
>  drivers/gpu/drm/i915/gvt/migration_version.c | 167 +++++++++++++++++++
>  drivers/gpu/drm/i915/gvt/vgpu.c              |  13 +-
>  5 files changed, 223 insertions(+), 3 deletions(-)
>  create mode 100644 drivers/gpu/drm/i915/gvt/migration_version.c
> 

(...)

> diff --git a/drivers/gpu/drm/i915/gvt/gvt.c b/drivers/gpu/drm/i915/gvt/gvt.c
> index 43f4242062dd..be2980e8ac75 100644
> --- a/drivers/gpu/drm/i915/gvt/gvt.c
> +++ b/drivers/gpu/drm/i915/gvt/gvt.c
> @@ -105,14 +105,53 @@ static ssize_t description_show(struct kobject *kobj, struct device *dev,
>  		       type->weight);
>  }
>  
> +static ssize_t migration_version_show(struct kobject *kobj, struct device *dev,
> +		char *buf)

Indentation looks a bit odd? (Also below.)

> +{
> +	struct intel_vgpu_type *type;
> +	void *gvt = kdev_to_i915(dev)->gvt;
> +
> +	type = intel_gvt_find_vgpu_type(gvt, kobject_name(kobj));
> +	if (!type || !type->migration_version) {
> +		gvt_err("Does not support migraion on type %s. Please search previous detailed log\n",

s/migraion/migration/ (also below)

Or reword to "Migration not supported on type %s."?

> +				kobject_name(kobj));
> +		return -ENODEV;
> +	}
> +
> +	return snprintf(buf, strlen(type->migration_version) + 2,
> +			"%s\n", type->migration_version);
> +}
> +
> +static ssize_t migration_version_store(struct kobject *kobj, struct device *dev,
> +		const char *buf, size_t count)
> +{
> +	int ret = 0;
> +	struct intel_vgpu_type *type;
> +	void *gvt = kdev_to_i915(dev)->gvt;
> +
> +	type = intel_gvt_find_vgpu_type(gvt, kobject_name(kobj));
> +	if (!type || !type->migration_version) {
> +		gvt_err("Does not support migraion on type %s. Please search previous detailed log\n",
> +				kobject_name(kobj));
> +		return -ENODEV;
> +	}
> +
> +	ret = intel_gvt_check_vfio_migration_version(gvt,
> +			type->migration_version, buf);
> +
> +	return (ret < 0 ? ret : count);
> +}
> +
>  static MDEV_TYPE_ATTR_RO(available_instances);
>  static MDEV_TYPE_ATTR_RO(device_api);
>  static MDEV_TYPE_ATTR_RO(description);
> +static MDEV_TYPE_ATTR_RW(migration_version);
>  
>  static struct attribute *gvt_type_attrs[] = {
>  	&mdev_type_attr_available_instances.attr,
>  	&mdev_type_attr_device_api.attr,
>  	&mdev_type_attr_description.attr,
> +	&mdev_type_attr_migration_version.attr,
>  	NULL,
>  };

(...)

> +char *
> +intel_gvt_get_vfio_migration_version(struct intel_gvt *gvt,
> +		const char *vgpu_type)
> +{
> +	int cnt = 0;
> +	struct drm_i915_private *dev_priv = gvt->dev_priv;
> +	char *version = NULL;
> +
> +	/* currently only gen8 & gen9 are supported */
> +	if (!IS_GEN(dev_priv, 8) && !IS_GEN(dev_priv, 9)) {
> +		gvt_err("Local hardware does not support migration on %d\n",
> +				INTEL_INFO(dev_priv)->gen);
> +		return NULL;
> +	}
> +
> +	if (GVT_VFIO_MIGRATION_SOFTWARE_VERSION == INV_SOFTWARE_VERSION) {
> +		gvt_err("Local GVT does not support migration\n");
> +		return NULL;
> +	}
> +
> +	version = kzalloc(MIGRATION_VERSION_TOTAL_LEN, GFP_KERNEL);
> +
> +	if (unlikely(!version)) {
> +		gvt_err("memory allocation failed when get local migraiton version\n");

s/migraiton/migration/

Or "cannot allocate memory for local migration version"?

> +		return NULL;
> +	}
> +
> +	/* vendor id + device id + vgpu type + software version */
> +	cnt = snprintf(version, MIGRATION_VERSION_TOTAL_LEN, PRINTF_FORMAT,
> +			PCI_VENDOR_ID_INTEL,
> +			INTEL_DEVID(dev_priv),
> +			vgpu_type,
> +			GVT_VFIO_MIGRATION_SOFTWARE_VERSION);
> +
> +	if (cnt)
> +		return version;
> +
> +	gvt_err("string generation failed when get local migration version\n");
> +	return NULL;
> +}

(...)

Only some nitpicks from me, but I'm not really familiar with this
driver. Overall, this looks sane to me, so have an

Acked-by: Cornelia Huck <cohuck@redhat.com>

Re: [Qemu-devel] [PATCH v3 2/2] drm/i915/gvt: export migration_version to mdev sysfs for Intel vGPU

[Cornelia Huck]

On Sun, 26 May 2019 23:44:37 -0400
Yan Zhao <yan.y.zhao@intel.com> wrote:

> This feature implements the migration_version attribute for Intel's vGPU
> mdev devices.
> 
> migration_version attribute is rw.
> It's used to check migration compatibility for two mdev devices of the
> same mdev type.
> migration_version string is defined by vendor driver and opaque to
> userspace.
> 
> For Intel vGPU of gen8 and gen9, the format of migration_version string
> is:
>   <vendor id>-<device id>-<vgpu type>-<software version>.
> 
> For future platforms, the format of migration_version string is to be
> expanded to include more meta data to identify Intel vGPUs for live
> migration compatibility check
> 
> For old platforms, and for GVT not supporting vGPU live migration
> feature, -ENODEV is returned on read(2)/write(2) of migration_version
> attribute.
> For vGPUs running old GVT who do not expose migration_version
> attribute, live migration is regarded as not supported for those vGPUs.
> 
> Cc: Alex Williamson <alex.williamson@redhat.com>
> Cc: Erik Skultety <eskultet@redhat.com>
> Cc: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> Cc: Cornelia Huck <cohuck@redhat.com>
> Cc: "Tian, Kevin" <kevin.tian@intel.com>
> Cc: Zhenyu Wang <zhenyuw@linux.intel.com>
> Cc: "Wang, Zhi A" <zhi.a.wang@intel.com>
> c: Neo Jia <cjia@nvidia.com>
> Cc: Kirti Wankhede <kwankhede@nvidia.com>
> 
> Signed-off-by: Yan Zhao <yan.y.zhao@intel.com>
> 
> ---
> v3:
> 1. renamed version to migration_version
> (Christophe de Dinechin, Cornelia Huck, Alex Williamson)
> 2. instead of generating migration version strings each time, storing
> them in vgpu types generated during initialization.
> (Zhenyu Wang, Cornelia Huck)
> 3. replaced multiple snprintf to one big snprintf in
> intel_gvt_get_vfio_migration_version()
> (Dr. David Alan Gilbert)
> 4. printed detailed error log
> (Alex Williamson, Erik Skultety, Cornelia Huck, Dr. David Alan Gilbert)
> 5. incorporated <software version> into migration_version string
> (Alex Williamson)
> 6. do not use ifndef macro to switch off migration_version attribute
> (Zhenyu Wang)
> 
> v2:
> 1. removed 32 common part of version string
> (Alex Williamson)
> 2. do not register version attribute for GVT not supporting live
> migration.(Cornelia Huck)
> 3. for platforms out of gen8, gen9, return -EINVAL --> -ENODEV for
> incompatible. (Cornelia Huck)
> ---
>  drivers/gpu/drm/i915/gvt/Makefile            |   2 +-
>  drivers/gpu/drm/i915/gvt/gvt.c               |  39 +++++
>  drivers/gpu/drm/i915/gvt/gvt.h               |   5 +
>  drivers/gpu/drm/i915/gvt/migration_version.c | 167 +++++++++++++++++++
>  drivers/gpu/drm/i915/gvt/vgpu.c              |  13 +-
>  5 files changed, 223 insertions(+), 3 deletions(-)
>  create mode 100644 drivers/gpu/drm/i915/gvt/migration_version.c
> 

(...)

> diff --git a/drivers/gpu/drm/i915/gvt/gvt.c b/drivers/gpu/drm/i915/gvt/gvt.c
> index 43f4242062dd..be2980e8ac75 100644
> --- a/drivers/gpu/drm/i915/gvt/gvt.c
> +++ b/drivers/gpu/drm/i915/gvt/gvt.c
> @@ -105,14 +105,53 @@ static ssize_t description_show(struct kobject *kobj, struct device *dev,
>  		       type->weight);
>  }
>  
> +static ssize_t migration_version_show(struct kobject *kobj, struct device *dev,
> +		char *buf)

Indentation looks a bit odd? (Also below.)

> +{
> +	struct intel_vgpu_type *type;
> +	void *gvt = kdev_to_i915(dev)->gvt;
> +
> +	type = intel_gvt_find_vgpu_type(gvt, kobject_name(kobj));
> +	if (!type || !type->migration_version) {
> +		gvt_err("Does not support migraion on type %s. Please search previous detailed log\n",

s/migraion/migration/ (also below)

Or reword to "Migration not supported on type %s."?

> +				kobject_name(kobj));
> +		return -ENODEV;
> +	}
> +
> +	return snprintf(buf, strlen(type->migration_version) + 2,
> +			"%s\n", type->migration_version);
> +}
> +
> +static ssize_t migration_version_store(struct kobject *kobj, struct device *dev,
> +		const char *buf, size_t count)
> +{
> +	int ret = 0;
> +	struct intel_vgpu_type *type;
> +	void *gvt = kdev_to_i915(dev)->gvt;
> +
> +	type = intel_gvt_find_vgpu_type(gvt, kobject_name(kobj));
> +	if (!type || !type->migration_version) {
> +		gvt_err("Does not support migraion on type %s. Please search previous detailed log\n",
> +				kobject_name(kobj));
> +		return -ENODEV;
> +	}
> +
> +	ret = intel_gvt_check_vfio_migration_version(gvt,
> +			type->migration_version, buf);
> +
> +	return (ret < 0 ? ret : count);
> +}
> +
>  static MDEV_TYPE_ATTR_RO(available_instances);
>  static MDEV_TYPE_ATTR_RO(device_api);
>  static MDEV_TYPE_ATTR_RO(description);
> +static MDEV_TYPE_ATTR_RW(migration_version);
>  
>  static struct attribute *gvt_type_attrs[] = {
>  	&mdev_type_attr_available_instances.attr,
>  	&mdev_type_attr_device_api.attr,
>  	&mdev_type_attr_description.attr,
> +	&mdev_type_attr_migration_version.attr,
>  	NULL,
>  };

(...)

> +char *
> +intel_gvt_get_vfio_migration_version(struct intel_gvt *gvt,
> +		const char *vgpu_type)
> +{
> +	int cnt = 0;
> +	struct drm_i915_private *dev_priv = gvt->dev_priv;
> +	char *version = NULL;
> +
> +	/* currently only gen8 & gen9 are supported */
> +	if (!IS_GEN(dev_priv, 8) && !IS_GEN(dev_priv, 9)) {
> +		gvt_err("Local hardware does not support migration on %d\n",
> +				INTEL_INFO(dev_priv)->gen);
> +		return NULL;
> +	}
> +
> +	if (GVT_VFIO_MIGRATION_SOFTWARE_VERSION == INV_SOFTWARE_VERSION) {
> +		gvt_err("Local GVT does not support migration\n");
> +		return NULL;
> +	}
> +
> +	version = kzalloc(MIGRATION_VERSION_TOTAL_LEN, GFP_KERNEL);
> +
> +	if (unlikely(!version)) {
> +		gvt_err("memory allocation failed when get local migraiton version\n");

s/migraiton/migration/

Or "cannot allocate memory for local migration version"?

> +		return NULL;
> +	}
> +
> +	/* vendor id + device id + vgpu type + software version */
> +	cnt = snprintf(version, MIGRATION_VERSION_TOTAL_LEN, PRINTF_FORMAT,
> +			PCI_VENDOR_ID_INTEL,
> +			INTEL_DEVID(dev_priv),
> +			vgpu_type,
> +			GVT_VFIO_MIGRATION_SOFTWARE_VERSION);
> +
> +	if (cnt)
> +		return version;
> +
> +	gvt_err("string generation failed when get local migration version\n");
> +	return NULL;
> +}

(...)

Only some nitpicks from me, but I'm not really familiar with this
driver. Overall, this looks sane to me, so have an

Acked-by: Cornelia Huck <cohuck@redhat.com>

Re: [PATCH v3 1/2] vfio/mdev: add migration_version attribute for mdev device

[Yan Zhao]

On Tue, May 28, 2019 at 04:53:32PM +0800, Cornelia Huck wrote:
> On Sun, 26 May 2019 23:43:42 -0400
> Yan Zhao <yan.y.zhao@intel.com> wrote:
> 
> > migration_version attribute is used to check migration compatibility
> > between two mdev device of the same mdev type.
> 
> s/device/devices/
>
yes... sorry and thanks :)

> > The key is that it's rw and its data is opaque to userspace.
> > 
> > Userspace reads migration_version of mdev device at source side and
> > writes the value to migration_version attribute of mdev device at target
> > side. It judges migration compatibility according to whether the read
> > and write operations succeed or fail.
> > 
> > As this attribute is under mdev_type node, userspace is able to know
> > whether two mdev devices are compatible before a mdev device is created.
> > 
> > userspace needs to check whether the two mdev devices are of the same
> > mdev type before checking the migration_version attribute. It also needs
> > to check device creation parameters if aggregation is supported in
> > future.
> > 
> >              __    userspace
> >               /\              \
> >              /                 \write
> >             / read              \
> >    ________/__________       ___\|/_____________
> >   | migration_version |     | migration_version |-->check migration
> >   ---------------------     ---------------------   compatibility
> >     mdev device A               mdev device B
> > 
> > Cc: Alex Williamson <alex.williamson@redhat.com>
> > Cc: Erik Skultety <eskultet@redhat.com>
> > Cc: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> > Cc: Cornelia Huck <cohuck@redhat.com>
> > Cc: "Tian, Kevin" <kevin.tian@intel.com>
> > Cc: Zhenyu Wang <zhenyuw@linux.intel.com>
> > Cc: "Wang, Zhi A" <zhi.a.wang@intel.com>
> > Cc: Neo Jia <cjia@nvidia.com>
> > Cc: Kirti Wankhede <kwankhede@nvidia.com>
> > Cc: Daniel P. Berrangé <berrange@redhat.com>
> > Cc: Christophe de Dinechin <dinechin@redhat.com>
> > 
> > Signed-off-by: Yan Zhao <yan.y.zhao@intel.com>
> > 
> > ---
> > v3:
> > 1. renamed version to migration_version
> > (Christophe de Dinechin, Cornelia Huck, Alex Williamson)
> > 2. let errno to be freely defined by vendor driver
> > (Alex Williamson, Erik Skultety, Cornelia Huck, Dr. David Alan Gilbert)
> > 3. let checking mdev_type be prerequisite of migration compatibility
> > check. (Alex Williamson)
> > 4. reworded example usage section.
> > (most of this section came from Alex Williamson)
> > 5. reworded attribute intention section (Cornelia Huck)
> > 
> > v2:
> > 1. added detailed intent and usage
> > 2. made definition of version string completely private to vendor driver
> >    (Alex Williamson)
> > 3. abandoned changes to sample mdev drivers (Alex Williamson)
> > 4. mandatory --> optional (Cornelia Huck)
> > 5. added description for errno (Cornelia Huck)
> > ---
> >  Documentation/vfio-mediated-device.txt | 113 +++++++++++++++++++++++++
> >  1 file changed, 113 insertions(+)
> > 
> 
> While I probably would have written a more compact description, your
> version is fine with me as well.
> 
> Reviewed-by: Cornelia Huck <cohuck@redhat.com>
Thank you Cornelia!

> _______________________________________________
> intel-gvt-dev mailing list
> intel-gvt-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gvt-dev

Re: [Qemu-devel] [PATCH v3 1/2] vfio/mdev: add migration_version attribute for mdev device

[Yan Zhao]

On Tue, May 28, 2019 at 04:53:32PM +0800, Cornelia Huck wrote:
> On Sun, 26 May 2019 23:43:42 -0400
> Yan Zhao <yan.y.zhao@intel.com> wrote:
> 
> > migration_version attribute is used to check migration compatibility
> > between two mdev device of the same mdev type.
> 
> s/device/devices/
>
yes... sorry and thanks :)

> > The key is that it's rw and its data is opaque to userspace.
> > 
> > Userspace reads migration_version of mdev device at source side and
> > writes the value to migration_version attribute of mdev device at target
> > side. It judges migration compatibility according to whether the read
> > and write operations succeed or fail.
> > 
> > As this attribute is under mdev_type node, userspace is able to know
> > whether two mdev devices are compatible before a mdev device is created.
> > 
> > userspace needs to check whether the two mdev devices are of the same
> > mdev type before checking the migration_version attribute. It also needs
> > to check device creation parameters if aggregation is supported in
> > future.
> > 
> >              __    userspace
> >               /\              \
> >              /                 \write
> >             / read              \
> >    ________/__________       ___\|/_____________
> >   | migration_version |     | migration_version |-->check migration
> >   ---------------------     ---------------------   compatibility
> >     mdev device A               mdev device B
> > 
> > Cc: Alex Williamson <alex.williamson@redhat.com>
> > Cc: Erik Skultety <eskultet@redhat.com>
> > Cc: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> > Cc: Cornelia Huck <cohuck@redhat.com>
> > Cc: "Tian, Kevin" <kevin.tian@intel.com>
> > Cc: Zhenyu Wang <zhenyuw@linux.intel.com>
> > Cc: "Wang, Zhi A" <zhi.a.wang@intel.com>
> > Cc: Neo Jia <cjia@nvidia.com>
> > Cc: Kirti Wankhede <kwankhede@nvidia.com>
> > Cc: Daniel P. Berrangé <berrange@redhat.com>
> > Cc: Christophe de Dinechin <dinechin@redhat.com>
> > 
> > Signed-off-by: Yan Zhao <yan.y.zhao@intel.com>
> > 
> > ---
> > v3:
> > 1. renamed version to migration_version
> > (Christophe de Dinechin, Cornelia Huck, Alex Williamson)
> > 2. let errno to be freely defined by vendor driver
> > (Alex Williamson, Erik Skultety, Cornelia Huck, Dr. David Alan Gilbert)
> > 3. let checking mdev_type be prerequisite of migration compatibility
> > check. (Alex Williamson)
> > 4. reworded example usage section.
> > (most of this section came from Alex Williamson)
> > 5. reworded attribute intention section (Cornelia Huck)
> > 
> > v2:
> > 1. added detailed intent and usage
> > 2. made definition of version string completely private to vendor driver
> >    (Alex Williamson)
> > 3. abandoned changes to sample mdev drivers (Alex Williamson)
> > 4. mandatory --> optional (Cornelia Huck)
> > 5. added description for errno (Cornelia Huck)
> > ---
> >  Documentation/vfio-mediated-device.txt | 113 +++++++++++++++++++++++++
> >  1 file changed, 113 insertions(+)
> > 
> 
> While I probably would have written a more compact description, your
> version is fine with me as well.
> 
> Reviewed-by: Cornelia Huck <cohuck@redhat.com>
Thank you Cornelia!

> _______________________________________________
> intel-gvt-dev mailing list
> intel-gvt-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gvt-dev

Re: [PATCH v3 2/2] drm/i915/gvt: export migration_version to mdev sysfs for Intel vGPU

[Yan Zhao]

On Tue, May 28, 2019 at 05:01:35PM +0800, Cornelia Huck wrote:
> On Sun, 26 May 2019 23:44:37 -0400
> Yan Zhao <yan.y.zhao@intel.com> wrote:
> 
> > This feature implements the migration_version attribute for Intel's vGPU
> > mdev devices.
> > 
> > migration_version attribute is rw.
> > It's used to check migration compatibility for two mdev devices of the
> > same mdev type.
> > migration_version string is defined by vendor driver and opaque to
> > userspace.
> > 
> > For Intel vGPU of gen8 and gen9, the format of migration_version string
> > is:
> >   <vendor id>-<device id>-<vgpu type>-<software version>.
> > 
> > For future platforms, the format of migration_version string is to be
> > expanded to include more meta data to identify Intel vGPUs for live
> > migration compatibility check
> > 
> > For old platforms, and for GVT not supporting vGPU live migration
> > feature, -ENODEV is returned on read(2)/write(2) of migration_version
> > attribute.
> > For vGPUs running old GVT who do not expose migration_version
> > attribute, live migration is regarded as not supported for those vGPUs.
> > 
> > Cc: Alex Williamson <alex.williamson@redhat.com>
> > Cc: Erik Skultety <eskultet@redhat.com>
> > Cc: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> > Cc: Cornelia Huck <cohuck@redhat.com>
> > Cc: "Tian, Kevin" <kevin.tian@intel.com>
> > Cc: Zhenyu Wang <zhenyuw@linux.intel.com>
> > Cc: "Wang, Zhi A" <zhi.a.wang@intel.com>
> > c: Neo Jia <cjia@nvidia.com>
> > Cc: Kirti Wankhede <kwankhede@nvidia.com>
> > 
> > Signed-off-by: Yan Zhao <yan.y.zhao@intel.com>
> > 
> > ---
> > v3:
> > 1. renamed version to migration_version
> > (Christophe de Dinechin, Cornelia Huck, Alex Williamson)
> > 2. instead of generating migration version strings each time, storing
> > them in vgpu types generated during initialization.
> > (Zhenyu Wang, Cornelia Huck)
> > 3. replaced multiple snprintf to one big snprintf in
> > intel_gvt_get_vfio_migration_version()
> > (Dr. David Alan Gilbert)
> > 4. printed detailed error log
> > (Alex Williamson, Erik Skultety, Cornelia Huck, Dr. David Alan Gilbert)
> > 5. incorporated <software version> into migration_version string
> > (Alex Williamson)
> > 6. do not use ifndef macro to switch off migration_version attribute
> > (Zhenyu Wang)
> > 
> > v2:
> > 1. removed 32 common part of version string
> > (Alex Williamson)
> > 2. do not register version attribute for GVT not supporting live
> > migration.(Cornelia Huck)
> > 3. for platforms out of gen8, gen9, return -EINVAL --> -ENODEV for
> > incompatible. (Cornelia Huck)
> > ---
> >  drivers/gpu/drm/i915/gvt/Makefile            |   2 +-
> >  drivers/gpu/drm/i915/gvt/gvt.c               |  39 +++++
> >  drivers/gpu/drm/i915/gvt/gvt.h               |   5 +
> >  drivers/gpu/drm/i915/gvt/migration_version.c | 167 +++++++++++++++++++
> >  drivers/gpu/drm/i915/gvt/vgpu.c              |  13 +-
> >  5 files changed, 223 insertions(+), 3 deletions(-)
> >  create mode 100644 drivers/gpu/drm/i915/gvt/migration_version.c
> > 
> 
> (...)
> 
> > diff --git a/drivers/gpu/drm/i915/gvt/gvt.c b/drivers/gpu/drm/i915/gvt/gvt.c
> > index 43f4242062dd..be2980e8ac75 100644
> > --- a/drivers/gpu/drm/i915/gvt/gvt.c
> > +++ b/drivers/gpu/drm/i915/gvt/gvt.c
> > @@ -105,14 +105,53 @@ static ssize_t description_show(struct kobject *kobj, struct device *dev,
> >  		       type->weight);
> >  }
> >  
> > +static ssize_t migration_version_show(struct kobject *kobj, struct device *dev,
> > +		char *buf)
> 
> Indentation looks a bit odd? (Also below.)
>
yes. Let me correct it in next revision.

> > +{
> > +	struct intel_vgpu_type *type;
> > +	void *gvt = kdev_to_i915(dev)->gvt;
> > +
> > +	type = intel_gvt_find_vgpu_type(gvt, kobject_name(kobj));
> > +	if (!type || !type->migration_version) {
> > +		gvt_err("Does not support migraion on type %s. Please search previous detailed log\n",
> 
> s/migraion/migration/ (also below)
>
Sorry for typos again. I'll be more careful next time. thank you:)

> Or reword to "Migration not supported on type %s."?
>
Yes, better :)

> > +				kobject_name(kobj));
> > +		return -ENODEV;
> > +	}
> > +
> > +	return snprintf(buf, strlen(type->migration_version) + 2,
> > +			"%s\n", type->migration_version);
> > +}
> > +
> > +static ssize_t migration_version_store(struct kobject *kobj, struct device *dev,
> > +		const char *buf, size_t count)
> > +{
> > +	int ret = 0;
> > +	struct intel_vgpu_type *type;
> > +	void *gvt = kdev_to_i915(dev)->gvt;
> > +
> > +	type = intel_gvt_find_vgpu_type(gvt, kobject_name(kobj));
> > +	if (!type || !type->migration_version) {
> > +		gvt_err("Does not support migraion on type %s. Please search previous detailed log\n",
> > +				kobject_name(kobj));
> > +		return -ENODEV;
> > +	}
> > +
> > +	ret = intel_gvt_check_vfio_migration_version(gvt,
> > +			type->migration_version, buf);
> > +
> > +	return (ret < 0 ? ret : count);
> > +}
> > +
> >  static MDEV_TYPE_ATTR_RO(available_instances);
> >  static MDEV_TYPE_ATTR_RO(device_api);
> >  static MDEV_TYPE_ATTR_RO(description);
> > +static MDEV_TYPE_ATTR_RW(migration_version);
> >  
> >  static struct attribute *gvt_type_attrs[] = {
> >  	&mdev_type_attr_available_instances.attr,
> >  	&mdev_type_attr_device_api.attr,
> >  	&mdev_type_attr_description.attr,
> > +	&mdev_type_attr_migration_version.attr,
> >  	NULL,
> >  };
> 
> (...)
> 
> > +char *
> > +intel_gvt_get_vfio_migration_version(struct intel_gvt *gvt,
> > +		const char *vgpu_type)
> > +{
> > +	int cnt = 0;
> > +	struct drm_i915_private *dev_priv = gvt->dev_priv;
> > +	char *version = NULL;
> > +
> > +	/* currently only gen8 & gen9 are supported */
> > +	if (!IS_GEN(dev_priv, 8) && !IS_GEN(dev_priv, 9)) {
> > +		gvt_err("Local hardware does not support migration on %d\n",
> > +				INTEL_INFO(dev_priv)->gen);
> > +		return NULL;
> > +	}
> > +
> > +	if (GVT_VFIO_MIGRATION_SOFTWARE_VERSION == INV_SOFTWARE_VERSION) {
> > +		gvt_err("Local GVT does not support migration\n");
> > +		return NULL;
> > +	}
> > +
> > +	version = kzalloc(MIGRATION_VERSION_TOTAL_LEN, GFP_KERNEL);
> > +
> > +	if (unlikely(!version)) {
> > +		gvt_err("memory allocation failed when get local migraiton version\n");
> 
> s/migraiton/migration/
> 
> Or "cannot allocate memory for local migration version"?
>
Ok.

> > +		return NULL;
> > +	}
> > +
> > +	/* vendor id + device id + vgpu type + software version */
> > +	cnt = snprintf(version, MIGRATION_VERSION_TOTAL_LEN, PRINTF_FORMAT,
> > +			PCI_VENDOR_ID_INTEL,
> > +			INTEL_DEVID(dev_priv),
> > +			vgpu_type,
> > +			GVT_VFIO_MIGRATION_SOFTWARE_VERSION);
> > +
> > +	if (cnt)
> > +		return version;
> > +
> > +	gvt_err("string generation failed when get local migration version\n");
> > +	return NULL;
> > +}
> 
> (...)
> 
> Only some nitpicks from me, but I'm not really familiar with this
> driver. Overall, this looks sane to me, so have an
> 
> Acked-by: Cornelia Huck <cohuck@redhat.com>


Thank you so much Cornelia :)



______________________________________________
> intel-gvt-dev mailing list
> intel-gvt-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gvt-dev

Re: [Qemu-devel] [PATCH v3 2/2] drm/i915/gvt: export migration_version to mdev sysfs for Intel vGPU

[Yan Zhao]

On Tue, May 28, 2019 at 05:01:35PM +0800, Cornelia Huck wrote:
> On Sun, 26 May 2019 23:44:37 -0400
> Yan Zhao <yan.y.zhao@intel.com> wrote:
> 
> > This feature implements the migration_version attribute for Intel's vGPU
> > mdev devices.
> > 
> > migration_version attribute is rw.
> > It's used to check migration compatibility for two mdev devices of the
> > same mdev type.
> > migration_version string is defined by vendor driver and opaque to
> > userspace.
> > 
> > For Intel vGPU of gen8 and gen9, the format of migration_version string
> > is:
> >   <vendor id>-<device id>-<vgpu type>-<software version>.
> > 
> > For future platforms, the format of migration_version string is to be
> > expanded to include more meta data to identify Intel vGPUs for live
> > migration compatibility check
> > 
> > For old platforms, and for GVT not supporting vGPU live migration
> > feature, -ENODEV is returned on read(2)/write(2) of migration_version
> > attribute.
> > For vGPUs running old GVT who do not expose migration_version
> > attribute, live migration is regarded as not supported for those vGPUs.
> > 
> > Cc: Alex Williamson <alex.williamson@redhat.com>
> > Cc: Erik Skultety <eskultet@redhat.com>
> > Cc: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> > Cc: Cornelia Huck <cohuck@redhat.com>
> > Cc: "Tian, Kevin" <kevin.tian@intel.com>
> > Cc: Zhenyu Wang <zhenyuw@linux.intel.com>
> > Cc: "Wang, Zhi A" <zhi.a.wang@intel.com>
> > c: Neo Jia <cjia@nvidia.com>
> > Cc: Kirti Wankhede <kwankhede@nvidia.com>
> > 
> > Signed-off-by: Yan Zhao <yan.y.zhao@intel.com>
> > 
> > ---
> > v3:
> > 1. renamed version to migration_version
> > (Christophe de Dinechin, Cornelia Huck, Alex Williamson)
> > 2. instead of generating migration version strings each time, storing
> > them in vgpu types generated during initialization.
> > (Zhenyu Wang, Cornelia Huck)
> > 3. replaced multiple snprintf to one big snprintf in
> > intel_gvt_get_vfio_migration_version()
> > (Dr. David Alan Gilbert)
> > 4. printed detailed error log
> > (Alex Williamson, Erik Skultety, Cornelia Huck, Dr. David Alan Gilbert)
> > 5. incorporated <software version> into migration_version string
> > (Alex Williamson)
> > 6. do not use ifndef macro to switch off migration_version attribute
> > (Zhenyu Wang)
> > 
> > v2:
> > 1. removed 32 common part of version string
> > (Alex Williamson)
> > 2. do not register version attribute for GVT not supporting live
> > migration.(Cornelia Huck)
> > 3. for platforms out of gen8, gen9, return -EINVAL --> -ENODEV for
> > incompatible. (Cornelia Huck)
> > ---
> >  drivers/gpu/drm/i915/gvt/Makefile            |   2 +-
> >  drivers/gpu/drm/i915/gvt/gvt.c               |  39 +++++
> >  drivers/gpu/drm/i915/gvt/gvt.h               |   5 +
> >  drivers/gpu/drm/i915/gvt/migration_version.c | 167 +++++++++++++++++++
> >  drivers/gpu/drm/i915/gvt/vgpu.c              |  13 +-
> >  5 files changed, 223 insertions(+), 3 deletions(-)
> >  create mode 100644 drivers/gpu/drm/i915/gvt/migration_version.c
> > 
> 
> (...)
> 
> > diff --git a/drivers/gpu/drm/i915/gvt/gvt.c b/drivers/gpu/drm/i915/gvt/gvt.c
> > index 43f4242062dd..be2980e8ac75 100644
> > --- a/drivers/gpu/drm/i915/gvt/gvt.c
> > +++ b/drivers/gpu/drm/i915/gvt/gvt.c
> > @@ -105,14 +105,53 @@ static ssize_t description_show(struct kobject *kobj, struct device *dev,
> >  		       type->weight);
> >  }
> >  
> > +static ssize_t migration_version_show(struct kobject *kobj, struct device *dev,
> > +		char *buf)
> 
> Indentation looks a bit odd? (Also below.)
>
yes. Let me correct it in next revision.

> > +{
> > +	struct intel_vgpu_type *type;
> > +	void *gvt = kdev_to_i915(dev)->gvt;
> > +
> > +	type = intel_gvt_find_vgpu_type(gvt, kobject_name(kobj));
> > +	if (!type || !type->migration_version) {
> > +		gvt_err("Does not support migraion on type %s. Please search previous detailed log\n",
> 
> s/migraion/migration/ (also below)
>
Sorry for typos again. I'll be more careful next time. thank you:)

> Or reword to "Migration not supported on type %s."?
>
Yes, better :)

> > +				kobject_name(kobj));
> > +		return -ENODEV;
> > +	}
> > +
> > +	return snprintf(buf, strlen(type->migration_version) + 2,
> > +			"%s\n", type->migration_version);
> > +}
> > +
> > +static ssize_t migration_version_store(struct kobject *kobj, struct device *dev,
> > +		const char *buf, size_t count)
> > +{
> > +	int ret = 0;
> > +	struct intel_vgpu_type *type;
> > +	void *gvt = kdev_to_i915(dev)->gvt;
> > +
> > +	type = intel_gvt_find_vgpu_type(gvt, kobject_name(kobj));
> > +	if (!type || !type->migration_version) {
> > +		gvt_err("Does not support migraion on type %s. Please search previous detailed log\n",
> > +				kobject_name(kobj));
> > +		return -ENODEV;
> > +	}
> > +
> > +	ret = intel_gvt_check_vfio_migration_version(gvt,
> > +			type->migration_version, buf);
> > +
> > +	return (ret < 0 ? ret : count);
> > +}
> > +
> >  static MDEV_TYPE_ATTR_RO(available_instances);
> >  static MDEV_TYPE_ATTR_RO(device_api);
> >  static MDEV_TYPE_ATTR_RO(description);
> > +static MDEV_TYPE_ATTR_RW(migration_version);
> >  
> >  static struct attribute *gvt_type_attrs[] = {
> >  	&mdev_type_attr_available_instances.attr,
> >  	&mdev_type_attr_device_api.attr,
> >  	&mdev_type_attr_description.attr,
> > +	&mdev_type_attr_migration_version.attr,
> >  	NULL,
> >  };
> 
> (...)
> 
> > +char *
> > +intel_gvt_get_vfio_migration_version(struct intel_gvt *gvt,
> > +		const char *vgpu_type)
> > +{
> > +	int cnt = 0;
> > +	struct drm_i915_private *dev_priv = gvt->dev_priv;
> > +	char *version = NULL;
> > +
> > +	/* currently only gen8 & gen9 are supported */
> > +	if (!IS_GEN(dev_priv, 8) && !IS_GEN(dev_priv, 9)) {
> > +		gvt_err("Local hardware does not support migration on %d\n",
> > +				INTEL_INFO(dev_priv)->gen);
> > +		return NULL;
> > +	}
> > +
> > +	if (GVT_VFIO_MIGRATION_SOFTWARE_VERSION == INV_SOFTWARE_VERSION) {
> > +		gvt_err("Local GVT does not support migration\n");
> > +		return NULL;
> > +	}
> > +
> > +	version = kzalloc(MIGRATION_VERSION_TOTAL_LEN, GFP_KERNEL);
> > +
> > +	if (unlikely(!version)) {
> > +		gvt_err("memory allocation failed when get local migraiton version\n");
> 
> s/migraiton/migration/
> 
> Or "cannot allocate memory for local migration version"?
>
Ok.

> > +		return NULL;
> > +	}
> > +
> > +	/* vendor id + device id + vgpu type + software version */
> > +	cnt = snprintf(version, MIGRATION_VERSION_TOTAL_LEN, PRINTF_FORMAT,
> > +			PCI_VENDOR_ID_INTEL,
> > +			INTEL_DEVID(dev_priv),
> > +			vgpu_type,
> > +			GVT_VFIO_MIGRATION_SOFTWARE_VERSION);
> > +
> > +	if (cnt)
> > +		return version;
> > +
> > +	gvt_err("string generation failed when get local migration version\n");
> > +	return NULL;
> > +}
> 
> (...)
> 
> Only some nitpicks from me, but I'm not really familiar with this
> driver. Overall, this looks sane to me, so have an
> 
> Acked-by: Cornelia Huck <cohuck@redhat.com>


Thank you so much Cornelia :)



______________________________________________
> intel-gvt-dev mailing list
> intel-gvt-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gvt-dev

Re: [PATCH v3 2/2] drm/i915/gvt: export migration_version to mdev sysfs for Intel vGPU

[Zhenyu Wang]

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

On 2019.05.26 23:44:37 -0400, Yan Zhao wrote:
> This feature implements the migration_version attribute for Intel's vGPU
> mdev devices.
> 
> migration_version attribute is rw.
> It's used to check migration compatibility for two mdev devices of the
> same mdev type.
> migration_version string is defined by vendor driver and opaque to
> userspace.
> 
> For Intel vGPU of gen8 and gen9, the format of migration_version string
> is:
>   <vendor id>-<device id>-<vgpu type>-<software version>.
> 
> For future platforms, the format of migration_version string is to be
> expanded to include more meta data to identify Intel vGPUs for live
> migration compatibility check
> 
> For old platforms, and for GVT not supporting vGPU live migration
> feature, -ENODEV is returned on read(2)/write(2) of migration_version
> attribute.
> For vGPUs running old GVT who do not expose migration_version
> attribute, live migration is regarded as not supported for those vGPUs.
> 
> Cc: Alex Williamson <alex.williamson@redhat.com>
> Cc: Erik Skultety <eskultet@redhat.com>
> Cc: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> Cc: Cornelia Huck <cohuck@redhat.com>
> Cc: "Tian, Kevin" <kevin.tian@intel.com>
> Cc: Zhenyu Wang <zhenyuw@linux.intel.com>
> Cc: "Wang, Zhi A" <zhi.a.wang@intel.com>
> c: Neo Jia <cjia@nvidia.com>
> Cc: Kirti Wankhede <kwankhede@nvidia.com>
> 
> Signed-off-by: Yan Zhao <yan.y.zhao@intel.com>
> 
> ---
> v3:
> 1. renamed version to migration_version
> (Christophe de Dinechin, Cornelia Huck, Alex Williamson)
> 2. instead of generating migration version strings each time, storing
> them in vgpu types generated during initialization.
> (Zhenyu Wang, Cornelia Huck)
> 3. replaced multiple snprintf to one big snprintf in
> intel_gvt_get_vfio_migration_version()
> (Dr. David Alan Gilbert)
> 4. printed detailed error log
> (Alex Williamson, Erik Skultety, Cornelia Huck, Dr. David Alan Gilbert)
> 5. incorporated <software version> into migration_version string
> (Alex Williamson)
> 6. do not use ifndef macro to switch off migration_version attribute
> (Zhenyu Wang)
> 
> v2:
> 1. removed 32 common part of version string
> (Alex Williamson)
> 2. do not register version attribute for GVT not supporting live
> migration.(Cornelia Huck)
> 3. for platforms out of gen8, gen9, return -EINVAL --> -ENODEV for
> incompatible. (Cornelia Huck)
> ---
>  drivers/gpu/drm/i915/gvt/Makefile            |   2 +-
>  drivers/gpu/drm/i915/gvt/gvt.c               |  39 +++++
>  drivers/gpu/drm/i915/gvt/gvt.h               |   5 +
>  drivers/gpu/drm/i915/gvt/migration_version.c | 167 +++++++++++++++++++
>  drivers/gpu/drm/i915/gvt/vgpu.c              |  13 +-
>  5 files changed, 223 insertions(+), 3 deletions(-)
>  create mode 100644 drivers/gpu/drm/i915/gvt/migration_version.c
> 
> diff --git a/drivers/gpu/drm/i915/gvt/Makefile b/drivers/gpu/drm/i915/gvt/Makefile
> index 271fb46d4dd0..a9d561c93ab8 100644
> --- a/drivers/gpu/drm/i915/gvt/Makefile
> +++ b/drivers/gpu/drm/i915/gvt/Makefile
> @@ -3,7 +3,7 @@ GVT_DIR := gvt
>  GVT_SOURCE := gvt.o aperture_gm.o handlers.o vgpu.o trace_points.o firmware.o \
>  	interrupt.o gtt.o cfg_space.o opregion.o mmio.o display.o edid.o \
>  	execlist.o scheduler.o sched_policy.o mmio_context.o cmd_parser.o debugfs.o \
> -	fb_decoder.o dmabuf.o page_track.o
> +	fb_decoder.o dmabuf.o page_track.o migration_version.o
>  
>  ccflags-y				+= -I$(src) -I$(src)/$(GVT_DIR)
>  i915-y					+= $(addprefix $(GVT_DIR)/, $(GVT_SOURCE))
> diff --git a/drivers/gpu/drm/i915/gvt/gvt.c b/drivers/gpu/drm/i915/gvt/gvt.c
> index 43f4242062dd..be2980e8ac75 100644
> --- a/drivers/gpu/drm/i915/gvt/gvt.c
> +++ b/drivers/gpu/drm/i915/gvt/gvt.c
> @@ -105,14 +105,53 @@ static ssize_t description_show(struct kobject *kobj, struct device *dev,
>  		       type->weight);
>  }
>  
> +static ssize_t migration_version_show(struct kobject *kobj, struct device *dev,
> +		char *buf)
> +{
> +	struct intel_vgpu_type *type;
> +	void *gvt = kdev_to_i915(dev)->gvt;
> +
> +	type = intel_gvt_find_vgpu_type(gvt, kobject_name(kobj));
> +	if (!type || !type->migration_version) {
> +		gvt_err("Does not support migraion on type %s. Please search previous detailed log\n",
> +				kobject_name(kobj));
> +		return -ENODEV;
> +	}
> +
> +	return snprintf(buf, strlen(type->migration_version) + 2,
> +			"%s\n", type->migration_version);
> +}
> +
> +static ssize_t migration_version_store(struct kobject *kobj, struct device *dev,
> +		const char *buf, size_t count)
> +{
> +	int ret = 0;
> +	struct intel_vgpu_type *type;
> +	void *gvt = kdev_to_i915(dev)->gvt;
> +
> +	type = intel_gvt_find_vgpu_type(gvt, kobject_name(kobj));
> +	if (!type || !type->migration_version) {
> +		gvt_err("Does not support migraion on type %s. Please search previous detailed log\n",
> +				kobject_name(kobj));
> +		return -ENODEV;
> +	}
> +
> +	ret = intel_gvt_check_vfio_migration_version(gvt,
> +			type->migration_version, buf);
> +
> +	return (ret < 0 ? ret : count);
> +}
> +
>  static MDEV_TYPE_ATTR_RO(available_instances);
>  static MDEV_TYPE_ATTR_RO(device_api);
>  static MDEV_TYPE_ATTR_RO(description);
> +static MDEV_TYPE_ATTR_RW(migration_version);
>  
>  static struct attribute *gvt_type_attrs[] = {
>  	&mdev_type_attr_available_instances.attr,
>  	&mdev_type_attr_device_api.attr,
>  	&mdev_type_attr_description.attr,
> +	&mdev_type_attr_migration_version.attr,
>  	NULL,
>  };
>  
> diff --git a/drivers/gpu/drm/i915/gvt/gvt.h b/drivers/gpu/drm/i915/gvt/gvt.h
> index f5a328b5290a..265319d35c25 100644
> --- a/drivers/gpu/drm/i915/gvt/gvt.h
> +++ b/drivers/gpu/drm/i915/gvt/gvt.h
> @@ -296,6 +296,7 @@ struct intel_vgpu_type {
>  	unsigned int fence;
>  	unsigned int weight;
>  	enum intel_vgpu_edid resolution;
> +	char *migration_version;
>  };
>  
>  struct intel_gvt {
> @@ -687,6 +688,10 @@ void intel_gvt_debugfs_remove_vgpu(struct intel_vgpu *vgpu);
>  int intel_gvt_debugfs_init(struct intel_gvt *gvt);
>  void intel_gvt_debugfs_clean(struct intel_gvt *gvt);
>  
> +ssize_t intel_gvt_check_vfio_migration_version(struct intel_gvt *gvt,
> +		const char *self, const char *remote);
> +char *intel_gvt_get_vfio_migration_version(struct intel_gvt *gvt,
> +		const char *vgpu_type);
>  
>  #include "trace.h"
>  #include "mpt.h"
> diff --git a/drivers/gpu/drm/i915/gvt/migration_version.c b/drivers/gpu/drm/i915/gvt/migration_version.c
> new file mode 100644
> index 000000000000..e6acf188b29a
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/gvt/migration_version.c
> @@ -0,0 +1,167 @@
> +/*
> + * Copyright(c) 2011-2017 Intel Corporation. All rights reserved.
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the next
> + * paragraph) shall be included in all copies or substantial portions of the
> + * Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
> + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
> + * SOFTWARE.
> + *
> + * Authors:
> + *    Yan Zhao <yan.y.zhao@intel.com>
> + */
> +#include <linux/vfio.h>
> +#include "i915_drv.h"
> +#include "gvt.h"
> +
> +#define INV_SOFTWARE_VERSION (-1U)
> +#define VENDOR_ID_LEN (4)
> +#define DEVICE_ID_LEN (4)
> +#define VGPU_TYPE_LEN (16)
> +#define SOFTWARE_VER_LEN (8)
> +
> +/* total length of vfio migration version string.
> + * never exceed limit of PATH_MAX (4096)
> + */
> +#define MIGRATION_VERSION_TOTAL_LEN (VENDOR_ID_LEN + DEVICE_ID_LEN + \
> +					VGPU_TYPE_LEN + SOFTWARE_VER_LEN + 4)
> +
> +#define GVT_VFIO_MIGRATION_SOFTWARE_VERSION INV_SOFTWARE_VERSION
> +
> +
> +#define PRINTF_FORMAT "%04x-%04x-%s-%08x"
> +#define SCANF_FORMAT "%x-%x-%16[^-]-%x"
> +
> +enum incompatible_reason {
> +	IREASON_WRONG_REMOTE_FORMAT = 0,
> +	IREASON_HARDWARE_MISMATCH,
> +	IREASON_SOFTWARE_VERSION_MISMATCH,
> +	IREASON_VGPU_TYPE_MISMATCH,
> +};
> +
> +static const char *const incompatible_reason_str[] = {
> +	[IREASON_WRONG_REMOTE_FORMAT] =
> +		"wrong string format. probably wrong GVT version",
> +	[IREASON_HARDWARE_MISMATCH] =
> +		"physical device not matched",
> +	[IREASON_SOFTWARE_VERSION_MISMATCH] =
> +		"migration software version not matched",
> +	[IREASON_VGPU_TYPE_MISMATCH] =
> +		"vgpu type not matched"
> +};
> +
> +static bool is_compatible(const char *local, const char *remote)
> +{
> +	bool ret;
> +
> +	ret = sysfs_streq(local, remote);
> +
> +	if (!ret) {
> +		int vid_l = 0, did_l = 0, vid_r = 0, did_r = 0;
> +		char type_l[VGPU_TYPE_LEN], type_r[VGPU_TYPE_LEN];
> +		u32 sv_l = 0, sv_r = 0;
> +		int rl = 0, rr = 0;
> +		enum incompatible_reason reason = IREASON_WRONG_REMOTE_FORMAT;
> +
> +		memset(type_l, 0, sizeof(type_l));
> +		memset(type_r, 0, sizeof(type_r));
> +
> +		rl = sscanf(local, SCANF_FORMAT,
> +				&vid_l, &did_l, type_l, &sv_l);
> +		rr = sscanf(remote, SCANF_FORMAT,
> +				&vid_r, &did_r, type_r, &sv_r);
> +
> +		if (rl == rr) {
> +			if (vid_l != vid_r || did_l != did_r)
> +				reason = IREASON_HARDWARE_MISMATCH;
> +			else if (sv_l != sv_r)
> +				reason = IREASON_SOFTWARE_VERSION_MISMATCH;
> +			else if (strncmp(type_l, type_r, VGPU_TYPE_LEN))
> +				reason = IREASON_VGPU_TYPE_MISMATCH;
> +		}
> +
> +		gvt_err("Migration version mismatched. Possible reason: %s. Local migration version:%s, Remote migration version:%s\n",
> +				incompatible_reason_str[reason], local,	remote);
> +
> +	}
> +	return ret;
> +
> +}
> +
> +
> +char *
> +intel_gvt_get_vfio_migration_version(struct intel_gvt *gvt,
> +		const char *vgpu_type)
> +{
> +	int cnt = 0;
> +	struct drm_i915_private *dev_priv = gvt->dev_priv;
> +	char *version = NULL;
> +
> +	/* currently only gen8 & gen9 are supported */
> +	if (!IS_GEN(dev_priv, 8) && !IS_GEN(dev_priv, 9)) {
> +		gvt_err("Local hardware does not support migration on %d\n",
> +				INTEL_INFO(dev_priv)->gen);
> +		return NULL;
> +	}
> +
> +	if (GVT_VFIO_MIGRATION_SOFTWARE_VERSION == INV_SOFTWARE_VERSION) {
> +		gvt_err("Local GVT does not support migration\n");
> +		return NULL;
> +	}
> +
> +	version = kzalloc(MIGRATION_VERSION_TOTAL_LEN, GFP_KERNEL);
> +
> +	if (unlikely(!version)) {
> +		gvt_err("memory allocation failed when get local migraiton version\n");
> +		return NULL;
> +	}
> +
> +	/* vendor id + device id + vgpu type + software version */
> +	cnt = snprintf(version, MIGRATION_VERSION_TOTAL_LEN, PRINTF_FORMAT,
> +			PCI_VENDOR_ID_INTEL,
> +			INTEL_DEVID(dev_priv),
> +			vgpu_type,
> +			GVT_VFIO_MIGRATION_SOFTWARE_VERSION);
> +
> +	if (cnt)
> +		return version;
> +
> +	gvt_err("string generation failed when get local migration version\n");
> +	return NULL;

Handle version kfree for possible error here. The approach looks fine to me.
I think some of those messages might be too chatty or maybe debug message instead.
Others are good to me.

Acked-by: Zhenyu Wang <zhenyuw@linux.intel.com>

> +}
> +
> +ssize_t intel_gvt_check_vfio_migration_version(struct intel_gvt *gvt,
> +		const char *self, const char *remote)
> +{
> +	struct drm_i915_private *dev_priv = gvt->dev_priv;
> +
> +	/* currently only gen8 & gen9 are supported */
> +	if (!IS_GEN(dev_priv, 8) && !IS_GEN(dev_priv, 9)) {
> +		gvt_err("Local hardware does not support migration on %d\n",
> +				INTEL_INFO(dev_priv)->gen);
> +		return -ENODEV;
> +	}
> +
> +	if (GVT_VFIO_MIGRATION_SOFTWARE_VERSION == INV_SOFTWARE_VERSION) {
> +		gvt_err("Local GVT does not support migration\n");
> +		return -ENODEV;
> +	}
> +
> +	if (!is_compatible(self, remote))
> +		return -EINVAL;
> +
> +	return 0;
> +}
> diff --git a/drivers/gpu/drm/i915/gvt/vgpu.c b/drivers/gpu/drm/i915/gvt/vgpu.c
> index 44ce3c2b9ac1..7642b21641bd 100644
> --- a/drivers/gpu/drm/i915/gvt/vgpu.c
> +++ b/drivers/gpu/drm/i915/gvt/vgpu.c
> @@ -155,13 +155,18 @@ int intel_gvt_init_vgpu_types(struct intel_gvt *gvt)
>  			sprintf(gvt->types[i].name, "GVTg_V5_%s",
>  						vgpu_types[i].name);
>  
> -		gvt_dbg_core("type[%d]: %s avail %u low %u high %u fence %u weight %u res %s\n",
> +		gvt->types[i].migration_version =
> +			intel_gvt_get_vfio_migration_version(gvt,
> +					gvt->types[i].name);
> +		gvt_dbg_core("type[%d]: %s avail %u low %u high %u fence %u weight %u res %s, migratio_version:%s\n",
>  			     i, gvt->types[i].name,
>  			     gvt->types[i].avail_instance,
>  			     gvt->types[i].low_gm_size,
>  			     gvt->types[i].high_gm_size, gvt->types[i].fence,
>  			     gvt->types[i].weight,
> -			     vgpu_edid_str(gvt->types[i].resolution));
> +			     vgpu_edid_str(gvt->types[i].resolution),
> +			     (gvt->types[i].migration_version ?
> +			     gvt->types[i].migration_version : "null"));
>  	}
>  
>  	gvt->num_types = i;
> @@ -170,6 +175,10 @@ int intel_gvt_init_vgpu_types(struct intel_gvt *gvt)
>  
>  void intel_gvt_clean_vgpu_types(struct intel_gvt *gvt)
>  {
> +	int i;
> +
> +	for (i = 0; i < gvt->num_types; i++)
> +		kfree(gvt->types[i].migration_version);
>  	kfree(gvt->types);
>  }
>  
> -- 
> 2.17.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 --]

Re: [Qemu-devel] [PATCH v3 2/2] drm/i915/gvt: export migration_version to mdev sysfs for Intel vGPU

[Zhenyu Wang]

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

On 2019.05.26 23:44:37 -0400, Yan Zhao wrote:
> This feature implements the migration_version attribute for Intel's vGPU
> mdev devices.
> 
> migration_version attribute is rw.
> It's used to check migration compatibility for two mdev devices of the
> same mdev type.
> migration_version string is defined by vendor driver and opaque to
> userspace.
> 
> For Intel vGPU of gen8 and gen9, the format of migration_version string
> is:
>   <vendor id>-<device id>-<vgpu type>-<software version>.
> 
> For future platforms, the format of migration_version string is to be
> expanded to include more meta data to identify Intel vGPUs for live
> migration compatibility check
> 
> For old platforms, and for GVT not supporting vGPU live migration
> feature, -ENODEV is returned on read(2)/write(2) of migration_version
> attribute.
> For vGPUs running old GVT who do not expose migration_version
> attribute, live migration is regarded as not supported for those vGPUs.
> 
> Cc: Alex Williamson <alex.williamson@redhat.com>
> Cc: Erik Skultety <eskultet@redhat.com>
> Cc: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> Cc: Cornelia Huck <cohuck@redhat.com>
> Cc: "Tian, Kevin" <kevin.tian@intel.com>
> Cc: Zhenyu Wang <zhenyuw@linux.intel.com>
> Cc: "Wang, Zhi A" <zhi.a.wang@intel.com>
> c: Neo Jia <cjia@nvidia.com>
> Cc: Kirti Wankhede <kwankhede@nvidia.com>
> 
> Signed-off-by: Yan Zhao <yan.y.zhao@intel.com>
> 
> ---
> v3:
> 1. renamed version to migration_version
> (Christophe de Dinechin, Cornelia Huck, Alex Williamson)
> 2. instead of generating migration version strings each time, storing
> them in vgpu types generated during initialization.
> (Zhenyu Wang, Cornelia Huck)
> 3. replaced multiple snprintf to one big snprintf in
> intel_gvt_get_vfio_migration_version()
> (Dr. David Alan Gilbert)
> 4. printed detailed error log
> (Alex Williamson, Erik Skultety, Cornelia Huck, Dr. David Alan Gilbert)
> 5. incorporated <software version> into migration_version string
> (Alex Williamson)
> 6. do not use ifndef macro to switch off migration_version attribute
> (Zhenyu Wang)
> 
> v2:
> 1. removed 32 common part of version string
> (Alex Williamson)
> 2. do not register version attribute for GVT not supporting live
> migration.(Cornelia Huck)
> 3. for platforms out of gen8, gen9, return -EINVAL --> -ENODEV for
> incompatible. (Cornelia Huck)
> ---
>  drivers/gpu/drm/i915/gvt/Makefile            |   2 +-
>  drivers/gpu/drm/i915/gvt/gvt.c               |  39 +++++
>  drivers/gpu/drm/i915/gvt/gvt.h               |   5 +
>  drivers/gpu/drm/i915/gvt/migration_version.c | 167 +++++++++++++++++++
>  drivers/gpu/drm/i915/gvt/vgpu.c              |  13 +-
>  5 files changed, 223 insertions(+), 3 deletions(-)
>  create mode 100644 drivers/gpu/drm/i915/gvt/migration_version.c
> 
> diff --git a/drivers/gpu/drm/i915/gvt/Makefile b/drivers/gpu/drm/i915/gvt/Makefile
> index 271fb46d4dd0..a9d561c93ab8 100644
> --- a/drivers/gpu/drm/i915/gvt/Makefile
> +++ b/drivers/gpu/drm/i915/gvt/Makefile
> @@ -3,7 +3,7 @@ GVT_DIR := gvt
>  GVT_SOURCE := gvt.o aperture_gm.o handlers.o vgpu.o trace_points.o firmware.o \
>  	interrupt.o gtt.o cfg_space.o opregion.o mmio.o display.o edid.o \
>  	execlist.o scheduler.o sched_policy.o mmio_context.o cmd_parser.o debugfs.o \
> -	fb_decoder.o dmabuf.o page_track.o
> +	fb_decoder.o dmabuf.o page_track.o migration_version.o
>  
>  ccflags-y				+= -I$(src) -I$(src)/$(GVT_DIR)
>  i915-y					+= $(addprefix $(GVT_DIR)/, $(GVT_SOURCE))
> diff --git a/drivers/gpu/drm/i915/gvt/gvt.c b/drivers/gpu/drm/i915/gvt/gvt.c
> index 43f4242062dd..be2980e8ac75 100644
> --- a/drivers/gpu/drm/i915/gvt/gvt.c
> +++ b/drivers/gpu/drm/i915/gvt/gvt.c
> @@ -105,14 +105,53 @@ static ssize_t description_show(struct kobject *kobj, struct device *dev,
>  		       type->weight);
>  }
>  
> +static ssize_t migration_version_show(struct kobject *kobj, struct device *dev,
> +		char *buf)
> +{
> +	struct intel_vgpu_type *type;
> +	void *gvt = kdev_to_i915(dev)->gvt;
> +
> +	type = intel_gvt_find_vgpu_type(gvt, kobject_name(kobj));
> +	if (!type || !type->migration_version) {
> +		gvt_err("Does not support migraion on type %s. Please search previous detailed log\n",
> +				kobject_name(kobj));
> +		return -ENODEV;
> +	}
> +
> +	return snprintf(buf, strlen(type->migration_version) + 2,
> +			"%s\n", type->migration_version);
> +}
> +
> +static ssize_t migration_version_store(struct kobject *kobj, struct device *dev,
> +		const char *buf, size_t count)
> +{
> +	int ret = 0;
> +	struct intel_vgpu_type *type;
> +	void *gvt = kdev_to_i915(dev)->gvt;
> +
> +	type = intel_gvt_find_vgpu_type(gvt, kobject_name(kobj));
> +	if (!type || !type->migration_version) {
> +		gvt_err("Does not support migraion on type %s. Please search previous detailed log\n",
> +				kobject_name(kobj));
> +		return -ENODEV;
> +	}
> +
> +	ret = intel_gvt_check_vfio_migration_version(gvt,
> +			type->migration_version, buf);
> +
> +	return (ret < 0 ? ret : count);
> +}
> +
>  static MDEV_TYPE_ATTR_RO(available_instances);
>  static MDEV_TYPE_ATTR_RO(device_api);
>  static MDEV_TYPE_ATTR_RO(description);
> +static MDEV_TYPE_ATTR_RW(migration_version);
>  
>  static struct attribute *gvt_type_attrs[] = {
>  	&mdev_type_attr_available_instances.attr,
>  	&mdev_type_attr_device_api.attr,
>  	&mdev_type_attr_description.attr,
> +	&mdev_type_attr_migration_version.attr,
>  	NULL,
>  };
>  
> diff --git a/drivers/gpu/drm/i915/gvt/gvt.h b/drivers/gpu/drm/i915/gvt/gvt.h
> index f5a328b5290a..265319d35c25 100644
> --- a/drivers/gpu/drm/i915/gvt/gvt.h
> +++ b/drivers/gpu/drm/i915/gvt/gvt.h
> @@ -296,6 +296,7 @@ struct intel_vgpu_type {
>  	unsigned int fence;
>  	unsigned int weight;
>  	enum intel_vgpu_edid resolution;
> +	char *migration_version;
>  };
>  
>  struct intel_gvt {
> @@ -687,6 +688,10 @@ void intel_gvt_debugfs_remove_vgpu(struct intel_vgpu *vgpu);
>  int intel_gvt_debugfs_init(struct intel_gvt *gvt);
>  void intel_gvt_debugfs_clean(struct intel_gvt *gvt);
>  
> +ssize_t intel_gvt_check_vfio_migration_version(struct intel_gvt *gvt,
> +		const char *self, const char *remote);
> +char *intel_gvt_get_vfio_migration_version(struct intel_gvt *gvt,
> +		const char *vgpu_type);
>  
>  #include "trace.h"
>  #include "mpt.h"
> diff --git a/drivers/gpu/drm/i915/gvt/migration_version.c b/drivers/gpu/drm/i915/gvt/migration_version.c
> new file mode 100644
> index 000000000000..e6acf188b29a
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/gvt/migration_version.c
> @@ -0,0 +1,167 @@
> +/*
> + * Copyright(c) 2011-2017 Intel Corporation. All rights reserved.
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the next
> + * paragraph) shall be included in all copies or substantial portions of the
> + * Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
> + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
> + * SOFTWARE.
> + *
> + * Authors:
> + *    Yan Zhao <yan.y.zhao@intel.com>
> + */
> +#include <linux/vfio.h>
> +#include "i915_drv.h"
> +#include "gvt.h"
> +
> +#define INV_SOFTWARE_VERSION (-1U)
> +#define VENDOR_ID_LEN (4)
> +#define DEVICE_ID_LEN (4)
> +#define VGPU_TYPE_LEN (16)
> +#define SOFTWARE_VER_LEN (8)
> +
> +/* total length of vfio migration version string.
> + * never exceed limit of PATH_MAX (4096)
> + */
> +#define MIGRATION_VERSION_TOTAL_LEN (VENDOR_ID_LEN + DEVICE_ID_LEN + \
> +					VGPU_TYPE_LEN + SOFTWARE_VER_LEN + 4)
> +
> +#define GVT_VFIO_MIGRATION_SOFTWARE_VERSION INV_SOFTWARE_VERSION
> +
> +
> +#define PRINTF_FORMAT "%04x-%04x-%s-%08x"
> +#define SCANF_FORMAT "%x-%x-%16[^-]-%x"
> +
> +enum incompatible_reason {
> +	IREASON_WRONG_REMOTE_FORMAT = 0,
> +	IREASON_HARDWARE_MISMATCH,
> +	IREASON_SOFTWARE_VERSION_MISMATCH,
> +	IREASON_VGPU_TYPE_MISMATCH,
> +};
> +
> +static const char *const incompatible_reason_str[] = {
> +	[IREASON_WRONG_REMOTE_FORMAT] =
> +		"wrong string format. probably wrong GVT version",
> +	[IREASON_HARDWARE_MISMATCH] =
> +		"physical device not matched",
> +	[IREASON_SOFTWARE_VERSION_MISMATCH] =
> +		"migration software version not matched",
> +	[IREASON_VGPU_TYPE_MISMATCH] =
> +		"vgpu type not matched"
> +};
> +
> +static bool is_compatible(const char *local, const char *remote)
> +{
> +	bool ret;
> +
> +	ret = sysfs_streq(local, remote);
> +
> +	if (!ret) {
> +		int vid_l = 0, did_l = 0, vid_r = 0, did_r = 0;
> +		char type_l[VGPU_TYPE_LEN], type_r[VGPU_TYPE_LEN];
> +		u32 sv_l = 0, sv_r = 0;
> +		int rl = 0, rr = 0;
> +		enum incompatible_reason reason = IREASON_WRONG_REMOTE_FORMAT;
> +
> +		memset(type_l, 0, sizeof(type_l));
> +		memset(type_r, 0, sizeof(type_r));
> +
> +		rl = sscanf(local, SCANF_FORMAT,
> +				&vid_l, &did_l, type_l, &sv_l);
> +		rr = sscanf(remote, SCANF_FORMAT,
> +				&vid_r, &did_r, type_r, &sv_r);
> +
> +		if (rl == rr) {
> +			if (vid_l != vid_r || did_l != did_r)
> +				reason = IREASON_HARDWARE_MISMATCH;
> +			else if (sv_l != sv_r)
> +				reason = IREASON_SOFTWARE_VERSION_MISMATCH;
> +			else if (strncmp(type_l, type_r, VGPU_TYPE_LEN))
> +				reason = IREASON_VGPU_TYPE_MISMATCH;
> +		}
> +
> +		gvt_err("Migration version mismatched. Possible reason: %s. Local migration version:%s, Remote migration version:%s\n",
> +				incompatible_reason_str[reason], local,	remote);
> +
> +	}
> +	return ret;
> +
> +}
> +
> +
> +char *
> +intel_gvt_get_vfio_migration_version(struct intel_gvt *gvt,
> +		const char *vgpu_type)
> +{
> +	int cnt = 0;
> +	struct drm_i915_private *dev_priv = gvt->dev_priv;
> +	char *version = NULL;
> +
> +	/* currently only gen8 & gen9 are supported */
> +	if (!IS_GEN(dev_priv, 8) && !IS_GEN(dev_priv, 9)) {
> +		gvt_err("Local hardware does not support migration on %d\n",
> +				INTEL_INFO(dev_priv)->gen);
> +		return NULL;
> +	}
> +
> +	if (GVT_VFIO_MIGRATION_SOFTWARE_VERSION == INV_SOFTWARE_VERSION) {
> +		gvt_err("Local GVT does not support migration\n");
> +		return NULL;
> +	}
> +
> +	version = kzalloc(MIGRATION_VERSION_TOTAL_LEN, GFP_KERNEL);
> +
> +	if (unlikely(!version)) {
> +		gvt_err("memory allocation failed when get local migraiton version\n");
> +		return NULL;
> +	}
> +
> +	/* vendor id + device id + vgpu type + software version */
> +	cnt = snprintf(version, MIGRATION_VERSION_TOTAL_LEN, PRINTF_FORMAT,
> +			PCI_VENDOR_ID_INTEL,
> +			INTEL_DEVID(dev_priv),
> +			vgpu_type,
> +			GVT_VFIO_MIGRATION_SOFTWARE_VERSION);
> +
> +	if (cnt)
> +		return version;
> +
> +	gvt_err("string generation failed when get local migration version\n");
> +	return NULL;

Handle version kfree for possible error here. The approach looks fine to me.
I think some of those messages might be too chatty or maybe debug message instead.
Others are good to me.

Acked-by: Zhenyu Wang <zhenyuw@linux.intel.com>

> +}
> +
> +ssize_t intel_gvt_check_vfio_migration_version(struct intel_gvt *gvt,
> +		const char *self, const char *remote)
> +{
> +	struct drm_i915_private *dev_priv = gvt->dev_priv;
> +
> +	/* currently only gen8 & gen9 are supported */
> +	if (!IS_GEN(dev_priv, 8) && !IS_GEN(dev_priv, 9)) {
> +		gvt_err("Local hardware does not support migration on %d\n",
> +				INTEL_INFO(dev_priv)->gen);
> +		return -ENODEV;
> +	}
> +
> +	if (GVT_VFIO_MIGRATION_SOFTWARE_VERSION == INV_SOFTWARE_VERSION) {
> +		gvt_err("Local GVT does not support migration\n");
> +		return -ENODEV;
> +	}
> +
> +	if (!is_compatible(self, remote))
> +		return -EINVAL;
> +
> +	return 0;
> +}
> diff --git a/drivers/gpu/drm/i915/gvt/vgpu.c b/drivers/gpu/drm/i915/gvt/vgpu.c
> index 44ce3c2b9ac1..7642b21641bd 100644
> --- a/drivers/gpu/drm/i915/gvt/vgpu.c
> +++ b/drivers/gpu/drm/i915/gvt/vgpu.c
> @@ -155,13 +155,18 @@ int intel_gvt_init_vgpu_types(struct intel_gvt *gvt)
>  			sprintf(gvt->types[i].name, "GVTg_V5_%s",
>  						vgpu_types[i].name);
>  
> -		gvt_dbg_core("type[%d]: %s avail %u low %u high %u fence %u weight %u res %s\n",
> +		gvt->types[i].migration_version =
> +			intel_gvt_get_vfio_migration_version(gvt,
> +					gvt->types[i].name);
> +		gvt_dbg_core("type[%d]: %s avail %u low %u high %u fence %u weight %u res %s, migratio_version:%s\n",
>  			     i, gvt->types[i].name,
>  			     gvt->types[i].avail_instance,
>  			     gvt->types[i].low_gm_size,
>  			     gvt->types[i].high_gm_size, gvt->types[i].fence,
>  			     gvt->types[i].weight,
> -			     vgpu_edid_str(gvt->types[i].resolution));
> +			     vgpu_edid_str(gvt->types[i].resolution),
> +			     (gvt->types[i].migration_version ?
> +			     gvt->types[i].migration_version : "null"));
>  	}
>  
>  	gvt->num_types = i;
> @@ -170,6 +175,10 @@ int intel_gvt_init_vgpu_types(struct intel_gvt *gvt)
>  
>  void intel_gvt_clean_vgpu_types(struct intel_gvt *gvt)
>  {
> +	int i;
> +
> +	for (i = 0; i < gvt->num_types; i++)
> +		kfree(gvt->types[i].migration_version);
>  	kfree(gvt->types);
>  }
>  
> -- 
> 2.17.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 --]

Re: [PATCH v3 2/2] drm/i915/gvt: export migration_version to mdev sysfs for Intel vGPU

[Yan Zhao]

On Wed, May 29, 2019 at 11:07:50AM +0800, Zhenyu Wang wrote:
> On 2019.05.26 23:44:37 -0400, Yan Zhao wrote:
> > This feature implements the migration_version attribute for Intel's vGPU
> > mdev devices.
> > 
> > migration_version attribute is rw.
> > It's used to check migration compatibility for two mdev devices of the
> > same mdev type.
> > migration_version string is defined by vendor driver and opaque to
> > userspace.
> > 
> > For Intel vGPU of gen8 and gen9, the format of migration_version string
> > is:
> >   <vendor id>-<device id>-<vgpu type>-<software version>.
> > 
> > For future platforms, the format of migration_version string is to be
> > expanded to include more meta data to identify Intel vGPUs for live
> > migration compatibility check
> > 
> > For old platforms, and for GVT not supporting vGPU live migration
> > feature, -ENODEV is returned on read(2)/write(2) of migration_version
> > attribute.
> > For vGPUs running old GVT who do not expose migration_version
> > attribute, live migration is regarded as not supported for those vGPUs.
> > 
> > Cc: Alex Williamson <alex.williamson@redhat.com>
> > Cc: Erik Skultety <eskultet@redhat.com>
> > Cc: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> > Cc: Cornelia Huck <cohuck@redhat.com>
> > Cc: "Tian, Kevin" <kevin.tian@intel.com>
> > Cc: Zhenyu Wang <zhenyuw@linux.intel.com>
> > Cc: "Wang, Zhi A" <zhi.a.wang@intel.com>
> > c: Neo Jia <cjia@nvidia.com>
> > Cc: Kirti Wankhede <kwankhede@nvidia.com>
> > 
> > Signed-off-by: Yan Zhao <yan.y.zhao@intel.com>
> > 
> > ---
> > v3:
> > 1. renamed version to migration_version
> > (Christophe de Dinechin, Cornelia Huck, Alex Williamson)
> > 2. instead of generating migration version strings each time, storing
> > them in vgpu types generated during initialization.
> > (Zhenyu Wang, Cornelia Huck)
> > 3. replaced multiple snprintf to one big snprintf in
> > intel_gvt_get_vfio_migration_version()
> > (Dr. David Alan Gilbert)
> > 4. printed detailed error log
> > (Alex Williamson, Erik Skultety, Cornelia Huck, Dr. David Alan Gilbert)
> > 5. incorporated <software version> into migration_version string
> > (Alex Williamson)
> > 6. do not use ifndef macro to switch off migration_version attribute
> > (Zhenyu Wang)
> > 
> > v2:
> > 1. removed 32 common part of version string
> > (Alex Williamson)
> > 2. do not register version attribute for GVT not supporting live
> > migration.(Cornelia Huck)
> > 3. for platforms out of gen8, gen9, return -EINVAL --> -ENODEV for
> > incompatible. (Cornelia Huck)
> > ---
> >  drivers/gpu/drm/i915/gvt/Makefile            |   2 +-
> >  drivers/gpu/drm/i915/gvt/gvt.c               |  39 +++++
> >  drivers/gpu/drm/i915/gvt/gvt.h               |   5 +
> >  drivers/gpu/drm/i915/gvt/migration_version.c | 167 +++++++++++++++++++
> >  drivers/gpu/drm/i915/gvt/vgpu.c              |  13 +-
> >  5 files changed, 223 insertions(+), 3 deletions(-)
> >  create mode 100644 drivers/gpu/drm/i915/gvt/migration_version.c
> > 
> > diff --git a/drivers/gpu/drm/i915/gvt/Makefile b/drivers/gpu/drm/i915/gvt/Makefile
> > index 271fb46d4dd0..a9d561c93ab8 100644
> > --- a/drivers/gpu/drm/i915/gvt/Makefile
> > +++ b/drivers/gpu/drm/i915/gvt/Makefile
> > @@ -3,7 +3,7 @@ GVT_DIR := gvt
> >  GVT_SOURCE := gvt.o aperture_gm.o handlers.o vgpu.o trace_points.o firmware.o \
> >  	interrupt.o gtt.o cfg_space.o opregion.o mmio.o display.o edid.o \
> >  	execlist.o scheduler.o sched_policy.o mmio_context.o cmd_parser.o debugfs.o \
> > -	fb_decoder.o dmabuf.o page_track.o
> > +	fb_decoder.o dmabuf.o page_track.o migration_version.o
> >  
> >  ccflags-y				+= -I$(src) -I$(src)/$(GVT_DIR)
> >  i915-y					+= $(addprefix $(GVT_DIR)/, $(GVT_SOURCE))
> > diff --git a/drivers/gpu/drm/i915/gvt/gvt.c b/drivers/gpu/drm/i915/gvt/gvt.c
> > index 43f4242062dd..be2980e8ac75 100644
> > --- a/drivers/gpu/drm/i915/gvt/gvt.c
> > +++ b/drivers/gpu/drm/i915/gvt/gvt.c
> > @@ -105,14 +105,53 @@ static ssize_t description_show(struct kobject *kobj, struct device *dev,
> >  		       type->weight);
> >  }
> >  
> > +static ssize_t migration_version_show(struct kobject *kobj, struct device *dev,
> > +		char *buf)
> > +{
> > +	struct intel_vgpu_type *type;
> > +	void *gvt = kdev_to_i915(dev)->gvt;
> > +
> > +	type = intel_gvt_find_vgpu_type(gvt, kobject_name(kobj));
> > +	if (!type || !type->migration_version) {
> > +		gvt_err("Does not support migraion on type %s. Please search previous detailed log\n",
> > +				kobject_name(kobj));
> > +		return -ENODEV;
> > +	}
> > +
> > +	return snprintf(buf, strlen(type->migration_version) + 2,
> > +			"%s\n", type->migration_version);
> > +}
> > +
> > +static ssize_t migration_version_store(struct kobject *kobj, struct device *dev,
> > +		const char *buf, size_t count)
> > +{
> > +	int ret = 0;
> > +	struct intel_vgpu_type *type;
> > +	void *gvt = kdev_to_i915(dev)->gvt;
> > +
> > +	type = intel_gvt_find_vgpu_type(gvt, kobject_name(kobj));
> > +	if (!type || !type->migration_version) {
> > +		gvt_err("Does not support migraion on type %s. Please search previous detailed log\n",
> > +				kobject_name(kobj));
> > +		return -ENODEV;
> > +	}
> > +
> > +	ret = intel_gvt_check_vfio_migration_version(gvt,
> > +			type->migration_version, buf);
> > +
> > +	return (ret < 0 ? ret : count);
> > +}
> > +
> >  static MDEV_TYPE_ATTR_RO(available_instances);
> >  static MDEV_TYPE_ATTR_RO(device_api);
> >  static MDEV_TYPE_ATTR_RO(description);
> > +static MDEV_TYPE_ATTR_RW(migration_version);
> >  
> >  static struct attribute *gvt_type_attrs[] = {
> >  	&mdev_type_attr_available_instances.attr,
> >  	&mdev_type_attr_device_api.attr,
> >  	&mdev_type_attr_description.attr,
> > +	&mdev_type_attr_migration_version.attr,
> >  	NULL,
> >  };
> >  
> > diff --git a/drivers/gpu/drm/i915/gvt/gvt.h b/drivers/gpu/drm/i915/gvt/gvt.h
> > index f5a328b5290a..265319d35c25 100644
> > --- a/drivers/gpu/drm/i915/gvt/gvt.h
> > +++ b/drivers/gpu/drm/i915/gvt/gvt.h
> > @@ -296,6 +296,7 @@ struct intel_vgpu_type {
> >  	unsigned int fence;
> >  	unsigned int weight;
> >  	enum intel_vgpu_edid resolution;
> > +	char *migration_version;
> >  };
> >  
> >  struct intel_gvt {
> > @@ -687,6 +688,10 @@ void intel_gvt_debugfs_remove_vgpu(struct intel_vgpu *vgpu);
> >  int intel_gvt_debugfs_init(struct intel_gvt *gvt);
> >  void intel_gvt_debugfs_clean(struct intel_gvt *gvt);
> >  
> > +ssize_t intel_gvt_check_vfio_migration_version(struct intel_gvt *gvt,
> > +		const char *self, const char *remote);
> > +char *intel_gvt_get_vfio_migration_version(struct intel_gvt *gvt,
> > +		const char *vgpu_type);
> >  
> >  #include "trace.h"
> >  #include "mpt.h"
> > diff --git a/drivers/gpu/drm/i915/gvt/migration_version.c b/drivers/gpu/drm/i915/gvt/migration_version.c
> > new file mode 100644
> > index 000000000000..e6acf188b29a
> > --- /dev/null
> > +++ b/drivers/gpu/drm/i915/gvt/migration_version.c
> > @@ -0,0 +1,167 @@
> > +/*
> > + * Copyright(c) 2011-2017 Intel Corporation. All rights reserved.
> > + *
> > + * Permission is hereby granted, free of charge, to any person obtaining a
> > + * copy of this software and associated documentation files (the "Software"),
> > + * to deal in the Software without restriction, including without limitation
> > + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> > + * and/or sell copies of the Software, and to permit persons to whom the
> > + * Software is furnished to do so, subject to the following conditions:
> > + *
> > + * The above copyright notice and this permission notice (including the next
> > + * paragraph) shall be included in all copies or substantial portions of the
> > + * Software.
> > + *
> > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> > + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> > + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
> > + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> > + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
> > + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
> > + * SOFTWARE.
> > + *
> > + * Authors:
> > + *    Yan Zhao <yan.y.zhao@intel.com>
> > + */
> > +#include <linux/vfio.h>
> > +#include "i915_drv.h"
> > +#include "gvt.h"
> > +
> > +#define INV_SOFTWARE_VERSION (-1U)
> > +#define VENDOR_ID_LEN (4)
> > +#define DEVICE_ID_LEN (4)
> > +#define VGPU_TYPE_LEN (16)
> > +#define SOFTWARE_VER_LEN (8)
> > +
> > +/* total length of vfio migration version string.
> > + * never exceed limit of PATH_MAX (4096)
> > + */
> > +#define MIGRATION_VERSION_TOTAL_LEN (VENDOR_ID_LEN + DEVICE_ID_LEN + \
> > +					VGPU_TYPE_LEN + SOFTWARE_VER_LEN + 4)
> > +
> > +#define GVT_VFIO_MIGRATION_SOFTWARE_VERSION INV_SOFTWARE_VERSION
> > +
> > +
> > +#define PRINTF_FORMAT "%04x-%04x-%s-%08x"
> > +#define SCANF_FORMAT "%x-%x-%16[^-]-%x"
> > +
> > +enum incompatible_reason {
> > +	IREASON_WRONG_REMOTE_FORMAT = 0,
> > +	IREASON_HARDWARE_MISMATCH,
> > +	IREASON_SOFTWARE_VERSION_MISMATCH,
> > +	IREASON_VGPU_TYPE_MISMATCH,
> > +};
> > +
> > +static const char *const incompatible_reason_str[] = {
> > +	[IREASON_WRONG_REMOTE_FORMAT] =
> > +		"wrong string format. probably wrong GVT version",
> > +	[IREASON_HARDWARE_MISMATCH] =
> > +		"physical device not matched",
> > +	[IREASON_SOFTWARE_VERSION_MISMATCH] =
> > +		"migration software version not matched",
> > +	[IREASON_VGPU_TYPE_MISMATCH] =
> > +		"vgpu type not matched"
> > +};
> > +
> > +static bool is_compatible(const char *local, const char *remote)
> > +{
> > +	bool ret;
> > +
> > +	ret = sysfs_streq(local, remote);
> > +
> > +	if (!ret) {
> > +		int vid_l = 0, did_l = 0, vid_r = 0, did_r = 0;
> > +		char type_l[VGPU_TYPE_LEN], type_r[VGPU_TYPE_LEN];
> > +		u32 sv_l = 0, sv_r = 0;
> > +		int rl = 0, rr = 0;
> > +		enum incompatible_reason reason = IREASON_WRONG_REMOTE_FORMAT;
> > +
> > +		memset(type_l, 0, sizeof(type_l));
> > +		memset(type_r, 0, sizeof(type_r));
> > +
> > +		rl = sscanf(local, SCANF_FORMAT,
> > +				&vid_l, &did_l, type_l, &sv_l);
> > +		rr = sscanf(remote, SCANF_FORMAT,
> > +				&vid_r, &did_r, type_r, &sv_r);
> > +
> > +		if (rl == rr) {
> > +			if (vid_l != vid_r || did_l != did_r)
> > +				reason = IREASON_HARDWARE_MISMATCH;
> > +			else if (sv_l != sv_r)
> > +				reason = IREASON_SOFTWARE_VERSION_MISMATCH;
> > +			else if (strncmp(type_l, type_r, VGPU_TYPE_LEN))
> > +				reason = IREASON_VGPU_TYPE_MISMATCH;
> > +		}
> > +
> > +		gvt_err("Migration version mismatched. Possible reason: %s. Local migration version:%s, Remote migration version:%s\n",
> > +				incompatible_reason_str[reason], local,	remote);
> > +
> > +	}
> > +	return ret;
> > +
> > +}
> > +
> > +
> > +char *
> > +intel_gvt_get_vfio_migration_version(struct intel_gvt *gvt,
> > +		const char *vgpu_type)
> > +{
> > +	int cnt = 0;
> > +	struct drm_i915_private *dev_priv = gvt->dev_priv;
> > +	char *version = NULL;
> > +
> > +	/* currently only gen8 & gen9 are supported */
> > +	if (!IS_GEN(dev_priv, 8) && !IS_GEN(dev_priv, 9)) {
> > +		gvt_err("Local hardware does not support migration on %d\n",
> > +				INTEL_INFO(dev_priv)->gen);
> > +		return NULL;
> > +	}
> > +
> > +	if (GVT_VFIO_MIGRATION_SOFTWARE_VERSION == INV_SOFTWARE_VERSION) {
> > +		gvt_err("Local GVT does not support migration\n");
> > +		return NULL;
> > +	}
> > +
> > +	version = kzalloc(MIGRATION_VERSION_TOTAL_LEN, GFP_KERNEL);
> > +
> > +	if (unlikely(!version)) {
> > +		gvt_err("memory allocation failed when get local migraiton version\n");
> > +		return NULL;
> > +	}
> > +
> > +	/* vendor id + device id + vgpu type + software version */
> > +	cnt = snprintf(version, MIGRATION_VERSION_TOTAL_LEN, PRINTF_FORMAT,
> > +			PCI_VENDOR_ID_INTEL,
> > +			INTEL_DEVID(dev_priv),
> > +			vgpu_type,
> > +			GVT_VFIO_MIGRATION_SOFTWARE_VERSION);
> > +
> > +	if (cnt)
> > +		return version;
> > +
> > +	gvt_err("string generation failed when get local migration version\n");
> > +	return NULL;
> 
> Handle version kfree for possible error here. The approach looks fine to me.
> I think some of those messages might be too chatty or maybe debug message instead.
> Others are good to me.
> 
> Acked-by: Zhenyu Wang <zhenyuw@linux.intel.com>
> 
Got it. will add the kfree in next revision.
But debug message is only available for debug version of driver? If that's true,
maybe keep it printed also for non-debug version is better? as it is intended to
be checked by userspace software to see what's going wrong on migration
compatibility test.
Thank you:)

> > +}
> > +
> > +ssize_t intel_gvt_check_vfio_migration_version(struct intel_gvt *gvt,
> > +		const char *self, const char *remote)
> > +{
> > +	struct drm_i915_private *dev_priv = gvt->dev_priv;
> > +
> > +	/* currently only gen8 & gen9 are supported */
> > +	if (!IS_GEN(dev_priv, 8) && !IS_GEN(dev_priv, 9)) {
> > +		gvt_err("Local hardware does not support migration on %d\n",
> > +				INTEL_INFO(dev_priv)->gen);
> > +		return -ENODEV;
> > +	}
> > +
> > +	if (GVT_VFIO_MIGRATION_SOFTWARE_VERSION == INV_SOFTWARE_VERSION) {
> > +		gvt_err("Local GVT does not support migration\n");
> > +		return -ENODEV;
> > +	}
> > +
> > +	if (!is_compatible(self, remote))
> > +		return -EINVAL;
> > +
> > +	return 0;
> > +}
> > diff --git a/drivers/gpu/drm/i915/gvt/vgpu.c b/drivers/gpu/drm/i915/gvt/vgpu.c
> > index 44ce3c2b9ac1..7642b21641bd 100644
> > --- a/drivers/gpu/drm/i915/gvt/vgpu.c
> > +++ b/drivers/gpu/drm/i915/gvt/vgpu.c
> > @@ -155,13 +155,18 @@ int intel_gvt_init_vgpu_types(struct intel_gvt *gvt)
> >  			sprintf(gvt->types[i].name, "GVTg_V5_%s",
> >  						vgpu_types[i].name);
> >  
> > -		gvt_dbg_core("type[%d]: %s avail %u low %u high %u fence %u weight %u res %s\n",
> > +		gvt->types[i].migration_version =
> > +			intel_gvt_get_vfio_migration_version(gvt,
> > +					gvt->types[i].name);
> > +		gvt_dbg_core("type[%d]: %s avail %u low %u high %u fence %u weight %u res %s, migratio_version:%s\n",
> >  			     i, gvt->types[i].name,
> >  			     gvt->types[i].avail_instance,
> >  			     gvt->types[i].low_gm_size,
> >  			     gvt->types[i].high_gm_size, gvt->types[i].fence,
> >  			     gvt->types[i].weight,
> > -			     vgpu_edid_str(gvt->types[i].resolution));
> > +			     vgpu_edid_str(gvt->types[i].resolution),
> > +			     (gvt->types[i].migration_version ?
> > +			     gvt->types[i].migration_version : "null"));
> >  	}
> >  
> >  	gvt->num_types = i;
> > @@ -170,6 +175,10 @@ int intel_gvt_init_vgpu_types(struct intel_gvt *gvt)
> >  
> >  void intel_gvt_clean_vgpu_types(struct intel_gvt *gvt)
> >  {
> > +	int i;
> > +
> > +	for (i = 0; i < gvt->num_types; i++)
> > +		kfree(gvt->types[i].migration_version);
> >  	kfree(gvt->types);
> >  }
> >  
> > -- 
> > 2.17.1
> > 
> 
> -- 
> Open Source Technology Center, Intel ltd.
> 
> $gpg --keyserver wwwkeys.pgp.net --recv-keys 4D781827

Re: [Qemu-devel] [PATCH v3 2/2] drm/i915/gvt: export migration_version to mdev sysfs for Intel vGPU

[Yan Zhao]

On Wed, May 29, 2019 at 11:07:50AM +0800, Zhenyu Wang wrote:
> On 2019.05.26 23:44:37 -0400, Yan Zhao wrote:
> > This feature implements the migration_version attribute for Intel's vGPU
> > mdev devices.
> > 
> > migration_version attribute is rw.
> > It's used to check migration compatibility for two mdev devices of the
> > same mdev type.
> > migration_version string is defined by vendor driver and opaque to
> > userspace.
> > 
> > For Intel vGPU of gen8 and gen9, the format of migration_version string
> > is:
> >   <vendor id>-<device id>-<vgpu type>-<software version>.
> > 
> > For future platforms, the format of migration_version string is to be
> > expanded to include more meta data to identify Intel vGPUs for live
> > migration compatibility check
> > 
> > For old platforms, and for GVT not supporting vGPU live migration
> > feature, -ENODEV is returned on read(2)/write(2) of migration_version
> > attribute.
> > For vGPUs running old GVT who do not expose migration_version
> > attribute, live migration is regarded as not supported for those vGPUs.
> > 
> > Cc: Alex Williamson <alex.williamson@redhat.com>
> > Cc: Erik Skultety <eskultet@redhat.com>
> > Cc: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> > Cc: Cornelia Huck <cohuck@redhat.com>
> > Cc: "Tian, Kevin" <kevin.tian@intel.com>
> > Cc: Zhenyu Wang <zhenyuw@linux.intel.com>
> > Cc: "Wang, Zhi A" <zhi.a.wang@intel.com>
> > c: Neo Jia <cjia@nvidia.com>
> > Cc: Kirti Wankhede <kwankhede@nvidia.com>
> > 
> > Signed-off-by: Yan Zhao <yan.y.zhao@intel.com>
> > 
> > ---
> > v3:
> > 1. renamed version to migration_version
> > (Christophe de Dinechin, Cornelia Huck, Alex Williamson)
> > 2. instead of generating migration version strings each time, storing
> > them in vgpu types generated during initialization.
> > (Zhenyu Wang, Cornelia Huck)
> > 3. replaced multiple snprintf to one big snprintf in
> > intel_gvt_get_vfio_migration_version()
> > (Dr. David Alan Gilbert)
> > 4. printed detailed error log
> > (Alex Williamson, Erik Skultety, Cornelia Huck, Dr. David Alan Gilbert)
> > 5. incorporated <software version> into migration_version string
> > (Alex Williamson)
> > 6. do not use ifndef macro to switch off migration_version attribute
> > (Zhenyu Wang)
> > 
> > v2:
> > 1. removed 32 common part of version string
> > (Alex Williamson)
> > 2. do not register version attribute for GVT not supporting live
> > migration.(Cornelia Huck)
> > 3. for platforms out of gen8, gen9, return -EINVAL --> -ENODEV for
> > incompatible. (Cornelia Huck)
> > ---
> >  drivers/gpu/drm/i915/gvt/Makefile            |   2 +-
> >  drivers/gpu/drm/i915/gvt/gvt.c               |  39 +++++
> >  drivers/gpu/drm/i915/gvt/gvt.h               |   5 +
> >  drivers/gpu/drm/i915/gvt/migration_version.c | 167 +++++++++++++++++++
> >  drivers/gpu/drm/i915/gvt/vgpu.c              |  13 +-
> >  5 files changed, 223 insertions(+), 3 deletions(-)
> >  create mode 100644 drivers/gpu/drm/i915/gvt/migration_version.c
> > 
> > diff --git a/drivers/gpu/drm/i915/gvt/Makefile b/drivers/gpu/drm/i915/gvt/Makefile
> > index 271fb46d4dd0..a9d561c93ab8 100644
> > --- a/drivers/gpu/drm/i915/gvt/Makefile
> > +++ b/drivers/gpu/drm/i915/gvt/Makefile
> > @@ -3,7 +3,7 @@ GVT_DIR := gvt
> >  GVT_SOURCE := gvt.o aperture_gm.o handlers.o vgpu.o trace_points.o firmware.o \
> >  	interrupt.o gtt.o cfg_space.o opregion.o mmio.o display.o edid.o \
> >  	execlist.o scheduler.o sched_policy.o mmio_context.o cmd_parser.o debugfs.o \
> > -	fb_decoder.o dmabuf.o page_track.o
> > +	fb_decoder.o dmabuf.o page_track.o migration_version.o
> >  
> >  ccflags-y				+= -I$(src) -I$(src)/$(GVT_DIR)
> >  i915-y					+= $(addprefix $(GVT_DIR)/, $(GVT_SOURCE))
> > diff --git a/drivers/gpu/drm/i915/gvt/gvt.c b/drivers/gpu/drm/i915/gvt/gvt.c
> > index 43f4242062dd..be2980e8ac75 100644
> > --- a/drivers/gpu/drm/i915/gvt/gvt.c
> > +++ b/drivers/gpu/drm/i915/gvt/gvt.c
> > @@ -105,14 +105,53 @@ static ssize_t description_show(struct kobject *kobj, struct device *dev,
> >  		       type->weight);
> >  }
> >  
> > +static ssize_t migration_version_show(struct kobject *kobj, struct device *dev,
> > +		char *buf)
> > +{
> > +	struct intel_vgpu_type *type;
> > +	void *gvt = kdev_to_i915(dev)->gvt;
> > +
> > +	type = intel_gvt_find_vgpu_type(gvt, kobject_name(kobj));
> > +	if (!type || !type->migration_version) {
> > +		gvt_err("Does not support migraion on type %s. Please search previous detailed log\n",
> > +				kobject_name(kobj));
> > +		return -ENODEV;
> > +	}
> > +
> > +	return snprintf(buf, strlen(type->migration_version) + 2,
> > +			"%s\n", type->migration_version);
> > +}
> > +
> > +static ssize_t migration_version_store(struct kobject *kobj, struct device *dev,
> > +		const char *buf, size_t count)
> > +{
> > +	int ret = 0;
> > +	struct intel_vgpu_type *type;
> > +	void *gvt = kdev_to_i915(dev)->gvt;
> > +
> > +	type = intel_gvt_find_vgpu_type(gvt, kobject_name(kobj));
> > +	if (!type || !type->migration_version) {
> > +		gvt_err("Does not support migraion on type %s. Please search previous detailed log\n",
> > +				kobject_name(kobj));
> > +		return -ENODEV;
> > +	}
> > +
> > +	ret = intel_gvt_check_vfio_migration_version(gvt,
> > +			type->migration_version, buf);
> > +
> > +	return (ret < 0 ? ret : count);
> > +}
> > +
> >  static MDEV_TYPE_ATTR_RO(available_instances);
> >  static MDEV_TYPE_ATTR_RO(device_api);
> >  static MDEV_TYPE_ATTR_RO(description);
> > +static MDEV_TYPE_ATTR_RW(migration_version);
> >  
> >  static struct attribute *gvt_type_attrs[] = {
> >  	&mdev_type_attr_available_instances.attr,
> >  	&mdev_type_attr_device_api.attr,
> >  	&mdev_type_attr_description.attr,
> > +	&mdev_type_attr_migration_version.attr,
> >  	NULL,
> >  };
> >  
> > diff --git a/drivers/gpu/drm/i915/gvt/gvt.h b/drivers/gpu/drm/i915/gvt/gvt.h
> > index f5a328b5290a..265319d35c25 100644
> > --- a/drivers/gpu/drm/i915/gvt/gvt.h
> > +++ b/drivers/gpu/drm/i915/gvt/gvt.h
> > @@ -296,6 +296,7 @@ struct intel_vgpu_type {
> >  	unsigned int fence;
> >  	unsigned int weight;
> >  	enum intel_vgpu_edid resolution;
> > +	char *migration_version;
> >  };
> >  
> >  struct intel_gvt {
> > @@ -687,6 +688,10 @@ void intel_gvt_debugfs_remove_vgpu(struct intel_vgpu *vgpu);
> >  int intel_gvt_debugfs_init(struct intel_gvt *gvt);
> >  void intel_gvt_debugfs_clean(struct intel_gvt *gvt);
> >  
> > +ssize_t intel_gvt_check_vfio_migration_version(struct intel_gvt *gvt,
> > +		const char *self, const char *remote);
> > +char *intel_gvt_get_vfio_migration_version(struct intel_gvt *gvt,
> > +		const char *vgpu_type);
> >  
> >  #include "trace.h"
> >  #include "mpt.h"
> > diff --git a/drivers/gpu/drm/i915/gvt/migration_version.c b/drivers/gpu/drm/i915/gvt/migration_version.c
> > new file mode 100644
> > index 000000000000..e6acf188b29a
> > --- /dev/null
> > +++ b/drivers/gpu/drm/i915/gvt/migration_version.c
> > @@ -0,0 +1,167 @@
> > +/*
> > + * Copyright(c) 2011-2017 Intel Corporation. All rights reserved.
> > + *
> > + * Permission is hereby granted, free of charge, to any person obtaining a
> > + * copy of this software and associated documentation files (the "Software"),
> > + * to deal in the Software without restriction, including without limitation
> > + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> > + * and/or sell copies of the Software, and to permit persons to whom the
> > + * Software is furnished to do so, subject to the following conditions:
> > + *
> > + * The above copyright notice and this permission notice (including the next
> > + * paragraph) shall be included in all copies or substantial portions of the
> > + * Software.
> > + *
> > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> > + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> > + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
> > + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> > + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
> > + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
> > + * SOFTWARE.
> > + *
> > + * Authors:
> > + *    Yan Zhao <yan.y.zhao@intel.com>
> > + */
> > +#include <linux/vfio.h>
> > +#include "i915_drv.h"
> > +#include "gvt.h"
> > +
> > +#define INV_SOFTWARE_VERSION (-1U)
> > +#define VENDOR_ID_LEN (4)
> > +#define DEVICE_ID_LEN (4)
> > +#define VGPU_TYPE_LEN (16)
> > +#define SOFTWARE_VER_LEN (8)
> > +
> > +/* total length of vfio migration version string.
> > + * never exceed limit of PATH_MAX (4096)
> > + */
> > +#define MIGRATION_VERSION_TOTAL_LEN (VENDOR_ID_LEN + DEVICE_ID_LEN + \
> > +					VGPU_TYPE_LEN + SOFTWARE_VER_LEN + 4)
> > +
> > +#define GVT_VFIO_MIGRATION_SOFTWARE_VERSION INV_SOFTWARE_VERSION
> > +
> > +
> > +#define PRINTF_FORMAT "%04x-%04x-%s-%08x"
> > +#define SCANF_FORMAT "%x-%x-%16[^-]-%x"
> > +
> > +enum incompatible_reason {
> > +	IREASON_WRONG_REMOTE_FORMAT = 0,
> > +	IREASON_HARDWARE_MISMATCH,
> > +	IREASON_SOFTWARE_VERSION_MISMATCH,
> > +	IREASON_VGPU_TYPE_MISMATCH,
> > +};
> > +
> > +static const char *const incompatible_reason_str[] = {
> > +	[IREASON_WRONG_REMOTE_FORMAT] =
> > +		"wrong string format. probably wrong GVT version",
> > +	[IREASON_HARDWARE_MISMATCH] =
> > +		"physical device not matched",
> > +	[IREASON_SOFTWARE_VERSION_MISMATCH] =
> > +		"migration software version not matched",
> > +	[IREASON_VGPU_TYPE_MISMATCH] =
> > +		"vgpu type not matched"
> > +};
> > +
> > +static bool is_compatible(const char *local, const char *remote)
> > +{
> > +	bool ret;
> > +
> > +	ret = sysfs_streq(local, remote);
> > +
> > +	if (!ret) {
> > +		int vid_l = 0, did_l = 0, vid_r = 0, did_r = 0;
> > +		char type_l[VGPU_TYPE_LEN], type_r[VGPU_TYPE_LEN];
> > +		u32 sv_l = 0, sv_r = 0;
> > +		int rl = 0, rr = 0;
> > +		enum incompatible_reason reason = IREASON_WRONG_REMOTE_FORMAT;
> > +
> > +		memset(type_l, 0, sizeof(type_l));
> > +		memset(type_r, 0, sizeof(type_r));
> > +
> > +		rl = sscanf(local, SCANF_FORMAT,
> > +				&vid_l, &did_l, type_l, &sv_l);
> > +		rr = sscanf(remote, SCANF_FORMAT,
> > +				&vid_r, &did_r, type_r, &sv_r);
> > +
> > +		if (rl == rr) {
> > +			if (vid_l != vid_r || did_l != did_r)
> > +				reason = IREASON_HARDWARE_MISMATCH;
> > +			else if (sv_l != sv_r)
> > +				reason = IREASON_SOFTWARE_VERSION_MISMATCH;
> > +			else if (strncmp(type_l, type_r, VGPU_TYPE_LEN))
> > +				reason = IREASON_VGPU_TYPE_MISMATCH;
> > +		}
> > +
> > +		gvt_err("Migration version mismatched. Possible reason: %s. Local migration version:%s, Remote migration version:%s\n",
> > +				incompatible_reason_str[reason], local,	remote);
> > +
> > +	}
> > +	return ret;
> > +
> > +}
> > +
> > +
> > +char *
> > +intel_gvt_get_vfio_migration_version(struct intel_gvt *gvt,
> > +		const char *vgpu_type)
> > +{
> > +	int cnt = 0;
> > +	struct drm_i915_private *dev_priv = gvt->dev_priv;
> > +	char *version = NULL;
> > +
> > +	/* currently only gen8 & gen9 are supported */
> > +	if (!IS_GEN(dev_priv, 8) && !IS_GEN(dev_priv, 9)) {
> > +		gvt_err("Local hardware does not support migration on %d\n",
> > +				INTEL_INFO(dev_priv)->gen);
> > +		return NULL;
> > +	}
> > +
> > +	if (GVT_VFIO_MIGRATION_SOFTWARE_VERSION == INV_SOFTWARE_VERSION) {
> > +		gvt_err("Local GVT does not support migration\n");
> > +		return NULL;
> > +	}
> > +
> > +	version = kzalloc(MIGRATION_VERSION_TOTAL_LEN, GFP_KERNEL);
> > +
> > +	if (unlikely(!version)) {
> > +		gvt_err("memory allocation failed when get local migraiton version\n");
> > +		return NULL;
> > +	}
> > +
> > +	/* vendor id + device id + vgpu type + software version */
> > +	cnt = snprintf(version, MIGRATION_VERSION_TOTAL_LEN, PRINTF_FORMAT,
> > +			PCI_VENDOR_ID_INTEL,
> > +			INTEL_DEVID(dev_priv),
> > +			vgpu_type,
> > +			GVT_VFIO_MIGRATION_SOFTWARE_VERSION);
> > +
> > +	if (cnt)
> > +		return version;
> > +
> > +	gvt_err("string generation failed when get local migration version\n");
> > +	return NULL;
> 
> Handle version kfree for possible error here. The approach looks fine to me.
> I think some of those messages might be too chatty or maybe debug message instead.
> Others are good to me.
> 
> Acked-by: Zhenyu Wang <zhenyuw@linux.intel.com>
> 
Got it. will add the kfree in next revision.
But debug message is only available for debug version of driver? If that's true,
maybe keep it printed also for non-debug version is better? as it is intended to
be checked by userspace software to see what's going wrong on migration
compatibility test.
Thank you:)

> > +}
> > +
> > +ssize_t intel_gvt_check_vfio_migration_version(struct intel_gvt *gvt,
> > +		const char *self, const char *remote)
> > +{
> > +	struct drm_i915_private *dev_priv = gvt->dev_priv;
> > +
> > +	/* currently only gen8 & gen9 are supported */
> > +	if (!IS_GEN(dev_priv, 8) && !IS_GEN(dev_priv, 9)) {
> > +		gvt_err("Local hardware does not support migration on %d\n",
> > +				INTEL_INFO(dev_priv)->gen);
> > +		return -ENODEV;
> > +	}
> > +
> > +	if (GVT_VFIO_MIGRATION_SOFTWARE_VERSION == INV_SOFTWARE_VERSION) {
> > +		gvt_err("Local GVT does not support migration\n");
> > +		return -ENODEV;
> > +	}
> > +
> > +	if (!is_compatible(self, remote))
> > +		return -EINVAL;
> > +
> > +	return 0;
> > +}
> > diff --git a/drivers/gpu/drm/i915/gvt/vgpu.c b/drivers/gpu/drm/i915/gvt/vgpu.c
> > index 44ce3c2b9ac1..7642b21641bd 100644
> > --- a/drivers/gpu/drm/i915/gvt/vgpu.c
> > +++ b/drivers/gpu/drm/i915/gvt/vgpu.c
> > @@ -155,13 +155,18 @@ int intel_gvt_init_vgpu_types(struct intel_gvt *gvt)
> >  			sprintf(gvt->types[i].name, "GVTg_V5_%s",
> >  						vgpu_types[i].name);
> >  
> > -		gvt_dbg_core("type[%d]: %s avail %u low %u high %u fence %u weight %u res %s\n",
> > +		gvt->types[i].migration_version =
> > +			intel_gvt_get_vfio_migration_version(gvt,
> > +					gvt->types[i].name);
> > +		gvt_dbg_core("type[%d]: %s avail %u low %u high %u fence %u weight %u res %s, migratio_version:%s\n",
> >  			     i, gvt->types[i].name,
> >  			     gvt->types[i].avail_instance,
> >  			     gvt->types[i].low_gm_size,
> >  			     gvt->types[i].high_gm_size, gvt->types[i].fence,
> >  			     gvt->types[i].weight,
> > -			     vgpu_edid_str(gvt->types[i].resolution));
> > +			     vgpu_edid_str(gvt->types[i].resolution),
> > +			     (gvt->types[i].migration_version ?
> > +			     gvt->types[i].migration_version : "null"));
> >  	}
> >  
> >  	gvt->num_types = i;
> > @@ -170,6 +175,10 @@ int intel_gvt_init_vgpu_types(struct intel_gvt *gvt)
> >  
> >  void intel_gvt_clean_vgpu_types(struct intel_gvt *gvt)
> >  {
> > +	int i;
> > +
> > +	for (i = 0; i < gvt->num_types; i++)
> > +		kfree(gvt->types[i].migration_version);
> >  	kfree(gvt->types);
> >  }
> >  
> > -- 
> > 2.17.1
> > 
> 
> -- 
> Open Source Technology Center, Intel ltd.
> 
> $gpg --keyserver wwwkeys.pgp.net --recv-keys 4D781827