[v2,2/2] drm/i915/gvt: export mdev device version to sysfs for Intel vGPU

Commit Message

Yan Zhao May 6, 2019, 1:51 a.m. UTC

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

version attribute is rw.
It's used to check device compatibility for two mdev devices.
version string format and length are private for vendor driver. vendor
driver is able to define them freely.

For Intel vGPU of gen8 and gen9, the mdev device version
consists of 3 fields: "vendor id" + "device id" + "mdev type".

Reading from a vGPU's version attribute, a string is returned in below
format: <vendor id>-<device id>-<mdev type>. e.g.
8086-193b-i915-GVTg_V5_2.

Writing a string to a vGPU's version attribute will trigger GVT to check
whether a vGPU identified by the written string is compatible with
current vGPU owning this version attribute. errno is returned if the two
vGPUs are incompatible. The length of written string is returned in
compatible case.

For other platforms, and for GVT not supporting vGPU live migration
feature, errnos are returned when read/write of mdev devices' version
attributes.

For old GVT versions where no version attributes exposed in sysfs, it is
regarded as not supporting vGPU live migration.

For future platforms, besides the current 2 fields in vendor proprietary
part, more fields may be added to identify Intel vGPU well for live
migration purpose.

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)

Cc: Alex Williamson <[e-mail]>
Cc: Erik Skultety <[e-mail]>
Cc: "Dr. David Alan Gilbert" <[e-mail]>
Cc: Cornelia Huck <[e-mail]>
Cc: "Tian, Kevin" <[e-mail]>
Cc: Zhenyu Wang <[e-mail]>
Cc: "Wang, Zhi A" <[e-mail]>
c: Neo Jia <[e-mail]>
Cc: Kirti Wankhede <[e-mail]>

Signed-off-by: Yan Zhao <[e-mail]>
---
 drivers/gpu/drm/i915/gvt/Makefile         |  2 +-
 drivers/gpu/drm/i915/gvt/device_version.c | 87 +++++++++++++++++++++++
 drivers/gpu/drm/i915/gvt/gvt.c            | 51 +++++++++++++
 drivers/gpu/drm/i915/gvt/gvt.h            |  6 ++
 4 files changed, 145 insertions(+), 1 deletion(-)
 create mode 100644 drivers/gpu/drm/i915/gvt/device_version.c

Comments

Zhenyu Wang May 6, 2019, 3:20 a.m. UTC | #1

On 2019.05.05 21:51:02 -0400, Yan Zhao wrote:
> This feature implements the version attribute for Intel's vGPU mdev
> devices.
> 
> version attribute is rw.
> It's used to check device compatibility for two mdev devices.
> version string format and length are private for vendor driver. vendor
> driver is able to define them freely.
> 
> For Intel vGPU of gen8 and gen9, the mdev device version
> consists of 3 fields: "vendor id" + "device id" + "mdev type".
> 
> Reading from a vGPU's version attribute, a string is returned in below
> format: <vendor id>-<device id>-<mdev type>. e.g.
> 8086-193b-i915-GVTg_V5_2.
> 
> Writing a string to a vGPU's version attribute will trigger GVT to check
> whether a vGPU identified by the written string is compatible with
> current vGPU owning this version attribute. errno is returned if the two
> vGPUs are incompatible. The length of written string is returned in
> compatible case.
> 
> For other platforms, and for GVT not supporting vGPU live migration
> feature, errnos are returned when read/write of mdev devices' version
> attributes.
> 
> For old GVT versions where no version attributes exposed in sysfs, it is
> regarded as not supporting vGPU live migration.
> 
> For future platforms, besides the current 2 fields in vendor proprietary
> part, more fields may be added to identify Intel vGPU well for live
> migration purpose.
> 
> 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)
> 
> Cc: Alex Williamson <[e-mail]>
> Cc: Erik Skultety <[e-mail]>
> Cc: "Dr. David Alan Gilbert" <[e-mail]>
> Cc: Cornelia Huck <[e-mail]>
> Cc: "Tian, Kevin" <[e-mail]>
> Cc: Zhenyu Wang <[e-mail]>
> Cc: "Wang, Zhi A" <[e-mail]>
> c: Neo Jia <[e-mail]>
> Cc: Kirti Wankhede <[e-mail]>
> 
> Signed-off-by: Yan Zhao <[e-mail]>
> ---
>  drivers/gpu/drm/i915/gvt/Makefile         |  2 +-
>  drivers/gpu/drm/i915/gvt/device_version.c | 87 +++++++++++++++++++++++
>  drivers/gpu/drm/i915/gvt/gvt.c            | 51 +++++++++++++
>  drivers/gpu/drm/i915/gvt/gvt.h            |  6 ++
>  4 files changed, 145 insertions(+), 1 deletion(-)
>  create mode 100644 drivers/gpu/drm/i915/gvt/device_version.c
> 
> diff --git a/drivers/gpu/drm/i915/gvt/Makefile b/drivers/gpu/drm/i915/gvt/Makefile
> index 271fb46d4dd0..54e209a23899 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 device_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/device_version.c b/drivers/gpu/drm/i915/gvt/device_version.c
> new file mode 100644
> index 000000000000..bd4cdcbdba95
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/gvt/device_version.c
> @@ -0,0 +1,87 @@
> +/*
> + * 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 <[e-mail]>
> + */
> +#include <linux/vfio.h>
> +#include "i915_drv.h"
> +
> +static bool is_compatible(const char *self, const char *remote)
> +{
> +	if (strlen(remote) != strlen(self))
> +		return false;
> +
> +	return (strncmp(self, remote, strlen(self))) ? false : true;
> +}
> +
> +ssize_t intel_gvt_get_vfio_device_version_len(struct drm_i915_private *dev_priv)
> +{
> +	if (!IS_GEN(dev_priv, 8) && !IS_GEN(dev_priv, 9))
> +		return -ENODEV;
> +
> +	return PAGE_SIZE;
> +}
> +
> +ssize_t intel_gvt_get_vfio_device_version(struct drm_i915_private *dev_priv,
> +		char *buf, const char *mdev_type)
> +{
> +	int cnt = 0, ret = 0;
> +	const char *str = NULL;
> +

> +	/* currently only gen8 & gen9 are supported */
> +	if (!IS_GEN(dev_priv, 8) && !IS_GEN(dev_priv, 9))
> +		return -ENODEV;
> +
> +	/* vendor id + device id + mdev type */
> +	/* vendor id */
> +	cnt = snprintf(buf, 5, "%04x", PCI_VENDOR_ID_INTEL);
> +	buf += cnt;
> +	ret += cnt;
> +
> +	/* device id */
> +	cnt = snprintf(buf, 6, "-%04x", INTEL_DEVID(dev_priv));
> +	buf += cnt;
> +	ret += cnt;
> +
> +	/* mdev type */
> +	str = mdev_type;
> +	cnt = snprintf(buf, strlen(str) + 3, "-%s\n", mdev_type);
> +	buf += cnt;
> +	ret += cnt;
> +
> +	return ret;
> +}
> +
> +ssize_t intel_gvt_check_vfio_device_version(struct drm_i915_private *dev_priv,
> +		const char *self, const char *remote)
> +{
> +
> +	/* currently only gen8 & gen9 are supported */
> +	if (!IS_GEN(dev_priv, 8) && !IS_GEN(dev_priv, 9))
> +		return -ENODEV;
> +
> +	if (!is_compatible(self, remote))
> +		return -EINVAL;
> +
> +	return 0;
> +}
> diff --git a/drivers/gpu/drm/i915/gvt/gvt.c b/drivers/gpu/drm/i915/gvt/gvt.c
> index 43f4242062dd..19f16eec5a4c 100644
> --- a/drivers/gpu/drm/i915/gvt/gvt.c
> +++ b/drivers/gpu/drm/i915/gvt/gvt.c
> @@ -105,14 +105,65 @@ static ssize_t description_show(struct kobject *kobj, struct device *dev,
>  		       type->weight);
>  }
>  
> +#ifdef GVT_MIGRATION_VERSION

No extra define.

> +static ssize_t version_show(struct kobject *kobj, struct device *dev,
> +		char *buf)
> +{
> +	struct drm_i915_private *i915 = kdev_to_i915(dev);
> +	const char *mdev_type = kobject_name(kobj);
> +
> +	return intel_gvt_get_vfio_device_version(i915, buf, mdev_type);
> +}
> +
> +static ssize_t version_store(struct kobject *kobj, struct device *dev,
> +		const char *buf, size_t count)
> +{
> +	char *remote = NULL, *self = NULL;
> +	int len, ret = 0;
> +	struct drm_i915_private *i915 = kdev_to_i915(dev);
> +	const char *mdev_type = kobject_name(kobj);
> +
> +	len = intel_gvt_get_vfio_device_version_len(i915);
> +	if (len < 0)
> +		return len;
> +
> +	self = kmalloc(len, GFP_KERNEL);
> +	if (!self)
> +		return -ENOMEM;
> +
> +	ret = intel_gvt_get_vfio_device_version(i915, self, mdev_type);
> +	if (ret < 0)
> +		goto out;
> +
> +	remote = kstrndup(buf, count, GFP_KERNEL);
> +	if (!remote) {
> +		ret = -ENOMEM;
> +		goto out;
> +	}

Please make device version as attribute for vgpu instead of allocating memory
everytime to generate it.

> +
> +	ret = intel_gvt_check_vfio_device_version(i915, self, remote);
> +
> +out:
> +	kfree(self);
> +	kfree(remote);
> +	return (ret < 0 ? ret : count);
> +}
> +#endif
> +
>  static MDEV_TYPE_ATTR_RO(available_instances);
>  static MDEV_TYPE_ATTR_RO(device_api);
>  static MDEV_TYPE_ATTR_RO(description);
> +#ifdef GVT_MIGRATION_VERSION
> +static MDEV_TYPE_ATTR_RW(version);
> +#endif

Don't need extra define.

>  
>  static struct attribute *gvt_type_attrs[] = {
>  	&mdev_type_attr_available_instances.attr,
>  	&mdev_type_attr_device_api.attr,
>  	&mdev_type_attr_description.attr,
> +#ifdef GVT_MIGRATION_VERSION
> +	&mdev_type_attr_version.attr,
> +#endif
>  	NULL,
>  };

I think you need another group of attrs for type that could support
migration, it will be assigned during host init for current platform
with driver support. So just add new group of attrs for like
gvt_migration_type_attrs[] with version.

>  
> diff --git a/drivers/gpu/drm/i915/gvt/gvt.h b/drivers/gpu/drm/i915/gvt/gvt.h
> index f5a328b5290a..4062f6b26acf 100644
> --- a/drivers/gpu/drm/i915/gvt/gvt.h
> +++ b/drivers/gpu/drm/i915/gvt/gvt.h
> @@ -687,6 +687,12 @@ 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_get_vfio_device_version(struct drm_i915_private *i915,
> +		char *buf, const char *mdev_type);
> +ssize_t intel_gvt_check_vfio_device_version(struct drm_i915_private *dev_priv,
> +		const char *self, const char *remote);
> +ssize_t
> +intel_gvt_get_vfio_device_version_len(struct drm_i915_private *dev_priv);
>  
>  #include "trace.h"
>  #include "mpt.h"
> -- 
> 2.17.1
>

Zhenyu Wang May 6, 2019, 7:41 a.m. UTC | #2

On 2019.05.06 11:20:32 +0800, Zhenyu Wang wrote:
> On 2019.05.05 21:51:02 -0400, Yan Zhao wrote:
> > This feature implements the version attribute for Intel's vGPU mdev
> > devices.
> > 
> > version attribute is rw.
> > It's used to check device compatibility for two mdev devices.
> > version string format and length are private for vendor driver. vendor
> > driver is able to define them freely.
> > 
> > For Intel vGPU of gen8 and gen9, the mdev device version
> > consists of 3 fields: "vendor id" + "device id" + "mdev type".
> > 
> > Reading from a vGPU's version attribute, a string is returned in below
> > format: <vendor id>-<device id>-<mdev type>. e.g.
> > 8086-193b-i915-GVTg_V5_2.
> > 
> > Writing a string to a vGPU's version attribute will trigger GVT to check
> > whether a vGPU identified by the written string is compatible with
> > current vGPU owning this version attribute. errno is returned if the two
> > vGPUs are incompatible. The length of written string is returned in
> > compatible case.
> > 
> > For other platforms, and for GVT not supporting vGPU live migration
> > feature, errnos are returned when read/write of mdev devices' version
> > attributes.
> > 
> > For old GVT versions where no version attributes exposed in sysfs, it is
> > regarded as not supporting vGPU live migration.
> > 
> > For future platforms, besides the current 2 fields in vendor proprietary
> > part, more fields may be added to identify Intel vGPU well for live
> > migration purpose.
> > 
> > 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)
> > 
> > Cc: Alex Williamson <[e-mail]>
> > Cc: Erik Skultety <[e-mail]>
> > Cc: "Dr. David Alan Gilbert" <[e-mail]>
> > Cc: Cornelia Huck <[e-mail]>
> > Cc: "Tian, Kevin" <[e-mail]>
> > Cc: Zhenyu Wang <[e-mail]>
> > Cc: "Wang, Zhi A" <[e-mail]>
> > c: Neo Jia <[e-mail]>
> > Cc: Kirti Wankhede <[e-mail]>
> > 
> > Signed-off-by: Yan Zhao <[e-mail]>
> > ---
> >  drivers/gpu/drm/i915/gvt/Makefile         |  2 +-
> >  drivers/gpu/drm/i915/gvt/device_version.c | 87 +++++++++++++++++++++++
> >  drivers/gpu/drm/i915/gvt/gvt.c            | 51 +++++++++++++
> >  drivers/gpu/drm/i915/gvt/gvt.h            |  6 ++
> >  4 files changed, 145 insertions(+), 1 deletion(-)
> >  create mode 100644 drivers/gpu/drm/i915/gvt/device_version.c
> > 
> > diff --git a/drivers/gpu/drm/i915/gvt/Makefile b/drivers/gpu/drm/i915/gvt/Makefile
> > index 271fb46d4dd0..54e209a23899 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 device_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/device_version.c b/drivers/gpu/drm/i915/gvt/device_version.c
> > new file mode 100644
> > index 000000000000..bd4cdcbdba95
> > --- /dev/null
> > +++ b/drivers/gpu/drm/i915/gvt/device_version.c
> > @@ -0,0 +1,87 @@
> > +/*
> > + * 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 <[e-mail]>
> > + */
> > +#include <linux/vfio.h>
> > +#include "i915_drv.h"
> > +
> > +static bool is_compatible(const char *self, const char *remote)
> > +{
> > +	if (strlen(remote) != strlen(self))
> > +		return false;
> > +
> > +	return (strncmp(self, remote, strlen(self))) ? false : true;
> > +}
> > +
> > +ssize_t intel_gvt_get_vfio_device_version_len(struct drm_i915_private *dev_priv)
> > +{
> > +	if (!IS_GEN(dev_priv, 8) && !IS_GEN(dev_priv, 9))
> > +		return -ENODEV;
> > +
> > +	return PAGE_SIZE;
> > +}
> > +
> > +ssize_t intel_gvt_get_vfio_device_version(struct drm_i915_private *dev_priv,
> > +		char *buf, const char *mdev_type)
> > +{
> > +	int cnt = 0, ret = 0;
> > +	const char *str = NULL;
> > +
> 
> > +	/* currently only gen8 & gen9 are supported */
> > +	if (!IS_GEN(dev_priv, 8) && !IS_GEN(dev_priv, 9))
> > +		return -ENODEV;
> > +
> > +	/* vendor id + device id + mdev type */
> > +	/* vendor id */
> > +	cnt = snprintf(buf, 5, "%04x", PCI_VENDOR_ID_INTEL);
> > +	buf += cnt;
> > +	ret += cnt;
> > +
> > +	/* device id */
> > +	cnt = snprintf(buf, 6, "-%04x", INTEL_DEVID(dev_priv));
> > +	buf += cnt;
> > +	ret += cnt;
> > +
> > +	/* mdev type */
> > +	str = mdev_type;
> > +	cnt = snprintf(buf, strlen(str) + 3, "-%s\n", mdev_type);
> > +	buf += cnt;
> > +	ret += cnt;
> > +
> > +	return ret;
> > +}
> > +
> > +ssize_t intel_gvt_check_vfio_device_version(struct drm_i915_private *dev_priv,
> > +		const char *self, const char *remote)
> > +{
> > +
> > +	/* currently only gen8 & gen9 are supported */
> > +	if (!IS_GEN(dev_priv, 8) && !IS_GEN(dev_priv, 9))
> > +		return -ENODEV;
> > +
> > +	if (!is_compatible(self, remote))
> > +		return -EINVAL;
> > +
> > +	return 0;
> > +}
> > diff --git a/drivers/gpu/drm/i915/gvt/gvt.c b/drivers/gpu/drm/i915/gvt/gvt.c
> > index 43f4242062dd..19f16eec5a4c 100644
> > --- a/drivers/gpu/drm/i915/gvt/gvt.c
> > +++ b/drivers/gpu/drm/i915/gvt/gvt.c
> > @@ -105,14 +105,65 @@ static ssize_t description_show(struct kobject *kobj, struct device *dev,
> >  		       type->weight);
> >  }
> >  
> > +#ifdef GVT_MIGRATION_VERSION
> 
> No extra define.
> 
> > +static ssize_t version_show(struct kobject *kobj, struct device *dev,
> > +		char *buf)
> > +{
> > +	struct drm_i915_private *i915 = kdev_to_i915(dev);
> > +	const char *mdev_type = kobject_name(kobj);
> > +
> > +	return intel_gvt_get_vfio_device_version(i915, buf, mdev_type);
> > +}
> > +
> > +static ssize_t version_store(struct kobject *kobj, struct device *dev,
> > +		const char *buf, size_t count)
> > +{
> > +	char *remote = NULL, *self = NULL;
> > +	int len, ret = 0;
> > +	struct drm_i915_private *i915 = kdev_to_i915(dev);
> > +	const char *mdev_type = kobject_name(kobj);
> > +
> > +	len = intel_gvt_get_vfio_device_version_len(i915);
> > +	if (len < 0)
> > +		return len;
> > +
> > +	self = kmalloc(len, GFP_KERNEL);
> > +	if (!self)
> > +		return -ENOMEM;
> > +
> > +	ret = intel_gvt_get_vfio_device_version(i915, self, mdev_type);
> > +	if (ret < 0)
> > +		goto out;
> > +
> > +	remote = kstrndup(buf, count, GFP_KERNEL);
> > +	if (!remote) {
> > +		ret = -ENOMEM;
> > +		goto out;
> > +	}
> 
> Please make device version as attribute for vgpu instead of allocating memory
> everytime to generate it.
>

Seems this is attribute for mdev type instead of instance, I was wrong
to take it as vgpu instance attribute, so we could add it for vgpu type
definition for device with migration.

> > +
> > +	ret = intel_gvt_check_vfio_device_version(i915, self, remote);
> > +
> > +out:
> > +	kfree(self);
> > +	kfree(remote);
> > +	return (ret < 0 ? ret : count);
> > +}
> > +#endif
> > +
> >  static MDEV_TYPE_ATTR_RO(available_instances);
> >  static MDEV_TYPE_ATTR_RO(device_api);
> >  static MDEV_TYPE_ATTR_RO(description);
> > +#ifdef GVT_MIGRATION_VERSION
> > +static MDEV_TYPE_ATTR_RW(version);
> > +#endif
> 
> Don't need extra define.
> 
> >  
> >  static struct attribute *gvt_type_attrs[] = {
> >  	&mdev_type_attr_available_instances.attr,
> >  	&mdev_type_attr_device_api.attr,
> >  	&mdev_type_attr_description.attr,
> > +#ifdef GVT_MIGRATION_VERSION
> > +	&mdev_type_attr_version.attr,
> > +#endif
> >  	NULL,
> >  };
> 
> I think you need another group of attrs for type that could support
> migration, it will be assigned during host init for current platform
> with driver support. So just add new group of attrs for like
> gvt_migration_type_attrs[] with version.
> 
> >  
> > diff --git a/drivers/gpu/drm/i915/gvt/gvt.h b/drivers/gpu/drm/i915/gvt/gvt.h
> > index f5a328b5290a..4062f6b26acf 100644
> > --- a/drivers/gpu/drm/i915/gvt/gvt.h
> > +++ b/drivers/gpu/drm/i915/gvt/gvt.h
> > @@ -687,6 +687,12 @@ 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_get_vfio_device_version(struct drm_i915_private *i915,
> > +		char *buf, const char *mdev_type);
> > +ssize_t intel_gvt_check_vfio_device_version(struct drm_i915_private *dev_priv,
> > +		const char *self, const char *remote);
> > +ssize_t
> > +intel_gvt_get_vfio_device_version_len(struct drm_i915_private *dev_priv);
> >  
> >  #include "trace.h"
> >  #include "mpt.h"
> > -- 
> > 2.17.1
> > 
> 
> -- 
> Open Source Technology Center, Intel ltd.
> 
> $gpg --keyserver wwwkeys.pgp.net --recv-keys 4D781827

Yan Zhao May 7, 2019, 5:43 a.m. UTC | #3

On Mon, May 06, 2019 at 03:41:58PM +0800, Zhenyu Wang wrote:
> On 2019.05.06 11:20:32 +0800, Zhenyu Wang wrote:
> > On 2019.05.05 21:51:02 -0400, Yan Zhao wrote:
> > > This feature implements the version attribute for Intel's vGPU mdev
> > > devices.
> > >
> > > version attribute is rw.
> > > It's used to check device compatibility for two mdev devices.
> > > version string format and length are private for vendor driver. vendor
> > > driver is able to define them freely.
> > >
> > > For Intel vGPU of gen8 and gen9, the mdev device version
> > > consists of 3 fields: "vendor id" + "device id" + "mdev type".
> > >
> > > Reading from a vGPU's version attribute, a string is returned in below
> > > format: <vendor id>-<device id>-<mdev type>. e.g.
> > > 8086-193b-i915-GVTg_V5_2.
> > >
> > > Writing a string to a vGPU's version attribute will trigger GVT to check
> > > whether a vGPU identified by the written string is compatible with
> > > current vGPU owning this version attribute. errno is returned if the two
> > > vGPUs are incompatible. The length of written string is returned in
> > > compatible case.
> > >
> > > For other platforms, and for GVT not supporting vGPU live migration
> > > feature, errnos are returned when read/write of mdev devices' version
> > > attributes.
> > >
> > > For old GVT versions where no version attributes exposed in sysfs, it is
> > > regarded as not supporting vGPU live migration.
> > >
> > > For future platforms, besides the current 2 fields in vendor proprietary
> > > part, more fields may be added to identify Intel vGPU well for live
> > > migration purpose.
> > >
> > > 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)
> > >
> > > Cc: Alex Williamson <[e-mail]>
> > > Cc: Erik Skultety <[e-mail]>
> > > Cc: "Dr. David Alan Gilbert" <[e-mail]>
> > > Cc: Cornelia Huck <[e-mail]>
> > > Cc: "Tian, Kevin" <[e-mail]>
> > > Cc: Zhenyu Wang <[e-mail]>
> > > Cc: "Wang, Zhi A" <[e-mail]>
> > > c: Neo Jia <[e-mail]>
> > > Cc: Kirti Wankhede <[e-mail]>
> > >
> > > Signed-off-by: Yan Zhao <[e-mail]>
> > > ---
> > >  drivers/gpu/drm/i915/gvt/Makefile         |  2 +-
> > >  drivers/gpu/drm/i915/gvt/device_version.c | 87 +++++++++++++++++++++++
> > >  drivers/gpu/drm/i915/gvt/gvt.c            | 51 +++++++++++++
> > >  drivers/gpu/drm/i915/gvt/gvt.h            |  6 ++
> > >  4 files changed, 145 insertions(+), 1 deletion(-)
> > >  create mode 100644 drivers/gpu/drm/i915/gvt/device_version.c
> > >
> > > diff --git a/drivers/gpu/drm/i915/gvt/Makefile b/drivers/gpu/drm/i915/gvt/Makefile
> > > index 271fb46d4dd0..54e209a23899 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 device_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/device_version.c b/drivers/gpu/drm/i915/gvt/device_version.c
> > > new file mode 100644
> > > index 000000000000..bd4cdcbdba95
> > > --- /dev/null
> > > +++ b/drivers/gpu/drm/i915/gvt/device_version.c
> > > @@ -0,0 +1,87 @@
> > > +/*
> > > + * 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 <[e-mail]>
> > > + */
> > > +#include <linux/vfio.h>
> > > +#include "i915_drv.h"
> > > +
> > > +static bool is_compatible(const char *self, const char *remote)
> > > +{
> > > +   if (strlen(remote) != strlen(self))
> > > +           return false;
> > > +
> > > +   return (strncmp(self, remote, strlen(self))) ? false : true;
> > > +}
> > > +
> > > +ssize_t intel_gvt_get_vfio_device_version_len(struct drm_i915_private *dev_priv)
> > > +{
> > > +   if (!IS_GEN(dev_priv, 8) && !IS_GEN(dev_priv, 9))
> > > +           return -ENODEV;
> > > +
> > > +   return PAGE_SIZE;
> > > +}
> > > +
> > > +ssize_t intel_gvt_get_vfio_device_version(struct drm_i915_private *dev_priv,
> > > +           char *buf, const char *mdev_type)
> > > +{
> > > +   int cnt = 0, ret = 0;
> > > +   const char *str = NULL;
> > > +
> >
> > > +   /* currently only gen8 & gen9 are supported */
> > > +   if (!IS_GEN(dev_priv, 8) && !IS_GEN(dev_priv, 9))
> > > +           return -ENODEV;
> > > +
> > > +   /* vendor id + device id + mdev type */
> > > +   /* vendor id */
> > > +   cnt = snprintf(buf, 5, "%04x", PCI_VENDOR_ID_INTEL);
> > > +   buf += cnt;
> > > +   ret += cnt;
> > > +
> > > +   /* device id */
> > > +   cnt = snprintf(buf, 6, "-%04x", INTEL_DEVID(dev_priv));
> > > +   buf += cnt;
> > > +   ret += cnt;
> > > +
> > > +   /* mdev type */
> > > +   str = mdev_type;
> > > +   cnt = snprintf(buf, strlen(str) + 3, "-%s\n", mdev_type);
> > > +   buf += cnt;
> > > +   ret += cnt;
> > > +
> > > +   return ret;
> > > +}
> > > +
> > > +ssize_t intel_gvt_check_vfio_device_version(struct drm_i915_private *dev_priv,
> > > +           const char *self, const char *remote)
> > > +{
> > > +
> > > +   /* currently only gen8 & gen9 are supported */
> > > +   if (!IS_GEN(dev_priv, 8) && !IS_GEN(dev_priv, 9))
> > > +           return -ENODEV;
> > > +
> > > +   if (!is_compatible(self, remote))
> > > +           return -EINVAL;
> > > +
> > > +   return 0;
> > > +}
> > > diff --git a/drivers/gpu/drm/i915/gvt/gvt.c b/drivers/gpu/drm/i915/gvt/gvt.c
> > > index 43f4242062dd..19f16eec5a4c 100644
> > > --- a/drivers/gpu/drm/i915/gvt/gvt.c
> > > +++ b/drivers/gpu/drm/i915/gvt/gvt.c
> > > @@ -105,14 +105,65 @@ static ssize_t description_show(struct kobject *kobj, struct device *dev,
> > >                    type->weight);
> > >  }
> > >
> > > +#ifdef GVT_MIGRATION_VERSION
> >
> > No extra define.
> >
ok. I'll try to define a variable in struct intel_gvt.
e.g. 

gvt->is_migration_enabled.

> > > +static ssize_t version_show(struct kobject *kobj, struct device *dev,
> > > +           char *buf)
> > > +{
> > > +   struct drm_i915_private *i915 = kdev_to_i915(dev);
> > > +   const char *mdev_type = kobject_name(kobj);
> > > +
> > > +   return intel_gvt_get_vfio_device_version(i915, buf, mdev_type);
> > > +}
> > > +
> > > +static ssize_t version_store(struct kobject *kobj, struct device *dev,
> > > +           const char *buf, size_t count)
> > > +{
> > > +   char *remote = NULL, *self = NULL;
> > > +   int len, ret = 0;
> > > +   struct drm_i915_private *i915 = kdev_to_i915(dev);
> > > +   const char *mdev_type = kobject_name(kobj);
> > > +
> > > +   len = intel_gvt_get_vfio_device_version_len(i915);
> > > +   if (len < 0)
> > > +           return len;
> > > +
> > > +   self = kmalloc(len, GFP_KERNEL);
> > > +   if (!self)
> > > +           return -ENOMEM;
> > > +
> > > +   ret = intel_gvt_get_vfio_device_version(i915, self, mdev_type);
> > > +   if (ret < 0)
> > > +           goto out;
> > > +
> > > +   remote = kstrndup(buf, count, GFP_KERNEL);
> > > +   if (!remote) {
> > > +           ret = -ENOMEM;
> > > +           goto out;
> > > +   }
> >
> > Please make device version as attribute for vgpu instead of allocating memory
> > everytime to generate it.
> >
> 
> Seems this is attribute for mdev type instead of instance, I was wrong
> to take it as vgpu instance attribute, so we could add it for vgpu type
> definition for device with migration.
>
ok. I'll try to add one version field in struct intel_vgpu_type.

> > > +
> > > +   ret = intel_gvt_check_vfio_device_version(i915, self, remote);
> > > +
> > > +out:
> > > +   kfree(self);
> > > +   kfree(remote);
> > > +   return (ret < 0 ? ret : count);
> > > +}
> > > +#endif
> > > +
> > >  static MDEV_TYPE_ATTR_RO(available_instances);
> > >  static MDEV_TYPE_ATTR_RO(device_api);
> > >  static MDEV_TYPE_ATTR_RO(description);
> > > +#ifdef GVT_MIGRATION_VERSION
> > > +static MDEV_TYPE_ATTR_RW(version);
> > > +#endif
> >
> > Don't need extra define.
> >
> > >
> > >  static struct attribute *gvt_type_attrs[] = {
> > >     &mdev_type_attr_available_instances.attr,
> > >     &mdev_type_attr_device_api.attr,
> > >     &mdev_type_attr_description.attr,
> > > +#ifdef GVT_MIGRATION_VERSION
> > > +   &mdev_type_attr_version.attr,
> > > +#endif
> > >     NULL,
> > >  };
> >
> > I think you need another group of attrs for type that could support
> > migration, it will be assigned during host init for current platform
> > with driver support. So just add new group of attrs for like
> > gvt_migration_type_attrs[] with version.
> >
> > >
> > > diff --git a/drivers/gpu/drm/i915/gvt/gvt.h b/drivers/gpu/drm/i915/gvt/gvt.h
> > > index f5a328b5290a..4062f6b26acf 100644
> > > --- a/drivers/gpu/drm/i915/gvt/gvt.h
> > > +++ b/drivers/gpu/drm/i915/gvt/gvt.h
> > > @@ -687,6 +687,12 @@ 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_get_vfio_device_version(struct drm_i915_private *i915,
> > > +           char *buf, const char *mdev_type);
> > > +ssize_t intel_gvt_check_vfio_device_version(struct drm_i915_private *dev_priv,
> > > +           const char *self, const char *remote);
> > > +ssize_t
> > > +intel_gvt_get_vfio_device_version_len(struct drm_i915_private *dev_priv);
> > >
> > >  #include "trace.h"
> > >  #include "mpt.h"
> > > --
> > > 2.17.1
> > >
> >
> > --
> > Open Source Technology Center, Intel ltd.
> >
> > $gpg --keyserver wwwkeys.pgp.net --recv-keys 4D781827
> 
> 
> 
> --
> Open Source Technology Center, Intel ltd.
> 
> $gpg --keyserver wwwkeys.pgp.net --recv-keys 4D781827


> _______________________________________________
> intel-gvt-dev mailing list
> [e-mail]
> https://lists.freedesktop.org/mailman/listinfo/intel-gvt-dev

Cornelia Huck May 7, 2019, 9:27 a.m. UTC | #4

On Sun,  5 May 2019 21:51:02 -0400
Yan Zhao <[e-mail]> wrote:

> This feature implements the version attribute for Intel's vGPU mdev
> devices.
> 
> version attribute is rw.
> It's used to check device compatibility for two mdev devices.
> version string format and length are private for vendor driver. vendor
> driver is able to define them freely.
> 
> For Intel vGPU of gen8 and gen9, the mdev device version
> consists of 3 fields: "vendor id" + "device id" + "mdev type".
> 
> Reading from a vGPU's version attribute, a string is returned in below
> format: <vendor id>-<device id>-<mdev type>. e.g.
> 8086-193b-i915-GVTg_V5_2.
> 
> Writing a string to a vGPU's version attribute will trigger GVT to check
> whether a vGPU identified by the written string is compatible with
> current vGPU owning this version attribute. errno is returned if the two
> vGPUs are incompatible. The length of written string is returned in
> compatible case.
> 
> For other platforms, and for GVT not supporting vGPU live migration
> feature, errnos are returned when read/write of mdev devices' version
> attributes.
> 
> For old GVT versions where no version attributes exposed in sysfs, it is
> regarded as not supporting vGPU live migration.
> 
> For future platforms, besides the current 2 fields in vendor proprietary
> part, more fields may be added to identify Intel vGPU well for live
> migration purpose.
> 
> 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)

Should go below '---'.

> 
> Cc: Alex Williamson <[e-mail]>
> Cc: Erik Skultety <[e-mail]>
> Cc: "Dr. David Alan Gilbert" <[e-mail]>
> Cc: Cornelia Huck <[e-mail]>
> Cc: "Tian, Kevin" <[e-mail]>
> Cc: Zhenyu Wang <[e-mail]>
> Cc: "Wang, Zhi A" <[e-mail]>
> c: Neo Jia <[e-mail]>
> Cc: Kirti Wankhede <[e-mail]>
> 
> Signed-off-by: Yan Zhao <[e-mail]>
> ---
>  drivers/gpu/drm/i915/gvt/Makefile         |  2 +-
>  drivers/gpu/drm/i915/gvt/device_version.c | 87 +++++++++++++++++++++++
>  drivers/gpu/drm/i915/gvt/gvt.c            | 51 +++++++++++++
>  drivers/gpu/drm/i915/gvt/gvt.h            |  6 ++
>  4 files changed, 145 insertions(+), 1 deletion(-)
>  create mode 100644 drivers/gpu/drm/i915/gvt/device_version.c
> 

(...)

> diff --git a/drivers/gpu/drm/i915/gvt/device_version.c b/drivers/gpu/drm/i915/gvt/device_version.c
> new file mode 100644
> index 000000000000..bd4cdcbdba95
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/gvt/device_version.c
> @@ -0,0 +1,87 @@
> +/*
> + * 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 <[e-mail]>
> + */
> +#include <linux/vfio.h>
> +#include "i915_drv.h"
> +
> +static bool is_compatible(const char *self, const char *remote)
> +{
> +	if (strlen(remote) != strlen(self))
> +		return false;
> +
> +	return (strncmp(self, remote, strlen(self))) ? false : true;
> +}
> +
> +ssize_t intel_gvt_get_vfio_device_version_len(struct drm_i915_private *dev_priv)
> +{
> +	if (!IS_GEN(dev_priv, 8) && !IS_GEN(dev_priv, 9))
> +		return -ENODEV;
> +
> +	return PAGE_SIZE;
> +}
> +
> +ssize_t intel_gvt_get_vfio_device_version(struct drm_i915_private *dev_priv,
> +		char *buf, const char *mdev_type)
> +{
> +	int cnt = 0, ret = 0;
> +	const char *str = NULL;
> +
> +	/* currently only gen8 & gen9 are supported */
> +	if (!IS_GEN(dev_priv, 8) && !IS_GEN(dev_priv, 9))
> +		return -ENODEV;
> +
> +	/* vendor id + device id + mdev type */
> +	/* vendor id */
> +	cnt = snprintf(buf, 5, "%04x", PCI_VENDOR_ID_INTEL);
> +	buf += cnt;
> +	ret += cnt;
> +
> +	/* device id */
> +	cnt = snprintf(buf, 6, "-%04x", INTEL_DEVID(dev_priv));
> +	buf += cnt;
> +	ret += cnt;
> +
> +	/* mdev type */
> +	str = mdev_type;
> +	cnt = snprintf(buf, strlen(str) + 3, "-%s\n", mdev_type);
> +	buf += cnt;
> +	ret += cnt;
> +
> +	return ret;

I'm not familiar with this driver; but would it make sense to pre-build
the version on init? It does not look to me like the values could
change dynamically.

> +}
> +
> +ssize_t intel_gvt_check_vfio_device_version(struct drm_i915_private *dev_priv,
> +		const char *self, const char *remote)
> +{
> +
> +	/* currently only gen8 & gen9 are supported */
> +	if (!IS_GEN(dev_priv, 8) && !IS_GEN(dev_priv, 9))
> +		return -ENODEV;
> +
> +	if (!is_compatible(self, remote))
> +		return -EINVAL;
> +
> +	return 0;
> +}

Return values look reasonable to me. I'll leave discussions regarding
where the attribute should go to folks familiar with this driver.

Dr. David Alan Gilbert May 8, 2019, 10:50 a.m. UTC | #5

* Yan Zhao ([e-mail]) wrote:
> This feature implements the version attribute for Intel's vGPU mdev
> devices.
> 
> version attribute is rw.
> It's used to check device compatibility for two mdev devices.
> version string format and length are private for vendor driver. vendor
> driver is able to define them freely.
> 
> For Intel vGPU of gen8 and gen9, the mdev device version
> consists of 3 fields: "vendor id" + "device id" + "mdev type".
> 
> Reading from a vGPU's version attribute, a string is returned in below
> format: <vendor id>-<device id>-<mdev type>. e.g.
> 8086-193b-i915-GVTg_V5_2.
> 
> Writing a string to a vGPU's version attribute will trigger GVT to check
> whether a vGPU identified by the written string is compatible with
> current vGPU owning this version attribute. errno is returned if the two
> vGPUs are incompatible. The length of written string is returned in
> compatible case.
> 
> For other platforms, and for GVT not supporting vGPU live migration
> feature, errnos are returned when read/write of mdev devices' version
> attributes.
> 
> For old GVT versions where no version attributes exposed in sysfs, it is
> regarded as not supporting vGPU live migration.
> 
> For future platforms, besides the current 2 fields in vendor proprietary
> part, more fields may be added to identify Intel vGPU well for live
> migration purpose.
> 
> 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)
> 
> Cc: Alex Williamson <[e-mail]>
> Cc: Erik Skultety <[e-mail]>
> Cc: "Dr. David Alan Gilbert" <[e-mail]>
> Cc: Cornelia Huck <[e-mail]>
> Cc: "Tian, Kevin" <[e-mail]>
> Cc: Zhenyu Wang <[e-mail]>
> Cc: "Wang, Zhi A" <[e-mail]>
> c: Neo Jia <[e-mail]>
> Cc: Kirti Wankhede <[e-mail]>
> 
> Signed-off-by: Yan Zhao <[e-mail]>
> ---
>  drivers/gpu/drm/i915/gvt/Makefile         |  2 +-
>  drivers/gpu/drm/i915/gvt/device_version.c | 87 +++++++++++++++++++++++
>  drivers/gpu/drm/i915/gvt/gvt.c            | 51 +++++++++++++
>  drivers/gpu/drm/i915/gvt/gvt.h            |  6 ++
>  4 files changed, 145 insertions(+), 1 deletion(-)
>  create mode 100644 drivers/gpu/drm/i915/gvt/device_version.c
> 
> diff --git a/drivers/gpu/drm/i915/gvt/Makefile b/drivers/gpu/drm/i915/gvt/Makefile
> index 271fb46d4dd0..54e209a23899 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 device_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/device_version.c b/drivers/gpu/drm/i915/gvt/device_version.c
> new file mode 100644
> index 000000000000..bd4cdcbdba95
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/gvt/device_version.c
> @@ -0,0 +1,87 @@
> +/*
> + * 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 <[e-mail]>
> + */
> +#include <linux/vfio.h>
> +#include "i915_drv.h"
> +
> +static bool is_compatible(const char *self, const char *remote)
> +{
> +	if (strlen(remote) != strlen(self))
> +		return false;
> +
> +	return (strncmp(self, remote, strlen(self))) ? false : true;
> +}
> +
> +ssize_t intel_gvt_get_vfio_device_version_len(struct drm_i915_private *dev_priv)
> +{
> +	if (!IS_GEN(dev_priv, 8) && !IS_GEN(dev_priv, 9))
> +		return -ENODEV;
> +
> +	return PAGE_SIZE;
> +}
> +
> +ssize_t intel_gvt_get_vfio_device_version(struct drm_i915_private *dev_priv,
> +		char *buf, const char *mdev_type)
> +{
> +	int cnt = 0, ret = 0;
> +	const char *str = NULL;
> +
> +	/* currently only gen8 & gen9 are supported */
> +	if (!IS_GEN(dev_priv, 8) && !IS_GEN(dev_priv, 9))
> +		return -ENODEV;
> +
> +	/* vendor id + device id + mdev type */
> +	/* vendor id */
> +	cnt = snprintf(buf, 5, "%04x", PCI_VENDOR_ID_INTEL);
> +	buf += cnt;
> +	ret += cnt;
> +
> +	/* device id */
> +	cnt = snprintf(buf, 6, "-%04x", INTEL_DEVID(dev_priv));
> +	buf += cnt;
> +	ret += cnt;
> +
> +	/* mdev type */
> +	str = mdev_type;
> +	cnt = snprintf(buf, strlen(str) + 3, "-%s\n", mdev_type);
> +	buf += cnt;
> +	ret += cnt;

Why not just one big snprintf?

Dave

> +	return ret;
> +}
> +
> +ssize_t intel_gvt_check_vfio_device_version(struct drm_i915_private *dev_priv,
> +		const char *self, const char *remote)
> +{
> +
> +	/* currently only gen8 & gen9 are supported */
> +	if (!IS_GEN(dev_priv, 8) && !IS_GEN(dev_priv, 9))
> +		return -ENODEV;
> +
> +	if (!is_compatible(self, remote))
> +		return -EINVAL;
> +
> +	return 0;
> +}
> diff --git a/drivers/gpu/drm/i915/gvt/gvt.c b/drivers/gpu/drm/i915/gvt/gvt.c
> index 43f4242062dd..19f16eec5a4c 100644
> --- a/drivers/gpu/drm/i915/gvt/gvt.c
> +++ b/drivers/gpu/drm/i915/gvt/gvt.c
> @@ -105,14 +105,65 @@ static ssize_t description_show(struct kobject *kobj, struct device *dev,
>  		       type->weight);
>  }
>  
> +#ifdef GVT_MIGRATION_VERSION
> +static ssize_t version_show(struct kobject *kobj, struct device *dev,
> +		char *buf)
> +{
> +	struct drm_i915_private *i915 = kdev_to_i915(dev);
> +	const char *mdev_type = kobject_name(kobj);
> +
> +	return intel_gvt_get_vfio_device_version(i915, buf, mdev_type);
> +}
> +
> +static ssize_t version_store(struct kobject *kobj, struct device *dev,
> +		const char *buf, size_t count)
> +{
> +	char *remote = NULL, *self = NULL;
> +	int len, ret = 0;
> +	struct drm_i915_private *i915 = kdev_to_i915(dev);
> +	const char *mdev_type = kobject_name(kobj);
> +
> +	len = intel_gvt_get_vfio_device_version_len(i915);
> +	if (len < 0)
> +		return len;
> +
> +	self = kmalloc(len, GFP_KERNEL);
> +	if (!self)
> +		return -ENOMEM;
> +
> +	ret = intel_gvt_get_vfio_device_version(i915, self, mdev_type);
> +	if (ret < 0)
> +		goto out;
> +
> +	remote = kstrndup(buf, count, GFP_KERNEL);
> +	if (!remote) {
> +		ret = -ENOMEM;
> +		goto out;
> +	}
> +
> +	ret = intel_gvt_check_vfio_device_version(i915, self, remote);
> +
> +out:
> +	kfree(self);
> +	kfree(remote);
> +	return (ret < 0 ? ret : count);
> +}
> +#endif
> +
>  static MDEV_TYPE_ATTR_RO(available_instances);
>  static MDEV_TYPE_ATTR_RO(device_api);
>  static MDEV_TYPE_ATTR_RO(description);
> +#ifdef GVT_MIGRATION_VERSION
> +static MDEV_TYPE_ATTR_RW(version);
> +#endif
>  
>  static struct attribute *gvt_type_attrs[] = {
>  	&mdev_type_attr_available_instances.attr,
>  	&mdev_type_attr_device_api.attr,
>  	&mdev_type_attr_description.attr,
> +#ifdef GVT_MIGRATION_VERSION
> +	&mdev_type_attr_version.attr,
> +#endif
>  	NULL,
>  };
>  
> diff --git a/drivers/gpu/drm/i915/gvt/gvt.h b/drivers/gpu/drm/i915/gvt/gvt.h
> index f5a328b5290a..4062f6b26acf 100644
> --- a/drivers/gpu/drm/i915/gvt/gvt.h
> +++ b/drivers/gpu/drm/i915/gvt/gvt.h
> @@ -687,6 +687,12 @@ 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_get_vfio_device_version(struct drm_i915_private *i915,
> +		char *buf, const char *mdev_type);
> +ssize_t intel_gvt_check_vfio_device_version(struct drm_i915_private *dev_priv,
> +		const char *self, const char *remote);
> +ssize_t
> +intel_gvt_get_vfio_device_version_len(struct drm_i915_private *dev_priv);
>  
>  #include "trace.h"
>  #include "mpt.h"
> -- 
> 2.17.1
> 
--
Dr. David Alan Gilbert / [e-mail] / Manchester, UK

Yan Zhao May 8, 2019, 12:02 p.m. UTC | #6

On Tue, May 07, 2019 at 05:27:53PM +0800, Cornelia Huck wrote:
> On Sun,  5 May 2019 21:51:02 -0400
> Yan Zhao <[e-mail]> wrote:
> 
> > This feature implements the version attribute for Intel's vGPU mdev
> > devices.
> > 
> > version attribute is rw.
> > It's used to check device compatibility for two mdev devices.
> > version string format and length are private for vendor driver. vendor
> > driver is able to define them freely.
> > 
> > For Intel vGPU of gen8 and gen9, the mdev device version
> > consists of 3 fields: "vendor id" + "device id" + "mdev type".
> > 
> > Reading from a vGPU's version attribute, a string is returned in below
> > format: <vendor id>-<device id>-<mdev type>. e.g.
> > 8086-193b-i915-GVTg_V5_2.
> > 
> > Writing a string to a vGPU's version attribute will trigger GVT to check
> > whether a vGPU identified by the written string is compatible with
> > current vGPU owning this version attribute. errno is returned if the two
> > vGPUs are incompatible. The length of written string is returned in
> > compatible case.
> > 
> > For other platforms, and for GVT not supporting vGPU live migration
> > feature, errnos are returned when read/write of mdev devices' version
> > attributes.
> > 
> > For old GVT versions where no version attributes exposed in sysfs, it is
> > regarded as not supporting vGPU live migration.
> > 
> > For future platforms, besides the current 2 fields in vendor proprietary
> > part, more fields may be added to identify Intel vGPU well for live
> > migration purpose.
> > 
> > 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)
> 
> Should go below '---'.
>
got it. will change it in next revision.

> > 
> > Cc: Alex Williamson <[e-mail]>
> > Cc: Erik Skultety <[e-mail]>
> > Cc: "Dr. David Alan Gilbert" <[e-mail]>
> > Cc: Cornelia Huck <[e-mail]>
> > Cc: "Tian, Kevin" <[e-mail]>
> > Cc: Zhenyu Wang <[e-mail]>
> > Cc: "Wang, Zhi A" <[e-mail]>
> > c: Neo Jia <[e-mail]>
> > Cc: Kirti Wankhede <[e-mail]>
> > 
> > Signed-off-by: Yan Zhao <[e-mail]>
> > ---
> >  drivers/gpu/drm/i915/gvt/Makefile         |  2 +-
> >  drivers/gpu/drm/i915/gvt/device_version.c | 87 +++++++++++++++++++++++
> >  drivers/gpu/drm/i915/gvt/gvt.c            | 51 +++++++++++++
> >  drivers/gpu/drm/i915/gvt/gvt.h            |  6 ++
> >  4 files changed, 145 insertions(+), 1 deletion(-)
> >  create mode 100644 drivers/gpu/drm/i915/gvt/device_version.c
> > 
> 
> (...)
> 
> > diff --git a/drivers/gpu/drm/i915/gvt/device_version.c b/drivers/gpu/drm/i915/gvt/device_version.c
> > new file mode 100644
> > index 000000000000..bd4cdcbdba95
> > --- /dev/null
> > +++ b/drivers/gpu/drm/i915/gvt/device_version.c
> > @@ -0,0 +1,87 @@
> > +/*
> > + * 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 <[e-mail]>
> > + */
> > +#include <linux/vfio.h>
> > +#include "i915_drv.h"
> > +
> > +static bool is_compatible(const char *self, const char *remote)
> > +{
> > +	if (strlen(remote) != strlen(self))
> > +		return false;
> > +
> > +	return (strncmp(self, remote, strlen(self))) ? false : true;
> > +}
> > +
> > +ssize_t intel_gvt_get_vfio_device_version_len(struct drm_i915_private *dev_priv)
> > +{
> > +	if (!IS_GEN(dev_priv, 8) && !IS_GEN(dev_priv, 9))
> > +		return -ENODEV;
> > +
> > +	return PAGE_SIZE;
> > +}
> > +
> > +ssize_t intel_gvt_get_vfio_device_version(struct drm_i915_private *dev_priv,
> > +		char *buf, const char *mdev_type)
> > +{
> > +	int cnt = 0, ret = 0;
> > +	const char *str = NULL;
> > +
> > +	/* currently only gen8 & gen9 are supported */
> > +	if (!IS_GEN(dev_priv, 8) && !IS_GEN(dev_priv, 9))
> > +		return -ENODEV;
> > +
> > +	/* vendor id + device id + mdev type */
> > +	/* vendor id */
> > +	cnt = snprintf(buf, 5, "%04x", PCI_VENDOR_ID_INTEL);
> > +	buf += cnt;
> > +	ret += cnt;
> > +
> > +	/* device id */
> > +	cnt = snprintf(buf, 6, "-%04x", INTEL_DEVID(dev_priv));
> > +	buf += cnt;
> > +	ret += cnt;
> > +
> > +	/* mdev type */
> > +	str = mdev_type;
> > +	cnt = snprintf(buf, strlen(str) + 3, "-%s\n", mdev_type);
> > +	buf += cnt;
> > +	ret += cnt;
> > +
> > +	return ret;
> 
> I'm not familiar with this driver; but would it make sense to pre-build
> the version on init? It does not look to me like the values could
> change dynamically.
>
yes. I intended to save some memory by not pre-building the version on init, as
migration is a rare event. but as these version strings are not big, moving them
to init is also good. I'll do it in next revision.
thanks:)


> > +}
> > +
> > +ssize_t intel_gvt_check_vfio_device_version(struct drm_i915_private *dev_priv,
> > +		const char *self, const char *remote)
> > +{
> > +
> > +	/* currently only gen8 & gen9 are supported */
> > +	if (!IS_GEN(dev_priv, 8) && !IS_GEN(dev_priv, 9))
> > +		return -ENODEV;
> > +
> > +	if (!is_compatible(self, remote))
> > +		return -EINVAL;
> > +
> > +	return 0;
> > +}
> 
> Return values look reasonable to me. I'll leave discussions regarding
> where the attribute should go to folks familiar with this driver.
ok. thanks :)

Yan Zhao May 8, 2019, 12:10 p.m. UTC | #7

On Wed, May 08, 2019 at 06:50:33PM +0800, Dr. David Alan Gilbert wrote:
> * Yan Zhao ([e-mail]) wrote:
> > This feature implements the version attribute for Intel's vGPU mdev
> > devices.
> > 
> > version attribute is rw.
> > It's used to check device compatibility for two mdev devices.
> > version string format and length are private for vendor driver. vendor
> > driver is able to define them freely.
> > 
> > For Intel vGPU of gen8 and gen9, the mdev device version
> > consists of 3 fields: "vendor id" + "device id" + "mdev type".
> > 
> > Reading from a vGPU's version attribute, a string is returned in below
> > format: <vendor id>-<device id>-<mdev type>. e.g.
> > 8086-193b-i915-GVTg_V5_2.
> > 
> > Writing a string to a vGPU's version attribute will trigger GVT to check
> > whether a vGPU identified by the written string is compatible with
> > current vGPU owning this version attribute. errno is returned if the two
> > vGPUs are incompatible. The length of written string is returned in
> > compatible case.
> > 
> > For other platforms, and for GVT not supporting vGPU live migration
> > feature, errnos are returned when read/write of mdev devices' version
> > attributes.
> > 
> > For old GVT versions where no version attributes exposed in sysfs, it is
> > regarded as not supporting vGPU live migration.
> > 
> > For future platforms, besides the current 2 fields in vendor proprietary
> > part, more fields may be added to identify Intel vGPU well for live
> > migration purpose.
> > 
> > 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)
> > 
> > Cc: Alex Williamson <[e-mail]>
> > Cc: Erik Skultety <[e-mail]>
> > Cc: "Dr. David Alan Gilbert" <[e-mail]>
> > Cc: Cornelia Huck <[e-mail]>
> > Cc: "Tian, Kevin" <[e-mail]>
> > Cc: Zhenyu Wang <[e-mail]>
> > Cc: "Wang, Zhi A" <[e-mail]>
> > c: Neo Jia <[e-mail]>
> > Cc: Kirti Wankhede <[e-mail]>
> > 
> > Signed-off-by: Yan Zhao <[e-mail]>
> > ---
> >  drivers/gpu/drm/i915/gvt/Makefile         |  2 +-
> >  drivers/gpu/drm/i915/gvt/device_version.c | 87 +++++++++++++++++++++++
> >  drivers/gpu/drm/i915/gvt/gvt.c            | 51 +++++++++++++
> >  drivers/gpu/drm/i915/gvt/gvt.h            |  6 ++
> >  4 files changed, 145 insertions(+), 1 deletion(-)
> >  create mode 100644 drivers/gpu/drm/i915/gvt/device_version.c
> > 
> > diff --git a/drivers/gpu/drm/i915/gvt/Makefile b/drivers/gpu/drm/i915/gvt/Makefile
> > index 271fb46d4dd0..54e209a23899 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 device_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/device_version.c b/drivers/gpu/drm/i915/gvt/device_version.c
> > new file mode 100644
> > index 000000000000..bd4cdcbdba95
> > --- /dev/null
> > +++ b/drivers/gpu/drm/i915/gvt/device_version.c
> > @@ -0,0 +1,87 @@
> > +/*
> > + * 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 <[e-mail]>
> > + */
> > +#include <linux/vfio.h>
> > +#include "i915_drv.h"
> > +
> > +static bool is_compatible(const char *self, const char *remote)
> > +{
> > +	if (strlen(remote) != strlen(self))
> > +		return false;
> > +
> > +	return (strncmp(self, remote, strlen(self))) ? false : true;
> > +}
> > +
> > +ssize_t intel_gvt_get_vfio_device_version_len(struct drm_i915_private *dev_priv)
> > +{
> > +	if (!IS_GEN(dev_priv, 8) && !IS_GEN(dev_priv, 9))
> > +		return -ENODEV;
> > +
> > +	return PAGE_SIZE;
> > +}
> > +
> > +ssize_t intel_gvt_get_vfio_device_version(struct drm_i915_private *dev_priv,
> > +		char *buf, const char *mdev_type)
> > +{
> > +	int cnt = 0, ret = 0;
> > +	const char *str = NULL;
> > +
> > +	/* currently only gen8 & gen9 are supported */
> > +	if (!IS_GEN(dev_priv, 8) && !IS_GEN(dev_priv, 9))
> > +		return -ENODEV;
> > +
> > +	/* vendor id + device id + mdev type */
> > +	/* vendor id */
> > +	cnt = snprintf(buf, 5, "%04x", PCI_VENDOR_ID_INTEL);
> > +	buf += cnt;
> > +	ret += cnt;
> > +
> > +	/* device id */
> > +	cnt = snprintf(buf, 6, "-%04x", INTEL_DEVID(dev_priv));
> > +	buf += cnt;
> > +	ret += cnt;
> > +
> > +	/* mdev type */
> > +	str = mdev_type;
> > +	cnt = snprintf(buf, strlen(str) + 3, "-%s\n", mdev_type);
> > +	buf += cnt;
> > +	ret += cnt;
> 
> Why not just one big snprintf?
> 
> Dave

oh, right:)
I'll use one big snprintf in next revision. thank you:)

> 
> > +	return ret;
> > +}
> > +
> > +ssize_t intel_gvt_check_vfio_device_version(struct drm_i915_private *dev_priv,
> > +		const char *self, const char *remote)
> > +{
> > +
> > +	/* currently only gen8 & gen9 are supported */
> > +	if (!IS_GEN(dev_priv, 8) && !IS_GEN(dev_priv, 9))
> > +		return -ENODEV;
> > +
> > +	if (!is_compatible(self, remote))
> > +		return -EINVAL;
> > +
> > +	return 0;
> > +}
> > diff --git a/drivers/gpu/drm/i915/gvt/gvt.c b/drivers/gpu/drm/i915/gvt/gvt.c
> > index 43f4242062dd..19f16eec5a4c 100644
> > --- a/drivers/gpu/drm/i915/gvt/gvt.c
> > +++ b/drivers/gpu/drm/i915/gvt/gvt.c
> > @@ -105,14 +105,65 @@ static ssize_t description_show(struct kobject *kobj, struct device *dev,
> >  		       type->weight);
> >  }
> >  
> > +#ifdef GVT_MIGRATION_VERSION
> > +static ssize_t version_show(struct kobject *kobj, struct device *dev,
> > +		char *buf)
> > +{
> > +	struct drm_i915_private *i915 = kdev_to_i915(dev);
> > +	const char *mdev_type = kobject_name(kobj);
> > +
> > +	return intel_gvt_get_vfio_device_version(i915, buf, mdev_type);
> > +}
> > +
> > +static ssize_t version_store(struct kobject *kobj, struct device *dev,
> > +		const char *buf, size_t count)
> > +{
> > +	char *remote = NULL, *self = NULL;
> > +	int len, ret = 0;
> > +	struct drm_i915_private *i915 = kdev_to_i915(dev);
> > +	const char *mdev_type = kobject_name(kobj);
> > +
> > +	len = intel_gvt_get_vfio_device_version_len(i915);
> > +	if (len < 0)
> > +		return len;
> > +
> > +	self = kmalloc(len, GFP_KERNEL);
> > +	if (!self)
> > +		return -ENOMEM;
> > +
> > +	ret = intel_gvt_get_vfio_device_version(i915, self, mdev_type);
> > +	if (ret < 0)
> > +		goto out;
> > +
> > +	remote = kstrndup(buf, count, GFP_KERNEL);
> > +	if (!remote) {
> > +		ret = -ENOMEM;
> > +		goto out;
> > +	}
> > +
> > +	ret = intel_gvt_check_vfio_device_version(i915, self, remote);
> > +
> > +out:
> > +	kfree(self);
> > +	kfree(remote);
> > +	return (ret < 0 ? ret : count);
> > +}
> > +#endif
> > +
> >  static MDEV_TYPE_ATTR_RO(available_instances);
> >  static MDEV_TYPE_ATTR_RO(device_api);
> >  static MDEV_TYPE_ATTR_RO(description);
> > +#ifdef GVT_MIGRATION_VERSION
> > +static MDEV_TYPE_ATTR_RW(version);
> > +#endif
> >  
> >  static struct attribute *gvt_type_attrs[] = {
> >  	&mdev_type_attr_available_instances.attr,
> >  	&mdev_type_attr_device_api.attr,
> >  	&mdev_type_attr_description.attr,
> > +#ifdef GVT_MIGRATION_VERSION
> > +	&mdev_type_attr_version.attr,
> > +#endif
> >  	NULL,
> >  };
> >  
> > diff --git a/drivers/gpu/drm/i915/gvt/gvt.h b/drivers/gpu/drm/i915/gvt/gvt.h
> > index f5a328b5290a..4062f6b26acf 100644
> > --- a/drivers/gpu/drm/i915/gvt/gvt.h
> > +++ b/drivers/gpu/drm/i915/gvt/gvt.h
> > @@ -687,6 +687,12 @@ 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_get_vfio_device_version(struct drm_i915_private *i915,
> > +		char *buf, const char *mdev_type);
> > +ssize_t intel_gvt_check_vfio_device_version(struct drm_i915_private *dev_priv,
> > +		const char *self, const char *remote);
> > +ssize_t
> > +intel_gvt_get_vfio_device_version_len(struct drm_i915_private *dev_priv);
> >  
> >  #include "trace.h"
> >  #include "mpt.h"
> > -- 
> > 2.17.1
> > 
> --
> Dr. David Alan Gilbert / [e-mail] / Manchester, UK
> _______________________________________________
> intel-gvt-dev mailing list
> [e-mail]
> https://lists.freedesktop.org/mailman/listinfo/intel-gvt-dev

Patch

diff --git a/drivers/gpu/drm/i915/gvt/Makefile b/drivers/gpu/drm/i915/gvt/Makefile
index 271fb46d4dd0..54e209a23899 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 device_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/device_version.c b/drivers/gpu/drm/i915/gvt/device_version.c
new file mode 100644
index 000000000000..bd4cdcbdba95
--- /dev/null
+++ b/drivers/gpu/drm/i915/gvt/device_version.c
@@ -0,0 +1,87 @@ 
+/*
+ * 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 <[e-mail]>
+ */
+#include <linux/vfio.h>
+#include "i915_drv.h"
+
+static bool is_compatible(const char *self, const char *remote)
+{
+	if (strlen(remote) != strlen(self))
+		return false;
+
+	return (strncmp(self, remote, strlen(self))) ? false : true;
+}
+
+ssize_t intel_gvt_get_vfio_device_version_len(struct drm_i915_private *dev_priv)
+{
+	if (!IS_GEN(dev_priv, 8) && !IS_GEN(dev_priv, 9))
+		return -ENODEV;
+
+	return PAGE_SIZE;
+}
+
+ssize_t intel_gvt_get_vfio_device_version(struct drm_i915_private *dev_priv,
+		char *buf, const char *mdev_type)
+{
+	int cnt = 0, ret = 0;
+	const char *str = NULL;
+
+	/* currently only gen8 & gen9 are supported */
+	if (!IS_GEN(dev_priv, 8) && !IS_GEN(dev_priv, 9))
+		return -ENODEV;
+
+	/* vendor id + device id + mdev type */
+	/* vendor id */
+	cnt = snprintf(buf, 5, "%04x", PCI_VENDOR_ID_INTEL);
+	buf += cnt;
+	ret += cnt;
+
+	/* device id */
+	cnt = snprintf(buf, 6, "-%04x", INTEL_DEVID(dev_priv));
+	buf += cnt;
+	ret += cnt;
+
+	/* mdev type */
+	str = mdev_type;
+	cnt = snprintf(buf, strlen(str) + 3, "-%s\n", mdev_type);
+	buf += cnt;
+	ret += cnt;
+
+	return ret;
+}
+
+ssize_t intel_gvt_check_vfio_device_version(struct drm_i915_private *dev_priv,
+		const char *self, const char *remote)
+{
+
+	/* currently only gen8 & gen9 are supported */
+	if (!IS_GEN(dev_priv, 8) && !IS_GEN(dev_priv, 9))
+		return -ENODEV;
+
+	if (!is_compatible(self, remote))
+		return -EINVAL;
+
+	return 0;
+}
diff --git a/drivers/gpu/drm/i915/gvt/gvt.c b/drivers/gpu/drm/i915/gvt/gvt.c
index 43f4242062dd..19f16eec5a4c 100644
--- a/drivers/gpu/drm/i915/gvt/gvt.c
+++ b/drivers/gpu/drm/i915/gvt/gvt.c
@@ -105,14 +105,65 @@  static ssize_t description_show(struct kobject *kobj, struct device *dev,
 		       type->weight);
 }
 
+#ifdef GVT_MIGRATION_VERSION
+static ssize_t version_show(struct kobject *kobj, struct device *dev,
+		char *buf)
+{
+	struct drm_i915_private *i915 = kdev_to_i915(dev);
+	const char *mdev_type = kobject_name(kobj);
+
+	return intel_gvt_get_vfio_device_version(i915, buf, mdev_type);
+}
+
+static ssize_t version_store(struct kobject *kobj, struct device *dev,
+		const char *buf, size_t count)
+{
+	char *remote = NULL, *self = NULL;
+	int len, ret = 0;
+	struct drm_i915_private *i915 = kdev_to_i915(dev);
+	const char *mdev_type = kobject_name(kobj);
+
+	len = intel_gvt_get_vfio_device_version_len(i915);
+	if (len < 0)
+		return len;
+
+	self = kmalloc(len, GFP_KERNEL);
+	if (!self)
+		return -ENOMEM;
+
+	ret = intel_gvt_get_vfio_device_version(i915, self, mdev_type);
+	if (ret < 0)
+		goto out;
+
+	remote = kstrndup(buf, count, GFP_KERNEL);
+	if (!remote) {
+		ret = -ENOMEM;
+		goto out;
+	}
+
+	ret = intel_gvt_check_vfio_device_version(i915, self, remote);
+
+out:
+	kfree(self);
+	kfree(remote);
+	return (ret < 0 ? ret : count);
+}
+#endif
+
 static MDEV_TYPE_ATTR_RO(available_instances);
 static MDEV_TYPE_ATTR_RO(device_api);
 static MDEV_TYPE_ATTR_RO(description);
+#ifdef GVT_MIGRATION_VERSION
+static MDEV_TYPE_ATTR_RW(version);
+#endif
 
 static struct attribute *gvt_type_attrs[] = {
 	&mdev_type_attr_available_instances.attr,
 	&mdev_type_attr_device_api.attr,
 	&mdev_type_attr_description.attr,
+#ifdef GVT_MIGRATION_VERSION
+	&mdev_type_attr_version.attr,
+#endif
 	NULL,
 };
 
diff --git a/drivers/gpu/drm/i915/gvt/gvt.h b/drivers/gpu/drm/i915/gvt/gvt.h
index f5a328b5290a..4062f6b26acf 100644
--- a/drivers/gpu/drm/i915/gvt/gvt.h
+++ b/drivers/gpu/drm/i915/gvt/gvt.h
@@ -687,6 +687,12 @@  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_get_vfio_device_version(struct drm_i915_private *i915,
+		char *buf, const char *mdev_type);
+ssize_t intel_gvt_check_vfio_device_version(struct drm_i915_private *dev_priv,
+		const char *self, const char *remote);
+ssize_t
+intel_gvt_get_vfio_device_version_len(struct drm_i915_private *dev_priv);
 
 #include "trace.h"
 #include "mpt.h"