All of lore.kernel.org
 help / color / mirror / Atom feed
* [Intel-gfx] [PATCH 1/2] drm/i915/gvt: Move mdev attribute groups into kvmgt module
@ 2021-04-26  9:41 Zhenyu Wang
  2021-04-26  9:41 ` [Intel-gfx] [PATCH 2/2] Revert "vfio/gvt: Make DRM_I915_GVT depend on VFIO_MDEV" Zhenyu Wang
                   ` (2 more replies)
  0 siblings, 3 replies; 9+ messages in thread
From: Zhenyu Wang @ 2021-04-26  9:41 UTC (permalink / raw)
  To: intel-gfx; +Cc: Arnd Bergmann, intel-gvt-dev, Jason Gunthorpe

As kvmgt module contains all handling for VFIO/mdev, leaving mdev attribute
groups in gvt module caused dependency issue. Although it was there for possible
other hypervisor usage, that turns out never to be true. So this moves all mdev
handling into kvmgt module completely to resolve dependency issue.

Cc: Arnd Bergmann <arnd@kernel.org>
Cc: Jason Gunthorpe <jgg@ziepe.ca>
Cc: Alex Williamson <alex.williamson@redhat.com>
Signed-off-by: Zhenyu Wang <zhenyuw@linux.intel.com>
---
 drivers/gpu/drm/i915/gvt/gvt.c       | 124 +------------------------
 drivers/gpu/drm/i915/gvt/gvt.h       |   3 -
 drivers/gpu/drm/i915/gvt/hypercall.h |   2 +-
 drivers/gpu/drm/i915/gvt/kvmgt.c     | 129 +++++++++++++++++++++++++--
 drivers/gpu/drm/i915/gvt/mpt.h       |   4 +-
 5 files changed, 126 insertions(+), 136 deletions(-)

diff --git a/drivers/gpu/drm/i915/gvt/gvt.c b/drivers/gpu/drm/i915/gvt/gvt.c
index e7c2babcee8b..cbac409f6c8a 100644
--- a/drivers/gpu/drm/i915/gvt/gvt.c
+++ b/drivers/gpu/drm/i915/gvt/gvt.c
@@ -46,118 +46,6 @@ static const char * const supported_hypervisors[] = {
 	[INTEL_GVT_HYPERVISOR_KVM] = "KVM",
 };
 
-static struct intel_vgpu_type *
-intel_gvt_find_vgpu_type(struct intel_gvt *gvt, unsigned int type_group_id)
-{
-	if (WARN_ON(type_group_id >= gvt->num_types))
-		return NULL;
-	return &gvt->types[type_group_id];
-}
-
-static ssize_t available_instances_show(struct mdev_type *mtype,
-					struct mdev_type_attribute *attr,
-					char *buf)
-{
-	struct intel_vgpu_type *type;
-	unsigned int num = 0;
-	void *gvt = kdev_to_i915(mtype_get_parent_dev(mtype))->gvt;
-
-	type = intel_gvt_find_vgpu_type(gvt, mtype_get_type_group_id(mtype));
-	if (!type)
-		num = 0;
-	else
-		num = type->avail_instance;
-
-	return sprintf(buf, "%u\n", num);
-}
-
-static ssize_t device_api_show(struct mdev_type *mtype,
-			       struct mdev_type_attribute *attr, char *buf)
-{
-	return sprintf(buf, "%s\n", VFIO_DEVICE_API_PCI_STRING);
-}
-
-static ssize_t description_show(struct mdev_type *mtype,
-				struct mdev_type_attribute *attr, char *buf)
-{
-	struct intel_vgpu_type *type;
-	void *gvt = kdev_to_i915(mtype_get_parent_dev(mtype))->gvt;
-
-	type = intel_gvt_find_vgpu_type(gvt, mtype_get_type_group_id(mtype));
-	if (!type)
-		return 0;
-
-	return sprintf(buf, "low_gm_size: %dMB\nhigh_gm_size: %dMB\n"
-		       "fence: %d\nresolution: %s\n"
-		       "weight: %d\n",
-		       BYTES_TO_MB(type->low_gm_size),
-		       BYTES_TO_MB(type->high_gm_size),
-		       type->fence, vgpu_edid_str(type->resolution),
-		       type->weight);
-}
-
-static MDEV_TYPE_ATTR_RO(available_instances);
-static MDEV_TYPE_ATTR_RO(device_api);
-static MDEV_TYPE_ATTR_RO(description);
-
-static struct attribute *gvt_type_attrs[] = {
-	&mdev_type_attr_available_instances.attr,
-	&mdev_type_attr_device_api.attr,
-	&mdev_type_attr_description.attr,
-	NULL,
-};
-
-static struct attribute_group *gvt_vgpu_type_groups[] = {
-	[0 ... NR_MAX_INTEL_VGPU_TYPES - 1] = NULL,
-};
-
-static bool intel_get_gvt_attrs(struct attribute_group ***intel_vgpu_type_groups)
-{
-	*intel_vgpu_type_groups = gvt_vgpu_type_groups;
-	return true;
-}
-
-static int intel_gvt_init_vgpu_type_groups(struct intel_gvt *gvt)
-{
-	int i, j;
-	struct intel_vgpu_type *type;
-	struct attribute_group *group;
-
-	for (i = 0; i < gvt->num_types; i++) {
-		type = &gvt->types[i];
-
-		group = kzalloc(sizeof(struct attribute_group), GFP_KERNEL);
-		if (WARN_ON(!group))
-			goto unwind;
-
-		group->name = type->name;
-		group->attrs = gvt_type_attrs;
-		gvt_vgpu_type_groups[i] = group;
-	}
-
-	return 0;
-
-unwind:
-	for (j = 0; j < i; j++) {
-		group = gvt_vgpu_type_groups[j];
-		kfree(group);
-	}
-
-	return -ENOMEM;
-}
-
-static void intel_gvt_cleanup_vgpu_type_groups(struct intel_gvt *gvt)
-{
-	int i;
-	struct attribute_group *group;
-
-	for (i = 0; i < gvt->num_types; i++) {
-		group = gvt_vgpu_type_groups[i];
-		gvt_vgpu_type_groups[i] = NULL;
-		kfree(group);
-	}
-}
-
 static const struct intel_gvt_ops intel_gvt_ops = {
 	.emulate_cfg_read = intel_vgpu_emulate_cfg_read,
 	.emulate_cfg_write = intel_vgpu_emulate_cfg_write,
@@ -169,8 +57,6 @@ static const struct intel_gvt_ops intel_gvt_ops = {
 	.vgpu_reset = intel_gvt_reset_vgpu,
 	.vgpu_activate = intel_gvt_activate_vgpu,
 	.vgpu_deactivate = intel_gvt_deactivate_vgpu,
-	.gvt_find_vgpu_type = intel_gvt_find_vgpu_type,
-	.get_gvt_attrs = intel_get_gvt_attrs,
 	.vgpu_query_plane = intel_vgpu_query_plane,
 	.vgpu_get_dmabuf = intel_vgpu_get_dmabuf,
 	.write_protect_handler = intel_vgpu_page_track_handler,
@@ -274,7 +160,6 @@ void intel_gvt_clean_device(struct drm_i915_private *i915)
 		return;
 
 	intel_gvt_destroy_idle_vgpu(gvt->idle_vgpu);
-	intel_gvt_cleanup_vgpu_type_groups(gvt);
 	intel_gvt_clean_vgpu_types(gvt);
 
 	intel_gvt_debugfs_clean(gvt);
@@ -363,12 +248,6 @@ int intel_gvt_init_device(struct drm_i915_private *i915)
 	if (ret)
 		goto out_clean_thread;
 
-	ret = intel_gvt_init_vgpu_type_groups(gvt);
-	if (ret) {
-		gvt_err("failed to init vgpu type groups: %d\n", ret);
-		goto out_clean_types;
-	}
-
 	vgpu = intel_gvt_create_idle_vgpu(gvt);
 	if (IS_ERR(vgpu)) {
 		ret = PTR_ERR(vgpu);
@@ -454,7 +333,8 @@ EXPORT_SYMBOL_GPL(intel_gvt_register_hypervisor);
 void
 intel_gvt_unregister_hypervisor(void)
 {
-	intel_gvt_hypervisor_host_exit(intel_gvt_host.dev);
+	void *gvt = (void *)kdev_to_i915(intel_gvt_host.dev)->gvt;
+	intel_gvt_hypervisor_host_exit(intel_gvt_host.dev, gvt);
 	module_put(THIS_MODULE);
 }
 EXPORT_SYMBOL_GPL(intel_gvt_unregister_hypervisor);
diff --git a/drivers/gpu/drm/i915/gvt/gvt.h b/drivers/gpu/drm/i915/gvt/gvt.h
index 88ab360fcb31..0c0615602343 100644
--- a/drivers/gpu/drm/i915/gvt/gvt.h
+++ b/drivers/gpu/drm/i915/gvt/gvt.h
@@ -574,9 +574,6 @@ struct intel_gvt_ops {
 	void (*vgpu_reset)(struct intel_vgpu *);
 	void (*vgpu_activate)(struct intel_vgpu *);
 	void (*vgpu_deactivate)(struct intel_vgpu *);
-	struct intel_vgpu_type *(*gvt_find_vgpu_type)(
-		struct intel_gvt *gvt, unsigned int type_group_id);
-	bool (*get_gvt_attrs)(struct attribute_group ***intel_vgpu_type_groups);
 	int (*vgpu_query_plane)(struct intel_vgpu *vgpu, void *);
 	int (*vgpu_get_dmabuf)(struct intel_vgpu *vgpu, unsigned int);
 	int (*write_protect_handler)(struct intel_vgpu *, u64, void *,
diff --git a/drivers/gpu/drm/i915/gvt/hypercall.h b/drivers/gpu/drm/i915/gvt/hypercall.h
index b79da5124f83..f33e3cbd0439 100644
--- a/drivers/gpu/drm/i915/gvt/hypercall.h
+++ b/drivers/gpu/drm/i915/gvt/hypercall.h
@@ -49,7 +49,7 @@ enum hypervisor_type {
 struct intel_gvt_mpt {
 	enum hypervisor_type type;
 	int (*host_init)(struct device *dev, void *gvt, const void *ops);
-	void (*host_exit)(struct device *dev);
+	void (*host_exit)(struct device *dev, void *gvt);
 	int (*attach_vgpu)(void *vgpu, unsigned long *handle);
 	void (*detach_vgpu)(void *vgpu);
 	int (*inject_msi)(unsigned long handle, u32 addr, u16 data);
diff --git a/drivers/gpu/drm/i915/gvt/kvmgt.c b/drivers/gpu/drm/i915/gvt/kvmgt.c
index 65ff43cfc0f7..fe0ddd687d44 100644
--- a/drivers/gpu/drm/i915/gvt/kvmgt.c
+++ b/drivers/gpu/drm/i915/gvt/kvmgt.c
@@ -144,6 +144,112 @@ static inline bool handle_valid(unsigned long handle)
 	return !!(handle & ~0xff);
 }
 
+static struct intel_vgpu_type *
+intel_gvt_find_vgpu_type(struct intel_gvt *gvt, unsigned int type_group_id)
+{
+	if (WARN_ON(type_group_id >= gvt->num_types))
+		return NULL;
+	return &gvt->types[type_group_id];
+}
+
+static ssize_t available_instances_show(struct mdev_type *mtype,
+					struct mdev_type_attribute *attr,
+					char *buf)
+{
+	struct intel_vgpu_type *type;
+	unsigned int num = 0;
+	void *gvt = kdev_to_i915(mtype_get_parent_dev(mtype))->gvt;
+
+	type = intel_gvt_find_vgpu_type(gvt, mtype_get_type_group_id(mtype));
+	if (!type)
+		num = 0;
+	else
+		num = type->avail_instance;
+
+	return sprintf(buf, "%u\n", num);
+}
+
+static ssize_t device_api_show(struct mdev_type *mtype,
+			       struct mdev_type_attribute *attr, char *buf)
+{
+	return sprintf(buf, "%s\n", VFIO_DEVICE_API_PCI_STRING);
+}
+
+static ssize_t description_show(struct mdev_type *mtype,
+				struct mdev_type_attribute *attr, char *buf)
+{
+	struct intel_vgpu_type *type;
+	void *gvt = kdev_to_i915(mtype_get_parent_dev(mtype))->gvt;
+
+	type = intel_gvt_find_vgpu_type(gvt, mtype_get_type_group_id(mtype));
+	if (!type)
+		return 0;
+
+	return sprintf(buf, "low_gm_size: %dMB\nhigh_gm_size: %dMB\n"
+		       "fence: %d\nresolution: %s\n"
+		       "weight: %d\n",
+		       BYTES_TO_MB(type->low_gm_size),
+		       BYTES_TO_MB(type->high_gm_size),
+		       type->fence, vgpu_edid_str(type->resolution),
+		       type->weight);
+}
+
+static MDEV_TYPE_ATTR_RO(available_instances);
+static MDEV_TYPE_ATTR_RO(device_api);
+static MDEV_TYPE_ATTR_RO(description);
+
+static struct attribute *gvt_type_attrs[] = {
+	&mdev_type_attr_available_instances.attr,
+	&mdev_type_attr_device_api.attr,
+	&mdev_type_attr_description.attr,
+	NULL,
+};
+
+static struct attribute_group *gvt_vgpu_type_groups[] = {
+	[0 ... NR_MAX_INTEL_VGPU_TYPES - 1] = NULL,
+};
+
+static int intel_gvt_init_vgpu_type_groups(struct intel_gvt *gvt)
+{
+	int i, j;
+	struct intel_vgpu_type *type;
+	struct attribute_group *group;
+
+	for (i = 0; i < gvt->num_types; i++) {
+		type = &gvt->types[i];
+
+		group = kzalloc(sizeof(struct attribute_group), GFP_KERNEL);
+		if (WARN_ON(!group))
+			goto unwind;
+
+		group->name = type->name;
+		group->attrs = gvt_type_attrs;
+		gvt_vgpu_type_groups[i] = group;
+	}
+
+	return 0;
+
+unwind:
+	for (j = 0; j < i; j++) {
+		group = gvt_vgpu_type_groups[j];
+		kfree(group);
+	}
+
+	return -ENOMEM;
+}
+
+static void intel_gvt_cleanup_vgpu_type_groups(struct intel_gvt *gvt)
+{
+	int i;
+	struct attribute_group *group;
+
+	for (i = 0; i < gvt->num_types; i++) {
+		group = gvt_vgpu_type_groups[i];
+		gvt_vgpu_type_groups[i] = NULL;
+		kfree(group);
+	}
+}
+
 static int kvmgt_guest_init(struct mdev_device *mdev);
 static void intel_vgpu_release_work(struct work_struct *work);
 static bool kvmgt_guest_exit(struct kvmgt_guest_info *info);
@@ -700,8 +806,8 @@ static int intel_vgpu_create(struct mdev_device *mdev)
 	pdev = mdev_parent_dev(mdev);
 	gvt = kdev_to_i915(pdev)->gvt;
 
-	type = intel_gvt_ops->gvt_find_vgpu_type(gvt,
-						 mdev_get_type_group_id(mdev));
+	type = intel_gvt_find_vgpu_type(gvt,
+					mdev_get_type_group_id(mdev));
 	if (!type) {
 		ret = -EINVAL;
 		goto out;
@@ -1667,19 +1773,26 @@ static struct mdev_parent_ops intel_vgpu_ops = {
 
 static int kvmgt_host_init(struct device *dev, void *gvt, const void *ops)
 {
-	struct attribute_group **kvm_vgpu_type_groups;
+	int ret;
+
+	ret = intel_gvt_init_vgpu_type_groups((struct intel_gvt *)gvt);
+	if (ret)
+		return ret;
 
 	intel_gvt_ops = ops;
-	if (!intel_gvt_ops->get_gvt_attrs(&kvm_vgpu_type_groups))
-		return -EFAULT;
-	intel_vgpu_ops.supported_type_groups = kvm_vgpu_type_groups;
+	intel_vgpu_ops.supported_type_groups = gvt_vgpu_type_groups;
 
-	return mdev_register_device(dev, &intel_vgpu_ops);
+	ret = mdev_register_device(dev, &intel_vgpu_ops);
+	if (ret)
+		intel_gvt_cleanup_vgpu_type_groups((struct intel_gvt *)gvt);
+
+	return ret;
 }
 
-static void kvmgt_host_exit(struct device *dev)
+static void kvmgt_host_exit(struct device *dev, void *gvt)
 {
 	mdev_unregister_device(dev);
+	intel_gvt_cleanup_vgpu_type_groups((struct intel_gvt *)gvt);
 }
 
 static int kvmgt_page_track_add(unsigned long handle, u64 gfn)
diff --git a/drivers/gpu/drm/i915/gvt/mpt.h b/drivers/gpu/drm/i915/gvt/mpt.h
index 550a456e936f..e6c5a792a49a 100644
--- a/drivers/gpu/drm/i915/gvt/mpt.h
+++ b/drivers/gpu/drm/i915/gvt/mpt.h
@@ -63,13 +63,13 @@ static inline int intel_gvt_hypervisor_host_init(struct device *dev,
 /**
  * intel_gvt_hypervisor_host_exit - exit GVT-g host side
  */
-static inline void intel_gvt_hypervisor_host_exit(struct device *dev)
+static inline void intel_gvt_hypervisor_host_exit(struct device *dev, void *gvt)
 {
 	/* optional to provide */
 	if (!intel_gvt_host.mpt->host_exit)
 		return;
 
-	intel_gvt_host.mpt->host_exit(dev);
+	intel_gvt_host.mpt->host_exit(dev, gvt);
 }
 
 /**
-- 
2.31.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [Intel-gfx] [PATCH 2/2] Revert "vfio/gvt: Make DRM_I915_GVT depend on VFIO_MDEV"
  2021-04-26  9:41 [Intel-gfx] [PATCH 1/2] drm/i915/gvt: Move mdev attribute groups into kvmgt module Zhenyu Wang
@ 2021-04-26  9:41 ` Zhenyu Wang
  2021-04-26 16:55   ` Alex Williamson
  2021-04-26 12:09 ` [Intel-gfx] ✗ Fi.CI.BUILD: failure for series starting with [1/2] drm/i915/gvt: Move mdev attribute groups into kvmgt module Patchwork
       [not found] ` <20210426133924.GK2047089@ziepe.ca>
  2 siblings, 1 reply; 9+ messages in thread
From: Zhenyu Wang @ 2021-04-26  9:41 UTC (permalink / raw)
  To: intel-gfx; +Cc: Arnd Bergmann, intel-gvt-dev, Jason Gunthorpe

This reverts commit 07e543f4f9d116d6b4240644191dee6388ef4a85.

With all mdev handing moved to kvmgt module, only kvmgt should depend
on VFIO_MDEV. So revert this one.

Cc: Arnd Bergmann <arnd@kernel.org>
Cc: Jason Gunthorpe <jgg@ziepe.ca>
Cc: Alex Williamson <alex.williamson@redhat.com>
Signed-off-by: Zhenyu Wang <zhenyuw@linux.intel.com>
---
 drivers/gpu/drm/i915/Kconfig | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/Kconfig b/drivers/gpu/drm/i915/Kconfig
index 8f15bfb5faac..93f4d059fc89 100644
--- a/drivers/gpu/drm/i915/Kconfig
+++ b/drivers/gpu/drm/i915/Kconfig
@@ -102,7 +102,6 @@ config DRM_I915_GVT
 	bool "Enable Intel GVT-g graphics virtualization host support"
 	depends on DRM_I915
 	depends on 64BIT
-	depends on VFIO_MDEV
 	default n
 	help
 	  Choose this option if you want to enable Intel GVT-g graphics
-- 
2.31.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [Intel-gfx] ✗ Fi.CI.BUILD: failure for series starting with [1/2] drm/i915/gvt: Move mdev attribute groups into kvmgt module
  2021-04-26  9:41 [Intel-gfx] [PATCH 1/2] drm/i915/gvt: Move mdev attribute groups into kvmgt module Zhenyu Wang
  2021-04-26  9:41 ` [Intel-gfx] [PATCH 2/2] Revert "vfio/gvt: Make DRM_I915_GVT depend on VFIO_MDEV" Zhenyu Wang
@ 2021-04-26 12:09 ` Patchwork
       [not found] ` <20210426133924.GK2047089@ziepe.ca>
  2 siblings, 0 replies; 9+ messages in thread
From: Patchwork @ 2021-04-26 12:09 UTC (permalink / raw)
  To: Zhenyu Wang; +Cc: intel-gfx

== Series Details ==

Series: series starting with [1/2] drm/i915/gvt: Move mdev attribute groups into kvmgt module
URL   : https://patchwork.freedesktop.org/series/89478/
State : failure

== Summary ==

Applying: drm/i915/gvt: Move mdev attribute groups into kvmgt module
error: sha1 information is lacking or useless (drivers/gpu/drm/i915/gvt/gvt.c).
error: could not build fake ancestor
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0001 drm/i915/gvt: Move mdev attribute groups into kvmgt module
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".


_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [Intel-gfx] [PATCH 2/2] Revert "vfio/gvt: Make DRM_I915_GVT depend on VFIO_MDEV"
  2021-04-26  9:41 ` [Intel-gfx] [PATCH 2/2] Revert "vfio/gvt: Make DRM_I915_GVT depend on VFIO_MDEV" Zhenyu Wang
@ 2021-04-26 16:55   ` Alex Williamson
       [not found]     ` <20210426174017.GL2047089@ziepe.ca>
  0 siblings, 1 reply; 9+ messages in thread
From: Alex Williamson @ 2021-04-26 16:55 UTC (permalink / raw)
  To: Zhenyu Wang; +Cc: Arnd Bergmann, intel-gfx, intel-gvt-dev, Jason Gunthorpe

On Mon, 26 Apr 2021 17:41:43 +0800
Zhenyu Wang <zhenyuw@linux.intel.com> wrote:

> This reverts commit 07e543f4f9d116d6b4240644191dee6388ef4a85.

07e543f4f9d1 ("vfio/gvt: Make DRM_I915_GVT depend on VFIO_MDEV")

> With all mdev handing moved to kvmgt module, only kvmgt should depend
> on VFIO_MDEV. So revert this one.
> 
> Cc: Arnd Bergmann <arnd@kernel.org>
> Cc: Jason Gunthorpe <jgg@ziepe.ca>
> Cc: Alex Williamson <alex.williamson@redhat.com>
> Signed-off-by: Zhenyu Wang <zhenyuw@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/Kconfig | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/Kconfig b/drivers/gpu/drm/i915/Kconfig
> index 8f15bfb5faac..93f4d059fc89 100644
> --- a/drivers/gpu/drm/i915/Kconfig
> +++ b/drivers/gpu/drm/i915/Kconfig
> @@ -102,7 +102,6 @@ config DRM_I915_GVT
>  	bool "Enable Intel GVT-g graphics virtualization host support"
>  	depends on DRM_I915
>  	depends on 64BIT
> -	depends on VFIO_MDEV
>  	default n
>  	help
>  	  Choose this option if you want to enable Intel GVT-g graphics

I take it that this retracts your ack from
https://lore.kernel.org/dri-devel/20210425032356.GH1551@zhen-hp.sh.intel.com/
I'll drop it from consideration for pushing through my tree unless
indicated otherwise.  Thanks,

Alex

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [Intel-gfx] [PATCH 1/2] drm/i915/gvt: Move mdev attribute groups into kvmgt module
       [not found] ` <20210426133924.GK2047089@ziepe.ca>
@ 2021-04-27  2:45   ` Zhenyu Wang
       [not found]     ` <20210427121235.GM2047089@ziepe.ca>
  0 siblings, 1 reply; 9+ messages in thread
From: Zhenyu Wang @ 2021-04-27  2:45 UTC (permalink / raw)
  To: Jason Gunthorpe; +Cc: Arnd Bergmann, intel-gfx, intel-gvt-dev


[-- Attachment #1.1: Type: text/plain, Size: 1695 bytes --]

On 2021.04.26 10:39:24 -0300, Jason Gunthorpe wrote:
> On Mon, Apr 26, 2021 at 05:41:42PM +0800, Zhenyu Wang wrote:
> > @@ -1667,19 +1773,26 @@ static struct mdev_parent_ops intel_vgpu_ops = {
> >  
> >  static int kvmgt_host_init(struct device *dev, void *gvt, const void *ops)
> >  {
> > -	struct attribute_group **kvm_vgpu_type_groups;
> > +	int ret;
> > +
> > +	ret = intel_gvt_init_vgpu_type_groups((struct intel_gvt *)gvt);
> > +	if (ret)
> > +		return ret;
> >  
> >  	intel_gvt_ops = ops;
> > -	if (!intel_gvt_ops->get_gvt_attrs(&kvm_vgpu_type_groups))
> > -		return -EFAULT;
> > -	intel_vgpu_ops.supported_type_groups = kvm_vgpu_type_groups;
> > +	intel_vgpu_ops.supported_type_groups = gvt_vgpu_type_groups;
> 
> This patch is an improvement, but this fictional dynamic behavior is
> still wrong. The supported_type_groups directly flows from the
> vgpu_types array in vgpu.c and it should not be split up like this
> 
> The code copies the rodata vgpu_types into dynamic memory gvt->types
> then copies that dynamic memory into a dynamic gvt_vgpu_type_groups,
> which makes very little sense at all.

vgpu_types is static for we want fixed vgpu mdev type, but actual supported
types depend on HW resources e.g aperture mem, fence, etc, that's dynamic for
gvt->types, so gvt_vgpu_type_groups is dynamic from gvt->types.

> 
> vgpu_types should be moved to kvmgt and everything should be static,
> like every other mdev driver. Copy the 'type' information from the
> gpu_types static when the mdev is created.
> 

I don't think that's necessary, as it's not static for gvt at all.
The logic to handle type resource change can still be in vgpu.c


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

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [Intel-gfx] [PATCH 2/2] Revert "vfio/gvt: Make DRM_I915_GVT depend on VFIO_MDEV"
       [not found]     ` <20210426174017.GL2047089@ziepe.ca>
@ 2021-04-27  5:31       ` Zhenyu Wang
  2021-04-27 19:39         ` Alex Williamson
  0 siblings, 1 reply; 9+ messages in thread
From: Zhenyu Wang @ 2021-04-27  5:31 UTC (permalink / raw)
  To: Jason Gunthorpe; +Cc: Arnd Bergmann, intel-gvt-dev, intel-gfx


[-- Attachment #1.1: Type: text/plain, Size: 1825 bytes --]

On 2021.04.26 14:40:17 -0300, Jason Gunthorpe wrote:
> On Mon, Apr 26, 2021 at 10:55:55AM -0600, Alex Williamson wrote:
> > On Mon, 26 Apr 2021 17:41:43 +0800
> > Zhenyu Wang <zhenyuw@linux.intel.com> wrote:
> > 
> > > This reverts commit 07e543f4f9d116d6b4240644191dee6388ef4a85.
> > 
> > 07e543f4f9d1 ("vfio/gvt: Make DRM_I915_GVT depend on VFIO_MDEV")
> > 
> > > With all mdev handing moved to kvmgt module, only kvmgt should depend
> > > on VFIO_MDEV. So revert this one.
> > > 
> > > Cc: Arnd Bergmann <arnd@kernel.org>
> > > Cc: Jason Gunthorpe <jgg@ziepe.ca>
> > > Cc: Alex Williamson <alex.williamson@redhat.com>
> > > Signed-off-by: Zhenyu Wang <zhenyuw@linux.intel.com>
> > >  drivers/gpu/drm/i915/Kconfig | 1 -
> > >  1 file changed, 1 deletion(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/Kconfig b/drivers/gpu/drm/i915/Kconfig
> > > index 8f15bfb5faac..93f4d059fc89 100644
> > > +++ b/drivers/gpu/drm/i915/Kconfig
> > > @@ -102,7 +102,6 @@ config DRM_I915_GVT
> > >  	bool "Enable Intel GVT-g graphics virtualization host support"
> > >  	depends on DRM_I915
> > >  	depends on 64BIT
> > > -	depends on VFIO_MDEV
> > >  	default n
> > >  	help
> > >  	  Choose this option if you want to enable Intel GVT-g graphics
> > 
> > I take it that this retracts your ack from
> > https://lore.kernel.org/dri-devel/20210425032356.GH1551@zhen-hp.sh.intel.com/
> > I'll drop it from consideration for pushing through my tree unless
> > indicated otherwise.  Thanks,
> 
> In any event you'll need either Arnd's patch or this patch in your
> tree to avoid randconfig problems.
> 
> At this point I would take Arnd's and leave this to go next merge
> window.
> 

I'm ok with that, so won't block your vfio pull for merge window.
I'll send gvt fixes pull in next RC.

Thanks

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

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [Intel-gfx] [PATCH 2/2] Revert "vfio/gvt: Make DRM_I915_GVT depend on VFIO_MDEV"
  2021-04-27  5:31       ` Zhenyu Wang
@ 2021-04-27 19:39         ` Alex Williamson
  0 siblings, 0 replies; 9+ messages in thread
From: Alex Williamson @ 2021-04-27 19:39 UTC (permalink / raw)
  To: Zhenyu Wang; +Cc: Jason Gunthorpe, intel-gfx, intel-gvt-dev, Arnd Bergmann

On Tue, 27 Apr 2021 13:31:39 +0800
Zhenyu Wang <zhenyuw@linux.intel.com> wrote:

> On 2021.04.26 14:40:17 -0300, Jason Gunthorpe wrote:
> > On Mon, Apr 26, 2021 at 10:55:55AM -0600, Alex Williamson wrote:  
> > > On Mon, 26 Apr 2021 17:41:43 +0800
> > > Zhenyu Wang <zhenyuw@linux.intel.com> wrote:
> > >   
> > > > This reverts commit 07e543f4f9d116d6b4240644191dee6388ef4a85.  
> > > 
> > > 07e543f4f9d1 ("vfio/gvt: Make DRM_I915_GVT depend on VFIO_MDEV")
> > >   
> > > > With all mdev handing moved to kvmgt module, only kvmgt should depend
> > > > on VFIO_MDEV. So revert this one.
> > > > 
> > > > Cc: Arnd Bergmann <arnd@kernel.org>
> > > > Cc: Jason Gunthorpe <jgg@ziepe.ca>
> > > > Cc: Alex Williamson <alex.williamson@redhat.com>
> > > > Signed-off-by: Zhenyu Wang <zhenyuw@linux.intel.com>
> > > >  drivers/gpu/drm/i915/Kconfig | 1 -
> > > >  1 file changed, 1 deletion(-)
> > > > 
> > > > diff --git a/drivers/gpu/drm/i915/Kconfig b/drivers/gpu/drm/i915/Kconfig
> > > > index 8f15bfb5faac..93f4d059fc89 100644
> > > > +++ b/drivers/gpu/drm/i915/Kconfig
> > > > @@ -102,7 +102,6 @@ config DRM_I915_GVT
> > > >  	bool "Enable Intel GVT-g graphics virtualization host support"
> > > >  	depends on DRM_I915
> > > >  	depends on 64BIT
> > > > -	depends on VFIO_MDEV
> > > >  	default n
> > > >  	help
> > > >  	  Choose this option if you want to enable Intel GVT-g graphics  
> > > 
> > > I take it that this retracts your ack from
> > > https://lore.kernel.org/dri-devel/20210425032356.GH1551@zhen-hp.sh.intel.com/
> > > I'll drop it from consideration for pushing through my tree unless
> > > indicated otherwise.  Thanks,  
> > 
> > In any event you'll need either Arnd's patch or this patch in your
> > tree to avoid randconfig problems.
> > 
> > At this point I would take Arnd's and leave this to go next merge
> > window.
> >   
> 
> I'm ok with that, so won't block your vfio pull for merge window.
> I'll send gvt fixes pull in next RC.

Ok, I've pushed Arnd's fix to my next branch, I'll get it in for rc1.
Thanks,

Alex

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [Intel-gfx] [PATCH 1/2] drm/i915/gvt: Move mdev attribute groups into kvmgt module
       [not found]     ` <20210427121235.GM2047089@ziepe.ca>
@ 2021-04-28  5:32       ` Zhenyu Wang
       [not found]         ` <20210428172141.GW2047089@ziepe.ca>
  0 siblings, 1 reply; 9+ messages in thread
From: Zhenyu Wang @ 2021-04-28  5:32 UTC (permalink / raw)
  To: Jason Gunthorpe; +Cc: Arnd Bergmann, intel-gfx, intel-gvt-dev


[-- Attachment #1.1: Type: text/plain, Size: 1936 bytes --]

On 2021.04.27 09:12:35 -0300, Jason Gunthorpe wrote:
> On Tue, Apr 27, 2021 at 10:45:06AM +0800, Zhenyu Wang wrote:
> > On 2021.04.26 10:39:24 -0300, Jason Gunthorpe wrote:
> > > On Mon, Apr 26, 2021 at 05:41:42PM +0800, Zhenyu Wang wrote:
> > > > @@ -1667,19 +1773,26 @@ static struct mdev_parent_ops intel_vgpu_ops = {
> > > >  
> > > >  static int kvmgt_host_init(struct device *dev, void *gvt, const void *ops)
> > > >  {
> > > > -	struct attribute_group **kvm_vgpu_type_groups;
> > > > +	int ret;
> > > > +
> > > > +	ret = intel_gvt_init_vgpu_type_groups((struct intel_gvt *)gvt);
> > > > +	if (ret)
> > > > +		return ret;
> > > >  
> > > >  	intel_gvt_ops = ops;
> > > > -	if (!intel_gvt_ops->get_gvt_attrs(&kvm_vgpu_type_groups))
> > > > -		return -EFAULT;
> > > > -	intel_vgpu_ops.supported_type_groups = kvm_vgpu_type_groups;
> > > > +	intel_vgpu_ops.supported_type_groups = gvt_vgpu_type_groups;
> > > 
> > > This patch is an improvement, but this fictional dynamic behavior is
> > > still wrong. The supported_type_groups directly flows from the
> > > vgpu_types array in vgpu.c and it should not be split up like this
> > > 
> > > The code copies the rodata vgpu_types into dynamic memory gvt->types
> > > then copies that dynamic memory into a dynamic gvt_vgpu_type_groups,
> > > which makes very little sense at all.
> > 
> > vgpu_types is static for we want fixed vgpu mdev type, but actual supported
> > types depend on HW resources e.g aperture mem, fence, etc, that's dynamic for
> > gvt->types, so gvt_vgpu_type_groups is dynamic from gvt->types.
> 
> Well, that's worse then, intel_vgpu_ops.supported_type_groups is a
> static global, it cannot depend on HW variable calculations.
> 

sorry, maybe I didn't describe this properly, gvt_vgpu_type_groups is
static global, but the actual supported types is determined from
gvt->types and initialized before mdev device register.

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

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [Intel-gfx] [PATCH 1/2] drm/i915/gvt: Move mdev attribute groups into kvmgt module
       [not found]         ` <20210428172141.GW2047089@ziepe.ca>
@ 2021-04-29  2:40           ` Zhenyu Wang
  0 siblings, 0 replies; 9+ messages in thread
From: Zhenyu Wang @ 2021-04-29  2:40 UTC (permalink / raw)
  To: Jason Gunthorpe; +Cc: Arnd Bergmann, intel-gfx, intel-gvt-dev


[-- Attachment #1.1: Type: text/plain, Size: 2624 bytes --]

On 2021.04.28 14:21:41 -0300, Jason Gunthorpe wrote:
> On Wed, Apr 28, 2021 at 01:32:43PM +0800, Zhenyu Wang wrote:
> > On 2021.04.27 09:12:35 -0300, Jason Gunthorpe wrote:
> > > On Tue, Apr 27, 2021 at 10:45:06AM +0800, Zhenyu Wang wrote:
> > > > On 2021.04.26 10:39:24 -0300, Jason Gunthorpe wrote:
> > > > > On Mon, Apr 26, 2021 at 05:41:42PM +0800, Zhenyu Wang wrote:
> > > > > > @@ -1667,19 +1773,26 @@ static struct mdev_parent_ops intel_vgpu_ops = {
> > > > > >  
> > > > > >  static int kvmgt_host_init(struct device *dev, void *gvt, const void *ops)
> > > > > >  {
> > > > > > -	struct attribute_group **kvm_vgpu_type_groups;
> > > > > > +	int ret;
> > > > > > +
> > > > > > +	ret = intel_gvt_init_vgpu_type_groups((struct intel_gvt *)gvt);
> > > > > > +	if (ret)
> > > > > > +		return ret;
> > > > > >  
> > > > > >  	intel_gvt_ops = ops;
> > > > > > -	if (!intel_gvt_ops->get_gvt_attrs(&kvm_vgpu_type_groups))
> > > > > > -		return -EFAULT;
> > > > > > -	intel_vgpu_ops.supported_type_groups = kvm_vgpu_type_groups;
> > > > > > +	intel_vgpu_ops.supported_type_groups = gvt_vgpu_type_groups;
> > > > > 
> > > > > This patch is an improvement, but this fictional dynamic behavior is
> > > > > still wrong. The supported_type_groups directly flows from the
> > > > > vgpu_types array in vgpu.c and it should not be split up like this
> > > > > 
> > > > > The code copies the rodata vgpu_types into dynamic memory gvt->types
> > > > > then copies that dynamic memory into a dynamic gvt_vgpu_type_groups,
> > > > > which makes very little sense at all.
> > > > 
> > > > vgpu_types is static for we want fixed vgpu mdev type, but actual supported
> > > > types depend on HW resources e.g aperture mem, fence, etc, that's dynamic for
> > > > gvt->types, so gvt_vgpu_type_groups is dynamic from gvt->types.
> > > 
> > > Well, that's worse then, intel_vgpu_ops.supported_type_groups is a
> > > static global, it cannot depend on HW variable calculations.
> > 
> > sorry, maybe I didn't describe this properly, gvt_vgpu_type_groups is
> > static global, but the actual supported types is determined from
> > gvt->types and initialized before mdev device register.
> 
> No, I got it, I saw the code too.
> 
> Think about what happens if there are two GPUs in the system, you get
> two gvt's and with different properties and you are trying to squeeze
> that into a single static global - it doesn't work. 
> 

Well, that's not what this change trys to resolve and on gvt supported platform
that's not an available hardware configuration for IGD. So I think this is fine.


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

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2021-04-29  2:58 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-26  9:41 [Intel-gfx] [PATCH 1/2] drm/i915/gvt: Move mdev attribute groups into kvmgt module Zhenyu Wang
2021-04-26  9:41 ` [Intel-gfx] [PATCH 2/2] Revert "vfio/gvt: Make DRM_I915_GVT depend on VFIO_MDEV" Zhenyu Wang
2021-04-26 16:55   ` Alex Williamson
     [not found]     ` <20210426174017.GL2047089@ziepe.ca>
2021-04-27  5:31       ` Zhenyu Wang
2021-04-27 19:39         ` Alex Williamson
2021-04-26 12:09 ` [Intel-gfx] ✗ Fi.CI.BUILD: failure for series starting with [1/2] drm/i915/gvt: Move mdev attribute groups into kvmgt module Patchwork
     [not found] ` <20210426133924.GK2047089@ziepe.ca>
2021-04-27  2:45   ` [Intel-gfx] [PATCH 1/2] " Zhenyu Wang
     [not found]     ` <20210427121235.GM2047089@ziepe.ca>
2021-04-28  5:32       ` Zhenyu Wang
     [not found]         ` <20210428172141.GW2047089@ziepe.ca>
2021-04-29  2:40           ` Zhenyu Wang

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.